Skip to content

Commit

Permalink
xds: Make XdsClient.ResourceStore package-private
Browse files Browse the repository at this point in the history 
There's no reason to use the interface outside of
XdsClientImpl/ControlPlaneClient. Since XdsClientImpl implements the
interface directly, its methods are still public. That can be a future
cleanup.
  • Loading branch information
@ejona86
ejona86 committed Jan 3, 2025
1 parent bac8b32 commit 9a712c3
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 60 deletions.
2 changes: 1 addition & 1 deletion xds/src/main/java/io/grpc/xds/client/XdsClient.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ void handleResourceResponse(
void handleStreamClosed(Status error, boolean shouldTryFallback);
}

public interface ResourceStore {
interface ResourceStore {

/**
* Returns the collection of resources currently subscribed to which have an authority matching
Expand Down
74 changes: 15 additions & 59 deletions xds/src/test/java/io/grpc/xds/CsdsServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ private void verifyResponse(ClientStatusResponse response) {
assertThat(response.getConfigCount()).isEqualTo(1);
ClientConfig clientConfig = response.getConfig(0);
verifyClientConfigNode(clientConfig);
verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig);
assertThat(clientConfig.getGenericXdsConfigsList()).isEmpty();
assertThat(clientConfig.getClientScope()).isEmpty();
}

Expand All @@ -310,7 +310,7 @@ private Collection<String> verifyMultiResponse(ClientStatusResponse response, in
for (int i = 0; i < numExpected; i++) {
ClientConfig clientConfig = response.getConfig(i);
verifyClientConfigNode(clientConfig);
verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig);
assertThat(clientConfig.getGenericXdsConfigsList()).isEmpty();
clientScopes.add(clientConfig.getClientScope());
}

Expand Down Expand Up @@ -382,16 +382,6 @@ public void getClientConfigForXdsClient_subscribedResourcesToGenericXdsConfig()
.put(EDS, ImmutableMap.of("subscribedResourceName.EDS", METADATA_ACKED_EDS))
.buildOrThrow();
}

@Override
public Map<String, XdsResourceType<?>> getSubscribedResourceTypesWithTypeUrl() {
return ImmutableMap.of(
LDS.typeUrl(), LDS,
RDS.typeUrl(), RDS,
CDS.typeUrl(), CDS,
EDS.typeUrl(), EDS
);
}
};
ClientConfig clientConfig = CsdsService.getClientConfigForXdsClient(fakeXdsClient,
FAKE_CLIENT_SCOPE);
Expand All @@ -403,31 +393,31 @@ public Map<String, XdsResourceType<?>> getSubscribedResourceTypesWithTypeUrl() {
// is propagated to the correct resource types.
int xdsConfigCount = clientConfig.getGenericXdsConfigsCount();
assertThat(xdsConfigCount).isEqualTo(4);
Map<XdsResourceType<?>, GenericXdsConfig> configDumps = mapConfigDumps(fakeXdsClient,
clientConfig);
assertThat(configDumps.keySet()).containsExactly(LDS, RDS, CDS, EDS);
Map<String, GenericXdsConfig> configDumps = mapConfigDumps(clientConfig);
assertThat(configDumps.keySet())
.containsExactly(LDS.typeUrl(), RDS.typeUrl(), CDS.typeUrl(), EDS.typeUrl());

// LDS.
GenericXdsConfig genericXdsConfigLds = configDumps.get(LDS);
GenericXdsConfig genericXdsConfigLds = configDumps.get(LDS.typeUrl());
assertThat(genericXdsConfigLds.getName()).isEqualTo("subscribedResourceName.LDS");
assertThat(genericXdsConfigLds.getClientStatus()).isEqualTo(ClientResourceStatus.ACKED);
assertThat(genericXdsConfigLds.getVersionInfo()).isEqualTo(VERSION_ACK_LDS);
assertThat(genericXdsConfigLds.getXdsConfig()).isEqualTo(RAW_LISTENER);

// RDS.
GenericXdsConfig genericXdsConfigRds = configDumps.get(RDS);
GenericXdsConfig genericXdsConfigRds = configDumps.get(RDS.typeUrl());
assertThat(genericXdsConfigRds.getClientStatus()).isEqualTo(ClientResourceStatus.ACKED);
assertThat(genericXdsConfigRds.getVersionInfo()).isEqualTo(VERSION_ACK_RDS);
assertThat(genericXdsConfigRds.getXdsConfig()).isEqualTo(RAW_ROUTE_CONFIGURATION);

// CDS.
GenericXdsConfig genericXdsConfigCds = configDumps.get(CDS);
GenericXdsConfig genericXdsConfigCds = configDumps.get(CDS.typeUrl());
assertThat(genericXdsConfigCds.getClientStatus()).isEqualTo(ClientResourceStatus.ACKED);
assertThat(genericXdsConfigCds.getVersionInfo()).isEqualTo(VERSION_ACK_CDS);
assertThat(genericXdsConfigCds.getXdsConfig()).isEqualTo(RAW_CLUSTER);

// RDS.
GenericXdsConfig genericXdsConfigEds = configDumps.get(EDS);
GenericXdsConfig genericXdsConfigEds = configDumps.get(EDS.typeUrl());
assertThat(genericXdsConfigEds.getClientStatus()).isEqualTo(ClientResourceStatus.ACKED);
assertThat(genericXdsConfigEds.getVersionInfo()).isEqualTo(VERSION_ACK_EDS);
assertThat(genericXdsConfigEds.getXdsConfig()).isEqualTo(RAW_CLUSTER_LOAD_ASSIGNMENT);
Expand All @@ -438,23 +428,11 @@ public void getClientConfigForXdsClient_noSubscribedResources() throws Interrupt
ClientConfig clientConfig =
CsdsService.getClientConfigForXdsClient(XDS_CLIENT_NO_RESOURCES, FAKE_CLIENT_SCOPE);
verifyClientConfigNode(clientConfig);
verifyClientConfigNoResources(XDS_CLIENT_NO_RESOURCES, clientConfig);
assertThat(clientConfig.getGenericXdsConfigsList()).isEmpty();
assertThat(clientConfig.getClientScope()).isEqualTo(FAKE_CLIENT_SCOPE);
}
}

/**
* Assuming {@link MetadataToProtoTests} passes, and metadata converted to corresponding
* config dumps correctly, perform a minimal verification of the general shape of ClientConfig.
*/
private static void verifyClientConfigNoResources(FakeXdsClient xdsClient,
ClientConfig clientConfig) {
int xdsConfigCount = clientConfig.getGenericXdsConfigsCount();
assertThat(xdsConfigCount).isEqualTo(0);
Map<XdsResourceType<?>, GenericXdsConfig> configDumps = mapConfigDumps(xdsClient, clientConfig);
assertThat(configDumps).isEmpty();
}

/**
* Assuming {@link EnvoyProtoDataTest#convertNode} passes, perform a minimal check,
* just verify the node itself is the one we expect.
Expand All @@ -465,21 +443,17 @@ private static void verifyClientConfigNode(ClientConfig clientConfig) {
assertThat(node).isEqualTo(BOOTSTRAP_NODE.toEnvoyProtoNode());
}

private static Map<XdsResourceType<?>, GenericXdsConfig> mapConfigDumps(FakeXdsClient client,
ClientConfig config) {
Map<XdsResourceType<?>, GenericXdsConfig> xdsConfigMap = new HashMap<>();
private static Map<String, GenericXdsConfig> mapConfigDumps(ClientConfig config) {
Map<String, GenericXdsConfig> xdsConfigMap = new HashMap<>();
List<GenericXdsConfig> xdsConfigList = config.getGenericXdsConfigsList();
for (GenericXdsConfig genericXdsConfig : xdsConfigList) {
XdsResourceType<?> type = client.getSubscribedResourceTypesWithTypeUrl()
.get(genericXdsConfig.getTypeUrl());
assertThat(type).isNotNull();
assertThat(xdsConfigMap).doesNotContainKey(type);
xdsConfigMap.put(type, genericXdsConfig);
assertThat(xdsConfigMap).doesNotContainKey(genericXdsConfig.getTypeUrl());
xdsConfigMap.put(genericXdsConfig.getTypeUrl(), genericXdsConfig);
}
return xdsConfigMap;
}

private static class FakeXdsClient extends XdsClient implements XdsClient.ResourceStore {
private static class FakeXdsClient extends XdsClient {
protected Map<XdsResourceType<?>, Map<String, ResourceMetadata>>
getSubscribedResourcesMetadata() {
return ImmutableMap.of();
Expand All @@ -495,24 +469,6 @@ private static class FakeXdsClient extends XdsClient implements XdsClient.Resour
public BootstrapInfo getBootstrapInfo() {
return BOOTSTRAP_INFO;
}

@Nullable
@Override
public Collection<String> getSubscribedResources(
ServerInfo serverInfo, XdsResourceType<? extends ResourceUpdate> type) {
return null;
}

@Override
public void startMissingResourceTimers(Collection<String> resourceNames,
XdsResourceType<?> resourceType) {
// do nothing
}

@Override
public Map<String, XdsResourceType<?>> getSubscribedResourceTypesWithTypeUrl() {
return ImmutableMap.of();
}
}

private static class FakeXdsClientPoolFactory implements XdsClientPoolFactory {
Expand Down

0 comments on commit 9a712c3

Please sign in to comment.