Skip to content

Commit d9f26e5

Browse files
committed
xds: Fix LBs blindly propagating XdsClient errors
This is similar to 2a45524 (for #8950) but for additional similar cases.
1 parent 9536204 commit d9f26e5

File tree

4 files changed

+36
-5
lines changed

4 files changed

+36
-5
lines changed

xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,12 @@ void shutdown() {
246246
}
247247

248248
@Override
249-
public void onError(final Status error) {
249+
public void onError(Status error) {
250+
Status status = Status.UNAVAILABLE
251+
.withDescription(
252+
String.format("Unable to load CDS %s. xDS server returned: %s: %s",
253+
name, error.getCode(), error.getDescription()))
254+
.withCause(error.getCause());
250255
syncContext.execute(new Runnable() {
251256
@Override
252257
public void run() {
@@ -255,7 +260,7 @@ public void run() {
255260
}
256261
// All watchers should receive the same error, so we only propagate it once.
257262
if (ClusterState.this == root) {
258-
handleClusterDiscoveryError(error);
263+
handleClusterDiscoveryError(status);
259264
}
260265
}
261266
});

xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,11 @@ public void run() {
461461
if (shutdown) {
462462
return;
463463
}
464-
status = error;
464+
String resourceName = edsServiceName != null ? edsServiceName : name;
465+
status = Status.UNAVAILABLE
466+
.withDescription(String.format("Unable to load EDS %s. xDS server returned: %s: %s",
467+
resourceName, error.getCode(), error.getDescription()))
468+
.withCause(error.getCause());
465469
logger.log(XdsLogLevel.WARNING, "Received EDS error: {0}", error);
466470
handleEndpointResolutionError();
467471
}

xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,10 @@ public void aggregateCluster_discoveryErrorBeforeChildLbCreated_returnErrorPicke
459459
xdsClient.deliverError(error);
460460
verify(helper).updateBalancingState(
461461
eq(ConnectivityState.TRANSIENT_FAILURE), pickerCaptor.capture());
462-
assertPicker(pickerCaptor.getValue(), error, null);
462+
Status expectedError = Status.UNAVAILABLE.withDescription(
463+
"Unable to load CDS cluster-foo.googleapis.com. xDS server returned: "
464+
+ "RESOURCE_EXHAUSTED: OOM");
465+
assertPicker(pickerCaptor.getValue(), expectedError, null);
463466
assertThat(childBalancers).isEmpty();
464467
}
465468

@@ -481,7 +484,8 @@ public void aggregateCluster_discoveryErrorAfterChildLbCreated_propagateToChildL
481484

482485
Status error = Status.RESOURCE_EXHAUSTED.withDescription("OOM");
483486
xdsClient.deliverError(error);
484-
assertThat(childLb.upstreamError).isEqualTo(error);
487+
assertThat(childLb.upstreamError.getCode()).isEqualTo(Status.Code.UNAVAILABLE);
488+
assertThat(childLb.upstreamError.getDescription()).contains("RESOURCE_EXHAUSTED: OOM");
485489
assertThat(childLb.shutdown).isFalse(); // child LB may choose to keep working
486490
}
487491

xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -850,6 +850,24 @@ public void resolutionErrorBeforeChildLbCreated_returnErrorPickerIfAllClustersEn
850850
null);
851851
}
852852

853+
@Test
854+
public void resolutionErrorBeforeChildLbCreated_edsOnly_returnErrorPicker() {
855+
ClusterResolverConfig config = new ClusterResolverConfig(
856+
Arrays.asList(edsDiscoveryMechanism1), roundRobin);
857+
deliverLbConfig(config);
858+
assertThat(xdsClient.watchers.keySet()).containsExactly(EDS_SERVICE_NAME1);
859+
assertThat(childBalancers).isEmpty();
860+
reset(helper);
861+
xdsClient.deliverError(Status.RESOURCE_EXHAUSTED.withDescription("OOM"));
862+
assertThat(childBalancers).isEmpty();
863+
verify(helper).updateBalancingState(
864+
eq(ConnectivityState.TRANSIENT_FAILURE), pickerCaptor.capture());
865+
PickResult result = pickerCaptor.getValue().pickSubchannel(mock(PickSubchannelArgs.class));
866+
Status actualStatus = result.getStatus();
867+
assertThat(actualStatus.getCode()).isEqualTo(Status.Code.UNAVAILABLE);
868+
assertThat(actualStatus.getDescription()).contains("RESOURCE_EXHAUSTED: OOM");
869+
}
870+
853871
@Test
854872
public void handleNameResolutionErrorFromUpstream_beforeChildLbCreated_returnErrorPicker() {
855873
ClusterResolverConfig config = new ClusterResolverConfig(

0 commit comments

Comments
 (0)