Fix flaky test `TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate` in aggregate_cluster_test.go. (#9009)
Fixes #8989
This PR addresses the flakiness observed in the
`TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate ` test. Test
was using real DNS resolution which introduced non-deterministic delays
and race conditions during testing.
### Changes:
* **Mocked DNS Resolver**: Introduced a `setupDNS` helper function in
`cdsbalancer_test.go` that unregisters the default DNS resolver and
registers a manual resolver for the `dns` scheme. This allows us to
intercept DNS targets and provide mock addresses immediately.
* **Updated Tests**: Updated the following tests in
`aggregate_cluster_test.go` to use the fake DNS resolver:
* `TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate`
* `TestAggregateClusterSuccess_ThenUpdateChildClusters`
* `TestAggregateClusterSuccess_ThenChangeRootToEDS`
* **Adjusted Test Timeout**: Increased the `defaultTestTimeout` in
`cdsbalancer_test.go` from `5s` to `10s` to align with the standard test
timeout practices used across the `grpc-go` codebase.
### Testing/Validation:
* Successfully reproduced the flakiness locally by manually injecting a
sleep delay to simulate a slow real DNS resolution update in
`TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate`.
* Validated that applying the mock DNS resolver completely resolves the
flakiness.
RELEASE NOTES: N/Adiff --git a/internal/xds/balancer/cdsbalancer/aggregate_cluster_test.go b/internal/xds/balancer/cdsbalancer/aggregate_cluster_test.go
index 5dfbd6a..9cb040d 100644
--- a/internal/xds/balancer/cdsbalancer/aggregate_cluster_test.go
+++ b/internal/xds/balancer/cdsbalancer/aggregate_cluster_test.go
@@ -18,6 +18,7 @@
import (
"context"
+ "fmt"
"strings"
"testing"
"time"
@@ -31,6 +32,8 @@
"google.golang.org/grpc/internal/testutils/xds/e2e"
"google.golang.org/grpc/internal/xds/balancer/priority"
"google.golang.org/grpc/internal/xds/xdsclient/xdsresource/version"
+ "google.golang.org/grpc/resolver"
+ "google.golang.org/grpc/resolver/manual"
"google.golang.org/grpc/serviceconfig"
"google.golang.org/grpc/status"
@@ -68,6 +71,23 @@
})
}
+// verifyDNSResolution verifies that the DNS resolver is started for the expected target.
+func verifyDNSResolution(ctx context.Context, t *testing.T, dnsTargetCh chan resolver.Target, dnsR *manual.Resolver, host string, port uint32) {
+ t.Helper()
+ select {
+ case <-ctx.Done():
+ t.Fatal("Timeout waiting for DNS watch")
+ case target := <-dnsTargetCh:
+ addr := fmt.Sprintf("%s:%d", host, port)
+ if target.Endpoint() != addr {
+ t.Fatalf("DNS resolution started for target %q, want %q", target.Endpoint(), addr)
+ }
+ dnsR.UpdateState(resolver.State{
+ Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: addr}}}},
+ })
+ }
+}
+
// Tests the case where the cluster resource requested is a leaf cluster. The
// management server sends two updates for the same leaf cluster resource. The
// test verifies that the load balancing configuration pushed to the priority LB
@@ -167,6 +187,7 @@
// LogicalDNS and verifies that the load balancing configuration pushed to the
// priority LB policy contains the expected config.
func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
+ dnsTargetCh, dnsR := setupDNS(t)
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
@@ -204,6 +225,7 @@
if err := mgmtServer.Update(ctx, resources); err != nil {
t.Fatal(err)
}
+ verifyDNSResolution(ctx, t, dnsTargetCh, dnsR, dnsHostName, dnsPort)
wantChildCfg := &priority.LBConfig{
Children: map[string]*priority.Child{
@@ -236,6 +258,8 @@
if err := mgmtServer.Update(ctx, resources); err != nil {
t.Fatal(err)
}
+ verifyDNSResolution(ctx, t, dnsTargetCh, dnsR, dnsHostNameNew, dnsPort)
+
wantChildCfg = &priority.LBConfig{
Children: map[string]*priority.Child{
"priority-0-0": {
@@ -260,6 +284,7 @@
// configuration pushed to the priority LB policy contains a single discovery
// mechanism.
func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
+ dnsTargetCh, dnsR := setupDNS(t)
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
@@ -281,6 +306,7 @@
if err := mgmtServer.Update(ctx, resources); err != nil {
t.Fatal(err)
}
+ verifyDNSResolution(ctx, t, dnsTargetCh, dnsR, dnsHostName, dnsPort)
wantChildCfg := &priority.LBConfig{
Children: map[string]*priority.Child{
@@ -330,6 +356,7 @@
// cluster. In each of these cases, the test verifies that the load balancing
// configuration pushed to the priority LB policy contains the expected config.
func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T) {
+ dnsTargetCh, dnsR := setupDNS(t)
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
@@ -375,6 +402,8 @@
if err := mgmtServer.Update(ctx, resources); err != nil {
t.Fatal(err)
}
+ verifyDNSResolution(ctx, t, dnsTargetCh, dnsR, dnsHostName, dnsPort)
+
wantChildCfg = &priority.LBConfig{
Children: map[string]*priority.Child{
"priority-0-0": {
diff --git a/internal/xds/balancer/cdsbalancer/cdsbalancer_test.go b/internal/xds/balancer/cdsbalancer/cdsbalancer_test.go
index 965e662..7e1a9b2 100644
--- a/internal/xds/balancer/cdsbalancer/cdsbalancer_test.go
+++ b/internal/xds/balancer/cdsbalancer/cdsbalancer_test.go
@@ -81,7 +81,7 @@
host = "localhost"
port = 8080
dnsPort = uint32(8080)
- defaultTestTimeout = 5 * time.Second
+ defaultTestTimeout = 10 * time.Second
defaultTestShortTimeout = 10 * time.Millisecond // For events expected to *not* happen.
)
@@ -164,6 +164,27 @@
return lbCfgCh, resolverErrCh, exitIdleCh, closeCh
}
+// setupDNS unregisters the DNS resolver and registers a manual resolver for the
+// same scheme. This allows the test to fake the DNS resolution by supplying the
+// addresses of the test backends.
+//
+// Returns the following:
+// - a channel onto which the DNS target being resolved is written to by the
+// fake DNS resolver
+// - a manual resolver which is used to fake the actual DNS resolution
+func setupDNS(t *testing.T) (chan resolver.Target, *manual.Resolver) {
+ targetCh := make(chan resolver.Target, 1)
+
+ mr := manual.NewBuilderWithScheme("dns")
+ mr.BuildCallback = func(target resolver.Target, _ resolver.ClientConn, _ resolver.BuildOptions) { targetCh <- target }
+
+ dnsResolverBuilder := resolver.Get("dns")
+ resolver.Register(mr)
+
+ t.Cleanup(func() { resolver.Register(dnsResolverBuilder) })
+ return targetCh, mr
+}
+
// Performs the following setup required for tests:
// - Spins up an xDS management server and the provided onStreamRequest
// function is set to be called for every incoming request on the ADS stream.
