Skip to content

Commit

Permalink
Fix breaking tests
Browse files Browse the repository at this point in the history
  • Loading branch information
purnesh42H committed Dec 21, 2024
1 parent 6295269 commit d11cf5f
Show file tree
Hide file tree
Showing 26 changed files with 141 additions and 132 deletions.
25 changes: 16 additions & 9 deletions xds/csds/csds_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"google.golang.org/grpc/internal/pretty"
"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/csds"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
Expand Down Expand Up @@ -220,22 +221,25 @@ func (s) TestCSDS(t *testing.T) {
// Create a bootstrap contents pointing to the above management server.
nodeID := uuid.New().String()
bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)

config, err := bootstrap.NewConfigForTesting(bootstrapContents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)
// 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.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{
Name: xdsClient1Name,
Contents: bootstrapContents,
Name: xdsClient1Name,
})
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
defer xdsClose1()
const xdsClient2Name = "xds-csds-client-2"
xdsClient2, xdsClose2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{
Name: xdsClient2Name,
Contents: bootstrapContents,
Name: xdsClient2Name,
})
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
Expand Down Expand Up @@ -417,22 +421,25 @@ func (s) TestCSDS_NACK(t *testing.T) {
// Create a bootstrap contents pointing to the above management server.
nodeID := uuid.New().String()
bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
config, err := bootstrap.NewConfigForTesting(bootstrapContents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

// 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.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{
Name: xdsClient1Name,
Contents: bootstrapContents,
Name: xdsClient1Name,
})
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
defer xdsClose1()
const xdsClient2Name = "xds-csds-client-2"
xdsClient2, xdsClose2, err := xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{
Name: xdsClient2Name,
Contents: bootstrapContents,
Name: xdsClient2Name,
})
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
Expand Down
10 changes: 8 additions & 2 deletions xds/internal/balancer/clusterimpl/tests/balancer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ import (
"google.golang.org/grpc/internal/testutils"
"google.golang.org/grpc/internal/testutils/xds/e2e"
"google.golang.org/grpc/internal/testutils/xds/fakeserver"
"google.golang.org/grpc/internal/xds/bootstrap"
"google.golang.org/grpc/peer"
"google.golang.org/grpc/resolver"
"google.golang.org/grpc/status"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/protobuf/types/known/durationpb"

v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
Expand Down Expand Up @@ -77,13 +79,17 @@ func (s) TestConfigUpdateWithSameLoadReportingServerConfig(t *testing.T) {
mgmtServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{SupportLoadReportingService: true})

// Create bootstrap configuration pointing to the above management server.

nodeID := uuid.New().String()
bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, bc)
config, err := bootstrap.NewConfigForTesting(bc)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

// Create an xDS resolver with the above bootstrap configuration.
var resolverBuilder resolver.Builder
var err error
if newResolver := internal.NewXDSResolverWithConfigForTesting; newResolver != nil {
resolverBuilder, err = newResolver.(func([]byte) (resolver.Builder, error))(bc)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func (s) TestConfigUpdate_ChildPolicyChange(t *testing.T) {
mgmtServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{})

// Create bootstrap configuration pointing to the above management server.

nodeID := uuid.New().String()
bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, bc)
Expand Down
8 changes: 7 additions & 1 deletion xds/internal/httpfilter/fault/fault_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,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/metadata"
"google.golang.org/grpc/status"
"google.golang.org/protobuf/types/known/durationpb"
Expand All @@ -56,6 +57,7 @@ import (
_ "google.golang.org/grpc/xds/internal/balancer" // Register the balancers.
_ "google.golang.org/grpc/xds/internal/httpfilter/router" // Register the router filter.
_ "google.golang.org/grpc/xds/internal/resolver" // Register the xds_resolver.
"google.golang.org/grpc/xds/internal/xdsclient"
)

const defaultTestTimeout = 10 * time.Second
Expand Down Expand Up @@ -86,7 +88,11 @@ func clientSetup(t *testing.T) (*e2e.ManagementServer, string, uint32) {

// Create a bootstrap file in a temporary directory.
bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, managementServer.Address)
testutils.CreateBootstrapFileForTesting(t, bootstrapContents)
config, err := bootstrap.NewConfigForTesting(bootstrapContents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

// Create a local listener.
lis, err := testutils.LocalTCPListener()
Expand Down
8 changes: 7 additions & 1 deletion xds/internal/resolver/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ import (
iresolver "google.golang.org/grpc/internal/resolver"
"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/serviceconfig"
xdsresolver "google.golang.org/grpc/xds/internal/resolver"
"google.golang.org/grpc/xds/internal/xdsclient"
"google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version"

v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3"
Expand Down Expand Up @@ -224,7 +226,11 @@ func setupManagementServerForTest(ctx context.Context, t *testing.T, nodeID stri

// Create a bootstrap configuration specifying the above management server.
bootstrapContents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, bootstrapContents)
config, err := bootstrap.NewConfigForTesting(bootstrapContents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bootstrapContents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)
return mgmtServer, listenerResourceNamesCh, routeConfigResourceNamesCh
}

Expand Down
13 changes: 11 additions & 2 deletions xds/internal/resolver/xds_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,21 @@ import (
// xdsresolver.Scheme
const Scheme = "xds"

var xdsClientPool *xdsclient.Pool

// newBuilderWithConfigForTesting creates a new xds resolver builder using a
// specific xds bootstrap config, so tests can use multiple xds clients in
// different ClientConns at the same time. It creates the new xds client in
// the default pool with provided config.
func newBuilderWithConfigForTesting(config []byte) (resolver.Builder, error) {
return &xdsResolverBuilder{
newXDSClient: func(name string) (xdsclient.XDSClient, func(), error) {
return xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{Name: name, Contents: config})
config, err := bootstrap.NewConfigForTesting(config)
if err != nil {
return nil, nil, err
}
xdsClientPool = xdsclient.NewPool(config)
return xdsClientPool.NewClientForTesting(xdsclient.OptionsForTesting{Name: name})
},
}, nil
}
Expand All @@ -74,8 +81,10 @@ func init() {
internal.NewXDSResolverWithConfigForTesting = newBuilderWithConfigForTesting
internal.NewXDSResolverWithClientForTesting = newBuilderWithClientForTesting

xdsClientPool = xdsclient.DefaultPool

rinternal.NewWRR = wrr.NewRandom
rinternal.NewXDSClient = xdsclient.DefaultPool.NewClient
rinternal.NewXDSClient = func(name string) (xdsclient.XDSClient, func(), error) { return xdsClientPool.NewClient(name) }
}

type xdsResolverBuilder struct {
Expand Down
18 changes: 15 additions & 3 deletions xds/internal/resolver/xds_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ func (s) TestResolverBuilder_ClientCreationFails_NoBootstrap(t *testing.T) {
// fails with the expected error string.
func (s) TestResolverBuilder_AuthorityNotDefinedInBootstrap(t *testing.T) {
contents := e2e.DefaultBootstrapContents(t, "node-id", "dummy-management-server")
testutils.CreateBootstrapFileForTesting(t, contents)
config, err := bootstrap.NewConfigForTesting(contents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", contents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

builder := resolver.Get(xdsresolver.Scheme)
if builder == nil {
Expand Down Expand Up @@ -183,7 +187,11 @@ func (s) TestResolverResourceName(t *testing.T) {
if err != nil {
t.Fatalf("Failed to create bootstrap configuration: %v", err)
}
testutils.CreateBootstrapFileForTesting(t, contents)
config, err := bootstrap.NewConfigForTesting(contents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", contents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

buildResolverForTarget(t, resolver.Target{URL: *testutils.MustParseURL(tt.dialTarget)})
waitForResourceNames(ctx, t, lisCh, tt.wantResourceNames)
Expand Down Expand Up @@ -222,7 +230,11 @@ func (s) TestResolverWatchCallbackAfterClose(t *testing.T) {
// Create a bootstrap configuration specifying the above management server.
nodeID := uuid.New().String()
contents := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, contents)
config, err := bootstrap.NewConfigForTesting(contents)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", contents, err)
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)

// Configure resources on the management server.
listeners := []*v3listenerpb.Listener{e2e.DefaultClientListener(defaultTestServiceName, defaultTestRouteConfigName)}
Expand Down
4 changes: 0 additions & 4 deletions xds/internal/xdsclient/client_new.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,6 @@ type OptionsForTesting struct {
// Name is a unique name for this xDS client.
Name string

// Contents contain a JSON representation of the bootstrap configuration to
// be used when creating the xDS client.
Contents []byte

// WatchExpiryTimeout is the timeout for xDS resource watch expiry. If
// unspecified, uses the default value used in non-test code.
WatchExpiryTimeout time.Duration
Expand Down
28 changes: 14 additions & 14 deletions xds/internal/xdsclient/client_refcounted.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,11 @@ var (
defaultStreamBackoffFunc = backoff.DefaultExponential.Backoff
)

func (pool *Pool) clientRefCountedClose(name string) {
pool.mu.Lock()
defer pool.mu.Unlock()
func (p *Pool) clientRefCountedClose(name string) {
p.mu.Lock()
defer p.mu.Unlock()

client, ok := pool.clients[name]
client, ok := p.clients[name]
if !ok {
logger.Errorf("Attempt to close a non-existent xDS client with name %s", name)
return
Expand All @@ -52,37 +52,37 @@ func (pool *Pool) clientRefCountedClose(name string) {
}
client.clientImpl.close()
xdsClientImplCloseHook(name)
delete(pool.clients, name)
delete(p.clients, name)

}

// 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 (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()
func (p *Pool) newRefCounted(name string, config *bootstrap.Config, watchExpiryTimeout time.Duration, streamBackoff func(int) time.Duration) (XDSClient, func(), error) {
p.mu.Lock()
defer p.mu.Unlock()

if c := pool.clients[name]; c != nil {
if c := p.clients[name]; c != nil {
c.incrRef()
return c, grpcsync.OnceFunc(func() { pool.clientRefCountedClose(name) }), nil
return c, grpcsync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
}

// Create the new client implementation.
if pool.config == nil {
pool.config = config
if p.config == nil {
p.config = config
}
c, err := newClientImpl(config, watchExpiryTimeout, streamBackoff)
if err != nil {
return nil, nil, err
}
c.logger.Infof("Created client with name %q and bootstrap configuration:\n %s", name, config)
client := &clientRefCounted{clientImpl: c, refCount: 1}
pool.clients[name] = client
p.clients[name] = client
xdsClientImplCreateHook(name)

logger.Infof("xDS node ID: %s", config.Node().GetId())
return client, grpcsync.OnceFunc(func() { pool.clientRefCountedClose(name) }), nil
return client, grpcsync.OnceFunc(func() { p.clientRefCountedClose(name) }), nil
}

// clientRefCounted is ref-counted, and to be shared by the xds resolver and
Expand Down
Loading

0 comments on commit d11cf5f

Please sign in to comment.