Skip to content

Commit 8f48f60

Browse files
committed
fix flaky test
1 parent ccc8d7b commit 8f48f60

2 files changed

Lines changed: 82 additions & 1 deletion

File tree

internal/xds/balancer/cdsbalancer/aggregate_cluster_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package cdsbalancer
1818

1919
import (
2020
"context"
21+
"fmt"
2122
"strings"
2223
"testing"
2324
"time"
@@ -31,6 +32,7 @@ import (
3132
"google.golang.org/grpc/internal/testutils/xds/e2e"
3233
"google.golang.org/grpc/internal/xds/balancer/priority"
3334
"google.golang.org/grpc/internal/xds/xdsclient/xdsresource/version"
35+
"google.golang.org/grpc/resolver"
3436
"google.golang.org/grpc/serviceconfig"
3537
"google.golang.org/grpc/status"
3638

@@ -167,6 +169,7 @@ func (s) TestAggregateClusterSuccess_LeafNode(t *testing.T) {
167169
// LogicalDNS and verifies that the load balancing configuration pushed to the
168170
// priority LB policy contains the expected config.
169171
func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
172+
dnsTargetCh, dnsR := setupDNS(t)
170173
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
171174
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
172175

@@ -205,6 +208,20 @@ func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
205208
t.Fatal(err)
206209
}
207210

211+
// Ensure that the DNS resolver is started for the expected target.
212+
select {
213+
case <-ctx.Done():
214+
t.Fatal("Timeout waiting for DNS watch")
215+
case target := <-dnsTargetCh:
216+
addr := fmt.Sprintf("%s:%d", dnsHostName, dnsPort)
217+
if target.Endpoint() != addr {
218+
t.Fatalf("DNS resolution started for target %q, want %q", target.Endpoint(), addr)
219+
}
220+
dnsR.UpdateState(resolver.State{
221+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: addr}}}},
222+
})
223+
}
224+
208225
wantChildCfg := &priority.LBConfig{
209226
Children: map[string]*priority.Child{
210227
"priority-0-0": {
@@ -236,6 +253,18 @@ func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
236253
if err := mgmtServer.Update(ctx, resources); err != nil {
237254
t.Fatal(err)
238255
}
256+
257+
select {
258+
case <-ctx.Done():
259+
t.Fatal("Timeout waiting for DNS watch")
260+
case <-dnsTargetCh:
261+
addr1 := fmt.Sprintf("%s:%d", dnsHostName, dnsPort)
262+
addr2 := fmt.Sprintf("%s:%d", dnsHostNameNew, dnsPort)
263+
dnsR.UpdateState(resolver.State{
264+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: addr1}, {Addr: addr2}}}},
265+
})
266+
}
267+
239268
wantChildCfg = &priority.LBConfig{
240269
Children: map[string]*priority.Child{
241270
"priority-0-0": {
@@ -260,6 +289,7 @@ func (s) TestAggregateClusterSuccess_ThenUpdateChildClusters(t *testing.T) {
260289
// configuration pushed to the priority LB policy contains a single discovery
261290
// mechanism.
262291
func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
292+
dnsTargetCh, dnsR := setupDNS(t)
263293
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
264294
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
265295

@@ -282,6 +312,20 @@ func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
282312
t.Fatal(err)
283313
}
284314

315+
// Ensure that the DNS resolver is started for the expected target.
316+
select {
317+
case <-ctx.Done():
318+
t.Fatal("Timeout waiting for DNS watch")
319+
case target := <-dnsTargetCh:
320+
addr := fmt.Sprintf("%s:%d", dnsHostName, dnsPort)
321+
if target.Endpoint() != addr {
322+
t.Fatalf("DNS resolution started for target %q, want %q", target.Endpoint(), addr)
323+
}
324+
dnsR.UpdateState(resolver.State{
325+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: addr}}}},
326+
})
327+
}
328+
285329
wantChildCfg := &priority.LBConfig{
286330
Children: map[string]*priority.Child{
287331
"priority-0-0": {
@@ -330,6 +374,7 @@ func (s) TestAggregateClusterSuccess_ThenChangeRootToEDS(t *testing.T) {
330374
// cluster. In each of these cases, the test verifies that the load balancing
331375
// configuration pushed to the priority LB policy contains the expected config.
332376
func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T) {
377+
dnsTargetCh, dnsR := setupDNS(t)
333378
lbCfgCh, _, _, _ := registerWrappedPriorityPolicy(t)
334379
mgmtServer, nodeID, _ := setupWithManagementServer(t, nil, nil)
335380

@@ -375,6 +420,21 @@ func (s) TestAggregatedClusterSuccess_SwitchBetweenLeafAndAggregate(t *testing.T
375420
if err := mgmtServer.Update(ctx, resources); err != nil {
376421
t.Fatal(err)
377422
}
423+
424+
// Ensure that the DNS resolver is started for the expected target.
425+
select {
426+
case <-ctx.Done():
427+
t.Fatal("Timeout waiting for DNS watch")
428+
case target := <-dnsTargetCh:
429+
addr := fmt.Sprintf("%s:%d", dnsHostName, dnsPort)
430+
if target.Endpoint() != addr {
431+
t.Fatalf("DNS resolution started for target %q, want %q", target.Endpoint(), addr)
432+
}
433+
dnsR.UpdateState(resolver.State{
434+
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: addr}}}},
435+
})
436+
}
437+
378438
wantChildCfg = &priority.LBConfig{
379439
Children: map[string]*priority.Child{
380440
"priority-0-0": {

internal/xds/balancer/cdsbalancer/cdsbalancer_test.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ const (
8181
host = "localhost"
8282
port = 8080
8383
dnsPort = uint32(8080)
84-
defaultTestTimeout = 5 * time.Second
84+
defaultTestTimeout = 10 * time.Second
8585
defaultTestShortTimeout = 10 * time.Millisecond // For events expected to *not* happen.
8686
)
8787

@@ -164,6 +164,27 @@ func registerWrappedPriorityPolicy(t *testing.T) (chan serviceconfig.LoadBalanci
164164
return lbCfgCh, resolverErrCh, exitIdleCh, closeCh
165165
}
166166

167+
// setupDNS unregisters the DNS resolver and registers a manual resolver for the
168+
// same scheme. This allows the test to mock the DNS resolution by supplying the
169+
// addresses of the test backends.
170+
//
171+
// Returns the following:
172+
// - a channel onto which the DNS target being resolved is written to by the
173+
// mock DNS resolver
174+
// - a manual resolver which is used to mock the actual DNS resolution
175+
func setupDNS(t *testing.T) (chan resolver.Target, *manual.Resolver) {
176+
targetCh := make(chan resolver.Target, 1)
177+
178+
mr := manual.NewBuilderWithScheme("dns")
179+
mr.BuildCallback = func(target resolver.Target, _ resolver.ClientConn, _ resolver.BuildOptions) { targetCh <- target }
180+
181+
dnsResolverBuilder := resolver.Get("dns")
182+
resolver.Register(mr)
183+
184+
t.Cleanup(func() { resolver.Register(dnsResolverBuilder) })
185+
return targetCh, mr
186+
}
187+
167188
// Performs the following setup required for tests:
168189
// - Spins up an xDS management server and the provided onStreamRequest
169190
// function is set to be called for every incoming request on the ADS stream.

0 commit comments

Comments
 (0)