Skip to content

Commit 2581a57

Browse files
authored
xds: filter EDS localities with clarified specifications (#6874) (#6875)
Fix logic of filtering localites in EDS responses: - Each LocalityLbEndpoints message is allowed to contain 0 LbEndpoints. - LocalityLbEndpoints without or with 0 weight are ignored. - NACK responses with sparse locality priorities.
1 parent 45ef4af commit 2581a57

File tree

3 files changed

+22
-14
lines changed

3 files changed

+22
-14
lines changed

xds/src/main/java/io/grpc/xds/LocalityStore.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,7 @@ public void run() {
641641
&& !localityLbInfo.childBalancer.canHandleEmptyAddressListFromNameResolution()) {
642642
localityLbInfo.childBalancer.handleNameResolutionError(
643643
Status.UNAVAILABLE.withDescription(
644-
"No healthy address available from EDS update '" + localityLbEndpoints
645-
+ "' for locality '" + locality + "'"));
644+
"Locality " + locality + " has no healthy endpoint"));
646645
} else {
647646
localityLbInfo.childBalancer
648647
.handleResolvedAddresses(ResolvedAddresses.newBuilder()

xds/src/main/java/io/grpc/xds/XdsClientImpl.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -946,18 +946,23 @@ private void handleEdsResponse(DiscoveryResponse edsResponse) {
946946
errorMessage = "ClusterLoadAssignment " + clusterName + " : no locality endpoints.";
947947
break;
948948
}
949-
950-
// The policy.disable_overprovisioning field must be set to true.
951-
// TODO(chengyuanzhang): temporarily not requiring this field to be set, should push
952-
// server implementors to do this or TBD with design.
953-
949+
Set<Integer> priorities = new HashSet<>();
950+
int maxPriority = -1;
954951
for (io.envoyproxy.envoy.api.v2.endpoint.LocalityLbEndpoints localityLbEndpoints
955952
: assignment.getEndpointsList()) {
956-
// The lb_endpoints field for LbEndpoint must contain at least one entry.
957-
if (localityLbEndpoints.getLbEndpointsCount() == 0) {
958-
errorMessage = "ClusterLoadAssignment " + clusterName + " : locality with no endpoint.";
953+
// Filter out localities without or with 0 weight.
954+
if (!localityLbEndpoints.hasLoadBalancingWeight()
955+
|| localityLbEndpoints.getLoadBalancingWeight().getValue() < 1) {
956+
continue;
957+
}
958+
int localityPriority = localityLbEndpoints.getPriority();
959+
if (localityPriority < 0) {
960+
errorMessage =
961+
"ClusterLoadAssignment " + clusterName + " : locality with negative priority.";
959962
break;
960963
}
964+
maxPriority = Math.max(maxPriority, localityPriority);
965+
priorities.add(localityPriority);
961966
// The endpoint field of each lb_endpoints must be set.
962967
// Inside of it: the address field must be set.
963968
for (io.envoyproxy.envoy.api.v2.endpoint.LbEndpoint lbEndpoint
@@ -980,6 +985,10 @@ private void handleEdsResponse(DiscoveryResponse edsResponse) {
980985
if (errorMessage != null) {
981986
break;
982987
}
988+
if (priorities.size() != maxPriority + 1) {
989+
errorMessage = "ClusterLoadAssignment " + clusterName + " : sparse priorities.";
990+
break;
991+
}
983992
for (io.envoyproxy.envoy.api.v2.ClusterLoadAssignment.Policy.DropOverload dropOverload
984993
: assignment.getPolicy().getDropOverloadsList()) {
985994
updateBuilder.addDropPolicy(DropOverload.fromEnvoyProtoDropOverload(dropOverload));

xds/src/test/java/io/grpc/xds/XdsClientImplTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,7 +2001,7 @@ public void multipleEndpointWatchers() {
20012001
buildLocalityLbEndpoints("region2", "zone2", "subzone2",
20022002
ImmutableList.of(
20032003
buildLbEndpoint("192.168.234.52", 8888, HealthStatus.UNKNOWN, 5)),
2004-
6, 1)),
2004+
6, 0)),
20052005
ImmutableList.<ClusterLoadAssignment.Policy.DropOverload>of())));
20062006

20072007
response = buildDiscoveryResponse("1", clusterLoadAssignments,
@@ -2027,7 +2027,7 @@ public void multipleEndpointWatchers() {
20272027
new LocalityLbEndpoints(
20282028
ImmutableList.of(
20292029
new LbEndpoint("192.168.234.52", 8888,
2030-
5, true)), 6, 1));
2030+
5, true)), 6, 0));
20312031
}
20322032

20332033
/**
@@ -2180,7 +2180,7 @@ public void addRemoveEndpointWatchers() {
21802180
buildLocalityLbEndpoints("region2", "zone2", "subzone2",
21812181
ImmutableList.of(
21822182
buildLbEndpoint("192.168.312.6", 443, HealthStatus.HEALTHY, 1)),
2183-
6, 1)),
2183+
6, 0)),
21842184
ImmutableList.<Policy.DropOverload>of())));
21852185

21862186
response = buildDiscoveryResponse("1", clusterLoadAssignments,
@@ -2205,7 +2205,7 @@ public void addRemoveEndpointWatchers() {
22052205
new LocalityLbEndpoints(
22062206
ImmutableList.of(
22072207
new LbEndpoint("192.168.312.6", 443, 1, true)),
2208-
6, 1));
2208+
6, 0));
22092209

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

0 commit comments

Comments
 (0)