Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clusterresolver: remove redundant tests #6388

Merged
merged 2 commits into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 0 additions & 78 deletions xds/internal/balancer/clusterresolver/clusterresolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,84 +240,6 @@ func (s) TestSubConnStateChange(t *testing.T) {
}
}

// Given a list of resource names, verifies that EDS requests for the same are
// sent by the EDS balancer, through the fake xDS client.
func verifyExpectedRequests(ctx context.Context, fc *fakeclient.Client, resourceNames ...string) error {
for _, name := range resourceNames {
if name == "" {
// ResourceName empty string indicates a cancel.
if _, err := fc.WaitForCancelEDSWatch(ctx); err != nil {
return fmt.Errorf("timed out when expecting resource %q", name)
}
continue
}

resName, err := fc.WaitForWatchEDS(ctx)
if err != nil {
return fmt.Errorf("timed out when expecting resource %q, %p", name, fc)
}
if resName != name {
return fmt.Errorf("got EDS request for resource %q, expected: %q", resName, name)
}
}
return nil
}

// TestClientWatchEDS verifies that the xdsClient inside the top-level EDS LB
// policy registers an EDS watch for expected resource upon receiving an update
// from gRPC.
func (s) TestClientWatchEDS(t *testing.T) {
edsLBCh := testutils.NewChannel()
xdsC, cleanup := setup(edsLBCh)
defer cleanup()

builder := balancer.Get(Name)
edsB := builder.Build(newNoopTestClientConn(), balancer.BuildOptions{})
if edsB == nil {
t.Fatalf("builder.Build(%s) failed and returned nil", Name)
}
defer edsB.Close()

ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
// If eds service name is not set, should watch for cluster name.
if err := edsB.UpdateClientConnState(balancer.ClientConnState{
ResolverState: xdsclient.SetClient(resolver.State{}, xdsC),
BalancerConfig: newLBConfigWithOneEDS("cluster-1"),
}); err != nil {
t.Fatal(err)
}
if err := verifyExpectedRequests(ctx, xdsC, "cluster-1"); err != nil {
t.Fatal(err)
}

// Update with an non-empty edsServiceName should trigger an EDS watch for
// the same.
if err := edsB.UpdateClientConnState(balancer.ClientConnState{
ResolverState: xdsclient.SetClient(resolver.State{}, xdsC),
BalancerConfig: newLBConfigWithOneEDS("foobar-1"),
}); err != nil {
t.Fatal(err)
}
if err := verifyExpectedRequests(ctx, xdsC, "", "foobar-1"); err != nil {
t.Fatal(err)
}

// Also test the case where the edsServerName changes from one non-empty
// name to another, and make sure a new watch is registered. The previously
// registered watch will be cancelled, which will result in an EDS request
// with no resource names being sent to the server.
if err := edsB.UpdateClientConnState(balancer.ClientConnState{
ResolverState: xdsclient.SetClient(resolver.State{}, xdsC),
BalancerConfig: newLBConfigWithOneEDS("foobar-2"),
}); err != nil {
t.Fatal(err)
}
if err := verifyExpectedRequests(ctx, xdsC, "", "foobar-2"); err != nil {
t.Fatal(err)
}
}

func newLBConfigWithOneEDS(edsServiceName string) *LBConfig {
return &LBConfig{
DiscoveryMechanisms: []DiscoveryMechanism{{
Expand Down
105 changes: 0 additions & 105 deletions xds/internal/balancer/clusterresolver/priority_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -832,108 +832,3 @@ func (s) TestEDSPriority_FirstPriorityRemoved(t *testing.T) {
t.Fatal(err)
}
}

// Watch resources from EDS and DNS, with EDS as the higher priority. Lower
// priority is used when higher priority is not ready.
func (s) TestFallbackToDNS(t *testing.T) {
const testDNSEndpointAddr = "3.1.4.1:5"
// dnsTargetCh, dnsCloseCh, resolveNowCh, dnsR, cleanup := setupDNS()
dnsTargetCh, _, resolveNowCh, dnsR, cleanupDNS := setupDNS()
defer cleanupDNS()
edsb, cc, xdsC, cleanup := setupTestEDS(t, nil)
defer cleanup()

if err := edsb.UpdateClientConnState(balancer.ClientConnState{
BalancerConfig: &LBConfig{
DiscoveryMechanisms: []DiscoveryMechanism{
{
Type: DiscoveryMechanismTypeEDS,
Cluster: testClusterName,
},
{
Type: DiscoveryMechanismTypeLogicalDNS,
DNSHostname: testDNSTarget,
},
},
xdsLBPolicy: *wrrLocalityLBConfig,
},
}); err != nil {
t.Fatal(err)
}

ctx, ctxCancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer ctxCancel()
select {
case target := <-dnsTargetCh:
if diff := cmp.Diff(target, resolver.Target{Scheme: "dns", URL: *testutils.MustParseURL("dns:///" + testDNSTarget)}); diff != "" {
t.Fatalf("got unexpected DNS target to watch, diff (-got, +want): %v", diff)
}
case <-ctx.Done():
t.Fatal("Timed out waiting for building DNS resolver")
}

// One locality with one backend.
clab1 := xdstestutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil)
clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil)
xdsC.InvokeWatchEDSCallback("", parseEDSRespProtoForTesting(clab1.Build()), nil)

// Also send a DNS update, because the balancer needs both updates from all
// resources to move on.
dnsR.UpdateState(resolver.State{Addresses: []resolver.Address{{Addr: testDNSEndpointAddr}}})

addrs0 := <-cc.NewSubConnAddrsCh
if got, want := addrs0[0].Addr, testEndpointAddrs[0]; got != want {
t.Fatalf("sc is created with addr %v, want %v", got, want)
}
sc0 := <-cc.NewSubConnCh

// p0 is ready.
edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Connecting})
edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Ready})

// Test roundrobin with only p0 subconns.
if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil {
t.Fatal(err)
}

// Turn down 0, p1 (DNS) will be used.
edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})

// The transient failure above should not trigger a re-resolve to the DNS
// resolver. Need to read to clear the channel, to avoid potential deadlock
// writing to the channel later.
shortCtx, shortCancel := context.WithTimeout(context.Background(), defaultTestShortTimeout)
defer shortCancel()
select {
case <-resolveNowCh:
t.Fatal("unexpected re-resolve trigger by transient failure from EDS endpoint")
case <-shortCtx.Done():
}

// The addresses used to create new SubConn should be the DNS endpoint.
addrs1 := <-cc.NewSubConnAddrsCh
if got, want := addrs1[0].Addr, testDNSEndpointAddr; got != want {
t.Fatalf("sc is created with addr %v, want %v", got, want)
}
sc1 := <-cc.NewSubConnCh
edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Connecting})
edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready})

// Test pick with 1.
if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil {
t.Fatal(err)
}

// Turn down the DNS endpoint, this should trigger an re-resolve in the DNS
// resolver.
edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure})

// The transient failure above should trigger a re-resolve to the DNS
// resolver. Need to read to clear the channel, to avoid potential deadlock
// writing to the channel later.
select {
case <-resolveNowCh:
case <-ctx.Done():
t.Fatal("Timed out waiting for re-resolve")
}
}
59 changes: 0 additions & 59 deletions xds/internal/balancer/clusterresolver/resource_resolver_test.go

This file was deleted.