From 52d9416739a60150e6ecfeea9737e4afbeabd8d9 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Wed, 10 Nov 2021 15:03:10 -0800 Subject: [PATCH] xds/client: move transport_helper from xdsclient to a separate struct (#4968) --- xds/csds/csds.go | 4 +- xds/googledirectpath/googlec2p.go | 2 +- xds/googledirectpath/googlec2p_test.go | 2 +- .../clusterresolver/clusterresolver_test.go | 2 +- xds/internal/httpfilter/fault/fault_test.go | 6 +- xds/internal/resolver/xds_resolver.go | 15 +- xds/internal/testutils/fakeserver/server.go | 14 - xds/internal/xdsclient/bootstrap/bootstrap.go | 2 +- .../xdsclient/bootstrap/bootstrap_test.go | 2 +- xds/internal/xdsclient/client.go | 197 ++------------ xds/internal/xdsclient/client_test.go | 39 +-- xds/internal/xdsclient/controller.go | 38 +++ .../xdsclient/controller/controller.go | 168 ++++++++++++ .../xdsclient/controller/loadreport.go | 144 ++++++++++ .../transport.go} | 210 +++++---------- .../ack_test.go => controller/v2_ack_test.go} | 33 ++- .../cds_test.go => controller/v2_cds_test.go} | 39 +-- .../xdsclient/controller/v2_client_test.go | 212 +++++++++++++++ .../eds_test.go => controller/v2_eds_test.go} | 14 +- .../lds_test.go => controller/v2_lds_test.go} | 12 +- .../rds_test.go => controller/v2_rds_test.go} | 15 +- .../v2_testutils_test.go} | 246 ++---------------- .../{ => controller/version}/v2/client.go | 120 ++------- .../{ => controller/version}/v2/loadreport.go | 0 .../{ => controller/version}/v3/client.go | 118 ++------- .../{ => controller/version}/v3/loadreport.go | 0 .../xdsclient/controller/version/version.go | 123 +++++++++ xds/internal/xdsclient/dump_test.go | 9 +- xds/internal/xdsclient/loadreport.go | 110 +------- xds/internal/xdsclient/loadreport_test.go | 11 +- xds/internal/xdsclient/pubsub/interface.go | 39 +++ xds/internal/xdsclient/watchers.go | 16 +- .../xdsclient/watchers_cluster_test.go | 36 +-- .../xdsclient/watchers_endpoints_test.go | 28 +- .../xdsclient/watchers_listener_test.go | 32 +-- xds/internal/xdsclient/watchers_route_test.go | 24 +- xds/internal/xdsclient/xdsclient_test.go | 5 +- .../xdsclient/xdsresource/filter_chain.go | 2 +- .../xdsresource/filter_chain_test.go | 2 +- xds/internal/xdsclient/xdsresource/type.go | 2 +- .../xdsclient/xdsresource/unmarshal_cds.go | 2 +- .../xdsresource/unmarshal_cds_test.go | 2 +- .../xdsresource/unmarshal_eds_test.go | 2 +- .../xdsclient/xdsresource/unmarshal_lds.go | 2 +- .../xdsresource/unmarshal_lds_test.go | 2 +- .../xdsclient/xdsresource/unmarshal_rds.go | 2 +- .../xdsresource/unmarshal_rds_test.go | 2 +- .../xdsresource}/version/version.go | 0 xds/xds.go | 16 +- 49 files changed, 1051 insertions(+), 1072 deletions(-) create mode 100644 xds/internal/xdsclient/controller.go create mode 100644 xds/internal/xdsclient/controller/controller.go create mode 100644 xds/internal/xdsclient/controller/loadreport.go rename xds/internal/xdsclient/{transport_helper.go => controller/transport.go} (60%) rename xds/internal/xdsclient/{v2/ack_test.go => controller/v2_ack_test.go} (94%) rename xds/internal/xdsclient/{v2/cds_test.go => controller/v2_cds_test.go} (84%) create mode 100644 xds/internal/xdsclient/controller/v2_client_test.go rename xds/internal/xdsclient/{v2/eds_test.go => controller/v2_eds_test.go} (93%) rename xds/internal/xdsclient/{v2/lds_test.go => controller/v2_lds_test.go} (94%) rename xds/internal/xdsclient/{v2/rds_test.go => controller/v2_rds_test.go} (93%) rename xds/internal/xdsclient/{v2/client_test.go => controller/v2_testutils_test.go} (63%) rename xds/internal/xdsclient/{ => controller/version}/v2/client.go (51%) rename xds/internal/xdsclient/{ => controller/version}/v2/loadreport.go (100%) rename xds/internal/xdsclient/{ => controller/version}/v3/client.go (52%) rename xds/internal/xdsclient/{ => controller/version}/v3/loadreport.go (100%) create mode 100644 xds/internal/xdsclient/controller/version/version.go create mode 100644 xds/internal/xdsclient/pubsub/interface.go rename xds/internal/{ => xdsclient/xdsresource}/version/version.go (100%) diff --git a/xds/csds/csds.go b/xds/csds/csds.go index a54afaf2cb35..f1e67f1ba63b 100644 --- a/xds/csds/csds.go +++ b/xds/csds/csds.go @@ -40,8 +40,8 @@ import ( "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/protobuf/types/known/timestamppb" - _ "google.golang.org/grpc/xds/internal/xdsclient/v2" // Register v2 xds_client. - _ "google.golang.org/grpc/xds/internal/xdsclient/v3" // Register v3 xds_client. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // Register v2 xds_client. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v3" // Register v3 xds_client. ) var ( diff --git a/xds/googledirectpath/googlec2p.go b/xds/googledirectpath/googlec2p.go index 4f753e49c71a..726ecec43187 100644 --- a/xds/googledirectpath/googlec2p.go +++ b/xds/googledirectpath/googlec2p.go @@ -39,9 +39,9 @@ import ( "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/resolver" _ "google.golang.org/grpc/xds" // To register xds resolvers and balancers. - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/structpb" ) diff --git a/xds/googledirectpath/googlec2p_test.go b/xds/googledirectpath/googlec2p_test.go index c8162317cc30..4b0ab5395503 100644 --- a/xds/googledirectpath/googlec2p_test.go +++ b/xds/googledirectpath/googlec2p_test.go @@ -29,9 +29,9 @@ import ( "google.golang.org/grpc" "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/resolver" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/structpb" ) diff --git a/xds/internal/balancer/clusterresolver/clusterresolver_test.go b/xds/internal/balancer/clusterresolver/clusterresolver_test.go index 808f3050e3e2..035188851a13 100644 --- a/xds/internal/balancer/clusterresolver/clusterresolver_test.go +++ b/xds/internal/balancer/clusterresolver/clusterresolver_test.go @@ -35,7 +35,7 @@ import ( "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" - _ "google.golang.org/grpc/xds/internal/xdsclient/v2" // V2 client registration. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // V2 client registration. ) const ( diff --git a/xds/internal/httpfilter/fault/fault_test.go b/xds/internal/httpfilter/fault/fault_test.go index 1bad1f92d65d..6ee5e654c7dc 100644 --- a/xds/internal/httpfilter/fault/fault_test.go +++ b/xds/internal/httpfilter/fault/fault_test.go @@ -52,9 +52,9 @@ import ( tpb "github.com/envoyproxy/go-control-plane/envoy/type/v3" testpb "google.golang.org/grpc/test/grpc_testing" - _ "google.golang.org/grpc/xds/internal/balancer" // Register the balancers. - _ "google.golang.org/grpc/xds/internal/resolver" // Register the xds_resolver. - _ "google.golang.org/grpc/xds/internal/xdsclient/v3" // Register the v3 xDS API client. + _ "google.golang.org/grpc/xds/internal/balancer" // Register the balancers. + _ "google.golang.org/grpc/xds/internal/resolver" // Register the xds_resolver. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v3" // Register the v3 xDS API client. ) const defaultTestTimeout = 10 * time.Second diff --git a/xds/internal/resolver/xds_resolver.go b/xds/internal/resolver/xds_resolver.go index 41e81a31865c..2192051ae2f6 100644 --- a/xds/internal/resolver/xds_resolver.go +++ b/xds/internal/resolver/xds_resolver.go @@ -73,9 +73,7 @@ func (b *xdsResolverBuilder) Build(t resolver.Target, cc resolver.ClientConn, op } defer func() { if retErr != nil { - if r.client != nil { - r.client.Close() - } + r.Close() } }() r.logger = prefixLogger(r) @@ -304,8 +302,15 @@ func (*xdsResolver) ResolveNow(o resolver.ResolveNowOptions) {} // Close closes the resolver, and also closes the underlying xdsClient. func (r *xdsResolver) Close() { - r.cancelWatch() - r.client.Close() + // Note that Close needs to check for nils even if some of them are always + // set in the constructor. This is because the constructor defers Close() in + // error cases, and the fields might not be set when the error happens. + if r.cancelWatch != nil { + r.cancelWatch() + } + if r.client != nil { + r.client.Close() + } r.closed.Fire() r.logger.Infof("Shutdown") } diff --git a/xds/internal/testutils/fakeserver/server.go b/xds/internal/testutils/fakeserver/server.go index d37c1c3ef0e2..94412171003e 100644 --- a/xds/internal/testutils/fakeserver/server.go +++ b/xds/internal/testutils/fakeserver/server.go @@ -20,7 +20,6 @@ package fakeserver import ( - "context" "fmt" "io" "net" @@ -29,7 +28,6 @@ import ( "github.com/golang/protobuf/proto" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/status" @@ -134,18 +132,6 @@ func StartServer() (*Server, func(), error) { return s, func() { server.Stop() }, nil } -// XDSClientConn returns a grpc.ClientConn connected to the fakeServer. -func (xdsS *Server) XDSClientConn() (*grpc.ClientConn, func(), error) { - ctx, cancel := context.WithTimeout(context.Background(), defaultDialTimeout) - defer cancel() - - cc, err := grpc.DialContext(ctx, xdsS.Address, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock()) - if err != nil { - return nil, nil, fmt.Errorf("grpc.DialContext(%s) failed: %v", xdsS.Address, err) - } - return cc, func() { cc.Close() }, nil -} - type xdsServer struct { reqChan *testutils.Channel respChan chan *Response diff --git a/xds/internal/xdsclient/bootstrap/bootstrap.go b/xds/internal/xdsclient/bootstrap/bootstrap.go index fd68367054ad..8123d94de5a3 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap.go @@ -38,7 +38,7 @@ import ( "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/internal/xds/env" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) const ( diff --git a/xds/internal/xdsclient/bootstrap/bootstrap_test.go b/xds/internal/xdsclient/bootstrap/bootstrap_test.go index 6348be1324ae..dd1536845704 100644 --- a/xds/internal/xdsclient/bootstrap/bootstrap_test.go +++ b/xds/internal/xdsclient/bootstrap/bootstrap_test.go @@ -38,7 +38,7 @@ import ( "google.golang.org/grpc/credentials/tls/certprovider" "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/xds/env" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) var ( diff --git a/xds/internal/xdsclient/client.go b/xds/internal/xdsclient/client.go index 56441142cc5a..13e8265b65c6 100644 --- a/xds/internal/xdsclient/client.go +++ b/xds/internal/xdsclient/client.go @@ -21,134 +21,16 @@ package xdsclient import ( - "context" - "errors" "fmt" - "sync" "time" - "github.com/golang/protobuf/proto" - "google.golang.org/grpc" - "google.golang.org/grpc/internal/backoff" "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpcsync" - "google.golang.org/grpc/keepalive" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" - "google.golang.org/grpc/xds/internal/xdsclient/load" "google.golang.org/grpc/xds/internal/xdsclient/pubsub" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" ) -var ( - m = make(map[version.TransportAPI]APIClientBuilder) -) - -// RegisterAPIClientBuilder registers a client builder for xDS transport protocol -// version specified by b.Version(). -// -// NOTE: this function must only be called during initialization time (i.e. in -// an init() function), and is not thread-safe. If multiple builders are -// registered for the same version, the one registered last will take effect. -func RegisterAPIClientBuilder(b APIClientBuilder) { - m[b.Version()] = b -} - -// getAPIClientBuilder returns the client builder registered for the provided -// xDS transport API version. -func getAPIClientBuilder(version version.TransportAPI) APIClientBuilder { - if b, ok := m[version]; ok { - return b - } - return nil -} - -// BuildOptions contains options to be passed to client builders. -type BuildOptions struct { - // Parent is a top-level xDS client which has the intelligence to take - // appropriate action based on xDS responses received from the management - // server. - Parent UpdateHandler - // Validator performs post unmarshal validation checks. - Validator xdsresource.UpdateValidatorFunc - // NodeProto contains the Node proto to be used in xDS requests. The actual - // type depends on the transport protocol version used. - NodeProto proto.Message - // Backoff returns the amount of time to backoff before retrying broken - // streams. - Backoff func(int) time.Duration - // Logger provides enhanced logging capabilities. - Logger *grpclog.PrefixLogger -} - -// APIClientBuilder creates an xDS client for a specific xDS transport protocol -// version. -type APIClientBuilder interface { - // Build builds a transport protocol specific implementation of the xDS - // client based on the provided clientConn to the management server and the - // provided options. - Build(*grpc.ClientConn, BuildOptions) (APIClient, error) - // Version returns the xDS transport protocol version used by clients build - // using this builder. - Version() version.TransportAPI -} - -// APIClient represents the functionality provided by transport protocol -// version specific implementations of the xDS client. -// -// TODO: unexport this interface and all the methods after the PR to make -// xdsClient sharable by clients. AddWatch and RemoveWatch are exported for -// v2/v3 to override because they need to keep track of LDS name for RDS to use. -// After the share xdsClient change, that's no longer necessary. After that, we -// will still keep this interface for testing purposes. -type APIClient interface { - // AddWatch adds a watch for an xDS resource given its type and name. - AddWatch(xdsresource.ResourceType, string) - - // RemoveWatch cancels an already registered watch for an xDS resource - // given its type and name. - RemoveWatch(xdsresource.ResourceType, string) - - // reportLoad starts an LRS stream to periodically report load using the - // provided ClientConn, which represent a connection to the management - // server. - reportLoad(ctx context.Context, cc *grpc.ClientConn, opts loadReportingOptions) - - // Close cleans up resources allocated by the API client. - Close() -} - -// loadReportingOptions contains configuration knobs for reporting load data. -type loadReportingOptions struct { - loadStore *load.Store -} - -// UpdateHandler receives and processes (by taking appropriate actions) xDS -// resource updates from an APIClient for a specific version. -type UpdateHandler interface { - // NewListeners handles updates to xDS listener resources. - NewListeners(map[string]xdsresource.ListenerUpdateErrTuple, xdsresource.UpdateMetadata) - // NewRouteConfigs handles updates to xDS RouteConfiguration resources. - NewRouteConfigs(map[string]xdsresource.RouteConfigUpdateErrTuple, xdsresource.UpdateMetadata) - // NewClusters handles updates to xDS Cluster resources. - NewClusters(map[string]xdsresource.ClusterUpdateErrTuple, xdsresource.UpdateMetadata) - // NewEndpoints handles updates to xDS ClusterLoadAssignment (or tersely - // referred to as Endpoints) resources. - NewEndpoints(map[string]xdsresource.EndpointsUpdateErrTuple, xdsresource.UpdateMetadata) - // NewConnectionError handles connection errors from the xDS stream. The - // error will be reported to all the resource watchers. - NewConnectionError(err error) -} - -// Function to be overridden in tests. -var newAPIClient = func(apiVersion version.TransportAPI, cc *grpc.ClientConn, opts BuildOptions) (APIClient, error) { - cb := getAPIClientBuilder(apiVersion) - if cb == nil { - return nil, fmt.Errorf("no client builder for xDS API version: %v", apiVersion) - } - return cb.Build(cc, opts) -} - // clientImpl is the real implementation of the xds client. The exported Client // is a wrapper of this struct with a ref count. // @@ -157,83 +39,39 @@ var newAPIClient = func(apiVersion version.TransportAPI, cc *grpc.ClientConn, op // style of ccBalancerWrapper so that the Client type does not implement these // exported methods. type clientImpl struct { - done *grpcsync.Event - config *bootstrap.Config - cc *grpc.ClientConn // Connection to the management server. - apiClient APIClient + done *grpcsync.Event + config *bootstrap.Config + + controller controllerInterface logger *grpclog.PrefixLogger pubsub *pubsub.Pubsub - - // Changes to map lrsClients and the lrsClient inside the map need to be - // protected by lrsMu. - lrsMu sync.Mutex - lrsClients map[string]*lrsClient } // newWithConfig returns a new xdsClient with the given config. func newWithConfig(config *bootstrap.Config, watchExpiryTimeout time.Duration) (_ *clientImpl, retErr error) { - switch { - case config.XDSServer == nil: - return nil, errors.New("xds: no xds_server provided") - case config.XDSServer.ServerURI == "": - return nil, errors.New("xds: no xds_server name provided in options") - case config.XDSServer.Creds == nil: - return nil, errors.New("xds: no credentials provided in options") - case config.XDSServer.NodeProto == nil: - return nil, errors.New("xds: no node_proto provided in options") - } - - dopts := []grpc.DialOption{ - config.XDSServer.Creds, - grpc.WithKeepaliveParams(keepalive.ClientParameters{ - Time: 5 * time.Minute, - Timeout: 20 * time.Second, - }), - } - c := &clientImpl{ - done: grpcsync.NewEvent(), - config: config, - lrsClients: make(map[string]*lrsClient), + done: grpcsync.NewEvent(), + config: config, } defer func() { if retErr != nil { - if c.cc != nil { - c.cc.Close() - } - if c.pubsub != nil { - c.pubsub.Close() - } - if c.apiClient != nil { - c.apiClient.Close() - } + c.Close() } }() - cc, err := grpc.Dial(config.XDSServer.ServerURI, dopts...) - if err != nil { - // An error from a non-blocking dial indicates something serious. - return nil, fmt.Errorf("xds: failed to dial balancer {%s}: %v", config.XDSServer.ServerURI, err) - } - c.cc = cc c.logger = prefixLogger(c) c.logger.Infof("Created ClientConn to xDS management server: %s", config.XDSServer) c.pubsub = pubsub.New(watchExpiryTimeout, c.logger) - apiClient, err := newAPIClient(config.XDSServer.TransportAPI, cc, BuildOptions{ - Parent: c, - Validator: c.updateValidator, - NodeProto: config.XDSServer.NodeProto, - Backoff: backoff.DefaultExponential.Backoff, - Logger: c.logger, - }) + controller, err := newController(config.XDSServer, c.pubsub, c.updateValidator, c.logger) if err != nil { - return nil, err + return nil, fmt.Errorf("xds: failed to connect to the control plane: %v", err) } - c.apiClient = apiClient + c.controller = controller + c.logger.Infof("Created") return c, nil } @@ -252,9 +90,16 @@ func (c *clientImpl) Close() { c.done.Fire() // TODO: Should we invoke the registered callbacks here with an error that // the client is closed? - c.apiClient.Close() - c.cc.Close() - c.pubsub.Close() + + // Note that Close needs to check for nils even if some of them are always + // set in the constructor. This is because the constructor defers Close() in + // error cases, and the fields might not be set when the error happens. + if c.controller != nil { + c.controller.Close() + } + if c.pubsub != nil { + c.pubsub.Close() + } c.logger.Infof("Shutdown") } diff --git a/xds/internal/xdsclient/client_test.go b/xds/internal/xdsclient/client_test.go index f20d6112a84c..cd2b98950a65 100644 --- a/xds/internal/xdsclient/client_test.go +++ b/xds/internal/xdsclient/client_test.go @@ -26,6 +26,9 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "google.golang.org/grpc/internal/grpclog" + "google.golang.org/grpc/xds/internal/xdsclient/load" + "google.golang.org/grpc/xds/internal/xdsclient/pubsub" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/protobuf/types/known/anypb" @@ -35,7 +38,6 @@ import ( "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" xdstestutils "google.golang.org/grpc/xds/internal/testutils" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" "google.golang.org/protobuf/testing/protocmp" ) @@ -79,24 +81,24 @@ func clientOpts(balancerName string, overrideWatchExpiryTimeout bool) (*bootstra }, watchExpiryTimeout } -type testAPIClient struct { +type testController struct { done *grpcsync.Event addWatches map[xdsresource.ResourceType]*testutils.Channel removeWatches map[xdsresource.ResourceType]*testutils.Channel } -func overrideNewAPIClient() (*testutils.Channel, func()) { - origNewAPIClient := newAPIClient +func overrideNewController() (*testutils.Channel, func()) { + origNewController := newController ch := testutils.NewChannel() - newAPIClient = func(apiVersion version.TransportAPI, cc *grpc.ClientConn, opts BuildOptions) (APIClient, error) { - ret := newTestAPIClient() + newController = func(config *bootstrap.ServerConfig, pubsub *pubsub.Pubsub, validator xdsresource.UpdateValidatorFunc, logger *grpclog.PrefixLogger) (controllerInterface, error) { + ret := newTestController() ch.Send(ret) return ret, nil } - return ch, func() { newAPIClient = origNewAPIClient } + return ch, func() { newController = origNewController } } -func newTestAPIClient() *testAPIClient { +func newTestController() *testController { addWatches := map[xdsresource.ResourceType]*testutils.Channel{ xdsresource.ListenerResource: testutils.NewChannel(), xdsresource.RouteConfigResource: testutils.NewChannel(), @@ -109,32 +111,33 @@ func newTestAPIClient() *testAPIClient { xdsresource.ClusterResource: testutils.NewChannel(), xdsresource.EndpointsResource: testutils.NewChannel(), } - return &testAPIClient{ + return &testController{ done: grpcsync.NewEvent(), addWatches: addWatches, removeWatches: removeWatches, } } -func (c *testAPIClient) AddWatch(resourceType xdsresource.ResourceType, resourceName string) { +func (c *testController) AddWatch(resourceType xdsresource.ResourceType, resourceName string) { c.addWatches[resourceType].Send(resourceName) } -func (c *testAPIClient) RemoveWatch(resourceType xdsresource.ResourceType, resourceName string) { +func (c *testController) RemoveWatch(resourceType xdsresource.ResourceType, resourceName string) { c.removeWatches[resourceType].Send(resourceName) } -func (c *testAPIClient) reportLoad(context.Context, *grpc.ClientConn, loadReportingOptions) { +func (c *testController) ReportLoad(server string) (*load.Store, func()) { + panic("ReportLoad is not implemented") } -func (c *testAPIClient) Close() { +func (c *testController) Close() { c.done.Fire() } // TestWatchCallAnotherWatch covers the case where watch() is called inline by a // callback. It makes sure it doesn't cause a deadlock. func (s) TestWatchCallAnotherWatch(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -149,7 +152,7 @@ func (s) TestWatchCallAnotherWatch(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() firstTime := true @@ -269,7 +272,7 @@ func (s) TestClientNewSingleton(t *testing.T) { } defer func() { bootstrapNewConfig = oldBootstrapNewConfig }() - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() // The first New(). Should create a Client and a new APIClient. @@ -284,7 +287,7 @@ func (s) TestClientNewSingleton(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) // Call New() again. They should all return the same client implementation, // and should not create new API client. @@ -343,7 +346,7 @@ func (s) TestClientNewSingleton(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient2 := c2.(*testAPIClient) + apiClient2 := c2.(*testController) // The client wrapper with ref count should be the same. if client2 != client { diff --git a/xds/internal/xdsclient/controller.go b/xds/internal/xdsclient/controller.go new file mode 100644 index 000000000000..431a14498e1f --- /dev/null +++ b/xds/internal/xdsclient/controller.go @@ -0,0 +1,38 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package xdsclient + +import ( + "google.golang.org/grpc/internal/grpclog" + "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/controller" + "google.golang.org/grpc/xds/internal/xdsclient/load" + "google.golang.org/grpc/xds/internal/xdsclient/pubsub" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" +) + +type controllerInterface interface { + AddWatch(resourceType xdsresource.ResourceType, resourceName string) + RemoveWatch(resourceType xdsresource.ResourceType, resourceName string) + ReportLoad(server string) (*load.Store, func()) + Close() +} + +var newController = func(config *bootstrap.ServerConfig, pubsub *pubsub.Pubsub, validator xdsresource.UpdateValidatorFunc, logger *grpclog.PrefixLogger) (controllerInterface, error) { + return controller.New(config, pubsub, validator, logger) +} diff --git a/xds/internal/xdsclient/controller/controller.go b/xds/internal/xdsclient/controller/controller.go new file mode 100644 index 000000000000..09283d7423ff --- /dev/null +++ b/xds/internal/xdsclient/controller/controller.go @@ -0,0 +1,168 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package controller contains implementation to connect to the control plane. +// Including starting the ClientConn, starting the xDS stream, and +// sending/receiving messages. +// +// All the messages are parsed by the resource package (e.g. +// UnmarshalListener()) and sent to the Pubsub watchers. +package controller + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/internal/backoff" + "google.golang.org/grpc/internal/buffer" + "google.golang.org/grpc/internal/grpclog" + "google.golang.org/grpc/keepalive" + "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/controller/version" + "google.golang.org/grpc/xds/internal/xdsclient/pubsub" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" +) + +// Controller manages the connection and stream to the control plane. +// +// It keeps track of what resources are being watched, and send new requests +// when new watches are added. +// +// It takes a pubsub (as an interface) as input. When a response is received, +// it's parsed, and the updates are sent to the pubsub. +type Controller struct { + config *bootstrap.ServerConfig + updateHandler pubsub.UpdateHandler + updateValidator xdsresource.UpdateValidatorFunc + logger *grpclog.PrefixLogger + + cc *grpc.ClientConn // Connection to the management server. + vClient version.VersionedClient + stopRunGoroutine context.CancelFunc + + backoff func(int) time.Duration + streamCh chan grpc.ClientStream + sendCh *buffer.Unbounded + + mu sync.Mutex + // Message specific watch infos, protected by the above mutex. These are + // written to, after successfully reading from the update channel, and are + // read from when recovering from a broken stream to resend the xDS + // messages. When the user of this client object cancels a watch call, + // these are set to nil. All accesses to the map protected and any value + // inside the map should be protected with the above mutex. + watchMap map[xdsresource.ResourceType]map[string]bool + // versionMap contains the version that was acked (the version in the ack + // request that was sent on wire). The key is rType, the value is the + // version string, becaues the versions for different resource types should + // be independent. + versionMap map[xdsresource.ResourceType]string + // nonceMap contains the nonce from the most recent received response. + nonceMap map[xdsresource.ResourceType]string + + // Changes to map lrsClients and the lrsClient inside the map need to be + // protected by lrsMu. + // + // TODO: after LRS refactoring, each controller should only manage the LRS + // stream to its server. LRS streams to other servers should be managed by + // other controllers. + lrsMu sync.Mutex + lrsClients map[string]*lrsClient +} + +// New creates a new controller. +func New(config *bootstrap.ServerConfig, updateHandler pubsub.UpdateHandler, validator xdsresource.UpdateValidatorFunc, logger *grpclog.PrefixLogger) (_ *Controller, retErr error) { + switch { + case config == nil: + return nil, errors.New("xds: no xds_server provided") + case config.ServerURI == "": + return nil, errors.New("xds: no xds_server name provided in options") + case config.Creds == nil: + return nil, errors.New("xds: no credentials provided in options") + case config.NodeProto == nil: + return nil, errors.New("xds: no node_proto provided in options") + } + + dopts := []grpc.DialOption{ + config.Creds, + grpc.WithKeepaliveParams(keepalive.ClientParameters{ + Time: 5 * time.Minute, + Timeout: 20 * time.Second, + }), + } + + ret := &Controller{ + config: config, + updateValidator: validator, + updateHandler: updateHandler, + + backoff: backoff.DefaultExponential.Backoff, // TODO: should this be configurable? + streamCh: make(chan grpc.ClientStream, 1), + sendCh: buffer.NewUnbounded(), + watchMap: make(map[xdsresource.ResourceType]map[string]bool), + versionMap: make(map[xdsresource.ResourceType]string), + nonceMap: make(map[xdsresource.ResourceType]string), + + lrsClients: make(map[string]*lrsClient), + } + + defer func() { + if retErr != nil { + ret.Close() + } + }() + + cc, err := grpc.Dial(config.ServerURI, dopts...) + if err != nil { + // An error from a non-blocking dial indicates something serious. + return nil, fmt.Errorf("xds: failed to dial control plane {%s}: %v", config.ServerURI, err) + } + ret.cc = cc + + builder := version.GetAPIClientBuilder(config.TransportAPI) + if builder == nil { + return nil, fmt.Errorf("no client builder for xDS API version: %v", config.TransportAPI) + } + apiClient, err := builder(version.BuildOptions{NodeProto: config.NodeProto, Logger: logger}) + if err != nil { + return nil, err + } + ret.vClient = apiClient + + ctx, cancel := context.WithCancel(context.Background()) + ret.stopRunGoroutine = cancel + go ret.run(ctx) + + return ret, nil +} + +// Close closes the controller. +func (t *Controller) Close() { + // Note that Close needs to check for nils even if some of them are always + // set in the constructor. This is because the constructor defers Close() in + // error cases, and the fields might not be set when the error happens. + if t.stopRunGoroutine != nil { + t.stopRunGoroutine() + } + if t.cc != nil { + t.cc.Close() + } +} diff --git a/xds/internal/xdsclient/controller/loadreport.go b/xds/internal/xdsclient/controller/loadreport.go new file mode 100644 index 000000000000..f8cfd017e415 --- /dev/null +++ b/xds/internal/xdsclient/controller/loadreport.go @@ -0,0 +1,144 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package controller + +import ( + "context" + + "google.golang.org/grpc" + "google.golang.org/grpc/xds/internal/xdsclient/controller/version" + "google.golang.org/grpc/xds/internal/xdsclient/load" +) + +// ReportLoad starts an load reporting stream to the given server. If the server +// is not an empty string, and is different from the management server, a new +// ClientConn will be created. +// +// The same options used for creating the Client will be used (including +// NodeProto, and dial options if necessary). +// +// It returns a Store for the user to report loads, a function to cancel the +// load reporting stream. +// +// TODO: LRS refactor; maybe a new controller should be created for a separate +// server, so that the same stream can be shared by different reporters to the +// same server, even if they originate from different Controllers. +func (c *Controller) ReportLoad(server string) (*load.Store, func()) { + c.lrsMu.Lock() + defer c.lrsMu.Unlock() + + // If there's already a client to this server, use it. Otherwise, create + // one. + lrsC, ok := c.lrsClients[server] + if !ok { + lrsC = newLRSClient(c, server) + c.lrsClients[server] = lrsC + } + + store := lrsC.ref() + return store, func() { + // This is a callback, need to hold lrsMu. + c.lrsMu.Lock() + defer c.lrsMu.Unlock() + if lrsC.unRef() { + // Delete the lrsClient from map if this is the last reference. + delete(c.lrsClients, server) + } + } +} + +// lrsClient maps to one lrsServer. It contains: +// - a ClientConn to this server (only if it's different from the management +// server) +// - a load.Store that contains loads only for this server +type lrsClient struct { + parent *Controller + server string + + cc *grpc.ClientConn // nil if the server is same as the management server + refCount int + cancelStream func() + loadStore *load.Store +} + +// newLRSClient creates a new LRS stream to the server. +func newLRSClient(parent *Controller, server string) *lrsClient { + return &lrsClient{ + parent: parent, + server: server, + refCount: 0, + } +} + +// ref increments the refCount. If this is the first ref, it starts the LRS stream. +// +// Not thread-safe, caller needs to synchronize. +func (lrsC *lrsClient) ref() *load.Store { + lrsC.refCount++ + if lrsC.refCount == 1 { + lrsC.startStream() + } + return lrsC.loadStore +} + +// unRef decrements the refCount, and closes the stream if refCount reaches 0 +// (and close the cc if cc is not xDS cc). It returns whether refCount reached 0 +// after this call. +// +// Not thread-safe, caller needs to synchronize. +func (lrsC *lrsClient) unRef() (closed bool) { + lrsC.refCount-- + if lrsC.refCount != 0 { + return false + } + lrsC.parent.logger.Infof("Stopping load report to server: %s", lrsC.server) + lrsC.cancelStream() + if lrsC.cc != nil { + lrsC.cc.Close() + } + return true +} + +// startStream starts the LRS stream to the server. If server is not the same +// management server from the parent, it also creates a ClientConn. +func (lrsC *lrsClient) startStream() { + var cc *grpc.ClientConn + + lrsC.parent.logger.Infof("Starting load report to server: %s", lrsC.server) + if lrsC.server == "" || lrsC.server == lrsC.parent.config.ServerURI { + // Reuse the xDS client if server is the same. + cc = lrsC.parent.cc + } else { + lrsC.parent.logger.Infof("LRS server is different from management server, starting a new ClientConn") + ccNew, err := grpc.Dial(lrsC.server, lrsC.parent.config.Creds) + if err != nil { + // An error from a non-blocking dial indicates something serious. + lrsC.parent.logger.Infof("xds: failed to dial load report server {%s}: %v", lrsC.server, err) + return + } + cc = ccNew + lrsC.cc = ccNew + } + + var ctx context.Context + ctx, lrsC.cancelStream = context.WithCancel(context.Background()) + + // Create the store and stream. + lrsC.loadStore = load.NewStore() + go lrsC.parent.reportLoad(ctx, cc, version.LoadReportingOptions{LoadStore: lrsC.loadStore}) +} diff --git a/xds/internal/xdsclient/transport_helper.go b/xds/internal/xdsclient/controller/transport.go similarity index 60% rename from xds/internal/xdsclient/transport_helper.go rename to xds/internal/xdsclient/controller/transport.go index ba456eb390ea..b7746ed883c3 100644 --- a/xds/internal/xdsclient/transport_helper.go +++ b/xds/internal/xdsclient/controller/transport.go @@ -16,140 +16,23 @@ * */ -package xdsclient +package controller import ( "context" - "sync" + "fmt" "time" "github.com/golang/protobuf/proto" + "google.golang.org/grpc" + controllerversion "google.golang.org/grpc/xds/internal/xdsclient/controller/version" + xdsresourceversion "google.golang.org/grpc/xds/internal/xdsclient/controller/version" "google.golang.org/grpc/xds/internal/xdsclient/load" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" - - "google.golang.org/grpc" - "google.golang.org/grpc/internal/buffer" - "google.golang.org/grpc/internal/grpclog" ) -// ErrResourceTypeUnsupported is an error used to indicate an unsupported xDS -// resource type. The wrapped ErrStr contains the details. -type ErrResourceTypeUnsupported struct { - ErrStr string -} - -// Error helps implements the error interface. -func (e ErrResourceTypeUnsupported) Error() string { - return e.ErrStr -} - -// VersionedClient is the interface to be provided by the transport protocol -// specific client implementations. This mainly deals with the actual sending -// and receiving of messages. -type VersionedClient interface { - // NewStream returns a new xDS client stream specific to the underlying - // transport protocol version. - NewStream(ctx context.Context) (grpc.ClientStream, error) - - // SendRequest constructs and sends out a DiscoveryRequest message specific - // to the underlying transport protocol version. - SendRequest(s grpc.ClientStream, resourceNames []string, rType xdsresource.ResourceType, version, nonce, errMsg string) error - - // RecvResponse uses the provided stream to receive a response specific to - // the underlying transport protocol version. - RecvResponse(s grpc.ClientStream) (proto.Message, error) - - // HandleResponse parses and validates the received response and notifies - // the top-level client which in turn notifies the registered watchers. - // - // Return values are: resourceType, version, nonce, error. - // If the provided protobuf message contains a resource type which is not - // supported, implementations must return an error of type - // ErrResourceTypeUnsupported. - HandleResponse(proto.Message) (xdsresource.ResourceType, string, string, error) - - // NewLoadStatsStream returns a new LRS client stream specific to the underlying - // transport protocol version. - NewLoadStatsStream(ctx context.Context, cc *grpc.ClientConn) (grpc.ClientStream, error) - - // SendFirstLoadStatsRequest constructs and sends the first request on the - // LRS stream. - SendFirstLoadStatsRequest(s grpc.ClientStream) error - - // HandleLoadStatsResponse receives the first response from the server which - // contains the load reporting interval and the clusters for which the - // server asks the client to report load for. - // - // If the response sets SendAllClusters to true, the returned clusters is - // nil. - HandleLoadStatsResponse(s grpc.ClientStream) (clusters []string, _ time.Duration, _ error) - - // SendLoadStatsRequest will be invoked at regular intervals to send load - // report with load data reported since the last time this method was - // invoked. - SendLoadStatsRequest(s grpc.ClientStream, loads []*load.Data) error -} - -// TransportHelper contains all xDS transport protocol related functionality -// which is common across different versioned client implementations. -// -// TransportHelper takes care of sending and receiving xDS requests and -// responses on an ADS stream. It also takes care of ACK/NACK handling. It -// delegates to the actual versioned client implementations wherever -// appropriate. -// -// Implements the APIClient interface which makes it possible for versioned -// client implementations to embed this type, and thereby satisfy the interface -// requirements. -type TransportHelper struct { - cancelCtx context.CancelFunc - - vClient VersionedClient - logger *grpclog.PrefixLogger - backoff func(int) time.Duration - streamCh chan grpc.ClientStream - sendCh *buffer.Unbounded - - mu sync.Mutex - // Message specific watch infos, protected by the above mutex. These are - // written to, after successfully reading from the update channel, and are - // read from when recovering from a broken stream to resend the xDS - // messages. When the user of this client object cancels a watch call, - // these are set to nil. All accesses to the map protected and any value - // inside the map should be protected with the above mutex. - watchMap map[xdsresource.ResourceType]map[string]bool - // versionMap contains the version that was acked (the version in the ack - // request that was sent on wire). The key is rType, the value is the - // version string, becaues the versions for different resource types should - // be independent. - versionMap map[xdsresource.ResourceType]string - // nonceMap contains the nonce from the most recent received response. - nonceMap map[xdsresource.ResourceType]string -} - -// NewTransportHelper creates a new transport helper to be used by versioned -// client implementations. -func NewTransportHelper(vc VersionedClient, logger *grpclog.PrefixLogger, backoff func(int) time.Duration) *TransportHelper { - ctx, cancelCtx := context.WithCancel(context.Background()) - t := &TransportHelper{ - cancelCtx: cancelCtx, - vClient: vc, - logger: logger, - backoff: backoff, - - streamCh: make(chan grpc.ClientStream, 1), - sendCh: buffer.NewUnbounded(), - watchMap: make(map[xdsresource.ResourceType]map[string]bool), - versionMap: make(map[xdsresource.ResourceType]string), - nonceMap: make(map[xdsresource.ResourceType]string), - } - - go t.run(ctx) - return t -} - // AddWatch adds a watch for an xDS resource given its type and name. -func (t *TransportHelper) AddWatch(rType xdsresource.ResourceType, resourceName string) { +func (t *Controller) AddWatch(rType xdsresource.ResourceType, resourceName string) { t.sendCh.Put(&watchAction{ rType: rType, remove: false, @@ -159,7 +42,7 @@ func (t *TransportHelper) AddWatch(rType xdsresource.ResourceType, resourceName // RemoveWatch cancels an already registered watch for an xDS resource // given its type and name. -func (t *TransportHelper) RemoveWatch(rType xdsresource.ResourceType, resourceName string) { +func (t *Controller) RemoveWatch(rType xdsresource.ResourceType, resourceName string) { t.sendCh.Put(&watchAction{ rType: rType, remove: true, @@ -167,15 +50,10 @@ func (t *TransportHelper) RemoveWatch(rType xdsresource.ResourceType, resourceNa }) } -// Close closes the transport helper. -func (t *TransportHelper) Close() { - t.cancelCtx() -} - // run starts an ADS stream (and backs off exponentially, if the previous // stream failed without receiving a single reply) and runs the sender and // receiver routines to send and receive data from the stream respectively. -func (t *TransportHelper) run(ctx context.Context) { +func (t *Controller) run(ctx context.Context) { go t.send(ctx) // TODO: start a goroutine monitoring ClientConn's connectivity state, and // report error (and log) when stats is transient failure. @@ -201,7 +79,7 @@ func (t *TransportHelper) run(ctx context.Context) { } retries++ - stream, err := t.vClient.NewStream(ctx) + stream, err := t.vClient.NewStream(ctx, t.cc) if err != nil { t.logger.Warningf("xds: ADS stream creation failed: %v", err) continue @@ -235,7 +113,7 @@ func (t *TransportHelper) run(ctx context.Context) { // Note that this goroutine doesn't do anything to the old stream when there's a // new one. In fact, there should be only one stream in progress, and new one // should only be created when the old one fails (recv returns an error). -func (t *TransportHelper) send(ctx context.Context) { +func (t *Controller) send(ctx context.Context) { var stream grpc.ClientStream for { select { @@ -288,7 +166,7 @@ func (t *TransportHelper) send(ctx context.Context) { // that here because the stream has just started and Send() usually returns // quickly (once it pushes the message onto the transport layer) and is only // ever blocked if we don't have enough flow control quota. -func (t *TransportHelper) sendExisting(stream grpc.ClientStream) bool { +func (t *Controller) sendExisting(stream grpc.ClientStream) bool { t.mu.Lock() defer t.mu.Unlock() @@ -308,16 +186,19 @@ func (t *TransportHelper) sendExisting(stream grpc.ClientStream) bool { // recv receives xDS responses on the provided ADS stream and branches out to // message specific handlers. -func (t *TransportHelper) recv(stream grpc.ClientStream) bool { +func (t *Controller) recv(stream grpc.ClientStream) bool { success := false for { resp, err := t.vClient.RecvResponse(stream) if err != nil { + t.updateHandler.NewConnectionError(err) t.logger.Warningf("ADS stream is closed with error: %v", err) return success } - rType, version, nonce, err := t.vClient.HandleResponse(resp) - if e, ok := err.(ErrResourceTypeUnsupported); ok { + + rType, version, nonce, err := t.handleResponse(resp) + + if e, ok := err.(xdsresourceversion.ErrResourceTypeUnsupported); ok { t.logger.Warningf("%s", e.ErrStr) continue } @@ -343,6 +224,43 @@ func (t *TransportHelper) recv(stream grpc.ClientStream) bool { } } +func (t *Controller) handleResponse(resp proto.Message) (xdsresource.ResourceType, string, string, error) { + rType, resource, version, nonce, err := t.vClient.ParseResponse(resp) + if err != nil { + return rType, version, nonce, err + } + opts := &xdsresource.UnmarshalOptions{ + Version: version, + Resources: resource, + Logger: t.logger, + UpdateValidator: t.updateValidator, + } + var md xdsresource.UpdateMetadata + switch rType { + case xdsresource.ListenerResource: + var update map[string]xdsresource.ListenerUpdateErrTuple + update, md, err = xdsresource.UnmarshalListener(opts) + t.updateHandler.NewListeners(update, md) + case xdsresource.RouteConfigResource: + var update map[string]xdsresource.RouteConfigUpdateErrTuple + update, md, err = xdsresource.UnmarshalRouteConfig(opts) + t.updateHandler.NewRouteConfigs(update, md) + case xdsresource.ClusterResource: + var update map[string]xdsresource.ClusterUpdateErrTuple + update, md, err = xdsresource.UnmarshalCluster(opts) + t.updateHandler.NewClusters(update, md) + case xdsresource.EndpointsResource: + var update map[string]xdsresource.EndpointsUpdateErrTuple + update, md, err = xdsresource.UnmarshalEndpoints(opts) + t.updateHandler.NewEndpoints(update, md) + default: + return rType, "", "", xdsresourceversion.ErrResourceTypeUnsupported{ + ErrStr: fmt.Sprintf("Resource type %v unknown in response from server", rType), + } + } + return rType, version, nonce, err +} + func mapToSlice(m map[string]bool) []string { ret := make([]string, 0, len(m)) for i := range m { @@ -360,7 +278,7 @@ type watchAction struct { // processWatchInfo pulls the fields needed by the request from a watchAction. // // It also updates the watch map. -func (t *TransportHelper) processWatchInfo(w *watchAction) (target []string, rType xdsresource.ResourceType, ver, nonce string) { +func (t *Controller) processWatchInfo(w *watchAction) (target []string, rType xdsresource.ResourceType, ver, nonce string) { t.mu.Lock() defer t.mu.Unlock() @@ -404,7 +322,7 @@ type ackAction struct { // processAckInfo pulls the fields needed by the ack request from a ackAction. // // If no active watch is found for this ack, it returns false for send. -func (t *TransportHelper) processAckInfo(ack *ackAction, stream grpc.ClientStream) (target []string, rType xdsresource.ResourceType, version, nonce string, send bool) { +func (t *Controller) processAckInfo(ack *ackAction, stream grpc.ClientStream) (target []string, rType xdsresource.ResourceType, version, nonce string, send bool) { if ack.stream != stream { // If ACK's stream isn't the current sending stream, this means the ACK // was pushed to queue before the old stream broke, and a new stream has @@ -450,7 +368,7 @@ func (t *TransportHelper) processAckInfo(ack *ackAction, stream grpc.ClientStrea // reportLoad starts an LRS stream to report load data to the management server. // It blocks until the context is cancelled. -func (t *TransportHelper) reportLoad(ctx context.Context, cc *grpc.ClientConn, opts loadReportingOptions) { +func (t *Controller) reportLoad(ctx context.Context, cc *grpc.ClientConn, opts controllerversion.LoadReportingOptions) { retries := 0 for { if ctx.Err() != nil { @@ -472,28 +390,28 @@ func (t *TransportHelper) reportLoad(ctx context.Context, cc *grpc.ClientConn, o retries++ stream, err := t.vClient.NewLoadStatsStream(ctx, cc) if err != nil { - logger.Warningf("lrs: failed to create stream: %v", err) + t.logger.Warningf("lrs: failed to create stream: %v", err) continue } - logger.Infof("lrs: created LRS stream") + t.logger.Infof("lrs: created LRS stream") if err := t.vClient.SendFirstLoadStatsRequest(stream); err != nil { - logger.Warningf("lrs: failed to send first request: %v", err) + t.logger.Warningf("lrs: failed to send first request: %v", err) continue } clusters, interval, err := t.vClient.HandleLoadStatsResponse(stream) if err != nil { - logger.Warning(err) + t.logger.Warningf("%v", err) continue } retries = 0 - t.sendLoads(ctx, stream, opts.loadStore, clusters, interval) + t.sendLoads(ctx, stream, opts.LoadStore, clusters, interval) } } -func (t *TransportHelper) sendLoads(ctx context.Context, stream grpc.ClientStream, store *load.Store, clusterNames []string, interval time.Duration) { +func (t *Controller) sendLoads(ctx context.Context, stream grpc.ClientStream, store *load.Store, clusterNames []string, interval time.Duration) { tick := time.NewTicker(interval) defer tick.Stop() for { @@ -503,7 +421,7 @@ func (t *TransportHelper) sendLoads(ctx context.Context, stream grpc.ClientStrea return } if err := t.vClient.SendLoadStatsRequest(stream, store.Stats(clusterNames)); err != nil { - logger.Warning(err) + t.logger.Warningf("%v", err) return } } diff --git a/xds/internal/xdsclient/v2/ack_test.go b/xds/internal/xdsclient/controller/v2_ack_test.go similarity index 94% rename from xds/internal/xdsclient/v2/ack_test.go rename to xds/internal/xdsclient/controller/v2_ack_test.go index 51716676dd44..6680de7911b2 100644 --- a/xds/internal/xdsclient/v2/ack_test.go +++ b/xds/internal/xdsclient/controller/v2_ack_test.go @@ -15,7 +15,7 @@ * limitations under the License. */ -package v2 +package controller import ( "context" @@ -28,12 +28,11 @@ import ( "github.com/golang/protobuf/proto" anypb "github.com/golang/protobuf/ptypes/any" "github.com/google/go-cmp/cmp" - "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/xds/internal/testutils/fakeserver" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) const ( @@ -41,12 +40,12 @@ const ( defaultTestShortTimeout = 10 * time.Millisecond ) -func startXDSV2Client(t *testing.T, cc *grpc.ClientConn) (v2c *client, cbLDS, cbRDS, cbCDS, cbEDS *testutils.Channel, cleanup func()) { +func startXDSV2Client(t *testing.T, controlPlaneAddr string) (v2c *Controller, cbLDS, cbRDS, cbCDS, cbEDS *testutils.Channel, cleanup func()) { cbLDS = testutils.NewChannel() cbRDS = testutils.NewChannel() cbCDS = testutils.NewChannel() cbEDS = testutils.NewChannel() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { t.Logf("Received %v callback with {%+v}", rType, d) switch rType { @@ -68,7 +67,7 @@ func startXDSV2Client(t *testing.T, cc *grpc.ClientConn) (v2c *client, cbLDS, cb } } }, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, controlPlaneAddr, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } @@ -117,7 +116,7 @@ func sendXDSRespWithVersion(ch chan<- *fakeserver.Response, respWithoutVersion * // startXDS calls watch to send the first request. It then sends a good response // and checks for ack. -func startXDS(ctx context.Context, t *testing.T, rType xdsresource.ResourceType, v2c *client, reqChan *testutils.Channel, req *xdspb.DiscoveryRequest, preVersion string, preNonce string) { +func startXDS(ctx context.Context, t *testing.T, rType xdsresource.ResourceType, v2c *Controller, reqChan *testutils.Channel, req *xdspb.DiscoveryRequest, preVersion string, preNonce string) { nameToWatch := "" switch rType { case xdsresource.ListenerResource: @@ -198,10 +197,10 @@ func (s) TestV2ClientAck(t *testing.T) { versionEDS = 4000 ) - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, cbLDS, cbRDS, cbCDS, cbEDS, v2cCleanup := startXDSV2Client(t, cc) + v2c, cbLDS, cbRDS, cbCDS, cbEDS, v2cCleanup := startXDSV2Client(t, fakeServer.Address) defer v2cCleanup() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -271,10 +270,10 @@ func (s) TestV2ClientAck(t *testing.T) { func (s) TestV2ClientAckFirstIsNack(t *testing.T) { var versionLDS = 1000 - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, cbLDS, _, _, _, v2cCleanup := startXDSV2Client(t, cc) + v2c, cbLDS, _, _, _, v2cCleanup := startXDSV2Client(t, fakeServer.Address) defer v2cCleanup() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -306,10 +305,10 @@ func (s) TestV2ClientAckFirstIsNack(t *testing.T) { func (s) TestV2ClientAckNackAfterNewWatch(t *testing.T) { var versionLDS = 1000 - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, cbLDS, _, _, _, v2cCleanup := startXDSV2Client(t, cc) + v2c, cbLDS, _, _, _, v2cCleanup := startXDSV2Client(t, fakeServer.Address) defer v2cCleanup() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -351,10 +350,10 @@ func (s) TestV2ClientAckNackAfterNewWatch(t *testing.T) { func (s) TestV2ClientAckNewWatchAfterCancel(t *testing.T) { var versionCDS = 3000 - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, _, _, cbCDS, _, v2cCleanup := startXDSV2Client(t, cc) + v2c, _, _, cbCDS, _, v2cCleanup := startXDSV2Client(t, fakeServer.Address) defer v2cCleanup() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) @@ -409,10 +408,10 @@ func (s) TestV2ClientAckNewWatchAfterCancel(t *testing.T) { func (s) TestV2ClientAckCancelResponseRace(t *testing.T) { var versionCDS = 3000 - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, _, _, cbCDS, _, v2cCleanup := startXDSV2Client(t, cc) + v2c, _, _, cbCDS, _, v2cCleanup := startXDSV2Client(t, fakeServer.Address) defer v2cCleanup() ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) diff --git a/xds/internal/xdsclient/v2/cds_test.go b/xds/internal/xdsclient/controller/v2_cds_test.go similarity index 84% rename from xds/internal/xdsclient/v2/cds_test.go rename to xds/internal/xdsclient/controller/v2_cds_test.go index e44ee0cc48b9..20485dc1c280 100644 --- a/xds/internal/xdsclient/v2/cds_test.go +++ b/xds/internal/xdsclient/controller/v2_cds_test.go @@ -16,7 +16,7 @@ * */ -package v2 +package controller import ( "testing" @@ -27,8 +27,8 @@ import ( anypb "github.com/golang/protobuf/ptypes/any" "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) const ( @@ -119,31 +119,6 @@ func (s) TestCDSHandleResponse(t *testing.T) { }, wantUpdateErr: false, }, - // Response does not contain Cluster proto. - { - name: "no-cluster-proto-in-response", - cdsResponse: badResourceTypeInLDSResponse, - wantErr: true, - wantUpdate: nil, - wantUpdateMD: xdsresource.UpdateMetadata{ - Status: xdsresource.ServiceStatusNACKed, - ErrState: &xdsresource.UpdateErrorMetadata{ - Err: cmpopts.AnyError, - }, - }, - wantUpdateErr: false, - }, - // Response contains no clusters. - { - name: "no-cluster", - cdsResponse: &xdspb.DiscoveryResponse{}, - wantErr: false, - wantUpdate: nil, - wantUpdateMD: xdsresource.UpdateMetadata{ - Status: xdsresource.ServiceStatusACKed, - }, - wantUpdateErr: false, - }, // Response contains one good cluster we are not interested in. { name: "one-uninteresting-cluster", @@ -190,22 +165,22 @@ func (s) TestCDSHandleResponse(t *testing.T) { // TestCDSHandleResponseWithoutWatch tests the case where the v2Client receives // a CDS response without a registered watcher. func (s) TestCDSHandleResponseWithoutWatch(t *testing.T) { - _, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) {}, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } defer v2c.Close() - if v2c.handleCDSResponse(badResourceTypeInLDSResponse) == nil { + if _, _, _, err := v2c.handleResponse(badResourceTypeInLDSResponse); err == nil { t.Fatal("v2c.handleCDSResponse() succeeded, should have failed") } - if v2c.handleCDSResponse(goodCDSResponse1) != nil { + if _, _, _, err := v2c.handleResponse(goodCDSResponse1); err != nil { t.Fatal("v2c.handleCDSResponse() succeeded, should have failed") } } diff --git a/xds/internal/xdsclient/controller/v2_client_test.go b/xds/internal/xdsclient/controller/v2_client_test.go new file mode 100644 index 000000000000..942f18649034 --- /dev/null +++ b/xds/internal/xdsclient/controller/v2_client_test.go @@ -0,0 +1,212 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package controller + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/golang/protobuf/proto" + "google.golang.org/grpc/internal/testutils" + "google.golang.org/grpc/resolver" + "google.golang.org/grpc/resolver/manual" + "google.golang.org/grpc/xds/internal/testutils/fakeserver" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // Register the v2 xDS API client. +) + +// TestV2ClientBackoffAfterRecvError verifies if the v2Client backs off when it +// encounters a Recv error while receiving an LDS response. +func (s) TestV2ClientBackoffAfterRecvError(t *testing.T) { + fakeServer, cleanup := startServer(t) + defer cleanup() + + // Override the v2Client backoff function with this, so that we can verify + // that a backoff actually was triggered. + boCh := make(chan int, 1) + clientBackoff := func(v int) time.Duration { + boCh <- v + return 0 + } + + callbackCh := make(chan struct{}) + v2c, err := newTestController(&testUpdateReceiver{ + f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) { close(callbackCh) }, + }, fakeServer.Address, goodNodeProto, clientBackoff, nil) + if err != nil { + t.Fatal(err) + } + defer v2c.Close() + t.Log("Started xds v2Client...") + + v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { + t.Fatalf("Timeout expired when expecting an LDS request") + } + t.Log("FakeServer received request...") + + fakeServer.XDSResponseChan <- &fakeserver.Response{Err: errors.New("RPC error")} + t.Log("Bad LDS response pushed to fakeServer...") + + timer := time.NewTimer(defaultTestTimeout) + select { + case <-timer.C: + t.Fatal("Timeout when expecting LDS update") + case <-boCh: + timer.Stop() + t.Log("v2Client backed off before retrying...") + case <-callbackCh: + t.Fatal("Received unexpected LDS callback") + } + + if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { + t.Fatalf("Timeout expired when expecting an LDS request") + } + t.Log("FakeServer received request after backoff...") +} + +// TestV2ClientRetriesAfterBrokenStream verifies the case where a stream +// encountered a Recv() error, and is expected to send out xDS requests for +// registered watchers once it comes back up again. +func (s) TestV2ClientRetriesAfterBrokenStream(t *testing.T) { + fakeServer, cleanup := startServer(t) + defer cleanup() + + callbackCh := testutils.NewChannel() + v2c, err := newTestController(&testUpdateReceiver{ + f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { + if rType == xdsresource.ListenerResource { + if u, ok := d[goodLDSTarget1]; ok { + t.Logf("Received LDS callback with ldsUpdate {%+v}", u) + callbackCh.Send(struct{}{}) + } + } + }, + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) + if err != nil { + t.Fatal(err) + } + defer v2c.Close() + t.Log("Started xds v2Client...") + + v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { + t.Fatalf("Timeout expired when expecting an LDS request") + } + t.Log("FakeServer received request...") + + fakeServer.XDSResponseChan <- &fakeserver.Response{Resp: goodLDSResponse1} + t.Log("Good LDS response pushed to fakeServer...") + + if _, err := callbackCh.Receive(ctx); err != nil { + t.Fatal("Timeout when expecting LDS update") + } + + // Read the ack, so the next request is sent after stream re-creation. + if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { + t.Fatalf("Timeout expired when expecting an LDS ACK") + } + + fakeServer.XDSResponseChan <- &fakeserver.Response{Err: errors.New("RPC error")} + t.Log("Bad LDS response pushed to fakeServer...") + + val, err := fakeServer.XDSRequestChan.Receive(ctx) + if err != nil { + t.Fatalf("Timeout expired when expecting LDS update") + } + gotRequest := val.(*fakeserver.Request) + if !proto.Equal(gotRequest.Req, goodLDSRequest) { + t.Fatalf("gotRequest: %+v, wantRequest: %+v", gotRequest.Req, goodLDSRequest) + } +} + +// TestV2ClientWatchWithoutStream verifies the case where a watch is started +// when the xds stream is not created. The watcher should not receive any update +// (because there won't be any xds response, and timeout is done at a upper +// level). And when the stream is re-created, the watcher should get future +// updates. +func (s) TestV2ClientWatchWithoutStream(t *testing.T) { + fakeServer, sCleanup, err := fakeserver.StartServer() + if err != nil { + t.Fatalf("Failed to start fake xDS server: %v", err) + } + defer sCleanup() + + const scheme = "xds-client-test-whatever" + rb := manual.NewBuilderWithScheme(scheme) + rb.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: "no.such.server"}}}) + resolver.Register(rb) + defer resolver.UnregisterForTesting(scheme) + + callbackCh := testutils.NewChannel() + v2c, err := newTestController(&testUpdateReceiver{ + f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { + if rType == xdsresource.ListenerResource { + if u, ok := d[goodLDSTarget1]; ok { + t.Logf("Received LDS callback with ldsUpdate {%+v}", u) + callbackCh.Send(u) + } + } + }, + }, scheme+":///whatever", goodNodeProto, func(int) time.Duration { return 0 }, nil) + if err != nil { + t.Fatal(err) + } + defer v2c.Close() + t.Log("Started xds v2Client...") + + // This watch is started when the xds-ClientConn is in Transient Failure, + // and no xds stream is created. + v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) + + // The watcher should receive an update, with a timeout error in it. + sCtx, sCancel := context.WithTimeout(context.Background(), defaultTestShortTimeout) + defer sCancel() + if v, err := callbackCh.Receive(sCtx); err == nil { + t.Fatalf("Expect an timeout error from watcher, got %v", v) + } + + // Send the real server address to the ClientConn, the stream should be + // created, and the previous watch should be sent. + rb.UpdateState(resolver.State{ + Addresses: []resolver.Address{{Addr: fakeServer.Address}}, + }) + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { + t.Fatalf("Timeout expired when expecting an LDS request") + } + t.Log("FakeServer received request...") + + fakeServer.XDSResponseChan <- &fakeserver.Response{Resp: goodLDSResponse1} + t.Log("Good LDS response pushed to fakeServer...") + + if v, err := callbackCh.Receive(ctx); err != nil { + t.Fatal("Timeout when expecting LDS update") + } else if _, ok := v.(xdsresource.ListenerUpdateErrTuple); !ok { + t.Fatalf("Expect an LDS update from watcher, got %v", v) + } +} diff --git a/xds/internal/xdsclient/v2/eds_test.go b/xds/internal/xdsclient/controller/v2_eds_test.go similarity index 93% rename from xds/internal/xdsclient/v2/eds_test.go rename to xds/internal/xdsclient/controller/v2_eds_test.go index 9503c0397b91..aaa84f9c3d63 100644 --- a/xds/internal/xdsclient/v2/eds_test.go +++ b/xds/internal/xdsclient/controller/v2_eds_test.go @@ -16,7 +16,7 @@ * */ -package v2 +package controller import ( "testing" @@ -28,8 +28,8 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/xds/internal" xtestutils "google.golang.org/grpc/xds/internal/testutils" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) var ( @@ -179,22 +179,22 @@ func (s) TestEDSHandleResponse(t *testing.T) { // TestEDSHandleResponseWithoutWatch tests the case where the v2Client // receives an EDS response without a registered EDS watcher. func (s) TestEDSHandleResponseWithoutWatch(t *testing.T) { - _, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) {}, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } defer v2c.Close() - if v2c.handleEDSResponse(badResourceTypeInEDSResponse) == nil { + if _, _, _, err := v2c.handleResponse(badResourceTypeInEDSResponse); err == nil { t.Fatal("v2c.handleEDSResponse() succeeded, should have failed") } - if v2c.handleEDSResponse(goodEDSResponse1) != nil { + if _, _, _, err := v2c.handleResponse(goodEDSResponse1); err != nil { t.Fatal("v2c.handleEDSResponse() succeeded, should have failed") } } diff --git a/xds/internal/xdsclient/v2/lds_test.go b/xds/internal/xdsclient/controller/v2_lds_test.go similarity index 94% rename from xds/internal/xdsclient/v2/lds_test.go rename to xds/internal/xdsclient/controller/v2_lds_test.go index 008b29f4db9d..56b292988b03 100644 --- a/xds/internal/xdsclient/v2/lds_test.go +++ b/xds/internal/xdsclient/controller/v2_lds_test.go @@ -16,7 +16,7 @@ * */ -package v2 +package controller import ( "testing" @@ -177,22 +177,22 @@ func (s) TestLDSHandleResponse(t *testing.T) { // TestLDSHandleResponseWithoutWatch tests the case where the client receives // an LDS response without a registered watcher. func (s) TestLDSHandleResponseWithoutWatch(t *testing.T) { - _, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) {}, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } defer v2c.Close() - if v2c.handleLDSResponse(badResourceTypeInLDSResponse) == nil { + if _, _, _, err := v2c.handleResponse(badResourceTypeInLDSResponse); err == nil { t.Fatal("v2c.handleLDSResponse() succeeded, should have failed") } - if v2c.handleLDSResponse(goodLDSResponse1) != nil { + if _, _, _, err := v2c.handleResponse(goodLDSResponse1); err != nil { t.Fatal("v2c.handleLDSResponse() succeeded, should have failed") } } diff --git a/xds/internal/xdsclient/v2/rds_test.go b/xds/internal/xdsclient/controller/v2_rds_test.go similarity index 93% rename from xds/internal/xdsclient/v2/rds_test.go rename to xds/internal/xdsclient/controller/v2_rds_test.go index 9ac3b3041ed8..0b3dbfc8cfaf 100644 --- a/xds/internal/xdsclient/v2/rds_test.go +++ b/xds/internal/xdsclient/controller/v2_rds_test.go @@ -16,7 +16,7 @@ * */ -package v2 +package controller import ( "context" @@ -26,7 +26,6 @@ import ( xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/xds/internal/testutils/fakeserver" - "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" ) @@ -35,7 +34,7 @@ import ( // This is called by RDS tests to start LDS first, because LDS is a // pre-requirement for RDS, and RDS handle would fail without an existing LDS // watch. -func doLDS(ctx context.Context, t *testing.T, v2c xdsclient.APIClient, fakeServer *fakeserver.Server) { +func doLDS(ctx context.Context, t *testing.T, v2c *Controller, fakeServer *fakeserver.Server) { v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { t.Fatalf("Timeout waiting for LDS request: %v", err) @@ -179,12 +178,12 @@ func (s) TestRDSHandleResponseWithRouting(t *testing.T) { // TestRDSHandleResponseWithoutRDSWatch tests the case where the v2Client // receives an RDS response without a registered RDS watcher. func (s) TestRDSHandleResponseWithoutRDSWatch(t *testing.T) { - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) {}, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } @@ -194,11 +193,11 @@ func (s) TestRDSHandleResponseWithoutRDSWatch(t *testing.T) { defer cancel() doLDS(ctx, t, v2c, fakeServer) - if v2c.handleRDSResponse(badResourceTypeInRDSResponse) == nil { + if _, _, _, err := v2c.handleResponse(badResourceTypeInRDSResponse); err == nil { t.Fatal("v2c.handleRDSResponse() succeeded, should have failed") } - if v2c.handleRDSResponse(goodRDSResponse1) != nil { + if _, _, _, err := v2c.handleResponse(goodRDSResponse1); err != nil { t.Fatal("v2c.handleRDSResponse() succeeded, should have failed") } } diff --git a/xds/internal/xdsclient/v2/client_test.go b/xds/internal/xdsclient/controller/v2_testutils_test.go similarity index 63% rename from xds/internal/xdsclient/v2/client_test.go rename to xds/internal/xdsclient/controller/v2_testutils_test.go index 28edae57b0f9..dfd195827d46 100644 --- a/xds/internal/xdsclient/v2/client_test.go +++ b/xds/internal/xdsclient/controller/v2_testutils_test.go @@ -16,11 +16,10 @@ * */ -package v2 +package controller import ( "context" - "errors" "testing" "time" @@ -32,12 +31,11 @@ import ( "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/resolver" - "google.golang.org/grpc/resolver/manual" "google.golang.org/grpc/xds/internal/testutils/fakeserver" - "google.golang.org/grpc/xds/internal/version" - "google.golang.org/grpc/xds/internal/xdsclient" + "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/pubsub" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/testing/protocmp" xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" @@ -349,7 +347,7 @@ func (t *testUpdateReceiver) newUpdate(rType xdsresource.ResourceType, d map[str func testWatchHandle(t *testing.T, test *watchHandleTestcase) { t.Helper() - fakeServer, cc, cleanup := startServerAndGetCC(t) + fakeServer, cleanup := startServer(t) defer cleanup() type updateErr struct { @@ -359,7 +357,7 @@ func testWatchHandle(t *testing.T, test *watchHandleTestcase) { } gotUpdateCh := testutils.NewChannel() - v2c, err := newV2Client(&testUpdateReceiver{ + v2c, err := newTestController(&testUpdateReceiver{ f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { if rType == test.rType { switch test.rType { @@ -390,7 +388,7 @@ func testWatchHandle(t *testing.T, test *watchHandleTestcase) { } } }, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) + }, fakeServer.Address, goodNodeProto, func(int) time.Duration { return 0 }, nil) if err != nil { t.Fatal(err) } @@ -414,18 +412,7 @@ func testWatchHandle(t *testing.T, test *watchHandleTestcase) { // // Also note that this won't trigger ACK, so there's no need to clear the // request channel afterwards. - var handleXDSResp func(response *xdspb.DiscoveryResponse) error - switch test.rType { - case xdsresource.ListenerResource: - handleXDSResp = v2c.handleLDSResponse - case xdsresource.RouteConfigResource: - handleXDSResp = v2c.handleRDSResponse - case xdsresource.ClusterResource: - handleXDSResp = v2c.handleCDSResponse - case xdsresource.EndpointsResource: - handleXDSResp = v2c.handleEDSResponse - } - if err := handleXDSResp(test.responseToHandle); (err != nil) != test.wantHandleErr { + if _, _, _, err := v2c.handleResponse(test.responseToHandle); (err != nil) != test.wantHandleErr { t.Fatalf("v2c.handleRDSResponse() returned err: %v, wantErr: %v", err, test.wantHandleErr) } @@ -454,220 +441,31 @@ func testWatchHandle(t *testing.T, test *watchHandleTestcase) { } } -// startServerAndGetCC starts a fake XDS server and also returns a ClientConn +// startServer starts a fake XDS server and also returns a ClientConn // connected to it. -func startServerAndGetCC(t *testing.T) (*fakeserver.Server, *grpc.ClientConn, func()) { +func startServer(t *testing.T) (*fakeserver.Server, func()) { t.Helper() - fs, sCleanup, err := fakeserver.StartServer() if err != nil { t.Fatalf("Failed to start fake xDS server: %v", err) } - - cc, ccCleanup, err := fs.XDSClientConn() - if err != nil { - sCleanup() - t.Fatalf("Failed to get a clientConn to the fake xDS server: %v", err) - } - return fs, cc, func() { - sCleanup() - ccCleanup() - } + return fs, sCleanup } -func newV2Client(p xdsclient.UpdateHandler, cc *grpc.ClientConn, n *basepb.Node, b func(int) time.Duration, l *grpclog.PrefixLogger) (*client, error) { - c, err := newClient(cc, xdsclient.BuildOptions{ - Parent: p, - NodeProto: n, - Backoff: b, - Logger: l, - }) +func newTestController(p pubsub.UpdateHandler, controlPlanAddr string, n *basepb.Node, b func(int) time.Duration, l *grpclog.PrefixLogger) (*Controller, error) { + c, err := New(&bootstrap.ServerConfig{ + ServerURI: controlPlanAddr, + Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), + TransportAPI: version.TransportV2, + NodeProto: n, + }, p, nil, l) if err != nil { return nil, err } - return c.(*client), nil -} - -// TestV2ClientBackoffAfterRecvError verifies if the v2Client backs off when it -// encounters a Recv error while receiving an LDS response. -func (s) TestV2ClientBackoffAfterRecvError(t *testing.T) { - fakeServer, cc, cleanup := startServerAndGetCC(t) - defer cleanup() - - // Override the v2Client backoff function with this, so that we can verify - // that a backoff actually was triggered. - boCh := make(chan int, 1) - clientBackoff := func(v int) time.Duration { - boCh <- v - return 0 - } - - callbackCh := make(chan struct{}) - v2c, err := newV2Client(&testUpdateReceiver{ - f: func(xdsresource.ResourceType, map[string]interface{}, xdsresource.UpdateMetadata) { close(callbackCh) }, - }, cc, goodNodeProto, clientBackoff, nil) - if err != nil { - t.Fatal(err) - } - defer v2c.Close() - t.Log("Started xds v2Client...") - - v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { - t.Fatalf("Timeout expired when expecting an LDS request") - } - t.Log("FakeServer received request...") - - fakeServer.XDSResponseChan <- &fakeserver.Response{Err: errors.New("RPC error")} - t.Log("Bad LDS response pushed to fakeServer...") - - timer := time.NewTimer(defaultTestTimeout) - select { - case <-timer.C: - t.Fatal("Timeout when expecting LDS update") - case <-boCh: - timer.Stop() - t.Log("v2Client backed off before retrying...") - case <-callbackCh: - t.Fatal("Received unexpected LDS callback") - } - - if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { - t.Fatalf("Timeout expired when expecting an LDS request") - } - t.Log("FakeServer received request after backoff...") -} - -// TestV2ClientRetriesAfterBrokenStream verifies the case where a stream -// encountered a Recv() error, and is expected to send out xDS requests for -// registered watchers once it comes back up again. -func (s) TestV2ClientRetriesAfterBrokenStream(t *testing.T) { - fakeServer, cc, cleanup := startServerAndGetCC(t) - defer cleanup() - - callbackCh := testutils.NewChannel() - v2c, err := newV2Client(&testUpdateReceiver{ - f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { - if rType == xdsresource.ListenerResource { - if u, ok := d[goodLDSTarget1]; ok { - t.Logf("Received LDS callback with ldsUpdate {%+v}", u) - callbackCh.Send(struct{}{}) - } - } - }, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) - if err != nil { - t.Fatal(err) - } - defer v2c.Close() - t.Log("Started xds v2Client...") - - v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { - t.Fatalf("Timeout expired when expecting an LDS request") - } - t.Log("FakeServer received request...") - - fakeServer.XDSResponseChan <- &fakeserver.Response{Resp: goodLDSResponse1} - t.Log("Good LDS response pushed to fakeServer...") - - if _, err := callbackCh.Receive(ctx); err != nil { - t.Fatal("Timeout when expecting LDS update") - } - - // Read the ack, so the next request is sent after stream re-creation. - if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { - t.Fatalf("Timeout expired when expecting an LDS ACK") - } - - fakeServer.XDSResponseChan <- &fakeserver.Response{Err: errors.New("RPC error")} - t.Log("Bad LDS response pushed to fakeServer...") - - val, err := fakeServer.XDSRequestChan.Receive(ctx) - if err != nil { - t.Fatalf("Timeout expired when expecting LDS update") - } - gotRequest := val.(*fakeserver.Request) - if !proto.Equal(gotRequest.Req, goodLDSRequest) { - t.Fatalf("gotRequest: %+v, wantRequest: %+v", gotRequest.Req, goodLDSRequest) - } -} - -// TestV2ClientWatchWithoutStream verifies the case where a watch is started -// when the xds stream is not created. The watcher should not receive any update -// (because there won't be any xds response, and timeout is done at a upper -// level). And when the stream is re-created, the watcher should get future -// updates. -func (s) TestV2ClientWatchWithoutStream(t *testing.T) { - fakeServer, sCleanup, err := fakeserver.StartServer() - if err != nil { - t.Fatalf("Failed to start fake xDS server: %v", err) - } - defer sCleanup() - - const scheme = "xds-client-test-whatever" - rb := manual.NewBuilderWithScheme(scheme) - rb.InitialState(resolver.State{Addresses: []resolver.Address{{Addr: "no.such.server"}}}) - - cc, err := grpc.Dial(scheme+":///whatever", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(rb)) - if err != nil { - t.Fatalf("Failed to dial ClientConn: %v", err) - } - defer cc.Close() - - callbackCh := testutils.NewChannel() - v2c, err := newV2Client(&testUpdateReceiver{ - f: func(rType xdsresource.ResourceType, d map[string]interface{}, md xdsresource.UpdateMetadata) { - if rType == xdsresource.ListenerResource { - if u, ok := d[goodLDSTarget1]; ok { - t.Logf("Received LDS callback with ldsUpdate {%+v}", u) - callbackCh.Send(u) - } - } - }, - }, cc, goodNodeProto, func(int) time.Duration { return 0 }, nil) - if err != nil { - t.Fatal(err) - } - defer v2c.Close() - t.Log("Started xds v2Client...") - - // This watch is started when the xds-ClientConn is in Transient Failure, - // and no xds stream is created. - v2c.AddWatch(xdsresource.ListenerResource, goodLDSTarget1) - - // The watcher should receive an update, with a timeout error in it. - sCtx, sCancel := context.WithTimeout(context.Background(), defaultTestShortTimeout) - defer sCancel() - if v, err := callbackCh.Receive(sCtx); err == nil { - t.Fatalf("Expect an timeout error from watcher, got %v", v) - } - - // Send the real server address to the ClientConn, the stream should be - // created, and the previous watch should be sent. - rb.UpdateState(resolver.State{ - Addresses: []resolver.Address{{Addr: fakeServer.Address}}, - }) - - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - if _, err := fakeServer.XDSRequestChan.Receive(ctx); err != nil { - t.Fatalf("Timeout expired when expecting an LDS request") - } - t.Log("FakeServer received request...") - - fakeServer.XDSResponseChan <- &fakeserver.Response{Resp: goodLDSResponse1} - t.Log("Good LDS response pushed to fakeServer...") - - if v, err := callbackCh.Receive(ctx); err != nil { - t.Fatal("Timeout when expecting LDS update") - } else if _, ok := v.(xdsresource.ListenerUpdateErrTuple); !ok { - t.Fatalf("Expect an LDS update from watcher, got %v", v) - } + // This direct setting backoff seems a bit hacky, but should be OK for the + // tests. Or we need to make it configurable in New(). + c.backoff = b + return c, nil } func newStringP(s string) *string { diff --git a/xds/internal/xdsclient/v2/client.go b/xds/internal/xdsclient/controller/version/v2/client.go similarity index 51% rename from xds/internal/xdsclient/v2/client.go rename to xds/internal/xdsclient/controller/version/v2/client.go index 12d0509dc524..ae3ae559e5dd 100644 --- a/xds/internal/xdsclient/v2/client.go +++ b/xds/internal/xdsclient/controller/version/v2/client.go @@ -28,9 +28,10 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/pretty" - "google.golang.org/grpc/xds/internal/version" - "google.golang.org/grpc/xds/internal/xdsclient" + controllerversion "google.golang.org/grpc/xds/internal/xdsclient/controller/version" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + xdsresourceversion "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" + "google.golang.org/protobuf/types/known/anypb" v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" v2corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" @@ -39,42 +40,24 @@ import ( ) func init() { - xdsclient.RegisterAPIClientBuilder(clientBuilder{}) + controllerversion.RegisterAPIClientBuilder(xdsresourceversion.TransportV2, newClient) } var ( resourceTypeToURL = map[xdsresource.ResourceType]string{ - xdsresource.ListenerResource: version.V2ListenerURL, - xdsresource.RouteConfigResource: version.V2RouteConfigURL, - xdsresource.ClusterResource: version.V2ClusterURL, - xdsresource.EndpointsResource: version.V2EndpointsURL, + xdsresource.ListenerResource: xdsresourceversion.V2ListenerURL, + xdsresource.RouteConfigResource: xdsresourceversion.V2RouteConfigURL, + xdsresource.ClusterResource: xdsresourceversion.V2ClusterURL, + xdsresource.EndpointsResource: xdsresourceversion.V2EndpointsURL, } ) -type clientBuilder struct{} - -func (clientBuilder) Build(cc *grpc.ClientConn, opts xdsclient.BuildOptions) (xdsclient.APIClient, error) { - return newClient(cc, opts) -} - -func (clientBuilder) Version() version.TransportAPI { - return version.TransportV2 -} - -func newClient(cc *grpc.ClientConn, opts xdsclient.BuildOptions) (xdsclient.APIClient, error) { +func newClient(opts controllerversion.BuildOptions) (controllerversion.VersionedClient, error) { nodeProto, ok := opts.NodeProto.(*v2corepb.Node) if !ok { return nil, fmt.Errorf("xds: unsupported Node proto type: %T, want %T", opts.NodeProto, (*v2corepb.Node)(nil)) } - v2c := &client{ - cc: cc, - parent: opts.Parent, - nodeProto: nodeProto, - logger: opts.Logger, - updateValidator: opts.Validator, - } - v2c.ctx, v2c.cancelCtx = context.WithCancel(context.Background()) - v2c.TransportHelper = xdsclient.NewTransportHelper(v2c, opts.Logger, opts.Backoff) + v2c := &client{nodeProto: nodeProto, logger: opts.Logger} return v2c, nil } @@ -84,24 +67,15 @@ type adsStream v2adsgrpc.AggregatedDiscoveryService_StreamAggregatedResourcesCli // single ADS stream on which the different types of xDS requests and responses // are multiplexed. type client struct { - *xdsclient.TransportHelper - - ctx context.Context - cancelCtx context.CancelFunc - parent xdsclient.UpdateHandler + nodeProto *v2corepb.Node logger *grpclog.PrefixLogger - - // ClientConn to the xDS gRPC server. Owned by the parent xdsClient. - cc *grpc.ClientConn - nodeProto *v2corepb.Node - updateValidator xdsresource.UpdateValidatorFunc } -func (v2c *client) NewStream(ctx context.Context) (grpc.ClientStream, error) { - return v2adsgrpc.NewAggregatedDiscoveryServiceClient(v2c.cc).StreamAggregatedResources(v2c.ctx, grpc.WaitForReady(true)) +func (v2c *client) NewStream(ctx context.Context, cc *grpc.ClientConn) (grpc.ClientStream, error) { + return v2adsgrpc.NewAggregatedDiscoveryServiceClient(cc).StreamAggregatedResources(ctx, grpc.WaitForReady(true)) } -// sendRequest sends out a DiscoveryRequest for the given resourceNames, of type +// SendRequest sends out a DiscoveryRequest for the given resourceNames, of type // rType, on the provided stream. // // version is the ack version to be sent with the request @@ -143,7 +117,6 @@ func (v2c *client) RecvResponse(s grpc.ClientStream) (proto.Message, error) { resp, err := stream.Recv() if err != nil { - v2c.parent.NewConnectionError(err) return nil, fmt.Errorf("xds: stream.Recv() failed: %v", err) } v2c.logger.Infof("ADS response received, type: %v", resp.GetTypeUrl()) @@ -151,11 +124,11 @@ func (v2c *client) RecvResponse(s grpc.ClientStream) (proto.Message, error) { return resp, nil } -func (v2c *client) HandleResponse(r proto.Message) (xdsresource.ResourceType, string, string, error) { +func (v2c *client) ParseResponse(r proto.Message) (xdsresource.ResourceType, []*anypb.Any, string, string, error) { rType := xdsresource.UnknownResource resp, ok := r.(*v2xdspb.DiscoveryResponse) if !ok { - return rType, "", "", fmt.Errorf("xds: unsupported message type: %T", resp) + return rType, nil, "", "", fmt.Errorf("xds: unsupported message type: %T", resp) } // Note that the xDS transport protocol is versioned independently of @@ -166,74 +139,17 @@ func (v2c *client) HandleResponse(r proto.Message) (xdsresource.ResourceType, st url := resp.GetTypeUrl() switch { case xdsresource.IsListenerResource(url): - err = v2c.handleLDSResponse(resp) rType = xdsresource.ListenerResource case xdsresource.IsRouteConfigResource(url): - err = v2c.handleRDSResponse(resp) rType = xdsresource.RouteConfigResource case xdsresource.IsClusterResource(url): - err = v2c.handleCDSResponse(resp) rType = xdsresource.ClusterResource case xdsresource.IsEndpointsResource(url): - err = v2c.handleEDSResponse(resp) rType = xdsresource.EndpointsResource default: - return rType, "", "", xdsclient.ErrResourceTypeUnsupported{ + return rType, nil, "", "", controllerversion.ErrResourceTypeUnsupported{ ErrStr: fmt.Sprintf("Resource type %v unknown in response from server", resp.GetTypeUrl()), } } - return rType, resp.GetVersionInfo(), resp.GetNonce(), err -} - -// handleLDSResponse processes an LDS response received from the management -// server. On receipt of a good response, it also invokes the registered watcher -// callback. -func (v2c *client) handleLDSResponse(resp *v2xdspb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalListener(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v2c.logger, - UpdateValidator: v2c.updateValidator, - }) - v2c.parent.NewListeners(update, md) - return err -} - -// handleRDSResponse processes an RDS response received from the management -// server. On receipt of a good response, it caches validated resources and also -// invokes the registered watcher callback. -func (v2c *client) handleRDSResponse(resp *v2xdspb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalRouteConfig(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v2c.logger, - UpdateValidator: v2c.updateValidator, - }) - v2c.parent.NewRouteConfigs(update, md) - return err -} - -// handleCDSResponse processes an CDS response received from the management -// server. On receipt of a good response, it also invokes the registered watcher -// callback. -func (v2c *client) handleCDSResponse(resp *v2xdspb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalCluster(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v2c.logger, - UpdateValidator: v2c.updateValidator, - }) - v2c.parent.NewClusters(update, md) - return err -} - -func (v2c *client) handleEDSResponse(resp *v2xdspb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalEndpoints(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v2c.logger, - UpdateValidator: v2c.updateValidator, - }) - v2c.parent.NewEndpoints(update, md) - return err + return rType, resp.GetResources(), resp.GetVersionInfo(), resp.GetNonce(), err } diff --git a/xds/internal/xdsclient/v2/loadreport.go b/xds/internal/xdsclient/controller/version/v2/loadreport.go similarity index 100% rename from xds/internal/xdsclient/v2/loadreport.go rename to xds/internal/xdsclient/controller/version/v2/loadreport.go diff --git a/xds/internal/xdsclient/v3/client.go b/xds/internal/xdsclient/controller/version/v3/client.go similarity index 52% rename from xds/internal/xdsclient/v3/client.go rename to xds/internal/xdsclient/controller/version/v3/client.go index 622ae2a434f9..1c7f11ad2527 100644 --- a/xds/internal/xdsclient/v3/client.go +++ b/xds/internal/xdsclient/controller/version/v3/client.go @@ -29,9 +29,10 @@ import ( "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/pretty" - "google.golang.org/grpc/xds/internal/version" - "google.golang.org/grpc/xds/internal/xdsclient" + controllerversion "google.golang.org/grpc/xds/internal/xdsclient/controller/version" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + xdsresourceversion "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" + "google.golang.org/protobuf/types/known/anypb" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3adsgrpc "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" @@ -39,42 +40,26 @@ import ( ) func init() { - xdsclient.RegisterAPIClientBuilder(clientBuilder{}) + controllerversion.RegisterAPIClientBuilder(xdsresourceversion.TransportV3, newClient) } var ( resourceTypeToURL = map[xdsresource.ResourceType]string{ - xdsresource.ListenerResource: version.V3ListenerURL, - xdsresource.RouteConfigResource: version.V3RouteConfigURL, - xdsresource.ClusterResource: version.V3ClusterURL, - xdsresource.EndpointsResource: version.V3EndpointsURL, + xdsresource.ListenerResource: xdsresourceversion.V3ListenerURL, + xdsresource.RouteConfigResource: xdsresourceversion.V3RouteConfigURL, + xdsresource.ClusterResource: xdsresourceversion.V3ClusterURL, + xdsresource.EndpointsResource: xdsresourceversion.V3EndpointsURL, } ) -type clientBuilder struct{} - -func (clientBuilder) Build(cc *grpc.ClientConn, opts xdsclient.BuildOptions) (xdsclient.APIClient, error) { - return newClient(cc, opts) -} - -func (clientBuilder) Version() version.TransportAPI { - return version.TransportV3 -} - -func newClient(cc *grpc.ClientConn, opts xdsclient.BuildOptions) (xdsclient.APIClient, error) { +func newClient(opts controllerversion.BuildOptions) (controllerversion.VersionedClient, error) { nodeProto, ok := opts.NodeProto.(*v3corepb.Node) if !ok { return nil, fmt.Errorf("xds: unsupported Node proto type: %T, want %T", opts.NodeProto, v3corepb.Node{}) } v3c := &client{ - cc: cc, - parent: opts.Parent, - nodeProto: nodeProto, - logger: opts.Logger, - updateValidator: opts.Validator, + nodeProto: nodeProto, logger: opts.Logger, } - v3c.ctx, v3c.cancelCtx = context.WithCancel(context.Background()) - v3c.TransportHelper = xdsclient.NewTransportHelper(v3c, opts.Logger, opts.Backoff) return v3c, nil } @@ -84,24 +69,15 @@ type adsStream v3adsgrpc.AggregatedDiscoveryService_StreamAggregatedResourcesCli // single ADS stream on which the different types of xDS requests and responses // are multiplexed. type client struct { - *xdsclient.TransportHelper - - ctx context.Context - cancelCtx context.CancelFunc - parent xdsclient.UpdateHandler + nodeProto *v3corepb.Node logger *grpclog.PrefixLogger - - // ClientConn to the xDS gRPC server. Owned by the parent xdsClient. - cc *grpc.ClientConn - nodeProto *v3corepb.Node - updateValidator xdsresource.UpdateValidatorFunc } -func (v3c *client) NewStream(ctx context.Context) (grpc.ClientStream, error) { - return v3adsgrpc.NewAggregatedDiscoveryServiceClient(v3c.cc).StreamAggregatedResources(v3c.ctx, grpc.WaitForReady(true)) +func (v3c *client) NewStream(ctx context.Context, cc *grpc.ClientConn) (grpc.ClientStream, error) { + return v3adsgrpc.NewAggregatedDiscoveryServiceClient(cc).StreamAggregatedResources(ctx, grpc.WaitForReady(true)) } -// sendRequest sends out a DiscoveryRequest for the given resourceNames, of type +// SendRequest sends out a DiscoveryRequest for the given resourceNames, of type // rType, on the provided stream. // // version is the ack version to be sent with the request @@ -143,7 +119,6 @@ func (v3c *client) RecvResponse(s grpc.ClientStream) (proto.Message, error) { resp, err := stream.Recv() if err != nil { - v3c.parent.NewConnectionError(err) return nil, fmt.Errorf("xds: stream.Recv() failed: %v", err) } v3c.logger.Infof("ADS response received, type: %v", resp.GetTypeUrl()) @@ -151,11 +126,11 @@ func (v3c *client) RecvResponse(s grpc.ClientStream) (proto.Message, error) { return resp, nil } -func (v3c *client) HandleResponse(r proto.Message) (xdsresource.ResourceType, string, string, error) { +func (v3c *client) ParseResponse(r proto.Message) (xdsresource.ResourceType, []*anypb.Any, string, string, error) { rType := xdsresource.UnknownResource resp, ok := r.(*v3discoverypb.DiscoveryResponse) if !ok { - return rType, "", "", fmt.Errorf("xds: unsupported message type: %T", resp) + return rType, nil, "", "", fmt.Errorf("xds: unsupported message type: %T", resp) } // Note that the xDS transport protocol is versioned independently of @@ -166,74 +141,17 @@ func (v3c *client) HandleResponse(r proto.Message) (xdsresource.ResourceType, st url := resp.GetTypeUrl() switch { case xdsresource.IsListenerResource(url): - err = v3c.handleLDSResponse(resp) rType = xdsresource.ListenerResource case xdsresource.IsRouteConfigResource(url): - err = v3c.handleRDSResponse(resp) rType = xdsresource.RouteConfigResource case xdsresource.IsClusterResource(url): - err = v3c.handleCDSResponse(resp) rType = xdsresource.ClusterResource case xdsresource.IsEndpointsResource(url): - err = v3c.handleEDSResponse(resp) rType = xdsresource.EndpointsResource default: - return rType, "", "", xdsclient.ErrResourceTypeUnsupported{ + return rType, nil, "", "", controllerversion.ErrResourceTypeUnsupported{ ErrStr: fmt.Sprintf("Resource type %v unknown in response from server", resp.GetTypeUrl()), } } - return rType, resp.GetVersionInfo(), resp.GetNonce(), err -} - -// handleLDSResponse processes an LDS response received from the management -// server. On receipt of a good response, it also invokes the registered watcher -// callback. -func (v3c *client) handleLDSResponse(resp *v3discoverypb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalListener(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v3c.logger, - UpdateValidator: v3c.updateValidator, - }) - v3c.parent.NewListeners(update, md) - return err -} - -// handleRDSResponse processes an RDS response received from the management -// server. On receipt of a good response, it caches validated resources and also -// invokes the registered watcher callback. -func (v3c *client) handleRDSResponse(resp *v3discoverypb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalRouteConfig(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v3c.logger, - UpdateValidator: v3c.updateValidator, - }) - v3c.parent.NewRouteConfigs(update, md) - return err -} - -// handleCDSResponse processes an CDS response received from the management -// server. On receipt of a good response, it also invokes the registered watcher -// callback. -func (v3c *client) handleCDSResponse(resp *v3discoverypb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalCluster(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v3c.logger, - UpdateValidator: v3c.updateValidator, - }) - v3c.parent.NewClusters(update, md) - return err -} - -func (v3c *client) handleEDSResponse(resp *v3discoverypb.DiscoveryResponse) error { - update, md, err := xdsresource.UnmarshalEndpoints(&xdsresource.UnmarshalOptions{ - Version: resp.GetVersionInfo(), - Resources: resp.GetResources(), - Logger: v3c.logger, - UpdateValidator: v3c.updateValidator, - }) - v3c.parent.NewEndpoints(update, md) - return err + return rType, resp.GetResources(), resp.GetVersionInfo(), resp.GetNonce(), err } diff --git a/xds/internal/xdsclient/v3/loadreport.go b/xds/internal/xdsclient/controller/version/v3/loadreport.go similarity index 100% rename from xds/internal/xdsclient/v3/loadreport.go rename to xds/internal/xdsclient/controller/version/v3/loadreport.go diff --git a/xds/internal/xdsclient/controller/version/version.go b/xds/internal/xdsclient/controller/version/version.go new file mode 100644 index 000000000000..f79a21e294fa --- /dev/null +++ b/xds/internal/xdsclient/controller/version/version.go @@ -0,0 +1,123 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +// Package version defines APIs to deal with different versions of xDS. +package version + +import ( + "context" + "time" + + "github.com/golang/protobuf/proto" + "google.golang.org/grpc" + "google.golang.org/grpc/internal/grpclog" + "google.golang.org/grpc/xds/internal/xdsclient/load" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" + "google.golang.org/protobuf/types/known/anypb" +) + +var ( + m = make(map[version.TransportAPI]func(opts BuildOptions) (VersionedClient, error)) +) + +// RegisterAPIClientBuilder registers a client builder for xDS transport protocol +// version specified by b.Version(). +// +// NOTE: this function must only be called during initialization time (i.e. in +// an init() function), and is not thread-safe. If multiple builders are +// registered for the same version, the one registered last will take effect. +func RegisterAPIClientBuilder(v version.TransportAPI, f func(opts BuildOptions) (VersionedClient, error)) { + m[v] = f +} + +// GetAPIClientBuilder returns the client builder registered for the provided +// xDS transport API version. +func GetAPIClientBuilder(version version.TransportAPI) func(opts BuildOptions) (VersionedClient, error) { + if f, ok := m[version]; ok { + return f + } + return nil +} + +// BuildOptions contains options to be passed to client builders. +type BuildOptions struct { + // NodeProto contains the Node proto to be used in xDS requests. The actual + // type depends on the transport protocol version used. + NodeProto proto.Message + // // Backoff returns the amount of time to backoff before retrying broken + // // streams. + // Backoff func(int) time.Duration + // Logger provides enhanced logging capabilities. + Logger *grpclog.PrefixLogger +} + +// LoadReportingOptions contains configuration knobs for reporting load data. +type LoadReportingOptions struct { + LoadStore *load.Store +} + +// ErrResourceTypeUnsupported is an error used to indicate an unsupported xDS +// resource type. The wrapped ErrStr contains the details. +type ErrResourceTypeUnsupported struct { + ErrStr string +} + +// Error helps implements the error interface. +func (e ErrResourceTypeUnsupported) Error() string { + return e.ErrStr +} + +// VersionedClient is the interface to version specific operations of the +// client. +// +// It mainly deals with the type assertion from proto.Message to the real v2/v3 +// types, and grpc.Stream to the versioned stream types. +type VersionedClient interface { + // NewStream returns a new xDS client stream specific to the underlying + // transport protocol version. + NewStream(ctx context.Context, cc *grpc.ClientConn) (grpc.ClientStream, error) + // SendRequest constructs and sends out a DiscoveryRequest message specific + // to the underlying transport protocol version. + SendRequest(s grpc.ClientStream, resourceNames []string, rType xdsresource.ResourceType, version, nonce, errMsg string) error + // RecvResponse uses the provided stream to receive a response specific to + // the underlying transport protocol version. + RecvResponse(s grpc.ClientStream) (proto.Message, error) + // ParseResponse type asserts message to the versioned response, and + // retrieves the fields. + ParseResponse(r proto.Message) (xdsresource.ResourceType, []*anypb.Any, string, string, error) + + // The following are LRS methods. + + // NewLoadStatsStream returns a new LRS client stream specific to the + // underlying transport protocol version. + NewLoadStatsStream(ctx context.Context, cc *grpc.ClientConn) (grpc.ClientStream, error) + // SendFirstLoadStatsRequest constructs and sends the first request on the + // LRS stream. + SendFirstLoadStatsRequest(s grpc.ClientStream) error + // HandleLoadStatsResponse receives the first response from the server which + // contains the load reporting interval and the clusters for which the + // server asks the client to report load for. + // + // If the response sets SendAllClusters to true, the returned clusters is + // nil. + HandleLoadStatsResponse(s grpc.ClientStream) (clusters []string, _ time.Duration, _ error) + // SendLoadStatsRequest will be invoked at regular intervals to send load + // report with load data reported since the last time this method was + // invoked. + SendLoadStatsRequest(s grpc.ClientStream, loads []*load.Data) error +} diff --git a/xds/internal/xdsclient/dump_test.go b/xds/internal/xdsclient/dump_test.go index 0b6d22c7d89f..ccc98898a9cd 100644 --- a/xds/internal/xdsclient/dump_test.go +++ b/xds/internal/xdsclient/dump_test.go @@ -30,6 +30,7 @@ import ( v3httppb "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "google.golang.org/grpc/xds/internal/xdsclient/pubsub" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" "google.golang.org/protobuf/testing/protocmp" "google.golang.org/protobuf/types/known/anypb" @@ -86,7 +87,7 @@ func (s) TestLDSConfigDump(t *testing.T) { t.Fatalf("failed to create client: %v", err) } defer client.Close() - updateHandler := client.(xdsclient.UpdateHandler) + updateHandler := client.(pubsub.UpdateHandler) // Expected unknown. if err := compareDump(client.DumpLDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil { @@ -202,7 +203,7 @@ func (s) TestRDSConfigDump(t *testing.T) { t.Fatalf("failed to create client: %v", err) } defer client.Close() - updateHandler := client.(xdsclient.UpdateHandler) + updateHandler := client.(pubsub.UpdateHandler) // Expected unknown. if err := compareDump(client.DumpRDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil { @@ -318,7 +319,7 @@ func (s) TestCDSConfigDump(t *testing.T) { t.Fatalf("failed to create client: %v", err) } defer client.Close() - updateHandler := client.(xdsclient.UpdateHandler) + updateHandler := client.(pubsub.UpdateHandler) // Expected unknown. if err := compareDump(client.DumpCDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil { @@ -420,7 +421,7 @@ func (s) TestEDSConfigDump(t *testing.T) { t.Fatalf("failed to create client: %v", err) } defer client.Close() - updateHandler := client.(xdsclient.UpdateHandler) + updateHandler := client.(pubsub.UpdateHandler) // Expected unknown. if err := compareDump(client.DumpEDS, "", map[string]xdsresource.UpdateWithMD{}); err != nil { diff --git a/xds/internal/xdsclient/loadreport.go b/xds/internal/xdsclient/loadreport.go index 4df9a5c0c3a4..21400c1321b8 100644 --- a/xds/internal/xdsclient/loadreport.go +++ b/xds/internal/xdsclient/loadreport.go @@ -17,12 +17,7 @@ package xdsclient -import ( - "context" - - "google.golang.org/grpc" - "google.golang.org/grpc/xds/internal/xdsclient/load" -) +import "google.golang.org/grpc/xds/internal/xdsclient/load" // ReportLoad starts an load reporting stream to the given server. If the server // is not an empty string, and is different from the management server, a new @@ -34,106 +29,5 @@ import ( // It returns a Store for the user to report loads, a function to cancel the // load reporting stream. func (c *clientImpl) ReportLoad(server string) (*load.Store, func()) { - c.lrsMu.Lock() - defer c.lrsMu.Unlock() - - // If there's already a client to this server, use it. Otherwise, create - // one. - lrsC, ok := c.lrsClients[server] - if !ok { - lrsC = newLRSClient(c, server) - c.lrsClients[server] = lrsC - } - - store := lrsC.ref() - return store, func() { - // This is a callback, need to hold lrsMu. - c.lrsMu.Lock() - defer c.lrsMu.Unlock() - if lrsC.unRef() { - // Delete the lrsClient from map if this is the last reference. - delete(c.lrsClients, server) - } - } -} - -// lrsClient maps to one lrsServer. It contains: -// - a ClientConn to this server (only if it's different from the management -// server) -// - a load.Store that contains loads only for this server -type lrsClient struct { - parent *clientImpl - server string - - cc *grpc.ClientConn // nil if the server is same as the management server - refCount int - cancelStream func() - loadStore *load.Store -} - -// newLRSClient creates a new LRS stream to the server. -func newLRSClient(parent *clientImpl, server string) *lrsClient { - return &lrsClient{ - parent: parent, - server: server, - refCount: 0, - } -} - -// ref increments the refCount. If this is the first ref, it starts the LRS stream. -// -// Not thread-safe, caller needs to synchronize. -func (lrsC *lrsClient) ref() *load.Store { - lrsC.refCount++ - if lrsC.refCount == 1 { - lrsC.startStream() - } - return lrsC.loadStore -} - -// unRef decrements the refCount, and closes the stream if refCount reaches 0 -// (and close the cc if cc is not xDS cc). It returns whether refCount reached 0 -// after this call. -// -// Not thread-safe, caller needs to synchronize. -func (lrsC *lrsClient) unRef() (closed bool) { - lrsC.refCount-- - if lrsC.refCount != 0 { - return false - } - lrsC.parent.logger.Infof("Stopping load report to server: %s", lrsC.server) - lrsC.cancelStream() - if lrsC.cc != nil { - lrsC.cc.Close() - } - return true -} - -// startStream starts the LRS stream to the server. If server is not the same -// management server from the parent, it also creates a ClientConn. -func (lrsC *lrsClient) startStream() { - var cc *grpc.ClientConn - - lrsC.parent.logger.Infof("Starting load report to server: %s", lrsC.server) - if lrsC.server == "" || lrsC.server == lrsC.parent.config.XDSServer.ServerURI { - // Reuse the xDS client if server is the same. - cc = lrsC.parent.cc - } else { - lrsC.parent.logger.Infof("LRS server is different from management server, starting a new ClientConn") - ccNew, err := grpc.Dial(lrsC.server, lrsC.parent.config.XDSServer.Creds) - if err != nil { - // An error from a non-blocking dial indicates something serious. - lrsC.parent.logger.Infof("xds: failed to dial load report server {%s}: %v", lrsC.server, err) - return - } - cc = ccNew - lrsC.cc = ccNew - } - - var ctx context.Context - ctx, lrsC.cancelStream = context.WithCancel(context.Background()) - - // Create the store and stream. - lrsC.loadStore = load.NewStore() - go lrsC.parent.apiClient.reportLoad(ctx, cc, loadReportingOptions{loadStore: lrsC.loadStore}) + return c.controller.ReportLoad(server) } diff --git a/xds/internal/xdsclient/loadreport_test.go b/xds/internal/xdsclient/loadreport_test.go index db31de6cf78b..8b19f80287cc 100644 --- a/xds/internal/xdsclient/loadreport_test.go +++ b/xds/internal/xdsclient/loadreport_test.go @@ -16,7 +16,7 @@ * */ -package xdsclient_test +package xdsclient import ( "context" @@ -33,17 +33,14 @@ import ( "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "google.golang.org/grpc/xds/internal/testutils/fakeserver" - "google.golang.org/grpc/xds/internal/version" - "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/testing/protocmp" - _ "google.golang.org/grpc/xds/internal/xdsclient/v2" // Register the v2 xDS API client. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // Register the v2 xDS API client. ) const ( - defaultTestTimeout = 5 * time.Second - defaultTestShortTimeout = 10 * time.Millisecond // For events expected to *not* happen. defaultClientWatchExpiryTimeout = 15 * time.Second ) @@ -54,7 +51,7 @@ func (s) TestLRSClient(t *testing.T) { } defer sCleanup() - xdsC, err := xdsclient.NewWithConfigForTesting(&bootstrap.Config{ + xdsC, err := NewWithConfigForTesting(&bootstrap.Config{ XDSServer: &bootstrap.ServerConfig{ ServerURI: fs.Address, Creds: grpc.WithTransportCredentials(insecure.NewCredentials()), diff --git a/xds/internal/xdsclient/pubsub/interface.go b/xds/internal/xdsclient/pubsub/interface.go new file mode 100644 index 000000000000..334ec101e29d --- /dev/null +++ b/xds/internal/xdsclient/pubsub/interface.go @@ -0,0 +1,39 @@ +/* + * + * Copyright 2021 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package pubsub + +import "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" + +// UpdateHandler receives and processes (by taking appropriate actions) xDS +// resource updates from an APIClient for a specific version. +// +// It's a subset of the APIs of a *Pubsub. +type UpdateHandler interface { + // NewListeners handles updates to xDS listener resources. + NewListeners(map[string]xdsresource.ListenerUpdateErrTuple, xdsresource.UpdateMetadata) + // NewRouteConfigs handles updates to xDS RouteConfiguration resources. + NewRouteConfigs(map[string]xdsresource.RouteConfigUpdateErrTuple, xdsresource.UpdateMetadata) + // NewClusters handles updates to xDS Cluster resources. + NewClusters(map[string]xdsresource.ClusterUpdateErrTuple, xdsresource.UpdateMetadata) + // NewEndpoints handles updates to xDS ClusterLoadAssignment (or tersely + // referred to as Endpoints) resources. + NewEndpoints(map[string]xdsresource.EndpointsUpdateErrTuple, xdsresource.UpdateMetadata) + // NewConnectionError handles connection errors from the xDS stream. The + // error will be reported to all the resource watchers. + NewConnectionError(err error) +} diff --git a/xds/internal/xdsclient/watchers.go b/xds/internal/xdsclient/watchers.go index bf9a07c061af..fe59dbbd6f68 100644 --- a/xds/internal/xdsclient/watchers.go +++ b/xds/internal/xdsclient/watchers.go @@ -29,11 +29,11 @@ import ( func (c *clientImpl) WatchListener(serviceName string, cb func(xdsresource.ListenerUpdate, error)) (cancel func()) { first, cancelF := c.pubsub.WatchListener(serviceName, cb) if first { - c.apiClient.AddWatch(xdsresource.ListenerResource, serviceName) + c.controller.AddWatch(xdsresource.ListenerResource, serviceName) } return func() { if cancelF() { - c.apiClient.RemoveWatch(xdsresource.ListenerResource, serviceName) + c.controller.RemoveWatch(xdsresource.ListenerResource, serviceName) } } } @@ -46,11 +46,11 @@ func (c *clientImpl) WatchListener(serviceName string, cb func(xdsresource.Liste func (c *clientImpl) WatchRouteConfig(routeName string, cb func(xdsresource.RouteConfigUpdate, error)) (cancel func()) { first, cancelF := c.pubsub.WatchRouteConfig(routeName, cb) if first { - c.apiClient.AddWatch(xdsresource.RouteConfigResource, routeName) + c.controller.AddWatch(xdsresource.RouteConfigResource, routeName) } return func() { if cancelF() { - c.apiClient.RemoveWatch(xdsresource.RouteConfigResource, routeName) + c.controller.RemoveWatch(xdsresource.RouteConfigResource, routeName) } } } @@ -67,11 +67,11 @@ func (c *clientImpl) WatchRouteConfig(routeName string, cb func(xdsresource.Rout func (c *clientImpl) WatchCluster(clusterName string, cb func(xdsresource.ClusterUpdate, error)) (cancel func()) { first, cancelF := c.pubsub.WatchCluster(clusterName, cb) if first { - c.apiClient.AddWatch(xdsresource.ClusterResource, clusterName) + c.controller.AddWatch(xdsresource.ClusterResource, clusterName) } return func() { if cancelF() { - c.apiClient.RemoveWatch(xdsresource.ClusterResource, clusterName) + c.controller.RemoveWatch(xdsresource.ClusterResource, clusterName) } } } @@ -87,11 +87,11 @@ func (c *clientImpl) WatchCluster(clusterName string, cb func(xdsresource.Cluste func (c *clientImpl) WatchEndpoints(clusterName string, cb func(xdsresource.EndpointsUpdate, error)) (cancel func()) { first, cancelF := c.pubsub.WatchEndpoints(clusterName, cb) if first { - c.apiClient.AddWatch(xdsresource.EndpointsResource, clusterName) + c.controller.AddWatch(xdsresource.EndpointsResource, clusterName) } return func() { if cancelF() { - c.apiClient.RemoveWatch(xdsresource.EndpointsResource, clusterName) + c.controller.RemoveWatch(xdsresource.EndpointsResource, clusterName) } } } diff --git a/xds/internal/xdsclient/watchers_cluster_test.go b/xds/internal/xdsclient/watchers_cluster_test.go index 729990ad95bb..98be38869bc6 100644 --- a/xds/internal/xdsclient/watchers_cluster_test.go +++ b/xds/internal/xdsclient/watchers_cluster_test.go @@ -35,7 +35,7 @@ import ( // - an update for another resource name // - an update is received after cancel() func (s) TestClusterWatch(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -50,7 +50,7 @@ func (s) TestClusterWatch(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() cancelWatch := client.WatchCluster(testCDSName, func(update xdsresource.ClusterUpdate, err error) { @@ -92,7 +92,7 @@ func (s) TestClusterWatch(t *testing.T) { // TestClusterTwoWatchSameResourceName covers the case where an update is received // after two watch() for the same resource name. func (s) TestClusterTwoWatchSameResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -107,7 +107,7 @@ func (s) TestClusterTwoWatchSameResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) var clusterUpdateChs []*testutils.Channel var cancelLastWatch func() @@ -164,7 +164,7 @@ func (s) TestClusterTwoWatchSameResourceName(t *testing.T) { // TestClusterThreeWatchDifferentResourceName covers the case where an update is // received after three watch() for different resource names. func (s) TestClusterThreeWatchDifferentResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -179,7 +179,7 @@ func (s) TestClusterThreeWatchDifferentResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) // Two watches for the same name. var clusterUpdateChs []*testutils.Channel @@ -229,7 +229,7 @@ func (s) TestClusterThreeWatchDifferentResourceName(t *testing.T) { // TestClusterWatchAfterCache covers the case where watch is called after the update // is in cache. func (s) TestClusterWatchAfterCache(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -244,7 +244,7 @@ func (s) TestClusterWatchAfterCache(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() client.WatchCluster(testCDSName, func(update xdsresource.ClusterUpdate, err error) { @@ -290,7 +290,7 @@ func (s) TestClusterWatchAfterCache(t *testing.T) { // an CDS response for the request that it sends out. We want the watch callback // to be invoked with an error once the watchExpiryTimer fires. func (s) TestClusterWatchExpiryTimer(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, true)) @@ -305,7 +305,7 @@ func (s) TestClusterWatchExpiryTimer(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() client.WatchCluster(testCDSName, func(u xdsresource.ClusterUpdate, err error) { @@ -329,7 +329,7 @@ func (s) TestClusterWatchExpiryTimer(t *testing.T) { // an CDS response for the request that it sends out. We want no error even // after expiry timeout. func (s) TestClusterWatchExpiryTimerStop(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, true)) @@ -344,7 +344,7 @@ func (s) TestClusterWatchExpiryTimerStop(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() client.WatchCluster(testCDSName, func(u xdsresource.ClusterUpdate, err error) { @@ -377,7 +377,7 @@ func (s) TestClusterWatchExpiryTimerStop(t *testing.T) { // - one more update without the removed resource // - the callback (above) shouldn't receive any update func (s) TestClusterResourceRemoved(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -392,7 +392,7 @@ func (s) TestClusterResourceRemoved(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh1 := testutils.NewChannel() client.WatchCluster(testCDSName+"1", func(update xdsresource.ClusterUpdate, err error) { @@ -460,7 +460,7 @@ func (s) TestClusterResourceRemoved(t *testing.T) { // TestClusterWatchNACKError covers the case that an update is NACK'ed, and the // watcher should also receive the error. func (s) TestClusterWatchNACKError(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -475,7 +475,7 @@ func (s) TestClusterWatchNACKError(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) clusterUpdateCh := testutils.NewChannel() cancelWatch := client.WatchCluster(testCDSName, func(update xdsresource.ClusterUpdate, err error) { @@ -500,7 +500,7 @@ func (s) TestClusterWatchNACKError(t *testing.T) { // But the watchers with valid resources should receive the update, those with // invalida resources should receive an error. func (s) TestClusterWatchPartialValid(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -515,7 +515,7 @@ func (s) TestClusterWatchPartialValid(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const badResourceName = "bad-resource" updateChs := make(map[string]*testutils.Channel) diff --git a/xds/internal/xdsclient/watchers_endpoints_test.go b/xds/internal/xdsclient/watchers_endpoints_test.go index eddc17ed1bd6..4ae59d2f1e92 100644 --- a/xds/internal/xdsclient/watchers_endpoints_test.go +++ b/xds/internal/xdsclient/watchers_endpoints_test.go @@ -53,7 +53,7 @@ var ( // - an update for another resource name (which doesn't trigger callback) // - an update is received after cancel() func (s) TestEndpointsWatch(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -68,7 +68,7 @@ func (s) TestEndpointsWatch(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) endpointsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchEndpoints(testCDSName, func(update xdsresource.EndpointsUpdate, err error) { @@ -110,7 +110,7 @@ func (s) TestEndpointsWatch(t *testing.T) { // TestEndpointsTwoWatchSameResourceName covers the case where an update is received // after two watch() for the same resource name. func (s) TestEndpointsTwoWatchSameResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -125,7 +125,7 @@ func (s) TestEndpointsTwoWatchSameResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const count = 2 var ( @@ -184,7 +184,7 @@ func (s) TestEndpointsTwoWatchSameResourceName(t *testing.T) { // TestEndpointsThreeWatchDifferentResourceName covers the case where an update is // received after three watch() for different resource names. func (s) TestEndpointsThreeWatchDifferentResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -199,7 +199,7 @@ func (s) TestEndpointsThreeWatchDifferentResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) // Two watches for the same name. var endpointsUpdateChs []*testutils.Channel @@ -249,7 +249,7 @@ func (s) TestEndpointsThreeWatchDifferentResourceName(t *testing.T) { // TestEndpointsWatchAfterCache covers the case where watch is called after the update // is in cache. func (s) TestEndpointsWatchAfterCache(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -264,7 +264,7 @@ func (s) TestEndpointsWatchAfterCache(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) endpointsUpdateCh := testutils.NewChannel() client.WatchEndpoints(testCDSName, func(update xdsresource.EndpointsUpdate, err error) { @@ -308,7 +308,7 @@ func (s) TestEndpointsWatchAfterCache(t *testing.T) { // an CDS response for the request that it sends out. We want the watch callback // to be invoked with an error once the watchExpiryTimer fires. func (s) TestEndpointsWatchExpiryTimer(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, true)) @@ -323,7 +323,7 @@ func (s) TestEndpointsWatchExpiryTimer(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) endpointsUpdateCh := testutils.NewChannel() client.WatchEndpoints(testCDSName, func(update xdsresource.EndpointsUpdate, err error) { @@ -346,7 +346,7 @@ func (s) TestEndpointsWatchExpiryTimer(t *testing.T) { // TestEndpointsWatchNACKError covers the case that an update is NACK'ed, and // the watcher should also receive the error. func (s) TestEndpointsWatchNACKError(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -361,7 +361,7 @@ func (s) TestEndpointsWatchNACKError(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) endpointsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchEndpoints(testCDSName, func(update xdsresource.EndpointsUpdate, err error) { @@ -384,7 +384,7 @@ func (s) TestEndpointsWatchNACKError(t *testing.T) { // But the watchers with valid resources should receive the update, those with // invalida resources should receive an error. func (s) TestEndpointsWatchPartialValid(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -399,7 +399,7 @@ func (s) TestEndpointsWatchPartialValid(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const badResourceName = "bad-resource" updateChs := make(map[string]*testutils.Channel) diff --git a/xds/internal/xdsclient/watchers_listener_test.go b/xds/internal/xdsclient/watchers_listener_test.go index 3ced8b81f7a0..7446975a1511 100644 --- a/xds/internal/xdsclient/watchers_listener_test.go +++ b/xds/internal/xdsclient/watchers_listener_test.go @@ -35,7 +35,7 @@ import ( // - an update for another resource name // - an update is received after cancel() func (s) TestLDSWatch(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -50,7 +50,7 @@ func (s) TestLDSWatch(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) ldsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchListener(testLDSName, func(update xdsresource.ListenerUpdate, err error) { @@ -91,7 +91,7 @@ func (s) TestLDSWatch(t *testing.T) { // TestLDSTwoWatchSameResourceName covers the case where an update is received // after two watch() for the same resource name. func (s) TestLDSTwoWatchSameResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -106,7 +106,7 @@ func (s) TestLDSTwoWatchSameResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const count = 2 var ( @@ -166,7 +166,7 @@ func (s) TestLDSTwoWatchSameResourceName(t *testing.T) { // TestLDSThreeWatchDifferentResourceName covers the case where an update is // received after three watch() for different resource names. func (s) TestLDSThreeWatchDifferentResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -181,7 +181,7 @@ func (s) TestLDSThreeWatchDifferentResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) var ldsUpdateChs []*testutils.Channel const count = 2 @@ -232,7 +232,7 @@ func (s) TestLDSThreeWatchDifferentResourceName(t *testing.T) { // TestLDSWatchAfterCache covers the case where watch is called after the update // is in cache. func (s) TestLDSWatchAfterCache(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -247,7 +247,7 @@ func (s) TestLDSWatchAfterCache(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) ldsUpdateCh := testutils.NewChannel() client.WatchListener(testLDSName, func(update xdsresource.ListenerUpdate, err error) { @@ -294,7 +294,7 @@ func (s) TestLDSWatchAfterCache(t *testing.T) { // - one more update without the removed resource // - the callback (above) shouldn't receive any update func (s) TestLDSResourceRemoved(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -309,7 +309,7 @@ func (s) TestLDSResourceRemoved(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) ldsUpdateCh1 := testutils.NewChannel() client.WatchListener(testLDSName+"1", func(update xdsresource.ListenerUpdate, err error) { @@ -376,7 +376,7 @@ func (s) TestLDSResourceRemoved(t *testing.T) { // TestListenerWatchNACKError covers the case that an update is NACK'ed, and the // watcher should also receive the error. func (s) TestListenerWatchNACKError(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -391,7 +391,7 @@ func (s) TestListenerWatchNACKError(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) ldsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchListener(testLDSName, func(update xdsresource.ListenerUpdate, err error) { @@ -414,7 +414,7 @@ func (s) TestListenerWatchNACKError(t *testing.T) { // But the watchers with valid resources should receive the update, those with // invalida resources should receive an error. func (s) TestListenerWatchPartialValid(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -429,7 +429,7 @@ func (s) TestListenerWatchPartialValid(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const badResourceName = "bad-resource" updateChs := make(map[string]*testutils.Channel) @@ -472,7 +472,7 @@ func (s) TestListenerWatchPartialValid(t *testing.T) { // TestListenerWatch_RedundantUpdateSupression tests scenarios where an update // with an unmodified resource is suppressed, and modified resource is not. func (s) TestListenerWatch_RedundantUpdateSupression(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -487,7 +487,7 @@ func (s) TestListenerWatch_RedundantUpdateSupression(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) ldsUpdateCh := testutils.NewChannel() client.WatchListener(testLDSName, func(update xdsresource.ListenerUpdate, err error) { diff --git a/xds/internal/xdsclient/watchers_route_test.go b/xds/internal/xdsclient/watchers_route_test.go index d01e03911eae..ea7b06ae1fd9 100644 --- a/xds/internal/xdsclient/watchers_route_test.go +++ b/xds/internal/xdsclient/watchers_route_test.go @@ -35,7 +35,7 @@ import ( // - an update for another resource name (which doesn't trigger callback) // - an update is received after cancel() func (s) TestRDSWatch(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -50,7 +50,7 @@ func (s) TestRDSWatch(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) rdsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchRouteConfig(testRDSName, func(update xdsresource.RouteConfigUpdate, err error) { @@ -99,7 +99,7 @@ func (s) TestRDSWatch(t *testing.T) { // TestRDSTwoWatchSameResourceName covers the case where an update is received // after two watch() for the same resource name. func (s) TestRDSTwoWatchSameResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -114,7 +114,7 @@ func (s) TestRDSTwoWatchSameResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const count = 2 var ( @@ -181,7 +181,7 @@ func (s) TestRDSTwoWatchSameResourceName(t *testing.T) { // TestRDSThreeWatchDifferentResourceName covers the case where an update is // received after three watch() for different resource names. func (s) TestRDSThreeWatchDifferentResourceName(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -196,7 +196,7 @@ func (s) TestRDSThreeWatchDifferentResourceName(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) // Two watches for the same name. var rdsUpdateChs []*testutils.Channel @@ -260,7 +260,7 @@ func (s) TestRDSThreeWatchDifferentResourceName(t *testing.T) { // TestRDSWatchAfterCache covers the case where watch is called after the update // is in cache. func (s) TestRDSWatchAfterCache(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -275,7 +275,7 @@ func (s) TestRDSWatchAfterCache(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) rdsUpdateCh := testutils.NewChannel() client.WatchRouteConfig(testRDSName, func(update xdsresource.RouteConfigUpdate, err error) { @@ -325,7 +325,7 @@ func (s) TestRDSWatchAfterCache(t *testing.T) { // TestRouteWatchNACKError covers the case that an update is NACK'ed, and the // watcher should also receive the error. func (s) TestRouteWatchNACKError(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -340,7 +340,7 @@ func (s) TestRouteWatchNACKError(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) rdsUpdateCh := testutils.NewChannel() cancelWatch := client.WatchRouteConfig(testCDSName, func(update xdsresource.RouteConfigUpdate, err error) { @@ -363,7 +363,7 @@ func (s) TestRouteWatchNACKError(t *testing.T) { // But the watchers with valid resources should receive the update, those with // invalida resources should receive an error. func (s) TestRouteWatchPartialValid(t *testing.T) { - apiClientCh, cleanup := overrideNewAPIClient() + apiClientCh, cleanup := overrideNewController() defer cleanup() client, err := newWithConfig(clientOpts(testXDSServer, false)) @@ -378,7 +378,7 @@ func (s) TestRouteWatchPartialValid(t *testing.T) { if err != nil { t.Fatalf("timeout when waiting for API client to be created: %v", err) } - apiClient := c.(*testAPIClient) + apiClient := c.(*testController) const badResourceName = "bad-resource" updateChs := make(map[string]*testutils.Channel) diff --git a/xds/internal/xdsclient/xdsclient_test.go b/xds/internal/xdsclient/xdsclient_test.go index 23d6d6f54183..92423a59f296 100644 --- a/xds/internal/xdsclient/xdsclient_test.go +++ b/xds/internal/xdsclient/xdsclient_test.go @@ -26,10 +26,11 @@ import ( "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/grpctest" "google.golang.org/grpc/xds/internal/testutils" - "google.golang.org/grpc/xds/internal/version" "google.golang.org/grpc/xds/internal/xdsclient" "google.golang.org/grpc/xds/internal/xdsclient/bootstrap" - _ "google.golang.org/grpc/xds/internal/xdsclient/v2" // Register the v2 API client. + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" + + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // Register the v2 API client. ) type s struct { diff --git a/xds/internal/xdsclient/xdsresource/filter_chain.go b/xds/internal/xdsclient/xdsresource/filter_chain.go index 10c779229622..88dc81305b26 100644 --- a/xds/internal/xdsclient/xdsresource/filter_chain.go +++ b/xds/internal/xdsclient/xdsresource/filter_chain.go @@ -31,7 +31,7 @@ import ( "google.golang.org/grpc/internal/resolver" "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/httpfilter" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) const ( diff --git a/xds/internal/xdsclient/xdsresource/filter_chain_test.go b/xds/internal/xdsclient/xdsresource/filter_chain_test.go index dc1ea75778bf..daa3ad46cbfb 100644 --- a/xds/internal/xdsclient/xdsresource/filter_chain_test.go +++ b/xds/internal/xdsclient/xdsresource/filter_chain_test.go @@ -43,7 +43,7 @@ import ( "google.golang.org/grpc/xds/internal/httpfilter" "google.golang.org/grpc/xds/internal/httpfilter/router" "google.golang.org/grpc/xds/internal/testutils/e2e" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) const ( diff --git a/xds/internal/xdsclient/xdsresource/type.go b/xds/internal/xdsclient/xdsresource/type.go index bb7e9c1520e9..c64f7c609c62 100644 --- a/xds/internal/xdsclient/xdsresource/type.go +++ b/xds/internal/xdsclient/xdsresource/type.go @@ -20,7 +20,7 @@ package xdsresource import ( "time" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/anypb" ) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_cds.go b/xds/internal/xdsclient/xdsresource/unmarshal_cds.go index a1c6c3ea7a62..01ebe7135326 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_cds.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_cds.go @@ -32,7 +32,7 @@ import ( "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/internal/xds/matcher" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/anypb" ) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go index 3a56965bdc4e..16874e3c1789 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_cds_test.go @@ -36,7 +36,7 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/internal/xds/matcher" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/wrapperspb" ) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go index 770dbf4c5253..324d7d250f69 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_eds_test.go @@ -32,7 +32,7 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/xds/internal" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" ) func (s) TestEDSParseRespProto(t *testing.T) { diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_lds.go b/xds/internal/xdsclient/xdsresource/unmarshal_lds.go index 3a1d0f63156f..f9663d05bee3 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_lds.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_lds.go @@ -32,7 +32,7 @@ import ( "google.golang.org/grpc/internal/grpclog" "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/xds/internal/httpfilter" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/anypb" ) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_lds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_lds_test.go index 503a634f1f9a..101735c25cab 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_lds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_lds_test.go @@ -32,7 +32,7 @@ import ( _ "google.golang.org/grpc/xds/internal/httpfilter/rbac" _ "google.golang.org/grpc/xds/internal/httpfilter/router" "google.golang.org/grpc/xds/internal/testutils/e2e" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/durationpb" v1udpatypepb "github.com/cncf/udpa/go/udpa/type/v1" diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_rds.go b/xds/internal/xdsclient/xdsresource/unmarshal_rds.go index 9388f9a8d2e3..3e33a0d61bf6 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_rds.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_rds.go @@ -31,7 +31,7 @@ import ( "google.golang.org/grpc/internal/pretty" "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/clusterspecifier" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/anypb" ) diff --git a/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go b/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go index b48940927ba4..b06317c373fe 100644 --- a/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go +++ b/xds/internal/xdsclient/xdsresource/unmarshal_rds_test.go @@ -32,7 +32,7 @@ import ( "google.golang.org/grpc/internal/xds/env" "google.golang.org/grpc/xds/internal/clusterspecifier" "google.golang.org/grpc/xds/internal/httpfilter" - "google.golang.org/grpc/xds/internal/version" + "google.golang.org/grpc/xds/internal/xdsclient/xdsresource/version" "google.golang.org/protobuf/types/known/durationpb" v2xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" diff --git a/xds/internal/version/version.go b/xds/internal/xdsclient/xdsresource/version/version.go similarity index 100% rename from xds/internal/version/version.go rename to xds/internal/xdsclient/xdsresource/version/version.go diff --git a/xds/xds.go b/xds/xds.go index 27547b56d226..818af0367ad0 100644 --- a/xds/xds.go +++ b/xds/xds.go @@ -36,14 +36,14 @@ import ( "google.golang.org/grpc/resolver" "google.golang.org/grpc/xds/csds" - _ "google.golang.org/grpc/credentials/tls/certprovider/pemfile" // Register the file watcher certificate provider plugin. - _ "google.golang.org/grpc/xds/internal/balancer" // Register the balancers. - _ "google.golang.org/grpc/xds/internal/httpfilter/fault" // Register the fault injection filter. - _ "google.golang.org/grpc/xds/internal/httpfilter/rbac" // Register the RBAC filter. - _ "google.golang.org/grpc/xds/internal/httpfilter/router" // Register the router filter. - xdsresolver "google.golang.org/grpc/xds/internal/resolver" // Register the xds_resolver. - _ "google.golang.org/grpc/xds/internal/xdsclient/v2" // Register the v2 xDS API client. - _ "google.golang.org/grpc/xds/internal/xdsclient/v3" // Register the v3 xDS API client. + _ "google.golang.org/grpc/credentials/tls/certprovider/pemfile" // Register the file watcher certificate provider plugin. + _ "google.golang.org/grpc/xds/internal/balancer" // Register the balancers. + _ "google.golang.org/grpc/xds/internal/httpfilter/fault" // Register the fault injection filter. + _ "google.golang.org/grpc/xds/internal/httpfilter/rbac" // Register the RBAC filter. + _ "google.golang.org/grpc/xds/internal/httpfilter/router" // Register the router filter. + xdsresolver "google.golang.org/grpc/xds/internal/resolver" // Register the xds_resolver. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v2" // Register the v2 xDS API client. + _ "google.golang.org/grpc/xds/internal/xdsclient/controller/version/v3" // Register the v3 xDS API client. ) func init() {