From 25aa454862ea2dc12639b52908c1f3358f60e58c Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Mon, 11 Mar 2024 17:23:37 -0700 Subject: [PATCH 01/10] Checkpoint --- .../main/java/io/grpc/xds/CsdsService.java | 52 +++++++++++-------- .../InternalSharedXdsClientPoolProvider.java | 2 +- .../grpc/xds/SharedXdsClientPoolProvider.java | 26 +++++++--- .../io/grpc/xds/XdsClientPoolFactory.java | 7 ++- .../java/io/grpc/xds/XdsNameResolver.java | 13 +++-- .../io/grpc/xds/XdsNameResolverProvider.java | 2 +- .../java/io/grpc/xds/CsdsServiceTest.java | 13 ++++- 7 files changed, 76 insertions(+), 39 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/CsdsService.java b/xds/src/main/java/io/grpc/xds/CsdsService.java index 0102836660c..4e50d30cdc5 100644 --- a/xds/src/main/java/io/grpc/xds/CsdsService.java +++ b/xds/src/main/java/io/grpc/xds/CsdsService.java @@ -39,6 +39,8 @@ import io.grpc.xds.client.XdsClient.ResourceMetadata.ResourceMetadataStatus; import io.grpc.xds.client.XdsClient.ResourceMetadata.UpdateFailureState; import io.grpc.xds.client.XdsResourceType; +import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; @@ -117,33 +119,39 @@ public void onCompleted() { private boolean handleRequest( ClientStatusRequest request, StreamObserver responseObserver) { - StatusException error; - try { - responseObserver.onNext(getConfigDumpForRequest(request)); - return true; - } catch (StatusException e) { - error = e; - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - logger.log(Level.FINE, "Server interrupted while building CSDS config dump", e); - error = Status.ABORTED.withDescription("Thread interrupted").withCause(e).asException(); - } catch (RuntimeException e) { - logger.log(Level.WARNING, "Unexpected error while building CSDS config dump", e); - error = - Status.INTERNAL.withDescription("Unexpected internal error").withCause(e).asException(); - } - responseObserver.onError(error); - return false; - } + StatusException error = null; - private ClientStatusResponse getConfigDumpForRequest(ClientStatusRequest request) - throws StatusException, InterruptedException { if (request.getNodeMatchersCount() > 0) { - throw new StatusException( + error = new StatusException( Status.INVALID_ARGUMENT.withDescription("node_matchers not supported")); + } else { + List targets = xdsClientPoolFactory.getTargets(); + for (int i = 0; i < targets.size() && error == null; i++) { + String target = targets.get(i); + try { + responseObserver.onNext(getConfigDumpForRequest(target, request)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + logger.log(Level.FINE, "Server interrupted while building CSDS config dump", e); + error = Status.ABORTED.withDescription("Thread interrupted").withCause(e).asException(); + } catch (RuntimeException e) { + logger.log(Level.WARNING, "Unexpected error while building CSDS config dump", e); + error = + Status.INTERNAL.withDescription("Unexpected internal error").withCause(e).asException(); + } + } } - ObjectPool xdsClientPool = xdsClientPoolFactory.get(); + if (error == null) { + return true; // All clients reported without error + } + responseObserver.onError(error); + return false; + } + + private ClientStatusResponse getConfigDumpForRequest(String target, ClientStatusRequest request) + throws InterruptedException { + ObjectPool xdsClientPool = xdsClientPoolFactory.get(target); if (xdsClientPool == null) { return ClientStatusResponse.getDefaultInstance(); } diff --git a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java index 39b9ed0d095..0073cce1a88 100644 --- a/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/InternalSharedXdsClientPoolProvider.java @@ -36,6 +36,6 @@ public static void setDefaultProviderBootstrapOverride(Map bootstrap) public static ObjectPool getOrCreate(String target) throws XdsInitializationException { - return SharedXdsClientPoolProvider.getDefaultProvider().getOrCreate(); + return SharedXdsClientPoolProvider.getDefaultProvider().getOrCreate(target); } } diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index 5ae1f5bbce5..10a5d92b644 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -20,6 +20,8 @@ import static io.grpc.xds.GrpcXdsTransportFactory.DEFAULT_XDS_TRANSPORT_FACTORY; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.sun.javafx.UnmodifiableArrayList; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; @@ -31,7 +33,9 @@ import io.grpc.xds.client.XdsClientImpl; import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.internal.security.TlsContextManagerImpl; +import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; @@ -53,7 +57,8 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory { private final Bootstrapper bootstrapper; private final Object lock = new Object(); private final AtomicReference> bootstrapOverride = new AtomicReference<>(); - private volatile ObjectPool xdsClientPool; +// private volatile ObjectPool xdsClientPool; + private Map> targetToXdsClientMap = new ConcurrentHashMap<>(); SharedXdsClientPoolProvider() { this(new GrpcBootstrapperImpl()); @@ -75,16 +80,16 @@ public void setBootstrapOverride(Map bootstrap) { @Override @Nullable - public ObjectPool get() { - return xdsClientPool; + public ObjectPool get(String target) { + return targetToXdsClientMap.get(target); } @Override - public ObjectPool getOrCreate() throws XdsInitializationException { - ObjectPool ref = xdsClientPool; + public ObjectPool getOrCreate(String target) throws XdsInitializationException { + ObjectPool ref = targetToXdsClientMap.get(target); if (ref == null) { synchronized (lock) { - ref = xdsClientPool; + ref = targetToXdsClientMap.get(target); if (ref == null) { BootstrapInfo bootstrapInfo; Map rawBootstrap = bootstrapOverride.get(); @@ -96,13 +101,20 @@ public ObjectPool getOrCreate() throws XdsInitializationException { if (bootstrapInfo.servers().isEmpty()) { throw new XdsInitializationException("No xDS server provided"); } - ref = xdsClientPool = new RefCountedXdsClientObjectPool(bootstrapInfo); + ref = new RefCountedXdsClientObjectPool(bootstrapInfo); + targetToXdsClientMap.put(target, ref); } } } return ref; } + @Override + public ImmutableList getTargets() { + return ImmutableList.copyOf(targetToXdsClientMap.keySet()); + } + + private static class SharedXdsClientPoolProviderHolder { private static final SharedXdsClientPoolProvider instance = new SharedXdsClientPoolProvider(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java index c649b3b3069..313eb675116 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientPoolFactory.java @@ -19,6 +19,7 @@ import io.grpc.internal.ObjectPool; import io.grpc.xds.client.XdsClient; import io.grpc.xds.client.XdsInitializationException; +import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -26,7 +27,9 @@ interface XdsClientPoolFactory { void setBootstrapOverride(Map bootstrap); @Nullable - ObjectPool get(); + ObjectPool get(String target); - ObjectPool getOrCreate() throws XdsInitializationException; + ObjectPool getOrCreate(String target) throws XdsInitializationException; + + List getTargets(); } diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 9ad9b6e82f0..53b71811a9a 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -66,6 +66,7 @@ import io.grpc.xds.client.XdsClient.ResourceWatcher; import io.grpc.xds.client.XdsLogger; import io.grpc.xds.client.XdsLogger.XdsLogLevel; +import java.net.URI; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -104,6 +105,7 @@ final class XdsNameResolver extends NameResolver { private final XdsLogger logger; @Nullable private final String targetAuthority; + private final String target; private final String serviceAuthority; // Encoded version of the service authority as per // https://datatracker.ietf.org/doc/html/rfc3986#section-3.2. @@ -133,23 +135,24 @@ final class XdsNameResolver extends NameResolver { private boolean receivedConfig; XdsNameResolver( - @Nullable String targetAuthority, String name, @Nullable String overrideAuthority, + URI targetUri, String name, @Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser, SynchronizationContext syncContext, ScheduledExecutorService scheduler, @Nullable Map bootstrapOverride) { - this(targetAuthority, name, overrideAuthority, serviceConfigParser, syncContext, scheduler, + this(targetUri, targetUri.getAuthority(), name, overrideAuthority, serviceConfigParser, syncContext, scheduler, SharedXdsClientPoolProvider.getDefaultProvider(), ThreadSafeRandomImpl.instance, FilterRegistry.getDefaultRegistry(), bootstrapOverride); } @VisibleForTesting XdsNameResolver( - @Nullable String targetAuthority, String name, @Nullable String overrideAuthority, - ServiceConfigParser serviceConfigParser, + URI targetUri, @Nullable String targetAuthority, String name, + @Nullable String overrideAuthority, ServiceConfigParser serviceConfigParser, SynchronizationContext syncContext, ScheduledExecutorService scheduler, XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random, FilterRegistry filterRegistry, @Nullable Map bootstrapOverride) { this.targetAuthority = targetAuthority; + target = targetUri.toString(); // The name might have multiple slashes so encode it before verifying. serviceAuthority = checkNotNull(name, "name"); @@ -180,7 +183,7 @@ public String getServiceAuthority() { public void start(Listener2 listener) { this.listener = checkNotNull(listener, "listener"); try { - xdsClientPool = xdsClientPoolFactory.getOrCreate(); + xdsClientPool = xdsClientPoolFactory.getOrCreate(target); } catch (Exception e) { listener.onError( Status.UNAVAILABLE.withDescription("Failed to initialize xDS").withCause(e)); diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java b/xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java index 598be07fcd8..8d0e59eaa91 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java @@ -78,7 +78,7 @@ public XdsNameResolver newNameResolver(URI targetUri, Args args) { targetUri); String name = targetPath.substring(1); return new XdsNameResolver( - targetUri.getAuthority(), name, args.getOverrideAuthority(), + targetUri, name, args.getOverrideAuthority(), args.getServiceConfigParser(), args.getSynchronizationContext(), args.getScheduledExecutorService(), bootstrapOverride); diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index bf330b1007a..39b9c6ad942 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -53,6 +53,7 @@ import io.grpc.xds.client.XdsClient; import io.grpc.xds.client.XdsClient.ResourceMetadata; import io.grpc.xds.client.XdsClient.ResourceMetadata.ResourceMetadataStatus; +import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.client.XdsResourceType; import java.util.Collection; import java.util.HashMap; @@ -471,7 +472,7 @@ private FakeXdsClientPoolFactory(@Nullable XdsClient xdsClient) { @Override @Nullable - public ObjectPool get() { + public ObjectPool get(String target) { return new ObjectPool() { @Override public XdsClient getObject() { @@ -485,6 +486,16 @@ public XdsClient returnObject(Object object) { }; } + @Override + public ObjectPool getOrCreate(String target) throws XdsInitializationException { + return null; + } + + @Override + public List getTargets() { + return null; + } + @Override public void setBootstrapOverride(Map bootstrap) { throw new UnsupportedOperationException("Should not be called"); From d8f7094526a13611ee892d37fb706f058ebaf904 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Tue, 12 Mar 2024 17:09:43 -0700 Subject: [PATCH 02/10] XdsClient Fallback working --- .../io/grpc/census/CensusModulesTest.java | 9 +- .../main/java/io/grpc/xds/CsdsService.java | 42 +- .../io/grpc/xds/GrpcXdsTransportFactory.java | 7 + .../grpc/xds/SharedXdsClientPoolProvider.java | 41 +- .../java/io/grpc/xds/XdsListenerResource.java | 5 + .../java/io/grpc/xds/XdsNameResolver.java | 6 +- .../java/io/grpc/xds/XdsServerWrapper.java | 2 +- .../io/grpc/xds/client/BootstrapperImpl.java | 16 + .../grpc/xds/client/ControlPlaneClient.java | 149 +++-- .../java/io/grpc/xds/client/XdsClient.java | 27 +- .../io/grpc/xds/client/XdsClientImpl.java | 605 +++++++++++++++--- .../io/grpc/xds/client/XdsResourceType.java | 4 + .../grpc/xds/client/XdsTransportFactory.java | 5 + .../java/io/grpc/xds/ControlPlaneRule.java | 105 ++- .../java/io/grpc/xds/CsdsServiceTest.java | 38 +- .../io/grpc/xds/GrpcBootstrapperImplTest.java | 1 + .../grpc/xds/GrpcXdsClientImplTestBase.java | 4 +- .../io/grpc/xds/GrpcXdsClientImplV3Test.java | 36 +- .../xds/SharedXdsClientPoolProviderTest.java | 21 +- .../io/grpc/xds/XdsClientFallbackTest.java | 377 +++++++++++ .../io/grpc/xds/XdsClientFederationTest.java | 3 +- .../java/io/grpc/xds/XdsNameResolverTest.java | 66 +- .../java/io/grpc/xds/XdsServerTestHelper.java | 9 +- .../grpc/xds/XdsTestControlPlaneService.java | 9 +- 24 files changed, 1350 insertions(+), 237 deletions(-) create mode 100644 xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java diff --git a/census/src/test/java/io/grpc/census/CensusModulesTest.java b/census/src/test/java/io/grpc/census/CensusModulesTest.java index 6ccaf78314f..9e0b4d935d3 100644 --- a/census/src/test/java/io/grpc/census/CensusModulesTest.java +++ b/census/src/test/java/io/grpc/census/CensusModulesTest.java @@ -56,6 +56,7 @@ import io.grpc.ClientInterceptors; import io.grpc.ClientStreamTracer; import io.grpc.Context; +import io.grpc.KnownLength; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.ServerCall; @@ -99,6 +100,7 @@ import io.opencensus.trace.Tracer; import io.opencensus.trace.propagation.BinaryFormat; import io.opencensus.trace.propagation.SpanContextParseException; +import java.io.IOException; import java.io.InputStream; import java.util.HashSet; import java.util.List; @@ -136,7 +138,7 @@ public class CensusModulesTest { ClientStreamTracer.StreamInfo.newBuilder() .setCallOptions(CallOptions.DEFAULT.withOption(NAME_RESOLUTION_DELAYED, 10L)).build(); - private static class StringInputStream extends InputStream { + private static class StringInputStream extends InputStream implements KnownLength { final String string; StringInputStream(String string) { @@ -149,6 +151,11 @@ public int read() { // passed to the InProcess server and consumed by MARSHALLER.parse(). throw new UnsupportedOperationException("Should not be called"); } + + @Override + public int available() throws IOException { + return string == null ? 0 : string.length(); + } } private static final MethodDescriptor.Marshaller MARSHALLER = diff --git a/xds/src/main/java/io/grpc/xds/CsdsService.java b/xds/src/main/java/io/grpc/xds/CsdsService.java index 4e50d30cdc5..a296beb45d0 100644 --- a/xds/src/main/java/io/grpc/xds/CsdsService.java +++ b/xds/src/main/java/io/grpc/xds/CsdsService.java @@ -39,7 +39,7 @@ import io.grpc.xds.client.XdsClient.ResourceMetadata.ResourceMetadataStatus; import io.grpc.xds.client.XdsClient.ResourceMetadata.UpdateFailureState; import io.grpc.xds.client.XdsResourceType; -import java.util.HashMap; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.ExecutionException; @@ -126,20 +126,32 @@ private boolean handleRequest( Status.INVALID_ARGUMENT.withDescription("node_matchers not supported")); } else { List targets = xdsClientPoolFactory.getTargets(); + List clientConfigs = new ArrayList<>(targets.size()); + for (int i = 0; i < targets.size() && error == null; i++) { - String target = targets.get(i); try { - responseObserver.onNext(getConfigDumpForRequest(target, request)); + ClientConfig clientConfig = getConfigForRequest(targets.get(i)); + if (clientConfig != null) { + clientConfigs.add(clientConfig); + } } catch (InterruptedException e) { Thread.currentThread().interrupt(); logger.log(Level.FINE, "Server interrupted while building CSDS config dump", e); error = Status.ABORTED.withDescription("Thread interrupted").withCause(e).asException(); } catch (RuntimeException e) { logger.log(Level.WARNING, "Unexpected error while building CSDS config dump", e); - error = - Status.INTERNAL.withDescription("Unexpected internal error").withCause(e).asException(); + error = Status.INTERNAL.withDescription("Unexpected internal error").withCause(e) + .asException(); } } + + try { + responseObserver.onNext(getStatusResponse(clientConfigs)); + } catch (RuntimeException e) { + logger.log(Level.WARNING, "Unexpected error while processing CSDS config dump", e); + error = Status.INTERNAL.withDescription("Unexpected internal error").withCause(e) + .asException(); + } } if (error == null) { @@ -149,19 +161,16 @@ private boolean handleRequest( return false; } - private ClientStatusResponse getConfigDumpForRequest(String target, ClientStatusRequest request) - throws InterruptedException { + private ClientConfig getConfigForRequest(String target) throws InterruptedException { ObjectPool xdsClientPool = xdsClientPoolFactory.get(target); if (xdsClientPool == null) { - return ClientStatusResponse.getDefaultInstance(); + return null; } XdsClient xdsClient = null; try { xdsClient = xdsClientPool.getObject(); - return ClientStatusResponse.newBuilder() - .addConfig(getClientConfigForXdsClient(xdsClient)) - .build(); + return getClientConfigForXdsClient(xdsClient, target); } finally { if (xdsClient != null) { xdsClientPool.returnObject(xdsClient); @@ -169,9 +178,18 @@ private ClientStatusResponse getConfigDumpForRequest(String target, ClientStatus } } + private ClientStatusResponse getStatusResponse(List clientConfigs) { + if (clientConfigs.isEmpty()) { + return ClientStatusResponse.getDefaultInstance(); + } + return ClientStatusResponse.newBuilder().addAllConfig(clientConfigs).build(); + } + @VisibleForTesting - static ClientConfig getClientConfigForXdsClient(XdsClient xdsClient) throws InterruptedException { + static ClientConfig getClientConfigForXdsClient(XdsClient xdsClient, String target) + throws InterruptedException { ClientConfig.Builder builder = ClientConfig.newBuilder() + .setClientScope(target) .setNode(xdsClient.getBootstrapInfo().node().toEnvoyProtoNode()); Map, Map> metadataByType = diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 74c28ba2d2d..4f622791f0d 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -22,6 +22,7 @@ import io.grpc.CallOptions; import io.grpc.ChannelCredentials; import io.grpc.ClientCall; +import io.grpc.ConnectivityState; import io.grpc.Context; import io.grpc.Grpc; import io.grpc.ManagedChannel; @@ -84,6 +85,12 @@ public void shutdown() { channel.shutdown(); } + @Override + public boolean isConnected() { + ConnectivityState state = channel.getState(false); + return state == ConnectivityState.READY; + } + private class XdsStreamingCall implements XdsTransportFactory.StreamingCall { diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index 10a5d92b644..6ddcd559a9f 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -21,7 +21,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import com.sun.javafx.UnmodifiableArrayList; import io.grpc.internal.ExponentialBackoffPolicy; import io.grpc.internal.GrpcUtil; import io.grpc.internal.ObjectPool; @@ -33,7 +32,6 @@ import io.grpc.xds.client.XdsClientImpl; import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.internal.security.TlsContextManagerImpl; -import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledExecutorService; @@ -57,8 +55,7 @@ final class SharedXdsClientPoolProvider implements XdsClientPoolFactory { private final Bootstrapper bootstrapper; private final Object lock = new Object(); private final AtomicReference> bootstrapOverride = new AtomicReference<>(); -// private volatile ObjectPool xdsClientPool; - private Map> targetToXdsClientMap = new ConcurrentHashMap<>(); + private final Map> targetToXdsClientMap = new ConcurrentHashMap<>(); SharedXdsClientPoolProvider() { this(new GrpcBootstrapperImpl()); @@ -101,7 +98,7 @@ public ObjectPool getOrCreate(String target) throws XdsInitialization if (bootstrapInfo.servers().isEmpty()) { throw new XdsInitializationException("No xDS server provided"); } - ref = new RefCountedXdsClientObjectPool(bootstrapInfo); + ref = new RefCountedXdsClientObjectPool(bootstrapInfo, target); targetToXdsClientMap.put(target, ref); } } @@ -122,7 +119,11 @@ private static class SharedXdsClientPoolProviderHolder { @ThreadSafe @VisibleForTesting static class RefCountedXdsClientObjectPool implements ObjectPool { + + private static final ExponentialBackoffPolicy.Provider BACKOFF_POLICY_PROVIDER = + new ExponentialBackoffPolicy.Provider(); private final BootstrapInfo bootstrapInfo; + private final String target; // The target associated with the xDS client. private final Object lock = new Object(); @GuardedBy("lock") private ScheduledExecutorService scheduler; @@ -132,8 +133,9 @@ static class RefCountedXdsClientObjectPool implements ObjectPool { private int refCount; @VisibleForTesting - RefCountedXdsClientObjectPool(BootstrapInfo bootstrapInfo) { + RefCountedXdsClientObjectPool(BootstrapInfo bootstrapInfo, String target) { this.bootstrapInfo = checkNotNull(bootstrapInfo); + this.target = target; } @Override @@ -144,15 +146,19 @@ public XdsClient getObject() { log.log(Level.INFO, "xDS node ID: {0}", bootstrapInfo.node().getId()); } scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); - xdsClient = new XdsClientImpl( - DEFAULT_XDS_TRANSPORT_FACTORY, - bootstrapInfo, - scheduler, - new ExponentialBackoffPolicy.Provider(), - GrpcUtil.STOPWATCH_SUPPLIER, - TimeProvider.SYSTEM_TIME_PROVIDER, - MessagePrinter.INSTANCE, - new TlsContextManagerImpl(bootstrapInfo)); + try { + xdsClient = new XdsClientImpl( + DEFAULT_XDS_TRANSPORT_FACTORY, + bootstrapInfo, + scheduler, + BACKOFF_POLICY_PROVIDER, + GrpcUtil.STOPWATCH_SUPPLIER, + TimeProvider.SYSTEM_TIME_PROVIDER, + MessagePrinter.INSTANCE, + new TlsContextManagerImpl(bootstrapInfo)); + } catch (Exception e) { + throw new RuntimeException(e); + } } refCount++; return xdsClient; @@ -179,5 +185,10 @@ XdsClient getXdsClientForTest() { return xdsClient; } } + + public String getTarget() { + return target; + } } + } diff --git a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java index af77d128ae7..4d6050f3bbe 100644 --- a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java @@ -114,6 +114,11 @@ protected LdsUpdate doParse(Args args, Message unpackedMessage) } } + @Override + public boolean isSharedName() { + return true; + } + private LdsUpdate processClientSideListener(Listener listener) throws ResourceInvalidException { // Unpack HttpConnectionManager from the Listener. diff --git a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java index 53b71811a9a..f0329387fc9 100644 --- a/xds/src/main/java/io/grpc/xds/XdsNameResolver.java +++ b/xds/src/main/java/io/grpc/xds/XdsNameResolver.java @@ -139,9 +139,9 @@ final class XdsNameResolver extends NameResolver { ServiceConfigParser serviceConfigParser, SynchronizationContext syncContext, ScheduledExecutorService scheduler, @Nullable Map bootstrapOverride) { - this(targetUri, targetUri.getAuthority(), name, overrideAuthority, serviceConfigParser, syncContext, scheduler, - SharedXdsClientPoolProvider.getDefaultProvider(), ThreadSafeRandomImpl.instance, - FilterRegistry.getDefaultRegistry(), bootstrapOverride); + this(targetUri, targetUri.getAuthority(), name, overrideAuthority, serviceConfigParser, + syncContext, scheduler, SharedXdsClientPoolProvider.getDefaultProvider(), + ThreadSafeRandomImpl.instance, FilterRegistry.getDefaultRegistry(), bootstrapOverride); } @VisibleForTesting diff --git a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java index bf8603fb3e4..dfb7c4fb7db 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerWrapper.java @@ -171,7 +171,7 @@ public void run() { private void internalStart() { try { - xdsClientPool = xdsClientPoolFactory.getOrCreate(); + xdsClientPool = xdsClientPoolFactory.getOrCreate(""); } catch (Exception e) { StatusException statusException = Status.UNAVAILABLE.withDescription( "Failed to initialize xDS").withCause(e).asException(); diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index 7ef739c8048..2782af06825 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -41,6 +41,9 @@ @Internal public abstract class BootstrapperImpl extends Bootstrapper { + public static final String GRPC_EXPERIMENTAL_XDS_FALLBACK = + "GRPC_EXPERIMENTAL_XDS_FALLBACK"; + // Client features. @VisibleForTesting public static final String CLIENT_FEATURE_DISABLE_OVERPROVISIONING = @@ -59,11 +62,18 @@ protected BootstrapperImpl() { logger = XdsLogger.withLogId(InternalLogId.allocate("bootstrapper", null)); } + // Delayed initialization of xdsFallbackEnabled to allow for flag initialization. + public static boolean isEnabledXdsFallback() { + return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_FALLBACK, false); + } + protected abstract String getJsonContent() throws IOException, XdsInitializationException; protected abstract Object getImplSpecificConfig(Map serverConfig, String serverUri) throws XdsInitializationException; + + /** * Reads and parses bootstrap config. The config is expected to be in JSON format. */ @@ -102,6 +112,9 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) throw new XdsInitializationException("Invalid bootstrap: 'xds_servers' does not exist."); } List servers = parseServerInfos(rawServerConfigs, logger); + if (servers.size() > 1 && !isEnabledXdsFallback()) { + servers = ImmutableList.of(servers.get(0)); + } builder.servers(servers); Node.Builder nodeBuilder = Node.newBuilder(); @@ -208,6 +221,9 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) if (rawAuthorityServers == null || rawAuthorityServers.isEmpty()) { authorityServers = servers; } else { + if (rawAuthorityServers.size() > 1 && !isEnabledXdsFallback()) { + rawAuthorityServers = ImmutableList.of(rawAuthorityServers.get(0)); + } authorityServers = parseServerInfos(rawAuthorityServers, logger); } authorityInfoMapBuilder.put( diff --git a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java index 3074d1120ad..673c8cff66e 100644 --- a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java +++ b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java @@ -16,7 +16,6 @@ package io.grpc.xds.client; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -72,7 +71,6 @@ final class ControlPlaneClient { private final BackoffPolicy.Provider backoffPolicyProvider; private final Stopwatch stopwatch; private final Node bootstrapNode; - private final XdsClient xdsClient; // Last successfully applied version_info for each resource type. Starts with empty string. // A version_info is used to update management server with client's most recent knowledge of @@ -80,13 +78,15 @@ final class ControlPlaneClient { private final Map, String> versions = new HashMap<>(); private boolean shutdown; + private boolean hasBeenActive; + private boolean lastStateWasReady; @Nullable private AdsStream adsStream; @Nullable private BackoffPolicy retryBackoffPolicy; @Nullable private ScheduledHandle rpcRetryTimer; - private MessagePrettyPrinter messagePrinter; + private final MessagePrettyPrinter messagePrinter; /** An entity that manages ADS RPCs over a single channel. */ ControlPlaneClient( @@ -100,7 +100,6 @@ final class ControlPlaneClient { SynchronizationContext syncContext, BackoffPolicy.Provider backoffPolicyProvider, Supplier stopwatchSupplier, - XdsClient xdsClient, MessagePrettyPrinter messagePrinter) { this.serverInfo = checkNotNull(serverInfo, "serverInfo"); this.xdsTransport = checkNotNull(xdsTransport, "xdsTransport"); @@ -110,7 +109,6 @@ final class ControlPlaneClient { this.timeService = checkNotNull(timeService, "timeService"); this.syncContext = checkNotNull(syncContext, "syncContext"); this.backoffPolicyProvider = checkNotNull(backoffPolicyProvider, "backoffPolicyProvider"); - this.xdsClient = checkNotNull(xdsClient, "xdsClient"); this.messagePrinter = checkNotNull(messagePrinter, "messagePrinter"); stopwatch = checkNotNull(stopwatchSupplier, "stopwatchSupplier").get(); logId = InternalLogId.allocate("xds-client", serverInfo.target()); @@ -140,18 +138,31 @@ public String toString() { return logId.toString(); } + public ServerInfo getServerInfo() { + return serverInfo; + } + /** * Updates the resource subscription for the given resource type. */ // Must be synchronized. - void adjustResourceSubscription(XdsResourceType resourceType) { + void adjustResourceSubscription(XdsResourceType resourceType, String authority) { if (isInBackoff()) { return; } if (adsStream == null) { startRpcStream(); + // when the stream becomes ready, it will send the discovery requests + return; } - Collection resources = resourceStore.getSubscribedResources(serverInfo, resourceType); + + // We will do the rest of the method as part of the readyHandler when the stream is ready. + if (!lastStateWasReady) { + return; + } + + Collection resources = + resourceStore.getSubscribedResources(serverInfo, resourceType, authority); if (resources == null) { resources = Collections.emptyList(); } @@ -200,7 +211,7 @@ void nackResponse(XdsResourceType type, String nonce, String errorDetail) { */ // Must be synchronized. boolean isInBackoff() { - return rpcRetryTimer != null && rpcRetryTimer.isPending(); + return rpcRetryTimer != null || (hasBeenActive && !xdsTransport.isConnected()); } // Must be synchronized. @@ -208,6 +219,10 @@ boolean isReady() { return adsStream != null && adsStream.call != null && adsStream.call.isReady(); } + boolean isConnected() { + return xdsTransport.isConnected(); + } + /** * Starts a timer for each requested resource that hasn't been responded to and * has been waiting for the channel to get ready. @@ -218,12 +233,22 @@ void readyHandler() { return; } - if (isInBackoff()) { + if (rpcRetryTimer != null) { rpcRetryTimer.cancel(); rpcRetryTimer = null; } - xdsClient.startSubscriberTimersIfNeeded(serverInfo); + hasBeenActive = true; + if (!lastStateWasReady) { + lastStateWasReady = true; + xdsResponseHandler.handleStreamRestarted(serverInfo); + } + } + + void connect() { + if (adsStream == null) { + startRpcStream(); + } } /** @@ -234,27 +259,54 @@ void readyHandler() { private void startRpcStream() { checkState(adsStream == null, "Previous adsStream has not been cleared yet"); adsStream = new AdsStream(); + adsStream.start(); logger.log(XdsLogLevel.INFO, "ADS stream started"); stopwatch.reset().start(); } + void sendDiscoveryRequests(String authority) { + if (adsStream == null) { + startRpcStream(); + // when the stream becomes ready, it will send the discovery requests + return; + } + + if (isConnected()) { + Set> subscribedResourceTypes = + new HashSet<>(resourceStore.getSubscribedResourceTypesWithTypeUrl().values()); + + for (XdsResourceType type : subscribedResourceTypes) { + adjustResourceSubscription(type, authority); + } + } + } + + @SuppressWarnings("rawtypes") + boolean hasSubscribedResources(String authority) { + for (XdsResourceType type : resourceStore.getSubscribedResourceTypesWithTypeUrl().values()) { + Collection subscribedResources = + resourceStore.getSubscribedResources(serverInfo, type, authority); + if (subscribedResources != null && !subscribedResources.isEmpty()) { + return true; + } + } + return false; + } + @VisibleForTesting public final class RpcRetryTask implements Runnable { @Override public void run() { - if (shutdown) { + logger.log(XdsLogLevel.DEBUG, "Retry timeout. Restart ADS stream {0}", logId); + if (shutdown || isReady()) { return; } - startRpcStream(); - Set> subscribedResourceTypes = - new HashSet<>(resourceStore.getSubscribedResourceTypesWithTypeUrl().values()); - for (XdsResourceType type : subscribedResourceTypes) { - Collection resources = resourceStore.getSubscribedResources(serverInfo, type); - if (resources != null) { - adsStream.sendDiscoveryRequest(type, resources); - } + + if (adsStream == null) { + startRpcStream(); } - xdsResponseHandler.handleStreamRestarted(serverInfo); + + // handling CPC management is triggered in readyHandler } } @@ -281,6 +333,9 @@ private class AdsStream implements EventHandler { private AdsStream() { this.call = xdsTransport.createStreamingCall(methodDescriptor.getFullMethodName(), methodDescriptor.getRequestMarshaller(), methodDescriptor.getResponseMarshaller()); + } + + void start() { call.start(this); } @@ -309,6 +364,9 @@ void sendDiscoveryRequest(XdsResourceType type, String versionInfo, builder.setErrorDetail(error); } DiscoveryRequest request = builder.build(); + if (isConnected()) { + resourceStore.assignResourcesToOwner(type, resources, ControlPlaneClient.this); + } call.sendMessage(request); if (logger.isLoggable(XdsLogLevel.DEBUG)) { logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", messagePrinter.print(request)); @@ -326,6 +384,11 @@ final void sendDiscoveryRequest(XdsResourceType type, Collection reso @Override public void onReady() { + logger.log(XdsLogLevel.DEBUG, "ADS stream ready {0}", logId); + if (shutdown || closed) { + return; + } + syncContext.execute(ControlPlaneClient.this::readyHandler); } @@ -357,12 +420,9 @@ public void run() { @Override public void onStatusReceived(final Status status) { + lastStateWasReady = false; syncContext.execute(() -> { - if (status.isOk()) { - handleRpcStreamClosed(Status.UNAVAILABLE.withDescription(CLOSED_BY_SERVER)); - } else { - handleRpcStreamClosed(status); - } + handleRpcStreamClosed(status); }); } @@ -381,7 +441,7 @@ final void handleRpcResponse(XdsResourceType type, String versionInfo, ListMust be synchronized. + */ void handleStreamRestarted(ServerInfo serverInfo); } @@ -427,6 +431,25 @@ public interface ResourceStore { Collection getSubscribedResources(ServerInfo serverInfo, XdsResourceType type); + /** + * Like {@link #getSubscribedResources(ServerInfo, XdsResourceType)}, but limits the results to + * those matching the given authority. + */ + @Nullable + Collection getSubscribedResources( + ServerInfo serverInfo, XdsResourceType type, String authority); + Map> getSubscribedResourceTypesWithTypeUrl(); + + /** + * Assigns the given resources to the given owner. Generally, the owner is a ControlPlaneClient + * @param type the type of the resources + * @param resources the names of the resources + * @param owner the object that should take over ownership of the resources + */ + default void assignResourcesToOwner(XdsResourceType type, Collection resources, + Object owner) { + // Default is a no-op implemention which is useful for test cases where everything is mocked + } } } 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 79147cd9862..db9d686eb9d 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java @@ -19,6 +19,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.xds.client.Bootstrapper.XDSTP_SCHEME; +import static io.grpc.xds.client.ControlPlaneClient.CLOSED_BY_SERVER; import static io.grpc.xds.client.XdsResourceType.ParsedResource; import static io.grpc.xds.client.XdsResourceType.ValidatedResourceUpdate; @@ -26,6 +27,7 @@ import com.google.common.base.Joiner; import com.google.common.base.Stopwatch; import com.google.common.base.Supplier; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ListenableFuture; @@ -41,12 +43,13 @@ import io.grpc.xds.client.Bootstrapper.AuthorityInfo; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.client.XdsClient.ResourceStore; -import io.grpc.xds.client.XdsClient.XdsResponseHandler; import io.grpc.xds.client.XdsLogger.XdsLogLevel; import java.net.URI; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -60,7 +63,7 @@ * XdsClient implementation. */ @Internal -public final class XdsClientImpl extends XdsClient implements XdsResponseHandler, ResourceStore { +public final class XdsClientImpl extends XdsClient implements ResourceStore { // Longest time to wait, since the subscription to some resource, for concluding its absence. @VisibleForTesting @@ -74,21 +77,25 @@ public void uncaughtException(Thread t, Throwable e) { XdsLogLevel.ERROR, "Uncaught exception in XdsClient SynchronizationContext. Panic!", e); - // TODO(chengyuanzhang): better error handling. + // TODO: better error handling. throw new AssertionError(e); } }); - private final Map loadStatsManagerMap = - new HashMap<>(); - final Map serverLrsClientMap = - new HashMap<>(); + private final Map loadStatsManagerMap = new HashMap<>(); + final Map serverLrsClientMap = new HashMap<>(); + /** Map of authority to its active control plane client (affected by xds fallback). */ + private final Map activeCpClients = new HashMap<>(); private final Map serverCpClientMap = new HashMap<>(); + + /** Maps resource type to the corresponding map of subscribers (keyed by resource name). */ private final Map, Map>> resourceSubscribers = new HashMap<>(); + /** Maps typeUrl to the corresponding XdsResourceType. */ private final Map> subscribedResourceTypeUrls = new HashMap<>(); + private final XdsTransportFactory xdsTransportFactory; private final Bootstrapper.BootstrapInfo bootstrapInfo; private final ScheduledExecutorService timeService; @@ -123,48 +130,6 @@ public XdsClientImpl( logger.log(XdsLogLevel.INFO, "Created"); } - @Override - public void handleResourceResponse( - XdsResourceType xdsResourceType, ServerInfo serverInfo, String versionInfo, - List resources, String nonce, ProcessingTracker processingTracker) { - checkNotNull(xdsResourceType, "xdsResourceType"); - syncContext.throwIfNotInThisSynchronizationContext(); - Set toParseResourceNames = - xdsResourceType.shouldRetrieveResourceKeysForArgs() - ? getResourceKeys(xdsResourceType) - : null; - XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, versionInfo, nonce, - bootstrapInfo, securityConfig, toParseResourceNames); - handleResourceUpdate(args, resources, xdsResourceType, processingTracker); - } - - @Override - public void handleStreamClosed(Status error) { - syncContext.throwIfNotInThisSynchronizationContext(); - cleanUpResourceTimers(); - for (Map> subscriberMap : - resourceSubscribers.values()) { - for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (!subscriber.hasResult()) { - subscriber.onError(error, null); - } - } - } - } - - @Override - public void handleStreamRestarted(ServerInfo serverInfo) { - syncContext.throwIfNotInThisSynchronizationContext(); - for (Map> subscriberMap : - resourceSubscribers.values()) { - for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (subscriber.serverInfo.equals(serverInfo)) { - subscriber.restartTimer(); - } - } - } - } - @Override public void shutdown() { syncContext.execute( @@ -181,7 +146,7 @@ public void run() { for (final LoadReportClient lrsClient : serverLrsClientMap.values()) { lrsClient.stopLoadReporting(); } - cleanUpResourceTimers(); + cleanUpResourceTimers(null); } }); } @@ -196,22 +161,79 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { return Collections.unmodifiableMap(subscribedResourceTypeUrls); } + @Override + public void assignResourcesToOwner(XdsResourceType type, Collection resources, + Object owner) { + if (isShutDown()) { + return; + } + + Map> resourceSubscriberMap = + resourceSubscribers.get(type); + + ControlPlaneClient controlPlaneClient = (ControlPlaneClient) owner; + for (String resource : resources) { + ResourceSubscriber subscriber = resourceSubscriberMap.get(resource); + if (subscriber != null && subscriber.controlPlaneClient == null) { + subscriber.controlPlaneClient = controlPlaneClient; + } + } + } + + public void assignUnassignedResources(String authority) { + if (isShutDown()) { + return; + } + + ControlPlaneClient controlPlaneClient = activeCpClients.get(authority); + if (controlPlaneClient == null) { + return; + } + + for (XdsResourceType resourceType : resourceSubscribers.keySet()) { + for (ResourceSubscriber subscriber + : resourceSubscribers.get(resourceType).values()) { + if (subscriber.controlPlaneClient == null + && Objects.equals(subscriber.authority, authority)) { + subscriber.controlPlaneClient = controlPlaneClient; + } + } + } + } + + @Nullable + @Override + public Collection getSubscribedResources( + ServerInfo serverInfo, XdsResourceType type) { + return getSubscribedResources(serverInfo, type, null, false); + } + @Nullable @Override - public Collection getSubscribedResources(ServerInfo serverInfo, - XdsResourceType type) { + public Collection getSubscribedResources( + ServerInfo serverInfo, XdsResourceType type, String authority) { + return getSubscribedResources(serverInfo, type, authority, true); + } + + private Collection getSubscribedResources(ServerInfo serverInfo, XdsResourceType type, String authority, boolean useAuthority) { Map> resources = resourceSubscribers.getOrDefault(type, Collections.emptyMap()); ImmutableSet.Builder builder = ImmutableSet.builder(); - for (String key : resources.keySet()) { - if (resources.get(key).serverInfo.equals(serverInfo)) { - builder.add(key); + String target = serverInfo.target(); + + for (String resourceName : resources.keySet()) { + ResourceSubscriber resource = resources.get(resourceName); + if ( resource.isCpcMutable() || (target.equals(resource.getTarget()) + && (!useAuthority || Objects.equals(authority, resource.authority)))) { + builder.add(resourceName); } } Collection retVal = builder.build(); return retVal.isEmpty() ? null : retVal; } + // As XdsClient APIs becomes resource agnostic, subscribed resource types are dynamic. // ResourceTypes that do not have subscribers does not show up in the snapshot keys. @Override @@ -225,7 +247,7 @@ public void run() { // A map from a "resource type" to a map ("resource name": "resource metadata") ImmutableMap.Builder, Map> metadataSnapshot = ImmutableMap.builder(); - for (XdsResourceType resourceType: resourceSubscribers.keySet()) { + for (XdsResourceType resourceType : resourceSubscribers.keySet()) { ImmutableMap.Builder metadataMap = ImmutableMap.builder(); for (Map.Entry> resourceEntry : resourceSubscribers.get(resourceType).entrySet()) { @@ -246,9 +268,9 @@ public Object getSecurityConfig() { @Override public void watchXdsResource(XdsResourceType type, - String resourceName, - ResourceWatcher watcher, - Executor watcherExecutor) { + String resourceName, + ResourceWatcher watcher, + Executor watcherExecutor) { syncContext.execute(new Runnable() { @Override @SuppressWarnings("unchecked") @@ -263,31 +285,69 @@ public void run() { logger.log(XdsLogLevel.INFO, "Subscribe {0} resource {1}", type, resourceName); subscriber = new ResourceSubscriber<>(type, resourceName); resourceSubscribers.get(type).put(resourceName, subscriber); + subscriber.addWatcher(watcher, watcherExecutor); + if (subscriber.errorDescription != null) { + return; + } + + ControlPlaneClient cpcToUse; + if (subscriber.controlPlaneClient == null) { + cpcToUse = activeCpClients.get(subscriber.authority); + } else { + cpcToUse = subscriber.controlPlaneClient; + } + if (subscriber.controlPlaneClient != null) { - subscriber.controlPlaneClient.adjustResourceSubscription(type); + doFallbackIfNecessary(subscriber.controlPlaneClient, subscriber.authority); } + if (cpcToUse != null) { + cpcToUse.adjustResourceSubscription(type, subscriber.authority); + } else { + // No active control plane client for the authority. Start a new one. + ImmutableList serverInfos = getServerInfos(subscriber.authority); + if (serverInfos == null) { + logger.log(XdsLogLevel.INFO, + "Request for unknown authority {}", subscriber.authority); + return; + } + cpcToUse = getOrCreateControlPlaneClient(serverInfos); + if (cpcToUse != null) { + logger.log(XdsLogLevel.DEBUG, "Start new control plane client {0}", + cpcToUse.getServerInfo()); + } else { + logger.log(XdsLogLevel.WARNING, "No active control plane client for authority {0}", + subscriber.authority); + } + } + } else { + subscriber.addWatcher(watcher, watcherExecutor); } - subscriber.addWatcher(watcher, watcherExecutor); } }); } @Override public void cancelXdsResourceWatch(XdsResourceType type, - String resourceName, - ResourceWatcher watcher) { + String resourceName, + ResourceWatcher watcher) { syncContext.execute(new Runnable() { @Override @SuppressWarnings("unchecked") public void run() { ResourceSubscriber subscriber = (ResourceSubscriber) resourceSubscribers.get(type).get(resourceName); + if (subscriber == null) { + logger.log(XdsLogLevel.WARNING, "double cancel of resource watch for {0}:{1}", + type.typeName(), resourceName); + return; + } subscriber.removeWatcher(watcher); if (!subscriber.isWatched()) { subscriber.cancelResourceWatch(); resourceSubscribers.get(type).remove(resourceName); - if (subscriber.controlPlaneClient != null) { - subscriber.controlPlaneClient.adjustResourceSubscription(type); + ControlPlaneClient controlPlaneClient = subscriber.controlPlaneClient; + if (controlPlaneClient != null) { + controlPlaneClient.adjustResourceSubscription(type, subscriber.authority); } if (resourceSubscribers.get(type).isEmpty()) { resourceSubscribers.remove(type); @@ -354,17 +414,30 @@ public void run() { return; } + // TODO add Jitter so that server isn't swamped when it comes up if this is a restart + ControlPlaneClient cpc = serverCpClientMap.get(serverInfo); + if (cpc == null) { + return; + } + for (Map> subscriberMap : resourceSubscribers.values()) { for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (subscriber.serverInfo.equals(serverInfo) && subscriber.respTimer == null) { + if (cpc == subscriber.controlPlaneClient && subscriber.respTimer == null) { subscriber.restartTimer(); } } } + } }); } + @VisibleForTesting + public boolean isSubscriberCpcConnected(XdsResourceType type, String resourceName) { + ControlPlaneClient cpc = resourceSubscribers.get(type).get(resourceName).controlPlaneClient; + return cpc != null && cpc.isConnected(); + } + private Set getResourceKeys(XdsResourceType xdsResourceType) { if (!resourceSubscribers.containsKey(xdsResourceType)) { return null; @@ -373,33 +446,60 @@ private Set getResourceKeys(XdsResourceType xdsResourceType) { return resourceSubscribers.get(xdsResourceType).keySet(); } - private void cleanUpResourceTimers() { + private void cleanUpResourceTimers(ControlPlaneClient cpcForThisStream) { for (Map> subscriberMap : resourceSubscribers.values()) { for (ResourceSubscriber subscriber : subscriberMap.values()) { - subscriber.stopTimer(); + if (cpcForThisStream == null || cpcForThisStream.equals(subscriber.controlPlaneClient)) { + subscriber.stopTimer(); + } + } + } + } + + public ControlPlaneClient getOrCreateControlPlaneClient(ImmutableList serverInfos) { + if (serverInfos.isEmpty()) { + return null; + } + + for (ServerInfo serverInfo : serverInfos) { + if (serverCpClientMap.containsKey(serverInfo)) { + ControlPlaneClient controlPlaneClient = serverCpClientMap.get(serverInfo); + if (controlPlaneClient.isInBackoff()) { + continue; + } + return controlPlaneClient; + } else { + ControlPlaneClient cpc = getOrCreateControlPlaneClient(serverInfo); + logger.log(XdsLogLevel.DEBUG, "Created control plane client {0}", cpc); + return cpc; } } + + // Everything existed and is in backoff so return null + return null; } - public ControlPlaneClient getOrCreateControlPlaneClient(ServerInfo serverInfo) { + private ControlPlaneClient getOrCreateControlPlaneClient(ServerInfo serverInfo) { syncContext.throwIfNotInThisSynchronizationContext(); if (serverCpClientMap.containsKey(serverInfo)) { return serverCpClientMap.get(serverInfo); } XdsTransportFactory.XdsTransport xdsTransport = xdsTransportFactory.create(serverInfo); + ControlPlaneClient controlPlaneClient = new ControlPlaneClient( xdsTransport, serverInfo, bootstrapInfo.node(), - this, + new ResponseHandler(serverInfo), this, timeService, syncContext, backoffPolicyProvider, stopwatchSupplier, - this, - messagePrinter); + messagePrinter + ); + serverCpClientMap.put(serverInfo, controlPlaneClient); LoadStatsManager2 loadStatsManager = new LoadStatsManager2(stopwatchSupplier); @@ -418,46 +518,73 @@ public Map getServerLrsClientMap() { return ImmutableMap.copyOf(serverLrsClientMap); } - @Nullable - private ServerInfo getServerInfo(String resource) { + private String getAuthority(String resource) { + String authority; if (resource.startsWith(XDSTP_SCHEME)) { URI uri = URI.create(resource); - String authority = uri.getAuthority(); + authority = uri.getAuthority(); if (authority == null) { authority = ""; } + } else { + authority = null; + } + + return authority; + } + + @Nullable + private ImmutableList getServerInfos(String authority) { + if (authority != null) { AuthorityInfo authorityInfo = bootstrapInfo.authorities().get(authority); if (authorityInfo == null || authorityInfo.xdsServers().isEmpty()) { return null; } - return authorityInfo.xdsServers().get(0); + return authorityInfo.xdsServers(); } else { - return bootstrapInfo.servers().get(0); // use first server + return bootstrapInfo.servers(); + } + } + + private List getAuthoritiesForServerInfo(ServerInfo serverInfo) { + List authorities = new ArrayList<>(); + for (Map.Entry entry : bootstrapInfo.authorities().entrySet()) { + if (entry.getValue().xdsServers().contains(serverInfo)) { + authorities.add(entry.getKey()); + } + } + + if (bootstrapInfo.servers().contains(serverInfo)) { + authorities.add(null); } + + return authorities; } @SuppressWarnings("unchecked") private void handleResourceUpdate( XdsResourceType.Args args, List resources, XdsResourceType xdsResourceType, ProcessingTracker processingTracker) { + ControlPlaneClient controlPlaneClient = serverCpClientMap.get(args.serverInfo); + ValidatedResourceUpdate result = xdsResourceType.parse(args, resources); logger.log(XdsLogger.XdsLogLevel.INFO, "Received {0} Response version {1} nonce {2}. Parsed resources: {3}", - xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources); + xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources); Map> parsedResources = result.parsedResources; Set invalidResources = result.invalidResources; List errors = result.errors; String errorDetail = null; if (errors.isEmpty()) { checkArgument(invalidResources.isEmpty(), "found invalid resources but missing errors"); - serverCpClientMap.get(args.serverInfo).ackResponse(xdsResourceType, args.versionInfo, + controlPlaneClient.ackResponse(xdsResourceType, args.versionInfo, args.nonce); } else { errorDetail = Joiner.on('\n').join(errors); logger.log(XdsLogLevel.WARNING, "Failed processing {0} Response version {1} nonce {2}. Errors:\n{3}", xdsResourceType.typeName(), args.versionInfo, args.nonce, errorDetail); - serverCpClientMap.get(args.serverInfo).nackResponse(xdsResourceType, args.nonce, errorDetail); + controlPlaneClient.nackResponse(xdsResourceType, args.nonce, errorDetail); } long updateTime = timeProvider.currentTimeNanos(); @@ -467,6 +594,9 @@ private void handleResourceUpdate( String resourceName = entry.getKey(); ResourceSubscriber subscriber = (ResourceSubscriber) entry.getValue(); if (parsedResources.containsKey(resourceName)) { + if (subscriber.isCpcMutable()) { + subscriber.controlPlaneClient = controlPlaneClient; + } // Happy path: the resource updated successfully. Notify the watchers of the update. subscriber.onData(parsedResources.get(resourceName), args.versionInfo, updateTime, processingTracker); @@ -496,51 +626,153 @@ private void handleResourceUpdate( // For State of the World services, notify watchers when their watched resource is missing // from the ADS update. Note that we can only do this if the resource update is coming from // the same xDS server that the ResourceSubscriber is subscribed to. - if (subscriber.serverInfo.equals(args.serverInfo)) { + if (controlPlaneClient.equals(subscriber.controlPlaneClient)) { subscriber.onAbsent(processingTracker); } } } + /** + * Try to fallback to a lower priority control plane client. + * + * @param oldCpc The control plane that we are falling back from + * @param activeServerInfo Matches the currently active control plane client + * @param serverInfos The full ordered list of xds servers + * @param authority The authority within which we are falling back + * @return true if a fallback was successful, false otherwise. + */ + private boolean doFallbackForAuthority(ControlPlaneClient oldCpc, ServerInfo activeServerInfo, + List serverInfos, String authority) { + if (serverInfos == null || serverInfos.size() < 2) { + return false; + } + + // Build a list of servers with lower priority than the current server. + int i = serverInfos.indexOf(activeServerInfo); + List fallbackServers = (i > -1) + ? serverInfos.subList(i, serverInfos.size()) + : null; + + ControlPlaneClient fallbackCpc = + fallbackServers != null + ? getOrCreateControlPlaneClient(ImmutableList.copyOf(fallbackServers)) + : null; + + return fallBackToCpc(fallbackCpc, authority, oldCpc); + } + + private boolean fallBackToCpc(ControlPlaneClient fallbackCpc, String authority, + ControlPlaneClient oldCpc) { + boolean didFallback = false; + if (fallbackCpc != null && ! fallbackCpc.isInBackoff()) { + logger.log(XdsLogLevel.INFO, "Falling back to XDS server {0}", + fallbackCpc.getServerInfo().target()); + + setCpcForAuthority(authority, fallbackCpc); + fallbackCpc.sendDiscoveryRequests(authority); + didFallback = true; + } else { + logger.log(XdsLogLevel.WARNING, "No working fallback XDS Servers found from {0}", + oldCpc.getServerInfo().target()); + } + return didFallback; + } + + private void doFallbackIfNecessary(ControlPlaneClient cpc, String authority) { + if (cpc == null || !BootstrapperImpl.isEnabledXdsFallback()) { + return; + } + + ControlPlaneClient activeCpClient = activeCpClients.get(authority); + if (cpc == activeCpClient) { + return; + } + if (activeCpClient == null) { + setCpcForAuthority(authority, cpc); + return; + } + + fallBackToCpc(cpc, authority, activeCpClient); + } + + private void setCpcForAuthority(String authority, ControlPlaneClient cpc) { + activeCpClients.put(authority, cpc); + + // Take ownership of resource's whose names are shared across xds servers (ex. LDS) + for (Map.Entry, Map>> subscriberTypeEntry + : resourceSubscribers.entrySet()) { + if (!subscriberTypeEntry.getKey().isSharedName()) { + continue; + } + for (ResourceSubscriber subscriber : subscriberTypeEntry.getValue().values()) { + if (subscriber.controlPlaneClient == cpc + || !Objects.equals(subscriber.authority, authority)) { + continue; + } + subscriber.data = null; + subscriber.absent = false; + subscriber.controlPlaneClient = cpc; + subscriber.restartTimer(); + } + } + + // Rely on the watchers to clear out the old cache values and rely on the + // backup xds servers having unique resource names + } + /** * Tracks a single subscribed resource. */ private final class ResourceSubscriber { - @Nullable private final ServerInfo serverInfo; - @Nullable private final ControlPlaneClient controlPlaneClient; + @Nullable + private final ImmutableList serverInfos; + @Nullable + private ControlPlaneClient controlPlaneClient; + private final String authority; private final XdsResourceType type; private final String resource; private final Map, Executor> watchers = new HashMap<>(); - @Nullable private T data; + @Nullable + private T data; private boolean absent; // Tracks whether the deletion has been ignored per bootstrap server feature. // See https://github.com/grpc/proposal/blob/master/A53-xds-ignore-resource-deletion.md private boolean resourceDeletionIgnored; - @Nullable private ScheduledHandle respTimer; - @Nullable private ResourceMetadata metadata; - @Nullable private String errorDescription; + @Nullable + private ScheduledHandle respTimer; + @Nullable + private ResourceMetadata metadata; + @Nullable + private String errorDescription; + private boolean cpcFixed = false; ResourceSubscriber(XdsResourceType type, String resource) { syncContext.throwIfNotInThisSynchronizationContext(); this.type = type; this.resource = resource; - this.serverInfo = getServerInfo(resource); - if (serverInfo == null) { + this.authority = getAuthority(resource); + this.serverInfos = getServerInfos(authority); + if (serverInfos == null) { this.errorDescription = "Wrong configuration: xds server does not exist for resource " + resource; this.controlPlaneClient = null; return; } + // Initialize metadata in UNKNOWN state to cover the case when resource subscriber, // is created but not yet requested because the client is in backoff. this.metadata = ResourceMetadata.newResourceMetadataUnknown(); ControlPlaneClient controlPlaneClient = null; try { - controlPlaneClient = getOrCreateControlPlaneClient(serverInfo); - if (controlPlaneClient.isInBackoff()) { + controlPlaneClient = getOrCreateControlPlaneClient(serverInfos); + if (controlPlaneClient == null) { return; } + + if (!controlPlaneClient.isConnected()) { + controlPlaneClient.connect(); // Make sure we are at least trying to connect + } } catch (IllegalArgumentException e) { controlPlaneClient = null; this.errorDescription = "Bad configuration: " + e.getMessage(); @@ -552,6 +784,21 @@ private final class ResourceSubscriber { restartTimer(); } + @Override + public String toString() { + return "ResourceSubscriber{" + + "resource='" + resource + '\'' + + ", controlPlaneClient=" + controlPlaneClient + + ", authority='" + authority + '\'' + + ", type=" + type + + ", watchers=" + watchers.size() + + ", data=" + data + + ", absent=" + absent + + ", resourceDeletionIgnored=" + resourceDeletionIgnored + + ", errorDescription='" + errorDescription + '\'' + + '}'; + } + void addWatcher(ResourceWatcher watcher, Executor watcherExecutor) { checkArgument(!watchers.containsKey(watcher), "watcher %s already registered", watcher); watchers.put(watcher, watcherExecutor); @@ -570,7 +817,7 @@ void addWatcher(ResourceWatcher watcher, Executor watcherExecutor) { }); } - void removeWatcher(ResourceWatcher watcher) { + void removeWatcher(ResourceWatcher watcher) { checkArgument(watchers.containsKey(watcher), "watcher %s not registered", watcher); watchers.remove(watcher); } @@ -579,7 +826,8 @@ void restartTimer() { if (data != null || absent) { // resource already resolved return; } - if (!controlPlaneClient.isReady()) { // When client becomes ready, it triggers a restartTimer + if (controlPlaneClient == null || !controlPlaneClient.isReady()) { + // When client becomes ready, it triggers a restartTimer for all relevant subscribers. return; } @@ -601,6 +849,9 @@ public String toString() { // Initial fetch scheduled or rescheduled, transition metadata state to REQUESTED. metadata = ResourceMetadata.newResourceMetadataRequested(); + if (respTimer != null) { + respTimer.cancel(); + } respTimer = syncContext.schedule( new ResourceNotFound(), INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS, timeService); @@ -624,8 +875,7 @@ void cancelResourceWatch() { message += " for which we previously ignored a deletion"; logLevel = XdsLogLevel.FORCE_INFO; } - logger.log(logLevel, message, type, resource, - serverInfo != null ? serverInfo.target() : "unknown"); + logger.log(logLevel, message, type, resource, getTarget()); } boolean isWatched() { @@ -647,10 +897,11 @@ void onData(ParsedResource parsedResource, String version, long updateTime, ResourceUpdate oldData = this.data; this.data = parsedResource.getResourceUpdate(); absent = false; + cpcFixed = true; if (resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_INFO, "xds server {0}: server returned new version " + "of resource for which we previously ignored a deletion: type {1} name {2}", - serverInfo != null ? serverInfo.target() : "unknown", type, resource); + getTarget(), type, resource); resourceDeletionIgnored = false; } if (!Objects.equals(oldData, data)) { @@ -667,6 +918,16 @@ void onData(ParsedResource parsedResource, String version, long updateTime, } } + private String getTarget() { + return (serverInfos != null && controlPlaneClient != null) + ? controlPlaneClient.getServerInfo().target() + : "unknown"; + } + + private boolean isCpcMutable() { + return !cpcFixed || type.isSharedName(); + } + void onAbsent(@Nullable ProcessingTracker processingTracker) { if (respTimer != null && respTimer.isPending()) { // too early to conclude absence return; @@ -675,12 +936,13 @@ void onAbsent(@Nullable ProcessingTracker processingTracker) { // Ignore deletion of State of the World resources when this feature is on, // and the resource is reusable. boolean ignoreResourceDeletionEnabled = - serverInfo != null && serverInfo.ignoreResourceDeletion(); + serverInfos != null && controlPlaneClient != null + && controlPlaneClient.getServerInfo().ignoreResourceDeletion(); if (ignoreResourceDeletionEnabled && type.isFullStateOfTheWorld() && data != null) { if (!resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_WARNING, "xds server {0}: ignoring deletion for resource type {1} name {2}}", - serverInfo.target(), type, resource); + getTarget(), type, resource); resourceDeletionIgnored = true; } return; @@ -747,4 +1009,151 @@ private void notifyWatcher(ResourceWatcher watcher, T update) { } } + private class ResponseHandler implements XdsResponseHandler { + final ServerInfo serverInfo; + + ResponseHandler(ServerInfo serverInfo) { + this.serverInfo = serverInfo; + } + + @Override + public void handleResourceResponse( + XdsResourceType xdsResourceType, ServerInfo serverInfo, String versionInfo, + List resources, String nonce, ProcessingTracker processingTracker) { + checkNotNull(xdsResourceType, "xdsResourceType"); + syncContext.throwIfNotInThisSynchronizationContext(); + Set toParseResourceNames = + xdsResourceType.shouldRetrieveResourceKeysForArgs() + ? getResourceKeys(xdsResourceType) + : null; + XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, versionInfo, nonce, + bootstrapInfo, securityConfig, toParseResourceNames); + handleResourceUpdate(args, resources, xdsResourceType, processingTracker); + } + + @Override + public void handleStreamClosed(Status status) { + syncContext.throwIfNotInThisSynchronizationContext(); + ControlPlaneClient cpcForThisStream = serverCpClientMap.get(serverInfo); + cleanUpResourceTimers(cpcForThisStream); + + Status error = status.isOk() ? Status.UNAVAILABLE.withDescription(CLOSED_BY_SERVER) : status; + boolean checkForFallback = !status.isOk() && BootstrapperImpl.isEnabledXdsFallback(); + + for (Map> subscriberMap : + resourceSubscribers.values()) { + for (ResourceSubscriber subscriber : subscriberMap.values()) { + if (subscriber.hasResult() + || (subscriber.cpcFixed && !cpcForThisStream.equals(subscriber.controlPlaneClient))) { + continue; + } + if (checkForFallback) { + // try to fallback to lower priority control plane client + if (doFallbackForAuthority(cpcForThisStream, + serverInfo, subscriber.serverInfos, subscriber.authority)) { + return; + } + checkForFallback = false; + } + + subscriber.onError(error, null); + } + } + } + + @Override + public void handleStreamRestarted(ServerInfo serverInfo) { + if (isShutDown()) { + return; + } + + syncContext.throwIfNotInThisSynchronizationContext(); + ControlPlaneClient controlPlaneClient = serverCpClientMap.get(serverInfo); + if (controlPlaneClient == null) { + return; + } + + for (String authority : getAuthoritiesForServerInfo(serverInfo)) { + if (controlPlaneClient.hasSubscribedResources(authority)) { + internalHandleStreamReady(serverInfo, controlPlaneClient, authority); + } + } + } + + private void internalHandleStreamReady( + ServerInfo serverInfo, ControlPlaneClient controlPlaneClient, String authority) { + ControlPlaneClient activeCpClient = activeCpClients.get(authority); + if (activeCpClient == null) { + setCpcForAuthority(authority, controlPlaneClient); + restartMatchingSubscriberTimers(controlPlaneClient, authority); + controlPlaneClient.sendDiscoveryRequests(authority); + return; // Since nothing was active can ignore fallback + } + + if (activeCpClient.isReady() + && compareCpClients(activeCpClient, controlPlaneClient, authority) < 0) { + logger.log(XdsLogLevel.INFO, "Ignoring stream restart for lower priority server {0}", + serverInfo.target()); + return; + } + + if (activeCpClient != controlPlaneClient) { + setCpcForAuthority(authority, controlPlaneClient); + } else { + assignUnassignedResources(authority); + } + + restartMatchingSubscriberTimers(controlPlaneClient, authority); + controlPlaneClient.sendDiscoveryRequests(authority); + + // If fallback isn't enabled, we're done. + if (!BootstrapperImpl.isEnabledXdsFallback()) { + return; + } + + // Shutdown any lower priority control plane clients. + Iterator iterator = serverCpClientMap.values().iterator(); + while (iterator.hasNext()) { + ControlPlaneClient client = iterator.next(); + if (compareCpClients(client, controlPlaneClient, authority) > 0 + && !activeCpClients.containsValue(client)) { + iterator.remove(); + client.shutdown(); // TODO someday add an exponential backoff to mitigate flapping + } + } + } + + private void restartMatchingSubscriberTimers( + ControlPlaneClient controlPlaneClient, String authority) { + // Restart the timers for all the watched resources that are associated with this stream + for (Map> subscriberMap : + resourceSubscribers.values()) { + for (ResourceSubscriber subscriber : subscriberMap.values()) { + if ((subscriber.controlPlaneClient == null) + || (subscriber.controlPlaneClient.equals(controlPlaneClient) + && Objects.equals(subscriber.authority, authority))) { + subscriber.restartTimer(); + } + } + } + } + } + + private int compareCpClients(ControlPlaneClient base, ControlPlaneClient other, + String authority) { + if (base == other || other == null) { + return 0; + } + + ImmutableList serverInfos = getServerInfos(authority); + ServerInfo baseServerInfo = base.getServerInfo(); + ServerInfo otherServerInfo = other.getServerInfo(); + + if (serverInfos == null || !serverInfos.contains(baseServerInfo) + || !serverInfos.contains(otherServerInfo)) { + return -100; // At least one of them isn't serving this authority + } + + return serverInfos.indexOf(baseServerInfo) - serverInfos.indexOf(otherServerInfo); + } } diff --git a/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java b/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java index 8c3d31604e4..7f2b16b7521 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java @@ -79,6 +79,10 @@ protected String extractResourceName(Message unpackedResource) { // the resources that need an update. protected abstract boolean isFullStateOfTheWorld(); + public boolean isSharedName() { + return false; + } + @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10847") public static class Args { final ServerInfo serverInfo; diff --git a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java index ec700bd6dc9..6e8b2f7462a 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java @@ -36,6 +36,11 @@ StreamingCall createStreamingCall( MethodDescriptor.Marshaller respMarshaller); void shutdown(); + + /** + * Returns true if this transport is currently connected to the xDS server. + */ + boolean isConnected(); } /** diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index 1ddf9620434..babf3d8358f 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -55,6 +55,7 @@ import io.grpc.InsecureServerCredentials; import io.grpc.NameResolverRegistry; import io.grpc.Server; +import java.io.IOException; import java.util.Collections; import java.util.Map; import java.util.UUID; @@ -86,9 +87,15 @@ public class ControlPlaneRule extends TestWatcher { private XdsTestControlPlaneService controlPlaneService; private XdsTestLoadReportingService loadReportingService; private XdsNameResolverProvider nameResolverProvider; + private final int port; public ControlPlaneRule() { + this(0); + } + + public ControlPlaneRule(int port) { serverHostName = "test-server"; + this.port = port; } public ControlPlaneRule setServerHostName(String serverHostName) { @@ -115,11 +122,7 @@ public Server getServer() { try { controlPlaneService = new XdsTestControlPlaneService(); loadReportingService = new XdsTestLoadReportingService(); - server = Grpc.newServerBuilderForPort(0, InsecureServerCredentials.create()) - .addService(controlPlaneService) - .addService(loadReportingService) - .build() - .start(); + createAndStartTdServer(); } catch (Exception e) { throw new AssertionError("unable to start the control plane server", e); } @@ -144,6 +147,38 @@ public Server getServer() { NameResolverRegistry.getDefaultRegistry().deregister(nameResolverProvider); } + /** + * Will shutdown existing server if needed. + * Then creates a new server in the same way as {@link #starting(Description)} and starts it. + */ + public void restartTdServer() { + + if (getServer() != null && !getServer().isShutdown()) { + getServer().shutdownNow(); + try { + if (!getServer().awaitTermination(5, TimeUnit.SECONDS)) { + logger.log(Level.SEVERE, "Timed out waiting for server shutdown"); + } + } catch (InterruptedException e) { + throw new AssertionError("unable to shut down control plane server", e); + } + } + + try { + createAndStartTdServer(); + } catch (Exception e) { + throw new AssertionError("unable to restart the control plane server", e); + } + } + + private void createAndStartTdServer() throws IOException { + server = Grpc.newServerBuilderForPort(port, InsecureServerCredentials.create()) + .addService(controlPlaneService) + .addService(loadReportingService) + .build() + .start(); + } + /** * For test purpose, use boostrapOverride to programmatically provide bootstrap info. */ @@ -173,44 +208,67 @@ void setLdsConfig(Listener serverListener, Listener clientListener) { } void setRdsConfig(RouteConfiguration routeConfiguration) { - getService().setXdsConfig(ADS_TYPE_URL_RDS, ImmutableMap.of(RDS_NAME, routeConfiguration)); + setRdsConfig(RDS_NAME, routeConfiguration); + } + + public void setRdsConfig(String rdsName, RouteConfiguration routeConfiguration) { + getService().setXdsConfig(ADS_TYPE_URL_RDS, ImmutableMap.of(rdsName, routeConfiguration)); } void setCdsConfig(Cluster cluster) { + setCdsConfig(CLUSTER_NAME, cluster); + } + + void setCdsConfig(String clusterName, Cluster cluster) { getService().setXdsConfig(ADS_TYPE_URL_CDS, - ImmutableMap.of(CLUSTER_NAME, cluster)); + ImmutableMap.of(clusterName, cluster)); } void setEdsConfig(ClusterLoadAssignment clusterLoadAssignment) { + setEdsConfig(EDS_NAME, clusterLoadAssignment); + } + + void setEdsConfig(String edsName, ClusterLoadAssignment clusterLoadAssignment) { getService().setXdsConfig(ADS_TYPE_URL_EDS, - ImmutableMap.of(EDS_NAME, clusterLoadAssignment)); + ImmutableMap.of(edsName, clusterLoadAssignment)); } /** * Builds a new default RDS configuration. */ static RouteConfiguration buildRouteConfiguration(String authority) { - io.envoyproxy.envoy.config.route.v3.VirtualHost virtualHost = VirtualHost.newBuilder() + return buildRouteConfiguration(authority, RDS_NAME, CLUSTER_NAME); + } + + static RouteConfiguration buildRouteConfiguration(String authority, String rdsName, + String clusterName) { + VirtualHost.Builder vhBuilder = VirtualHost.newBuilder() + .setName(rdsName) .addDomains(authority) .addRoutes( Route.newBuilder() .setMatch( RouteMatch.newBuilder().setPrefix("/").build()) .setRoute( - RouteAction.newBuilder().setCluster(CLUSTER_NAME).build()).build()).build(); - return RouteConfiguration.newBuilder().setName(RDS_NAME).addVirtualHosts(virtualHost).build(); + RouteAction.newBuilder().setCluster(clusterName).build()).build()); + VirtualHost virtualHost = vhBuilder.build(); + return RouteConfiguration.newBuilder().setName(rdsName).addVirtualHosts(virtualHost).build(); } /** * Builds a new default CDS configuration. */ static Cluster buildCluster() { + return buildCluster(CLUSTER_NAME, EDS_NAME); + } + + static Cluster buildCluster(String clusterName, String edsName) { return Cluster.newBuilder() - .setName(CLUSTER_NAME) + .setName(clusterName) .setType(Cluster.DiscoveryType.EDS) .setEdsClusterConfig( Cluster.EdsClusterConfig.newBuilder() - .setServiceName(EDS_NAME) + .setServiceName(edsName) .setEdsConfig( ConfigSource.newBuilder() .setAds(AggregatedConfigSource.newBuilder().build()) @@ -224,6 +282,11 @@ static Cluster buildCluster() { * Builds a new default EDS configuration. */ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int port) { + return buildClusterLoadAssignment(hostName, port, EDS_NAME); + } + + static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int port, + String edsName) { Address address = Address.newBuilder() .setSocketAddress( SocketAddress.newBuilder().setAddress(hostName).setPortValue(port).build()).build(); @@ -237,7 +300,7 @@ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int por .setHealthStatus(HealthStatus.HEALTHY) .build()).build(); return ClusterLoadAssignment.newBuilder() - .setClusterName(EDS_NAME) + .setClusterName(edsName) .addEndpoints(endpoints) .build(); } @@ -246,8 +309,17 @@ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int por * Builds a new client listener. */ static Listener buildClientListener(String name) { + return buildClientListener(name, "terminal-filter"); + } + + + static Listener buildClientListener(String name, String identifier) { + return buildClientListener(name, identifier, RDS_NAME); + } + + static Listener buildClientListener(String name, String identifier, String rdsName) { HttpFilter httpFilter = HttpFilter.newBuilder() - .setName("terminal-filter") + .setName(identifier) .setTypedConfig(Any.pack(Router.newBuilder().build())) .setIsOptional(true) .build(); @@ -256,7 +328,7 @@ static Listener buildClientListener(String name) { .HttpConnectionManager.newBuilder() .setRds( Rds.newBuilder() - .setRouteConfigName(RDS_NAME) + .setRouteConfigName(rdsName) .setConfigSource( ConfigSource.newBuilder() .setAds(AggregatedConfigSource.getDefaultInstance()))) @@ -312,4 +384,5 @@ static Listener buildServerListener() { .addFilterChains(filterChain) .build(); } + } diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 39b9c6ad942..11c8c2608af 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -53,9 +53,9 @@ import io.grpc.xds.client.XdsClient; import io.grpc.xds.client.XdsClient.ResourceMetadata; import io.grpc.xds.client.XdsClient.ResourceMetadata.ResourceMetadataStatus; -import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.client.XdsResourceType; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -106,7 +106,7 @@ public void setUp() { // because true->false return mutation prevents fetchClientStatus from completing the request. csdsStub = ClientStatusDiscoveryServiceGrpc .newBlockingStub(grpcServerRule.getChannel()) - .withDeadline(Deadline.after(3, TimeUnit.SECONDS)); + .withDeadline(Deadline.after(30, TimeUnit.SECONDS)); csdsAsyncStub = ClientStatusDiscoveryServiceGrpc.newStub(grpcServerRule.getChannel()); } @@ -199,13 +199,13 @@ public void streamClientStatus_happyPath() { @Override @Nullable - public ObjectPool get() { + public ObjectPool get(String target) { // xDS client not ready on the first call, then becomes ready. if (!calledOnce) { calledOnce = true; return null; } else { - return super.get(); + return super.get(target); } } }); @@ -351,7 +351,7 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { ); } }; - ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient); + ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient, "fake"); verifyClientConfigNode(clientConfig); @@ -391,7 +391,8 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { @Test public void getClientConfigForXdsClient_noSubscribedResources() throws InterruptedException { - ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES); + ClientConfig clientConfig = + CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES, "fake"); verifyClientConfigNode(clientConfig); verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); } @@ -457,14 +458,28 @@ public Collection getSubscribedResources(ServerInfo serverInfo, return null; } + @Nullable + @Override + public Collection getSubscribedResources( + ServerInfo serverInfo, XdsResourceType type, String authority) { + return null; + } + @Override public Map> getSubscribedResourceTypesWithTypeUrl() { return ImmutableMap.of(); } + + @Override + public void assignResourcesToOwner(XdsResourceType type, Collection resources, + Object owner) { + // No-op. + } } private static class FakeXdsClientPoolFactory implements XdsClientPoolFactory { @Nullable private final XdsClient xdsClient; + private static final List TARGETS = Collections.singletonList(""); private FakeXdsClientPoolFactory(@Nullable XdsClient xdsClient) { this.xdsClient = xdsClient; @@ -472,7 +487,7 @@ private FakeXdsClientPoolFactory(@Nullable XdsClient xdsClient) { @Override @Nullable - public ObjectPool get(String target) { + public ObjectPool get(String notUsedTarget) { return new ObjectPool() { @Override public XdsClient getObject() { @@ -486,14 +501,9 @@ public XdsClient returnObject(Object object) { }; } - @Override - public ObjectPool getOrCreate(String target) throws XdsInitializationException { - return null; - } - @Override public List getTargets() { - return null; + return TARGETS; } @Override @@ -502,7 +512,7 @@ public void setBootstrapOverride(Map bootstrap) { } @Override - public ObjectPool getOrCreate() { + public ObjectPool getOrCreate(String target) { throw new UnsupportedOperationException("Should not be called"); } } diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index 30ea76b54f2..eaa8aaa9e47 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -65,6 +65,7 @@ public class GrpcBootstrapperImplTest { @Before public void setUp() { saveEnvironment(); + System.setProperty(BootstrapperImpl.GRPC_EXPERIMENTAL_XDS_FALLBACK, "true"); bootstrapper.bootstrapPathFromEnvVar = BOOTSTRAP_FILE_PATH; } diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java index 6b04edcb9b8..d41630cdb4a 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplTestBase.java @@ -340,8 +340,7 @@ public XdsTransport create(ServerInfo serverInfo) { } }; - xdsServerInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, - ignoreResourceDeletion()); + xdsServerInfo = ServerInfo.create(SERVER_URI, CHANNEL_CREDENTIALS, ignoreResourceDeletion()); BootstrapInfo bootstrapInfo = Bootstrapper.BootstrapInfo.builder() .servers(Collections.singletonList(xdsServerInfo)) @@ -2974,6 +2973,7 @@ public void flowControlAbsent() throws Exception { anotherWatcher, fakeWatchClock.getScheduledExecutorService()); verifyResourceMetadataRequested(CDS, CDS_RESOURCE); verifyResourceMetadataRequested(CDS, anotherCdsResource); + DiscoveryRpcCall call = resourceDiscoveryCalls.poll(); call.verifyRequest(CDS, Arrays.asList(CDS_RESOURCE, anotherCdsResource), "", "", NODE); assertThat(fakeWatchClock.runDueTasks()).isEqualTo(2); diff --git a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java index 2b2ce5cbd72..40a9bff514f 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java +++ b/xds/src/test/java/io/grpc/xds/GrpcXdsClientImplV3Test.java @@ -17,6 +17,7 @@ package io.grpc.xds; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.AdditionalAnswers.delegatesTo; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -144,7 +145,8 @@ public StreamObserver streamAggregatedResources( assertThat(adsEnded.get()).isTrue(); // ensure previous call was ended adsEnded.set(false); @SuppressWarnings("unchecked") - StreamObserver requestObserver = mock(StreamObserver.class); + StreamObserver requestObserver = + mock(StreamObserver.class, delegatesTo(new MockStreamObserver())); DiscoveryRpcCall call = new DiscoveryRpcCallV3(requestObserver, responseObserver); resourceDiscoveryCalls.offer(call); Context.current().addListener( @@ -874,6 +876,19 @@ public boolean matches(DiscoveryRequest argument) { } return node.equals(argument.getNode()); } + + @Override + public String toString() { + return "DiscoveryRequestMatcher{" + + "node=" + node + + ", versionInfo='" + versionInfo + '\'' + + ", typeUrl='" + typeUrl + '\'' + + ", resources=" + resources + + ", responseNonce='" + responseNonce + '\'' + + ", errorCode=" + errorCode + + ", errorMessages=" + errorMessages + + '}'; + } } /** @@ -901,4 +916,23 @@ public boolean matches(LoadStatsRequest argument) { return actual.equals(expected); } } + + private static class MockStreamObserver implements StreamObserver { + private final List requests = new ArrayList<>(); + + @Override + public void onNext(DiscoveryRequest value) { + requests.add(value); + } + + @Override + public void onError(Throwable t) { + // Ignore + } + + @Override + public void onCompleted() { + // Ignore + } + } } diff --git a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java index 0687b51aea6..ee164938b2d 100644 --- a/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java +++ b/xds/src/test/java/io/grpc/xds/SharedXdsClientPoolProviderTest.java @@ -51,6 +51,7 @@ public class SharedXdsClientPoolProviderTest { @Rule public final ExpectedException thrown = ExpectedException.none(); private final Node node = Node.newBuilder().setId("SharedXdsClientPoolProviderTest").build(); + private static final String DUMMY_TARGET = "dummy"; @Mock private GrpcBootstrapperImpl bootstrapper; @@ -63,8 +64,8 @@ public void noServer() throws XdsInitializationException { SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(bootstrapper); thrown.expect(XdsInitializationException.class); thrown.expectMessage("No xDS server provided"); - provider.getOrCreate(); - assertThat(provider.get()).isNull(); + provider.getOrCreate(DUMMY_TARGET); + assertThat(provider.get(DUMMY_TARGET)).isNull(); } @Test @@ -75,12 +76,12 @@ public void sharedXdsClientObjectPool() throws XdsInitializationException { when(bootstrapper.bootstrap()).thenReturn(bootstrapInfo); SharedXdsClientPoolProvider provider = new SharedXdsClientPoolProvider(bootstrapper); - assertThat(provider.get()).isNull(); - ObjectPool xdsClientPool = provider.getOrCreate(); + assertThat(provider.get(DUMMY_TARGET)).isNull(); + ObjectPool xdsClientPool = provider.getOrCreate(DUMMY_TARGET); verify(bootstrapper).bootstrap(); - assertThat(provider.getOrCreate()).isSameInstanceAs(xdsClientPool); - assertThat(provider.get()).isNotNull(); - assertThat(provider.get()).isSameInstanceAs(xdsClientPool); + assertThat(provider.getOrCreate(DUMMY_TARGET)).isSameInstanceAs(xdsClientPool); + assertThat(provider.get(DUMMY_TARGET)).isNotNull(); + assertThat(provider.get(DUMMY_TARGET)).isSameInstanceAs(xdsClientPool); verifyNoMoreInteractions(bootstrapper); } @@ -90,7 +91,7 @@ public void refCountedXdsClientObjectPool_delayedCreation() { BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); RefCountedXdsClientObjectPool xdsClientPool = - new RefCountedXdsClientObjectPool(bootstrapInfo); + new RefCountedXdsClientObjectPool(bootstrapInfo, DUMMY_TARGET); assertThat(xdsClientPool.getXdsClientForTest()).isNull(); XdsClient xdsClient = xdsClientPool.getObject(); assertThat(xdsClientPool.getXdsClientForTest()).isNotNull(); @@ -103,7 +104,7 @@ public void refCountedXdsClientObjectPool_refCounted() { BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); RefCountedXdsClientObjectPool xdsClientPool = - new RefCountedXdsClientObjectPool(bootstrapInfo); + new RefCountedXdsClientObjectPool(bootstrapInfo, DUMMY_TARGET); // getObject once XdsClient xdsClient = xdsClientPool.getObject(); assertThat(xdsClient).isNotNull(); @@ -123,7 +124,7 @@ public void refCountedXdsClientObjectPool_getObjectCreatesNewInstanceIfAlreadySh BootstrapInfo bootstrapInfo = BootstrapInfo.builder().servers(Collections.singletonList(server)).node(node).build(); RefCountedXdsClientObjectPool xdsClientPool = - new RefCountedXdsClientObjectPool(bootstrapInfo); + new RefCountedXdsClientObjectPool(bootstrapInfo, DUMMY_TARGET); XdsClient xdsClient1 = xdsClientPool.getObject(); assertThat(xdsClientPool.returnObject(xdsClient1)).isNull(); assertThat(xdsClient1.isShutDown()).isTrue(); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java new file mode 100644 index 00000000000..798600a6851 --- /dev/null +++ b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java @@ -0,0 +1,377 @@ +/* + * Copyright 2024 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc.xds; + +import static junit.framework.TestCase.assertFalse; +import static org.mockito.AdditionalAnswers.delegatesTo; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.grpc.Status; +import io.grpc.internal.ObjectPool; +import io.grpc.xds.client.BootstrapperImpl; +import io.grpc.xds.client.XdsClient; +import io.grpc.xds.client.XdsClientImpl; +import io.grpc.xds.client.XdsInitializationException; +import java.net.InetSocketAddress; +import java.util.Collections; +import java.util.Map; +import java.util.UUID; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.InOrder; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +@RunWith(JUnit4.class) +public class XdsClientFallbackTest { + private static final Logger log = Logger.getLogger(XdsClientFallbackTest.class.getName()); + + private static final String MAIN_SERVER = "main-server"; + private static final String FALLBACK_SERVER = "fallback-server"; + private static final String DUMMY_TARGET = "TEST_TARGET"; + private static final String RDS_NAME = "route-config.googleapis.com"; + private static final String FALLBACK_RDS_NAME = "fallback-" + RDS_NAME; + private static final String CLUSTER_NAME = "cluster0"; + private static final String FALLBACK_CLUSTER_NAME = "fallback-" + CLUSTER_NAME; + private static final String EDS_NAME = "eds-service-0"; + private static final String FALLBACK_EDS_NAME = "fallback-" + EDS_NAME; + private static final HttpConnectionManager MAIN_HTTP_CONNECTION_MANAGER = + HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of( + new Filter.NamedFilterConfig(MAIN_SERVER, RouterFilter.ROUTER_CONFIG))); + private static final HttpConnectionManager FALLBACK_HTTP_CONNECTION_MANAGER = + HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of( + new Filter.NamedFilterConfig(FALLBACK_SERVER, RouterFilter.ROUTER_CONFIG))); + private ObjectPool xdsClientPool; + private XdsClient xdsClient; + + private XdsClient.ResourceWatcher raalLdsWatcher = + new XdsClient.ResourceWatcher() { + + @Override + public void onChanged(XdsListenerResource.LdsUpdate update) { + log.log(Level.FINE, "LDS update: " + update); + } + + @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); + } + }; + + @SuppressWarnings("unchecked") + private XdsClient.ResourceWatcher ldsWatcher = + mock(XdsClient.ResourceWatcher.class, delegatesTo(raalLdsWatcher)); + @Mock + private XdsClient.ResourceWatcher ldsWatcher2; + + @Mock + private XdsClient.ResourceWatcher rdsWatcher; + @Mock + private XdsClient.ResourceWatcher rdsWatcher2; + @Mock + private XdsClient.ResourceWatcher rdsWatcher3; + + private XdsClient.ResourceWatcher raalCdsWatcher = + new XdsClient.ResourceWatcher() { + + @Override + public void onChanged(XdsClusterResource.CdsUpdate update) { + log.log(Level.FINE, "CDS update: " + update); + } + + @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); + } + }; + + @SuppressWarnings("unchecked") + private XdsClient.ResourceWatcher cdsWatcher = + mock(XdsClient.ResourceWatcher.class, delegatesTo(raalCdsWatcher)); + @Mock + private XdsClient.ResourceWatcher cdsWatcher2; + + @Rule(order = 0) + public ControlPlaneRule mainTdServer = + new ControlPlaneRule(8090).setServerHostName(MAIN_SERVER); + + @Rule(order = 1) + public ControlPlaneRule fallbackServer = + new ControlPlaneRule(8095).setServerHostName(MAIN_SERVER); + + @Rule public final MockitoRule mocks = MockitoJUnit.rule(); + + @Before + public void setUp() throws XdsInitializationException { + System.setProperty(BootstrapperImpl.GRPC_EXPERIMENTAL_XDS_FALLBACK, "true"); + setAdsConfig(mainTdServer, MAIN_SERVER); + setAdsConfig(fallbackServer, FALLBACK_SERVER); + + SharedXdsClientPoolProvider clientPoolProvider = new SharedXdsClientPoolProvider(); + clientPoolProvider.setBootstrapOverride(defaultBootstrapOverride()); + xdsClientPool = clientPoolProvider.getOrCreate(DUMMY_TARGET); + } + + @After + public void cleanUp() { + xdsClientPool.returnObject(xdsClient); + } + + private static void setAdsConfig(ControlPlaneRule controlPlane, String serverName) { + InetSocketAddress edsInetSocketAddress = + (InetSocketAddress) controlPlane.getServer().getListenSockets().get(0); + boolean isMainServer = serverName.equals(MAIN_SERVER); + String rdsName = isMainServer + ? RDS_NAME + : FALLBACK_RDS_NAME; + String clusterName = isMainServer ? CLUSTER_NAME : FALLBACK_CLUSTER_NAME; + String edsName = isMainServer ? EDS_NAME : FALLBACK_EDS_NAME; + + controlPlane.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener(MAIN_SERVER, serverName)); + + controlPlane.setRdsConfig(rdsName, + ControlPlaneRule.buildRouteConfiguration(MAIN_SERVER, rdsName, clusterName)); + controlPlane.setCdsConfig(clusterName, ControlPlaneRule.buildCluster(clusterName, edsName)); + + controlPlane.setEdsConfig(edsName, + ControlPlaneRule.buildClusterLoadAssignment(edsInetSocketAddress.getHostName(), + edsInetSocketAddress.getPort(), edsName)); + log.log(Level.FINE, + String.format("Set ADS config for %s with address %s", serverName, edsInetSocketAddress)); + } + + private void restartServer(TdServerType type) { + switch (type) { + case MAIN: + mainTdServer.restartTdServer(); + setAdsConfig(mainTdServer, MAIN_SERVER); + break; + case FALLBACK: + fallbackServer.restartTdServer(); + setAdsConfig(fallbackServer, FALLBACK_SERVER); + break; + default: + throw new IllegalArgumentException("Unknown server type: " + type); + } + } + + // This is basically a control test to make sure everything is set up correctly. + @Test + public void everything_okay() { + restartServer(TdServerType.MAIN); + restartServer(TdServerType.FALLBACK); + xdsClient = xdsClientPool.getObject(); + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); + verify(ldsWatcher, timeout(5000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener( + MAIN_HTTP_CONNECTION_MANAGER)); + + xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); + verify(rdsWatcher, timeout(5000)).onChanged(any()); + } + + @Test + public void mainServerDown_fallbackServerUp() { + mainTdServer.getServer().shutdownNow(); + restartServer(TdServerType.FALLBACK); + xdsClient = xdsClientPool.getObject(); + log.log(Level.FINE, "Fallback port = " + fallbackServer.getServer().getPort()); + + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); + + verify(ldsWatcher, timeout(5000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener( + FALLBACK_HTTP_CONNECTION_MANAGER)); + } + + @Test + public void both_down_restart_main() { + mainTdServer.getServer().shutdownNow(); + fallbackServer.getServer().shutdownNow(); + xdsClient = xdsClientPool.getObject(); + + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); + verify(ldsWatcher, timeout(5000).times(0)).onChanged(any()); + + restartServer(TdServerType.MAIN); + + xdsClient.watchXdsResource( + XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); + + verify(ldsWatcher, timeout(300000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + mainTdServer.getServer().shutdownNow(); + } + + @Test + public void mainDown_fallbackUp_restart_main() { + mainTdServer.getServer().shutdownNow(); + fallbackServer.restartTdServer(); + xdsClient = xdsClientPool.getObject(); + 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)); + xdsClient.watchXdsResource(XdsClusterResource.getInstance(), FALLBACK_CLUSTER_NAME, cdsWatcher); + inOrder.verify(cdsWatcher, timeout(5000)).onChanged(any()); + + restartServer(TdServerType.MAIN); + + verify(ldsWatcher, timeout(5000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + + xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); + inOrder.verify(rdsWatcher, timeout(5000)).onChanged(any()); + + xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); + inOrder.verify(cdsWatcher2, timeout(5000)).onChanged(any()); + + // verify that connection to fallback server is closed + assertFalse("Should have disconnected from fallback server", + ((XdsClientImpl) xdsClient).isSubscriberCpcConnected(XdsClusterResource.getInstance(), + FALLBACK_CLUSTER_NAME)); + + } + + // This test takes a long time because of the 16 sec timeout for non-existent resource + @Test + public void connect_then_mainServerDown_fallbackServerUp() { + restartServer(TdServerType.MAIN); + restartServer(TdServerType.FALLBACK); + xdsClient = xdsClientPool.getObject(); + + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); + + verify(ldsWatcher, timeout(5000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + + xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); + verify(rdsWatcher, timeout(5000)).onChanged(any()); + + mainTdServer.getServer().shutdownNow(); + + // Shouldn't do fallback since all watchers are loaded + verify(ldsWatcher, timeout(5000).times(0)).onChanged( + 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)); + // Make sure that rdsWatcher wasn't called again + verify(rdsWatcher, times(1)).onChanged(any()); + verify(rdsWatcher2, timeout(5000)).onChanged(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(16000)).onChanged(any()); + + xdsClient.watchXdsResource( + XdsRouteConfigureResource.getInstance(), FALLBACK_RDS_NAME, rdsWatcher3); + verify(rdsWatcher3, timeout(5000)).onChanged(any()); + + // Test that resource defined in main but not fallback is handled correctly + xdsClient.watchXdsResource( + XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); + verify(cdsWatcher2, timeout(16000)).onResourceDoesNotExist(eq(CLUSTER_NAME)); + } + + @Test + public void connect_then_mainServerRestart_fallbackServerdown() { + restartServer(TdServerType.MAIN); + xdsClient = xdsClientPool.getObject(); + + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); + + verify(ldsWatcher, timeout(5000)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + + mainTdServer.getServer().shutdownNow(); + fallbackServer.getServer().shutdownNow(); + + xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher); + restartServer(TdServerType.MAIN); + verify(cdsWatcher, timeout(5000)).onChanged(any()); + verify(ldsWatcher, timeout(5000).times(2)).onChanged( + XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); + } + + private Map defaultBootstrapOverride() { + return ImmutableMap.of( + "node", ImmutableMap.of( + "id", UUID.randomUUID().toString(), + "cluster", CLUSTER_NAME), + "xds_servers", ImmutableList.of( + ImmutableMap.of( + "server_uri", "localhost:" + mainTdServer.getServer().getPort(), + "channel_creds", Collections.singletonList( + ImmutableMap.of("type", "insecure") + ), + "server_features", Collections.singletonList("xds_v3") + ), + ImmutableMap.of( + "server_uri", "localhost:" + fallbackServer.getServer().getPort(), + "channel_creds", Collections.singletonList( + ImmutableMap.of("type", "insecure") + ), + "server_features", Collections.singletonList("xds_v3") + ) + ), + "fallback-policy", "fallback" + ); + } + + private enum TdServerType { + MAIN, + FALLBACK + } +} diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 149c1d6170d..0b8e89de721 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -72,12 +72,13 @@ public class XdsClientFederationTest { private ObjectPool xdsClientPool; private XdsClient xdsClient; + private static final String DUMMY_TARGET = "dummy"; @Before public void setUp() throws XdsInitializationException { SharedXdsClientPoolProvider clientPoolProvider = new SharedXdsClientPoolProvider(); clientPoolProvider.setBootstrapOverride(defaultBootstrapOverride()); - xdsClientPool = clientPoolProvider.getOrCreate(); + xdsClientPool = clientPoolProvider.getOrCreate(DUMMY_TARGET); xdsClient = xdsClientPool.getObject(); } diff --git a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java index 28871850e72..24c2a43b83a 100644 --- a/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java @@ -95,11 +95,15 @@ import io.grpc.xds.client.XdsInitializationException; import io.grpc.xds.client.XdsResourceType; import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; @@ -166,15 +170,22 @@ public ConfigOrError parseServiceConfig(Map rawServiceConfig) { private XdsNameResolver resolver; private TestCall testCall; private boolean originalEnableTimeout; + private URI targetUri; @Before public void setUp() { + try { + targetUri = new URI(AUTHORITY); + } catch (URISyntaxException e) { + targetUri = null; + } + originalEnableTimeout = XdsNameResolver.enableTimeout; XdsNameResolver.enableTimeout = true; FilterRegistry filterRegistry = FilterRegistry.newRegistry().register( new FaultFilter(mockRandom, new AtomicLong()), RouterFilter.INSTANCE); - resolver = new XdsNameResolver(null, AUTHORITY, null, + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, filterRegistry, null); } @@ -199,16 +210,22 @@ public void setBootstrapOverride(Map bootstrap) { @Override @Nullable - public ObjectPool get() { + public ObjectPool get(String target) { throw new UnsupportedOperationException("Should not be called"); } @Override - public ObjectPool getOrCreate() throws XdsInitializationException { + public ObjectPool getOrCreate(String target) throws XdsInitializationException { throw new XdsInitializationException("Fail to read bootstrap file"); } + + @Override + public List getTargets() { + return null; + } }; - resolver = new XdsNameResolver(null, AUTHORITY, null, + + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -221,7 +238,7 @@ public ObjectPool getOrCreate() throws XdsInitializationException { @Test public void resolving_withTargetAuthorityNotFound() { - resolver = new XdsNameResolver( + resolver = new XdsNameResolver(targetUri, "notfound.google.com", AUTHORITY, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -243,7 +260,7 @@ public void resolving_noTargetAuthority_templateWithoutXdstp() { String serviceAuthority = "[::FFFF:129.144.52.38]:80"; expectedLdsResourceName = "[::FFFF:129.144.52.38]:80/id=1"; resolver = new XdsNameResolver( - null, serviceAuthority, null, serviceConfigParser, syncContext, + targetUri, null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -264,7 +281,7 @@ public void resolving_noTargetAuthority_templateWithXdstp() { "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/" + "%5B::FFFF:129.144.52.38%5D:80?id=1"; resolver = new XdsNameResolver( - null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler, + targetUri, null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); verify(mockListener, never()).onError(any(Status.class)); @@ -284,7 +301,7 @@ public void resolving_noTargetAuthority_xdstpWithMultipleSlashes() { "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/" + "path/to/service?id=1"; resolver = new XdsNameResolver( - null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler, + targetUri, null, serviceAuthority, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); @@ -311,7 +328,7 @@ public void resolving_targetAuthorityInAuthoritiesMap() { .build(); expectedLdsResourceName = "xdstp://xds.authority.com/envoy.config.listener.v3.Listener/" + "%5B::FFFF:129.144.52.38%5D:80?bar=2&foo=1"; // query param canonified - resolver = new XdsNameResolver( + resolver = new XdsNameResolver(targetUri, "xds.authority.com", serviceAuthority, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -343,7 +360,7 @@ public void resolving_ldsResourceUpdateRdsName() { .clientDefaultListenerResourceNameTemplate("test-%s") .node(Node.newBuilder().build()) .build(); - resolver = new XdsNameResolver(null, AUTHORITY, null, + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); // use different ldsResourceName and service authority. The virtualhost lookup should use @@ -524,7 +541,7 @@ public void resolving_matchingVirtualHostNotFound_matchingOverrideAuthority() { Collections.singletonList(route), ImmutableMap.of()); - resolver = new XdsNameResolver(null, AUTHORITY, "random", + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, "random", serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -547,7 +564,7 @@ public void resolving_matchingVirtualHostNotFound_notMatchingOverrideAuthority() Collections.singletonList(route), ImmutableMap.of()); - resolver = new XdsNameResolver(null, AUTHORITY, "random", + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, "random", serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -558,7 +575,7 @@ public void resolving_matchingVirtualHostNotFound_notMatchingOverrideAuthority() @Test public void resolving_matchingVirtualHostNotFoundForOverrideAuthority() { - resolver = new XdsNameResolver(null, AUTHORITY, AUTHORITY, + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, AUTHORITY, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -643,8 +660,8 @@ public void resolved_fallbackToHttpMaxStreamDurationAsTimeout() { public void retryPolicyInPerMethodConfigGeneratedByResolverIsValid() { ServiceConfigParser realParser = new ScParser( true, 5, 5, new AutoConfiguredLoadBalancerFactory("pick-first")); - resolver = new XdsNameResolver(null, AUTHORITY, null, realParser, syncContext, scheduler, - xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, null, realParser, syncContext, + scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient(); RetryPolicy retryPolicy = RetryPolicy.create( @@ -847,7 +864,7 @@ public void resolved_rpcHashingByChannelId() { resolver.shutdown(); reset(mockListener); when(mockRandom.nextLong()).thenReturn(123L); - resolver = new XdsNameResolver(null, AUTHORITY, null, serviceConfigParser, + resolver = new XdsNameResolver(targetUri, null, AUTHORITY, null, serviceConfigParser, syncContext, scheduler, xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null); resolver.start(mockListener); @@ -1896,17 +1913,20 @@ private PickSubchannelArgs newPickSubchannelArgs( } private final class FakeXdsClientPoolFactory implements XdsClientPoolFactory { + Set targets = new HashSet<>(); + @Override public void setBootstrapOverride(Map bootstrap) {} @Override @Nullable - public ObjectPool get() { + public ObjectPool get(String target) { throw new UnsupportedOperationException("Should not be called"); } @Override - public ObjectPool getOrCreate() throws XdsInitializationException { + public ObjectPool getOrCreate(String target) throws XdsInitializationException { + targets.add(target); return new ObjectPool() { @Override public XdsClient getObject() { @@ -1919,6 +1939,16 @@ public XdsClient returnObject(Object object) { } }; } + + @Override + public List getTargets() { + if (targets.isEmpty()) { + List targetList = new ArrayList<>(); + targetList.add(targetUri.toString()); + return targetList; + } + return new ArrayList<>(targets); + } } private class FakeXdsClient extends XdsClient { diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index 5d59e97335e..791318c5355 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -146,12 +146,12 @@ public void setBootstrapOverride(Map bootstrap) { @Override @Nullable - public ObjectPool get() { + public ObjectPool get(String target) { throw new UnsupportedOperationException("Should not be called"); } @Override - public ObjectPool getOrCreate() throws XdsInitializationException { + public ObjectPool getOrCreate(String target) throws XdsInitializationException { return new ObjectPool() { @Override public XdsClient getObject() { @@ -165,6 +165,11 @@ public XdsClient returnObject(Object object) { } }; } + + @Override + public List getTargets() { + return Collections.singletonList("fake-target"); + } } static final class FakeXdsClient extends XdsClient { diff --git a/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java b/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java index c51327dc84d..cc12e3863ba 100644 --- a/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java +++ b/xds/src/test/java/io/grpc/xds/XdsTestControlPlaneService.java @@ -135,12 +135,15 @@ public void run() { new Object[]{value.getResourceNamesList(), value.getErrorDetail()}); return; } + String resourceType = value.getTypeUrl(); - if (!value.getResponseNonce().isEmpty() - && !String.valueOf(xdsNonces.get(resourceType)).equals(value.getResponseNonce())) { + if (!value.getResponseNonce().isEmpty() && xdsNonces.containsKey(resourceType) + && !String.valueOf(xdsNonces.get(resourceType).get(responseObserver)) + .equals(value.getResponseNonce())) { logger.log(Level.FINE, "Resource nonce does not match, ignore."); return; } + Set requestedResourceNames = new HashSet<>(value.getResourceNamesList()); if (subscribers.get(resourceType).containsKey(responseObserver) && subscribers.get(resourceType).get(responseObserver) @@ -149,9 +152,11 @@ public void run() { value.getResourceNamesList()); return; } + if (!xdsNonces.get(resourceType).containsKey(responseObserver)) { xdsNonces.get(resourceType).put(responseObserver, new AtomicInteger(0)); } + DiscoveryResponse response = generateResponse(resourceType, String.valueOf(xdsVersions.get(resourceType)), String.valueOf(xdsNonces.get(resourceType).get(responseObserver)), From 05fab7d41b4ed5c02e9ab51c728270ca255cd92f Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 14:00:49 -0700 Subject: [PATCH 03/10] Revert fallback specific changes --- .../java/io/grpc/xds/XdsListenerResource.java | 5 - .../io/grpc/xds/client/BootstrapperImpl.java | 16 - .../grpc/xds/client/ControlPlaneClient.java | 149 ++--- .../java/io/grpc/xds/client/XdsClient.java | 27 +- .../io/grpc/xds/client/XdsClientImpl.java | 605 +++--------------- .../io/grpc/xds/client/XdsResourceType.java | 4 - .../grpc/xds/client/XdsTransportFactory.java | 5 - .../io/grpc/xds/GrpcBootstrapperImplTest.java | 1 - .../io/grpc/xds/XdsClientFallbackTest.java | 377 ----------- 9 files changed, 139 insertions(+), 1050 deletions(-) delete mode 100644 xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java diff --git a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java index 4d6050f3bbe..af77d128ae7 100644 --- a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java @@ -114,11 +114,6 @@ protected LdsUpdate doParse(Args args, Message unpackedMessage) } } - @Override - public boolean isSharedName() { - return true; - } - private LdsUpdate processClientSideListener(Listener listener) throws ResourceInvalidException { // Unpack HttpConnectionManager from the Listener. diff --git a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java index 2782af06825..7ef739c8048 100644 --- a/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/BootstrapperImpl.java @@ -41,9 +41,6 @@ @Internal public abstract class BootstrapperImpl extends Bootstrapper { - public static final String GRPC_EXPERIMENTAL_XDS_FALLBACK = - "GRPC_EXPERIMENTAL_XDS_FALLBACK"; - // Client features. @VisibleForTesting public static final String CLIENT_FEATURE_DISABLE_OVERPROVISIONING = @@ -62,18 +59,11 @@ protected BootstrapperImpl() { logger = XdsLogger.withLogId(InternalLogId.allocate("bootstrapper", null)); } - // Delayed initialization of xdsFallbackEnabled to allow for flag initialization. - public static boolean isEnabledXdsFallback() { - return GrpcUtil.getFlag(GRPC_EXPERIMENTAL_XDS_FALLBACK, false); - } - protected abstract String getJsonContent() throws IOException, XdsInitializationException; protected abstract Object getImplSpecificConfig(Map serverConfig, String serverUri) throws XdsInitializationException; - - /** * Reads and parses bootstrap config. The config is expected to be in JSON format. */ @@ -112,9 +102,6 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) throw new XdsInitializationException("Invalid bootstrap: 'xds_servers' does not exist."); } List servers = parseServerInfos(rawServerConfigs, logger); - if (servers.size() > 1 && !isEnabledXdsFallback()) { - servers = ImmutableList.of(servers.get(0)); - } builder.servers(servers); Node.Builder nodeBuilder = Node.newBuilder(); @@ -221,9 +208,6 @@ protected BootstrapInfo.Builder bootstrapBuilder(Map rawData) if (rawAuthorityServers == null || rawAuthorityServers.isEmpty()) { authorityServers = servers; } else { - if (rawAuthorityServers.size() > 1 && !isEnabledXdsFallback()) { - rawAuthorityServers = ImmutableList.of(rawAuthorityServers.get(0)); - } authorityServers = parseServerInfos(rawAuthorityServers, logger); } authorityInfoMapBuilder.put( diff --git a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java index 673c8cff66e..3074d1120ad 100644 --- a/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java +++ b/xds/src/main/java/io/grpc/xds/client/ControlPlaneClient.java @@ -16,6 +16,7 @@ package io.grpc.xds.client; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; @@ -71,6 +72,7 @@ final class ControlPlaneClient { private final BackoffPolicy.Provider backoffPolicyProvider; private final Stopwatch stopwatch; private final Node bootstrapNode; + private final XdsClient xdsClient; // Last successfully applied version_info for each resource type. Starts with empty string. // A version_info is used to update management server with client's most recent knowledge of @@ -78,15 +80,13 @@ final class ControlPlaneClient { private final Map, String> versions = new HashMap<>(); private boolean shutdown; - private boolean hasBeenActive; - private boolean lastStateWasReady; @Nullable private AdsStream adsStream; @Nullable private BackoffPolicy retryBackoffPolicy; @Nullable private ScheduledHandle rpcRetryTimer; - private final MessagePrettyPrinter messagePrinter; + private MessagePrettyPrinter messagePrinter; /** An entity that manages ADS RPCs over a single channel. */ ControlPlaneClient( @@ -100,6 +100,7 @@ final class ControlPlaneClient { SynchronizationContext syncContext, BackoffPolicy.Provider backoffPolicyProvider, Supplier stopwatchSupplier, + XdsClient xdsClient, MessagePrettyPrinter messagePrinter) { this.serverInfo = checkNotNull(serverInfo, "serverInfo"); this.xdsTransport = checkNotNull(xdsTransport, "xdsTransport"); @@ -109,6 +110,7 @@ final class ControlPlaneClient { this.timeService = checkNotNull(timeService, "timeService"); this.syncContext = checkNotNull(syncContext, "syncContext"); this.backoffPolicyProvider = checkNotNull(backoffPolicyProvider, "backoffPolicyProvider"); + this.xdsClient = checkNotNull(xdsClient, "xdsClient"); this.messagePrinter = checkNotNull(messagePrinter, "messagePrinter"); stopwatch = checkNotNull(stopwatchSupplier, "stopwatchSupplier").get(); logId = InternalLogId.allocate("xds-client", serverInfo.target()); @@ -138,31 +140,18 @@ public String toString() { return logId.toString(); } - public ServerInfo getServerInfo() { - return serverInfo; - } - /** * Updates the resource subscription for the given resource type. */ // Must be synchronized. - void adjustResourceSubscription(XdsResourceType resourceType, String authority) { + void adjustResourceSubscription(XdsResourceType resourceType) { if (isInBackoff()) { return; } if (adsStream == null) { startRpcStream(); - // when the stream becomes ready, it will send the discovery requests - return; } - - // We will do the rest of the method as part of the readyHandler when the stream is ready. - if (!lastStateWasReady) { - return; - } - - Collection resources = - resourceStore.getSubscribedResources(serverInfo, resourceType, authority); + Collection resources = resourceStore.getSubscribedResources(serverInfo, resourceType); if (resources == null) { resources = Collections.emptyList(); } @@ -211,7 +200,7 @@ void nackResponse(XdsResourceType type, String nonce, String errorDetail) { */ // Must be synchronized. boolean isInBackoff() { - return rpcRetryTimer != null || (hasBeenActive && !xdsTransport.isConnected()); + return rpcRetryTimer != null && rpcRetryTimer.isPending(); } // Must be synchronized. @@ -219,10 +208,6 @@ boolean isReady() { return adsStream != null && adsStream.call != null && adsStream.call.isReady(); } - boolean isConnected() { - return xdsTransport.isConnected(); - } - /** * Starts a timer for each requested resource that hasn't been responded to and * has been waiting for the channel to get ready. @@ -233,22 +218,12 @@ void readyHandler() { return; } - if (rpcRetryTimer != null) { + if (isInBackoff()) { rpcRetryTimer.cancel(); rpcRetryTimer = null; } - hasBeenActive = true; - if (!lastStateWasReady) { - lastStateWasReady = true; - xdsResponseHandler.handleStreamRestarted(serverInfo); - } - } - - void connect() { - if (adsStream == null) { - startRpcStream(); - } + xdsClient.startSubscriberTimersIfNeeded(serverInfo); } /** @@ -259,54 +234,27 @@ void connect() { private void startRpcStream() { checkState(adsStream == null, "Previous adsStream has not been cleared yet"); adsStream = new AdsStream(); - adsStream.start(); logger.log(XdsLogLevel.INFO, "ADS stream started"); stopwatch.reset().start(); } - void sendDiscoveryRequests(String authority) { - if (adsStream == null) { - startRpcStream(); - // when the stream becomes ready, it will send the discovery requests - return; - } - - if (isConnected()) { - Set> subscribedResourceTypes = - new HashSet<>(resourceStore.getSubscribedResourceTypesWithTypeUrl().values()); - - for (XdsResourceType type : subscribedResourceTypes) { - adjustResourceSubscription(type, authority); - } - } - } - - @SuppressWarnings("rawtypes") - boolean hasSubscribedResources(String authority) { - for (XdsResourceType type : resourceStore.getSubscribedResourceTypesWithTypeUrl().values()) { - Collection subscribedResources = - resourceStore.getSubscribedResources(serverInfo, type, authority); - if (subscribedResources != null && !subscribedResources.isEmpty()) { - return true; - } - } - return false; - } - @VisibleForTesting public final class RpcRetryTask implements Runnable { @Override public void run() { - logger.log(XdsLogLevel.DEBUG, "Retry timeout. Restart ADS stream {0}", logId); - if (shutdown || isReady()) { + if (shutdown) { return; } - - if (adsStream == null) { - startRpcStream(); + startRpcStream(); + Set> subscribedResourceTypes = + new HashSet<>(resourceStore.getSubscribedResourceTypesWithTypeUrl().values()); + for (XdsResourceType type : subscribedResourceTypes) { + Collection resources = resourceStore.getSubscribedResources(serverInfo, type); + if (resources != null) { + adsStream.sendDiscoveryRequest(type, resources); + } } - - // handling CPC management is triggered in readyHandler + xdsResponseHandler.handleStreamRestarted(serverInfo); } } @@ -333,9 +281,6 @@ private class AdsStream implements EventHandler { private AdsStream() { this.call = xdsTransport.createStreamingCall(methodDescriptor.getFullMethodName(), methodDescriptor.getRequestMarshaller(), methodDescriptor.getResponseMarshaller()); - } - - void start() { call.start(this); } @@ -364,9 +309,6 @@ void sendDiscoveryRequest(XdsResourceType type, String versionInfo, builder.setErrorDetail(error); } DiscoveryRequest request = builder.build(); - if (isConnected()) { - resourceStore.assignResourcesToOwner(type, resources, ControlPlaneClient.this); - } call.sendMessage(request); if (logger.isLoggable(XdsLogLevel.DEBUG)) { logger.log(XdsLogLevel.DEBUG, "Sent DiscoveryRequest\n{0}", messagePrinter.print(request)); @@ -384,11 +326,6 @@ final void sendDiscoveryRequest(XdsResourceType type, Collection reso @Override public void onReady() { - logger.log(XdsLogLevel.DEBUG, "ADS stream ready {0}", logId); - if (shutdown || closed) { - return; - } - syncContext.execute(ControlPlaneClient.this::readyHandler); } @@ -420,9 +357,12 @@ public void run() { @Override public void onStatusReceived(final Status status) { - lastStateWasReady = false; syncContext.execute(() -> { - handleRpcStreamClosed(status); + if (status.isOk()) { + handleRpcStreamClosed(Status.UNAVAILABLE.withDescription(CLOSED_BY_SERVER)); + } else { + handleRpcStreamClosed(status); + } }); } @@ -441,7 +381,7 @@ final void handleRpcResponse(XdsResourceType type, String versionInfo, ListMust be synchronized. - */ + /** Called when the ADS stream has been recreated. */ + // Must be synchronized. void handleStreamRestarted(ServerInfo serverInfo); } @@ -431,25 +427,6 @@ public interface ResourceStore { Collection getSubscribedResources(ServerInfo serverInfo, XdsResourceType type); - /** - * Like {@link #getSubscribedResources(ServerInfo, XdsResourceType)}, but limits the results to - * those matching the given authority. - */ - @Nullable - Collection getSubscribedResources( - ServerInfo serverInfo, XdsResourceType type, String authority); - Map> getSubscribedResourceTypesWithTypeUrl(); - - /** - * Assigns the given resources to the given owner. Generally, the owner is a ControlPlaneClient - * @param type the type of the resources - * @param resources the names of the resources - * @param owner the object that should take over ownership of the resources - */ - default void assignResourcesToOwner(XdsResourceType type, Collection resources, - Object owner) { - // Default is a no-op implemention which is useful for test cases where everything is mocked - } } } 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 db9d686eb9d..79147cd9862 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsClientImpl.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static io.grpc.xds.client.Bootstrapper.XDSTP_SCHEME; -import static io.grpc.xds.client.ControlPlaneClient.CLOSED_BY_SERVER; import static io.grpc.xds.client.XdsResourceType.ParsedResource; import static io.grpc.xds.client.XdsResourceType.ValidatedResourceUpdate; @@ -27,7 +26,6 @@ import com.google.common.base.Joiner; import com.google.common.base.Stopwatch; import com.google.common.base.Supplier; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.ListenableFuture; @@ -43,13 +41,12 @@ import io.grpc.xds.client.Bootstrapper.AuthorityInfo; import io.grpc.xds.client.Bootstrapper.ServerInfo; import io.grpc.xds.client.XdsClient.ResourceStore; +import io.grpc.xds.client.XdsClient.XdsResponseHandler; import io.grpc.xds.client.XdsLogger.XdsLogLevel; import java.net.URI; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -63,7 +60,7 @@ * XdsClient implementation. */ @Internal -public final class XdsClientImpl extends XdsClient implements ResourceStore { +public final class XdsClientImpl extends XdsClient implements XdsResponseHandler, ResourceStore { // Longest time to wait, since the subscription to some resource, for concluding its absence. @VisibleForTesting @@ -77,25 +74,21 @@ public void uncaughtException(Thread t, Throwable e) { XdsLogLevel.ERROR, "Uncaught exception in XdsClient SynchronizationContext. Panic!", e); - // TODO: better error handling. + // TODO(chengyuanzhang): better error handling. throw new AssertionError(e); } }); - private final Map loadStatsManagerMap = new HashMap<>(); - final Map serverLrsClientMap = new HashMap<>(); - /** Map of authority to its active control plane client (affected by xds fallback). */ - private final Map activeCpClients = new HashMap<>(); + private final Map loadStatsManagerMap = + new HashMap<>(); + final Map serverLrsClientMap = + new HashMap<>(); private final Map serverCpClientMap = new HashMap<>(); - - /** Maps resource type to the corresponding map of subscribers (keyed by resource name). */ private final Map, Map>> resourceSubscribers = new HashMap<>(); - /** Maps typeUrl to the corresponding XdsResourceType. */ private final Map> subscribedResourceTypeUrls = new HashMap<>(); - private final XdsTransportFactory xdsTransportFactory; private final Bootstrapper.BootstrapInfo bootstrapInfo; private final ScheduledExecutorService timeService; @@ -130,6 +123,48 @@ public XdsClientImpl( logger.log(XdsLogLevel.INFO, "Created"); } + @Override + public void handleResourceResponse( + XdsResourceType xdsResourceType, ServerInfo serverInfo, String versionInfo, + List resources, String nonce, ProcessingTracker processingTracker) { + checkNotNull(xdsResourceType, "xdsResourceType"); + syncContext.throwIfNotInThisSynchronizationContext(); + Set toParseResourceNames = + xdsResourceType.shouldRetrieveResourceKeysForArgs() + ? getResourceKeys(xdsResourceType) + : null; + XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, versionInfo, nonce, + bootstrapInfo, securityConfig, toParseResourceNames); + handleResourceUpdate(args, resources, xdsResourceType, processingTracker); + } + + @Override + public void handleStreamClosed(Status error) { + syncContext.throwIfNotInThisSynchronizationContext(); + cleanUpResourceTimers(); + for (Map> subscriberMap : + resourceSubscribers.values()) { + for (ResourceSubscriber subscriber : subscriberMap.values()) { + if (!subscriber.hasResult()) { + subscriber.onError(error, null); + } + } + } + } + + @Override + public void handleStreamRestarted(ServerInfo serverInfo) { + syncContext.throwIfNotInThisSynchronizationContext(); + for (Map> subscriberMap : + resourceSubscribers.values()) { + for (ResourceSubscriber subscriber : subscriberMap.values()) { + if (subscriber.serverInfo.equals(serverInfo)) { + subscriber.restartTimer(); + } + } + } + } + @Override public void shutdown() { syncContext.execute( @@ -146,7 +181,7 @@ public void run() { for (final LoadReportClient lrsClient : serverLrsClientMap.values()) { lrsClient.stopLoadReporting(); } - cleanUpResourceTimers(null); + cleanUpResourceTimers(); } }); } @@ -161,79 +196,22 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { return Collections.unmodifiableMap(subscribedResourceTypeUrls); } - @Override - public void assignResourcesToOwner(XdsResourceType type, Collection resources, - Object owner) { - if (isShutDown()) { - return; - } - - Map> resourceSubscriberMap = - resourceSubscribers.get(type); - - ControlPlaneClient controlPlaneClient = (ControlPlaneClient) owner; - for (String resource : resources) { - ResourceSubscriber subscriber = resourceSubscriberMap.get(resource); - if (subscriber != null && subscriber.controlPlaneClient == null) { - subscriber.controlPlaneClient = controlPlaneClient; - } - } - } - - public void assignUnassignedResources(String authority) { - if (isShutDown()) { - return; - } - - ControlPlaneClient controlPlaneClient = activeCpClients.get(authority); - if (controlPlaneClient == null) { - return; - } - - for (XdsResourceType resourceType : resourceSubscribers.keySet()) { - for (ResourceSubscriber subscriber - : resourceSubscribers.get(resourceType).values()) { - if (subscriber.controlPlaneClient == null - && Objects.equals(subscriber.authority, authority)) { - subscriber.controlPlaneClient = controlPlaneClient; - } - } - } - } - - @Nullable - @Override - public Collection getSubscribedResources( - ServerInfo serverInfo, XdsResourceType type) { - return getSubscribedResources(serverInfo, type, null, false); - } - @Nullable @Override - public Collection getSubscribedResources( - ServerInfo serverInfo, XdsResourceType type, String authority) { - return getSubscribedResources(serverInfo, type, authority, true); - } - - private Collection getSubscribedResources(ServerInfo serverInfo, XdsResourceType type, String authority, boolean useAuthority) { + public Collection getSubscribedResources(ServerInfo serverInfo, + XdsResourceType type) { Map> resources = resourceSubscribers.getOrDefault(type, Collections.emptyMap()); ImmutableSet.Builder builder = ImmutableSet.builder(); - String target = serverInfo.target(); - - for (String resourceName : resources.keySet()) { - ResourceSubscriber resource = resources.get(resourceName); - if ( resource.isCpcMutable() || (target.equals(resource.getTarget()) - && (!useAuthority || Objects.equals(authority, resource.authority)))) { - builder.add(resourceName); + for (String key : resources.keySet()) { + if (resources.get(key).serverInfo.equals(serverInfo)) { + builder.add(key); } } Collection retVal = builder.build(); return retVal.isEmpty() ? null : retVal; } - // As XdsClient APIs becomes resource agnostic, subscribed resource types are dynamic. // ResourceTypes that do not have subscribers does not show up in the snapshot keys. @Override @@ -247,7 +225,7 @@ public void run() { // A map from a "resource type" to a map ("resource name": "resource metadata") ImmutableMap.Builder, Map> metadataSnapshot = ImmutableMap.builder(); - for (XdsResourceType resourceType : resourceSubscribers.keySet()) { + for (XdsResourceType resourceType: resourceSubscribers.keySet()) { ImmutableMap.Builder metadataMap = ImmutableMap.builder(); for (Map.Entry> resourceEntry : resourceSubscribers.get(resourceType).entrySet()) { @@ -268,9 +246,9 @@ public Object getSecurityConfig() { @Override public void watchXdsResource(XdsResourceType type, - String resourceName, - ResourceWatcher watcher, - Executor watcherExecutor) { + String resourceName, + ResourceWatcher watcher, + Executor watcherExecutor) { syncContext.execute(new Runnable() { @Override @SuppressWarnings("unchecked") @@ -285,69 +263,31 @@ public void run() { logger.log(XdsLogLevel.INFO, "Subscribe {0} resource {1}", type, resourceName); subscriber = new ResourceSubscriber<>(type, resourceName); resourceSubscribers.get(type).put(resourceName, subscriber); - subscriber.addWatcher(watcher, watcherExecutor); - if (subscriber.errorDescription != null) { - return; - } - - ControlPlaneClient cpcToUse; - if (subscriber.controlPlaneClient == null) { - cpcToUse = activeCpClients.get(subscriber.authority); - } else { - cpcToUse = subscriber.controlPlaneClient; - } - if (subscriber.controlPlaneClient != null) { - doFallbackIfNecessary(subscriber.controlPlaneClient, subscriber.authority); + subscriber.controlPlaneClient.adjustResourceSubscription(type); } - if (cpcToUse != null) { - cpcToUse.adjustResourceSubscription(type, subscriber.authority); - } else { - // No active control plane client for the authority. Start a new one. - ImmutableList serverInfos = getServerInfos(subscriber.authority); - if (serverInfos == null) { - logger.log(XdsLogLevel.INFO, - "Request for unknown authority {}", subscriber.authority); - return; - } - cpcToUse = getOrCreateControlPlaneClient(serverInfos); - if (cpcToUse != null) { - logger.log(XdsLogLevel.DEBUG, "Start new control plane client {0}", - cpcToUse.getServerInfo()); - } else { - logger.log(XdsLogLevel.WARNING, "No active control plane client for authority {0}", - subscriber.authority); - } - } - } else { - subscriber.addWatcher(watcher, watcherExecutor); } + subscriber.addWatcher(watcher, watcherExecutor); } }); } @Override public void cancelXdsResourceWatch(XdsResourceType type, - String resourceName, - ResourceWatcher watcher) { + String resourceName, + ResourceWatcher watcher) { syncContext.execute(new Runnable() { @Override @SuppressWarnings("unchecked") public void run() { ResourceSubscriber subscriber = (ResourceSubscriber) resourceSubscribers.get(type).get(resourceName); - if (subscriber == null) { - logger.log(XdsLogLevel.WARNING, "double cancel of resource watch for {0}:{1}", - type.typeName(), resourceName); - return; - } subscriber.removeWatcher(watcher); if (!subscriber.isWatched()) { subscriber.cancelResourceWatch(); resourceSubscribers.get(type).remove(resourceName); - ControlPlaneClient controlPlaneClient = subscriber.controlPlaneClient; - if (controlPlaneClient != null) { - controlPlaneClient.adjustResourceSubscription(type, subscriber.authority); + if (subscriber.controlPlaneClient != null) { + subscriber.controlPlaneClient.adjustResourceSubscription(type); } if (resourceSubscribers.get(type).isEmpty()) { resourceSubscribers.remove(type); @@ -414,30 +354,17 @@ public void run() { return; } - // TODO add Jitter so that server isn't swamped when it comes up if this is a restart - ControlPlaneClient cpc = serverCpClientMap.get(serverInfo); - if (cpc == null) { - return; - } - for (Map> subscriberMap : resourceSubscribers.values()) { for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (cpc == subscriber.controlPlaneClient && subscriber.respTimer == null) { + if (subscriber.serverInfo.equals(serverInfo) && subscriber.respTimer == null) { subscriber.restartTimer(); } } } - } }); } - @VisibleForTesting - public boolean isSubscriberCpcConnected(XdsResourceType type, String resourceName) { - ControlPlaneClient cpc = resourceSubscribers.get(type).get(resourceName).controlPlaneClient; - return cpc != null && cpc.isConnected(); - } - private Set getResourceKeys(XdsResourceType xdsResourceType) { if (!resourceSubscribers.containsKey(xdsResourceType)) { return null; @@ -446,60 +373,33 @@ private Set getResourceKeys(XdsResourceType xdsResourceType) { return resourceSubscribers.get(xdsResourceType).keySet(); } - private void cleanUpResourceTimers(ControlPlaneClient cpcForThisStream) { + private void cleanUpResourceTimers() { for (Map> subscriberMap : resourceSubscribers.values()) { for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (cpcForThisStream == null || cpcForThisStream.equals(subscriber.controlPlaneClient)) { - subscriber.stopTimer(); - } - } - } - } - - public ControlPlaneClient getOrCreateControlPlaneClient(ImmutableList serverInfos) { - if (serverInfos.isEmpty()) { - return null; - } - - for (ServerInfo serverInfo : serverInfos) { - if (serverCpClientMap.containsKey(serverInfo)) { - ControlPlaneClient controlPlaneClient = serverCpClientMap.get(serverInfo); - if (controlPlaneClient.isInBackoff()) { - continue; - } - return controlPlaneClient; - } else { - ControlPlaneClient cpc = getOrCreateControlPlaneClient(serverInfo); - logger.log(XdsLogLevel.DEBUG, "Created control plane client {0}", cpc); - return cpc; + subscriber.stopTimer(); } } - - // Everything existed and is in backoff so return null - return null; } - private ControlPlaneClient getOrCreateControlPlaneClient(ServerInfo serverInfo) { + public ControlPlaneClient getOrCreateControlPlaneClient(ServerInfo serverInfo) { syncContext.throwIfNotInThisSynchronizationContext(); if (serverCpClientMap.containsKey(serverInfo)) { return serverCpClientMap.get(serverInfo); } XdsTransportFactory.XdsTransport xdsTransport = xdsTransportFactory.create(serverInfo); - ControlPlaneClient controlPlaneClient = new ControlPlaneClient( xdsTransport, serverInfo, bootstrapInfo.node(), - new ResponseHandler(serverInfo), + this, this, timeService, syncContext, backoffPolicyProvider, stopwatchSupplier, - messagePrinter - ); - + this, + messagePrinter); serverCpClientMap.put(serverInfo, controlPlaneClient); LoadStatsManager2 loadStatsManager = new LoadStatsManager2(stopwatchSupplier); @@ -518,73 +418,46 @@ public Map getServerLrsClientMap() { return ImmutableMap.copyOf(serverLrsClientMap); } - private String getAuthority(String resource) { - String authority; + @Nullable + private ServerInfo getServerInfo(String resource) { if (resource.startsWith(XDSTP_SCHEME)) { URI uri = URI.create(resource); - authority = uri.getAuthority(); + String authority = uri.getAuthority(); if (authority == null) { authority = ""; } - } else { - authority = null; - } - - return authority; - } - - @Nullable - private ImmutableList getServerInfos(String authority) { - if (authority != null) { AuthorityInfo authorityInfo = bootstrapInfo.authorities().get(authority); if (authorityInfo == null || authorityInfo.xdsServers().isEmpty()) { return null; } - return authorityInfo.xdsServers(); + return authorityInfo.xdsServers().get(0); } else { - return bootstrapInfo.servers(); - } - } - - private List getAuthoritiesForServerInfo(ServerInfo serverInfo) { - List authorities = new ArrayList<>(); - for (Map.Entry entry : bootstrapInfo.authorities().entrySet()) { - if (entry.getValue().xdsServers().contains(serverInfo)) { - authorities.add(entry.getKey()); - } - } - - if (bootstrapInfo.servers().contains(serverInfo)) { - authorities.add(null); + return bootstrapInfo.servers().get(0); // use first server } - - return authorities; } @SuppressWarnings("unchecked") private void handleResourceUpdate( XdsResourceType.Args args, List resources, XdsResourceType xdsResourceType, ProcessingTracker processingTracker) { - ControlPlaneClient controlPlaneClient = serverCpClientMap.get(args.serverInfo); - ValidatedResourceUpdate result = xdsResourceType.parse(args, resources); logger.log(XdsLogger.XdsLogLevel.INFO, "Received {0} Response version {1} nonce {2}. Parsed resources: {3}", - xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources); + xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources); Map> parsedResources = result.parsedResources; Set invalidResources = result.invalidResources; List errors = result.errors; String errorDetail = null; if (errors.isEmpty()) { checkArgument(invalidResources.isEmpty(), "found invalid resources but missing errors"); - controlPlaneClient.ackResponse(xdsResourceType, args.versionInfo, + serverCpClientMap.get(args.serverInfo).ackResponse(xdsResourceType, args.versionInfo, args.nonce); } else { errorDetail = Joiner.on('\n').join(errors); logger.log(XdsLogLevel.WARNING, "Failed processing {0} Response version {1} nonce {2}. Errors:\n{3}", xdsResourceType.typeName(), args.versionInfo, args.nonce, errorDetail); - controlPlaneClient.nackResponse(xdsResourceType, args.nonce, errorDetail); + serverCpClientMap.get(args.serverInfo).nackResponse(xdsResourceType, args.nonce, errorDetail); } long updateTime = timeProvider.currentTimeNanos(); @@ -594,9 +467,6 @@ private void handleResourceUpdate( String resourceName = entry.getKey(); ResourceSubscriber subscriber = (ResourceSubscriber) entry.getValue(); if (parsedResources.containsKey(resourceName)) { - if (subscriber.isCpcMutable()) { - subscriber.controlPlaneClient = controlPlaneClient; - } // Happy path: the resource updated successfully. Notify the watchers of the update. subscriber.onData(parsedResources.get(resourceName), args.versionInfo, updateTime, processingTracker); @@ -626,153 +496,51 @@ private void handleResourceUpdate( // For State of the World services, notify watchers when their watched resource is missing // from the ADS update. Note that we can only do this if the resource update is coming from // the same xDS server that the ResourceSubscriber is subscribed to. - if (controlPlaneClient.equals(subscriber.controlPlaneClient)) { + if (subscriber.serverInfo.equals(args.serverInfo)) { subscriber.onAbsent(processingTracker); } } } - /** - * Try to fallback to a lower priority control plane client. - * - * @param oldCpc The control plane that we are falling back from - * @param activeServerInfo Matches the currently active control plane client - * @param serverInfos The full ordered list of xds servers - * @param authority The authority within which we are falling back - * @return true if a fallback was successful, false otherwise. - */ - private boolean doFallbackForAuthority(ControlPlaneClient oldCpc, ServerInfo activeServerInfo, - List serverInfos, String authority) { - if (serverInfos == null || serverInfos.size() < 2) { - return false; - } - - // Build a list of servers with lower priority than the current server. - int i = serverInfos.indexOf(activeServerInfo); - List fallbackServers = (i > -1) - ? serverInfos.subList(i, serverInfos.size()) - : null; - - ControlPlaneClient fallbackCpc = - fallbackServers != null - ? getOrCreateControlPlaneClient(ImmutableList.copyOf(fallbackServers)) - : null; - - return fallBackToCpc(fallbackCpc, authority, oldCpc); - } - - private boolean fallBackToCpc(ControlPlaneClient fallbackCpc, String authority, - ControlPlaneClient oldCpc) { - boolean didFallback = false; - if (fallbackCpc != null && ! fallbackCpc.isInBackoff()) { - logger.log(XdsLogLevel.INFO, "Falling back to XDS server {0}", - fallbackCpc.getServerInfo().target()); - - setCpcForAuthority(authority, fallbackCpc); - fallbackCpc.sendDiscoveryRequests(authority); - didFallback = true; - } else { - logger.log(XdsLogLevel.WARNING, "No working fallback XDS Servers found from {0}", - oldCpc.getServerInfo().target()); - } - return didFallback; - } - - private void doFallbackIfNecessary(ControlPlaneClient cpc, String authority) { - if (cpc == null || !BootstrapperImpl.isEnabledXdsFallback()) { - return; - } - - ControlPlaneClient activeCpClient = activeCpClients.get(authority); - if (cpc == activeCpClient) { - return; - } - if (activeCpClient == null) { - setCpcForAuthority(authority, cpc); - return; - } - - fallBackToCpc(cpc, authority, activeCpClient); - } - - private void setCpcForAuthority(String authority, ControlPlaneClient cpc) { - activeCpClients.put(authority, cpc); - - // Take ownership of resource's whose names are shared across xds servers (ex. LDS) - for (Map.Entry, Map>> subscriberTypeEntry - : resourceSubscribers.entrySet()) { - if (!subscriberTypeEntry.getKey().isSharedName()) { - continue; - } - for (ResourceSubscriber subscriber : subscriberTypeEntry.getValue().values()) { - if (subscriber.controlPlaneClient == cpc - || !Objects.equals(subscriber.authority, authority)) { - continue; - } - subscriber.data = null; - subscriber.absent = false; - subscriber.controlPlaneClient = cpc; - subscriber.restartTimer(); - } - } - - // Rely on the watchers to clear out the old cache values and rely on the - // backup xds servers having unique resource names - } - /** * Tracks a single subscribed resource. */ private final class ResourceSubscriber { - @Nullable - private final ImmutableList serverInfos; - @Nullable - private ControlPlaneClient controlPlaneClient; - private final String authority; + @Nullable private final ServerInfo serverInfo; + @Nullable private final ControlPlaneClient controlPlaneClient; private final XdsResourceType type; private final String resource; private final Map, Executor> watchers = new HashMap<>(); - @Nullable - private T data; + @Nullable private T data; private boolean absent; // Tracks whether the deletion has been ignored per bootstrap server feature. // See https://github.com/grpc/proposal/blob/master/A53-xds-ignore-resource-deletion.md private boolean resourceDeletionIgnored; - @Nullable - private ScheduledHandle respTimer; - @Nullable - private ResourceMetadata metadata; - @Nullable - private String errorDescription; - private boolean cpcFixed = false; + @Nullable private ScheduledHandle respTimer; + @Nullable private ResourceMetadata metadata; + @Nullable private String errorDescription; ResourceSubscriber(XdsResourceType type, String resource) { syncContext.throwIfNotInThisSynchronizationContext(); this.type = type; this.resource = resource; - this.authority = getAuthority(resource); - this.serverInfos = getServerInfos(authority); - if (serverInfos == null) { + this.serverInfo = getServerInfo(resource); + if (serverInfo == null) { this.errorDescription = "Wrong configuration: xds server does not exist for resource " + resource; this.controlPlaneClient = null; return; } - // Initialize metadata in UNKNOWN state to cover the case when resource subscriber, // is created but not yet requested because the client is in backoff. this.metadata = ResourceMetadata.newResourceMetadataUnknown(); ControlPlaneClient controlPlaneClient = null; try { - controlPlaneClient = getOrCreateControlPlaneClient(serverInfos); - if (controlPlaneClient == null) { + controlPlaneClient = getOrCreateControlPlaneClient(serverInfo); + if (controlPlaneClient.isInBackoff()) { return; } - - if (!controlPlaneClient.isConnected()) { - controlPlaneClient.connect(); // Make sure we are at least trying to connect - } } catch (IllegalArgumentException e) { controlPlaneClient = null; this.errorDescription = "Bad configuration: " + e.getMessage(); @@ -784,21 +552,6 @@ private final class ResourceSubscriber { restartTimer(); } - @Override - public String toString() { - return "ResourceSubscriber{" - + "resource='" + resource + '\'' - + ", controlPlaneClient=" + controlPlaneClient - + ", authority='" + authority + '\'' - + ", type=" + type - + ", watchers=" + watchers.size() - + ", data=" + data - + ", absent=" + absent - + ", resourceDeletionIgnored=" + resourceDeletionIgnored - + ", errorDescription='" + errorDescription + '\'' - + '}'; - } - void addWatcher(ResourceWatcher watcher, Executor watcherExecutor) { checkArgument(!watchers.containsKey(watcher), "watcher %s already registered", watcher); watchers.put(watcher, watcherExecutor); @@ -817,7 +570,7 @@ void addWatcher(ResourceWatcher watcher, Executor watcherExecutor) { }); } - void removeWatcher(ResourceWatcher watcher) { + void removeWatcher(ResourceWatcher watcher) { checkArgument(watchers.containsKey(watcher), "watcher %s not registered", watcher); watchers.remove(watcher); } @@ -826,8 +579,7 @@ void restartTimer() { if (data != null || absent) { // resource already resolved return; } - if (controlPlaneClient == null || !controlPlaneClient.isReady()) { - // When client becomes ready, it triggers a restartTimer for all relevant subscribers. + if (!controlPlaneClient.isReady()) { // When client becomes ready, it triggers a restartTimer return; } @@ -849,9 +601,6 @@ public String toString() { // Initial fetch scheduled or rescheduled, transition metadata state to REQUESTED. metadata = ResourceMetadata.newResourceMetadataRequested(); - if (respTimer != null) { - respTimer.cancel(); - } respTimer = syncContext.schedule( new ResourceNotFound(), INITIAL_RESOURCE_FETCH_TIMEOUT_SEC, TimeUnit.SECONDS, timeService); @@ -875,7 +624,8 @@ void cancelResourceWatch() { message += " for which we previously ignored a deletion"; logLevel = XdsLogLevel.FORCE_INFO; } - logger.log(logLevel, message, type, resource, getTarget()); + logger.log(logLevel, message, type, resource, + serverInfo != null ? serverInfo.target() : "unknown"); } boolean isWatched() { @@ -897,11 +647,10 @@ void onData(ParsedResource parsedResource, String version, long updateTime, ResourceUpdate oldData = this.data; this.data = parsedResource.getResourceUpdate(); absent = false; - cpcFixed = true; if (resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_INFO, "xds server {0}: server returned new version " + "of resource for which we previously ignored a deletion: type {1} name {2}", - getTarget(), type, resource); + serverInfo != null ? serverInfo.target() : "unknown", type, resource); resourceDeletionIgnored = false; } if (!Objects.equals(oldData, data)) { @@ -918,16 +667,6 @@ void onData(ParsedResource parsedResource, String version, long updateTime, } } - private String getTarget() { - return (serverInfos != null && controlPlaneClient != null) - ? controlPlaneClient.getServerInfo().target() - : "unknown"; - } - - private boolean isCpcMutable() { - return !cpcFixed || type.isSharedName(); - } - void onAbsent(@Nullable ProcessingTracker processingTracker) { if (respTimer != null && respTimer.isPending()) { // too early to conclude absence return; @@ -936,13 +675,12 @@ void onAbsent(@Nullable ProcessingTracker processingTracker) { // Ignore deletion of State of the World resources when this feature is on, // and the resource is reusable. boolean ignoreResourceDeletionEnabled = - serverInfos != null && controlPlaneClient != null - && controlPlaneClient.getServerInfo().ignoreResourceDeletion(); + serverInfo != null && serverInfo.ignoreResourceDeletion(); if (ignoreResourceDeletionEnabled && type.isFullStateOfTheWorld() && data != null) { if (!resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_WARNING, "xds server {0}: ignoring deletion for resource type {1} name {2}}", - getTarget(), type, resource); + serverInfo.target(), type, resource); resourceDeletionIgnored = true; } return; @@ -1009,151 +747,4 @@ private void notifyWatcher(ResourceWatcher watcher, T update) { } } - private class ResponseHandler implements XdsResponseHandler { - final ServerInfo serverInfo; - - ResponseHandler(ServerInfo serverInfo) { - this.serverInfo = serverInfo; - } - - @Override - public void handleResourceResponse( - XdsResourceType xdsResourceType, ServerInfo serverInfo, String versionInfo, - List resources, String nonce, ProcessingTracker processingTracker) { - checkNotNull(xdsResourceType, "xdsResourceType"); - syncContext.throwIfNotInThisSynchronizationContext(); - Set toParseResourceNames = - xdsResourceType.shouldRetrieveResourceKeysForArgs() - ? getResourceKeys(xdsResourceType) - : null; - XdsResourceType.Args args = new XdsResourceType.Args(serverInfo, versionInfo, nonce, - bootstrapInfo, securityConfig, toParseResourceNames); - handleResourceUpdate(args, resources, xdsResourceType, processingTracker); - } - - @Override - public void handleStreamClosed(Status status) { - syncContext.throwIfNotInThisSynchronizationContext(); - ControlPlaneClient cpcForThisStream = serverCpClientMap.get(serverInfo); - cleanUpResourceTimers(cpcForThisStream); - - Status error = status.isOk() ? Status.UNAVAILABLE.withDescription(CLOSED_BY_SERVER) : status; - boolean checkForFallback = !status.isOk() && BootstrapperImpl.isEnabledXdsFallback(); - - for (Map> subscriberMap : - resourceSubscribers.values()) { - for (ResourceSubscriber subscriber : subscriberMap.values()) { - if (subscriber.hasResult() - || (subscriber.cpcFixed && !cpcForThisStream.equals(subscriber.controlPlaneClient))) { - continue; - } - if (checkForFallback) { - // try to fallback to lower priority control plane client - if (doFallbackForAuthority(cpcForThisStream, - serverInfo, subscriber.serverInfos, subscriber.authority)) { - return; - } - checkForFallback = false; - } - - subscriber.onError(error, null); - } - } - } - - @Override - public void handleStreamRestarted(ServerInfo serverInfo) { - if (isShutDown()) { - return; - } - - syncContext.throwIfNotInThisSynchronizationContext(); - ControlPlaneClient controlPlaneClient = serverCpClientMap.get(serverInfo); - if (controlPlaneClient == null) { - return; - } - - for (String authority : getAuthoritiesForServerInfo(serverInfo)) { - if (controlPlaneClient.hasSubscribedResources(authority)) { - internalHandleStreamReady(serverInfo, controlPlaneClient, authority); - } - } - } - - private void internalHandleStreamReady( - ServerInfo serverInfo, ControlPlaneClient controlPlaneClient, String authority) { - ControlPlaneClient activeCpClient = activeCpClients.get(authority); - if (activeCpClient == null) { - setCpcForAuthority(authority, controlPlaneClient); - restartMatchingSubscriberTimers(controlPlaneClient, authority); - controlPlaneClient.sendDiscoveryRequests(authority); - return; // Since nothing was active can ignore fallback - } - - if (activeCpClient.isReady() - && compareCpClients(activeCpClient, controlPlaneClient, authority) < 0) { - logger.log(XdsLogLevel.INFO, "Ignoring stream restart for lower priority server {0}", - serverInfo.target()); - return; - } - - if (activeCpClient != controlPlaneClient) { - setCpcForAuthority(authority, controlPlaneClient); - } else { - assignUnassignedResources(authority); - } - - restartMatchingSubscriberTimers(controlPlaneClient, authority); - controlPlaneClient.sendDiscoveryRequests(authority); - - // If fallback isn't enabled, we're done. - if (!BootstrapperImpl.isEnabledXdsFallback()) { - return; - } - - // Shutdown any lower priority control plane clients. - Iterator iterator = serverCpClientMap.values().iterator(); - while (iterator.hasNext()) { - ControlPlaneClient client = iterator.next(); - if (compareCpClients(client, controlPlaneClient, authority) > 0 - && !activeCpClients.containsValue(client)) { - iterator.remove(); - client.shutdown(); // TODO someday add an exponential backoff to mitigate flapping - } - } - } - - private void restartMatchingSubscriberTimers( - ControlPlaneClient controlPlaneClient, String authority) { - // Restart the timers for all the watched resources that are associated with this stream - for (Map> subscriberMap : - resourceSubscribers.values()) { - for (ResourceSubscriber subscriber : subscriberMap.values()) { - if ((subscriber.controlPlaneClient == null) - || (subscriber.controlPlaneClient.equals(controlPlaneClient) - && Objects.equals(subscriber.authority, authority))) { - subscriber.restartTimer(); - } - } - } - } - } - - private int compareCpClients(ControlPlaneClient base, ControlPlaneClient other, - String authority) { - if (base == other || other == null) { - return 0; - } - - ImmutableList serverInfos = getServerInfos(authority); - ServerInfo baseServerInfo = base.getServerInfo(); - ServerInfo otherServerInfo = other.getServerInfo(); - - if (serverInfos == null || !serverInfos.contains(baseServerInfo) - || !serverInfos.contains(otherServerInfo)) { - return -100; // At least one of them isn't serving this authority - } - - return serverInfos.indexOf(baseServerInfo) - serverInfos.indexOf(otherServerInfo); - } } diff --git a/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java b/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java index 7f2b16b7521..8c3d31604e4 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsResourceType.java @@ -79,10 +79,6 @@ protected String extractResourceName(Message unpackedResource) { // the resources that need an update. protected abstract boolean isFullStateOfTheWorld(); - public boolean isSharedName() { - return false; - } - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/10847") public static class Args { final ServerInfo serverInfo; diff --git a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java index 6e8b2f7462a..ec700bd6dc9 100644 --- a/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/client/XdsTransportFactory.java @@ -36,11 +36,6 @@ StreamingCall createStreamingCall( MethodDescriptor.Marshaller respMarshaller); void shutdown(); - - /** - * Returns true if this transport is currently connected to the xDS server. - */ - boolean isConnected(); } /** diff --git a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java index eaa8aaa9e47..30ea76b54f2 100644 --- a/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java +++ b/xds/src/test/java/io/grpc/xds/GrpcBootstrapperImplTest.java @@ -65,7 +65,6 @@ public class GrpcBootstrapperImplTest { @Before public void setUp() { saveEnvironment(); - System.setProperty(BootstrapperImpl.GRPC_EXPERIMENTAL_XDS_FALLBACK, "true"); bootstrapper.bootstrapPathFromEnvVar = BOOTSTRAP_FILE_PATH; } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java deleted file mode 100644 index 798600a6851..00000000000 --- a/xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java +++ /dev/null @@ -1,377 +0,0 @@ -/* - * Copyright 2024 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc.xds; - -import static junit.framework.TestCase.assertFalse; -import static org.mockito.AdditionalAnswers.delegatesTo; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.inOrder; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.timeout; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import io.grpc.Status; -import io.grpc.internal.ObjectPool; -import io.grpc.xds.client.BootstrapperImpl; -import io.grpc.xds.client.XdsClient; -import io.grpc.xds.client.XdsClientImpl; -import io.grpc.xds.client.XdsInitializationException; -import java.net.InetSocketAddress; -import java.util.Collections; -import java.util.Map; -import java.util.UUID; -import java.util.logging.Level; -import java.util.logging.Logger; -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; -import org.mockito.InOrder; -import org.mockito.Mock; -import org.mockito.junit.MockitoJUnit; -import org.mockito.junit.MockitoRule; - -@RunWith(JUnit4.class) -public class XdsClientFallbackTest { - private static final Logger log = Logger.getLogger(XdsClientFallbackTest.class.getName()); - - private static final String MAIN_SERVER = "main-server"; - private static final String FALLBACK_SERVER = "fallback-server"; - private static final String DUMMY_TARGET = "TEST_TARGET"; - private static final String RDS_NAME = "route-config.googleapis.com"; - private static final String FALLBACK_RDS_NAME = "fallback-" + RDS_NAME; - private static final String CLUSTER_NAME = "cluster0"; - private static final String FALLBACK_CLUSTER_NAME = "fallback-" + CLUSTER_NAME; - private static final String EDS_NAME = "eds-service-0"; - private static final String FALLBACK_EDS_NAME = "fallback-" + EDS_NAME; - private static final HttpConnectionManager MAIN_HTTP_CONNECTION_MANAGER = - HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of( - new Filter.NamedFilterConfig(MAIN_SERVER, RouterFilter.ROUTER_CONFIG))); - private static final HttpConnectionManager FALLBACK_HTTP_CONNECTION_MANAGER = - HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of( - new Filter.NamedFilterConfig(FALLBACK_SERVER, RouterFilter.ROUTER_CONFIG))); - private ObjectPool xdsClientPool; - private XdsClient xdsClient; - - private XdsClient.ResourceWatcher raalLdsWatcher = - new XdsClient.ResourceWatcher() { - - @Override - public void onChanged(XdsListenerResource.LdsUpdate update) { - log.log(Level.FINE, "LDS update: " + update); - } - - @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); - } - }; - - @SuppressWarnings("unchecked") - private XdsClient.ResourceWatcher ldsWatcher = - mock(XdsClient.ResourceWatcher.class, delegatesTo(raalLdsWatcher)); - @Mock - private XdsClient.ResourceWatcher ldsWatcher2; - - @Mock - private XdsClient.ResourceWatcher rdsWatcher; - @Mock - private XdsClient.ResourceWatcher rdsWatcher2; - @Mock - private XdsClient.ResourceWatcher rdsWatcher3; - - private XdsClient.ResourceWatcher raalCdsWatcher = - new XdsClient.ResourceWatcher() { - - @Override - public void onChanged(XdsClusterResource.CdsUpdate update) { - log.log(Level.FINE, "CDS update: " + update); - } - - @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); - } - }; - - @SuppressWarnings("unchecked") - private XdsClient.ResourceWatcher cdsWatcher = - mock(XdsClient.ResourceWatcher.class, delegatesTo(raalCdsWatcher)); - @Mock - private XdsClient.ResourceWatcher cdsWatcher2; - - @Rule(order = 0) - public ControlPlaneRule mainTdServer = - new ControlPlaneRule(8090).setServerHostName(MAIN_SERVER); - - @Rule(order = 1) - public ControlPlaneRule fallbackServer = - new ControlPlaneRule(8095).setServerHostName(MAIN_SERVER); - - @Rule public final MockitoRule mocks = MockitoJUnit.rule(); - - @Before - public void setUp() throws XdsInitializationException { - System.setProperty(BootstrapperImpl.GRPC_EXPERIMENTAL_XDS_FALLBACK, "true"); - setAdsConfig(mainTdServer, MAIN_SERVER); - setAdsConfig(fallbackServer, FALLBACK_SERVER); - - SharedXdsClientPoolProvider clientPoolProvider = new SharedXdsClientPoolProvider(); - clientPoolProvider.setBootstrapOverride(defaultBootstrapOverride()); - xdsClientPool = clientPoolProvider.getOrCreate(DUMMY_TARGET); - } - - @After - public void cleanUp() { - xdsClientPool.returnObject(xdsClient); - } - - private static void setAdsConfig(ControlPlaneRule controlPlane, String serverName) { - InetSocketAddress edsInetSocketAddress = - (InetSocketAddress) controlPlane.getServer().getListenSockets().get(0); - boolean isMainServer = serverName.equals(MAIN_SERVER); - String rdsName = isMainServer - ? RDS_NAME - : FALLBACK_RDS_NAME; - String clusterName = isMainServer ? CLUSTER_NAME : FALLBACK_CLUSTER_NAME; - String edsName = isMainServer ? EDS_NAME : FALLBACK_EDS_NAME; - - controlPlane.setLdsConfig(ControlPlaneRule.buildServerListener(), - ControlPlaneRule.buildClientListener(MAIN_SERVER, serverName)); - - controlPlane.setRdsConfig(rdsName, - ControlPlaneRule.buildRouteConfiguration(MAIN_SERVER, rdsName, clusterName)); - controlPlane.setCdsConfig(clusterName, ControlPlaneRule.buildCluster(clusterName, edsName)); - - controlPlane.setEdsConfig(edsName, - ControlPlaneRule.buildClusterLoadAssignment(edsInetSocketAddress.getHostName(), - edsInetSocketAddress.getPort(), edsName)); - log.log(Level.FINE, - String.format("Set ADS config for %s with address %s", serverName, edsInetSocketAddress)); - } - - private void restartServer(TdServerType type) { - switch (type) { - case MAIN: - mainTdServer.restartTdServer(); - setAdsConfig(mainTdServer, MAIN_SERVER); - break; - case FALLBACK: - fallbackServer.restartTdServer(); - setAdsConfig(fallbackServer, FALLBACK_SERVER); - break; - default: - throw new IllegalArgumentException("Unknown server type: " + type); - } - } - - // This is basically a control test to make sure everything is set up correctly. - @Test - public void everything_okay() { - restartServer(TdServerType.MAIN); - restartServer(TdServerType.FALLBACK); - xdsClient = xdsClientPool.getObject(); - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener( - MAIN_HTTP_CONNECTION_MANAGER)); - - xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - verify(rdsWatcher, timeout(5000)).onChanged(any()); - } - - @Test - public void mainServerDown_fallbackServerUp() { - mainTdServer.getServer().shutdownNow(); - restartServer(TdServerType.FALLBACK); - xdsClient = xdsClientPool.getObject(); - log.log(Level.FINE, "Fallback port = " + fallbackServer.getServer().getPort()); - - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener( - FALLBACK_HTTP_CONNECTION_MANAGER)); - } - - @Test - public void both_down_restart_main() { - mainTdServer.getServer().shutdownNow(); - fallbackServer.getServer().shutdownNow(); - xdsClient = xdsClientPool.getObject(); - - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - verify(ldsWatcher, timeout(5000).times(0)).onChanged(any()); - - restartServer(TdServerType.MAIN); - - xdsClient.watchXdsResource( - XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - - verify(ldsWatcher, timeout(300000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - mainTdServer.getServer().shutdownNow(); - } - - @Test - public void mainDown_fallbackUp_restart_main() { - mainTdServer.getServer().shutdownNow(); - fallbackServer.restartTdServer(); - xdsClient = xdsClientPool.getObject(); - 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)); - xdsClient.watchXdsResource(XdsClusterResource.getInstance(), FALLBACK_CLUSTER_NAME, cdsWatcher); - inOrder.verify(cdsWatcher, timeout(5000)).onChanged(any()); - - restartServer(TdServerType.MAIN); - - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - - xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - inOrder.verify(rdsWatcher, timeout(5000)).onChanged(any()); - - xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); - inOrder.verify(cdsWatcher2, timeout(5000)).onChanged(any()); - - // verify that connection to fallback server is closed - assertFalse("Should have disconnected from fallback server", - ((XdsClientImpl) xdsClient).isSubscriberCpcConnected(XdsClusterResource.getInstance(), - FALLBACK_CLUSTER_NAME)); - - } - - // This test takes a long time because of the 16 sec timeout for non-existent resource - @Test - public void connect_then_mainServerDown_fallbackServerUp() { - restartServer(TdServerType.MAIN); - restartServer(TdServerType.FALLBACK); - xdsClient = xdsClientPool.getObject(); - - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - - xdsClient.watchXdsResource(XdsRouteConfigureResource.getInstance(), RDS_NAME, rdsWatcher); - verify(rdsWatcher, timeout(5000)).onChanged(any()); - - mainTdServer.getServer().shutdownNow(); - - // Shouldn't do fallback since all watchers are loaded - verify(ldsWatcher, timeout(5000).times(0)).onChanged( - 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)); - // Make sure that rdsWatcher wasn't called again - verify(rdsWatcher, times(1)).onChanged(any()); - verify(rdsWatcher2, timeout(5000)).onChanged(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(16000)).onChanged(any()); - - xdsClient.watchXdsResource( - XdsRouteConfigureResource.getInstance(), FALLBACK_RDS_NAME, rdsWatcher3); - verify(rdsWatcher3, timeout(5000)).onChanged(any()); - - // Test that resource defined in main but not fallback is handled correctly - xdsClient.watchXdsResource( - XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher2); - verify(cdsWatcher2, timeout(16000)).onResourceDoesNotExist(eq(CLUSTER_NAME)); - } - - @Test - public void connect_then_mainServerRestart_fallbackServerdown() { - restartServer(TdServerType.MAIN); - xdsClient = xdsClientPool.getObject(); - - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), MAIN_SERVER, ldsWatcher); - - verify(ldsWatcher, timeout(5000)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - - mainTdServer.getServer().shutdownNow(); - fallbackServer.getServer().shutdownNow(); - - xdsClient.watchXdsResource(XdsClusterResource.getInstance(), CLUSTER_NAME, cdsWatcher); - restartServer(TdServerType.MAIN); - verify(cdsWatcher, timeout(5000)).onChanged(any()); - verify(ldsWatcher, timeout(5000).times(2)).onChanged( - XdsListenerResource.LdsUpdate.forApiListener(MAIN_HTTP_CONNECTION_MANAGER)); - } - - private Map defaultBootstrapOverride() { - return ImmutableMap.of( - "node", ImmutableMap.of( - "id", UUID.randomUUID().toString(), - "cluster", CLUSTER_NAME), - "xds_servers", ImmutableList.of( - ImmutableMap.of( - "server_uri", "localhost:" + mainTdServer.getServer().getPort(), - "channel_creds", Collections.singletonList( - ImmutableMap.of("type", "insecure") - ), - "server_features", Collections.singletonList("xds_v3") - ), - ImmutableMap.of( - "server_uri", "localhost:" + fallbackServer.getServer().getPort(), - "channel_creds", Collections.singletonList( - ImmutableMap.of("type", "insecure") - ), - "server_features", Collections.singletonList("xds_v3") - ) - ), - "fallback-policy", "fallback" - ); - } - - private enum TdServerType { - MAIN, - FALLBACK - } -} From c7ab615e0ee64cfcaed136f41ef67492ef132420 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 14:07:33 -0700 Subject: [PATCH 04/10] Remove more fallback --- .../java/io/grpc/xds/GrpcXdsTransportFactory.java | 7 ------- xds/src/test/java/io/grpc/xds/CsdsServiceTest.java | 12 ------------ 2 files changed, 19 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java index 4f622791f0d..74c28ba2d2d 100644 --- a/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java +++ b/xds/src/main/java/io/grpc/xds/GrpcXdsTransportFactory.java @@ -22,7 +22,6 @@ import io.grpc.CallOptions; import io.grpc.ChannelCredentials; import io.grpc.ClientCall; -import io.grpc.ConnectivityState; import io.grpc.Context; import io.grpc.Grpc; import io.grpc.ManagedChannel; @@ -85,12 +84,6 @@ public void shutdown() { channel.shutdown(); } - @Override - public boolean isConnected() { - ConnectivityState state = channel.getState(false); - return state == ConnectivityState.READY; - } - private class XdsStreamingCall implements XdsTransportFactory.StreamingCall { diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 11c8c2608af..d66ef532fb2 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -458,23 +458,11 @@ public Collection getSubscribedResources(ServerInfo serverInfo, return null; } - @Nullable - @Override - public Collection getSubscribedResources( - ServerInfo serverInfo, XdsResourceType type, String authority) { - return null; - } - @Override public Map> getSubscribedResourceTypesWithTypeUrl() { return ImmutableMap.of(); } - @Override - public void assignResourcesToOwner(XdsResourceType type, Collection resources, - Object owner) { - // No-op. - } } private static class FakeXdsClientPoolFactory implements XdsClientPoolFactory { From 7ed5914bc756f5de866291c048d3a9da3f5dec32 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 15:11:49 -0700 Subject: [PATCH 05/10] Restore ConsusModulesTest since it wasn't needed for splitting XdsClient --- .../src/test/java/io/grpc/census/CensusModulesTest.java | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/census/src/test/java/io/grpc/census/CensusModulesTest.java b/census/src/test/java/io/grpc/census/CensusModulesTest.java index 9e0b4d935d3..6ccaf78314f 100644 --- a/census/src/test/java/io/grpc/census/CensusModulesTest.java +++ b/census/src/test/java/io/grpc/census/CensusModulesTest.java @@ -56,7 +56,6 @@ import io.grpc.ClientInterceptors; import io.grpc.ClientStreamTracer; import io.grpc.Context; -import io.grpc.KnownLength; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.ServerCall; @@ -100,7 +99,6 @@ import io.opencensus.trace.Tracer; import io.opencensus.trace.propagation.BinaryFormat; import io.opencensus.trace.propagation.SpanContextParseException; -import java.io.IOException; import java.io.InputStream; import java.util.HashSet; import java.util.List; @@ -138,7 +136,7 @@ public class CensusModulesTest { ClientStreamTracer.StreamInfo.newBuilder() .setCallOptions(CallOptions.DEFAULT.withOption(NAME_RESOLUTION_DELAYED, 10L)).build(); - private static class StringInputStream extends InputStream implements KnownLength { + private static class StringInputStream extends InputStream { final String string; StringInputStream(String string) { @@ -151,11 +149,6 @@ public int read() { // passed to the InProcess server and consumed by MARSHALLER.parse(). throw new UnsupportedOperationException("Should not be called"); } - - @Override - public int available() throws IOException { - return string == null ? 0 : string.length(); - } } private static final MethodDescriptor.Marshaller MARSHALLER = From 9f6c61fc98fcb587cc9d048a0c79636d29621b87 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 15:32:19 -0700 Subject: [PATCH 06/10] Add tests for clientScope being set --- xds/src/test/java/io/grpc/xds/CsdsServiceTest.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index d66ef532fb2..0e2a866c8aa 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -86,6 +86,7 @@ public class CsdsServiceTest { private static final XdsResourceType CDS = XdsClusterResource.getInstance(); private static final XdsResourceType RDS = XdsRouteConfigureResource.getInstance(); private static final XdsResourceType EDS = XdsEndpointResource.getInstance(); + public static final String FAKE_CLIENT_SCOPE = "fake"; @RunWith(JUnit4.class) public static class ServiceTests { @@ -273,6 +274,7 @@ private void verifyResponse(ClientStatusResponse response) { ClientConfig clientConfig = response.getConfig(0); verifyClientConfigNode(clientConfig); verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); + assertThat(clientConfig.getClientScope()).isEmpty(); } private void verifyRequestInvalidResponseStatus(Status status) { @@ -351,9 +353,11 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { ); } }; - ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient, "fake"); + ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient, + FAKE_CLIENT_SCOPE); verifyClientConfigNode(clientConfig); + assertThat(clientConfig.getClientScope()).isEqualTo(FAKE_CLIENT_SCOPE); // Minimal verification to confirm that the data/metadata XdsClient provides, // is propagated to the correct resource types. @@ -392,9 +396,10 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { @Test public void getClientConfigForXdsClient_noSubscribedResources() throws InterruptedException { ClientConfig clientConfig = - CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES, "fake"); + CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES, FAKE_CLIENT_SCOPE); verifyClientConfigNode(clientConfig); verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); + assertThat(clientConfig.getClientScope()).isEqualTo(FAKE_CLIENT_SCOPE); } } From 3dd98ca20582fe61d311158cfcccbe1d4c931d5d Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 15:36:21 -0700 Subject: [PATCH 07/10] Restore ControlPlaneRule since it wasn't needed for splitting XdsClient --- .../java/io/grpc/xds/ControlPlaneRule.java | 105 +++--------------- 1 file changed, 16 insertions(+), 89 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index babf3d8358f..1ddf9620434 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -55,7 +55,6 @@ import io.grpc.InsecureServerCredentials; import io.grpc.NameResolverRegistry; import io.grpc.Server; -import java.io.IOException; import java.util.Collections; import java.util.Map; import java.util.UUID; @@ -87,15 +86,9 @@ public class ControlPlaneRule extends TestWatcher { private XdsTestControlPlaneService controlPlaneService; private XdsTestLoadReportingService loadReportingService; private XdsNameResolverProvider nameResolverProvider; - private final int port; public ControlPlaneRule() { - this(0); - } - - public ControlPlaneRule(int port) { serverHostName = "test-server"; - this.port = port; } public ControlPlaneRule setServerHostName(String serverHostName) { @@ -122,7 +115,11 @@ public Server getServer() { try { controlPlaneService = new XdsTestControlPlaneService(); loadReportingService = new XdsTestLoadReportingService(); - createAndStartTdServer(); + server = Grpc.newServerBuilderForPort(0, InsecureServerCredentials.create()) + .addService(controlPlaneService) + .addService(loadReportingService) + .build() + .start(); } catch (Exception e) { throw new AssertionError("unable to start the control plane server", e); } @@ -147,38 +144,6 @@ public Server getServer() { NameResolverRegistry.getDefaultRegistry().deregister(nameResolverProvider); } - /** - * Will shutdown existing server if needed. - * Then creates a new server in the same way as {@link #starting(Description)} and starts it. - */ - public void restartTdServer() { - - if (getServer() != null && !getServer().isShutdown()) { - getServer().shutdownNow(); - try { - if (!getServer().awaitTermination(5, TimeUnit.SECONDS)) { - logger.log(Level.SEVERE, "Timed out waiting for server shutdown"); - } - } catch (InterruptedException e) { - throw new AssertionError("unable to shut down control plane server", e); - } - } - - try { - createAndStartTdServer(); - } catch (Exception e) { - throw new AssertionError("unable to restart the control plane server", e); - } - } - - private void createAndStartTdServer() throws IOException { - server = Grpc.newServerBuilderForPort(port, InsecureServerCredentials.create()) - .addService(controlPlaneService) - .addService(loadReportingService) - .build() - .start(); - } - /** * For test purpose, use boostrapOverride to programmatically provide bootstrap info. */ @@ -208,67 +173,44 @@ void setLdsConfig(Listener serverListener, Listener clientListener) { } void setRdsConfig(RouteConfiguration routeConfiguration) { - setRdsConfig(RDS_NAME, routeConfiguration); - } - - public void setRdsConfig(String rdsName, RouteConfiguration routeConfiguration) { - getService().setXdsConfig(ADS_TYPE_URL_RDS, ImmutableMap.of(rdsName, routeConfiguration)); + getService().setXdsConfig(ADS_TYPE_URL_RDS, ImmutableMap.of(RDS_NAME, routeConfiguration)); } void setCdsConfig(Cluster cluster) { - setCdsConfig(CLUSTER_NAME, cluster); - } - - void setCdsConfig(String clusterName, Cluster cluster) { getService().setXdsConfig(ADS_TYPE_URL_CDS, - ImmutableMap.of(clusterName, cluster)); + ImmutableMap.of(CLUSTER_NAME, cluster)); } void setEdsConfig(ClusterLoadAssignment clusterLoadAssignment) { - setEdsConfig(EDS_NAME, clusterLoadAssignment); - } - - void setEdsConfig(String edsName, ClusterLoadAssignment clusterLoadAssignment) { getService().setXdsConfig(ADS_TYPE_URL_EDS, - ImmutableMap.of(edsName, clusterLoadAssignment)); + ImmutableMap.of(EDS_NAME, clusterLoadAssignment)); } /** * Builds a new default RDS configuration. */ static RouteConfiguration buildRouteConfiguration(String authority) { - return buildRouteConfiguration(authority, RDS_NAME, CLUSTER_NAME); - } - - static RouteConfiguration buildRouteConfiguration(String authority, String rdsName, - String clusterName) { - VirtualHost.Builder vhBuilder = VirtualHost.newBuilder() - .setName(rdsName) + io.envoyproxy.envoy.config.route.v3.VirtualHost virtualHost = VirtualHost.newBuilder() .addDomains(authority) .addRoutes( Route.newBuilder() .setMatch( RouteMatch.newBuilder().setPrefix("/").build()) .setRoute( - RouteAction.newBuilder().setCluster(clusterName).build()).build()); - VirtualHost virtualHost = vhBuilder.build(); - return RouteConfiguration.newBuilder().setName(rdsName).addVirtualHosts(virtualHost).build(); + RouteAction.newBuilder().setCluster(CLUSTER_NAME).build()).build()).build(); + return RouteConfiguration.newBuilder().setName(RDS_NAME).addVirtualHosts(virtualHost).build(); } /** * Builds a new default CDS configuration. */ static Cluster buildCluster() { - return buildCluster(CLUSTER_NAME, EDS_NAME); - } - - static Cluster buildCluster(String clusterName, String edsName) { return Cluster.newBuilder() - .setName(clusterName) + .setName(CLUSTER_NAME) .setType(Cluster.DiscoveryType.EDS) .setEdsClusterConfig( Cluster.EdsClusterConfig.newBuilder() - .setServiceName(edsName) + .setServiceName(EDS_NAME) .setEdsConfig( ConfigSource.newBuilder() .setAds(AggregatedConfigSource.newBuilder().build()) @@ -282,11 +224,6 @@ static Cluster buildCluster(String clusterName, String edsName) { * Builds a new default EDS configuration. */ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int port) { - return buildClusterLoadAssignment(hostName, port, EDS_NAME); - } - - static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int port, - String edsName) { Address address = Address.newBuilder() .setSocketAddress( SocketAddress.newBuilder().setAddress(hostName).setPortValue(port).build()).build(); @@ -300,7 +237,7 @@ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int por .setHealthStatus(HealthStatus.HEALTHY) .build()).build(); return ClusterLoadAssignment.newBuilder() - .setClusterName(edsName) + .setClusterName(EDS_NAME) .addEndpoints(endpoints) .build(); } @@ -309,17 +246,8 @@ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, int por * Builds a new client listener. */ static Listener buildClientListener(String name) { - return buildClientListener(name, "terminal-filter"); - } - - - static Listener buildClientListener(String name, String identifier) { - return buildClientListener(name, identifier, RDS_NAME); - } - - static Listener buildClientListener(String name, String identifier, String rdsName) { HttpFilter httpFilter = HttpFilter.newBuilder() - .setName(identifier) + .setName("terminal-filter") .setTypedConfig(Any.pack(Router.newBuilder().build())) .setIsOptional(true) .build(); @@ -328,7 +256,7 @@ static Listener buildClientListener(String name, String identifier, String rdsNa .HttpConnectionManager.newBuilder() .setRds( Rds.newBuilder() - .setRouteConfigName(rdsName) + .setRouteConfigName(RDS_NAME) .setConfigSource( ConfigSource.newBuilder() .setAds(AggregatedConfigSource.getDefaultInstance()))) @@ -384,5 +312,4 @@ static Listener buildServerListener() { .addFilterChains(filterChain) .build(); } - } From 21f30f4425a441da33fa9fb4ea4f0fcff1e1c654 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 18:09:21 -0700 Subject: [PATCH 08/10] Add test for multiple xds clients used by CsdsService --- .../java/io/grpc/xds/CsdsServiceTest.java | 64 +++++++++++++++++-- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 0e2a866c8aa..9651519dcc3 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -54,8 +54,8 @@ import io.grpc.xds.client.XdsClient.ResourceMetadata; import io.grpc.xds.client.XdsClient.ResourceMetadata.ResourceMetadataStatus; import io.grpc.xds.client.XdsResourceType; +import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -269,6 +269,31 @@ public void streamClientStatus_onClientError() { assertThat(responseObserver.getError()).isNull(); } + @Test + public void multipleXdsClients() { + FakeXdsClient xdsClient1 = new FakeXdsClient(); + FakeXdsClient xdsClient2 = new FakeXdsClient(); + Map clientMap = new HashMap<>(); + clientMap.put("target1", xdsClient1); + clientMap.put("target2", xdsClient2); + FakeXdsClientPoolFactory factory = new FakeXdsClientPoolFactory(clientMap); + CsdsService csdsService = new CsdsService(factory); + grpcServerRule.getServiceRegistry().addService(csdsService); + + StreamRecorder responseObserver = StreamRecorder.create(); + StreamObserver requestObserver = + csdsAsyncStub.streamClientStatus(responseObserver); + + requestObserver.onNext(REQUEST); + requestObserver.onCompleted(); + + List responses = responseObserver.getValues(); + assertThat(responses).hasSize(1); + Collection targets = verifyMultiResponse(responses.get(0), 2); + assertThat(targets).containsExactly("target1", "target2"); + responseObserver.onCompleted(); + } + private void verifyResponse(ClientStatusResponse response) { assertThat(response.getConfigCount()).isEqualTo(1); ClientConfig clientConfig = response.getConfig(0); @@ -277,6 +302,20 @@ private void verifyResponse(ClientStatusResponse response) { assertThat(clientConfig.getClientScope()).isEmpty(); } + private Collection verifyMultiResponse(ClientStatusResponse response, int numExpected) { + assertThat(response.getConfigCount()).isEqualTo(numExpected); + + List clientScopes = new ArrayList<>(); + for (int i = 0; i < numExpected; i++) { + ClientConfig clientConfig = response.getConfig(i); + verifyClientConfigNode(clientConfig); + verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); + clientScopes.add(clientConfig.getClientScope()); + } + + return clientScopes; + } + private void verifyRequestInvalidResponseStatus(Status status) { assertThat(status.getCode()).isEqualTo(Code.INVALID_ARGUMENT); assertThat(status.getDescription()).isEqualTo("node_matchers not supported"); @@ -471,20 +510,31 @@ public Map> getSubscribedResourceTypesWithTypeUrl() { } private static class FakeXdsClientPoolFactory implements XdsClientPoolFactory { - @Nullable private final XdsClient xdsClient; - private static final List TARGETS = Collections.singletonList(""); + private final Map xdsClientMap = new HashMap<>(); + private boolean isOldStyle + ; private FakeXdsClientPoolFactory(@Nullable XdsClient xdsClient) { - this.xdsClient = xdsClient; + if (xdsClient != null) { + xdsClientMap.put("", xdsClient); + } + isOldStyle = true; + } + + private FakeXdsClientPoolFactory(Map xdsClientMap) { + this.xdsClientMap.putAll(xdsClientMap); + isOldStyle = false; } @Override @Nullable - public ObjectPool get(String notUsedTarget) { + public ObjectPool get(String target) { + String targetToUse = isOldStyle ? "" : target; + return new ObjectPool() { @Override public XdsClient getObject() { - return xdsClient; + return xdsClientMap.get(targetToUse); } @Override @@ -496,7 +546,7 @@ public XdsClient returnObject(Object object) { @Override public List getTargets() { - return TARGETS; + return new ArrayList<>(xdsClientMap.keySet()); } @Override From fd039befbd8500a180ab25e48e8bacc32a38f043 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Wed, 21 Aug 2024 18:10:41 -0700 Subject: [PATCH 09/10] Reset deadline to 3 seconds instead of 30. --- xds/src/test/java/io/grpc/xds/CsdsServiceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index 9651519dcc3..63b9cda043c 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -107,7 +107,7 @@ public void setUp() { // because true->false return mutation prevents fetchClientStatus from completing the request. csdsStub = ClientStatusDiscoveryServiceGrpc .newBlockingStub(grpcServerRule.getChannel()) - .withDeadline(Deadline.after(30, TimeUnit.SECONDS)); + .withDeadline(Deadline.after(3, TimeUnit.SECONDS)); csdsAsyncStub = ClientStatusDiscoveryServiceGrpc.newStub(grpcServerRule.getChannel()); } From 10071ce81e034af4291da5905fad85a3f3306133 Mon Sep 17 00:00:00 2001 From: Larry Safran Date: Thu, 22 Aug 2024 18:15:46 -0700 Subject: [PATCH 10/10] Remove unnecessary try/catch as requested by code review. --- .../grpc/xds/SharedXdsClientPoolProvider.java | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java index 6ddcd559a9f..c9195896d82 100644 --- a/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java +++ b/xds/src/main/java/io/grpc/xds/SharedXdsClientPoolProvider.java @@ -146,19 +146,15 @@ public XdsClient getObject() { log.log(Level.INFO, "xDS node ID: {0}", bootstrapInfo.node().getId()); } scheduler = SharedResourceHolder.get(GrpcUtil.TIMER_SERVICE); - try { - xdsClient = new XdsClientImpl( - DEFAULT_XDS_TRANSPORT_FACTORY, - bootstrapInfo, - scheduler, - BACKOFF_POLICY_PROVIDER, - GrpcUtil.STOPWATCH_SUPPLIER, - TimeProvider.SYSTEM_TIME_PROVIDER, - MessagePrinter.INSTANCE, - new TlsContextManagerImpl(bootstrapInfo)); - } catch (Exception e) { - throw new RuntimeException(e); - } + xdsClient = new XdsClientImpl( + DEFAULT_XDS_TRANSPORT_FACTORY, + bootstrapInfo, + scheduler, + BACKOFF_POLICY_PROVIDER, + GrpcUtil.STOPWATCH_SUPPLIER, + TimeProvider.SYSTEM_TIME_PROVIDER, + MessagePrinter.INSTANCE, + new TlsContextManagerImpl(bootstrapInfo)); } refCount++; return xdsClient;