From 36e0af66fe63cc72984654eaa5d45a19e84357a7 Mon Sep 17 00:00:00 2001
From: Terry Wilson <tmwilson@google.com>
Date: Wed, 30 Aug 2023 15:40:34 -0700
Subject: [PATCH] core: de-expermentalize pick first config parsing (#10531)

---
 .../PickFirstLoadBalancerProvider.java        | 26 ++++++-------------
 .../PickFirstLoadBalancerProviderTest.java    | 19 +++++---------
 2 files changed, 14 insertions(+), 31 deletions(-)

diff --git a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java
index e3e955ce32..8e24349929 100644
--- a/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java
+++ b/core/src/main/java/io/grpc/internal/PickFirstLoadBalancerProvider.java
@@ -16,8 +16,6 @@
 
 package io.grpc.internal;
 
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Strings;
 import io.grpc.LoadBalancer;
 import io.grpc.LoadBalancerProvider;
 import io.grpc.NameResolver;
@@ -33,11 +31,7 @@ import java.util.Map;
  * down the address list and sticks to the first that works.
  */
 public final class PickFirstLoadBalancerProvider extends LoadBalancerProvider {
-  private static final String NO_CONFIG = "no service config";
   private static final String SHUFFLE_ADDRESS_LIST_KEY = "shuffleAddressList";
-  private static final String CONFIG_FLAG_NAME = "GRPC_EXPERIMENTAL_PICKFIRST_LB_CONFIG";
-  @VisibleForTesting
-  static boolean enablePickFirstConfig = !Strings.isNullOrEmpty(System.getenv(CONFIG_FLAG_NAME));
 
   @Override
   public boolean isAvailable() {
@@ -62,18 +56,14 @@ public final class PickFirstLoadBalancerProvider extends LoadBalancerProvider {
   @Override
   public ConfigOrError parseLoadBalancingPolicyConfig(
       Map<String, ?> rawLoadBalancingPolicyConfig) {
-    if (enablePickFirstConfig) {
-      try {
-        return ConfigOrError.fromConfig(
-            new PickFirstLoadBalancerConfig(JsonUtil.getBoolean(rawLoadBalancingPolicyConfig,
-                SHUFFLE_ADDRESS_LIST_KEY)));
-      } catch (RuntimeException e) {
-        return ConfigOrError.fromError(
-            Status.UNAVAILABLE.withCause(e).withDescription(
-                "Failed parsing configuration for " + getPolicyName()));
-      }
-    } else {
-      return ConfigOrError.fromConfig(NO_CONFIG);
+    try {
+      return ConfigOrError.fromConfig(
+          new PickFirstLoadBalancerConfig(JsonUtil.getBoolean(rawLoadBalancingPolicyConfig,
+              SHUFFLE_ADDRESS_LIST_KEY)));
+    } catch (RuntimeException e) {
+      return ConfigOrError.fromError(
+          Status.UNAVAILABLE.withCause(e).withDescription(
+              "Failed parsing configuration for " + getPolicyName()));
     }
   }
 }
diff --git a/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerProviderTest.java b/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerProviderTest.java
index 1c224eb1c3..3aa9b1872c 100644
--- a/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerProviderTest.java
+++ b/core/src/test/java/io/grpc/internal/PickFirstLoadBalancerProviderTest.java
@@ -22,7 +22,6 @@ import io.grpc.NameResolver.ConfigOrError;
 import io.grpc.internal.PickFirstLoadBalancer.PickFirstLoadBalancerConfig;
 import java.util.HashMap;
 import java.util.Map;
-import org.junit.After;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -30,14 +29,8 @@ import org.junit.runners.JUnit4;
 @RunWith(JUnit4.class)
 public class PickFirstLoadBalancerProviderTest {
 
-  @After
-  public void resetConfigFlag() {
-    PickFirstLoadBalancerProvider.enablePickFirstConfig = false;
-  }
-
   @Test
-  public void parseWithConfigEnabled() {
-    PickFirstLoadBalancerProvider.enablePickFirstConfig = true;
+  public void parseWithConfig() {
     Map<String, Object> rawConfig = new HashMap<>();
     rawConfig.put("shuffleAddressList", true);
     ConfigOrError parsedConfig = new PickFirstLoadBalancerProvider().parseLoadBalancingPolicyConfig(
@@ -45,17 +38,17 @@ public class PickFirstLoadBalancerProviderTest {
     PickFirstLoadBalancerConfig config = (PickFirstLoadBalancerConfig) parsedConfig.getConfig();
 
     assertThat(config.shuffleAddressList).isTrue();
+    assertThat(config.randomSeed).isNull();
   }
 
   @Test
-  public void parseWithConfigDisabled() {
-    PickFirstLoadBalancerProvider.enablePickFirstConfig = false;
+  public void parseWithoutConfig() {
     Map<String, Object> rawConfig = new HashMap<>();
-    rawConfig.put("shuffleAddressList", true);
     ConfigOrError parsedConfig = new PickFirstLoadBalancerProvider().parseLoadBalancingPolicyConfig(
         rawConfig);
-    String config = (String) parsedConfig.getConfig();
+    PickFirstLoadBalancerConfig config = (PickFirstLoadBalancerConfig) parsedConfig.getConfig();
 
-    assertThat(config).isEqualTo("no service config");
+    assertThat(config.shuffleAddressList).isNull();
+    assertThat(config.randomSeed).isNull();
   }
 }