From 4b59a6c07f1442c06f16e8fca883e26598d04d66 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 08:00:17 -0800 Subject: [PATCH 01/10] xds: del events to watchers of same control plane When XdsClient learns that a control plane no longer tracks a resource, it should only notify watchers associated with that control plane. This matters in control plane federation cases when more than one control plane is in use. --- .../main/java/io/grpc/xds/XdsClientImpl.java | 7 +- .../java/io/grpc/xds/ControlPlaneRule.java | 17 +- .../io/grpc/xds/XdsClientFederationTest.java | 153 ++++++++++++++++++ 3 files changed, 172 insertions(+), 5 deletions(-) create mode 100644 xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index e67bff12871..b1a5a710663 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -477,8 +477,11 @@ private void handleResourceUpdate(XdsResourceType.Arg } // For State of the World services, notify watchers when their watched resource is missing - // from the ADS update. - subscriber.onAbsent(); + // 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.target().equals(args.serverInfo.target())) { + subscriber.onAbsent(); + } } } diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index ea764e67e40..c03003cf53f 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -76,15 +76,23 @@ public class ControlPlaneRule extends TestWatcher { private static final String EDS_NAME = "eds-service-0"; private static final String SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT = "grpc/server?udpa.resource.listening_address="; - private static final String SERVER_HOST_NAME = "test-server"; private static final String HTTP_CONNECTION_MANAGER_TYPE_URL = "type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3" + ".HttpConnectionManager"; + private String serverHostName; private Server server; private XdsTestControlPlaneService controlPlaneService; private XdsNameResolverProvider nameResolverProvider; + public ControlPlaneRule(String serverHostName) { + this.serverHostName = serverHostName; + } + + public ControlPlaneRule() { + this("test-server"); + } + /** * Returns the test control plane service interface. */ @@ -118,14 +126,17 @@ public Server getServer() { @Override protected void finished(Description description) { if (server != null) { - server.shutdownNow(); + System.out.println(">>> shutting down server on port " + server.getPort()); + server.shutdown(); try { + logger.info("awaiting termination"); if (!server.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); } + logger.info("server terminated"); } NameResolverRegistry.getDefaultRegistry().deregister(nameResolverProvider); } @@ -155,7 +166,7 @@ public Server getServer() { void setLdsConfig(Listener serverListener, Listener clientListener) { getService().setXdsConfig(ADS_TYPE_URL_LDS, ImmutableMap.of(SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT, serverListener, - SERVER_HOST_NAME, clientListener)); + serverHostName, clientListener)); } void setRdsConfig(RouteConfiguration routeConfiguration) { diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java new file mode 100644 index 00000000000..f425732cfcd --- /dev/null +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -0,0 +1,153 @@ +/* + * Copyright 2023 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 org.mockito.ArgumentMatchers.isA; +import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import io.grpc.xds.XdsClient.ResourceWatcher; +import io.grpc.xds.XdsListenerResource.LdsUpdate; +import java.util.Collections; +import java.util.Map; +import java.util.UUID; +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.Mock; +import org.mockito.MockitoAnnotations; + +/** + * Tests for xDS control plane federation scenarios. + */ +@RunWith(JUnit4.class) +public class XdsClientFederationTest { + + @Mock + private ResourceWatcher mockDirectPathWatcher; + + @Mock + private ResourceWatcher mockWatcher; + + private static final String SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT = + "grpc/server?udpa.resource.listening_address="; + + @Rule + public ControlPlaneRule trafficdirector = new ControlPlaneRule("test-server"); + + @Rule + public ControlPlaneRule directpathPa = new ControlPlaneRule( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + + private XdsClient xdsClient; + private boolean originalFederationStatus; + + @Before + public void setUp() throws XdsInitializationException { + MockitoAnnotations.initMocks(this); + + originalFederationStatus = BootstrapperImpl.enableFederation; + BootstrapperImpl.enableFederation = true; + + SharedXdsClientPoolProvider clientPoolProvider = new SharedXdsClientPoolProvider(); + clientPoolProvider.setBootstrapOverride(defaultBootstrapOverride()); + xdsClient = clientPoolProvider.getOrCreate().getObject(); + } + + @After + public void cleanUp() throws InterruptedException { + BootstrapperImpl.enableFederation = originalFederationStatus; + } + + // Assures that resource deletions happening in one control plane do not trigger deletion events + /// in watchers of resources on other control planes. + @Test + public void isolatedResourceDeletions() throws InterruptedException { + // Add the mock watcher for the normal server resource. The test control plane will send + // a deletion event. + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); + verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); + + // Add the watcher for the DirectPath server. + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); + verify(mockDirectPathWatcher, timeout(20000)).onResourceDoesNotExist( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); + + // Add a normal server resource and observe a changed event on the normal watcher and + // a onResourceDoesNotExist() on the DirectPath watcher as it's resource is not yet created. + trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener("test-server")); + verify(mockWatcher, timeout(20000)).onChanged(isA(LdsUpdate.class)); + verify(mockDirectPathWatcher, timeout(20000)).onResourceDoesNotExist( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + + // Modifying the DirectPath server resource triggers a changed event on the DirectPath watcher. + directpathPa.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); + verify(mockDirectPathWatcher, timeout(20000)).onChanged(isA(LdsUpdate.class)); + + // And the crux of the test: deleting a resource (here by renaming it) in one control plane + // (here the "normal TrafficDirector" one) should not trigger an onResourceDoesNotExist() call + // on a watcher of another control plane (here the DirectPath one). + trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), + ControlPlaneRule.buildClientListener("new-server")); + verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); + verifyNoMoreInteractions(mockDirectPathWatcher); + } + + private Map defaultBootstrapOverride() { + return ImmutableMap.of( + "node", ImmutableMap.of( + "id", UUID.randomUUID().toString(), + "cluster", "cluster0"), + "xds_servers", ImmutableList.of( + ImmutableMap.of( + "server_uri", "localhost:" + trafficdirector.getServer().getPort(), + "channel_creds", Collections.singletonList( + ImmutableMap.of("type", "insecure") + ), + "server_features", Collections.singletonList("xds_v3") + ) + ), + "authorities", ImmutableMap.of( + "", ImmutableMap.of(), + "server-one", ImmutableMap.of( + "xds_servers", ImmutableList.of( + ImmutableMap.of( + "server_uri", "localhost:" + directpathPa.getServer().getPort(), + "channel_creds", Collections.singletonList( + ImmutableMap.of("type", "insecure") + ), + "server_features", Collections.singletonList("xds_v3") + ) + ) + ), + "server-two", ImmutableMap.of() + ), + "server_listener_resource_name_template", SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT + ); + } +} From 0a6e34db73fe988f59ecab5a4633144332bc9c81 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 09:03:39 -0800 Subject: [PATCH 02/10] Review feedback changes --- xds/src/main/java/io/grpc/xds/XdsClientImpl.java | 2 +- xds/src/test/java/io/grpc/xds/ControlPlaneRule.java | 1 - xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java index b1a5a710663..af5fb5d3507 100644 --- a/xds/src/main/java/io/grpc/xds/XdsClientImpl.java +++ b/xds/src/main/java/io/grpc/xds/XdsClientImpl.java @@ -479,7 +479,7 @@ private void handleResourceUpdate(XdsResourceType.Arg // 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.target().equals(args.serverInfo.target())) { + if (subscriber.serverInfo.equals(args.serverInfo)) { subscriber.onAbsent(); } } diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index c03003cf53f..4306dabd2fb 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -126,7 +126,6 @@ public Server getServer() { @Override protected void finished(Description description) { if (server != null) { - System.out.println(">>> shutting down server on port " + server.getPort()); server.shutdown(); try { logger.info("awaiting termination"); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index f425732cfcd..b65cdef1d1f 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -93,7 +93,6 @@ public void isolatedResourceDeletions() throws InterruptedException { "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); verify(mockDirectPathWatcher, timeout(20000)).onResourceDoesNotExist( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); - verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); // Add a normal server resource and observe a changed event on the normal watcher and // a onResourceDoesNotExist() on the DirectPath watcher as it's resource is not yet created. From f18c9d9a0ce415af889314e33f71c34c27b6479c Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 09:24:11 -0800 Subject: [PATCH 03/10] Knock it off with all that verify() --- .../io/grpc/xds/XdsClientFederationTest.java | 27 +++++-------------- 1 file changed, 6 insertions(+), 21 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index b65cdef1d1f..5891231e593 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -16,10 +16,9 @@ package io.grpc.xds; -import static org.mockito.ArgumentMatchers.isA; import static org.mockito.Mockito.timeout; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -83,38 +82,24 @@ public void cleanUp() throws InterruptedException { /// in watchers of resources on other control planes. @Test public void isolatedResourceDeletions() throws InterruptedException { - // Add the mock watcher for the normal server resource. The test control plane will send - // a deletion event. xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); - verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); - - // Add the watcher for the DirectPath server. xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); - verify(mockDirectPathWatcher, timeout(20000)).onResourceDoesNotExist( - "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); - // Add a normal server resource and observe a changed event on the normal watcher and - // a onResourceDoesNotExist() on the DirectPath watcher as it's resource is not yet created. trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener("test-server")); - verify(mockWatcher, timeout(20000)).onChanged(isA(LdsUpdate.class)); - verify(mockDirectPathWatcher, timeout(20000)).onResourceDoesNotExist( - "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); - - // Modifying the DirectPath server resource triggers a changed event on the DirectPath watcher. directpathPa.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); - verify(mockDirectPathWatcher, timeout(20000)).onChanged(isA(LdsUpdate.class)); - // And the crux of the test: deleting a resource (here by renaming it) in one control plane - // (here the "normal TrafficDirector" one) should not trigger an onResourceDoesNotExist() call - // on a watcher of another control plane (here the DirectPath one). + // Deleting a resource (here by renaming it) in one control plan (here the "normal + // TrafficDirector" one) should not trigger an onResourceDoesNotExist() call on a watcher of + // another control plane (here the DirectPath one). trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener("new-server")); verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); - verifyNoMoreInteractions(mockDirectPathWatcher); + verify(mockDirectPathWatcher, times(0)).onResourceDoesNotExist( + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); } private Map defaultBootstrapOverride() { From a3b120ca35cce809ee9f17800ed1bad6905a7459 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 10:18:04 -0800 Subject: [PATCH 04/10] Remove extra logging --- xds/src/test/java/io/grpc/xds/ControlPlaneRule.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index 4306dabd2fb..e25fc5871bb 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -128,14 +128,12 @@ public Server getServer() { if (server != null) { server.shutdown(); try { - logger.info("awaiting termination"); if (!server.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); } - logger.info("server terminated"); } NameResolverRegistry.getDefaultRegistry().deregister(nameResolverProvider); } From 627d5125e66fab6bcdb252066565fe88b293bebd Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 11:01:32 -0800 Subject: [PATCH 05/10] fluent ControlPlaneRule --- xds/src/test/java/io/grpc/xds/ControlPlaneRule.java | 9 +++++---- .../test/java/io/grpc/xds/XdsClientFederationTest.java | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index e25fc5871bb..f7dd3373903 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -85,12 +85,13 @@ public class ControlPlaneRule extends TestWatcher { private XdsTestControlPlaneService controlPlaneService; private XdsNameResolverProvider nameResolverProvider; - public ControlPlaneRule(String serverHostName) { - this.serverHostName = serverHostName; + public ControlPlaneRule() { + serverHostName = "test-server"; } - public ControlPlaneRule() { - this("test-server"); + public ControlPlaneRule setServerHostName(String serverHostName) { + this.serverHostName = serverHostName; + return this; } /** diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 5891231e593..efa47c1340e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -52,10 +52,10 @@ public class XdsClientFederationTest { "grpc/server?udpa.resource.listening_address="; @Rule - public ControlPlaneRule trafficdirector = new ControlPlaneRule("test-server"); + public ControlPlaneRule trafficdirector = new ControlPlaneRule().setServerHostName("test-server"); @Rule - public ControlPlaneRule directpathPa = new ControlPlaneRule( + public ControlPlaneRule directpathPa = new ControlPlaneRule().setServerHostName( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); private XdsClient xdsClient; From 35066ccf0a49bf031818fc883ad20d62e451235d Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 11:14:18 -0800 Subject: [PATCH 06/10] More review comment changes --- .../java/io/grpc/xds/ControlPlaneRule.java | 2 +- .../io/grpc/xds/XdsClientFederationTest.java | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java index f7dd3373903..90f249b6b47 100644 --- a/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java +++ b/xds/src/test/java/io/grpc/xds/ControlPlaneRule.java @@ -127,7 +127,7 @@ public Server getServer() { @Override protected void finished(Description description) { if (server != null) { - server.shutdown(); + server.shutdownNow(); try { if (!server.awaitTermination(5, TimeUnit.SECONDS)) { logger.log(Level.SEVERE, "Timed out waiting for server shutdown"); diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index efa47c1340e..945ca0b932f 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -48,9 +48,6 @@ public class XdsClientFederationTest { @Mock private ResourceWatcher mockWatcher; - private static final String SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT = - "grpc/server?udpa.resource.listening_address="; - @Rule public ControlPlaneRule trafficdirector = new ControlPlaneRule().setServerHostName("test-server"); @@ -76,10 +73,13 @@ public void setUp() throws XdsInitializationException { @After public void cleanUp() throws InterruptedException { BootstrapperImpl.enableFederation = originalFederationStatus; + xdsClient.shutdown(); } - // Assures that resource deletions happening in one control plane do not trigger deletion events - /// in watchers of resources on other control planes. + /** + * Assures that resource deletions happening in one control plane do not trigger deletion events + * in watchers of resources on other control planes. + */ @Test public void isolatedResourceDeletions() throws InterruptedException { xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); @@ -92,9 +92,10 @@ public void isolatedResourceDeletions() throws InterruptedException { ControlPlaneRule.buildClientListener( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); - // Deleting a resource (here by renaming it) in one control plan (here the "normal - // TrafficDirector" one) should not trigger an onResourceDoesNotExist() call on a watcher of - // another control plane (here the DirectPath one). + // By setting the LDS config with a new server name we effectively make the old server to go + // away as it is not in the configuration anymore. This change in one control plain (here the + // "normal TrafficDirector" one) should not trigger an onResourceDoesNotExist() call on a + // watcher of another control plane (here the DirectPath one). trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener("new-server")); verify(mockWatcher, timeout(20000)).onResourceDoesNotExist("test-server"); @@ -130,8 +131,7 @@ public void isolatedResourceDeletions() throws InterruptedException { ) ), "server-two", ImmutableMap.of() - ), - "server_listener_resource_name_template", SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT + ) ); } } From 1c1a38819846004351d3bdf77ce6dc7b593c61c1 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 13:02:28 -0800 Subject: [PATCH 07/10] TYPO --- xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 945ca0b932f..14178b0b6dc 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -93,7 +93,7 @@ public void isolatedResourceDeletions() throws InterruptedException { "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); // By setting the LDS config with a new server name we effectively make the old server to go - // away as it is not in the configuration anymore. This change in one control plain (here the + // away as it is not in the configuration anymore. This change in one control plane (here the // "normal TrafficDirector" one) should not trigger an onResourceDoesNotExist() call on a // watcher of another control plane (here the DirectPath one). trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), From 081251f61ae12675eceb7b4307ba7ecbc9932998 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 14:13:54 -0800 Subject: [PATCH 08/10] more test order, verify() shenanigans --- .../io/grpc/xds/XdsClientFederationTest.java | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 14178b0b6dc..ce14bfef67c 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import io.grpc.xds.Filter.NamedFilterConfig; import io.grpc.xds.XdsClient.ResourceWatcher; import io.grpc.xds.XdsListenerResource.LdsUpdate; import java.util.Collections; @@ -82,16 +83,25 @@ public void cleanUp() throws InterruptedException { */ @Test public void isolatedResourceDeletions() throws InterruptedException { - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); - xdsClient.watchXdsResource(XdsListenerResource.getInstance(), - "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); - trafficdirector.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener("test-server")); directpathPa.setLdsConfig(ControlPlaneRule.buildServerListener(), ControlPlaneRule.buildClientListener( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server")); + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), "test-server", mockWatcher); + xdsClient.watchXdsResource(XdsListenerResource.getInstance(), + "xdstp://server-one/envoy.config.listener.v3.Listener/test-server", mockDirectPathWatcher); + + verify(mockWatcher, timeout(2000)).onChanged( + LdsUpdate.forApiListener( + HttpConnectionManager.forRdsName(0, "route-config.googleapis.com", ImmutableList.of( + new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG))))); + verify(mockDirectPathWatcher, timeout(2000)).onChanged( + LdsUpdate.forApiListener( + HttpConnectionManager.forRdsName(0, "route-config.googleapis.com", ImmutableList.of( + new NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG))))); + // By setting the LDS config with a new server name we effectively make the old server to go // away as it is not in the configuration anymore. This change in one control plane (here the // "normal TrafficDirector" one) should not trigger an onResourceDoesNotExist() call on a From 92359448fe472927fe4e4e59b9eb8f98096e6b26 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 14:20:56 -0800 Subject: [PATCH 09/10] return XdsClient to the pool --- xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index ce14bfef67c..0e616fb71fd 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import io.grpc.internal.ObjectPool; import io.grpc.xds.Filter.NamedFilterConfig; import io.grpc.xds.XdsClient.ResourceWatcher; import io.grpc.xds.XdsListenerResource.LdsUpdate; @@ -56,6 +57,7 @@ public class XdsClientFederationTest { public ControlPlaneRule directpathPa = new ControlPlaneRule().setServerHostName( "xdstp://server-one/envoy.config.listener.v3.Listener/test-server"); + private ObjectPool xdsClientPool; private XdsClient xdsClient; private boolean originalFederationStatus; @@ -68,13 +70,15 @@ public void setUp() throws XdsInitializationException { SharedXdsClientPoolProvider clientPoolProvider = new SharedXdsClientPoolProvider(); clientPoolProvider.setBootstrapOverride(defaultBootstrapOverride()); - xdsClient = clientPoolProvider.getOrCreate().getObject(); + xdsClientPool = clientPoolProvider.getOrCreate(); + xdsClient = xdsClientPool.getObject(); } @After public void cleanUp() throws InterruptedException { BootstrapperImpl.enableFederation = originalFederationStatus; xdsClient.shutdown(); + xdsClientPool.returnObject(xdsClient); } /** From 8364c01b29f415b3e4655d091a5013571cfe7fca Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 16 Feb 2023 14:26:47 -0800 Subject: [PATCH 10/10] Remove unnecessary XdsClient close() --- xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java index 0e616fb71fd..54d428b340b 100644 --- a/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsClientFederationTest.java @@ -77,7 +77,6 @@ public void setUp() throws XdsInitializationException { @After public void cleanUp() throws InterruptedException { BootstrapperImpl.enableFederation = originalFederationStatus; - xdsClient.shutdown(); xdsClientPool.returnObject(xdsClient); }