Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: filter EDS localities with clarified specifications (v1.28.x backport) #6875

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions xds/src/main/java/io/grpc/xds/LocalityStore.java
Expand Up @@ -641,8 +641,7 @@ public void run() {
&& !localityLbInfo.childBalancer.canHandleEmptyAddressListFromNameResolution()) {
localityLbInfo.childBalancer.handleNameResolutionError(
Status.UNAVAILABLE.withDescription(
"No healthy address available from EDS update '" + localityLbEndpoints
+ "' for locality '" + locality + "'"));
"Locality " + locality + " has no healthy endpoint"));
} else {
localityLbInfo.childBalancer
.handleResolvedAddresses(ResolvedAddresses.newBuilder()
Expand Down
25 changes: 17 additions & 8 deletions xds/src/main/java/io/grpc/xds/XdsClientImpl.java
Expand Up @@ -946,18 +946,23 @@ private void handleEdsResponse(DiscoveryResponse edsResponse) {
errorMessage = "ClusterLoadAssignment " + clusterName + " : no locality endpoints.";
break;
}

// The policy.disable_overprovisioning field must be set to true.
// TODO(chengyuanzhang): temporarily not requiring this field to be set, should push
// server implementors to do this or TBD with design.

Set<Integer> priorities = new HashSet<>();
int maxPriority = -1;
for (io.envoyproxy.envoy.api.v2.endpoint.LocalityLbEndpoints localityLbEndpoints
: assignment.getEndpointsList()) {
// The lb_endpoints field for LbEndpoint must contain at least one entry.
if (localityLbEndpoints.getLbEndpointsCount() == 0) {
errorMessage = "ClusterLoadAssignment " + clusterName + " : locality with no endpoint.";
// Filter out localities without or with 0 weight.
if (!localityLbEndpoints.hasLoadBalancingWeight()
|| localityLbEndpoints.getLoadBalancingWeight().getValue() < 1) {
continue;
}
int localityPriority = localityLbEndpoints.getPriority();
if (localityPriority < 0) {
errorMessage =
"ClusterLoadAssignment " + clusterName + " : locality with negative priority.";
break;
}
maxPriority = Math.max(maxPriority, localityPriority);
priorities.add(localityPriority);
// The endpoint field of each lb_endpoints must be set.
// Inside of it: the address field must be set.
for (io.envoyproxy.envoy.api.v2.endpoint.LbEndpoint lbEndpoint
Expand All @@ -980,6 +985,10 @@ private void handleEdsResponse(DiscoveryResponse edsResponse) {
if (errorMessage != null) {
break;
}
if (priorities.size() != maxPriority + 1) {
errorMessage = "ClusterLoadAssignment " + clusterName + " : sparse priorities.";
break;
}
for (io.envoyproxy.envoy.api.v2.ClusterLoadAssignment.Policy.DropOverload dropOverload
: assignment.getPolicy().getDropOverloadsList()) {
updateBuilder.addDropPolicy(DropOverload.fromEnvoyProtoDropOverload(dropOverload));
Expand Down
8 changes: 4 additions & 4 deletions xds/src/test/java/io/grpc/xds/XdsClientImplTest.java
Expand Up @@ -2001,7 +2001,7 @@ public void multipleEndpointWatchers() {
buildLocalityLbEndpoints("region2", "zone2", "subzone2",
ImmutableList.of(
buildLbEndpoint("192.168.234.52", 8888, HealthStatus.UNKNOWN, 5)),
6, 1)),
6, 0)),
ImmutableList.<ClusterLoadAssignment.Policy.DropOverload>of())));

response = buildDiscoveryResponse("1", clusterLoadAssignments,
Expand All @@ -2027,7 +2027,7 @@ public void multipleEndpointWatchers() {
new LocalityLbEndpoints(
ImmutableList.of(
new LbEndpoint("192.168.234.52", 8888,
5, true)), 6, 1));
5, true)), 6, 0));
}

/**
Expand Down Expand Up @@ -2180,7 +2180,7 @@ public void addRemoveEndpointWatchers() {
buildLocalityLbEndpoints("region2", "zone2", "subzone2",
ImmutableList.of(
buildLbEndpoint("192.168.312.6", 443, HealthStatus.HEALTHY, 1)),
6, 1)),
6, 0)),
ImmutableList.<Policy.DropOverload>of())));

response = buildDiscoveryResponse("1", clusterLoadAssignments,
Expand All @@ -2205,7 +2205,7 @@ public void addRemoveEndpointWatchers() {
new LocalityLbEndpoints(
ImmutableList.of(
new LbEndpoint("192.168.312.6", 443, 1, true)),
6, 1));
6, 0));

// Cancel one of the watcher.
xdsClient.cancelEndpointDataWatch("cluster-foo.googleapis.com", watcher1);
Expand Down