From e7a208fbf6b2dd727de95e2ab4a257e23c06084a Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Tue, 27 Oct 2026 23:33:13 -0400 Subject: [PATCH 1/2] xds: Fail CDS cluster when EDS update has no usable endpoints When a previously READY EDS cluster received an update with no usable endpoints, CdsLoadBalancer2 passed the error to the delegate via handleNameResolutionError(). The priority/MultiChild subtree ignores that while READY, so the stale picker kept selecting endpoints the control plane had removed. Before ClusterResolverLB was merged into CdsLB2, this case published TRANSIENT_FAILURE and shut down the child policy. Use fail() for this case, and have fail() replace the shut-down GracefulSwitchLoadBalancer so a later update creates a fresh child policy instead of reusing the one that was shut down. Fixes #13141 Co-Authored-By: Claude Opus 5.5 --- .../java/io/grpc/xds/CdsLoadBalancer2.java | 4 +- .../xds/ClusterResolverLoadBalancerTest.java | 47 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java index 3a5ba17ac7a..7eddbf6e6ee 100644 --- a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java +++ b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java @@ -156,8 +156,7 @@ public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) { if (addresses.isEmpty()) { Status status = Status.UNAVAILABLE .withDescription("No usable endpoint from cluster: " + clusterName); - delegate.handleNameResolutionError(status); - return status; + return fail(status); } Object gracefulConfig = GracefulSwitchLoadBalancer.createLoadBalancingPolicyConfig( lbRegistry.getProvider(PRIORITY_POLICY_NAME), @@ -223,6 +222,7 @@ public void shutdown() { @CheckReturnValue // don't forget to return up the stack after the fail call private Status fail(Status error) { delegate.shutdown(); + delegate = new GracefulSwitchLoadBalancer(cdsLbHelper); addBackendServicePickDetailsLabel = false; helper.updateBalancingState( TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(error))); diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index f5fb6beb175..051b9e2590f 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -62,6 +62,7 @@ import io.grpc.HttpConnectProxiedSocketAddress; import io.grpc.InternalEquivalentAddressGroup; import io.grpc.LoadBalancer; +import io.grpc.LoadBalancer.FixedResultPicker; import io.grpc.LoadBalancer.Helper; import io.grpc.LoadBalancer.PickDetailsConsumer; import io.grpc.LoadBalancer.PickResult; @@ -887,6 +888,52 @@ public void handleEdsResource_noHealthyEndpoint() { assertPicker(pickerCaptor.getValue(), expectedStatus, null); } + @Test + public void handleEdsResource_allEndpointsRemovedAfterReady_failsAndRecovers() { + ClusterLoadAssignment clusterLoadAssignment = ClusterLoadAssignment.newBuilder() + .setClusterName(EDS_SERVICE_NAME) + .addEndpoints(LocalityLbEndpoints.newBuilder() + .setLoadBalancingWeight(UInt32Value.of(100)) + .setLocality(LOCALITY1) + .addLbEndpoints(newSocketLbEndpoint("127.0.0.1", 8000))) + .build(); + controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, ImmutableMap.of( + EDS_SERVICE_NAME, clusterLoadAssignment)); + startXdsDepManager(); + + FakeLoadBalancer childBalancer = Iterables.getOnlyElement(childBalancers); + childBalancer.helper.updateBalancingState( + ConnectivityState.READY, new FixedResultPicker(PickResult.withNoResult())); + verify(helper).updateBalancingState(eq(ConnectivityState.READY), any()); + + // An empty ClusterLoadAssignment must not leave the previous READY picker in place + controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, ImmutableMap.of( + EDS_SERVICE_NAME, + ClusterLoadAssignment.newBuilder().setClusterName(EDS_SERVICE_NAME).build())); + + assertThat(childBalancer.shutdown).isTrue(); + assertThat(childBalancers).isEmpty(); + verify(helper).updateBalancingState( + eq(ConnectivityState.TRANSIENT_FAILURE), pickerCaptor.capture()); + Status expectedStatus = Status.UNAVAILABLE + .withDescription("No usable endpoint from cluster: " + CLUSTER); + assertPicker(pickerCaptor.getValue(), expectedStatus, null); + + // Endpoints come back: a new child policy is created + controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, ImmutableMap.of( + EDS_SERVICE_NAME, clusterLoadAssignment.toBuilder() + .setEndpoints(0, clusterLoadAssignment.getEndpoints(0).toBuilder() + .setLbEndpoints(0, newSocketLbEndpoint("127.0.0.2", 8000))) + .build())); + + FakeLoadBalancer newChildBalancer = Iterables.getOnlyElement(childBalancers); + assertThat(newChildBalancer).isNotSameInstanceAs(childBalancer); + assertThat(newChildBalancer.shutdown).isFalse(); + assertAddressesEqual( + Arrays.asList(new EquivalentAddressGroup(newInetSocketAddress("127.0.0.2", 8000))), + newChildBalancer.addresses); + } + @Test public void onlyLogicalDnsCluster_endpointsResolved() { boolean originalVal = LoadStatsManager2.isEnabledOrcaLrsPropagation; From 2fc377563d7cd70e46e950919d704198b7324cd0 Mon Sep 17 00:00:00 2001 From: Megrez Lu Date: Sun, 1 Nov 2026 22:35:03 -0500 Subject: [PATCH 2/2] xds: Verify READY reaches the channel after CDS recovery in test Have the new child policy report READY after endpoints come back, to confirm the error picker is replaced. Co-Authored-By: Claude Opus 5.5 --- .../java/io/grpc/xds/ClusterResolverLoadBalancerTest.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index 051b9e2590f..16e4a35a665 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -31,6 +31,7 @@ import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -932,6 +933,11 @@ public void handleEdsResource_allEndpointsRemovedAfterReady_failsAndRecovers() { assertAddressesEqual( Arrays.asList(new EquivalentAddressGroup(newInetSocketAddress("127.0.0.2", 8000))), newChildBalancer.addresses); + + // The new child's READY state reaches the channel, replacing the error picker + newChildBalancer.helper.updateBalancingState( + ConnectivityState.READY, new FixedResultPicker(PickResult.withNoResult())); + verify(helper, times(2)).updateBalancingState(eq(ConnectivityState.READY), any()); } @Test