resolver/manual: support restarts, required for channel idleness (#6638)

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)
 	}

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.
+	// Guards access to below fields.
+	mu sync.Mutex
 	CC resolver.ClientConn
-	bootstrapState *resolver.State
+	// 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)
 }