From 9fde4a846a0bb6271e8de6feb401367d895975fd Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Mon, 26 Aug 2024 15:13:28 +0530 Subject: [PATCH 1/2] Avoid making a DNS request in aggregated_cluster_test --- .../balancer/clusterresolver/e2e_test/aggregate_cluster_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go index e3cf2a99d5f6..0b5b2b02b3a0 100644 --- a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go +++ b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go @@ -862,7 +862,7 @@ func (s) TestAggregateCluster_BadDNS_GoodEDS(t *testing.T) { NodeID: nodeID, Clusters: []*v3clusterpb.Cluster{ makeAggregateClusterResource(clusterName, []string{dnsClusterName, edsClusterName}), - makeLogicalDNSClusterResource(dnsClusterName, "bad.ip.v4.address", 8080), + makeLogicalDNSClusterResource(dnsClusterName, "bad%ipv4%address", 8080), // Invalid URL to avoid making a DNS request. e2e.DefaultCluster(edsClusterName, edsServiceName, e2e.SecurityLevelNone), }, Endpoints: []*v3endpointpb.ClusterLoadAssignment{e2e.DefaultEndpoint(edsServiceName, "localhost", []uint32{uint32(edsPort)})}, From ecbaa5060b749915f9f7d2ced6a70dbb88f4a1ed Mon Sep 17 00:00:00 2001 From: Arjan Bal Date: Tue, 27 Aug 2024 04:09:23 +0530 Subject: [PATCH 2/2] Mock DNS resolver --- resolver/manual/manual.go | 4 +++- .../e2e_test/aggregate_cluster_test.go | 20 ++++++++++++++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/resolver/manual/manual.go b/resolver/manual/manual.go index f2efa2a2cb5a..09e864a89d35 100644 --- a/resolver/manual/manual.go +++ b/resolver/manual/manual.go @@ -76,9 +76,11 @@ func (r *Resolver) InitialState(s resolver.State) { // Build returns itself for Resolver, because it's both a builder and a resolver. func (r *Resolver) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) { - r.BuildCallback(target, cc, opts) r.mu.Lock() defer r.mu.Unlock() + // Call BuildCallback after locking to avoid a race when UpdateState + // or ReportError is called before Build returns. + r.BuildCallback(target, cc, opts) r.CC = cc if r.lastSeenState != nil { err := r.CC.UpdateState(*r.lastSeenState) diff --git a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go index 0b5b2b02b3a0..8bde92bc6556 100644 --- a/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go +++ b/xds/internal/balancer/clusterresolver/e2e_test/aggregate_cluster_test.go @@ -840,6 +840,7 @@ func (s) TestAggregateCluster_BadEDSFromError_GoodToBadDNS(t *testing.T) { // good update, this test verifies the cluster_resolver balancer correctly falls // back from the LOGICAL_DNS cluster to the EDS cluster. func (s) TestAggregateCluster_BadDNS_GoodEDS(t *testing.T) { + dnsTargetCh, dnsR := setupDNS(t) // Start an xDS management server. managementServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{AllowResourceSubset: true}) @@ -857,12 +858,14 @@ func (s) TestAggregateCluster_BadDNS_GoodEDS(t *testing.T) { const ( edsClusterName = clusterName + "-eds" dnsClusterName = clusterName + "-dns" + dnsHostName = "bad.ip.v4.address" + dnsPort = 8080 ) resources := e2e.UpdateOptions{ NodeID: nodeID, Clusters: []*v3clusterpb.Cluster{ makeAggregateClusterResource(clusterName, []string{dnsClusterName, edsClusterName}), - makeLogicalDNSClusterResource(dnsClusterName, "bad%ipv4%address", 8080), // Invalid URL to avoid making a DNS request. + makeLogicalDNSClusterResource(dnsClusterName, dnsHostName, dnsPort), e2e.DefaultCluster(edsClusterName, edsServiceName, e2e.SecurityLevelNone), }, Endpoints: []*v3endpointpb.ClusterLoadAssignment{e2e.DefaultEndpoint(edsServiceName, "localhost", []uint32{uint32(edsPort)})}, @@ -879,6 +882,21 @@ func (s) TestAggregateCluster_BadDNS_GoodEDS(t *testing.T) { cc, cleanup := setupAndDial(t, bootstrapContents) defer cleanup() + // Ensure that the DNS resolver is started for the expected target. + select { + case <-ctx.Done(): + t.Fatal("Timeout when waiting for DNS resolver to be started") + case target := <-dnsTargetCh: + got, want := target.Endpoint(), fmt.Sprintf("%s:%d", dnsHostName, dnsPort) + if got != want { + t.Fatalf("DNS resolution started for target %q, want %q", got, want) + } + } + + // Produce a bad resolver update from the DNS resolver. + dnsErr := fmt.Errorf("DNS error") + dnsR.ReportError(dnsErr) + // RPCs should work, higher level DNS cluster errors so should fallback to // EDS cluster. client := testgrpc.NewTestServiceClient(cc)