Skip to content

xds: clean up xDS fallback balancer filtering out grpclb balancer addresses #6752

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
merged 1 commit into from
Feb 26, 2020
Merged

xds: clean up xDS fallback balancer filtering out grpclb balancer addresses #6752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 0 additions & 17 deletions xds/src/main/java/io/grpc/xds/FallbackLb.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,14 @@

import static com.google.common.base.Preconditions.checkNotNull;
import static io.grpc.ConnectivityState.TRANSIENT_FAILURE;
import static io.grpc.xds.XdsLoadBalancerProvider.XDS_POLICY_NAME;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import io.grpc.Attributes;
import io.grpc.EquivalentAddressGroup;
import io.grpc.LoadBalancer;
import io.grpc.LoadBalancerRegistry;
import io.grpc.NameResolver.ConfigOrError;
import io.grpc.Status;
import io.grpc.internal.GrpcAttributes;
import io.grpc.internal.ServiceConfigUtil.LbConfig;
import io.grpc.util.ForwardingLoadBalancer;
import io.grpc.util.GracefulSwitchLoadBalancer;
Expand Down Expand Up @@ -112,20 +109,6 @@ public void handleResolvedAddresses(ResolvedAddresses resolvedAddresses) {
fallbackPolicyLb.switchTo(lbRegistry.getProvider(newFallbackPolicyName));

List<EquivalentAddressGroup> servers = resolvedAddresses.getAddresses();
// Some addresses in the list may be grpclb-v1 balancer addresses, so if the fallback policy
// does not support grpclb-v1 balancer addresses, then we need to exclude them from the list.
// TODO(chengyuanzhang): delete the following logic after changing internal resolver
// to not include grpclb server addresses.
if (!newFallbackPolicyName.equals("grpclb") && !newFallbackPolicyName.equals(XDS_POLICY_NAME)) {
ImmutableList.Builder<EquivalentAddressGroup> backends = ImmutableList.builder();
for (EquivalentAddressGroup eag : resolvedAddresses.getAddresses()) {
if (eag.getAttributes().get(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY) == null) {
backends.add(eag);
}
}
servers = backends.build();
}

// TODO(zhangkun83): FIXME(#5496): this is a temporary hack.
if (servers.isEmpty()
&& !fallbackPolicyLb.canHandleEmptyAddressListFromNameResolution()) {
Expand Down
108 changes: 4 additions & 104 deletions xds/src/test/java/io/grpc/xds/FallbackLbTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,16 @@
import io.grpc.LoadBalancer.SubchannelPicker;
import io.grpc.LoadBalancerProvider;
import io.grpc.LoadBalancerRegistry;
import io.grpc.Status;
import io.grpc.Status.Code;
import io.grpc.internal.GrpcAttributes;
import io.grpc.internal.JsonParser;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
import org.mockito.ArgumentCaptor;

/**
* Tests for {@link FallbackLb}.
Expand Down Expand Up @@ -245,20 +240,13 @@ public void handlePolicyChanges() throws Exception {
verify(balancer2).shutdown();
}


@Test
public void handleBackendsEagsOnly() throws Exception {
EquivalentAddressGroup eag0 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8080)));
Attributes attributes = Attributes
.newBuilder()
.set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "this is a balancer address")
.build();
public void propagateAddressesToFallbackPolicy() throws Exception {
EquivalentAddressGroup eag1 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8081)), attributes);
ImmutableList.<SocketAddress>of(new InetSocketAddress(8080)));
EquivalentAddressGroup eag2 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8082)));
List<EquivalentAddressGroup> eags = ImmutableList.of(eag0, eag1, eag2);
List<EquivalentAddressGroup> eags = ImmutableList.of(eag1, eag2);

String lbConfigRaw = "{"
+ "\"fallbackPolicy\" : [{\"fallback_1\" : { \"fallback_1_option\" : \"yes\"}}]"
Expand All @@ -273,99 +261,11 @@ public void handleBackendsEagsOnly() throws Exception {
LoadBalancer balancer1 = balancers1.get(0);
verify(balancer1).handleResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(ImmutableList.of(eag0, eag2))
.setAddresses(ImmutableList.of(eag1, eag2))
.setAttributes(
Attributes.newBuilder()
.set(ATTR_LOAD_BALANCING_CONFIG, ImmutableMap.of("fallback_1_option", "yes"))
.build())
.build());
}

@Test
public void resolvingWithOnlyGrpclbAddresses_NoBackendAddress() throws Exception {
Attributes attributes = Attributes
.newBuilder()
.set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "this is a balancer address")
.build();
EquivalentAddressGroup eag1 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8081)), attributes);
EquivalentAddressGroup eag2 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8082)), attributes);
List<EquivalentAddressGroup> eags = ImmutableList.of(eag1, eag2);
String lbConfigRaw = "{"
+ "\"fallbackPolicy\" : [{\"fallback_1\" : { \"fallback_1_option\" : \"yes\"}}]"
+ "}";
@SuppressWarnings("unchecked")
Map<String, ?> lbConfig = (Map<String, ?>) JsonParser.parse(lbConfigRaw);
fallbackLb.handleResolvedAddresses(ResolvedAddresses.newBuilder()
.setAddresses(eags)
.setAttributes(Attributes.newBuilder().set(ATTR_LOAD_BALANCING_CONFIG, lbConfig).build())
.build());

LoadBalancer balancer1 = balancers1.get(0);
ArgumentCaptor<Status> statusCaptor = ArgumentCaptor.forClass(Status.class);
verify(balancer1).handleNameResolutionError(statusCaptor.capture());
assertThat(statusCaptor.getValue().getCode()).isEqualTo(Code.UNAVAILABLE);
}

@Test
public void handleGrpclbAddresses() throws Exception {
final AtomicReference<LoadBalancer> balancer = new AtomicReference<>();
LoadBalancerProvider grpclbProvider = new LoadBalancerProvider() {
@Override
public boolean isAvailable() {
return true;
}

@Override
public int getPriority() {
return 5;
}

@Override
public String getPolicyName() {
return "grpclb";
}

@Override
public LoadBalancer newLoadBalancer(Helper helper) {
balancer.set(mock(LoadBalancer.class));
return balancer.get();
}
};
LoadBalancerRegistry lbRegistry = new LoadBalancerRegistry();
lbRegistry.register(grpclbProvider);
fallbackLb = new FallbackLb(helper, lbRegistry);

EquivalentAddressGroup eag0 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8080)));
Attributes attributes = Attributes
.newBuilder()
.set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "this is a balancer address")
.build();
EquivalentAddressGroup eag1 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8081)), attributes);
EquivalentAddressGroup eag2 = new EquivalentAddressGroup(
ImmutableList.<SocketAddress>of(new InetSocketAddress(8082)));
List<EquivalentAddressGroup> eags = ImmutableList.of(eag0, eag1, eag2);

String lbConfigRaw = "{"
+ "\"fallbackPolicy\" : [{\"grpclb\" : { \"grpclb_option\" : \"yes\"}}]"
+ "}";
@SuppressWarnings("unchecked")
Map<String, ?> lbConfig = (Map<String, ?>) JsonParser.parse(lbConfigRaw);
fallbackLb.handleResolvedAddresses(ResolvedAddresses.newBuilder()
.setAddresses(eags)
.setAttributes(Attributes.newBuilder().set(ATTR_LOAD_BALANCING_CONFIG, lbConfig).build())
.build());

verify(balancer.get()).handleResolvedAddresses(
ResolvedAddresses.newBuilder()
.setAddresses(eags)
.setAttributes(
Attributes.newBuilder()
.set(ATTR_LOAD_BALANCING_CONFIG, ImmutableMap.of("grpclb_option", "yes"))
.build())
.build());
}
}