xds/resolver: fix panic when two LDS updates are receives without RDS in between (#4327)

Also confirmed that the LDS updates shouldn't trigger state update without the
RDS.
This commit is contained in:
Menghan Li 2021-04-08 11:34:02 -07:00 committed by GitHub
parent 493d388ad2
commit 1895da54b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 51 additions and 1 deletions

View File

@ -132,7 +132,15 @@ func (w *serviceUpdateWatcher) handleLDSResp(update xdsclient.ListenerUpdate, er
//
// If the route name did change, then we must wait until the first RDS
// update before reporting this LDS config.
w.serviceCb(w.lastUpdate, nil)
if w.lastUpdate.virtualHost != nil {
// We want to send an update with the new fields from the new LDS
// (e.g. max stream duration), and old fields from the the previous
// RDS.
//
// But note that this should only happen when virtual host is set,
// which means an RDS was received.
w.serviceCb(w.lastUpdate, nil)
}
return
}
w.rdsName = update.RouteConfigName

View File

@ -1030,6 +1030,48 @@ func (s) TestXDSResolverResourceNotFoundError(t *testing.T) {
}
}
// TestXDSResolverMultipleLDSUpdates tests the case where two LDS updates with
// the same RDS name to watch are received without an RDS in between. Those LDS
// updates shouldn't trigger service config update.
//
// This test case also makes sure the resolver doesn't panic.
func (s) TestXDSResolverMultipleLDSUpdates(t *testing.T) {
xdsC := fakeclient.NewClient()
xdsR, tcc, cancel := testSetup(t, setupOpts{
xdsClientFunc: func() (xdsClientInterface, error) { return xdsC, nil },
})
defer func() {
cancel()
xdsR.Close()
}()
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
defer cancel()
waitForWatchListener(ctx, t, xdsC, targetStr)
xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, HTTPFilters: routerFilterList}, nil)
waitForWatchRouteConfig(ctx, t, xdsC, routeStr)
defer replaceRandNumGenerator(0)()
// Send a new LDS update, with the same fields.
xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, HTTPFilters: routerFilterList}, nil)
ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout)
defer cancel()
// Should NOT trigger a state update.
gotState, err := tcc.stateCh.Receive(ctx)
if err == nil {
t.Fatalf("ClientConn.UpdateState received %v, want timeout error", gotState)
}
// Send a new LDS update, with the same RDS name, but different fields.
xdsC.InvokeWatchListenerCallback(xdsclient.ListenerUpdate{RouteConfigName: routeStr, MaxStreamDuration: time.Second, HTTPFilters: routerFilterList}, nil)
ctx, cancel = context.WithTimeout(context.Background(), defaultTestShortTimeout)
defer cancel()
gotState, err = tcc.stateCh.Receive(ctx)
if err == nil {
t.Fatalf("ClientConn.UpdateState received %v, want timeout error", gotState)
}
}
type filterBuilder struct {
httpfilter.Filter // embedded as we do not need to implement registry / parsing in this test.
path *[]string