diff --git a/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java b/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java index 329e912e08..d7eb9383f0 100644 --- a/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java +++ b/xds/src/main/java/io/grpc/xds/AbstractXdsClient.java @@ -19,14 +19,6 @@ package io.grpc.xds; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; -import static io.grpc.xds.XdsClusterResource.ADS_TYPE_URL_CDS; -import static io.grpc.xds.XdsClusterResource.ADS_TYPE_URL_CDS_V2; -import static io.grpc.xds.XdsEndpointResource.ADS_TYPE_URL_EDS; -import static io.grpc.xds.XdsEndpointResource.ADS_TYPE_URL_EDS_V2; -import static io.grpc.xds.XdsListenerResource.ADS_TYPE_URL_LDS; -import static io.grpc.xds.XdsListenerResource.ADS_TYPE_URL_LDS_V2; -import static io.grpc.xds.XdsRouteConfigureResource.ADS_TYPE_URL_RDS; -import static io.grpc.xds.XdsRouteConfigureResource.ADS_TYPE_URL_RDS_V2; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Stopwatch; @@ -54,8 +46,10 @@ import io.grpc.xds.XdsLogger.XdsLogLevel; import java.util.Collection; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -235,7 +229,9 @@ final class AbstractXdsClient { return; } startRpcStream(); - for (XdsResourceType type : resourceStore.getXdsResourceTypes()) { + Set> subscribedResourceTypes = + new HashSet<>(resourceStore.getSubscribedResourceTypesWithTypeUrl().values()); + for (XdsResourceType type : subscribedResourceTypes) { Collection resources = resourceStore.getSubscribedResources(serverInfo, type); if (resources != null) { adsStream.sendDiscoveryRequest(type, resources); @@ -247,27 +243,8 @@ final class AbstractXdsClient { @VisibleForTesting @Nullable - static XdsResourceType fromTypeUrl(String typeUrl) { - switch (typeUrl) { - case ADS_TYPE_URL_LDS: - // fall trough - case ADS_TYPE_URL_LDS_V2: - return XdsListenerResource.getInstance(); - case ADS_TYPE_URL_RDS: - // fall through - case ADS_TYPE_URL_RDS_V2: - return XdsRouteConfigureResource.getInstance(); - case ADS_TYPE_URL_CDS: - // fall through - case ADS_TYPE_URL_CDS_V2: - return XdsClusterResource.getInstance(); - case ADS_TYPE_URL_EDS: - // fall through - case ADS_TYPE_URL_EDS_V2: - return XdsEndpointResource.getInstance(); - default: - return null; - } + XdsResourceType fromTypeUrl(String typeUrl) { + return resourceStore.getSubscribedResourceTypesWithTypeUrl().get(typeUrl); } private abstract class AbstractAdsStream { diff --git a/xds/src/main/java/io/grpc/xds/XdsClient.java b/xds/src/main/java/io/grpc/xds/XdsClient.java index 3f4c98ef69..82af865115 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClient.java +++ b/xds/src/main/java/io/grpc/xds/XdsClient.java @@ -371,6 +371,6 @@ abstract class XdsClient { Collection getSubscribedResources(ServerInfo serverInfo, XdsResourceType type); - Collection> getXdsResourceTypes(); + Map> getSubscribedResourceTypesWithTypeUrl(); } } diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index 41195c5cbe..c1d6290a5f 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -91,6 +91,7 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou private final Map, Map>> resourceSubscribers = new HashMap<>(); + private final Map> subscribedResourceTypeUrls = new HashMap<>(); private final LoadStatsManager2 loadStatsManager; private final Map serverLrsClientMap = new HashMap<>(); private final XdsChannelFactory xdsChannelFactory; @@ -228,8 +229,8 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou } @Override - public Collection> getXdsResourceTypes() { - return resourceSubscribers.keySet(); + public Map> getSubscribedResourceTypesWithTypeUrl() { + return Collections.unmodifiableMap(subscribedResourceTypeUrls); } @Nullable @@ -289,6 +290,8 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou public void run() { if (!resourceSubscribers.containsKey(type)) { resourceSubscribers.put(type, new HashMap<>()); + subscribedResourceTypeUrls.put(type.typeUrl(), type); + subscribedResourceTypeUrls.put(type.typeUrlV2(), type); } ResourceSubscriber subscriber = (ResourceSubscriber) resourceSubscribers.get(type).get(resourceName);; @@ -319,6 +322,8 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou if (!subscriber.isWatched()) { subscriber.cancelResourceWatch(); resourceSubscribers.get(type).remove(resourceName); + subscribedResourceTypeUrls.remove(type.typeUrl()); + subscribedResourceTypeUrls.remove(type.typeUrlV2()); if (subscriber.xdsChannel != null) { subscriber.xdsChannel.adjustResourceSubscription(type); } diff --git a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java index b37999d38e..5272a6d297 100644 --- a/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java +++ b/xds/src/test/java/io/grpc/xds/CsdsServiceTest.java @@ -50,6 +50,7 @@ import io.grpc.xds.Bootstrapper.ServerInfo; import io.grpc.xds.XdsClient.ResourceMetadata; import io.grpc.xds.XdsClient.ResourceMetadata.ResourceMetadataStatus; import io.grpc.xds.XdsNameResolverProvider.XdsClientPoolFactory; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -75,7 +76,7 @@ public class CsdsServiceTest { ServerInfo.create(SERVER_URI, InsecureChannelCredentials.create(), true))) .node(BOOTSTRAP_NODE) .build(); - private static final XdsClient XDS_CLIENT_NO_RESOURCES = new FakeXdsClient(); + private static final FakeXdsClient XDS_CLIENT_NO_RESOURCES = new FakeXdsClient(); private static final XdsResourceType LDS = XdsListenerResource.getInstance(); private static final XdsResourceType CDS = XdsClusterResource.getInstance(); private static final XdsResourceType RDS = XdsRouteConfigureResource.getInstance(); @@ -263,7 +264,7 @@ public class CsdsServiceTest { assertThat(response.getConfigCount()).isEqualTo(1); ClientConfig clientConfig = response.getConfig(0); verifyClientConfigNode(clientConfig); - verifyClientConfigNoResources(clientConfig); + verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); } private void verifyRequestInvalidResponseStatus(Status status) { @@ -320,7 +321,7 @@ public class CsdsServiceTest { @Test public void getClientConfigForXdsClient_subscribedResourcesToGenericXdsConfig() throws InterruptedException { - ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(new FakeXdsClient() { + FakeXdsClient fakeXdsClient = new FakeXdsClient() { @Override protected Map, Map> getSubscribedResourcesMetadata() { @@ -331,7 +332,18 @@ public class CsdsServiceTest { .put(EDS, ImmutableMap.of("subscribedResourceName.EDS", METADATA_ACKED_EDS)) .buildOrThrow(); } - }); + + @Override + public Map> getSubscribedResourceTypesWithTypeUrl() { + return ImmutableMap.of( + LDS.typeUrl(), LDS, + RDS.typeUrl(), RDS, + CDS.typeUrl(), CDS, + EDS.typeUrl(), EDS + ); + } + }; + ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient); verifyClientConfigNode(clientConfig); @@ -339,7 +351,8 @@ public class CsdsServiceTest { // is propagated to the correct resource types. int xdsConfigCount = clientConfig.getGenericXdsConfigsCount(); assertThat(xdsConfigCount).isEqualTo(4); - Map, GenericXdsConfig> configDumps = mapConfigDumps(clientConfig); + Map, GenericXdsConfig> configDumps = mapConfigDumps(fakeXdsClient, + clientConfig); assertThat(configDumps.keySet()).containsExactly(LDS, RDS, CDS, EDS); // LDS. @@ -372,7 +385,7 @@ public class CsdsServiceTest { public void getClientConfigForXdsClient_noSubscribedResources() throws InterruptedException { ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES); verifyClientConfigNode(clientConfig); - verifyClientConfigNoResources(clientConfig); + verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig); } } @@ -380,10 +393,11 @@ public class CsdsServiceTest { * Assuming {@link MetadataToProtoTests} passes, and metadata converted to corresponding * config dumps correctly, perform a minimal verification of the general shape of ClientConfig. */ - private static void verifyClientConfigNoResources(ClientConfig clientConfig) { + private static void verifyClientConfigNoResources(FakeXdsClient xdsClient, + ClientConfig clientConfig) { int xdsConfigCount = clientConfig.getGenericXdsConfigsCount(); assertThat(xdsConfigCount).isEqualTo(0); - Map, GenericXdsConfig> configDumps = mapConfigDumps(clientConfig); + Map, GenericXdsConfig> configDumps = mapConfigDumps(xdsClient, clientConfig); assertThat(configDumps).isEmpty(); } @@ -397,11 +411,13 @@ public class CsdsServiceTest { assertThat(node).isEqualTo(BOOTSTRAP_NODE.toEnvoyProtoNode()); } - private static Map, GenericXdsConfig> mapConfigDumps(ClientConfig config) { + private static Map, GenericXdsConfig> mapConfigDumps(FakeXdsClient client, + ClientConfig config) { Map, GenericXdsConfig> xdsConfigMap = new HashMap<>(); List xdsConfigList = config.getGenericXdsConfigsList(); for (GenericXdsConfig genericXdsConfig : xdsConfigList) { - XdsResourceType type = AbstractXdsClient.fromTypeUrl(genericXdsConfig.getTypeUrl()); + XdsResourceType type = client.getSubscribedResourceTypesWithTypeUrl() + .get(genericXdsConfig.getTypeUrl()); assertThat(type).isNotNull(); assertThat(xdsConfigMap).doesNotContainKey(type); xdsConfigMap.put(type, genericXdsConfig); @@ -409,7 +425,7 @@ public class CsdsServiceTest { return xdsConfigMap; } - private static class FakeXdsClient extends XdsClient { + private static class FakeXdsClient extends XdsClient implements XdsClient.ResourceStore { protected Map, Map> getSubscribedResourcesMetadata() { return ImmutableMap.of(); @@ -425,6 +441,18 @@ public class CsdsServiceTest { BootstrapInfo getBootstrapInfo() { return BOOTSTRAP_INFO; } + + @Nullable + @Override + public Collection getSubscribedResources(ServerInfo serverInfo, + XdsResourceType type) { + return null; + } + + @Override + public Map> getSubscribedResourceTypesWithTypeUrl() { + return ImmutableMap.of(); + } } private static class FakeXdsClientPoolFactory implements XdsClientPoolFactory { diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java index f50ed2a923..31d10abd84 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java @@ -400,19 +400,26 @@ public abstract class XdsClientImplTestBase { int ldsSize, int cdsSize, int rdsSize, int edsSize) { Map, Map> subscribedResourcesMetadata = awaitSubscribedResourcesMetadata(); - verifyResourceCount(subscribedResourcesMetadata, LDS, ldsSize); - verifyResourceCount(subscribedResourcesMetadata, CDS, cdsSize); - verifyResourceCount(subscribedResourcesMetadata, RDS, rdsSize); - verifyResourceCount(subscribedResourcesMetadata, EDS, edsSize); + Map> subscribedTypeUrls = + xdsClient.getSubscribedResourceTypesWithTypeUrl(); + verifyResourceCount(subscribedTypeUrls, subscribedResourcesMetadata, LDS, ldsSize); + verifyResourceCount(subscribedTypeUrls, subscribedResourcesMetadata, CDS, cdsSize); + verifyResourceCount(subscribedTypeUrls, subscribedResourcesMetadata, RDS, rdsSize); + verifyResourceCount(subscribedTypeUrls, subscribedResourcesMetadata, EDS, edsSize); } private void verifyResourceCount( + Map> subscribedTypeUrls, Map, Map> subscribedResourcesMetadata, XdsResourceType type, int size) { if (size == 0) { + assertThat(subscribedTypeUrls.containsKey(type.typeUrl())).isFalse(); + assertThat(subscribedTypeUrls.containsKey(type.typeUrlV2())).isFalse(); assertThat(subscribedResourcesMetadata.containsKey(type)).isFalse(); } else { + assertThat(subscribedTypeUrls.containsKey(type.typeUrl())).isTrue(); + assertThat(subscribedTypeUrls.containsKey(type.typeUrlV2())).isTrue(); assertThat(subscribedResourcesMetadata.get(type)).hasSize(size); } }