diff options
author | Chengyuan Zhang <chengyuanzhang@google.com> | 2020-03-30 14:24:48 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-03-30 14:24:48 -0700 |
commit | 68391e4d1b0d9ae5ed7277eb8b2920e99f2dc1b0 (patch) | |
tree | 7ee170d890245287441c0521bc1b8b5d227f0657 | |
parent | e081f414a726eb812430fd0be0b4da4b9deaf70b (diff) | |
download | grpc-grpc-java-68391e4d1b0d9ae5ed7277eb8b2920e99f2dc1b0.tar.gz |
xds: filter EDS localities with clarified specifications (#6874)
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.
-rw-r--r-- | xds/src/main/java/io/grpc/xds/LocalityStore.java | 3 | ||||
-rw-r--r-- | xds/src/main/java/io/grpc/xds/XdsClientImpl.java | 25 | ||||
-rw-r--r-- | xds/src/test/java/io/grpc/xds/XdsClientImplTest.java | 8 |
3 files changed, 22 insertions, 14 deletions
diff --git a/xds/src/main/java/io/grpc/xds/LocalityStore.java b/xds/src/main/java/io/grpc/xds/LocalityStore.java index 2cfa1eeb9..60a24d5e7 100644 --- a/xds/src/main/java/io/grpc/xds/LocalityStore.java +++ b/xds/src/main/java/io/grpc/xds/LocalityStore.java @@ -622,8 +622,7 @@ interface LocalityStore { && !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() diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index 822137081..1f8053281 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -1173,18 +1173,23 @@ final class XdsClientImpl extends XdsClient { 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 @@ -1207,6 +1212,10 @@ final class XdsClientImpl extends XdsClient { 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)); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java index 3b62af82f..0bb57b658 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTest.java @@ -2132,7 +2132,7 @@ public class XdsClientImplTest { 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, @@ -2158,7 +2158,7 @@ public class XdsClientImplTest { new LocalityLbEndpoints( ImmutableList.of( new LbEndpoint("192.168.234.52", 8888, - 5, true)), 6, 1)); + 5, true)), 6, 0)); } /** @@ -2311,7 +2311,7 @@ public class XdsClientImplTest { 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, @@ -2336,7 +2336,7 @@ public class XdsClientImplTest { 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); |