diff --git a/xds/src/main/java/io/grpc/xds/ClientXdsClient.java b/xds/src/main/java/io/grpc/xds/ClientXdsClient.java index bec73d1c71d4..6b5a007d31ec 100644 --- 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 index 4fe067752d69..cb1a3371a2e3 100644 --- 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 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 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 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 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 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(