xds: Use acceptResolvedAddresses() for WeightedTarget children (#12053)

Convert the tests to use acceptResolvedAddresses() as well.
This commit is contained in:
Eric Anderson 2025-05-07 23:04:16 -07:00 committed by GitHub
parent 3f5fdf1266
commit 80cc988b3c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 26 additions and 12 deletions

View File

@ -87,8 +87,9 @@ final class WeightedTargetLoadBalancer extends LoadBalancer {
} }
} }
targets = newTargets; targets = newTargets;
Status status = Status.OK;
for (String targetName : targets.keySet()) { for (String targetName : targets.keySet()) {
childBalancers.get(targetName).handleResolvedAddresses( Status newStatus = childBalancers.get(targetName).acceptResolvedAddresses(
resolvedAddresses.toBuilder() resolvedAddresses.toBuilder()
.setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), targetName)) .setAddresses(AddressFilter.filter(resolvedAddresses.getAddresses(), targetName))
.setLoadBalancingPolicyConfig(targets.get(targetName).childConfig) .setLoadBalancingPolicyConfig(targets.get(targetName).childConfig)
@ -96,6 +97,9 @@ final class WeightedTargetLoadBalancer extends LoadBalancer {
.set(CHILD_NAME, targetName) .set(CHILD_NAME, targetName)
.build()) .build())
.build()); .build());
if (!newStatus.isOk()) {
status = newStatus;
}
} }
// Cleanup removed targets. // Cleanup removed targets.
@ -108,7 +112,7 @@ final class WeightedTargetLoadBalancer extends LoadBalancer {
childBalancers.keySet().retainAll(targets.keySet()); childBalancers.keySet().retainAll(targets.keySet());
childHelpers.keySet().retainAll(targets.keySet()); childHelpers.keySet().retainAll(targets.keySet());
updateOverallBalancingState(); updateOverallBalancingState();
return Status.OK; return status;
} }
@Override @Override

View File

@ -113,6 +113,7 @@ public class WeightedTargetLoadBalancerTest {
public LoadBalancer newLoadBalancer(Helper helper) { public LoadBalancer newLoadBalancer(Helper helper) {
childHelpers.add(helper); childHelpers.add(helper);
LoadBalancer childBalancer = mock(LoadBalancer.class); LoadBalancer childBalancer = mock(LoadBalancer.class);
when(childBalancer.acceptResolvedAddresses(any())).thenReturn(Status.OK);
childBalancers.add(childBalancer); childBalancers.add(childBalancer);
fooLbCreated++; fooLbCreated++;
return childBalancer; return childBalancer;
@ -139,6 +140,7 @@ public class WeightedTargetLoadBalancerTest {
public LoadBalancer newLoadBalancer(Helper helper) { public LoadBalancer newLoadBalancer(Helper helper) {
childHelpers.add(helper); childHelpers.add(helper);
LoadBalancer childBalancer = mock(LoadBalancer.class); LoadBalancer childBalancer = mock(LoadBalancer.class);
when(childBalancer.acceptResolvedAddresses(any())).thenReturn(Status.OK);
childBalancers.add(childBalancer); childBalancers.add(childBalancer);
barLbCreated++; barLbCreated++;
return childBalancer; return childBalancer;
@ -180,7 +182,7 @@ public class WeightedTargetLoadBalancerTest {
} }
@Test @Test
public void handleResolvedAddresses() { public void acceptResolvedAddresses() {
ArgumentCaptor<ResolvedAddresses> resolvedAddressesCaptor = ArgumentCaptor<ResolvedAddresses> resolvedAddressesCaptor =
ArgumentCaptor.forClass(ResolvedAddresses.class); ArgumentCaptor.forClass(ResolvedAddresses.class);
Attributes.Key<Object> fakeKey = Attributes.Key.create("fake_key"); Attributes.Key<Object> fakeKey = Attributes.Key.create("fake_key");
@ -203,12 +205,13 @@ public class WeightedTargetLoadBalancerTest {
eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2")); eag2 = AddressFilter.setPathFilter(eag2, ImmutableList.of("target2"));
EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]); EquivalentAddressGroup eag3 = new EquivalentAddressGroup(socketAddresses[3]);
eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3")); eag3 = AddressFilter.setPathFilter(eag3, ImmutableList.of("target3"));
weightedTargetLb.handleResolvedAddresses( Status status = weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3)) .setAddresses(ImmutableList.of(eag0, eag1, eag2, eag3))
.setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build()) .setAttributes(Attributes.newBuilder().set(fakeKey, fakeValue).build())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
.build()); .build());
assertThat(status.isOk()).isTrue();
verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult())); verify(helper).updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
assertThat(childBalancers).hasSize(4); assertThat(childBalancers).hasSize(4);
assertThat(childHelpers).hasSize(4); assertThat(childHelpers).hasSize(4);
@ -216,7 +219,7 @@ public class WeightedTargetLoadBalancerTest {
assertThat(barLbCreated).isEqualTo(2); assertThat(barLbCreated).isEqualTo(2);
for (int i = 0; i < childBalancers.size(); i++) { for (int i = 0; i < childBalancers.size(); i++) {
verify(childBalancers.get(i)).handleResolvedAddresses(resolvedAddressesCaptor.capture()); verify(childBalancers.get(i)).acceptResolvedAddresses(resolvedAddressesCaptor.capture());
ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue(); ResolvedAddresses resolvedAddresses = resolvedAddressesCaptor.getValue();
assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]); assertThat(resolvedAddresses.getLoadBalancingPolicyConfig()).isEqualTo(configs[i]);
assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue); assertThat(resolvedAddresses.getAttributes().get(fakeKey)).isEqualTo(fakeValue);
@ -226,6 +229,11 @@ public class WeightedTargetLoadBalancerTest {
.containsExactly(socketAddresses[i]); .containsExactly(socketAddresses[i]);
} }
// Even when a child return an error from the update, the other children should still receive
// their updates.
Status acceptReturnStatus = Status.UNAVAILABLE.withDescription("Didn't like something");
when(childBalancers.get(2).acceptResolvedAddresses(any())).thenReturn(acceptReturnStatus);
// Update new weighted target config for a typical workflow. // Update new weighted target config for a typical workflow.
// target0 removed. target1, target2, target3 changed weight and config. target4 added. // target0 removed. target1, target2, target3 changed weight and config. target4 added.
int[] newWeights = new int[]{11, 22, 33, 44}; int[] newWeights = new int[]{11, 22, 33, 44};
@ -243,11 +251,12 @@ public class WeightedTargetLoadBalancerTest {
"target4", "target4",
new WeightedPolicySelection( new WeightedPolicySelection(
newWeights[3], newChildConfig(fooLbProvider, newConfigs[3]))); newWeights[3], newChildConfig(fooLbProvider, newConfigs[3])));
weightedTargetLb.handleResolvedAddresses( status = weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of()) .setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(newTargets))
.build()); .build());
assertThat(status.getCode()).isEqualTo(acceptReturnStatus.getCode());
verify(helper, atLeast(2)) verify(helper, atLeast(2))
.updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult())); .updateBalancingState(eq(CONNECTING), pickerReturns(PickResult.withNoResult()));
assertThat(childBalancers).hasSize(5); assertThat(childBalancers).hasSize(5);
@ -258,7 +267,7 @@ public class WeightedTargetLoadBalancerTest {
verify(childBalancers.get(0)).shutdown(); verify(childBalancers.get(0)).shutdown();
for (int i = 1; i < childBalancers.size(); i++) { for (int i = 1; i < childBalancers.size(); i++) {
verify(childBalancers.get(i), atLeastOnce()) verify(childBalancers.get(i), atLeastOnce())
.handleResolvedAddresses(resolvedAddressesCaptor.capture()); .acceptResolvedAddresses(resolvedAddressesCaptor.capture());
assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig()) assertThat(resolvedAddressesCaptor.getValue().getLoadBalancingPolicyConfig())
.isEqualTo(newConfigs[i - 1]); .isEqualTo(newConfigs[i - 1]);
} }
@ -286,7 +295,7 @@ public class WeightedTargetLoadBalancerTest {
"target2", weightedLbConfig2, "target2", weightedLbConfig2,
// {foo, 40, config3} // {foo, 40, config3}
"target3", weightedLbConfig3); "target3", weightedLbConfig3);
weightedTargetLb.handleResolvedAddresses( weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of()) .setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@ -313,7 +322,7 @@ public class WeightedTargetLoadBalancerTest {
"target2", weightedLbConfig2, "target2", weightedLbConfig2,
// {foo, 40, config3} // {foo, 40, config3}
"target3", weightedLbConfig3); "target3", weightedLbConfig3);
weightedTargetLb.handleResolvedAddresses( weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of()) .setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@ -395,7 +404,7 @@ public class WeightedTargetLoadBalancerTest {
Map<String, WeightedPolicySelection> targets = ImmutableMap.of( Map<String, WeightedPolicySelection> targets = ImmutableMap.of(
"target0", weightedLbConfig0, "target0", weightedLbConfig0,
"target1", weightedLbConfig1); "target1", weightedLbConfig1);
weightedTargetLb.handleResolvedAddresses( weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of()) .setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@ -421,7 +430,7 @@ public class WeightedTargetLoadBalancerTest {
weights[0], newChildConfig(fakeLbProvider, configs[0])), weights[0], newChildConfig(fakeLbProvider, configs[0])),
"target3", new WeightedPolicySelection( "target3", new WeightedPolicySelection(
weights[3], newChildConfig(fakeLbProvider, configs[3]))); weights[3], newChildConfig(fakeLbProvider, configs[3])));
weightedTargetLb.handleResolvedAddresses( weightedTargetLb.acceptResolvedAddresses(
ResolvedAddresses.newBuilder() ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.<EquivalentAddressGroup>of()) .setAddresses(ImmutableList.<EquivalentAddressGroup>of())
.setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets)) .setLoadBalancingPolicyConfig(new WeightedTargetConfig(targets))
@ -470,9 +479,10 @@ public class WeightedTargetLoadBalancerTest {
} }
@Override @Override
public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) { public Status acceptResolvedAddresses(ResolvedAddresses resolvedAddresses) {
helper.updateBalancingState( helper.updateBalancingState(
TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL))); TRANSIENT_FAILURE, new FixedResultPicker(PickResult.withError(Status.INTERNAL)));
return Status.OK;
} }
@Override @Override