core: do not fail RPCs if balancer address exists but grpclb is unavailable (#5129)

When service owner turns on grpclb through service config, it
shouldn't break existing clients that don't have grpclb in their
classpath.

Resolves #4602

core/src/main/java/io/grpc/internal/AutoConfiguredLoadBalancerFactory.java

@@ -20,7 +20,6 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import io.grpc.Attributes;
-import io.grpc.ChannelLogger;
 import io.grpc.ChannelLogger.ChannelLogLevel;
 import io.grpc.ConnectivityState;
 import io.grpc.ConnectivityStateInfo;
@@ -33,17 +32,31 @@ import io.grpc.LoadBalancer.SubchannelPicker;
 import io.grpc.LoadBalancerProvider;
 import io.grpc.LoadBalancerRegistry;
 import io.grpc.Status;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.logging.Logger;
 import javax.annotation.Nullable;
 
 @VisibleForTesting
 public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factory {
   private static final String DEFAULT_POLICY = "pick_first";
+  private static final Logger logger =
+      Logger.getLogger(AutoConfiguredLoadBalancerFactory.class.getName());
 
-  private static final LoadBalancerRegistry registry = LoadBalancerRegistry.getDefaultRegistry();
+  private final LoadBalancerRegistry registry;
+
+  public AutoConfiguredLoadBalancerFactory() {
+    this(LoadBalancerRegistry.getDefaultRegistry());
+  }
+
+  @VisibleForTesting
+  AutoConfiguredLoadBalancerFactory(LoadBalancerRegistry registry) {
+    this.registry = checkNotNull(registry, "registry");
+  }
 
   @Override
   public LoadBalancer newLoadBalancer(Helper helper) {
@@ -65,12 +78,12 @@ public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factor
     public void shutdown() {}
   }
 
-
   @VisibleForTesting
-  public static final class AutoConfiguredLoadBalancer extends LoadBalancer {
+  public final class AutoConfiguredLoadBalancer extends LoadBalancer {
     private final Helper helper;
     private LoadBalancer delegate;
     private LoadBalancerProvider delegateProvider;
+    private boolean roundRobinDueToGrpclbDepMissing;
 
     AutoConfiguredLoadBalancer(Helper helper) {
       this.helper = helper;
@@ -94,7 +107,7 @@ public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factor
       Map<String, Object> configMap = attributes.get(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG);
       PolicySelection selection;
       try {
-        selection = decideLoadBalancerProvider(servers, configMap, helper.getChannelLogger());
+        selection = decideLoadBalancerProvider(servers, configMap);
       } catch (PolicyException e) {
         Status s = Status.INTERNAL.withDescription(e.getMessage());
         helper.updateBalancingState(ConnectivityState.TRANSIENT_FAILURE, new FailingPicker(s));
@@ -122,7 +135,7 @@ public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factor
         attributes =
             attributes.toBuilder().set(ATTR_LOAD_BALANCING_CONFIG, selection.config).build();
       }
-      getDelegate().handleResolvedAddressGroups(servers, attributes);
+      getDelegate().handleResolvedAddressGroups(selection.serverList, attributes);
     }
 
     @Override
@@ -155,73 +168,93 @@ public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factor
     LoadBalancerProvider getDelegateProvider() {
       return delegateProvider;
     }
-  }
 
-  /**
+    /**
-   * Picks a load balancer based on given criteria.  In order of preference:
+     * Picks a load balancer based on given criteria.  In order of preference:
-   *
+     *
-   * <ol>
+     * <ol>
-   *   <li>User provided lb on the channel.  This is a degenerate case and not handled here.</li>
+     *   <li>User provided lb on the channel.  This is a degenerate case and not handled here.</li>
-   *   <li>"grpclb" if any gRPC LB balancer addresses are present</li>
+     *   <li>"grpclb" if any gRPC LB balancer addresses are present</li>
-   *   <li>The policy picked by the service config</li>
+     *   <li>The policy picked by the service config</li>
-   *   <li>"pick_first" if the service config choice does not specify</li>
+     *   <li>"pick_first" if the service config choice does not specify</li>
-   * </ol>
+     * </ol>
-   *
+     *
-   * @param servers The list of servers reported
+     * @param servers The list of servers reported
-   * @param config the service config object
+     * @param config the service config object
-   * @return the new load balancer factory, never null
+     * @return the new load balancer factory, never null
-   */
+     */
-  @VisibleForTesting
+    @VisibleForTesting
-  static PolicySelection decideLoadBalancerProvider(
+    PolicySelection decideLoadBalancerProvider(
-      List<EquivalentAddressGroup> servers, @Nullable Map<String, Object> config,
+        List<EquivalentAddressGroup> servers, @Nullable Map<String, Object> config)
-      ChannelLogger channelLogger) throws PolicyException {
+        throws PolicyException {
-    // Check for balancer addresses
+      // Check for balancer addresses
-    boolean haveBalancerAddress = false;
+      boolean haveBalancerAddress = false;
-    for (EquivalentAddressGroup s : servers) {
+      List<EquivalentAddressGroup> backendAddrs = new ArrayList<EquivalentAddressGroup>();
-      if (s.getAttributes().get(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY) != null) {
+      for (EquivalentAddressGroup s : servers) {
-        haveBalancerAddress = true;
+        if (s.getAttributes().get(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY) != null) {
-        break;
+          haveBalancerAddress = true;
-      }
+        } else {
-    }
+          backendAddrs.add(s);
-
-    if (haveBalancerAddress) {
-      LoadBalancerProvider provider =
-          getProviderOrThrow("grpclb", "NameResolver has returned balancer addresses");
-      return new PolicySelection(provider, null);
-    }
-
-    if (config != null) {
-      List<Map<String, Object>> lbConfigs =
-          ServiceConfigUtil.getLoadBalancingConfigsFromServiceConfig(config);
-      LinkedHashSet<String> policiesTried = new LinkedHashSet<>();
-      for (Map<String, Object> lbConfig : lbConfigs) {
-        if (lbConfig.size() != 1) {
-          throw new PolicyException(
-              "There are " + lbConfig.size()
-              + " load-balancing configs in a list item. Exactly one is expected. Config="
-              + lbConfig);
         }
-        Entry<String, Object> entry = lbConfig.entrySet().iterator().next();
+      }
-        String policy = entry.getKey();
+
-        LoadBalancerProvider provider = registry.getProvider(policy);
+      if (haveBalancerAddress) {
-        if (provider != null) {
+        LoadBalancerProvider grpclbProvider = registry.getProvider("grpclb");
-          if (!policiesTried.isEmpty()) {
+        if (grpclbProvider == null) {
-            channelLogger.log(
+          if (backendAddrs.isEmpty()) {
-                ChannelLogLevel.DEBUG,
+            throw new PolicyException(
-                "{0} specified by Service Config are not available", policiesTried);
+                "Received ONLY balancer addresses but grpclb runtime is missing");
           }
-          return new PolicySelection(provider, (Map) entry.getValue());
+          if (!roundRobinDueToGrpclbDepMissing) {
+            roundRobinDueToGrpclbDepMissing = true;
+            String errorMsg = "Found balancer addresses but grpclb runtime is missing."
+                + " Will use round_robin. Please include grpc-grpclb in your runtime depedencies.";
+            helper.getChannelLogger().log(ChannelLogLevel.ERROR, errorMsg);
+            logger.warning(errorMsg);
+          }
+          return new PolicySelection(
+              getProviderOrThrow(
+                  "round_robin", "received balancer addresses but grpclb runtime is missing"),
+              backendAddrs, null);
+        } else {
+          return new PolicySelection(grpclbProvider, servers, null);
         }
-        policiesTried.add(policy);
       }
-      throw new PolicyException(
+      roundRobinDueToGrpclbDepMissing = false;
-          "None of " + policiesTried + " specified by Service Config are available.");
+
+      if (config != null) {
+        List<Map<String, Object>> lbConfigs =
+            ServiceConfigUtil.getLoadBalancingConfigsFromServiceConfig(config);
+        LinkedHashSet<String> policiesTried = new LinkedHashSet<>();
+        for (Map<String, Object> lbConfig : lbConfigs) {
+          if (lbConfig.size() != 1) {
+            throw new PolicyException(
+                "There are " + lbConfig.size()
+                + " load-balancing configs in a list item. Exactly one is expected. Config="
+                + lbConfig);
+          }
+          Entry<String, Object> entry = lbConfig.entrySet().iterator().next();
+          String policy = entry.getKey();
+          LoadBalancerProvider provider = registry.getProvider(policy);
+          if (provider != null) {
+            if (!policiesTried.isEmpty()) {
+              helper.getChannelLogger().log(
+                  ChannelLogLevel.DEBUG,
+                  "{0} specified by Service Config are not available", policiesTried);
+            }
+            return new PolicySelection(provider, servers, (Map) entry.getValue());
+          }
+          policiesTried.add(policy);
+        }
+        throw new PolicyException(
+            "None of " + policiesTried + " specified by Service Config are available.");
+      }
+      return new PolicySelection(
+          getProviderOrThrow(DEFAULT_POLICY, "using default policy"), servers, null);
     }
-    return new PolicySelection(
-        getProviderOrThrow(DEFAULT_POLICY, "using default policy"), null);
   }
 
-  private static LoadBalancerProvider getProviderOrThrow(String policy, String choiceReason)
+  private LoadBalancerProvider getProviderOrThrow(String policy, String choiceReason)
       throws PolicyException {
     LoadBalancerProvider provider = registry.getProvider(policy);
     if (provider == null) {
@@ -243,11 +276,15 @@ public final class AutoConfiguredLoadBalancerFactory extends LoadBalancer.Factor
   @VisibleForTesting
   static final class PolicySelection {
     final LoadBalancerProvider provider;
+    final List<EquivalentAddressGroup> serverList;
     @Nullable final Map<String, Object> config;
 
     @SuppressWarnings("unchecked")
-    PolicySelection(LoadBalancerProvider provider, @Nullable Map<?, ?> config) {
+    PolicySelection(
+        LoadBalancerProvider provider, List<EquivalentAddressGroup> serverList,
+        @Nullable Map<?, ?> config) {
       this.provider = checkNotNull(provider, "provider");
+      this.serverList = Collections.unmodifiableList(checkNotNull(serverList, "serverList"));
       this.config = (Map<String, Object>) config;
     }
   }

core/src/test/java/io/grpc/internal/AutoConfiguredLoadBalancerFactoryTest.java

@@ -24,6 +24,7 @@ import static org.mockito.AdditionalAnswers.delegatesTo;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
+import static org.mockito.Matchers.startsWith;
 import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -145,8 +146,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
   @Test
   public void handleResolvedAddressGroups_keepOldBalancer() {
     final List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(new SocketAddress(){}, Attributes.EMPTY));
     Helper helper = new TestHelper() {
       @Override
       public Subchannel createSubchannel(List<EquivalentAddressGroup> addrs, Attributes attrs) {
@@ -177,10 +177,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
             .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig)
             .build();
     final List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
-                new SocketAddress(){},
-                Attributes.EMPTY));
     Helper helper = new TestHelper() {
       @Override
       public Subchannel createSubchannel(List<EquivalentAddressGroup> addrs, Attributes attrs) {
@@ -231,10 +228,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
             .set(GrpcAttributes.NAME_RESOLVER_SERVICE_CONFIG, serviceConfig)
             .build();
     final List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
-                new SocketAddress(){},
-                Attributes.EMPTY));
     Helper helper = new TestHelper() {
         @Override
         public void updateBalancingState(ConnectivityState newState, SubchannelPicker newPicker) {
@@ -249,7 +243,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
     verify(testLbBalancerProvider).newLoadBalancer(same(helper));
     assertThat(lb.getDelegate()).isSameAs(testLbBalancer);
     ArgumentCaptor<Attributes> attrsCaptor = ArgumentCaptor.forClass(null);
-    verify(testLbBalancer).handleResolvedAddressGroups(same(servers), attrsCaptor.capture());
+    verify(testLbBalancer).handleResolvedAddressGroups(eq(servers), attrsCaptor.capture());
     assertThat(attrsCaptor.getValue().get(ATTR_LOAD_BALANCING_CONFIG))
         .isEqualTo(Collections.singletonMap("setting1", "high"));
     verifyNoMoreInteractions(testLbBalancer);
@@ -263,7 +257,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
     lb.handleResolvedAddressGroups(servers, serviceConfigAttrs);
 
     verify(testLbBalancer, times(2))
-        .handleResolvedAddressGroups(same(servers), attrsCaptor.capture());
+        .handleResolvedAddressGroups(eq(servers), attrsCaptor.capture());
     // But the balancer config is changed.
     assertThat(attrsCaptor.getValue().get(ATTR_LOAD_BALANCING_CONFIG))
         .isEqualTo(Collections.singletonMap("setting1", "low"));
@@ -272,40 +266,74 @@ public class AutoConfiguredLoadBalancerFactoryTest {
     verify(testLbBalancerProvider).newLoadBalancer(any(Helper.class));
   }
 
+  @Test
+  public void handleResolvedAddressGroups_propagateOnlyBackendAddrsToDelegate() throws Exception {
+    // This case only happens when grpclb is missing.  We will use a local registry
+    LoadBalancerRegistry registry = new LoadBalancerRegistry();
+    registry.register(new PickFirstLoadBalancerProvider());
+    registry.register(new FakeRoundRobinLoadBalancerProvider());
+
+    final List<EquivalentAddressGroup> servers =
+        Arrays.asList(
+            new EquivalentAddressGroup(new SocketAddress(){}),
+            new EquivalentAddressGroup(
+                new SocketAddress(){},
+                Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()));
+    Helper helper = new TestHelper() {
+        @Override
+        public void updateBalancingState(ConnectivityState newState, SubchannelPicker newPicker) {
+          // noop
+        }
+      };
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) new AutoConfiguredLoadBalancerFactory(registry)
+            .newLoadBalancer(helper);
+
+    lb.handleResolvedAddressGroups(servers, Attributes.EMPTY);
+
+    assertThat(lb.getDelegate()).isSameAs(testLbBalancer);
+    verify(testLbBalancer).handleResolvedAddressGroups(
+        eq(Collections.singletonList(servers.get(0))), any(Attributes.class));
+  }
+
   @Test
   public void decideLoadBalancerProvider_noBalancerAddresses_noServiceConfig_pickFirst()
       throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig = null;
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(new SocketAddress(){}, Attributes.EMPTY));
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-    PolicySelection selection =
-        AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
-            servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider).isInstanceOf(PickFirstLoadBalancerProvider.class);
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isNull();
     verifyZeroInteractions(channelLogger);
   }
 
   @Test
   public void decideLoadBalancerProvider_oneBalancer_noServiceConfig_grpclb() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig = null;
     List<EquivalentAddressGroup> servers =
         Collections.singletonList(
             new EquivalentAddressGroup(
                 new SocketAddress(){},
                 Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-        servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider).isInstanceOf(GrpclbLoadBalancerProvider.class);
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isNull();
     verifyZeroInteractions(channelLogger);
   }
 
   @Test
   public void decideLoadBalancerProvider_grpclbOverridesServiceConfigLbPolicy() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig = new HashMap<String, Object>();
     serviceConfig.put("loadBalancingPolicy", "round_robin");
     List<EquivalentAddressGroup> servers =
@@ -313,10 +341,10 @@ public class AutoConfiguredLoadBalancerFactoryTest {
             new EquivalentAddressGroup(
                 new SocketAddress(){},
                 Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-        servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider).isInstanceOf(GrpclbLoadBalancerProvider.class);
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isNull();
     verifyZeroInteractions(channelLogger);
   }
@@ -324,6 +352,8 @@ public class AutoConfiguredLoadBalancerFactoryTest {
   @SuppressWarnings("unchecked")
   @Test
   public void decideLoadBalancerProvider_grpclbOverridesServiceConfigLbConfig() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig =
         parseConfig("{\"loadBalancingConfig\": [ {\"round_robin\": {} } ] }");
     List<EquivalentAddressGroup> servers =
@@ -331,25 +361,83 @@ public class AutoConfiguredLoadBalancerFactoryTest {
             new EquivalentAddressGroup(
                 new SocketAddress(){},
                 Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-        servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider).isInstanceOf(GrpclbLoadBalancerProvider.class);
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isNull();
     verifyZeroInteractions(channelLogger);
   }
 
   @Test
-  public void decideLoadBalancerProvider_serviceConfigLbPolicyOverridesDefault() throws Exception {
+  public void decideLoadBalancerProvider_grpclbProviderNotFound_fallbackToRoundRobin()
-    Map<String, Object> serviceConfig = new HashMap<String, Object>();
+      throws Exception {
-    serviceConfig.put("loadBalancingPolicy", "round_robin");
+    LoadBalancerRegistry registry = new LoadBalancerRegistry();
+    registry.register(new PickFirstLoadBalancerProvider());
+    registry.register(new FakeRoundRobinLoadBalancerProvider());
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) new AutoConfiguredLoadBalancerFactory(registry)
+            .newLoadBalancer(new TestHelper());
+    Map<String, Object> serviceConfig =
+        parseConfig("{\"loadBalancingConfig\": [ {\"round_robin\": {} } ] }");
+    List<EquivalentAddressGroup> servers =
+        Arrays.asList(
+            new EquivalentAddressGroup(
+                new SocketAddress(){},
+                Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()),
+            new EquivalentAddressGroup(new SocketAddress(){}));
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
+
+    assertThat(selection.provider).isInstanceOf(FakeRoundRobinLoadBalancerProvider.class);
+    assertThat(selection.config).isNull();
+    verify(channelLogger).log(
+        eq(ChannelLogLevel.ERROR),
+        startsWith("Found balancer addresses but grpclb runtime is missing"));
+
+    // Called for the second time, the warning is only logged once
+    selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
+
+    assertThat(selection.provider).isInstanceOf(FakeRoundRobinLoadBalancerProvider.class);
+    // Balancer addresses are filtered out in the server list passed to round_robin
+    assertThat(selection.serverList).isEqualTo(Collections.singletonList(servers.get(1)));
+    assertThat(selection.config).isNull();
+    verifyNoMoreInteractions(channelLogger);
+  }
+
+  @Test
+  public void decideLoadBalancerProvider_grpclbProviderNotFound_noBackendAddress()
+      throws Exception {
+    LoadBalancerRegistry registry = new LoadBalancerRegistry();
+    registry.register(new PickFirstLoadBalancerProvider());
+    registry.register(new FakeRoundRobinLoadBalancerProvider());
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) new AutoConfiguredLoadBalancerFactory(registry)
+            .newLoadBalancer(new TestHelper());
+    Map<String, Object> serviceConfig =
+        parseConfig("{\"loadBalancingConfig\": [ {\"round_robin\": {} } ] }");
     List<EquivalentAddressGroup> servers =
         Collections.singletonList(
             new EquivalentAddressGroup(
                 new SocketAddress(){},
-                Attributes.EMPTY));
+                Attributes.newBuilder().set(GrpcAttributes.ATTR_LB_ADDR_AUTHORITY, "ok").build()));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+    try {
-        servers, serviceConfig, channelLogger);
+      lb.decideLoadBalancerProvider(servers, serviceConfig);
+      fail("Should throw");
+    } catch (PolicyException e) {
+      assertThat(e.getMessage())
+          .isEqualTo("Received ONLY balancer addresses but grpclb runtime is missing");
+    }
+  }
+
+  @Test
+  public void decideLoadBalancerProvider_serviceConfigLbPolicyOverridesDefault() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
+    Map<String, Object> serviceConfig = new HashMap<String, Object>();
+    serviceConfig.put("loadBalancingPolicy", "round_robin");
+    List<EquivalentAddressGroup> servers =
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
 
     assertThat(selection.provider.getClass().getName()).isEqualTo(
         "io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider");
@@ -359,34 +447,31 @@ public class AutoConfiguredLoadBalancerFactoryTest {
 
   @Test
   public void decideLoadBalancerProvider_serviceConfigLbConfigOverridesDefault() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig =
         parseConfig("{\"loadBalancingConfig\": [ {\"round_robin\": {\"setting1\": \"high\"} } ] }");
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-                new SocketAddress(){},
-                Attributes.EMPTY));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
-        servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider.getClass().getName()).isEqualTo(
         "io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider");
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isEqualTo(Collections.singletonMap("setting1", "high"));
     verifyZeroInteractions(channelLogger);
   }
 
   @Test
   public void decideLoadBalancerProvider_serviceConfigLbPolicyFailsOnUnknown() {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig = new HashMap<String, Object>();
     serviceConfig.put("loadBalancingPolicy", "MAGIC_BALANCER");
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
-                new SocketAddress(){},
-                Attributes.EMPTY));
     try {
-      AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+      lb.decideLoadBalancerProvider(servers, serviceConfig);
-          servers, serviceConfig, channelLogger);
       fail();
     } catch (PolicyException e) {
       assertThat(e.getMessage()).isEqualTo(
@@ -396,16 +481,14 @@ public class AutoConfiguredLoadBalancerFactoryTest {
 
   @Test
   public void decideLoadBalancerProvider_serviceConfigLbConfigFailsOnUnknown() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig =
         parseConfig("{\"loadBalancingConfig\": [ {\"magic_balancer\": {} } ] }");
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
-                new SocketAddress(){},
-                Attributes.EMPTY));
     try {
-      AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
+      lb.decideLoadBalancerProvider(servers, serviceConfig);
-          servers, serviceConfig, channelLogger);
       fail();
     } catch (PolicyException e) {
       assertThat(e.getMessage()).isEqualTo(
@@ -415,19 +498,18 @@ public class AutoConfiguredLoadBalancerFactoryTest {
 
   @Test
   public void decideLoadBalancerProvider_serviceConfigLbConfigSkipUnknown() throws Exception {
+    AutoConfiguredLoadBalancer lb =
+        (AutoConfiguredLoadBalancer) lbf.newLoadBalancer(new TestHelper());
     Map<String, Object> serviceConfig =
         parseConfig(
             "{\"loadBalancingConfig\": [ {\"magic_balancer\": {} }, {\"round_robin\": {} } ] }");
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(
+    PolicySelection selection = lb.decideLoadBalancerProvider(servers, serviceConfig);
-                new SocketAddress(){},
-                Attributes.EMPTY));
-    PolicySelection selection = AutoConfiguredLoadBalancerFactory.decideLoadBalancerProvider(
-        servers, serviceConfig, channelLogger);
 
     assertThat(selection.provider.getClass().getName()).isEqualTo(
         "io.grpc.util.SecretRoundRobinLoadBalancerProvider$Provider");
+    assertThat(selection.serverList).isEqualTo(servers);
     assertThat(selection.config).isEqualTo(Collections.emptyMap());
     verify(channelLogger).log(
         eq(ChannelLogLevel.DEBUG),
@@ -439,8 +521,7 @@ public class AutoConfiguredLoadBalancerFactoryTest {
   public void channelTracing_lbPolicyChanged() {
     final FakeClock clock = new FakeClock();
     List<EquivalentAddressGroup> servers =
-        Collections.singletonList(
+        Collections.singletonList(new EquivalentAddressGroup(new SocketAddress(){}));
-            new EquivalentAddressGroup(new SocketAddress(){}, Attributes.EMPTY));
     Helper helper = new TestHelper() {
       @Override
       public Subchannel createSubchannel(List<EquivalentAddressGroup> addrs, Attributes attrs) {
@@ -630,4 +711,28 @@ public class AutoConfiguredLoadBalancerFactoryTest {
       return testLbBalancer;
     }
   }
+
+  private final class FakeRoundRobinLoadBalancerProvider extends LoadBalancerProvider {
+    static final String POLICY_NAME = "round_robin";
+
+    @Override
+    public boolean isAvailable() {
+      return true;
+    }
+
+    @Override
+    public int getPriority() {
+      return 5;
+    }
+
+    @Override
+    public String getPolicyName() {
+      return POLICY_NAME;
+    }
+
+    @Override
+    public LoadBalancer newLoadBalancer(Helper helper) {
+      return testLbBalancer;
+    }
+  }
 }