xds: Check for env variable before doing custom LB config (#9165)

This commit is contained in:
Terry Wilson 2022-05-12 12:57:11 -07:00 committed by GitHub
parent 36d1d5fe45
commit c6bfce034f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 21 deletions

View File

@ -162,6 +162,12 @@ final class ClientXdsClient extends XdsClient implements XdsResponseHandler, Res
!Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST")) !Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST"))
? Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST")) ? Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST"))
: Boolean.parseBoolean(System.getProperty("io.grpc.xds.experimentalEnableLeastRequest")); : Boolean.parseBoolean(System.getProperty("io.grpc.xds.experimentalEnableLeastRequest"));
@VisibleForTesting
static boolean enableCustomLbConfig =
!Strings.isNullOrEmpty(System.getenv("GRPC_EXPERIMENTAL_ENABLE_CUSTOM_LB_CONFIG"))
? Boolean.parseBoolean(System.getenv("GRPC_EXPERIMENTAL_ENABLE_CUSTOM_LB_CONFIG"))
: Boolean.parseBoolean(
System.getProperty("io.grpc.xds.experimentalEnableCustomLbConfig"));
private static final String TYPE_URL_HTTP_CONNECTION_MANAGER_V2 = private static final String TYPE_URL_HTTP_CONNECTION_MANAGER_V2 =
"type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2" "type.googleapis.com/envoy.config.filter.network.http_connection_manager.v2"
+ ".HttpConnectionManager"; + ".HttpConnectionManager";
@ -1636,7 +1642,7 @@ final class ClientXdsClient extends XdsClient implements XdsResponseHandler, Res
CdsUpdate.Builder updateBuilder = structOrError.getStruct(); CdsUpdate.Builder updateBuilder = structOrError.getStruct();
ImmutableMap<String, ?> lbPolicyConfig = LoadBalancerConfigFactory.newConfig(cluster, ImmutableMap<String, ?> lbPolicyConfig = LoadBalancerConfigFactory.newConfig(cluster,
enableLeastRequest); enableLeastRequest, enableCustomLbConfig);
// Validate the LB config by trying to parse it with the corresponding LB provider. // Validate the LB config by trying to parse it with the corresponding LB provider.
LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(lbPolicyConfig); LbConfig lbConfig = ServiceConfigUtil.unwrapLoadBalancingConfig(lbPolicyConfig);

View File

@ -78,11 +78,12 @@ class LoadBalancerConfigFactory {
* *
* @throws ResourceInvalidException If the {@link Cluster} has an invalid LB configuration. * @throws ResourceInvalidException If the {@link Cluster} has an invalid LB configuration.
*/ */
static ImmutableMap<String, ?> newConfig(Cluster cluster, boolean enableLeastRequest) static ImmutableMap<String, ?> newConfig(Cluster cluster, boolean enableLeastRequest,
boolean enableCustomLbConfig)
throws ResourceInvalidException { throws ResourceInvalidException {
// The new load_balancing_policy will always be used if it is set, but for backward // The new load_balancing_policy will always be used if it is set, but for backward
// compatibility we will fall back to using the old lb_policy field if the new field is not set. // compatibility we will fall back to using the old lb_policy field if the new field is not set.
if (cluster.hasLoadBalancingPolicy()) { if (cluster.hasLoadBalancingPolicy() && enableCustomLbConfig) {
try { try {
return LoadBalancingPolicyConverter.convertToServiceConfig(cluster.getLoadBalancingPolicy(), return LoadBalancingPolicyConverter.convertToServiceConfig(cluster.getLoadBalancingPolicy(),
0); 0);

View File

@ -106,14 +106,14 @@ public class LoadBalancerConfigFactoryTest {
public void roundRobin() throws ResourceInvalidException { public void roundRobin() throws ResourceInvalidException {
Cluster cluster = newCluster(buildWrrPolicy(ROUND_ROBIN_POLICY)); Cluster cluster = newCluster(buildWrrPolicy(ROUND_ROBIN_POLICY));
assertThat(newLbConfig(cluster, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG);
} }
@Test @Test
public void roundRobin_legacy() throws ResourceInvalidException { public void roundRobin_legacy() throws ResourceInvalidException {
Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.ROUND_ROBIN).build(); Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.ROUND_ROBIN).build();
assertThat(newLbConfig(cluster, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG);
} }
@Test @Test
@ -122,7 +122,7 @@ public class LoadBalancerConfigFactoryTest {
.setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(RING_HASH_POLICY)) .setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(RING_HASH_POLICY))
.build(); .build();
assertThat(newLbConfig(cluster, true)).isEqualTo(VALID_RING_HASH_CONFIG); assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_RING_HASH_CONFIG);
} }
@Test @Test
@ -132,7 +132,7 @@ public class LoadBalancerConfigFactoryTest {
.setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE)) .setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE))
.setHashFunction(HashFunction.XX_HASH)).build(); .setHashFunction(HashFunction.XX_HASH)).build();
assertThat(newLbConfig(cluster, true)).isEqualTo(VALID_RING_HASH_CONFIG); assertThat(newLbConfig(cluster, true, true)).isEqualTo(VALID_RING_HASH_CONFIG);
} }
@Test @Test
@ -144,7 +144,7 @@ public class LoadBalancerConfigFactoryTest {
.setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE)) .setMaximumRingSize(UInt64Value.of(RING_HASH_MAX_RING_SIZE))
.setHashFunction(RingHash.HashFunction.MURMUR_HASH_2).build()))).build()); .setHashFunction(RingHash.HashFunction.MURMUR_HASH_2).build()))).build());
assertResourceInvalidExceptionThrown(cluster, true, "Invalid ring hash function"); assertResourceInvalidExceptionThrown(cluster, true, true, "Invalid ring hash function");
} }
@Test @Test
@ -152,7 +152,7 @@ public class LoadBalancerConfigFactoryTest {
Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.RING_HASH).setRingHashLbConfig( Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.RING_HASH).setRingHashLbConfig(
RingHashLbConfig.newBuilder().setHashFunction(HashFunction.MURMUR_HASH_2)).build(); RingHashLbConfig.newBuilder().setHashFunction(HashFunction.MURMUR_HASH_2)).build();
assertResourceInvalidExceptionThrown(cluster, true, "invalid ring hash function"); assertResourceInvalidExceptionThrown(cluster, true, true, "invalid ring hash function");
} }
@Test @Test
@ -163,7 +163,7 @@ public class LoadBalancerConfigFactoryTest {
.setLeastRequestLbConfig( .setLeastRequestLbConfig(
LeastRequestLbConfig.newBuilder().setChoiceCount(UInt32Value.of(10))).build(); LeastRequestLbConfig.newBuilder().setChoiceCount(UInt32Value.of(10))).build();
LbConfig lbConfig = newLbConfig(cluster, true); LbConfig lbConfig = newLbConfig(cluster, true, true);
assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental"); assertThat(lbConfig.getPolicyName()).isEqualTo("wrr_locality_experimental");
List<LbConfig> childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList( List<LbConfig> childConfigs = ServiceConfigUtil.unwrapLoadBalancingConfigList(
@ -180,14 +180,14 @@ public class LoadBalancerConfigFactoryTest {
Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.LEAST_REQUEST).build(); Cluster cluster = Cluster.newBuilder().setLbPolicy(LbPolicy.LEAST_REQUEST).build();
assertResourceInvalidExceptionThrown(cluster, false, "unsupported lb policy"); assertResourceInvalidExceptionThrown(cluster, false, true, "unsupported lb policy");
} }
@Test @Test
public void customRootLb_providerRegistered() throws ResourceInvalidException { public void customRootLb_providerRegistered() throws ResourceInvalidException {
LoadBalancerRegistry.getDefaultRegistry().register(CUSTOM_POLICY_PROVIDER); LoadBalancerRegistry.getDefaultRegistry().register(CUSTOM_POLICY_PROVIDER);
assertThat(newLbConfig(newCluster(CUSTOM_POLICY), false)).isEqualTo(VALID_CUSTOM_CONFIG); assertThat(newLbConfig(newCluster(CUSTOM_POLICY), false, true)).isEqualTo(VALID_CUSTOM_CONFIG);
} }
@Test @Test
@ -196,7 +196,7 @@ public class LoadBalancerConfigFactoryTest {
.setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(CUSTOM_POLICY)) .setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder().addPolicies(CUSTOM_POLICY))
.build(); .build();
assertResourceInvalidExceptionThrown(cluster, false, "Invalid LoadBalancingPolicy"); assertResourceInvalidExceptionThrown(cluster, false, true, "Invalid LoadBalancingPolicy");
} }
// When a provider for the endpoint picking custom policy is available, the configuration should // When a provider for the endpoint picking custom policy is available, the configuration should
@ -208,7 +208,7 @@ public class LoadBalancerConfigFactoryTest {
Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder() Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder()
.addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build(); .addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build();
assertThat(newLbConfig(cluster, false)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR); assertThat(newLbConfig(cluster, false, true)).isEqualTo(VALID_CUSTOM_CONFIG_IN_WRR);
} }
// When a provider for the custom wrr_locality child policy is NOT available, we should fall back // When a provider for the custom wrr_locality child policy is NOT available, we should fall back
@ -218,7 +218,7 @@ public class LoadBalancerConfigFactoryTest {
Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder() Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(LoadBalancingPolicy.newBuilder()
.addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build(); .addPolicies(buildWrrPolicy(CUSTOM_POLICY, ROUND_ROBIN_POLICY))).build();
assertThat(newLbConfig(cluster, false)).isEqualTo(VALID_ROUND_ROBIN_CONFIG); assertThat(newLbConfig(cluster, false, true)).isEqualTo(VALID_ROUND_ROBIN_CONFIG);
} }
// When a provider for the custom wrr_locality child policy is NOT available and no alternative // When a provider for the custom wrr_locality child policy is NOT available and no alternative
@ -228,7 +228,18 @@ public class LoadBalancerConfigFactoryTest {
Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy( Cluster cluster = Cluster.newBuilder().setLoadBalancingPolicy(
LoadBalancingPolicy.newBuilder().addPolicies(buildWrrPolicy(CUSTOM_POLICY))).build(); LoadBalancingPolicy.newBuilder().addPolicies(buildWrrPolicy(CUSTOM_POLICY))).build();
assertResourceInvalidExceptionThrown(cluster, false, "Invalid LoadBalancingPolicy"); assertResourceInvalidExceptionThrown(cluster, false, true, "Invalid LoadBalancingPolicy");
}
@Test
public void customConfig_notEnabled() throws ResourceInvalidException {
Cluster cluster = Cluster.newBuilder()
.setLoadBalancingPolicy(
LoadBalancingPolicy.newBuilder().addPolicies(RING_HASH_POLICY))
.build();
// Custom LB flag not set, so we use old logic that will default to round_robin.
assertThat(newLbConfig(cluster, true, false)).isEqualTo(VALID_ROUND_ROBIN_CONFIG);
} }
@Test @Test
@ -255,7 +266,7 @@ public class LoadBalancerConfigFactoryTest {
buildWrrPolicy( buildWrrPolicy(
ROUND_ROBIN_POLICY))))))))))))))))))).build(); ROUND_ROBIN_POLICY))))))))))))))))))).build();
assertResourceInvalidExceptionThrown(cluster, false, assertResourceInvalidExceptionThrown(cluster, false, true,
"Maximum LB config recursion depth reached"); "Maximum LB config recursion depth reached");
} }
@ -271,16 +282,17 @@ public class LoadBalancerConfigFactoryTest {
.build()))).build(); .build()))).build();
} }
private LbConfig newLbConfig(Cluster cluster, boolean enableLeastRequest) private LbConfig newLbConfig(Cluster cluster, boolean enableLeastRequest,
boolean enableCustomConfig)
throws ResourceInvalidException { throws ResourceInvalidException {
return ServiceConfigUtil.unwrapLoadBalancingConfig( return ServiceConfigUtil.unwrapLoadBalancingConfig(
LoadBalancerConfigFactory.newConfig(cluster, enableLeastRequest)); LoadBalancerConfigFactory.newConfig(cluster, enableLeastRequest, enableCustomConfig));
} }
private void assertResourceInvalidExceptionThrown(Cluster cluster, boolean enableLeastRequest, private void assertResourceInvalidExceptionThrown(Cluster cluster, boolean enableLeastRequest,
String expectedMessage) { boolean enableCustomConfig, String expectedMessage) {
try { try {
newLbConfig(cluster, enableLeastRequest); newLbConfig(cluster, enableLeastRequest, enableCustomConfig);
} catch (ResourceInvalidException e) { } catch (ResourceInvalidException e) {
assertThat(e).hasMessageThat().contains(expectedMessage); assertThat(e).hasMessageThat().contains(expectedMessage);
return; return;