eds/lrs: handle nil when LRS is disabled (#4086)

grpc · Jan 11, 2021 · f60ed8a · f60ed8a
1 parent 39242ac
commit f60ed8a
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 8 deletions.
diff --git a/xds/internal/balancer/edsbalancer/eds_impl_test.go b/xds/internal/balancer/edsbalancer/eds_impl_test.go
@@ -746,3 +746,33 @@ func (s) TestEDS_LoadReport(t *testing.T) {
 		t.Errorf("store.stats() returned unexpected diff (-want +got):\n%s", diff)
 	}
 }
+
+// TestEDS_LoadReportDisabled covers the case that LRS is disabled. It makes
+// sure the EDS implementation isn't broken (doesn't panic).
+func (s) TestEDS_LoadReportDisabled(t *testing.T) {
+	lsWrapper := &loadStoreWrapper{}
+	lsWrapper.updateServiceName(testClusterNames[0])
+	// Not calling lsWrapper.updateLoadStore(loadStore) because LRS is disabled.
+	cw := &xdsClientWrapper{
+		loadWrapper: lsWrapper,
+	}
+
+	cc := testutils.NewTestClientConn(t)
+	edsb := newEDSBalancerImpl(cc, nil, cw, nil)
+	edsb.enqueueChildBalancerStateUpdate = edsb.updateState
+
+	// One localities, with one backend.
+	clab1 := testutils.NewClusterLoadAssignmentBuilder(testClusterNames[0], nil)
+	clab1.AddLocality(testSubZones[0], 1, 0, testEndpointAddrs[:1], nil)
+	edsb.handleEDSResponse(parseEDSRespProtoForTesting(clab1.Build()))
+	sc1 := <-cc.NewSubConnCh
+	edsb.handleSubConnStateChange(sc1, connectivity.Connecting)
+	edsb.handleSubConnStateChange(sc1, connectivity.Ready)
+
+	// Test roundrobin with two subconns.
+	p1 := <-cc.NewPickerCh
+	// We call picks to make sure they don't panic.
+	for i := 0; i < 10; i++ {
+		p1.Pick(balancer.PickInfo{})
+	}
+}
diff --git a/xds/internal/balancer/edsbalancer/xds_client_wrapper.go b/xds/internal/balancer/edsbalancer/xds_client_wrapper.go
@@ -72,25 +72,33 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) {
 func (lsw *loadStoreWrapper) CallStarted(locality string) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallStarted(locality)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallStarted(locality)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallFinished(locality string, err error) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallFinished(locality, err)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallFinished(locality, err)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallServerLoad(locality, name, val)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallServerLoad(locality, name, val)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallDropped(category string) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallDropped(category)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallDropped(category)
+	}
 }
 
 // xdsclientWrapper is responsible for getting the xds client from attributes or

diff --git a/xds/internal/balancer/lrs/balancer.go b/xds/internal/balancer/lrs/balancer.go
@@ -187,25 +187,33 @@ func (lsw *loadStoreWrapper) updateLoadStore(store *load.Store) {
 func (lsw *loadStoreWrapper) CallStarted(locality string) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallStarted(locality)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallStarted(locality)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallFinished(locality string, err error) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallFinished(locality, err)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallFinished(locality, err)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallServerLoad(locality, name string, val float64) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallServerLoad(locality, name, val)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallServerLoad(locality, name, val)
+	}
 }
 
 func (lsw *loadStoreWrapper) CallDropped(category string) {
 	lsw.mu.RLock()
 	defer lsw.mu.RUnlock()
-	lsw.perCluster.CallDropped(category)
+	if lsw.perCluster != nil {
+		lsw.perCluster.CallDropped(category)
+	}
 }
 
 type xdsClientWrapper struct {