From cf12acebee5906184e16a7691b0dc142ffc73516 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 23 Dec 2024 06:13:24 +0000 Subject: [PATCH] Revert "cleanup: replace dial with newclient (#7920)" This reverts commit a21e192176002a217aaa1ff2a026ad3316159130. --- balancer/grpclb/grpclb_test.go | 39 ++++++++++++++++------------------ balancer/rls/picker_test.go | 12 +++++------ resolver_balancer_ext_test.go | 10 ++++----- test/creds_test.go | 29 ++++++++++++------------- test/end2end_test.go | 2 +- 5 files changed, 42 insertions(+), 50 deletions(-) diff --git a/balancer/grpclb/grpclb_test.go b/balancer/grpclb/grpclb_test.go index e234ace6576f..62dc947e0ee5 100644 --- a/balancer/grpclb/grpclb_test.go +++ b/balancer/grpclb/grpclb_test.go @@ -460,7 +460,7 @@ func (s) TestGRPCLB_Basic(t *testing.T) { } cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() @@ -517,7 +517,7 @@ func (s) TestGRPCLB_Weighted(t *testing.T) { } cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() @@ -597,7 +597,7 @@ func (s) TestGRPCLB_DropRequest(t *testing.T) { } cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() testC := testgrpc.NewTestServiceClient(cc) @@ -769,7 +769,7 @@ func (s) TestGRPCLB_BalancerDisconnects(t *testing.T) { } cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() testC := testgrpc.NewTestServiceClient(cc) @@ -940,7 +940,7 @@ func (s) TestGRPCLB_ExplicitFallback(t *testing.T) { } cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() testC := testgrpc.NewTestServiceClient(cc) @@ -1008,12 +1008,11 @@ func (s) TestGRPCLB_FallBackWithNoServerAddress(t *testing.T) { grpc.WithTransportCredentials(&serverNameCheckCreds{}), grpc.WithContextDialer(fakeNameDialer), } - cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) + cc, err := grpc.Dial(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() - cc.Connect() testC := testgrpc.NewTestServiceClient(cc) ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -1103,11 +1102,10 @@ func (s) TestGRPCLB_PickFirst(t *testing.T) { grpc.WithTransportCredentials(&serverNameCheckCreds{}), grpc.WithContextDialer(fakeNameDialer), } - cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) + cc, err := grpc.Dial(r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend: %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } - cc.Connect() defer cc.Close() // Push a service config with grpclb as the load balancing policy and @@ -1200,7 +1198,7 @@ func (s) TestGRPCLB_BackendConnectionErrorPropagation(t *testing.T) { grpc.WithTransportCredentials(&serverNameCheckCreds{}), grpc.WithContextDialer(fakeNameDialer)) if err != nil { - t.Fatalf("Failed to create a client for the backend: %v", err) + t.Fatalf("Failed to create new client to the backend %v", err) } defer cc.Close() testC := testgrpc.NewTestServiceClient(cc) @@ -1243,11 +1241,10 @@ func testGRPCLBEmptyServerList(t *testing.T, svcfg string) { grpc.WithTransportCredentials(&serverNameCheckCreds{}), grpc.WithContextDialer(fakeNameDialer), } - cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, dopts...) + cc, err := grpc.DialContext(ctx, r.Scheme()+":///"+beServerName, dopts...) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } - cc.Connect() defer cc.Close() testC := testgrpc.NewTestServiceClient(cc) @@ -1314,16 +1311,15 @@ func (s) TestGRPCLBWithTargetNameFieldInConfig(t *testing.T) { // Push the backend address to the remote balancer. tss.ls.sls <- sl - cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, + cc, err := grpc.Dial(r.Scheme()+":///"+beServerName, grpc.WithResolvers(r), grpc.WithTransportCredentials(&serverNameCheckCreds{}), grpc.WithContextDialer(fakeNameDialer), grpc.WithUserAgent(testUserAgent)) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } defer cc.Close() - cc.Connect() testC := testgrpc.NewTestServiceClient(cc) // Push a resolver update with grpclb configuration which does not contain the @@ -1422,14 +1418,15 @@ func runAndCheckStats(t *testing.T, drop bool, statsChan chan *lbpb.ClientStats, tss.ls.statsDura = 100 * time.Millisecond creds := serverNameCheckCreds{} - cc, err := grpc.NewClient(r.Scheme()+":///"+beServerName, grpc.WithResolvers(r), + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cc, err := grpc.DialContext(ctx, r.Scheme()+":///"+beServerName, grpc.WithResolvers(r), grpc.WithTransportCredentials(&creds), grpc.WithPerRPCCredentials(failPreRPCCred{}), grpc.WithContextDialer(fakeNameDialer)) if err != nil { - t.Fatalf("Failed to create a client for the backend %v", err) + t.Fatalf("Failed to dial to the backend %v", err) } - cc.Connect() defer cc.Close() rstate := resolver.State{ServiceConfig: r.CC.ParseServiceConfig(grpclbConfig)} diff --git a/balancer/rls/picker_test.go b/balancer/rls/picker_test.go index a0bdbc827921..cb08c433f9e8 100644 --- a/balancer/rls/picker_test.go +++ b/balancer/rls/picker_test.go @@ -267,9 +267,9 @@ func (s) Test_RLSDefaultTargetPicksMetric(t *testing.T) { r := startManualResolverWithConfig(t, rlsConfig) tmr := stats.NewTestMetricsRecorder() - cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) + cc, err := grpc.Dial(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) if err != nil { - t.Fatalf("grpc.NewClient() failed: %v", err) + t.Fatalf("grpc.Dial() failed: %v", err) } defer cc.Close() @@ -314,9 +314,9 @@ func (s) Test_RLSTargetPicksMetric(t *testing.T) { tmr := stats.NewTestMetricsRecorder() // Dial the backend. - cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) + cc, err := grpc.Dial(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) if err != nil { - t.Fatalf("grpc.NewClient() failed: %v", err) + t.Fatalf("grpc.Dial() failed: %v", err) } defer cc.Close() @@ -352,9 +352,9 @@ func (s) Test_RLSFailedPicksMetric(t *testing.T) { tmr := stats.NewTestMetricsRecorder() // Dial the backend. - cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) + cc, err := grpc.Dial(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr)) if err != nil { - t.Fatalf("grpc.NewClient() failed: %v", err) + t.Fatalf("grpc.Dial() failed: %v", err) } defer cc.Close() diff --git a/resolver_balancer_ext_test.go b/resolver_balancer_ext_test.go index 75fac51a66db..417a5781ef80 100644 --- a/resolver_balancer_ext_test.go +++ b/resolver_balancer_ext_test.go @@ -66,12 +66,11 @@ func (s) TestResolverBalancerInteraction(t *testing.T) { rb.ResolveNowCallback = func(resolver.ResolveNowOptions) { close(rnCh) } resolver.Register(rb) - cc, err := grpc.NewClient(name+":///", grpc.WithTransportCredentials(insecure.NewCredentials())) + cc, err := grpc.Dial(name+":///", grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - t.Fatalf("grpc.NewClient error: %v", err) + t.Fatalf("grpc.Dial error: %v", err) } defer cc.Close() - cc.Connect() select { case <-rnCh: case <-time.After(defaultTestTimeout): @@ -110,12 +109,11 @@ func (s) TestResolverBuildFailure(t *testing.T) { resolver.Register(&resolverBuilderWithErr{errCh: resErrCh, scheme: name}) resErrCh <- nil - cc, err := grpc.NewClient(name+":///", grpc.WithTransportCredentials(insecure.NewCredentials())) + cc, err := grpc.Dial(name+":///", grpc.WithTransportCredentials(insecure.NewCredentials())) if err != nil { - t.Fatalf("grpc.NewClient error: %v", err) + t.Fatalf("grpc.Dial error: %v", err) } defer cc.Close() - cc.Connect() enterIdle(cc) const errStr = "test error from resolver builder" t.Log("pushing res err") diff --git a/test/creds_test.go b/test/creds_test.go index fe8b552e719c..74ee24c02965 100644 --- a/test/creds_test.go +++ b/test/creds_test.go @@ -236,9 +236,9 @@ func (s) TestFailFastRPCErrorOnBadCertificates(t *testing.T) { opts := []grpc.DialOption{grpc.WithTransportCredentials(clientAlwaysFailCred{})} ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() - cc, err := grpc.NewClient(te.srvAddr, opts...) + cc, err := grpc.DialContext(ctx, te.srvAddr, opts...) if err != nil { - t.Fatalf("NewClient(_) = %v, want %v", err, nil) + t.Fatalf("Dial(_) = %v, want %v", err, nil) } defer cc.Close() @@ -255,9 +255,11 @@ func (s) TestWaitForReadyRPCErrorOnBadCertificates(t *testing.T) { defer te.tearDown() opts := []grpc.DialOption{grpc.WithTransportCredentials(clientAlwaysFailCred{})} - cc, err := grpc.NewClient(te.srvAddr, opts...) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + cc, err := grpc.DialContext(ctx, te.srvAddr, opts...) if err != nil { - t.Fatalf("NewClient(_) = %v, want %v", err, nil) + t.Fatalf("Dial(_) = %v, want %v", err, nil) } defer cc.Close() @@ -270,9 +272,7 @@ func (s) TestWaitForReadyRPCErrorOnBadCertificates(t *testing.T) { testutils.AwaitState(ctx, t, cc, connectivity.TransientFailure) tc := testgrpc.NewTestServiceClient(cc) - // Use a short context as WaitForReady waits for context expiration before - // failing the RPC. - ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestShortTimeout) defer cancel() if _, err = tc.EmptyCall(ctx, &testpb.Empty{}, grpc.WaitForReady(true)); !strings.Contains(err.Error(), clientAlwaysFailCredErrorMsg) { t.Fatalf("TestService/EmptyCall(_, _) = _, %v, want err.Error() contains %q", err, clientAlwaysFailCredErrorMsg) @@ -438,12 +438,11 @@ func (s) TestCredsHandshakeAuthority(t *testing.T) { r := manual.NewBuilderWithScheme("whatever") - cc, err := grpc.NewClient(r.Scheme()+":///"+testAuthority, grpc.WithTransportCredentials(cred), grpc.WithResolvers(r)) + cc, err := grpc.Dial(r.Scheme()+":///"+testAuthority, grpc.WithTransportCredentials(cred), grpc.WithResolvers(r)) if err != nil { - t.Fatalf("grpc.NewClient(%q) = %v", lis.Addr().String(), err) + t.Fatalf("grpc.Dial(%q) = %v", lis.Addr().String(), err) } defer cc.Close() - cc.Connect() r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: lis.Addr().String()}}}) ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -471,12 +470,11 @@ func (s) TestCredsHandshakeServerNameAuthority(t *testing.T) { r := manual.NewBuilderWithScheme("whatever") - cc, err := grpc.NewClient(r.Scheme()+":///"+testAuthority, grpc.WithTransportCredentials(cred), grpc.WithResolvers(r)) + cc, err := grpc.Dial(r.Scheme()+":///"+testAuthority, grpc.WithTransportCredentials(cred), grpc.WithResolvers(r)) if err != nil { - t.Fatalf("grpc.NewClient(%q) = %v", lis.Addr().String(), err) + t.Fatalf("grpc.Dial(%q) = %v", lis.Addr().String(), err) } defer cc.Close() - cc.Connect() r.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: lis.Addr().String(), ServerName: testServerName}}}) ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -527,12 +525,11 @@ func (s) TestServerCredsDispatch(t *testing.T) { go s.Serve(lis) defer s.Stop() - cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(cred)) + cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(cred)) if err != nil { - t.Fatalf("grpc.NewClient(%q) = %v", lis.Addr().String(), err) + t.Fatalf("grpc.Dial(%q) = %v", lis.Addr().String(), err) } defer cc.Close() - cc.Connect() rawConn := cred.getRawConn() // Give grpc a chance to see the error and potentially close the connection. diff --git a/test/end2end_test.go b/test/end2end_test.go index 19972ecebe17..2238f8f12422 100644 --- a/test/end2end_test.go +++ b/test/end2end_test.go @@ -609,7 +609,7 @@ func (te *test) listenAndServe(ts testgrpc.TestServiceServer, listen func(networ if te.serverInitialConnWindowSize > 0 { sopts = append(sopts, grpc.InitialConnWindowSize(te.serverInitialConnWindowSize)) } - la := ":0" + la := "localhost:0" switch te.e.network { case "unix": la = "/tmp/testsock" + fmt.Sprintf("%d", time.Now().UnixNano())