Skip to content

Commit d816b8e

Browse files
kannanjgithubkannanjgithub
committed
Changes
1 parent 6f0a920 commit d816b8e

File tree

5 files changed

+268
-102
lines changed

5 files changed

+268
-102
lines changed

‎xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import static io.grpc.xds.XdsLbPolicies.PRIORITY_POLICY_NAME;
2424
import static java.util.Objects.requireNonNull;
2525

26-
import com.google.common.annotations.VisibleForTesting;
2726
import com.google.errorprone.annotations.CheckReturnValue;
2827
import io.grpc.InternalLogId;
2928
import io.grpc.LoadBalancer;
@@ -64,14 +63,9 @@ final class CdsLoadBalancer2 extends LoadBalancer {
6463
private Subscription clusterSubscription;
6564
private LoadBalancer childLb;
6665

67-
CdsLoadBalancer2(Helper helper) {
68-
this(helper, LoadBalancerRegistry.getDefaultRegistry());
69-
}
70-
71-
@VisibleForTesting
7266
CdsLoadBalancer2(Helper helper, LoadBalancerRegistry lbRegistry) {
7367
this.helper = checkNotNull(helper, "helper");
74-
this.lbRegistry = checkNotNull(lbRegistry, "lbRegistry");
68+
this.lbRegistry = lbRegistry;
7569
logger = XdsLogger.withLogId(InternalLogId.allocate("cds-lb", helper.getAuthority()));
7670
logger.log(XdsLogLevel.INFO, "Created");
7771
}
@@ -122,21 +116,21 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
122116
DiscoveryMechanism instance;
123117
if (result.clusterType() == ClusterType.EDS) {
124118
instance = DiscoveryMechanism.forEds(
125-
clusterName,
126-
result.edsServiceName(),
127-
result.lrsServerInfo(),
128-
result.maxConcurrentRequests(),
129-
result.upstreamTlsContext(),
130-
result.filterMetadata(),
131-
result.outlierDetection());
119+
clusterName,
120+
result.edsServiceName(),
121+
result.lrsServerInfo(),
122+
result.maxConcurrentRequests(),
123+
result.upstreamTlsContext(),
124+
result.filterMetadata(),
125+
result.outlierDetection());
132126
} else {
133127
instance = DiscoveryMechanism.forLogicalDns(
134-
clusterName,
135-
result.dnsHostName(),
136-
result.lrsServerInfo(),
137-
result.maxConcurrentRequests(),
138-
result.upstreamTlsContext(),
139-
result.filterMetadata());
128+
clusterName,
129+
result.dnsHostName(),
130+
result.lrsServerInfo(),
131+
result.maxConcurrentRequests(),
132+
result.upstreamTlsContext(),
133+
result.filterMetadata());
140134
}
141135
childConfig = new ClusterResolverConfig(
142136
instance,

‎xds/src/main/java/io/grpc/xds/CdsLoadBalancerProvider.java‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import io.grpc.LoadBalancer;
2424
import io.grpc.LoadBalancer.Helper;
2525
import io.grpc.LoadBalancerProvider;
26+
import io.grpc.LoadBalancerRegistry;
2627
import io.grpc.NameResolver.ConfigOrError;
2728
import io.grpc.Status;
2829
import io.grpc.internal.JsonUtil;
@@ -51,9 +52,24 @@ public String getPolicyName() {
5152
return XdsLbPolicies.CDS_POLICY_NAME;
5253
}
5354

55+
private final LoadBalancerRegistry loadBalancerRegistry;
56+
57+
public CdsLoadBalancerProvider() {
58+
this.loadBalancerRegistry = null;
59+
}
60+
61+
public CdsLoadBalancerProvider(LoadBalancerRegistry loadBalancerRegistry) {
62+
this.loadBalancerRegistry = loadBalancerRegistry;
63+
}
64+
5465
@Override
5566
public LoadBalancer newLoadBalancer(Helper helper) {
56-
return new CdsLoadBalancer2(helper);
67+
LoadBalancerRegistry loadBalancerRegistry = this.loadBalancerRegistry;
68+
if (loadBalancerRegistry == null) {
69+
loadBalancerRegistry = LoadBalancerRegistry.getDefaultRegistry();
70+
}
71+
72+
return new CdsLoadBalancer2(helper, loadBalancerRegistry);
5773
}
5874

5975
@Override

‎xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancerProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,14 @@ public LoadBalancer newLoadBalancer(Helper helper) {
6969
}
7070

7171
static final class ClusterResolverConfig {
72-
// Clusters to be resolved.
72+
// Cluster to be resolved.
7373
final DiscoveryMechanism discoveryMechanism;
7474
// GracefulSwitch configuration
7575
final Object lbConfig;
7676
private final boolean isHttp11ProxyAvailable;
7777

7878
ClusterResolverConfig(DiscoveryMechanism discoveryMechanism, Object lbConfig,
79-
boolean isHttp11ProxyAvailable) {
79+
boolean isHttp11ProxyAvailable) {
8080
this.discoveryMechanism = checkNotNull(discoveryMechanism, "discoveryMechanism");
8181
this.lbConfig = checkNotNull(lbConfig, "lbConfig");
8282
this.isHttp11ProxyAvailable = isHttp11ProxyAvailable;

‎xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java‎

Lines changed: 66 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package io.grpc.xds;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20-
import static io.grpc.xds.XdsLbPolicies.CDS_POLICY_NAME;
2120
import static io.grpc.xds.XdsLbPolicies.CLUSTER_RESOLVER_POLICY_NAME;
2221
import static io.grpc.xds.XdsLbPolicies.PRIORITY_POLICY_NAME;
2322
import static io.grpc.xds.XdsTestControlPlaneService.ADS_TYPE_URL_CDS;
@@ -152,8 +151,6 @@ public class CdsLoadBalancer2Test {
152151

153152
@Before
154153
public void setUp() throws Exception {
155-
lbRegistry.register(new FakeLoadBalancerProvider(PRIORITY_POLICY_NAME));
156-
lbRegistry.register(new FakeLoadBalancerProvider(CDS_POLICY_NAME));
157154
lbRegistry.register(new FakeLoadBalancerProvider(CLUSTER_RESOLVER_POLICY_NAME));
158155
lbRegistry.register(new FakeLoadBalancerProvider("round_robin"));
159156
lbRegistry.register(
@@ -162,7 +159,9 @@ public void setUp() throws Exception {
162159
new LeastRequestLoadBalancerProvider()));
163160
lbRegistry.register(new FakeLoadBalancerProvider("wrr_locality_experimental",
164161
new WrrLocalityLoadBalancerProvider()));
165-
loadBalancer = new CdsLoadBalancer2(helper, lbRegistry);
162+
CdsLoadBalancerProvider cdsLoadBalancerProvider = new CdsLoadBalancerProvider(lbRegistry);
163+
lbRegistry.register(cdsLoadBalancerProvider);
164+
loadBalancer = (CdsLoadBalancer2) cdsLoadBalancerProvider.newLoadBalancer(helper);
166165

167166
cleanupRule.register(InProcessServerBuilder
168167
.forName("control-plane.example.com")
@@ -407,11 +406,16 @@ public void dynamicCluster() {
407406
}
408407

409408
@Test
410-
public void discoverAggregateCluster() {
409+
public void discoverAggregateCluster_createsPriorityLbPolicy() {
410+
lbRegistry.register(new FakeLoadBalancerProvider(PRIORITY_POLICY_NAME));
411+
CdsLoadBalancerProvider cdsLoadBalancerProvider = new CdsLoadBalancerProvider(lbRegistry);
412+
lbRegistry.register(cdsLoadBalancerProvider);
413+
loadBalancer = (CdsLoadBalancer2) cdsLoadBalancerProvider.newLoadBalancer(helper);
414+
411415
String cluster1 = "cluster-01.googleapis.com";
412416
String cluster2 = "cluster-02.googleapis.com";
413417
controlPlaneService.setXdsConfig(ADS_TYPE_URL_CDS, ImmutableMap.of(
414-
// cluster1 (aggr.) -> [cluster2 (EDS), cluster3 (EDS)]
418+
// CLUSTER (aggr.) -> [cluster1 (EDS), cluster2 (EDS)]
415419
CLUSTER, Cluster.newBuilder()
416420
.setName(CLUSTER)
417421
.setClusterType(Cluster.CustomClusterType.newBuilder()
@@ -438,15 +442,60 @@ public void discoverAggregateCluster() {
438442
PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig childConfig1 =
439443
childLbConfig.childConfigs.get(cluster1);
440444
assertThat(childConfig1.toString()).isEqualTo("PriorityChildConfig{childConfig="
441-
+ "GracefulSwitchLoadBalancer.Config{childFactory=FakeLoadBalancerProvider{policy="
442-
+ "cds_experimental, priority=0, available=true}, childConfig=CdsConfig{name="
443-
+ "cluster-01.googleapis.com, isDynamic=false}}, ignoreReresolution=false}");
445+
+ "GracefulSwitchLoadBalancer.Config{childFactory=CdsLoadBalancerProvider{"
446+
+ "policy=cds_experimental, priority=5, available=true}, childConfig=CdsConfig{"
447+
+ "name=cluster-01.googleapis.com, isDynamic=false}}, ignoreReresolution=false}");
444448
PriorityLoadBalancerProvider.PriorityLbConfig.PriorityChildConfig childConfig2 =
445449
childLbConfig.childConfigs.get(cluster2);
446450
assertThat(childConfig2.toString()).isEqualTo("PriorityChildConfig{childConfig="
447-
+ "GracefulSwitchLoadBalancer.Config{childFactory=FakeLoadBalancerProvider{policy="
448-
+ "cds_experimental, priority=1, available=true}, childConfig=CdsConfig{name="
449-
+ "cluster-02.googleapis.com, isDynamic=false}}, ignoreReresolution=false}");
451+
+ "GracefulSwitchLoadBalancer.Config{childFactory=CdsLoadBalancerProvider{"
452+
+ "policy=cds_experimental, priority=5, available=true}, childConfig=CdsConfig{"
453+
+ "name=cluster-02.googleapis.com, isDynamic=false}}, ignoreReresolution=false}");
454+
}
455+
456+
@Test
457+
// Both priorities will get tried using real priority LB policy.
458+
public void discoverAggregateCluster_testChildCdsLbPolicyParsing() {
459+
lbRegistry.register(new PriorityLoadBalancerProvider());
460+
CdsLoadBalancerProvider cdsLoadBalancerProvider = new CdsLoadBalancerProvider(lbRegistry);
461+
lbRegistry.register(cdsLoadBalancerProvider);
462+
loadBalancer = (CdsLoadBalancer2) cdsLoadBalancerProvider.newLoadBalancer(helper);
463+
464+
String cluster1 = "cluster-01.googleapis.com";
465+
String cluster2 = "cluster-02.googleapis.com";
466+
controlPlaneService.setXdsConfig(ADS_TYPE_URL_CDS, ImmutableMap.of(
467+
// CLUSTER (aggr.) -> [cluster1 (EDS), cluster2 (EDS)]
468+
CLUSTER, Cluster.newBuilder()
469+
.setName(CLUSTER)
470+
.setClusterType(Cluster.CustomClusterType.newBuilder()
471+
.setName("envoy.clusters.aggregate")
472+
.setTypedConfig(Any.pack(ClusterConfig.newBuilder()
473+
.addClusters(cluster1)
474+
.addClusters(cluster2)
475+
.build())))
476+
.build(),
477+
cluster1, EDS_CLUSTER.toBuilder().setName(cluster1).build(),
478+
cluster2, EDS_CLUSTER.toBuilder().setName(cluster2).build()));
479+
startXdsDepManager();
480+
481+
verify(helper, never()).updateBalancingState(eq(ConnectivityState.TRANSIENT_FAILURE), any());
482+
assertThat(childBalancers).hasSize(2);
483+
ClusterResolverConfig cluster1ResolverConfig =
484+
(ClusterResolverConfig) childBalancers.get(0).config;
485+
assertThat(cluster1ResolverConfig.discoveryMechanism.cluster)
486+
.isEqualTo("cluster-01.googleapis.com");
487+
assertThat(cluster1ResolverConfig.discoveryMechanism.type)
488+
.isEqualTo(DiscoveryMechanism.Type.EDS);
489+
assertThat(cluster1ResolverConfig.discoveryMechanism.edsServiceName)
490+
.isEqualTo("backend-service-1.googleapis.com");
491+
ClusterResolverConfig cluster2ResolverConfig =
492+
(ClusterResolverConfig) childBalancers.get(1).config;
493+
assertThat(cluster2ResolverConfig.discoveryMechanism.cluster)
494+
.isEqualTo("cluster-02.googleapis.com");
495+
assertThat(cluster2ResolverConfig.discoveryMechanism.type)
496+
.isEqualTo(DiscoveryMechanism.Type.EDS);
497+
assertThat(cluster2ResolverConfig.discoveryMechanism.edsServiceName)
498+
.isEqualTo("backend-service-1.googleapis.com");
450499
}
451500

452501
@Test
@@ -474,6 +523,11 @@ public void aggregateCluster_noChildren() {
474523

475524
@Test
476525
public void aggregateCluster_noNonAggregateClusterExits_returnErrorPicker() {
526+
lbRegistry.register(new PriorityLoadBalancerProvider());
527+
CdsLoadBalancerProvider cdsLoadBalancerProvider = new CdsLoadBalancerProvider(lbRegistry);
528+
lbRegistry.register(cdsLoadBalancerProvider);
529+
loadBalancer = (CdsLoadBalancer2) cdsLoadBalancerProvider.newLoadBalancer(helper);
530+
477531
String cluster1 = "cluster-01.googleapis.com";
478532
controlPlaneService.setXdsConfig(ADS_TYPE_URL_CDS, ImmutableMap.of(
479533
// CLUSTER (aggr.) -> [cluster1 (missing)]

0 commit comments

Comments
 (0)