From efec9fa90ed0509de0212a22f0a3770436bba119 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Fri, 1 Oct 2021 16:16:13 -0700 Subject: [PATCH 1/5] override bootstrap for xds server --- xds/src/main/java/io/grpc/xds/XdsServerBuilder.java | 13 +++++++++++-- .../java/io/grpc/xds/XdsSdsClientServerTest.java | 2 +- .../test/java/io/grpc/xds/XdsServerBuilderTest.java | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index c95c1e6d48f..0db6990caef 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -37,9 +37,12 @@ import io.grpc.netty.NettyServerBuilder; import io.grpc.xds.FilterChainMatchingProtocolNegotiators.FilterChainMatchingNegotiatorServerFactory; import io.grpc.xds.XdsNameResolverProvider.XdsClientPoolFactory; + +import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; +import javax.annotation.Nullable; /** * A version of {@link ServerBuilder} to create xDS managed servers. @@ -131,8 +134,14 @@ public Server build() { } @VisibleForTesting - XdsServerBuilder xdsClientPoolFactory(XdsClientPoolFactory xdsClientPoolFactory) { - this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); + XdsServerBuilder xdsClientPoolFactory(@Nullable XdsClientPoolFactory xdsClientPoolFactory, + @Nullable Map bootstrapOverride) { + if (xdsClientPoolFactory != null) { + this.xdsClientPoolFactory = xdsClientPoolFactory; + } + if (bootstrapOverride != null) { + this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride); + } return this; } diff --git a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java index 579542a2777..ba50d7cbd6c 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java @@ -337,7 +337,7 @@ private void buildServerWithFallbackServerCredentials( throws Exception { ServerCredentials xdsCredentials = XdsServerCredentials.create(fallbackCredentials); XdsServerBuilder builder = XdsServerBuilder.forPort(0, xdsCredentials) - .xdsClientPoolFactory(fakePoolFactory) + .xdsClientPoolFactory(fakePoolFactory, null) .addService(new SimpleServiceImpl()); buildServer(builder, downstreamTlsContext); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index 0d15c1f660e..eaf081d2c6b 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -77,7 +77,7 @@ private void buildBuilder(XdsServerBuilder.XdsServingStatusListener xdsServingSt builder = XdsServerBuilder.forPort( port, XdsServerCredentials.create(InsecureServerCredentials.create())); - builder.xdsClientPoolFactory(new FakeXdsClientPoolFactory(xdsClient)); + builder.xdsClientPoolFactory(new FakeXdsClientPoolFactory(xdsClient), null); if (xdsServingStatusListener != null) { builder.xdsServingStatusListener(xdsServingStatusListener); } From c648976804b7815e2a464ad8b926ad15701898b4 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Mon, 4 Oct 2021 10:07:11 -0700 Subject: [PATCH 2/5] pubic --- xds/src/main/java/io/grpc/xds/XdsServerBuilder.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index 0db6990caef..e98bf8a71c9 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -22,7 +22,6 @@ import static io.grpc.xds.InternalXdsAttributes.ATTR_DRAIN_GRACE_NANOS; import static io.grpc.xds.InternalXdsAttributes.ATTR_FILTER_CHAIN_SELECTOR_MANAGER; -import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.DoNotCall; import io.grpc.Attributes; import io.grpc.ExperimentalApi; @@ -133,8 +132,11 @@ public Server build() { filterChainSelectorManager, xdsClientPoolFactory, filterRegistry); } - @VisibleForTesting - XdsServerBuilder xdsClientPoolFactory(@Nullable XdsClientPoolFactory xdsClientPoolFactory, + /** + * Allows injecting {@link XdsClientPoolFactory} and/or overriding bootstrap configuration, useful + * for testing. + */ + public XdsServerBuilder xdsClientPoolFactory(@Nullable XdsClientPoolFactory xdsClientPoolFactory, @Nullable Map bootstrapOverride) { if (xdsClientPoolFactory != null) { this.xdsClientPoolFactory = xdsClientPoolFactory; From c28fed2f5ace8167d26e9f9ae0c33cf833c30415 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 7 Oct 2021 12:04:11 -0700 Subject: [PATCH 3/5] not public --- xds/src/main/java/io/grpc/xds/XdsServerBuilder.java | 3 +-- xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java | 2 +- xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index e98bf8a71c9..efffed858a5 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -36,7 +36,6 @@ import io.grpc.netty.NettyServerBuilder; import io.grpc.xds.FilterChainMatchingProtocolNegotiators.FilterChainMatchingNegotiatorServerFactory; import io.grpc.xds.XdsNameResolverProvider.XdsClientPoolFactory; - import java.util.Map; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -136,7 +135,7 @@ public Server build() { * Allows injecting {@link XdsClientPoolFactory} and/or overriding bootstrap configuration, useful * for testing. */ - public XdsServerBuilder xdsClientPoolFactory(@Nullable XdsClientPoolFactory xdsClientPoolFactory, + XdsServerBuilder xdsClientPoolFactoryForTest(@Nullable XdsClientPoolFactory xdsClientPoolFactory, @Nullable Map bootstrapOverride) { if (xdsClientPoolFactory != null) { this.xdsClientPoolFactory = xdsClientPoolFactory; diff --git a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java index ba50d7cbd6c..31de46a0ff1 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java @@ -337,7 +337,7 @@ private void buildServerWithFallbackServerCredentials( throws Exception { ServerCredentials xdsCredentials = XdsServerCredentials.create(fallbackCredentials); XdsServerBuilder builder = XdsServerBuilder.forPort(0, xdsCredentials) - .xdsClientPoolFactory(fakePoolFactory, null) + .xdsClientPoolFactoryForTest(fakePoolFactory, null) .addService(new SimpleServiceImpl()); buildServer(builder, downstreamTlsContext); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index eaf081d2c6b..b1fe87ea495 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -77,7 +77,7 @@ private void buildBuilder(XdsServerBuilder.XdsServingStatusListener xdsServingSt builder = XdsServerBuilder.forPort( port, XdsServerCredentials.create(InsecureServerCredentials.create())); - builder.xdsClientPoolFactory(new FakeXdsClientPoolFactory(xdsClient), null); + builder.xdsClientPoolFactoryForTest(new FakeXdsClientPoolFactory(xdsClient), null); if (xdsServingStatusListener != null) { builder.xdsServingStatusListener(xdsServingStatusListener); } From 4d21ce08ee4f3131182f06cfbc9d3ddeebee9da6 Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 7 Oct 2021 13:29:10 -0700 Subject: [PATCH 4/5] separate method overrideBootstrapForTest --- .../java/io/grpc/xds/XdsServerBuilder.java | 22 +++++++++---------- .../io/grpc/xds/XdsSdsClientServerTest.java | 2 +- .../io/grpc/xds/XdsServerBuilderTest.java | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index efffed858a5..d2937901938 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -22,6 +22,7 @@ import static io.grpc.xds.InternalXdsAttributes.ATTR_DRAIN_GRACE_NANOS; import static io.grpc.xds.InternalXdsAttributes.ATTR_FILTER_CHAIN_SELECTOR_MANAGER; +import com.google.common.annotations.VisibleForTesting; import com.google.errorprone.annotations.DoNotCall; import io.grpc.Attributes; import io.grpc.ExperimentalApi; @@ -40,7 +41,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; -import javax.annotation.Nullable; /** * A version of {@link ServerBuilder} to create xDS managed servers. @@ -131,18 +131,18 @@ public Server build() { filterChainSelectorManager, xdsClientPoolFactory, filterRegistry); } + @VisibleForTesting + XdsServerBuilder xdsClientPoolFactory(XdsClientPoolFactory xdsClientPoolFactory) { + this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory"); + return this; + } + /** - * Allows injecting {@link XdsClientPoolFactory} and/or overriding bootstrap configuration, useful - * for testing. + * Allows providing bootstrap override, useful for testing. */ - XdsServerBuilder xdsClientPoolFactoryForTest(@Nullable XdsClientPoolFactory xdsClientPoolFactory, - @Nullable Map bootstrapOverride) { - if (xdsClientPoolFactory != null) { - this.xdsClientPoolFactory = xdsClientPoolFactory; - } - if (bootstrapOverride != null) { - this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride); - } + public XdsServerBuilder overrideBootstrapForTest(Map bootstrapOverride) { + this.xdsClientPoolFactory.setBootstrapOverride( + checkNotNull(bootstrapOverride, "bootstrapOverride")); return this; } diff --git a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java index 31de46a0ff1..579542a2777 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java @@ -337,7 +337,7 @@ private void buildServerWithFallbackServerCredentials( throws Exception { ServerCredentials xdsCredentials = XdsServerCredentials.create(fallbackCredentials); XdsServerBuilder builder = XdsServerBuilder.forPort(0, xdsCredentials) - .xdsClientPoolFactoryForTest(fakePoolFactory, null) + .xdsClientPoolFactory(fakePoolFactory) .addService(new SimpleServiceImpl()); buildServer(builder, downstreamTlsContext); } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index b1fe87ea495..0d15c1f660e 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -77,7 +77,7 @@ private void buildBuilder(XdsServerBuilder.XdsServingStatusListener xdsServingSt builder = XdsServerBuilder.forPort( port, XdsServerCredentials.create(InsecureServerCredentials.create())); - builder.xdsClientPoolFactoryForTest(new FakeXdsClientPoolFactory(xdsClient), null); + builder.xdsClientPoolFactory(new FakeXdsClientPoolFactory(xdsClient)); if (xdsServingStatusListener != null) { builder.xdsServingStatusListener(xdsServingStatusListener); } From ad7fcde7bc809ba9c3831f72179b45d53537c89b Mon Sep 17 00:00:00 2001 From: yifeizhuang Date: Thu, 7 Oct 2021 14:34:32 -0700 Subject: [PATCH 5/5] new shared client pool provider --- xds/src/main/java/io/grpc/xds/XdsServerBuilder.java | 7 +++++-- .../test/java/io/grpc/xds/XdsServerBuilderTest.java | 13 ++++++++++++- .../test/java/io/grpc/xds/XdsServerTestHelper.java | 3 ++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java index d2937901938..d4df317a7e9 100644 --- a/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java +++ b/xds/src/main/java/io/grpc/xds/XdsServerBuilder.java @@ -141,8 +141,11 @@ XdsServerBuilder xdsClientPoolFactory(XdsClientPoolFactory xdsClientPoolFactory) * Allows providing bootstrap override, useful for testing. */ public XdsServerBuilder overrideBootstrapForTest(Map bootstrapOverride) { - this.xdsClientPoolFactory.setBootstrapOverride( - checkNotNull(bootstrapOverride, "bootstrapOverride")); + checkNotNull(bootstrapOverride, "bootstrapOverride"); + if (this.xdsClientPoolFactory == SharedXdsClientPoolProvider.getDefaultProvider()) { + this.xdsClientPoolFactory = new SharedXdsClientPoolProvider(); + } + this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride); return this; } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java index 0d15c1f660e..d67ed9d09fc 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java @@ -40,7 +40,9 @@ import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.SocketAddress; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -65,6 +67,7 @@ public class XdsServerBuilderTest { private int port; private TlsContextManager tlsContextManager; private FakeXdsClient xdsClient = new FakeXdsClient(); + private FakeXdsClientPoolFactory xdsClientPoolFactory = new FakeXdsClientPoolFactory(xdsClient); private void buildServer(XdsServerBuilder.XdsServingStatusListener xdsServingStatusListener) throws IOException { @@ -77,7 +80,7 @@ private void buildBuilder(XdsServerBuilder.XdsServingStatusListener xdsServingSt builder = XdsServerBuilder.forPort( port, XdsServerCredentials.create(InsecureServerCredentials.create())); - builder.xdsClientPoolFactory(new FakeXdsClientPoolFactory(xdsClient)); + builder.xdsClientPoolFactory(xdsClientPoolFactory); if (xdsServingStatusListener != null) { builder.xdsServingStatusListener(xdsServingStatusListener); } @@ -292,4 +295,12 @@ public void drainGraceTime_negativeThrows() throws IOException { assertThat(expected).hasMessageThat().contains("drain grace time"); } } + + @Test + public void testOverrideBootstrap() throws Exception { + Map b = new HashMap<>(); + buildBuilder(null); + builder.overrideBootstrapForTest(b); + assertThat(xdsClientPoolFactory.savedBootstrap).isEqualTo(b); + } } diff --git a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java index f289c4726fb..ffe4a72f522 100644 --- a/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java +++ b/xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java @@ -113,6 +113,7 @@ static final class FakeXdsClientPoolFactory implements XdsNameResolverProvider.XdsClientPoolFactory { private XdsClient xdsClient; + Map savedBootstrap; FakeXdsClientPoolFactory(XdsClient xdsClient) { this.xdsClient = xdsClient; @@ -120,7 +121,7 @@ static final class FakeXdsClientPoolFactory @Override public void setBootstrapOverride(Map bootstrap) { - throw new UnsupportedOperationException("Should not be called"); + this.savedBootstrap = bootstrap; } @Override