Skip to content

xds: allow injecting bootstrapOverride in xdsNameResolverProvider #8358

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
YifeiZhuang merged 5 commits into from
Aug 11, 2021
Merged

xds: allow injecting bootstrapOverride in xdsNameResolverProvider #8358

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
84cacc7
add override bootstrap in XdsNameResolver
YifeiZhuang Jul 13, 2021
00fd6da
Merge branch 'master' of https://github.com/grpc/grpc-java into zivy/…
YifeiZhuang Jul 30, 2021
8f38f34
fix
YifeiZhuang Aug 6, 2021
cf4b667
fix
YifeiZhuang Aug 6, 2021
e09f744
fix...
YifeiZhuang Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions xds/src/main/java/io/grpc/xds/XdsNameResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,22 +124,24 @@ final class XdsNameResolver extends NameResolver {
private ResolveState resolveState;

XdsNameResolver(String name, ServiceConfigParser serviceConfigParser,
SynchronizationContext syncContext, ScheduledExecutorService scheduler) {
SynchronizationContext syncContext, ScheduledExecutorService scheduler,
@Nullable Map<String, ?> bootstrapOverride) {
this(name, serviceConfigParser, syncContext, scheduler,
SharedXdsClientPoolProvider.getDefaultProvider(), ThreadSafeRandomImpl.instance,
FilterRegistry.getDefaultRegistry());
FilterRegistry.getDefaultRegistry(), bootstrapOverride);
}

@VisibleForTesting
XdsNameResolver(String name, ServiceConfigParser serviceConfigParser,
SynchronizationContext syncContext, ScheduledExecutorService scheduler,
XdsClientPoolFactory xdsClientPoolFactory, ThreadSafeRandom random,
FilterRegistry filterRegistry) {
FilterRegistry filterRegistry, @Nullable Map<String, ?> bootstrapOverride) {
authority = GrpcUtil.checkAuthority(checkNotNull(name, "name"));
this.serviceConfigParser = checkNotNull(serviceConfigParser, "serviceConfigParser");
this.syncContext = checkNotNull(syncContext, "syncContext");
this.scheduler = checkNotNull(scheduler, "scheduler");
this.xdsClientPoolFactory = checkNotNull(xdsClientPoolFactory, "xdsClientPoolFactory");
this.xdsClientPoolFactory.setBootstrapOverride(bootstrapOverride);
Copy link
Copy Markdown
Member

@ejona86 ejona86 Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mutates the default factory, which would impact other xds clients. Create a new instance if bootstrapOverride is set.

this.random = checkNotNull(random, "random");
this.filterRegistry = checkNotNull(filterRegistry, "filterRegistry");
logId = InternalLogId.allocate("xds-resolver", name);
Expand Down
22 changes: 18 additions & 4 deletions xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,24 @@
@Internal
public final class XdsNameResolverProvider extends NameResolverProvider {

private static final String SCHEME = "xds";
private String scheme = "xds";
private Map<String, ?> bootstrapOverride;

/**
* A convenient method to create a {@link XdsNameResolverProvider} with custom scheme and
* bootstrap.
*/
public static XdsNameResolverProvider createForTest(String scheme,
@Nullable Map<String, ?> bootstrapOverride) {
XdsNameResolverProvider provider = new XdsNameResolverProvider();
provider.scheme = checkNotNull(scheme, "scheme");
Copy link
Copy Markdown
Member

@ejona86 ejona86 Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a constructor? That'd allow the fields to be final. You'd need to make a no-arg constructor as well.

provider.bootstrapOverride = bootstrapOverride;
return provider;
}

@Override
public XdsNameResolver newNameResolver(URI targetUri, Args args) {
if (SCHEME.equals(targetUri.getScheme())) {
if (scheme.equals(targetUri.getScheme())) {
String targetPath = checkNotNull(targetUri.getPath(), "targetPath");
Preconditions.checkArgument(
targetPath.startsWith("/"),
Expand All @@ -54,14 +67,15 @@ public XdsNameResolver newNameResolver(URI targetUri, Args args) {
targetUri);
String name = targetPath.substring(1);
return new XdsNameResolver(name, args.getServiceConfigParser(),
args.getSynchronizationContext(), args.getScheduledExecutorService());
args.getSynchronizationContext(), args.getScheduledExecutorService(),
bootstrapOverride);
}
return null;
}

@Override
public String getDefaultScheme() {
return SCHEME;
return scheme;
}

@Override
Expand Down
50 changes: 50 additions & 0 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,20 @@
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;

import com.google.common.collect.ImmutableMap;
import io.grpc.ChannelLogger;
import io.grpc.InternalServiceProviders;
import io.grpc.NameResolver;
import io.grpc.NameResolver.ServiceConfigParser;
import io.grpc.NameResolverProvider;
import io.grpc.NameResolverRegistry;
import io.grpc.SynchronizationContext;
import io.grpc.internal.FakeClock;
import io.grpc.internal.GrpcUtil;
import java.net.URI;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
Expand Down Expand Up @@ -114,4 +119,49 @@ public void invalidName_hostnameContainsUnderscore() {
// Expected
}
}

@Test
public void newProvider_createForTest() {
NameResolverRegistry registry = new NameResolverRegistry();
XdsNameResolverProvider provider0 = XdsNameResolverProvider.createForTest("no-scheme", null);
registry.register(provider0);
XdsNameResolverProvider provider1 = XdsNameResolverProvider.createForTest("new-xds-scheme",
new HashMap<String, String>());
registry.register(provider1);
assertThat(registry.asFactory()
.newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNotNull();
assertThat(registry.asFactory()
.newNameResolver(URI.create("no-scheme:///localhost"), args)).isNotNull();
registry.deregister(provider1);
assertThat(registry.asFactory()
.newNameResolver(URI.create("new-xds-scheme:///localhost"), args)).isNull();
registry.deregister(provider0);
}

@Test
public void newProvider_createForTest_overrideBootstrap() {
Map<String, ?> b = ImmutableMap.of(
"node", ImmutableMap.of(
"id", "ENVOY_NODE_ID",
"cluster", "ENVOY_CLUSTER"),
"xds_servers", Collections.singletonList(
ImmutableMap.of(
"server_uri", "trafficdirector.googleapis.com:443",
"channel_creds", Collections.singletonList(
ImmutableMap.of("type", "insecure")
)
)
)
);
NameResolverRegistry registry = new NameResolverRegistry();
XdsNameResolverProvider provider = XdsNameResolverProvider.createForTest("no-scheme", b);
registry.register(provider);
NameResolver resolver = registry.asFactory()
.newNameResolver(URI.create("no-scheme:///localhost"), args);
resolver.start(mock(NameResolver.Listener2.class));
assertThat(resolver).isInstanceOf(XdsNameResolver.class);
assertThat(((XdsNameResolver)resolver).getXdsClient().getBootstrapInfo().getNode().getId())
.isEqualTo("ENVOY_NODE_ID");
registry.deregister(provider);
}
}
13 changes: 6 additions & 7 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ScheduledExecutorService;
Expand Down Expand Up @@ -153,7 +154,7 @@ public void setUp() {
new FaultFilter(mockRandom, new AtomicLong()),
RouterFilter.INSTANCE);
resolver = new XdsNameResolver(AUTHORITY, serviceConfigParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, filterRegistry);
xdsClientPoolFactory, mockRandom, filterRegistry, null);
}

@After
Expand All @@ -172,7 +173,6 @@ public void resolving_failToCreateXdsClientPool() {
XdsClientPoolFactory xdsClientPoolFactory = new XdsClientPoolFactory() {
@Override
public void setBootstrapOverride(Map<String, ?> bootstrap) {
throw new UnsupportedOperationException("Should not be called");
}

@Override
Expand All @@ -186,8 +186,9 @@ public ObjectPool<XdsClient> getOrCreate() throws XdsInitializationException {
throw new XdsInitializationException("Fail to read bootstrap file");
}
};
Map<String, String> b = new HashMap<>();
resolver = new XdsNameResolver(AUTHORITY, serviceConfigParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry());
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), b);
resolver.start(mockListener);
verify(mockListener).onError(errorCaptor.capture());
Status error = errorCaptor.getValue();
Expand Down Expand Up @@ -437,7 +438,7 @@ public void retryPolicyInPerMethodConfigGeneratedByResolverIsValid() {
ServiceConfigParser realParser = new ScParser(
true, 5, 5, new AutoConfiguredLoadBalancerFactory("pick-first"));
resolver = new XdsNameResolver(AUTHORITY, realParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry());
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);
resolver.start(mockListener);
FakeXdsClient xdsClient = (FakeXdsClient) resolver.getXdsClient();
RetryPolicy retryPolicy = RetryPolicy.create(
Expand Down Expand Up @@ -640,7 +641,7 @@ public void resolved_rpcHashingByChannelId() {
resolver.shutdown();
reset(mockListener);
resolver = new XdsNameResolver(AUTHORITY, serviceConfigParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry());
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), null);
resolver.start(mockListener);
xdsClient = (FakeXdsClient) resolver.getXdsClient();
xdsClient.deliverLdsUpdate(
Expand Down Expand Up @@ -1701,10 +1702,8 @@ public void routeMatching_withHeaders() {
}

private final class FakeXdsClientPoolFactory implements XdsClientPoolFactory {

@Override
public void setBootstrapOverride(Map<String, ?> bootstrap) {
throw new UnsupportedOperationException("Should not be called");
}

@Override
Expand Down