xds/client: cleanup Dump to remove unnecessary version field (#4978)

xds/csds/csds.go

@@ -174,8 +174,8 @@ func nodeProtoToV3(n proto.Message) *v3corepb.Node {
  return node
 }
 
-func dumpToGenericXdsConfig(typeURL string, dumpF func() (string, map[string]xdsresource.UpdateWithMD)) []*v3statuspb.ClientConfig_GenericXdsConfig {
- _, dump := dumpF()
+func dumpToGenericXdsConfig(typeURL string, dumpF func() map[string]xdsresource.UpdateWithMD) []*v3statuspb.ClientConfig_GenericXdsConfig {
+ dump := dumpF()
  ret := make([]*v3statuspb.ClientConfig_GenericXdsConfig, 0, len(dump))
  for name, d := range dump {
  config := &v3statuspb.ClientConfig_GenericXdsConfig{

xds/internal/xdsclient/attributes.go

@@ -38,10 +38,10 @@ type XDSClient interface {
  WatchEndpoints(clusterName string, edsCb func(xdsresource.EndpointsUpdate, error)) (cancel func())
  ReportLoad(server string) (*load.Store, func())
 
- DumpLDS() (string, map[string]xdsresource.UpdateWithMD)
- DumpRDS() (string, map[string]xdsresource.UpdateWithMD)
- DumpCDS() (string, map[string]xdsresource.UpdateWithMD)
- DumpEDS() (string, map[string]xdsresource.UpdateWithMD)
+ DumpLDS() map[string]xdsresource.UpdateWithMD
+ DumpRDS() map[string]xdsresource.UpdateWithMD
+ DumpCDS() map[string]xdsresource.UpdateWithMD
+ DumpEDS() map[string]xdsresource.UpdateWithMD
 
  BootstrapConfig() *bootstrap.Config
  Close()

xds/internal/xdsclient/dump.go

@@ -23,21 +23,21 @@ import (
 )
 
 // DumpLDS returns the status and contents of LDS.
-func (c *clientImpl) DumpLDS() (string, map[string]xdsresource.UpdateWithMD) {
+func (c *clientImpl) DumpLDS() map[string]xdsresource.UpdateWithMD {
  return c.pubsub.Dump(xdsresource.ListenerResource)
 }
 
 // DumpRDS returns the status and contents of RDS.
-func (c *clientImpl) DumpRDS() (string, map[string]xdsresource.UpdateWithMD) {
+func (c *clientImpl) DumpRDS() map[string]xdsresource.UpdateWithMD {
  return c.pubsub.Dump(xdsresource.RouteConfigResource)
 }
 
 // DumpCDS returns the status and contents of CDS.
-func (c *clientImpl) DumpCDS() (string, map[string]xdsresource.UpdateWithMD) {
+func (c *clientImpl) DumpCDS() map[string]xdsresource.UpdateWithMD {
  return c.pubsub.Dump(xdsresource.ClusterResource)
 }
 
 // DumpEDS returns the status and contents of EDS.
-func (c *clientImpl) DumpEDS() (string, map[string]xdsresource.UpdateWithMD) {
+func (c *clientImpl) DumpEDS() map[string]xdsresource.UpdateWithMD {
  return c.pubsub.Dump(xdsresource.EndpointsResource)
 }

xds/internal/xdsclient/dump_test.go

@@ -30,6 +30,7 @@ import (
  v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
  "github.com/google/go-cmp/cmp"
  "github.com/google/go-cmp/cmp/cmpopts"
+ "google.golang.org/grpc/xds/internal/xdsclient"
  "google.golang.org/grpc/xds/internal/xdsclient/pubsub"
  "google.golang.org/grpc/xds/internal/xdsclient/xdsresource"
  "google.golang.org/protobuf/testing/protocmp"
@@ -40,7 +41,6 @@ import (
  "google.golang.org/grpc/credentials/insecure"
  "google.golang.org/grpc/internal/testutils"
  xdstestutils "google.golang.org/grpc/xds/internal/testutils"
- "google.golang.org/grpc/xds/internal/xdsclient"
  "google.golang.org/grpc/xds/internal/xdsclient/bootstrap"
 )
 
@@ -90,7 +90,7 @@ func (s) TestLDSConfigDump(t *testing.T) {
  updateHandler := client.(pubsub.UpdateHandler)
 
  // Expected unknown.
- if err := compareDump(client.DumpLDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil {
+ if err := compareDump(client.DumpLDS, map[string]xdsresource.UpdateWithMD{}); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -101,7 +101,7 @@ func (s) TestLDSConfigDump(t *testing.T) {
  wantRequested[n] = xdsresource.UpdateWithMD{MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusRequested}}
  }
  // Expected requested.
- if err := compareDump(client.DumpLDS, "", wantRequested); err != nil {
+ if err := compareDump(client.DumpLDS, wantRequested); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -117,7 +117,7 @@ func (s) TestLDSConfigDump(t *testing.T) {
  updateHandler.NewListeners(update0, xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: testVersion})
 
  // Expect ACK.
- if err := compareDump(client.DumpLDS, testVersion, want0); err != nil {
+ if err := compareDump(client.DumpLDS, want0); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -157,7 +157,7 @@ func (s) TestLDSConfigDump(t *testing.T) {
  MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: nackVersion},
  Raw: listenerRaws[ldsTargets[1]],
  }
- if err := compareDump(client.DumpLDS, nackVersion, wantDump); err != nil {
+ if err := compareDump(client.DumpLDS, wantDump); err != nil {
  t.Fatalf(err.Error())
  }
 }
@@ -206,7 +206,7 @@ func (s) TestRDSConfigDump(t *testing.T) {
  updateHandler := client.(pubsub.UpdateHandler)
 
  // Expected unknown.
- if err := compareDump(client.DumpRDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil {
+ if err := compareDump(client.DumpRDS, map[string]xdsresource.UpdateWithMD{}); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -217,7 +217,7 @@ func (s) TestRDSConfigDump(t *testing.T) {
  wantRequested[n] = xdsresource.UpdateWithMD{MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusRequested}}
  }
  // Expected requested.
- if err := compareDump(client.DumpRDS, "", wantRequested); err != nil {
+ if err := compareDump(client.DumpRDS, wantRequested); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -233,7 +233,7 @@ func (s) TestRDSConfigDump(t *testing.T) {
  updateHandler.NewRouteConfigs(update0, xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: testVersion})
 
  // Expect ACK.
- if err := compareDump(client.DumpRDS, testVersion, want0); err != nil {
+ if err := compareDump(client.DumpRDS, want0); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -272,7 +272,7 @@ func (s) TestRDSConfigDump(t *testing.T) {
  MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: nackVersion},
  Raw: routeRaws[rdsTargets[1]],
  }
- if err := compareDump(client.DumpRDS, nackVersion, wantDump); err != nil {
+ if err := compareDump(client.DumpRDS, wantDump); err != nil {
  t.Fatalf(err.Error())
  }
 }
@@ -322,7 +322,7 @@ func (s) TestCDSConfigDump(t *testing.T) {
  updateHandler := client.(pubsub.UpdateHandler)
 
  // Expected unknown.
- if err := compareDump(client.DumpCDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil {
+ if err := compareDump(client.DumpCDS, map[string]xdsresource.UpdateWithMD{}); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -333,7 +333,7 @@ func (s) TestCDSConfigDump(t *testing.T) {
  wantRequested[n] = xdsresource.UpdateWithMD{MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusRequested}}
  }
  // Expected requested.
- if err := compareDump(client.DumpCDS, "", wantRequested); err != nil {
+ if err := compareDump(client.DumpCDS, wantRequested); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -349,7 +349,7 @@ func (s) TestCDSConfigDump(t *testing.T) {
  updateHandler.NewClusters(update0, xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: testVersion})
 
  // Expect ACK.
- if err := compareDump(client.DumpCDS, testVersion, want0); err != nil {
+ if err := compareDump(client.DumpCDS, want0); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -388,7 +388,7 @@ func (s) TestCDSConfigDump(t *testing.T) {
  MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: nackVersion},
  Raw: clusterRaws[cdsTargets[1]],
  }
- if err := compareDump(client.DumpCDS, nackVersion, wantDump); err != nil {
+ if err := compareDump(client.DumpCDS, wantDump); err != nil {
  t.Fatalf(err.Error())
  }
 }
@@ -424,7 +424,7 @@ func (s) TestEDSConfigDump(t *testing.T) {
  updateHandler := client.(pubsub.UpdateHandler)
 
  // Expected unknown.
- if err := compareDump(client.DumpEDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil {
+ if err := compareDump(client.DumpEDS, map[string]xdsresource.UpdateWithMD{}); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -435,7 +435,7 @@ func (s) TestEDSConfigDump(t *testing.T) {
  wantRequested[n] = xdsresource.UpdateWithMD{MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusRequested}}
  }
  // Expected requested.
- if err := compareDump(client.DumpEDS, "", wantRequested); err != nil {
+ if err := compareDump(client.DumpEDS, wantRequested); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -451,7 +451,7 @@ func (s) TestEDSConfigDump(t *testing.T) {
  updateHandler.NewEndpoints(update0, xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: testVersion})
 
  // Expect ACK.
- if err := compareDump(client.DumpEDS, testVersion, want0); err != nil {
+ if err := compareDump(client.DumpEDS, want0); err != nil {
  t.Fatalf(err.Error())
  }
 
@@ -490,16 +490,13 @@ func (s) TestEDSConfigDump(t *testing.T) {
  MD: xdsresource.UpdateMetadata{Status: xdsresource.ServiceStatusACKed, Version: nackVersion},
  Raw: endpointRaws[edsTargets[1]],
  }
- if err := compareDump(client.DumpEDS, nackVersion, wantDump); err != nil {
+ if err := compareDump(client.DumpEDS, wantDump); err != nil {
  t.Fatalf(err.Error())
  }
 }
 
-func compareDump(dumpFunc func() (string, map[string]xdsresource.UpdateWithMD), wantVersion string, wantDump interface{}) error {
- v, dump := dumpFunc()
- if v != wantVersion {
- return fmt.Errorf("Dump() returned version %q, want %q", v, wantVersion)
- }
+func compareDump(dumpFunc func() map[string]xdsresource.UpdateWithMD, wantDump interface{}) error {
+ dump := dumpFunc()
  cmpOpts := cmp.Options{
  cmpopts.EquateEmpty(),
  cmp.Comparer(func(a, b time.Time) bool { return true }),

xds/internal/xdsclient/pubsub/dump.go

@@ -50,35 +50,30 @@ func rawFromCache(s string, cache interface{}) *anypb.Any {
 }
 
 // Dump dumps the resource for the given type.
-func (pb *Pubsub) Dump(t xdsresource.ResourceType) (string, map[string]xdsresource.UpdateWithMD) {
+func (pb *Pubsub) Dump(t xdsresource.ResourceType) map[string]xdsresource.UpdateWithMD {
  pb.mu.Lock()
  defer pb.mu.Unlock()
 
  var (
- version string
- md map[string]xdsresource.UpdateMetadata
- cache interface{}
+ md map[string]xdsresource.UpdateMetadata
+ cache interface{}
  )
  switch t {
  case xdsresource.ListenerResource:
- version = pb.ldsVersion
  md = pb.ldsMD
  cache = pb.ldsCache
  case xdsresource.RouteConfigResource:
- version = pb.rdsVersion
  md = pb.rdsMD
  cache = pb.rdsCache
  case xdsresource.ClusterResource:
- version = pb.cdsVersion
  md = pb.cdsMD
  cache = pb.cdsCache
  case xdsresource.EndpointsResource:
- version = pb.edsVersion
  md = pb.edsMD
  cache = pb.edsCache
  default:
  pb.logger.Errorf("dumping resource of unknown type: %v", t)
- return "", nil
+ return nil
  }
 
  ret := make(map[string]xdsresource.UpdateWithMD, len(md))
@@ -88,5 +83,5 @@ func (pb *Pubsub) Dump(t xdsresource.ResourceType) (string, map[string]xdsresour
  Raw: rawFromCache(s, cache),
  }
  }
- return version, ret
+ return ret
 }

xds/internal/xdsclient/pubsub/pubsub.go

@@ -46,19 +46,15 @@ type Pubsub struct {
  // All the following maps are to keep the updates/metadata in a cache.
  mu sync.Mutex
  ldsWatchers map[string]map[*watchInfo]bool
- ldsVersion string // Only used in CSDS.
  ldsCache map[string]xdsresource.ListenerUpdate
  ldsMD map[string]xdsresource.UpdateMetadata
  rdsWatchers map[string]map[*watchInfo]bool
- rdsVersion string // Only used in CSDS.
  rdsCache map[string]xdsresource.RouteConfigUpdate
  rdsMD map[string]xdsresource.UpdateMetadata
  cdsWatchers map[string]map[*watchInfo]bool
- cdsVersion string // Only used in CSDS.
  cdsCache map[string]xdsresource.ClusterUpdate
  cdsMD map[string]xdsresource.UpdateMetadata
  edsWatchers map[string]map[*watchInfo]bool
- edsVersion string // Only used in CSDS.
  edsCache map[string]xdsresource.EndpointsUpdate
  edsMD map[string]xdsresource.UpdateMetadata
 }

xds/internal/xdsclient/pubsub/update.go

@@ -79,10 +79,6 @@ func (pb *Pubsub) NewListeners(updates map[string]xdsresource.ListenerUpdateErrT
  pb.mu.Lock()
  defer pb.mu.Unlock()
 
- pb.ldsVersion = metadata.Version
- if metadata.ErrState != nil {
- pb.ldsVersion = metadata.ErrState.Version
- }
  for name, uErr := range updates {
  if s, ok := pb.ldsWatchers[name]; ok {
  if uErr.Err != nil {
@@ -145,10 +141,6 @@ func (pb *Pubsub) NewRouteConfigs(updates map[string]xdsresource.RouteConfigUpda
  defer pb.mu.Unlock()
 
  // If no error received, the status is ACK.
- pb.rdsVersion = metadata.Version
- if metadata.ErrState != nil {
- pb.rdsVersion = metadata.ErrState.Version
- }
  for name, uErr := range updates {
  if s, ok := pb.rdsWatchers[name]; ok {
  if uErr.Err != nil {
@@ -193,10 +185,6 @@ func (pb *Pubsub) NewClusters(updates map[string]xdsresource.ClusterUpdateErrTup
  pb.mu.Lock()
  defer pb.mu.Unlock()
 
- pb.cdsVersion = metadata.Version
- if metadata.ErrState != nil {
- pb.cdsVersion = metadata.ErrState.Version
- }
  for name, uErr := range updates {
  if s, ok := pb.cdsWatchers[name]; ok {
  if uErr.Err != nil {
@@ -260,10 +248,6 @@ func (pb *Pubsub) NewEndpoints(updates map[string]xdsresource.EndpointsUpdateErr
  pb.mu.Lock()
  defer pb.mu.Unlock()
 
- pb.edsVersion = metadata.Version
- if metadata.ErrState != nil {
- pb.edsVersion = metadata.ErrState.Version
- }
  for name, uErr := range updates {
  if s, ok := pb.edsWatchers[name]; ok {
  if uErr.Err != nil {