diff --git a/clientconn_test.go b/clientconn_test.go index e614045bee01..eecea1b6d2c0 100644 --- a/clientconn_test.go +++ b/clientconn_test.go @@ -1058,9 +1058,8 @@ func (s) TestUpdateAddresses_NoopIfCalledWithSameAddresses(t *testing.T) { } func (s) TestDefaultServiceConfig(t *testing.T) { - r := manual.NewBuilderWithScheme("whatever") - addr := r.Scheme() + ":///non.existent" - js := `{ + const defaultSC = ` +{ "methodConfig": [ { "name": [ @@ -1073,10 +1072,40 @@ func (s) TestDefaultServiceConfig(t *testing.T) { } ] }` - testInvalidDefaultServiceConfig(t) - testDefaultServiceConfigWhenResolverServiceConfigDisabled(t, r, addr, js) - testDefaultServiceConfigWhenResolverDoesNotReturnServiceConfig(t, r, addr, js) - testDefaultServiceConfigWhenResolverReturnInvalidServiceConfig(t, r, addr, js) + tests := []struct { + name string + testF func(t *testing.T, r *manual.Resolver, addr, sc string) + sc string + }{ + { + name: "invalid-service-config", + testF: testInvalidDefaultServiceConfig, + sc: "", + }, + { + name: "resolver-service-config-disabled", + testF: testDefaultServiceConfigWhenResolverServiceConfigDisabled, + sc: defaultSC, + }, + { + name: "resolver-does-not-return-service-config", + testF: testDefaultServiceConfigWhenResolverDoesNotReturnServiceConfig, + sc: defaultSC, + }, + { + name: "resolver-returns-invalid-service-config", + testF: testDefaultServiceConfigWhenResolverReturnInvalidServiceConfig, + sc: defaultSC, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + r := manual.NewBuilderWithScheme(test.name) + addr := r.Scheme() + ":///non.existent" + test.testF(t, r, addr, test.sc) + }) + } } func verifyWaitForReadyEqualsTrue(cc *ClientConn) bool { @@ -1091,8 +1120,8 @@ func verifyWaitForReadyEqualsTrue(cc *ClientConn) bool { return i != 10 } -func testInvalidDefaultServiceConfig(t *testing.T) { - _, err := Dial("fake.com", WithTransportCredentials(insecure.NewCredentials()), WithDefaultServiceConfig("")) +func testInvalidDefaultServiceConfig(t *testing.T, r *manual.Resolver, addr, sc string) { + _, err := Dial(addr, WithTransportCredentials(insecure.NewCredentials()), WithResolvers(r), WithDefaultServiceConfig(sc)) if !strings.Contains(err.Error(), invalidDefaultServiceConfigErrPrefix) { t.Fatalf("Dial got err: %v, want err contains: %v", err, invalidDefaultServiceConfigErrPrefix) } diff --git a/resolver/manual/manual.go b/resolver/manual/manual.go index e6b0f14cd941..0a4262342f35 100644 --- a/resolver/manual/manual.go +++ b/resolver/manual/manual.go @@ -26,7 +26,9 @@ import ( "google.golang.org/grpc/resolver" ) -// NewBuilderWithScheme creates a new test resolver builder with the given scheme. +// NewBuilderWithScheme creates a new manual resolver builder with the given +// scheme. Every instance of the manual resolver may only ever be used with a +// single grpc.ClientConn. Otherwise, bad things will happen. func NewBuilderWithScheme(scheme string) *Resolver { return &Resolver{ BuildCallback: func(resolver.Target, resolver.ClientConn, resolver.BuildOptions) {}, @@ -58,30 +60,34 @@ type Resolver struct { scheme string // Fields actually belong to the resolver. - mu sync.Mutex // Guards access to CC. - CC resolver.ClientConn - bootstrapState *resolver.State + // Guards access to below fields. + mu sync.Mutex + CC resolver.ClientConn + // Storing the most recent state update makes this resolver resilient to + // restarts, which is possible with channel idleness. + lastSeenState *resolver.State } // InitialState adds initial state to the resolver so that UpdateState doesn't // need to be explicitly called after Dial. func (r *Resolver) InitialState(s resolver.State) { - r.bootstrapState = &s + r.lastSeenState = &s } // Build returns itself for Resolver, because it's both a builder and a resolver. func (r *Resolver) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { + r.BuildCallback(target, cc, opts) r.mu.Lock() r.CC = cc - r.mu.Unlock() - r.BuildCallback(target, cc, opts) - if r.bootstrapState != nil { - r.UpdateState(*r.bootstrapState) + if r.lastSeenState != nil { + err := r.CC.UpdateState(*r.lastSeenState) + go r.UpdateStateCallback(err) } + r.mu.Unlock() return r, nil } -// Scheme returns the test scheme. +// Scheme returns the manual resolver's scheme. func (r *Resolver) Scheme() string { return r.scheme } @@ -100,6 +106,7 @@ func (r *Resolver) Close() { func (r *Resolver) UpdateState(s resolver.State) { r.mu.Lock() err := r.CC.UpdateState(s) + r.lastSeenState = &s r.mu.Unlock() r.UpdateStateCallback(err) }