Skip to content

Commit

Permalink
Merge with master
Browse files Browse the repository at this point in the history
  • Loading branch information
@larry-safran
larry-safran committed Feb 25, 2025
1 parent 0ba5c84 commit 86ea936
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 37 deletions.
12 changes: 6 additions & 6 deletions xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,8 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
XdsConfig xdsConfig = resolvedAddresses.getAttributes().get(XdsAttributes.XDS_CONFIG);

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

logger.log(XdsLogLevel.DEBUG, "Received resolution result: {0}", resolvedAddresses);
Expand Down Expand Up @@ -1155,8 +1155,8 @@ private void handleClusterDiscovered() {
childLb.shutdown();
childLb = null;
}
Status unavailable = Status.UNAVAILABLE.withDescription(String.format(
"CDS error: found 0 leaf (logical DNS or EDS) clusters for root cluster %s", root.name));
Status unavailable = Status.UNAVAILABLE.withDescription(
"CDS error: found 0 leaf (logical DNS or EDS) clusters for root cluster " + root.name);
helper.updateBalancingState(
TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(unavailable)));
return;
Expand Down Expand Up @@ -1244,7 +1244,7 @@ private ClusterStateDetails(String name, StatusOr<XdsClusterConfig> configOr) {
// We should only see leaf clusters here.
assert config.getChildren() instanceof XdsClusterConfig.EndpointConfig;
StatusOr<EdsUpdate> endpointConfigOr =
((XdsClusterConfig.EndpointConfig) config.getChildren()).getEndpoint();
((XdsClusterConfig.EndpointConfig) config.getChildren()).getEndpoint();
if (endpointConfigOr.hasValue()) {
endpointConfig = endpointConfigOr.getValue();
} else {
Expand Down Expand Up @@ -1327,7 +1327,7 @@ private void update(final CdsUpdate update, StatusOr<EdsUpdate> endpointConfig)
case EDS:
isLeaf = true;
assert endpointConfig != null;
if (endpointConfig.getStatus() != null) {
if (!endpointConfig.getStatus().isOk()) {
logger.log(XdsLogLevel.INFO, "EDS cluster {0}, edsServiceName: {1}, error: {2}",
update.clusterName(), update.edsServiceName(), endpointConfig.getStatus());
} else {
Expand Down
3 changes: 2 additions & 1 deletion xds/src/main/java/io/grpc/xds/XdsDependencyManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,8 @@ public void onChanged(XdsListenerResource.LdsUpdate update) {

if (virtualHosts != null) {
// No RDS watcher since we are getting RDS updates via LDS
boolean updateSuccessful = updateRoutes(virtualHosts, this, activeVirtualHost, this.rdsName == null);
boolean updateSuccessful =
updateRoutes(virtualHosts, this, activeVirtualHost, this.rdsName == null);
this.rdsName = null;
if (!updateSuccessful) {
lastXdsConfig = null;
Expand Down
130 changes: 100 additions & 30 deletions xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import io.grpc.SynchronizationContext;
import io.grpc.internal.ExponentialBackoffPolicy;
import io.grpc.internal.GrpcUtil;
import io.grpc.internal.ObjectPool;
import io.grpc.util.OutlierDetectionLoadBalancer.OutlierDetectionLoadBalancerConfig;
import io.grpc.xds.CdsLoadBalancer2.ClusterResolverConfig;
import io.grpc.xds.CdsLoadBalancer2.ClusterResolverConfig.DiscoveryMechanism;
Expand Down Expand Up @@ -92,6 +91,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.Executor;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -274,7 +274,8 @@ public void basicTest() throws XdsResourceType.ResourceInvalidException {
}

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

validateClusterImplConfig(getClusterImplConfig(childBalancers, CLUSTER), CLUSTER, EDS_SERVICE_NAME,
null, LRS_SERVER_INFO, 100L, upstreamTlsContext, outlierDetection);
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, CLUSTER), CLUSTER,
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 100L, upstreamTlsContext, outlierDetection);

PriorityLbConfig.PriorityChildConfig priorityChildConfig =
getPriorityChildConfig(childBalancers, CLUSTER);
assertThat(getChildProvider(priorityChildConfig.childConfig)
.getPolicyName()).isEqualTo("round_robin");
}

private static ClusterImplConfig getClusterImplConfig(List<FakeLoadBalancer> childBalancers,
String cluster) {
private static Object getConfigOfPriorityGrandChild(List<FakeLoadBalancer> childBalancers,
String cluster) {
PriorityLbConfig.PriorityChildConfig priorityChildConfig =
getPriorityChildConfig(childBalancers, cluster);
assertNotNull("No cluster " + cluster + " in childBalancers", priorityChildConfig);
Object clusterImplConfig = getChildConfig(priorityChildConfig.childConfig);
if (clusterImplConfig instanceof ClusterImplConfig) {
return (ClusterImplConfig) clusterImplConfig;
}
if (clusterImplConfig instanceof OutlierDetectionLoadBalancerConfig) {
clusterImplConfig = getChildConfig(((OutlierDetectionLoadBalancerConfig) clusterImplConfig).childConfig);
}

assertThat(clusterImplConfig).isInstanceOf(ClusterImplConfig.class);
return (ClusterImplConfig) clusterImplConfig;
return clusterImplConfig;
}

private static PriorityLbConfig.PriorityChildConfig getPriorityChildConfig(List<FakeLoadBalancer> childBalancers, String cluster) {
private static PriorityLbConfig.PriorityChildConfig
getPriorityChildConfig(List<FakeLoadBalancer> childBalancers, String cluster) {
for (FakeLoadBalancer fakeLB : childBalancers) {
if (fakeLB.config instanceof PriorityLbConfig) {
Map<String, PriorityLbConfig.PriorityChildConfig> childConfigs =
Expand Down Expand Up @@ -523,17 +517,15 @@ public void aggregateCluster_descendantClustersRevoked() throws IOException {
xdsClient.deliverCdsUpdate(cluster2, update2);
xdsClient.createAndDeliverEdsUpdate(update1.edsServiceName());

validateClusterImplConfig(getClusterImplConfig(childBalancers, cluster1), cluster1,
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 200L,
upstreamTlsContext, outlierDetection);
validateClusterImplConfig(getClusterImplConfig(childBalancers, cluster2), cluster2,
null, DNS_HOST_NAME, LRS_SERVER_INFO, 100L, null,
null);
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, cluster1), cluster1,
EDS_SERVICE_NAME, null, LRS_SERVER_INFO, 200L, upstreamTlsContext, outlierDetection);
validateClusterImplConfig(getConfigOfPriorityGrandChild(childBalancers, cluster2), cluster2,
null, DNS_HOST_NAME, LRS_SERVER_INFO, 100L, null, null);

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

// TODO anything calling this needs to be updated to use validateClusterImplConfig
private static void validateDiscoveryMechanism(
DiscoveryMechanism instance, String name,
@Nullable String edsServiceName, @Nullable String dnsHostName,
@Nullable ServerInfo lrsServerInfo, @Nullable Long maxConcurrentRequests,
@Nullable UpstreamTlsContext tlsContext, @Nullable OutlierDetection outlierDetection) {
assertThat(instance.cluster).isEqualTo(name);
assertThat(instance.edsServiceName).isEqualTo(edsServiceName);
// assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
assertThat(instance.lrsServerInfo).isEqualTo(lrsServerInfo);
assertThat(instance.maxConcurrentRequests).isEqualTo(maxConcurrentRequests);
assertThat(instance.tlsContext).isEqualTo(tlsContext);
// assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
}

private static boolean outlierDetectionEquals(OutlierDetection outlierDetection,
OutlierDetectionLoadBalancerConfig oDLbConfig) {
if (outlierDetection == null || oDLbConfig == null) {
return true;
}

OutlierDetectionLoadBalancerConfig defaultConfig =
new OutlierDetectionLoadBalancerConfig.Builder().build();
// split out for readability and debugging
Long expectedBaseEjectionTimeNanos = outlierDetection.baseEjectionTimeNanos() == null
? outlierDetection.baseEjectionTimeNanos()
: defaultConfig.baseEjectionTimeNanos;

Long expectedIntervalNanos = outlierDetection.intervalNanos() == null
? outlierDetection.intervalNanos()
: defaultConfig.intervalNanos;

OutlierDetectionLoadBalancerConfig.FailurePercentageEjection expectedFailurePercentageEjection =
outlierDetection.failurePercentageEjection() == null
? outlierDetection.failurePercentageEjection()
: defaultConfig.failurePercentageEjection;

OutlierDetectionLoadBalancerConfig.SuccessRateEjection expectedSuccessRateEjection =
outlierDetection.successRateEjection() == null
? outlierDetection.successRateEjection()
: defaultConfig.successRateEjection;

Long expectedMaxEjectionTimeNanos = outlierDetection.maxEjectionTimeNanos() == null
? outlierDetection.maxEjectionTimeNanos()
: defaultConfig.maxEjectionTimeNanos;

Integer expectedMaxEjectionPercent = outlierDetection.maxEjectionPercent() == null
? outlierDetection.maxEjectionPercent()
: defaultConfig.maxEjectionPercent;

boolean baseEjNanosEqual =
Objects.equals(expectedBaseEjectionTimeNanos, oDLbConfig.baseEjectionTimeNanos);
boolean intervalNanosEqual = Objects.equals(expectedIntervalNanos, oDLbConfig.intervalNanos);
boolean failurePctEqual = Objects.equals(expectedFailurePercentageEjection,
oDLbConfig.failurePercentageEjection);
boolean successRateEjectEqual =
Objects.equals(expectedSuccessRateEjection, oDLbConfig.successRateEjection);
boolean maxEjectTimeEqual =
Objects.equals(expectedMaxEjectionTimeNanos, oDLbConfig.maxEjectionTimeNanos);
boolean maxEjectPctEqual =
Objects.equals(expectedMaxEjectionPercent, oDLbConfig.maxEjectionPercent);

return baseEjNanosEqual && intervalNanosEqual && failurePctEqual && successRateEjectEqual
&& maxEjectTimeEqual && maxEjectPctEqual;
}

private static void validateClusterImplConfig(
ClusterImplConfig instance, String name,
Object lbConfig, String name,
@Nullable String edsServiceName, @Nullable String dnsHostName,
@Nullable ServerInfo lrsServerInfo, @Nullable Long maxConcurrentRequests,
@Nullable UpstreamTlsContext tlsContext, @Nullable OutlierDetection outlierDetection) {
ClusterImplConfig instance;

if (lbConfig instanceof OutlierDetectionLoadBalancerConfig) {
instance = (ClusterImplConfig)
getChildConfig(((OutlierDetectionLoadBalancerConfig) lbConfig).childConfig);
assertThat(outlierDetectionEquals(outlierDetection,
(OutlierDetectionLoadBalancerConfig) lbConfig)).isTrue();

} else {
instance = (ClusterImplConfig) lbConfig;
}

assertThat(instance.cluster).isEqualTo(name);
assertThat(instance.edsServiceName).isEqualTo(edsServiceName);
// assertThat(instance.dnsHostName).isEqualTo(dnsHostName);
assertThat(instance.lrsServerInfo).isEqualTo(lrsServerInfo);
assertThat(instance.maxConcurrentRequests).isEqualTo(maxConcurrentRequests);
assertThat(instance.tlsContext).isEqualTo(tlsContext);
// assertThat(instance.outlierDetection).isEqualTo(outlierDetection);
// TODO look in instance.childConfig for dns
}

private final class FakeLoadBalancerProvider extends LoadBalancerProvider {
Expand Down Expand Up @@ -1216,8 +1271,23 @@ private List<EquivalentAddressGroup> buildEagsForCluster(
}

private Object buildLbConfig(XdsConfig xdsConfig) {
// TODO build it for real
return new CdsConfig(CLUSTER);
ImmutableMap<String, StatusOr<XdsConfig.XdsClusterConfig>> clusters = xdsConfig.getClusters();
if (clusters == null || clusters.isEmpty()) {
return null;
}

// find the aggregate in xdsConfig.getClusters()
for (Map.Entry<String, StatusOr<XdsConfig.XdsClusterConfig>> entry : clusters.entrySet()) {
CdsUpdate.ClusterType clusterType =
entry.getValue().getValue().getClusterResource().clusterType();
if (clusterType == CdsUpdate.ClusterType.AGGREGATE) {
return new CdsConfig(entry.getKey());
}
}

// If no aggregate grab the first leaf cluster
String clusterName = clusters.keySet().stream().findFirst().get();
return new CdsConfig(clusterName);
}

@Override
Expand Down

0 comments on commit 86ea936

Please sign in to comment.