rls: LB config cleanup (#3443)

1. Store the RLS server name (lookupService field) as a string instead
   of as a resolver.Target as this is passed to grpc.Dial when creating
   the control channel.
2. Store the default target specified in the ServiceConfig in a field in
   the lbConfig struct.
3. Move some constants defined in builder.go to config.go because they
   are only used in the latter.

balancer/rls/internal/builder.go

@@ -21,23 +21,7 @@
 // Package rls implements the RLS LB policy.
 package rls
 
-import (
-	"time"
-)
-
-const (
-	rlsBalancerName = "rls"
-	// This is max duration that we are willing to cache RLS responses. If the
-	// service config doesn't specify a value for max_age or if it specified a
-	// value greater that this, we will use this value instead.
-	maxMaxAge = 5 * time.Minute
-	// If lookup_service_timeout is not specified in the service config, we use
-	// a default of 10 seconds.
-	defaultLookupServiceTimeout = 10 * time.Second
-	// This is set to the targetNameField in the child policy config during
-	// service config validation.
-	dummyChildPolicyTarget = "target_name_to_be_filled_in_later"
-)
+const rlsBalancerName = "rls"
 
 // rlsBB helps build RLS load balancers and parse the service config to be
 // passed to the RLS load balancer.

balancer/rls/internal/config.go

@@ -38,6 +38,19 @@ import (
 	"google.golang.org/grpc/serviceconfig"
 )
 
+const (
+	// This is max duration that we are willing to cache RLS responses. If the
+	// service config doesn't specify a value for max_age or if it specified a
+	// value greater that this, we will use this value instead.
+	maxMaxAge = 5 * time.Minute
+	// If lookup_service_timeout is not specified in the service config, we use
+	// a default of 10 seconds.
+	defaultLookupServiceTimeout = 10 * time.Second
+	// This is set to the targetNameField in the child policy config during
+	// service config validation.
+	dummyChildPolicyTarget = "target_name_to_be_filled_in_later"
+)
+
 // lbConfig contains the parsed and validated contents of the
 // loadBalancingConfig section of the service config. The RLS LB policy will
 // use this to directly access config data instead of ploughing through proto
@@ -46,12 +59,13 @@ type lbConfig struct {
 	serviceconfig.LoadBalancingConfig
 
 	kbMap                keys.BuilderMap
-	lookupService        resolver.Target
+	lookupService        string
 	lookupServiceTimeout time.Duration
 	maxAge               time.Duration
 	staleAge             time.Duration
 	cacheSizeBytes       int64
 	rpStrategy           rlspb.RouteLookupConfig_RequestProcessingStrategy
+	defaultTarget        string
 	cpName               string
 	cpTargetField        string
 	cpConfig             map[string]json.RawMessage
@@ -233,12 +247,13 @@ func (*rlsBB) ParseConfig(c json.RawMessage) (serviceconfig.LoadBalancingConfig,
 
 	return &lbConfig{
 		kbMap:                kbMap,
-		lookupService:        parsedTarget,
+		lookupService:        lookupService,
 		lookupServiceTimeout: lookupServiceTimeout,
 		maxAge:               maxAge,
 		staleAge:             staleAge,
 		cacheSizeBytes:       cacheSizeBytes,
 		rpStrategy:           rpStrategy,
+		defaultTarget:        defaultTarget,
 		// TODO(easwars): Once we refactor validateChildPolicyConfig and make
 		// it a method on the lbConfig object, we could directly store the
 		// balancer.Builder and/or balancer.ConfigParser here instead of the

balancer/rls/internal/config_test.go

@@ -30,10 +30,8 @@ import (
 	"github.com/google/go-cmp/cmp"
 
 	"google.golang.org/grpc/balancer"
+	_ "google.golang.org/grpc/balancer/grpclb" // grpclb for config parsing.
 	rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1"
-	"google.golang.org/grpc/resolver"
-
-	_ "google.golang.org/grpc/balancer/grpclb"               // grpclb for config parsing.
 	_ "google.golang.org/grpc/internal/resolver/passthrough" // passthrough resolver.
 )
 
@@ -61,6 +59,7 @@ func (lbCfg *lbConfig) Equal(other *lbConfig) bool {
 		lbCfg.staleAge == other.staleAge &&
 		lbCfg.cacheSizeBytes == other.cacheSizeBytes &&
 		lbCfg.rpStrategy == other.rpStrategy &&
+		lbCfg.defaultTarget == other.defaultTarget &&
 		lbCfg.cpName == other.cpName &&
 		lbCfg.cpTargetField == other.cpTargetField &&
 		cmp.Equal(lbCfg.cpConfig, other.cpConfig)
@@ -103,12 +102,13 @@ func TestParseConfig(t *testing.T) {
 				"childPolicyConfigTargetFieldName": "service_name"
 			}`),
 			wantCfg: &lbConfig{
-				lookupService:        resolver.Target{Scheme: "passthrough", Endpoint: "target"},
+				lookupService:        "passthrough:///target",
 				lookupServiceTimeout: 10 * time.Second, // This is the default value.
 				maxAge:               5 * time.Minute,  // This is max maxAge.
 				staleAge:             time.Duration(0), // StaleAge is ignore because it was higher than maxAge.
 				cacheSizeBytes:       1000,
 				rpStrategy:           rlspb.RouteLookupConfig_ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS,
+				defaultTarget:        "passthrough:///default",
 				cpName:               "grpclb",
 				cpTargetField:        "service_name",
 				cpConfig:             map[string]json.RawMessage{"childPolicy": json.RawMessage(`[{"pickfirst": {}}]`)},
@@ -134,12 +134,13 @@ func TestParseConfig(t *testing.T) {
 				"childPolicyConfigTargetFieldName": "service_name"
 			}`),
 			wantCfg: &lbConfig{
-				lookupService:        resolver.Target{Scheme: "passthrough", Endpoint: "target"},
+				lookupService:        "passthrough:///target",
 				lookupServiceTimeout: 100 * time.Second,
 				maxAge:               60 * time.Second,
 				staleAge:             50 * time.Second,
 				cacheSizeBytes:       1000,
 				rpStrategy:           rlspb.RouteLookupConfig_ASYNC_LOOKUP_DEFAULT_TARGET_ON_MISS,
+				defaultTarget:        "passthrough:///default",
 				cpName:               "grpclb",
 				cpTargetField:        "service_name",
 				cpConfig:             map[string]json.RawMessage{"childPolicy": json.RawMessage(`[{"pickfirst": {}}]`)},