From 629526913e335d96faa6b775d6c17a61ea4eb401 Mon Sep 17 00:00:00 2001 From: Purnesh Dixit Date: Mon, 16 Dec 2024 02:33:11 +0530 Subject: [PATCH] use pool everywhere instead of new client functions --- xds/csds/csds_e2e_test.go | 8 +-- .../cdsbalancer/cdsbalancer_security_test.go | 5 +- .../balancer/cdsbalancer/cdsbalancer_test.go | 10 ++-- .../e2e_test/aggregate_cluster_test.go | 6 +- .../clusterresolver/e2e_test/balancer_test.go | 6 +- .../clusterresolver/e2e_test/eds_impl_test.go | 36 ++++++----- xds/internal/resolver/xds_resolver.go | 4 +- xds/internal/resolver/xds_resolver_test.go | 8 ++- xds/internal/server/rds_handler_test.go | 6 +- xds/internal/testutils/fakeclient/client.go | 8 +-- xds/internal/xdsclient/client_new.go | 55 +++-------------- xds/internal/xdsclient/client_refcounted.go | 8 +-- .../xdsclient/client_refcounted_test.go | 14 ++--- xds/internal/xdsclient/clientimpl.go | 2 +- xds/internal/xdsclient/clientimpl_dump.go | 3 +- xds/internal/xdsclient/pool.go | 55 +++++++---------- .../tests/ads_stream_ack_nack_test.go | 6 +- .../tests/ads_stream_backoff_test.go | 6 +- .../tests/ads_stream_flow_control_test.go | 6 +- .../tests/ads_stream_restart_test.go | 6 +- .../xdsclient/tests/ads_stream_watch_test.go | 6 +- .../xdsclient/tests/authority_test.go | 5 +- .../xdsclient/tests/cds_watchers_test.go | 50 +++++++++------- xds/internal/xdsclient/tests/dump_test.go | 8 +-- .../xdsclient/tests/eds_watchers_test.go | 40 ++++++++----- xds/internal/xdsclient/tests/fallback_test.go | 15 +++-- .../tests/federation_watchers_test.go | 5 +- .../xdsclient/tests/lds_watchers_test.go | 60 +++++++++++-------- .../xdsclient/tests/misc_watchers_test.go | 10 ++-- .../xdsclient/tests/rds_watchers_test.go | 40 ++++++++----- .../xdsclient/tests/resource_update_test.go | 10 ++-- xds/internal/xdsclient/xdsresource/errors.go | 2 +- xds/server.go | 17 ++---- xds/server_ext_test.go | 26 ++++---- xds/server_options.go | 6 +- xds/server_test.go | 16 ++--- 36 files changed, 298 insertions(+), 276 deletions(-) diff --git a/xds/csds/csds_e2e_test.go b/xds/csds/csds_e2e_test.go index 3c838afb67fc..cfbb85d94936 100644 --- a/xds/csds/csds_e2e_test.go +++ b/xds/csds/csds_e2e_test.go @@ -224,7 +224,7 @@ func (s) TestCSDS(t *testing.T) { // Create two xDS clients, with different names. These should end up // creating two different xDS clients. const xdsClient1Name = "xds-csds-client-1" - xdsClient1, xdsClose1, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + xdsClient1, xdsClose1, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: xdsClient1Name, Contents: bootstrapContents, }) @@ -233,7 +233,7 @@ func (s) TestCSDS(t *testing.T) { } defer xdsClose1() const xdsClient2Name = "xds-csds-client-2" - xdsClient2, xdsClose2, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + xdsClient2, xdsClose2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: xdsClient2Name, Contents: bootstrapContents, }) @@ -421,7 +421,7 @@ func (s) TestCSDS_NACK(t *testing.T) { // Create two xDS clients, with different names. These should end up // creating two different xDS clients. const xdsClient1Name = "xds-csds-client-1" - xdsClient1, xdsClose1, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + xdsClient1, xdsClose1, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: xdsClient1Name, Contents: bootstrapContents, }) @@ -430,7 +430,7 @@ func (s) TestCSDS_NACK(t *testing.T) { } defer xdsClose1() const xdsClient2Name = "xds-csds-client-2" - xdsClient2, xdsClose2, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + xdsClient2, xdsClose2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: xdsClient2Name, Contents: bootstrapContents, }) diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go index 83f8d274b819..a46b3b987e0f 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_security_test.go @@ -138,10 +138,11 @@ func registerWrappedCDSPolicyWithNewSubConnOverride(t *testing.T, ch chan *xdscr func setupForSecurityTests(t *testing.T, bootstrapContents []byte, clientCreds, serverCreds credentials.TransportCredentials) (*grpc.ClientConn, string) { t.Helper() - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsClient, xdsClose, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go index 75da48a61d58..eb8aa1575eff 100644 --- a/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go +++ b/xds/internal/balancer/cdsbalancer/cdsbalancer_test.go @@ -241,10 +241,11 @@ func setupWithManagementServerAndListener(t *testing.T, lis net.Listener) (*e2e. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) xdsC, xdsClose, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -367,10 +368,11 @@ func (s) TestConfigurationUpdate_EmptyCluster(t *testing.T) { nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) xdsClient, xdsClose, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go index d23c5280fd99..f4c388166992 100644 --- a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go +++ b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go @@ -36,6 +36,7 @@ import ( "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/internal/testutils/pickfirst" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/peer" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" @@ -1180,10 +1181,11 @@ func (s) TestAggregateCluster_Fallback_EDS_ResourceNotFound(t *testing.T) { // Create an xDS client talking to the above management server, configured // with a short watch expiry timeout. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsClient, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, diff --git a/xds/internal/balancer/clusterresolver/e2e_test/balancer_test.go b/xds/internal/balancer/clusterresolver/e2e_test/balancer_test.go index aa15f204743b..5fab34c33c68 100644 --- a/xds/internal/balancer/clusterresolver/e2e_test/balancer_test.go +++ b/xds/internal/balancer/clusterresolver/e2e_test/balancer_test.go @@ -38,6 +38,7 @@ import ( "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" "google.golang.org/grpc/serviceconfig" @@ -74,10 +75,11 @@ func setupAndDial(t *testing.T, bootstrapContents []byte) (*grpc.ClientConn, fun t.Helper() // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsC, xdsClose, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/balancer/clusterresolver/e2e_test/eds_impl_test.go b/xds/internal/balancer/clusterresolver/e2e_test/eds_impl_test.go index b19cc62cdf73..54cedc85fed2 100644 --- a/xds/internal/balancer/clusterresolver/e2e_test/eds_impl_test.go +++ b/xds/internal/balancer/clusterresolver/e2e_test/eds_impl_test.go @@ -40,6 +40,7 @@ import ( "google.golang.org/grpc/internal/testutils" rrutil "google.golang.org/grpc/internal/testutils/roundrobin" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/peer" "google.golang.org/grpc/resolver" "google.golang.org/grpc/resolver/manual" @@ -155,10 +156,11 @@ func (s) TestEDS_OneLocality(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -293,10 +295,11 @@ func (s) TestEDS_MultipleLocalities(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -460,10 +463,11 @@ func (s) TestEDS_EndpointsHealth(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -536,10 +540,11 @@ func (s) TestEDS_EmptyUpdate(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -938,10 +943,11 @@ func (s) TestEDS_BadUpdateWithoutPreviousGoodUpdate(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsClient, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1013,10 +1019,11 @@ func (s) TestEDS_BadUpdateWithPreviousGoodUpdate(t *testing.T) { } // Create an xDS client for use by the cluster_resolver LB policy. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsClient, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1088,10 +1095,11 @@ func (s) TestEDS_ResourceNotFound(t *testing.T) { // with a short watch expiry timeout. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) xdsClient, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index 8ebffc2d9dea..03a874911df3 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -51,7 +51,7 @@ const Scheme = "xds" func newBuilderWithConfigForTesting(config []byte) (resolver.Builder, error) { return &xdsResolverBuilder{ newXDSClient: func(name string) (xdsclient.XDSClient, func(), error) { - return xdsclient.NewForTesting(xdsclient.OptionsForTesting{Name: name, Contents: config}) + return xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{Name: name, Contents: config}) }, }, nil } @@ -75,7 +75,7 @@ func init() { internal.NewXDSResolverWithClientForTesting = newBuilderWithClientForTesting rinternal.NewWRR = wrr.NewRandom - rinternal.NewXDSClient = xdsclient.New + rinternal.NewXDSClient = xdsclient.DefaultPool.NewClient } type xdsResolverBuilder struct { diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index c7965b206860..3cfef9b72dce 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -254,7 +254,11 @@ func (s) TestResolverCloseClosesXDSClient(t *testing.T) { closeCh := make(chan struct{}) rinternal.NewXDSClient = func(string) (xdsclient.XDSClient, func(), error) { bc := e2e.DefaultBootstrapContents(t, uuid.New().String(), "dummy-management-server-address") - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) + if err != nil { + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) + } + pool := xdsclient.NewPool(config) if err != nil { t.Fatalf("Failed to create an xDS client pool: %v", err) } @@ -336,7 +340,7 @@ func (s) TestResolverBadServiceUpdate(t *testing.T) { // returned by the resolver matches expectations, and that the config selector // returned by the resolver picks clusters based on the route configuration // received from the management server. -func (s) TestResolverGoodServiceUpdate(t *testing.T) { +func TestResolverGoodServiceUpdate(t *testing.T) { for _, tt := range []struct { name string routeConfig *v3routepb.RouteConfiguration diff --git a/xds/internal/server/rds_handler_test.go b/xds/internal/server/rds_handler_test.go index b6a18c61b835..f658aab28feb 100644 --- a/xds/internal/server/rds_handler_test.go +++ b/xds/internal/server/rds_handler_test.go @@ -29,6 +29,7 @@ import ( "github.com/google/uuid" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" @@ -111,10 +112,11 @@ func xdsSetupForTests(t *testing.T) (*e2e.ManagementServer, string, chan []strin nodeID := uuid.New().String() bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsC, cancel, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/testutils/fakeclient/client.go b/xds/internal/testutils/fakeclient/client.go index 806a207fabe5..59b168106d46 100644 --- a/xds/internal/testutils/fakeclient/client.go +++ b/xds/internal/testutils/fakeclient/client.go @@ -28,7 +28,7 @@ import ( "google.golang.org/grpc/xds/internal/xdsclient/load" ) -// Client is a fake implementation of an xds client. It exposes a bunch of +// Client is a fake implementation of an xDS client. It exposes a bunch of // channels to signal the occurrence of various events. type Client struct { // Embed XDSClient so this fake client implements the interface, but it's @@ -89,17 +89,17 @@ func (xdsC *Client) SetBootstrapConfig(cfg *bootstrap.Config) { xdsC.bootstrapCfg = cfg } -// Name returns the name of the xds client. +// Name returns the name of the xDS client. func (xdsC *Client) Name() string { return xdsC.name } -// NewClient returns a new fake xds client. +// NewClient returns a new fake xDS client. func NewClient() *Client { return NewClientWithName("") } -// NewClientWithName returns a new fake xds client with the provided name. This +// NewClientWithName returns a new fake xDS client with the provided name. This // is used in cases where multiple clients are created in the tests and we need // to make sure the client is created for the expected balancer name. func NewClientWithName(name string) *Client { diff --git a/xds/internal/xdsclient/client_new.go b/xds/internal/xdsclient/client_new.go index 76f2ca1deb3f..0924a67dacd2 100644 --- a/xds/internal/xdsclient/client_new.go +++ b/xds/internal/xdsclient/client_new.go @@ -37,26 +37,6 @@ import ( // value, and is defined in gRFC A71. const NameForServer = "#server" -// New returns an xDS client configured with bootstrap configuration specified -// by the ordered list: -// - file name containing the configuration specified by GRPC_XDS_BOOTSTRAP -// - actual configuration specified by GRPC_XDS_BOOTSTRAP_CONFIG -// - fallback configuration set using bootstrap.SetFallbackBootstrapConfig -// -// gRPC client implementations are expected to pass the channel's target URI for -// the name field, while server implementations are expected to pass a dedicated -// well-known value "#server", as specified in gRFC A71. The returned client is -// a reference counted implementation shared among callers using the same name. -// -// The second return value represents a close function which releases the -// caller's reference on the returned client. The caller is expected to invoke -// it once they are done using the client. The underlying client will be closed -// only when all references are released, and it is safe for the caller to -// invoke this close function multiple times. -func New(name string) (XDSClient, func(), error) { - return DefaultPool.NewClient(name) -} - // newClientImpl returns a new xdsClient with the given config. func newClientImpl(config *bootstrap.Config, watchExpiryTimeout time.Duration, streamBackoff func(int) time.Duration) (*clientImpl, error) { ctx, cancel := context.WithCancel(context.Background()) @@ -119,37 +99,16 @@ type OptionsForTesting struct { StreamBackoffAfterFailure func(int) time.Duration } -// NewForTesting returns an xDS client configured with the provided options -// from the default pool. -// -// The second return value represents a close function which the caller is -// expected to invoke once they are done using the client. It is safe for the -// caller to invoke this close function multiple times. -// -// # Testing Only -// -// This function should ONLY be used for testing purposes. -func NewForTesting(opts OptionsForTesting) (XDSClient, func(), error) { - return DefaultPool.NewClientForTesting(opts) -} - -// GetForTesting returns an xDS client created earlier using the given name in -// the default pool. -// -// The second return value represents a close function which the caller is -// expected to invoke once they are done using the client. It is safe for the -// caller to invoke this close function multiple times. -// -// # Testing Only -// -// This function should ONLY be used for testing purposes. -func GetForTesting(name string) (XDSClient, func(), error) { - return DefaultPool.GetClientForTesting(name) -} - func init() { internal.TriggerXDSResourceNotFoundForTesting = triggerXDSResourceNotFoundForTesting xdsclientinternal.ResourceWatchStateForTesting = resourceWatchStateForTesting + DefaultPool = &Pool{clients: make(map[string]*clientRefCounted)} + config, err := bootstrap.GetConfiguration() + if err != nil { + logger.Warningf("Failed to read xDS bootstrap config from env vars: %v", err) + return + } + DefaultPool.config = config } func triggerXDSResourceNotFoundForTesting(client XDSClient, typ xdsresource.Type, name string) error { diff --git a/xds/internal/xdsclient/client_refcounted.go b/xds/internal/xdsclient/client_refcounted.go index 0b438866be1c..68d555fc7e3a 100644 --- a/xds/internal/xdsclient/client_refcounted.go +++ b/xds/internal/xdsclient/client_refcounted.go @@ -38,7 +38,7 @@ var ( defaultStreamBackoffFunc = backoff.DefaultExponential.Backoff ) -func clientRefCountedClose(name string, pool *Pool) { +func (pool *Pool) clientRefCountedClose(name string) { pool.mu.Lock() defer pool.mu.Unlock() @@ -59,13 +59,13 @@ func clientRefCountedClose(name string, pool *Pool) { // newRefCounted creates a new reference counted xDS client implementation for // name, if one does not exist already. If an xDS client for the given name // exists, it gets a reference to it and returns it. -func newRefCounted(name string, pool *Pool, config *bootstrap.Config, watchExpiryTimeout time.Duration, streamBackoff func(int) time.Duration) (XDSClient, func(), error) { +func (pool *Pool) newRefCounted(name string, config *bootstrap.Config, watchExpiryTimeout time.Duration, streamBackoff func(int) time.Duration) (XDSClient, func(), error) { pool.mu.Lock() defer pool.mu.Unlock() if c := pool.clients[name]; c != nil { c.incrRef() - return c, grpcsync.OnceFunc(func() { clientRefCountedClose(name, pool) }), nil + return c, grpcsync.OnceFunc(func() { pool.clientRefCountedClose(name) }), nil } // Create the new client implementation. @@ -82,7 +82,7 @@ func newRefCounted(name string, pool *Pool, config *bootstrap.Config, watchExpir xdsClientImplCreateHook(name) logger.Infof("xDS node ID: %s", config.Node().GetId()) - return client, grpcsync.OnceFunc(func() { clientRefCountedClose(name, pool) }), nil + return client, grpcsync.OnceFunc(func() { pool.clientRefCountedClose(name) }), nil } // clientRefCounted is ref-counted, and to be shared by the xds resolver and diff --git a/xds/internal/xdsclient/client_refcounted_test.go b/xds/internal/xdsclient/client_refcounted_test.go index 0824ad385b26..26ef00e16469 100644 --- a/xds/internal/xdsclient/client_refcounted_test.go +++ b/xds/internal/xdsclient/client_refcounted_test.go @@ -55,7 +55,7 @@ func (s) TestClientNew_Single(t *testing.T) { defer func() { xdsClientImplCloseHook = origClientImplCloseHook }() // The first call to New() should create a new client. - _, closeFunc, err := New(t.Name()) + _, closeFunc, err := DefaultPool.NewClient(t.Name()) if err != nil { t.Fatalf("Failed to create xDS client: %v", err) } @@ -71,7 +71,7 @@ func (s) TestClientNew_Single(t *testing.T) { closeFuncs := make([]func(), count) for i := 0; i < count; i++ { func() { - _, closeFuncs[i], err = New(t.Name()) + _, closeFuncs[i], err = DefaultPool.NewClient(t.Name()) if err != nil { t.Fatalf("%d-th call to New() failed with error: %v", i, err) } @@ -109,7 +109,7 @@ func (s) TestClientNew_Single(t *testing.T) { // Calling New() again, after the previous Client was actually closed, // should create a new one. - _, closeFunc, err = New(t.Name()) + _, closeFunc, err = DefaultPool.NewClient(t.Name()) if err != nil { t.Fatalf("Failed to create xDS client: %v", err) } @@ -147,7 +147,7 @@ func (s) TestClientNew_Multiple(t *testing.T) { // Create two xDS clients. client1Name := t.Name() + "-1" - _, closeFunc1, err := New(client1Name) + _, closeFunc1, err := DefaultPool.NewClient(client1Name) if err != nil { t.Fatalf("Failed to create xDS client: %v", err) } @@ -162,7 +162,7 @@ func (s) TestClientNew_Multiple(t *testing.T) { } client2Name := t.Name() + "-2" - _, closeFunc2, err := New(client2Name) + _, closeFunc2, err := DefaultPool.NewClient(client2Name) if err != nil { t.Fatalf("Failed to create xDS client: %v", err) } @@ -184,7 +184,7 @@ func (s) TestClientNew_Multiple(t *testing.T) { defer wg.Done() for i := 0; i < count; i++ { var err error - _, closeFuncs1[i], err = New(client1Name) + _, closeFuncs1[i], err = DefaultPool.NewClient(client1Name) if err != nil { t.Errorf("%d-th call to New() failed with error: %v", i, err) } @@ -194,7 +194,7 @@ func (s) TestClientNew_Multiple(t *testing.T) { defer wg.Done() for i := 0; i < count; i++ { var err error - _, closeFuncs2[i], err = New(client2Name) + _, closeFuncs2[i], err = DefaultPool.NewClient(client2Name) if err != nil { t.Errorf("%d-th call to New() failed with error: %v", i, err) } diff --git a/xds/internal/xdsclient/clientimpl.go b/xds/internal/xdsclient/clientimpl.go index bb8d9040022f..5d7ff8056752 100644 --- a/xds/internal/xdsclient/clientimpl.go +++ b/xds/internal/xdsclient/clientimpl.go @@ -38,7 +38,7 @@ var _ XDSClient = &clientImpl{} // ErrClientClosed is returned when the xDS client is closed. var ErrClientClosed = errors.New("xds: the xDS client is closed") -// clientImpl is the real implementation of the xds client. The exported Client +// clientImpl is the real implementation of the xDS client. The exported Client // is a wrapper of this struct with a ref count. type clientImpl struct { // The following fields are initialized at creation time and are read-only diff --git a/xds/internal/xdsclient/clientimpl_dump.go b/xds/internal/xdsclient/clientimpl_dump.go index 559ac388d6da..f5130db46723 100644 --- a/xds/internal/xdsclient/clientimpl_dump.go +++ b/xds/internal/xdsclient/clientimpl_dump.go @@ -35,7 +35,8 @@ func (c *clientImpl) dumpResources() *v3statuspb.ClientConfig { } } -// DumpResources returns the status and contents of all xDS resources. +// DumpResources returns the status and contents of all xDS resources. It uses +// xDS clients from the default pool. func DumpResources() *v3statuspb.ClientStatusResponse { DefaultPool.mu.Lock() defer DefaultPool.mu.Unlock() diff --git a/xds/internal/xdsclient/pool.go b/xds/internal/xdsclient/pool.go index b346eea9b422..914f949f3faf 100644 --- a/xds/internal/xdsclient/pool.go +++ b/xds/internal/xdsclient/pool.go @@ -28,52 +28,48 @@ import ( ) var ( - // DefaultPool is the default pool for xds clients. It is created at the - // init time. + // DefaultPool is the default pool for xDS clients. It is created at the + // init time by reading bootstrap configuration from env vars. DefaultPool *Pool ) -// Pool represents pool of xds clients. +// Pool represents pool of xDS clients who share the same bootstrap +// configuration. +// +// Note that mu should ideally only have to guard clients. But here, we need +// it to guard config as well since SetFallbackBootstrapConfig writes to +// config. type Pool struct { mu sync.Mutex clients map[string]*clientRefCounted config *bootstrap.Config } -func init() { - DefaultPool = &Pool{clients: make(map[string]*clientRefCounted)} -} - -// NewPool creates a new xds client pool with the given bootstrap contents. -func NewPool(bootstrapContents []byte) (*Pool, error) { - config, err := bootstrap.NewConfigForTesting(bootstrapContents) - if err != nil { - return nil, err - } +// NewPool creates a new xDS client pool with the given bootstrap contents. +func NewPool(config *bootstrap.Config) *Pool { return &Pool{ clients: make(map[string]*clientRefCounted), config: config, - }, nil + } } -// NewClient returns an xds client with the given name from the pool. If the -// client doesn't already exist, it creates a new xds client and adds it to the +// NewClient returns an xDS client with the given name from the pool. If the +// client doesn't already exist, it creates a new xDS client and adds it to the // pool. // // The second return value represents a close function which the caller is // expected to invoke once they are done using the client. It is safe for the // caller to invoke this close function multiple times. func (p *Pool) NewClient(name string) (XDSClient, func(), error) { - config, err := bootstrap.GetConfiguration() - if err != nil { - return nil, nil, fmt.Errorf("xds: failed to get xDS bootstrap config: %v", err) + if p.config == nil { + return nil, nil, fmt.Errorf("bootstrap configuration not set in the pool") } - return newRefCounted(name, p, config, defaultWatchExpiryTimeout, backoff.DefaultExponential.Backoff) + return p.newRefCounted(name, p.config, defaultWatchExpiryTimeout, backoff.DefaultExponential.Backoff) } -// NewClientForTesting returns an xds client configured with the provided +// NewClientForTesting returns an xDS client configured with the provided // options from the pool. If the client doesn't already exist, it creates a new -// xds client and adds it to the pool. +// xDS client and adds it to the pool. // // The second return value represents a close function which the caller is // expected to invoke once they are done using the client. It is safe for the @@ -97,13 +93,12 @@ func (p *Pool) NewClientForTesting(opts OptionsForTesting) (XDSClient, func(), e if err != nil { return nil, nil, err } - return newRefCounted(opts.Name, p, config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure) + return p.newRefCounted(opts.Name, config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure) } - - return newRefCounted(opts.Name, p, p.config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure) + return p.newRefCounted(opts.Name, p.config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure) } -// GetClientForTesting returns an xds client created earlier using the given +// GetClientForTesting returns an xDS client created earlier using the given // name from the pool. // // The second return value represents a close function which the caller is @@ -122,19 +117,15 @@ func (p *Pool) GetClientForTesting(name string) (XDSClient, func(), error) { return nil, nil, fmt.Errorf("xDS client with name %q not found", name) } c.incrRef() - return c, grpcsync.OnceFunc(func() { clientRefCountedClose(name, p) }), nil + return c, grpcsync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil } // SetFallbackBootstrapConfig to specify a bootstrap configuration to use a // fallback when the bootstrap env vars are not specified. -func (p *Pool) SetFallbackBootstrapConfig(bootstrapContents []byte) error { +func (p *Pool) SetFallbackBootstrapConfig(config *bootstrap.Config) error { p.mu.Lock() defer p.mu.Unlock() - config, err := bootstrap.NewConfigForTesting(bootstrapContents) - if err != nil { - return err - } p.config = config return nil } diff --git a/xds/internal/xdsclient/tests/ads_stream_ack_nack_test.go b/xds/internal/xdsclient/tests/ads_stream_ack_nack_test.go index 8a4ef431bb57..93e9872d93ff 100644 --- a/xds/internal/xdsclient/tests/ads_stream_ack_nack_test.go +++ b/xds/internal/xdsclient/tests/ads_stream_ack_nack_test.go @@ -31,6 +31,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/protobuf/proto" @@ -381,10 +382,11 @@ func (s) TestADS_ACK_NACK_ResourceIsNotRequestedAnymore(t *testing.T) { // Create an xDS client with bootstrap pointing to the above server. bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) testutils.CreateBootstrapFileForTesting(t, bc) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/ads_stream_backoff_test.go b/xds/internal/xdsclient/tests/ads_stream_backoff_test.go index 659e22558869..e0865abb1327 100644 --- a/xds/internal/xdsclient/tests/ads_stream_backoff_test.go +++ b/xds/internal/xdsclient/tests/ads_stream_backoff_test.go @@ -32,6 +32,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" @@ -46,10 +47,11 @@ import ( func createXDSClientWithBackoff(t *testing.T, bootstrapContents []byte, streamBackoff func(int) time.Duration) xdsclient.XDSClient { t.Helper() - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), StreamBackoffAfterFailure: streamBackoff, diff --git a/xds/internal/xdsclient/tests/ads_stream_flow_control_test.go b/xds/internal/xdsclient/tests/ads_stream_flow_control_test.go index e6df37995634..3b0827af33db 100644 --- a/xds/internal/xdsclient/tests/ads_stream_flow_control_test.go +++ b/xds/internal/xdsclient/tests/ads_stream_flow_control_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" xdsclientinternal "google.golang.org/grpc/xds/internal/xdsclient/internal" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" @@ -150,10 +151,11 @@ func overrideADSStreamCreation(t *testing.T) chan *wrappedADSStream { func createXDSClient(t *testing.T, bootstrapContents []byte) xdsclient.XDSClient { t.Helper() - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/ads_stream_restart_test.go b/xds/internal/xdsclient/tests/ads_stream_restart_test.go index e4f9b6141b9c..5cf0540de246 100644 --- a/xds/internal/xdsclient/tests/ads_stream_restart_test.go +++ b/xds/internal/xdsclient/tests/ads_stream_restart_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/uuid" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" @@ -112,10 +113,11 @@ func (s) TestADS_ResourcesAreRequestedAfterStreamRestart(t *testing.T) { bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) // Create an xDS client with the above bootstrap configuration. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/ads_stream_watch_test.go b/xds/internal/xdsclient/tests/ads_stream_watch_test.go index 44ecc169c976..df676b0f5ffb 100644 --- a/xds/internal/xdsclient/tests/ads_stream_watch_test.go +++ b/xds/internal/xdsclient/tests/ads_stream_watch_test.go @@ -27,6 +27,7 @@ import ( "github.com/google/uuid" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" + "google.golang.org/grpc/internal/xds/bootstrap" xdsinternal "google.golang.org/grpc/xds/internal" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/internal" @@ -147,10 +148,11 @@ func (s) TestADS_WatchState_TimerFires(t *testing.T) { nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) testutils.CreateBootstrapFileForTesting(t, bc) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, diff --git a/xds/internal/xdsclient/tests/authority_test.go b/xds/internal/xdsclient/tests/authority_test.go index 04a904566ad4..47f893beff34 100644 --- a/xds/internal/xdsclient/tests/authority_test.go +++ b/xds/internal/xdsclient/tests/authority_test.go @@ -100,10 +100,11 @@ func setupForAuthorityTests(ctx context.Context, t *testing.T) (*testutils.Liste if err != nil { t.Fatalf("Failed to create bootstrap configuration: %v", err) } - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, diff --git a/xds/internal/xdsclient/tests/cds_watchers_test.go b/xds/internal/xdsclient/tests/cds_watchers_test.go index f68d40055ed1..1f0652cabd1d 100644 --- a/xds/internal/xdsclient/tests/cds_watchers_test.go +++ b/xds/internal/xdsclient/tests/cds_watchers_test.go @@ -210,10 +210,11 @@ func (s) TestCDSWatch(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -363,10 +364,11 @@ func (s) TestCDSWatch_TwoWatchesForSameResourceName(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -469,10 +471,11 @@ func (s) TestCDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -569,10 +572,11 @@ func (s) TestCDSWatch_ResourceCaching(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -645,10 +649,11 @@ func (s) TestCDSWatch_ExpiryTimerFiresBeforeResponse(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) testutils.CreateBootstrapFileForTesting(t, bc) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -688,10 +693,11 @@ func (s) TestCDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) testutils.CreateBootstrapFileForTesting(t, bc) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -774,10 +780,11 @@ func (s) TestCDSWatch_ResourceRemoved(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -889,10 +896,11 @@ func (s) TestCDSWatch_NACKError(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -961,10 +969,11 @@ func (s) TestCDSWatch_PartialValid(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1056,10 +1065,11 @@ func (s) TestCDSWatch_PartialResponse(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/dump_test.go b/xds/internal/xdsclient/tests/dump_test.go index bb4b2719113e..f2d29ef77035 100644 --- a/xds/internal/xdsclient/tests/dump_test.go +++ b/xds/internal/xdsclient/tests/dump_test.go @@ -140,7 +140,7 @@ func (s) TestDumpResources_ManyToOne(t *testing.T) { // Create two xDS clients with the above bootstrap contents. client1Name := t.Name() + "-1" - client1, close1, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client1, close1, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: client1Name, Contents: bc, }) @@ -149,7 +149,7 @@ func (s) TestDumpResources_ManyToOne(t *testing.T) { } defer close1() client2Name := t.Name() + "-2" - client2, close2, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client2, close2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: client2Name, Contents: bc, }) @@ -409,7 +409,7 @@ func (s) TestDumpResources_ManyToMany(t *testing.T) { // Create two xDS clients with the above bootstrap contents. client1Name := t.Name() + "-1" - client1, close1, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client1, close1, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: client1Name, Contents: bc, }) @@ -418,7 +418,7 @@ func (s) TestDumpResources_ManyToMany(t *testing.T) { } defer close1() client2Name := t.Name() + "-2" - client2, close2, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client2, close2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: client2Name, Contents: bc, }) diff --git a/xds/internal/xdsclient/tests/eds_watchers_test.go b/xds/internal/xdsclient/tests/eds_watchers_test.go index 09a0bba4ba50..c637fb0e7d35 100644 --- a/xds/internal/xdsclient/tests/eds_watchers_test.go +++ b/xds/internal/xdsclient/tests/eds_watchers_test.go @@ -240,10 +240,11 @@ func (s) TestEDSWatch(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -433,10 +434,11 @@ func (s) TestEDSWatch_TwoWatchesForSameResourceName(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -540,10 +542,11 @@ func (s) TestEDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -645,10 +648,11 @@ func (s) TestEDSWatch_ResourceCaching(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), Contents: bc, @@ -733,10 +737,11 @@ func (s) TestEDSWatch_ExpiryTimerFiresBeforeResponse(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) testutils.CreateBootstrapFileForTesting(t, bc) - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -777,10 +782,11 @@ func (s) TestEDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client talking to the above management server. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -849,10 +855,11 @@ func (s) TestEDSWatch_NACKError(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), Contents: bc, @@ -922,10 +929,11 @@ func (s) TestEDSWatch_PartialValid(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/fallback_test.go b/xds/internal/xdsclient/tests/fallback_test.go index 2d0de86ea611..a295f6da73a9 100644 --- a/xds/internal/xdsclient/tests/fallback_test.go +++ b/xds/internal/xdsclient/tests/fallback_test.go @@ -163,10 +163,11 @@ func (s) TestFallback_OnStartup(t *testing.T) { // Create an xDS client with the above bootstrap configuration and a short // idle channel expiry timeout. This ensures that connections to lower // priority servers get closed quickly, for the test to verify. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsC, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -368,10 +369,11 @@ func (s) TestFallback_MidUpdate(t *testing.T) { // Create an xDS client with the above bootstrap configuration and a short // idle channel expiry timeout. This ensures that connections to lower // priority servers get closed quickly, for the test to verify. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsC, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -563,10 +565,11 @@ func (s) TestFallback_MidStartup(t *testing.T) { // Create an xDS client with the above bootstrap configuration and a short // idle channel expiry timeout. This ensures that connections to lower // priority servers get closed quickly, for the test to verify. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) xdsC, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/federation_watchers_test.go b/xds/internal/xdsclient/tests/federation_watchers_test.go index 5360e635f5c2..205087b78c68 100644 --- a/xds/internal/xdsclient/tests/federation_watchers_test.go +++ b/xds/internal/xdsclient/tests/federation_watchers_test.go @@ -69,10 +69,11 @@ func setupForFederationWatchersTest(t *testing.T) (*e2e.ManagementServer, string t.Fatalf("Failed to create bootstrap configuration: %v", err) } // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/lds_watchers_test.go b/xds/internal/xdsclient/tests/lds_watchers_test.go index e1fada5d9100..9e186ae8cb7b 100644 --- a/xds/internal/xdsclient/tests/lds_watchers_test.go +++ b/xds/internal/xdsclient/tests/lds_watchers_test.go @@ -266,10 +266,11 @@ func (s) TestLDSWatch(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -419,10 +420,11 @@ func (s) TestLDSWatch_TwoWatchesForSameResourceName(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -526,10 +528,11 @@ func (s) TestLDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -621,10 +624,11 @@ func (s) TestLDSWatch_ResourceCaching(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -698,10 +702,11 @@ func (s) TestLDSWatch_ExpiryTimerFiresBeforeResponse(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client talking to the above management server. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -741,10 +746,11 @@ func (s) TestLDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client talking to the above management server. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -828,10 +834,11 @@ func (s) TestLDSWatch_ResourceRemoved(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -945,10 +952,11 @@ func (s) TestLDSWatch_NewWatcherForRemovedResource(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1025,10 +1033,11 @@ func (s) TestLDSWatch_NACKError(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1082,10 +1091,11 @@ func (s) TestLDSWatch_ResourceCaching_NACKError(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1184,10 +1194,11 @@ func (s) TestLDSWatch_PartialValid(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -1276,10 +1287,11 @@ func (s) TestLDSWatch_PartialResponse(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), Contents: bc, diff --git a/xds/internal/xdsclient/tests/misc_watchers_test.go b/xds/internal/xdsclient/tests/misc_watchers_test.go index 87834c67edbb..347df9b5548a 100644 --- a/xds/internal/xdsclient/tests/misc_watchers_test.go +++ b/xds/internal/xdsclient/tests/misc_watchers_test.go @@ -126,10 +126,11 @@ func (s) TestWatchCallAnotherWatch(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -239,10 +240,11 @@ func (s) TestNodeProtoSentOnlyInFirstRequest(t *testing.T) { bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/rds_watchers_test.go b/xds/internal/xdsclient/tests/rds_watchers_test.go index 619dfc4e3b88..6065e3e26fc0 100644 --- a/xds/internal/xdsclient/tests/rds_watchers_test.go +++ b/xds/internal/xdsclient/tests/rds_watchers_test.go @@ -242,10 +242,11 @@ func (s) TestRDSWatch(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -435,10 +436,11 @@ func (s) TestRDSWatch_TwoWatchesForSameResourceName(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -542,10 +544,11 @@ func (s) TestRDSWatch_ThreeWatchesForDifferentResourceNames(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -647,10 +650,11 @@ func (s) TestRDSWatch_ResourceCaching(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) @@ -734,10 +738,11 @@ func (s) TestRDSWatch_ExpiryTimerFiresBeforeResponse(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client talking to the above management server. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -777,10 +782,11 @@ func (s) TestRDSWatch_ValidResponseCancelsExpiryTimerBehavior(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client talking to the above management server. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, @@ -849,10 +855,11 @@ func (s) TestRDSWatch_NACKError(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), Contents: bc, @@ -922,10 +929,11 @@ func (s) TestRDSWatch_PartialValid(t *testing.T) { testutils.CreateBootstrapFileForTesting(t, bc) // Create an xDS client with the above bootstrap contents. - pool, err := xdsclient.NewPool(bc) + config, err := bootstrap.NewConfigForTesting(bc) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err) } + pool := xdsclient.NewPool(config) client, close, err := pool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), }) diff --git a/xds/internal/xdsclient/tests/resource_update_test.go b/xds/internal/xdsclient/tests/resource_update_test.go index ea3920978920..4cbe947c1068 100644 --- a/xds/internal/xdsclient/tests/resource_update_test.go +++ b/xds/internal/xdsclient/tests/resource_update_test.go @@ -283,7 +283,7 @@ func (s) TestHandleListenerResponseFromManagementServer(t *testing.T) { // Create an xDS client talking to the above management server. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - client, close, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client, close, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, Contents: bc, @@ -360,7 +360,7 @@ func (s) TestHandleListenerResponseFromManagementServer(t *testing.T) { // involving receipt of an RDS response from the management server. The test // verifies that the internal state of the xDS client (parsed resource and // metadata) matches expectations. -func TestHandleRouteConfigResponseFromManagementServer(t *testing.T) { +func (s) TestHandleRouteConfigResponseFromManagementServer(t *testing.T) { const ( resourceName1 = "resource-name-1" resourceName2 = "resource-name-2" @@ -559,7 +559,7 @@ func TestHandleRouteConfigResponseFromManagementServer(t *testing.T) { // Create an xDS client talking to the above management server. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - client, close, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client, close, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, Contents: bc, @@ -796,7 +796,7 @@ func (s) TestHandleClusterResponseFromManagementServer(t *testing.T) { // Create an xDS client talking to the above management server. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - client, close, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client, close, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, Contents: bc, @@ -1145,7 +1145,7 @@ func (s) TestHandleEndpointsResponseFromManagementServer(t *testing.T) { // Create an xDS client talking to the above management server. nodeID := uuid.New().String() bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address) - client, close, err := xdsclient.NewForTesting(xdsclient.OptionsForTesting{ + client, close, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{ Name: t.Name(), WatchExpiryTimeout: defaultTestWatchExpiryTimeout, Contents: bc, diff --git a/xds/internal/xdsclient/xdsresource/errors.go b/xds/internal/xdsclient/xdsresource/errors.go index 7bac4469b78b..d47d6283fe15 100644 --- a/xds/internal/xdsclient/xdsresource/errors.go +++ b/xds/internal/xdsclient/xdsresource/errors.go @@ -51,7 +51,7 @@ func (e *xdsClientError) Error() string { return e.desc } -// NewErrorf creates an xds client error. The callbacks are called with this +// NewErrorf creates an xDS client error. The callbacks are called with this // error, to pass additional information about the error. func NewErrorf(t ErrorType, format string, args ...any) error { return &xdsClientError{t: t, desc: fmt.Sprintf(format, args...)} diff --git a/xds/server.go b/xds/server.go index a1b2e1d055f9..31d79bb452ca 100644 --- a/xds/server.go +++ b/xds/server.go @@ -42,10 +42,8 @@ import ( const serverPrefix = "[xds-server %p] " var ( - // These new functions will be overridden in unit tests. - newXDSClient = func(name string) (xdsclient.XDSClient, func(), error) { - return xdsclient.New(name) - } + // These will be overriden in unit tests. + xDSClientPool = xdsclient.DefaultPool newGRPCServer = func(opts ...grpc.ServerOption) grpcServer { return grpc.NewServer(opts...) } @@ -92,16 +90,11 @@ func NewGRPCServer(opts ...grpc.ServerOption) (*GRPCServer, error) { // Initializing the xDS client upfront (instead of at serving time) // simplifies the code by eliminating the need for a mutex to protect the // xdsC and xdsClientClose fields. - newXDSClient := newXDSClient + xDSClientPool := xDSClientPool if s.opts.clientPoolForTesting != nil { - // Bootstrap file contents may be specified as a server option for tests. - newXDSClient = func(name string) (xdsclient.XDSClient, func(), error) { - return s.opts.clientPoolForTesting.NewClientForTesting(xdsclient.OptionsForTesting{ - Name: name, - }) - } + xDSClientPool = s.opts.clientPoolForTesting } - xdsClient, xdsClientClose, err := newXDSClient(xdsclient.NameForServer) + xdsClient, xdsClientClose, err := xDSClientPool.NewClient(xdsclient.NameForServer) if err != nil { return nil, fmt.Errorf("xDS client creation failed: %v", err) } diff --git a/xds/server_ext_test.go b/xds/server_ext_test.go index 55d021c7bf76..6086b551734c 100644 --- a/xds/server_ext_test.go +++ b/xds/server_ext_test.go @@ -144,10 +144,11 @@ func (s) TestServingModeChanges(t *testing.T) { } }, } - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) sopts := []grpc.ServerOption{grpc.Creds(insecure.NewCredentials()), modeChangeOpt, xds.ClientPoolForTesting(pool)} if stub.S, err = xds.NewGRPCServer(sopts...); err != nil { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) @@ -225,7 +226,7 @@ func (s) TestServingModeChanges(t *testing.T) { // TestServingModeChanges_MultipleServers tests two servers with unique // bootstrap configuration are able to serve two different clients with same -// name if they are in different xds client pools. +// name if they are in different xDS client pools. // // It tests the Server's logic as it transitions from Not Ready to Ready, then // then to Not Ready. Before it goes Ready, connections should be accepted and @@ -318,19 +319,21 @@ func (s) TestServingModeChanges_MultipleServers(t *testing.T) { }, } - // Create two xds client pools with different bootstrap contents. - pool1, err := xdsclient.NewPool(bootstrapContents1) + // Create two xDS client pools with different bootstrap contents. + config1, err := bootstrap.NewConfigForTesting(bootstrapContents1) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents1, err) } + pool1 := xdsclient.NewPool(config1) sopts1 := []grpc.ServerOption{grpc.Creds(insecure.NewCredentials()), modeChangeOpt, xds.ClientPoolForTesting(pool1)} if stub1.S, err = xds.NewGRPCServer(sopts1...); err != nil { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) } - pool2, err := xdsclient.NewPool(bootstrapContents2) + config2, err := bootstrap.NewConfigForTesting(bootstrapContents2) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents2, err) } + pool2 := xdsclient.NewPool(config2) sopts2 := []grpc.ServerOption{grpc.Creds(insecure.NewCredentials()), modeChangeOpt, xds.ClientPoolForTesting(pool2)} if stub2.S, err = xds.NewGRPCServer(sopts2...); err != nil { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) @@ -414,7 +417,7 @@ func (s) TestServingModeChanges_MultipleServers(t *testing.T) { } defer close2() - // Compare the bootstrap configurations for both xds clients. + // Compare the bootstrap configurations for both xDS clients. if want, _ := bootstrap.NewConfigForTesting(bootstrapContents1); !cmp.Equal(want, xdsC1.BootstrapConfig()) { t.Fatalf("want %v bootstrap config from xdsC1, got %v", want, xdsC1.BootstrapConfig()) } @@ -522,10 +525,11 @@ func (s) TestResourceNotFoundRDS(t *testing.T) { } }, } - pool, err := xdsclient.NewPool(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { - t.Fatalf("Failed to create an xDS client pool: %v", err) + t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err) } + pool := xdsclient.NewPool(config) sopts := []grpc.ServerOption{grpc.Creds(insecure.NewCredentials()), modeChangeOpt, xds.ClientPoolForTesting(pool)} if stub.S, err = xds.NewGRPCServer(sopts...); err != nil { t.Fatalf("Failed to create an xDS enabled gRPC server: %v", err) diff --git a/xds/server_options.go b/xds/server_options.go index 931a64088117..ce9c7c9994a0 100644 --- a/xds/server_options.go +++ b/xds/server_options.go @@ -23,6 +23,7 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/xds/internal/xdsclient" ) @@ -75,15 +76,16 @@ type ServingModeChangeArgs struct { // `ClientPoolForTesting` to set the pool directly. It will be removed in // later release. func BootstrapContentsForTesting(bootstrapContents []byte) grpc.ServerOption { - err := xdsclient.DefaultPool.SetFallbackBootstrapConfig(bootstrapContents) + config, err := bootstrap.NewConfigForTesting(bootstrapContents) if err != nil { return &serverOption{apply: func(o *serverOptions) { o.clientPoolForTesting = nil }} } + xdsclient.DefaultPool.SetFallbackBootstrapConfig(config) return &serverOption{apply: func(o *serverOptions) { o.clientPoolForTesting = xdsclient.DefaultPool }} } // ClientPoolForTesting returns a grpc.ServerOption with the pool for xds -// clients. It allows users to set a pool for xds clients sharing the bootstrap +// clients. It allows users to set a pool for xDS clients sharing the bootstrap // contents for this server. // // # Testing Only diff --git a/xds/server_test.go b/xds/server_test.go index 7576263a091e..2e67f4bffd90 100644 --- a/xds/server_test.go +++ b/xds/server_test.go @@ -181,16 +181,14 @@ func (s) TestNewServer_Failure(t *testing.T) { }{ { desc: "bootstrap env var not set", - serverOpts: []grpc.ServerOption{grpc.Creds(xdsCreds)}, - wantErr: "failed to get xDS bootstrap config", + serverOpts: []grpc.ServerOption{grpc.Creds(xdsCreds), ClientPoolForTesting(xdsclient.NewPool(nil))}, + wantErr: "bootstrap configuration not set in the pool", }, { desc: "empty bootstrap config", serverOpts: []grpc.ServerOption{ grpc.Creds(xdsCreds), - func() grpc.ServerOption { - return BootstrapContentsForTesting([]byte(`{}`)) - }(), + ClientPoolForTesting(xdsclient.NewPool(nil)), }, wantErr: "xDS client creation failed", }, @@ -480,11 +478,9 @@ func (s) TestServeSuccess(t *testing.T) { // TestNewServer_ClientCreationFailure tests the case where the xDS client // creation fails and verifies that the call to NewGRPCServer() fails. func (s) TestNewServer_ClientCreationFailure(t *testing.T) { - origNewXDSClient := newXDSClient - newXDSClient = func(string) (xdsclient.XDSClient, func(), error) { - return nil, nil, errors.New("xdsClient creation failed") - } - defer func() { newXDSClient = origNewXDSClient }() + origXDSClientPool := xDSClientPool + xDSClientPool = xdsclient.NewPool(nil) + defer func() { xDSClientPool = origXDSClientPool }() if _, err := NewGRPCServer(); err == nil { t.Fatal("NewGRPCServer() succeeded when expected to fail")