Skip to content

Commit 86ea936

Browse files
larry-safranlarry-safran
committed
Merge with master
1 parent 0ba5c84 commit 86ea936

File tree

3 files changed

+108
-37
lines changed

3 files changed

+108
-37
lines changed

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

+6-6
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,8 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
269269
XdsConfig xdsConfig = resolvedAddresses.getAttributes().get(XdsAttributes.XDS_CONFIG);
270270

271271
if (xdsConfig.getClusters().get(rootClusterName) == null) {
272-
return Status.UNAVAILABLE.withDescription(
273-
"CDS resource not found for root cluster: " + rootClusterName);
272+
return Status.UNAVAILABLE.withDescription(
273+
"CDS resource not found for root cluster: " + rootClusterName);
274274
}
275275

276276
logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
@@ -1155,8 +1155,8 @@ private void handleClusterDiscovered() {
11551155
childLb.shutdown();
11561156
childLb = null;
11571157
}
1158-
Status unavailable = Status.UNAVAILABLE.withDescription(String.format(
1159-
"CDS error: found 0 leaf (logical DNS or EDS) clusters for root cluster %s", root.name));
1158+
Status unavailable = Status.UNAVAILABLE.withDescription(
1159+
"CDS error: found 0 leaf (logical DNS or EDS) clusters for root cluster " + root.name);
11601160
helper.updateBalancingState(
11611161
TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(unavailable)));
11621162
return;
@@ -1244,7 +1244,7 @@ private ClusterStateDetails(String name, StatusOr<XdsClusterConfig> configOr) {
12441244
// We should only see leaf clusters here.
12451245
assert config.getChildren() instanceof XdsClusterConfig.EndpointConfig;
12461246
StatusOr<EdsUpdate> endpointConfigOr =
1247-
((XdsClusterConfig.EndpointConfig) config.getChildren()).getEndpoint();
1247+
((XdsClusterConfig.EndpointConfig) config.getChildren()).getEndpoint();
12481248
if (endpointConfigOr.hasValue()) {
12491249
endpointConfig = endpointConfigOr.getValue();
12501250
} else {
@@ -1327,7 +1327,7 @@ private void update(final CdsUpdate update, StatusOr<EdsUpdate> endpointConfig)
13271327
case EDS:
13281328
isLeaf = true;
13291329
assert endpointConfig != null;
1330-
if (endpointConfig.getStatus() != null) {
1330+
if (!endpointConfig.getStatus().isOk()) {
13311331
logger.log(XdsLogLevel.INFO, "EDS cluster {0}, edsServiceName: {1}, error: {2}",
13321332
update.clusterName(), update.edsServiceName(), endpointConfig.getStatus());
13331333
} else {

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -697,7 +697,8 @@ public void onChanged(XdsListenerResource.LdsUpdate update) {
697697

698698
if (virtualHosts != null) {
699699
// No RDS watcher since we are getting RDS updates via LDS
700-
boolean updateSuccessful = updateRoutes(virtualHosts, this, activeVirtualHost, this.rdsName == null);
700+
boolean updateSuccessful =
701+
updateRoutes(virtualHosts, this, activeVirtualHost, this.rdsName == null);
701702
this.rdsName = null;
702703
if (!updateSuccessful) {
703704
lastXdsConfig = null;

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

+100-30
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@
6060
import io.grpc.SynchronizationContext;
6161
import io.grpc.internal.ExponentialBackoffPolicy;
6262
import io.grpc.internal.GrpcUtil;
63-
import io.grpc.internal.ObjectPool;
6463
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
6564
import io.grpc.xds.CdsLoadBalancer2.ClusterResolverConfig;
6665
import io.grpc.xds.CdsLoadBalancer2.ClusterResolverConfig.DiscoveryMechanism;
@@ -92,6 +91,7 @@
9291
import java.util.HashMap;
9392
import java.util.List;
9493
import java.util.Map;
94+
import java.util.Objects;
9595
import java.util.concurrent.Executor;
9696
import java.util.stream.Collectors;
9797
import javax.annotation.Nullable;
@@ -274,7 +274,8 @@ public void basicTest() throws XdsResourceType.ResourceInvalidException {
274274
}
275275

276276
@Test
277-
//TODO: Code looks broken creating a second LB instead of updating the existing one or shutting it down
277+
//TODO: Code looks broken creating a second LB instead of updating the existing one or shutting
278+
// it down
278279
public void discoverTopLevelEdsCluster() {
279280
configWatcher.watchCluster(CLUSTER);
280281
CdsUpdate update =
@@ -284,33 +285,26 @@ public void discoverTopLevelEdsCluster() {
284285
xdsClient.deliverCdsUpdate(CLUSTER, update);
285286
assertThat(childBalancers).hasSize(1);
286287

287-
validateClusterImplConfig(getClusterImplConfig(childBalancers, CLUSTER), CLUSTER, EDS_SERVICE_NAME,
288-
null, LRS_SERVER_INFO, 100L, upstreamTlsContext, outlierDetection);
288+
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, CLUSTER), CLUSTER,
289+
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 100L, upstreamTlsContext, outlierDetection);
289290

290291
PriorityLbConfig.PriorityChildConfig priorityChildConfig =
291292
getPriorityChildConfig(childBalancers, CLUSTER);
292293
assertThat(getChildProvider(priorityChildConfig.childConfig)
293294
.getPolicyName()).isEqualTo("round_robin");
294295
}
295296

296-
private static ClusterImplConfig getClusterImplConfig(List<FakeLoadBalancer> childBalancers,
297-
String cluster) {
297+
private static Object getConfigOfPriorityGrandChild(List<FakeLoadBalancer> childBalancers,
298+
String cluster) {
298299
PriorityLbConfig.PriorityChildConfig priorityChildConfig =
299300
getPriorityChildConfig(childBalancers, cluster);
300301
assertNotNull("No cluster " + cluster + " in childBalancers", priorityChildConfig);
301302
Object clusterImplConfig = getChildConfig(priorityChildConfig.childConfig);
302-
if (clusterImplConfig instanceof ClusterImplConfig) {
303-
return (ClusterImplConfig) clusterImplConfig;
304-
}
305-
if (clusterImplConfig instanceof OutlierDetectionLoadBalancerConfig) {
306-
clusterImplConfig = getChildConfig(((OutlierDetectionLoadBalancerConfig) clusterImplConfig).childConfig);
307-
}
308-
309-
assertThat(clusterImplConfig).isInstanceOf(ClusterImplConfig.class);
310-
return (ClusterImplConfig) clusterImplConfig;
303+
return clusterImplConfig;
311304
}
312305

313-
private static PriorityLbConfig.PriorityChildConfig getPriorityChildConfig(List<FakeLoadBalancer> childBalancers, String cluster) {
306+
private static PriorityLbConfig.PriorityChildConfig
307+
getPriorityChildConfig(List<FakeLoadBalancer> childBalancers, String cluster) {
314308
for (FakeLoadBalancer fakeLB : childBalancers) {
315309
if (fakeLB.config instanceof PriorityLbConfig) {
316310
Map<String, PriorityLbConfig.PriorityChildConfig> childConfigs =
@@ -523,17 +517,15 @@ public void aggregateCluster_descendantClustersRevoked() throws IOException {
523517
xdsClient.deliverCdsUpdate(cluster2, update2);
524518
xdsClient.createAndDeliverEdsUpdate(update1.edsServiceName());
525519

526-
validateClusterImplConfig(getClusterImplConfig(childBalancers, cluster1), cluster1,
527-
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 200L,
528-
upstreamTlsContext, outlierDetection);
529-
validateClusterImplConfig(getClusterImplConfig(childBalancers, cluster2), cluster2,
530-
null, DNS_HOST_NAME, LRS_SERVER_INFO, 100L, null,
531-
null);
520+
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, cluster1), cluster1,
521+
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 200L, upstreamTlsContext, outlierDetection);
522+
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, cluster2), cluster2,
523+
null, DNS_HOST_NAME, LRS_SERVER_INFO, 100L, null, null);
532524

533525
// Revoke cluster1, should still be able to proceed with cluster2.
534526
xdsClient.deliverResourceNotExist(cluster1);
535527
assertThat(xdsClient.watchers.keySet()).containsExactly(CLUSTER, cluster1, cluster2);
536-
validateClusterImplConfig(getClusterImplConfig(childBalancers, CLUSTER),
528+
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, CLUSTER),
537529
cluster2, null, DNS_HOST_NAME, LRS_SERVER_INFO, 100L, null, null);
538530
verify(helper, never()).updateBalancingState(
539531
eq(ConnectivityState.TRANSIENT_FAILURE), any(SubchannelPicker.class));
@@ -908,32 +900,95 @@ private static void assertPicker(SubchannelPicker picker, Status expectedStatus,
908900
}
909901
}
910902

903+
// TODO anything calling this needs to be updated to use validateClusterImplConfig
911904
private static void validateDiscoveryMechanism(
912905
DiscoveryMechanism instance, String name,
913906
@Nullable String edsServiceName, @Nullable String dnsHostName,
914907
@Nullable ServerInfo lrsServerInfo, @Nullable Long maxConcurrentRequests,
915908
@Nullable UpstreamTlsContext tlsContext, @Nullable OutlierDetection outlierDetection) {
916909
assertThat(instance.cluster).isEqualTo(name);
917910
assertThat(instance.edsServiceName).isEqualTo(edsServiceName);
918-
// assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
911+
assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
919912
assertThat(instance.lrsServerInfo).isEqualTo(lrsServerInfo);
920913
assertThat(instance.maxConcurrentRequests).isEqualTo(maxConcurrentRequests);
921914
assertThat(instance.tlsContext).isEqualTo(tlsContext);
922-
// assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
915+
assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
916+
}
917+
918+
private static boolean outlierDetectionEquals(OutlierDetection outlierDetection,
919+
OutlierDetectionLoadBalancerConfig oDLbConfig) {
920+
if (outlierDetection == null || oDLbConfig == null) {
921+
return true;
922+
}
923+
924+
OutlierDetectionLoadBalancerConfig defaultConfig =
925+
new OutlierDetectionLoadBalancerConfig.Builder().build();
926+
// split out for readability and debugging
927+
Long expectedBaseEjectionTimeNanos = outlierDetection.baseEjectionTimeNanos() == null
928+
? outlierDetection.baseEjectionTimeNanos()
929+
: defaultConfig.baseEjectionTimeNanos;
930+
931+
Long expectedIntervalNanos = outlierDetection.intervalNanos() == null
932+
? outlierDetection.intervalNanos()
933+
: defaultConfig.intervalNanos;
934+
935+
OutlierDetectionLoadBalancerConfig.FailurePercentageEjection expectedFailurePercentageEjection =
936+
outlierDetection.failurePercentageEjection() == null
937+
? outlierDetection.failurePercentageEjection()
938+
: defaultConfig.failurePercentageEjection;
939+
940+
OutlierDetectionLoadBalancerConfig.SuccessRateEjection expectedSuccessRateEjection =
941+
outlierDetection.successRateEjection() == null
942+
? outlierDetection.successRateEjection()
943+
: defaultConfig.successRateEjection;
944+
945+
Long expectedMaxEjectionTimeNanos = outlierDetection.maxEjectionTimeNanos() == null
946+
? outlierDetection.maxEjectionTimeNanos()
947+
: defaultConfig.maxEjectionTimeNanos;
948+
949+
Integer expectedMaxEjectionPercent = outlierDetection.maxEjectionPercent() == null
950+
? outlierDetection.maxEjectionPercent()
951+
: defaultConfig.maxEjectionPercent;
952+
953+
boolean baseEjNanosEqual =
954+
Objects.equals(expectedBaseEjectionTimeNanos, oDLbConfig.baseEjectionTimeNanos);
955+
boolean intervalNanosEqual = Objects.equals(expectedIntervalNanos, oDLbConfig.intervalNanos);
956+
boolean failurePctEqual = Objects.equals(expectedFailurePercentageEjection,
957+
oDLbConfig.failurePercentageEjection);
958+
boolean successRateEjectEqual =
959+
Objects.equals(expectedSuccessRateEjection, oDLbConfig.successRateEjection);
960+
boolean maxEjectTimeEqual =
961+
Objects.equals(expectedMaxEjectionTimeNanos, oDLbConfig.maxEjectionTimeNanos);
962+
boolean maxEjectPctEqual =
963+
Objects.equals(expectedMaxEjectionPercent, oDLbConfig.maxEjectionPercent);
964+
965+
return baseEjNanosEqual && intervalNanosEqual && failurePctEqual && successRateEjectEqual
966+
&& maxEjectTimeEqual && maxEjectPctEqual;
923967
}
924968

925969
private static void validateClusterImplConfig(
926-
ClusterImplConfig instance, String name,
970+
Object lbConfig, String name,
927971
@Nullable String edsServiceName, @Nullable String dnsHostName,
928972
@Nullable ServerInfo lrsServerInfo, @Nullable Long maxConcurrentRequests,
929973
@Nullable UpstreamTlsContext tlsContext, @Nullable OutlierDetection outlierDetection) {
974+
ClusterImplConfig instance;
975+
976+
if (lbConfig instanceof OutlierDetectionLoadBalancerConfig) {
977+
instance = (ClusterImplConfig)
978+
getChildConfig(((OutlierDetectionLoadBalancerConfig) lbConfig).childConfig);
979+
assertThat(outlierDetectionEquals(outlierDetection,
980+
(OutlierDetectionLoadBalancerConfig) lbConfig)).isTrue();
981+
982+
} else {
983+
instance = (ClusterImplConfig) lbConfig;
984+
}
985+
930986
assertThat(instance.cluster).isEqualTo(name);
931987
assertThat(instance.edsServiceName).isEqualTo(edsServiceName);
932-
// assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
933988
assertThat(instance.lrsServerInfo).isEqualTo(lrsServerInfo);
934989
assertThat(instance.maxConcurrentRequests).isEqualTo(maxConcurrentRequests);
935990
assertThat(instance.tlsContext).isEqualTo(tlsContext);
936-
// assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
991+
// TODO look in instance.childConfig for dns
937992
}
938993

939994
private final class FakeLoadBalancerProvider extends LoadBalancerProvider {
@@ -1216,8 +1271,23 @@ private List<EquivalentAddressGroup> buildEagsForCluster(
12161271
}
12171272

12181273
private Object buildLbConfig(XdsConfig xdsConfig) {
1219-
// TODO build it for real
1220-
return new CdsConfig(CLUSTER);
1274+
ImmutableMap<String, StatusOr<XdsConfig.XdsClusterConfig>> clusters = xdsConfig.getClusters();
1275+
if (clusters == null || clusters.isEmpty()) {
1276+
return null;
1277+
}
1278+
1279+
// find the aggregate in xdsConfig.getClusters()
1280+
for (Map.Entry<String, StatusOr<XdsConfig.XdsClusterConfig>> entry : clusters.entrySet()) {
1281+
CdsUpdate.ClusterType clusterType =
1282+
entry.getValue().getValue().getClusterResource().clusterType();
1283+
if (clusterType == CdsUpdate.ClusterType.AGGREGATE) {
1284+
return new CdsConfig(entry.getKey());
1285+
}
1286+
}
1287+
1288+
// If no aggregate grab the first leaf cluster
1289+
String clusterName = clusters.keySet().stream().findFirst().get();
1290+
return new CdsConfig(clusterName);
12211291
}
12221292

12231293
@Override

0 commit comments

Comments
 (0)