From a3a98a9da5762fe3cdca9428c8bd7f9c66b080ae Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Mon, 5 May 2025 14:28:59 +0530 Subject: [PATCH 1/3] xds: client watcher API update --- .../java/io/grpc/xds/CdsLoadBalancer2.java | 93 ++ .../grpc/xds/ClusterResolverLoadBalancer.java | 34 + .../io/grpc/xds/XdsDependencyManager.java | 149 ++- .../java/io/grpc/xds/XdsServerWrapper.java | 63 +- .../java/io/grpc/xds/client/XdsClient.java | 34 +- .../io/grpc/xds/client/XdsClientImpl.java | 26 +- .../io/grpc/xds/CdsLoadBalancer2Test.java | 24 + .../xds/ClusterResolverLoadBalancerTest.java | 6 + .../grpc/xds/GrpcXdsClientImplTestBase.java | 1127 ++++++++++++----- .../io/grpc/xds/XdsClientFallbackTest.java | 191 +-- .../io/grpc/xds/XdsClientFederationTest.java | 35 +- .../XdsClientWrapperForServerSdsTestMisc.java | 11 +- .../java/io/grpc/xds/XdsNameResolverTest.java | 58 +- .../io/grpc/xds/XdsServerBuilderTest.java | 10 +- .../java/io/grpc/xds/XdsServerTestHelper.java | 13 +- .../io/grpc/xds/XdsServerWrapperTest.java | 37 +- 16 files changed, 1338 insertions(+), 573 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java index bb44071a484..05e1b497d0d 100644 --- a/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java +++ b/xds/src/main/java/io/grpc/xds/CdsLoadBalancer2.java @@ -26,6 +26,7 @@ import io.grpc.LoadBalancerRegistry; import io.grpc.NameResolver; import io.grpc.Status; +// import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.internal.ObjectPool; import io.grpc.util.GracefulSwitchLoadBalancer; @@ -419,6 +420,98 @@ public void onChanged(final CdsUpdate update) { handleClusterDiscovered(); } + + // After A74 maybe: + /*@Override + public void onResourceChanged(StatusOr update) { + if (shutdown) { + return; + } + + discovered = true; + if (!update.hasValue()) { + Status error = Status.UNAVAILABLE + .withDescription(String.format("Unable to load CDS %s. xDS server returned: %s: %s", + name, update.getStatus().getCode(), update.getStatus().getDescription())) + .withCause(update.getStatus().getCause()); + if (ClusterState.this == root) { + handleClusterDiscoveryError(error); + } + result = null; + if (childClusterStates != null) { + for (ClusterState state : childClusterStates.values()) { + state.shutdown(); + } + childClusterStates = null; + } + handleClusterDiscovered(); + return; + } + + CdsUpdate cdsUpdate = update.getValue(); + logger.log(XdsLogLevel.DEBUG, "Received cluster update {0}", cdsUpdate); + result = cdsUpdate; + + if (cdsUpdate.clusterType() == ClusterType.AGGREGATE) { + isLeaf = false; + logger.log(XdsLogLevel.INFO, "Aggregate cluster {0}, underlying clusters: {1}", + cdsUpdate.clusterName(), cdsUpdate.prioritizedClusterNames()); + Map newChildStates = new LinkedHashMap<>(); + for (String cluster : cdsUpdate.prioritizedClusterNames()) { + if (newChildStates.containsKey(cluster)) { + logger.log(XdsLogLevel.WARNING, + String.format("duplicate cluster name %s in aggregate %s is being ignored", + cluster, cdsUpdate.clusterName())); + continue; + } + if (childClusterStates == null || !childClusterStates.containsKey(cluster)) { + ClusterState childState; + if (clusterStates.containsKey(cluster)) { + childState = clusterStates.get(cluster); + if (childState.shutdown) { + childState.start(); + } + } else { + childState = new ClusterState(cluster); + clusterStates.put(cluster, childState); + childState.start(); + } + newChildStates.put(cluster, childState); + } else { + newChildStates.put(cluster, childClusterStates.remove(cluster)); + } + } + if (childClusterStates != null) { // stop subscribing to revoked child clusters + for (ClusterState watcher : childClusterStates.values()) { + watcher.shutdown(); + } + } + childClusterStates = newChildStates; + } else if (cdsUpdate.clusterType() == ClusterType.EDS) { + isLeaf = true; + logger.log(XdsLogLevel.INFO, "EDS cluster {0}, edsServiceName: {1}", + cdsUpdate.clusterName(), cdsUpdate.edsServiceName()); + } else { // logical DNS + isLeaf = true; + logger.log(XdsLogLevel.INFO, "Logical DNS cluster {0}", cdsUpdate.clusterName()); + } + handleClusterDiscovered(); + } + + @Override + public void onAmbientError(Status error) { + if (shutdown) { + return; + } + 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()); + // All watchers should receive the same error, so we only propagate it once. + if (ClusterState.this == root) { + handleClusterDiscoveryError(status); + } + }*/ } } } diff --git a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java index c92f592ebc8..233f3ee3dda 100644 --- a/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java +++ b/xds/src/main/java/io/grpc/xds/ClusterResolverLoadBalancer.java @@ -33,6 +33,7 @@ import io.grpc.NameResolver; import io.grpc.NameResolver.ResolutionResult; import io.grpc.Status; +// import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.SynchronizationContext.ScheduledHandle; import io.grpc.internal.BackoffPolicy; @@ -394,6 +395,19 @@ protected void shutdown() { @Override public void onChanged(final EdsUpdate update) { + // Maybe after A74 + /*if (!resourceUpdate.getStatus().isOk()) { + Status error = resourceUpdate.getStatus(); + 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(); + return; + } + final EdsUpdate update = resourceUpdate.getValue();*/ class EndpointsUpdated implements Runnable { @Override public void run() { @@ -570,6 +584,26 @@ public void onError(final Status error) { logger.log(XdsLogLevel.WARNING, "Received EDS error: {0}", error); handleEndpointResolutionError(); } + + // Maybe after A74 + /*@Override + public void onAmbientError(final Status error) { + syncContext.execute(new Runnable() { + @Override + public void run() { + if (shutdown) { + return; + } + logger.log( + XdsLogLevel.WARNING, "Received transient error from EDS resource: {0}", error); + status = Status.UNAVAILABLE + .withDescription( + "Transient error while watching EDS resource: " + error.getDescription()) + .withCause(error.getCause()); + handleEndpointResolutionError(); + } + }); + }*/ } private final class LogicalDnsClusterState extends ClusterState { diff --git a/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java b/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java index 8cd3119727d..2f41e9631d5 100644 --- a/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java +++ b/xds/src/main/java/io/grpc/xds/XdsDependencyManager.java @@ -16,7 +16,6 @@ package io.grpc.xds; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.xds.client.XdsClient.ResourceUpdate; import static io.grpc.xds.client.XdsLogger.XdsLogLevel.DEBUG; @@ -29,9 +28,12 @@ import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.xds.VirtualHost.Route.RouteAction.ClusterWeight; +import io.grpc.xds.XdsClusterResource.CdsUpdate; import io.grpc.xds.XdsClusterResource.CdsUpdate.ClusterType; import io.grpc.xds.XdsConfig.XdsClusterConfig.AggregateConfig; import io.grpc.xds.XdsConfig.XdsClusterConfig.EndpointConfig; +import io.grpc.xds.XdsEndpointResource.EdsUpdate; +import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.XdsRouteConfigureResource.RdsUpdate; import io.grpc.xds.client.XdsClient; import io.grpc.xds.client.XdsClient.ResourceWatcher; @@ -225,7 +227,7 @@ private void cancelClusterWatcherTree(CdsWatcher root, Object parentContext) { return; } - XdsClusterResource.CdsUpdate cdsUpdate = root.getData().getValue(); + CdsUpdate cdsUpdate = root.getData().getValue(); switch (cdsUpdate.clusterType()) { case EDS: String edsServiceName = root.getEdsServiceName(); @@ -307,9 +309,9 @@ StatusOr buildUpdate() { } builder.setVirtualHost(activeVirtualHost); - Map> edsWatchers = + Map> edsWatchers = getWatchers(ENDPOINT_RESOURCE); - Map> cdsWatchers = + Map> cdsWatchers = getWatchers(CLUSTER_RESOURCE); // Only care about aggregates from LDS/RDS or subscriptions and the leaf clusters @@ -343,20 +345,20 @@ private Map> getWatchers( private void addLeavesToBuilder( XdsConfig.XdsConfigBuilder builder, - Map> edsWatchers, + Map> edsWatchers, Set leafNames) { for (String clusterName : leafNames) { CdsWatcher cdsWatcher = getCluster(clusterName); - StatusOr cdsUpdateOr = cdsWatcher.getData(); + StatusOr cdsUpdateOr = cdsWatcher.getData(); if (!cdsUpdateOr.hasValue()) { builder.addCluster(clusterName, StatusOr.fromStatus(cdsUpdateOr.getStatus())); continue; } - XdsClusterResource.CdsUpdate cdsUpdate = cdsUpdateOr.getValue(); + CdsUpdate cdsUpdate = cdsUpdateOr.getValue(); if (cdsUpdate.clusterType() == ClusterType.EDS) { - XdsWatcherBase edsWatcher = + XdsWatcherBase edsWatcher = edsWatchers.get(cdsWatcher.getEdsServiceName()); EndpointConfig child; if (edsWatcher != null) { @@ -377,20 +379,20 @@ private void addLeavesToBuilder( // Adds the top-level clusters to the builder and returns the leaf cluster names private Set addTopLevelClustersToBuilder( XdsConfig.XdsConfigBuilder builder, - Map> edsWatchers, - Map> cdsWatchers, + Map> edsWatchers, + Map> cdsWatchers, List topLevelClusters) { Set leafClusterNames = new HashSet<>(); for (String clusterName : topLevelClusters) { CdsWatcher cdsWatcher = (CdsWatcher) cdsWatchers.get(clusterName); - StatusOr cdsWatcherDataOr = cdsWatcher.getData(); + StatusOr cdsWatcherDataOr = cdsWatcher.getData(); if (!cdsWatcher.hasDataValue()) { builder.addCluster(clusterName, StatusOr.fromStatus(cdsWatcherDataOr.getStatus())); continue; } - XdsClusterResource.CdsUpdate cdsUpdate = cdsWatcherDataOr.getValue(); + CdsUpdate cdsUpdate = cdsWatcherDataOr.getValue(); XdsConfig.XdsClusterConfig.ClusterChild child; switch (cdsUpdate.clusterType()) { case AGGREGATE: @@ -400,7 +402,7 @@ private Set addTopLevelClustersToBuilder( leafClusterNames.addAll(leafNames); break; case EDS: - XdsWatcherBase edsWatcher = + XdsWatcherBase edsWatcher = edsWatchers.get(cdsWatcher.getEdsServiceName()); if (edsWatcher != null) { child = new EndpointConfig(edsWatcher.getData()); @@ -424,12 +426,12 @@ private Set addTopLevelClustersToBuilder( return leafClusterNames; } - private void addLeafNames(Set leafNames, XdsClusterResource.CdsUpdate cdsUpdate) { + private void addLeafNames(Set leafNames, CdsUpdate cdsUpdate) { for (String cluster : cdsUpdate.prioritizedClusterNames()) { if (leafNames.contains(cluster)) { continue; } - StatusOr data = getCluster(cluster).getData(); + StatusOr data = getCluster(cluster).getData(); if (data == null || !data.hasValue() || data.getValue() == null) { leafNames.add(cluster); continue; @@ -443,7 +445,7 @@ private void addLeafNames(Set leafNames, XdsClusterResource.CdsUpdate cd } private static boolean isTopLevelCluster( - XdsWatcherBase cdsWatcher) { + XdsWatcherBase cdsWatcher) { return ((CdsWatcher)cdsWatcher).parentContexts.values().stream() .anyMatch(depth -> depth == 1); } @@ -582,33 +584,37 @@ private abstract class XdsWatcherBase @Nullable private StatusOr data; - private XdsWatcherBase(XdsResourceType type, String resourceName) { this.type = checkNotNull(type, "type"); this.resourceName = checkNotNull(resourceName, "resourceName"); } @Override - public void onError(Status error) { - checkNotNull(error, "error"); - // Don't update configuration on error, if we've already received configuration - if (!hasDataValue()) { + public void onResourceChanged(StatusOr result) { + checkNotNull(result, "result"); + if (cancelled) { + return; + } + + if (result.hasValue()) { + setData(result.getValue()); + } else { setDataAsStatus(Status.UNAVAILABLE.withDescription( - String.format("Error retrieving %s: %s: %s", - toContextString(), error.getCode(), error.getDescription()))); - maybePublishConfig(); + String.format("Error retrieving %s: %s", + toContextString(), result.getStatus().getDescription()))); } + maybePublishConfig(); } @Override - public void onResourceDoesNotExist(String resourceName) { + public void onAmbientError(Status error) { + checkNotNull(error, "error"); if (cancelled) { return; } - - checkArgument(this.resourceName.equals(resourceName), "Resource name does not match"); setDataAsStatus(Status.UNAVAILABLE.withDescription( - toContextString() + " does not exist" + nodeInfo())); + String.format("Error retrieving %s: %s: %s", + toContextString(), error.getCode(), error.getDescription()))); maybePublishConfig(); } @@ -657,9 +663,21 @@ private LdsWatcher(String resourceName) { } @Override - public void onChanged(XdsListenerResource.LdsUpdate update) { - checkNotNull(update, "update"); + public void onResourceChanged(StatusOr updateOrError) { + checkNotNull(updateOrError, "LdsUpdateOrError"); + if (cancelled) { + return; + } + if (!updateOrError.hasValue()) { + setDataAsStatus(updateOrError.getStatus()); + cleanUpRdsWatcher(); + rdsName = null; + maybePublishConfig(); + return; + } + LdsUpdate update = updateOrError.getValue(); + checkNotNull(update, "ldsUpdate"); HttpConnectionManager httpConnectionManager = update.httpConnectionManager(); List virtualHosts; String rdsName; @@ -695,20 +713,6 @@ public void onChanged(XdsListenerResource.LdsUpdate update) { maybePublishConfig(); } - @Override - public void onResourceDoesNotExist(String resourceName) { - if (cancelled) { - return; - } - - checkArgument(resourceName().equals(resourceName), "Resource name does not match"); - setDataAsStatus(Status.UNAVAILABLE.withDescription( - toContextString() + " does not exist" + nodeInfo())); - cleanUpRdsWatcher(); - rdsName = null; - maybePublishConfig(); - } - private void cleanUpRdsWatcher() { RdsWatcher oldRdsWatcher = getRdsWatcher(); if (oldRdsWatcher != null) { @@ -785,13 +789,27 @@ public RdsWatcher(String resourceName) { } @Override - public void onChanged(RdsUpdate update) { - checkNotNull(update, "update"); - List oldVirtualHosts = hasDataValue() - ? getData().getValue().virtualHosts - : Collections.emptyList(); - setData(update); - updateRoutes(update.virtualHosts, this, oldVirtualHosts, true); + public void onResourceChanged(StatusOr updateOrError) { + checkNotNull(updateOrError, "RdsUpdateOrError"); + if (cancelled) { + return; + } + + if (updateOrError.hasValue()) { + RdsUpdate update = updateOrError.getValue(); + checkNotNull(update, "RdsUpdate"); + List oldVirtualHosts = hasDataValue() + ? getData().getValue().virtualHosts + : Collections.emptyList(); + setData(update); + updateRoutes(update.virtualHosts, this, oldVirtualHosts, true); + } else { + setDataAsStatus(Status.UNAVAILABLE.withDescription( + String.format("Error retrieving %s: %s: %s", + toContextString(), + updateOrError.getStatus().getCode(), + updateOrError.getStatus().getDescription()))); + } maybePublishConfig(); } @@ -804,7 +822,7 @@ public StatusOr getRdsUpdate() { } } - private class CdsWatcher extends XdsWatcherBase { + private class CdsWatcher extends XdsWatcherBase { Map parentContexts = new HashMap<>(); CdsWatcher(String resourceName, Object parentContext, int depth) { @@ -813,8 +831,19 @@ private class CdsWatcher extends XdsWatcherBase { } @Override - public void onChanged(XdsClusterResource.CdsUpdate update) { - checkNotNull(update, "update"); + public void onResourceChanged(StatusOr updateOrError) { + checkNotNull(updateOrError, "CdsUpdateOrError"); + if (cancelled) { + return; + } + if (!updateOrError.hasValue()) { + setDataAsStatus(updateOrError.getStatus()); + maybePublishConfig(); + return; + } + + CdsUpdate update = updateOrError.getValue(); + checkNotNull(update, "CdsUpdate"); switch (update.clusterType()) { case EDS: setData(update); @@ -875,7 +904,7 @@ public void onChanged(XdsClusterResource.CdsUpdate update) { } public String getEdsServiceName() { - XdsClusterResource.CdsUpdate cdsUpdate = getData().getValue(); + CdsUpdate cdsUpdate = getData().getValue(); assert cdsUpdate.clusterType() == ClusterType.EDS; String edsServiceName = cdsUpdate.edsServiceName(); if (edsServiceName == null) { @@ -885,7 +914,7 @@ public String getEdsServiceName() { } } - private class EdsWatcher extends XdsWatcherBase { + private class EdsWatcher extends XdsWatcherBase { private final Set parentContexts = new HashSet<>(); private EdsWatcher(String resourceName, CdsWatcher parentContext) { @@ -894,8 +923,12 @@ private EdsWatcher(String resourceName, CdsWatcher parentContext) { } @Override - public void onChanged(XdsEndpointResource.EdsUpdate update) { - setData(checkNotNull(update, "update")); + public void onResourceChanged(StatusOr updateOrError) { + checkNotNull(updateOrError, "CdsUpdateOrError"); + if (cancelled || !updateOrError.hasValue()) { + return; + } + setData(checkNotNull(updateOrError.getValue(), "update")); maybePublishConfig(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index 6625bd8178a..a245556d598 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -39,6 +39,7 @@ import io.grpc.ServerServiceDefinition; import io.grpc.Status; import io.grpc.StatusException; +import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.SynchronizationContext.ScheduledHandle; import io.grpc.internal.GrpcUtil; @@ -378,10 +379,19 @@ private DiscoveryState(String resourceName) { } @Override - public void onChanged(final LdsUpdate update) { + public void onResourceChanged(StatusOr updateOrError) { if (stopped) { return; } + if (!updateOrError.hasValue()) { + Status error = updateOrError.getStatus(); + StatusException statusException = error.withDescription( + error.getDescription() + " xDS node ID: " + + xdsClient.getBootstrapInfo().node().getId()).asException(); + handleConfigNotFound(statusException); + return; + } + LdsUpdate update = updateOrError.getValue(); logger.log(Level.FINEST, "Received Lds update {0}", update); checkNotNull(update.listener(), "update"); if (!pendingRds.isEmpty()) { @@ -433,18 +443,7 @@ public void onChanged(final LdsUpdate update) { } @Override - public void onResourceDoesNotExist(final String resourceName) { - if (stopped) { - return; - } - StatusException statusException = Status.UNAVAILABLE.withDescription( - String.format("Listener %s unavailable, xDS node ID: %s", resourceName, - xdsClient.getBootstrapInfo().node().getId())).asException(); - handleConfigNotFound(statusException); - } - - @Override - public void onError(final Status error) { + public void onAmbientError(Status error) { if (stopped) { return; } @@ -745,33 +744,29 @@ private RouteDiscoveryState(String resourceName) { } @Override - public void onChanged(final RdsUpdate update) { + public void onResourceChanged(StatusOr updateOrError) { syncContext.execute(new Runnable() { @Override public void run() { if (!routeDiscoveryStates.containsKey(resourceName)) { return; } - if (savedVirtualHosts == null && !isPending) { - logger.log(Level.WARNING, "Received valid Rds {0} configuration.", resourceName); - } - savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts); - updateRdsRoutingConfig(); - maybeUpdateSelector(); - } - }); - } - - @Override - public void onResourceDoesNotExist(final String resourceName) { - syncContext.execute(new Runnable() { - @Override - public void run() { - if (!routeDiscoveryStates.containsKey(resourceName)) { - return; + if (!updateOrError.hasValue()) { + Status error = updateOrError.getStatus(); + String description = error.getDescription() == null + ? "" : error.getDescription() + " "; + Status errorWithNodeId = error.withDescription( + description + "xDS node ID: " + xdsClient.getBootstrapInfo().node().getId()); + logger.log(Level.WARNING, "Error loading RDS resource {0} from XdsClient: {1}.", + new Object[]{resourceName, errorWithNodeId}); + savedVirtualHosts = null; + } else { + RdsUpdate update = updateOrError.getValue(); + if (savedVirtualHosts == null && !isPending) { + logger.log(Level.WARNING, "Received valid Rds {0} configuration.", resourceName); + } + savedVirtualHosts = ImmutableList.copyOf(update.virtualHosts); } - logger.log(Level.WARNING, "Rds {0} unavailable", resourceName); - savedVirtualHosts = null; updateRdsRoutingConfig(); maybeUpdateSelector(); } @@ -779,7 +774,7 @@ public void run() { } @Override - public void onError(final Status error) { + public void onAmbientError(Status error) { syncContext.execute(new Runnable() { @Override public void run() { diff --git a/xds/src/main/java/io/grpc/xds/client/XdsClient.java b/xds/src/main/java/io/grpc/xds/client/XdsClient.java index 1b53f6778c7..5a5bfbf0215 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClient.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClient.java @@ -27,6 +27,7 @@ import com.google.protobuf.Any; import io.grpc.ExperimentalApi; import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.xds.client.Bootstrapper.ServerInfo; import java.net.URI; import java.net.URISyntaxException; @@ -127,7 +128,21 @@ public interface ResourceUpdate {} public interface ResourceWatcher { /** - * Called when the resource discovery RPC encounters some transient error. + * Called when a new version of the resource is received, or when the resource becomes invalid. + * + *

If an error is passed, the watcher must stop using the resource and treat + * it as non-existent. + */ + default void onResourceChanged(StatusOr update) { + // do nothing + } + + /** + * Called for ambient errors (e.g., connection lost to xDS server). + * The watcher may retain its current state. + * + *

This error can be cleared by another call to onAmbientError() with OK, + * or a new onResourceChanged(). * *

Note that we expect that the implementer to: * - Comply with the guarantee to not generate certain statuses by the library: @@ -136,16 +151,27 @@ public interface ResourceWatcher { * - Keep {@link Status} description in one form or another, as it contains valuable debugging * information. */ - void onError(Status error); + default void onAmbientError(Status status) { + // do nothing + } + + // APIs (onError, onResourceDoesNotExist and onChanged) will be deleted in subsequent commit. + default void onError(Status error) { + // do nothing + } /** * Called when the requested resource is not available. * * @param resourceName name of the resource requested in discovery request. */ - void onResourceDoesNotExist(String resourceName); + default void onResourceDoesNotExist(String resourceName) { + // do nothing + } - void onChanged(T update); + default void onChanged(T update) { + // do nothing + } } /** diff --git a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java index 034779ed023..cdc279fbc62 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java @@ -34,6 +34,7 @@ import io.grpc.Internal; import io.grpc.InternalLogId; import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.SynchronizationContext.ScheduledHandle; import io.grpc.internal.BackoffPolicy; @@ -730,14 +731,15 @@ void addWatcher(ResourceWatcher watcher, Executor watcherExecutor) { T savedData = data; boolean savedAbsent = absent; watcherExecutor.execute(() -> { - if (errorDescription != null) { - watcher.onError(Status.INVALID_ARGUMENT.withDescription(errorDescription)); - return; - } if (savedData != null) { - notifyWatcher(watcher, savedData); + if (errorDescription != null) { + watcher.onAmbientError(Status.INVALID_ARGUMENT.withDescription(errorDescription)); + } else { + notifyWatcher(watcher, savedData); + } } else if (savedAbsent) { - watcher.onResourceDoesNotExist(resource); + watcher.onResourceChanged(StatusOr.fromStatus(Status.NOT_FOUND.withDescription( + "Resource " + resource + " does not exist"))); } }); } @@ -879,7 +881,8 @@ void onAbsent(@Nullable ProcessingTracker processingTracker, ServerInfo serverIn } watchers.get(watcher).execute(() -> { try { - watcher.onResourceDoesNotExist(resource); + watcher.onResourceChanged(StatusOr.fromStatus(Status.NOT_FOUND.withDescription( + "Resource " + resource + " does not exist"))); } finally { if (processingTracker != null) { processingTracker.onComplete(); @@ -909,7 +912,12 @@ void onError(Status error, @Nullable ProcessingTracker tracker) { } watchers.get(watcher).execute(() -> { try { - watcher.onError(errorAugmented); + if (data == null) { + watcher.onResourceChanged(StatusOr.fromStatus(errorAugmented)); + } else { + // Have cached data: continue using it + watcher.onAmbientError(errorAugmented); + } } finally { if (tracker != null) { tracker.onComplete(); @@ -926,7 +934,7 @@ void onRejected(String rejectedVersion, long rejectedTime, String rejectedDetail } private void notifyWatcher(ResourceWatcher watcher, T update) { - watcher.onChanged(update); + watcher.onResourceChanged(StatusOr.fromValue(update)); } } diff --git a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java index 479bde76ce5..251492b816d 100644 --- a/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java +++ b/xds/src/test/java/io/grpc/xds/CdsLoadBalancer2Test.java @@ -46,6 +46,7 @@ import io.grpc.NameResolver; import io.grpc.Status; import io.grpc.Status.Code; +// import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.internal.ObjectPool; import io.grpc.util.GracefulSwitchLoadBalancerAccessor; @@ -870,5 +871,28 @@ private void deliverError(Status error) { .flatMap(List::stream) .forEach(w -> w.onError(error)); } + + // After A74 maybe: + /*private void deliverCdsUpdate(String clusterName, CdsUpdate update) { + if (watchers.containsKey(clusterName)) { + List> resourceWatchers = + ImmutableList.copyOf(watchers.get(clusterName)); + resourceWatchers.forEach(w -> w.onResourceChanged(StatusOr.fromValue(update))); + } + } + + private void deliverResourceNotExist(String clusterName) { + if (watchers.containsKey(clusterName)) { + ImmutableList.copyOf(watchers.get(clusterName)) + .forEach(w -> w.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Resource " + clusterName + " does not exist")))); + } + } + + private void deliverError(Status error) { + watchers.values().stream() + .flatMap(List::stream) + .forEach(w -> w.onResourceChanged(StatusOr.fromStatus(error))); + }*/ } } diff --git a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java index d0176d7aa38..34a460a73dc 100644 --- a/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java +++ b/xds/src/test/java/io/grpc/xds/ClusterResolverLoadBalancerTest.java @@ -1302,18 +1302,24 @@ void deliverClusterLoadAssignment(String resource, List dropOverlo if (watchers.containsKey(resource)) { watchers.get(resource).onChanged( new XdsEndpointResource.EdsUpdate(resource, localityLbEndpointsMap, dropOverloads)); + /*watchers.get(resource).onResourceChanged(StatusOr.fromValue( + new XdsEndpointResource.EdsUpdate(resource, localityLbEndpointsMap, dropOverloads)));*/ } } void deliverResourceNotFound(String resource) { if (watchers.containsKey(resource)) { watchers.get(resource).onResourceDoesNotExist(resource); + /*Status status = Status.NOT_FOUND + .withDescription("Resource " + resource + " does not exist"); + watchers.get(resource).onResourceChanged(StatusOr.fromStatus(status));*/ } } void deliverError(Status error) { for (ResourceWatcher watcher : watchers.values()) { watcher.onError(error); + // watcher.onAmbientError(error); } } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 51c07cb3537..2972ee52370 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -20,8 +20,8 @@ import static com.google.common.truth.Truth.assertWithMessage; import static io.grpc.xds.GrpcXdsTransportFactory.DEFAULT_XDS_TRANSPORT_FACTORY; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; @@ -58,6 +58,7 @@ import io.grpc.Server; import io.grpc.Status; import io.grpc.Status.Code; +import io.grpc.StatusOr; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; import io.grpc.internal.BackoffPolicy; @@ -269,10 +270,6 @@ public long currentTimeNanos() { @Captor private ArgumentCaptor ldsUpdateCaptor; @Captor - private ArgumentCaptor rdsUpdateCaptor; - @Captor - private ArgumentCaptor cdsUpdateCaptor; - @Captor private ArgumentCaptor edsUpdateCaptor; @Captor private ArgumentCaptor errorCaptor; @@ -670,7 +667,10 @@ public void ldsResourceNotFound() { verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); // Server failed to return subscribed resource within expected time window. fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(ldsResourceWatcher).onResourceDoesNotExist(LDS_RESOURCE); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() + && statusOr.getStatus().getCode() == Status.Code.NOT_FOUND + && statusOr.getStatus().getDescription().contains(LDS_RESOURCE))); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataDoesNotExist(LDS, LDS_RESOURCE); // Check metric data. @@ -683,11 +683,11 @@ public void ldsResourceUpdated_withXdstpResourceName_withUnknownAuthority() { "xdstp://unknown.example.com/envoy.config.listener.v3.Listener/listener1"; xdsClient.watchXdsResource(XdsListenerResource.getInstance(), ldsResourceName, ldsResourceWatcher); - verify(ldsResourceWatcher).onError(errorCaptor.capture()); - Status error = errorCaptor.getValue(); - assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); - assertThat(error.getDescription()).isEqualTo( - "Wrong configuration: xds server does not exist for resource " + ldsResourceName); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() + && statusOr.getStatus().getCode() == Status.Code.INVALID_ARGUMENT + && statusOr.getStatus().getDescription().equals( + "Wrong configuration: xds server does not exist for resource " + ldsResourceName))); assertThat(resourceDiscoveryCalls.poll()).isNull(); xdsClient.cancelXdsResourceWatch(XdsListenerResource.getInstance(), ldsResourceName, ldsResourceWatcher); @@ -729,7 +729,7 @@ public void ldsResponseErrorHandling_someResourcesFailedUnpack() { verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); // The response is NACKed with the same error message. call.verifyRequestNack(LDS, LDS_RESOURCE, "", "0000", NODE, errors); - verify(ldsResourceWatcher).onChanged(any(LdsUpdate.class)); + verify(ldsResourceWatcher).onResourceChanged(any()); } /** @@ -897,8 +897,17 @@ public void ldsResourceFound_containsVirtualHosts() { // Client sends an ACK LDS request. call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (Exception e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -912,8 +921,17 @@ public void wrappedLdsResource() { // Client sends an ACK LDS request. call.sendResponse(LDS, mf.buildWrappedResource(testListenerVhosts), VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (Exception e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -931,8 +949,17 @@ public void wrappedLdsResource_preferWrappedName() { call.sendResponse(LDS, mf.buildWrappedResourceWithName(innerResource, LDS_RESOURCE), VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, innerResource, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -946,8 +973,17 @@ public void ldsResourceFound_containsRdsName() { // Client sends an ACK LDS request. call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerRds(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerRds, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -965,8 +1001,17 @@ public void cachedLdsResource_data() { ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), LDS_RESOURCE, watcher); - verify(watcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(watcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerRds(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); call.verifyNoMoreRequest(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerRds, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -978,11 +1023,30 @@ public void cachedLdsResource_absent() { DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE, ldsResourceWatcher); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(ldsResourceWatcher).onResourceDoesNotExist(LDS_RESOURCE); - // Add another watcher. + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getStatus().getDescription()).isEqualTo(LDS_RESOURCE); + return true; + } catch (AssertionError e) { + return false; + } + })); ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), LDS_RESOURCE, watcher); - verify(watcher).onResourceDoesNotExist(LDS_RESOURCE); + verify(watcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getStatus().getDescription()).isEqualTo(LDS_RESOURCE); + return true; + } catch (AssertionError e) { + return false; + } + })); call.verifyNoMoreRequest(); verifyResourceMetadataDoesNotExist(LDS, LDS_RESOURCE); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -997,15 +1061,33 @@ public void ldsResourceUpdated() { // Initial LDS response. call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); // Updated LDS response. call.sendResponse(LDS, testListenerRds, VERSION_2, "0001"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_2, "0001", NODE); - verify(ldsResourceWatcher, times(2)).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerRds, VERSION_2, TIME_INCREMENT * 2); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); assertThat(channelForCustomAuthority).isNull(); @@ -1021,8 +1103,17 @@ public void cancelResourceWatcherNotRemoveUrlSubscribers() { // Initial LDS response. call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), @@ -1035,8 +1126,17 @@ public void cancelResourceWatcherNotRemoveUrlSubscribers() { mf.buildRouteConfiguration("new", mf.buildOpaqueVirtualHosts(VHOST_SIZE)))); call.sendResponse(LDS, testListenerVhosts2, VERSION_2, "0001"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_2, "0001", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts2, VERSION_2, TIME_INCREMENT * 2); } @@ -1054,8 +1154,17 @@ public void ldsResourceUpdated_withXdstpResourceName() { mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE)))); call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, ldsResourceName, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked( LDS, ldsResourceName, testListenerVhosts, VERSION_1, TIME_INCREMENT); } @@ -1072,8 +1181,17 @@ public void ldsResourceUpdated_withXdstpResourceName_withEmptyAuthority() { mf.buildRouteConfiguration("do not care", mf.buildOpaqueVirtualHosts(VHOST_SIZE)))); call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, ldsResourceName, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked( LDS, ldsResourceName, testListenerVhosts, VERSION_1, TIME_INCREMENT); } @@ -1141,11 +1259,20 @@ public void rdsResourceUpdated_withXdstpResourceName_unknownAuthority() { "xdstp://unknown.example.com/envoy.config.route.v3.RouteConfiguration/route1"; xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), rdsResourceName, rdsResourceWatcher); - verify(rdsResourceWatcher).onError(errorCaptor.capture()); - Status error = errorCaptor.getValue(); - assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); - assertThat(error.getDescription()).isEqualTo( - "Wrong configuration: xds server does not exist for resource " + rdsResourceName); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + Status error = statusOr.getStatus(); + assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); + assertThat(error.getDescription()).isEqualTo( + "Wrong configuration: xds server does not exist for resource " + rdsResourceName); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(resourceDiscoveryCalls.size()).isEqualTo(0); xdsClient.cancelXdsResourceWatch( XdsRouteConfigureResource.getInstance(), rdsResourceName, rdsResourceWatcher); @@ -1176,11 +1303,20 @@ public void cdsResourceUpdated_withXdstpResourceName_unknownAuthority() { String cdsResourceName = "xdstp://unknown.example.com/envoy.config.cluster.v3.Cluster/cluster1"; xdsClient.watchXdsResource(XdsClusterResource.getInstance(), cdsResourceName, cdsResourceWatcher); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); - Status error = errorCaptor.getValue(); - assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); - assertThat(error.getDescription()).isEqualTo( - "Wrong configuration: xds server does not exist for resource " + cdsResourceName); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + Status error = statusOr.getStatus(); + assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); + assertThat(error.getDescription()).isEqualTo( + "Wrong configuration: xds server does not exist for resource " + cdsResourceName); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(resourceDiscoveryCalls.poll()).isNull(); xdsClient.cancelXdsResourceWatch(XdsClusterResource.getInstance(), cdsResourceName, cdsResourceWatcher); @@ -1216,11 +1352,20 @@ public void edsResourceUpdated_withXdstpResourceName_unknownAuthority() { "xdstp://unknown.example.com/envoy.config.endpoint.v3.ClusterLoadAssignment/cluster1"; xdsClient.watchXdsResource(XdsEndpointResource.getInstance(), edsResourceName, edsResourceWatcher); - verify(edsResourceWatcher).onError(errorCaptor.capture()); - Status error = errorCaptor.getValue(); - assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); - assertThat(error.getDescription()).isEqualTo( - "Wrong configuration: xds server does not exist for resource " + edsResourceName); + verify(edsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + Status error = statusOr.getStatus(); + assertThat(error.getCode()).isEqualTo(Code.INVALID_ARGUMENT); + assertThat(error.getDescription()).isEqualTo( + "Wrong configuration: xds server does not exist for resource " + edsResourceName); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(resourceDiscoveryCalls.poll()).isNull(); xdsClient.cancelXdsResourceWatch(XdsEndpointResource.getInstance(), edsResourceName, edsResourceWatcher); @@ -1266,11 +1411,17 @@ public void ldsResourceUpdate_withFaultInjection() { // Client sends an ACK LDS request. call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(StatusOr.class); + verify(ldsResourceWatcher).onResourceChanged(captor.capture()); + + StatusOr result = captor.getValue(); + assertThat(result.hasValue()).isTrue(); + LdsUpdate ldsUpdate = result.getValue(); + verifyResourceMetadataAcked(LDS, LDS_RESOURCE, listener, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); - LdsUpdate ldsUpdate = ldsUpdateCaptor.getValue(); assertThat(ldsUpdate.httpConnectionManager().virtualHosts()).hasSize(2); assertThat(ldsUpdate.httpConnectionManager().httpFilterConfigs().get(0).name) .isEqualTo("envoy.fault"); @@ -1303,15 +1454,34 @@ public void ldsResourceDeleted() { // Initial LDS response. call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); // Empty LDS response deletes the listener. call.sendResponse(LDS, Collections.emptyList(), VERSION_2, "0001"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_2, "0001", NODE); - verify(ldsResourceWatcher).onResourceDoesNotExist(LDS_RESOURCE); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getStatus().getDescription()).contains(LDS_RESOURCE); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataDoesNotExist(LDS, LDS_RESOURCE); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); } @@ -1331,8 +1501,17 @@ public void ldsResourceDeleted_ignoreResourceDeletion() { // Initial LDS response. call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); @@ -1342,14 +1521,24 @@ public void ldsResourceDeleted_ignoreResourceDeletion() { // The resource is still ACKED at VERSION_1 (no changes). verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(1, 0, 0, 0); - // onResourceDoesNotExist not called - verify(ldsResourceWatcher, never()).onResourceDoesNotExist(LDS_RESOURCE); + verify(ldsResourceWatcher, never()).onResourceChanged( + argThat(arg -> !arg.hasValue() && arg.getStatus().getCode() == Status.Code.NOT_FOUND)); + verify(ldsResourceWatcher, never()).onAmbientError(any()); // Next update is correct, and contains the listener again. call.sendResponse(LDS, testListenerVhosts, VERSION_3, "0003"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_3, "0003", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); // LDS is now ACKEd at VERSION_3. verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_3, TIME_INCREMENT * 3); @@ -1374,9 +1563,12 @@ public void multipleLdsWatchers() { verifySubscribedResourcesMetadataSizes(2, 0, 0, 0); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(ldsResourceWatcher).onResourceDoesNotExist(LDS_RESOURCE); - verify(watcher1).onResourceDoesNotExist(ldsResourceTwo); - verify(watcher2).onResourceDoesNotExist(ldsResourceTwo); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().contains(LDS_RESOURCE))); + verify(watcher1).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().contains(ldsResourceTwo))); + verify(watcher2).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().contains(ldsResourceTwo))); verifyResourceMetadataDoesNotExist(LDS, LDS_RESOURCE); verifyResourceMetadataDoesNotExist(LDS, ldsResourceTwo); verifySubscribedResourcesMetadataSizes(2, 0, 0, 0); @@ -1384,15 +1576,42 @@ public void multipleLdsWatchers() { Any listenerTwo = Any.pack(mf.buildListenerWithApiListenerForRds(ldsResourceTwo, RDS_RESOURCE)); call.sendResponse(LDS, ImmutableList.of(testListenerVhosts, listenerTwo), VERSION_1, "0000"); // ResourceWatcher called with listenerVhosts. - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); // watcher1 called with listenerTwo. - verify(watcher1).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(watcher2).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerRds(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(ldsUpdateCaptor.getValue().httpConnectionManager().virtualHosts()).isNull(); // watcher2 called with listenerTwo. - verify(watcher2).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(watcher2).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerRds(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(ldsUpdateCaptor.getValue().httpConnectionManager().virtualHosts()).isNull(); // Metadata of both listeners is stored. verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); @@ -1415,7 +1634,8 @@ public void rdsResourceNotFound() { verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); // Server failed to return subscribed resource within expected time window. fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(rdsResourceWatcher).onResourceDoesNotExist(RDS_RESOURCE); + verify(rdsResourceWatcher).onResourceChanged(argThat( + arg -> !arg.hasValue() && arg.getStatus().getDescription().contains(RDS_RESOURCE))); assertThat(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataDoesNotExist(RDS, RDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); @@ -1456,7 +1676,7 @@ public void rdsResponseErrorHandling_someResourcesFailedUnpack() { verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); // The response is NACKed with the same error message. call.verifyRequestNack(RDS, RDS_RESOURCE, "", "0000", NODE, errors); - verify(rdsResourceWatcher).onChanged(any(RdsUpdate.class)); + verify(rdsResourceWatcher).onResourceChanged(any()); } @Test @@ -1501,7 +1721,7 @@ public void rdsResponseErrorHandling_nackWeightedSumZero() { verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); // The response is NACKed with the same error message. call.verifyRequestNack(RDS, RDS_RESOURCE, "", "0000", NODE, errors); - verify(rdsResourceWatcher, never()).onChanged(any(RdsUpdate.class)); + verify(rdsResourceWatcher, never()).onResourceChanged(any()); } /** @@ -1583,8 +1803,17 @@ public void rdsResourceFound() { // Client sends an ACK RDS request. call.verifyRequest(RDS, RDS_RESOURCE, VERSION_1, "0000", NODE); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); @@ -1598,8 +1827,17 @@ public void wrappedRdsResource() { // Client sends an ACK RDS request. call.verifyRequest(RDS, RDS_RESOURCE, VERSION_1, "0000", NODE); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(RDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); @@ -1617,8 +1855,17 @@ public void cachedRdsResource_data() { ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_RESOURCE, watcher); - verify(watcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(watcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); call.verifyNoMoreRequest(); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); @@ -1630,11 +1877,13 @@ public void cachedRdsResource_absent() { DiscoveryRpcCall call = startResourceWatcher(XdsRouteConfigureResource.getInstance(), RDS_RESOURCE, rdsResourceWatcher); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(rdsResourceWatcher).onResourceDoesNotExist(RDS_RESOURCE); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().equals(RDS_RESOURCE))); // Add another watcher. ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_RESOURCE, watcher); - verify(watcher).onResourceDoesNotExist(RDS_RESOURCE); + verify(watcher).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().equals(RDS_RESOURCE))); call.verifyNoMoreRequest(); verifyResourceMetadataDoesNotExist(RDS, RDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 0, 1, 0); @@ -1649,8 +1898,17 @@ public void rdsResourceUpdated() { // Initial RDS response. call.sendResponse(RDS, testRouteConfig, VERSION_1, "0000"); call.verifyRequest(RDS, RDS_RESOURCE, VERSION_1, "0000", NODE); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); // Updated RDS response. @@ -1660,8 +1918,17 @@ public void rdsResourceUpdated() { // Client sends an ACK RDS request. call.verifyRequest(RDS, RDS_RESOURCE, VERSION_2, "0001", NODE); - verify(rdsResourceWatcher, times(2)).onChanged(rdsUpdateCaptor.capture()); - assertThat(rdsUpdateCaptor.getValue().virtualHosts).hasSize(4); + verify(rdsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().virtualHosts).hasSize(4); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, routeConfigUpdated, VERSION_2, TIME_INCREMENT * 2); } @@ -1708,15 +1975,33 @@ public void rdsResourceDeletedByLdsApiListener() { DiscoveryRpcCall call = resourceDiscoveryCalls.poll(); call.sendResponse(LDS, testListenerRds, VERSION_1, "0000"); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerRds(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerRds(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerRds, VERSION_1, TIME_INCREMENT); verifyResourceMetadataRequested(RDS, RDS_RESOURCE); verifySubscribedResourcesMetadataSizes(1, 0, 1, 0); call.sendResponse(RDS, testRouteConfig, VERSION_1, "0000"); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerRds, VERSION_1, TIME_INCREMENT); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); verifySubscribedResourcesMetadataSizes(1, 0, 1, 0); @@ -1726,8 +2011,17 @@ public void rdsResourceDeletedByLdsApiListener() { // Note that this must work the same despite the ignore_resource_deletion feature is on. // This happens because the Listener is getting replaced, and not deleted. call.sendResponse(LDS, testListenerVhosts, VERSION_2, "0001"); - verify(ldsResourceWatcher, times(2)).onChanged(ldsUpdateCaptor.capture()); - verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenListenerVhosts(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyNoMoreInteractions(rdsResourceWatcher); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked( @@ -1759,20 +2053,37 @@ public void rdsResourcesDeletedByLdsTcpListener() { // referencing RDS_RESOURCE. DiscoveryRpcCall call = resourceDiscoveryCalls.poll(); call.sendResponse(LDS, packedListener, VERSION_1, "0000"); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - - assertThat(ldsUpdateCaptor.getValue().listener().filterChains()).hasSize(1); - FilterChain parsedFilterChain = Iterables.getOnlyElement( - ldsUpdateCaptor.getValue().listener().filterChains()); - assertThat(parsedFilterChain.httpConnectionManager().rdsName()).isEqualTo(RDS_RESOURCE); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().listener().filterChains()).hasSize(1); + FilterChain parsedFilterChain = Iterables.getOnlyElement( + statusOr.getValue().listener().filterChains()); + assertThat(parsedFilterChain.httpConnectionManager().rdsName()).isEqualTo(RDS_RESOURCE); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(LDS, LISTENER_RESOURCE, packedListener, VERSION_1, TIME_INCREMENT); verifyResourceMetadataRequested(RDS, RDS_RESOURCE); verifySubscribedResourcesMetadataSizes(1, 0, 1, 0); // Simulates receiving the requested RDS resource. call.sendResponse(RDS, testRouteConfig, VERSION_1, "0000"); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenRouteConfig(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); // Simulates receiving an updated version of the requested LDS resource as a TCP listener @@ -1790,12 +2101,22 @@ public void rdsResourcesDeletedByLdsTcpListener() { packedListener = Any.pack(mf.buildListenerWithFilterChain(LISTENER_RESOURCE, 7000, "0.0.0.0", filterChain)); call.sendResponse(LDS, packedListener, VERSION_2, "0001"); - verify(ldsResourceWatcher, times(2)).onChanged(ldsUpdateCaptor.capture()); - assertThat(ldsUpdateCaptor.getValue().listener().filterChains()).hasSize(1); - parsedFilterChain = Iterables.getOnlyElement( - ldsUpdateCaptor.getValue().listener().filterChains()); - assertThat(parsedFilterChain.httpConnectionManager().virtualHosts()).hasSize(VHOST_SIZE); - verify(rdsResourceWatcher, never()).onResourceDoesNotExist(RDS_RESOURCE); + verify(ldsResourceWatcher, times(2)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().listener().filterChains()).hasSize(1); + FilterChain parsedFilterChain = Iterables.getOnlyElement( + statusOr.getValue().listener().filterChains()); + assertThat(parsedFilterChain.httpConnectionManager().virtualHosts()).hasSize(VHOST_SIZE); + return true; + } catch (AssertionError e) { + return false; + } + })); + verify(rdsResourceWatcher, never()).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() && statusOr.getStatus().getDescription().equals(RDS_RESOURCE))); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked( LDS, LISTENER_RESOURCE, packedListener, VERSION_2, TIME_INCREMENT * 3); @@ -1808,6 +2129,7 @@ public void multipleRdsWatchers() { String rdsResourceTwo = "route-bar.googleapis.com"; ResourceWatcher watcher1 = mock(ResourceWatcher.class); ResourceWatcher watcher2 = mock(ResourceWatcher.class); + ArgumentCaptor> rdsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_RESOURCE, rdsResourceWatcher); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), rdsResourceTwo, watcher1); @@ -1820,16 +2142,20 @@ public void multipleRdsWatchers() { verifySubscribedResourcesMetadataSizes(0, 0, 2, 0); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(rdsResourceWatcher).onResourceDoesNotExist(RDS_RESOURCE); - verify(watcher1).onResourceDoesNotExist(rdsResourceTwo); - verify(watcher2).onResourceDoesNotExist(rdsResourceTwo); + verify(rdsResourceWatcher).onResourceChanged(rdsUpdateCaptor.capture()); + assertThat(rdsUpdateCaptor.getValue().getStatus().getDescription()).contains(RDS_RESOURCE); + verify(watcher1).onResourceChanged(rdsUpdateCaptor.capture()); + assertThat(rdsUpdateCaptor.getValue().getStatus().getDescription()).contains(rdsResourceTwo); + verify(watcher2).onResourceChanged(rdsUpdateCaptor.capture()); + assertThat(rdsUpdateCaptor.getValue().getStatus().getDescription()).contains(rdsResourceTwo); verifyResourceMetadataDoesNotExist(RDS, RDS_RESOURCE); verifyResourceMetadataDoesNotExist(RDS, rdsResourceTwo); verifySubscribedResourcesMetadataSizes(0, 0, 2, 0); call.sendResponse(RDS, testRouteConfig, VERSION_1, "0000"); - verify(rdsResourceWatcher).onChanged(rdsUpdateCaptor.capture()); - verifyGoldenRouteConfig(rdsUpdateCaptor.getValue()); + verify(rdsResourceWatcher, times(2)) + .onResourceChanged(rdsUpdateCaptor.capture()); + verifyGoldenRouteConfig(rdsUpdateCaptor.getValue().getValue()); verifyNoMoreInteractions(watcher1, watcher2); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); verifyResourceMetadataDoesNotExist(RDS, rdsResourceTwo); @@ -1838,10 +2164,10 @@ public void multipleRdsWatchers() { Any routeConfigTwo = Any.pack(mf.buildRouteConfiguration(rdsResourceTwo, mf.buildOpaqueVirtualHosts(4))); call.sendResponse(RDS, routeConfigTwo, VERSION_2, "0002"); - verify(watcher1).onChanged(rdsUpdateCaptor.capture()); - assertThat(rdsUpdateCaptor.getValue().virtualHosts).hasSize(4); - verify(watcher2).onChanged(rdsUpdateCaptor.capture()); - assertThat(rdsUpdateCaptor.getValue().virtualHosts).hasSize(4); + verify(watcher1).onResourceChanged(rdsUpdateCaptor.capture()); + assertThat(rdsUpdateCaptor.getValue().getValue().virtualHosts).hasSize(4); + verify(watcher2).onResourceChanged(rdsUpdateCaptor.capture()); + assertThat(rdsUpdateCaptor.getValue().getValue().virtualHosts).hasSize(4); verifyNoMoreInteractions(rdsResourceWatcher); verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT); verifyResourceMetadataAcked(RDS, rdsResourceTwo, routeConfigTwo, VERSION_2, TIME_INCREMENT * 2); @@ -1867,7 +2193,11 @@ public void cdsResourceNotFound() { verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); // Server failed to return subscribed resource within expected time window. fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(cdsResourceWatcher).onResourceDoesNotExist(CDS_RESOURCE); + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(captor.capture()); + StatusOr result = captor.getValue(); + assertThat(result.getStatus().getDescription()).contains(CDS_RESOURCE); assertThat(fakeClock.getPendingTasks(CDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataDoesNotExist(CDS, CDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -1909,7 +2239,7 @@ public void cdsResponseErrorHandling_someResourcesFailedUnpack() { verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); // The response is NACKed with the same error message. call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, errors); - verify(cdsResourceWatcher).onChanged(any(CdsUpdate.class)); + verify(cdsResourceWatcher).onResourceChanged(any()); } /** @@ -2099,8 +2429,17 @@ public void cdsResourceFound() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenClusterRoundRobin(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(CDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); @@ -2115,8 +2454,17 @@ public void wrappedCdsResource() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + verifyGoldenClusterRoundRobin(statusOr.getValue()); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(CDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); @@ -2136,20 +2484,31 @@ public void cdsResourceFound_leastRequestLbPolicy() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); - assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); - assertThat(cdsUpdate.edsServiceName()).isNull(); - LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(cdsUpdate.lbPolicyConfig()); - assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); - List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( - JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); - assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("least_request_experimental"); - assertThat(childConfigs.get(0).getRawConfigValue().get("choiceCount")).isEqualTo(3); - assertThat(cdsUpdate.lrsServerInfo()).isNull(); - assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); - assertThat(cdsUpdate.upstreamTlsContext()).isNull(); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().clusterName()).isEqualTo(CDS_RESOURCE); + assertThat(statusOr.getValue().clusterType()).isEqualTo(ClusterType.EDS); + assertThat(statusOr.getValue().edsServiceName()).isNull(); + LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig( + statusOr.getValue().lbPolicyConfig()); + assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); + List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( + JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); + assertThat(childConfigs.get(0).getPolicyName()) + .isEqualTo("least_request_experimental"); + assertThat(childConfigs.get(0).getRawConfigValue().get("choiceCount")) + .isEqualTo(3); + assertThat(statusOr.getValue().lrsServerInfo()).isNull(); + assertThat(statusOr.getValue().maxConcurrentRequests()).isNull(); + assertThat(statusOr.getValue().upstreamTlsContext()).isNull(); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(CDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterRingHash, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2168,20 +2527,29 @@ public void cdsResourceFound_ringHashLbPolicy() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); - assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); - assertThat(cdsUpdate.edsServiceName()).isNull(); - LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(cdsUpdate.lbPolicyConfig()); - assertThat(lbConfig.getPolicyName()).isEqualTo("ring_hash_experimental"); - assertThat(JsonUtil.getNumberAsLong(lbConfig.getRawConfigValue(), "minRingSize")).isEqualTo( - 10L); - assertThat(JsonUtil.getNumberAsLong(lbConfig.getRawConfigValue(), "maxRingSize")).isEqualTo( - 100L); - assertThat(cdsUpdate.lrsServerInfo()).isNull(); - assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); - assertThat(cdsUpdate.upstreamTlsContext()).isNull(); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().clusterName()).isEqualTo(CDS_RESOURCE); + assertThat(statusOr.getValue().clusterType()).isEqualTo(ClusterType.EDS); + assertThat(statusOr.getValue().edsServiceName()).isNull(); + LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig( + statusOr.getValue().lbPolicyConfig()); + assertThat(lbConfig.getPolicyName()).isEqualTo("ring_hash_experimental"); + assertThat(JsonUtil.getNumberAsLong(lbConfig.getRawConfigValue(), "minRingSize")) + .isEqualTo(10L); + assertThat(JsonUtil.getNumberAsLong(lbConfig.getRawConfigValue(), "maxRingSize")) + .isEqualTo(100L); + assertThat(statusOr.getValue().lrsServerInfo()).isNull(); + assertThat(statusOr.getValue().maxConcurrentRequests()).isNull(); + assertThat(statusOr.getValue().upstreamTlsContext()).isNull(); + return true; + } catch (AssertionError e) { + return false; + } + })); assertThat(fakeClock.getPendingTasks(CDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterRingHash, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2199,16 +2567,26 @@ public void cdsResponseWithAggregateCluster() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); - assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.AGGREGATE); - LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(cdsUpdate.lbPolicyConfig()); - assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); - List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( - JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); - assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); - assertThat(cdsUpdate.prioritizedClusterNames()).containsExactlyElementsIn(candidates).inOrder(); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().clusterName()).isEqualTo(CDS_RESOURCE); + assertThat(statusOr.getValue().clusterType()).isEqualTo(ClusterType.AGGREGATE); + LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig( + statusOr.getValue().lbPolicyConfig()); + assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); + List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( + JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); + assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); + assertThat(statusOr.getValue().prioritizedClusterNames()) + .containsExactlyElementsIn(candidates).inOrder(); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterAggregate, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); } @@ -2224,19 +2602,28 @@ public void cdsResponseWithCircuitBreakers() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); - assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); - assertThat(cdsUpdate.edsServiceName()).isNull(); - LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(cdsUpdate.lbPolicyConfig()); - assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); - List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( - JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); - assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); - assertThat(cdsUpdate.lrsServerInfo()).isNull(); - assertThat(cdsUpdate.maxConcurrentRequests()).isEqualTo(200L); - assertThat(cdsUpdate.upstreamTlsContext()).isNull(); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getValue().clusterName()).isEqualTo(CDS_RESOURCE); + assertThat(statusOr.getValue().clusterType()).isEqualTo(ClusterType.EDS); + assertThat(statusOr.getValue().edsServiceName()).isNull(); + LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig( + statusOr.getValue().lbPolicyConfig()); + assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); + List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( + JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); + assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); + assertThat(statusOr.getValue().lrsServerInfo()).isNull(); + assertThat(statusOr.getValue().maxConcurrentRequests()).isEqualTo(200L); + assertThat(statusOr.getValue().upstreamTlsContext()).isNull(); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterCircuitBreakers, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2267,14 +2654,25 @@ public void cdsResponseWithUpstreamTlsContext() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher, times(1)) - .onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - CommonTlsContext.CertificateProviderInstance certificateProviderInstance = - cdsUpdate.upstreamTlsContext().getCommonTlsContext().getCombinedValidationContext() - .getValidationContextCertificateProviderInstance(); - assertThat(certificateProviderInstance.getInstanceName()).isEqualTo("cert-instance-name"); - assertThat(certificateProviderInstance.getCertificateName()).isEqualTo("cert1"); + verify(cdsResourceWatcher, times(1)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + CommonTlsContext.CertificateProviderInstance certificateProviderInstance = + statusOr.getValue().upstreamTlsContext() + .getCommonTlsContext() + .getCombinedValidationContext() + .getValidationContextCertificateProviderInstance(); + assertThat(certificateProviderInstance.getInstanceName()) + .isEqualTo("cert-instance-name"); + assertThat(certificateProviderInstance.getCertificateName()) + .isEqualTo("cert1"); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterEds, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); } @@ -2304,13 +2702,25 @@ public void cdsResponseWithNewUpstreamTlsContext() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher, times(1)).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - CertificateProviderPluginInstance certificateProviderInstance = - cdsUpdate.upstreamTlsContext().getCommonTlsContext().getValidationContext() - .getCaCertificateProviderInstance(); - assertThat(certificateProviderInstance.getInstanceName()).isEqualTo("cert-instance-name"); - assertThat(certificateProviderInstance.getCertificateName()).isEqualTo("cert1"); + verify(cdsResourceWatcher, times(1)).onResourceChanged(argThat(statusOr -> { + if (!statusOr.hasValue()) { + return false; + } + try { + CertificateProviderPluginInstance certificateProviderInstance = + statusOr.getValue().upstreamTlsContext() + .getCommonTlsContext() + .getValidationContext() + .getCaCertificateProviderInstance(); + assertThat(certificateProviderInstance.getInstanceName()) + .isEqualTo("cert-instance-name"); + assertThat(certificateProviderInstance.getCertificateName()) + .isEqualTo("cert1"); + return true; + } catch (AssertionError e) { + return false; + } + })); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, clusterEds, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); } @@ -2337,7 +2747,7 @@ public void cdsResponseErrorHandling_badUpstreamTlsContext() { + "ca_certificate_provider_instance or system_root_certs is required in " + "upstream-tls-context"; call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg)); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); + verify(cdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); } @@ -2380,8 +2790,11 @@ public void cdsResponseWithOutlierDetection() { // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher, times(1)).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); + @SuppressWarnings("unchecked") + ArgumentCaptor> cdsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher, times(1)) + .onResourceChanged(cdsUpdateCaptor.capture()); + CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue().getValue(); // The outlier detection config in CdsUpdate should match what we get from xDS. EnvoyServerProtoData.OutlierDetection outlierDetection = cdsUpdate.outlierDetection(); @@ -2442,7 +2855,7 @@ public void cdsResponseWithInvalidOutlierDetectionNacks() { + "io.grpc.xds.client.XdsResourceType$ResourceInvalidException: outlier_detection " + "max_ejection_percent is > 100"; call.verifyRequestNack(CDS, CDS_RESOURCE, "", "0000", NODE, ImmutableList.of(errorMsg)); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); + verify(cdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); } @@ -2537,7 +2950,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)); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); + verify(cdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); } @@ -2580,13 +2993,14 @@ public void cachedCdsResource_data() { ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CDS_RESOURCE, watcher); - verify(watcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + ArgumentCaptor> cdsUpdateCaptor1 = ArgumentCaptor.forClass(StatusOr.class); + verify(watcher).onResourceChanged(cdsUpdateCaptor1.capture()); + StatusOr statusOrCdsUpdate = cdsUpdateCaptor1.getValue(); + verifyGoldenClusterRoundRobin(statusOrCdsUpdate.getValue()); call.verifyNoMoreRequest(); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); - } @Test @@ -2595,10 +3009,15 @@ public void cachedCdsResource_absent() { DiscoveryRpcCall call = startResourceWatcher(XdsClusterResource.getInstance(), CDS_RESOURCE, cdsResourceWatcher); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(cdsResourceWatcher).onResourceDoesNotExist(CDS_RESOURCE); + ArgumentCaptor> statusOrCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + StatusOr statusOr = statusOrCaptor.getValue(); + assertThat(statusOr.getStatus().getDescription()).contains(CDS_RESOURCE); ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CDS_RESOURCE, watcher); - verify(watcher).onResourceDoesNotExist(CDS_RESOURCE); + verify(watcher).onResourceChanged(statusOrCaptor.capture()); + StatusOr watcherStatusOr = statusOrCaptor.getValue(); + assertThat(watcherStatusOr.getStatus().getDescription()).contains(CDS_RESOURCE); call.verifyNoMoreRequest(); verifyResourceMetadataDoesNotExist(CDS, CDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2618,8 +3037,10 @@ public void cdsResourceUpdated() { null, null, false, null, null)); call.sendResponse(CDS, clusterDns, VERSION_1, "0000"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); + @SuppressWarnings("unchecked") + ArgumentCaptor> statusOrCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + CdsUpdate cdsUpdate = statusOrCaptor.getValue().getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.LOGICAL_DNS); assertThat(cdsUpdate.dnsHostName()).isEqualTo(dnsHostAddr + ":" + dnsHostPort); @@ -2641,8 +3062,8 @@ public void cdsResourceUpdated() { )); call.sendResponse(CDS, clusterEds, VERSION_2, "0001"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_2, "0001", NODE); - verify(cdsResourceWatcher, times(2)).onChanged(cdsUpdateCaptor.capture()); - cdsUpdate = cdsUpdateCaptor.getValue(); + verify(cdsResourceWatcher, times(2)).onResourceChanged(statusOrCaptor.capture()); + cdsUpdate = statusOrCaptor.getValue().getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); assertThat(cdsUpdate.edsServiceName()).isEqualTo(edsService); @@ -2683,27 +3104,27 @@ public void cdsResourceUpdatedWithDuplicate() { // Configure with round robin, the update should be sent to the watcher. call.sendResponse(CDS, roundRobinConfig, VERSION_2, "0001"); - verify(cdsResourceWatcher, times(1)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(1)).onResourceChanged(any()); // Second update is identical, watcher should not get an additional update. call.sendResponse(CDS, roundRobinConfig, VERSION_2, "0002"); - verify(cdsResourceWatcher, times(1)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(1)).onResourceChanged(any()); // Now we switch to ring hash so the watcher should be notified. call.sendResponse(CDS, ringHashConfig, VERSION_2, "0003"); - verify(cdsResourceWatcher, times(2)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(2)).onResourceChanged(any()); // Second update to ring hash should not result in watcher being notified. call.sendResponse(CDS, ringHashConfig, VERSION_2, "0004"); - verify(cdsResourceWatcher, times(2)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(2)).onResourceChanged(any()); // Now we switch to least request so the watcher should be notified. call.sendResponse(CDS, leastRequestConfig, VERSION_2, "0005"); - verify(cdsResourceWatcher, times(3)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(3)).onResourceChanged(any()); // Second update to least request should not result in watcher being notified. call.sendResponse(CDS, leastRequestConfig, VERSION_2, "0006"); - verify(cdsResourceWatcher, times(3)).onChanged(isA(CdsUpdate.class)); + verify(cdsResourceWatcher, times(3)).onResourceChanged(any()); } @Test @@ -2717,8 +3138,10 @@ public void cdsResourceDeleted() { // Initial CDS response. call.sendResponse(CDS, testClusterRoundRobin, VERSION_1, "0000"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + @SuppressWarnings("unchecked") + ArgumentCaptor> statusOrCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + verifyGoldenClusterRoundRobin(statusOrCaptor.getValue().getValue()); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2726,7 +3149,9 @@ public void cdsResourceDeleted() { // Empty CDS response deletes the cluster. call.sendResponse(CDS, Collections.emptyList(), VERSION_2, "0001"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_2, "0001", NODE); - verify(cdsResourceWatcher).onResourceDoesNotExist(CDS_RESOURCE); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + StatusOr statusOr = statusOrCaptor.getValue(); + assertThat(statusOr.getStatus().getDescription()).contains(CDS_RESOURCE); verifyResourceMetadataDoesNotExist(CDS, CDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); } @@ -2746,8 +3171,10 @@ public void cdsResourceDeleted_ignoreResourceDeletion() { // Initial CDS response. call.sendResponse(CDS, testClusterRoundRobin, VERSION_1, "0000"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + @SuppressWarnings("unchecked") + ArgumentCaptor> statusOrCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + verifyGoldenClusterRoundRobin(statusOrCaptor.getValue().getValue()); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2760,14 +3187,23 @@ public void cdsResourceDeleted_ignoreResourceDeletion() { verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); - // onResourceDoesNotExist must not be called. - verify(ldsResourceWatcher, never()).onResourceDoesNotExist(CDS_RESOURCE); + verify(ldsResourceWatcher).onResourceChanged(argThat(statusOr -> { + if (statusOr.hasValue()) { + return false; + } + try { + assertThat(statusOr.getStatus().getDescription()).doesNotContain(CDS_RESOURCE); + return true; + } catch (AssertionError e) { + return false; + } + })); // Next update is correct, and contains the cluster again. call.sendResponse(CDS, testClusterRoundRobin, VERSION_3, "0003"); call.verifyRequest(CDS, CDS_RESOURCE, VERSION_3, "0003", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - verifyGoldenClusterRoundRobin(cdsUpdateCaptor.getValue()); + verify(cdsResourceWatcher).onResourceChanged(statusOrCaptor.capture()); + verifyGoldenClusterRoundRobin(statusOrCaptor.getValue().getValue()); verifyResourceMetadataAcked(CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_3, TIME_INCREMENT * 3); verifySubscribedResourcesMetadataSizes(0, 1, 0, 0); @@ -2790,9 +3226,12 @@ public void multipleCdsWatchers() { verifySubscribedResourcesMetadataSizes(0, 2, 0, 0); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(cdsResourceWatcher).onResourceDoesNotExist(CDS_RESOURCE); - verify(watcher1).onResourceDoesNotExist(cdsResourceTwo); - verify(watcher2).onResourceDoesNotExist(cdsResourceTwo); + verify(cdsResourceWatcher).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(CDS_RESOURCE))); + verify(watcher1).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(cdsResourceTwo))); + verify(watcher2).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(cdsResourceTwo))); verifyResourceMetadataDoesNotExist(CDS, CDS_RESOURCE); verifyResourceMetadataDoesNotExist(CDS, cdsResourceTwo); verifySubscribedResourcesMetadataSizes(0, 2, 0, 0); @@ -2806,8 +3245,11 @@ public void multipleCdsWatchers() { Any.pack(mf.buildEdsCluster(cdsResourceTwo, edsService, "round_robin", null, null, true, null, "envoy.transport_sockets.tls", null, null))); call.sendResponse(CDS, clusters, VERSION_1, "0000"); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); + ArgumentCaptor> cdsUpdateCaptor1 = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(cdsUpdateCaptor1.capture()); + StatusOr cdsUpdateStatusOr = cdsUpdateCaptor1.getValue(); + assertThat(cdsUpdateStatusOr.getStatus().isOk()).isTrue(); + CdsUpdate cdsUpdate = cdsUpdateStatusOr.getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.LOGICAL_DNS); assertThat(cdsUpdate.dnsHostName()).isEqualTo(dnsHostAddr + ":" + dnsHostPort); @@ -2816,11 +3258,10 @@ public void multipleCdsWatchers() { List childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); - assertThat(cdsUpdate.lrsServerInfo()).isNull(); - assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); - assertThat(cdsUpdate.upstreamTlsContext()).isNull(); - verify(watcher1).onChanged(cdsUpdateCaptor.capture()); - cdsUpdate = cdsUpdateCaptor.getValue(); + verify(watcher1).onResourceChanged(cdsUpdateCaptor1.capture()); + cdsUpdateStatusOr = cdsUpdateCaptor1.getValue(); + assertThat(cdsUpdateStatusOr.getStatus().isOk()).isTrue(); + cdsUpdate = cdsUpdateStatusOr.getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(cdsResourceTwo); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); assertThat(cdsUpdate.edsServiceName()).isEqualTo(edsService); @@ -2829,11 +3270,10 @@ public void multipleCdsWatchers() { childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( JsonUtil.getListOfObjects(lbConfig.getRawConfigValue(), "childPolicy")); assertThat(childConfigs.get(0).getPolicyName()).isEqualTo("round_robin"); - assertThat(cdsUpdate.lrsServerInfo()).isEqualTo(xdsServerInfo); - assertThat(cdsUpdate.maxConcurrentRequests()).isNull(); - assertThat(cdsUpdate.upstreamTlsContext()).isNull(); - verify(watcher2).onChanged(cdsUpdateCaptor.capture()); - cdsUpdate = cdsUpdateCaptor.getValue(); + verify(watcher2).onResourceChanged(cdsUpdateCaptor1.capture()); + cdsUpdateStatusOr = cdsUpdateCaptor1.getValue(); + assertThat(cdsUpdateStatusOr.getStatus().isOk()).isTrue(); + cdsUpdate = cdsUpdateStatusOr.getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(cdsResourceTwo); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); assertThat(cdsUpdate.edsServiceName()).isEqualTo(edsService); @@ -2868,7 +3308,8 @@ public void edsResourceNotFound() { verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); // Server failed to return subscribed resource within expected time window. fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(edsResourceWatcher).onResourceDoesNotExist(EDS_RESOURCE); + verify(edsResourceWatcher).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(EDS_RESOURCE))); assertThat(fakeClock.getPendingTasks(EDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataDoesNotExist(EDS, EDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); @@ -2892,7 +3333,7 @@ public void edsCleanupNonceAfterUnsubscription() { call.sendResponse(EDS, resourcesV1.values().asList(), VERSION_1, "0000"); // {A.1} -> ACK, version 1 call.verifyRequest(EDS, "A.1", VERSION_1, "0000", NODE); - verify(edsResourceWatcher, times(1)).onChanged(any()); + verify(edsResourceWatcher, times(1)).onResourceChanged(any()); // trigger an EDS resource unsubscription. xdsClient.cancelXdsResourceWatch(XdsEndpointResource.getInstance(), "A.1", edsResourceWatcher); @@ -2943,9 +3384,10 @@ public void edsResponseErrorHandling_someResourcesFailedUnpack() { verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); // The response is NACKed with the same error message. call.verifyRequestNack(EDS, EDS_RESOURCE, "", "0000", NODE, errors); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); - EdsUpdate edsUpdate = edsUpdateCaptor.getValue(); - assertThat(edsUpdate.clusterName).isEqualTo(EDS_RESOURCE); + @SuppressWarnings("unchecked") + ArgumentCaptor> edsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsResourceWatcher).onResourceChanged(edsUpdateCaptor.capture()); + assertThat(edsUpdateCaptor.getValue().getValue().clusterName).isEqualTo(EDS_RESOURCE); } /** @@ -3028,8 +3470,10 @@ public void edsResourceFound() { // Client sent an ACK EDS request. call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0000", NODE); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); - validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue()); + @SuppressWarnings("unchecked") + ArgumentCaptor> edsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsResourceWatcher).onResourceChanged(edsUpdateCaptor.capture()); + validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue().getValue()); verifyResourceMetadataAcked(EDS, EDS_RESOURCE, testClusterLoadAssignment, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); @@ -3043,8 +3487,10 @@ public void wrappedEdsResourceFound() { // Client sent an ACK EDS request. call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0000", NODE); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); - validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue()); + @SuppressWarnings("unchecked") + ArgumentCaptor> edsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsResourceWatcher).onResourceChanged(edsUpdateCaptor.capture()); + validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue().getValue()); verifyResourceMetadataAcked(EDS, EDS_RESOURCE, testClusterLoadAssignment, VERSION_1, TIME_INCREMENT); verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); @@ -3062,8 +3508,10 @@ public void cachedEdsResource_data() { // Add another watcher. ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsEndpointResource.getInstance(), EDS_RESOURCE, watcher); - verify(watcher).onChanged(edsUpdateCaptor.capture()); - validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue()); + ArgumentCaptor> edsUpdateCaptor = + ArgumentCaptor.forClass(StatusOr.class); + verify(watcher).onResourceChanged(edsUpdateCaptor.capture()); + validateGoldenClusterLoadAssignment(edsUpdateCaptor.getValue().getValue()); call.verifyNoMoreRequest(); verifyResourceMetadataAcked(EDS, EDS_RESOURCE, testClusterLoadAssignment, VERSION_1, TIME_INCREMENT); @@ -3076,10 +3524,12 @@ public void cachedEdsResource_absent() { DiscoveryRpcCall call = startResourceWatcher(XdsEndpointResource.getInstance(), EDS_RESOURCE, edsResourceWatcher); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(edsResourceWatcher).onResourceDoesNotExist(EDS_RESOURCE); + verify(edsResourceWatcher).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(EDS_RESOURCE))); ResourceWatcher watcher = mock(ResourceWatcher.class); xdsClient.watchXdsResource(XdsEndpointResource.getInstance(), EDS_RESOURCE, watcher); - verify(watcher).onResourceDoesNotExist(EDS_RESOURCE); + verify(watcher).onResourceChanged(argThat(statusOr -> + statusOr.getStatus().getDescription().contains(EDS_RESOURCE))); call.verifyNoMoreRequest(); verifyResourceMetadataDoesNotExist(EDS, EDS_RESOURCE); verifySubscribedResourcesMetadataSizes(0, 0, 0, 1); @@ -3110,7 +3560,10 @@ public void flowControlAbsent() throws Exception { fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); assertThat(fakeWatchClock.getPendingTasks().size()).isEqualTo(2); CyclicBarrier barrier = new CyclicBarrier(2); - doAnswer(blockUpdate(barrier)).when(cdsResourceWatcher).onChanged(any(CdsUpdate.class)); + doAnswer(invocation -> { + barrier.await(); + return null; + }).when(cdsResourceWatcher).onResourceChanged(any()); CountDownLatch latch = new CountDownLatch(1); new Thread(() -> { @@ -3132,16 +3585,18 @@ public void flowControlAbsent() throws Exception { verifyResourceMetadataAcked( CDS, CDS_RESOURCE, testClusterRoundRobin, VERSION_1, TIME_INCREMENT); barrier.await(); - verify(cdsResourceWatcher, atLeastOnce()).onChanged(any()); + verify(cdsResourceWatcher, atLeastOnce()).onResourceChanged(any()); String errorMsg = "CDS response Cluster 'cluster.googleapis.com2' validation error: " + "Cluster cluster.googleapis.com2: unspecified cluster discovery type"; call.verifyRequestNack(CDS, Arrays.asList(CDS_RESOURCE, anotherCdsResource), VERSION_1, "0001", NODE, Arrays.asList(errorMsg)); barrier.await(); latch.await(10, TimeUnit.SECONDS); - verify(cdsResourceWatcher, times(2)).onChanged(any()); - verify(anotherWatcher).onResourceDoesNotExist(eq(anotherCdsResource)); - verify(anotherWatcher).onError(any()); + verify(cdsResourceWatcher, times(2)).onResourceChanged(any()); + ArgumentCaptor> captor = ArgumentCaptor.forClass(StatusOr.class); + verify(anotherWatcher).onResourceChanged(captor.capture()); + assertThat(captor.getValue().getStatus().getDescription()).contains(anotherCdsResource); + verify(anotherWatcher).onAmbientError(any()); } private Answer blockUpdate(CyclicBarrier barrier) { @@ -3180,7 +3635,7 @@ public void simpleFlowControl() throws Exception { assertThat(call.isReady()).isFalse(); CyclicBarrier barrier = new CyclicBarrier(2); - doAnswer(blockUpdate(barrier)).when(edsResourceWatcher).onChanged(any(EdsUpdate.class)); + doAnswer(blockUpdate(barrier)).when(edsResourceWatcher).onResourceChanged(any()); CountDownLatch latch = new CountDownLatch(1); new Thread(() -> { @@ -3195,12 +3650,14 @@ public void simpleFlowControl() throws Exception { verifyResourceMetadataAcked(EDS, EDS_RESOURCE, testClusterLoadAssignment, VERSION_1, TIME_INCREMENT); barrier.await(); - verify(edsResourceWatcher, atLeastOnce()).onChanged(edsUpdateCaptor.capture()); - EdsUpdate edsUpdate = edsUpdateCaptor.getAllValues().get(0); + @SuppressWarnings("unchecked") + ArgumentCaptor> edsCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsResourceWatcher, atLeastOnce()).onResourceChanged(edsCaptor.capture()); + EdsUpdate edsUpdate = edsCaptor.getAllValues().get(0).getValue(); validateGoldenClusterLoadAssignment(edsUpdate); barrier.await(); latch.await(10, TimeUnit.SECONDS); - verify(edsResourceWatcher, times(2)).onChanged(any()); + verify(edsResourceWatcher, times(2)).onResourceChanged(any()); verifyResourceMetadataAcked(EDS, EDS_RESOURCE, updatedClusterLoadAssignment, VERSION_2, TIME_INCREMENT * 2); } @@ -3212,7 +3669,7 @@ public void flowControlUnknownType() { call.sendResponse(CDS, testClusterRoundRobin, VERSION_1, "0000"); call.sendResponse(EDS, testClusterLoadAssignment, VERSION_1, "0000"); call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0000", NODE); - verify(edsResourceWatcher).onChanged(any()); + verify(edsResourceWatcher).onResourceChanged(any()); } @Test @@ -3224,8 +3681,10 @@ public void edsResourceUpdated() { // Initial EDS response. call.sendResponse(EDS, testClusterLoadAssignment, VERSION_1, "0000"); call.verifyRequest(EDS, EDS_RESOURCE, VERSION_1, "0000", NODE); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); - EdsUpdate edsUpdate = edsUpdateCaptor.getValue(); + @SuppressWarnings("unchecked") + ArgumentCaptor> captor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsResourceWatcher).onResourceChanged(captor.capture()); + EdsUpdate edsUpdate = captor.getValue().getValue(); validateGoldenClusterLoadAssignment(edsUpdate); verifyResourceMetadataAcked(EDS, EDS_RESOURCE, testClusterLoadAssignment, VERSION_1, TIME_INCREMENT); @@ -3237,8 +3696,8 @@ public void edsResourceUpdated() { ImmutableList.of())); call.sendResponse(EDS, updatedClusterLoadAssignment, VERSION_2, "0001"); - verify(edsResourceWatcher, times(2)).onChanged(edsUpdateCaptor.capture()); - edsUpdate = edsUpdateCaptor.getValue(); + verify(edsResourceWatcher, times(2)).onResourceChanged(captor.capture()); + edsUpdate = captor.getValue().getValue(); assertThat(edsUpdate.clusterName).isEqualTo(EDS_RESOURCE); assertThat(edsUpdate.dropPolicies).isEmpty(); assertThat(edsUpdate.localityLbEndpointsMap) @@ -3302,12 +3761,13 @@ public void edsResourceDeletedByCds() { Any.pack(mf.buildEdsCluster(CDS_RESOURCE, EDS_RESOURCE, "round_robin", null, null, false, null, "envoy.transport_sockets.tls", null, null))); call.sendResponse(CDS, clusters, VERSION_1, "0000"); - verify(cdsWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); + ArgumentCaptor> cdsCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsWatcher).onResourceChanged(cdsCaptor.capture()); + CdsUpdate cdsUpdate = cdsCaptor.getValue().getValue(); assertThat(cdsUpdate.edsServiceName()).isEqualTo(null); assertThat(cdsUpdate.lrsServerInfo()).isEqualTo(xdsServerInfo); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - cdsUpdate = cdsUpdateCaptor.getValue(); + verify(cdsResourceWatcher).onResourceChanged(cdsCaptor.capture()); + cdsUpdate = cdsCaptor.getValue().getValue(); assertThat(cdsUpdate.edsServiceName()).isEqualTo(EDS_RESOURCE); assertThat(cdsUpdate.lrsServerInfo()).isNull(); verifyResourceMetadataAcked(CDS, resource, clusters.get(0), VERSION_1, TIME_INCREMENT); @@ -3331,10 +3791,11 @@ public void edsResourceDeletedByCds() { "endpoint-host-name"), 1, 0)), ImmutableList.of(mf.buildDropOverload("lb", 100))))); call.sendResponse(EDS, clusterLoadAssignments, VERSION_1, "0000"); - verify(edsWatcher).onChanged(edsUpdateCaptor.capture()); - assertThat(edsUpdateCaptor.getValue().clusterName).isEqualTo(resource); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); - assertThat(edsUpdateCaptor.getValue().clusterName).isEqualTo(EDS_RESOURCE); + ArgumentCaptor> edsCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(edsWatcher).onResourceChanged(edsCaptor.capture()); + assertThat(edsCaptor.getValue().getValue().clusterName).isEqualTo(resource); + verify(edsResourceWatcher).onResourceChanged(edsCaptor.capture()); + assertThat(edsCaptor.getValue().getValue().clusterName).isEqualTo(EDS_RESOURCE); verifyResourceMetadataAcked( EDS, EDS_RESOURCE, clusterLoadAssignments.get(0), VERSION_1, TIME_INCREMENT * 2); @@ -3352,12 +3813,14 @@ public void edsResourceDeletedByCds() { "envoy.transport_sockets.tls", null, null ))); call.sendResponse(CDS, clusters, VERSION_2, "0001"); - verify(cdsResourceWatcher, times(2)).onChanged(cdsUpdateCaptor.capture()); - assertThat(cdsUpdateCaptor.getValue().edsServiceName()).isNull(); + verify(cdsResourceWatcher, times(2)).onResourceChanged(cdsCaptor.capture()); + assertThat(cdsCaptor.getValue().getValue().edsServiceName()).isNull(); // Note that the endpoint must be deleted even if the ignore_resource_deletion feature. // This happens because the cluster CDS_RESOURCE is getting replaced, and not deleted. - verify(edsResourceWatcher, never()).onResourceDoesNotExist(EDS_RESOURCE); - verify(edsResourceWatcher, never()).onResourceDoesNotExist(resource); + verify(edsResourceWatcher, never()).onResourceChanged( + argThat(statusOr -> !statusOr.getStatus().getDescription().contains(EDS_RESOURCE))); + verify(edsResourceWatcher, never()).onResourceChanged( + argThat(statusOr -> !statusOr.getStatus().getDescription().contains(resource))); verifyNoMoreInteractions(cdsWatcher, edsWatcher); verifyResourceMetadataAcked( EDS, EDS_RESOURCE, clusterLoadAssignments.get(0), VERSION_1, TIME_INCREMENT * 2); @@ -3383,15 +3846,18 @@ public void multipleEdsWatchers() { verifySubscribedResourcesMetadataSizes(0, 0, 0, 2); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(edsResourceWatcher).onResourceDoesNotExist(EDS_RESOURCE); - verify(watcher1).onResourceDoesNotExist(edsResourceTwo); - verify(watcher2).onResourceDoesNotExist(edsResourceTwo); + verify(edsResourceWatcher).onResourceChanged( + argThat(statusOr -> !statusOr.getStatus().getDescription().contains(EDS_RESOURCE))); + verify(watcher1).onResourceChanged( + argThat(statusOr -> !statusOr.getStatus().getDescription().contains(edsResourceTwo))); + verify(watcher2).onResourceChanged( + argThat(statusOr -> !statusOr.getStatus().getDescription().contains(edsResourceTwo))); verifyResourceMetadataDoesNotExist(EDS, EDS_RESOURCE); verifyResourceMetadataDoesNotExist(EDS, edsResourceTwo); verifySubscribedResourcesMetadataSizes(0, 0, 0, 2); call.sendResponse(EDS, testClusterLoadAssignment, VERSION_1, "0000"); - verify(edsResourceWatcher).onChanged(edsUpdateCaptor.capture()); + verify(edsResourceWatcher).onResourceChanged(StatusOr.fromValue(edsUpdateCaptor.capture())); EdsUpdate edsUpdate = edsUpdateCaptor.getValue(); validateGoldenClusterLoadAssignment(edsUpdate); verifyNoMoreInteractions(watcher1, watcher2); @@ -3409,7 +3875,7 @@ public void multipleEdsWatchers() { ImmutableList.of())); call.sendResponse(EDS, clusterLoadAssignmentTwo, VERSION_2, "0001"); - verify(watcher1).onChanged(edsUpdateCaptor.capture()); + verify(watcher1).onResourceChanged(StatusOr.fromValue(edsUpdateCaptor.capture())); edsUpdate = edsUpdateCaptor.getValue(); assertThat(edsUpdate.clusterName).isEqualTo(edsResourceTwo); assertThat(edsUpdate.dropPolicies).isEmpty(); @@ -3421,7 +3887,7 @@ public void multipleEdsWatchers() { LbEndpoint.create("172.44.2.2", 8000, 3, true, "endpoint-host-name", ImmutableMap.of())), 2, 0, ImmutableMap.of())); - verify(watcher2).onChanged(edsUpdateCaptor.capture()); + verify(watcher2).onResourceChanged(StatusOr.fromValue(edsUpdateCaptor.capture())); edsUpdate = edsUpdateCaptor.getValue(); assertThat(edsUpdate.clusterName).isEqualTo(edsResourceTwo); assertThat(edsUpdate.dropPolicies).isEmpty(); @@ -3442,6 +3908,7 @@ public void multipleEdsWatchers() { } @Test + @SuppressWarnings("unchecked") public void useIndependentRpcContext() { // Simulates making RPCs within the context of an inbound RPC. CancellableContext cancellableContext = Context.current().withCancellation(); @@ -3453,10 +3920,10 @@ public void useIndependentRpcContext() { // The inbound RPC finishes and closes its context. The outbound RPC's control plane RPC // should not be impacted. cancellableContext.close(); - verify(ldsResourceWatcher, never()).onError(any(Status.class)); + verify(ldsResourceWatcher, never()).onAmbientError(any()); call.sendResponse(LDS, testListenerRds, VERSION_1, "0000"); - verify(ldsResourceWatcher).onChanged(any(LdsUpdate.class)); + verify(ldsResourceWatcher).onResourceChanged(any()); } finally { cancellableContext.detach(prevContext); } @@ -3477,10 +3944,10 @@ public void streamClosedWithNoResponse() { callback_ReportServerConnection(); verifyServerConnection(1, false, xdsServerInfo.target()); verify(ldsResourceWatcher, Mockito.timeout(1000).times(1)) - .onError(errorCaptor.capture()); + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, "ADS stream closed with OK before receiving a response"); - verify(rdsResourceWatcher).onError(errorCaptor.capture()); + verify(rdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, "ADS stream closed with OK before receiving a response"); } @@ -3510,8 +3977,8 @@ public void streamClosedAfterSendingResponses() { // Check metric data. callback_ReportServerConnection(); verifyServerConnection(3, true, xdsServerInfo.target()); - verify(ldsResourceWatcher, never()).onError(errorCaptor.capture()); - verify(rdsResourceWatcher, never()).onError(errorCaptor.capture()); + verify(ldsResourceWatcher, never()).onAmbientError(any(Status.class)); + verify(rdsResourceWatcher, never()).onAmbientError(any(Status.class)); } @Test @@ -3535,13 +4002,13 @@ public void streamClosedAndRetryWithBackoff() { fakeClock.forwardNanos(1000L); // Make sure retry isn't based on stopwatch 0 call.sendError(Status.UNKNOWN.asException()); verify(ldsResourceWatcher, Mockito.timeout(1000).times(1)) - .onError(errorCaptor.capture()); + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNKNOWN, ""); - verify(rdsResourceWatcher).onError(errorCaptor.capture()); + verify(rdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNKNOWN, ""); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); + verify(cdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNKNOWN, ""); - verify(edsResourceWatcher).onError(errorCaptor.capture()); + verify(edsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNKNOWN, ""); // Check metric data. @@ -3568,13 +4035,17 @@ public void streamClosedAndRetryWithBackoff() { // Management server becomes unreachable. String errorMsg = "my fault"; call.sendError(Status.UNAVAILABLE.withDescription(errorMsg).asException()); - verify(ldsResourceWatcher, times(2)).onError(errorCaptor.capture()); + verify(ldsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); - verify(rdsResourceWatcher, times(2)).onError(errorCaptor.capture()); + verify(rdsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); - verify(cdsResourceWatcher, times(2)).onError(errorCaptor.capture()); + verify(cdsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); - verify(edsResourceWatcher, times(2)).onError(errorCaptor.capture()); + verify(edsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); // Check metric data. @@ -3613,11 +4084,15 @@ public void streamClosedAndRetryWithBackoff() { call.sendError(Status.DEADLINE_EXCEEDED.asException()); // Already received LDS and RDS, so they only error twice. - verify(ldsResourceWatcher, times(2)).onError(errorCaptor.capture()); - verify(rdsResourceWatcher, times(2)).onError(errorCaptor.capture()); - verify(cdsResourceWatcher, times(3)).onError(errorCaptor.capture()); + verify(ldsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); + verify(rdsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); + verify(cdsResourceWatcher, times(3)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.DEADLINE_EXCEEDED, ""); - verify(edsResourceWatcher, times(3)).onError(errorCaptor.capture()); + verify(edsResourceWatcher, times(3)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.DEADLINE_EXCEEDED, ""); // Check metric data. @@ -3644,11 +4119,15 @@ public void streamClosedAndRetryWithBackoff() { // Management server becomes unreachable again. call.sendError(Status.UNAVAILABLE.asException()); - verify(ldsResourceWatcher, times(2)).onError(errorCaptor.capture()); - verify(rdsResourceWatcher, times(2)).onError(errorCaptor.capture()); - verify(cdsResourceWatcher, times(4)).onError(errorCaptor.capture()); + verify(ldsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); + verify(rdsResourceWatcher, times(2)) + .onAmbientError(errorCaptor.capture()); + verify(cdsResourceWatcher, times(4)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); - verify(edsResourceWatcher, times(4)).onError(errorCaptor.capture()); + verify(edsResourceWatcher, times(4)) + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); // Check metric data. @@ -3691,9 +4170,9 @@ public void streamClosedAndRetryRaceWithAddRemoveWatchers() { verifyServerConnection(1, true, xdsServerInfo.target()); call.sendError(Status.UNAVAILABLE.asException()); verify(ldsResourceWatcher, Mockito.timeout(1000).times(1)) - .onError(errorCaptor.capture()); + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); - verify(rdsResourceWatcher).onError(errorCaptor.capture()); + verify(rdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); ScheduledTask retryTask = Iterables.getOnlyElement(fakeClock.getPendingTasks(RPC_RETRY_TASK_FILTER)); @@ -3767,10 +4246,10 @@ public void streamClosedAndRetryRestartsResourceInitialFetchTimerForUnresolvedRe call.sendError(Status.UNAVAILABLE.asException()); assertThat(cdsResourceTimeout.isCancelled()).isTrue(); assertThat(edsResourceTimeout.isCancelled()).isTrue(); - verify(ldsResourceWatcher, never()).onError(errorCaptor.capture()); - verify(rdsResourceWatcher, never()).onError(errorCaptor.capture()); - verify(cdsResourceWatcher, never()).onError(errorCaptor.capture()); - verify(edsResourceWatcher, never()).onError(errorCaptor.capture()); + verify(ldsResourceWatcher, never()).onAmbientError(errorCaptor.capture()); + verify(rdsResourceWatcher, never()).onAmbientError(errorCaptor.capture()); + verify(cdsResourceWatcher, never()).onAmbientError(errorCaptor.capture()); + verify(edsResourceWatcher, never()).onAmbientError(errorCaptor.capture()); // Check metric data. callback_ReportServerConnection(); verifyServerConnection(4, true, xdsServerInfo.target()); @@ -3779,9 +4258,9 @@ public void streamClosedAndRetryRestartsResourceInitialFetchTimerForUnresolvedRe fakeClock.forwardTime(5, TimeUnit.SECONDS); DiscoveryRpcCall call2 = resourceDiscoveryCalls.poll(); call2.sendError(Status.UNAVAILABLE.asException()); - verify(cdsResourceWatcher).onError(errorCaptor.capture()); + verify(cdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); - verify(edsResourceWatcher).onError(errorCaptor.capture()); + verify(edsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, ""); fakeClock.forwardTime(5, TimeUnit.SECONDS); @@ -3840,8 +4319,10 @@ public void serverSideListenerFound() { call.sendResponse(LDS, listeners, "0", "0000"); // Client sends an ACK LDS request. call.verifyRequest(LDS, Collections.singletonList(LISTENER_RESOURCE), "0", "0000", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); - EnvoyServerProtoData.Listener parsedListener = ldsUpdateCaptor.getValue().listener(); + @SuppressWarnings("unchecked") + ArgumentCaptor> ldsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(ldsResourceWatcher).onResourceChanged(ldsUpdateCaptor.capture()); + EnvoyServerProtoData.Listener parsedListener = ldsUpdateCaptor.getValue().getValue().listener(); assertThat(parsedListener.name()).isEqualTo(LISTENER_RESOURCE); assertThat(parsedListener.address()).isEqualTo("0.0.0.0:7000"); assertThat(parsedListener.defaultFilterChain()).isNull(); @@ -3878,7 +4359,8 @@ public void serverSideListenerNotFound() { verifyNoInteractions(ldsResourceWatcher); fakeClock.forwardTime(XdsClientImpl.INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS); - verify(ldsResourceWatcher).onResourceDoesNotExist(LISTENER_RESOURCE); + verify(ldsResourceWatcher).onResourceChanged(argThat( + statusOr -> statusOr.getStatus().getDescription().contains(LISTENER_RESOURCE))); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); } @@ -3904,7 +4386,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)); - verify(ldsResourceWatcher).onError(errorCaptor.capture()); + verify(ldsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); } @@ -3931,7 +4413,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)); - verify(ldsResourceWatcher).onError(errorCaptor.capture()); + verify(ldsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, errorMsg); } @@ -3962,7 +4444,8 @@ public void sendingToStoppedServer() throws Exception { .build() .start()); fakeClock.forwardTime(5, TimeUnit.SECONDS); - verify(ldsResourceWatcher, never()).onResourceDoesNotExist(LDS_RESOURCE); + verify(ldsResourceWatcher, never()).onResourceChanged(argThat( + statusOr -> statusOr.getStatus().getDescription().contains(LDS_RESOURCE))); fakeClock.forwardTime(20, TimeUnit.SECONDS); // Trigger rpcRetryTimer DiscoveryRpcCall call = resourceDiscoveryCalls.poll(3, TimeUnit.SECONDS); // Check metric data. @@ -3983,7 +4466,7 @@ public void sendingToStoppedServer() throws Exception { // Send a response and do verifications call.sendResponse(LDS, mf.buildWrappedResource(testListenerVhosts), VERSION_1, "0001"); call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0001", NODE); - verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture()); + verify(ldsResourceWatcher).onResourceChanged(StatusOr.fromValue(ldsUpdateCaptor.capture())); verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); assertThat(fakeClock.getPendingTasks(LDS_RESOURCE_FETCH_TIMEOUT_TASK_FILTER)).isEmpty(); verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT); @@ -4005,7 +4488,7 @@ public void sendToBadUrl() throws Exception { client.watchXdsResource(XdsListenerResource.getInstance(), LDS_RESOURCE, ldsResourceWatcher); fakeClock.forwardTime(20, TimeUnit.SECONDS); verify(ldsResourceWatcher, Mockito.timeout(5000).atLeastOnce()) - .onError(errorCaptor.capture()); + .onAmbientError(errorCaptor.capture()); assertThat(errorCaptor.getValue().getDescription()).contains(garbageUri); client.shutdown(); } @@ -4021,9 +4504,10 @@ public void circuitBreakingConversionOf32bitIntTo64bitLongForMaxRequestNegativeV // Client sent an ACK CDS request. call.verifyRequest(CDS, CDS_RESOURCE, VERSION_1, "0000", NODE); - verify(cdsResourceWatcher).onChanged(cdsUpdateCaptor.capture()); - CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue(); - + @SuppressWarnings("unchecked") + ArgumentCaptor> cdsUpdateCaptor = ArgumentCaptor.forClass(StatusOr.class); + verify(cdsResourceWatcher).onResourceChanged(cdsUpdateCaptor.capture()); + CdsUpdate cdsUpdate = cdsUpdateCaptor.getValue().getValue(); assertThat(cdsUpdate.clusterName()).isEqualTo(CDS_RESOURCE); assertThat(cdsUpdate.clusterType()).isEqualTo(ClusterType.EDS); assertThat(cdsUpdate.maxConcurrentRequests()).isEqualTo(4294967295L); @@ -4037,7 +4521,8 @@ public void sendToNonexistentServer() throws Exception { // file. Assume localhost doesn't speak HTTP/2 on the finger port XdsClientImpl client = createXdsClient("localhost:79"); client.watchXdsResource(XdsListenerResource.getInstance(), LDS_RESOURCE, ldsResourceWatcher); - verify(ldsResourceWatcher, Mockito.timeout(5000).times(1)).onError(ArgumentMatchers.any()); + verify(ldsResourceWatcher, Mockito.timeout(5000).times(1)) + .onAmbientError(ArgumentMatchers.any()); assertThat(fakeClock.numPendingTasks()).isEqualTo(1); //retry assertThat(fakeClock.getPendingTasks().iterator().next().toString().contains("RpcRetryTask")) .isTrue(); @@ -4104,10 +4589,10 @@ public void serverFailureMetricReport() { // Management server closes the RPC stream before sending any response. call.sendCompleted(); verify(ldsResourceWatcher, Mockito.timeout(1000).times(1)) - .onError(errorCaptor.capture()); + .onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, "ADS stream closed with OK before receiving a response"); - verify(rdsResourceWatcher).onError(errorCaptor.capture()); + verify(rdsResourceWatcher).onAmbientError(errorCaptor.capture()); verifyStatusWithNodeId(errorCaptor.getValue(), Code.UNAVAILABLE, "ADS stream closed with OK before receiving a response"); verifyServerFailureCount(1, 1, xdsServerInfo.target()); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java index 97c2695f209..ce81ad8c5c7 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -21,7 +21,7 @@ import static io.grpc.xds.GrpcXdsTransportFactory.DEFAULT_XDS_TRANSPORT_FACTORY; import static org.mockito.AdditionalAnswers.delegatesTo; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -35,9 +35,12 @@ import io.grpc.Grpc; import io.grpc.MetricRecorder; import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.FakeClock; import io.grpc.internal.ObjectPool; +import io.grpc.xds.XdsClusterResource.CdsUpdate; +import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.client.Bootstrapper; import io.grpc.xds.client.CommonBootstrapperTestUtils; import io.grpc.xds.client.LoadReportClient; @@ -99,25 +102,24 @@ public class XdsClientFallbackTest { private XdsClientMetricReporter xdsClientMetricReporter; @Captor - private ArgumentCaptor errorCaptor; + private ArgumentCaptor> errorCaptor; private final XdsClient.ResourceWatcher raalLdsWatcher = new XdsClient.ResourceWatcher() { @Override - public void onChanged(XdsListenerResource.LdsUpdate update) { - log.log(Level.FINE, "LDS update: " + update); + public void onResourceChanged(StatusOr update) { + if (update.getStatus().isOk()) { + log.log(Level.FINE, "LDS update: " + update.getValue()); + } else { + log.log(Level.FINE, "LDS update error: " + update.getStatus().getDescription()); + } } @Override - public void onError(Status error) { - log.log(Level.FINE, "LDS update error: " + error.getDescription()); - } - - @Override - public void onResourceDoesNotExist(String resourceName) { - log.log(Level.FINE, "LDS resource does not exist: " + resourceName); + public void onAmbientError(Status error) { + log.log(Level.FINE, "LDS Ambient error: " + error.getDescription()); } }; @@ -138,18 +140,17 @@ public void onResourceDoesNotExist(String resourceName) { new XdsClient.ResourceWatcher() { @Override - public void onChanged(XdsClusterResource.CdsUpdate update) { - log.log(Level.FINE, "CDS update: " + update); + public void onResourceChanged(StatusOr update) { + if (update.getStatus().isOk()) { + log.log(Level.FINE, "CDS update: " + update.getValue()); + } else { + log.log(Level.FINE, "CDS update error: " + update.getStatus().getDescription()); + } } @Override - public void onError(Status error) { - log.log(Level.FINE, "CDS update error: " + error.getDescription()); - } - - @Override - public void onResourceDoesNotExist(String resourceName) { - log.log(Level.FINE, "CDS resource does not exist: " + resourceName); + public void onAmbientError(Status error) { + log.log(Level.FINE, "CDS Ambient error: " + error.getDescription()); } }; @@ -222,12 +223,12 @@ public void everything_okay() { fallbackServer.restartXdsServer(); xdsClient = xdsClientPool.getObject(); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( XdsListenerResource.LdsUpdate.forApiListener( - MAIN_HTTP_CONNECTION_MANAGER)); + MAIN_HTTP_CONNECTION_MANAGER))); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - verify(rdsWatcher, timeout(5000)).onChanged(any()); + verify(rdsWatcher, timeout(5000)).onResourceChanged(any()); } @Test @@ -239,9 +240,9 @@ public void mainServerDown_fallbackServerUp() { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( XdsListenerResource.LdsUpdate.forApiListener( - FALLBACK_HTTP_CONNECTION_MANAGER)); + FALLBACK_HTTP_CONNECTION_MANAGER))); } @Test @@ -252,7 +253,8 @@ public void useBadAuthority() { String badPrefix = "xdstp://authority.xds.bad/envoy.config.listener.v3.Listener/"; xdsClient.watchXdsResource(XdsListenerResource.getInstance(), badPrefix + "listener.googleapis.com", ldsWatcher); - inOrder.verify(ldsWatcher, timeout(5000)).onError(any()); + inOrder.verify(ldsWatcher, timeout(5000)).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), badPrefix + "route-config.googleapis.bad", rdsWatcher); @@ -260,15 +262,18 @@ public void useBadAuthority() { badPrefix + "route-config2.googleapis.bad", rdsWatcher2); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), badPrefix + "route-config3.googleapis.bad", rdsWatcher3); - inOrder.verify(rdsWatcher, timeout(5000).times(1)).onError(any()); - inOrder.verify(rdsWatcher2, timeout(5000).times(1)).onError(any()); - inOrder.verify(rdsWatcher3, timeout(5000).times(1)).onError(any()); - verify(rdsWatcher, never()).onChanged(any()); + inOrder.verify(rdsWatcher, timeout(5000).times(1)).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); + inOrder.verify(rdsWatcher2, timeout(5000).times(1)).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); + inOrder.verify(rdsWatcher3, timeout(5000).times(1)).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); + verify(rdsWatcher, never()).onResourceChanged(argThat(StatusOr::hasValue)); // even after an error, a valid one will still work xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher2); - verify(ldsWatcher2, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher2, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); } @Test @@ -278,21 +283,24 @@ public void both_down_restart_main() { xdsClient = xdsClientPool.getObject(); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000).atLeastOnce()).onError(any()); - verify(ldsWatcher, timeout(5000).times(0)).onChanged(any()); + verify(ldsWatcher, timeout(5000).atLeastOnce()).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); + verify(ldsWatcher, timeout(5000).times(0)).onResourceChanged(any()); + xdsClient.watchXdsResource( XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher2); - verify(rdsWatcher2, timeout(5000).atLeastOnce()).onError(any()); + verify(rdsWatcher2, timeout(5000).atLeastOnce()).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk())); mainXdsServer.restartXdsServer(); xdsClient.watchXdsResource( XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - verify(ldsWatcher, timeout(16000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - verify(rdsWatcher, timeout(5000)).onChanged(any()); - verify(rdsWatcher2, timeout(5000)).onChanged(any()); + verify(ldsWatcher, timeout(16000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); + verify(rdsWatcher, timeout(5000)).onResourceChanged(any()); + verify(rdsWatcher2, timeout(5000)).onResourceChanged(any()); } @Test @@ -303,10 +311,10 @@ public void mainDown_fallbackUp_restart_main() { InOrder inOrder = inOrder(ldsWatcher, rdsWatcher, cdsWatcher, cdsWatcher2); xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - inOrder.verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER)); + inOrder.verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER))); xdsClient.watchXdsResource(XdsClusterResource.getInstance(), FALLBACK_CLUSTER_NAME, cdsWatcher); - inOrder.verify(cdsWatcher, timeout(5000)).onChanged(any()); + inOrder.verify(cdsWatcher, timeout(5000)).onResourceChanged(any()); assertThat(fallbackServer.getService().getSubscriberCounts() .get("type.googleapis.com/envoy.config.listener.v3.Listener")).isEqualTo(1); @@ -314,15 +322,15 @@ public void mainDown_fallbackUp_restart_main() { mainXdsServer.restartXdsServer(); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - inOrder.verify(rdsWatcher, timeout(5000)).onChanged(any()); + inOrder.verify(rdsWatcher, timeout(5000)).onResourceChanged(any()); verifyNoSubscribers(fallbackServer); xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); - inOrder.verify(cdsWatcher2, timeout(5000)).onChanged(any()); + inOrder.verify(cdsWatcher2, timeout(5000)).onResourceChanged(any()); verifyNoSubscribers(fallbackServer); assertThat(mainXdsServer.getService().getSubscriberCounts() @@ -362,11 +370,11 @@ public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - verify(rdsWatcher, timeout(5000)).onChanged(any()); + verify(rdsWatcher, timeout(5000)).onResourceChanged(any()); mainXdsServer.getServer().shutdownNow(); // Sleep for the ADS stream disconnect to be processed and for the retry to fail. Between those @@ -378,38 +386,46 @@ public XdsTransport create(Bootstrapper.ServerInfo serverInfo) { } // Shouldn't do fallback since all watchers are loaded - verify(ldsWatcher, never()).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher, never()).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER))); // Should just get from cache xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher2); xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher2); - verify(ldsWatcher2, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - verify(ldsWatcher, never()).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher2, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); + verify(ldsWatcher, never()).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER))); // Make sure that rdsWatcher wasn't called again - verify(rdsWatcher, times(1)).onChanged(any()); - verify(rdsWatcher2, timeout(5000)).onChanged(any()); + verify(rdsWatcher, times(1)).onResourceChanged(any()); + verify(rdsWatcher2, timeout(5000)).onResourceChanged(any()); // Asking for something not in cache should force a fallback xdsClient.watchXdsResource(XdsClusterResource.getInstance(), FALLBACK_CLUSTER_NAME, cdsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER)); - verify(ldsWatcher2, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER)); - verify(cdsWatcher, timeout(5000)).onChanged(any()); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER))); + verify(ldsWatcher2, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(FALLBACK_HTTP_CONNECTION_MANAGER))); + verify(cdsWatcher, timeout(5000)).onResourceChanged(any()); xdsClient.watchXdsResource( XdsRouteConfigureResource.getInstance(), FALLBACK_RDS_NAME, rdsWatcher3); - verify(rdsWatcher3, timeout(5000)).onChanged(any()); + verify(rdsWatcher3, timeout(5000)).onResourceChanged(any()); // Test that resource defined in main but not fallback is handled correctly xdsClient.watchXdsResource( XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); - verify(cdsWatcher2, never()).onResourceDoesNotExist(eq(CLUSTER_NAME)); - fakeClock.forwardTime(15000, TimeUnit.MILLISECONDS); // Does not exist timer - verify(cdsWatcher2, timeout(5000)).onResourceDoesNotExist(eq(CLUSTER_NAME)); + verify(cdsWatcher2, never()).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() + && statusOr.getStatus().getCode() == Status.Code.NOT_FOUND + && statusOr.getStatus().getDescription().contains(CLUSTER_NAME) + )); + fakeClock.forwardTime(15000, TimeUnit.MILLISECONDS); + verify(cdsWatcher2, timeout(5000)).onResourceChanged(argThat(statusOr -> + !statusOr.hasValue() + && statusOr.getStatus().getCode() == Status.Code.NOT_FOUND + && statusOr.getStatus().getDescription().contains(CLUSTER_NAME) + )); xdsClient.shutdown(); executor.shutdown(); } @@ -421,8 +437,8 @@ public void connect_then_mainServerRestart_fallbackServerdown() { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); mainXdsServer.getServer().shutdownNow(); fallbackServer.getServer().shutdownNow(); @@ -431,9 +447,9 @@ public void connect_then_mainServerRestart_fallbackServerdown() { mainXdsServer.restartXdsServer(); - verify(cdsWatcher, timeout(5000)).onChanged(any()); - verify(ldsWatcher, timeout(5000).atLeastOnce()).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(cdsWatcher, timeout(5000)).onResourceChanged(any()); + verify(ldsWatcher, timeout(5000).atLeastOnce()).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); } @Test @@ -448,10 +464,11 @@ public void fallbackFromBadUrlToGoodOne() { client.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); fakeClock.forwardTime(20, TimeUnit.SECONDS); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener( - MAIN_HTTP_CONNECTION_MANAGER)); - verify(ldsWatcher, never()).onError(any()); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); + verify(ldsWatcher, never()).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk() + )); client.shutdown(); } @@ -467,10 +484,12 @@ public void testGoodUrlFollowedByBadUrl() { new ExponentialBackoffPolicy.Provider(), MessagePrinter.INSTANCE, xdsClientMetricReporter); client.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( XdsListenerResource.LdsUpdate.forApiListener( - MAIN_HTTP_CONNECTION_MANAGER)); - verify(ldsWatcher, never()).onError(any()); + MAIN_HTTP_CONNECTION_MANAGER))); + verify(ldsWatcher, never()).onResourceChanged(argThat( + statusOr -> !statusOr.getStatus().isOk() + )); client.shutdown(); } @@ -487,9 +506,12 @@ public void testTwoBadUrl() { client.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); fakeClock.forwardTime(20, TimeUnit.SECONDS); - verify(ldsWatcher, Mockito.timeout(5000).atLeastOnce()).onError(errorCaptor.capture()); - assertThat(errorCaptor.getValue().getDescription()).contains(garbageUri2); - verify(ldsWatcher, never()).onChanged(any()); + verify(ldsWatcher, Mockito.timeout(5000).atLeastOnce()) + .onResourceChanged(errorCaptor.capture()); + assertThat(errorCaptor.getValue().getStatus().getDescription()).contains(garbageUri2); + verify(ldsWatcher, never()).onResourceChanged(argThat( + statusOr -> statusOr.getStatus().isOk() + )); client.shutdown(); } @@ -509,8 +531,8 @@ public void used_then_mainServerRestart_fallbackServerUp() { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + verify(ldsWatcher, timeout(5000)).onResourceChanged(StatusOr.fromValue( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER))); mainXdsServer.restartXdsServer(); @@ -519,7 +541,7 @@ public void used_then_mainServerRestart_fallbackServerUp() { xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher); - verify(cdsWatcher, timeout(5000)).onChanged(any()); + verify(cdsWatcher, timeout(5000)).onResourceChanged(any()); assertThat(getLrsServerInfo("localhost:" + fallbackServer.getServer().getPort())).isNull(); } @@ -547,5 +569,4 @@ public void used_then_mainServerRestart_fallbackServerUp() { "fallback-policy", "fallback" ); } - } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index b2b713e9a8e..2caa88b5d40 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -24,6 +24,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import io.grpc.MetricRecorder; +import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.internal.ObjectPool; import io.grpc.xds.Filter.NamedFilterConfig; import io.grpc.xds.XdsListenerResource.LdsUpdate; @@ -105,14 +107,22 @@ public void isolatedResourceDeletions() { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); - verify(mockWatcher, timeout(10000)).onChanged( - LdsUpdate.forApiListener( - HttpConnectionManager.forRdsName(0, "route-config.googleapis.com", ImmutableList.of( - new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG))))); - verify(mockDirectPathWatcher, timeout(10000)).onChanged( - LdsUpdate.forApiListener( - HttpConnectionManager.forRdsName(0, "route-config.googleapis.com", ImmutableList.of( - new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG))))); + verify(mockWatcher, timeout(10000)).onResourceChanged( + StatusOr.fromValue(LdsUpdate.forApiListener( + HttpConnectionManager.forRdsName( + 0, + "route-config.googleapis.com", + ImmutableList.of( + new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG) + ))))); + verify(mockDirectPathWatcher, timeout(10000)).onResourceChanged( + StatusOr.fromValue(LdsUpdate.forApiListener( + HttpConnectionManager.forRdsName( + 0, + "route-config.googleapis.com", + ImmutableList.of( + new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG) + ))))); // By setting the LDS config with a new server name we effectively make the old server to go // away as it is not in the configuration anymore. This change in one control plane (here the @@ -120,9 +130,12 @@ public void isolatedResourceDeletions() { // watcher of another control plane (here the DirectPath one). trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener("new-server")); - verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); - verify(mockDirectPathWatcher, times(0)).onResourceDoesNotExist( - "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + verify(mockWatcher, timeout(20000)).onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Resource test-server does not exist"))); + verify(mockDirectPathWatcher, times(0)).onResourceChanged( + StatusOr.fromStatus(Status.NOT_FOUND.withDescription( + "Resource xdstp://server-one/envoy.config.listener.v3.Listener/" + + "test-server does not exist"))); } /** diff --git a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java index f3f4d74eb2f..194ddd6bc75 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java @@ -35,6 +35,7 @@ import io.grpc.Server; import io.grpc.ServerBuilder; import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.inprocess.InProcessSocketAddress; import io.grpc.internal.TestUtils.NoopChannelLogger; import io.grpc.netty.GrpcHttp2ConnectionHandler; @@ -169,7 +170,7 @@ public void run() { ImmutableList.of(), null); LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(tcpListener); - xdsClient.ldsWatcher.onChanged(listenerUpdate); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromValue(listenerUpdate)); verify(listener, timeout(5000)).onServing(); start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS); FilterChainSelector selector = selectorManager.getSelectorToUpdateSelector(); @@ -190,7 +191,8 @@ public void run() { } }); String ldsWatched = xdsClient.ldsResource.get(5, TimeUnit.SECONDS); - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsWatched); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Lds resource: " + ldsWatched + "not found"))); verify(listener, timeout(5000)).onNotServing(any()); try { start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS); @@ -275,7 +277,8 @@ public void releaseOldSupplierOnNotFound_verifyClose() throws Exception { getSslContextProviderSupplier(selectorManager.getSelectorToUpdateSelector()); assertThat(returnedSupplier.getTlsContext()).isSameInstanceAs(tlsContext1); callUpdateSslContext(returnedSupplier); - xdsClient.ldsWatcher.onResourceDoesNotExist("not-found Error"); + xdsClient.ldsWatcher.onResourceChanged( + StatusOr.fromStatus(Status.NOT_FOUND.withDescription("not-found Error"))); verify(tlsContextManager, times(1)).releaseServerSslContextProvider(eq(sslContextProvider1)); } @@ -292,7 +295,7 @@ public void releaseOldSupplierOnTemporaryError_noClose() throws Exception { getSslContextProviderSupplier(selectorManager.getSelectorToUpdateSelector()); assertThat(returnedSupplier.getTlsContext()).isSameInstanceAs(tlsContext1); callUpdateSslContext(returnedSupplier); - xdsClient.ldsWatcher.onError(Status.CANCELLED); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.CANCELLED)); verify(tlsContextManager, never()).releaseServerSslContextProvider(eq(sslContextProvider1)); } diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 622084d4306..315ba6abc42 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -68,6 +68,7 @@ import io.grpc.NoopClientCall.NoopClientCallListener; import io.grpc.Status; import io.grpc.Status.Code; +import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.internal.AutoConfiguredLoadBalancerFactory; import io.grpc.internal.FakeClock; @@ -2509,8 +2510,9 @@ public void cancelXdsResourceWatch(XdsResourceType void deliverLdsUpdateOnly(long httpMaxStreamDurationNano, List virtualHosts) { syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( - httpMaxStreamDurationNano, virtualHosts, null))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( + httpMaxStreamDurationNano, virtualHosts, null)))); }); } @@ -2521,8 +2523,9 @@ void deliverLdsUpdate(long httpMaxStreamDurationNano, List virtualH } syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( - httpMaxStreamDurationNano, virtualHosts, null))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( + httpMaxStreamDurationNano, virtualHosts, null)))); createAndDeliverClusterUpdates(this, clusterNames.toArray(new String[0])); }); } @@ -2535,8 +2538,9 @@ void deliverLdsUpdate(final List routes) { List clusterNames = getClusterNames(routes); syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( - 0L, Collections.singletonList(virtualHost), null))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( + 0L, Collections.singletonList(virtualHost), null)))); if (!clusterNames.isEmpty()) { createAndDeliverClusterUpdates(this, clusterNames.toArray(new String[0])); } @@ -2545,8 +2549,9 @@ void deliverLdsUpdate(final List routes) { void deliverLdsUpdateWithFilters(VirtualHost vhost, List filterConfigs) { syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( - 0L, Collections.singletonList(vhost), filterConfigs))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( + 0L, Collections.singletonList(vhost), filterConfigs)))); }); } @@ -2593,8 +2598,9 @@ void deliverLdsUpdateWithFaultInjection( Collections.singletonList(route), overrideConfig); syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( - 0L, Collections.singletonList(virtualHost), filterChain))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts( + 0L, Collections.singletonList(virtualHost), filterChain)))); createAndDeliverClusterUpdates(this, cluster); }); } @@ -2609,8 +2615,9 @@ void deliverLdsUpdateForRdsNameWithFaultInjection( new NamedFilterConfig(FAULT_FILTER_INSTANCE_NAME, httpFilterFaultConfig), new NamedFilterConfig(ROUTER_FILTER_INSTANCE_NAME, RouterFilter.ROUTER_CONFIG)); syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forRdsName( - 0L, rdsName, filterChain))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forRdsName( + 0L, rdsName, filterChain)))); }); } @@ -2622,14 +2629,17 @@ void deliverLdsUpdateForRdsNameWithFilters( String rdsName, @Nullable List filterConfigs) { syncContext.execute(() -> { - ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forRdsName( - 0, rdsName, filterConfigs))); + ldsWatcher.onResourceChanged(StatusOr.fromValue( + LdsUpdate.forApiListener(HttpConnectionManager.forRdsName(0, rdsName, filterConfigs)) + )); }); } void deliverLdsResourceNotFound() { syncContext.execute(() -> { - ldsWatcher.onResourceDoesNotExist(expectedLdsResourceName); + ldsWatcher.onResourceChanged( + StatusOr.fromStatus(Status.NOT_FOUND.withDescription( + "Resource " + expectedLdsResourceName + " does not exist"))); }); } @@ -2691,7 +2701,8 @@ void deliverRdsUpdateWithFaultInjection( Collections.singletonList(route), overrideConfig); syncContext.execute(() -> { - rdsWatcher.onChanged(new RdsUpdate(Collections.singletonList(virtualHost))); + rdsWatcher.onResourceChanged(StatusOr.fromValue( + new RdsUpdate(Collections.singletonList(virtualHost)))); createAndDeliverClusterUpdates(this, cluster1); }); } @@ -2701,7 +2712,7 @@ void deliverRdsUpdate(String resourceName, List virtualHosts) { return; } syncContext.execute(() -> { - rdsWatcher.onChanged(new RdsUpdate(virtualHosts)); + rdsWatcher.onResourceChanged(StatusOr.fromValue(new RdsUpdate(virtualHosts))); }); } @@ -2714,7 +2725,8 @@ void deliverRdsResourceNotFound(String resourceName) { return; } syncContext.execute(() -> { - rdsWatcher.onResourceDoesNotExist(rdsResource); + rdsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("RDS resource not found: " + rdsResource))); }); } @@ -2725,7 +2737,7 @@ private void deliverCdsUpdate(String clusterName, CdsUpdate update) { syncContext.execute(() -> { List> resourceWatchers = ImmutableList.copyOf(cdsWatchers.get(clusterName)); - resourceWatchers.forEach(w -> w.onChanged(update)); + resourceWatchers.forEach(w -> w.onResourceChanged(StatusOr.fromValue(update))); }); } @@ -2736,7 +2748,7 @@ private void deliverEdsUpdate(String name, EdsUpdate update) { } List> resourceWatchers = ImmutableList.copyOf(edsWatchers.get(name)); - resourceWatchers.forEach(w -> w.onChanged(update)); + resourceWatchers.forEach(w -> w.onResourceChanged(StatusOr.fromValue(update))); }); } @@ -2744,18 +2756,18 @@ private void deliverEdsUpdate(String name, EdsUpdate update) { void deliverError(final Status error) { if (ldsWatcher != null) { syncContext.execute(() -> { - ldsWatcher.onError(error); + ldsWatcher.onAmbientError(error); }); } if (rdsWatcher != null) { syncContext.execute(() -> { - rdsWatcher.onError(error); + rdsWatcher.onAmbientError(error); }); } syncContext.execute(() -> { cdsWatchers.values().stream() .flatMap(List::stream) - .forEach(w -> w.onError(error)); + .forEach(w -> w.onAmbientError(error)); }); } } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index d28c7d7c607..f9e529a020e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -32,6 +32,7 @@ import io.grpc.ServerServiceDefinition; import io.grpc.Status; import io.grpc.StatusException; +import io.grpc.StatusOr; import io.grpc.testing.GrpcCleanupRule; import io.grpc.xds.XdsServerTestHelper.FakeXdsClient; import io.grpc.xds.XdsServerTestHelper.FakeXdsClientPoolFactory; @@ -195,13 +196,14 @@ public void xdsServer_discoverState() throws Exception { CommonTlsContextTestsUtil.buildTestInternalDownstreamTlsContext("CERT1", "VA1"), tlsContextManager); future.get(5000, TimeUnit.MILLISECONDS); - xdsClient.ldsWatcher.onError(Status.ABORTED); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.ABORTED)); verify(mockXdsServingStatusListener, never()).onNotServing(any(StatusException.class)); reset(mockXdsServingStatusListener); - xdsClient.ldsWatcher.onError(Status.CANCELLED); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.CANCELLED)); verify(mockXdsServingStatusListener, never()).onNotServing(any(StatusException.class)); reset(mockXdsServingStatusListener); - xdsClient.ldsWatcher.onResourceDoesNotExist("not found error"); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("not found error"))); verify(mockXdsServingStatusListener).onNotServing(any(StatusException.class)); reset(mockXdsServingStatusListener); XdsServerTestHelper.generateListenerUpdate( @@ -249,7 +251,7 @@ public void xdsServerStartSecondUpdateAndError() tlsContextManager); verify(mockXdsServingStatusListener, never()).onNotServing(any(Throwable.class)); verifyServer(future, mockXdsServingStatusListener, null); - xdsClient.ldsWatcher.onError(Status.ABORTED); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.ABORTED)); verifyServer(null, mockXdsServingStatusListener, null); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index ec9f4c54c31..55505d90c72 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -23,6 +23,8 @@ import com.google.common.util.concurrent.SettableFuture; import io.grpc.InsecureChannelCredentials; import io.grpc.MetricRecorder; +import io.grpc.Status; +import io.grpc.StatusOr; import io.grpc.internal.ObjectPool; import io.grpc.xds.EnvoyServerProtoData.ConnectionSourceType; import io.grpc.xds.EnvoyServerProtoData.FilterChain; @@ -291,7 +293,7 @@ private String awaitLdsResource(Duration timeout) { } void deliverLdsUpdate(LdsUpdate ldsUpdate) { - execute(() -> ldsWatcher.onChanged(ldsUpdate)); + execute(() -> ldsWatcher.onResourceChanged(StatusOr.fromValue(ldsUpdate))); } void deliverLdsUpdate( @@ -306,11 +308,13 @@ void deliverLdsUpdate(FilterChain filterChain, @Nullable FilterChain defaultFilt } void deliverLdsResourceNotFound() { - execute(() -> ldsWatcher.onResourceDoesNotExist(awaitLdsResource(DEFAULT_TIMEOUT))); + execute(() -> ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("LDS resource not found")))); } void deliverRdsUpdate(String resourceName, List virtualHosts) { - execute(() -> rdsWatchers.get(resourceName).onChanged(new RdsUpdate(virtualHosts))); + execute(() -> rdsWatchers.get(resourceName).onResourceChanged( + StatusOr.fromValue(new RdsUpdate(virtualHosts)))); } void deliverRdsUpdate(String resourceName, VirtualHost virtualHost) { @@ -318,7 +322,8 @@ void deliverRdsUpdate(String resourceName, VirtualHost virtualHost) { } void deliverRdsResourceNotFound(String resourceName) { - execute(() -> rdsWatchers.get(resourceName).onResourceDoesNotExist(resourceName)); + execute(() -> rdsWatchers.get(resourceName).onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("RDS resource not found: " + resourceName)))); } } } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java index e5f0f44cbae..c12a961a963 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerWrapperTest.java @@ -48,6 +48,7 @@ import io.grpc.ServerInterceptor; import io.grpc.Status; import io.grpc.StatusException; +import io.grpc.StatusOr; import io.grpc.SynchronizationContext; import io.grpc.internal.FakeClock; import io.grpc.testing.TestMethodDescriptors; @@ -338,7 +339,8 @@ public void run() { } }); String ldsResource = xdsClient.ldsResource.get(5, TimeUnit.SECONDS); - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Lds Resource: " + ldsResource + " not found"))); verify(listener, timeout(5000)).onNotServing(any()); try { start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS); @@ -526,8 +528,8 @@ public void run() { verify(listener).onServing(); verify(mockServer).start(); - // server shutdown after resourceDoesNotExist - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Lds Resource not found"))); verify(mockServer).shutdown(); // re-deliver lds resource @@ -702,7 +704,7 @@ public void run() { EnvoyServerProtoData.FilterChain f1 = createFilterChain("filter-chain-1", createRds("r0")); xdsClient.deliverLdsUpdate(Arrays.asList(f0, f1), null); xdsClient.awaitRds(FakeXdsClient.DEFAULT_TIMEOUT); - xdsClient.rdsWatchers.get("r0").onError(Status.CANCELLED); + xdsClient.rdsWatchers.get("r0").onResourceChanged(StatusOr.fromStatus(Status.CANCELLED)); start.get(5000, TimeUnit.MILLISECONDS); assertThat(selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().size()) .isEqualTo(2); @@ -722,13 +724,14 @@ public void run() { Collections.singletonList(createVirtualHost("virtual-host-1"))); assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); - xdsClient.rdsWatchers.get("r0").onError(Status.CANCELLED); + xdsClient.rdsWatchers.get("r0").onResourceChanged(StatusOr.fromStatus(Status.CANCELLED)); realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); assertThat(realConfig.virtualHosts()).isEqualTo( Collections.singletonList(createVirtualHost("virtual-host-1"))); assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); - xdsClient.rdsWatchers.get("r0").onResourceDoesNotExist("r0"); + xdsClient.rdsWatchers.get("r0").onResourceChanged( + StatusOr.fromStatus(Status.NOT_FOUND.withDescription("Resource r0 not found"))); realConfig = selectorManager.getSelectorToUpdateSelector().getRoutingConfigs().get(f1).get(); assertThat(realConfig.virtualHosts()).isEmpty(); assertThat(realConfig.interceptors()).isEmpty(); @@ -748,7 +751,8 @@ public void run() { } }); String ldsResource = xdsClient.ldsResource.get(5, TimeUnit.SECONDS); - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("LDS resource: " + ldsResource + " does not exist"))); verify(listener, timeout(5000)).onNotServing(any()); try { start.get(START_WAIT_AFTER_LISTENER_MILLIS, TimeUnit.MILLISECONDS); @@ -762,7 +766,7 @@ public void run() { FilterChain filterChain0 = createFilterChain("filter-chain-0", createRds("rds")); SslContextProviderSupplier sslSupplier0 = filterChain0.sslContextProviderSupplier(); xdsClient.deliverLdsUpdate(Collections.singletonList(filterChain0), null); - xdsClient.ldsWatcher.onError(Status.INTERNAL); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.INTERNAL)); assertThat(selectorManager.getSelectorToUpdateSelector()) .isSameInstanceAs(FilterChainSelector.NO_FILTER_CHAIN); ResourceWatcher saveRdsWatcher = xdsClient.rdsWatchers.get("rds"); @@ -801,7 +805,7 @@ public void run() { xdsClient.deliverRdsUpdate("rds", Collections.singletonList(createVirtualHost("virtual-host-2"))); assertThat(sslSupplier1.isShutdown()).isFalse(); - xdsClient.ldsWatcher.onError(Status.DEADLINE_EXCEEDED); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus(Status.DEADLINE_EXCEEDED)); verify(mockBuilder, times(1)).build(); verify(mockServer, times(2)).start(); verify(listener, times(2)).onNotServing(any(StatusException.class)); @@ -815,8 +819,8 @@ public void run() { assertThat(sslSupplier1.isShutdown()).isFalse(); - // not serving after serving - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Resource does not exist"))); assertThat(xdsClient.rdsWatchers).isEmpty(); verify(mockServer, times(2)).shutdown(); when(mockServer.isShutdown()).thenReturn(true); @@ -824,9 +828,8 @@ public void run() { .isSameInstanceAs(FilterChainSelector.NO_FILTER_CHAIN); verify(listener, times(3)).onNotServing(any(StatusException.class)); assertThat(sslSupplier1.isShutdown()).isTrue(); - // no op - saveRdsWatcher.onChanged( - new RdsUpdate(Collections.singletonList(createVirtualHost("virtual-host-1")))); + saveRdsWatcher.onResourceChanged(StatusOr.fromValue(new RdsUpdate( + Collections.singletonList(createVirtualHost("virtual-host-1"))))); verify(mockBuilder, times(1)).build(); verify(mockServer, times(2)).start(); verify(listener, times(1)).onServing(); @@ -855,7 +858,8 @@ public void run() { assertThat(realConfig.interceptors()).isEqualTo(ImmutableMap.of()); assertThat(executor.numPendingTasks()).isEqualTo(1); - xdsClient.ldsWatcher.onResourceDoesNotExist(ldsResource); + xdsClient.ldsWatcher.onResourceChanged(StatusOr.fromStatus( + Status.NOT_FOUND.withDescription("Resource does not exist"))); verify(mockServer, times(3)).shutdown(); verify(listener, times(4)).onNotServing(any(StatusException.class)); verify(listener, times(1)).onNotServing(any(IOException.class)); @@ -1278,7 +1282,8 @@ public ServerCall.Listener interceptCall(ServerCall Date: Mon, 5 May 2025 15:21:20 +0530 Subject: [PATCH 2/3] merge conflict took old method handleConfigNotFound() --- xds/src/main/java/io/grpc/xds/XdsServerWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index fcb087b61ec..630f982f0e0 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -392,7 +392,7 @@ public void onResourceChanged(StatusOr updateOrError) { StatusException statusException = error.withDescription( error.getDescription() + " xDS node ID: " + xdsClient.getBootstrapInfo().node().getId()).asException(); - handleConfigNotFound(statusException); + handleConfigNotFoundOrMismatch(statusException); return; } LdsUpdate update = updateOrError.getValue(); From 25914801113ba5390c6fb524187995147bf80465 Mon Sep 17 00:00:00 2001 From: MV Shiva Prasad Date: Tue, 6 May 2025 10:07:05 +0530 Subject: [PATCH 3/3] use CertificateProviderPluginInstance --- .../test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index e308780986d..f94b839a7ba 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -2656,11 +2656,10 @@ public void cdsResponseWithUpstreamTlsContext() { return false; } try { - CommonTlsContext.CertificateProviderInstance certificateProviderInstance = + CertificateProviderPluginInstance certificateProviderInstance = statusOr.getValue().upstreamTlsContext() - .getCommonTlsContext() - .getCombinedValidationContext() - .getValidationContextCertificateProviderInstance(); + .getCommonTlsContext().getCombinedValidationContext() + .getDefaultValidationContext().getCaCertificateProviderInstance(); assertThat(certificateProviderInstance.getInstanceName()) .isEqualTo("cert-instance-name"); assertThat(certificateProviderInstance.getCertificateName())