diff options
author | John Cai <jcai@gitlab.com> | 2022-10-10 16:37:32 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-10-10 16:37:32 +0300 |
commit | 625ad956b9d046d984090c7137a535307bf54a87 (patch) | |
tree | 166976cf56f63b4d08baf88ebcc679eef686c0cb | |
parent | e49ea29543b2d8e71bfe4bdc3b295f785bd24fb1 (diff) | |
parent | c9c6e13d1d341f1dc3aa092e1a22aafa0cee14ba (diff) |
Merge branch 'pks-gofumpt-v0.4.0' into 'master'
Makefile: Update gofumpt to v0.4.0
See merge request https://gitlab.com/gitlab-org/gitaly/-/merge_requests/4907
Merged-by: John Cai <jcai@gitlab.com>
Approved-by: karthik nayak <knayak@gitlab.com>
Approved-by: John Cai <jcai@gitlab.com>
Co-authored-by: Patrick Steinhardt <psteinhardt@gitlab.com>
32 files changed, 162 insertions, 139 deletions
@@ -100,7 +100,7 @@ endif GOLANGCI_LINT_VERSION ?= v1.48.0 PROTOLINT_VERSION ?= v0.38.1 GOCOVER_COBERTURA_VERSION ?= aaee18c8195c3f2d90e5ef80ca918d265463842a -GOFUMPT_VERSION ?= v0.3.1 +GOFUMPT_VERSION ?= v0.4.0 GOIMPORTS_VERSION ?= v0.1.10 GOTESTSUM_VERSION ?= v1.8.1 GO_LICENSES_VERSION ?= v1.2.1 diff --git a/client/dial_test.go b/client/dial_test.go index e0870ee32..70858b6ef 100644 --- a/client/dial_test.go +++ b/client/dial_test.go @@ -566,6 +566,7 @@ func startUnixListener(tb testing.TB, factory func(credentials.TransportCredenti } // startTLSListener will start a secure TLS listener on a random unused port +// //go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out testdata/gitalycert.pem -keyout testdata/gitalykey.pem -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost" -addext "subjectAltName = IP:127.0.0.1, DNS:localhost" func startTLSListener(tb testing.TB, factory func(credentials.TransportCredentials) *grpc.Server) (func(), string) { listener, err := net.Listen("tcp", "localhost:0") diff --git a/cmd/praefect/main.go b/cmd/praefect/main.go index 76cb2071b..078db25df 100644 --- a/cmd/praefect/main.go +++ b/cmd/praefect/main.go @@ -3,18 +3,18 @@ // // Additionally, praefect has subcommands for common tasks: // -// SQL Ping +// # SQL Ping // // The subcommand "sql-ping" checks if the database configured in the config // file is reachable: // -// praefect -config PATH_TO_CONFIG sql-ping +// praefect -config PATH_TO_CONFIG sql-ping // -// SQL Migrate +// # SQL Migrate // // The subcommand "sql-migrate" will apply any outstanding SQL migrations. // -// praefect -config PATH_TO_CONFIG sql-migrate [-ignore-unknown=true|false] +// praefect -config PATH_TO_CONFIG sql-migrate [-ignore-unknown=true|false] // // By default, the migration will ignore any unknown migrations that are // not known by the Praefect binary. @@ -24,36 +24,36 @@ // The subcommand "sql-migrate-status" will show which SQL migrations have // been applied and which ones have not: // -// praefect -config PATH_TO_CONFIG sql-migrate-status +// praefect -config PATH_TO_CONFIG sql-migrate-status // -// Dial Nodes +// # Dial Nodes // // The subcommand "dial-nodes" helps diagnose connection problems to Gitaly or // Praefect. The subcommand works by sourcing the connection information from // the config file, and then dialing and health checking the remote nodes. // -// praefect -config PATH_TO_CONFIG dial-nodes +// praefect -config PATH_TO_CONFIG dial-nodes // -// Dataloss +// # Dataloss // // The subcommand "dataloss" identifies Gitaly nodes which are missing data from the // previous write-enabled primary node. It does so by looking through incomplete // replication jobs. This is useful for identifying potential data loss from a failover // event. // -// praefect -config PATH_TO_CONFIG dataloss [-virtual-storage <virtual-storage>] +// praefect -config PATH_TO_CONFIG dataloss [-virtual-storage <virtual-storage>] // // "-virtual-storage" specifies which virtual storage to check for data loss. If not specified, // the check is performed for every configured virtual storage. // -// Accept Dataloss +// # Accept Dataloss // // The subcommand "accept-dataloss" allows for accepting data loss in a repository to enable it for // writing again. The current version of the repository on the authoritative storage is set to be // the latest version and replications to other nodes are scheduled in order to bring them consistent // with the new authoritative version. // -// praefect -config PATH_TO_CONFIG accept-dataloss -virtual-storage <virtual-storage> -relative-path <relative-path> -authoritative-storage <authoritative-storage> +// praefect -config PATH_TO_CONFIG accept-dataloss -virtual-storage <virtual-storage> -relative-path <relative-path> -authoritative-storage <authoritative-storage> package main import ( diff --git a/internal/backchannel/backchannel.go b/internal/backchannel/backchannel.go index e32494f63..a7da22f15 100644 --- a/internal/backchannel/backchannel.go +++ b/internal/backchannel/backchannel.go @@ -17,15 +17,15 @@ // The server side uses listenmux to support clients that are unaware of the multiplexing. // // Usage: -// 1. Implement a ServerFactory, which is simply a function that returns a Server that can serve on the backchannel -// connection. Plug in the ClientHandshake to the Clientconn via grpc.WithTransportCredentials when dialing. -// This ensures all connections established by gRPC work with a multiplexing session and have a backchannel Server serving. -// 2. Create a *listenmux.Mux and register a *ServerHandshaker with it. -// 3. Pass the *listenmux.Mux into the grpc Server using grpc.Creds. -// The Handshake method is called on each newly established connection that presents the backchannel magic bytes. It dials back to the client's backchannel server. Server -// makes the backchannel connection's available later via the Registry's Backchannel method. The ID of the -// peer associated with the current RPC handler can be fetched via GetPeerID. The returned ID can be used -// to access the correct backchannel connection from the Registry. +// 1. Implement a ServerFactory, which is simply a function that returns a Server that can serve on the backchannel +// connection. Plug in the ClientHandshake to the Clientconn via grpc.WithTransportCredentials when dialing. +// This ensures all connections established by gRPC work with a multiplexing session and have a backchannel Server serving. +// 2. Create a *listenmux.Mux and register a *ServerHandshaker with it. +// 3. Pass the *listenmux.Mux into the grpc Server using grpc.Creds. +// The Handshake method is called on each newly established connection that presents the backchannel magic bytes. It dials back to the client's backchannel server. Server +// makes the backchannel connection's available later via the Registry's Backchannel method. The ID of the +// peer associated with the current RPC handler can be fetched via GetPeerID. The returned ID can be used +// to access the correct backchannel connection from the Registry. package backchannel import ( diff --git a/internal/backup/locator.go b/internal/backup/locator.go index 1cedd5913..ed9d6cc72 100644 --- a/internal/backup/locator.go +++ b/internal/backup/locator.go @@ -20,9 +20,10 @@ import ( // files. // // Structure: -// <repo relative path>.bundle -// <repo relative path>.refs -// <repo relative path>/custom_hooks.tar +// +// <repo relative path>.bundle +// <repo relative path>.refs +// <repo relative path>/custom_hooks.tar type LegacyLocator struct{} // BeginFull returns the static paths for a legacy repository backup @@ -65,11 +66,12 @@ func (l LegacyLocator) newFull(repo *gitalypb.Repository) *Step { // file named LATEST. // // Structure: -// <repo relative path>/LATEST -// <repo relative path>/<backup id>/LATEST -// <repo relative path>/<backup id>/<nnn>.bundle -// <repo relative path>/<backup id>/<nnn>.refs -// <repo relative path>/<backup id>/<nnn>.custom_hooks.tar +// +// <repo relative path>/LATEST +// <repo relative path>/<backup id>/LATEST +// <repo relative path>/<backup id>/<nnn>.bundle +// <repo relative path>/<backup id>/<nnn>.refs +// <repo relative path>/<backup id>/<nnn>.custom_hooks.tar type PointerLocator struct { Sink Sink Fallback Locator diff --git a/internal/bootstrap/bootstrap.go b/internal/bootstrap/bootstrap.go index 23b39ff66..a0251fee0 100644 --- a/internal/bootstrap/bootstrap.go +++ b/internal/bootstrap/bootstrap.go @@ -51,8 +51,9 @@ type upgrader interface { } // New performs tableflip initialization -// pidFile is optional, if provided it will always contain the current process PID -// upgradesEnabled controls the upgrade process on SIGHUP signal +// +// pidFile is optional, if provided it will always contain the current process PID +// upgradesEnabled controls the upgrade process on SIGHUP signal // // first boot: // * gitaly starts as usual, we will refer to it as p1 @@ -63,16 +64,17 @@ type upgrader interface { // * upg.Exit() channel will be closed when an upgrades completed successfully and the process must terminate // // graceful upgrade: -// * user replaces gitaly binary and/or config file -// * user sends SIGHUP to p1 -// * p1 will fork and exec the new gitaly, we will refer to it as p2 -// * from now on p1 will ignore other SIGHUP -// * if p2 terminates with a non-zero exit code, SIGHUP handling will be restored -// * p2 will follow the "first boot" sequence but upg.Fds will provide sockets and files from p1, when available -// * when p2 invokes upg.Ready() all the shared file descriptors not claimed by p2 will be closed -// * upg.Exit() channel in p1 will be closed now and p1 can gracefully terminate already accepted connections -// * upgrades cannot starts again if p1 and p2 are both running, an hard termination should be scheduled to overcome -// freezes during a graceful shutdown +// - user replaces gitaly binary and/or config file +// - user sends SIGHUP to p1 +// - p1 will fork and exec the new gitaly, we will refer to it as p2 +// - from now on p1 will ignore other SIGHUP +// - if p2 terminates with a non-zero exit code, SIGHUP handling will be restored +// - p2 will follow the "first boot" sequence but upg.Fds will provide sockets and files from p1, when available +// - when p2 invokes upg.Ready() all the shared file descriptors not claimed by p2 will be closed +// - upg.Exit() channel in p1 will be closed now and p1 can gracefully terminate already accepted connections +// - upgrades cannot starts again if p1 and p2 are both running, an hard termination should be scheduled to overcome +// freezes during a graceful shutdown +// // gitaly-wrapper is supposed to set EnvUpgradesEnabled in order to enable graceful upgrades func New(totalConn *prometheus.CounterVec) (*Bootstrap, error) { pidFile := os.Getenv(EnvPidFile) diff --git a/internal/git/gitpipe/ls_tree.go b/internal/git/gitpipe/ls_tree.go index c7567ff8e..b48489e4f 100644 --- a/internal/git/gitpipe/ls_tree.go +++ b/internal/git/gitpipe/ls_tree.go @@ -47,9 +47,9 @@ func LsTreeWithSkip(skipResult func(*RevisionResult) bool) LsTreeOption { // LsTree runs git-ls-tree(1) for the given revisions. The returned channel will // contain all object IDs listed by this command. This might include: -// - Blobs -// - Trees, unless you're calling it with LsTreeWithRecursive() -// - Submodules, referring to the commit of the submodule +// - Blobs +// - Trees, unless you're calling it with LsTreeWithRecursive() +// - Submodules, referring to the commit of the submodule func LsTree( ctx context.Context, repo *localrepo.Repo, diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go index 0086fcad0..6022f1e6b 100644 --- a/internal/git/gittest/repo.go +++ b/internal/git/gittest/repo.go @@ -335,7 +335,7 @@ func AddWorktree(tb testing.TB, cfg config.Cfg, repoPath string, worktreeName st // date causes commit-graphs to become corrupt with the following error that's likely caused by // an overflow: // -// commit date for commit ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69 in commit-graph is 15668040695 != 9223372036854775 +// commit date for commit ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69 in commit-graph is 15668040695 != 9223372036854775 // // This is not a new error, but something that has existed for quite a while already in Git. And // while the bug can also be easily hit in Gitaly because we do write commit-graphs in pool @@ -355,9 +355,9 @@ func AddWorktree(tb testing.TB, cfg config.Cfg, repoPath string, worktreeName st // // You can easily test whether this bug still exists via the following commands: // -// $ git clone _build/testrepos/gitlab-test.git -// $ git -C gitlab-test commit-graph write -// $ git -C gitlab-test commit-graph verify +// $ git clone _build/testrepos/gitlab-test.git +// $ git -C gitlab-test commit-graph write +// $ git -C gitlab-test commit-graph verify func FixGitLabTestRepoForCommitGraphs(tb testing.TB, cfg config.Cfg, repoPath string) { Exec(tb, cfg, "-C", repoPath, "update-ref", "-d", "refs/heads/spooky-stuff", "ba3343bc4fa403a8dfbfcab7fc1a8c29ee34bd69") } diff --git a/internal/git/stats/git.go b/internal/git/stats/git.go index 5ec5dace0..ce9794c72 100644 --- a/internal/git/stats/git.go +++ b/internal/git/stats/git.go @@ -40,20 +40,25 @@ func LogObjectsInfo(ctx context.Context, repo git.RepositoryExecutor) { } } -/* readObjectInfoStatistic parses output of 'git count-objects -v' command and represents it as dictionary +/* + readObjectInfoStatistic parses output of 'git count-objects -v' command and represents it as dictionary + current supported format is: - count: 12 - packs: 2 - size-garbage: 934 - alternate: /some/path/to/.git/objects - alternate: "/some/other path/to/.git/objects" + + count: 12 + packs: 2 + size-garbage: 934 + alternate: /some/path/to/.git/objects + alternate: "/some/other path/to/.git/objects" + will result in: - { - "count": 12, - "packs": 2, - "size-garbage": 934, - "alternate": ["/some/path/to/.git/objects", "/some/other path/to/.git/objects"] - } + + { + "count": 12, + "packs": 2, + "size-garbage": 934, + "alternate": ["/some/path/to/.git/objects", "/some/other path/to/.git/objects"] + } */ func readObjectInfoStatistic(reader io.Reader) (map[string]interface{}, error) { stats := map[string]interface{}{} diff --git a/internal/git/trailerparser/trailerparser.go b/internal/git/trailerparser/trailerparser.go index 64e97cf96..ab1d115be 100644 --- a/internal/git/trailerparser/trailerparser.go +++ b/internal/git/trailerparser/trailerparser.go @@ -28,7 +28,7 @@ const ( // The expected input is a single line containing trailers in the following // format: // -// KEY:VALUE\0KEY:VALUE +// KEY:VALUE\0KEY:VALUE // // Where \0 is a NULL byte. The input should not end in a NULL byte. // @@ -47,12 +47,12 @@ const ( // The limits this parser imposes on the sizes/amounts are loosely based on // trailers found in GitLab's own repository. Here are just a few examples: // -// Change-Id: I009c716ce2475b9efa3fd07aee9215fca7a1c150 -// Changelog: https://github.com/nahi/httpclient/blob/b51d7a8bb78f71726b08fbda5abfb900d627569f/CHANGELOG.md#changes-in-282 -// Co-Authored-By: Alex Kalderimis <akalderimis@gitlab.com> -// fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/44458 -// Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> -// See: https://gitlab.com/gitlab-org/gitlab-ee/blob/ff9ad690650c23665439499d23f3ed22103b55bb/spec/spec_helper.rb#L50 +// Change-Id: I009c716ce2475b9efa3fd07aee9215fca7a1c150 +// Changelog: https://github.com/nahi/httpclient/blob/b51d7a8bb78f71726b08fbda5abfb900d627569f/CHANGELOG.md#changes-in-282 +// Co-Authored-By: Alex Kalderimis <akalderimis@gitlab.com> +// fixes: https://gitlab.com/gitlab-org/gitlab-ce/issues/44458 +// Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> +// See: https://gitlab.com/gitlab-org/gitlab-ee/blob/ff9ad690650c23665439499d23f3ed22103b55bb/spec/spec_helper.rb#L50 func Parse(input []byte) []*gitalypb.CommitTrailer { // The choice of a nil slice instead of an empty one is deliberate: gRPC // turns empty slices into nil. diff --git a/internal/gitlab/http_client_test.go b/internal/gitlab/http_client_test.go index 4f6bf150a..1179fbb3d 100644 --- a/internal/gitlab/http_client_test.go +++ b/internal/gitlab/http_client_test.go @@ -29,6 +29,7 @@ type postReceiveRequest struct { // TestAllowedVerifyParams uses client cert fixtures to test TLS connections. To // regenerate these certs, run `go generate access_test.go`. +// //go:generate openssl req -newkey rsa:4096 -new -nodes -x509 -days 3650 -out testdata/certs/server.crt -keyout testdata/certs/server.key -subj "/C=US/ST=California/L=San Francisco/O=GitLab/OU=GitLab-Shell/CN=localhost" -addext "subjectAltName = IP:127.0.0.1" func TestAccess_verifyParams(t *testing.T) { user, password := "user", "password" diff --git a/internal/helper/security.go b/internal/helper/security.go index f074c5605..c46e1e938 100644 --- a/internal/helper/security.go +++ b/internal/helper/security.go @@ -6,7 +6,8 @@ import ( ) // Pattern taken from Regular Expressions Cookbook, slightly modified though -// |Scheme |User |Named/IPv4 host|IPv6+ host +// +// |Scheme |User |Named/IPv4 host|IPv6+ host var hostPattern = regexp.MustCompile(`(?i)([a-z][a-z0-9+\-.]*://)([a-z0-9\-._~%!$&'()*+,;=:]+@)([a-z0-9\-._~%]+|\[[a-z0-9\-._~%!$&'()*+,;=:]+\])`) // SanitizeString will clean password and tokens from URLs, and replace them diff --git a/internal/log/log_test.go b/internal/log/log_test.go index 7c72064fb..060dc475f 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -442,8 +442,9 @@ func TestStreamLogDataCatcherServerInterceptor(t *testing.T) { }) } -//nolint:forbidigo // We cannot use `testhelper.Context()` because of a cyclic dependency between // this package and the `testhelper` package. +// +//nolint:forbidigo // We cannot use `testhelper.Context()` because of a cyclic dependency between func createContext() context.Context { return context.Background() } diff --git a/internal/logsanitizer/url.go b/internal/logsanitizer/url.go index 8640570f5..5c59eb5c2 100644 --- a/internal/logsanitizer/url.go +++ b/internal/logsanitizer/url.go @@ -7,7 +7,8 @@ import ( ) // Pattern taken from Regular Expressions Cookbook, slightly modified though -// |Scheme |User |Named/IPv4 host|IPv6+ host +// +// |Scheme |User |Named/IPv4 host|IPv6+ host var hostPattern = regexp.MustCompile(`(?i)([a-z][a-z0-9+\-.]*://)?([a-z0-9\-._~%!$&'()*+,;=:]+@)([a-z0-9\-._~%]+|\[[a-z0-9\-._~%!$&'()*+,;=:]+\])`) // URLSanitizerHook stores which gRPC methods to perform sanitization for. diff --git a/internal/metadata/featureflag/context_test.go b/internal/metadata/featureflag/context_test.go index 934a3abfa..b92904a44 100644 --- a/internal/metadata/featureflag/context_test.go +++ b/internal/metadata/featureflag/context_test.go @@ -16,8 +16,9 @@ var ( ffB = FeatureFlag{"feature-b", false} ) -//nolint:forbidigo // We cannot use `testhelper.Context()` given that it would inject feature flags // already. +// +//nolint:forbidigo // We cannot use `testhelper.Context()` given that it would inject feature flags func createContext() context.Context { return context.Background() } diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go index ac1e844cf..e000c24cf 100644 --- a/internal/praefect/coordinator.go +++ b/internal/praefect/coordinator.go @@ -844,18 +844,18 @@ func (c *Coordinator) createTransactionFinalizer( // getUpdatedAndOutdatedSecondaries returns all nodes which can be considered up-to-date or outdated // after the given transaction. A node is considered outdated, if one of the following is true: // -// - No subtransactions were created and the RPC was successful on the primary. This really is only -// a safeguard in case the RPC wasn't aware of transactions and thus failed to correctly assert -// its state matches across nodes. This is rather pessimistic, as it could also indicate that an -// RPC simply didn't change anything. If the RPC was a failure on the primary and there were no -// subtransactions, we assume no changes were done and that the nodes failed prior to voting. +// - No subtransactions were created and the RPC was successful on the primary. This really is only +// a safeguard in case the RPC wasn't aware of transactions and thus failed to correctly assert +// its state matches across nodes. This is rather pessimistic, as it could also indicate that an +// RPC simply didn't change anything. If the RPC was a failure on the primary and there were no +// subtransactions, we assume no changes were done and that the nodes failed prior to voting. // -// - The node failed to be part of the quorum. As a special case, if the primary fails the vote, all -// nodes need to get replication jobs. +// - The node failed to be part of the quorum. As a special case, if the primary fails the vote, all +// nodes need to get replication jobs. // -// - The node has a different error state than the primary. If both primary and secondary have -// returned the same error, then we assume they did the same thing and failed in the same -// controlled way. +// - The node has a different error state than the primary. If both primary and secondary have +// returned the same error, then we assume they did the same thing and failed in the same +// controlled way. // // Note that this function cannot and should not fail: if anything goes wrong, we need to create // replication jobs to repair state. diff --git a/internal/praefect/datastore/queue.go b/internal/praefect/datastore/queue.go index df894fcdc..494875018 100644 --- a/internal/praefect/datastore/queue.go +++ b/internal/praefect/datastore/queue.go @@ -476,8 +476,10 @@ func (rq PostgresReplicationEventQueue) StartHealthUpdate(ctx context.Context, t // AcknowledgeStale moves replication events that are 'in_progress' state for too long (more then staleAfter) // into the next state: -// 'failed' - in case it has more attempts to be executed -// 'dead' - in case it has no more attempts to be executed +// +// 'failed' - in case it has more attempts to be executed +// 'dead' - in case it has no more attempts to be executed +// // The job considered 'in_progress' if it has corresponding entry in the 'replication_queue_job_lock' table. // When moving from 'in_progress' to other state the entry from 'replication_queue_job_lock' table will be // removed and entry in the 'replication_queue_lock' will be updated if needed (release of the lock). diff --git a/internal/praefect/grpc-proxy/proxy/handler.go b/internal/praefect/grpc-proxy/proxy/handler.go index 82c252526..b0ca206ca 100644 --- a/internal/praefect/grpc-proxy/proxy/handler.go +++ b/internal/praefect/grpc-proxy/proxy/handler.go @@ -24,10 +24,11 @@ var clientStreamDescForProxying = &grpc.StreamDesc{ // RegisterStreamHandlers sets up stream handlers for a set of gRPC methods for a given service. // streamers is a map of method to grpc.StreamHandler eg: // -// streamHandler := func(srv interface{}, stream ServerStream) error { -// /** do some stuff **/ -// return nil -// } +// streamHandler := func(srv interface{}, stream ServerStream) error { +// /** do some stuff **/ +// return nil +// } +// // RegisterStreamHandlers(grpcServer, "MyGrpcService", map[string]grpc.StreamHandler{"Method1": streamHandler}) // note: multiple calls with the same serviceName will result in a fatal func RegisterStreamHandlers(server *grpc.Server, serviceName string, streamers map[string]grpc.StreamHandler) { diff --git a/internal/praefect/nodes/health_manager.go b/internal/praefect/nodes/health_manager.go index ca758f459..21cdfc9fe 100644 --- a/internal/praefect/nodes/health_manager.go +++ b/internal/praefect/nodes/health_manager.go @@ -20,13 +20,13 @@ type HealthClients map[string]map[string]grpc_health_v1.HealthClient // HealthManager monitors the health status of the storage cluster. The monitoring frequency // is controlled by the Ticker passed in to Run method. On each tick, the HealthManager: // -// 1. Runs health checks on configured physical storages by performing a gRPC call -// to the health checking endpoint. If an error tracker is configured, it also considers -// its view of the node's health. -// 2. Stores its health check results in the `node_status` table. -// 3. Checks if the clusters consensus of healthy nodes has changed by querying the `node_status` -// table for results of the other Praefect instances. If so, it sends to the Updated channel -// to signal a change in the cluster status. +// 1. Runs health checks on configured physical storages by performing a gRPC call +// to the health checking endpoint. If an error tracker is configured, it also considers +// its view of the node's health. +// 2. Stores its health check results in the `node_status` table. +// 3. Checks if the clusters consensus of healthy nodes has changed by querying the `node_status` +// table for results of the other Praefect instances. If so, it sends to the Updated channel +// to signal a change in the cluster status. // // To determine the participants for the quorum, we use a lightweight service discovery protocol. // A Praefect instance is deemed to be voting member if it has a recent health check in the diff --git a/internal/praefect/nodes/ping.go b/internal/praefect/nodes/ping.go index e0bcd9b9b..8b1b67540 100644 --- a/internal/praefect/nodes/ping.go +++ b/internal/praefect/nodes/ping.go @@ -191,7 +191,8 @@ func (t *TextPrinter) Printf(format string, args ...interface{}) { } // CheckNode checks network connectivity by issuing a healthcheck request, and -// also calls the ServerInfo RPC to check disk read/write access. +// +// also calls the ServerInfo RPC to check disk read/write access. func (p *Ping) CheckNode(ctx context.Context) { p.log("dialing...") cc, err := p.dial(ctx) diff --git a/internal/praefect/nodes/sql_elector.go b/internal/praefect/nodes/sql_elector.go index 224e5b3e4..12d12eded 100644 --- a/internal/praefect/nodes/sql_elector.go +++ b/internal/praefect/nodes/sql_elector.go @@ -42,19 +42,20 @@ type sqlCandidate struct { // 1. For each node, Praefect updates a row in a new table // (`node_status`) with the following information: // -// a. The name of the Praefect instance (`praefect_name`) -// b. The name of the virtual storage name (`shard_name`) -// c. The name of the Gitaly storage name (`storage_name`) -// d. The timestamp of the last time Praefect tried to reach that node (`last_contact_attempt_at`) -// e. The timestamp of the last successful health check (`last_seen_active_at`) +// a. The name of the Praefect instance (`praefect_name`) +// b. The name of the virtual storage name (`shard_name`) +// c. The name of the Gitaly storage name (`storage_name`) +// d. The timestamp of the last time Praefect tried to reach that node (`last_contact_attempt_at`) +// e. The timestamp of the last successful health check (`last_seen_active_at`) // // 2. Once the health checks are complete, Praefect node does a `SELECT` from // `node_status` to determine healthy nodes. A healthy node is // defined by: -// a. A node that has a recent successful error check (e.g. one in -// the last 10 s). -// b. A majority of the available Praefect nodes have entries that -// match the two above. +// +// a. A node that has a recent successful error check (e.g. one in +// the last 10 s). +// b. A majority of the available Praefect nodes have entries that +// match the two above. // // To determine the majority, we use a lightweight service discovery // protocol: a Praefect node is deemed a voting member if the diff --git a/internal/praefect/reconciler/reconciler.go b/internal/praefect/reconciler/reconciler.go index 1af241668..680323d06 100644 --- a/internal/praefect/reconciler/reconciler.go +++ b/internal/praefect/reconciler/reconciler.go @@ -99,14 +99,14 @@ type job struct { // // It currently handles fixing two discrepancies: // -// 1. Assigned storage having an outdated replica of a repository. This is fixed by scheduling -// an `update` type job from any healthy storage with an up to date replica. These are only -// scheduled if there is no other active `update` type job targeting the outdated replica. -// 2. Unassigned storage having an unnecessary replica. This is fixed by scheduling a `delete_replica` -// type job to remove the unneeded replica from the storage. These are only scheduled if all assigned -// storages are up to date and the replica is not used as a source or target storage in any other job. -// Only one job of this type is allowed to be queued for a given repository at a time. This is to avoid -// deleting too many replicas if assignments are changed while the jobs are queued. +// 1. Assigned storage having an outdated replica of a repository. This is fixed by scheduling +// an `update` type job from any healthy storage with an up to date replica. These are only +// scheduled if there is no other active `update` type job targeting the outdated replica. +// 2. Unassigned storage having an unnecessary replica. This is fixed by scheduling a `delete_replica` +// type job to remove the unneeded replica from the storage. These are only scheduled if all assigned +// storages are up to date and the replica is not used as a source or target storage in any other job. +// Only one job of this type is allowed to be queued for a given repository at a time. This is to avoid +// deleting too many replicas if assignments are changed while the jobs are queued. // // The fixes are only scheduled if the target node is healthy, and if there is a healthy source node // available should the job need one. diff --git a/internal/praefect/server.go b/internal/praefect/server.go index 202eb2e6d..a31d4b071 100644 --- a/internal/praefect/server.go +++ b/internal/praefect/server.go @@ -1,5 +1,7 @@ -/*Package praefect is a Gitaly reverse proxy for transparently routing gRPC -calls to a set of Gitaly services.*/ +/* +Package praefect is a Gitaly reverse proxy for transparently routing gRPC +calls to a set of Gitaly services. +*/ package praefect import ( diff --git a/internal/streamcache/cache.go b/internal/streamcache/cache.go index 333be63c7..857140220 100644 --- a/internal/streamcache/cache.go +++ b/internal/streamcache/cache.go @@ -12,7 +12,7 @@ // readers). A cache entry consists of a key, an maximum age, a // pipe and the error result of the thing writing to the pipe. // -// Eviction +// # Eviction // // There are two eviction goroutines: one for Cache and one for filestore. // The Cache eviction goroutine evicts entries after a set amount of time, diff --git a/internal/testhelper/gitlabtest.go b/internal/testhelper/gitlabtest.go index 46632874b..bfcd0856f 100644 --- a/internal/testhelper/gitlabtest.go +++ b/internal/testhelper/gitlabtest.go @@ -6,9 +6,9 @@ import ( ) /* - This is a manually maintained map to remove duplicate variable - assignments. Please do not use go generate or such to maintain - these, as we'd effectively test one parser against another. +This is a manually maintained map to remove duplicate variable +assignments. Please do not use go generate or such to maintain +these, as we'd effectively test one parser against another. */ var commitMap = map[string]*gitalypb.GitCommit{ "b83d6e391c22777fca1ed3012fce84f633d7fed0": { diff --git a/internal/testhelper/testdb/db.go b/internal/testhelper/testdb/db.go index c909859c1..0a42e4468 100644 --- a/internal/testhelper/testdb/db.go +++ b/internal/testhelper/testdb/db.go @@ -126,9 +126,11 @@ func (db DB) Close() error { // Must be used only for testing. // The new database with empty relations will be created for each call of this function. // It uses env vars: -// PGHOST - required, URL/socket/dir -// PGPORT - required, binding port -// PGUSER - optional, user - `$ whoami` would be used if not provided +// +// PGHOST - required, URL/socket/dir +// PGPORT - required, binding port +// PGUSER - optional, user - `$ whoami` would be used if not provided +// // Once the test is completed the database will be dropped on test cleanup execution. func New(tb testing.TB) DB { tb.Helper() diff --git a/packed_binaries.go b/packed_binaries.go index 6b0774988..f3b532aba 100644 --- a/packed_binaries.go +++ b/packed_binaries.go @@ -11,10 +11,10 @@ import ( // buildDir is the directory path where our build target places the built binaries. const buildDir = "_build/bin" -//go:embed _build/bin/gitaly-hooks _build/bin/gitaly-ssh _build/bin/gitaly-git2go _build/bin/gitaly-lfs-smudge -// // packedBinariesFS contains embedded binaries. If you modify the above embeddings, you must also update // GITALY_PACKED_EXECUTABLES in Makefile and packedBinaries in internal/gitaly/config/config.go. +// +//go:embed _build/bin/gitaly-hooks _build/bin/gitaly-ssh _build/bin/gitaly-git2go _build/bin/gitaly-lfs-smudge var packedBinariesFS embed.FS // UnpackAuxiliaryBinaries unpacks the packed auxiliary binaries of Gitaly into destination directory. diff --git a/streamio/stream.go b/streamio/stream.go index 68e6bd687..9d953eb66 100644 --- a/streamio/stream.go +++ b/streamio/stream.go @@ -1,7 +1,6 @@ // Package streamio contains wrappers intended for turning gRPC streams // that send/receive messages with a []byte field into io.Writers and // io.Readers. -// package streamio import ( diff --git a/tools/protoc-gen-gitaly-lint/lint.go b/tools/protoc-gen-gitaly-lint/lint.go index 8bb4c9e70..28dcd54ad 100644 --- a/tools/protoc-gen-gitaly-lint/lint.go +++ b/tools/protoc-gen-gitaly-lint/lint.go @@ -14,9 +14,9 @@ import ( // ensureMethodOpType will ensure that method includes the op_type option. // See proto example below: // -// rpc ExampleMethod(ExampleMethodRequest) returns (ExampleMethodResponse) { -// option (op_type).op = ACCESSOR; -// } +// rpc ExampleMethod(ExampleMethodRequest) returns (ExampleMethodResponse) { +// option (op_type).op = ACCESSOR; +// } func ensureMethodOpType(fileDesc *descriptorpb.FileDescriptorProto, m *descriptorpb.MethodDescriptorProto, req *pluginpb.CodeGeneratorRequest) error { opMsg, err := protoutil.GetOpExtension(m) if err != nil { diff --git a/tools/protoc-gen-gitaly-lint/main.go b/tools/protoc-gen-gitaly-lint/main.go index be7de808b..9a1a610c8 100644 --- a/tools/protoc-gen-gitaly-lint/main.go +++ b/tools/protoc-gen-gitaly-lint/main.go @@ -1,49 +1,49 @@ // Command protoc-gen-gitaly-lint is designed to be used as a protobuf compiler // plugin to verify Gitaly processes are being followed when writing RPC's. // -// Usage +// # Usage // // The protoc-gen-gitaly linter can be chained into any protoc workflow that // requires verification that Gitaly RPC guidelines are followed. Typically // this can be done by adding the following argument to an existing protoc // command: // -// --gitaly_lint_out=. +// --gitaly_lint_out=. // // For example, you may add the linter as an argument to the command responsible // for generating Go code: // -// protoc --go_out=. --gitaly_lint_out=. *.proto +// protoc --go_out=. --gitaly_lint_out=. *.proto // // Or, you can run the Gitaly linter by itself. To try out, run the following // command while in the project root: // -// protoc --gitaly_lint_out=. ./go/internal/cmd/protoc-gen-gitaly-lint/testdata/incomplete.proto +// protoc --gitaly_lint_out=. ./go/internal/cmd/protoc-gen-gitaly-lint/testdata/incomplete.proto // // You should see some errors printed to screen for improperly written // RPC's in the incomplete.proto file. // -// Prerequisites +// # Prerequisites // // The protobuf compiler (protoc) can be obtained from the GitHub page: // https://github.com/protocolbuffers/protobuf/releases // -// Background +// # Background // // The protobuf compiler accepts plugins to analyze protobuf files and generate // language specific code. // // These plugins require the following executable naming convention: // -// protoc-gen-$NAME +// protoc-gen-$NAME // // Where $NAME is the plugin name of the compiler desired. The protobuf compiler // will search the PATH until an executable with that name is found for a // desired plugin. For example, the following protoc command: // -// protoc --gitaly_lint_out=. *.proto +// protoc --gitaly_lint_out=. *.proto // -// The above will search the PATH for an executable named protoc-gen-gitaly-lint +// # The above will search the PATH for an executable named protoc-gen-gitaly-lint // // The plugin accepts a protobuf message in STDIN that describes the parsed // protobuf files. A response is sent back on STDOUT that contains any errors. diff --git a/tools/protoc-gen-gitaly-lint/method.go b/tools/protoc-gen-gitaly-lint/method.go index 39329b521..5d2c00741 100644 --- a/tools/protoc-gen-gitaly-lint/method.go +++ b/tools/protoc-gen-gitaly-lint/method.go @@ -32,8 +32,8 @@ func (ml methodLinter) validateAccessor() error { } // validateMutator will ensure the following rules: -// - Mutator RPC's with repository level scope must specify a target repo -// - Mutator RPC's without target repo must not be scoped at repo level +// - Mutator RPC's with repository level scope must specify a target repo +// - Mutator RPC's without target repo must not be scoped at repo level func (ml methodLinter) validateMutator() error { switch scope := ml.opMsg.GetScopeLevel(); scope { diff --git a/tools/protoc-gen-gitaly-protolist/main.go b/tools/protoc-gen-gitaly-protolist/main.go index 7e775360e..c05facdae 100644 --- a/tools/protoc-gen-gitaly-protolist/main.go +++ b/tools/protoc-gen-gitaly-protolist/main.go @@ -3,7 +3,7 @@ // // This plugin can be accessed by invoking the protoc compiler with the following arguments: // -// protoc --plugin=protoc-gen-gitaly-protolist --gitaly_protolist_out=. +// protoc --plugin=protoc-gen-gitaly-protolist --gitaly_protolist_out=. // // The plugin accepts a protobuf message in STDIN that describes the parsed protobuf files. A // response is sent back on STDOUT that contains any errors. |