diff --git a/Makefile b/Makefile index e56a4cc21a1b7a3b9adf82d95d1b98773724893e..fd62790f50cbec09141028b217305e5a83ed9132 100644 --- a/Makefile +++ b/Makefile @@ -224,7 +224,7 @@ GIT_FILTER_REPO_SOURCE_DIR ?= ${DEPENDENCY_DIR}/git-filter-repo ## Go packages to test when using the test-go target. TEST_PACKAGES ?= ${SOURCE_DIR}/... ## Test options passed to `go test`. -TEST_OPTIONS ?= -count=1 +TEST_OPTIONS ?= -count=1 -p=4 ## Specify the output format used to print tests ["standard-verbose", "standard-quiet", "short"] TEST_FORMAT ?= short ## Specify the location where the JUnit-style format shall be written to. @@ -267,7 +267,7 @@ find_go_sources = $(shell find ${SOURCE_DIR} -type d \( -path "${SO run_go_tests = PATH='${SOURCE_DIR}/internal/testhelper/testdata/home/bin:${PATH}' \ TEST_TMP_DIR='${TEST_TMP_DIR}' \ TEST_LOG_DIR='${TEST_LOG_DIR}' \ - ${GOTESTSUM} --format ${TEST_FORMAT} --junitfile '${TEST_JUNIT_REPORT}' --jsonfile '${TEST_JSON_REPORT}' -- -ldflags '${GO_LDFLAGS}' -tags '${SERVER_BUILD_TAGS}' ${TEST_OPTIONS} ${TEST_PACKAGES} + ${GOTESTSUM} --rerun-fails --packages $(TEST_PACKAGES) --format ${TEST_FORMAT} --junitfile '${TEST_JUNIT_REPORT}' --jsonfile '${TEST_JSON_REPORT}' -- -ldflags '${GO_LDFLAGS}' -tags '${SERVER_BUILD_TAGS}' ${TEST_OPTIONS} ${TEST_PACKAGES} ## Test options passed to `dlv test`. DEBUG_OPTIONS ?= $(patsubst -%,-test.%,${TEST_OPTIONS}) diff --git a/client/dial.go b/client/dial.go index ed4e02a6673fc752a0c9c3ce48242a2841618612..af69c373522f7f7b79357dd8edc30b2a0a9bf95d 100644 --- a/client/dial.go +++ b/client/dial.go @@ -18,17 +18,21 @@ import ( // DefaultDialOpts hold the default DialOptions for connection to Gitaly over UNIX-socket var DefaultDialOpts = []grpc.DialOption{} -// DialContext dials the Gitaly at the given address with the provided options. Valid address formats are -// 'unix:' for Unix sockets, 'tcp://' for insecure TCP connections and 'tls://' -// for TCP+TLS connections. +// DialContext creates a client connection to a Gitaly at the given address with the provided options. Valid address +// formats are +// - 'unix:' for Unix sockets +// - 'tcp://' for insecure TCP connections to an IP or hostname (resolved via DNS). +// - 'tls://' for TCP+TLS connections to an IP or hostname (resolved via DNS). +// - 'dns:///' for insecure TCP connections that should be resolved by the +// specified authoritative DNS server. Note that it's not possible to use TLS in conjunction with a DNS authority. // -// The returned ClientConns are configured with tracing and correlation id interceptors to ensure they are propagated +// The returned ClientConn is configured with tracing and correlation id interceptors to ensure they are propagated // correctly. They're also configured to send Keepalives with settings matching what Gitaly expects. // // connOpts should not contain `grpc.WithInsecure` as DialContext determines whether it is needed or not from the // scheme. `grpc.TransportCredentials` should not be provided either as those are handled internally as well. func DialContext(ctx context.Context, rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, error) { - return client.Dial(ctx, rawAddress, client.WithGrpcOptions(connOpts)) + return client.New(ctx, rawAddress, client.WithGrpcOptions(connOpts)) } // Dial calls DialContext with the provided arguments and context.Background. Refer to DialContext's documentation @@ -47,7 +51,7 @@ func DialSidechannel(ctx context.Context, rawAddress string, sr *SidechannelRegi // FailOnNonTempDialError helps to identify if remote listener is ready to accept new connections. func FailOnNonTempDialError() []grpc.DialOption { - return client.FailOnNonTempDialError() + return []grpc.DialOption{} } // HealthCheckDialer uses provided dialer as an actual dialer, but issues a health check request to the remote diff --git a/client/dial_test.go b/client/dial_test.go index 72738c23bc44cdf7f2ef4ce7512a15191bea9db9..62a770185380b97dff2b85499361982dcad8409a 100644 --- a/client/dial_test.go +++ b/client/dial_test.go @@ -117,12 +117,6 @@ func TestDial(t *testing.T) { rawAddress: "", expectDialFailure: true, }, - { - name: "dial fail if there is no listener on address", - rawAddress: "tcp://invalid.address", - dialOpts: FailOnNonTempDialError(), - expectDialFailure: true, - }, } for _, tc := range tests { diff --git a/cmd/gitaly-hooks/hooks.go b/cmd/gitaly-hooks/hooks.go index aa53056fc57e49e95e540b89d92ff2422419005a..c10ae9d9a9a9a8dcb2663ec3efe199214286f2e5 100644 --- a/cmd/gitaly-hooks/hooks.go +++ b/cmd/gitaly-hooks/hooks.go @@ -218,7 +218,7 @@ func dialGitaly(ctx context.Context, payload gitcmd.HooksPayload) (*grpc.ClientC dialOpts = append(dialOpts, grpc.WithChainUnaryInterceptor(unaryInterceptors...)) dialOpts = append(dialOpts, grpc.WithChainStreamInterceptor(streamInterceptors...)) - conn, err := client.Dial(ctx, "unix://"+payload.InternalSocket, client.WithGrpcOptions(dialOpts)) + conn, err := client.New(ctx, "unix://"+payload.InternalSocket, client.WithGrpcOptions(dialOpts)) if err != nil { return nil, fmt.Errorf("error when dialing: %w", err) } diff --git a/cmd/gitaly-ssh/auth_test.go b/cmd/gitaly-ssh/auth_test.go index 54d165ed79e21d44400a079610a915cba4ecd23a..e44467dbf4ff7c601e0f04fd46cf1facc1e33be5 100644 --- a/cmd/gitaly-ssh/auth_test.go +++ b/cmd/gitaly-ssh/auth_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/miekg/dns" "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" @@ -102,6 +103,44 @@ func TestConnectivity(t *testing.T) { return runGitaly(t, cfg), certificate.CertPath }, }, + { + name: "dns", + addr: func(t *testing.T, cfg config.Cfg) (string, string) { + // Configure a Gitaly server that listens over TCP. + cfg.ListenAddr = "localhost:0" + gitalyAddr := runGitaly(t, cfg) + gitalyPort := strings.Split(gitalyAddr, ":")[2] + + // Start a DNS server that responds to anything with the loopback address. + dnsServer := testhelper.NewFakeDNSServer(t).WithHandler(dns.TypeA, func(host string) []string { + return []string{"127.0.0.1"} + }).Start() + + return fmt.Sprintf("dns://%s/%s", dnsServer.Addr(), "localhost:"+gitalyPort), "" + }, + }, + { + name: "dns (no authority)", + addr: func(t *testing.T, cfg config.Cfg) (string, string) { + // Configure a Gitaly server that listens over TCP. + cfg.ListenAddr = "localhost:0" + gitalyAddr := runGitaly(t, cfg) + gitalyPort := strings.Split(gitalyAddr, ":")[2] + + return "dns:///localhost:" + gitalyPort, "" + }, + }, + { + name: "tcp with dns address (no authority)", + addr: func(t *testing.T, cfg config.Cfg) (string, string) { + // Configure a Gitaly server that listens over TCP. + cfg.ListenAddr = "localhost:0" + gitalyAddr := runGitaly(t, cfg) + gitalyPort := strings.Split(gitalyAddr, ":")[2] + + return fmt.Sprintf("tcp://localhost:%s", gitalyPort), "" + }, + }, } payload, err := protojson.Marshal(&gitalypb.SSHUploadPackWithSidechannelRequest{ diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 0ab3045aa500ab8ca4d7390094a2656295be129a..7d97ace61d53fdafa78721ee25d79e2bee9d611a 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -484,7 +484,7 @@ func TestManager_Restore_latest(t *testing.T) { ctx := testhelper.Context(t) - cc, err := client.Dial(ctx, cfg.SocketPath) + cc, err := client.New(ctx, cfg.SocketPath) require.NoError(t, err) defer testhelper.MustClose(t, cc) @@ -849,7 +849,7 @@ func TestManager_Restore_specific(t *testing.T) { ctx := testhelper.Context(t) - cc, err := client.Dial(ctx, cfg.SocketPath) + cc, err := client.New(ctx, cfg.SocketPath) require.NoError(t, err) defer testhelper.MustClose(t, cc) diff --git a/internal/backup/repository_test.go b/internal/backup/repository_test.go index cc2d1d1f076be994c210500838f01dcb2f74d097..3ca54168d0a86bdba3eeb5c4be52118e68489e87 100644 --- a/internal/backup/repository_test.go +++ b/internal/backup/repository_test.go @@ -292,7 +292,7 @@ func TestCreateBundlePatterns_HandleEOF(t *testing.T) { cfg.SocketPath = testserver.RunGitalyServer(t, cfg, setup.RegisterAll) - conn, err := client.Dial(ctx, cfg.SocketPath) + conn, err := client.New(ctx, cfg.SocketPath) require.NoError(t, err) defer testhelper.MustClose(t, conn) @@ -311,7 +311,7 @@ func TestRemoteRepository_ResetRefs_HandleEOF(t *testing.T) { cfg.SocketPath = testserver.RunGitalyServer(t, cfg, setup.RegisterAll) ctx := testhelper.Context(t) - conn, err := client.Dial(ctx, cfg.SocketPath) + conn, err := client.New(ctx, cfg.SocketPath) require.NoError(t, err) defer testhelper.MustClose(t, conn) diff --git a/internal/cli/gitaly/serve.go b/internal/cli/gitaly/serve.go index d989396336a0ee3d06699c6383821297bd92b0c2..9440308cb4598469cecbc01411a4817596b03840 100644 --- a/internal/cli/gitaly/serve.go +++ b/internal/cli/gitaly/serve.go @@ -274,13 +274,12 @@ func run(appCtx *cli.Context, cfg config.Cfg, logger log.Logger) error { conns := client.NewPool( client.WithDialer(client.HealthCheckDialer( func(ctx context.Context, address string, opts []grpc.DialOption) (*grpc.ClientConn, error) { - return client.Dial(ctx, address, client.WithGrpcOptions(opts)) + return client.New(ctx, address, client.WithGrpcOptions(opts)) }, )), - client.WithDialOptions(append( - client.FailOnNonTempDialError(), + client.WithDialOptions( client.UnaryInterceptor(), - client.StreamInterceptor())..., + client.StreamInterceptor(), ), ) defer func() { diff --git a/internal/cli/gitaly/subcmd_hooks.go b/internal/cli/gitaly/subcmd_hooks.go index 9b81003f3512e2a38fd09f3461979cdc57bd0c3c..16e8e02f79098f017d32d4b8e67e1e020d7b2fce 100644 --- a/internal/cli/gitaly/subcmd_hooks.go +++ b/internal/cli/gitaly/subcmd_hooks.go @@ -143,7 +143,6 @@ func dial(ctx context.Context, addr, token string, timeout time.Duration, opts . defer cancel() opts = append(opts, - grpc.WithBlock(), client.UnaryInterceptor(), client.StreamInterceptor(), ) @@ -156,7 +155,7 @@ func dial(ctx context.Context, addr, token string, timeout time.Duration, opts . ) } - return client.Dial(ctx, addr, client.WithGrpcOptions(opts)) + return client.New(ctx, addr, client.WithGrpcOptions(opts)) } func getAddressWithScheme(cfg config.Cfg) (string, error) { diff --git a/internal/cli/gitaly/subcmd_hooks_test.go b/internal/cli/gitaly/subcmd_hooks_test.go index 01b4942a49f25f49ca57c8ec83c14ba9c0f92573..c647c5701bd358d7e60eb4e02ba8219b694ff62c 100644 --- a/internal/cli/gitaly/subcmd_hooks_test.go +++ b/internal/cli/gitaly/subcmd_hooks_test.go @@ -223,7 +223,7 @@ func newRepositoryClient(tb testing.TB, ctx context.Context, cfg config.Cfg, ser if cfg.Auth.Token != "" { connOpts = append(connOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token))) } - conn, err := client.Dial(ctx, serverSocketPath, client.WithGrpcOptions(connOpts)) + conn, err := client.New(ctx, serverSocketPath, client.WithGrpcOptions(connOpts)) require.NoError(tb, err) tb.Cleanup(func() { require.NoError(tb, conn.Close()) }) diff --git a/internal/cli/praefect/subcmd.go b/internal/cli/praefect/subcmd.go index bb171b2a32a472fd9fa77bc6547f8cbdcc28abbf..1568151706a3356544906b65dea026d07c58d09b 100644 --- a/internal/cli/praefect/subcmd.go +++ b/internal/cli/praefect/subcmd.go @@ -60,7 +60,6 @@ func subCmdDial(ctx context.Context, addr, token string, timeout time.Duration, defer cancel() opts = append(opts, - grpc.WithBlock(), client.UnaryInterceptor(), client.StreamInterceptor(), ) @@ -73,7 +72,7 @@ func subCmdDial(ctx context.Context, addr, token string, timeout time.Duration, ) } - return client.Dial(ctx, addr, client.WithGrpcOptions(opts)) + return client.New(ctx, addr, client.WithGrpcOptions(opts)) } type requiredParameterError string diff --git a/internal/cli/praefect/subcmd_list_untracked_repositories_test.go b/internal/cli/praefect/subcmd_list_untracked_repositories_test.go index 7f530203269f18e5e34c0b25d2530b7e7d812119..3a026a0986525ba2792fdce847fb9970809f63ed 100644 --- a/internal/cli/praefect/subcmd_list_untracked_repositories_test.go +++ b/internal/cli/praefect/subcmd_list_untracked_repositories_test.go @@ -56,7 +56,7 @@ func TestListUntrackedRepositoriesCommand(t *testing.T) { praefectServer := testserver.StartPraefect(t, conf) - cc, err := client.Dial(ctx, praefectServer.Address()) + cc, err := client.New(ctx, praefectServer.Address()) require.NoError(t, err) defer func() { require.NoError(t, cc.Close()) }() repoClient := gitalypb.NewRepositoryServiceClient(cc) diff --git a/internal/cli/praefect/subcmd_remove_repository_test.go b/internal/cli/praefect/subcmd_remove_repository_test.go index 3f0a6b62b247194d749b35beb95f05530de62fd6..44e76b16da4897f3df0f94a1930ef690efb20fed 100644 --- a/internal/cli/praefect/subcmd_remove_repository_test.go +++ b/internal/cli/praefect/subcmd_remove_repository_test.go @@ -38,11 +38,11 @@ func TestRemoveRepositorySubcommand(t *testing.T) { gitalyOneAddr := testserver.RunGitalyServer(t, gitalyOneCfg, setup.RegisterAll, testserver.WithDisablePraefect()) gitalyTwoSrv := testserver.StartGitalyServer(t, gitalyTwoCfg, setup.RegisterAll, testserver.WithDisablePraefect()) - gitalyOneConfig, err := client.Dial(ctx, gitalyOneAddr) + gitalyOneConfig, err := client.New(ctx, gitalyOneAddr) require.NoError(t, err) defer testhelper.MustClose(t, gitalyOneConfig) - gitalyTwoConfig, err := client.Dial(ctx, gitalyTwoSrv.Address()) + gitalyTwoConfig, err := client.New(ctx, gitalyTwoSrv.Address()) require.NoError(t, err) defer testhelper.MustClose(t, gitalyTwoConfig) @@ -70,7 +70,7 @@ func TestRemoveRepositorySubcommand(t *testing.T) { praefectServer := testserver.StartPraefect(t, conf) - cc, err := client.Dial(ctx, praefectServer.Address()) + cc, err := client.New(ctx, praefectServer.Address()) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, cc.Close()) }) repoClient := gitalypb.NewRepositoryServiceClient(cc) diff --git a/internal/cli/praefect/subcmd_test.go b/internal/cli/praefect/subcmd_test.go index 6b560404279873716345ca27434679dc54251f49..53612e39f13e6a4bae99be60f0ab3c4552901954 100644 --- a/internal/cli/praefect/subcmd_test.go +++ b/internal/cli/praefect/subcmd_test.go @@ -55,11 +55,9 @@ func listenAndServe(tb testing.TB, svcs []svcRegistrar) (net.Listener, testhelpe errCh := make(chan error, 1) go func() { errCh <- srv.Serve(ln) }() - ctx := testhelper.Context(tb) - // verify the service is up addr := fmt.Sprintf("%s://%s", ln.Addr().Network(), ln.Addr()) - cc, err := grpc.DialContext(ctx, addr, grpc.WithBlock(), grpc.WithTransportCredentials(insecure.NewCredentials())) + cc, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(tb, err) require.NoError(tb, cc.Close()) diff --git a/internal/cli/praefect/subcmd_track_repositories_test.go b/internal/cli/praefect/subcmd_track_repositories_test.go index 77a27186352f842e3c614dab446fef028c84dcc2..75fabc7cc5330bd1e5a95ae08a1d892feec357aa 100644 --- a/internal/cli/praefect/subcmd_track_repositories_test.go +++ b/internal/cli/praefect/subcmd_track_repositories_test.go @@ -69,7 +69,7 @@ func TestTrackRepositoriesSubcommand(t *testing.T) { } confPath := writeConfigToFile(t, conf) - gitalyOneConn, err := client.Dial(ctx, gitalyOneAddr) + gitalyOneConn, err := client.New(ctx, gitalyOneAddr) require.NoError(t, err) defer testhelper.MustClose(t, gitalyOneConn) diff --git a/internal/cli/praefect/subcmd_track_repository_test.go b/internal/cli/praefect/subcmd_track_repository_test.go index 332988d80168fd35472d0f92b1e406f954d9a81d..53509a1e864473057dd1e103a1668e797a912072 100644 --- a/internal/cli/praefect/subcmd_track_repository_test.go +++ b/internal/cli/praefect/subcmd_track_repository_test.go @@ -64,11 +64,11 @@ func TestTrackRepositorySubcommand(t *testing.T) { } confPath := writeConfigToFile(t, conf) - gitalyOneConn, err := client.Dial(ctx, gitalyOneAddr) + gitalyOneConn, err := client.New(ctx, gitalyOneAddr) require.NoError(t, err) defer testhelper.MustClose(t, gitalyOneConn) - gitalyTwoConn, err := client.Dial(ctx, gitalyTwoSrv.Address()) + gitalyTwoConn, err := client.New(ctx, gitalyTwoSrv.Address()) require.NoError(t, err) defer testhelper.MustClose(t, gitalyTwoConn) diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go index f3a5dd42e921e28266db98db3065d39aeba3791f..fb15ffff9465cd974b5c26ea0d3b1ede9c343d8a 100644 --- a/internal/git/gittest/repo.go +++ b/internal/git/gittest/repo.go @@ -118,7 +118,7 @@ func DialService(tb testing.TB, ctx context.Context, cfg config.Cfg) *grpc.Clien require.FailNow(tb, "cannot dial service without configured address") } - conn, err := client.Dial(ctx, addr, client.WithGrpcOptions(dialOptions)) + conn, err := client.New(ctx, addr, client.WithGrpcOptions(dialOptions)) require.NoError(tb, err) tb.Cleanup(func() { testhelper.MustClose(tb, conn) }) return conn diff --git a/internal/gitaly/server/server_factory_test.go b/internal/gitaly/server/server_factory_test.go index c6e27d372122d5d62114865c4fa0d25f2905973a..eee8b0b657a981ef59713e6fa5bab170805c5a0b 100644 --- a/internal/gitaly/server/server_factory_test.go +++ b/internal/gitaly/server/server_factory_test.go @@ -48,7 +48,7 @@ func TestGitalyServerFactory(t *testing.T) { creds := cert.TransportCredentials(t) - cc, err = grpc.DialContext(ctx, listener.Addr().String(), grpc.WithTransportCredentials(creds)) + cc, err = grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(creds)) require.NoError(t, err) } else { srv, err := sf.CreateExternal(false) @@ -64,7 +64,7 @@ func TestGitalyServerFactory(t *testing.T) { endpoint, err := starter.ComposeEndpoint(schema, listener.Addr().String()) require.NoError(t, err) - cc, err = client.Dial(ctx, endpoint) + cc, err = client.New(ctx, endpoint) require.NoError(t, err) } t.Cleanup(func() { assert.NoError(t, cc.Close()) }) @@ -296,7 +296,7 @@ func TestGitalyServerFactory_closeOrder(t *testing.T) { go testhelper.MustServe(t, server, ln) - conn, err := grpc.DialContext(ctx, ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + conn, err := grpc.NewClient(ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) t.Cleanup(func() { testhelper.MustClose(t, conn) }) *builder.conn = conn diff --git a/internal/gitaly/service/operations/testhelper_test.go b/internal/gitaly/service/operations/testhelper_test.go index a7037dd9f1930cba2a716d203f8e0f77f8135811..8ace9b6178534bdc15966fa198cf195d1715b4ad 100644 --- a/internal/gitaly/service/operations/testhelper_test.go +++ b/internal/gitaly/service/operations/testhelper_test.go @@ -103,7 +103,7 @@ func newOperationClient(tb testing.TB, serverSocketPath string) (gitalypb.Operat } func newMuxedOperationClient(t *testing.T, ctx context.Context, serverSocketPath, authToken string, handshaker client.Handshaker) gitalypb.OperationServiceClient { - conn, err := client.Dial(ctx, serverSocketPath, client.WithGrpcOptions([]grpc.DialOption{grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(authToken))}), client.WithHandshaker(handshaker)) + conn, err := client.New(ctx, serverSocketPath, client.WithGrpcOptions([]grpc.DialOption{grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(authToken))}), client.WithHandshaker(handshaker)) require.NoError(t, err) t.Cleanup(func() { conn.Close() }) return gitalypb.NewOperationServiceClient(conn) diff --git a/internal/gitaly/service/partition/testhelper_test.go b/internal/gitaly/service/partition/testhelper_test.go index e020b7e695fccaacf0b1cd580114c7c711c6f9fa..1698be1f96e01678c842eabf830b447538c16c58 100644 --- a/internal/gitaly/service/partition/testhelper_test.go +++ b/internal/gitaly/service/partition/testhelper_test.go @@ -26,7 +26,7 @@ func setupServices(tb testing.TB, opt ...testserver.GitalyServerOpt) (config.Cfg addr := testserver.RunGitalyServer(tb, cfg, setup.RegisterAll, opt...) cfg.SocketPath = addr - cc, err := client.Dial(testhelper.Context(tb), cfg.SocketPath) + cc, err := client.New(testhelper.Context(tb), cfg.SocketPath) require.NoError(tb, err) tb.Cleanup(func() { testhelper.MustClose(tb, cc) }) diff --git a/internal/gitaly/service/repository/testhelper_test.go b/internal/gitaly/service/repository/testhelper_test.go index 39b618ce59a5a86e06ac1026154d22ec1ffbbfb0..329a05ee4ebc0c158e588668b0a3b9d9bc0cfb24 100644 --- a/internal/gitaly/service/repository/testhelper_test.go +++ b/internal/gitaly/service/repository/testhelper_test.go @@ -35,7 +35,7 @@ func newRepositoryClient(tb testing.TB, cfg config.Cfg, serverSocketPath string) if cfg.Auth.Token != "" { connOpts = append(connOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token))) } - conn, err := client.Dial(testhelper.Context(tb), serverSocketPath, client.WithGrpcOptions(connOpts)) + conn, err := client.New(testhelper.Context(tb), serverSocketPath, client.WithGrpcOptions(connOpts)) require.NoError(tb, err) tb.Cleanup(func() { require.NoError(tb, conn.Close()) }) @@ -43,7 +43,7 @@ func newRepositoryClient(tb testing.TB, cfg config.Cfg, serverSocketPath string) } func newMuxedRepositoryClient(t *testing.T, ctx context.Context, cfg config.Cfg, serverSocketPath string, handshaker client.Handshaker) gitalypb.RepositoryServiceClient { - conn, err := client.Dial(ctx, serverSocketPath, + conn, err := client.New(ctx, serverSocketPath, client.WithGrpcOptions([]grpc.DialOption{ grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(cfg.Auth.Token)), }), diff --git a/internal/gitaly/service/smarthttp/testhelper_test.go b/internal/gitaly/service/smarthttp/testhelper_test.go index 251fdacb81a3c063e370de43964b701e110e8ba7..d1f34d8d90586a220d403ea7a2895d7059bf7826 100644 --- a/internal/gitaly/service/smarthttp/testhelper_test.go +++ b/internal/gitaly/service/smarthttp/testhelper_test.go @@ -63,7 +63,7 @@ func newSmartHTTPClient(t *testing.T, serverSocketPath, token string) gitalypb.S func newMuxedSmartHTTPClient(t *testing.T, ctx context.Context, serverSocketPath, token string, serverFactory backchannel.ServerFactory) gitalypb.SmartHTTPServiceClient { t.Helper() - conn, err := client.Dial( + conn, err := client.New( ctx, serverSocketPath, client.WithGrpcOptions([]grpc.DialOption{grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(token))}), diff --git a/internal/gitaly/service/smarthttp/upload_pack_test.go b/internal/gitaly/service/smarthttp/upload_pack_test.go index 8fab391132a98f8ac558ce66e332a5d9ec98dc43..07949ca4ac9d88ad57afacbf504e4edf0772af62 100644 --- a/internal/gitaly/service/smarthttp/upload_pack_test.go +++ b/internal/gitaly/service/smarthttp/upload_pack_test.go @@ -273,7 +273,7 @@ func (m mockStreamCache) Fetch(ctx context.Context, key string, dst io.Writer, c } func newRepositoryClient(tb testing.TB, cfg config.Cfg) gitalypb.RepositoryServiceClient { - conn, err := client.Dial(testhelper.Context(tb), cfg.SocketPath) + conn, err := client.New(testhelper.Context(tb), cfg.SocketPath) require.NoError(tb, err) tb.Cleanup(func() { require.NoError(tb, conn.Close()) }) diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go index db750f5d653394009a816c5201c5f3c84bf6930a..edf4970eb75f5e951ce0e24e8156303027f65d3e 100644 --- a/internal/gitaly/service/ssh/upload_pack_test.go +++ b/internal/gitaly/service/ssh/upload_pack_test.go @@ -685,7 +685,7 @@ func (m mockStreamCache) Fetch(ctx context.Context, key string, dst io.Writer, c } func newRepositoryClient(tb testing.TB, cfg config.Cfg) gitalypb.RepositoryServiceClient { - conn, err := client.Dial(testhelper.Context(tb), cfg.SocketPath) + conn, err := client.New(testhelper.Context(tb), cfg.SocketPath) require.NoError(tb, err) tb.Cleanup(func() { require.NoError(tb, conn.Close()) }) diff --git a/internal/gitaly/storage/storagemgr/middleware_ext_test.go b/internal/gitaly/storage/storagemgr/middleware_ext_test.go index 86467d0d7fa10db9a3e80b0f87c74470a75b23bb..5beca35d7ae016b03d394b0d6587d52a68328a15 100644 --- a/internal/gitaly/storage/storagemgr/middleware_ext_test.go +++ b/internal/gitaly/storage/storagemgr/middleware_ext_test.go @@ -42,7 +42,7 @@ func TestMiddleware_partitioning_hint(t *testing.T) { { desc: "implicit CreateFork hint", createFork: func(t *testing.T, ctx context.Context, cfg config.Cfg, alternate *gitalypb.Repository) *gitalypb.Repository { - cc, err := client.Dial(ctx, cfg.ListenAddr) + cc, err := client.New(ctx, cfg.ListenAddr) require.NoError(t, err) defer testhelper.MustClose(t, cc) @@ -65,7 +65,7 @@ func TestMiddleware_partitioning_hint(t *testing.T) { { desc: "explicit partitioning hint with implicit CreateFork hint", createFork: func(t *testing.T, ctx context.Context, cfg config.Cfg, alternate *gitalypb.Repository) *gitalypb.Repository { - cc, err := client.Dial(ctx, cfg.ListenAddr) + cc, err := client.New(ctx, cfg.ListenAddr) require.NoError(t, err) defer testhelper.MustClose(t, cc) @@ -96,7 +96,7 @@ func TestMiddleware_partitioning_hint(t *testing.T) { { desc: "explicit partitioning hint with an additional repository fails", createFork: func(t *testing.T, ctx context.Context, cfg config.Cfg, alternate *gitalypb.Repository) *gitalypb.Repository { - cc, err := client.Dial(ctx, cfg.ListenAddr) + cc, err := client.New(ctx, cfg.ListenAddr) require.NoError(t, err) defer testhelper.MustClose(t, cc) @@ -133,7 +133,7 @@ func TestMiddleware_partitioning_hint(t *testing.T) { RelativePath: gittest.NewObjectPoolName(t), }) - cc, err := client.Dial(ctx, cfg.ListenAddr) + cc, err := client.New(ctx, cfg.ListenAddr) require.NoError(t, err) defer testhelper.MustClose(t, cc) diff --git a/internal/gitaly/storage/storagemgr/middleware_test.go b/internal/gitaly/storage/storagemgr/middleware_test.go index 39a6843a027f6a585c82b470573c953d87bc62ef..0cd3ca080f70d1acb5d30c89e6a3282be54e038c 100644 --- a/internal/gitaly/storage/storagemgr/middleware_test.go +++ b/internal/gitaly/storage/storagemgr/middleware_test.go @@ -634,7 +634,7 @@ messages and behavior by erroring out the requests before they even hit this int testserver.WithLogger(log.FromLogrusEntry(entry)), ) - clientConn, err := client.Dial(ctx, serverAddress) + clientConn, err := client.New(ctx, serverAddress) require.NoError(t, err) defer clientConn.Close() @@ -815,7 +815,7 @@ messages and behavior by erroring out the requests before they even hit this int }) }) - clientConn, err := client.Dial(ctx, serverAddress) + clientConn, err := client.New(ctx, serverAddress) require.NoError(t, err) defer clientConn.Close() diff --git a/internal/gitaly/transaction/manager.go b/internal/gitaly/transaction/manager.go index e25d5cb54631a79c182765801a87a39d8283cf9e..54db2dff812c51e643d50a9846cf44f53f39f913 100644 --- a/internal/gitaly/transaction/manager.go +++ b/internal/gitaly/transaction/manager.go @@ -67,10 +67,9 @@ func NewManager(cfg config.Cfg, logger log.Logger, backchannels *backchannel.Reg return &PoolManager{ logger: logger.WithField("component", "transaction.PoolManager"), backchannels: backchannels, - conns: client.NewPool(client.WithDialOptions(append( - client.FailOnNonTempDialError(), + conns: client.NewPool(client.WithDialOptions( client.UnaryInterceptor(), - client.StreamInterceptor())..., + client.StreamInterceptor(), )), votingDelayMetric: prometheus.NewHistogram( prometheus.HistogramOpts{ diff --git a/internal/gitaly/transaction/manager_test.go b/internal/gitaly/transaction/manager_test.go index c37257ee5e2f1b3a3ac492de60ba13a3f5d03a8b..d4c5b845234ccbe59bc10df1eec2fc368f4e347d 100644 --- a/internal/gitaly/transaction/manager_test.go +++ b/internal/gitaly/transaction/manager_test.go @@ -48,7 +48,7 @@ func TestPoolManager_Vote(t *testing.T) { logger := testhelper.NewLogger(t) registry := backchannel.NewRegistry() - backchannelConn, err := client.Dial(ctx, transactionServerAddr) + backchannelConn, err := client.New(ctx, transactionServerAddr) require.NoError(t, err) defer backchannelConn.Close() @@ -198,7 +198,7 @@ func TestPoolManager_Stop(t *testing.T) { logger := testhelper.NewLogger(t) registry := backchannel.NewRegistry() - backchannelConn, err := client.Dial(ctx, transactionServerAddr) + backchannelConn, err := client.New(ctx, transactionServerAddr) require.NoError(t, err) defer backchannelConn.Close() diff --git a/internal/grpc/client/address_parser.go b/internal/grpc/client/address_parser.go index 1d0d560cb03d5bf0b61fc31dcb9a47b9683bf123..05d266eb1473bf7a872ee4dc4bd8f32503f7b8aa 100644 --- a/internal/grpc/client/address_parser.go +++ b/internal/grpc/client/address_parser.go @@ -7,7 +7,7 @@ import ( ) // extractHostFromRemoteURL will convert Gitaly-style URL addresses of the form -// scheme://host:port to the "host:port" addresses used by `grpc.Dial` +// scheme://host:port to the "host:port" addresses used by `grpc.NewClient` func extractHostFromRemoteURL(rawAddress string) (hostAndPort string, err error) { u, err := url.Parse(rawAddress) if err != nil { diff --git a/internal/grpc/client/dial.go b/internal/grpc/client/dial.go index 4aad0e23cc75ca0930898e15711adce8b80088e1..4219d43b5d27d48631c395fe50cf17bca29c2ab2 100644 --- a/internal/grpc/client/dial.go +++ b/internal/grpc/client/dial.go @@ -31,6 +31,7 @@ const ( tlsConnection unixConnection dnsConnection + dnsPlusTLSConnection ) func getConnectionType(rawAddress string) connectionType { @@ -66,7 +67,7 @@ type dialConfig struct { creds credentials.TransportCredentials } -// DialOption is an option that can be passed to Dial. +// DialOption is an option that can be passed to NewClient. type DialOption func(*dialConfig) // WithHandshaker sets up the given handshaker so that it's passed as the transport credentials @@ -79,7 +80,7 @@ func WithHandshaker(handshaker Handshaker) DialOption { } // WithGrpcOptions will set up the given gRPC dial options so that they will be used when calling -// `grpc.DialContext()`. +// `grpc.NewClient()`. func WithGrpcOptions(opts []grpc.DialOption) DialOption { return func(cfg *dialConfig) { cfg.grpcOpts = append(cfg.grpcOpts, opts...) @@ -95,9 +96,9 @@ func WithTransportCredentials(creds credentials.TransportCredentials) DialOption } } -// Dial dials a Gitaly node serving at the given address. Dial is used by the public 'client' package -// and the expected behavior is mostly documented there. -func Dial(ctx context.Context, rawAddress string, opts ...DialOption) (*grpc.ClientConn, error) { +// New creates a dormant connection to a Gitaly node serving at the given address. New is used by the public 'client' +// package and the expected behavior is mostly documented there. +func New(ctx context.Context, rawAddress string, opts ...DialOption) (*grpc.ClientConn, error) { var dialCfg dialConfig for _, opt := range opts { opt(&dialCfg) @@ -131,7 +132,6 @@ func Dial(ctx context.Context, rawAddress string, opts ...DialOption) (*grpc.Cli return nil, fmt.Errorf("failed to parse target for 'dns' connection: %w", err) } canonicalAddress = rawAddress // DNS Resolver will handle this - case unixConnection: canonicalAddress = rawAddress // This will be overridden by the custom dialer... connOpts = append( @@ -208,7 +208,7 @@ func Dial(ctx context.Context, rawAddress string, opts ...DialOption) (*grpc.Cli grpc.WithDefaultServiceConfig(defaultServiceConfig()), ) - conn, err := grpc.DialContext(ctx, canonicalAddress, connOpts...) + conn, err := grpc.NewClient(canonicalAddress, connOpts...) if err != nil { return nil, fmt.Errorf("failed to dial %q connection: %w", canonicalAddress, err) } @@ -280,14 +280,6 @@ func defaultServiceConfig() string { return string(configJSON) } -// FailOnNonTempDialError helps to identify if remote listener is ready to accept new connections. -func FailOnNonTempDialError() []grpc.DialOption { - return []grpc.DialOption{ - grpc.WithBlock(), - grpc.FailOnNonTempDialError(true), - } -} - // HealthCheckDialer uses provided dialer as an actual dialer, but issues a health check request to the remote // to verify the connection was set properly and could be used with no issues. func HealthCheckDialer(base Dialer) Dialer { diff --git a/internal/grpc/client/dial_ext_test.go b/internal/grpc/client/dial_ext_test.go index 2be6d3e01f409dc4bad8669c1d032f743a3d547a..f4a73221cf9a25e3fa4033449308c96d77d5102f 100644 --- a/internal/grpc/client/dial_ext_test.go +++ b/internal/grpc/client/dial_ext_test.go @@ -106,7 +106,7 @@ func TestRetryPolicy(t *testing.T) { SkipSnapshotInvalidation: true, }) - conn, err := client.Dial(ctx, cfg.SocketPath) + conn, err := client.New(ctx, cfg.SocketPath) require.NoError(t, err) defer testhelper.MustClose(t, conn) diff --git a/internal/grpc/client/dial_test.go b/internal/grpc/client/dial_test.go index 39636f213bb990692e770b83d283626e6a9f89d5..6f3ba367c9f205e508311ac8d3c1f8a03c384e2d 100644 --- a/internal/grpc/client/dial_test.go +++ b/internal/grpc/client/dial_test.go @@ -47,7 +47,7 @@ func TestDial(t *testing.T) { ctx := testhelper.Context(t) t.Run("non-muxed conn", func(t *testing.T) { - nonMuxedConn, err := Dial(ctx, "tcp://"+ln.Addr().String()) + nonMuxedConn, err := New(ctx, "tcp://"+ln.Addr().String()) require.NoError(t, err) defer func() { require.NoError(t, nonMuxedConn.Close()) }() @@ -57,7 +57,7 @@ func TestDial(t *testing.T) { t.Run("muxed conn", func(t *testing.T) { handshaker := backchannel.NewClientHandshaker(logger, func() backchannel.Server { return grpc.NewServer() }, backchannel.DefaultConfiguration()) - nonMuxedConn, err := Dial(ctx, "tcp://"+ln.Addr().String(), WithHandshaker(handshaker)) + nonMuxedConn, err := New(ctx, "tcp://"+ln.Addr().String(), WithHandshaker(handshaker)) require.NoError(t, err) defer func() { require.NoError(t, nonMuxedConn.Close()) }() diff --git a/internal/grpc/client/pool.go b/internal/grpc/client/pool.go index a79112dc8c8006b31218cfe5502d18a5f2243340..26a350332ce1b19647755f789717890f8c772178 100644 --- a/internal/grpc/client/pool.go +++ b/internal/grpc/client/pool.go @@ -28,7 +28,7 @@ func WithDialer(dialer Dialer) PoolOption { } } -// WithDialOptions sets gRPC options to use for the gRPC Dial call. +// WithDialOptions sets gRPC options to use for the gRPC NewClient call. func WithDialOptions(dialOptions ...grpc.DialOption) PoolOption { return func(options *poolConfig) { options.dialOptions = dialOptions @@ -50,7 +50,7 @@ type Pool struct { func NewPool(opts ...PoolOption) *Pool { cfg := poolConfig{ dialer: func(ctx context.Context, address string, opts []grpc.DialOption) (*grpc.ClientConn, error) { - return Dial(ctx, address, WithGrpcOptions(opts)) + return New(ctx, address, WithGrpcOptions(opts)) }, } for _, opt := range opts { diff --git a/internal/grpc/grpcstats/stats_test.go b/internal/grpc/grpcstats/stats_test.go index 04a393737d1655b5dab744298cf000926d5485d3..19c5ef0307325f99f4bf311caf1754a6cbf1b3cb 100644 --- a/internal/grpc/grpcstats/stats_test.go +++ b/internal/grpc/grpcstats/stats_test.go @@ -101,7 +101,7 @@ func TestPayloadBytes(t *testing.T) { t.Cleanup(srv.GracefulStop) go func() { assert.NoError(t, srv.Serve(lis)) }() - cc, err := client.Dial(ctx, "unix://"+sock.Name()) + cc, err := client.New(ctx, "unix://"+sock.Name()) require.NoError(t, err) t.Cleanup(func() { require.NoError(t, cc.Close()) }) diff --git a/internal/grpc/middleware/cache/cache_test.go b/internal/grpc/middleware/cache/cache_test.go index e911fd85d80476ef82bfc0b867307538793f9e57..9251fbfe47acdf0427c0a57fd1b32acb9d7345d9 100644 --- a/internal/grpc/middleware/cache/cache_test.go +++ b/internal/grpc/middleware/cache/cache_test.go @@ -130,10 +130,8 @@ func TestInvalidators(t *testing.T) { }() t.Cleanup(server.Stop) - conn, err := grpc.DialContext( - ctx, + conn, err := grpc.NewClient( listener.Addr().String(), - grpc.WithBlock(), grpc.WithTransportCredentials(insecure.NewCredentials()), ) require.NoError(t, err) diff --git a/internal/grpc/middleware/customfieldshandler/customfields_handler_test.go b/internal/grpc/middleware/customfieldshandler/customfields_handler_test.go index 1ad7cdc319cb174e72ec5a7bab10546a6618bbf7..f47207f65eaf16063b8d0cde7715c2c093c2707b 100644 --- a/internal/grpc/middleware/customfieldshandler/customfields_handler_test.go +++ b/internal/grpc/middleware/customfieldshandler/customfields_handler_test.go @@ -132,7 +132,7 @@ func TestInterceptor(t *testing.T) { t.Run(tt.name, func(t *testing.T) { hook.Reset() - conn, err := grpc.DialContext(ctx, "", grpc.WithContextDialer(getBufDialer(listener)), grpc.WithTransportCredentials(insecure.NewCredentials())) + conn, err := grpc.NewClient("passthrough://", grpc.WithContextDialer(getBufDialer(listener)), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer conn.Close() diff --git a/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go b/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go index 2b72eb4c0e7fdb0515ddada8f9c916b4c8d6b765..6d82abde348ae5cb53f2df0c7e96265fff700935 100644 --- a/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go +++ b/internal/grpc/middleware/requestinfohandler/requestinfohandler_test.go @@ -845,7 +845,7 @@ func setupServer(tb testing.TB, ctx context.Context) (*mockServer, mockClient) { listener := bufconn.Listen(1) go testhelper.MustServe(tb, server, listener) - conn, err := grpc.DialContext(ctx, listener.Addr().String(), + conn, err := grpc.NewClient("passthrough://"+listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return listener.DialContext(ctx) diff --git a/internal/grpc/middleware/sentryhandler/sentryhandler_test.go b/internal/grpc/middleware/sentryhandler/sentryhandler_test.go index 61d67ed1066b5dbc95a07557cbd8ea4181df30cd..9ca48ebe4da22b992ca0e12e2f4b378618d9fba7 100644 --- a/internal/grpc/middleware/sentryhandler/sentryhandler_test.go +++ b/internal/grpc/middleware/sentryhandler/sentryhandler_test.go @@ -415,7 +415,7 @@ func (s *mockServiceServer) setup(tb testing.TB, ctx context.Context) grpc_testi listener := bufconn.Listen(1) go testhelper.MustServe(tb, server, listener) - conn, err := grpc.DialContext(ctx, listener.Addr().String(), + conn, err := grpc.NewClient("passthrough://"+listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return listener.DialContext(ctx) diff --git a/internal/grpc/proxy/handler_ext_test.go b/internal/grpc/proxy/handler_ext_test.go index ed99af19a0f890f3163882e12df51b39150b6c82..d66b1d3335e48a639726aea080f18b2e02645cb7 100644 --- a/internal/grpc/proxy/handler_ext_test.go +++ b/internal/grpc/proxy/handler_ext_test.go @@ -367,7 +367,7 @@ func TestProxyErrorPropagation(t *testing.T) { }() defer backendServer.Stop() - backendClientConn, err := grpc.DialContext(ctx, "unix://"+backendListener.Addr().String(), + backendClientConn, err := grpc.NewClient("unix://"+backendListener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDefaultCallOptions(grpc.ForceCodec(proxy.NewCodec())), ) @@ -399,7 +399,7 @@ func TestProxyErrorPropagation(t *testing.T) { }() defer proxyServer.Stop() - proxyClientConn, err := grpc.DialContext(ctx, "unix://"+proxyListener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + proxyClientConn, err := grpc.NewClient("unix://"+proxyListener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer testhelper.MustClose(t, proxyClientConn) @@ -509,7 +509,7 @@ func TestRegisterStreamHandlers(t *testing.T) { go testhelper.MustServe(t, server, listener) defer server.Stop() - conn, err := client.Dial(ctx, "tcp://"+listener.Addr().String(), client.WithGrpcOptions([]grpc.DialOption{grpc.WithBlock()})) + conn, err := client.New(ctx, "tcp://"+listener.Addr().String()) require.NoError(t, err) defer conn.Close() client := grpc_testing.NewTestServiceClient(conn) diff --git a/internal/grpc/proxy/proxy_test_testhelper_test.go b/internal/grpc/proxy/proxy_test_testhelper_test.go index 3fef99bda1291e7ddf7eeaa3cf12b03797e03817..957b990b4edc2ab3b90ffc7d793ac19cf8eadc4e 100644 --- a/internal/grpc/proxy/proxy_test_testhelper_test.go +++ b/internal/grpc/proxy/proxy_test_testhelper_test.go @@ -34,11 +34,9 @@ func newBackendPinger(tb testing.TB, ctx context.Context) (*grpc.ClientConn, *in require.NoError(tb, srvr.Serve(listener)) }() - cc, err := grpc.DialContext( - ctx, + cc, err := grpc.NewClient( listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock(), grpc.WithDefaultCallOptions( grpc.ForceCodec(proxy.NewCodec()), ), @@ -72,11 +70,9 @@ func newProxy(tb testing.TB, ctx context.Context, director proxy.StreamDirector, require.NoError(tb, proxySrvr.Serve(listener)) }() - proxyCC, err := grpc.DialContext( - ctx, + proxyCC, err := grpc.NewClient( listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), - grpc.WithBlock(), ) require.NoError(tb, err) diff --git a/internal/grpc/sidechannel/conn.go b/internal/grpc/sidechannel/conn.go index 7e8830bd8e5b367b29ffd623b304c7b73eb56fc0..e03a3f5297f941c8ea9ad191d55ebbe9ce0475a8 100644 --- a/internal/grpc/sidechannel/conn.go +++ b/internal/grpc/sidechannel/conn.go @@ -188,5 +188,5 @@ func (cc *ClientConn) CloseWrite() error { // also injects sr as a sidechannel registry, so that Gitaly can establish sidechannels back to the client. func Dial(ctx context.Context, registry *Registry, logger log.Logger, rawAddress string, connOpts []grpc.DialOption) (*grpc.ClientConn, error) { clientHandshaker := NewClientHandshaker(logger, registry) - return client.Dial(ctx, rawAddress, client.WithGrpcOptions(connOpts), client.WithHandshaker(clientHandshaker)) + return client.New(ctx, rawAddress, client.WithGrpcOptions(connOpts), client.WithHandshaker(clientHandshaker)) } diff --git a/internal/praefect/coordinator_test.go b/internal/praefect/coordinator_test.go index 88ab9dbad5e61fc54fc51dec32c2d058b1489e24..bc85d9aa9ec4c173e8ee1cef75864be92bd33519 100644 --- a/internal/praefect/coordinator_test.go +++ b/internal/praefect/coordinator_test.go @@ -1828,6 +1828,8 @@ func TestAbsentCorrelationID(t *testing.T) { } func TestCoordinatorEnqueueFailure(t *testing.T) { + testhelper.SkipWithMacOS(t, "this test is extremely flaky on macos platform") + t.Parallel() conf := config.Config{ VirtualStorages: []*config.VirtualStorage{ @@ -2180,7 +2182,7 @@ func TestCoordinator_grpcErrorHandling(t *testing.T) { gitalypb.RegisterOperationServiceServer(srv, operationServer) }, testserver.WithDiskCache(&mockDiskCache{}), testserver.WithDisablePraefect()) - conn, err := client.Dial(ctx, addr, client.WithGrpcOptions([]grpc.DialOption{ + conn, err := client.New(ctx, addr, client.WithGrpcOptions([]grpc.DialOption{ grpc.WithDefaultCallOptions(grpc.ForceCodec(proxy.NewCodec())), })) require.NoError(t, err) diff --git a/internal/praefect/get_object_pool_test.go b/internal/praefect/get_object_pool_test.go index 989f96e0cba2c5d325c83cbe78b32392b20a978c..4292730675f3db166d201aedc6a8e891f7af0a1b 100644 --- a/internal/praefect/get_object_pool_test.go +++ b/internal/praefect/get_object_pool_test.go @@ -96,7 +96,7 @@ func TestGetObjectPoolHandler(t *testing.T) { go testhelper.MustServe(t, srv, ln) - clientConn, err := grpc.DialContext(ctx, "unix:"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + clientConn, err := grpc.NewClient("unix:"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) t.Cleanup(func() { testhelper.MustClose(t, clientConn) diff --git a/internal/praefect/nodes/manager.go b/internal/praefect/nodes/manager.go index 9c7e9086371d73d97506202e6664e458812bc90f..58c5f1365c070598d7e43ea9643d40d115864cd1 100644 --- a/internal/praefect/nodes/manager.go +++ b/internal/praefect/nodes/manager.go @@ -160,7 +160,7 @@ func Dial( client.StreamInterceptor(), } - return client.Dial(ctx, node.Address, client.WithGrpcOptions(dialOpts), client.WithHandshaker(handshaker)) + return client.New(ctx, node.Address, client.WithGrpcOptions(dialOpts), client.WithHandshaker(handshaker)) } // NewManager creates a new NodeMgr based on virtual storage configs diff --git a/internal/praefect/nodes/manager_test.go b/internal/praefect/nodes/manager_test.go index 0d29c752c89030f131da11866583aefabe9fe175..eb42be71ef3beab6ed8961bfdaa5d7b1fa4c8077 100644 --- a/internal/praefect/nodes/manager_test.go +++ b/internal/praefect/nodes/manager_test.go @@ -472,7 +472,7 @@ func TestNodeStatus_IsHealthy(t *testing.T) { healthSrv := testhelper.NewServerWithHealth(t, socket) - clientConn, err := client.Dial(ctx, address) + clientConn, err := client.New(ctx, address) require.NoError(t, err) defer func() { require.NoError(t, clientConn.Close()) }() diff --git a/internal/praefect/nodes/ping.go b/internal/praefect/nodes/ping.go index e6e56180f05309f53c550f5429806799b6153b78..5a04ca58ab04804bd3400f65b6f28d18d3bbc129 100644 --- a/internal/praefect/nodes/ping.go +++ b/internal/praefect/nodes/ping.go @@ -71,7 +71,6 @@ func (p *Ping) Address() string { func (p *Ping) dial(ctx context.Context) (*grpc.ClientConn, error) { opts := []grpc.DialOption{ - grpc.WithBlock(), client.UnaryInterceptor(), client.StreamInterceptor(), } @@ -80,7 +79,7 @@ func (p *Ping) dial(ctx context.Context) (*grpc.ClientConn, error) { opts = append(opts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(p.token))) } - return client.Dial(ctx, p.address, client.WithGrpcOptions(opts)) + return client.New(ctx, p.address, client.WithGrpcOptions(opts)) } func (p *Ping) healthCheck(ctx context.Context, cc *grpc.ClientConn) (grpc_health_v1.HealthCheckResponse_ServingStatus, error) { diff --git a/internal/praefect/remove_repository_test.go b/internal/praefect/remove_repository_test.go index 50792ddbc522989e330511cc1fd9f1ba463375c5..d89692f109da8ea4daa5fb23757953bdfd318e81 100644 --- a/internal/praefect/remove_repository_test.go +++ b/internal/praefect/remove_repository_test.go @@ -67,7 +67,7 @@ func TestRemoveRepositoryHandler(t *testing.T) { gitalyOneRepoPath := filepath.Join(gitalyOneCfg.Storages[0].Path, relativePath) gitalyOneAddr := testserver.RunGitalyServer(t, gitalyOneCfg, setup.RegisterAll, testserver.WithDisablePraefect()) - gitalyOneConn, err := client.Dial(ctx, gitalyOneAddr) + gitalyOneConn, err := client.New(ctx, gitalyOneAddr) require.NoError(t, err) defer testhelper.MustClose(t, gitalyOneConn) @@ -79,7 +79,7 @@ func TestRemoveRepositoryHandler(t *testing.T) { gitalyTwoRepoPath := filepath.Join(gitalyTwoCfg.Storages[0].Path, relativePath) gitalyTwoAddr := testserver.RunGitalyServer(t, gitalyTwoCfg, setup.RegisterAll, testserver.WithDisablePraefect()) - gitalyTwoConn, err := client.Dial(ctx, gitalyTwoAddr) + gitalyTwoConn, err := client.New(ctx, gitalyTwoAddr) require.NoError(t, err) defer testhelper.MustClose(t, gitalyTwoConn) @@ -131,7 +131,7 @@ func TestRemoveRepositoryHandler(t *testing.T) { go testhelper.MustServe(t, srv, ln) - clientConn, err := grpc.DialContext(ctx, "unix:"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + clientConn, err := grpc.NewClient("unix:"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer clientConn.Close() diff --git a/internal/praefect/replicator_pg_test.go b/internal/praefect/replicator_pg_test.go index 02f59bf6ab981790dd5ec80a21e11e57d8150612..eb99aed8062ef7539ece82fdd4d9bf07f82d7b5e 100644 --- a/internal/praefect/replicator_pg_test.go +++ b/internal/praefect/replicator_pg_test.go @@ -36,7 +36,7 @@ func TestReplicatorInvalidSourceRepository(t *testing.T) { defer srv.Stop() go testhelper.MustServe(t, srv, ln) - targetCC, err := client.Dial(ctx, ln.Addr().Network()+":"+ln.Addr().String()) + targetCC, err := client.New(ctx, ln.Addr().Network()+":"+ln.Addr().String()) require.NoError(t, err) defer testhelper.MustClose(t, targetCC) diff --git a/internal/praefect/repository_exists_test.go b/internal/praefect/repository_exists_test.go index 6c4ed1f92c835f517c019ba13df351557a7098e1..ca36e542469e9ff659d3e2d8e76d877e6124fce6 100644 --- a/internal/praefect/repository_exists_test.go +++ b/internal/praefect/repository_exists_test.go @@ -98,7 +98,7 @@ func TestRepositoryExistsHandler(t *testing.T) { go testhelper.MustServe(t, srv, ln) - clientConn, err := grpc.DialContext(ctx, "unix://"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + clientConn, err := grpc.NewClient("unix://"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer testhelper.MustClose(t, clientConn) diff --git a/internal/praefect/server_factory_test.go b/internal/praefect/server_factory_test.go index 5910de7c7aa8727c0afe61f80bc9e0503ace7159..992f82751b2d6f187fc6db1661ec562864963ed1 100644 --- a/internal/praefect/server_factory_test.go +++ b/internal/praefect/server_factory_test.go @@ -208,7 +208,7 @@ func TestServerFactory(t *testing.T) { creds := insecure.NewCredentials() - cc, err := client.Dial(ctx, praefectAddr) + cc, err := client.New(ctx, praefectAddr) require.NoError(t, err) defer func() { require.NoError(t, cc.Close()) }() ctx := testhelper.Context(t) @@ -237,7 +237,7 @@ func TestServerFactory(t *testing.T) { creds := certificate.TransportCredentials(t) - cc, err := grpc.DialContext(ctx, listener.Addr().String(), grpc.WithTransportCredentials(creds)) + cc, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(creds)) require.NoError(t, err) defer func() { require.NoError(t, cc.Close()) }() @@ -291,7 +291,7 @@ func TestServerFactory(t *testing.T) { address, err := starter.ComposeEndpoint(listener.Addr().Network(), listener.Addr().String()) require.NoError(t, err) - conn, err := client.Dial(ctx, address, cfg.dialOpts...) + conn, err := client.New(ctx, address, cfg.dialOpts...) require.NoError(t, err) defer func() { require.NoError(t, conn.Close()) }() diff --git a/internal/praefect/server_test.go b/internal/praefect/server_test.go index 70916f0dc5a41e33110f38f0c7f31d8efd27f843..6de18fd257dcd7a69463ee56c131feb1d8bf1bd6 100644 --- a/internal/praefect/server_test.go +++ b/internal/praefect/server_test.go @@ -936,6 +936,7 @@ func TestProxyWrites(t *testing.T) { } func TestErrorThreshold(t *testing.T) { + testhelper.SkipWithMacOS(t, "this test is extremely flaky on macos platform") t.Parallel() backendToken := "" backend, cleanup := newMockDownstream(t, backendToken, func(srv *grpc.Server) { diff --git a/internal/praefect/testserver.go b/internal/praefect/testserver.go index 15bc462246153713d689f063e62fc8245f0964b2..a7b33afba94e4993f9d06e3cecd03deffcc7aab1 100644 --- a/internal/praefect/testserver.go +++ b/internal/praefect/testserver.go @@ -113,7 +113,6 @@ func listenAvailPort(tb testing.TB) (net.Listener, int) { func dialLocalPort(tb testing.TB, ctx context.Context, port int, backend bool) *grpc.ClientConn { opts := []grpc.DialOption{ - grpc.WithBlock(), grpc.WithUnaryInterceptor(correlation.UnaryClientCorrelationInterceptor()), grpc.WithStreamInterceptor(correlation.StreamClientCorrelationInterceptor()), } @@ -124,7 +123,7 @@ func dialLocalPort(tb testing.TB, ctx context.Context, port int, backend bool) * ) } - cc, err := client.Dial( + cc, err := client.New( ctx, fmt.Sprintf("tcp://localhost:%d", port), client.WithGrpcOptions(opts), diff --git a/internal/praefect/walkrepos_test.go b/internal/praefect/walkrepos_test.go index 63301caf587fe0dce0b8d82a80a030e3a2060420..7b025838673ed82afcc8200ecb010e1e86a39e15 100644 --- a/internal/praefect/walkrepos_test.go +++ b/internal/praefect/walkrepos_test.go @@ -65,7 +65,7 @@ func TestWalkReposHandler(t *testing.T) { go testhelper.MustServe(t, srv, ln) - clientConn, err := grpc.DialContext(ctx, "unix://"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + clientConn, err := grpc.NewClient("unix://"+ln.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) require.NoError(t, err) defer testhelper.MustClose(t, clientConn) diff --git a/internal/testhelper/testserver/gitaly.go b/internal/testhelper/testserver/gitaly.go index 88054eb6022f7f90d316a96a8ed9270cb1f91f63..3e1dc546ded20555a4338af6fe9c88ca150b6b10 100644 --- a/internal/testhelper/testserver/gitaly.go +++ b/internal/testhelper/testserver/gitaly.go @@ -145,7 +145,6 @@ func (gs GitalyServer) Address() string { // waitHealthy waits until the server hosted at address becomes healthy. func waitHealthy(tb testing.TB, ctx context.Context, addr string, authToken string) { grpcOpts := []grpc.DialOption{ - grpc.WithBlock(), client.UnaryInterceptor(), client.StreamInterceptor(), } @@ -153,7 +152,7 @@ func waitHealthy(tb testing.TB, ctx context.Context, addr string, authToken stri grpcOpts = append(grpcOpts, grpc.WithPerRPCCredentials(gitalyauth.RPCCredentialsV2(authToken))) } - conn, err := client.Dial(ctx, addr, client.WithGrpcOptions(grpcOpts)) + conn, err := client.New(ctx, addr, client.WithGrpcOptions(grpcOpts)) require.NoError(tb, err) defer testhelper.MustClose(tb, conn) diff --git a/tools/test-boot/main.go b/tools/test-boot/main.go index a9739b00b7e7250138610af27b086747ea5b7ccc..9d4dffeb9dcf1d2423759e3aab0835229bd2e7b1 100644 --- a/tools/test-boot/main.go +++ b/tools/test-boot/main.go @@ -15,7 +15,6 @@ import ( "github.com/urfave/cli/v2" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/client" - "google.golang.org/grpc" ) type gitalyConfig struct { @@ -112,9 +111,7 @@ func spawnAndWait(ctx context.Context, gitalyBin, configPath, socketPath string) for i := 0; i < 300; i++ { ctx, cancel := context.WithTimeout(ctx, 100*time.Millisecond) - conn, err := client.Dial(ctx, "unix://"+socketPath, client.WithGrpcOptions([]grpc.DialOption{ - grpc.WithBlock(), - })) + conn, err := client.New(ctx, "unix://"+socketPath) cancel()