From 6e6ee7114ef6559687971a520686856d6aa06485 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Mon, 4 Nov 2024 19:22:05 +0000 Subject: [PATCH 1/2] xds/resolver: fix flaky test TestResolverRemovedWithRPCs with a workaround --- xds/internal/resolver/xds_resolver_test.go | 34 +++++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index e5d569402ec8..7a31c3786555 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -548,10 +548,36 @@ func (s) TestResolverRemovedWithRPCs(t *testing.T) { } } - // Re-add the listener and expect everything to work again. - configureResourcesOnManagementServer(ctx, t, mgmtServer, nodeID, listeners, routes) - // Read the update pushed by the resolver to the ClientConn. - cs = verifyUpdateFromResolver(ctx, t, stateCh, wantDefaultServiceConfig) + // Workaround for https://github.com/envoyproxy/go-control-plane/issues/431. + // + // The xDS client can miss route configurations due to a race condition + // between resource removal and re-addition. To avoid this, continuously + // push new versions of the resources to the server, ensuring the client + // eventually receives the configuration. +waitForStateUpdate: + for { + sCtx, sCancel := context.WithTimeout(ctx, defaultTestShortTimeout) + defer sCancel() + + configureResourcesOnManagementServer(ctx, t, mgmtServer, nodeID, listeners, routes) + + select { + case state = <-stateCh: + if err := state.ServiceConfig.Err; err != nil { + t.Fatalf("Received error in service config: %v", state.ServiceConfig.Err) + } + wantSCParsed := internal.ParseServiceConfig.(func(string) *serviceconfig.ParseResult)(wantDefaultServiceConfig) + if !internal.EqualServiceConfigForTesting(state.ServiceConfig.Config, wantSCParsed.Config) { + t.Fatalf("Got service config:\n%s \nWant service config:\n%s", cmp.Diff(nil, state.ServiceConfig.Config), cmp.Diff(nil, wantSCParsed.Config)) + } + break waitForStateUpdate + case <-sCtx.Done(): + } + } + cs = iresolver.GetConfigSelector(state) + if cs == nil { + t.Fatal("Received nil config selector in update from resolver") + } res, err = cs.SelectConfig(iresolver.RPCInfo{Context: ctx, Method: "/service/method"}) if err != nil { From 6e49d131191ec0c6842c660fe2869bc07863f528 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Mon, 4 Nov 2024 23:24:59 +0000 Subject: [PATCH 2/2] add todo --- xds/internal/resolver/xds_resolver_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/xds/internal/resolver/xds_resolver_test.go b/xds/internal/resolver/xds_resolver_test.go index 7a31c3786555..77e8c47e6cd5 100644 --- a/xds/internal/resolver/xds_resolver_test.go +++ b/xds/internal/resolver/xds_resolver_test.go @@ -554,6 +554,9 @@ func (s) TestResolverRemovedWithRPCs(t *testing.T) { // between resource removal and re-addition. To avoid this, continuously // push new versions of the resources to the server, ensuring the client // eventually receives the configuration. + // + // TODO(https://github.com/grpc/grpc-go/issues/7807): Remove this workaround + // once the issue is fixed. waitForStateUpdate: for { sCtx, sCancel := context.WithTimeout(ctx, defaultTestShortTimeout)