From 2c7f9005d37f011034798df2b73a6e48e70d483c Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Tue, 26 Apr 2022 17:26:37 -0700 Subject: [PATCH 1/3] fix --- .../main/java/io/grpc/xds/ClientXdsClient.java | 18 ++++++++++++------ .../io/grpc/xds/ClientXdsClientTestBase.java | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 3202bb703ae..79c8783da62 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -1843,7 +1843,7 @@ public void handleEdsResponse( private static EdsUpdate processClusterLoadAssignment(ClusterLoadAssignment assignment) throws ResourceInvalidException { - Set priorities = new HashSet<>(); + Map> priorities = new HashMap<>(); Map localityLbEndpointsMap = new LinkedHashMap<>(); List dropOverloads = new ArrayList<>(); int maxPriority = -1; @@ -1859,14 +1859,20 @@ private static EdsUpdate processClusterLoadAssignment(ClusterLoadAssignment assi } LocalityLbEndpoints localityLbEndpoints = structOrError.getStruct(); - maxPriority = Math.max(maxPriority, localityLbEndpoints.priority()); - priorities.add(localityLbEndpoints.priority()); + int priority = localityLbEndpoints.priority(); + maxPriority = Math.max(maxPriority, priority); // Note endpoints with health status other than HEALTHY and UNKNOWN are still // handed over to watching parties. It is watching parties' responsibility to // filter out unhealthy endpoints. See EnvoyProtoData.LbEndpoint#isHealthy(). - localityLbEndpointsMap.put( - parseLocality(localityLbEndpointsProto.getLocality()), - localityLbEndpoints); + Locality locality = parseLocality(localityLbEndpointsProto.getLocality()); + localityLbEndpointsMap.put(locality, localityLbEndpoints); + if (!priorities.containsKey(priority)) { + priorities.put(localityLbEndpoints.priority(), new HashSet<>()); + } + if (!priorities.get(priority).add(locality)) { + throw new ResourceInvalidException("duplicate locality " + locality + + " for priority:" + priority); + } } if (priorities.size() != maxPriority + 1) { throw new ResourceInvalidException("ClusterLoadAssignment has sparse priorities"); diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index bd662b1da0d..6271b8847e8 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -2342,6 +2342,24 @@ public void edsResourceUpdated() { verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); } + @Test + public void edsDuplicateLocalityInTheSamePriority() { + DiscoveryRpcCall call = startResourceWatcher(EDS, EDS_RESOURCE, edsResourceWatcher); + verifyResourceMetadataRequested(EDS, EDS_RESOURCE); + + // Updated EDS response. + Any updatedClusterLoadAssignment = Any.pack(mf.buildClusterLoadAssignment(EDS_RESOURCE, + ImmutableList.of(mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", + mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 0), + mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", + mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1) + ), + ImmutableList.of())); + call.sendResponse(EDS, updatedClusterLoadAssignment, VERSION_1, "0001"); + call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0001", NODE); + verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); + } + @Test public void edsResourceDeletedByCds() { String resource = "backend-service.googleapis.com"; From f9b2cbfb617f4d7a350876281eacfccb8aecfeb7 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Wed, 27 Apr 2022 13:26:54 -0700 Subject: [PATCH 2/3] fix test.. --- xds/src/main/java/io/grpc/xds/ClientXdsClient.java | 6 +++--- .../java/io/grpc/xds/ClientXdsClientTestBase.java | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index 79c8783da62..31d4d318f71 100644 --- a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java @@ -1867,11 +1867,11 @@ private static EdsUpdate processClusterLoadAssignment(ClusterLoadAssignment assi Locality locality = parseLocality(localityLbEndpointsProto.getLocality()); localityLbEndpointsMap.put(locality, localityLbEndpoints); if (!priorities.containsKey(priority)) { - priorities.put(localityLbEndpoints.priority(), new HashSet<>()); + priorities.put(priority, new HashSet<>()); } if (!priorities.get(priority).add(locality)) { - throw new ResourceInvalidException("duplicate locality " + locality - + " for priority:" + priority); + throw new ResourceInvalidException("ClusterLoadAssignment has duplicate locality:" + + locality + " for priority:" + priority); } } if (priorities.size() != maxPriority + 1) { diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index 6271b8847e8..b79f8997465 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -2350,14 +2350,18 @@ public void edsDuplicateLocalityInTheSamePriority() { // Updated EDS response. Any updatedClusterLoadAssignment = Any.pack(mf.buildClusterLoadAssignment(EDS_RESOURCE, ImmutableList.of(mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", - mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 0), + mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1), mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1) ), ImmutableList.of())); - call.sendResponse(EDS, updatedClusterLoadAssignment, VERSION_1, "0001"); - call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0001", NODE); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); + call.sendResponse(EDS, updatedClusterLoadAssignment, "0", "0001"); + String errorMsg = "EDS response ClusterLoadAssignment" + + " \'cluster-load-assignment.googleapis.com\' " + + "validation error: ClusterLoadAssignment has duplicate " + + "locality:Locality{region=region2, zone=zone2, subZone=subzone2} for priority:1"; + call.verifyRequestNack(EDS, EDS_RESOURCE, "", "0001", NODE, ImmutableList.of( + errorMsg)); } @Test From 2535d2068f7b37a8d96665932e6128c16141ffa0 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 28 Apr 2022 15:55:02 -0700 Subject: [PATCH 3/3] same locality differnet lbEndpoints --- xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java index b79f8997465..5e57add5c13 100644 --- a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java +++ b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java @@ -2349,10 +2349,11 @@ public void edsDuplicateLocalityInTheSamePriority() { // Updated EDS response. Any updatedClusterLoadAssignment = Any.pack(mf.buildClusterLoadAssignment(EDS_RESOURCE, - ImmutableList.of(mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", - mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1), + ImmutableList.of( + mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", + mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1), mf.buildLocalityLbEndpoints("region2", "zone2", "subzone2", - mf.buildLbEndpoint("172.44.2.2", 8000, "unknown", 3), 2, 1) + mf.buildLbEndpoint("172.44.2.3", 8080, "healthy", 10), 2, 1) ), ImmutableList.of())); call.sendResponse(EDS, updatedClusterLoadAssignment, "0", "0001");