xds/internal/resolver: final bit of test cleanup

grpc · easwars · Oct 13, 2023 · Oct 12, 2023 · Oct 13, 2023 · Oct 13, 2023
commit 4fdb8468e5468c56961ad1a2d37e241f9346df64
diff --git a/xds/internal/resolver/helpers_test.go b/xds/internal/resolver/helpers_test.go
@@ -21,6 +21,7 @@ package resolver_test
 import (
 	"context"
 	"fmt"
+	"strings"
 	"testing"
 	"time"
 
@@ -161,6 +162,21 @@ func verifyNoUpdateFromResolver(ctx context.Context, t *testing.T, stateCh chan
 	}
 }
 
+// verifyErrorFromResolver waits for the resolver to push an error and verifies
+// that it matches the expected error.
+func verifyErrorFromResolver(ctx context.Context, t *testing.T, errCh chan error, wantErr string) {
+	t.Helper()
+
+	select {
+	case <-ctx.Done():
+		t.Fatal("Timeout when waiting for error to be propagated to the ClientConn")
+	case gotErr := <-errCh:
+		if gotErr == nil || !strings.Contains(gotErr.Error(), wantErr) {
+			t.Fatalf("Received error from resolver %q, want %q", gotErr, wantErr)
+		}
+	}
+}
+
 // Spins up an xDS management server and sets up an xDS bootstrap configuration
 // file that points to it.
 //

diff --git a/xds/internal/resolver/internal/internal.go b/xds/internal/resolver/internal/internal.go
@@ -0,0 +1,30 @@
+/*
+ *
+ * Copyright 2023 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+// Package internal contains functionality internal to the xDS resolver.
+package internal
+
+// The following variables are overridden in tests.
+var (
+	// NewWRR is the function used to create a new weighted round robin
+	// implementation.
+	NewWRR any //  func() wrr.WRR
+
+	// NewXDSClient is the function used to create a new xDS client.
+	NewXDSClient any // func() (xdsclient.XDSClient, func(), error)
+)
diff --git a/xds/internal/resolver/serviceconfig.go b/xds/internal/resolver/serviceconfig.go
@@ -40,6 +40,7 @@ import (
 	"google.golang.org/grpc/xds/internal/balancer/clustermanager"
 	"google.golang.org/grpc/xds/internal/balancer/ringhash"
 	"google.golang.org/grpc/xds/internal/httpfilter"
+	rinternal "google.golang.org/grpc/xds/internal/resolver/internal"
 	"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
 )
 
@@ -348,9 +349,6 @@ func (cs *configSelector) stop() {
 	}
 }
 
-// A global for testing.
-var newWRR = wrr.NewRandom
-
 // newConfigSelector creates the config selector for su; may add entries to
 // r.activeClusters for previously-unseen clusters.
 func (r *xdsResolver) newConfigSelector(su serviceUpdate) (*configSelector, error) {
@@ -366,7 +364,7 @@ func (r *xdsResolver) newConfigSelector(su serviceUpdate) (*configSelector, erro
 	}
 
 	for i, rt := range su.virtualHost.Routes {
-		clusters := newWRR()
+		clusters := rinternal.NewWRR.(func() wrr.WRR)()
 		if rt.ClusterSpecifierPlugin != "" {
 			clusterName := clusterSpecifierPluginPrefix + rt.ClusterSpecifierPlugin
 			clusters.Add(&routeCluster{

diff --git a/xds/internal/resolver/serviceconfig_test.go b/xds/internal/resolver/serviceconfig_test.go
@@ -25,13 +25,23 @@ import (
 
 	xxhash "github.com/cespare/xxhash/v2"
 	"github.com/google/go-cmp/cmp"
+	"google.golang.org/grpc/internal/grpctest"
 	"google.golang.org/grpc/internal/grpcutil"
 	iresolver "google.golang.org/grpc/internal/resolver"
+	"google.golang.org/grpc/internal/testutils"
 	"google.golang.org/grpc/metadata"
 	_ "google.golang.org/grpc/xds/internal/balancer/cdsbalancer" // To parse LB config
 	"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
 )
 
+type s struct {
+	grpctest.Tester
+}
+
+func Test(t *testing.T) {
+	grpctest.RunSubTests(t, s{})
+}
+
 func (s) TestPruneActiveClusters(t *testing.T) {
 	r := &xdsResolver{activeClusters: map[string]*clusterInfo{
 		"zero":        {refCount: 0},
@@ -53,7 +63,7 @@ func (s) TestGenerateRequestHash(t *testing.T) {
 	const channelID = 12378921
 	cs := &configSelector{
 		r: &xdsResolver{
-			cc:        &testClientConn{},
+			cc:        &testutils.ResolverClientConn{Logger: t},
 			channelID: channelID,
 		},
 	}

diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go
@@ -31,7 +31,9 @@ import (
 	"google.golang.org/grpc/internal/grpcsync"
 	"google.golang.org/grpc/internal/pretty"
 	iresolver "google.golang.org/grpc/internal/resolver"
+	"google.golang.org/grpc/internal/wrr"
 	"google.golang.org/grpc/resolver"
+	rinternal "google.golang.org/grpc/xds/internal/resolver/internal"
 	"google.golang.org/grpc/xds/internal/xdsclient"
 	"google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
 	"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
@@ -54,12 +56,12 @@ func newBuilderForTesting(config []byte) (resolver.Builder, error) {
 	}, nil
 }
 
-// For overriding in unittests.
-var newXDSClient = func() (xdsclient.XDSClient, func(), error) { return xdsclient.New() }
-
 func init() {
 	resolver.Register(&xdsResolverBuilder{})
 	internal.NewXDSResolverWithConfigForTesting = newBuilderForTesting
+
+	rinternal.NewWRR = wrr.NewRandom
+	rinternal.NewXDSClient = xdsclient.New
 }
 
 type xdsResolverBuilder struct {
@@ -86,7 +88,7 @@ func (b *xdsResolverBuilder) Build(target resolver.Target, cc resolver.ClientCon
 	r.logger = prefixLogger(r)
 	r.logger.Infof("Creating resolver for target: %+v", target)
 
-	newXDSClient := newXDSClient
+	newXDSClient := rinternal.NewXDSClient.(func() (xdsclient.XDSClient, func(), error))
 	if b.newXDSClient != nil {
 		newXDSClient = b.newXDSClient
 	}
@@ -115,7 +117,7 @@ func (b *xdsResolverBuilder) Build(target resolver.Target, cc resolver.ClientCon
 	}
 	if xc, ok := creds.(interface{ UsesXDS() bool }); ok && xc.UsesXDS() {
 		if len(bootstrapConfig.CertProviderConfigs) == 0 {
-			return nil, errors.New("xds: xdsCreds specified but certificate_providers config missing in bootstrap file")
+			return nil, fmt.Errorf("xds: use of xDS credentials is specified, but certificate_providers config missing in bootstrap file")
 		}
 	}