xds:fix cancel xds watcher accidentally removes the url

grpc · Jan 11, 2023 · 8ba123a · 8ba123a
1 parent eb391fd
commit 8ba123a
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 1 deletion.
diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java
@@ -332,12 +332,13 @@ public void run() {
  if (!subscriber.isWatched()) {
  subscriber.cancelResourceWatch();
  resourceSubscribers.get(type).remove(resourceName);
- subscribedResourceTypeUrls.remove(type.typeUrl());
  if (subscriber.xdsChannel != null) {
  subscriber.xdsChannel.adjustResourceSubscription(type);
  }
  if (resourceSubscribers.get(type).isEmpty()) {
  resourceSubscribers.remove(type);
+ subscribedResourceTypeUrls.remove(type.typeUrl());
+
  }
  }
  }

diff --git a/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java b/xds/src/test/java/io/grpc/xds/XdsClientImplTestBase.java
@@ -938,6 +938,35 @@ public void ldsResourceUpdated() {
  assertThat(channelForEmptyAuthority).isNull();
  }
 
+ @Test
+ public void cancelResourceWatcherNotRemoveUrlSubscribers() {
+ DiscoveryRpcCall call = startResourceWatcher(XdsListenerResource.getInstance(), LDS_RESOURCE,
+ ldsResourceWatcher);
+ verifyResourceMetadataRequested(LDS, LDS_RESOURCE);
+
+ // Initial LDS response.
+ call.sendResponse(LDS, testListenerVhosts, VERSION_1, "0000");
+ call.verifyRequest(LDS, LDS_RESOURCE, VERSION_1, "0000", NODE);
+ verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
+ verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
+ verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts, VERSION_1, TIME_INCREMENT);
+
+ xdsClient.watchXdsResource(XdsListenerResource.getInstance(),
+ LDS_RESOURCE + "1", ldsResourceWatcher);
+ xdsClient.cancelXdsResourceWatch(XdsListenerResource.getInstance(), LDS_RESOURCE + "1",
+ ldsResourceWatcher);
+
+ // Updated LDS response.
+ Any testListenerVhosts2 = Any.pack(mf.buildListenerWithApiListener(LDS_RESOURCE,
+ mf.buildRouteConfiguration("new", mf.buildOpaqueVirtualHosts(VHOST_SIZE))));
+ call.sendResponse(LDS, testListenerVhosts2, VERSION_2, "0001");
+ call.verifyRequest(LDS, LDS_RESOURCE, VERSION_2, "0001", NODE);
+ verify(ldsResourceWatcher).onChanged(ldsUpdateCaptor.capture());
+ verifyGoldenListenerVhosts(ldsUpdateCaptor.getValue());
+ verifyResourceMetadataAcked(LDS, LDS_RESOURCE, testListenerVhosts2, VERSION_2,
+ TIME_INCREMENT * 2);
+ }
+
  @Test
  public void ldsResourceUpdated_withXdstpResourceName() {
  BootstrapperImpl.enableFederation = true;