xds: include node ID in RPC failure status messages from the XdsClient

grpc · Apr 20, 2022 · b0bd2dd · b0bd2dd
1 parent 3a303af
commit b0bd2dd
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 23 deletions.
diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java
@@ -2537,8 +2537,17 @@ void onError(Status error) {
         respTimer.cancel();
         respTimer = null;
       }
+
+      // Include node ID in RPC failure status messages that originate from XdsClient.
+      // We expect all watchers to propagate the description to the channel, and expose it
+      // to the caller.
+      String description = error.getDescription() == null ? "" : error.getDescription();
+      Status errorAugmented = Status.fromCode(error.getCode())
+          .withDescription(description + " Node ID: " + bootstrapInfo.node().getId())
+          .withCause(error.getCause());
+
       for (ResourceWatcher watcher : watchers) {
-        watcher.onError(error);
+        watcher.onError(errorAugmented);
       }
     }
 

diff --git a/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java b/xds/src/test/java/io/grpc/xds/ClientXdsClientTestBase.java
@@ -126,7 +126,8 @@ public abstract class ClientXdsClientTestBase {
   private static final String VERSION_1 = "42";
   private static final String VERSION_2 = "43";
   private static final String VERSION_3 = "44";
-  private static final Node NODE = Node.newBuilder().build();
+  private static final String NODE_ID = "cool-node-id";
+  private static final Node NODE = Node.newBuilder().setId(NODE_ID).build();
   private static final Any FAILING_ANY = MessageFactory.FAILING_ANY;
   private static final ChannelCredentials CHANNEL_CREDENTIALS = InsecureChannelCredentials.create();
   private final ServerInfo lrsServerInfo =
@@ -314,7 +315,7 @@ ManagedChannel create(ServerInfo serverInfo) {
         Bootstrapper.BootstrapInfo.builder()
             .servers(Arrays.asList(
                 Bootstrapper.ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, useProtocolV3())))
-            .node(EnvoyProtoData.Node.newBuilder().build())
+            .node(NODE)
             .authorities(ImmutableMap.of(
                 "authority.xds.com",
                 AuthorityInfo.create(
@@ -467,6 +468,16 @@ private ResourceMetadata verifyResourceMetadata(
     return metadata;
   }
 
+  private void verifyWatcherOnError(ResourceWatcher watcher, String errorMsg) {
+    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
+    verify(watcher).onError(captor.capture());
+    Status errorStatus = captor.getValue();
+    assertThat(errorStatus.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
+    // Watcher.onError propagates status description to the channel, and we want to
+    // augment the description with the node id.
+    assertThat(errorStatus.getDescription()).isEqualTo(errorMsg + " Node ID: " + NODE_ID);
+  }
+
   /**
    * Helper method to validate {@link XdsClient.EdsUpdate} created for the test CDS resource
    * {@link ClientXdsClientTestBase#testClusterLoadAssignment}.
@@ -1840,11 +1851,7 @@ public void cdsResponseErrorHandling_badUpstreamTlsContext() {
             + "io.grpc.xds.ClientXdsClient$ResourceInvalidException: "
             + "ca_certificate_provider_instance is required in upstream-tls-context";
     call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg));
-    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
-    verify(cdsResourceWatcher).onError(captor.capture());
-    Status errorStatus = captor.getValue();
-    assertThat(errorStatus.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
-    assertThat(errorStatus.getDescription()).isEqualTo(errorMsg);
+    verifyWatcherOnError(cdsResourceWatcher, errorMsg);
   }
 
   /**
@@ -1866,11 +1873,7 @@ public void cdsResponseErrorHandling_badTransportSocketName() {
     String errorMsg = "CDS response Cluster 'cluster.googleapis.com' validation error: "
         + "transport-socket with name envoy.transport_sockets.bad not supported.";
     call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg));
-    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
-    verify(cdsResourceWatcher).onError(captor.capture());
-    Status errorStatus = captor.getValue();
-    assertThat(errorStatus.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
-    assertThat(errorStatus.getDescription()).isEqualTo(errorMsg);
+    verifyWatcherOnError(cdsResourceWatcher, errorMsg);
   }
 
   @Test
@@ -2740,11 +2743,7 @@ public void serverSideListenerResponseErrorHandling_badDownstreamTlsContext() {
         + "0.0.0.0:7000\' validation error: "
         + "common-tls-context is required in downstream-tls-context";
     call.verifyRequestNack(LDS, LISTENER_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg));
-    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
-    verify(ldsResourceWatcher).onError(captor.capture());
-    Status errorStatus = captor.getValue();
-    assertThat(errorStatus.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
-    assertThat(errorStatus.getDescription()).isEqualTo(errorMsg);
+    verifyWatcherOnError(ldsResourceWatcher, errorMsg);
   }
 
   @Test
@@ -2770,11 +2769,7 @@ public void serverSideListenerResponseErrorHandling_badTransportSocketName() {
         + "transport-socket with name envoy.transport_sockets.bad1 not supported.";
     call.verifyRequestNack(LDS, LISTENER_RESOURCE, "", "0000", NODE, ImmutableList.of(
         errorMsg));
-    ArgumentCaptor<Status> captor = ArgumentCaptor.forClass(Status.class);
-    verify(ldsResourceWatcher).onError(captor.capture());
-    Status errorStatus = captor.getValue();
-    assertThat(errorStatus.getCode()).isEqualTo(Status.UNAVAILABLE.getCode());
-    assertThat(errorStatus.getDescription()).isEqualTo(errorMsg);
+    verifyWatcherOnError(ldsResourceWatcher, errorMsg);
   }
 
   private DiscoveryRpcCall startResourceWatcher(