From bbc5f61abb1fe354cc9f7b064ce732867d6af7e4 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Wed, 26 May 2021 12:02:18 -0700 Subject: [PATCH] xds: use load assignment endpoint address in Cluster as the DNS hostname for LOGICAL_DNS (#8151) Fixes the source of hostname used for DNS resolution in the cluster_resolver LB policy for LOGICAL_DNS clusters. The change includes: - parse the single endpoint address from the embedded Cluster resource in CDS responses as the DNS hostname for LOGICAL_DNS cluster and include it in CdsUpdate being notified to the CDS LB policy. - propagate the DNS hostname to the cluster_resolver LB policy via its LB config (DiscoveryMechanism for LOGICAL_DNS cluster). - cluster_resolver LB policy takes the DNS hostname from the DiscoveryMechanism for LOGICAL_DNS cluster and use it as the name for DNS resolution. --- .../java/io/grpc/xds/CdsLoadBalancer2.java | 4 +- .../java/io/grpc/xds/ClientXdsClient.java | 36 ++++++++++++- .../grpc/xds/ClusterResolverLoadBalancer.java | 48 ++++++++--------- .../ClusterResolverLoadBalancerProvider.java | 27 ++++++---- xds/src/main/java/io/grpc/xds/XdsClient.java | 16 +++++- .../io/grpc/xds/CdsLoadBalancer2Test.java | 54 ++++++++++--------- .../io/grpc/xds/ClientXdsClientTestBase.java | 20 ++++--- .../io/grpc/xds/ClientXdsClientV2Test.java | 12 ++++- .../io/grpc/xds/ClientXdsClientV3Test.java | 12 ++++- .../xds/ClusterResolverLoadBalancerTest.java | 51 +++++++++--------- 10 files changed, 180 insertions(+), 100 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java index d0286b268d..e91e76090a 100644 --- a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java +++ b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java @@ -161,8 +161,8 @@ final class CdsLoadBalancer2 extends LoadBalancer { clusterState.result.upstreamTlsContext()); } else { // logical DNS instance = DiscoveryMechanism.forLogicalDns( - clusterState.name, clusterState.result.lrsServerName(), - clusterState.result.maxConcurrentRequests(), + clusterState.name, clusterState.result.dnsHostName(), + clusterState.result.lrsServerName(), clusterState.result.maxConcurrentRequests(), clusterState.result.upstreamTlsContext()); } instances.add(instance); diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 6000564ed9..19b8d0c1bb 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -41,6 +41,8 @@ import io.envoyproxy.envoy.config.cluster.v3.Cluster.LbPolicy; import io.envoyproxy.envoy.config.cluster.v3.Cluster.RingHashLbConfig; import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions; import io.envoyproxy.envoy.config.core.v3.RoutingPriority; +import io.envoyproxy.envoy.config.core.v3.SocketAddress; +import io.envoyproxy.envoy.config.core.v3.SocketAddress.PortSpecifierCase; import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment; import io.envoyproxy.envoy.config.listener.v3.Listener; import io.envoyproxy.envoy.config.route.v3.RouteConfiguration; @@ -929,8 +931,40 @@ final class ClientXdsClient extends AbstractXdsClient { return StructOrError.fromStruct(CdsUpdate.forEds( clusterName, edsServiceName, lrsServerName, maxConcurrentRequests, upstreamTlsContext)); } else if (type.equals(DiscoveryType.LOGICAL_DNS)) { + if (!cluster.hasLoadAssignment()) { + return StructOrError.fromError( + "Cluster " + clusterName + ": LOGICAL_DNS clusters must have a single host"); + } + ClusterLoadAssignment assignment = cluster.getLoadAssignment(); + if (assignment.getEndpointsCount() != 1 + || assignment.getEndpoints(0).getLbEndpointsCount() != 1) { + return StructOrError.fromError( + "Cluster " + clusterName + ": LOGICAL_DNS clusters must have a single " + + "locality_lb_endpoint and a single lb_endpoint"); + } + io.envoyproxy.envoy.config.endpoint.v3.LbEndpoint lbEndpoint = + assignment.getEndpoints(0).getLbEndpoints(0); + if (!lbEndpoint.hasEndpoint() || !lbEndpoint.getEndpoint().hasAddress() + || !lbEndpoint.getEndpoint().getAddress().hasSocketAddress()) { + return StructOrError.fromError( + "Cluster " + clusterName + + ": LOGICAL_DNS clusters must have an endpoint with address and socket_address"); + } + SocketAddress socketAddress = lbEndpoint.getEndpoint().getAddress().getSocketAddress(); + if (!socketAddress.getResolverName().isEmpty()) { + return StructOrError.fromError( + "Cluster " + clusterName + + ": LOGICAL DNS clusters must NOT have a custom resolver name set"); + } + if (socketAddress.getPortSpecifierCase() != PortSpecifierCase.PORT_VALUE) { + return StructOrError.fromError( + "Cluster " + clusterName + + ": LOGICAL DNS clusters socket_address must have port_value"); + } + String dnsHostName = + String.format("%s:%d", socketAddress.getAddress(), socketAddress.getPortValue()); return StructOrError.fromStruct(CdsUpdate.forLogicalDns( - clusterName, lrsServerName, maxConcurrentRequests, upstreamTlsContext)); + clusterName, dnsHostName, lrsServerName, maxConcurrentRequests, upstreamTlsContext)); } return StructOrError.fromError( "Cluster " + clusterName + ": unsupported built-in discovery type: " + type); diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java index e4a848413d..3f9209d347 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java @@ -79,7 +79,6 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { // to an empty locality. private static final Locality LOGICAL_DNS_CLUSTER_LOCALITY = Locality.create("", "", ""); private final XdsLogger logger; - private final String authority; private final SynchronizationContext syncContext; private final ScheduledExecutorService timeService; private final LoadBalancerRegistry lbRegistry; @@ -99,7 +98,6 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { BackoffPolicy.Provider backoffPolicyProvider) { this.lbRegistry = checkNotNull(lbRegistry, "lbRegistry"); this.backoffPolicyProvider = checkNotNull(backoffPolicyProvider, "backoffPolicyProvider"); - this.authority = checkNotNull(checkNotNull(helper, "helper").getAuthority(), "authority"); this.syncContext = checkNotNull(helper.getSynchronizationContext(), "syncContext"); this.timeService = checkNotNull(helper.getScheduledExecutorService(), "timeService"); delegate = new GracefulSwitchLoadBalancer(helper); @@ -178,8 +176,8 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { state = new EdsClusterState(instance.cluster, instance.edsServiceName, instance.lrsServerName, instance.maxConcurrentRequests, instance.tlsContext); } else { // logical DNS - state = new LogicalDnsClusterState(instance.cluster, instance.lrsServerName, - instance.maxConcurrentRequests, instance.tlsContext); + state = new LogicalDnsClusterState(instance.cluster, instance.dnsHostName, + instance.lrsServerName, instance.maxConcurrentRequests, instance.tlsContext); } clusterStates.put(instance.cluster, state); state.start(); @@ -305,10 +303,6 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { private abstract class ClusterState { // Name of the cluster to be resolved. protected final String name; - // The resource name to be used for resolving endpoints via EDS. - // Always null if the cluster is a logical DNS cluster. - @Nullable - protected final String edsServiceName; @Nullable protected final String lrsServerName; @Nullable @@ -324,11 +318,9 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { protected ClusterResolutionResult result; protected boolean shutdown; - private ClusterState(String name, @Nullable String edsServiceName, - @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, - @Nullable UpstreamTlsContext tlsContext) { + private ClusterState(String name, @Nullable String lrsServerName, + @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { this.name = name; - this.edsServiceName = edsServiceName; this.lrsServerName = lrsServerName; this.maxConcurrentRequests = maxConcurrentRequests; this.tlsContext = tlsContext; @@ -342,11 +334,14 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { } private final class EdsClusterState extends ClusterState implements EdsResourceWatcher { + @Nullable + private final String edsServiceName; private EdsClusterState(String name, @Nullable String edsServiceName, @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { - super(name, edsServiceName, lrsServerName, maxConcurrentRequests, tlsContext); + super(name, lrsServerName, maxConcurrentRequests, tlsContext); + this.edsServiceName = edsServiceName; } @Override @@ -469,6 +464,7 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { } private final class LogicalDnsClusterState extends ClusterState { + private final String dnsHostName; private final NameResolver.Factory nameResolverFactory; private final NameResolver.Args nameResolverArgs; private NameResolver resolver; @@ -477,9 +473,11 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { @Nullable private ScheduledHandle scheduledRefresh; - private LogicalDnsClusterState(String name, @Nullable String lrsServerName, - @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { - super(name, null, lrsServerName, maxConcurrentRequests, tlsContext); + private LogicalDnsClusterState(String name, String dnsHostName, + @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, + @Nullable UpstreamTlsContext tlsContext) { + super(name, lrsServerName, maxConcurrentRequests, tlsContext); + this.dnsHostName = checkNotNull(dnsHostName, "dnsHostName"); nameResolverFactory = checkNotNull(helper.getNameResolverRegistry().asFactory(), "nameResolverFactory"); nameResolverArgs = checkNotNull(helper.getNameResolverArgs(), "nameResolverArgs"); @@ -489,10 +487,10 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { void start() { URI uri; try { - uri = new URI("dns", "", "/" + authority, null); + uri = new URI("dns", "", "/" + dnsHostName, null); } catch (URISyntaxException e) { - status = - Status.INTERNAL.withDescription("Bug, invalid authority: " + authority).withCause(e); + status = Status.INTERNAL.withDescription( + "Bug, invalid URI creation: " + dnsHostName).withCause(e); handleEndpointResolutionError(); return; } @@ -564,8 +562,8 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { addresses.add(eag); } PriorityChildConfig priorityChildConfig = generateDnsBasedPriorityChildConfig( - name, edsServiceName, lrsServerName, maxConcurrentRequests, tlsContext, - lbRegistry, Collections.emptyList()); + name, lrsServerName, maxConcurrentRequests, tlsContext, lbRegistry, + Collections.emptyList()); status = Status.OK; resolved = true; result = new ClusterResolutionResult(addresses, priorityName, priorityChildConfig); @@ -645,14 +643,14 @@ final class ClusterResolverLoadBalancer extends LoadBalancer { *

priority LB -> cluster_impl LB (single hardcoded priority) -> pick_first */ private static PriorityChildConfig generateDnsBasedPriorityChildConfig( - String cluster, @Nullable String edsServiceName, @Nullable String lrsServerName, - @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext, - LoadBalancerRegistry lbRegistry, List dropOverloads) { + String cluster, @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, + @Nullable UpstreamTlsContext tlsContext, LoadBalancerRegistry lbRegistry, + List dropOverloads) { // Override endpoint-level LB policy with pick_first for logical DNS cluster. PolicySelection endpointLbPolicy = new PolicySelection(lbRegistry.getProvider("pick_first"), null); ClusterImplConfig clusterImplConfig = - new ClusterImplConfig(cluster, edsServiceName, lrsServerName, maxConcurrentRequests, + new ClusterImplConfig(cluster, null, lrsServerName, maxConcurrentRequests, dropOverloads, endpointLbPolicy, tlsContext); LoadBalancerProvider clusterImplLbProvider = lbRegistry.getProvider(XdsLbPolicies.CLUSTER_IMPL_POLICY_NAME); diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java index e62c70cb5e..33b150e667 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java @@ -119,6 +119,9 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi // Resource name for resolving endpoints via EDS. Only valid for EDS clusters. @Nullable final String edsServiceName; + // Hostname for resolving endpoints via DNS. Only valid for LOGICAL_DNS clusters. + @Nullable + final String dnsHostName; enum Type { EDS, @@ -126,11 +129,12 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi } private DiscoveryMechanism(String cluster, Type type, @Nullable String edsServiceName, - @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, - @Nullable UpstreamTlsContext tlsContext) { + @Nullable String dnsHostName, @Nullable String lrsServerName, + @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { this.cluster = checkNotNull(cluster, "cluster"); this.type = checkNotNull(type, "type"); this.edsServiceName = edsServiceName; + this.dnsHostName = dnsHostName; this.lrsServerName = lrsServerName; this.maxConcurrentRequests = maxConcurrentRequests; this.tlsContext = tlsContext; @@ -139,20 +143,21 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi static DiscoveryMechanism forEds(String cluster, @Nullable String edsServiceName, @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { - return new DiscoveryMechanism(cluster, Type.EDS, edsServiceName, lrsServerName, + return new DiscoveryMechanism(cluster, Type.EDS, edsServiceName, null, lrsServerName, maxConcurrentRequests, tlsContext); } - static DiscoveryMechanism forLogicalDns(String cluster, @Nullable String lrsServerName, - @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { - return new DiscoveryMechanism(cluster, Type.LOGICAL_DNS, null, lrsServerName, - maxConcurrentRequests, tlsContext); + static DiscoveryMechanism forLogicalDns(String cluster, String dnsHostName, + @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, + @Nullable UpstreamTlsContext tlsContext) { + return new DiscoveryMechanism(cluster, Type.LOGICAL_DNS, null, dnsHostName, + lrsServerName, maxConcurrentRequests, tlsContext); } @Override public int hashCode() { return Objects.hash(cluster, type, lrsServerName, maxConcurrentRequests, tlsContext, - edsServiceName); + edsServiceName, dnsHostName); } @Override @@ -167,6 +172,7 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi return cluster.equals(that.cluster) && type == that.type && Objects.equals(edsServiceName, that.edsServiceName) + && Objects.equals(dnsHostName, that.dnsHostName) && Objects.equals(lrsServerName, that.lrsServerName) && Objects.equals(maxConcurrentRequests, that.maxConcurrentRequests) && Objects.equals(tlsContext, that.tlsContext); @@ -178,12 +184,11 @@ public final class ClusterResolverLoadBalancerProvider extends LoadBalancerProvi MoreObjects.toStringHelper(this) .add("cluster", cluster) .add("type", type) + .add("edsServiceName", edsServiceName) + .add("dnsHostName", dnsHostName) .add("lrsServerName", lrsServerName) // Exclude tlsContext as its string representation is cumbersome. .add("maxConcurrentRequests", maxConcurrentRequests); - if (type == Type.EDS) { - toStringHelper.add("edsServiceName", edsServiceName); - } return toStringHelper.toString(); } } diff --git a/xds/src/main/java/io/grpc/xds/XdsClient.java b/xds/src/main/java/io/grpc/xds/XdsClient.java index 4033d75309..24c06e6022 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClient.java +++ b/xds/src/main/java/io/grpc/xds/XdsClient.java @@ -191,6 +191,12 @@ abstract class XdsClient { @Nullable abstract String edsServiceName(); + // Corresponding DNS name to be used if upstream endpoints of the cluster is resolvable + // via DNS. + // Only valid for LOGICAL_DNS cluster. + @Nullable + abstract String dnsHostName(); + // Load report server name for reporting loads via LRS. // Only valid for EDS or LOGICAL_DNS cluster. @Nullable @@ -235,13 +241,15 @@ abstract class XdsClient { .upstreamTlsContext(upstreamTlsContext); } - static Builder forLogicalDns(String clusterName, @Nullable String lrsServerName, - @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext upstreamTlsContext) { + static Builder forLogicalDns(String clusterName, String dnsHostName, + @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, + @Nullable UpstreamTlsContext upstreamTlsContext) { return new AutoValue_XdsClient_CdsUpdate.Builder() .clusterName(clusterName) .clusterType(ClusterType.LOGICAL_DNS) .minRingSize(0) .maxRingSize(0) + .dnsHostName(dnsHostName) .lrsServerName(lrsServerName) .maxConcurrentRequests(maxConcurrentRequests) .upstreamTlsContext(upstreamTlsContext); @@ -265,6 +273,7 @@ abstract class XdsClient { .add("minRingSize", minRingSize()) .add("maxRingSize", maxRingSize()) .add("edsServiceName", edsServiceName()) + .add("dnsHostName", dnsHostName()) .add("lrsServerName", lrsServerName()) .add("maxConcurrentRequests", maxConcurrentRequests()) // Exclude upstreamTlsContext as its string representation is cumbersome. @@ -295,6 +304,9 @@ abstract class XdsClient { // Private, use CdsUpdate.forEds() instead. protected abstract Builder edsServiceName(String edsServiceName); + // Private, use CdsUpdate.forLogicalDns() instead. + protected abstract Builder dnsHostName(String dnsHostName); + // Private, use one of the static factory methods instead. protected abstract Builder lrsServerName(String lrsServerName); diff --git a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java index 2287b8fc1b..5414bcd0f4 100644 --- a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java +++ b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java @@ -73,9 +73,9 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class CdsLoadBalancer2Test { - private static final String CLUSTER = "cluster-foo.googleapis.com"; // cluster of entry point - + private static final String CLUSTER = "cluster-foo.googleapis.com"; private static final String EDS_SERVICE_NAME = "backend-service-1.googleapis.com"; + private static final String DNS_HOST_NAME = "backend-service-dns.googleapis.com:443"; private static final String LRS_SERVER_NAME = "lrs.googleapis.com"; private final UpstreamTlsContext upstreamTlsContext = CommonTlsContextTestsUtil.buildUpstreamTlsContextFromFilenames( @@ -157,14 +157,14 @@ public class CdsLoadBalancer2Test { assertThat(childLbConfig.discoveryMechanisms).hasSize(1); DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, - LRS_SERVER_NAME, 100L, upstreamTlsContext); + null, LRS_SERVER_NAME, 100L, upstreamTlsContext); assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName()).isEqualTo("round_robin"); } @Test public void discoverTopLevelLogicalDnsCluster() { CdsUpdate update = - CdsUpdate.forLogicalDns(CLUSTER, LRS_SERVER_NAME, 100L, upstreamTlsContext) + CdsUpdate.forLogicalDns(CLUSTER, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, upstreamTlsContext) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(CLUSTER, update); assertThat(childBalancers).hasSize(1); @@ -174,7 +174,7 @@ public class CdsLoadBalancer2Test { assertThat(childLbConfig.discoveryMechanisms).hasSize(1); DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.LOGICAL_DNS, null, - LRS_SERVER_NAME, 100L, upstreamTlsContext); + DNS_HOST_NAME, LRS_SERVER_NAME, 100L, upstreamTlsContext); assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName()).isEqualTo("round_robin"); } @@ -198,8 +198,8 @@ public class CdsLoadBalancer2Test { FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); ClusterResolverConfig childLbConfig = (ClusterResolverConfig) childBalancer.config; DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); - assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.EDS, null, null, 100L, - upstreamTlsContext); + assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.EDS, null, null, null, + 100L, upstreamTlsContext); update = CdsUpdate.forEds(CLUSTER, EDS_SERVICE_NAME, LRS_SERVER_NAME, 200L, null) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); @@ -207,21 +207,21 @@ public class CdsLoadBalancer2Test { childLbConfig = (ClusterResolverConfig) childBalancer.config; instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, - LRS_SERVER_NAME, 200L, null); + null, LRS_SERVER_NAME, 200L, null); } @Test public void nonAggregateCluster_resourceRevoked() { CdsUpdate update = - CdsUpdate.forLogicalDns(CLUSTER, null, 100L, upstreamTlsContext) + CdsUpdate.forLogicalDns(CLUSTER, DNS_HOST_NAME, null, 100L, upstreamTlsContext) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(CLUSTER, update); assertThat(childBalancers).hasSize(1); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); ClusterResolverConfig childLbConfig = (ClusterResolverConfig) childBalancer.config; DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); - assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.LOGICAL_DNS, null, null, - 100L, upstreamTlsContext); + assertDiscoveryMechanism(instance, CLUSTER, DiscoveryMechanism.Type.LOGICAL_DNS, null, + DNS_HOST_NAME, null, 100L, upstreamTlsContext); xdsClient.deliverResourceNotExist(CLUSTER); assertThat(childBalancer.shutdown).isTrue(); @@ -260,7 +260,7 @@ public class CdsLoadBalancer2Test { xdsClient.deliverCdsUpdate(cluster3, update3); assertThat(childBalancers).isEmpty(); CdsUpdate update2 = - CdsUpdate.forLogicalDns(cluster2, null, 100L, null) + CdsUpdate.forLogicalDns(cluster2, DNS_HOST_NAME, null, 100L, null) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster2, update2); assertThat(childBalancers).isEmpty(); @@ -275,11 +275,12 @@ public class CdsLoadBalancer2Test { assertThat(childLbConfig.discoveryMechanisms).hasSize(3); // Clusters on higher level has higher priority: [cluster2, cluster3, cluster4] assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(0), cluster2, - DiscoveryMechanism.Type.LOGICAL_DNS, null, null, 100L, null); + DiscoveryMechanism.Type.LOGICAL_DNS, null, DNS_HOST_NAME, null, 100L, null); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(1), cluster3, - DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, LRS_SERVER_NAME, 200L, upstreamTlsContext); + DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, null, LRS_SERVER_NAME, 200L, + upstreamTlsContext); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(2), cluster4, - DiscoveryMechanism.Type.EDS, null, LRS_SERVER_NAME, 300L, null); + DiscoveryMechanism.Type.EDS, null, null, LRS_SERVER_NAME, 300L, null); assertThat(childLbConfig.lbPolicy.getProvider().getPolicyName()) .isEqualTo("ring_hash"); // dominated by top-level cluster's config assertThat(((RingHashConfig) childLbConfig.lbPolicy.getConfig()).minRingSize).isEqualTo(100L); @@ -318,16 +319,17 @@ public class CdsLoadBalancer2Test { .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster1, update1); CdsUpdate update2 = - CdsUpdate.forLogicalDns(cluster2, LRS_SERVER_NAME, 100L, null) + CdsUpdate.forLogicalDns(cluster2, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, null) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster2, update2); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); ClusterResolverConfig childLbConfig = (ClusterResolverConfig) childBalancer.config; assertThat(childLbConfig.discoveryMechanisms).hasSize(2); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(0), cluster1, - DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, LRS_SERVER_NAME, 200L, upstreamTlsContext); + DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, null, LRS_SERVER_NAME, 200L, + upstreamTlsContext); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(1), cluster2, - DiscoveryMechanism.Type.LOGICAL_DNS, null, LRS_SERVER_NAME, 100L, null); + DiscoveryMechanism.Type.LOGICAL_DNS, null, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, null); // Revoke cluster1, should still be able to proceed with cluster2. xdsClient.deliverResourceNotExist(cluster1); @@ -335,7 +337,7 @@ public class CdsLoadBalancer2Test { childLbConfig = (ClusterResolverConfig) childBalancer.config; assertThat(childLbConfig.discoveryMechanisms).hasSize(1); assertDiscoveryMechanism(Iterables.getOnlyElement(childLbConfig.discoveryMechanisms), cluster2, - DiscoveryMechanism.Type.LOGICAL_DNS, null, LRS_SERVER_NAME, 100L, null); + DiscoveryMechanism.Type.LOGICAL_DNS, null, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, null); verify(helper, never()).updateBalancingState( eq(ConnectivityState.TRANSIENT_FAILURE), any(SubchannelPicker.class)); @@ -364,16 +366,17 @@ public class CdsLoadBalancer2Test { .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster1, update1); CdsUpdate update2 = - CdsUpdate.forLogicalDns(cluster2, LRS_SERVER_NAME, 100L, null) + CdsUpdate.forLogicalDns(cluster2, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, null) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster2, update2); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); ClusterResolverConfig childLbConfig = (ClusterResolverConfig) childBalancer.config; assertThat(childLbConfig.discoveryMechanisms).hasSize(2); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(0), cluster1, - DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, LRS_SERVER_NAME, 200L, upstreamTlsContext); + DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, null, LRS_SERVER_NAME, 200L, + upstreamTlsContext); assertDiscoveryMechanism(childLbConfig.discoveryMechanisms.get(1), cluster2, - DiscoveryMechanism.Type.LOGICAL_DNS, null, LRS_SERVER_NAME, 100L, null); + DiscoveryMechanism.Type.LOGICAL_DNS, null, DNS_HOST_NAME, LRS_SERVER_NAME, 100L, null); xdsClient.deliverResourceNotExist(CLUSTER); assertThat(xdsClient.watchers.keySet()) @@ -420,7 +423,7 @@ public class CdsLoadBalancer2Test { assertThat(childLbConfig.discoveryMechanisms).hasSize(1); DiscoveryMechanism instance = Iterables.getOnlyElement(childLbConfig.discoveryMechanisms); assertDiscoveryMechanism(instance, cluster3, DiscoveryMechanism.Type.EDS, EDS_SERVICE_NAME, - LRS_SERVER_NAME, 100L, upstreamTlsContext); + null, LRS_SERVER_NAME, 100L, upstreamTlsContext); // cluster2 revoked xdsClient.deliverResourceNotExist(cluster2); @@ -460,7 +463,7 @@ public class CdsLoadBalancer2Test { .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(CLUSTER, update); CdsUpdate update1 = - CdsUpdate.forLogicalDns(cluster1, LRS_SERVER_NAME, 200L, null) + CdsUpdate.forLogicalDns(cluster1, DNS_HOST_NAME, LRS_SERVER_NAME, 200L, null) .lbPolicy(LbPolicy.ROUND_ROBIN).build(); xdsClient.deliverCdsUpdate(cluster1, update1); FakeLoadBalancer childLb = Iterables.getOnlyElement(childBalancers); @@ -509,12 +512,13 @@ public class CdsLoadBalancer2Test { } private static void assertDiscoveryMechanism(DiscoveryMechanism instance, String name, - DiscoveryMechanism.Type type, @Nullable String edsServiceName, + DiscoveryMechanism.Type type, @Nullable String edsServiceName, @Nullable String dnsHostName, @Nullable String lrsServerName, @Nullable Long maxConcurrentRequests, @Nullable UpstreamTlsContext tlsContext) { assertThat(instance.cluster).isEqualTo(name); assertThat(instance.type).isEqualTo(type); assertThat(instance.edsServiceName).isEqualTo(edsServiceName); + assertThat(instance.dnsHostName).isEqualTo(dnsHostName); assertThat(instance.lrsServerName).isEqualTo(lrsServerName); assertThat(instance.maxConcurrentRequests).isEqualTo(maxConcurrentRequests); assertThat(instance.tlsContext).isEqualTo(tlsContext); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index 6805772ed8..ff7c18fba4 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -1233,8 +1233,8 @@ public abstract class ClientXdsClientTestBase { null, true, mf.buildUpstreamTlsContext("secret1", "unix:/var/uds2"), null)); List clusters = ImmutableList.of( - Any.pack(mf.buildLogicalDnsCluster("cluster-bar.googleapis.com", "round_robin", null, - false, null, null)), + Any.pack(mf.buildLogicalDnsCluster("cluster-bar.googleapis.com", + "dns-service-bar.googleapis.com", 443, "round_robin", null, false, null, null)), clusterEds, Any.pack(mf.buildEdsCluster("cluster-baz.googleapis.com", null, "round_robin", null, false, null, null))); @@ -1305,14 +1305,18 @@ public abstract class ClientXdsClientTestBase { verifyResourceMetadataRequested(CDS, CDS_RESOURCE); // Initial CDS response. + String dnsHostAddr = "dns-service-bar.googleapis.com"; + int dnsHostPort = 443; Any clusterDns = - Any.pack(mf.buildLogicalDnsCluster(CDS_RESOURCE, "round_robin", null, false, null, null)); + Any.pack(mf.buildLogicalDnsCluster(CDS_RESOURCE, dnsHostAddr, dnsHostPort, "round_robin", + null, false, null, null)); call.sendResponse(CDS, clusterDns, VERSION_1, "0000"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.LOGICAL_DNS); + assertThat(cdsUpdate.dnsHostName()).isEqualTo(dnsHostAddr + ":" + dnsHostPort); assertThat(cdsUpdate.lbPolicy()).isEqualTo(LbPolicy.ROUND_ROBIN); assertThat(cdsUpdate.lrsServerName()).isNull(); assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); @@ -1389,9 +1393,12 @@ public abstract class ClientXdsClientTestBase { verifyResourceMetadataDoesNotExist(CDS, cdsResourceTwo); verifySubscribedResourcesMetadataSizes(0, 2, 0, 0); + String dnsHostAddr = "dns-service-bar.googleapis.com"; + int dnsHostPort = 443; String edsService = "eds-service-bar.googleapis.com"; List clusters = ImmutableList.of( - Any.pack(mf.buildLogicalDnsCluster(CDS_RESOURCE, "round_robin", null, false, null, null)), + Any.pack(mf.buildLogicalDnsCluster(CDS_RESOURCE, dnsHostAddr, dnsHostPort, "round_robin", + null, false, null, null)), Any.pack(mf.buildEdsCluster(cdsResourceTwo, edsService, "round_robin", null, true, null, null))); call.sendResponse(CDS, clusters, VERSION_1, "0000"); @@ -1399,6 +1406,7 @@ public abstract class ClientXdsClientTestBase { CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.LOGICAL_DNS); + assertThat(cdsUpdate.dnsHostName()).isEqualTo(dnsHostAddr + ":" + dnsHostPort); assertThat(cdsUpdate.lbPolicy()).isEqualTo(LbPolicy.ROUND_ROBIN); assertThat(cdsUpdate.lrsServerName()).isNull(); assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); @@ -2187,8 +2195,8 @@ public abstract class ClientXdsClientTestBase { String lbPolicy, @Nullable Message ringHashLbConfig, boolean enableLrs, @Nullable Message upstreamTlsContext, @Nullable Message circuitBreakers); - protected abstract Message buildLogicalDnsCluster(String clusterName, String lbPolicy, - @Nullable Message ringHashLbConfig, boolean enableLrs, + protected abstract Message buildLogicalDnsCluster(String clusterName, String dnsHostAddr, + int dnsHostPort, String lbPolicy, @Nullable Message ringHashLbConfig, boolean enableLrs, @Nullable Message upstreamTlsContext, @Nullable Message circuitBreakers); protected abstract Message buildAggregateCluster(String clusterName, String lbPolicy, diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java index 94e93c4e9b..27ef2ba9eb 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV2Test.java @@ -408,12 +408,20 @@ public class ClientXdsClientV2Test extends ClientXdsClientTestBase { } @Override - protected Message buildLogicalDnsCluster(String clusterName, String lbPolicy, - @Nullable Message ringHashLbConfig, boolean enableLrs, + protected Message buildLogicalDnsCluster(String clusterName, String dnsHostAddr, + int dnsHostPort, String lbPolicy, @Nullable Message ringHashLbConfig, boolean enableLrs, @Nullable Message upstreamTlsContext, @Nullable Message circuitBreakers) { Cluster.Builder builder = initClusterBuilder(clusterName, lbPolicy, ringHashLbConfig, enableLrs, upstreamTlsContext, circuitBreakers); builder.setType(DiscoveryType.LOGICAL_DNS); + builder.setLoadAssignment( + ClusterLoadAssignment.newBuilder().addEndpoints( + LocalityLbEndpoints.newBuilder().addLbEndpoints( + LbEndpoint.newBuilder().setEndpoint( + Endpoint.newBuilder().setAddress( + Address.newBuilder().setSocketAddress( + SocketAddress.newBuilder() + .setAddress(dnsHostAddr).setPortValue(dnsHostPort)))))).build()); return builder.build(); } diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java index 9332297ff9..a68decba88 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientV3Test.java @@ -453,12 +453,20 @@ public class ClientXdsClientV3Test extends ClientXdsClientTestBase { } @Override - protected Message buildLogicalDnsCluster(String clusterName, String lbPolicy, - @Nullable Message ringHashLbConfig, boolean enableLrs, + protected Message buildLogicalDnsCluster(String clusterName, String dnsHostAddr, + int dnsHostPort, String lbPolicy, @Nullable Message ringHashLbConfig, boolean enableLrs, @Nullable Message upstreamTlsContext, @Nullable Message circuitBreakers) { Cluster.Builder builder = initClusterBuilder(clusterName, lbPolicy, ringHashLbConfig, enableLrs, upstreamTlsContext, circuitBreakers); builder.setType(DiscoveryType.LOGICAL_DNS); + builder.setLoadAssignment( + ClusterLoadAssignment.newBuilder().addEndpoints( + LocalityLbEndpoints.newBuilder().addLbEndpoints( + LbEndpoint.newBuilder().setEndpoint( + Endpoint.newBuilder().setAddress( + Address.newBuilder().setSocketAddress( + SocketAddress.newBuilder() + .setAddress(dnsHostAddr).setPortValue(dnsHostPort)))))).build()); return builder.build(); } diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index 7026cad6a4..1e334c749b 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -102,6 +102,7 @@ public class ClusterResolverLoadBalancerTest { private static final String CLUSTER_DNS = "cluster-dns.googleapis.com"; private static final String EDS_SERVICE_NAME1 = "backend-service-foo.googleapis.com"; private static final String EDS_SERVICE_NAME2 = "backend-service-bar.googleapis.com"; + private static final String DNS_HOST_NAME = "dns-service.googleapis.com"; private static final String LRS_SERVER_NAME = "lrs.googleapis.com"; private final Locality locality1 = Locality.create("test-region-1", "test-zone-1", "test-subzone-1"); @@ -119,7 +120,7 @@ public class ClusterResolverLoadBalancerTest { private final DiscoveryMechanism edsDiscoveryMechanism2 = DiscoveryMechanism.forEds(CLUSTER2, EDS_SERVICE_NAME2, LRS_SERVER_NAME, 200L, tlsContext); private final DiscoveryMechanism logicalDnsDiscoveryMechanism = - DiscoveryMechanism.forLogicalDns(CLUSTER_DNS, LRS_SERVER_NAME, 300L, null); + DiscoveryMechanism.forLogicalDns(CLUSTER_DNS, DNS_HOST_NAME, LRS_SERVER_NAME, 300L, null); private final SynchronizationContext syncContext = new SynchronizationContext( new Thread.UncaughtExceptionHandler() { @@ -526,11 +527,10 @@ public class ClusterResolverLoadBalancerTest { ClusterResolverConfig config = new ClusterResolverConfig( Collections.singletonList(logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1"); EquivalentAddressGroup endpoint2 = makeAddress("endpoint-addr-2"); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverEndpointAddresses(Arrays.asList(endpoint1, endpoint2)); assertThat(childBalancers).hasSize(1); @@ -554,11 +554,10 @@ public class ClusterResolverLoadBalancerTest { ClusterResolverConfig config = new ClusterResolverConfig( Collections.singletonList(logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1"); EquivalentAddressGroup endpoint2 = makeAddress("endpoint-addr-2"); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverEndpointAddresses(Arrays.asList(endpoint1, endpoint2)); assertThat(resolver.refreshCount).isEqualTo(0); verify(helper).ignoreRefreshNameResolutionCheck(); @@ -574,9 +573,8 @@ public class ClusterResolverLoadBalancerTest { ClusterResolverConfig config = new ClusterResolverConfig( Collections.singletonList(logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); Status error = Status.UNAVAILABLE.withDescription("cannot reach DNS server"); resolver.deliverError(error); inOrder.verify(helper).updateBalancingState( @@ -621,10 +619,9 @@ public class ClusterResolverLoadBalancerTest { ClusterResolverConfig config = new ClusterResolverConfig( Collections.singletonList(logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); EquivalentAddressGroup endpoint = makeAddress("endpoint-addr"); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverEndpointAddresses(Collections.singletonList(endpoint)); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); assertAddressesEqual(Collections.singletonList(endpoint), childBalancer.addresses); @@ -662,12 +659,11 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1"); // DNS endpoint EquivalentAddressGroup endpoint2 = makeAddress("endpoint-addr-2"); // DNS endpoint EquivalentAddressGroup endpoint3 = makeAddress("endpoint-addr-3"); // EDS endpoint - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverEndpointAddresses(Arrays.asList(endpoint1, endpoint2)); LocalityLbEndpoints localityLbEndpoints = LocalityLbEndpoints.create( @@ -697,14 +693,13 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); xdsClient.deliverResourceNotFound(EDS_SERVICE_NAME1); verify(helper, never()).updateBalancingState( any(ConnectivityState.class), any(SubchannelPicker.class)); // wait for DNS results - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1"); EquivalentAddressGroup endpoint2 = makeAddress("endpoint-addr-2"); resolver.deliverEndpointAddresses(Arrays.asList(endpoint1, endpoint2)); @@ -722,7 +717,7 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); EquivalentAddressGroup endpoint = makeAddress("endpoint-addr-1"); @@ -732,7 +727,6 @@ public class ClusterResolverLoadBalancerTest { 10 /* localityWeight */, 1 /* priority */); xdsClient.deliverClusterLoadAssignment( EDS_SERVICE_NAME1, Collections.singletonMap(locality1, localityLbEndpoints)); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverError(Status.UNKNOWN.withDescription("I am lost")); assertThat(childBalancers).hasSize(1); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); @@ -754,7 +748,7 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); EquivalentAddressGroup endpoint = makeAddress("endpoint-addr-1"); @@ -765,7 +759,7 @@ public class ClusterResolverLoadBalancerTest { xdsClient.deliverClusterLoadAssignment( EDS_SERVICE_NAME1, Collections.singletonMap(locality1, localityLbEndpoints)); assertThat(childBalancers).isEmpty(); // not created until all clusters resolved. - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); + resolver.deliverError(Status.UNKNOWN.withDescription("I am lost")); // DNS resolution failed, but there are EDS endpoints can be used. @@ -789,14 +783,13 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); xdsClient.deliverError(Status.UNIMPLEMENTED.withDescription("not found")); assertThat(childBalancers).isEmpty(); verify(helper, never()).updateBalancingState( eq(ConnectivityState.TRANSIENT_FAILURE), any(SubchannelPicker.class)); // wait for DNS - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); Status dnsError = Status.UNKNOWN.withDescription("I am lost"); resolver.deliverError(dnsError); verify(helper).updateBalancingState( @@ -813,7 +806,7 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); Status upstreamError = Status.UNAVAILABLE.withDescription("unreachable"); @@ -829,7 +822,7 @@ public class ClusterResolverLoadBalancerTest { Arrays.asList(edsDiscoveryMechanism1, logicalDnsDiscoveryMechanism), roundRobin); deliverLbConfig(config); assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1); - assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = assertResolverCreated("/" + DNS_HOST_NAME); assertThat(childBalancers).isEmpty(); reset(helper); EquivalentAddressGroup endpoint1 = makeAddress("endpoint-addr-1"); @@ -840,7 +833,6 @@ public class ClusterResolverLoadBalancerTest { 10 /* localityWeight */, 1 /* priority */); xdsClient.deliverClusterLoadAssignment( EDS_SERVICE_NAME1, Collections.singletonMap(locality1, localityLbEndpoints)); - FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); resolver.deliverEndpointAddresses(Collections.singletonList(endpoint2)); assertThat(childBalancers).hasSize(1); FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); @@ -868,6 +860,13 @@ public class ClusterResolverLoadBalancerTest { .build()); } + private FakeNameResolver assertResolverCreated(String uriPath) { + assertThat(resolvers).hasSize(1); + FakeNameResolver resolver = Iterables.getOnlyElement(resolvers); + assertThat(resolver.targetUri.getPath()).isEqualTo(uriPath); + return resolver; + } + private static void assertPicker(SubchannelPicker picker, Status expectedStatus, @Nullable Subchannel expectedSubchannel) { PickResult result = picker.pickSubchannel(mock(PickSubchannelArgs.class)); @@ -981,8 +980,7 @@ public class ClusterResolverLoadBalancerTest { @Override public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) { assertThat(targetUri.getScheme()).isEqualTo("dns"); - assertThat(targetUri.getPath()).isEqualTo("/" + AUTHORITY); - FakeNameResolver resolver = new FakeNameResolver(); + FakeNameResolver resolver = new FakeNameResolver(targetUri); resolvers.add(resolver); return resolver; } @@ -1004,9 +1002,14 @@ public class ClusterResolverLoadBalancerTest { } private class FakeNameResolver extends NameResolver { + private final URI targetUri; private Listener2 listener; private int refreshCount; + private FakeNameResolver(URI targetUri) { + this.targetUri = targetUri; + } + @Override public String getServiceAuthority() { throw new UnsupportedOperationException("should not be called");