Skip to content
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

Jump to bottom

xds: allow injecting bootstrapOverride in xdsNameResolverProvider #8358

Merged
merged 5 commits into from Aug 11, 2021
Merged

xds: allow injecting bootstrapOverride in xdsNameResolverProvider #8358

Show file tree
Hide file tree
Changes from 2 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions xds/src/main/java/io/grpc/xds/XdsNameResolver.java
View file
Open in desktop
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
Member

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
15 changes: 12 additions & 3 deletions xds/src/main/java/io/grpc/xds/XdsNameResolverProvider.java
View file
Open in desktop
Expand Up @@ -43,8 +43,11 @@ public final class XdsNameResolverProvider extends NameResolverProvider {

private static final String SCHEME = "xds";

@Override
public XdsNameResolver newNameResolver(URI targetUri, Args args) {
/**
* Allows injecting bootstrapOverride to the name resolver.
* */
public XdsNameResolver newNameResolver(URI targetUri, Args args,
Copy link
Member

Choose a reason for hiding this comment

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

Who would call this method? I think we want bootstrapOverride to be passed to a constructor/factoryMethod for the provider itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, i didn't test it.
The bootstrapOverride needs to be saved here and then passed to xdsNameResolver when channel creates a new resolver.

@Nullable Map<String, ?> bootstrapOverride) {
if (SCHEME.equals(targetUri.getScheme())) {
String targetPath = checkNotNull(targetUri.getPath(), "targetPath");
Preconditions.checkArgument(
Expand All @@ -54,11 +57,17 @@ 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 XdsNameResolver newNameResolver(URI targetUri, Args args) {
return newNameResolver(targetUri, args, null);
}

@Override
public String getDefaultScheme() {
return SCHEME;
Expand Down
42 changes: 35 additions & 7 deletions xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
View file
Open in desktop
Expand Up @@ -37,6 +37,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.SettableFuture;
import com.google.protobuf.util.Durations;
import com.google.re2j.Pattern;
import io.grpc.CallOptions;
Expand Down Expand Up @@ -83,6 +84,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 +155,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 +174,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 +187,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 All @@ -196,6 +198,34 @@ public ObjectPool<XdsClient> getOrCreate() throws XdsInitializationException {
assertThat(error.getCause()).hasMessageThat().isEqualTo("Fail to read bootstrap file");
}

@Test
public void overrideBootstrap() throws Exception {
final SettableFuture<Map<String, ?>> bootstrapSettable = SettableFuture.create();
XdsClientPoolFactory xdsClientPoolFactory = new XdsClientPoolFactory() {

@Override
public void setBootstrapOverride(Map<String, ?> bootstrap) {
assertThat(bootstrapSettable.set(bootstrap)).isTrue();
}

@Override
@Nullable
public ObjectPool<XdsClient> get() {
throw new UnsupportedOperationException("Should not be called");
}

@Override
public ObjectPool<XdsClient> getOrCreate() throws XdsInitializationException {
assertThat(bootstrapSettable.isDone()).isTrue();
return null;
}
};
Map<String, String> b = new HashMap<>();
resolver = new XdsNameResolver(AUTHORITY, serviceConfigParser, syncContext, scheduler,
xdsClientPoolFactory, mockRandom, FilterRegistry.getDefaultRegistry(), b);
assertThat(bootstrapSettable.get()).isSameInstanceAs(b);
}

@Test
public void resolving_ldsResourceNotFound() {
resolver.start(mockListener);
Expand Down Expand Up @@ -437,7 +467,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 +670,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 +1731,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