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 20, 2024
1 parent 6785955 commit a6f39ea
Show file tree
Hide file tree
Showing 24 changed files with 121 additions and 116 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
7 changes: 6 additions & 1 deletion xds/internal/resolver/xds_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ const Scheme = "xds"
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
}
xdsclient.DefaultPool.SetFallbackBootstrapConfig(config)
return xdsclient.DefaultPool.NewClientForTesting(xdsclient.OptionsForTesting{Name: name})
},
}, nil
}
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
27 changes: 18 additions & 9 deletions xds/internal/xdsclient/client_refcounted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,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"
)

// Tests that multiple calls to New() with the same name returns the same
Expand All @@ -36,7 +37,11 @@ func (s) TestClientNew_Single(t *testing.T) {
// directory, and set the bootstrap env vars to point to it.
nodeID := uuid.New().String()
contents := e2e.DefaultBootstrapContents(t, nodeID, "non-existent-server-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)
}
pool := NewPool(config)

// Override the client creation hook to get notified.
origClientImplCreateHook := xdsClientImplCreateHook
Expand All @@ -55,7 +60,7 @@ func (s) TestClientNew_Single(t *testing.T) {
defer func() { xdsClientImplCloseHook = origClientImplCloseHook }()

// The first call to New() should create a new client.
_, closeFunc, err := DefaultPool.NewClient(t.Name())
_, closeFunc, err := pool.NewClient(t.Name())
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
Expand All @@ -71,7 +76,7 @@ func (s) TestClientNew_Single(t *testing.T) {
closeFuncs := make([]func(), count)
for i := 0; i < count; i++ {
func() {
_, closeFuncs[i], err = DefaultPool.NewClient(t.Name())
_, closeFuncs[i], err = pool.NewClient(t.Name())
if err != nil {
t.Fatalf("%d-th call to New() failed with error: %v", i, err)
}
Expand Down Expand Up @@ -109,7 +114,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 = DefaultPool.NewClient(t.Name())
_, closeFunc, err = pool.NewClient(t.Name())
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
Expand All @@ -127,7 +132,11 @@ func (s) TestClientNew_Multiple(t *testing.T) {
// directory, and set the bootstrap env vars to point to it.
nodeID := uuid.New().String()
contents := e2e.DefaultBootstrapContents(t, nodeID, "non-existent-server-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)
}
pool := NewPool(config)

// Override the client creation hook to get notified.
origClientImplCreateHook := xdsClientImplCreateHook
Expand All @@ -147,7 +156,7 @@ func (s) TestClientNew_Multiple(t *testing.T) {

// Create two xDS clients.
client1Name := t.Name() + "-1"
_, closeFunc1, err := DefaultPool.NewClient(client1Name)
_, closeFunc1, err := pool.NewClient(client1Name)
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
Expand All @@ -162,7 +171,7 @@ func (s) TestClientNew_Multiple(t *testing.T) {
}

client2Name := t.Name() + "-2"
_, closeFunc2, err := DefaultPool.NewClient(client2Name)
_, closeFunc2, err := pool.NewClient(client2Name)
if err != nil {
t.Fatalf("Failed to create xDS client: %v", err)
}
Expand All @@ -184,7 +193,7 @@ func (s) TestClientNew_Multiple(t *testing.T) {
defer wg.Done()
for i := 0; i < count; i++ {
var err error
_, closeFuncs1[i], err = DefaultPool.NewClient(client1Name)
_, closeFuncs1[i], err = pool.NewClient(client1Name)
if err != nil {
t.Errorf("%d-th call to New() failed with error: %v", i, err)
}
Expand All @@ -194,7 +203,7 @@ func (s) TestClientNew_Multiple(t *testing.T) {
defer wg.Done()
for i := 0; i < count; i++ {
var err error
_, closeFuncs2[i], err = DefaultPool.NewClient(client2Name)
_, closeFuncs2[i], err = pool.NewClient(client2Name)
if err != nil {
t.Errorf("%d-th call to New() failed with error: %v", i, err)
}
Expand Down
7 changes: 0 additions & 7 deletions xds/internal/xdsclient/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,13 +88,6 @@ func (p *Pool) NewClientForTesting(opts OptionsForTesting) (XDSClient, func(), e
if opts.StreamBackoffAfterFailure == nil {
opts.StreamBackoffAfterFailure = defaultStreamBackoffFunc
}
if opts.Contents != nil {
config, err := bootstrap.NewConfigForTesting(opts.Contents)
if err != nil {
return nil, nil, err
}
return p.newRefCounted(opts.Name, config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure)
}
return p.newRefCounted(opts.Name, p.config, opts.WatchExpiryTimeout, opts.StreamBackoffAfterFailure)
}

Expand Down
3 changes: 0 additions & 3 deletions xds/internal/xdsclient/tests/ads_stream_ack_nack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ func (s) TestADS_ACK_NACK_Simple(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)
client := createXDSClient(t, bc)

// Register a watch for a listener resource.
Expand Down Expand Up @@ -273,7 +272,6 @@ func (s) TestADS_NACK_InvalidFirstResponse(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)
client := createXDSClient(t, bc)

// Register a watch for a listener resource.
Expand Down Expand Up @@ -381,7 +379,6 @@ 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)
config, err := bootstrap.NewConfigForTesting(bc)
if err != nil {
t.Fatalf("Failed to create an bootstrap config from contents: %v, %v", bc, err)
Expand Down
3 changes: 0 additions & 3 deletions xds/internal/xdsclient/tests/ads_stream_backoff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ func (s) TestADS_BackoffAfterStreamFailure(t *testing.T) {
// Create an xDS client with bootstrap pointing to the above server.
nodeID := uuid.New().String()
bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, bc)
client := createXDSClientWithBackoff(t, bc, streamBackoff)

// Register a watch for a listener resource.
Expand Down Expand Up @@ -226,7 +225,6 @@ func (s) TestADS_RetriesAfterBrokenStream(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)
client := createXDSClientWithBackoff(t, bc, streamBackoff)

// Register a watch for a listener resource.
Expand Down Expand Up @@ -388,7 +386,6 @@ func (s) TestADS_ResourceRequestedBeforeStreamCreation(t *testing.T) {
// Create an xDS client with bootstrap pointing to the above server.
nodeID := uuid.New().String()
bc := e2e.DefaultBootstrapContents(t, nodeID, mgmtServer.Address)
testutils.CreateBootstrapFileForTesting(t, bc)
client := createXDSClientWithBackoff(t, bc, streamBackoff)

// Register a watch for a listener resource.
Expand Down
Loading

0 comments on commit a6f39ea

Please sign in to comment.