xds: Fix LBs blindly propagating XdsClient errors

This is similar to 2a45524 (for grpc#8950) but for additional similar cases.
ejona86 · Mar 24, 2022 · 67b41ab · 67b41ab
1 parent 8bc9b3b
commit 67b41ab
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 5 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java
@@ -246,7 +246,12 @@ void shutdown() {
       }
 
       @Override
-      public void onError(final Status error) {
+      public void onError(Status error) {
+        Status status = Status.UNAVAILABLE
+            .withDescription(
+                String.format("Unable to load CDS %s. xDS server returned: %s: %s",
+                  name, error.getCode(), error.getDescription()))
+            .withCause(error.getCause());
         syncContext.execute(new Runnable() {
           @Override
           public void run() {
@@ -255,7 +260,7 @@ public void run() {
             }
             // All watchers should receive the same error, so we only propagate it once.
             if (ClusterState.this == root) {
-              handleClusterDiscoveryError(error);
+              handleClusterDiscoveryError(status);
             }
           }
         });

diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java
@@ -461,7 +461,11 @@ public void run() {
             if (shutdown) {
               return;
             }
-            status = error;
+            String resourceName = edsServiceName != null ? edsServiceName : name;
+            status = Status.UNAVAILABLE
+                .withDescription(String.format("Unable to load EDS %s. xDS server returned: %s: %s",
+                      resourceName, error.getCode(), error.getDescription()))
+                .withCause(error.getCause());
             logger.log(XdsLogLevel.WARNING, "Received EDS error: {0}", error);
             handleEndpointResolutionError();
           }

diff --git a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java
@@ -459,7 +459,10 @@ public void aggregateCluster_discoveryErrorBeforeChildLbCreated_returnErrorPicke
     xdsClient.deliverError(error);
     verify(helper).updateBalancingState(
         eq(ConnectivityState.TRANSIENT_FAILURE), pickerCaptor.capture());
-    assertPicker(pickerCaptor.getValue(), error, null);
+    Status expectedError = Status.UNAVAILABLE.withDescription(
+        "Unable to load CDS cluster-foo.googleapis.com. xDS server returned: "
+        + "RESOURCE_EXHAUSTED: OOM");
+    assertPicker(pickerCaptor.getValue(), expectedError, null);
     assertThat(childBalancers).isEmpty();
   }
 
@@ -481,7 +484,8 @@ public void aggregateCluster_discoveryErrorAfterChildLbCreated_propagateToChildL
 
     Status error = Status.RESOURCE_EXHAUSTED.withDescription("OOM");
     xdsClient.deliverError(error);
-    assertThat(childLb.upstreamError).isEqualTo(error);
+    assertThat(childLb.upstreamError.getCode()).isEqualTo(Status.Code.UNAVAILABLE);
+    assertThat(childLb.upstreamError.getDescription()).contains("RESOURCE_EXHAUSTED: OOM");
     assertThat(childLb.shutdown).isFalse();  // child LB may choose to keep working
   }
 

diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java
@@ -850,6 +850,24 @@ public void resolutionErrorBeforeChildLbCreated_returnErrorPickerIfAllClustersEn
         null);
   }
 
+  @Test
+  public void resolutionErrorBeforeChildLbCreated_edsOnly_returnErrorPicker() {
+    ClusterResolverConfig config = new ClusterResolverConfig(
+        Arrays.asList(edsDiscoveryMechanism1), roundRobin);
+    deliverLbConfig(config);
+    assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1);
+    assertThat(childBalancers).isEmpty();
+    reset(helper);
+    xdsClient.deliverError(Status.RESOURCE_EXHAUSTED.withDescription("OOM"));
+    assertThat(childBalancers).isEmpty();
+    verify(helper).updateBalancingState(
+        eq(ConnectivityState.TRANSIENT_FAILURE), pickerCaptor.capture());
+    PickResult result = pickerCaptor.getValue().pickSubchannel(mock(PickSubchannelArgs.class));
+    Status actualStatus = result.getStatus();
+    assertThat(actualStatus.getCode()).isEqualTo(Status.Code.UNAVAILABLE);
+    assertThat(actualStatus.getDescription()).contains("RESOURCE_EXHAUSTED: OOM");
+  }
+
   @Test
   public void handleNameResolutionErrorFromUpstream_beforeChildLbCreated_returnErrorPicker() {
     ClusterResolverConfig config = new ClusterResolverConfig(