diff --git a/balancer/rls/internal/balancer.go b/balancer/rls/internal/balancer.go index a7dd7d9747af..cc04060d656b 100644 --- a/balancer/rls/internal/balancer.go +++ b/balancer/rls/internal/balancer.go @@ -22,7 +22,6 @@ package rls import ( "sync" - "time" "google.golang.org/grpc" "google.golang.org/grpc/balancer" @@ -37,9 +36,7 @@ var ( _ balancer.V2Balancer = (*rlsBalancer)(nil) // For overriding in tests. - newRLSClientFunc = func(cc *grpc.ClientConn, target string, timeout time.Duration) *rlsClient { - return newRLSClient(cc, target, timeout) - } + newRLSClientFunc = newRLSClient ) // rlsBalancer implements the RLS LB policy. @@ -65,7 +62,8 @@ type rlsBalancer struct { // the update will happen asynchronously. func (lb *rlsBalancer) run() { for { - // TODO(easwars): Handle other updates. + // TODO(easwars): Handle other updates like subConn state changes, RLS + // responses from the server etc. select { case u := <-lb.ccUpdateCh: lb.handleClientConnUpdate(u) @@ -90,14 +88,7 @@ func (lb *rlsBalancer) handleClientConnUpdate(ccs *balancer.ClientConnState) { } newCfg := ccs.BalancerConfig.(*lbConfig) - oldCfg := lb.lbCfg - if oldCfg == nil { - // This is the first time we are receiving a service config. - lb.lbCfg = &lbConfig{} - oldCfg = lb.lbCfg - } - - if oldCfg.Equal(newCfg) { + if lb.lbCfg.Equal(newCfg) { grpclog.Info("rls: new service config matches existing config") return } @@ -121,11 +112,12 @@ func (lb *rlsBalancer) UpdateClientConnState(ccs balancer.ClientConnState) error func (lb *rlsBalancer) ResolverError(error) { // ResolverError is called by gRPC when the name resolver reports an error. // TODO(easwars): How do we handle this? + grpclog.Fatal("rls: ResolverError is not yet unimplemented") } // UpdateSubConnState implements balancer.V2Balancer interface. func (lb *rlsBalancer) UpdateSubConnState(_ balancer.SubConn, _ balancer.SubConnState) { - grpclog.Error("rlsbalancer.UpdateSubConnState is not yet implemented") + grpclog.Fatal("rls: UpdateSubConnState is not yet implemented") } // Cleans up the resources allocated by the LB policy including the clientConn @@ -143,12 +135,12 @@ func (lb *rlsBalancer) Close() { // HandleSubConnStateChange implements balancer.Balancer interface. func (lb *rlsBalancer) HandleSubConnStateChange(_ balancer.SubConn, _ connectivity.State) { - grpclog.Errorf("UpdateSubConnState should be called instead of HandleSubConnStateChange") + grpclog.Fatal("UpdateSubConnState should be called instead of HandleSubConnStateChange") } // HandleResolvedAddrs implements balancer.Balancer interface. func (lb *rlsBalancer) HandleResolvedAddrs(_ []resolver.Address, _ error) { - grpclog.Errorf("UpdateClientConnState should be called instead of HandleResolvedAddrs") + grpclog.Fatal("UpdateClientConnState should be called instead of HandleResolvedAddrs") } // updateControlChannel updates the RLS client if required. diff --git a/balancer/rls/internal/balancer_test.go b/balancer/rls/internal/balancer_test.go index ee0cf17b7eb8..990372d0e98f 100644 --- a/balancer/rls/internal/balancer_test.go +++ b/balancer/rls/internal/balancer_test.go @@ -27,10 +27,10 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/balancer" - "google.golang.org/grpc/balancer/rls/internal/testutils" "google.golang.org/grpc/balancer/rls/internal/testutils/fakeserver" "google.golang.org/grpc/credentials" "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/testdata" ) diff --git a/balancer/rls/internal/builder.go b/balancer/rls/internal/builder.go index 79f163cac506..c38babff4d3d 100644 --- a/balancer/rls/internal/builder.go +++ b/balancer/rls/internal/builder.go @@ -47,6 +47,7 @@ func (*rlsBB) Build(cc balancer.ClientConn, opts balancer.BuildOptions) balancer done: grpcsync.NewEvent(), cc: cc, opts: opts, + lbCfg: &lbConfig{}, ccUpdateCh: make(chan *balancer.ClientConnState), } go lb.run() diff --git a/balancer/rls/internal/client_test.go b/balancer/rls/internal/client_test.go index f674be941c33..1b72d3b8e646 100644 --- a/balancer/rls/internal/client_test.go +++ b/balancer/rls/internal/client_test.go @@ -28,15 +28,12 @@ import ( "github.com/golang/protobuf/proto" "github.com/google/go-cmp/cmp" + "google.golang.org/grpc" rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1" - + "google.golang.org/grpc/balancer/rls/internal/testutils/fakeserver" "google.golang.org/grpc/codes" + "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/status" - - "google.golang.org/grpc/balancer/rls/internal/testutils" - - "google.golang.org/grpc" - "google.golang.org/grpc/balancer/rls/internal/testutils/fakeserver" ) const ( diff --git a/balancer/rls/internal/picker_test.go b/balancer/rls/internal/picker_test.go index 32c709771243..b14a9fad340a 100644 --- a/balancer/rls/internal/picker_test.go +++ b/balancer/rls/internal/picker_test.go @@ -28,15 +28,13 @@ import ( "testing" "time" - "google.golang.org/grpc/balancer/rls/internal/testutils" - - "google.golang.org/grpc/internal/grpcrand" - "github.com/google/go-cmp/cmp" "google.golang.org/grpc/balancer" "google.golang.org/grpc/balancer/rls/internal/cache" "google.golang.org/grpc/balancer/rls/internal/keys" rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1" + "google.golang.org/grpc/internal/grpcrand" + "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/metadata" ) diff --git a/balancer/rls/internal/testutils/fakeserver/fakeserver.go b/balancer/rls/internal/testutils/fakeserver/fakeserver.go index 193a3f4404d0..93947da4ccef 100644 --- a/balancer/rls/internal/testutils/fakeserver/fakeserver.go +++ b/balancer/rls/internal/testutils/fakeserver/fakeserver.go @@ -27,11 +27,10 @@ import ( "net" "time" - "google.golang.org/grpc/balancer/rls/internal/testutils" - "google.golang.org/grpc" rlsgrpc "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1" rlspb "google.golang.org/grpc/balancer/rls/internal/proto/grpc_lookup_v1" + "google.golang.org/grpc/internal/testutils" ) const ( diff --git a/balancer/rls/internal/testutils/channel.go b/internal/testutils/channel.go similarity index 96% rename from balancer/rls/internal/testutils/channel.go rename to internal/testutils/channel.go index 973f98725c02..35f67ea285c1 100644 --- a/balancer/rls/internal/testutils/channel.go +++ b/internal/testutils/channel.go @@ -15,7 +15,6 @@ * limitations under the License. */ -// Package testutils provides utility types, for use in tests. package testutils import (