Skip to content

Commit

Permalink
use pool everywhere instead of new client functions
Browse files Browse the repository at this point in the history
  • Loading branch information
purnesh42H committed Dec 21, 2024
1 parent 83c13ad commit 6295269
Show file tree
Hide file tree
Showing 36 changed files with 298 additions and 276 deletions.
8 changes: 4 additions & 4 deletions xds/csds/csds_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
Expand All @@ -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,
})
Expand Down Expand Up @@ -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,
})
Expand All @@ -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,
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
Expand Down
10 changes: 6 additions & 4 deletions xds/internal/balancer/cdsbalancer/cdsbalancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
})
Expand Down
36 changes: 22 additions & 14 deletions xds/internal/balancer/clusterresolver/e2e_test/eds_impl_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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(),
})
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions xds/internal/resolver/xds_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions xds/internal/resolver/xds_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions xds/internal/server/rds_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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(),
})
Expand Down
8 changes: 4 additions & 4 deletions xds/internal/testutils/fakeclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
55 changes: 7 additions & 48 deletions xds/internal/xdsclient/client_new.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit 6295269

Please sign in to comment.