From 1593b7ff7aec08e56915c7c64a395e5252791a00 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Fri, 12 Sep 2025 09:37:10 +0000 Subject: [PATCH 01/12] Implemented the removed Generic resource decoder --- internal/xds/server/listener_wrapper.go | 3 +- internal/xds/xdsclient/client.go | 4 +- internal/xds/xdsclient/clientimpl_test.go | 32 ++--- internal/xds/xdsclient/clientimpl_watchers.go | 8 +- internal/xds/xdsclient/resource_types.go | 8 +- .../xdsresource/cluster_resource_type.go | 117 ++++++++++++---- .../xdsresource/endpoints_resource_type.go | 96 +++++++++---- .../xdsresource/listener_resource_type.go | 118 ++++++++++++---- .../xdsclient/xdsresource/resource_type.go | 128 +----------------- .../xdsresource/route_config_resource_type.go | 92 +++++++++---- 10 files changed, 359 insertions(+), 247 deletions(-) diff --git a/internal/xds/server/listener_wrapper.go b/internal/xds/server/listener_wrapper.go index 1f7da61175e6..fab8419f0462 100644 --- a/internal/xds/server/listener_wrapper.go +++ b/internal/xds/server/listener_wrapper.go @@ -33,6 +33,7 @@ import ( internalgrpclog "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpcsync" "google.golang.org/grpc/internal/xds/bootstrap" + "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" ) @@ -58,7 +59,7 @@ type ServingModeCallback func(addr net.Addr, mode connectivity.ServingMode, err // XDSClient wraps the methods on the XDSClient which are required by // the listenerWrapper. type XDSClient interface { - WatchResource(rType xdsresource.Type, resourceName string, watcher xdsresource.ResourceWatcher) (cancel func()) + WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) BootstrapConfig() *bootstrap.Config } diff --git a/internal/xds/xdsclient/client.go b/internal/xds/xdsclient/client.go index 514273164402..f8e223176558 100644 --- a/internal/xds/xdsclient/client.go +++ b/internal/xds/xdsclient/client.go @@ -26,7 +26,7 @@ import ( v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/internal/xds/clients/lrsclient" - "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" + "google.golang.org/grpc/internal/xds/clients/xdsclient" ) // XDSClient is a full fledged gRPC client which queries a set of discovery APIs @@ -47,7 +47,7 @@ type XDSClient interface { // During a race (e.g. an xDS response is received while the user is calling // cancel()), there's a small window where the callback can be called after // the watcher is canceled. Callers need to handle this case. - WatchResource(rType xdsresource.Type, resourceName string, watcher xdsresource.ResourceWatcher) (cancel func()) + WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) ReportLoad(*bootstrap.ServerConfig) (*lrsclient.LoadStore, func(context.Context)) diff --git a/internal/xds/xdsclient/clientimpl_test.go b/internal/xds/xdsclient/clientimpl_test.go index d2fc8f7f9332..acc2b27dbe59 100644 --- a/internal/xds/xdsclient/clientimpl_test.go +++ b/internal/xds/xdsclient/clientimpl_test.go @@ -81,10 +81,10 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Node: clients.Node{ID: node.GetId(), Cluster: node.GetCluster(), Metadata: node.Metadata, Locality: clients.Locality{Region: node.Locality.Region, Zone: node.Locality.Zone, SubZone: node.Locality.SubZone}, UserAgentName: node.UserAgentName, UserAgentVersion: node.GetUserAgentVersion()}, Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ - version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericListenerResourceTypeDecoder(c)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericRouteConfigResourceTypeDecoder()}, - version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericEndpointsResourceTypeDecoder()}, + version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -119,10 +119,10 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Node: clients.Node{ID: node.GetId(), Cluster: node.GetCluster(), Metadata: node.Metadata, UserAgentName: node.UserAgentName, UserAgentVersion: node.GetUserAgentVersion()}, Authorities: map[string]xdsclient.Authority{"auth1": {XDSServers: []xdsclient.ServerConfig{expTopLevelS}}, "auth2": {XDSServers: []xdsclient.ServerConfig{expAuth2S}}}, ResourceTypes: map[string]xdsclient.ResourceType{ - version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericListenerResourceTypeDecoder(c)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericRouteConfigResourceTypeDecoder()}, - version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericClusterResourceTypeDecoder(c, gSCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericEndpointsResourceTypeDecoder()}, + version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gSCfgMap)}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gSCfgMap)}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -151,10 +151,10 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Node: clients.Node{ID: node.GetId(), Cluster: node.GetCluster(), Metadata: node.Metadata, UserAgentName: node.UserAgentName, UserAgentVersion: node.GetUserAgentVersion()}, Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ - version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericListenerResourceTypeDecoder(c)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericRouteConfigResourceTypeDecoder()}, - version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericEndpointsResourceTypeDecoder()}, + version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -183,10 +183,10 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Node: clients.Node{ID: node.GetId(), Cluster: node.GetCluster(), Metadata: node.Metadata, UserAgentName: node.UserAgentName, UserAgentVersion: node.GetUserAgentVersion()}, Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ - version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericListenerResourceTypeDecoder(c)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericRouteConfigResourceTypeDecoder()}, - version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewGenericClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewGenericEndpointsResourceTypeDecoder()}, + version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ diff --git a/internal/xds/xdsclient/clientimpl_watchers.go b/internal/xds/xdsclient/clientimpl_watchers.go index 398de1ed73b6..d77c08c0616a 100644 --- a/internal/xds/xdsclient/clientimpl_watchers.go +++ b/internal/xds/xdsclient/clientimpl_watchers.go @@ -17,15 +17,13 @@ package xdsclient -import ( - "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" -) +import "google.golang.org/grpc/internal/xds/clients/xdsclient" // WatchResource uses xDS to discover the resource associated with the provided // resource name. The resource type implementation determines how xDS responses // are are deserialized and validated, as received from the xDS management // server. Upon receipt of a response from the management server, an // appropriate callback on the watcher is invoked. -func (c *clientImpl) WatchResource(rType xdsresource.Type, resourceName string, watcher xdsresource.ResourceWatcher) (cancel func()) { - return c.XDSClient.WatchResource(rType.TypeURL(), resourceName, xdsresource.GenericResourceWatcher(watcher)) +func (c *clientImpl) WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) { + return c.XDSClient.WatchResource(rType.TypeURL, resourceName, watcher) } diff --git a/internal/xds/xdsclient/resource_types.go b/internal/xds/xdsclient/resource_types.go index 88451ab8257e..d007666da182 100644 --- a/internal/xds/xdsclient/resource_types.go +++ b/internal/xds/xdsclient/resource_types.go @@ -30,25 +30,25 @@ func supportedResourceTypes(config *bootstrap.Config, gServerCfgMap map[xdsclien TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, - Decoder: xdsresource.NewGenericListenerResourceTypeDecoder(config), + Decoder: xdsresource.NewListenerResourceTypeDecoder(config, gServerCfgMap), }, version.V3RouteConfigURL: { TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.NewGenericRouteConfigResourceTypeDecoder(), + Decoder: xdsresource.NewRouteConfigResourceTypeDecoder(), }, version.V3ClusterURL: { TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, - Decoder: xdsresource.NewGenericClusterResourceTypeDecoder(config, gServerCfgMap), + Decoder: xdsresource.NewClusterResourceTypeDecoder(config, gServerCfgMap), }, version.V3EndpointsURL: { TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.NewGenericEndpointsResourceTypeDecoder(), + Decoder: xdsresource.NewEndpointsResourceTypeDecoder(), }, } } diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index 2a6a08f90647..4939184991ca 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -18,6 +18,9 @@ package xdsresource import ( + "bytes" + "fmt" + "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/internal/xds/bootstrap" xdsclient "google.golang.org/grpc/internal/xds/clients/xdsclient" @@ -34,15 +37,15 @@ const ( var ( // Compile time interface checks. - _ Type = clusterResourceType{} + _ xdsclient.Decoder = clusterResourceType{} + _ xdsclient.ResourceData = (*ClusterResourceData)(nil) // Singleton instantiation of the resource type implementation. - clusterType = clusterResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3ClusterURL, - typeName: ClusterResourceTypeName, - allResourcesRequiredInSotW: true, - }, + ClusterResource = xdsclient.ResourceType{ + TypeURL: version.V3ClusterURL, + TypeName: ClusterResourceTypeName, + AllResourcesRequiredInSotW: true, + Decoder: &clusterResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3ClusterURL, typeName: ClusterResourceTypeName, allResourcesRequiredInSotW: true}}, } ) @@ -52,28 +55,50 @@ var ( // Implements the Type interface. type clusterResourceType struct { resourceTypeState + BootstrapConfig *bootstrap.Config + ServerConfigMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig } // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. -func (clusterResourceType) Decode(opts *DecodeOptions, resource *anypb.Any) (*DecodeResult, error) { - name, cluster, err := unmarshalClusterResource(resource, opts.ServerConfig) +func (ct clusterResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { + // Convert generic AnyProto -> anypb.Any + a := &anypb.Any{ + TypeUrl: resource.TypeURL, + Value: resource.Value, + } + + // Map generic decode options to internal options + internalOpts := &DecodeOptions{BootstrapConfig: ct.BootstrapConfig} + if gOpts.ServerConfig != nil && ct.ServerConfigMap != nil { + if sc, ok := ct.ServerConfigMap[*gOpts.ServerConfig]; ok { + internalOpts.ServerConfig = sc + } + } + + name, cluster, err := unmarshalClusterResource(a, internalOpts.ServerConfig) switch { case name == "": - // Name is unset only when protobuf deserialization fails. + // Name unset => protobuf deserialization failure. return nil, err case err != nil: - // Protobuf deserialization succeeded, but resource validation failed. - return &DecodeResult{Name: name, Resource: &ClusterResourceData{Resource: ClusterUpdate{}}}, err + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ClusterResourceData{Resource: ClusterUpdate{}}, + }, err } - // Perform extra validation here. - if err := securityConfigValidator(opts.BootstrapConfig, cluster.SecurityCfg); err != nil { - return &DecodeResult{Name: name, Resource: &ClusterResourceData{Resource: ClusterUpdate{}}}, err + if err := securityConfigValidator(internalOpts.BootstrapConfig, cluster.SecurityCfg); err != nil { + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ClusterResourceData{Resource: ClusterUpdate{}}, + }, err } - return &DecodeResult{Name: name, Resource: &ClusterResourceData{Resource: cluster}}, nil - + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ClusterResourceData{Resource: cluster}, + }, nil } // ClusterResourceData wraps the configuration of a Cluster resource as received @@ -109,6 +134,29 @@ func (c *ClusterResourceData) Raw() *anypb.Any { return c.Resource.Raw } +// Equal returns true if other is equal to c +func (c *ClusterResourceData) Equal(other xdsclient.ResourceData) bool { + if c == nil && other == nil { + return true + } + if (c == nil) != (other == nil) { + return false + } + if otherCRD, ok := other.(*ClusterResourceData); ok { + return c.RawEqual(otherCRD) + } + return bytes.Equal(c.Bytes(), other.Bytes()) +} + +// Bytes returns the underlying raw bytes of the clustered resource. +func (c *ClusterResourceData) Bytes() []byte { + raw := c.Raw() + if raw == nil { + return nil + } + return raw.Value +} + // ClusterWatcher wraps the callbacks to be invoked for different events // corresponding to the cluster resource being watched. gRFC A88 contains an // exhaustive list of what method is invoked under what conditions. @@ -133,8 +181,16 @@ type delegatingClusterWatcher struct { watcher ClusterWatcher } -func (d *delegatingClusterWatcher) ResourceChanged(data ResourceData, onDone func()) { - c := data.(*ClusterResourceData) +func (d *delegatingClusterWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { + if gData == nil { + d.watcher.ResourceError(fmt.Errorf("cluster resource missing"), onDone) + return + } + c, ok := gData.(*ClusterResourceData) + if !ok { + d.watcher.ResourceError(fmt.Errorf("delegatingClusterWatcher: unexpected resource data type %T", gData), onDone) + return + } d.watcher.ResourceChanged(c, onDone) } @@ -149,12 +205,23 @@ func (d *delegatingClusterWatcher) AmbientError(err error, onDone func()) { // WatchCluster uses xDS to discover the configuration associated with the // provided cluster resource name. func WatchCluster(p Producer, name string, w ClusterWatcher) (cancel func()) { - delegator := &delegatingClusterWatcher{watcher: w} - return p.WatchResource(clusterType, name, delegator) + var gw xdsclient.ResourceWatcher + if w != nil { + gw = &delegatingClusterWatcher{watcher: w} + } + return p.WatchResource(ClusterResource, name, gw) } -// NewGenericClusterResourceTypeDecoder returns a xdsclient.Decoder that -// wraps the xdsresource.clusterType. -func NewGenericClusterResourceTypeDecoder(bc *bootstrap.Config, gServerCfgMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig) xdsclient.Decoder { - return &GenericResourceTypeDecoder{ResourceType: clusterType, BootstrapConfig: bc, ServerConfigMap: gServerCfgMap} +// NewClusterResourceTypeDecoder returns a xdsclient.Decoder that has access to +// bootstrap config and server config mapping for decoding. +func NewClusterResourceTypeDecoder(bc *bootstrap.Config, gServerCfgMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig) xdsclient.Decoder { + return &clusterResourceType{ + resourceTypeState: resourceTypeState{ + typeURL: version.V3ClusterURL, + typeName: ClusterResourceTypeName, + allResourcesRequiredInSotW: true, + }, + BootstrapConfig: bc, + ServerConfigMap: gServerCfgMap, + } } diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index 7ca45ec6ad0c..9029ac458282 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -18,6 +18,9 @@ package xdsresource import ( + "bytes" + "fmt" + "google.golang.org/grpc/internal/pretty" xdsclient "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource/version" @@ -33,15 +36,15 @@ const ( var ( // Compile time interface checks. - _ Type = endpointsResourceType{} - - // Singleton instantiation of the resource type implementation. - endpointsType = endpointsResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3EndpointsURL, - typeName: "EndpointsResource", - allResourcesRequiredInSotW: false, - }, + _ xdsclient.Decoder = endpointsResourceType{} + _ xdsclient.ResourceData = (*EndpointsResourceData)(nil) + + // Exported generic ResourceType for endpoints. + EndpointsResource = xdsclient.ResourceType{ + TypeURL: version.V3EndpointsURL, + TypeName: EndpointsResourceTypeName, + AllResourcesRequiredInSotW: false, + Decoder: endpointsResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3EndpointsURL, typeName: EndpointsResourceTypeName, allResourcesRequiredInSotW: false}}, } ) @@ -55,19 +58,22 @@ type endpointsResourceType struct { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. -func (endpointsResourceType) Decode(_ *DecodeOptions, resource *anypb.Any) (*DecodeResult, error) { - name, rc, err := unmarshalEndpointsResource(resource) +func (et endpointsResourceType) Decode(resource xdsclient.AnyProto, _ xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { + // Build anypb.Any from the generic AnyProto passed by the generic client. + a := &anypb.Any{ + TypeUrl: resource.TypeURL, + Value: resource.Value, + } + + // Unmarshal using the helper (it expects an *anypb.Any) + name, rc, err := unmarshalEndpointsResource(a) switch { case name == "": - // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: - // Protobuf deserialization succeeded, but resource validation failed. - return &DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: EndpointsUpdate{}}}, err + return &xdsclient.DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: EndpointsUpdate{}}}, err } - - return &DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: rc}}, nil - + return &xdsclient.DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: rc}}, nil } // EndpointsResourceData wraps the configuration of an Endpoints resource as @@ -104,6 +110,29 @@ func (e *EndpointsResourceData) Raw() *anypb.Any { return e.Resource.Raw } +// Equal returns true if other is equal to c +func (e *EndpointsResourceData) Equal(other xdsclient.ResourceData) bool { + if e == nil && other == nil { + return true + } + if (e == nil) != (other == nil) { + return false + } + if otherERD, ok := other.(*EndpointsResourceData); ok { + return e.RawEqual(otherERD) + } + return bytes.Equal(e.Bytes(), other.Bytes()) +} + +// Bytes returns the underlying raw bytes of the clustered resource. +func (e *EndpointsResourceData) Bytes() []byte { + raw := e.Raw() + if raw == nil { + return nil + } + return raw.Value +} + // EndpointsWatcher wraps the callbacks to be invoked for different // events corresponding to the endpoints resource being watched. gRFC A88 // contains an exhaustive list of what method is invoked under what conditions. @@ -128,8 +157,16 @@ type delegatingEndpointsWatcher struct { watcher EndpointsWatcher } -func (d *delegatingEndpointsWatcher) ResourceChanged(data ResourceData, onDone func()) { - e := data.(*EndpointsResourceData) +func (d *delegatingEndpointsWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { + if gData == nil { + d.watcher.ResourceError(fmt.Errorf("endpoints resource missing"), onDone) + return + } + e, ok := gData.(*EndpointsResourceData) + if !ok { + d.watcher.ResourceError(fmt.Errorf("delegatingEndpointsWatcher: unexpected resource data type %T", gData), onDone) + return + } d.watcher.ResourceChanged(e, onDone) } @@ -144,12 +181,21 @@ func (d *delegatingEndpointsWatcher) AmbientError(err error, onDone func()) { // WatchEndpoints uses xDS to discover the configuration associated with the // provided endpoints resource name. func WatchEndpoints(p Producer, name string, w EndpointsWatcher) (cancel func()) { - delegator := &delegatingEndpointsWatcher{watcher: w} - return p.WatchResource(endpointsType, name, delegator) + var gw xdsclient.ResourceWatcher + if w != nil { + gw = &delegatingEndpointsWatcher{watcher: w} + } + return p.WatchResource(EndpointsResource, name, gw) } -// NewGenericEndpointsResourceTypeDecoder returns a xdsclient.Decoder that -// wraps the xdsresource.endpointsType. -func NewGenericEndpointsResourceTypeDecoder() xdsclient.Decoder { - return &GenericResourceTypeDecoder{ResourceType: endpointsType} +// NewEndpointsResourceTypeDecoder returns a xdsclient.Decoder for endpoints. +// Endpoints is stateless so this returns the zero-value endpointsResourceType. +func NewEndpointsResourceTypeDecoder() xdsclient.Decoder { + return endpointsResourceType{ + resourceTypeState: resourceTypeState{ + typeURL: version.V3EndpointsURL, + typeName: EndpointsResourceTypeName, + allResourcesRequiredInSotW: false, + }, + } } diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index 100a06f97b67..f0b1221ba0f6 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -18,6 +18,7 @@ package xdsresource import ( + "bytes" "fmt" "google.golang.org/grpc/internal/pretty" @@ -36,15 +37,15 @@ const ( var ( // Compile time interface checks. - _ Type = listenerResourceType{} + _ xdsclient.Decoder = listenerResourceType{} + _ xdsclient.ResourceData = (*ListenerResourceData)(nil) // Singleton instantiation of the resource type implementation. - listenerType = listenerResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3ListenerURL, - typeName: ListenerResourceTypeName, - allResourcesRequiredInSotW: true, - }, + ListenerResource = xdsclient.ResourceType{ + TypeURL: version.V3ListenerURL, + TypeName: ListenerResourceTypeName, + AllResourcesRequiredInSotW: true, + Decoder: &listenerResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3ListenerURL, typeName: ListenerResourceTypeName, allResourcesRequiredInSotW: true}}, } ) @@ -54,6 +55,8 @@ var ( // Implements the Type interface. type listenerResourceType struct { resourceTypeState + BootstrapConfig *bootstrap.Config + ServerConfigMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig } func securityConfigValidator(bc *bootstrap.Config, sc *SecurityConfig) error { @@ -87,23 +90,47 @@ func listenerValidator(bc *bootstrap.Config, lis ListenerUpdate) error { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. -func (listenerResourceType) Decode(opts *DecodeOptions, resource *anypb.Any) (*DecodeResult, error) { - name, listener, err := unmarshalListenerResource(resource) +func (lt listenerResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { + // Build an anypb.Any from the generic AnyProto. + a := &anypb.Any{ + TypeUrl: resource.TypeURL, + Value: resource.Value, + } + + // Map generic decode options to internal options: + internalOpts := &DecodeOptions{BootstrapConfig: lt.BootstrapConfig} + if gOpts.ServerConfig != nil && lt.ServerConfigMap != nil { + if sc, ok := lt.ServerConfigMap[*gOpts.ServerConfig]; ok { + internalOpts.ServerConfig = sc + } + } + + // Unmarshal the resource from the Any proto. + name, listener, err := unmarshalListenerResource(a) switch { case name == "": // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: // Protobuf deserialization succeeded, but resource validation failed. - return &DecodeResult{Name: name, Resource: &ListenerResourceData{Resource: ListenerUpdate{}}}, err + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ListenerResourceData{Resource: ListenerUpdate{}}, + }, err } - // Perform extra validation here. - if err := listenerValidator(opts.BootstrapConfig, listener); err != nil { - return &DecodeResult{Name: name, Resource: &ListenerResourceData{Resource: ListenerUpdate{}}}, err + // Additional validation that uses bootstrap config (internalOpts.BootstrapConfig might be nil). + if err := listenerValidator(internalOpts.BootstrapConfig, listener); err != nil { + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ListenerResourceData{Resource: ListenerUpdate{}}, + }, err } - return &DecodeResult{Name: name, Resource: &ListenerResourceData{Resource: listener}}, nil + return &xdsclient.DecodeResult{ + Name: name, + Resource: &ListenerResourceData{Resource: listener}, + }, nil } // ListenerResourceData wraps the configuration of a Listener resource as @@ -139,6 +166,29 @@ func (l *ListenerResourceData) Raw() *anypb.Any { return l.Resource.Raw } +// Equal returns true if other is equal to c +func (l *ListenerResourceData) Equal(other xdsclient.ResourceData) bool { + if l == nil && other == nil { + return true + } + if (l == nil) != (other == nil) { + return false + } + if otherLRD, ok := other.(*ListenerResourceData); ok { + return l.RawEqual(otherLRD) + } + return bytes.Equal(l.Bytes(), other.Bytes()) +} + +// Bytes returns the underlying raw bytes of the clustered resource. +func (l *ListenerResourceData) Bytes() []byte { + raw := l.Raw() + if raw == nil { + return nil + } + return raw.Value +} + // ListenerWatcher wraps the callbacks to be invoked for different // events corresponding to the listener resource being watched. gRFC A88 // contains an exhaustive list of what method is invoked under what conditions. @@ -163,10 +213,19 @@ type delegatingListenerWatcher struct { watcher ListenerWatcher } -func (d *delegatingListenerWatcher) ResourceChanged(data ResourceData, onDone func()) { - l := data.(*ListenerResourceData) - d.watcher.ResourceChanged(l, onDone) +func (d *delegatingListenerWatcher) ResourceChanged(gData xdsclient.ResourceData, done func()) { + if gData == nil { + d.watcher.ResourceError(fmt.Errorf("listener resource missing"), done) + return + } + lrd, ok := gData.(*ListenerResourceData) + if !ok { + d.watcher.ResourceError(fmt.Errorf("delegatingListenerWatcher: unexpected resource data type %T", gData), done) + return + } + d.watcher.ResourceChanged(lrd, done) } + func (d *delegatingListenerWatcher) ResourceError(err error, onDone func()) { d.watcher.ResourceError(err, onDone) } @@ -178,12 +237,25 @@ func (d *delegatingListenerWatcher) AmbientError(err error, onDone func()) { // WatchListener uses xDS to discover the configuration associated with the // provided listener resource name. func WatchListener(p Producer, name string, w ListenerWatcher) (cancel func()) { - delegator := &delegatingListenerWatcher{watcher: w} - return p.WatchResource(listenerType, name, delegator) + var gw xdsclient.ResourceWatcher + if w != nil { + gw = &delegatingListenerWatcher{watcher: w} + } + // Use the exported generic ResourceType value for listener. + return p.WatchResource(ListenerResource, name, gw) } -// NewGenericListenerResourceTypeDecoder returns a xdsclient.Decoder that wraps -// the xdsresource.listenerType. -func NewGenericListenerResourceTypeDecoder(bc *bootstrap.Config) xdsclient.Decoder { - return &GenericResourceTypeDecoder{ResourceType: listenerType, BootstrapConfig: bc} +// NewListenerResourceTypeDecoder returns a decoder for RouteConfig resources. +// RouteConfig is stateless so this just returns the zero-valued +// routeConfigResourceType. +func NewListenerResourceTypeDecoder(bc *bootstrap.Config, serverConfigMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig) xdsclient.Decoder { + return &listenerResourceType{ + resourceTypeState: resourceTypeState{ + typeURL: version.V3ListenerURL, + typeName: ListenerResourceTypeName, + allResourcesRequiredInSotW: true, + }, + BootstrapConfig: bc, + ServerConfigMap: serverConfigMap, + } } diff --git a/internal/xds/xdsclient/xdsresource/resource_type.go b/internal/xds/xdsclient/xdsresource/resource_type.go index 2c591312f1be..14d7284e60e0 100644 --- a/internal/xds/xdsclient/xdsresource/resource_type.go +++ b/internal/xds/xdsclient/xdsresource/resource_type.go @@ -25,8 +25,6 @@ package xdsresource import ( - "fmt" - xdsinternal "google.golang.org/grpc/internal/xds" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/internal/xds/clients/xdsclient" @@ -36,10 +34,10 @@ import ( func init() { xdsinternal.ResourceTypeMapForTesting = make(map[string]any) - xdsinternal.ResourceTypeMapForTesting[version.V3ListenerURL] = listenerType - xdsinternal.ResourceTypeMapForTesting[version.V3RouteConfigURL] = routeConfigType - xdsinternal.ResourceTypeMapForTesting[version.V3ClusterURL] = clusterType - xdsinternal.ResourceTypeMapForTesting[version.V3EndpointsURL] = endpointsType + xdsinternal.ResourceTypeMapForTesting[version.V3ListenerURL] = ListenerResource + xdsinternal.ResourceTypeMapForTesting[version.V3RouteConfigURL] = RouteConfigResource + xdsinternal.ResourceTypeMapForTesting[version.V3ClusterURL] = ClusterResource + xdsinternal.ResourceTypeMapForTesting[version.V3EndpointsURL] = EndpointsResource } // Producer contains a single method to discover resource configuration from a @@ -52,7 +50,7 @@ type Producer interface { // xDS responses are are deserialized and validated, as received from the // xDS management server. Upon receipt of a response from the management // server, an appropriate callback on the watcher is invoked. - WatchResource(rType Type, resourceName string, watcher ResourceWatcher) (cancel func()) + WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) } // ResourceWatcher is notified of the resource updates and errors that are @@ -170,119 +168,3 @@ func (r resourceTypeState) TypeName() string { func (r resourceTypeState) AllResourcesRequiredInSotW() bool { return r.allResourcesRequiredInSotW } - -// GenericResourceTypeDecoder wraps an xdsresource.Type and implements -// xdsclient.Decoder. -// -// TODO: #8313 - Delete this once the internal xdsclient usages are updated -// to use the generic xdsclient.ResourceType interface directly. -type GenericResourceTypeDecoder struct { - ResourceType Type - BootstrapConfig *bootstrap.Config - ServerConfigMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig -} - -// Decode deserialize and validate resource bytes of an xDS resource received -// from the xDS management server. -func (gd *GenericResourceTypeDecoder) Decode(resource xdsclient.AnyProto, gOpts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { - rProto := &anypb.Any{ - TypeUrl: resource.TypeURL, - Value: resource.Value, - } - opts := &DecodeOptions{BootstrapConfig: gd.BootstrapConfig} - if gOpts.ServerConfig != nil { - opts.ServerConfig = gd.ServerConfigMap[*gOpts.ServerConfig] - } - - result, err := gd.ResourceType.Decode(opts, rProto) - if result == nil { - return nil, err - } - if err != nil { - return &xdsclient.DecodeResult{Name: result.Name}, err - } - - return &xdsclient.DecodeResult{Name: result.Name, Resource: &genericResourceData{resourceData: result.Resource}}, nil -} - -// genericResourceData embed an xdsresource.ResourceData and implements -// xdsclient.ResourceData. -// -// TODO: #8313 - Delete this once the internal xdsclient usages are updated -// to use the generic xdsclient.ResourceData interface directly. -type genericResourceData struct { - resourceData ResourceData -} - -// Equal returns true if the passed in xdsclient.ResourceData -// is equal to that of the receiver. -func (grd *genericResourceData) Equal(other xdsclient.ResourceData) bool { - if other == nil { - return false - } - otherResourceData, ok := other.(*genericResourceData) - if !ok { - return false - } - return grd.resourceData.RawEqual(otherResourceData.resourceData) -} - -// Bytes returns the underlying raw bytes of the wrapped resource. -func (grd *genericResourceData) Bytes() []byte { - rawAny := grd.resourceData.Raw() - if rawAny == nil { - return nil - } - return rawAny.Value -} - -// genericResourceWatcher wraps xdsresource.ResourceWatcher and implements -// xdsclient.ResourceWatcher. -// -// TODO: #8313 - Delete this once the internal xdsclient usages are updated -// to use the generic xdsclient.ResourceWatcher interface directly. -type genericResourceWatcher struct { - xdsResourceWatcher ResourceWatcher -} - -// ResourceChanged indicates a new version of the wrapped resource is -// available. -func (gw *genericResourceWatcher) ResourceChanged(gData xdsclient.ResourceData, done func()) { - if gData == nil { - gw.xdsResourceWatcher.ResourceChanged(nil, done) - return - } - - grd, ok := gData.(*genericResourceData) - if !ok { - err := fmt.Errorf("genericResourceWatcher received unexpected xdsclient.ResourceData type %T, want *genericResourceData", gData) - gw.xdsResourceWatcher.ResourceError(err, done) - return - } - gw.xdsResourceWatcher.ResourceChanged(grd.resourceData, done) -} - -// ResourceError indicates an error occurred while trying to fetch or -// decode the associated wrapped resource. The previous version of the -// wrapped resource should be considered invalid. -func (gw *genericResourceWatcher) ResourceError(err error, done func()) { - gw.xdsResourceWatcher.ResourceError(err, done) -} - -// AmbientError indicates an error occurred after a resource has been -// received that should not modify the use of that wrapped resource but may -// provide useful information about the state of the XDSClient for debugging -// purposes. The previous version of the wrapped resource should still be -// considered valid. -func (gw *genericResourceWatcher) AmbientError(err error, done func()) { - gw.xdsResourceWatcher.AmbientError(err, done) -} - -// GenericResourceWatcher returns a xdsclient.ResourceWatcher that wraps an -// xdsresource.ResourceWatcher to make it compatible with xdsclient.ResourceWatcher. -func GenericResourceWatcher(xdsResourceWatcher ResourceWatcher) xdsclient.ResourceWatcher { - if xdsResourceWatcher == nil { - return nil - } - return &genericResourceWatcher{xdsResourceWatcher: xdsResourceWatcher} -} diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index 912dc1b762b4..723890b3e7db 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -18,6 +18,9 @@ package xdsresource import ( + "bytes" + "fmt" + "google.golang.org/grpc/internal/pretty" xdsclient "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource/version" @@ -33,15 +36,15 @@ const ( var ( // Compile time interface checks. - _ Type = routeConfigResourceType{} + _ xdsclient.Decoder = routeConfigResourceType{} + _ xdsclient.ResourceData = (*RouteConfigResourceData)(nil) // Singleton instantiation of the resource type implementation. - routeConfigType = routeConfigResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3RouteConfigURL, - typeName: "RouteConfigResource", - allResourcesRequiredInSotW: false, - }, + RouteConfigResource = xdsclient.ResourceType{ + TypeURL: version.V3RouteConfigURL, + TypeName: RouteConfigTypeName, + AllResourcesRequiredInSotW: false, + Decoder: routeConfigResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3RouteConfigURL, typeName: RouteConfigTypeName, allResourcesRequiredInSotW: false}}, } ) @@ -55,19 +58,21 @@ type routeConfigResourceType struct { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. -func (routeConfigResourceType) Decode(_ *DecodeOptions, resource *anypb.Any) (*DecodeResult, error) { - name, rc, err := unmarshalRouteConfigResource(resource) +func (rt routeConfigResourceType) Decode(resource xdsclient.AnyProto, _ xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { + // Reconstruct anypb.Any from the generic AnyProto provided by the client. + a := &anypb.Any{ + TypeUrl: resource.TypeURL, + Value: resource.Value, + } + + name, rc, err := unmarshalRouteConfigResource(a) switch { case name == "": - // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: - // Protobuf deserialization succeeded, but resource validation failed. - return &DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: RouteConfigUpdate{}}}, err + return &xdsclient.DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: RouteConfigUpdate{}}}, err } - - return &DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: rc}}, nil - + return &xdsclient.DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: rc}}, nil } // RouteConfigResourceData wraps the configuration of a RouteConfiguration @@ -105,6 +110,29 @@ func (r *RouteConfigResourceData) Raw() *anypb.Any { return r.Resource.Raw } +// Equal returns true if other is equal to c +func (r *RouteConfigResourceData) Equal(other xdsclient.ResourceData) bool { + if r == nil && other == nil { + return true + } + if (r == nil) != (other == nil) { + return false + } + if otherRCRD, ok := other.(*RouteConfigResourceData); ok { + return r.RawEqual(otherRCRD) + } + return bytes.Equal(r.Bytes(), other.Bytes()) +} + +// Bytes returns the underlying raw bytes of the clustered resource. +func (r *RouteConfigResourceData) Bytes() []byte { + raw := r.Raw() + if raw == nil { + return nil + } + return raw.Value +} + // RouteConfigWatcher wraps the callbacks to be invoked for different // events corresponding to the route configuration resource being watched. gRFC // A88 contains an exhaustive list of what method is invoked under what @@ -130,8 +158,16 @@ type delegatingRouteConfigWatcher struct { watcher RouteConfigWatcher } -func (d *delegatingRouteConfigWatcher) ResourceChanged(data ResourceData, onDone func()) { - rc := data.(*RouteConfigResourceData) +func (d *delegatingRouteConfigWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { + if gData == nil { + d.watcher.ResourceError(fmt.Errorf("route config resource missing"), onDone) + return + } + rc, ok := gData.(*RouteConfigResourceData) + if !ok { + d.watcher.ResourceError(fmt.Errorf("delegatingRouteConfigWatcher: unexpected resource data type %T", gData), onDone) + return + } d.watcher.ResourceChanged(rc, onDone) } @@ -146,12 +182,22 @@ func (d *delegatingRouteConfigWatcher) AmbientError(err error, onDone func()) { // WatchRouteConfig uses xDS to discover the configuration associated with the // provided route configuration resource name. func WatchRouteConfig(p Producer, name string, w RouteConfigWatcher) (cancel func()) { - delegator := &delegatingRouteConfigWatcher{watcher: w} - return p.WatchResource(routeConfigType, name, delegator) + var gw xdsclient.ResourceWatcher + if w != nil { + gw = &delegatingRouteConfigWatcher{watcher: w} + } + return p.WatchResource(RouteConfigResource, name, gw) } -// NewGenericRouteConfigResourceTypeDecoder returns a xdsclient.Decoder that -// wraps the xdsresource.routeConfigType. -func NewGenericRouteConfigResourceTypeDecoder() xdsclient.Decoder { - return &GenericResourceTypeDecoder{ResourceType: routeConfigType} +// NewRouteConfigResourceTypeDecoder returns a decoder for RouteConfig resources. +// RouteConfig is stateless so this just returns the zero-valued +// routeConfigResourceType. +func NewRouteConfigResourceTypeDecoder() xdsclient.Decoder { + return routeConfigResourceType{ + resourceTypeState: resourceTypeState{ + typeURL: version.V3RouteConfigURL, + typeName: RouteConfigTypeName, + allResourcesRequiredInSotW: false, + }, + } } From 6bdcc58214c07b9773a09b661f7513b1bf3c18d7 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Fri, 12 Sep 2025 09:52:29 +0000 Subject: [PATCH 02/12] small tweaks --- internal/xds/xdsclient/xdsresource/cluster_resource_type.go | 3 ++- internal/xds/xdsclient/xdsresource/endpoints_resource_type.go | 3 ++- internal/xds/xdsclient/xdsresource/listener_resource_type.go | 3 ++- .../xds/xdsclient/xdsresource/route_config_resource_type.go | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index 4939184991ca..fe32f9ba47e1 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -40,7 +40,8 @@ var ( _ xdsclient.Decoder = clusterResourceType{} _ xdsclient.ResourceData = (*ClusterResourceData)(nil) - // Singleton instantiation of the resource type implementation. + // ClusterResource is a singleton instance of xdsclient.ResourceType + // that defines the configuration for the Listener resource. ClusterResource = xdsclient.ResourceType{ TypeURL: version.V3ClusterURL, TypeName: ClusterResourceTypeName, diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index 9029ac458282..a2ff9a583811 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -39,7 +39,8 @@ var ( _ xdsclient.Decoder = endpointsResourceType{} _ xdsclient.ResourceData = (*EndpointsResourceData)(nil) - // Exported generic ResourceType for endpoints. + // EndpointsResource is a singleton instance of xdsclient.ResourceType + // that defines the configuration for the Listener resource. EndpointsResource = xdsclient.ResourceType{ TypeURL: version.V3EndpointsURL, TypeName: EndpointsResourceTypeName, diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index f0b1221ba0f6..68ed774daefc 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -40,7 +40,8 @@ var ( _ xdsclient.Decoder = listenerResourceType{} _ xdsclient.ResourceData = (*ListenerResourceData)(nil) - // Singleton instantiation of the resource type implementation. + // ListenerResource is a singleton instance of xdsclient.ResourceType + // that defines the configuration for the Listener resource. ListenerResource = xdsclient.ResourceType{ TypeURL: version.V3ListenerURL, TypeName: ListenerResourceTypeName, diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index 723890b3e7db..9f7b4b322629 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -39,7 +39,8 @@ var ( _ xdsclient.Decoder = routeConfigResourceType{} _ xdsclient.ResourceData = (*RouteConfigResourceData)(nil) - // Singleton instantiation of the resource type implementation. + // RouteConfigResource is a singleton instance of xdsclient.ResourceType + // that defines the configuration for the Listener resource. RouteConfigResource = xdsclient.ResourceType{ TypeURL: version.V3RouteConfigURL, TypeName: RouteConfigTypeName, From 61762496414dd3340e0f203f1fc01b5669e5b850 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Fri, 12 Sep 2025 10:41:32 +0000 Subject: [PATCH 03/12] Fixed the client impl watcher --- internal/xds/xdsclient/clientimpl.go | 4 ++-- internal/xds/xdsclient/clientimpl_watchers.go | 2 +- internal/xds/xdsclient/pool.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/xds/xdsclient/clientimpl.go b/internal/xds/xdsclient/clientimpl.go index b1f797993fd7..4867603af00d 100644 --- a/internal/xds/xdsclient/clientimpl.go +++ b/internal/xds/xdsclient/clientimpl.go @@ -81,7 +81,7 @@ var ( // interface with ref counting so that it can be shared by the xds resolver and // balancer implementations, across multiple ClientConns and Servers. type clientImpl struct { - *xdsclient.XDSClient // TODO: #8313 - get rid of embedding, if possible. + genericClient *xdsclient.XDSClient // The following fields are initialized at creation time and are read-only // after that. @@ -137,7 +137,7 @@ func newClientImpl(config *bootstrap.Config, metricsRecorder estats.MetricsRecor return nil, err } c := &clientImpl{ - XDSClient: client, + genericClient: client, xdsClientConfig: gConfig, bootstrapConfig: config, target: target, diff --git a/internal/xds/xdsclient/clientimpl_watchers.go b/internal/xds/xdsclient/clientimpl_watchers.go index d77c08c0616a..f08a30052ed3 100644 --- a/internal/xds/xdsclient/clientimpl_watchers.go +++ b/internal/xds/xdsclient/clientimpl_watchers.go @@ -25,5 +25,5 @@ import "google.golang.org/grpc/internal/xds/clients/xdsclient" // server. Upon receipt of a response from the management server, an // appropriate callback on the watcher is invoked. func (c *clientImpl) WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) { - return c.XDSClient.WatchResource(rType.TypeURL, resourceName, watcher) + return c.genericClient.WatchResource(rType.TypeURL, resourceName, watcher) } diff --git a/internal/xds/xdsclient/pool.go b/internal/xds/xdsclient/pool.go index eb0197e09a7f..25c63fa08e19 100644 --- a/internal/xds/xdsclient/pool.go +++ b/internal/xds/xdsclient/pool.go @@ -172,7 +172,7 @@ func (p *Pool) DumpResources() *v3statuspb.ClientStatusResponse { resp := &v3statuspb.ClientStatusResponse{} for key, client := range p.clients { - b, err := client.DumpResources() + b, err := client.genericClient.DumpResources() if err != nil { return nil } @@ -243,7 +243,7 @@ func (p *Pool) clientRefCountedClose(name string) { // This attempts to close the transport to the management server and could // theoretically call back into the xdsclient package again and deadlock. // Hence, this needs to be called without holding the lock. - client.Close() + client.genericClient.Close() xdsClientImplCloseHook(name) } From ceee228184ef9b2c7cb21fe0462d2f9968d05cd2 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Fri, 12 Sep 2025 14:03:16 +0000 Subject: [PATCH 04/12] Fixed the Load report --- .../xds/balancer/clusterimpl/clusterimpl.go | 3 +- .../balancer/loadstore/load_store_wrapper.go | 8 ++--- internal/xds/testutils/fakeclient/client.go | 6 ++-- internal/xds/xdsclient/client.go | 29 ++++++++++++++++++- .../xds/xdsclient/clientimpl_loadreport.go | 3 +- 5 files changed, 37 insertions(+), 12 deletions(-) diff --git a/internal/xds/balancer/clusterimpl/clusterimpl.go b/internal/xds/balancer/clusterimpl/clusterimpl.go index ffe0a3db55a8..06cf091edce1 100644 --- a/internal/xds/balancer/clusterimpl/clusterimpl.go +++ b/internal/xds/balancer/clusterimpl/clusterimpl.go @@ -42,7 +42,6 @@ import ( "google.golang.org/grpc/internal/xds/balancer/loadstore" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/internal/xds/clients" - "google.golang.org/grpc/internal/xds/clients/lrsclient" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/resolver" "google.golang.org/grpc/serviceconfig" @@ -236,7 +235,7 @@ func (b *clusterImplBalancer) updateLoadStore(newConfig *LBConfig) error { } } if startNewLoadReport { - var loadStore *lrsclient.LoadStore + var loadStore xdsclient.LoadStore if b.xdsClient != nil { loadStore, b.cancelLoadReport = b.xdsClient.ReportLoad(b.lrsServer) } diff --git a/internal/xds/balancer/loadstore/load_store_wrapper.go b/internal/xds/balancer/loadstore/load_store_wrapper.go index 89d5ad8751a1..38395605240b 100644 --- a/internal/xds/balancer/loadstore/load_store_wrapper.go +++ b/internal/xds/balancer/loadstore/load_store_wrapper.go @@ -23,7 +23,7 @@ import ( "sync" "google.golang.org/grpc/internal/xds/clients" - "google.golang.org/grpc/internal/xds/clients/lrsclient" + "google.golang.org/grpc/internal/xds/xdsclient" ) // NewWrapper creates a Wrapper. @@ -54,8 +54,8 @@ type Wrapper struct { // store and perCluster are initialized as nil. They are only set by the // balancer when LRS is enabled. Before that, all functions to record loads // are no-op. - store *lrsclient.LoadStore - perCluster *lrsclient.PerClusterReporter + store xdsclient.LoadStore + perCluster xdsclient.PerClusterReporter } // UpdateClusterAndService updates the cluster name and eds service for this @@ -77,7 +77,7 @@ func (lsw *Wrapper) UpdateClusterAndService(cluster, edsService string) { // UpdateLoadStore updates the load store for this wrapper. If it is changed // from before, the perCluster store in this wrapper will also be updated. -func (lsw *Wrapper) UpdateLoadStore(store *lrsclient.LoadStore) { +func (lsw *Wrapper) UpdateLoadStore(store xdsclient.LoadStore) { lsw.mu.Lock() defer lsw.mu.Unlock() if store == lsw.store { diff --git a/internal/xds/testutils/fakeclient/client.go b/internal/xds/testutils/fakeclient/client.go index fa465139ee94..21021c79b5df 100644 --- a/internal/xds/testutils/fakeclient/client.go +++ b/internal/xds/testutils/fakeclient/client.go @@ -40,7 +40,7 @@ type Client struct { name string loadReportCh *testutils.Channel lrsCancelCh *testutils.Channel - loadStore *lrsclient.LoadStore + loadStore xdsclient.LoadStore bootstrapCfg *bootstrap.Config } @@ -80,7 +80,7 @@ func (*stream) Recv() ([]byte, error) { } // ReportLoad starts reporting load about clusterName to server. -func (xdsC *Client) ReportLoad(server *bootstrap.ServerConfig) (loadStore *lrsclient.LoadStore, cancel func(context.Context)) { +func (xdsC *Client) ReportLoad(server *bootstrap.ServerConfig) (loadStore xdsclient.LoadStore, cancel func(context.Context)) { lrsClient, _ := lrsclient.New(lrsclient.Config{Node: clients.Node{ID: "fake-node-id"}, TransportBuilder: &transportBuilder{}}) xdsC.loadStore, _ = lrsClient.ReportLoad(clients.ServerIdentifier{ServerURI: server.ServerURI()}) @@ -100,7 +100,7 @@ func (xdsC *Client) WaitForCancelReportLoad(ctx context.Context) error { } // LoadStore returns the underlying load data store. -func (xdsC *Client) LoadStore() *lrsclient.LoadStore { +func (xdsC *Client) LoadStore() xdsclient.LoadStore { return xdsC.loadStore } diff --git a/internal/xds/xdsclient/client.go b/internal/xds/xdsclient/client.go index f8e223176558..b7a44f0de2f7 100644 --- a/internal/xds/xdsclient/client.go +++ b/internal/xds/xdsclient/client.go @@ -25,6 +25,7 @@ import ( v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3" "google.golang.org/grpc/internal/xds/bootstrap" + "google.golang.org/grpc/internal/xds/clients" "google.golang.org/grpc/internal/xds/clients/lrsclient" "google.golang.org/grpc/internal/xds/clients/xdsclient" ) @@ -49,11 +50,37 @@ type XDSClient interface { // the watcher is canceled. Callers need to handle this case. WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) - ReportLoad(*bootstrap.ServerConfig) (*lrsclient.LoadStore, func(context.Context)) + ReportLoad(*bootstrap.ServerConfig) (LoadStore, func(context.Context)) BootstrapConfig() *bootstrap.Config } +// PerClusterReporter is the minimal interface callers use to record per-cluster +// load and drops. It mirrors the PerClusterReporter methods from the concrete +// lrsclient.PerClusterReporter but is intentionally tiny. +type PerClusterReporter interface { + // CallStarted records that a call has started for the given locality. + CallStarted(locality clients.Locality) + + // CallFinished records that a call has finished for the given locality. + // Pass the error (nil if success) so implementations can update success/failure counters. + CallFinished(locality clients.Locality, err error) + + // CallServerLoad reports a numeric load metric for the given locality and cluster. + // (If your code uses a different name/parameters for this, match the concrete type.) + CallServerLoad(locality clients.Locality, clusterName string, value float64) + + // CallDropped records a dropped request of the given category. + CallDropped(category string) +} + +// LoadStore is the minimal interface returned by ReportLoad. It provides a +// way to get per-cluster reporters and to stop the store. +type LoadStore interface { + ReporterForCluster(clusterName, serviceName string) *lrsclient.PerClusterReporter + Stop(ctx context.Context) +} + // DumpResources returns the status and contents of all xDS resources. It uses // xDS clients from the default pool. func DumpResources() *v3statuspb.ClientStatusResponse { diff --git a/internal/xds/xdsclient/clientimpl_loadreport.go b/internal/xds/xdsclient/clientimpl_loadreport.go index ffd0c90b8f54..b8410c605ffa 100644 --- a/internal/xds/xdsclient/clientimpl_loadreport.go +++ b/internal/xds/xdsclient/clientimpl_loadreport.go @@ -24,14 +24,13 @@ import ( "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/internal/xds/clients" "google.golang.org/grpc/internal/xds/clients/grpctransport" - "google.golang.org/grpc/internal/xds/clients/lrsclient" ) // ReportLoad starts a load reporting stream to the given server. All load // reports to the same server share the LRS stream. // // It returns a lrsclient.LoadStore for the user to report loads. -func (c *clientImpl) ReportLoad(server *bootstrap.ServerConfig) (*lrsclient.LoadStore, func(context.Context)) { +func (c *clientImpl) ReportLoad(server *bootstrap.ServerConfig) (LoadStore, func(context.Context)) { load, err := c.lrsClient.ReportLoad(clients.ServerIdentifier{ ServerURI: server.ServerURI(), Extensions: grpctransport.ServerIdentifierExtension{ From ae544d4444619d04e7e7b6f2019a3d190f2f5076 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Sun, 14 Sep 2025 00:51:48 +0000 Subject: [PATCH 05/12] Fixed the xds client issues --- internal/xds/xdsclient/clientimpl_test.go | 16 ++++++++-------- internal/xds/xdsclient/resource_types.go | 4 ++-- .../xdsresource/cluster_resource_type.go | 1 - .../xdsresource/endpoints_resource_type.go | 12 ------------ .../xdsresource/listener_resource_type.go | 1 - .../xdsresource/route_config_resource_type.go | 13 ------------- 6 files changed, 10 insertions(+), 37 deletions(-) diff --git a/internal/xds/xdsclient/clientimpl_test.go b/internal/xds/xdsclient/clientimpl_test.go index acc2b27dbe59..60ac4c425a77 100644 --- a/internal/xds/xdsclient/clientimpl_test.go +++ b/internal/xds/xdsclient/clientimpl_test.go @@ -82,9 +82,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -120,9 +120,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{"auth1": {XDSServers: []xdsclient.ServerConfig{expTopLevelS}}, "auth2": {XDSServers: []xdsclient.ServerConfig{expAuth2S}}}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gSCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gSCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -152,9 +152,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -184,9 +184,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ diff --git a/internal/xds/xdsclient/resource_types.go b/internal/xds/xdsclient/resource_types.go index d007666da182..a3dfe9916d9f 100644 --- a/internal/xds/xdsclient/resource_types.go +++ b/internal/xds/xdsclient/resource_types.go @@ -36,7 +36,7 @@ func supportedResourceTypes(config *bootstrap.Config, gServerCfgMap map[xdsclien TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.NewRouteConfigResourceTypeDecoder(), + Decoder: xdsresource.RouteConfigResource.Decoder, }, version.V3ClusterURL: { TypeURL: version.V3ClusterURL, @@ -48,7 +48,7 @@ func supportedResourceTypes(config *bootstrap.Config, gServerCfgMap map[xdsclien TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.NewEndpointsResourceTypeDecoder(), + Decoder: xdsresource.EndpointsResource.Decoder, }, } } diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index fe32f9ba47e1..573a41569134 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -46,7 +46,6 @@ var ( TypeURL: version.V3ClusterURL, TypeName: ClusterResourceTypeName, AllResourcesRequiredInSotW: true, - Decoder: &clusterResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3ClusterURL, typeName: ClusterResourceTypeName, allResourcesRequiredInSotW: true}}, } ) diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index a2ff9a583811..fe9be05f7075 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -188,15 +188,3 @@ func WatchEndpoints(p Producer, name string, w EndpointsWatcher) (cancel func()) } return p.WatchResource(EndpointsResource, name, gw) } - -// NewEndpointsResourceTypeDecoder returns a xdsclient.Decoder for endpoints. -// Endpoints is stateless so this returns the zero-value endpointsResourceType. -func NewEndpointsResourceTypeDecoder() xdsclient.Decoder { - return endpointsResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3EndpointsURL, - typeName: EndpointsResourceTypeName, - allResourcesRequiredInSotW: false, - }, - } -} diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index 68ed774daefc..4c02d9539e6b 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -46,7 +46,6 @@ var ( TypeURL: version.V3ListenerURL, TypeName: ListenerResourceTypeName, AllResourcesRequiredInSotW: true, - Decoder: &listenerResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3ListenerURL, typeName: ListenerResourceTypeName, allResourcesRequiredInSotW: true}}, } ) diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index 9f7b4b322629..bf430b89109e 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -189,16 +189,3 @@ func WatchRouteConfig(p Producer, name string, w RouteConfigWatcher) (cancel fun } return p.WatchResource(RouteConfigResource, name, gw) } - -// NewRouteConfigResourceTypeDecoder returns a decoder for RouteConfig resources. -// RouteConfig is stateless so this just returns the zero-valued -// routeConfigResourceType. -func NewRouteConfigResourceTypeDecoder() xdsclient.Decoder { - return routeConfigResourceType{ - resourceTypeState: resourceTypeState{ - typeURL: version.V3RouteConfigURL, - typeName: RouteConfigTypeName, - allResourcesRequiredInSotW: false, - }, - } -} From e77e35694e8c9533d691e81d087dcecd1ec0fe8b Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 15 Sep 2025 04:58:30 +0000 Subject: [PATCH 06/12] Fixed the xdsclient issues --- internal/xds/xdsclient/clientimpl.go | 4 ++-- internal/xds/xdsclient/clientimpl_watchers.go | 2 +- internal/xds/xdsclient/pool.go | 4 ++-- internal/xds/xdsclient/xdsresource/resource_type.go | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/xds/xdsclient/clientimpl.go b/internal/xds/xdsclient/clientimpl.go index 4867603af00d..bb67bc92f1c1 100644 --- a/internal/xds/xdsclient/clientimpl.go +++ b/internal/xds/xdsclient/clientimpl.go @@ -81,7 +81,7 @@ var ( // interface with ref counting so that it can be shared by the xds resolver and // balancer implementations, across multiple ClientConns and Servers. type clientImpl struct { - genericClient *xdsclient.XDSClient + xdsClient *xdsclient.XDSClient // The following fields are initialized at creation time and are read-only // after that. @@ -137,7 +137,7 @@ func newClientImpl(config *bootstrap.Config, metricsRecorder estats.MetricsRecor return nil, err } c := &clientImpl{ - genericClient: client, + xdsClient: client, xdsClientConfig: gConfig, bootstrapConfig: config, target: target, diff --git a/internal/xds/xdsclient/clientimpl_watchers.go b/internal/xds/xdsclient/clientimpl_watchers.go index f08a30052ed3..37a8d545b842 100644 --- a/internal/xds/xdsclient/clientimpl_watchers.go +++ b/internal/xds/xdsclient/clientimpl_watchers.go @@ -25,5 +25,5 @@ import "google.golang.org/grpc/internal/xds/clients/xdsclient" // server. Upon receipt of a response from the management server, an // appropriate callback on the watcher is invoked. func (c *clientImpl) WatchResource(rType xdsclient.ResourceType, resourceName string, watcher xdsclient.ResourceWatcher) (cancel func()) { - return c.genericClient.WatchResource(rType.TypeURL, resourceName, watcher) + return c.xdsClient.WatchResource(rType.TypeURL, resourceName, watcher) } diff --git a/internal/xds/xdsclient/pool.go b/internal/xds/xdsclient/pool.go index 25c63fa08e19..f3376951115c 100644 --- a/internal/xds/xdsclient/pool.go +++ b/internal/xds/xdsclient/pool.go @@ -172,7 +172,7 @@ func (p *Pool) DumpResources() *v3statuspb.ClientStatusResponse { resp := &v3statuspb.ClientStatusResponse{} for key, client := range p.clients { - b, err := client.genericClient.DumpResources() + b, err := client.xdsClient.DumpResources() if err != nil { return nil } @@ -243,7 +243,7 @@ func (p *Pool) clientRefCountedClose(name string) { // This attempts to close the transport to the management server and could // theoretically call back into the xdsclient package again and deadlock. // Hence, this needs to be called without holding the lock. - client.genericClient.Close() + client.xdsClient.Close() xdsClientImplCloseHook(name) } diff --git a/internal/xds/xdsclient/xdsresource/resource_type.go b/internal/xds/xdsclient/xdsresource/resource_type.go index 14d7284e60e0..771096ee7e09 100644 --- a/internal/xds/xdsclient/xdsresource/resource_type.go +++ b/internal/xds/xdsclient/xdsresource/resource_type.go @@ -84,7 +84,7 @@ type ResourceWatcher interface { // Type wraps all resource-type specific functionality. Each supported resource // type will provide an implementation of this interface. -type Type interface { +type ResourceType interface { // TypeURL is the xDS type URL of this resource type for v3 transport. TypeURL() string @@ -109,7 +109,7 @@ type Type interface { // If protobuf deserialization fails or resource validation fails, // returns a non-nil error. Otherwise, returns a fully populated // DecodeResult. - Decode(*DecodeOptions, *anypb.Any) (*DecodeResult, error) + Decode(*xdsclient.AnyProto, *xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) } // ResourceData contains the configuration data sent by the xDS management From 15b88ffa47e3660a42549e33944838a439437e28 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 15 Sep 2025 07:07:27 +0000 Subject: [PATCH 07/12] small tweaks --- .../xdsclient/xdsresource/cluster_resource_type.go | 3 ++- .../xdsclient/xdsresource/endpoints_resource_type.go | 4 ++-- .../xdsclient/xdsresource/listener_resource_type.go | 5 +---- internal/xds/xdsclient/xdsresource/resource_type.go | 12 +++--------- .../xdsresource/route_config_resource_type.go | 5 +++-- 5 files changed, 11 insertions(+), 18 deletions(-) diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index 573a41569134..4d32e8f8b246 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -79,9 +79,10 @@ func (ct clusterResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclien name, cluster, err := unmarshalClusterResource(a, internalOpts.ServerConfig) switch { case name == "": - // Name unset => protobuf deserialization failure. + // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: + // Protobuf deserialization succeeded, but resource validation failed. return &xdsclient.DecodeResult{ Name: name, Resource: &ClusterResourceData{Resource: ClusterUpdate{}}, diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index fe9be05f7075..ea574192f6c5 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -60,18 +60,18 @@ type endpointsResourceType struct { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. func (et endpointsResourceType) Decode(resource xdsclient.AnyProto, _ xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { - // Build anypb.Any from the generic AnyProto passed by the generic client. a := &anypb.Any{ TypeUrl: resource.TypeURL, Value: resource.Value, } - // Unmarshal using the helper (it expects an *anypb.Any) name, rc, err := unmarshalEndpointsResource(a) switch { case name == "": + // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: + // Protobuf deserialization succeeded, but resource validation failed. return &xdsclient.DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: EndpointsUpdate{}}}, err } return &xdsclient.DecodeResult{Name: name, Resource: &EndpointsResourceData{Resource: rc}}, nil diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index 4c02d9539e6b..c4b158302fd2 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -91,7 +91,6 @@ func listenerValidator(bc *bootstrap.Config, lis ListenerUpdate) error { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. func (lt listenerResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { - // Build an anypb.Any from the generic AnyProto. a := &anypb.Any{ TypeUrl: resource.TypeURL, Value: resource.Value, @@ -104,8 +103,6 @@ func (lt listenerResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclie internalOpts.ServerConfig = sc } } - - // Unmarshal the resource from the Any proto. name, listener, err := unmarshalListenerResource(a) switch { case name == "": @@ -119,7 +116,7 @@ func (lt listenerResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclie }, err } - // Additional validation that uses bootstrap config (internalOpts.BootstrapConfig might be nil). + // Perform extra validation here. if err := listenerValidator(internalOpts.BootstrapConfig, listener); err != nil { return &xdsclient.DecodeResult{ Name: name, diff --git a/internal/xds/xdsclient/xdsresource/resource_type.go b/internal/xds/xdsclient/xdsresource/resource_type.go index 771096ee7e09..daeb8a4218ed 100644 --- a/internal/xds/xdsclient/xdsresource/resource_type.go +++ b/internal/xds/xdsclient/xdsresource/resource_type.go @@ -79,11 +79,8 @@ type ResourceWatcher interface { AmbientError(err error, done func()) } -// TODO: Once the implementation is complete, rename this interface as -// ResourceType and get rid of the existing ResourceType enum. - -// Type wraps all resource-type specific functionality. Each supported resource -// type will provide an implementation of this interface. +// ResourceType wraps all resource-type specific functionality. Each supported +// resource type will provide an implementation of this interface. type ResourceType interface { // TypeURL is the xDS type URL of this resource type for v3 transport. TypeURL() string @@ -92,10 +89,7 @@ type ResourceType interface { // can be used for logging/debugging purposes, as well in cases where the // resource type name is to be uniquely identified but the actual // functionality provided by the resource type is not required. - // - // TODO: once Type is renamed to ResourceType, rename TypeName to - // ResourceTypeName. - TypeName() string + ResourceTypeName() string // AllResourcesRequiredInSotW indicates whether this resource type requires // that all resources be present in every SotW response from the server. If diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index bf430b89109e..ea08b2d85899 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -60,7 +60,6 @@ type routeConfigResourceType struct { // Decode deserializes and validates an xDS resource serialized inside the // provided `Any` proto, as received from the xDS management server. func (rt routeConfigResourceType) Decode(resource xdsclient.AnyProto, _ xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) { - // Reconstruct anypb.Any from the generic AnyProto provided by the client. a := &anypb.Any{ TypeUrl: resource.TypeURL, Value: resource.Value, @@ -69,8 +68,10 @@ func (rt routeConfigResourceType) Decode(resource xdsclient.AnyProto, _ xdsclien name, rc, err := unmarshalRouteConfigResource(a) switch { case name == "": + // Name is unset only when protobuf deserialization fails. return nil, err case err != nil: + // Protobuf deserialization succeeded, but resource validation failed. return &xdsclient.DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: RouteConfigUpdate{}}}, err } return &xdsclient.DecodeResult{Name: name, Resource: &RouteConfigResourceData{Resource: rc}}, nil @@ -111,7 +112,7 @@ func (r *RouteConfigResourceData) Raw() *anypb.Any { return r.Resource.Raw } -// Equal returns true if other is equal to c +// Equal returns true if other is equal to r func (r *RouteConfigResourceData) Equal(other xdsclient.ResourceData) bool { if r == nil && other == nil { return true From f8ee66deaf28b81da63dc136746703e24bca6631 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Mon, 15 Sep 2025 07:43:35 +0000 Subject: [PATCH 08/12] small tweaks --- internal/xds/xdsclient/clientimpl_test.go | 16 ++++++++-------- internal/xds/xdsclient/resource_types.go | 4 ++-- .../xdsresource/endpoints_resource_type.go | 10 +++++++++- .../xds/xdsclient/xdsresource/resource_type.go | 2 +- .../xdsresource/route_config_resource_type.go | 10 +++++++++- 5 files changed, 29 insertions(+), 13 deletions(-) diff --git a/internal/xds/xdsclient/clientimpl_test.go b/internal/xds/xdsclient/clientimpl_test.go index 60ac4c425a77..acc2b27dbe59 100644 --- a/internal/xds/xdsclient/clientimpl_test.go +++ b/internal/xds/xdsclient/clientimpl_test.go @@ -82,9 +82,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -120,9 +120,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{"auth1": {XDSServers: []xdsclient.ServerConfig{expTopLevelS}}, "auth2": {XDSServers: []xdsclient.ServerConfig{expAuth2S}}}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gSCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gSCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -152,9 +152,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ @@ -184,9 +184,9 @@ func (s) TestBuildXDSClientConfig_Success(t *testing.T) { Authorities: map[string]xdsclient.Authority{}, ResourceTypes: map[string]xdsclient.ResourceType{ version.V3ListenerURL: {TypeURL: version.V3ListenerURL, TypeName: xdsresource.ListenerResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewListenerResourceTypeDecoder(c, gServerCfgMap)}, - version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.RouteConfigResource.Decoder}, + version.V3RouteConfigURL: {TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewRouteConfigResourceTypeDecoder()}, version.V3ClusterURL: {TypeURL: version.V3ClusterURL, TypeName: xdsresource.ClusterResourceTypeName, AllResourcesRequiredInSotW: true, Decoder: xdsresource.NewClusterResourceTypeDecoder(c, gServerCfgMap)}, - version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.EndpointsResource.Decoder}, + version.V3EndpointsURL: {TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, Decoder: xdsresource.NewEndpointsResourceTypeDecoder()}, }, MetricsReporter: &metricsReporter{recorder: stats.NewTestMetricsRecorder(), target: testTargetName}, TransportBuilder: grpctransport.NewBuilder(map[string]grpctransport.Config{ diff --git a/internal/xds/xdsclient/resource_types.go b/internal/xds/xdsclient/resource_types.go index a3dfe9916d9f..d007666da182 100644 --- a/internal/xds/xdsclient/resource_types.go +++ b/internal/xds/xdsclient/resource_types.go @@ -36,7 +36,7 @@ func supportedResourceTypes(config *bootstrap.Config, gServerCfgMap map[xdsclien TypeURL: version.V3RouteConfigURL, TypeName: xdsresource.RouteConfigTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.RouteConfigResource.Decoder, + Decoder: xdsresource.NewRouteConfigResourceTypeDecoder(), }, version.V3ClusterURL: { TypeURL: version.V3ClusterURL, @@ -48,7 +48,7 @@ func supportedResourceTypes(config *bootstrap.Config, gServerCfgMap map[xdsclien TypeURL: version.V3EndpointsURL, TypeName: xdsresource.EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, - Decoder: xdsresource.EndpointsResource.Decoder, + Decoder: xdsresource.NewEndpointsResourceTypeDecoder(), }, } } diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index ea574192f6c5..da730fbf1e10 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -45,7 +45,6 @@ var ( TypeURL: version.V3EndpointsURL, TypeName: EndpointsResourceTypeName, AllResourcesRequiredInSotW: false, - Decoder: endpointsResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3EndpointsURL, typeName: EndpointsResourceTypeName, allResourcesRequiredInSotW: false}}, } ) @@ -188,3 +187,12 @@ func WatchEndpoints(p Producer, name string, w EndpointsWatcher) (cancel func()) } return p.WatchResource(EndpointsResource, name, gw) } + +// NewEndpointsResourceTypeDecoder returns a decoder for Endpoint resources. +func NewEndpointsResourceTypeDecoder() xdsclient.Decoder { + return endpointsResourceType{resourceTypeState: resourceTypeState{ + typeURL: version.V3EndpointsURL, + typeName: EndpointsResourceTypeName, + allResourcesRequiredInSotW: false, + }} +} diff --git a/internal/xds/xdsclient/xdsresource/resource_type.go b/internal/xds/xdsclient/xdsresource/resource_type.go index daeb8a4218ed..ca48ae9bad0b 100644 --- a/internal/xds/xdsclient/xdsresource/resource_type.go +++ b/internal/xds/xdsclient/xdsresource/resource_type.go @@ -103,7 +103,7 @@ type ResourceType interface { // If protobuf deserialization fails or resource validation fails, // returns a non-nil error. Otherwise, returns a fully populated // DecodeResult. - Decode(*xdsclient.AnyProto, *xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) + Decode(*xdsclient.AnyProto, xdsclient.DecodeOptions) (*xdsclient.DecodeResult, error) } // ResourceData contains the configuration data sent by the xDS management diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index ea08b2d85899..9e4e7ec744a5 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -45,7 +45,6 @@ var ( TypeURL: version.V3RouteConfigURL, TypeName: RouteConfigTypeName, AllResourcesRequiredInSotW: false, - Decoder: routeConfigResourceType{resourceTypeState: resourceTypeState{typeURL: version.V3RouteConfigURL, typeName: RouteConfigTypeName, allResourcesRequiredInSotW: false}}, } ) @@ -190,3 +189,12 @@ func WatchRouteConfig(p Producer, name string, w RouteConfigWatcher) (cancel fun } return p.WatchResource(RouteConfigResource, name, gw) } + +// NewRouteConfigResourceTypeDecoder returns a decoder for RouteConfig resources. +func NewRouteConfigResourceTypeDecoder() xdsclient.Decoder { + return routeConfigResourceType{resourceTypeState: resourceTypeState{ + typeURL: version.V3RouteConfigURL, + typeName: RouteConfigTypeName, + allResourcesRequiredInSotW: false, + }} +} From 4f4f99d865dd023db7314293db27cffd4071b810 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Tue, 16 Sep 2025 06:21:23 +0000 Subject: [PATCH 09/12] Fixe the generic client issues --- .../balancer/cdsbalancer/cluster_watcher.go | 11 +++- .../clusterresolver/resource_resolver_eds.go | 11 +++- internal/xds/resolver/watch_service.go | 27 +++++++-- internal/xds/server/listener_wrapper.go | 14 +++-- internal/xds/server/rds_handler.go | 12 +++- internal/xds/xdsclient/metrics_test.go | 3 +- .../xds/xdsclient/tests/authority_test.go | 18 +++++- .../xds/xdsclient/tests/cds_watchers_test.go | 18 +++++- .../xds/xdsclient/tests/eds_watchers_test.go | 14 +++-- .../xds/xdsclient/tests/lds_watchers_test.go | 33 +++++++++-- .../xds/xdsclient/tests/rds_watchers_test.go | 18 +++++- .../xdsresource/cluster_resource_type.go | 54 +----------------- .../xdsresource/endpoints_resource_type.go | 54 +----------------- .../xdsresource/listener_resource_type.go | 54 +----------------- .../xdsresource/route_config_resource_type.go | 55 +------------------ xds/csds/csds_e2e_test.go | 11 ++-- 16 files changed, 157 insertions(+), 250 deletions(-) diff --git a/internal/xds/balancer/cdsbalancer/cluster_watcher.go b/internal/xds/balancer/cdsbalancer/cluster_watcher.go index dd702b125b32..9f4a387e0957 100644 --- a/internal/xds/balancer/cdsbalancer/cluster_watcher.go +++ b/internal/xds/balancer/cdsbalancer/cluster_watcher.go @@ -18,7 +18,9 @@ package cdsbalancer import ( "context" + "fmt" + "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" ) @@ -32,8 +34,13 @@ type clusterWatcher struct { parent *cdsBalancer } -func (cw *clusterWatcher) ResourceChanged(u *xdsresource.ClusterResourceData, onDone func()) { - handleUpdate := func(context.Context) { cw.parent.onClusterUpdate(cw.name, u.Resource); onDone() } +func (cw *clusterWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { + clusterData, ok := rd.(*xdsresource.ClusterResourceData) + if !ok { + cw.ResourceError(fmt.Errorf("clusterWatcher: unexpected resource data type %T", rd), onDone) + return + } + handleUpdate := func(context.Context) { cw.parent.onClusterUpdate(cw.name, clusterData.Resource); onDone() } cw.parent.serializer.ScheduleOr(handleUpdate, onDone) } diff --git a/internal/xds/balancer/clusterresolver/resource_resolver_eds.go b/internal/xds/balancer/clusterresolver/resource_resolver_eds.go index 18b517f111d9..39ec6e4bd586 100644 --- a/internal/xds/balancer/clusterresolver/resource_resolver_eds.go +++ b/internal/xds/balancer/clusterresolver/resource_resolver_eds.go @@ -23,6 +23,7 @@ import ( "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpcsync" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" ) @@ -76,9 +77,14 @@ func newEDSResolver(nameToWatch string, producer xdsresource.Producer, topLevelR } // ResourceChanged is invoked to report an update for the resource being watched. -func (er *edsDiscoveryMechanism) ResourceChanged(update *xdsresource.EndpointsResourceData, onDone func()) { +func (er *edsDiscoveryMechanism) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { + defer onDone() if er.stopped.HasFired() { - onDone() + return + } + update, ok := rd.(*xdsresource.EndpointsResourceData) + if !ok { + er.logger.Warningf("EDS discovery mechanism received unexpected resource data type %T", rd) return } @@ -88,7 +94,6 @@ func (er *edsDiscoveryMechanism) ResourceChanged(update *xdsresource.EndpointsRe er.topLevelResolver.onUpdate(onDone) } - func (er *edsDiscoveryMechanism) ResourceError(err error, onDone func()) { if er.stopped.HasFired() { onDone() diff --git a/internal/xds/resolver/watch_service.go b/internal/xds/resolver/watch_service.go index 44b885c44055..67426c7dc559 100644 --- a/internal/xds/resolver/watch_service.go +++ b/internal/xds/resolver/watch_service.go @@ -21,6 +21,7 @@ package resolver import ( "context" + "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" ) @@ -36,8 +37,20 @@ func newListenerWatcher(resourceName string, parent *xdsResolver) *listenerWatch return lw } -func (l *listenerWatcher) ResourceChanged(update *xdsresource.ListenerResourceData, onDone func()) { - handleUpdate := func(context.Context) { l.parent.onListenerResourceUpdate(update.Resource); onDone() } +func (l *listenerWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { + if rd == nil { + l.parent.onListenerResourceUpdate(xdsresource.ListenerUpdate{}) + onDone() + return + } + listenerData, ok := rd.(*xdsresource.ListenerResourceData) + if !ok { + l.parent.onListenerResourceUpdate(xdsresource.ListenerUpdate{}) + onDone() + return + } + + handleUpdate := func(context.Context) { l.parent.onListenerResourceUpdate(listenerData.Resource); onDone() } l.parent.serializer.ScheduleOr(handleUpdate, onDone) } @@ -68,9 +81,15 @@ func newRouteConfigWatcher(resourceName string, parent *xdsResolver) *routeConfi return rw } -func (r *routeConfigWatcher) ResourceChanged(u *xdsresource.RouteConfigResourceData, onDone func()) { +func (r *routeConfigWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { + rcData, ok := rd.(*xdsresource.RouteConfigResourceData) + if !ok { + onDone() + return + } + handleUpdate := func(context.Context) { - r.parent.onRouteConfigResourceUpdate(r.resourceName, u.Resource) + r.parent.onRouteConfigResourceUpdate(r.resourceName, rcData.Resource) onDone() } r.parent.serializer.ScheduleOr(handleUpdate, onDone) diff --git a/internal/xds/server/listener_wrapper.go b/internal/xds/server/listener_wrapper.go index fab8419f0462..a2022b85d27a 100644 --- a/internal/xds/server/listener_wrapper.go +++ b/internal/xds/server/listener_wrapper.go @@ -387,17 +387,23 @@ type ldsWatcher struct { name string } -func (lw *ldsWatcher) ResourceChanged(update *xdsresource.ListenerResourceData, onDone func()) { +func (lw *ldsWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { defer onDone() if lw.parent.closed.HasFired() { - lw.logger.Warningf("Resource %q received update: %#v after listener was closed", lw.name, update) + lw.logger.Warningf("Resource %q received update after listener was closed", lw.name) + return + } + listenerData, ok := rd.(*xdsresource.ListenerResourceData) + if !ok { + lw.logger.Warningf("LDS watcher received unexpected resource data type %T", rd) return } if lw.logger.V(2) { - lw.logger.Infof("LDS watch for resource %q received update: %#v", lw.name, update.Resource) + lw.logger.Infof("LDS watch for resource %q received update: %#v", lw.name, listenerData.Resource) } l := lw.parent - ilc := update.Resource.InboundListenerCfg + ilc := listenerData.Resource.InboundListenerCfg + // Make sure that the socket address on the received Listener resource // matches the address of the net.Listener passed to us by the user. This // check is done here instead of at the XDSClient layer because of the diff --git a/internal/xds/server/rds_handler.go b/internal/xds/server/rds_handler.go index bf78c37c8292..61ac03a5d638 100644 --- a/internal/xds/server/rds_handler.go +++ b/internal/xds/server/rds_handler.go @@ -22,6 +22,7 @@ import ( "sync" igrpclog "google.golang.org/grpc/internal/grpclog" + "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" ) @@ -135,7 +136,7 @@ type rdsWatcher struct { canceled bool // eats callbacks if true } -func (rw *rdsWatcher) ResourceChanged(update *xdsresource.RouteConfigResourceData, onDone func()) { +func (rw *rdsWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { defer onDone() rw.mu.Lock() if rw.canceled { @@ -143,12 +144,17 @@ func (rw *rdsWatcher) ResourceChanged(update *xdsresource.RouteConfigResourceDat return } rw.mu.Unlock() + rcData, ok := rd.(*xdsresource.RouteConfigResourceData) + if !ok { + rw.logger.Warningf("RDS watcher received unexpected resource data type %T", rd) + return + } if rw.logger.V(2) { - rw.logger.Infof("RDS watch for resource %q received update: %#v", rw.routeName, update.Resource) + rw.logger.Infof("RDS watch for resource %q received update: %#v", rw.routeName, rcData.Resource) } routeName := rw.routeName - rwu := rdsWatcherUpdate{data: &update.Resource} + rwu := rdsWatcherUpdate{data: &rcData.Resource} rw.parent.updates[routeName] = rwu rw.parent.callback(routeName, rwu) } diff --git a/internal/xds/xdsclient/metrics_test.go b/internal/xds/xdsclient/metrics_test.go index 26a38aea381e..37ba1c707bc0 100644 --- a/internal/xds/xdsclient/metrics_test.go +++ b/internal/xds/xdsclient/metrics_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/internal/testutils/stats" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" + "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" v3listenerpb "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" @@ -40,7 +41,7 @@ import ( type noopListenerWatcher struct{} -func (noopListenerWatcher) ResourceChanged(_ *xdsresource.ListenerResourceData, onDone func()) { +func (noopListenerWatcher) ResourceChanged(_ xdsclient.ResourceData, onDone func()) { onDone() } diff --git a/internal/xds/xdsclient/tests/authority_test.go b/internal/xds/xdsclient/tests/authority_test.go index fbea7cb8ec3c..68fb8eda6005 100644 --- a/internal/xds/xdsclient/tests/authority_test.go +++ b/internal/xds/xdsclient/tests/authority_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" xdstestutils "google.golang.org/grpc/internal/xds/testutils" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" @@ -316,7 +317,7 @@ func (s) TestAuthority_Fallback(t *testing.T) { if err != nil { t.Fatalf("Error when waiting for a resource update callback: %v", err) } - gotUpdate := v.(xdsresource.ClusterUpdate) + gotUpdate := v.(clusterUpdateErrTuple).update wantUpdate := xdsresource.ClusterUpdate{ ClusterName: clusterName, EDSServiceName: edsSecondaryName, @@ -351,8 +352,19 @@ func newClusterWatcherV2() *clusterWatcherV2 { } } -func (cw *clusterWatcherV2) ResourceChanged(update *xdsresource.ClusterResourceData, onDone func()) { - cw.updateCh.Send(update.Resource) +func (cw *clusterWatcherV2) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { + if rd == nil { + cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) + onDone() + return + } + clusterData, ok := rd.(*xdsresource.ClusterResourceData) + if !ok { + cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) + onDone() + return + } + cw.updateCh.Send(clusterUpdateErrTuple{update: clusterData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/cds_watchers_test.go b/internal/xds/xdsclient/tests/cds_watchers_test.go index e6368340294f..5d0e7977fb06 100644 --- a/internal/xds/xdsclient/tests/cds_watchers_test.go +++ b/internal/xds/xdsclient/tests/cds_watchers_test.go @@ -34,6 +34,7 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" @@ -44,7 +45,7 @@ import ( type noopClusterWatcher struct{} -func (noopClusterWatcher) ResourceChanged(_ *xdsresource.ClusterResourceData, onDone func()) { +func (noopClusterWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (noopClusterWatcher) ResourceError(_ error, onDone func()) { @@ -67,8 +68,19 @@ func newClusterWatcher() *clusterWatcher { return &clusterWatcher{updateCh: testutils.NewChannel()} } -func (cw *clusterWatcher) ResourceChanged(update *xdsresource.ClusterResourceData, onDone func()) { - cw.updateCh.Send(clusterUpdateErrTuple{update: update.Resource}) +func (cw *clusterWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { + if rd == nil { + cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) + onDone() + return + } + clusterData, ok := rd.(*xdsresource.ClusterResourceData) + if !ok { + cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) + onDone() + return + } + cw.updateCh.Send(clusterUpdateErrTuple{update: clusterData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/eds_watchers_test.go b/internal/xds/xdsclient/tests/eds_watchers_test.go index a76c58641439..56fbad117f81 100644 --- a/internal/xds/xdsclient/tests/eds_watchers_test.go +++ b/internal/xds/xdsclient/tests/eds_watchers_test.go @@ -34,6 +34,7 @@ import ( "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" "google.golang.org/grpc/internal/xds/clients" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" "google.golang.org/protobuf/types/known/wrapperspb" @@ -53,7 +54,7 @@ const ( type noopEndpointsWatcher struct{} -func (noopEndpointsWatcher) ResourceChanged(_ *xdsresource.EndpointsResourceData, onDone func()) { +func (noopEndpointsWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (noopEndpointsWatcher) ResourceError(_ error, onDone func()) { @@ -76,11 +77,14 @@ func newEndpointsWatcher() *endpointsWatcher { return &endpointsWatcher{updateCh: testutils.NewChannel()} } -func (ew *endpointsWatcher) ResourceChanged(update *xdsresource.EndpointsResourceData, onDone func()) { - ew.updateCh.Send(endpointsUpdateErrTuple{update: update.Resource}) - onDone() +func (ew *endpointsWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { + defer onDone() + endpointsData, ok := rd.(*xdsresource.EndpointsResourceData) + if !ok { + return + } + ew.updateCh.Send(endpointsUpdateErrTuple{update: endpointsData.Resource}) } - func (ew *endpointsWatcher) ResourceError(err error, onDone func()) { // When used with a go-control-plane management server that continuously // resends resources which are NACKed by the xDS client, using a `Replace()` diff --git a/internal/xds/xdsclient/tests/lds_watchers_test.go b/internal/xds/xdsclient/tests/lds_watchers_test.go index 877148639b2c..20bf98efce48 100644 --- a/internal/xds/xdsclient/tests/lds_watchers_test.go +++ b/internal/xds/xdsclient/tests/lds_watchers_test.go @@ -34,6 +34,7 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" + xdsClient "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" @@ -48,7 +49,7 @@ import ( type noopListenerWatcher struct{} -func (noopListenerWatcher) ResourceChanged(_ *xdsresource.ListenerResourceData, onDone func()) { +func (noopListenerWatcher) ResourceChanged(_ xdsClient.ResourceData, onDone func()) { onDone() } func (noopListenerWatcher) ResourceError(_ error, onDone func()) { @@ -71,8 +72,19 @@ func newListenerWatcher() *listenerWatcher { return &listenerWatcher{updateCh: testutils.NewChannel()} } -func (lw *listenerWatcher) ResourceChanged(update *xdsresource.ListenerResourceData, onDone func()) { - lw.updateCh.Send(listenerUpdateErrTuple{update: update.Resource}) +func (lw *listenerWatcher) ResourceChanged(rd xdsClient.ResourceData, onDone func()) { + if rd == nil { + lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) + onDone() + return + } + clusterData, ok := rd.(*xdsresource.ListenerResourceData) + if !ok { + lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) + onDone() + return + } + lw.updateCh.Send(listenerUpdateErrTuple{update: clusterData.Resource}) onDone() } @@ -100,8 +112,19 @@ func newListenerWatcherMultiple(size int) *listenerWatcherMultiple { return &listenerWatcherMultiple{updateCh: testutils.NewChannelWithSize(size)} } -func (lw *listenerWatcherMultiple) ResourceChanged(update *xdsresource.ListenerResourceData, onDone func()) { - lw.updateCh.Send(listenerUpdateErrTuple{update: update.Resource}) +func (lw *listenerWatcherMultiple) ResourceChanged(rd xdsClient.ResourceData, onDone func()) { + if rd == nil { + lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) + onDone() + return + } + listenerData, ok := rd.(*xdsresource.ListenerResourceData) + if !ok { + lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) + onDone() + return + } + lw.updateCh.Send(listenerUpdateErrTuple{update: listenerData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/rds_watchers_test.go b/internal/xds/xdsclient/tests/rds_watchers_test.go index 3ec333ad8a24..03efbb04536f 100644 --- a/internal/xds/xdsclient/tests/rds_watchers_test.go +++ b/internal/xds/xdsclient/tests/rds_watchers_test.go @@ -33,6 +33,7 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/testutils/xds/e2e" "google.golang.org/grpc/internal/xds/bootstrap" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" "google.golang.org/protobuf/types/known/wrapperspb" @@ -43,7 +44,7 @@ import ( type noopRouteConfigWatcher struct{} -func (noopRouteConfigWatcher) ResourceChanged(_ *xdsresource.RouteConfigResourceData, onDone func()) { +func (noopRouteConfigWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (noopRouteConfigWatcher) ResourceError(_ error, onDone func()) { @@ -66,8 +67,19 @@ func newRouteConfigWatcher() *routeConfigWatcher { return &routeConfigWatcher{updateCh: testutils.NewChannel()} } -func (rw *routeConfigWatcher) ResourceChanged(update *xdsresource.RouteConfigResourceData, onDone func()) { - rw.updateCh.Send(routeConfigUpdateErrTuple{update: update.Resource}) +func (rw *routeConfigWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { + if rd == nil { + rw.updateCh.Send(routeConfigUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) + onDone() + return + } + rcData, ok := rd.(*xdsresource.RouteConfigResourceData) + if !ok { + rw.updateCh.Send(routeConfigUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) + onDone() + return + } + rw.updateCh.Send(routeConfigUpdateErrTuple{update: rcData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index 4d32e8f8b246..776281226afa 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -19,7 +19,6 @@ package xdsresource import ( "bytes" - "fmt" "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/internal/xds/bootstrap" @@ -158,59 +157,10 @@ func (c *ClusterResourceData) Bytes() []byte { return raw.Value } -// ClusterWatcher wraps the callbacks to be invoked for different events -// corresponding to the cluster resource being watched. gRFC A88 contains an -// exhaustive list of what method is invoked under what conditions. -type ClusterWatcher interface { - // ResourceChanged indicates a new version of the resource is available. - ResourceChanged(resource *ClusterResourceData, done func()) - - // ResourceError indicates an error occurred while trying to fetch or - // decode the associated resource. The previous version of the resource - // should be considered invalid. - ResourceError(err error, done func()) - - // AmbientError indicates an error occurred after a resource has been - // received that should not modify the use of that resource but may provide - // useful information about the state of the XDSClient for debugging - // purposes. The previous version of the resource should still be - // considered valid. - AmbientError(err error, done func()) -} - -type delegatingClusterWatcher struct { - watcher ClusterWatcher -} - -func (d *delegatingClusterWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { - if gData == nil { - d.watcher.ResourceError(fmt.Errorf("cluster resource missing"), onDone) - return - } - c, ok := gData.(*ClusterResourceData) - if !ok { - d.watcher.ResourceError(fmt.Errorf("delegatingClusterWatcher: unexpected resource data type %T", gData), onDone) - return - } - d.watcher.ResourceChanged(c, onDone) -} - -func (d *delegatingClusterWatcher) ResourceError(err error, onDone func()) { - d.watcher.ResourceError(err, onDone) -} - -func (d *delegatingClusterWatcher) AmbientError(err error, onDone func()) { - d.watcher.AmbientError(err, onDone) -} - // WatchCluster uses xDS to discover the configuration associated with the // provided cluster resource name. -func WatchCluster(p Producer, name string, w ClusterWatcher) (cancel func()) { - var gw xdsclient.ResourceWatcher - if w != nil { - gw = &delegatingClusterWatcher{watcher: w} - } - return p.WatchResource(ClusterResource, name, gw) +func WatchCluster(p Producer, name string, w xdsclient.ResourceWatcher) (cancel func()) { + return p.WatchResource(ClusterResource, name, w) } // NewClusterResourceTypeDecoder returns a xdsclient.Decoder that has access to diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index da730fbf1e10..6479b1125393 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -19,7 +19,6 @@ package xdsresource import ( "bytes" - "fmt" "google.golang.org/grpc/internal/pretty" xdsclient "google.golang.org/grpc/internal/xds/clients/xdsclient" @@ -133,59 +132,10 @@ func (e *EndpointsResourceData) Bytes() []byte { return raw.Value } -// EndpointsWatcher wraps the callbacks to be invoked for different -// events corresponding to the endpoints resource being watched. gRFC A88 -// contains an exhaustive list of what method is invoked under what conditions. -type EndpointsWatcher interface { - // ResourceChanged indicates a new version of the resource is available. - ResourceChanged(resource *EndpointsResourceData, done func()) - - // ResourceError indicates an error occurred while trying to fetch or - // decode the associated resource. The previous version of the resource - // should be considered invalid. - ResourceError(err error, done func()) - - // AmbientError indicates an error occurred after a resource has been - // received that should not modify the use of that resource but may provide - // useful information about the state of the XDSClient for debugging - // purposes. The previous version of the resource should still be - // considered valid. - AmbientError(err error, done func()) -} - -type delegatingEndpointsWatcher struct { - watcher EndpointsWatcher -} - -func (d *delegatingEndpointsWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { - if gData == nil { - d.watcher.ResourceError(fmt.Errorf("endpoints resource missing"), onDone) - return - } - e, ok := gData.(*EndpointsResourceData) - if !ok { - d.watcher.ResourceError(fmt.Errorf("delegatingEndpointsWatcher: unexpected resource data type %T", gData), onDone) - return - } - d.watcher.ResourceChanged(e, onDone) -} - -func (d *delegatingEndpointsWatcher) ResourceError(err error, onDone func()) { - d.watcher.ResourceError(err, onDone) -} - -func (d *delegatingEndpointsWatcher) AmbientError(err error, onDone func()) { - d.watcher.AmbientError(err, onDone) -} - // WatchEndpoints uses xDS to discover the configuration associated with the // provided endpoints resource name. -func WatchEndpoints(p Producer, name string, w EndpointsWatcher) (cancel func()) { - var gw xdsclient.ResourceWatcher - if w != nil { - gw = &delegatingEndpointsWatcher{watcher: w} - } - return p.WatchResource(EndpointsResource, name, gw) +func WatchEndpoints(p Producer, name string, w xdsclient.ResourceWatcher) (cancel func()) { + return p.WatchResource(EndpointsResource, name, w) } // NewEndpointsResourceTypeDecoder returns a decoder for Endpoint resources. diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index c4b158302fd2..d25deee0e3a7 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -186,60 +186,10 @@ func (l *ListenerResourceData) Bytes() []byte { return raw.Value } -// ListenerWatcher wraps the callbacks to be invoked for different -// events corresponding to the listener resource being watched. gRFC A88 -// contains an exhaustive list of what method is invoked under what conditions. -type ListenerWatcher interface { - // ResourceChanged indicates a new version of the resource is available. - ResourceChanged(resource *ListenerResourceData, done func()) - - // ResourceError indicates an error occurred while trying to fetch or - // decode the associated resource. The previous version of the resource - // should be considered invalid. - ResourceError(err error, done func()) - - // AmbientError indicates an error occurred after a resource has been - // received that should not modify the use of that resource but may provide - // useful information about the state of the XDSClient for debugging - // purposes. The previous version of the resource should still be - // considered valid. - AmbientError(err error, done func()) -} - -type delegatingListenerWatcher struct { - watcher ListenerWatcher -} - -func (d *delegatingListenerWatcher) ResourceChanged(gData xdsclient.ResourceData, done func()) { - if gData == nil { - d.watcher.ResourceError(fmt.Errorf("listener resource missing"), done) - return - } - lrd, ok := gData.(*ListenerResourceData) - if !ok { - d.watcher.ResourceError(fmt.Errorf("delegatingListenerWatcher: unexpected resource data type %T", gData), done) - return - } - d.watcher.ResourceChanged(lrd, done) -} - -func (d *delegatingListenerWatcher) ResourceError(err error, onDone func()) { - d.watcher.ResourceError(err, onDone) -} - -func (d *delegatingListenerWatcher) AmbientError(err error, onDone func()) { - d.watcher.AmbientError(err, onDone) -} - // WatchListener uses xDS to discover the configuration associated with the // provided listener resource name. -func WatchListener(p Producer, name string, w ListenerWatcher) (cancel func()) { - var gw xdsclient.ResourceWatcher - if w != nil { - gw = &delegatingListenerWatcher{watcher: w} - } - // Use the exported generic ResourceType value for listener. - return p.WatchResource(ListenerResource, name, gw) +func WatchListener(p Producer, name string, w xdsclient.ResourceWatcher) (cancel func()) { + return p.WatchResource(ListenerResource, name, w) } // NewListenerResourceTypeDecoder returns a decoder for RouteConfig resources. diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index 9e4e7ec744a5..3f0ef69898a6 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -19,7 +19,6 @@ package xdsresource import ( "bytes" - "fmt" "google.golang.org/grpc/internal/pretty" xdsclient "google.golang.org/grpc/internal/xds/clients/xdsclient" @@ -134,60 +133,10 @@ func (r *RouteConfigResourceData) Bytes() []byte { return raw.Value } -// RouteConfigWatcher wraps the callbacks to be invoked for different -// events corresponding to the route configuration resource being watched. gRFC -// A88 contains an exhaustive list of what method is invoked under what -// conditions. -type RouteConfigWatcher interface { - // ResourceChanged indicates a new version of the resource is available. - ResourceChanged(resource *RouteConfigResourceData, done func()) - - // ResourceError indicates an error occurred while trying to fetch or - // decode the associated resource. The previous version of the resource - // should be considered invalid. - ResourceError(err error, done func()) - - // AmbientError indicates an error occurred after a resource has been - // received that should not modify the use of that resource but may provide - // useful information about the state of the XDSClient for debugging - // purposes. The previous version of the resource should still be - // considered valid. - AmbientError(err error, done func()) -} - -type delegatingRouteConfigWatcher struct { - watcher RouteConfigWatcher -} - -func (d *delegatingRouteConfigWatcher) ResourceChanged(gData xdsclient.ResourceData, onDone func()) { - if gData == nil { - d.watcher.ResourceError(fmt.Errorf("route config resource missing"), onDone) - return - } - rc, ok := gData.(*RouteConfigResourceData) - if !ok { - d.watcher.ResourceError(fmt.Errorf("delegatingRouteConfigWatcher: unexpected resource data type %T", gData), onDone) - return - } - d.watcher.ResourceChanged(rc, onDone) -} - -func (d *delegatingRouteConfigWatcher) ResourceError(err error, onDone func()) { - d.watcher.ResourceError(err, onDone) -} - -func (d *delegatingRouteConfigWatcher) AmbientError(err error, onDone func()) { - d.watcher.AmbientError(err, onDone) -} - // WatchRouteConfig uses xDS to discover the configuration associated with the // provided route configuration resource name. -func WatchRouteConfig(p Producer, name string, w RouteConfigWatcher) (cancel func()) { - var gw xdsclient.ResourceWatcher - if w != nil { - gw = &delegatingRouteConfigWatcher{watcher: w} - } - return p.WatchResource(RouteConfigResource, name, gw) +func WatchRouteConfig(p Producer, name string, w xdsclient.ResourceWatcher) (cancel func()) { + return p.WatchResource(RouteConfigResource, name, w) } // NewRouteConfigResourceTypeDecoder returns a decoder for RouteConfig resources. diff --git a/xds/csds/csds_e2e_test.go b/xds/csds/csds_e2e_test.go index d09deeb2682d..3f9f84c50b8b 100644 --- a/xds/csds/csds_e2e_test.go +++ b/xds/csds/csds_e2e_test.go @@ -51,6 +51,7 @@ import ( v3routepb "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" v3statuspb "github.com/envoyproxy/go-control-plane/envoy/service/status/v3" v3statuspbgrpc "github.com/envoyproxy/go-control-plane/envoy/service/status/v3" + clientimpl "google.golang.org/grpc/internal/xds/clients/xdsclient" _ "google.golang.org/grpc/internal/xds/httpfilter/router" // Register the router filter ) @@ -71,7 +72,7 @@ func Test(t *testing.T) { type nopListenerWatcher struct{} -func (nopListenerWatcher) ResourceChanged(_ *xdsresource.ListenerResourceData, onDone func()) { +func (nopListenerWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (nopListenerWatcher) ResourceError(_ error, onDone func()) { @@ -83,7 +84,7 @@ func (nopListenerWatcher) AmbientError(_ error, onDone func()) { type nopRouteConfigWatcher struct{} -func (nopRouteConfigWatcher) ResourceChanged(_ *xdsresource.RouteConfigResourceData, onDone func()) { +func (nopRouteConfigWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (nopRouteConfigWatcher) ResourceError(_ error, onDone func()) { @@ -95,7 +96,7 @@ func (nopRouteConfigWatcher) AmbientError(_ error, onDone func()) { type nopClusterWatcher struct{} -func (nopClusterWatcher) ResourceChanged(_ *xdsresource.ClusterResourceData, onDone func()) { +func (nopClusterWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (nopClusterWatcher) ResourceError(_ error, onDone func()) { @@ -107,7 +108,7 @@ func (nopClusterWatcher) AmbientError(_ error, onDone func()) { type nopEndpointsWatcher struct{} -func (nopEndpointsWatcher) ResourceChanged(_ *xdsresource.EndpointsResourceData, onDone func()) { +func (nopEndpointsWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { onDone() } func (nopEndpointsWatcher) ResourceError(_ error, onDone func()) { @@ -138,7 +139,7 @@ func newBlockingListenerWatcher(testCtxDone <-chan struct{}) *blockingListenerWa } } -func (w *blockingListenerWatcher) ResourceChanged(_ *xdsresource.ListenerResourceData, onDone func()) { +func (w *blockingListenerWatcher) ResourceChanged(_ clientimpl.ResourceData, onDone func()) { writeOnDone(w.testCtxDone, w.onDoneCh, onDone) } func (w *blockingListenerWatcher) ResourceError(_ error, onDone func()) { From 10f45047b59cb8478d02982f86231d32306c0eac Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Tue, 16 Sep 2025 07:51:04 +0000 Subject: [PATCH 10/12] Fixed the xds client issues --- .../xdsclient/xdsresource/cluster_resource_type.go | 6 +++--- .../xdsresource/endpoints_resource_type.go | 10 +++++----- .../xdsclient/xdsresource/listener_resource_type.go | 13 ++++++------- .../xdsresource/route_config_resource_type.go | 6 +++--- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index 776281226afa..a049ef1f21cf 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -40,7 +40,7 @@ var ( _ xdsclient.ResourceData = (*ClusterResourceData)(nil) // ClusterResource is a singleton instance of xdsclient.ResourceType - // that defines the configuration for the Listener resource. + // that defines the configuration for the Cluster resource. ClusterResource = xdsclient.ResourceType{ TypeURL: version.V3ClusterURL, TypeName: ClusterResourceTypeName, @@ -148,7 +148,7 @@ func (c *ClusterResourceData) Equal(other xdsclient.ResourceData) bool { return bytes.Equal(c.Bytes(), other.Bytes()) } -// Bytes returns the underlying raw bytes of the clustered resource. +// Bytes returns the underlying raw bytes of the Cluster resource. func (c *ClusterResourceData) Bytes() []byte { raw := c.Raw() if raw == nil { @@ -163,7 +163,7 @@ func WatchCluster(p Producer, name string, w xdsclient.ResourceWatcher) (cancel return p.WatchResource(ClusterResource, name, w) } -// NewClusterResourceTypeDecoder returns a xdsclient.Decoder that has access to +// NewClusterResourceTypeDecoder returns an xdsclient.Decoder that has access to // bootstrap config and server config mapping for decoding. func NewClusterResourceTypeDecoder(bc *bootstrap.Config, gServerCfgMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig) xdsclient.Decoder { return &clusterResourceType{ diff --git a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go index 6479b1125393..65906102e651 100644 --- a/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/endpoints_resource_type.go @@ -38,8 +38,8 @@ var ( _ xdsclient.Decoder = endpointsResourceType{} _ xdsclient.ResourceData = (*EndpointsResourceData)(nil) - // EndpointsResource is a singleton instance of xdsclient.ResourceType - // that defines the configuration for the Listener resource. + // EndpointsResource is a singleton instance of xdsclient.ResourceType that + // defines the configuration for the Endpoints resource. EndpointsResource = xdsclient.ResourceType{ TypeURL: version.V3EndpointsURL, TypeName: EndpointsResourceTypeName, @@ -109,7 +109,7 @@ func (e *EndpointsResourceData) Raw() *anypb.Any { return e.Resource.Raw } -// Equal returns true if other is equal to c +// Equal returns true if other xdsclient.ResourceData is equal to e. func (e *EndpointsResourceData) Equal(other xdsclient.ResourceData) bool { if e == nil && other == nil { return true @@ -123,7 +123,7 @@ func (e *EndpointsResourceData) Equal(other xdsclient.ResourceData) bool { return bytes.Equal(e.Bytes(), other.Bytes()) } -// Bytes returns the underlying raw bytes of the clustered resource. +// Bytes returns the underlying raw bytes of the Endpoints resource. func (e *EndpointsResourceData) Bytes() []byte { raw := e.Raw() if raw == nil { @@ -138,7 +138,7 @@ func WatchEndpoints(p Producer, name string, w xdsclient.ResourceWatcher) (cance return p.WatchResource(EndpointsResource, name, w) } -// NewEndpointsResourceTypeDecoder returns a decoder for Endpoint resources. +// NewEndpointsResourceTypeDecoder returns a decoder for Endpoints resources. func NewEndpointsResourceTypeDecoder() xdsclient.Decoder { return endpointsResourceType{resourceTypeState: resourceTypeState{ typeURL: version.V3EndpointsURL, diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index d25deee0e3a7..c661e4cd6660 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -40,8 +40,8 @@ var ( _ xdsclient.Decoder = listenerResourceType{} _ xdsclient.ResourceData = (*ListenerResourceData)(nil) - // ListenerResource is a singleton instance of xdsclient.ResourceType - // that defines the configuration for the Listener resource. + // ListenerResource is a singleton instance of xdsclient.ResourceType that + // defines the configuration for the Listener resource. ListenerResource = xdsclient.ResourceType{ TypeURL: version.V3ListenerURL, TypeName: ListenerResourceTypeName, @@ -163,7 +163,7 @@ func (l *ListenerResourceData) Raw() *anypb.Any { return l.Resource.Raw } -// Equal returns true if other is equal to c +// Equal returns true if other xdsclient.ResourceData is equal to l. func (l *ListenerResourceData) Equal(other xdsclient.ResourceData) bool { if l == nil && other == nil { return true @@ -177,7 +177,7 @@ func (l *ListenerResourceData) Equal(other xdsclient.ResourceData) bool { return bytes.Equal(l.Bytes(), other.Bytes()) } -// Bytes returns the underlying raw bytes of the clustered resource. +// Bytes returns the underlying raw bytes of the listener resource. func (l *ListenerResourceData) Bytes() []byte { raw := l.Raw() if raw == nil { @@ -192,9 +192,8 @@ func WatchListener(p Producer, name string, w xdsclient.ResourceWatcher) (cancel return p.WatchResource(ListenerResource, name, w) } -// NewListenerResourceTypeDecoder returns a decoder for RouteConfig resources. -// RouteConfig is stateless so this just returns the zero-valued -// routeConfigResourceType. +// NewListenerResourceTypeDecoder returns an xdsclient.Decoder that has access to +// bootstrap config and server config mapping for decoding. func NewListenerResourceTypeDecoder(bc *bootstrap.Config, serverConfigMap map[xdsclient.ServerConfig]*bootstrap.ServerConfig) xdsclient.Decoder { return &listenerResourceType{ resourceTypeState: resourceTypeState{ diff --git a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go index 3f0ef69898a6..847594aeaaee 100644 --- a/internal/xds/xdsclient/xdsresource/route_config_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/route_config_resource_type.go @@ -39,7 +39,7 @@ var ( _ xdsclient.ResourceData = (*RouteConfigResourceData)(nil) // RouteConfigResource is a singleton instance of xdsclient.ResourceType - // that defines the configuration for the Listener resource. + // that defines the configuration for the RouteConfig resource. RouteConfigResource = xdsclient.ResourceType{ TypeURL: version.V3RouteConfigURL, TypeName: RouteConfigTypeName, @@ -110,7 +110,7 @@ func (r *RouteConfigResourceData) Raw() *anypb.Any { return r.Resource.Raw } -// Equal returns true if other is equal to r +// Equal returns true if other xdsclient.ResourceData is equal to r. func (r *RouteConfigResourceData) Equal(other xdsclient.ResourceData) bool { if r == nil && other == nil { return true @@ -124,7 +124,7 @@ func (r *RouteConfigResourceData) Equal(other xdsclient.ResourceData) bool { return bytes.Equal(r.Bytes(), other.Bytes()) } -// Bytes returns the underlying raw bytes of the clustered resource. +// Bytes returns the underlying raw bytes of the RouteConfig resource. func (r *RouteConfigResourceData) Bytes() []byte { raw := r.Raw() if raw == nil { From 7bb5a2b4377b37c1cffcd8d0b094335c5bd81ee9 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Tue, 16 Sep 2025 11:44:56 +0000 Subject: [PATCH 11/12] Fixed the xds client issues --- .../balancer/cdsbalancer/cluster_watcher.go | 7 +----- .../clusterresolver/resource_resolver_eds.go | 9 +++---- .../xdsclient/test/lds_watchers_test.go | 7 +----- .../xdsclient/test/misc_watchers_test.go | 8 +------ internal/xds/resolver/watch_service.go | 20 ++-------------- internal/xds/server/listener_wrapper.go | 7 +----- internal/xds/server/rds_handler.go | 6 +---- .../xds/xdsclient/clientimpl_loadreport.go | 2 +- .../xds/xdsclient/tests/authority_test.go | 12 +--------- .../xds/xdsclient/tests/cds_watchers_test.go | 12 +--------- .../xds/xdsclient/tests/eds_watchers_test.go | 5 +--- .../xds/xdsclient/tests/lds_watchers_test.go | 24 ++----------------- .../xds/xdsclient/tests/rds_watchers_test.go | 12 +--------- .../xdsresource/cluster_resource_type.go | 10 ++------ .../xdsresource/listener_resource_type.go | 10 ++------ 15 files changed, 21 insertions(+), 130 deletions(-) diff --git a/internal/xds/balancer/cdsbalancer/cluster_watcher.go b/internal/xds/balancer/cdsbalancer/cluster_watcher.go index 9f4a387e0957..917748c035a1 100644 --- a/internal/xds/balancer/cdsbalancer/cluster_watcher.go +++ b/internal/xds/balancer/cdsbalancer/cluster_watcher.go @@ -18,7 +18,6 @@ package cdsbalancer import ( "context" - "fmt" "google.golang.org/grpc/internal/xds/clients/xdsclient" "google.golang.org/grpc/internal/xds/xdsclient/xdsresource" @@ -35,11 +34,7 @@ type clusterWatcher struct { } func (cw *clusterWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { - clusterData, ok := rd.(*xdsresource.ClusterResourceData) - if !ok { - cw.ResourceError(fmt.Errorf("clusterWatcher: unexpected resource data type %T", rd), onDone) - return - } + clusterData := rd.(*xdsresource.ClusterResourceData) handleUpdate := func(context.Context) { cw.parent.onClusterUpdate(cw.name, clusterData.Resource); onDone() } cw.parent.serializer.ScheduleOr(handleUpdate, onDone) } diff --git a/internal/xds/balancer/clusterresolver/resource_resolver_eds.go b/internal/xds/balancer/clusterresolver/resource_resolver_eds.go index 39ec6e4bd586..836b09e5ae39 100644 --- a/internal/xds/balancer/clusterresolver/resource_resolver_eds.go +++ b/internal/xds/balancer/clusterresolver/resource_resolver_eds.go @@ -78,22 +78,19 @@ func newEDSResolver(nameToWatch string, producer xdsresource.Producer, topLevelR // ResourceChanged is invoked to report an update for the resource being watched. func (er *edsDiscoveryMechanism) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { - defer onDone() if er.stopped.HasFired() { - return - } - update, ok := rd.(*xdsresource.EndpointsResourceData) - if !ok { - er.logger.Warningf("EDS discovery mechanism received unexpected resource data type %T", rd) + onDone() return } + update := rd.(*xdsresource.EndpointsResourceData) er.mu.Lock() er.update = &update.Resource er.mu.Unlock() er.topLevelResolver.onUpdate(onDone) } + func (er *edsDiscoveryMechanism) ResourceError(err error, onDone func()) { if er.stopped.HasFired() { onDone() diff --git a/internal/xds/clients/xdsclient/test/lds_watchers_test.go b/internal/xds/clients/xdsclient/test/lds_watchers_test.go index ae4783921aec..142f6eebd780 100644 --- a/internal/xds/clients/xdsclient/test/lds_watchers_test.go +++ b/internal/xds/clients/xdsclient/test/lds_watchers_test.go @@ -75,12 +75,7 @@ func newListenerWatcher() *listenerWatcher { } func (lw *listenerWatcher) ResourceChanged(update xdsclient.ResourceData, onDone func()) { - lisData, ok := update.(*listenerResourceData) - if !ok { - lw.resourceErrCh.Send(listenerUpdateErrTuple{resourceErr: fmt.Errorf("unexpected resource type: %T", update)}) - onDone() - return - } + lisData := update.(*listenerResourceData) select { case <-lw.updateCh.C: default: diff --git a/internal/xds/clients/xdsclient/test/misc_watchers_test.go b/internal/xds/clients/xdsclient/test/misc_watchers_test.go index e611c2240c43..d23243fcc01a 100644 --- a/internal/xds/clients/xdsclient/test/misc_watchers_test.go +++ b/internal/xds/clients/xdsclient/test/misc_watchers_test.go @@ -62,14 +62,8 @@ func newTestLDSWatcher(client *xdsclient.XDSClient, name1, name2 string) *testLD } func (lw *testLDSWatcher) ResourceChanged(update xdsclient.ResourceData, onDone func()) { - lisData, ok := update.(*listenerResourceData) - if !ok { - lw.updateCh.Send(listenerUpdateErrTuple{resourceErr: fmt.Errorf("unexpected resource type: %T", update)}) - onDone() - return - } + lisData := update.(*listenerResourceData) lw.updateCh.Send(listenerUpdateErrTuple{update: lisData.Resource}) - lw.cancel1 = lw.client.WatchResource(xdsresource.V3ListenerURL, lw.name1, lw.lw1) lw.cancel2 = lw.client.WatchResource(xdsresource.V3ListenerURL, lw.name2, lw.lw2) onDone() diff --git a/internal/xds/resolver/watch_service.go b/internal/xds/resolver/watch_service.go index 67426c7dc559..23f1409eeb7e 100644 --- a/internal/xds/resolver/watch_service.go +++ b/internal/xds/resolver/watch_service.go @@ -38,18 +38,7 @@ func newListenerWatcher(resourceName string, parent *xdsResolver) *listenerWatch } func (l *listenerWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { - if rd == nil { - l.parent.onListenerResourceUpdate(xdsresource.ListenerUpdate{}) - onDone() - return - } - listenerData, ok := rd.(*xdsresource.ListenerResourceData) - if !ok { - l.parent.onListenerResourceUpdate(xdsresource.ListenerUpdate{}) - onDone() - return - } - + listenerData := rd.(*xdsresource.ListenerResourceData) handleUpdate := func(context.Context) { l.parent.onListenerResourceUpdate(listenerData.Resource); onDone() } l.parent.serializer.ScheduleOr(handleUpdate, onDone) } @@ -82,12 +71,7 @@ func newRouteConfigWatcher(resourceName string, parent *xdsResolver) *routeConfi } func (r *routeConfigWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) { - rcData, ok := rd.(*xdsresource.RouteConfigResourceData) - if !ok { - onDone() - return - } - + rcData := rd.(*xdsresource.RouteConfigResourceData) handleUpdate := func(context.Context) { r.parent.onRouteConfigResourceUpdate(r.resourceName, rcData.Resource) onDone() diff --git a/internal/xds/server/listener_wrapper.go b/internal/xds/server/listener_wrapper.go index a2022b85d27a..cea7d746facb 100644 --- a/internal/xds/server/listener_wrapper.go +++ b/internal/xds/server/listener_wrapper.go @@ -393,17 +393,12 @@ func (lw *ldsWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) lw.logger.Warningf("Resource %q received update after listener was closed", lw.name) return } - listenerData, ok := rd.(*xdsresource.ListenerResourceData) - if !ok { - lw.logger.Warningf("LDS watcher received unexpected resource data type %T", rd) - return - } + listenerData := rd.(*xdsresource.ListenerResourceData) if lw.logger.V(2) { lw.logger.Infof("LDS watch for resource %q received update: %#v", lw.name, listenerData.Resource) } l := lw.parent ilc := listenerData.Resource.InboundListenerCfg - // Make sure that the socket address on the received Listener resource // matches the address of the net.Listener passed to us by the user. This // check is done here instead of at the XDSClient layer because of the diff --git a/internal/xds/server/rds_handler.go b/internal/xds/server/rds_handler.go index 61ac03a5d638..19edf38093fb 100644 --- a/internal/xds/server/rds_handler.go +++ b/internal/xds/server/rds_handler.go @@ -144,11 +144,7 @@ func (rw *rdsWatcher) ResourceChanged(rd xdsclient.ResourceData, onDone func()) return } rw.mu.Unlock() - rcData, ok := rd.(*xdsresource.RouteConfigResourceData) - if !ok { - rw.logger.Warningf("RDS watcher received unexpected resource data type %T", rd) - return - } + rcData := rd.(*xdsresource.RouteConfigResourceData) if rw.logger.V(2) { rw.logger.Infof("RDS watch for resource %q received update: %#v", rw.routeName, rcData.Resource) } diff --git a/internal/xds/xdsclient/clientimpl_loadreport.go b/internal/xds/xdsclient/clientimpl_loadreport.go index b8410c605ffa..1e95ddba22e5 100644 --- a/internal/xds/xdsclient/clientimpl_loadreport.go +++ b/internal/xds/xdsclient/clientimpl_loadreport.go @@ -29,7 +29,7 @@ import ( // ReportLoad starts a load reporting stream to the given server. All load // reports to the same server share the LRS stream. // -// It returns a lrsclient.LoadStore for the user to report loads. +// It returns a LoadStore for the user to report loads. func (c *clientImpl) ReportLoad(server *bootstrap.ServerConfig) (LoadStore, func(context.Context)) { load, err := c.lrsClient.ReportLoad(clients.ServerIdentifier{ ServerURI: server.ServerURI(), diff --git a/internal/xds/xdsclient/tests/authority_test.go b/internal/xds/xdsclient/tests/authority_test.go index 68fb8eda6005..3275393114a2 100644 --- a/internal/xds/xdsclient/tests/authority_test.go +++ b/internal/xds/xdsclient/tests/authority_test.go @@ -353,17 +353,7 @@ func newClusterWatcherV2() *clusterWatcherV2 { } func (cw *clusterWatcherV2) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { - if rd == nil { - cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) - onDone() - return - } - clusterData, ok := rd.(*xdsresource.ClusterResourceData) - if !ok { - cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) - onDone() - return - } + clusterData := rd.(*xdsresource.ClusterResourceData) cw.updateCh.Send(clusterUpdateErrTuple{update: clusterData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/cds_watchers_test.go b/internal/xds/xdsclient/tests/cds_watchers_test.go index 5d0e7977fb06..ba6069bb2976 100644 --- a/internal/xds/xdsclient/tests/cds_watchers_test.go +++ b/internal/xds/xdsclient/tests/cds_watchers_test.go @@ -69,17 +69,7 @@ func newClusterWatcher() *clusterWatcher { } func (cw *clusterWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { - if rd == nil { - cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) - onDone() - return - } - clusterData, ok := rd.(*xdsresource.ClusterResourceData) - if !ok { - cw.updateCh.Send(clusterUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) - onDone() - return - } + clusterData := rd.(*xdsresource.ClusterResourceData) cw.updateCh.Send(clusterUpdateErrTuple{update: clusterData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/eds_watchers_test.go b/internal/xds/xdsclient/tests/eds_watchers_test.go index 56fbad117f81..81d344382e0e 100644 --- a/internal/xds/xdsclient/tests/eds_watchers_test.go +++ b/internal/xds/xdsclient/tests/eds_watchers_test.go @@ -79,10 +79,7 @@ func newEndpointsWatcher() *endpointsWatcher { func (ew *endpointsWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { defer onDone() - endpointsData, ok := rd.(*xdsresource.EndpointsResourceData) - if !ok { - return - } + endpointsData := rd.(*xdsresource.EndpointsResourceData) ew.updateCh.Send(endpointsUpdateErrTuple{update: endpointsData.Resource}) } func (ew *endpointsWatcher) ResourceError(err error, onDone func()) { diff --git a/internal/xds/xdsclient/tests/lds_watchers_test.go b/internal/xds/xdsclient/tests/lds_watchers_test.go index 20bf98efce48..7b4f2f6aa941 100644 --- a/internal/xds/xdsclient/tests/lds_watchers_test.go +++ b/internal/xds/xdsclient/tests/lds_watchers_test.go @@ -73,17 +73,7 @@ func newListenerWatcher() *listenerWatcher { } func (lw *listenerWatcher) ResourceChanged(rd xdsClient.ResourceData, onDone func()) { - if rd == nil { - lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) - onDone() - return - } - clusterData, ok := rd.(*xdsresource.ListenerResourceData) - if !ok { - lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) - onDone() - return - } + clusterData := rd.(*xdsresource.ListenerResourceData) lw.updateCh.Send(listenerUpdateErrTuple{update: clusterData.Resource}) onDone() } @@ -113,17 +103,7 @@ func newListenerWatcherMultiple(size int) *listenerWatcherMultiple { } func (lw *listenerWatcherMultiple) ResourceChanged(rd xdsClient.ResourceData, onDone func()) { - if rd == nil { - lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) - onDone() - return - } - listenerData, ok := rd.(*xdsresource.ListenerResourceData) - if !ok { - lw.updateCh.Send(listenerUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) - onDone() - return - } + listenerData := rd.(*xdsresource.ListenerResourceData) lw.updateCh.Send(listenerUpdateErrTuple{update: listenerData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/tests/rds_watchers_test.go b/internal/xds/xdsclient/tests/rds_watchers_test.go index 03efbb04536f..fa9dc447eb78 100644 --- a/internal/xds/xdsclient/tests/rds_watchers_test.go +++ b/internal/xds/xdsclient/tests/rds_watchers_test.go @@ -68,17 +68,7 @@ func newRouteConfigWatcher() *routeConfigWatcher { } func (rw *routeConfigWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { - if rd == nil { - rw.updateCh.Send(routeConfigUpdateErrTuple{err: fmt.Errorf("watcher received nil resource data")}) - onDone() - return - } - rcData, ok := rd.(*xdsresource.RouteConfigResourceData) - if !ok { - rw.updateCh.Send(routeConfigUpdateErrTuple{err: fmt.Errorf("watcher received unexpected resource data type %T", rd)}) - onDone() - return - } + rcData := rd.(*xdsresource.RouteConfigResourceData) rw.updateCh.Send(routeConfigUpdateErrTuple{update: rcData.Resource}) onDone() } diff --git a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go index a049ef1f21cf..91fd28499983 100644 --- a/internal/xds/xdsclient/xdsresource/cluster_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/cluster_resource_type.go @@ -89,16 +89,10 @@ func (ct clusterResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclien } if err := securityConfigValidator(internalOpts.BootstrapConfig, cluster.SecurityCfg); err != nil { - return &xdsclient.DecodeResult{ - Name: name, - Resource: &ClusterResourceData{Resource: ClusterUpdate{}}, - }, err + return &xdsclient.DecodeResult{Name: name, Resource: &ClusterResourceData{Resource: ClusterUpdate{}}}, err } - return &xdsclient.DecodeResult{ - Name: name, - Resource: &ClusterResourceData{Resource: cluster}, - }, nil + return &xdsclient.DecodeResult{Name: name, Resource: &ClusterResourceData{Resource: cluster}}, nil } // ClusterResourceData wraps the configuration of a Cluster resource as received diff --git a/internal/xds/xdsclient/xdsresource/listener_resource_type.go b/internal/xds/xdsclient/xdsresource/listener_resource_type.go index c661e4cd6660..48c8ab1b979b 100644 --- a/internal/xds/xdsclient/xdsresource/listener_resource_type.go +++ b/internal/xds/xdsclient/xdsresource/listener_resource_type.go @@ -118,16 +118,10 @@ func (lt listenerResourceType) Decode(resource xdsclient.AnyProto, gOpts xdsclie // Perform extra validation here. if err := listenerValidator(internalOpts.BootstrapConfig, listener); err != nil { - return &xdsclient.DecodeResult{ - Name: name, - Resource: &ListenerResourceData{Resource: ListenerUpdate{}}, - }, err + return &xdsclient.DecodeResult{Name: name, Resource: &ListenerResourceData{Resource: ListenerUpdate{}}}, err } - return &xdsclient.DecodeResult{ - Name: name, - Resource: &ListenerResourceData{Resource: listener}, - }, nil + return &xdsclient.DecodeResult{Name: name, Resource: &ListenerResourceData{Resource: listener}}, nil } // ListenerResourceData wraps the configuration of a Listener resource as From c17c44cb5bd0c05c1c2cb15ccaa55af783909f63 Mon Sep 17 00:00:00 2001 From: Vinothkumar Date: Tue, 16 Sep 2025 11:56:25 +0000 Subject: [PATCH 12/12] small tweaks --- internal/xds/clients/xdsclient/test/lds_watchers_test.go | 7 ++++++- internal/xds/clients/xdsclient/test/misc_watchers_test.go | 8 +++++++- internal/xds/xdsclient/tests/eds_watchers_test.go | 2 +- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/xds/clients/xdsclient/test/lds_watchers_test.go b/internal/xds/clients/xdsclient/test/lds_watchers_test.go index 142f6eebd780..ae4783921aec 100644 --- a/internal/xds/clients/xdsclient/test/lds_watchers_test.go +++ b/internal/xds/clients/xdsclient/test/lds_watchers_test.go @@ -75,7 +75,12 @@ func newListenerWatcher() *listenerWatcher { } func (lw *listenerWatcher) ResourceChanged(update xdsclient.ResourceData, onDone func()) { - lisData := update.(*listenerResourceData) + lisData, ok := update.(*listenerResourceData) + if !ok { + lw.resourceErrCh.Send(listenerUpdateErrTuple{resourceErr: fmt.Errorf("unexpected resource type: %T", update)}) + onDone() + return + } select { case <-lw.updateCh.C: default: diff --git a/internal/xds/clients/xdsclient/test/misc_watchers_test.go b/internal/xds/clients/xdsclient/test/misc_watchers_test.go index d23243fcc01a..e611c2240c43 100644 --- a/internal/xds/clients/xdsclient/test/misc_watchers_test.go +++ b/internal/xds/clients/xdsclient/test/misc_watchers_test.go @@ -62,8 +62,14 @@ func newTestLDSWatcher(client *xdsclient.XDSClient, name1, name2 string) *testLD } func (lw *testLDSWatcher) ResourceChanged(update xdsclient.ResourceData, onDone func()) { - lisData := update.(*listenerResourceData) + lisData, ok := update.(*listenerResourceData) + if !ok { + lw.updateCh.Send(listenerUpdateErrTuple{resourceErr: fmt.Errorf("unexpected resource type: %T", update)}) + onDone() + return + } lw.updateCh.Send(listenerUpdateErrTuple{update: lisData.Resource}) + lw.cancel1 = lw.client.WatchResource(xdsresource.V3ListenerURL, lw.name1, lw.lw1) lw.cancel2 = lw.client.WatchResource(xdsresource.V3ListenerURL, lw.name2, lw.lw2) onDone() diff --git a/internal/xds/xdsclient/tests/eds_watchers_test.go b/internal/xds/xdsclient/tests/eds_watchers_test.go index 81d344382e0e..98a507a5505c 100644 --- a/internal/xds/xdsclient/tests/eds_watchers_test.go +++ b/internal/xds/xdsclient/tests/eds_watchers_test.go @@ -78,9 +78,9 @@ func newEndpointsWatcher() *endpointsWatcher { } func (ew *endpointsWatcher) ResourceChanged(rd clientimpl.ResourceData, onDone func()) { - defer onDone() endpointsData := rd.(*xdsresource.EndpointsResourceData) ew.updateCh.Send(endpointsUpdateErrTuple{update: endpointsData.Resource}) + onDone() } func (ew *endpointsWatcher) ResourceError(err error, onDone func()) { // When used with a go-control-plane management server that continuously