Skip to content

Commit

Permalink
cleanup: replace Dial with NewClient (grpc#7975)
Browse files Browse the repository at this point in the history
  • Loading branch information
@janardhanvissa
janardhanvissa authored Feb 17, 2025
1 parent ae2a04f commit 8528f43
Show file tree
Hide file tree
Showing 15 changed files with 108 additions and 82 deletions.
2 changes: 1 addition & 1 deletion clientconn_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1041,7 +1041,7 @@ func (s) TestUpdateAddresses_NoopIfCalledWithSameAddresses(t *testing.T) {
rb := manual.NewBuilderWithScheme("whatever")
rb.InitialState(resolver.State{Addresses: addrsList})

client, err := Dial("whatever:///this-gets-overwritten",
client, err := NewClient("whatever:///this-gets-overwritten",
WithTransportCredentials(insecure.NewCredentials()),
WithResolvers(rb),
WithConnectParams(ConnectParams{
Expand Down
16 changes: 7 additions & 9 deletions security/advancedtls/advancedtls_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ func (greeterServer) SayHello(_ context.Context, in *pb.HelloRequest) (*pb.Hello

// TODO(ZhenLian): remove shouldFail to the function signature to provider
// tests.
func callAndVerify(msg string, client pb.GreeterClient, shouldFail bool) error {
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
defer cancel()
func callAndVerify(ctx context.Context, msg string, client pb.GreeterClient, shouldFail bool) error {
_, err := client.SayHello(ctx, &pb.HelloRequest{Name: msg})
if want, got := shouldFail == true, err != nil; got != want {
return fmt.Errorf("want and got mismatch, want shouldFail=%v, got fail=%v, rpc error: %v", want, got, err)
Expand All @@ -98,24 +96,24 @@ func callAndVerify(msg string, client pb.GreeterClient, shouldFail bool) error {

// TODO(ZhenLian): remove shouldFail and add ...DialOption to the function
// signature to provider cleaner tests.
func callAndVerifyWithClientConn(connCtx context.Context, address string, msg string, creds credentials.TransportCredentials, shouldFail bool) (*grpc.ClientConn, pb.GreeterClient, error) {
func callAndVerifyWithClientConn(ctx context.Context, address string, msg string, creds credentials.TransportCredentials, shouldFail bool) (*grpc.ClientConn, pb.GreeterClient, error) {
var conn *grpc.ClientConn
var err error
// If we want the test to fail, we establish a non-blocking connection to
// avoid it hangs and killed by the context.
if shouldFail {
conn, err = grpc.DialContext(connCtx, address, grpc.WithTransportCredentials(creds))
conn, err = grpc.NewClient(address, grpc.WithTransportCredentials(creds))
if err != nil {
return nil, nil, fmt.Errorf("client failed to connect to %s. Error: %v", address, err)
}
} else {
conn, err = grpc.DialContext(connCtx, address, grpc.WithTransportCredentials(creds), grpc.WithBlock())
conn, err = grpc.NewClient(address, grpc.WithTransportCredentials(creds), grpc.WithBlock())
if err != nil {
return nil, nil, fmt.Errorf("client failed to connect to %s. Error: %v", address, err)
}
}
greetClient := pb.NewGreeterClient(conn)
err = callAndVerify(msg, greetClient, shouldFail)
err = callAndVerify(ctx, msg, greetClient, shouldFail)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -392,7 +390,7 @@ func (s) TestEnd2End(t *testing.T) {
stage.increase()
// ------------------------Scenario 2------------------------------------
// stage = 1, previous connection should still succeed
err = callAndVerify("rpc call 2", greetClient, false)
err = callAndVerify(ctx, "rpc call 2", greetClient, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -684,7 +682,7 @@ func (s) TestPEMFileProviderEnd2End(t *testing.T) {
test.certUpdateFunc()
time.Sleep(sleepInterval)
// The already-established connection should not be affected.
err = callAndVerify("rpc call 2", greetClient, false)
err = callAndVerify(ctx, "rpc call 2", greetClient, false)
if err != nil {
t.Fatal(err)
}
Expand Down
2 changes: 1 addition & 1 deletion server_ext_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func (s) TestServer_MaxHandlers(t *testing.T) {
func (s) TestStreamWorkers_RPCsAndStop(t *testing.T) {
ss := stubserver.StartTestService(t, nil, grpc.NumStreamWorkers(uint32(runtime.NumCPU())))
// This deferred stop takes care of stopping the server when one of the
// below grpc.Dials fail, and the test exits early.
// below grpc.NewClient fail, and the test exits early.
defer ss.Stop()

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
Expand Down
57 changes: 31 additions & 26 deletions test/balancer_switching_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import (
"google.golang.org/grpc"
"google.golang.org/grpc/balancer"
grpclbstate "google.golang.org/grpc/balancer/grpclb/state"
pickfirst "google.golang.org/grpc/balancer/pickfirst"
"google.golang.org/grpc/balancer/pickfirst"
"google.golang.org/grpc/credentials/insecure"
"google.golang.org/grpc/internal"
"google.golang.org/grpc/internal/balancer/stub"
Expand All @@ -47,6 +47,7 @@ const (
wantGRPCLBTraceDesc = `Channel switches to new LB policy "grpclb"`
wantRoundRobinTraceDesc = `Channel switches to new LB policy "round_robin"`
pickFirstServiceConfig = `{"loadBalancingConfig": [{"pick_first":{}}]}`
grpclbServiceConfig = `{"loadBalancingConfig": [{"grpclb":{}}]}`

// This is the number of stub backends set up at the start of each test. The
// first backend is used for the "grpclb" policy and the rest are used for
Expand Down Expand Up @@ -174,17 +175,18 @@ func (s) TestBalancerSwitch_grpclbToPickFirst(t *testing.T) {
addrs := stubBackendsToResolverAddrs(backends)
r := manual.NewBuilderWithScheme("whatever")
target := fmt.Sprintf("%s:///%s", r.Scheme(), loadBalancedServiceName)
cc, err := grpc.Dial(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()
cc.Connect()

// Push a resolver update with a GRPCLB service config and a single address
// pointing to the grpclb server we created above. This will cause the
// channel to switch to the "grpclb" balancer, which returns a single
// backend address.
grpclbConfig := parseServiceConfig(t, r, `{"loadBalancingPolicy": "grpclb"}`)
grpclbConfig := parseServiceConfig(t, r, grpclbServiceConfig)
state := resolver.State{ServiceConfig: grpclbConfig}
r.UpdateState(grpclbstate.Set(state, &grpclbstate.State{BalancerAddresses: []resolver.Address{{Addr: lbServer.Address()}}}))
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
Expand Down Expand Up @@ -223,15 +225,16 @@ func (s) TestBalancerSwitch_pickFirstToGRPCLB(t *testing.T) {
addrs := stubBackendsToResolverAddrs(backends)
r := manual.NewBuilderWithScheme("whatever")
target := fmt.Sprintf("%s:///%s", r.Scheme(), loadBalancedServiceName)
cc, err := grpc.Dial(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()

// Push a resolver update containing no grpclb server address. This should
// lead to the channel using the default LB policy which is pick_first.
r.UpdateState(resolver.State{Addresses: addrs[1:]})
// Set an empty initial resolver state. This should lead to the channel
// using the default LB policy which is pick_first.
r.InitialState(resolver.State{Addresses: addrs[1:]})

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
if err := pfutil.CheckRPCsToBackend(ctx, cc, addrs[1]); err != nil {
Expand All @@ -241,7 +244,7 @@ func (s) TestBalancerSwitch_pickFirstToGRPCLB(t *testing.T) {
// Push a resolver update with no service config and a single address pointing
// to the grpclb server we created above. This will cause the channel to
// switch to the "grpclb" balancer, which returns a single backend address.
grpclbConfig := parseServiceConfig(t, r, `{"loadBalancingPolicy": "grpclb"}`)
grpclbConfig := parseServiceConfig(t, r, grpclbServiceConfig)
state := resolver.State{ServiceConfig: grpclbConfig}
r.UpdateState(grpclbstate.Set(state, &grpclbstate.State{BalancerAddresses: []resolver.Address{{Addr: lbServer.Address()}}}))
client := testgrpc.NewTestServiceClient(cc)
Expand Down Expand Up @@ -276,12 +279,12 @@ func (s) TestBalancerSwitch_RoundRobinToGRPCLB(t *testing.T) {
addrs := stubBackendsToResolverAddrs(backends)
r := manual.NewBuilderWithScheme("whatever")
target := fmt.Sprintf("%s:///%s", r.Scheme(), loadBalancedServiceName)
cc, err := grpc.Dial(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(target, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()

cc.Connect()
// Note the use of the deprecated `loadBalancingPolicy` field here instead
// of the now recommended `loadBalancingConfig` field. The logic in the
// ClientConn which decides which balancer to switch to looks at the
Expand All @@ -292,7 +295,7 @@ func (s) TestBalancerSwitch_RoundRobinToGRPCLB(t *testing.T) {
// If we use the `loadBalancingPolicy` field, the switch to "grpclb" later on
// in the test will not happen as the ClientConn will continue to use the LB
// policy received in the first update.
scpr := parseServiceConfig(t, r, `{"loadBalancingPolicy": "round_robin"}`)
scpr := parseServiceConfig(t, r, rrServiceConfig)

// Push a resolver update with the service config specifying "round_robin".
r.UpdateState(resolver.State{Addresses: addrs[1:], ServiceConfig: scpr})
Expand All @@ -307,7 +310,7 @@ func (s) TestBalancerSwitch_RoundRobinToGRPCLB(t *testing.T) {
// pointing to the grpclb server we created above. This will cause the
// channel to switch to the "grpclb" balancer, which returns a single
// backend address.
grpclbConfig := parseServiceConfig(t, r, `{"loadBalancingPolicy": "grpclb"}`)
grpclbConfig := parseServiceConfig(t, r, grpclbServiceConfig)
state := resolver.State{ServiceConfig: grpclbConfig}
r.UpdateState(grpclbstate.Set(state, &grpclbstate.State{BalancerAddresses: []resolver.Address{{Addr: lbServer.Address()}}}))
if err := rrutil.CheckRoundRobinRPCs(ctx, client, addrs[:1]); err != nil {
Expand Down Expand Up @@ -336,11 +339,12 @@ func (s) TestBalancerSwitch_grpclbNotRegistered(t *testing.T) {
addrs := stubBackendsToResolverAddrs(backends)

r := manual.NewBuilderWithScheme("whatever")
cc, err := grpc.Dial(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()
cc.Connect()

// Push a resolver update which contains a bunch of stub server backends and a
// grpclb server address. The latter should get the ClientConn to try and
Expand All @@ -355,7 +359,7 @@ func (s) TestBalancerSwitch_grpclbNotRegistered(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
if err := pfutil.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil {
t.Fatal(err)
t.Fatalf("Pick_first backend readiness check failed: %v", err)
}

// Push a resolver update with the same addresses, but with a service config
Expand All @@ -367,7 +371,7 @@ func (s) TestBalancerSwitch_grpclbNotRegistered(t *testing.T) {
})
client := testgrpc.NewTestServiceClient(cc)
if err := rrutil.CheckRoundRobinRPCs(ctx, client, addrs); err != nil {
t.Fatal(err)
t.Fatalf("Round robin RPCs failed: %v", err)
}
}

Expand All @@ -394,10 +398,11 @@ func (s) TestBalancerSwitch_OldBalancerCallsShutdownInClose(t *testing.T) {
})

r := manual.NewBuilderWithScheme("whatever")
cc, err := grpc.Dial(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
cc.Connect()
defer cc.Close()

// Push a resolver update specifying our stub balancer as the LB policy.
Expand Down Expand Up @@ -453,12 +458,12 @@ func (s) TestBalancerSwitch_Graceful(t *testing.T) {
addrs := stubBackendsToResolverAddrs(backends)

r := manual.NewBuilderWithScheme("whatever")
cc, err := grpc.Dial(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(r.Scheme()+":///test.server", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()

cc.Connect()
// Push a resolver update with the service config specifying "round_robin".
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
Expand Down Expand Up @@ -513,14 +518,14 @@ func (s) TestBalancerSwitch_Graceful(t *testing.T) {
case <-ccUpdateCh:
}
if err := rrutil.CheckRoundRobinRPCs(ctx, client, addrs[1:]); err != nil {
t.Fatal(err)
t.Fatalf("RPCs routed to old balancer failed: %v", err)
}

// Ask our stub balancer to forward the earlier received ccUpdate to the
// underlying "pick_first" balancer which will result in a healthy picker
// being reported to the channel. RPCs should start using the new balancer.
close(waitToProceed)
if err := pfutil.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil {
t.Fatal(err)
t.Fatalf("RPCs routed to new balancer failed: %v", err)
}
}
5 changes: 3 additions & 2 deletions test/balancer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ func (s) TestServersSwap(t *testing.T) {
// Initialize client
r := manual.NewBuilderWithScheme("whatever")
r.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: addr1}}})
cc, err := grpc.DialContext(ctx, r.Scheme()+":///", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("Error creating client: %v", err)
}
Expand Down Expand Up @@ -622,11 +622,12 @@ func (s) TestWaitForReady(t *testing.T) {
// Initialize client
r := manual.NewBuilderWithScheme("whatever")

cc, err := grpc.DialContext(ctx, r.Scheme()+":///", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
if err != nil {
t.Fatalf("Error creating client: %v", err)
}
defer cc.Close()
cc.Connect()
client := testgrpc.NewTestServiceClient(cc)

// Report an error so non-WFR RPCs will give up early.
Expand Down
5 changes: 4 additions & 1 deletion test/clientconn_state_transition_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func testStateTransitionSingleAddress(t *testing.T, want []connectivity.State, s
connMu.Unlock()
}()

client, err := grpc.Dial("",
client, err := grpc.NewClient("passthrough:///",
grpc.WithTransportCredentials(insecure.NewCredentials()),
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, stateRecordingBalancerName)),
grpc.WithDialer(pl.Dialer()),
Expand All @@ -181,6 +181,9 @@ func testStateTransitionSingleAddress(t *testing.T, want []connectivity.State, s
defer cancel()
go testutils.StayConnected(ctx, client)

// Wait for the test balancer to be built before capturing it's state
// notification channel.
testutils.AwaitNotState(ctx, t, client, connectivity.Idle)
stateNotifications := testBalancerBuilder.nextStateNotifier()
for i := 0; i < len(want); i++ {
select {
Expand Down
32 changes: 15 additions & 17 deletions test/end2end_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -840,9 +840,9 @@ func (te *test) clientConnWithConnControl() (*grpc.ClientConn, *dialerWrapper) {
// overwrite the dialer before
opts = append(opts, grpc.WithDialer(dw.dialer))
var err error
te.cc, err = grpc.Dial(scheme+te.srvAddr, opts...)
te.cc, err = grpc.NewClient(scheme+te.srvAddr, opts...)
if err != nil {
te.t.Fatalf("Dial(%q) = %v", scheme+te.srvAddr, err)
te.t.Fatalf("NewClient(%q) = %v", scheme+te.srvAddr, err)
}
return te.cc, dw
}
Expand Down Expand Up @@ -3004,9 +3004,9 @@ func (s) TestTransparentRetry(t *testing.T) {
},
}
server.start(t, lis)
cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("failed to dial due to err: %v", err)
t.Fatalf("failed to create a client for the server: %v", err)
}
defer cc.Close()

Expand Down Expand Up @@ -4252,9 +4252,9 @@ func (s) TestFailfastRPCFailOnFatalHandshakeError(t *testing.T) {
}
defer lis.Close()

cc, err := grpc.Dial("passthrough:///"+lis.Addr().String(), grpc.WithTransportCredentials(&clientFailCreds{}))
cc, err := grpc.NewClient("passthrough:///"+lis.Addr().String(), grpc.WithTransportCredentials(&clientFailCreds{}))
if err != nil {
t.Fatalf("grpc.Dial(_) = %v", err)
t.Fatalf("grpc.NewClient(_) = %v", err)
}
defer cc.Close()

Expand Down Expand Up @@ -4298,9 +4298,9 @@ func (s) TestFlowControlLogicalRace(t *testing.T) {

go s.Serve(lis)

cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("grpc.Dial(%q) = %v", lis.Addr().String(), err)
t.Fatalf("grpc.NewClient(%q) = %v", lis.Addr().String(), err)
}
defer cc.Close()
cl := testgrpc.NewTestServiceClient(cc)
Expand Down Expand Up @@ -5070,9 +5070,9 @@ func (s) TestServeExitsWhenListenerClosed(t *testing.T) {
close(done)
}()

cc, err := grpc.Dial(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("Failed to dial server: %v", err)
t.Fatalf("Failed to create a client for server: %v", err)
}
defer cc.Close()
c := testgrpc.NewTestServiceClient(cc)
Expand Down Expand Up @@ -5244,11 +5244,9 @@ func (s) TestDisabledIOBuffers(t *testing.T) {
s.Serve(lis)
}()
defer s.Stop()
dctx, dcancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer dcancel()
cc, err := grpc.DialContext(dctx, lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithWriteBufferSize(0), grpc.WithReadBufferSize(0))
cc, err := grpc.NewClient(lis.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithWriteBufferSize(0), grpc.WithReadBufferSize(0))
if err != nil {
t.Fatalf("Failed to dial server")
t.Fatalf("Failed to create a client for server")
}
defer cc.Close()
c := testgrpc.NewTestServiceClient(cc)
Expand Down Expand Up @@ -5448,7 +5446,7 @@ func (s) TestNetPipeConn(t *testing.T) {
go s.Serve(pl)
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
cc, err := grpc.DialContext(ctx, "", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDialer(pl.Dialer()))
cc, err := grpc.NewClient("passthrough:///", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithDialer(pl.Dialer()))
if err != nil {
t.Fatalf("Error creating client: %v", err)
}
Expand Down Expand Up @@ -6426,9 +6424,9 @@ func (s) TestRPCBlockingOnPickerStatsCall(t *testing.T) {
ServiceConfig: sc,
})

cc, err := grpc.Dial(mr.Scheme()+":///", grpc.WithResolvers(mr), grpc.WithStatsHandler(sh), grpc.WithTransportCredentials(insecure.NewCredentials()))
cc, err := grpc.NewClient(mr.Scheme()+":///", grpc.WithResolvers(mr), grpc.WithStatsHandler(sh), grpc.WithTransportCredentials(insecure.NewCredentials()))
if err != nil {
t.Fatalf("grpc.Dial() failed: %v", err)
t.Fatalf("grpc.NewClient() failed: %v", err)
}
defer cc.Close()
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
Expand Down
Loading

0 comments on commit 8528f43

Please sign in to comment.