diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index a30b4755a1..f05210b51c 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -48,8 +48,6 @@ import io.grpc.xds.LoadStatsManager2.ClusterDropStats; import io.grpc.xds.LoadStatsManager2.ClusterLocalityStats; import io.grpc.xds.XdsClient.ResourceStore; import io.grpc.xds.XdsClient.XdsResponseHandler; -import io.grpc.xds.XdsClusterResource.CdsUpdate; -import io.grpc.xds.XdsListenerResource.LdsUpdate; import io.grpc.xds.XdsLogger.XdsLogLevel; import java.net.URI; import java.util.Collection; @@ -107,7 +105,6 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou private final XdsLogger logger; private volatile boolean isShutdown; - // TODO(zdapeng): rename to XdsClientImpl XdsClientImpl( XdsChannelFactory xdsChannelFactory, Bootstrapper.BootstrapInfo bootstrapInfo, @@ -398,7 +395,6 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou xdsResourceType.typeName(), args.versionInfo, args.nonce, result.unpackedResources); Map> parsedResources = result.parsedResources; Set invalidResources = result.invalidResources; - Set retainedResources = result.retainedResources; List errors = result.errors; String errorDetail = null; if (errors.isEmpty()) { @@ -432,16 +428,14 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou } // Nothing else to do for incremental ADS resources. - if (xdsResourceType.dependentResource() == null) { + if (!xdsResourceType.isFullStateOfTheWorld()) { continue; } // Handle State of the World ADS: invalid resources. if (invalidResources.contains(resourceName)) { // The resource is missing. Reuse the cached resource if possible. - if (subscriber.data != null) { - retainDependentResource(subscriber, retainedResources); - } else { + if (subscriber.data == null) { // No cached data. Notify the watchers of an invalid update. subscriber.onError(Status.UNAVAILABLE.withDescription(errorDetail)); } @@ -451,49 +445,6 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou // For State of the World services, notify watchers when their watched resource is missing // from the ADS update. subscriber.onAbsent(); - // Retain any dependent resources if the resource deletion is ignored - // per bootstrap ignore_resource_deletion server feature. - if (!subscriber.absent) { - retainDependentResource(subscriber, retainedResources); - } - } - - // LDS/CDS responses represents the state of the world, RDS/EDS resources not referenced in - // LDS/CDS resources should be deleted. - if (xdsResourceType.dependentResource() != null) { - XdsResourceType dependency = xdsResourceType.dependentResource(); - Map> dependentSubscribers = - resourceSubscribers.get(dependency); - if (dependentSubscribers == null) { - return; - } - for (String resource : dependentSubscribers.keySet()) { - if (!retainedResources.contains(resource)) { - dependentSubscribers.get(resource).onAbsent(); - } - } - } - } - - private void retainDependentResource( - ResourceSubscriber subscriber, Set retainedResources) { - if (subscriber.data == null) { - return; - } - String resourceName = null; - if (subscriber.type == XdsListenerResource.getInstance()) { - LdsUpdate ldsUpdate = (LdsUpdate) subscriber.data; - io.grpc.xds.HttpConnectionManager hcm = ldsUpdate.httpConnectionManager(); - if (hcm != null) { - resourceName = hcm.rdsName(); - } - } else if (subscriber.type == XdsClusterResource.getInstance()) { - CdsUpdate cdsUpdate = (CdsUpdate) subscriber.data; - resourceName = cdsUpdate.edsServiceName(); - } - - if (resourceName != null) { - retainedResources.add(resourceName); } } @@ -657,9 +608,7 @@ final class XdsClientImpl extends XdsClient implements XdsResponseHandler, Resou // and the resource is reusable. boolean ignoreResourceDeletionEnabled = serverInfo != null && serverInfo.ignoreResourceDeletion(); - boolean isStateOfTheWorld = (type == XdsListenerResource.getInstance() - || type == XdsClusterResource.getInstance()); - if (ignoreResourceDeletionEnabled && isStateOfTheWorld && data != null) { + if (ignoreResourceDeletionEnabled && type.isFullStateOfTheWorld() && data != null) { if (!resourceDeletionIgnored) { logger.log(XdsLogLevel.FORCE_WARNING, "xds server {0}: ignoring deletion for resource type {1} name {2}}", diff --git a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java index 82a977f7df..c0d6ebeefd 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClusterResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsClusterResource.java @@ -88,10 +88,9 @@ class XdsClusterResource extends XdsResourceType { return ADS_TYPE_URL_CDS_V2; } - @Nullable @Override - XdsResourceType dependentResource() { - return XdsEndpointResource.getInstance(); + boolean isFullStateOfTheWorld() { + return true; } @Override @@ -101,8 +100,7 @@ class XdsClusterResource extends XdsResourceType { } @Override - CdsUpdate doParse(Args args, Message unpackedMessage, - Set retainedResources, boolean isResourceV3) + CdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3) throws ResourceInvalidException { if (!(unpackedMessage instanceof Cluster)) { throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass()); @@ -111,12 +109,12 @@ class XdsClusterResource extends XdsResourceType { if (args.bootstrapInfo != null && args.bootstrapInfo.certProviders() != null) { certProviderInstances = args.bootstrapInfo.certProviders().keySet(); } - return processCluster((Cluster) unpackedMessage, retainedResources, certProviderInstances, + return processCluster((Cluster) unpackedMessage, certProviderInstances, args.serverInfo, args.loadBalancerRegistry); } @VisibleForTesting - static CdsUpdate processCluster(Cluster cluster, Set retainedEdsResources, + static CdsUpdate processCluster(Cluster cluster, Set certProviderInstances, Bootstrapper.ServerInfo serverInfo, LoadBalancerRegistry loadBalancerRegistry) @@ -124,7 +122,7 @@ class XdsClusterResource extends XdsResourceType { StructOrError structOrError; switch (cluster.getClusterDiscoveryTypeCase()) { case TYPE: - structOrError = parseNonAggregateCluster(cluster, retainedEdsResources, + structOrError = parseNonAggregateCluster(cluster, certProviderInstances, serverInfo); break; case CLUSTER_TYPE: @@ -178,8 +176,7 @@ class XdsClusterResource extends XdsResourceType { } private static StructOrError parseNonAggregateCluster( - Cluster cluster, Set edsResources, Set certProviderInstances, - Bootstrapper.ServerInfo serverInfo) { + Cluster cluster, Set certProviderInstances, Bootstrapper.ServerInfo serverInfo) { String clusterName = cluster.getName(); Bootstrapper.ServerInfo lrsServerInfo = null; Long maxConcurrentRequests = null; @@ -249,9 +246,6 @@ class XdsClusterResource extends XdsResourceType { // If the service_name field is set, that value will be used for the EDS request. if (!edsClusterConfig.getServiceName().isEmpty()) { edsServiceName = edsClusterConfig.getServiceName(); - edsResources.add(edsServiceName); - } else { - edsResources.add(clusterName); } return StructOrError.fromStruct(CdsUpdate.forEds( clusterName, edsServiceName, lrsServerInfo, maxConcurrentRequests, upstreamTlsContext, diff --git a/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java b/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java index c126d64331..a44fea0c57 100644 --- a/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsEndpointResource.java @@ -78,10 +78,9 @@ class XdsEndpointResource extends XdsResourceType { return ADS_TYPE_URL_EDS_V2; } - @Nullable @Override - XdsResourceType dependentResource() { - return null; + boolean isFullStateOfTheWorld() { + return false; } @Override @@ -90,8 +89,7 @@ class XdsEndpointResource extends XdsResourceType { } @Override - EdsUpdate doParse(Args args, Message unpackedMessage, - Set retainedResources, boolean isResourceV3) + EdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3) throws ResourceInvalidException { if (!(unpackedMessage instanceof ClusterLoadAssignment)) { throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass()); diff --git a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java index 5f7d6a27aa..f367bd9690 100644 --- a/xds/src/main/java/io/grpc/xds/XdsListenerResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsListenerResource.java @@ -97,15 +97,13 @@ class XdsListenerResource extends XdsResourceType { return ADS_TYPE_URL_LDS_V2; } - @Nullable @Override - XdsResourceType dependentResource() { - return XdsRouteConfigureResource.getInstance(); + boolean isFullStateOfTheWorld() { + return true; } @Override - LdsUpdate doParse(Args args, Message unpackedMessage, Set retainedResources, - boolean isResourceV3) + LdsUpdate doParse(Args args, Message unpackedMessage, boolean isResourceV3) throws ResourceInvalidException { if (!(unpackedMessage instanceof Listener)) { throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass()); @@ -114,15 +112,14 @@ class XdsListenerResource extends XdsResourceType { if (listener.hasApiListener()) { return processClientSideListener( - listener, retainedResources, args, enableFaultInjection && isResourceV3); + listener, args, enableFaultInjection && isResourceV3); } else { return processServerSideListener( - listener, retainedResources, args, enableRbac && isResourceV3); + listener, args, enableRbac && isResourceV3); } } - private LdsUpdate processClientSideListener( - Listener listener, Set rdsResources, Args args, boolean parseHttpFilter) + private LdsUpdate processClientSideListener(Listener listener, Args args, boolean parseHttpFilter) throws ResourceInvalidException { // Unpack HttpConnectionManager from the Listener. HttpConnectionManager hcm; @@ -135,24 +132,22 @@ class XdsListenerResource extends XdsResourceType { "Could not parse HttpConnectionManager config from ApiListener", e); } return LdsUpdate.forApiListener(parseHttpConnectionManager( - hcm, rdsResources, args.filterRegistry, parseHttpFilter, true /* isForClient */)); + hcm, args.filterRegistry, parseHttpFilter, true /* isForClient */)); } - private LdsUpdate processServerSideListener( - Listener proto, Set rdsResources, Args args, boolean parseHttpFilter) + private LdsUpdate processServerSideListener(Listener proto, Args args, boolean parseHttpFilter) throws ResourceInvalidException { Set certProviderInstances = null; if (args.bootstrapInfo != null && args.bootstrapInfo.certProviders() != null) { certProviderInstances = args.bootstrapInfo.certProviders().keySet(); } - return LdsUpdate.forTcpListener(parseServerSideListener( - proto, rdsResources, args.tlsContextManager, args.filterRegistry, certProviderInstances, - parseHttpFilter)); + return LdsUpdate.forTcpListener(parseServerSideListener(proto, args.tlsContextManager, + args.filterRegistry, certProviderInstances, parseHttpFilter)); } @VisibleForTesting static EnvoyServerProtoData.Listener parseServerSideListener( - Listener proto, Set rdsResources, TlsContextManager tlsContextManager, + Listener proto, TlsContextManager tlsContextManager, FilterRegistry filterRegistry, Set certProviderInstances, boolean parseHttpFilter) throws ResourceInvalidException { if (!proto.getTrafficDirection().equals(TrafficDirection.INBOUND) @@ -190,13 +185,13 @@ class XdsListenerResource extends XdsResourceType { Set uniqueSet = new HashSet<>(); for (io.envoyproxy.envoy.config.listener.v3.FilterChain fc : proto.getFilterChainsList()) { filterChains.add( - parseFilterChain(fc, rdsResources, tlsContextManager, filterRegistry, uniqueSet, + parseFilterChain(fc, tlsContextManager, filterRegistry, uniqueSet, certProviderInstances, parseHttpFilter)); } FilterChain defaultFilterChain = null; if (proto.hasDefaultFilterChain()) { defaultFilterChain = parseFilterChain( - proto.getDefaultFilterChain(), rdsResources, tlsContextManager, filterRegistry, + proto.getDefaultFilterChain(), tlsContextManager, filterRegistry, null, certProviderInstances, parseHttpFilter); } @@ -206,7 +201,7 @@ class XdsListenerResource extends XdsResourceType { @VisibleForTesting static FilterChain parseFilterChain( - io.envoyproxy.envoy.config.listener.v3.FilterChain proto, Set rdsResources, + io.envoyproxy.envoy.config.listener.v3.FilterChain proto, TlsContextManager tlsContextManager, FilterRegistry filterRegistry, Set uniqueSet, Set certProviderInstances, boolean parseHttpFilters) throws ResourceInvalidException { @@ -235,7 +230,7 @@ class XdsListenerResource extends XdsResourceType { + filter.getName() + " failed to unpack message", e); } io.grpc.xds.HttpConnectionManager httpConnectionManager = parseHttpConnectionManager( - hcmProto, rdsResources, filterRegistry, parseHttpFilters, false /* isForClient */); + hcmProto, filterRegistry, parseHttpFilters, false /* isForClient */); EnvoyServerProtoData.DownstreamTlsContext downstreamTlsContext = null; if (proto.hasTransportSocket()) { @@ -466,7 +461,7 @@ class XdsListenerResource extends XdsResourceType { @VisibleForTesting static io.grpc.xds.HttpConnectionManager parseHttpConnectionManager( - HttpConnectionManager proto, Set rdsResources, FilterRegistry filterRegistry, + HttpConnectionManager proto, FilterRegistry filterRegistry, boolean parseHttpFilter, boolean isForClient) throws ResourceInvalidException { if (enableRbac && proto.getXffNumTrustedHops() != 0) { throw new ResourceInvalidException( @@ -541,8 +536,6 @@ class XdsListenerResource extends XdsResourceType { throw new ResourceInvalidException( "HttpConnectionManager contains invalid RDS: must specify ADS or self ConfigSource"); } - // Collect the RDS resource referenced by this HttpConnectionManager. - rdsResources.add(rds.getRouteConfigName()); return io.grpc.xds.HttpConnectionManager.forRdsName( maxStreamDuration, rds.getRouteConfigName(), filterConfigs); } diff --git a/xds/src/main/java/io/grpc/xds/XdsResourceType.java b/xds/src/main/java/io/grpc/xds/XdsResourceType.java index a377ee35d7..987151d899 100644 --- a/xds/src/main/java/io/grpc/xds/XdsResourceType.java +++ b/xds/src/main/java/io/grpc/xds/XdsResourceType.java @@ -85,9 +85,12 @@ abstract class XdsResourceType { abstract String typeUrlV2(); - // Non-null for State of the World resources. - @Nullable - abstract XdsResourceType dependentResource(); + // Do not confuse with the SotW approach: it is the mechanism in which the client must specify all + // resource names it is interested in with each request. Different resource types may behave + // differently in this approach. For LDS and CDS resources, the server must return all resources + // that the client has subscribed to in each request. For RDS and EDS, the server may only return + // the resources that need an update. + abstract boolean isFullStateOfTheWorld(); static class Args { final ServerInfo serverInfo; @@ -125,7 +128,6 @@ abstract class XdsResourceType { Set unpackedResources = new HashSet<>(resources.size()); Set invalidResources = new HashSet<>(); List errors = new ArrayList<>(); - Set retainedResources = new HashSet<>(); for (int i = 0; i < resources.size(); i++) { Any resource = resources.get(i); @@ -156,7 +158,7 @@ abstract class XdsResourceType { T resourceUpdate; try { - resourceUpdate = doParse(args, unpackedMessage, retainedResources, isResourceV3); + resourceUpdate = doParse(args, unpackedMessage, isResourceV3); } catch (XdsClientImpl.ResourceInvalidException e) { errors.add(String.format("%s response %s '%s' validation error: %s", typeName(), unpackedClassName().getSimpleName(), cname, e.getMessage())); @@ -168,12 +170,11 @@ abstract class XdsResourceType { parsedResources.put(cname, new ParsedResource(resourceUpdate, resource)); } return new ValidatedResourceUpdate(parsedResources, unpackedResources, invalidResources, - errors, retainedResources); + errors); } - abstract T doParse(Args args, Message unpackedMessage, Set retainedResources, - boolean isResourceV3) + abstract T doParse(Args args, Message unpackedMessage, boolean isResourceV3) throws ResourceInvalidException; /** @@ -231,19 +232,16 @@ abstract class XdsResourceType { Set unpackedResources; Set invalidResources; List errors; - Set retainedResources; // validated resource update public ValidatedResourceUpdate(Map> parsedResources, Set unpackedResources, Set invalidResources, - List errors, - Set retainedResources) { + List errors) { this.parsedResources = parsedResources; this.unpackedResources = unpackedResources; this.invalidResources = invalidResources; this.errors = errors; - this.retainedResources = retainedResources; } } diff --git a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java index 166809c87e..d5a0eecc0e 100644 --- a/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java +++ b/xds/src/main/java/io/grpc/xds/XdsRouteConfigureResource.java @@ -107,10 +107,9 @@ class XdsRouteConfigureResource extends XdsResourceType { return ADS_TYPE_URL_RDS_V2; } - @Nullable @Override - XdsResourceType dependentResource() { - return null; + boolean isFullStateOfTheWorld() { + return false; } @Override @@ -119,8 +118,7 @@ class XdsRouteConfigureResource extends XdsResourceType { } @Override - RdsUpdate doParse(XdsResourceType.Args args, Message unpackedMessage, - Set retainedResources, boolean isResourceV3) + RdsUpdate doParse(XdsResourceType.Args args, Message unpackedMessage, boolean isResourceV3) throws ResourceInvalidException { if (!(unpackedMessage instanceof RouteConfiguration)) { throw new ResourceInvalidException("Invalid message type: " + unpackedMessage.getClass()); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java b/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java index 993fb910f3..8c6ccc2780 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplDataTest.java @@ -135,10 +135,8 @@ import io.grpc.xds.internal.Matchers.FractionMatcher; import io.grpc.xds.internal.Matchers.HeaderMatcher; 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.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -1301,7 +1299,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager with xff_num_trusted_hops unsupported"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* does not matter */, + hcm, filterRegistry, false /* does not matter */, true /* does not matter */); } @@ -1315,7 +1313,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager with original_ip_detection_extensions unsupported"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* does not matter */, false); + hcm, filterRegistry, false /* does not matter */, false); } @Test @@ -1330,7 +1328,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager neither has inlined route_config nor RDS"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* does not matter */, + hcm, filterRegistry, false /* does not matter */, true /* does not matter */); } @@ -1349,7 +1347,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("HttpConnectionManager contains duplicate HttpFilter: envoy.filter.foo"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* parseHttpFilter */, true /* does not matter */); } @@ -1367,7 +1365,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("The last HttpFilter must be a terminal filter: envoy.filter.bar"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* parseHttpFilter */, true /* does not matter */); } @@ -1385,7 +1383,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("A terminal HttpFilter must be the last filter: terminal"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* parseHttpFilter */, true); } @@ -1401,7 +1399,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("The last HttpFilter must be a terminal filter: envoy.filter.bar"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* parseHttpFilter */, true /* does not matter */); } @@ -1413,7 +1411,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Missing HttpFilter in HttpConnectionManager."); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, true /* parseHttpFilter */, + hcm, filterRegistry, true /* parseHttpFilter */, true /* does not matter */); } @@ -1459,7 +1457,7 @@ public class XdsClientImplDataTest { .build(); io.grpc.xds.HttpConnectionManager parsedHcm = XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); VirtualHost virtualHost = Iterables.getOnlyElement(parsedHcm.virtualHosts()); @@ -1536,7 +1534,7 @@ public class XdsClientImplDataTest { thrown.expectMessage("Multiple ClusterSpecifierPlugins with the same name: rls-plugin-1"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); } @@ -1585,7 +1583,7 @@ public class XdsClientImplDataTest { thrown.expectMessage("ClusterSpecifierPlugin for [invalid-plugin-name] not found"); XdsListenerResource.parseHttpConnectionManager( - hcm, new HashSet(), filterRegistry, false /* parseHttpFilter */, + hcm, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); } @@ -1655,8 +1653,8 @@ public class XdsClientImplDataTest { .addRoutes(optionalRoute)) .build(); io.grpc.xds.HttpConnectionManager parsedHcm = XdsListenerResource.parseHttpConnectionManager( - HttpConnectionManager.newBuilder().setRouteConfig(routeConfig).build(), - new HashSet<>(), filterRegistry, false /* parseHttpFilter */, true /* does not matter */); + HttpConnectionManager.newBuilder().setRouteConfig(routeConfig).build(), filterRegistry, + false /* parseHttpFilter */, true /* does not matter */); // Verify that the only route left is the one with the registered RLS plugin `rls-plugin-1`, // while the route with unregistered optional `optional-plugin-`1 has been skipped. @@ -1671,7 +1669,6 @@ public class XdsClientImplDataTest { @Test public void parseHttpConnectionManager_validateRdsConfigSource() throws Exception { XdsResourceType.enableRouteLookup = true; - Set rdsResources = new HashSet<>(); HttpConnectionManager hcm1 = HttpConnectionManager.newBuilder() @@ -1681,7 +1678,7 @@ public class XdsClientImplDataTest { ConfigSource.newBuilder().setAds(AggregatedConfigSource.getDefaultInstance()))) .build(); XdsListenerResource.parseHttpConnectionManager( - hcm1, rdsResources, filterRegistry, false /* parseHttpFilter */, + hcm1, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); HttpConnectionManager hcm2 = @@ -1692,7 +1689,7 @@ public class XdsClientImplDataTest { ConfigSource.newBuilder().setSelf(SelfConfigSource.getDefaultInstance()))) .build(); XdsListenerResource.parseHttpConnectionManager( - hcm2, rdsResources, filterRegistry, false /* parseHttpFilter */, + hcm2, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); HttpConnectionManager hcm3 = @@ -1707,7 +1704,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "HttpConnectionManager contains invalid RDS: must specify ADS or self ConfigSource"); XdsListenerResource.parseHttpConnectionManager( - hcm3, rdsResources, filterRegistry, false /* parseHttpFilter */, + hcm3, filterRegistry, false /* parseHttpFilter */, true /* does not matter */); } @@ -1892,7 +1889,7 @@ public class XdsClientImplDataTest { .build(); CdsUpdate update = XdsClusterResource.processCluster( - cluster, new HashSet(), null, LRS_SERVER_INFO, + cluster, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(update.lbPolicyConfig()); assertThat(lbConfig.getPolicyName()).isEqualTo("ring_hash_experimental"); @@ -1914,7 +1911,7 @@ public class XdsClientImplDataTest { .build(); CdsUpdate update = XdsClusterResource.processCluster( - cluster, new HashSet(), null, LRS_SERVER_INFO, + cluster, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(update.lbPolicyConfig()); assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); @@ -1942,13 +1939,12 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage( "Cluster cluster-foo.googleapis.com: transport-socket-matches not supported."); - XdsClusterResource.processCluster(cluster, new HashSet(), null, LRS_SERVER_INFO, + XdsClusterResource.processCluster(cluster, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); } @Test public void parseCluster_validateEdsSourceConfig() throws ResourceInvalidException { - Set retainedEdsResources = new HashSet<>(); Cluster cluster1 = Cluster.newBuilder() .setName("cluster-foo.googleapis.com") .setType(DiscoveryType.EDS) @@ -1960,7 +1956,7 @@ public class XdsClientImplDataTest { .setServiceName("service-foo.googleapis.com")) .setLbPolicy(LbPolicy.ROUND_ROBIN) .build(); - XdsClusterResource.processCluster(cluster1, retainedEdsResources, null, LRS_SERVER_INFO, + XdsClusterResource.processCluster(cluster1, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); Cluster cluster2 = Cluster.newBuilder() @@ -1974,7 +1970,7 @@ public class XdsClientImplDataTest { .setServiceName("service-foo.googleapis.com")) .setLbPolicy(LbPolicy.ROUND_ROBIN) .build(); - XdsClusterResource.processCluster(cluster2, retainedEdsResources, null, LRS_SERVER_INFO, + XdsClusterResource.processCluster(cluster2, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); Cluster cluster3 = Cluster.newBuilder() @@ -1993,7 +1989,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "Cluster cluster-foo.googleapis.com: field eds_cluster_config must be set to indicate to" + " use EDS over ADS or self ConfigSource"); - XdsClusterResource.processCluster(cluster3, retainedEdsResources, null, LRS_SERVER_INFO, + XdsClusterResource.processCluster(cluster3, null, LRS_SERVER_INFO, LoadBalancerRegistry.getDefaultRegistry()); } @@ -2007,7 +2003,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 with invalid traffic direction: OUTBOUND"); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null, true /* does not matter */); } @Test @@ -2017,7 +2013,7 @@ public class XdsClientImplDataTest { .setName("listener1") .build(); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null, true /* does not matter */); } @Test @@ -2031,7 +2027,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 cannot have listener_filters"); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null, true /* does not matter */); } @Test @@ -2045,7 +2041,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("Listener listener1 cannot have use_original_dst set to true"); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener,null, filterRegistry, null, true /* does not matter */); } @Test @@ -2094,7 +2090,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("FilterChainMatch must be unique. Found duplicate:"); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null, true /* does not matter */); } @Test @@ -2143,7 +2139,7 @@ public class XdsClientImplDataTest { thrown.expect(ResourceInvalidException.class); thrown.expectMessage("FilterChainMatch must be unique. Found duplicate:"); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener,null, filterRegistry, null, true /* does not matter */); } @Test @@ -2192,7 +2188,7 @@ public class XdsClientImplDataTest { .addAllFilterChains(Arrays.asList(filterChain1, filterChain2)) .build(); XdsListenerResource.parseServerSideListener( - listener, new HashSet(), null, filterRegistry, null, true /* does not matter */); + listener, null, filterRegistry, null, true /* does not matter */); } @Test @@ -2207,7 +2203,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter"); XdsListenerResource.parseFilterChain( - filterChain, new HashSet(), null, filterRegistry, null, null, + filterChain, null, filterRegistry, null, null, true /* does not matter */); } @@ -2226,7 +2222,7 @@ public class XdsClientImplDataTest { thrown.expectMessage( "FilterChain filter-chain-foo should contain exact one HttpConnectionManager filter"); XdsListenerResource.parseFilterChain( - filterChain, new HashSet(), null, filterRegistry, null, null, + filterChain, null, filterRegistry, null, null, true /* does not matter */); } @@ -2245,7 +2241,7 @@ public class XdsClientImplDataTest { "FilterChain filter-chain-foo contains filter envoy.http_connection_manager " + "without typed_config"); XdsListenerResource.parseFilterChain( - filterChain, new HashSet(), null, filterRegistry, null, null, + filterChain, null, filterRegistry, null, null, true /* does not matter */); } @@ -2268,7 +2264,7 @@ public class XdsClientImplDataTest { "FilterChain filter-chain-foo contains filter unsupported with unsupported " + "typed_config type unsupported-type-url"); XdsListenerResource.parseFilterChain( - filterChain, new HashSet(), null, filterRegistry, null, null, + filterChain, null, filterRegistry, null, null, true /* does not matter */); } @@ -2296,10 +2292,10 @@ public class XdsClientImplDataTest { .build(); EnvoyServerProtoData.FilterChain parsedFilterChain1 = XdsListenerResource.parseFilterChain( - filterChain1, new HashSet(), null, filterRegistry, null, + filterChain1, null, filterRegistry, null, null, true /* does not matter */); EnvoyServerProtoData.FilterChain parsedFilterChain2 = XdsListenerResource.parseFilterChain( - filterChain2, new HashSet(), null, filterRegistry, null, + filterChain2, null, filterRegistry, null, null, true /* does not matter */); assertThat(parsedFilterChain1.name()).isEqualTo(parsedFilterChain2.name()); } diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java index 31d10abd84..ca996af074 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java @@ -806,19 +806,14 @@ public abstract class XdsClientImplTestBase { verifyResourceMetadataAcked(LDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT); } call.verifyRequestNack(LDS, subscribedResourceNames, VERSION_1, "0001", NODE, errorsV2); - // {A.1} -> does not exist, missing from {A} + // {A.1} -> version 1 // {B.1} -> version 1 // {C.1} -> does not exist because {C} does not exist - verifyResourceMetadataDoesNotExist(RDS, "A.1"); + verifyResourceMetadataAcked(RDS, "A.1", resourcesV11.get("A.1"), VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked(RDS, "B.1", resourcesV11.get("B.1"), VERSION_1, TIME_INCREMENT * 2); - if (!ignoreResourceDeletion()) { - verifyResourceMetadataDoesNotExist(RDS, "C.1"); - } else { - // When resource deletion is disabled, {C.1} is not deleted when {C} is deleted. - // Verify {C.1} stays in the previous version VERSION_1. - verifyResourceMetadataAcked(RDS, "C.1", resourcesV11.get("C.1"), VERSION_1, - TIME_INCREMENT * 2); - } + // Verify {C.1} stays in the previous version VERSION_1, no matter {C} is deleted or not. + verifyResourceMetadataAcked(RDS, "C.1", resourcesV11.get("C.1"), VERSION_1, + TIME_INCREMENT * 2); } @Test @@ -1556,8 +1551,8 @@ public abstract class XdsClientImplTestBase { call.sendResponse(LDS, testListenerVhosts, VERSION_2, "0001"); verify(ldsResourceWatcher, times(2)).onChanged(ldsUpdateCaptor.capture()); verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue()); - verify(rdsResourceWatcher).onResourceDoesNotExist(RDS_RESOURCE); - verifyResourceMetadataDoesNotExist(RDS, RDS_RESOURCE); + verifyNoMoreInteractions(rdsResourceWatcher); + verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked( LDS, LDS_RESOURCE, testListenerVhosts, VERSION_2, TIME_INCREMENT * 3); verifySubscribedResourcesMetadataSizes(1, 0, 1, 0); @@ -1624,8 +1619,8 @@ public abstract class XdsClientImplTestBase { parsedFilterChain = Iterables.getOnlyElement( ldsUpdateCaptor.getValue().listener().filterChains()); assertThat(parsedFilterChain.httpConnectionManager().virtualHosts()).hasSize(VHOST_SIZE); - verify(rdsResourceWatcher).onResourceDoesNotExist(RDS_RESOURCE); - verifyResourceMetadataDoesNotExist(RDS, RDS_RESOURCE); + verify(rdsResourceWatcher, never()).onResourceDoesNotExist(RDS_RESOURCE); + verifyResourceMetadataAcked(RDS, RDS_RESOURCE, testRouteConfig, VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked( LDS, LISTENER_RESOURCE, packedListener, VERSION_2, TIME_INCREMENT * 3); verifySubscribedResourcesMetadataSizes(1, 0, 1, 0); @@ -1896,19 +1891,14 @@ public abstract class XdsClientImplTestBase { verifyResourceMetadataAcked(CDS, "C", resourcesV1.get("C"), VERSION_1, TIME_INCREMENT); } call.verifyRequestNack(CDS, subscribedResourceNames, VERSION_1, "0001", NODE, errorsV2); - // {A.1} -> does not exist, missing from {A} + // {A.1} -> version 1 // {B.1} -> version 1 // {C.1} -> does not exist because {C} does not exist - verifyResourceMetadataDoesNotExist(EDS, "A.1"); + verifyResourceMetadataAcked(EDS, "A.1", resourcesV11.get("A.1"), VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked(EDS, "B.1", resourcesV11.get("B.1"), VERSION_1, TIME_INCREMENT * 2); - if (!ignoreResourceDeletion()) { - verifyResourceMetadataDoesNotExist(EDS, "C.1"); - } else { - // When resource deletion is disabled, {C.1} is not deleted when {C} is deleted. - // Verify {C.1} stays in the previous version VERSION_1. - verifyResourceMetadataAcked(EDS, "C.1", resourcesV11.get("C.1"), VERSION_1, - TIME_INCREMENT * 2); - } + // Verify {C.1} stays in the previous version VERSION_1. {C1} deleted or not does not matter. + verifyResourceMetadataAcked(EDS, "C.1", resourcesV11.get("C.1"), VERSION_1, + TIME_INCREMENT * 2); } @Test @@ -3026,10 +3016,11 @@ public abstract class XdsClientImplTestBase { assertThat(cdsUpdateCaptor.getValue().edsServiceName()).isNull(); // Note that the endpoint must be deleted even if the ignore_resource_deletion feature. // This happens because the cluster CDS_RESOURCE is getting replaced, and not deleted. - verify(edsResourceWatcher).onResourceDoesNotExist(EDS_RESOURCE); + verify(edsResourceWatcher, never()).onResourceDoesNotExist(EDS_RESOURCE); verify(edsResourceWatcher, never()).onResourceDoesNotExist(resource); verifyNoMoreInteractions(cdsWatcher, edsWatcher); - verifyResourceMetadataDoesNotExist(EDS, EDS_RESOURCE); + verifyResourceMetadataAcked( + EDS, EDS_RESOURCE, clusterLoadAssignments.get(0), VERSION_1, TIME_INCREMENT * 2); verifyResourceMetadataAcked( EDS, resource, clusterLoadAssignments.get(1), VERSION_1, TIME_INCREMENT * 2); // no change verifyResourceMetadataAcked(CDS, resource, clusters.get(0), VERSION_2, TIME_INCREMENT * 3);