diff --git a/api/envoy/api/v2/core/config_source.proto b/api/envoy/api/v2/core/config_source.proto index ea86c2cd861a7..b1e0fa5824242 100644 --- a/api/envoy/api/v2/core/config_source.proto +++ b/api/envoy/api/v2/core/config_source.proto @@ -30,12 +30,15 @@ message ApiConfigSource { // the v2 protos is used. REST = 1; - // "State of the world" gRPC v2 API, using Discovery{Request,Response} protos. + // gRPC v2 API. GRPC = 2; - // "Delta" gRPC v2 API, using DeltaDiscovery{Request,Response} protos. - // Rather than sending Envoy the entire state with every update, the xDS server - // only sends what has changed since the last update. + // Using the delta xDS gRPC service, i.e. DeltaDiscovery{Request,Response} + // rather than Discovery{Request,Response}. Rather than sending Envoy the entire state + // with every update, the xDS server only sends what has changed since the last update. + // + // DELTA_GRPC is not yet entirely implemented! Initially, only CDS is available. + // Do not use for other xDSes. TODO(fredlas) update/remove this warning when appropriate. DELTA_GRPC = 3; } diff --git a/api/envoy/api/v3alpha/core/config_source.proto b/api/envoy/api/v3alpha/core/config_source.proto index d470fdef9af16..5da66d99fa83c 100644 --- a/api/envoy/api/v3alpha/core/config_source.proto +++ b/api/envoy/api/v3alpha/core/config_source.proto @@ -30,12 +30,15 @@ message ApiConfigSource { // the v2 protos is used. REST = 1; - // "State of the world" gRPC v2 API, using Discovery{Request,Response} protos. + // gRPC v2 API. GRPC = 2; - // "Delta" gRPC v2 API, using DeltaDiscovery{Request,Response} protos. - // Rather than sending Envoy the entire state with every update, the xDS server - // only sends what has changed since the last update. + // Using the delta xDS gRPC service, i.e. DeltaDiscovery{Request,Response} + // rather than Discovery{Request,Response}. Rather than sending Envoy the entire state + // with every update, the xDS server only sends what has changed since the last update. + // + // DELTA_GRPC is not yet entirely implemented! Initially, only CDS is available. + // Do not use for other xDSes. TODO(fredlas) update/remove this warning when appropriate. DELTA_GRPC = 3; } diff --git a/include/envoy/config/grpc_mux.h b/include/envoy/config/grpc_mux.h index c07b0bab16464..8d65e28d58faf 100644 --- a/include/envoy/config/grpc_mux.h +++ b/include/envoy/config/grpc_mux.h @@ -25,6 +25,48 @@ struct ControlPlaneStats { ALL_CONTROL_PLANE_STATS(GENERATE_COUNTER_STRUCT, GENERATE_GAUGE_STRUCT) }; +// TODO(fredlas) redundant to SubscriptionCallbacks; remove this one. +class GrpcMuxCallbacks { +public: + virtual ~GrpcMuxCallbacks() = default; + + /** + * Called when a configuration update is received. + * @param resources vector of fetched resources corresponding to the configuration update. + * @param version_info update version. + * @throw EnvoyException with reason if the configuration is rejected. Otherwise the configuration + * is accepted. Accepted configurations have their version_info reflected in subsequent + * requests. + */ + virtual void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) PURE; + + /** + * Called when either the subscription is unable to fetch a config update or when onConfigUpdate + * invokes an exception. + * @param reason supplies the update failure reason. + * @param e supplies any exception data on why the fetch failed. May be nullptr. + */ + virtual void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, + const EnvoyException* e) PURE; + + /** + * Obtain the "name" of a v2 API resource in a google.protobuf.Any, e.g. the route config name for + * a RouteConfiguration, based on the underlying resource type. + */ + virtual std::string resourceName(const ProtobufWkt::Any& resource) PURE; +}; + +/** + * Handle on an muxed gRPC subscription. The subscription is canceled on destruction. + */ +class GrpcMuxWatch { +public: + virtual ~GrpcMuxWatch() = default; +}; + +using GrpcMuxWatchPtr = std::unique_ptr; + struct Watch; /** @@ -40,12 +82,27 @@ class GrpcMux { */ virtual void start() PURE; + /** + * Start a configuration subscription asynchronously for some API type and resources. + * @param type_url type URL corresponding to xDS API, e.g. + * type.googleapis.com/envoy.api.v2.Cluster. + * @param resources set of resource names to watch for. If this is empty, then all + * resources for type_url will result in callbacks. + * @param callbacks the callbacks to be notified of configuration updates. These must be valid + * until GrpcMuxWatch is destroyed. + * @return GrpcMuxWatchPtr a handle to cancel the subscription with. E.g. when a cluster goes + * away, its EDS updates should be cancelled by destroying the GrpcMuxWatchPtr. + */ + virtual GrpcMuxWatchPtr subscribe(const std::string& type_url, + const std::set& resources, + GrpcMuxCallbacks& callbacks) PURE; + /** * Pause discovery requests for a given API type. This is useful when we're processing an update * for LDS or CDS and don't want a flood of updates for RDS or EDS respectively. Discovery * requests may later be resumed with resume(). * @param type_url type URL corresponding to xDS API, e.g. - * type.googleapis.com/envoy.api.v2.Cluster + * type.googleapis.com/envoy.api.v2.Cluster. */ virtual void pause(const std::string& type_url) PURE; @@ -56,30 +113,18 @@ class GrpcMux { */ virtual void resume(const std::string& type_url) PURE; + // TODO(fredlas) PR #8478 will remove this. /** - * Registers a GrpcSubscription with the GrpcMux. 'watch' may be null (meaning this is an add), - * or it may be the Watch* previously returned by this function (which makes it an update). - * @param type_url type URL corresponding to xDS API e.g. type.googleapis.com/envoy.api.v2.Cluster - * @param watch the Watch* to be updated, or nullptr to add one. - * @param resources the set of resource names for 'watch' to start out interested in. If empty, - * 'watch' is treated as interested in *all* resources (of type type_url). - * @param callbacks the callbacks that receive updates for 'resources' when they arrive. - * @param init_fetch_timeout how long to wait for this new subscription's first update. Ignored - * unless the addOrUpdateWatch() call is the first for 'type_url'. - * @return Watch* the opaque watch token added or updated, to be used in future addOrUpdateWatch - * calls. + * Whether this GrpcMux is delta. + * @return bool whether this GrpcMux is delta. */ + virtual bool isDelta() const PURE; + + // For delta virtual Watch* addOrUpdateWatch(const std::string& type_url, Watch* watch, const std::set& resources, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout) PURE; - - /** - * Cleanup of a Watch* added by addOrUpdateWatch(). Receiving a Watch* from addOrUpdateWatch() - * makes you responsible for eventually invoking this cleanup. - * @param type_url type URL corresponding to xDS API e.g. type.googleapis.com/envoy.api.v2.Cluster - * @param watch the watch to be cleaned up. - */ virtual void removeWatch(const std::string& type_url, Watch* watch) PURE; /** @@ -89,12 +134,6 @@ class GrpcMux { * @return bool whether the API is paused. */ virtual bool paused(const std::string& type_url) const PURE; - - /** - * Passes through to all multiplexed SubscriptionStates. To be called when something - * definitive happens with the initial fetch: either an update is successfully received, - * or some sort of error happened.*/ - virtual void disableInitFetchTimeoutTimer() PURE; }; using GrpcMuxPtr = std::unique_ptr; diff --git a/source/common/config/BUILD b/source/common/config/BUILD index 72cf3f94c9917..bbce50a5a4d40 100644 --- a/source/common/config/BUILD +++ b/source/common/config/BUILD @@ -43,12 +43,12 @@ envoy_cc_library( ) envoy_cc_library( - name = "grpc_subscription_lib", - srcs = ["grpc_subscription_impl.cc"], - hdrs = ["grpc_subscription_impl.h"], + name = "delta_subscription_lib", + srcs = ["delta_subscription_impl.cc"], + hdrs = ["delta_subscription_impl.h"], deps = [ - ":grpc_mux_lib", ":grpc_stream_lib", + ":new_grpc_mux_lib", ":utility_lib", "//include/envoy/config:subscription_interface", "//include/envoy/grpc:async_client_interface", @@ -66,35 +66,15 @@ envoy_cc_library( srcs = ["delta_subscription_state.cc"], hdrs = ["delta_subscription_state.h"], deps = [ - ":subscription_state_lib", - "//source/common/grpc:common_lib", - "//source/common/protobuf", - "@envoy_api//envoy/api/v2:pkg_cc_proto", - ], -) - -envoy_cc_library( - name = "sotw_subscription_state_lib", - srcs = ["sotw_subscription_state.cc"], - hdrs = ["sotw_subscription_state.h"], - deps = [ - ":subscription_state_lib", - "//source/common/grpc:common_lib", - "//source/common/protobuf", - "@envoy_api//envoy/api/v2:pkg_cc_proto", - ], -) - -envoy_cc_library( - name = "subscription_state_lib", - srcs = ["subscription_state.cc"], - hdrs = ["subscription_state.h"], - deps = [ - ":update_ack_lib", + ":pausable_ack_queue_lib", "//include/envoy/config:subscription_interface", "//include/envoy/event:dispatcher_interface", - "//include/envoy/local_info:local_info_interface", + "//source/common/common:assert_lib", + "//source/common/common:backoff_lib", "//source/common/common:minimal_logger_lib", + "//source/common/common:token_bucket_impl_lib", + "//source/common/grpc:common_lib", + "//source/common/protobuf", "@envoy_api//envoy/api/v2:pkg_cc_proto", ], ) @@ -137,11 +117,54 @@ envoy_cc_library( name = "grpc_mux_lib", srcs = ["grpc_mux_impl.cc"], hdrs = ["grpc_mux_impl.h"], + deps = [ + ":grpc_stream_lib", + ":utility_lib", + "//include/envoy/config:grpc_mux_interface", + "//include/envoy/config:subscription_interface", + "//include/envoy/upstream:cluster_manager_interface", + "//source/common/common:minimal_logger_lib", + "//source/common/protobuf", + ], +) + +envoy_cc_library( + name = "grpc_mux_subscription_lib", + srcs = ["grpc_mux_subscription_impl.cc"], + hdrs = ["grpc_mux_subscription_impl.h"], + deps = [ + "//include/envoy/config:grpc_mux_interface", + "//include/envoy/config:subscription_interface", + "//include/envoy/event:dispatcher_interface", + "//source/common/common:assert_lib", + "//source/common/common:minimal_logger_lib", + "//source/common/grpc:common_lib", + "//source/common/protobuf", + "@envoy_api//envoy/api/v2:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "grpc_subscription_lib", + hdrs = ["grpc_subscription_impl.h"], + deps = [ + ":grpc_mux_lib", + ":grpc_mux_subscription_lib", + "//include/envoy/config:subscription_interface", + "//include/envoy/event:dispatcher_interface", + "//include/envoy/grpc:async_client_interface", + "@envoy_api//envoy/api/v2/core:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "new_grpc_mux_lib", + srcs = ["new_grpc_mux_impl.cc"], + hdrs = ["new_grpc_mux_impl.h"], deps = [ ":delta_subscription_state_lib", ":grpc_stream_lib", ":pausable_ack_queue_lib", - ":sotw_subscription_state_lib", ":watch_map_lib", "//include/envoy/event:dispatcher_interface", "//include/envoy/grpc:async_client_interface", @@ -189,8 +212,8 @@ envoy_cc_library( srcs = ["pausable_ack_queue.cc"], hdrs = ["pausable_ack_queue.h"], deps = [ - ":update_ack_lib", "//source/common/common:assert_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", ], ) @@ -234,7 +257,9 @@ envoy_cc_library( srcs = ["subscription_factory_impl.cc"], hdrs = ["subscription_factory_impl.h"], deps = [ + ":delta_subscription_lib", ":filesystem_subscription_lib", + ":grpc_mux_subscription_lib", ":grpc_subscription_lib", ":http_subscription_lib", ":type_to_endpoint_lib", @@ -259,12 +284,6 @@ envoy_cc_library( ], ) -envoy_cc_library( - name = "update_ack_lib", - hdrs = ["update_ack.h"], - deps = ["@envoy_api//envoy/api/v2:pkg_cc_proto"], -) - envoy_cc_library( name = "utility_lib", srcs = ["utility.cc"], diff --git a/source/common/config/README.md b/source/common/config/README.md index cfcecd7678a18..3135bd56bdaa8 100644 --- a/source/common/config/README.md +++ b/source/common/config/README.md @@ -9,19 +9,24 @@ you can mostly forget the filesystem/REST/gRPC distinction, and you can especially forget about the gRPC flavors. All of that is specified in the bootstrap config, which is read and put into action by ClusterManagerImpl. -## If you are working on Envoy's gRPC xDS client logic itself, read on. +Note that there can be multiple active gRPC subscriptions for a single resource +type. This concept is called "resource watches". If one EDS subscription +subscribes to X and Y, and another subscribes to Y and Z, the underlying +subscription logic will maintain a subscription to the union: X Y and Z. Updates +to X will be delivered to the first object, Y to both, Z to the second. This +logic is implemented by WatchMap. + +### If you are working on Envoy's gRPC xDS client logic itself, read on. When using gRPC, xDS has two pairs of options: aggregated/non-aggregated, and delta/state-of-the-world updates. All four combinations of these are usable. -## Aggregated (ADS) vs not (xDS) - "Aggregated" means that EDS, CDS, etc resources are all carried by the same gRPC stream. For Envoy's implementation of xDS client logic, there is effectively no difference between aggregated xDS and non-aggregated: they both use the same request/response protos. The non-aggregated case is handled by running the aggregated logic, and just happening to only have 1 -xDS subscription type to "aggregate", i.e., GrpcMux only has one SubscriptionState -entry in its map. +xDS subscription type to "aggregate", i.e., NewGrpcMuxImpl only has one +DeltaSubscriptionState entry in its map. However, to the config server, there is a huge difference: when using ADS (caused by the user providing an ads_config in the bootstrap config), the gRPC client sets @@ -29,31 +34,14 @@ its method string to {Delta,Stream}AggregatedResources, as opposed to {Delta,Str {Delta,Stream}Routes, etc. So, despite using the same request/response protos, and having identical client code, they're actually different gRPC services. -## Delta vs state-of-the-world (SotW) - -Delta vs state-of-the-world is a question of wire format and protocol behavior. -The protos in question are named [Delta]Discovery{Request,Response}. GrpcMux can work -with either pair. Almost all GrpcMux logic is in the shared GrpcMuxImpl base class; -SotwGrpcMux and DeltaGrpcMux exist to be adapters for the specific protobuf types, since -protobufs are not amenable to polymorphism. - -All delta/SotW specific logic is handled by GrpcMux and SubscriptionState. GrpcSubscriptionImpl -simply holds a shared_ptr to a GrpcMux interface; it has no need to know about delta vs SotW. +Delta vs state-of-the-world is a question of wire format: the protos in question are named +[Delta]Discovery{Request,Response}. That is what the GrpcMux interface is useful for: its +NewGrpcMuxImpl (TODO may be renamed) implementation works with DeltaDiscovery{Request,Response} and has +delta-specific logic; its GrpxMuxImpl implementation (TODO will be merged into NewGrpcMuxImpl) works with Discovery{Request,Response} +and has SotW-specific logic. Both the delta and SotW Subscription implementations (TODO will be merged) hold a shared_ptr. +The shared_ptr allows for both non- and aggregated: if non-aggregated, you'll be the only holder of that shared_ptr. ![xDS_code_diagram](xDS_code_diagram.png) -The orange flow does not necessarily have to happen in response to the blue flow; there can be -spontaneous updates. ACKs are not shown in this diagram; they are also carried by the -[Delta]DiscoveryRequest protos. - -What does GrpcMux even do in this diagram? Just own things and pass through function calls? -Answer: 1) it sequences the requests and ACKs that the various type_urls send, 2) it handles the -protobuf vs polymorphism impedance mismatch, allowing all delta-vs-SotW-agnostic code -to be reused. - -Note that there can be multiple active gRPC subscriptions for a single resource -type. This concept is called "resource watches". If one EDS subscription -subscribes to X and Y, and another subscribes to Y and Z, the underlying -subscription logic will maintain a subscription to the union: X Y and Z. Updates -to X will be delivered to the first object, Y to both, Z to the second. This -logic is implemented by WatchMap. +Note that the orange flow does not necessarily have to happen in response to the blue flow; there can be spontaneous updates. ACKs are not shown in this diagram; they are also carred by the [Delta]DiscoveryRequest protos. +What does GrpcXdsContext even do in this diagram? Just own things and pass through function calls? Answer: it sequences the requests and ACKs that the various type_urls send. diff --git a/source/common/config/grpc_subscription_impl.cc b/source/common/config/delta_subscription_impl.cc similarity index 58% rename from source/common/config/grpc_subscription_impl.cc rename to source/common/config/delta_subscription_impl.cc index 77f003b061b57..ed34dac39bea0 100644 --- a/source/common/config/grpc_subscription_impl.cc +++ b/source/common/config/delta_subscription_impl.cc @@ -1,68 +1,66 @@ -#include "common/config/grpc_subscription_impl.h" +#include "common/config/delta_subscription_impl.h" namespace Envoy { namespace Config { -GrpcSubscriptionImpl::GrpcSubscriptionImpl(GrpcMuxSharedPtr grpc_mux, absl::string_view type_url, - SubscriptionCallbacks& callbacks, - SubscriptionStats stats, - std::chrono::milliseconds init_fetch_timeout, - bool is_aggregated) - : grpc_mux_(std::move(grpc_mux)), type_url_(type_url), callbacks_(callbacks), stats_(stats), +DeltaSubscriptionImpl::DeltaSubscriptionImpl(GrpcMuxSharedPtr context, absl::string_view type_url, + SubscriptionCallbacks& callbacks, + SubscriptionStats stats, + std::chrono::milliseconds init_fetch_timeout, + bool is_aggregated) + : context_(std::move(context)), type_url_(type_url), callbacks_(callbacks), stats_(stats), init_fetch_timeout_(init_fetch_timeout), is_aggregated_(is_aggregated) {} -GrpcSubscriptionImpl::~GrpcSubscriptionImpl() { +DeltaSubscriptionImpl::~DeltaSubscriptionImpl() { if (watch_) { - grpc_mux_->removeWatch(type_url_, watch_); + context_->removeWatch(type_url_, watch_); } } -void GrpcSubscriptionImpl::pause() { grpc_mux_->pause(type_url_); } +void DeltaSubscriptionImpl::pause() { context_->pause(type_url_); } -void GrpcSubscriptionImpl::resume() { grpc_mux_->resume(type_url_); } +void DeltaSubscriptionImpl::resume() { context_->resume(type_url_); } -// Config::Subscription -void GrpcSubscriptionImpl::start(const std::set& resources) { +// Config::DeltaSubscription +void DeltaSubscriptionImpl::start(const std::set& resources) { // ADS initial request batching relies on the users of the GrpcMux *not* calling start on it, // whereas non-ADS xDS users must call it themselves. if (!is_aggregated_) { - grpc_mux_->start(); + context_->start(); } - watch_ = grpc_mux_->addOrUpdateWatch(type_url_, watch_, resources, *this, init_fetch_timeout_); + watch_ = context_->addOrUpdateWatch(type_url_, watch_, resources, *this, init_fetch_timeout_); stats_.update_attempt_.inc(); } -void GrpcSubscriptionImpl::updateResourceInterest( +void DeltaSubscriptionImpl::updateResourceInterest( const std::set& update_to_these_names) { - watch_ = grpc_mux_->addOrUpdateWatch(type_url_, watch_, update_to_these_names, *this, - init_fetch_timeout_); + watch_ = context_->addOrUpdateWatch(type_url_, watch_, update_to_these_names, *this, + init_fetch_timeout_); stats_.update_attempt_.inc(); } // Config::SubscriptionCallbacks -void GrpcSubscriptionImpl::onConfigUpdate( +void DeltaSubscriptionImpl::onConfigUpdate( const Protobuf::RepeatedPtrField& resources, const std::string& version_info) { stats_.update_attempt_.inc(); callbacks_.onConfigUpdate(resources, version_info); - grpc_mux_->disableInitFetchTimeoutTimer(); stats_.update_success_.inc(); stats_.version_.set(HashUtil::xxHash64(version_info)); } -void GrpcSubscriptionImpl::onConfigUpdate( +void DeltaSubscriptionImpl::onConfigUpdate( const Protobuf::RepeatedPtrField& added_resources, const Protobuf::RepeatedPtrField& removed_resources, const std::string& system_version_info) { stats_.update_attempt_.inc(); callbacks_.onConfigUpdate(added_resources, removed_resources, system_version_info); - grpc_mux_->disableInitFetchTimeoutTimer(); stats_.update_success_.inc(); stats_.version_.set(HashUtil::xxHash64(system_version_info)); } -void GrpcSubscriptionImpl::onConfigUpdateFailed(ConfigUpdateFailureReason reason, - const EnvoyException* e) { +void DeltaSubscriptionImpl::onConfigUpdateFailed(ConfigUpdateFailureReason reason, + const EnvoyException* e) { switch (reason) { case Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure: // This is a gRPC-stream-level establishment failure, not an xDS-protocol-level failure. @@ -75,23 +73,21 @@ void GrpcSubscriptionImpl::onConfigUpdateFailed(ConfigUpdateFailureReason reason break; case Envoy::Config::ConfigUpdateFailureReason::FetchTimedout: stats_.init_fetch_timeout_.inc(); - grpc_mux_->disableInitFetchTimeoutTimer(); - callbacks_.onConfigUpdateFailed(reason, e); // TODO(fredlas) remove; it only makes sense to count start() and updateResourceInterest() // as attempts. stats_.update_attempt_.inc(); + callbacks_.onConfigUpdateFailed(reason, e); break; case Envoy::Config::ConfigUpdateFailureReason::UpdateRejected: // We expect Envoy exception to be thrown when update is rejected. ASSERT(e != nullptr); - grpc_mux_->disableInitFetchTimeoutTimer(); stats_.update_rejected_.inc(); callbacks_.onConfigUpdateFailed(reason, e); break; } } -std::string GrpcSubscriptionImpl::resourceName(const ProtobufWkt::Any& resource) { +std::string DeltaSubscriptionImpl::resourceName(const ProtobufWkt::Any& resource) { return callbacks_.resourceName(resource); } diff --git a/source/common/config/delta_subscription_impl.h b/source/common/config/delta_subscription_impl.h new file mode 100644 index 0000000000000..b45be7c4da85f --- /dev/null +++ b/source/common/config/delta_subscription_impl.h @@ -0,0 +1,71 @@ +#pragma once + +#include "envoy/config/subscription.h" + +#include "common/config/new_grpc_mux_impl.h" +#include "common/config/utility.h" + +namespace Envoy { +namespace Config { + +// DeltaSubscriptionImpl provides a top-level interface to the Envoy's gRPC communication with +// an xDS server, for use by the various xDS users within Envoy. It is built around a (shared) +// NewGrpcMuxImpl, and the further machinery underlying that. An xDS user indicates interest in +// various resources via start() and updateResourceInterest(). It receives updates to those +// resources via the SubscriptionCallbacks it provides. Multiple users can each have their own +// Subscription object for the same type_url; NewGrpcMuxImpl maintains a subscription to the +// union of interested resources, and delivers to the users just the resource updates that they +// are "watching" for. +// +// DeltaSubscriptionImpl and NewGrpcMuxImpl are both built to provide both regular xDS and ADS, +// distinguished by whether multiple DeltaSubscriptionImpls are sharing a single +// NewGrpcMuxImpl. (And by the gRPC method string, but that's taken care of over in +// SubscriptionFactory). +// +// Why does DeltaSubscriptionImpl itself implement the SubscriptionCallbacks interface? So that it +// can write to SubscriptionStats (which needs to live out here in the DeltaSubscriptionImpl) upon a +// config update. The idea is, DeltaSubscriptionImpl presents itself to WatchMap as the +// SubscriptionCallbacks, and then passes (after incrementing stats) all callbacks through to +// callbacks_, which are the real SubscriptionCallbacks. +class DeltaSubscriptionImpl : public Subscription, public SubscriptionCallbacks { +public: + // is_aggregated: whether the underlying mux/context is providing ADS to us and others, or whether + // it's all ours. The practical difference is that we ourselves must call start() on it only in + // the latter case. + DeltaSubscriptionImpl(GrpcMuxSharedPtr context, absl::string_view type_url, + SubscriptionCallbacks& callbacks, SubscriptionStats stats, + std::chrono::milliseconds init_fetch_timeout, bool is_aggregated); + ~DeltaSubscriptionImpl() override; + + void pause(); + void resume(); + + // Config::Subscription + void start(const std::set& resource_names) override; + void updateResourceInterest(const std::set& update_to_these_names) override; + + // Config::SubscriptionCallbacks (all pass through to callbacks_!) + void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) override; + void onConfigUpdate(const Protobuf::RepeatedPtrField& added_resources, + const Protobuf::RepeatedPtrField& removed_resources, + const std::string& system_version_info) override; + void onConfigUpdateFailed(ConfigUpdateFailureReason reason, const EnvoyException* e) override; + std::string resourceName(const ProtobufWkt::Any& resource) override; + + GrpcMuxSharedPtr getContextForTest() { return context_; } + +private: + GrpcMuxSharedPtr context_; + const std::string type_url_; + SubscriptionCallbacks& callbacks_; + SubscriptionStats stats_; + // NOTE: if another subscription of the same type_url has already been started, this value will be + // ignored in favor of the other subscription's. + std::chrono::milliseconds init_fetch_timeout_; + Watch* watch_{}; + const bool is_aggregated_; +}; + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/delta_subscription_state.cc b/source/common/config/delta_subscription_state.cc index 8eccd7343aa54..8fdfdca0ffca6 100644 --- a/source/common/config/delta_subscription_state.cc +++ b/source/common/config/delta_subscription_state.cc @@ -8,23 +8,18 @@ namespace Config { DeltaSubscriptionState::DeltaSubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, + const LocalInfo::LocalInfo& local_info, std::chrono::milliseconds init_fetch_timeout, Event::Dispatcher& dispatcher) - : SubscriptionState(std::move(type_url), callbacks, init_fetch_timeout, dispatcher) {} - -DeltaSubscriptionState::~DeltaSubscriptionState() = default; - -DeltaSubscriptionStateFactory::DeltaSubscriptionStateFactory(Event::Dispatcher& dispatcher) - : dispatcher_(dispatcher) {} - -DeltaSubscriptionStateFactory::~DeltaSubscriptionStateFactory() = default; - -std::unique_ptr -DeltaSubscriptionStateFactory::makeSubscriptionState(const std::string& type_url, - SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) { - return std::make_unique(type_url, callbacks, init_fetch_timeout, - dispatcher_); + : type_url_(std::move(type_url)), callbacks_(callbacks), local_info_(local_info), + init_fetch_timeout_(init_fetch_timeout) { + if (init_fetch_timeout_.count() > 0 && !init_fetch_timeout_timer_) { + init_fetch_timeout_timer_ = dispatcher.createTimer([this]() -> void { + ENVOY_LOG(warn, "delta config: initial fetch timed out for {}", type_url_); + callbacks_.onConfigUpdateFailed(ConfigUpdateFailureReason::FetchTimedout, nullptr); + }); + init_fetch_timeout_timer_->enableTimer(init_fetch_timeout_); + } } void DeltaSubscriptionState::updateSubscriptionInterest(const std::set& cur_added, @@ -56,13 +51,13 @@ bool DeltaSubscriptionState::subscriptionUpdatePending() const { !any_request_sent_yet_in_current_stream_; } -UpdateAck DeltaSubscriptionState::handleResponse(const void* response_proto_ptr) { - auto* response = static_cast(response_proto_ptr); +UpdateAck +DeltaSubscriptionState::handleResponse(const envoy::api::v2::DeltaDiscoveryResponse& message) { // We *always* copy the response's nonce into the next request, even if we're going to make that // request a NACK by setting error_detail. - UpdateAck ack(response->nonce(), type_url()); + UpdateAck ack(message.nonce(), type_url_); try { - handleGoodResponse(*response); + handleGoodResponse(message); } catch (const EnvoyException& e) { handleBadResponse(e, ack); } @@ -91,8 +86,8 @@ void DeltaSubscriptionState::handleGoodResponse( fmt::format("duplicate name {} found in the union of added+removed resources", name)); } } - callbacks().onConfigUpdate(message.resources(), message.removed_resources(), - message.system_version_info()); + callbacks_.onConfigUpdate(message.resources(), message.removed_resources(), + message.system_version_info()); for (const auto& resource : message.resources()) { setResourceVersion(resource.name(), resource.version()); } @@ -105,11 +100,11 @@ void DeltaSubscriptionState::handleGoodResponse( // initial_resource_versions messages, but will remind us to explicitly tell the server "I'm // cancelling my subscription" when we lose interest. for (const auto& resource_name : message.removed_resources()) { - if (resource_versions_.find(resource_name) != resource_versions_.end()) { + if (resource_names_.find(resource_name) != resource_names_.end()) { setResourceWaitingForServer(resource_name); } } - ENVOY_LOG(debug, "Delta config for {} accepted with {} resources added, {} removed", type_url(), + ENVOY_LOG(debug, "Delta config for {} accepted with {} resources added, {} removed", type_url_, message.resources().size(), message.removed_resources().size()); } @@ -118,19 +113,17 @@ void DeltaSubscriptionState::handleBadResponse(const EnvoyException& e, UpdateAc ack.error_detail_.set_code(Grpc::Status::WellKnownGrpcStatus::Internal); ack.error_detail_.set_message(e.what()); disableInitFetchTimeoutTimer(); - ENVOY_LOG(warn, "delta config for {} rejected: {}", type_url(), e.what()); - callbacks().onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, &e); + ENVOY_LOG(warn, "delta config for {} rejected: {}", type_url_, e.what()); + callbacks_.onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, &e); } void DeltaSubscriptionState::handleEstablishmentFailure() { - callbacks().onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, - nullptr); + callbacks_.onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, + nullptr); } -envoy::api::v2::DeltaDiscoveryRequest* DeltaSubscriptionState::getNextRequestInternal() { - auto* request = new envoy::api::v2::DeltaDiscoveryRequest; - request->set_type_url(type_url()); - +envoy::api::v2::DeltaDiscoveryRequest DeltaSubscriptionState::getNextRequestAckless() { + envoy::api::v2::DeltaDiscoveryRequest request; if (!any_request_sent_yet_in_current_stream_) { any_request_sent_yet_in_current_stream_ = true; // initial_resource_versions "must be populated for first request in a stream". @@ -141,7 +134,7 @@ envoy::api::v2::DeltaDiscoveryRequest* DeltaSubscriptionState::getNextRequestInt // Resources we are interested in, but are still waiting to get any version of from the // server, do not belong in initial_resource_versions. (But do belong in new subscriptions!) if (!resource.second.waitingForServer()) { - (*request->mutable_initial_resource_versions())[resource.first] = resource.second.version(); + (*request.mutable_initial_resource_versions())[resource.first] = resource.second.version(); } // As mentioned above, fill resource_names_subscribe with everything, including names we // have yet to receive any resource for. @@ -149,40 +142,50 @@ envoy::api::v2::DeltaDiscoveryRequest* DeltaSubscriptionState::getNextRequestInt } names_removed_.clear(); } - std::copy(names_added_.begin(), names_added_.end(), - Protobuf::RepeatedFieldBackInserter(request->mutable_resource_names_subscribe())); + Protobuf::RepeatedFieldBackInserter(request.mutable_resource_names_subscribe())); std::copy(names_removed_.begin(), names_removed_.end(), - Protobuf::RepeatedFieldBackInserter(request->mutable_resource_names_unsubscribe())); + Protobuf::RepeatedFieldBackInserter(request.mutable_resource_names_unsubscribe())); names_added_.clear(); names_removed_.clear(); + request.set_type_url(type_url_); + request.mutable_node()->MergeFrom(local_info_.node()); return request; } -void* DeltaSubscriptionState::getNextRequestAckless() { return getNextRequestInternal(); } - -void* DeltaSubscriptionState::getNextRequestWithAck(const UpdateAck& ack) { - envoy::api::v2::DeltaDiscoveryRequest* request = getNextRequestInternal(); - request->set_response_nonce(ack.nonce_); +envoy::api::v2::DeltaDiscoveryRequest +DeltaSubscriptionState::getNextRequestWithAck(const UpdateAck& ack) { + envoy::api::v2::DeltaDiscoveryRequest request = getNextRequestAckless(); + request.set_response_nonce(ack.nonce_); if (ack.error_detail_.code() != Grpc::Status::WellKnownGrpcStatus::Ok) { // Don't needlessly make the field present-but-empty if status is ok. - request->mutable_error_detail()->CopyFrom(ack.error_detail_); + request.mutable_error_detail()->CopyFrom(ack.error_detail_); } return request; } +void DeltaSubscriptionState::disableInitFetchTimeoutTimer() { + if (init_fetch_timeout_timer_) { + init_fetch_timeout_timer_->disableTimer(); + init_fetch_timeout_timer_.reset(); + } +} + void DeltaSubscriptionState::setResourceVersion(const std::string& resource_name, const std::string& resource_version) { resource_versions_[resource_name] = ResourceVersion(resource_version); + resource_names_.insert(resource_name); } void DeltaSubscriptionState::setResourceWaitingForServer(const std::string& resource_name) { resource_versions_[resource_name] = ResourceVersion(); + resource_names_.insert(resource_name); } void DeltaSubscriptionState::setLostInterestInResource(const std::string& resource_name) { resource_versions_.erase(resource_name); + resource_names_.erase(resource_name); } } // namespace Config diff --git a/source/common/config/delta_subscription_state.h b/source/common/config/delta_subscription_state.h index 31c34c0322d28..6477bdd014968 100644 --- a/source/common/config/delta_subscription_state.h +++ b/source/common/config/delta_subscription_state.h @@ -1,56 +1,55 @@ #pragma once #include "envoy/api/v2/discovery.pb.h" +#include "envoy/config/subscription.h" +#include "envoy/event/dispatcher.h" #include "envoy/grpc/status.h" +#include "envoy/local_info/local_info.h" #include "common/common/assert.h" #include "common/common/logger.h" -#include "common/config/subscription_state.h" - -#include "absl/types/optional.h" +#include "common/config/pausable_ack_queue.h" namespace Envoy { namespace Config { -// Tracks the state of a delta xDS-over-gRPC protocol session. -class DeltaSubscriptionState : public SubscriptionState { +// Tracks the xDS protocol state of an individual ongoing delta xDS session, i.e. a single type_url. +// There can be multiple DeltaSubscriptionStates active. They will always all be +// blissfully unaware of each other's existence, even when their messages are +// being multiplexed together by ADS. +class DeltaSubscriptionState : public Logger::Loggable { public: - // Note that, outside of tests, we expect callbacks to always be a WatchMap. DeltaSubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, + const LocalInfo::LocalInfo& local_info, std::chrono::milliseconds init_fetch_timeout, Event::Dispatcher& dispatcher); - ~DeltaSubscriptionState() override; // Update which resources we're interested in subscribing to. void updateSubscriptionInterest(const std::set& cur_added, - const std::set& cur_removed) override; + const std::set& cur_removed); // Whether there was a change in our subscription interest we have yet to inform the server of. - bool subscriptionUpdatePending() const override; + bool subscriptionUpdatePending() const; - void markStreamFresh() override { any_request_sent_yet_in_current_stream_ = false; } + void markStreamFresh() { any_request_sent_yet_in_current_stream_ = false; } - // message is expected to be a envoy::api::v2::DeltaDiscoveryResponse. - UpdateAck handleResponse(const void* response_proto_ptr) override; + UpdateAck handleResponse(const envoy::api::v2::DeltaDiscoveryResponse& message); - void handleEstablishmentFailure() override; + void handleEstablishmentFailure(); // Returns the next gRPC request proto to be sent off to the server, based on this object's // understanding of the current protocol state, and new resources that Envoy wants to request. - // Returns a new'd pointer, meant to be owned by the caller. - void* getNextRequestAckless() override; - // The WithAck version first calls the Ackless version, then adds in the passed-in ack. - // Returns a new'd pointer, meant to be owned by the caller. - void* getNextRequestWithAck(const UpdateAck& ack) override; + envoy::api::v2::DeltaDiscoveryRequest getNextRequestAckless(); + // The WithAck version first calls the Ack-less version, then adds in the passed-in ack. + envoy::api::v2::DeltaDiscoveryRequest getNextRequestWithAck(const UpdateAck& ack); DeltaSubscriptionState(const DeltaSubscriptionState&) = delete; DeltaSubscriptionState& operator=(const DeltaSubscriptionState&) = delete; private: - // Returns a new'd pointer, meant to be owned by the caller. - envoy::api::v2::DeltaDiscoveryRequest* getNextRequestInternal(); void handleGoodResponse(const envoy::api::v2::DeltaDiscoveryResponse& message); void handleBadResponse(const EnvoyException& e, UpdateAck& ack); + void disableInitFetchTimeoutTimer(); class ResourceVersion { public: @@ -85,6 +84,13 @@ class DeltaSubscriptionState : public SubscriptionState { // iterator into just its keys, e.g. for use in std::set_difference. std::set resource_names_; + const std::string type_url_; + // callbacks_ is expected to be a WatchMap. + SubscriptionCallbacks& callbacks_; + const LocalInfo::LocalInfo& local_info_; + std::chrono::milliseconds init_fetch_timeout_; + Event::TimerPtr init_fetch_timeout_timer_; + bool any_request_sent_yet_in_current_stream_{}; // Tracks changes in our subscription interest since the previous DeltaDiscoveryRequest we sent. @@ -94,17 +100,5 @@ class DeltaSubscriptionState : public SubscriptionState { std::set names_removed_; }; -class DeltaSubscriptionStateFactory : public SubscriptionStateFactory { -public: - DeltaSubscriptionStateFactory(Event::Dispatcher& dispatcher); - ~DeltaSubscriptionStateFactory() override; - std::unique_ptr - makeSubscriptionState(const std::string& type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) override; - -private: - Event::Dispatcher& dispatcher_; -}; - } // namespace Config } // namespace Envoy diff --git a/source/common/config/grpc_mux_impl.cc b/source/common/config/grpc_mux_impl.cc index 93662e0e21c49..3246dfbbd189d 100644 --- a/source/common/config/grpc_mux_impl.cc +++ b/source/common/config/grpc_mux_impl.cc @@ -1,297 +1,249 @@ #include "common/config/grpc_mux_impl.h" -#include "common/common/assert.h" -#include "common/common/backoff_strategy.h" -#include "common/common/token_bucket_impl.h" +#include + #include "common/config/utility.h" #include "common/protobuf/protobuf.h" -#include "common/protobuf/utility.h" namespace Envoy { namespace Config { -GrpcMuxImpl::GrpcMuxImpl(std::unique_ptr subscription_state_factory, - bool skip_subsequent_node, const LocalInfo::LocalInfo& local_info) - : subscription_state_factory_(std::move(subscription_state_factory)), - skip_subsequent_node_(skip_subsequent_node), local_info_(local_info) { +GrpcMuxImpl::GrpcMuxImpl(const LocalInfo::LocalInfo& local_info, + Grpc::RawAsyncClientPtr async_client, Event::Dispatcher& dispatcher, + const Protobuf::MethodDescriptor& service_method, + Runtime::RandomGenerator& random, Stats::Scope& scope, + const RateLimitSettings& rate_limit_settings, bool skip_subsequent_node) + : grpc_stream_(this, std::move(async_client), service_method, random, dispatcher, scope, + rate_limit_settings), + local_info_(local_info), skip_subsequent_node_(skip_subsequent_node), + first_stream_request_(true) { Config::Utility::checkLocalInfo("ads", local_info); } -Watch* GrpcMuxImpl::addOrUpdateWatch(const std::string& type_url, Watch* watch, - const std::set& resources, - SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) { - if (watch == nullptr) { - return addWatch(type_url, resources, callbacks, init_fetch_timeout); - } else { - updateWatch(type_url, watch, resources); - return watch; +GrpcMuxImpl::~GrpcMuxImpl() { + for (const auto& api_state : api_state_) { + for (auto watch : api_state.second.watches_) { + watch->clear(); + } } } -void GrpcMuxImpl::removeWatch(const std::string& type_url, Watch* watch) { - updateWatch(type_url, watch, {}); - watchMapFor(type_url).removeWatch(watch); -} - -void GrpcMuxImpl::pause(const std::string& type_url) { pausable_ack_queue_.pause(type_url); } - -void GrpcMuxImpl::resume(const std::string& type_url) { - pausable_ack_queue_.resume(type_url); - trySendDiscoveryRequests(); -} - -bool GrpcMuxImpl::paused(const std::string& type_url) const { - return pausable_ack_queue_.paused(type_url); -} +void GrpcMuxImpl::start() { grpc_stream_.establishNewStream(); } -void GrpcMuxImpl::genericHandleResponse(const std::string& type_url, - const void* response_proto_ptr) { - auto sub = subscriptions_.find(type_url); - if (sub == subscriptions_.end()) { - ENVOY_LOG(warn, - "The server sent an xDS response proto with type_url {}, which we have " - "not subscribed to. Ignoring.", - type_url); - return; +void GrpcMuxImpl::sendDiscoveryRequest(const std::string& type_url) { + if (!grpc_stream_.grpcStreamAvailable()) { + ENVOY_LOG(debug, "No stream available to sendDiscoveryRequest for {}", type_url); + return; // Drop this request; the reconnect will enqueue a new one. } - pausable_ack_queue_.push(sub->second->handleResponse(response_proto_ptr)); - trySendDiscoveryRequests(); -} - -void GrpcMuxImpl::start() { establishGrpcStream(); } -void GrpcMuxImpl::handleEstablishedStream() { - for (auto& sub : subscriptions_) { - sub.second->markStreamFresh(); + ApiState& api_state = api_state_[type_url]; + if (api_state.paused_) { + ENVOY_LOG(trace, "API {} paused during sendDiscoveryRequest(), setting pending.", type_url); + api_state.pending_ = true; + return; // Drop this request; the unpause will enqueue a new one. } - set_any_request_sent_yet_in_current_stream(false); - trySendDiscoveryRequests(); -} -void GrpcMuxImpl::disableInitFetchTimeoutTimer() { - for (auto& sub : subscriptions_) { - sub.second->disableInitFetchTimeoutTimer(); - } -} + auto& request = api_state.request_; + request.mutable_resource_names()->Clear(); -void GrpcMuxImpl::handleStreamEstablishmentFailure() { - // If this happens while Envoy is still initializing, the onConfigUpdateFailed() we ultimately - // call on CDS will cause LDS to start up, which adds to subscriptions_ here. So, to avoid a - // crash, the iteration needs to dance around a little: collect pointers to all - // SubscriptionStates, call on all those pointers we haven't yet called on, repeat if there are - // now more SubscriptionStates. - absl::flat_hash_map all_subscribed; - absl::flat_hash_map already_called; - do { - for (auto& sub : subscriptions_) { - all_subscribed[sub.first] = sub.second.get(); - } - for (auto& sub : all_subscribed) { - if (already_called.insert(sub).second) { // insert succeeded ==> not already called - sub.second->handleEstablishmentFailure(); + // Maintain a set to avoid dupes. + std::unordered_set resources; + for (const auto* watch : api_state.watches_) { + for (const std::string& resource : watch->resources_) { + if (resources.count(resource) == 0) { + resources.emplace(resource); + request.add_resource_names(resource); } } - } while (all_subscribed.size() != subscriptions_.size()); -} + } -Watch* GrpcMuxImpl::addWatch(const std::string& type_url, const std::set& resources, - SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) { - auto watch_map = watch_maps_.find(type_url); - if (watch_map == watch_maps_.end()) { - // We don't yet have a subscription for type_url! Make one! - watch_map = watch_maps_.emplace(type_url, std::make_unique()).first; - subscriptions_.emplace(type_url, subscription_state_factory_->makeSubscriptionState( - type_url, *watch_maps_[type_url], init_fetch_timeout)); - subscription_ordering_.emplace_back(type_url); + if (skip_subsequent_node_ && !first_stream_request_) { + request.clear_node(); } + ENVOY_LOG(trace, "Sending DiscoveryRequest for {}: {}", type_url, request.DebugString()); + grpc_stream_.sendMessage(request); + first_stream_request_ = false; - Watch* watch = watch_map->second->addWatch(callbacks); - // updateWatch() queues a discovery request if any of 'resources' are not yet subscribed. - updateWatch(type_url, watch, resources); - return watch; + // clear error_detail after the request is sent if it exists. + if (api_state_[type_url].request_.has_error_detail()) { + api_state_[type_url].request_.clear_error_detail(); + } } -// Updates the list of resource names watched by the given watch. If an added name is new across -// the whole subscription, or if a removed name has no other watch interested in it, then the -// subscription will enqueue and attempt to send an appropriate discovery request. -void GrpcMuxImpl::updateWatch(const std::string& type_url, Watch* watch, - const std::set& resources) { - ASSERT(watch != nullptr); - SubscriptionState& sub = subscriptionStateFor(type_url); - WatchMap& watch_map = watchMapFor(type_url); - - auto added_removed = watch_map.updateWatchInterest(watch, resources); - sub.updateSubscriptionInterest(added_removed.added_, added_removed.removed_); - - // Tell the server about our change in interest, if any. - if (sub.subscriptionUpdatePending()) { - trySendDiscoveryRequests(); +GrpcMuxWatchPtr GrpcMuxImpl::subscribe(const std::string& type_url, + const std::set& resources, + GrpcMuxCallbacks& callbacks) { + auto watch = + std::unique_ptr(new GrpcMuxWatchImpl(resources, callbacks, type_url, *this)); + ENVOY_LOG(debug, "gRPC mux subscribe for " + type_url); + + // Lazily kick off the requests based on first subscription. This has the + // convenient side-effect that we order messages on the channel based on + // Envoy's internal dependency ordering. + // TODO(gsagula): move TokenBucketImpl params to a config. + if (!api_state_[type_url].subscribed_) { + api_state_[type_url].request_.set_type_url(type_url); + api_state_[type_url].request_.mutable_node()->MergeFrom(local_info_.node()); + api_state_[type_url].subscribed_ = true; + subscriptions_.emplace_back(type_url); } -} -SubscriptionState& GrpcMuxImpl::subscriptionStateFor(const std::string& type_url) { - auto sub = subscriptions_.find(type_url); - RELEASE_ASSERT(sub != subscriptions_.end(), - fmt::format("Tried to look up SubscriptionState for non-existent subscription {}.", - type_url)); - return *sub->second; + // This will send an updated request on each subscription. + // TODO(htuch): For RDS/EDS, this will generate a new DiscoveryRequest on each resource we added. + // Consider in the future adding some kind of collation/batching during CDS/LDS updates so that we + // only send a single RDS/EDS update after the CDS/LDS update. + queueDiscoveryRequest(type_url); + + return watch; } -WatchMap& GrpcMuxImpl::watchMapFor(const std::string& type_url) { - auto watch_map = watch_maps_.find(type_url); - RELEASE_ASSERT( - watch_map != watch_maps_.end(), - fmt::format("Tried to look up WatchMap for non-existent subscription {}.", type_url)); - return *watch_map->second; +void GrpcMuxImpl::pause(const std::string& type_url) { + ENVOY_LOG(debug, "Pausing discovery requests for {}", type_url); + ApiState& api_state = api_state_[type_url]; + ASSERT(!api_state.paused_); + ASSERT(!api_state.pending_); + api_state.paused_ = true; } -void GrpcMuxImpl::trySendDiscoveryRequests() { - while (true) { - // Do any of our subscriptions even want to send a request? - absl::optional request_type_if_any = whoWantsToSendDiscoveryRequest(); - if (!request_type_if_any.has_value()) { - break; - } - // If so, which one (by type_url)? - std::string next_request_type_url = request_type_if_any.value(); - SubscriptionState& sub = subscriptionStateFor(next_request_type_url); - // Try again later if paused/rate limited/stream down. - if (!canSendDiscoveryRequest(next_request_type_url)) { - break; - } - // Get our subscription state to generate the appropriate discovery request, and send. - if (!pausable_ack_queue_.empty()) { - // Because ACKs take precedence over plain requests, if there is anything in the queue, it's - // safe to assume it's of the type_url that we're wanting to send. - // - // getNextRequestWithAck() returns a raw unowned pointer, which sendGrpcMessage deletes. - sendGrpcMessage(sub.getNextRequestWithAck(pausable_ack_queue_.front())); - pausable_ack_queue_.pop(); - } else { - // getNextRequestAckless() returns a raw unowned pointer, which sendGrpcMessage deletes. - sendGrpcMessage(sub.getNextRequestAckless()); - } +void GrpcMuxImpl::resume(const std::string& type_url) { + ENVOY_LOG(debug, "Resuming discovery requests for {}", type_url); + ApiState& api_state = api_state_[type_url]; + ASSERT(api_state.paused_); + api_state.paused_ = false; + + if (api_state.pending_) { + ASSERT(api_state.subscribed_); + queueDiscoveryRequest(type_url); + api_state.pending_ = false; } - maybeUpdateQueueSizeStat(pausable_ack_queue_.size()); } -// Checks whether external conditions allow sending a discovery request. (Does not check -// whether we *want* to send a discovery request). -bool GrpcMuxImpl::canSendDiscoveryRequest(const std::string& type_url) { - RELEASE_ASSERT( - !pausable_ack_queue_.paused(type_url), - fmt::format("canSendDiscoveryRequest() called on paused type_url {}. Pausedness is " - "supposed to be filtered out by whoWantsToSendDiscoveryRequest(). ", - type_url)); - - if (!grpcStreamAvailable()) { - ENVOY_LOG(trace, "No stream available to send a discovery request for {}.", type_url); - return false; - } else if (!rateLimitAllowsDrain()) { - ENVOY_LOG(trace, "{} discovery request hit rate limit; will try later.", type_url); +bool GrpcMuxImpl::paused(const std::string& type_url) const { + auto entry = api_state_.find(type_url); + if (entry == api_state_.end()) { return false; } - return true; + return entry->second.paused_; } -// Checks whether we have something to say in a discovery request, which can be an ACK and/or -// a subscription update. (Does not check whether we *can* send that discovery request). -// Returns the type_url we should send the discovery request for (if any). -// First, prioritizes ACKs over non-ACK subscription interest updates. -// Then, prioritizes non-ACK updates in the order the various types -// of subscriptions were activated. -absl::optional GrpcMuxImpl::whoWantsToSendDiscoveryRequest() { - // All ACKs are sent before plain updates. trySendDiscoveryRequests() relies on this. So, choose - // type_url from pausable_ack_queue_ if possible, before looking at pending updates. - if (!pausable_ack_queue_.empty()) { - return pausable_ack_queue_.front().type_url_; +void GrpcMuxImpl::onDiscoveryResponse( + std::unique_ptr&& message) { + const std::string& type_url = message->type_url(); + ENVOY_LOG(debug, "Received gRPC message for {} at version {}", type_url, message->version_info()); + if (api_state_.count(type_url) == 0) { + ENVOY_LOG(warn, "Ignoring the message for type URL {} as it has no current subscribers.", + type_url); + // TODO(yuval-k): This should never happen. consider dropping the stream as this is a + // protocol violation + return; } - // If we're looking to send multiple non-ACK requests, send them in the order that their - // subscriptions were initiated. - for (const auto& sub_type : subscription_ordering_) { - SubscriptionState& sub = subscriptionStateFor(sub_type); - if (sub.subscriptionUpdatePending() && !pausable_ack_queue_.paused(sub_type)) { - return sub_type; + if (api_state_[type_url].watches_.empty()) { + // update the nonce as we are processing this response. + api_state_[type_url].request_.set_response_nonce(message->nonce()); + if (message->resources().empty()) { + // No watches and no resources. This can happen when envoy unregisters from a + // resource that's removed from the server as well. For example, a deleted cluster + // triggers un-watching the ClusterLoadAssignment watch, and at the same time the + // xDS server sends an empty list of ClusterLoadAssignment resources. we'll accept + // this update. no need to send a discovery request, as we don't watch for anything. + api_state_[type_url].request_.set_version_info(message->version_info()); + } else { + // No watches and we have resources - this should not happen. send a NACK (by not + // updating the version). + ENVOY_LOG(warn, "Ignoring unwatched type URL {}", type_url); + queueDiscoveryRequest(type_url); + } + return; + } + try { + // To avoid O(n^2) explosion (e.g. when we have 1000s of EDS watches), we + // build a map here from resource name to resource and then walk watches_. + // We have to walk all watches (and need an efficient map as a result) to + // ensure we deliver empty config updates when a resource is dropped. + std::unordered_map resources; + GrpcMuxCallbacks& callbacks = api_state_[type_url].watches_.front()->callbacks_; + for (const auto& resource : message->resources()) { + if (type_url != resource.type_url()) { + throw EnvoyException( + fmt::format("{} does not match the message-wide type URL {} in DiscoveryResponse {}", + resource.type_url(), type_url, message->DebugString())); + } + const std::string resource_name = callbacks.resourceName(resource); + resources.emplace(resource_name, resource); + } + for (auto watch : api_state_[type_url].watches_) { + // onConfigUpdate should be called in all cases for single watch xDS (Cluster and + // Listener) even if the message does not have resources so that update_empty stat + // is properly incremented and state-of-the-world semantics are maintained. + if (watch->resources_.empty()) { + watch->callbacks_.onConfigUpdate(message->resources(), message->version_info()); + continue; + } + Protobuf::RepeatedPtrField found_resources; + for (const auto& watched_resource_name : watch->resources_) { + auto it = resources.find(watched_resource_name); + if (it != resources.end()) { + found_resources.Add()->MergeFrom(it->second); + } + } + // onConfigUpdate should be called only on watches(clusters/routes) that have + // updates in the message for EDS/RDS. + if (!found_resources.empty()) { + watch->callbacks_.onConfigUpdate(found_resources, message->version_info()); + } + } + // TODO(mattklein123): In the future if we start tracking per-resource versions, we + // would do that tracking here. + api_state_[type_url].request_.set_version_info(message->version_info()); + } catch (const EnvoyException& e) { + for (auto watch : api_state_[type_url].watches_) { + watch->callbacks_.onConfigUpdateFailed( + Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, &e); } + ::google::rpc::Status* error_detail = api_state_[type_url].request_.mutable_error_detail(); + error_detail->set_code(Grpc::Status::WellKnownGrpcStatus::Internal); + error_detail->set_message(e.what()); } - return absl::nullopt; + api_state_[type_url].request_.set_response_nonce(message->nonce()); + queueDiscoveryRequest(type_url); } -// Delta- and SotW-specific concrete subclasses: -GrpcMuxDelta::GrpcMuxDelta(Grpc::RawAsyncClientPtr&& async_client, Event::Dispatcher& dispatcher, - const Protobuf::MethodDescriptor& service_method, - Runtime::RandomGenerator& random, Stats::Scope& scope, - const RateLimitSettings& rate_limit_settings, - const LocalInfo::LocalInfo& local_info, bool skip_subsequent_node) - : GrpcMuxImpl(std::make_unique(dispatcher), skip_subsequent_node, - local_info), - grpc_stream_(this, std::move(async_client), service_method, random, dispatcher, scope, - rate_limit_settings) {} +void GrpcMuxImpl::onWriteable() { drainRequests(); } -// GrpcStreamCallbacks for GrpcMuxDelta -void GrpcMuxDelta::onStreamEstablished() { handleEstablishedStream(); } -void GrpcMuxDelta::onEstablishmentFailure() { handleStreamEstablishmentFailure(); } -void GrpcMuxDelta::onWriteable() { trySendDiscoveryRequests(); } -void GrpcMuxDelta::onDiscoveryResponse( - std::unique_ptr&& message) { - genericHandleResponse(message->type_url(), message.get()); +void GrpcMuxImpl::onStreamEstablished() { + first_stream_request_ = true; + for (const auto& type_url : subscriptions_) { + queueDiscoveryRequest(type_url); + } } -void GrpcMuxDelta::establishGrpcStream() { grpc_stream_.establishNewStream(); } -void GrpcMuxDelta::sendGrpcMessage(void* msg_proto_ptr) { - std::unique_ptr typed_proto( - static_cast(msg_proto_ptr)); - if (!any_request_sent_yet_in_current_stream() || !skip_subsequent_node()) { - typed_proto->mutable_node()->MergeFrom(local_info().node()); +void GrpcMuxImpl::onEstablishmentFailure() { + for (const auto& api_state : api_state_) { + for (auto watch : api_state.second.watches_) { + watch->callbacks_.onConfigUpdateFailed( + Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, nullptr); + } } - grpc_stream_.sendMessage(*typed_proto); - set_any_request_sent_yet_in_current_stream(true); } -void GrpcMuxDelta::maybeUpdateQueueSizeStat(uint64_t size) { - grpc_stream_.maybeUpdateQueueSizeStat(size); -} -bool GrpcMuxDelta::grpcStreamAvailable() const { return grpc_stream_.grpcStreamAvailable(); } -bool GrpcMuxDelta::rateLimitAllowsDrain() { return grpc_stream_.checkRateLimitAllowsDrain(); } -GrpcMuxSotw::GrpcMuxSotw(Grpc::RawAsyncClientPtr&& async_client, Event::Dispatcher& dispatcher, - const Protobuf::MethodDescriptor& service_method, - Runtime::RandomGenerator& random, Stats::Scope& scope, - const RateLimitSettings& rate_limit_settings, - const LocalInfo::LocalInfo& local_info, bool skip_subsequent_node) - : GrpcMuxImpl(std::make_unique(dispatcher), skip_subsequent_node, - local_info), - grpc_stream_(this, std::move(async_client), service_method, random, dispatcher, scope, - rate_limit_settings) {} - -// GrpcStreamCallbacks for GrpcMuxSotw -void GrpcMuxSotw::onStreamEstablished() { handleEstablishedStream(); } -void GrpcMuxSotw::onEstablishmentFailure() { handleStreamEstablishmentFailure(); } -void GrpcMuxSotw::onWriteable() { trySendDiscoveryRequests(); } -void GrpcMuxSotw::onDiscoveryResponse( - std::unique_ptr&& message) { - genericHandleResponse(message->type_url(), message.get()); +void GrpcMuxImpl::queueDiscoveryRequest(const std::string& queue_item) { + request_queue_.push(queue_item); + drainRequests(); } -void GrpcMuxSotw::establishGrpcStream() { grpc_stream_.establishNewStream(); } - -void GrpcMuxSotw::sendGrpcMessage(void* msg_proto_ptr) { - std::unique_ptr typed_proto( - static_cast(msg_proto_ptr)); - if (!any_request_sent_yet_in_current_stream() || !skip_subsequent_node()) { - typed_proto->mutable_node()->MergeFrom(local_info().node()); - } - grpc_stream_.sendMessage(*typed_proto); - set_any_request_sent_yet_in_current_stream(true); +void GrpcMuxImpl::clearRequestQueue() { + grpc_stream_.maybeUpdateQueueSizeStat(0); + request_queue_ = {}; } -void GrpcMuxSotw::maybeUpdateQueueSizeStat(uint64_t size) { - grpc_stream_.maybeUpdateQueueSizeStat(size); +void GrpcMuxImpl::drainRequests() { + while (!request_queue_.empty() && grpc_stream_.checkRateLimitAllowsDrain()) { + // Process the request, if rate limiting is not enabled at all or if it is under rate limit. + sendDiscoveryRequest(request_queue_.front()); + request_queue_.pop(); + } + grpc_stream_.maybeUpdateQueueSizeStat(request_queue_.size()); } -bool GrpcMuxSotw::grpcStreamAvailable() const { return grpc_stream_.grpcStreamAvailable(); } -bool GrpcMuxSotw::rateLimitAllowsDrain() { return grpc_stream_.checkRateLimitAllowsDrain(); } - } // namespace Config } // namespace Envoy diff --git a/source/common/config/grpc_mux_impl.h b/source/common/config/grpc_mux_impl.h index ff0d5db75e89e..3f55178f4e1e2 100644 --- a/source/common/config/grpc_mux_impl.h +++ b/source/common/config/grpc_mux_impl.h @@ -1,185 +1,142 @@ #pragma once -#include "envoy/api/v2/discovery.pb.h" -#include "envoy/common/token_bucket.h" +#include +#include + +#include "envoy/common/time.h" #include "envoy/config/grpc_mux.h" #include "envoy/config/subscription.h" +#include "envoy/event/dispatcher.h" +#include "envoy/grpc/status.h" +#include "envoy/upstream/cluster_manager.h" +#include "common/common/cleanup.h" #include "common/common/logger.h" -#include "common/config/delta_subscription_state.h" #include "common/config/grpc_stream.h" -#include "common/config/pausable_ack_queue.h" -#include "common/config/sotw_subscription_state.h" -#include "common/config/watch_map.h" -#include "common/grpc/common.h" +#include "common/config/utility.h" namespace Envoy { namespace Config { -// Manages subscriptions to one or more type of resource. The logical protocol -// state of those subscription(s) is handled by SubscriptionState. -// This class owns the GrpcStream used to talk to the server, maintains queuing -// logic to properly order the subscription(s)' various messages, and allows -// starting/stopping/pausing of the subscriptions. -class GrpcMuxImpl : public GrpcMux, Logger::Loggable { +/** + * ADS API implementation that fetches via gRPC. + */ +class GrpcMuxImpl : public GrpcMux, + public GrpcStreamCallbacks, + public Logger::Loggable { public: - GrpcMuxImpl(std::unique_ptr subscription_state_factory, - bool skip_subsequent_node, const LocalInfo::LocalInfo& local_info); + GrpcMuxImpl(const LocalInfo::LocalInfo& local_info, Grpc::RawAsyncClientPtr async_client, + Event::Dispatcher& dispatcher, const Protobuf::MethodDescriptor& service_method, + Runtime::RandomGenerator& random, Stats::Scope& scope, + const RateLimitSettings& rate_limit_settings, bool skip_subsequent_node); + ~GrpcMuxImpl() override; - Watch* addOrUpdateWatch(const std::string& type_url, Watch* watch, - const std::set& resources, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) override; - void removeWatch(const std::string& type_url, Watch* watch) override; + void start() override; + GrpcMuxWatchPtr subscribe(const std::string& type_url, const std::set& resources, + GrpcMuxCallbacks& callbacks) override; + // GrpcMux + // TODO(fredlas) PR #8478 will remove this. + bool isDelta() const override { return false; } void pause(const std::string& type_url) override; void resume(const std::string& type_url) override; bool paused(const std::string& type_url) const override; - void start() override; - void disableInitFetchTimeoutTimer() override; - -protected: - // Everything related to GrpcStream must remain abstract. GrpcStream (and the gRPC-using classes - // that underlie it) are templated on protobufs. That means that a single implementation that - // supports different types of protobufs cannot use polymorphism to share code. The workaround: - // the GrpcStream will be owned by a derived class, and all code that would touch grpc_stream_ is - // seen here in the base class as calls to abstract functions, to be provided by those derived - // classes. - virtual void establishGrpcStream() PURE; - // Deletes msg_proto_ptr. - virtual void sendGrpcMessage(void* msg_proto_ptr) PURE; - virtual void maybeUpdateQueueSizeStat(uint64_t size) PURE; - virtual bool grpcStreamAvailable() const PURE; - virtual bool rateLimitAllowsDrain() PURE; - - SubscriptionState& subscriptionStateFor(const std::string& type_url); - WatchMap& watchMapFor(const std::string& type_url); - void handleEstablishedStream(); - void handleStreamEstablishmentFailure(); - void genericHandleResponse(const std::string& type_url, const void* response_proto_ptr); - void trySendDiscoveryRequests(); - bool skip_subsequent_node() const { return skip_subsequent_node_; } - bool any_request_sent_yet_in_current_stream() const { - return any_request_sent_yet_in_current_stream_; - } - void set_any_request_sent_yet_in_current_stream(bool value) { - any_request_sent_yet_in_current_stream_ = value; - } - const LocalInfo::LocalInfo& local_info() const { return local_info_; } -private: - Watch* addWatch(const std::string& type_url, const std::set& resources, - SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout); - - // Updates the list of resource names watched by the given watch. If an added name is new across - // the whole subscription, or if a removed name has no other watch interested in it, then the - // subscription will enqueue and attempt to send an appropriate discovery request. - void updateWatch(const std::string& type_url, Watch* watch, - const std::set& resources); - - // Checks whether external conditions allow sending a DeltaDiscoveryRequest. (Does not check - // whether we *want* to send a DeltaDiscoveryRequest). - bool canSendDiscoveryRequest(const std::string& type_url); - - // Checks whether we have something to say in a DeltaDiscoveryRequest, which can be an ACK and/or - // a subscription update. (Does not check whether we *can* send that DeltaDiscoveryRequest). - // Returns the type_url we should send the DeltaDiscoveryRequest for (if any). - // First, prioritizes ACKs over non-ACK subscription interest updates. - // Then, prioritizes non-ACK updates in the order the various types - // of subscriptions were activated (as tracked by subscription_ordering_). - absl::optional whoWantsToSendDiscoveryRequest(); - - // Resource (N)ACKs we're waiting to send, stored in the order that they should be sent in. All - // of our different resource types' ACKs are mixed together in this queue. See class for - // description of how it interacts with pause() and resume(). - PausableAckQueue pausable_ack_queue_; - - // Makes SubscriptionStates, to be held in the subscriptions_ map. Whether this GrpcMux is doing - // delta or state of the world xDS is determined by which concrete subclass this variable gets. - std::unique_ptr subscription_state_factory_; - - // Map key is type_url. - // Only addWatch() should insert into these maps. - absl::flat_hash_map> subscriptions_; - absl::flat_hash_map> watch_maps_; - - // Determines the order of initial discovery requests. (Assumes that subscriptions are added - // to this GrpcMux in the order of Envoy's dependency ordering). - std::list subscription_ordering_; - - // Whether to enable the optimization of only including the node field in the very first - // discovery request in an xDS gRPC stream (really just one: *not* per-type_url). - const bool skip_subsequent_node_; - - // State to help with skip_subsequent_node's logic. - bool any_request_sent_yet_in_current_stream_{}; + Watch* addOrUpdateWatch(const std::string&, Watch*, const std::set&, + SubscriptionCallbacks&, std::chrono::milliseconds) override { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; + } + void removeWatch(const std::string&, Watch*) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - // Used to populate the [Delta]DiscoveryRequest's node field. That field is the same across - // all type_urls, and moreover, the 'skip_subsequent_node' logic needs to operate across all - // the type_urls. So, while the SubscriptionStates populate every other field of these messages, - // this one is up to GrpcMux. - const LocalInfo::LocalInfo& local_info_; -}; + void handleDiscoveryResponse(std::unique_ptr&& message); -class GrpcMuxDelta : public GrpcMuxImpl, - public GrpcStreamCallbacks { -public: - GrpcMuxDelta(Grpc::RawAsyncClientPtr&& async_client, Event::Dispatcher& dispatcher, - const Protobuf::MethodDescriptor& service_method, Runtime::RandomGenerator& random, - Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, - const LocalInfo::LocalInfo& local_info, bool skip_subsequent_node); + void sendDiscoveryRequest(const std::string& type_url); - // GrpcStreamCallbacks + // Config::GrpcStreamCallbacks void onStreamEstablished() override; void onEstablishmentFailure() override; + void onDiscoveryResponse(std::unique_ptr&& message) override; void onWriteable() override; - void - onDiscoveryResponse(std::unique_ptr&& message) override; -protected: - void establishGrpcStream() override; - void sendGrpcMessage(void* msg_proto_ptr) override; - void maybeUpdateQueueSizeStat(uint64_t size) override; - bool grpcStreamAvailable() const override; - bool rateLimitAllowsDrain() override; - -private: - GrpcStream - grpc_stream_; -}; - -class GrpcMuxSotw : public GrpcMuxImpl, - public GrpcStreamCallbacks { -public: - GrpcMuxSotw(Grpc::RawAsyncClientPtr&& async_client, Event::Dispatcher& dispatcher, - const Protobuf::MethodDescriptor& service_method, Runtime::RandomGenerator& random, - Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, - const LocalInfo::LocalInfo& local_info, bool skip_subsequent_node); - - // GrpcStreamCallbacks - void onStreamEstablished() override; - void onEstablishmentFailure() override; - void onWriteable() override; - void onDiscoveryResponse(std::unique_ptr&& message) override; GrpcStream& grpcStreamForTest() { return grpc_stream_; } -protected: - void establishGrpcStream() override; - void sendGrpcMessage(void* msg_proto_ptr) override; - void maybeUpdateQueueSizeStat(uint64_t size) override; - bool grpcStreamAvailable() const override; - bool rateLimitAllowsDrain() override; - private: + void drainRequests(); + void setRetryTimer(); + + struct GrpcMuxWatchImpl : public GrpcMuxWatch, RaiiListElement { + GrpcMuxWatchImpl(const std::set& resources, GrpcMuxCallbacks& callbacks, + const std::string& type_url, GrpcMuxImpl& parent) + : RaiiListElement(parent.api_state_[type_url].watches_, this), + resources_(resources), callbacks_(callbacks), type_url_(type_url), parent_(parent), + inserted_(true) {} + ~GrpcMuxWatchImpl() override { + if (inserted_) { + erase(); + if (!resources_.empty()) { + parent_.sendDiscoveryRequest(type_url_); + } + } + } + + void clear() { + inserted_ = false; + cancel(); + } + + std::set resources_; + GrpcMuxCallbacks& callbacks_; + const std::string type_url_; + GrpcMuxImpl& parent_; + + private: + bool inserted_; + }; + + // Per muxed API state. + struct ApiState { + // Watches on the returned resources for the API; + std::list watches_; + // Current DiscoveryRequest for API. + envoy::api::v2::DiscoveryRequest request_; + // Paused via pause()? + bool paused_{}; + // Was a DiscoveryRequest elided during a pause? + bool pending_{}; + // Has this API been tracked in subscriptions_? + bool subscribed_{}; + }; + + // Request queue management logic. + void queueDiscoveryRequest(const std::string& queue_item); + void clearRequestQueue(); + GrpcStream grpc_stream_; + const LocalInfo::LocalInfo& local_info_; + const bool skip_subsequent_node_; + bool first_stream_request_; + std::unordered_map api_state_; + // Envoy's dependency ordering. + std::list subscriptions_; + + // A queue to store requests while rate limited. Note that when requests cannot be sent due to the + // gRPC stream being down, this queue does not store them; rather, they are simply dropped. + // This string is a type URL. + std::queue request_queue_; }; -class NullGrpcMuxImpl : public GrpcMux { +class NullGrpcMuxImpl : public GrpcMux, GrpcStreamCallbacks { public: void start() override {} - + GrpcMuxWatchPtr subscribe(const std::string&, const std::set&, + GrpcMuxCallbacks&) override { + throw EnvoyException("ADS must be configured to support an ADS config source"); + } + // TODO(fredlas) PR #8478 will remove this. + bool isDelta() const override { return false; } void pause(const std::string&) override {} void resume(const std::string&) override {} bool paused(const std::string&) const override { return false; } @@ -191,7 +148,11 @@ class NullGrpcMuxImpl : public GrpcMux { void removeWatch(const std::string&, Watch*) override { throw EnvoyException("ADS must be configured to support an ADS config source"); } - void disableInitFetchTimeoutTimer() override {} + + void onWriteable() override {} + void onStreamEstablished() override {} + void onEstablishmentFailure() override {} + void onDiscoveryResponse(std::unique_ptr&&) override {} }; } // namespace Config diff --git a/source/common/config/grpc_mux_subscription_impl.cc b/source/common/config/grpc_mux_subscription_impl.cc new file mode 100644 index 0000000000000..06d1fd3b562fa --- /dev/null +++ b/source/common/config/grpc_mux_subscription_impl.cc @@ -0,0 +1,108 @@ +#include "common/config/grpc_mux_subscription_impl.h" + +#include "common/common/assert.h" +#include "common/common/logger.h" +#include "common/grpc/common.h" +#include "common/protobuf/protobuf.h" +#include "common/protobuf/utility.h" + +namespace Envoy { +namespace Config { + +GrpcMuxSubscriptionImpl::GrpcMuxSubscriptionImpl(GrpcMuxSharedPtr grpc_mux, + SubscriptionCallbacks& callbacks, + SubscriptionStats stats, + absl::string_view type_url, + Event::Dispatcher& dispatcher, + std::chrono::milliseconds init_fetch_timeout) + : grpc_mux_(grpc_mux), callbacks_(callbacks), stats_(stats), type_url_(type_url), + dispatcher_(dispatcher), init_fetch_timeout_(init_fetch_timeout) {} + +// Config::Subscription +void GrpcMuxSubscriptionImpl::start(const std::set& resources) { + if (init_fetch_timeout_.count() > 0) { + init_fetch_timeout_timer_ = dispatcher_.createTimer([this]() -> void { + callbacks_.onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::FetchTimedout, + nullptr); + }); + init_fetch_timeout_timer_->enableTimer(init_fetch_timeout_); + } + + watch_ = grpc_mux_->subscribe(type_url_, resources, *this); + // The attempt stat here is maintained for the purposes of having consistency between ADS and + // gRPC/filesystem/REST Subscriptions. Since ADS is push based and muxed, the notion of an + // "attempt" for a given xDS API combined by ADS is not really that meaningful. + stats_.update_attempt_.inc(); +} + +void GrpcMuxSubscriptionImpl::updateResourceInterest( + const std::set& update_to_these_names) { + // First destroy the watch, so that this subscribe doesn't send a request for both the + // previously watched resources and the new ones (we may have lost interest in some of the + // previously watched ones). + watch_.reset(); + watch_ = grpc_mux_->subscribe(type_url_, update_to_these_names, *this); + stats_.update_attempt_.inc(); +} + +// Config::GrpcMuxCallbacks +void GrpcMuxSubscriptionImpl::onConfigUpdate( + const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) { + disableInitFetchTimeoutTimer(); + // TODO(mattklein123): In the future if we start tracking per-resource versions, we need to + // supply those versions to onConfigUpdate() along with the xDS response ("system") + // version_info. This way, both types of versions can be tracked and exposed for debugging by + // the configuration update targets. + callbacks_.onConfigUpdate(resources, version_info); + stats_.update_success_.inc(); + stats_.update_attempt_.inc(); + stats_.version_.set(HashUtil::xxHash64(version_info)); + ENVOY_LOG(debug, "gRPC config for {} accepted with {} resources with version {}", type_url_, + resources.size(), version_info); +} + +void GrpcMuxSubscriptionImpl::onConfigUpdateFailed(ConfigUpdateFailureReason reason, + const EnvoyException* e) { + switch (reason) { + case Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure: + stats_.update_failure_.inc(); + ENVOY_LOG(debug, "gRPC update for {} failed", type_url_); + break; + case Envoy::Config::ConfigUpdateFailureReason::FetchTimedout: + stats_.init_fetch_timeout_.inc(); + disableInitFetchTimeoutTimer(); + ENVOY_LOG(warn, "gRPC config: initial fetch timed out for {}", type_url_); + break; + case Envoy::Config::ConfigUpdateFailureReason::UpdateRejected: + // We expect Envoy exception to be thrown when update is rejected. + ASSERT(e != nullptr); + disableInitFetchTimeoutTimer(); + stats_.update_rejected_.inc(); + ENVOY_LOG(warn, "gRPC config for {} rejected: {}", type_url_, e->what()); + break; + } + + stats_.update_attempt_.inc(); + if (reason == Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure) { + // New gRPC stream will be established and send requests again. + // If init_fetch_timeout is non-zero, server will continue startup after it timeout + return; + } + + callbacks_.onConfigUpdateFailed(reason, e); +} + +std::string GrpcMuxSubscriptionImpl::resourceName(const ProtobufWkt::Any& resource) { + return callbacks_.resourceName(resource); +} + +void GrpcMuxSubscriptionImpl::disableInitFetchTimeoutTimer() { + if (init_fetch_timeout_timer_) { + init_fetch_timeout_timer_->disableTimer(); + init_fetch_timeout_timer_.reset(); + } +} + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/grpc_mux_subscription_impl.h b/source/common/config/grpc_mux_subscription_impl.h new file mode 100644 index 0000000000000..299aa4f1480ee --- /dev/null +++ b/source/common/config/grpc_mux_subscription_impl.h @@ -0,0 +1,50 @@ +#pragma once + +#include "envoy/api/v2/discovery.pb.h" +#include "envoy/config/grpc_mux.h" +#include "envoy/config/subscription.h" +#include "envoy/event/dispatcher.h" + +#include "common/common/logger.h" + +namespace Envoy { +namespace Config { + +/** + * Adapter from typed Subscription to untyped GrpcMux. Also handles per-xDS API stats/logging. + */ +class GrpcMuxSubscriptionImpl : public Subscription, + GrpcMuxCallbacks, + Logger::Loggable { +public: + GrpcMuxSubscriptionImpl(GrpcMuxSharedPtr grpc_mux, SubscriptionCallbacks& callbacks, + SubscriptionStats stats, absl::string_view type_url, + Event::Dispatcher& dispatcher, + std::chrono::milliseconds init_fetch_timeout); + + // Config::Subscription + void start(const std::set& resource_names) override; + void updateResourceInterest(const std::set& update_to_these_names) override; + + // Config::GrpcMuxCallbacks + void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) override; + void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, + const EnvoyException* e) override; + std::string resourceName(const ProtobufWkt::Any& resource) override; + +private: + void disableInitFetchTimeoutTimer(); + + GrpcMuxSharedPtr grpc_mux_; + SubscriptionCallbacks& callbacks_; + SubscriptionStats stats_; + const std::string type_url_; + GrpcMuxWatchPtr watch_{}; + Event::Dispatcher& dispatcher_; + std::chrono::milliseconds init_fetch_timeout_; + Event::TimerPtr init_fetch_timeout_timer_; +}; + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/grpc_stream.h b/source/common/config/grpc_stream.h index 328462d65e65f..60cf40099a55e 100644 --- a/source/common/config/grpc_stream.h +++ b/source/common/config/grpc_stream.h @@ -96,10 +96,15 @@ class GrpcStream : public Grpc::AsyncStreamCallbacks, } void maybeUpdateQueueSizeStat(uint64_t size) { - // A change in queue length is not "meaningful" until it has persisted until here. - if (size != last_queue_size_) { + // Although request_queue_.push() happens elsewhere, the only time the queue is non-transiently + // non-empty is when it remains non-empty after a drain attempt. (The push() doesn't matter + // because we always attempt this drain immediately after the push). Basically, a change in + // queue length is not "meaningful" until it has persisted until here. We need the + // if(>0 || used) to keep this stat from being wrongly marked interesting by a pointless set(0) + // and needlessly taking up space. The first time we set(123), used becomes true, and so we will + // subsequently always do the set (including set(0)). + if (size > 0 || control_plane_stats_.pending_requests_.used()) { control_plane_stats_.pending_requests_.set(size); - last_queue_size_ = size; } } @@ -142,7 +147,6 @@ class GrpcStream : public Grpc::AsyncStreamCallbacks, TokenBucketPtr limit_request_; const bool rate_limiting_enabled_; Event::TimerPtr drain_request_timer_; - uint64_t last_queue_size_{}; }; } // namespace Config diff --git a/source/common/config/grpc_subscription_impl.h b/source/common/config/grpc_subscription_impl.h index 46968aa3d71b1..8914519746f21 100644 --- a/source/common/config/grpc_subscription_impl.h +++ b/source/common/config/grpc_subscription_impl.h @@ -1,68 +1,49 @@ #pragma once +#include "envoy/api/v2/core/base.pb.h" #include "envoy/config/subscription.h" +#include "envoy/event/dispatcher.h" +#include "envoy/grpc/async_client.h" #include "common/config/grpc_mux_impl.h" +#include "common/config/grpc_mux_subscription_impl.h" #include "common/config/utility.h" namespace Envoy { namespace Config { -// GrpcSubscriptionImpl provides a top-level interface to the Envoy's gRPC communication with -// an xDS server, for use by the various xDS users within Envoy. It is built around a (shared) -// GrpcMuxImpl, and the further machinery underlying that. An xDS user indicates interest in -// various resources via start() and updateResourceInterest(). It receives updates to those -// resources via the SubscriptionCallbacks it provides. Multiple users can each have their own -// Subscription object for the same type_url; GrpcMuxImpl maintains a subscription to the -// union of interested resources, and delivers to the users just the resource updates that they -// are "watching" for. -// -// GrpcSubscriptionImpl and GrpcMuxImpl are both built to provide both regular xDS and ADS, -// distinguished by whether multiple GrpcSubscriptionImpls are sharing a single GrpcMuxImpl. -// (Also distinguished by the gRPC method string, but that's taken care of in SubscriptionFactory). -// -// Why does GrpcSubscriptionImpl itself implement the SubscriptionCallbacks interface? So that it -// can write to SubscriptionStats (which needs to live out here in the GrpcSubscriptionImpl) upon a -// config update. GrpcSubscriptionImpl presents itself to WatchMap as the SubscriptionCallbacks, -// and then, after incrementing stats, passes through to the real callbacks_. -class GrpcSubscriptionImpl : public Subscription, public SubscriptionCallbacks { +class GrpcSubscriptionImpl : public Config::Subscription { public: - // is_aggregated: whether our GrpcMux is also providing ADS to other Subscriptions, or whether - // it's all ours. The practical difference is that we ourselves must call start() on it only if - // we are the sole owner. - GrpcSubscriptionImpl(GrpcMuxSharedPtr grpc_mux, absl::string_view type_url, + GrpcSubscriptionImpl(const LocalInfo::LocalInfo& local_info, Grpc::RawAsyncClientPtr async_client, + Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, + const Protobuf::MethodDescriptor& service_method, absl::string_view type_url, SubscriptionCallbacks& callbacks, SubscriptionStats stats, - std::chrono::milliseconds init_fetch_timeout, bool is_aggregated); - ~GrpcSubscriptionImpl() override; - - void pause(); - void resume(); + Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, + std::chrono::milliseconds init_fetch_timeout, bool skip_subsequent_node) + : callbacks_(callbacks), + grpc_mux_(std::make_shared(local_info, std::move(async_client), + dispatcher, service_method, random, scope, + rate_limit_settings, skip_subsequent_node)), + grpc_mux_subscription_(grpc_mux_, callbacks_, stats, type_url, dispatcher, + init_fetch_timeout) {} // Config::Subscription - void start(const std::set& resource_names) override; - void updateResourceInterest(const std::set& update_to_these_names) override; + void start(const std::set& resource_names) override { + // Subscribe first, so we get failure callbacks if grpc_mux_.start() fails. + grpc_mux_subscription_.start(resource_names); + grpc_mux_->start(); + } - // Config::SubscriptionCallbacks (all pass through to callbacks_!) - void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, - const std::string& version_info) override; - void onConfigUpdate(const Protobuf::RepeatedPtrField& added_resources, - const Protobuf::RepeatedPtrField& removed_resources, - const std::string& system_version_info) override; - void onConfigUpdateFailed(ConfigUpdateFailureReason reason, const EnvoyException* e) override; - std::string resourceName(const ProtobufWkt::Any& resource) override; + void updateResourceInterest(const std::set& update_to_these_names) override { + grpc_mux_subscription_.updateResourceInterest(update_to_these_names); + } - GrpcMuxSharedPtr getGrpcMuxForTest() { return grpc_mux_; } + std::shared_ptr grpcMux() { return grpc_mux_; } private: - GrpcMuxSharedPtr const grpc_mux_; - const std::string type_url_; - SubscriptionCallbacks& callbacks_; - SubscriptionStats stats_; - // NOTE: if another subscription of the same type_url has already been started, the value of - // init_fetch_timeout_ will be ignored in favor of the other subscription's. - const std::chrono::milliseconds init_fetch_timeout_; - Watch* watch_{}; - const bool is_aggregated_; + Config::SubscriptionCallbacks& callbacks_; + std::shared_ptr grpc_mux_; + GrpcMuxSubscriptionImpl grpc_mux_subscription_; }; } // namespace Config diff --git a/source/common/config/new_grpc_mux_impl.cc b/source/common/config/new_grpc_mux_impl.cc new file mode 100644 index 0000000000000..d22593d0bf997 --- /dev/null +++ b/source/common/config/new_grpc_mux_impl.cc @@ -0,0 +1,228 @@ +#include "common/config/new_grpc_mux_impl.h" + +#include "common/common/assert.h" +#include "common/common/backoff_strategy.h" +#include "common/common/token_bucket_impl.h" +#include "common/config/utility.h" +#include "common/protobuf/protobuf.h" +#include "common/protobuf/utility.h" + +namespace Envoy { +namespace Config { + +NewGrpcMuxImpl::NewGrpcMuxImpl(Grpc::RawAsyncClientPtr&& async_client, + Event::Dispatcher& dispatcher, + const Protobuf::MethodDescriptor& service_method, + Runtime::RandomGenerator& random, Stats::Scope& scope, + const RateLimitSettings& rate_limit_settings, + const LocalInfo::LocalInfo& local_info) + : dispatcher_(dispatcher), local_info_(local_info), + grpc_stream_(this, std::move(async_client), service_method, random, dispatcher, scope, + rate_limit_settings) {} + +Watch* NewGrpcMuxImpl::addOrUpdateWatch(const std::string& type_url, Watch* watch, + const std::set& resources, + SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) { + if (watch == nullptr) { + return addWatch(type_url, resources, callbacks, init_fetch_timeout); + } else { + updateWatch(type_url, watch, resources); + return watch; + } +} + +void NewGrpcMuxImpl::removeWatch(const std::string& type_url, Watch* watch) { + updateWatch(type_url, watch, {}); + auto entry = subscriptions_.find(type_url); + RELEASE_ASSERT(entry != subscriptions_.end(), + fmt::format("removeWatch() called for non-existent subscription {}.", type_url)); + entry->second->watch_map_.removeWatch(watch); +} + +void NewGrpcMuxImpl::pause(const std::string& type_url) { pausable_ack_queue_.pause(type_url); } + +void NewGrpcMuxImpl::resume(const std::string& type_url) { + pausable_ack_queue_.resume(type_url); + trySendDiscoveryRequests(); +} + +bool NewGrpcMuxImpl::paused(const std::string& type_url) const { + return pausable_ack_queue_.paused(type_url); +} + +void NewGrpcMuxImpl::onDiscoveryResponse( + std::unique_ptr&& message) { + ENVOY_LOG(debug, "Received DeltaDiscoveryResponse for {} at version {}", message->type_url(), + message->system_version_info()); + auto sub = subscriptions_.find(message->type_url()); + if (sub == subscriptions_.end()) { + ENVOY_LOG(warn, + "Dropping received DeltaDiscoveryResponse (with version {}) for non-existent " + "subscription {}.", + message->system_version_info(), message->type_url()); + return; + } + kickOffAck(sub->second->sub_state_.handleResponse(*message)); +} + +void NewGrpcMuxImpl::onStreamEstablished() { + for (auto& sub : subscriptions_) { + sub.second->sub_state_.markStreamFresh(); + } + trySendDiscoveryRequests(); +} + +void NewGrpcMuxImpl::onEstablishmentFailure() { + // If this happens while Envoy is still initializing, the onConfigUpdateFailed() we ultimately + // call on CDS will cause LDS to start up, which adds to subscriptions_ here. So, to avoid a + // crash, the iteration needs to dance around a little: collect pointers to all + // SubscriptionStates, call on all those pointers we haven't yet called on, repeat if there are + // now more SubscriptionStates. + absl::flat_hash_map all_subscribed; + absl::flat_hash_map already_called; + do { + for (auto& sub : subscriptions_) { + all_subscribed[sub.first] = &sub.second->sub_state_; + } + for (auto& sub : all_subscribed) { + if (already_called.insert(sub).second) { // insert succeeded ==> not already called + sub.second->handleEstablishmentFailure(); + } + } + } while (all_subscribed.size() != subscriptions_.size()); +} + +void NewGrpcMuxImpl::onWriteable() { trySendDiscoveryRequests(); } + +void NewGrpcMuxImpl::kickOffAck(UpdateAck ack) { + pausable_ack_queue_.push(std::move(ack)); + trySendDiscoveryRequests(); +} + +// TODO(fredlas) to be removed from the GrpcMux interface very soon. +GrpcMuxWatchPtr NewGrpcMuxImpl::subscribe(const std::string&, const std::set&, + GrpcMuxCallbacks&) { + NOT_IMPLEMENTED_GCOVR_EXCL_LINE; +} + +void NewGrpcMuxImpl::start() { grpc_stream_.establishNewStream(); } + +Watch* NewGrpcMuxImpl::addWatch(const std::string& type_url, const std::set& resources, + SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) { + auto entry = subscriptions_.find(type_url); + if (entry == subscriptions_.end()) { + // We don't yet have a subscription for type_url! Make one! + addSubscription(type_url, init_fetch_timeout); + return addWatch(type_url, resources, callbacks, init_fetch_timeout); + } + + Watch* watch = entry->second->watch_map_.addWatch(callbacks); + // updateWatch() queues a discovery request if any of 'resources' are not yet subscribed. + updateWatch(type_url, watch, resources); + return watch; +} + +// Updates the list of resource names watched by the given watch. If an added name is new across +// the whole subscription, or if a removed name has no other watch interested in it, then the +// subscription will enqueue and attempt to send an appropriate discovery request. +void NewGrpcMuxImpl::updateWatch(const std::string& type_url, Watch* watch, + const std::set& resources) { + ASSERT(watch != nullptr); + auto sub = subscriptions_.find(type_url); + RELEASE_ASSERT(sub != subscriptions_.end(), + fmt::format("Watch of {} has no subscription to update.", type_url)); + auto added_removed = sub->second->watch_map_.updateWatchInterest(watch, resources); + sub->second->sub_state_.updateSubscriptionInterest(added_removed.added_, added_removed.removed_); + // Tell the server about our change in interest, if any. + if (sub->second->sub_state_.subscriptionUpdatePending()) { + trySendDiscoveryRequests(); + } +} + +void NewGrpcMuxImpl::addSubscription(const std::string& type_url, + std::chrono::milliseconds init_fetch_timeout) { + subscriptions_.emplace(type_url, std::make_unique(type_url, init_fetch_timeout, + dispatcher_, local_info_)); + subscription_ordering_.emplace_back(type_url); +} + +void NewGrpcMuxImpl::trySendDiscoveryRequests() { + while (true) { + // Do any of our subscriptions even want to send a request? + absl::optional maybe_request_type = whoWantsToSendDiscoveryRequest(); + if (!maybe_request_type.has_value()) { + break; + } + // If so, which one (by type_url)? + std::string next_request_type_url = maybe_request_type.value(); + // If we don't have a subscription object for this request's type_url, drop the request. + auto sub = subscriptions_.find(next_request_type_url); + RELEASE_ASSERT(sub != subscriptions_.end(), + fmt::format("Tried to send discovery request for non-existent subscription {}.", + next_request_type_url)); + + // Try again later if paused/rate limited/stream down. + if (!canSendDiscoveryRequest(next_request_type_url)) { + break; + } + // Get our subscription state to generate the appropriate DeltaDiscoveryRequest, and send. + if (!pausable_ack_queue_.empty()) { + // Because ACKs take precedence over plain requests, if there is anything in the queue, it's + // safe to assume it's of the type_url that we're wanting to send. + grpc_stream_.sendMessage( + sub->second->sub_state_.getNextRequestWithAck(pausable_ack_queue_.front())); + pausable_ack_queue_.pop(); + } else { + grpc_stream_.sendMessage(sub->second->sub_state_.getNextRequestAckless()); + } + } + grpc_stream_.maybeUpdateQueueSizeStat(pausable_ack_queue_.size()); +} + +// Checks whether external conditions allow sending a DeltaDiscoveryRequest. (Does not check +// whether we *want* to send a DeltaDiscoveryRequest). +bool NewGrpcMuxImpl::canSendDiscoveryRequest(const std::string& type_url) { + RELEASE_ASSERT( + !pausable_ack_queue_.paused(type_url), + fmt::format("canSendDiscoveryRequest() called on paused type_url {}. Pausedness is " + "supposed to be filtered out by whoWantsToSendDiscoveryRequest(). ", + type_url)); + + if (!grpc_stream_.grpcStreamAvailable()) { + ENVOY_LOG(trace, "No stream available to send a discovery request for {}.", type_url); + return false; + } else if (!grpc_stream_.checkRateLimitAllowsDrain()) { + ENVOY_LOG(trace, "{} discovery request hit rate limit; will try later.", type_url); + return false; + } + return true; +} + +// Checks whether we have something to say in a DeltaDiscoveryRequest, which can be an ACK and/or +// a subscription update. (Does not check whether we *can* send that DeltaDiscoveryRequest). +// Returns the type_url we should send the DeltaDiscoveryRequest for (if any). +// First, prioritizes ACKs over non-ACK subscription interest updates. +// Then, prioritizes non-ACK updates in the order the various types +// of subscriptions were activated. +absl::optional NewGrpcMuxImpl::whoWantsToSendDiscoveryRequest() { + // All ACKs are sent before plain updates. trySendDiscoveryRequests() relies on this. So, choose + // type_url from pausable_ack_queue_ if possible, before looking at pending updates. + if (!pausable_ack_queue_.empty()) { + return pausable_ack_queue_.front().type_url_; + } + // If we're looking to send multiple non-ACK requests, send them in the order that their + // subscriptions were initiated. + for (const auto& sub_type : subscription_ordering_) { + auto sub = subscriptions_.find(sub_type); + if (sub != subscriptions_.end() && sub->second->sub_state_.subscriptionUpdatePending() && + !pausable_ack_queue_.paused(sub_type)) { + return sub->first; + } + } + return absl::nullopt; +} + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/new_grpc_mux_impl.h b/source/common/config/new_grpc_mux_impl.h new file mode 100644 index 0000000000000..4315138b0d448 --- /dev/null +++ b/source/common/config/new_grpc_mux_impl.h @@ -0,0 +1,120 @@ +#pragma once + +#include "envoy/api/v2/discovery.pb.h" +#include "envoy/common/token_bucket.h" +#include "envoy/config/grpc_mux.h" +#include "envoy/config/subscription.h" + +#include "common/common/logger.h" +#include "common/config/delta_subscription_state.h" +#include "common/config/grpc_stream.h" +#include "common/config/pausable_ack_queue.h" +#include "common/config/watch_map.h" +#include "common/grpc/common.h" + +namespace Envoy { +namespace Config { + +// Manages subscriptions to one or more type of resource. The logical protocol +// state of those subscription(s) is handled by DeltaSubscriptionState. +// This class owns the GrpcStream used to talk to the server, maintains queuing +// logic to properly order the subscription(s)' various messages, and allows +// starting/stopping/pausing of the subscriptions. +class NewGrpcMuxImpl : public GrpcMux, + public GrpcStreamCallbacks, + Logger::Loggable { +public: + NewGrpcMuxImpl(Grpc::RawAsyncClientPtr&& async_client, Event::Dispatcher& dispatcher, + const Protobuf::MethodDescriptor& service_method, Runtime::RandomGenerator& random, + Stats::Scope& scope, const RateLimitSettings& rate_limit_settings, + const LocalInfo::LocalInfo& local_info); + + Watch* addOrUpdateWatch(const std::string& type_url, Watch* watch, + const std::set& resources, SubscriptionCallbacks& callbacks, + std::chrono::milliseconds init_fetch_timeout) override; + void removeWatch(const std::string& type_url, Watch* watch) override; + + // TODO(fredlas) PR #8478 will remove this. + bool isDelta() const override { return true; } + + void pause(const std::string& type_url) override; + void resume(const std::string& type_url) override; + bool paused(const std::string& type_url) const override; + void + onDiscoveryResponse(std::unique_ptr&& message) override; + + void onStreamEstablished() override; + + void onEstablishmentFailure() override; + + void onWriteable() override; + + void kickOffAck(UpdateAck ack); + + // TODO(fredlas) remove these two from the GrpcMux interface. + GrpcMuxWatchPtr subscribe(const std::string&, const std::set&, + GrpcMuxCallbacks&) override; + void start() override; + +private: + Watch* addWatch(const std::string& type_url, const std::set& resources, + SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout); + + // Updates the list of resource names watched by the given watch. If an added name is new across + // the whole subscription, or if a removed name has no other watch interested in it, then the + // subscription will enqueue and attempt to send an appropriate discovery request. + void updateWatch(const std::string& type_url, Watch* watch, + const std::set& resources); + + void addSubscription(const std::string& type_url, std::chrono::milliseconds init_fetch_timeout); + + void trySendDiscoveryRequests(); + + // Checks whether external conditions allow sending a DeltaDiscoveryRequest. (Does not check + // whether we *want* to send a DeltaDiscoveryRequest). + bool canSendDiscoveryRequest(const std::string& type_url); + + // Checks whether we have something to say in a DeltaDiscoveryRequest, which can be an ACK and/or + // a subscription update. (Does not check whether we *can* send that DeltaDiscoveryRequest). + // Returns the type_url we should send the DeltaDiscoveryRequest for (if any). + // First, prioritizes ACKs over non-ACK subscription interest updates. + // Then, prioritizes non-ACK updates in the order the various types + // of subscriptions were activated. + absl::optional whoWantsToSendDiscoveryRequest(); + + Event::Dispatcher& dispatcher_; + const LocalInfo::LocalInfo& local_info_; + + // Resource (N)ACKs we're waiting to send, stored in the order that they should be sent in. All + // of our different resource types' ACKs are mixed together in this queue. See class for + // description of how it interacts with pause() and resume(). + PausableAckQueue pausable_ack_queue_; + + struct SubscriptionStuff { + SubscriptionStuff(const std::string& type_url, std::chrono::milliseconds init_fetch_timeout, + Event::Dispatcher& dispatcher, const LocalInfo::LocalInfo& local_info) + : sub_state_(type_url, watch_map_, local_info, init_fetch_timeout, dispatcher), + init_fetch_timeout_(init_fetch_timeout) {} + + WatchMap watch_map_; + DeltaSubscriptionState sub_state_; + const std::chrono::milliseconds init_fetch_timeout_; + + SubscriptionStuff(const SubscriptionStuff&) = delete; + SubscriptionStuff& operator=(const SubscriptionStuff&) = delete; + }; + // Map key is type_url. + absl::flat_hash_map> subscriptions_; + + // Determines the order of initial discovery requests. (Assumes that subscriptions are added in + // the order of Envoy's dependency ordering). + std::list subscription_ordering_; + + GrpcStream + grpc_stream_; +}; + +using NewGrpcMuxImplSharedPtr = std::shared_ptr; + +} // namespace Config +} // namespace Envoy diff --git a/source/common/config/pausable_ack_queue.h b/source/common/config/pausable_ack_queue.h index 875ff3b2f23b2..46222a8b2e3ce 100644 --- a/source/common/config/pausable_ack_queue.h +++ b/source/common/config/pausable_ack_queue.h @@ -2,13 +2,21 @@ #include -#include "common/config/update_ack.h" +#include "envoy/api/v2/discovery.pb.h" #include "absl/container/flat_hash_map.h" namespace Envoy { namespace Config { +struct UpdateAck { + UpdateAck(absl::string_view nonce, absl::string_view type_url) + : nonce_(nonce), type_url_(type_url) {} + std::string nonce_; + std::string type_url_; + ::google::rpc::Status error_detail_; +}; + // There is a head-of-line blocking issue resulting from the intersection of 1) ADS's need for // subscription request ordering and 2) the ability to "pause" one of the resource types within ADS. // We need a queue that understands ADS's resource type pausing. Specifically, we need front()/pop() diff --git a/source/common/config/sotw_subscription_state.cc b/source/common/config/sotw_subscription_state.cc deleted file mode 100644 index 956716876ac6d..0000000000000 --- a/source/common/config/sotw_subscription_state.cc +++ /dev/null @@ -1,128 +0,0 @@ -#include "common/config/sotw_subscription_state.h" - -#include "common/common/assert.h" -#include "common/common/hash.h" - -namespace Envoy { -namespace Config { - -SotwSubscriptionState::SotwSubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout, - Event::Dispatcher& dispatcher) - : SubscriptionState(std::move(type_url), callbacks, init_fetch_timeout, dispatcher) {} - -SotwSubscriptionState::~SotwSubscriptionState() = default; - -SotwSubscriptionStateFactory::SotwSubscriptionStateFactory(Event::Dispatcher& dispatcher) - : dispatcher_(dispatcher) {} - -SotwSubscriptionStateFactory::~SotwSubscriptionStateFactory() = default; - -std::unique_ptr -SotwSubscriptionStateFactory::makeSubscriptionState(const std::string& type_url, - SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) { - return std::make_unique(type_url, callbacks, init_fetch_timeout, - dispatcher_); -} - -void SotwSubscriptionState::updateSubscriptionInterest(const std::set& cur_added, - const std::set& cur_removed) { - for (const auto& a : cur_added) { - names_tracked_.insert(a); - } - for (const auto& r : cur_removed) { - names_tracked_.erase(r); - } - if (!cur_added.empty() || !cur_removed.empty()) { - update_pending_ = true; - } -} - -// Not having sent any requests yet counts as an "update pending" since you're supposed to resend -// the entirety of your interest at the start of a stream, even if nothing has changed. -bool SotwSubscriptionState::subscriptionUpdatePending() const { return update_pending_; } - -void SotwSubscriptionState::markStreamFresh() { - last_good_version_info_ = absl::nullopt; - last_good_nonce_ = absl::nullopt; - update_pending_ = true; -} - -UpdateAck SotwSubscriptionState::handleResponse(const void* response_proto_ptr) { - auto* response = static_cast(response_proto_ptr); - // We *always* copy the response's nonce into the next request, even if we're going to make that - // request a NACK by setting error_detail. - UpdateAck ack(response->nonce(), type_url()); - try { - handleGoodResponse(*response); - } catch (const EnvoyException& e) { - handleBadResponse(e, ack); - } - return ack; -} - -void SotwSubscriptionState::handleGoodResponse(const envoy::api::v2::DiscoveryResponse& message) { - disableInitFetchTimeoutTimer(); - for (const auto& resource : message.resources()) { - if (resource.type_url() != type_url()) { - throw EnvoyException(fmt::format("type URL {} embedded in an individual Any does not match " - "the message-wide type URL {} in DiscoveryResponse {}", - resource.type_url(), type_url(), message.DebugString())); - } - } - callbacks().onConfigUpdate(message.resources(), message.version_info()); - // Now that we're passed onConfigUpdate() without an exception thrown, we know we're good. - last_good_version_info_ = message.version_info(); - last_good_nonce_ = message.nonce(); - ENVOY_LOG(debug, "Config update for {} accepted with {} resources", type_url(), - message.resources().size()); -} - -void SotwSubscriptionState::handleBadResponse(const EnvoyException& e, UpdateAck& ack) { - // Note that error_detail being set is what indicates that a DeltaDiscoveryRequest is a NACK. - ack.error_detail_.set_code(Grpc::Status::WellKnownGrpcStatus::Internal); - ack.error_detail_.set_message(e.what()); - disableInitFetchTimeoutTimer(); - ENVOY_LOG(warn, "gRPC state-of-the-world config for {} rejected: {}", type_url(), e.what()); - callbacks().onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, &e); -} - -void SotwSubscriptionState::handleEstablishmentFailure() { - callbacks().onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure, - nullptr); -} - -envoy::api::v2::DiscoveryRequest* SotwSubscriptionState::getNextRequestInternal() { - auto* request = new envoy::api::v2::DiscoveryRequest; - request->set_type_url(type_url()); - - std::copy(names_tracked_.begin(), names_tracked_.end(), - Protobuf::RepeatedFieldBackInserter(request->mutable_resource_names())); - if (last_good_version_info_.has_value()) { - request->set_version_info(last_good_version_info_.value()); - } - // Default response_nonce to the last known good one. If we are being called by - // getNextRequestWithAck(), this value will be overwritten. - if (last_good_nonce_.has_value()) { - request->set_response_nonce(last_good_nonce_.value()); - } - - update_pending_ = false; - return request; -} - -void* SotwSubscriptionState::getNextRequestAckless() { return getNextRequestInternal(); } - -void* SotwSubscriptionState::getNextRequestWithAck(const UpdateAck& ack) { - envoy::api::v2::DiscoveryRequest* request = getNextRequestInternal(); - request->set_response_nonce(ack.nonce_); - if (ack.error_detail_.code() != Grpc::Status::WellKnownGrpcStatus::Ok) { - // Don't needlessly make the field present-but-empty if status is ok. - request->mutable_error_detail()->CopyFrom(ack.error_detail_); - } - return request; -} - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/sotw_subscription_state.h b/source/common/config/sotw_subscription_state.h deleted file mode 100644 index 1133341e45396..0000000000000 --- a/source/common/config/sotw_subscription_state.h +++ /dev/null @@ -1,83 +0,0 @@ -#pragma once - -#include "envoy/api/v2/discovery.pb.h" -#include "envoy/grpc/status.h" - -#include "common/common/assert.h" -#include "common/common/hash.h" -#include "common/config/subscription_state.h" - -#include "absl/types/optional.h" - -namespace Envoy { -namespace Config { - -// Tracks the state of a "state-of-the-world" (i.e. not delta) xDS-over-gRPC protocol session. -class SotwSubscriptionState : public SubscriptionState { -public: - // Note that, outside of tests, we expect callbacks to always be a WatchMap. - SotwSubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout, - Event::Dispatcher& dispatcher); - ~SotwSubscriptionState() override; - - // Update which resources we're interested in subscribing to. - void updateSubscriptionInterest(const std::set& cur_added, - const std::set& cur_removed) override; - - // Whether there was a change in our subscription interest we have yet to inform the server of. - bool subscriptionUpdatePending() const override; - - void markStreamFresh() override; - - // message is expected to be a envoy::api::v2::DiscoveryResponse. - UpdateAck handleResponse(const void* response_proto_ptr) override; - - void handleEstablishmentFailure() override; - - // Returns the next gRPC request proto to be sent off to the server, based on this object's - // understanding of the current protocol state, and new resources that Envoy wants to request. - // Returns a new'd pointer, meant to be owned by the caller. - void* getNextRequestAckless() override; - // The WithAck version first calls the Ackless version, then adds in the passed-in ack. - // Returns a new'd pointer, meant to be owned by the caller. - void* getNextRequestWithAck(const UpdateAck& ack) override; - - SotwSubscriptionState(const SotwSubscriptionState&) = delete; - SotwSubscriptionState& operator=(const SotwSubscriptionState&) = delete; - -private: - // Returns a new'd pointer, meant to be owned by the caller. - envoy::api::v2::DiscoveryRequest* getNextRequestInternal(); - - void handleGoodResponse(const envoy::api::v2::DiscoveryResponse& message); - void handleBadResponse(const EnvoyException& e, UpdateAck& ack); - - // The version_info carried by the last accepted DiscoveryResponse. - // Remains empty until one is accepted. - absl::optional last_good_version_info_; - // The nonce carried by the last accepted DiscoveryResponse. - // Remains empty until one is accepted. - // Used when it's time to make a spontaneous (i.e. not primarily meant as an ACK) request. - absl::optional last_good_nonce_; - - // Starts true because we should send a request upon subscription start. - bool update_pending_{true}; - - absl::flat_hash_set names_tracked_; -}; - -class SotwSubscriptionStateFactory : public SubscriptionStateFactory { -public: - SotwSubscriptionStateFactory(Event::Dispatcher& dispatcher); - ~SotwSubscriptionStateFactory() override; - std::unique_ptr - makeSubscriptionState(const std::string& type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) override; - -private: - Event::Dispatcher& dispatcher_; -}; - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index b514868b7b010..9217b612b18d6 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -1,9 +1,11 @@ #include "common/config/subscription_factory_impl.h" +#include "common/config/delta_subscription_impl.h" #include "common/config/filesystem_subscription_impl.h" -#include "common/config/grpc_mux_impl.h" +#include "common/config/grpc_mux_subscription_impl.h" #include "common/config/grpc_subscription_impl.h" #include "common/config/http_subscription_impl.h" +#include "common/config/new_grpc_mux_impl.h" #include "common/config/type_to_endpoint.h" #include "common/config/utility.h" #include "common/protobuf/protobuf.h" @@ -27,8 +29,9 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( switch (config.config_source_specifier_case()) { case envoy::api::v2::core::ConfigSource::kPath: { Utility::checkFilesystemSubscriptionBackingPath(config.path(), api_); - return std::make_unique(dispatcher_, config.path(), callbacks, - stats, validation_visitor_, api_); + result = std::make_unique( + dispatcher_, config.path(), callbacks, stats, validation_visitor_, api_); + break; } case envoy::api::v2::core::ConfigSource::kApiConfigSource: { const envoy::api::v2::core::ApiConfigSource& api_config_source = config.api_config_source(); @@ -40,46 +43,56 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( "Please specify an explicit supported api_type in the following config:\n" + config.DebugString()); case envoy::api::v2::core::ApiConfigSource::REST: - return std::make_unique( + result = std::make_unique( local_info_, cm_, api_config_source.cluster_names()[0], dispatcher_, random_, Utility::apiConfigSourceRefreshDelay(api_config_source), Utility::apiConfigSourceRequestTimeout(api_config_source), restMethod(type_url), callbacks, stats, Utility::configSourceInitialFetchTimeout(config), validation_visitor_); + break; case envoy::api::v2::core::ApiConfigSource::GRPC: - return std::make_unique( - std::make_shared(Utility::factoryForGrpcApiConfigSource( - cm_.grpcAsyncClientManager(), api_config_source, scope) - ->create(), - dispatcher_, sotwGrpcMethod(type_url), random_, scope, - Utility::parseRateLimitSettings(api_config_source), - local_info_, - api_config_source.set_node_on_first_message_only()), - type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), - /*is_aggregated=*/false); - case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: - return std::make_unique( - std::make_shared(Utility::factoryForGrpcApiConfigSource( - cm_.grpcAsyncClientManager(), api_config_source, scope) - ->create(), - dispatcher_, deltaGrpcMethod(type_url), random_, scope, - Utility::parseRateLimitSettings(api_config_source), - local_info_, - api_config_source.set_node_on_first_message_only()), - type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), - /*is_aggregated=*/false); + result = std::make_unique( + local_info_, + Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), + api_config_source, scope) + ->create(), + dispatcher_, random_, sotwGrpcMethod(type_url), type_url, callbacks, stats, scope, + Utility::parseRateLimitSettings(api_config_source), + Utility::configSourceInitialFetchTimeout(config), + api_config_source.set_node_on_first_message_only()); + break; + case envoy::api::v2::core::ApiConfigSource::DELTA_GRPC: { + Utility::checkApiConfigSourceSubscriptionBackingCluster(cm_.clusters(), api_config_source); + result = std::make_unique( + std::make_shared( + Config::Utility::factoryForGrpcApiConfigSource(cm_.grpcAsyncClientManager(), + api_config_source, scope) + ->create(), + dispatcher_, deltaGrpcMethod(type_url), random_, scope, + Utility::parseRateLimitSettings(api_config_source), local_info_), + type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), false); + break; + } default: NOT_REACHED_GCOVR_EXCL_LINE; } + break; } case envoy::api::v2::core::ConfigSource::kAds: { - return std::make_unique(cm_.adsMux(), type_url, callbacks, stats, - Utility::configSourceInitialFetchTimeout(config), - /*is_aggregated=*/true); + if (cm_.adsMux()->isDelta()) { + result = std::make_unique( + cm_.adsMux(), type_url, callbacks, stats, + Utility::configSourceInitialFetchTimeout(config), true); + } else { + result = std::make_unique( + cm_.adsMux(), callbacks, stats, type_url, dispatcher_, + Utility::configSourceInitialFetchTimeout(config)); + } + break; } default: throw EnvoyException("Missing config source specifier in envoy::api::v2::core::ConfigSource"); } - NOT_REACHED_GCOVR_EXCL_LINE; + return result; } } // namespace Config diff --git a/source/common/config/subscription_state.cc b/source/common/config/subscription_state.cc deleted file mode 100644 index bfac3bc4bb143..0000000000000 --- a/source/common/config/subscription_state.cc +++ /dev/null @@ -1,34 +0,0 @@ -#include "common/config/subscription_state.h" - -#include -#include - -#include "envoy/api/v2/discovery.pb.h" -#include "envoy/common/pure.h" -#include "envoy/config/subscription.h" - -namespace Envoy { -namespace Config { - -SubscriptionState::SubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout, - Event::Dispatcher& dispatcher) - : type_url_(std::move(type_url)), callbacks_(callbacks) { - if (init_fetch_timeout.count() > 0 && !init_fetch_timeout_timer_) { - init_fetch_timeout_timer_ = dispatcher.createTimer([this]() -> void { - ENVOY_LOG(warn, "config: initial fetch timed out for {}", type_url_); - callbacks_.onConfigUpdateFailed(ConfigUpdateFailureReason::FetchTimedout, nullptr); - }); - init_fetch_timeout_timer_->enableTimer(init_fetch_timeout); - } -} - -void SubscriptionState::disableInitFetchTimeoutTimer() { - if (init_fetch_timeout_timer_) { - init_fetch_timeout_timer_->disableTimer(); - init_fetch_timeout_timer_.reset(); - } -} - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/subscription_state.h b/source/common/config/subscription_state.h deleted file mode 100644 index 85bd56222deff..0000000000000 --- a/source/common/config/subscription_state.h +++ /dev/null @@ -1,79 +0,0 @@ -#pragma once - -#include -#include - -#include "envoy/api/v2/discovery.pb.h" -#include "envoy/common/pure.h" -#include "envoy/config/subscription.h" -#include "envoy/event/dispatcher.h" - -#include "common/config/update_ack.h" -#include "common/protobuf/protobuf.h" - -#include "absl/strings/string_view.h" - -namespace Envoy { -namespace Config { - -// Tracks the protocol state of an individual ongoing xDS-over-gRPC session, for a single type_url. -// There can be multiple SubscriptionStates active, one per type_url. They will all be -// blissfully unaware of each other's existence, even when their messages are being multiplexed -// together by ADS. -// This is the abstract parent class for both the delta and state-of-the-world xDS variants. -class SubscriptionState : public Logger::Loggable { -public: - // Note that, outside of tests, we expect callbacks to always be a WatchMap. - SubscriptionState(std::string type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout, Event::Dispatcher& dispatcher); - virtual ~SubscriptionState() = default; - - // Update which resources we're interested in subscribing to. - virtual void updateSubscriptionInterest(const std::set& cur_added, - const std::set& cur_removed) PURE; - - // Whether there was a change in our subscription interest we have yet to inform the server of. - virtual bool subscriptionUpdatePending() const PURE; - - virtual void markStreamFresh() PURE; - - // Implementations expect either a DeltaDiscoveryResponse or DiscoveryResponse. The caller is - // expected to know which it should be providing. - virtual UpdateAck handleResponse(const void* response_proto_ptr) PURE; - - virtual void handleEstablishmentFailure() PURE; - - // Returns the next gRPC request proto to be sent off to the server, based on this object's - // understanding of the current protocol state, and new resources that Envoy wants to request. - // Returns a new'd pointer, meant to be owned by the caller, who is expected to know what type the - // pointer actually is. - virtual void* getNextRequestAckless() PURE; - // The WithAck version first calls the Ackless version, then adds in the passed-in ack. - // Returns a new'd pointer, meant to be owned by the caller, who is expected to know what type the - // pointer actually is. - virtual void* getNextRequestWithAck(const UpdateAck& ack) PURE; - - void disableInitFetchTimeoutTimer(); - -protected: - std::string type_url() const { return type_url_; } - SubscriptionCallbacks& callbacks() const { return callbacks_; } - -private: - const std::string type_url_; - // callbacks_ is expected (outside of tests) to be a WatchMap. - SubscriptionCallbacks& callbacks_; - Event::TimerPtr init_fetch_timeout_timer_; -}; - -class SubscriptionStateFactory { -public: - virtual ~SubscriptionStateFactory() = default; - // Note that, outside of tests, we expect callbacks to always be a WatchMap. - virtual std::unique_ptr - makeSubscriptionState(const std::string& type_url, SubscriptionCallbacks& callbacks, - std::chrono::milliseconds init_fetch_timeout) PURE; -}; - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/update_ack.h b/source/common/config/update_ack.h deleted file mode 100644 index 193e86e436a08..0000000000000 --- a/source/common/config/update_ack.h +++ /dev/null @@ -1,21 +0,0 @@ -#pragma once - -#include - -#include "envoy/api/v2/discovery.pb.h" - -#include "absl/strings/string_view.h" - -namespace Envoy { -namespace Config { - -struct UpdateAck { - UpdateAck(absl::string_view nonce, absl::string_view type_url) - : nonce_(nonce), type_url_(type_url) {} - std::string nonce_; - std::string type_url_; - ::google::rpc::Status error_detail_; -}; - -} // namespace Config -} // namespace Envoy diff --git a/source/common/config/watch_map.h b/source/common/config/watch_map.h index e98c8af171221..9df852c7f7127 100644 --- a/source/common/config/watch_map.h +++ b/source/common/config/watch_map.h @@ -33,7 +33,8 @@ struct Watch { }; // NOTE: Users are responsible for eventually calling removeWatch() on the Watch* returned -// by addWatch(). However, we don't expect there to be new users of this class. +// by addWatch(). We don't expect there to be new users of this class beyond +// NewGrpcMuxImpl and DeltaSubscriptionImpl (TODO(fredlas) to be renamed). // // Manages "watches" of xDS resources. Several xDS callers might ask for a subscription to the same // resource name "X". The xDS machinery must return to each their very own subscription to X. diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 9694c7c9d6542..da694ab7ddaff 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -18,6 +18,7 @@ #include "common/common/enum_to_int.h" #include "common/common/fmt.h" #include "common/common/utility.h" +#include "common/config/new_grpc_mux_impl.h" #include "common/config/resources.h" #include "common/config/utility.h" #include "common/grpc/async_client_manager_impl.h" @@ -219,6 +220,8 @@ ClusterManagerImpl::ClusterManagerImpl( } } + const auto& dyn_resources = bootstrap.dynamic_resources(); + // Cluster loading happens in two phases: first all the primary clusters are loaded, and then all // the secondary clusters are loaded. As it currently stands all non-EDS clusters and EDS which // load endpoint definition from file are primary and @@ -240,26 +243,33 @@ ClusterManagerImpl::ClusterManagerImpl( // This is the only point where distinction between delta ADS and state-of-the-world ADS is made. // After here, we just have a GrpcMux interface held in ads_mux_, which hides // whether the backing implementation is delta or SotW. - if (bootstrap.dynamic_resources().has_ads_config()) { - auto& ads_config = bootstrap.dynamic_resources().ads_config(); - if (ads_config.api_type() == envoy::api::v2::core::ApiConfigSource::DELTA_GRPC) { - ads_mux_ = std::make_shared( - Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, ads_config, stats) + if (dyn_resources.has_ads_config()) { + if (dyn_resources.ads_config().api_type() == + envoy::api::v2::core::ApiConfigSource::DELTA_GRPC) { + auto& api_config_source = dyn_resources.has_ads_config() + ? dyn_resources.ads_config() + : dyn_resources.cds_config().api_config_source(); + ads_mux_ = std::make_shared( + Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, api_config_source, + stats) ->create(), main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.DeltaAggregatedResources"), - random_, stats_, Envoy::Config::Utility::parseRateLimitSettings(ads_config), local_info, - ads_config.set_node_on_first_message_only()); + random_, stats_, + Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), local_info); } else { - ads_mux_ = std::make_shared( - Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, ads_config, stats) + ads_mux_ = std::make_shared( + local_info, + Config::Utility::factoryForGrpcApiConfigSource(*async_client_manager_, + dyn_resources.ads_config(), stats) ->create(), main_thread_dispatcher, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), - random_, stats_, Envoy::Config::Utility::parseRateLimitSettings(ads_config), local_info, - ads_config.set_node_on_first_message_only()); + random_, stats_, + Envoy::Config::Utility::parseRateLimitSettings(dyn_resources.ads_config()), + bootstrap.dynamic_resources().ads_config().set_node_on_first_message_only()); } } else { ads_mux_ = std::make_unique(); @@ -296,8 +306,8 @@ ClusterManagerImpl::ClusterManagerImpl( }); // We can now potentially create the CDS API once the backing cluster exists. - if (bootstrap.dynamic_resources().has_cds_config()) { - cds_api_ = factory_.createCds(bootstrap.dynamic_resources().cds_config(), *this); + if (dyn_resources.has_cds_config()) { + cds_api_ = factory_.createCds(dyn_resources.cds_config(), *this); init_helper_.setCds(cds_api_.get()); } else { init_helper_.setCds(nullptr); diff --git a/test/common/config/BUILD b/test/common/config/BUILD index f65907bd3eafa..7367f62e35bb3 100644 --- a/test/common/config/BUILD +++ b/test/common/config/BUILD @@ -15,7 +15,7 @@ envoy_cc_test( srcs = ["delta_subscription_impl_test.cc"], deps = [ ":delta_subscription_test_harness", - "//source/common/config:grpc_subscription_lib", + "//source/common/config:delta_subscription_lib", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", "//test/mocks/config:config_mocks", @@ -31,7 +31,7 @@ envoy_cc_test( name = "delta_subscription_state_test", srcs = ["delta_subscription_state_test.cc"], deps = [ - "//source/common/config:delta_subscription_state_lib", + "//source/common/config:delta_subscription_lib", "//source/common/stats:isolated_store_lib", "//test/mocks:common_lib", "//test/mocks/config:config_mocks", @@ -76,7 +76,28 @@ envoy_cc_test( srcs = ["grpc_mux_impl_test.cc"], deps = [ "//source/common/config:grpc_mux_lib", - "//source/common/config:grpc_subscription_lib", + "//source/common/config:protobuf_link_hacks", + "//source/common/config:resources_lib", + "//source/common/protobuf", + "//source/common/stats:isolated_store_lib", + "//test/mocks:common_lib", + "//test/mocks/config:config_mocks", + "//test/mocks/event:event_mocks", + "//test/mocks/grpc:grpc_mocks", + "//test/mocks/local_info:local_info_mocks", + "//test/mocks/runtime:runtime_mocks", + "//test/test_common:logging_lib", + "//test/test_common:simulated_time_system_lib", + "//test/test_common:utility_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", + ], +) + +envoy_cc_test( + name = "new_grpc_mux_impl_test", + srcs = ["new_grpc_mux_impl_test.cc"], + deps = [ + "//source/common/config:new_grpc_mux_lib", "//source/common/config:protobuf_link_hacks", "//source/common/config:resources_lib", "//source/common/protobuf", @@ -139,7 +160,7 @@ envoy_cc_test_library( hdrs = ["delta_subscription_test_harness.h"], deps = [ ":subscription_test_harness", - "//source/common/config:grpc_subscription_lib", + "//source/common/config:delta_subscription_lib", "//source/common/grpc:common_lib", "//test/mocks/config:config_mocks", "//test/mocks/event:event_mocks", @@ -178,22 +199,6 @@ envoy_cc_test_library( ], ) -envoy_cc_test( - name = "sotw_subscription_state_test", - srcs = ["sotw_subscription_state_test.cc"], - deps = [ - "//source/common/config:sotw_subscription_state_lib", - "//source/common/stats:isolated_store_lib", - "//test/mocks:common_lib", - "//test/mocks/config:config_mocks", - "//test/mocks/event:event_mocks", - "//test/mocks/grpc:grpc_mocks", - "//test/mocks/local_info:local_info_mocks", - "//test/mocks/runtime:runtime_mocks", - "//test/test_common:logging_lib", - ], -) - envoy_cc_test( name = "subscription_factory_impl_test", srcs = ["subscription_factory_impl_test.cc"], diff --git a/test/common/config/delta_subscription_impl_test.cc b/test/common/config/delta_subscription_impl_test.cc index 65777ec1217f0..e6b783b6583fa 100644 --- a/test/common/config/delta_subscription_impl_test.cc +++ b/test/common/config/delta_subscription_impl_test.cc @@ -11,7 +11,7 @@ class DeltaSubscriptionImplTest : public DeltaSubscriptionTestHarness, public te DeltaSubscriptionImplTest() = default; // We need to destroy the subscription before the test's destruction, because the subscription's - // destructor removes its watch from the GrpcMuxDelta, and that removal process involves + // destructor removes its watch from the NewGrpcMuxImpl, and that removal process involves // some things held by the test fixture. void TearDown() override { doSubscriptionTearDown(); } }; @@ -71,8 +71,8 @@ TEST_F(DeltaSubscriptionImplTest, PauseQueuesAcks) { message->set_nonce(nonce); message->set_type_url(Config::TypeUrl::get().ClusterLoadAssignment); nonce_acks_required_.push(nonce); - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get())->onDiscoveryResponse(std::move(message)); + static_cast(subscription_->getContextForTest().get()) + ->onDiscoveryResponse(std::move(message)); } // The server gives us our first version of resource name2. // subscription_ now wants to ACK name1 and then name2 (but can't due to pause). @@ -85,8 +85,8 @@ TEST_F(DeltaSubscriptionImplTest, PauseQueuesAcks) { message->set_nonce(nonce); message->set_type_url(Config::TypeUrl::get().ClusterLoadAssignment); nonce_acks_required_.push(nonce); - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get())->onDiscoveryResponse(std::move(message)); + static_cast(subscription_->getContextForTest().get()) + ->onDiscoveryResponse(std::move(message)); } // The server gives us an updated version of resource name1. // subscription_ now wants to ACK name1A, then name2, then name1B (but can't due to pause). @@ -99,8 +99,8 @@ TEST_F(DeltaSubscriptionImplTest, PauseQueuesAcks) { message->set_nonce(nonce); message->set_type_url(Config::TypeUrl::get().ClusterLoadAssignment); nonce_acks_required_.push(nonce); - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get())->onDiscoveryResponse(std::move(message)); + static_cast(subscription_->getContextForTest().get()) + ->onDiscoveryResponse(std::move(message)); } // All ACK sendMessage()s will happen upon calling resume(). EXPECT_CALL(async_stream_, sendMessageRaw_(_, _)) @@ -135,11 +135,11 @@ TEST(DeltaSubscriptionImplFixturelessTest, NoGrpcStream) { const Protobuf::MethodDescriptor* method_descriptor = Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints"); - std::shared_ptr xds_context = std::make_shared( + std::shared_ptr xds_context = std::make_shared( std::unique_ptr(async_client), dispatcher, *method_descriptor, random, - stats_store, rate_limit_settings, local_info, true); + stats_store, rate_limit_settings, local_info); - auto subscription = std::make_unique( + std::unique_ptr subscription = std::make_unique( xds_context, Config::TypeUrl::get().ClusterLoadAssignment, callbacks, stats, std::chrono::milliseconds(12345), false); diff --git a/test/common/config/delta_subscription_state_test.cc b/test/common/config/delta_subscription_state_test.cc index bdcb52995ab53..f98dfe5aa05ef 100644 --- a/test/common/config/delta_subscription_state_test.cc +++ b/test/common/config/delta_subscription_state_test.cc @@ -22,18 +22,13 @@ const char TypeUrl[] = "type.googleapis.com/envoy.api.v2.Cluster"; class DeltaSubscriptionStateTest : public testing::Test { protected: DeltaSubscriptionStateTest() - : state_(TypeUrl, callbacks_, std::chrono::milliseconds(0U), dispatcher_) { + : state_(TypeUrl, callbacks_, local_info_, std::chrono::milliseconds(0U), dispatcher_) { state_.updateSubscriptionInterest({"name1", "name2", "name3"}, {}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name1", "name2", "name3")); } - std::unique_ptr getNextDeltaDiscoveryRequestAckless() { - auto* ptr = static_cast(state_.getNextRequestAckless()); - return std::unique_ptr(ptr); - } - UpdateAck deliverDiscoveryResponse( const Protobuf::RepeatedPtrField& added_resources, const Protobuf::RepeatedPtrField& removed_resources, @@ -47,7 +42,7 @@ class DeltaSubscriptionStateTest : public testing::Test { message.set_nonce(nonce.value()); } EXPECT_CALL(callbacks_, onConfigUpdate(_, _, _)).Times(expect_config_update_call ? 1 : 0); - return state_.handleResponse(&message); + return state_.handleResponse(message); } UpdateAck deliverBadDiscoveryResponse( @@ -60,10 +55,11 @@ class DeltaSubscriptionStateTest : public testing::Test { message.set_system_version_info(version_info); message.set_nonce(nonce); EXPECT_CALL(callbacks_, onConfigUpdate(_, _, _)).WillOnce(Throw(EnvoyException("oh no"))); - return state_.handleResponse(&message); + return state_.handleResponse(message); } NiceMock> callbacks_; + NiceMock local_info_; NiceMock dispatcher_; // We start out interested in three resources: name1, name2, and name3. DeltaSubscriptionState state_; @@ -84,15 +80,15 @@ populateRepeatedResource(std::vector> items) TEST_F(DeltaSubscriptionStateTest, SubscribeAndUnsubscribe) { { state_.updateSubscriptionInterest({"name4"}, {"name1"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name4")); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), UnorderedElementsAre("name1")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name4")); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name1")); } { state_.updateSubscriptionInterest({"name1"}, {"name3", "name4"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name1")); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), UnorderedElementsAre("name3", "name4")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name1")); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name3", "name4")); } } @@ -108,9 +104,9 @@ TEST_F(DeltaSubscriptionStateTest, SubscribeAndUnsubscribe) { TEST_F(DeltaSubscriptionStateTest, RemoveThenAdd) { state_.updateSubscriptionInterest({}, {"name3"}); state_.updateSubscriptionInterest({"name3"}, {}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name3")); - EXPECT_TRUE(cur_request->resource_names_unsubscribe().empty()); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name3")); + EXPECT_TRUE(cur_request.resource_names_unsubscribe().empty()); } // Due to how our implementation provides the required behavior tested in RemoveThenAdd, the @@ -124,9 +120,9 @@ TEST_F(DeltaSubscriptionStateTest, RemoveThenAdd) { TEST_F(DeltaSubscriptionStateTest, AddThenRemove) { state_.updateSubscriptionInterest({"name4"}, {}); state_.updateSubscriptionInterest({}, {"name4"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_TRUE(cur_request->resource_names_subscribe().empty()); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), UnorderedElementsAre("name4")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_TRUE(cur_request.resource_names_subscribe().empty()); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name4")); } // add/remove/add == add. @@ -134,9 +130,9 @@ TEST_F(DeltaSubscriptionStateTest, AddRemoveAdd) { state_.updateSubscriptionInterest({"name4"}, {}); state_.updateSubscriptionInterest({}, {"name4"}); state_.updateSubscriptionInterest({"name4"}, {}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name4")); - EXPECT_TRUE(cur_request->resource_names_unsubscribe().empty()); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name4")); + EXPECT_TRUE(cur_request.resource_names_unsubscribe().empty()); } // remove/add/remove == remove. @@ -144,9 +140,9 @@ TEST_F(DeltaSubscriptionStateTest, RemoveAddRemove) { state_.updateSubscriptionInterest({}, {"name3"}); state_.updateSubscriptionInterest({"name3"}, {}); state_.updateSubscriptionInterest({}, {"name3"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_TRUE(cur_request->resource_names_subscribe().empty()); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), UnorderedElementsAre("name3")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_TRUE(cur_request.resource_names_subscribe().empty()); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name3")); } // Starts with 1,2,3. 4 is added/removed/added. In those same updates, 1,2,3 are @@ -155,18 +151,18 @@ TEST_F(DeltaSubscriptionStateTest, BothAddAndRemove) { state_.updateSubscriptionInterest({"name4"}, {"name1", "name2", "name3"}); state_.updateSubscriptionInterest({"name1", "name2", "name3"}, {"name4"}); state_.updateSubscriptionInterest({"name4"}, {"name1", "name2", "name3"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name4")); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name4")); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name1", "name2", "name3")); } TEST_F(DeltaSubscriptionStateTest, CumulativeUpdates) { state_.updateSubscriptionInterest({"name4"}, {}); state_.updateSubscriptionInterest({"name5"}, {}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name4", "name5")); - EXPECT_TRUE(cur_request->resource_names_unsubscribe().empty()); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name4", "name5")); + EXPECT_TRUE(cur_request.resource_names_unsubscribe().empty()); } // Verifies that a sequence of good and bad responses from the server all get the appropriate @@ -218,11 +214,11 @@ TEST_F(DeltaSubscriptionStateTest, ResourceGoneLeadsToBlankInitialVersion) { populateRepeatedResource({{"name1", "version1A"}, {"name2", "version2A"}}); deliverDiscoveryResponse(add1_2, {}, "debugversion1"); state_.markStreamFresh(); // simulate a stream reconnection - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_EQ("version1A", cur_request->initial_resource_versions().at("name1")); - EXPECT_EQ("version2A", cur_request->initial_resource_versions().at("name2")); - EXPECT_EQ(cur_request->initial_resource_versions().end(), - cur_request->initial_resource_versions().find("name3")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_EQ("version1A", cur_request.initial_resource_versions().at("name1")); + EXPECT_EQ("version2A", cur_request.initial_resource_versions().at("name2")); + EXPECT_EQ(cur_request.initial_resource_versions().end(), + cur_request.initial_resource_versions().find("name3")); } { @@ -233,11 +229,11 @@ TEST_F(DeltaSubscriptionStateTest, ResourceGoneLeadsToBlankInitialVersion) { *remove2.Add() = "name2"; deliverDiscoveryResponse(add1_3, remove2, "debugversion2"); state_.markStreamFresh(); // simulate a stream reconnection - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_EQ("version1B", cur_request->initial_resource_versions().at("name1")); - EXPECT_EQ(cur_request->initial_resource_versions().end(), - cur_request->initial_resource_versions().find("name2")); - EXPECT_EQ("version3A", cur_request->initial_resource_versions().at("name3")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_EQ("version1B", cur_request.initial_resource_versions().at("name1")); + EXPECT_EQ(cur_request.initial_resource_versions().end(), + cur_request.initial_resource_versions().find("name2")); + EXPECT_EQ("version3A", cur_request.initial_resource_versions().at("name3")); } { @@ -247,17 +243,17 @@ TEST_F(DeltaSubscriptionStateTest, ResourceGoneLeadsToBlankInitialVersion) { *remove1_3.Add() = "name3"; deliverDiscoveryResponse({}, remove1_3, "debugversion3"); state_.markStreamFresh(); // simulate a stream reconnection - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_TRUE(cur_request->initial_resource_versions().empty()); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_TRUE(cur_request.initial_resource_versions().empty()); } { // ...but our own map should remember our interest. In particular, losing interest in a // resource should cause its name to appear in the next request's resource_names_unsubscribe. state_.updateSubscriptionInterest({"name4"}, {"name1", "name2"}); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names_subscribe(), UnorderedElementsAre("name4")); - EXPECT_THAT(cur_request->resource_names_unsubscribe(), UnorderedElementsAre("name1", "name2")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name4")); + EXPECT_THAT(cur_request.resource_names_unsubscribe(), UnorderedElementsAre("name1", "name2")); } } @@ -276,16 +272,16 @@ TEST_F(DeltaSubscriptionStateTest, SubscribeAndUnsubscribeAfterReconnect) { state_.updateSubscriptionInterest({"name4"}, {"name1"}); state_.markStreamFresh(); // simulate a stream reconnection - auto cur_request = getNextDeltaDiscoveryRequestAckless(); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); // Regarding the resource_names_subscribe field: // name1: do not include: we lost interest. // name2: yes do include: we're interested and we have a version of it. // name3: yes do include: even though we don't have a version of it, we are interested. // name4: yes do include: we are newly interested. (If this wasn't a stream reconnect, only name4 // would belong in this subscribe field). - EXPECT_THAT(cur_request->resource_names_subscribe(), + EXPECT_THAT(cur_request.resource_names_subscribe(), UnorderedElementsAre("name2", "name3", "name4")); - EXPECT_TRUE(cur_request->resource_names_unsubscribe().empty()); + EXPECT_TRUE(cur_request.resource_names_unsubscribe().empty()); } // initial_resource_versions should not be present on messages after the first in a stream. @@ -297,10 +293,10 @@ TEST_F(DeltaSubscriptionStateTest, InitialVersionMapFirstMessageOnly) { {{"name1", "version1A"}, {"name2", "version2A"}, {"name3", "version3A"}}); deliverDiscoveryResponse(add_all, {}, "debugversion1"); state_.markStreamFresh(); // simulate a stream reconnection - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_EQ("version1A", cur_request->initial_resource_versions().at("name1")); - EXPECT_EQ("version2A", cur_request->initial_resource_versions().at("name2")); - EXPECT_EQ("version3A", cur_request->initial_resource_versions().at("name3")); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_EQ("version1A", cur_request.initial_resource_versions().at("name1")); + EXPECT_EQ("version2A", cur_request.initial_resource_versions().at("name2")); + EXPECT_EQ("version3A", cur_request.initial_resource_versions().at("name3")); } // Then, after updating the resources but not reconnecting the stream, verify that initial // versions are not sent. @@ -313,8 +309,8 @@ TEST_F(DeltaSubscriptionStateTest, InitialVersionMapFirstMessageOnly) { {"name3", "version3B"}, {"name4", "version4A"}}); deliverDiscoveryResponse(add_all, {}, "debugversion2"); - auto cur_request = getNextDeltaDiscoveryRequestAckless(); - EXPECT_TRUE(cur_request->initial_resource_versions().empty()); + envoy::api::v2::DeltaDiscoveryRequest cur_request = state_.getNextRequestAckless(); + EXPECT_TRUE(cur_request.initial_resource_versions().empty()); } } @@ -363,15 +359,6 @@ TEST_F(DeltaSubscriptionStateTest, AddedAndRemoved) { ack.error_detail_.message()); } -TEST_F(DeltaSubscriptionStateTest, handleEstablishmentFailure) { - // Although establishment failure is not supposed to cause an onConfigUpdateFailed() on the - // ultimate actual subscription callbacks, DeltaSubscriptionState's callbacks are actually - // the WatchMap, which then calls GrpcSubscriptionImpl(s). It is the GrpcSubscriptionImpl - // that will decline to pass on an onConfigUpdateFailed(ConnectionFailure). - EXPECT_CALL(callbacks_, onConfigUpdateFailed(_, _)); - state_.handleEstablishmentFailure(); -} - } // namespace } // namespace Config } // namespace Envoy diff --git a/test/common/config/delta_subscription_test_harness.h b/test/common/config/delta_subscription_test_harness.h index 0d22193905826..bf35490d4b901 100644 --- a/test/common/config/delta_subscription_test_harness.h +++ b/test/common/config/delta_subscription_test_harness.h @@ -2,7 +2,7 @@ #include -#include "common/config/grpc_subscription_impl.h" +#include "common/config/delta_subscription_impl.h" #include "common/grpc/common.h" #include "test/common/config/subscription_test_harness.h" @@ -34,11 +34,11 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { node_.set_id("fo0"); EXPECT_CALL(local_info_, node()).WillRepeatedly(testing::ReturnRef(node_)); EXPECT_CALL(dispatcher_, createTimer_(_)); - grpc_mux_ = std::make_shared( + xds_context_ = std::make_shared( std::unique_ptr(async_client_), dispatcher_, *method_descriptor_, - random_, stats_store_, rate_limit_settings_, local_info_, false); - subscription_ = std::make_unique( - grpc_mux_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, + random_, stats_store_, rate_limit_settings_, local_info_); + subscription_ = std::make_unique( + xds_context_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, init_fetch_timeout, false); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); } @@ -148,8 +148,8 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { Envoy::Config::ConfigUpdateFailureReason::UpdateRejected, _)); expectSendMessage({}, {}, Grpc::Status::WellKnownGrpcStatus::Internal, "bad config", {}); } - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get())->onDiscoveryResponse(std::move(response)); + static_cast(subscription_->getContextForTest().get()) + ->onDiscoveryResponse(std::move(response)); Mock::VerifyAndClearExpectations(&async_stream_); } @@ -189,8 +189,8 @@ class DeltaSubscriptionTestHarness : public SubscriptionTestHarness { NiceMock random_; NiceMock local_info_; Grpc::MockAsyncStream async_stream_; - std::shared_ptr grpc_mux_; - std::unique_ptr subscription_; + std::shared_ptr xds_context_; + std::unique_ptr subscription_; std::string last_response_nonce_; std::set last_cluster_names_; Envoy::Config::RateLimitSettings rate_limit_settings_; diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index 6ea4d7602d1bc..c9fb873d61581 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -48,19 +48,19 @@ class GrpcMuxImplTestBase : public testing::Test { stats_.gauge("control_plane.connected_state", Stats::Gauge::ImportMode::NeverImport)) {} void setup() { - grpc_mux_ = std::make_unique( - std::unique_ptr(async_client_), dispatcher_, + grpc_mux_ = std::make_unique( + local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), - random_, stats_, rate_limit_settings_, local_info_, true); + random_, stats_, rate_limit_settings_, true); } void setup(const RateLimitSettings& custom_rate_limit_settings) { - grpc_mux_ = std::make_unique( - std::unique_ptr(async_client_), dispatcher_, + grpc_mux_ = std::make_unique( + local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), - random_, stats_, custom_rate_limit_settings, local_info_, true); + random_, stats_, custom_rate_limit_settings, true); } void expectSendMessage(const std::string& type_url, @@ -85,49 +85,15 @@ class GrpcMuxImplTestBase : public testing::Test { error_detail->set_code(error_code); error_detail->set_message(error_message); } - EXPECT_CALL( - async_stream_, - sendMessageRaw_(Grpc::ProtoBufferEqIgnoreRepeatedFieldOrdering(expected_request), false)); - } - - // These tests were written around GrpcMuxWatch, an RAII type returned by the old subscribe(). - // To preserve these tests for the new code, we need an RAII watch handler. That is - // GrpcSubscriptionImpl, but to keep things simple, we'll fake it. (What we really care about - // is the destructor, which is identical to the real one). - class FakeGrpcSubscription { - public: - FakeGrpcSubscription(GrpcMux* grpc_mux, std::string type_url, Watch* watch) - : grpc_mux_(grpc_mux), type_url_(std::move(type_url)), watch_(watch) {} - ~FakeGrpcSubscription() { grpc_mux_->removeWatch(type_url_, watch_); } - - private: - GrpcMux* const grpc_mux_; - std::string type_url_; - Watch* const watch_; - }; - - FakeGrpcSubscription makeWatch(const std::string& type_url, - const std::set& resources) { - return FakeGrpcSubscription(grpc_mux_.get(), type_url, - grpc_mux_->addOrUpdateWatch(type_url, nullptr, resources, - callbacks_, - std::chrono::milliseconds(0))); - } - - FakeGrpcSubscription - makeWatch(const std::string& type_url, const std::set& resources, - NiceMock>& callbacks) { - return FakeGrpcSubscription(grpc_mux_.get(), type_url, - grpc_mux_->addOrUpdateWatch(type_url, nullptr, resources, callbacks, - std::chrono::milliseconds(0))); + EXPECT_CALL(async_stream_, sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); } NiceMock dispatcher_; NiceMock random_; Grpc::MockAsyncClient* async_client_; Grpc::MockAsyncStream async_stream_; - std::unique_ptr grpc_mux_; - NiceMock> callbacks_; + std::unique_ptr grpc_mux_; + NiceMock callbacks_; NiceMock local_info_; Stats::IsolatedStoreImpl stats_; Envoy::Config::RateLimitSettings rate_limit_settings_; @@ -139,25 +105,33 @@ class GrpcMuxImplTest : public GrpcMuxImplTestBase { Event::SimulatedTimeSystem time_system_; }; -// Validate behavior when multiple type URL watches are maintained, watches are created/destroyed. +// TODO(fredlas) #8478 will delete this. +TEST_F(GrpcMuxImplTest, JustForCoverageTodoDelete) { + setup(); + NullGrpcMuxImpl fake; + EXPECT_FALSE(grpc_mux_->isDelta()); + EXPECT_FALSE(fake.isDelta()); +} + +// Validate behavior when multiple type URL watches are maintained, watches are created/destroyed +// (via RAII). TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { setup(); InSequence s; - - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x", "y"}); - FakeGrpcSubscription bar_sub = makeWatch("type_url_bar", {}); + auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_); + auto bar_sub = grpc_mux_->subscribe("bar", {}, callbacks_); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); - expectSendMessage("type_url_foo", {"x", "y"}, "", true); - expectSendMessage("type_url_bar", {}, ""); + expectSendMessage("foo", {"x", "y"}, "", true); + expectSendMessage("bar", {}, ""); grpc_mux_->start(); EXPECT_EQ(1, control_plane_connected_state_.value()); - expectSendMessage("type_url_bar", {"z"}, ""); - FakeGrpcSubscription bar_z_sub = makeWatch("type_url_bar", {"z"}); - expectSendMessage("type_url_bar", {"zz", "z"}, ""); - FakeGrpcSubscription bar_zz_sub = makeWatch("type_url_bar", {"zz"}); - expectSendMessage("type_url_bar", {"z"}, ""); - expectSendMessage("type_url_bar", {}, ""); - expectSendMessage("type_url_foo", {}, ""); + expectSendMessage("bar", {"z"}, ""); + auto bar_z_sub = grpc_mux_->subscribe("bar", {"z"}, callbacks_); + expectSendMessage("bar", {"zz", "z"}, ""); + auto bar_zz_sub = grpc_mux_->subscribe("bar", {"zz"}, callbacks_); + expectSendMessage("bar", {"z"}, ""); + expectSendMessage("bar", {}, ""); + expectSendMessage("foo", {}, ""); } // Validate behavior when multiple type URL watches are maintained and the stream is reset. @@ -174,13 +148,13 @@ TEST_F(GrpcMuxImplTest, ResetStream) { })); setup(); - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x", "y"}); - FakeGrpcSubscription bar_sub = makeWatch("type_url_bar", {}); - FakeGrpcSubscription baz_sub = makeWatch("type_url_baz", {"z"}); + auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_); + auto bar_sub = grpc_mux_->subscribe("bar", {}, callbacks_); + auto baz_sub = grpc_mux_->subscribe("baz", {"z"}, callbacks_); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); - expectSendMessage("type_url_foo", {"x", "y"}, "", true); - expectSendMessage("type_url_bar", {}, ""); - expectSendMessage("type_url_baz", {"z"}, ""); + expectSendMessage("foo", {"x", "y"}, "", true); + expectSendMessage("bar", {}, ""); + expectSendMessage("baz", {"z"}, ""); grpc_mux_->start(); EXPECT_CALL(callbacks_, @@ -192,122 +166,125 @@ TEST_F(GrpcMuxImplTest, ResetStream) { grpc_mux_->grpcStreamForTest().onRemoteClose(Grpc::Status::WellKnownGrpcStatus::Canceled, ""); EXPECT_EQ(0, control_plane_connected_state_.value()); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); - expectSendMessage("type_url_foo", {"x", "y"}, "", true); - expectSendMessage("type_url_bar", {}, ""); - expectSendMessage("type_url_baz", {"z"}, ""); + expectSendMessage("foo", {"x", "y"}, "", true); + expectSendMessage("bar", {}, ""); + expectSendMessage("baz", {"z"}, ""); timer_cb(); - expectSendMessage("type_url_baz", {}, ""); - expectSendMessage("type_url_foo", {}, ""); + expectSendMessage("baz", {}, ""); + expectSendMessage("foo", {}, ""); } // Validate pause-resume behavior. TEST_F(GrpcMuxImplTest, PauseResume) { setup(); InSequence s; - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x", "y"}); - grpc_mux_->pause("type_url_foo"); + auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_); + grpc_mux_->pause("foo"); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); grpc_mux_->start(); - expectSendMessage("type_url_foo", {"x", "y"}, "", true); - grpc_mux_->resume("type_url_foo"); - grpc_mux_->pause("type_url_bar"); - expectSendMessage("type_url_foo", {"z", "x", "y"}, ""); - FakeGrpcSubscription foo_z_sub = makeWatch("type_url_foo", {"z"}); - grpc_mux_->resume("type_url_bar"); - grpc_mux_->pause("type_url_foo"); - FakeGrpcSubscription foo_zz_sub = makeWatch("type_url_foo", {"zz"}); - expectSendMessage("type_url_foo", {"zz", "z", "x", "y"}, ""); - grpc_mux_->resume("type_url_foo"); - grpc_mux_->pause("type_url_foo"); + expectSendMessage("foo", {"x", "y"}, "", true); + grpc_mux_->resume("foo"); + grpc_mux_->pause("bar"); + expectSendMessage("foo", {"z", "x", "y"}, ""); + auto foo_z_sub = grpc_mux_->subscribe("foo", {"z"}, callbacks_); + grpc_mux_->resume("bar"); + grpc_mux_->pause("foo"); + auto foo_zz_sub = grpc_mux_->subscribe("foo", {"zz"}, callbacks_); + expectSendMessage("foo", {"zz", "z", "x", "y"}, ""); + grpc_mux_->resume("foo"); + grpc_mux_->pause("foo"); } // Validate behavior when type URL mismatches occur. TEST_F(GrpcMuxImplTest, TypeUrlMismatch) { setup(); - auto invalid_response = std::make_unique(); - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x", "y"}); + std::unique_ptr invalid_response( + new envoy::api::v2::DiscoveryResponse()); + InSequence s; + auto foo_sub = grpc_mux_->subscribe("foo", {"x", "y"}, callbacks_); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); - expectSendMessage("type_url_foo", {"x", "y"}, "", true); + expectSendMessage("foo", {"x", "y"}, "", true); grpc_mux_->start(); { - auto response = std::make_unique(); - response->set_type_url("type_url_bar"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); + response->set_type_url("bar"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } { - invalid_response->set_type_url("type_url_foo"); - invalid_response->mutable_resources()->Add()->set_type_url("type_url_bar"); + invalid_response->set_type_url("foo"); + invalid_response->mutable_resources()->Add()->set_type_url("bar"); EXPECT_CALL(callbacks_, onConfigUpdateFailed(_, _)) .WillOnce(Invoke([](Envoy::Config::ConfigUpdateFailureReason, const EnvoyException* e) { - EXPECT_TRUE( - IsSubstring("", "", - "type URL type_url_bar embedded in an individual Any does not match the " - "message-wide type URL type_url_foo in DiscoveryResponse", - e->what())); + EXPECT_TRUE(IsSubstring( + "", "", "bar does not match the message-wide type URL foo in DiscoveryResponse", + e->what())); })); expectSendMessage( - "type_url_foo", {"x", "y"}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Internal, - fmt::format("type URL type_url_bar embedded in an individual Any does not match the " - "message-wide type URL type_url_foo in DiscoveryResponse {}", + "foo", {"x", "y"}, "", false, "", Grpc::Status::WellKnownGrpcStatus::Internal, + fmt::format("bar does not match the message-wide type URL foo in DiscoveryResponse {}", invalid_response->DebugString())); - grpc_mux_->onDiscoveryResponse(std::move(invalid_response)); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(invalid_response)); } - expectSendMessage("type_url_foo", {}, ""); + expectSendMessage("foo", {}, ""); } // Validate behavior when watches has an unknown resource name. TEST_F(GrpcMuxImplTest, WildcardWatch) { setup(); + InSequence s; const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; - FakeGrpcSubscription foo_sub = makeWatch(type_url, {}); + auto foo_sub = grpc_mux_->subscribe(type_url, {}, callbacks_); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); expectSendMessage(type_url, {}, "", true); grpc_mux_->start(); - auto response = std::make_unique(); - response->set_type_url(type_url); - response->set_version_info("1"); - envoy::api::v2::ClusterLoadAssignment load_assignment; - load_assignment.set_cluster_name("x"); - response->add_resources()->PackFrom(load_assignment); - EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) - .WillOnce( - Invoke([&load_assignment](const Protobuf::RepeatedPtrField& resources, - const std::string&) { - EXPECT_EQ(1, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); - EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); - })); - expectSendMessage(type_url, {}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + { + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); + response->set_type_url(type_url); + response->set_version_info("1"); + envoy::api::v2::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("x"); + response->add_resources()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) + .WillOnce( + Invoke([&load_assignment](const Protobuf::RepeatedPtrField& resources, + const std::string&) { + EXPECT_EQ(1, resources.size()); + envoy::api::v2::ClusterLoadAssignment expected_assignment; + resources[0].UnpackTo(&expected_assignment); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + })); + expectSendMessage(type_url, {}, "1"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); + } } // Validate behavior when watches specify resources (potentially overlapping). TEST_F(GrpcMuxImplTest, WatchDemux) { setup(); - // We will not require InSequence here: an update that causes multiple onConfigUpdates - // causes them in an indeterminate order, based on the whims of the hash map. + InSequence s; const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; - NiceMock> foo_callbacks; - FakeGrpcSubscription foo_sub = makeWatch(type_url, {"x", "y"}, foo_callbacks); - NiceMock> bar_callbacks; - FakeGrpcSubscription bar_sub = makeWatch(type_url, {"y", "z"}, bar_callbacks); + NiceMock foo_callbacks; + auto foo_sub = grpc_mux_->subscribe(type_url, {"x", "y"}, foo_callbacks); + NiceMock bar_callbacks; + auto bar_sub = grpc_mux_->subscribe(type_url, {"y", "z"}, bar_callbacks); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); // Should dedupe the "x" resource. expectSendMessage(type_url, {"y", "z", "x"}, "", true); grpc_mux_->start(); - // Send just x; only foo_callbacks should receive an onConfigUpdate(). { - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_type_url(type_url); response->set_version_info("1"); envoy::api::v2::ClusterLoadAssignment load_assignment; @@ -324,13 +301,12 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); })); expectSendMessage(type_url, {"y", "z", "x"}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } - // Send x y and z; foo_ and bar_callbacks should both receive onConfigUpdate()s, carrying {x,y} - // and {y,z} respectively. { - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_type_url(type_url); response->set_version_info("2"); envoy::api::v2::ClusterLoadAssignment load_assignment_x; @@ -365,7 +341,7 @@ TEST_F(GrpcMuxImplTest, WatchDemux) { EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment_y)); })); expectSendMessage(type_url, {"y", "z", "x"}, "2"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } expectSendMessage(type_url, {"x", "y"}, "2"); @@ -377,20 +353,21 @@ TEST_F(GrpcMuxImplTest, MultipleWatcherWithEmptyUpdates) { setup(); InSequence s; const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; - NiceMock> foo_callbacks; - FakeGrpcSubscription foo_sub = makeWatch(type_url, {"x", "y"}, foo_callbacks); + NiceMock foo_callbacks; + auto foo_sub = grpc_mux_->subscribe(type_url, {"x", "y"}, foo_callbacks); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); expectSendMessage(type_url, {"x", "y"}, "", true); grpc_mux_->start(); - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_type_url(type_url); response->set_version_info("1"); EXPECT_CALL(foo_callbacks, onConfigUpdate(_, "1")).Times(0); expectSendMessage(type_url, {"x", "y"}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); expectSendMessage(type_url, {}, "1"); } @@ -399,14 +376,15 @@ TEST_F(GrpcMuxImplTest, MultipleWatcherWithEmptyUpdates) { TEST_F(GrpcMuxImplTest, SingleWatcherWithEmptyUpdates) { setup(); const std::string& type_url = Config::TypeUrl::get().Cluster; - NiceMock> foo_callbacks; - FakeGrpcSubscription foo_sub = makeWatch(type_url, {}, foo_callbacks); + NiceMock foo_callbacks; + auto foo_sub = grpc_mux_->subscribe(type_url, {}, foo_callbacks); EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); expectSendMessage(type_url, {}, "", true); grpc_mux_->start(); - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_type_url(type_url); response->set_version_info("1"); // Validate that onConfigUpdate is called with empty resources. @@ -414,7 +392,7 @@ TEST_F(GrpcMuxImplTest, SingleWatcherWithEmptyUpdates) { .WillOnce(Invoke([](const Protobuf::RepeatedPtrField& resources, const std::string&) { EXPECT_TRUE(resources.empty()); })); expectSendMessage(type_url, {}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } // Exactly one test requires a mock time system to provoke behavior that cannot @@ -448,15 +426,15 @@ TEST_F(GrpcMuxImplTestWithMockTimeSystem, TooManyRequestsWithDefaultSettings) { for (uint64_t i = 0; i < burst; i++) { std::unique_ptr response( new envoy::api::v2::DiscoveryResponse()); - response->set_version_info("type_url_baz"); - response->set_nonce("type_url_bar"); - response->set_type_url("type_url_foo"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + response->set_version_info("baz"); + response->set_nonce("bar"); + response->set_type_url("foo"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } }; - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x"}); - expectSendMessage("type_url_foo", {"x"}, "", true); + auto foo_sub = grpc_mux_->subscribe("foo", {"x"}, callbacks_); + expectSendMessage("foo", {"x"}, "", true); grpc_mux_->start(); // Exhausts the limit. @@ -501,24 +479,23 @@ TEST_F(GrpcMuxImplTestWithMockTimeSystem, TooManyRequestsWithEmptyRateLimitSetti for (uint64_t i = 0; i < burst; i++) { std::unique_ptr response( new envoy::api::v2::DiscoveryResponse()); - response->set_version_info("type_url_baz"); - response->set_nonce("type_url_bar"); - response->set_type_url("type_url_foo"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + response->set_version_info("baz"); + response->set_nonce("bar"); + response->set_type_url("foo"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } }; - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x"}); - expectSendMessage("type_url_foo", {"x"}, "", true); + auto foo_sub = grpc_mux_->subscribe("foo", {"x"}, callbacks_); + expectSendMessage("foo", {"x"}, "", true); grpc_mux_->start(); // Validate that drain_request_timer is enabled when there are no tokens. - EXPECT_CALL(*drain_request_timer, enableTimer(std::chrono::milliseconds(100), _)) - .Times(AtLeast(1)); - onReceiveMessage(110); - EXPECT_LE(10, stats_.counter("control_plane.rate_limit_enforced").value()); - EXPECT_LE( - 10, + EXPECT_CALL(*drain_request_timer, enableTimer(std::chrono::milliseconds(100), _)); + onReceiveMessage(99); + EXPECT_EQ(1, stats_.counter("control_plane.rate_limit_enforced").value()); + EXPECT_EQ( + 1, stats_.gauge("control_plane.pending_requests", Stats::Gauge::ImportMode::Accumulate).value()); } @@ -558,15 +535,15 @@ TEST_F(GrpcMuxImplTest, TooManyRequestsWithCustomRateLimitSettings) { for (uint64_t i = 0; i < burst; i++) { std::unique_ptr response( new envoy::api::v2::DiscoveryResponse()); - response->set_version_info("type_url_baz"); - response->set_nonce("type_url_bar"); - response->set_type_url("type_url_foo"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + response->set_version_info("baz"); + response->set_nonce("bar"); + response->set_type_url("foo"); + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); } }; - FakeGrpcSubscription foo_sub = makeWatch("type_url_foo", {"x"}); - expectSendMessage("type_url_foo", {"x"}, "", true); + auto foo_sub = grpc_mux_->subscribe("foo", {"x"}, callbacks_); + expectSendMessage("foo", {"x"}, "", true); grpc_mux_->start(); // Validate that rate limit is not enforced for 100 requests. @@ -577,10 +554,10 @@ TEST_F(GrpcMuxImplTest, TooManyRequestsWithCustomRateLimitSettings) { EXPECT_CALL(*drain_request_timer, enableTimer(std::chrono::milliseconds(500), _)) .Times(AtLeast(1)); onReceiveMessage(160); - EXPECT_LE(10, stats_.counter("control_plane.rate_limit_enforced").value()); + EXPECT_EQ(12, stats_.counter("control_plane.rate_limit_enforced").value()); Stats::Gauge& pending_requests = stats_.gauge("control_plane.pending_requests", Stats::Gauge::ImportMode::Accumulate); - EXPECT_LE(10, pending_requests.value()); + EXPECT_EQ(12, pending_requests.value()); // Validate that drain requests call when there are multiple requests in queue. time_system_.setMonotonicTime(std::chrono::seconds(10)); @@ -590,7 +567,7 @@ TEST_F(GrpcMuxImplTest, TooManyRequestsWithCustomRateLimitSettings) { EXPECT_EQ(0, pending_requests.value()); } -// Verifies that a message with no resources is accepted. +// Verifies that a message with no resources is accepted. TEST_F(GrpcMuxImplTest, UnwatchedTypeAcceptsEmptyResources) { setup(); @@ -602,66 +579,71 @@ TEST_F(GrpcMuxImplTest, UnwatchedTypeAcceptsEmptyResources) { { // subscribe and unsubscribe to simulate a cluster added and removed expectSendMessage(type_url, {"y"}, "", true); - FakeGrpcSubscription temp_sub = makeWatch(type_url, {"y"}); + auto temp_sub = grpc_mux_->subscribe(type_url, {"y"}, callbacks_); expectSendMessage(type_url, {}, ""); } // simulate the server sending empty CLA message to notify envoy that the CLA was removed. - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_nonce("bar"); response->set_version_info("1"); response->set_type_url(type_url); - // Although the update will change nothing for us, we will "accept" it, and so according - // to the spec we should ACK it. - expectSendMessage(type_url, {}, "1", false, "bar"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + // TODO(fredlas) the expectation of no discovery request here is against the xDS spec. + // The upcoming xDS overhaul (part of/followup to PR7293) will fix this. + // + // This contains zero resources. No discovery request should be sent. + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response)); - // When we become interested in "x", we should send a request indicating that interest. + // when we add the new subscription version should be 1 and nonce should be bar expectSendMessage(type_url, {"x"}, "1", false, "bar"); - FakeGrpcSubscription sub = makeWatch(type_url, {"x"}); - // Watch destroyed -> interest gone -> unsubscribe request. + // simulate a new cluster x is added. add CLA subscription for it. + auto sub = grpc_mux_->subscribe(type_url, {"x"}, callbacks_); expectSendMessage(type_url, {}, "1", false, "bar"); } -// Verifies that a message with some resources is accepted even when there are no watches. -// Rationale: SotW gRPC xDS has always been willing to accept updates that include -// uninteresting resources. It should not matter whether those uninteresting resources -// are accompanied by interesting ones. -// Note: this was previously "rejects", not "accepts". See -// https://github.com/envoyproxy/envoy/pull/8350#discussion_r328218220 for discussion. -TEST_F(GrpcMuxImplTest, UnwatchedTypeAcceptsResources) { +// Verifies that a message with some resources is rejected when there are no watches. +TEST_F(GrpcMuxImplTest, UnwatchedTypeRejectsResources) { setup(); + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; - grpc_mux_->start(); - // subscribe and unsubscribe so that the type is known to envoy - { - expectSendMessage(type_url, {"y"}, "", true); - expectSendMessage(type_url, {}, ""); - FakeGrpcSubscription delete_immediately = makeWatch(type_url, {"y"}); - } - auto response = std::make_unique(); + grpc_mux_->start(); + // subscribe and unsubscribe (by not keeping the return watch) so that the type is known to envoy + expectSendMessage(type_url, {"y"}, "", true); + expectSendMessage(type_url, {}, ""); + grpc_mux_->subscribe(type_url, {"y"}, callbacks_); + + // simulate the server sending CLA message to notify envoy that the CLA was added, + // even though envoy doesn't expect it. Envoy should reject this update. + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); + response->set_nonce("bar"); + response->set_version_info("1"); response->set_type_url(type_url); + envoy::api::v2::ClusterLoadAssignment load_assignment; load_assignment.set_cluster_name("x"); response->add_resources()->PackFrom(load_assignment); - response->set_version_info("1"); - expectSendMessage(type_url, {}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); + // The message should be rejected. + expectSendMessage(type_url, {}, "", false, "bar"); + EXPECT_LOG_CONTAINS("warning", "Ignoring unwatched type URL " + type_url, + grpc_mux_->grpcStreamForTest().onReceiveMessage(std::move(response))); } TEST_F(GrpcMuxImplTest, BadLocalInfoEmptyClusterName) { EXPECT_CALL(local_info_, clusterName()).WillOnce(ReturnRef(EMPTY_STRING)); EXPECT_THROW_WITH_MESSAGE( - GrpcMuxSotw( - std::unique_ptr(async_client_), dispatcher_, + GrpcMuxImpl( + local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), - random_, stats_, rate_limit_settings_, local_info_, true), + random_, stats_, rate_limit_settings_, true), EnvoyException, "ads: node 'id' and 'cluster' are required. Set it either in 'node' config or via " "--service-node and --service-cluster options."); @@ -670,52 +652,15 @@ TEST_F(GrpcMuxImplTest, BadLocalInfoEmptyClusterName) { TEST_F(GrpcMuxImplTest, BadLocalInfoEmptyNodeName) { EXPECT_CALL(local_info_, nodeName()).WillOnce(ReturnRef(EMPTY_STRING)); EXPECT_THROW_WITH_MESSAGE( - GrpcMuxSotw( - std::unique_ptr(async_client_), dispatcher_, + GrpcMuxImpl( + local_info_, std::unique_ptr(async_client_), dispatcher_, *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), - random_, stats_, rate_limit_settings_, local_info_, true), + random_, stats_, rate_limit_settings_, true), EnvoyException, "ads: node 'id' and 'cluster' are required. Set it either in 'node' config or via " "--service-node and --service-cluster options."); } - -// Test that we simply ignore a message for an unknown type_url, with no ill effects. -TEST_F(GrpcMuxImplTest, DiscoveryResponseNonexistentSub) { - setup(); - - const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; - grpc_mux_->addOrUpdateWatch(type_url, nullptr, {}, callbacks_, std::chrono::milliseconds(0)); - - EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); - expectSendMessage(type_url, {}, "", true); - grpc_mux_->start(); - { - auto unexpected_response = std::make_unique(); - unexpected_response->set_type_url("unexpected_type_url"); - unexpected_response->set_version_info("0"); - EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "0")).Times(0); - grpc_mux_->onDiscoveryResponse(std::move(unexpected_response)); - } - auto response = std::make_unique(); - response->set_type_url(type_url); - response->set_version_info("1"); - envoy::api::v2::ClusterLoadAssignment load_assignment; - load_assignment.set_cluster_name("x"); - response->add_resources()->PackFrom(load_assignment); - EXPECT_CALL(callbacks_, onConfigUpdate(_, "1")) - .WillOnce( - Invoke([&load_assignment](const Protobuf::RepeatedPtrField& resources, - const std::string&) { - EXPECT_EQ(1, resources.size()); - envoy::api::v2::ClusterLoadAssignment expected_assignment; - resources[0].UnpackTo(&expected_assignment); - EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); - })); - expectSendMessage(type_url, {}, "1"); - grpc_mux_->onDiscoveryResponse(std::move(response)); -} - } // namespace } // namespace Config } // namespace Envoy diff --git a/test/common/config/grpc_subscription_impl_test.cc b/test/common/config/grpc_subscription_impl_test.cc index b16a7c3d17995..f7bed58385554 100644 --- a/test/common/config/grpc_subscription_impl_test.cc +++ b/test/common/config/grpc_subscription_impl_test.cc @@ -22,7 +22,7 @@ TEST_F(GrpcSubscriptionImplTest, StreamCreationFailure) { EXPECT_CALL(random_, random()); EXPECT_CALL(*timer_, enableTimer(_, _)); subscription_->start({"cluster0", "cluster1"}); - EXPECT_TRUE(statsAre(2, 0, 0, 0, 0, 0)); + EXPECT_TRUE(statsAre(2, 0, 0, 1, 0, 0)); // Ensure this doesn't cause an issue by sending a request, since we don't // have a gRPC stream. subscription_->updateResourceInterest({"cluster2"}); @@ -32,7 +32,7 @@ TEST_F(GrpcSubscriptionImplTest, StreamCreationFailure) { expectSendMessage({"cluster2"}, "", true); timer_cb_(); - EXPECT_TRUE(statsAre(3, 0, 0, 0, 0, 0)); + EXPECT_TRUE(statsAre(3, 0, 0, 1, 0, 0)); verifyControlPlaneStats(1); } @@ -46,10 +46,8 @@ TEST_F(GrpcSubscriptionImplTest, RemoteStreamClose) { .Times(0); EXPECT_CALL(*timer_, enableTimer(_, _)); EXPECT_CALL(random_, random()); - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get()) - ->grpcStreamForTest() - .onRemoteClose(Grpc::Status::WellKnownGrpcStatus::Canceled, ""); + subscription_->grpcMux()->grpcStreamForTest().onRemoteClose( + Grpc::Status::WellKnownGrpcStatus::Canceled, ""); EXPECT_TRUE(statsAre(2, 0, 0, 1, 0, 0)); verifyControlPlaneStats(0); @@ -60,7 +58,7 @@ TEST_F(GrpcSubscriptionImplTest, RemoteStreamClose) { EXPECT_TRUE(statsAre(2, 0, 0, 1, 0, 0)); } -// Validate that when the management server gets multiple requests for the same version, it can +// Validate that When the management server gets multiple requests for the same version, it can // ignore later ones. This allows the nonce to be used. TEST_F(GrpcSubscriptionImplTest, RepeatedNonce) { InSequence s; diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h index 1c425b50c7e11..476230d68cb58 100644 --- a/test/common/config/grpc_subscription_test_harness.h +++ b/test/common/config/grpc_subscription_test_harness.h @@ -36,22 +36,16 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { : method_descriptor_(Protobuf::DescriptorPool::generated_pool()->FindMethodByName( "envoy.api.v2.EndpointDiscoveryService.StreamEndpoints")), async_client_(new NiceMock()), timer_(new Event::MockTimer()) { - node_.set_id("node_name"); - node_.set_cluster("cluster_name"); - node_.mutable_locality()->set_zone("zone_name"); - EXPECT_CALL(local_info_, node()).WillRepeatedly(testing::ReturnRef(node_)); + node_.set_id("fo0"); + EXPECT_CALL(local_info_, node()).WillOnce(testing::ReturnRef(node_)); EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) { timer_cb_ = timer_cb; return timer_; })); - subscription_ = std::make_unique( - std::make_shared(std::unique_ptr(async_client_), - dispatcher_, *method_descriptor_, random_, stats_store_, - rate_limit_settings_, local_info_, - /*skip_subsequent_node=*/true), - Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, init_fetch_timeout, - /*is_aggregated=*/false); + local_info_, std::unique_ptr(async_client_), dispatcher_, random_, + *method_descriptor_, Config::TypeUrl::get().ClusterLoadAssignment, callbacks_, stats_, + stats_store_, rate_limit_settings_, init_fetch_timeout, true); } ~GrpcSubscriptionTestHarness() override { EXPECT_CALL(async_stream_, sendMessageRaw_(_, false)); } @@ -83,9 +77,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { error_detail->set_code(error_code); error_detail->set_message(error_message); } - EXPECT_CALL( - async_stream_, - sendMessageRaw_(Grpc::ProtoBufferEqIgnoreRepeatedFieldOrdering(expected_request), false)); + EXPECT_CALL(async_stream_, sendMessageRaw_(Grpc::ProtoBufferEq(expected_request), false)); } void startSubscription(const std::set& cluster_names) override { @@ -97,7 +89,8 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { void deliverConfigUpdate(const std::vector& cluster_names, const std::string& version, bool accept) override { - auto response = std::make_unique(); + std::unique_ptr response( + new envoy::api::v2::DiscoveryResponse()); response->set_version_info(version); last_response_nonce_ = std::to_string(HashUtil::xxHash64(version)); response->set_nonce(last_response_nonce_); @@ -122,19 +115,35 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { expectSendMessage(last_cluster_names_, version_, false, Grpc::Status::WellKnownGrpcStatus::Internal, "bad config"); } - auto shared_mux = subscription_->getGrpcMuxForTest(); - static_cast(shared_mux.get())->onDiscoveryResponse(std::move(response)); + subscription_->grpcMux()->onDiscoveryResponse(std::move(response)); Mock::VerifyAndClearExpectations(&async_stream_); } void updateResourceInterest(const std::set& cluster_names) override { + // The "watch" mechanism means that updates that lose interest in a resource + // will first generate a request for [still watched resources, i.e. without newly unwatched + // ones] before generating the request for all of cluster_names. + // TODO(fredlas) this unnecessary second request will stop happening once the watch mechanism is + // no longer internally used by GrpcSubscriptionImpl. + std::set both; + for (const auto& n : cluster_names) { + if (last_cluster_names_.find(n) != last_cluster_names_.end()) { + both.insert(n); + } + } + expectSendMessage(both, version_); expectSendMessage(cluster_names, version_); subscription_->updateResourceInterest(cluster_names); last_cluster_names_ = cluster_names; } void expectConfigUpdateFailed() override { - EXPECT_CALL(callbacks_, onConfigUpdateFailed(_, nullptr)); + EXPECT_CALL(callbacks_, onConfigUpdateFailed(_, nullptr)) + .WillOnce([this](ConfigUpdateFailureReason reason, const EnvoyException*) { + if (reason == ConfigUpdateFailureReason::FetchTimedout) { + stats_.init_fetch_timeout_.inc(); + } + }); } void expectEnableInitFetchTimeoutTimer(std::chrono::milliseconds timeout) override { diff --git a/test/common/config/new_grpc_mux_impl_test.cc b/test/common/config/new_grpc_mux_impl_test.cc new file mode 100644 index 0000000000000..57b1373dab22b --- /dev/null +++ b/test/common/config/new_grpc_mux_impl_test.cc @@ -0,0 +1,117 @@ +#include + +#include "envoy/api/v2/discovery.pb.h" +#include "envoy/api/v2/eds.pb.h" + +#include "common/common/empty_string.h" +#include "common/config/new_grpc_mux_impl.h" +#include "common/config/protobuf_link_hacks.h" +#include "common/config/resources.h" +#include "common/config/utility.h" +#include "common/protobuf/protobuf.h" +#include "common/stats/isolated_store_impl.h" + +#include "test/mocks/common.h" +#include "test/mocks/config/mocks.h" +#include "test/mocks/event/mocks.h" +#include "test/mocks/grpc/mocks.h" +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/runtime/mocks.h" +#include "test/test_common/logging.h" +#include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_time.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::_; +using testing::Invoke; +using testing::NiceMock; +using testing::Return; + +namespace Envoy { +namespace Config { +namespace { + +// We test some mux specific stuff below, other unit test coverage for singleton use of +// NewGrpcMuxImpl is provided in [grpc_]subscription_impl_test.cc. +class NewGrpcMuxImplTestBase : public testing::Test { +public: + NewGrpcMuxImplTestBase() + : async_client_(new Grpc::MockAsyncClient()), + control_plane_connected_state_( + stats_.gauge("control_plane.connected_state", Stats::Gauge::ImportMode::NeverImport)) {} + + void setup() { + grpc_mux_ = std::make_unique( + std::unique_ptr(async_client_), dispatcher_, + *Protobuf::DescriptorPool::generated_pool()->FindMethodByName( + "envoy.service.discovery.v2.AggregatedDiscoveryService.StreamAggregatedResources"), + random_, stats_, rate_limit_settings_, local_info_); + } + + NiceMock dispatcher_; + NiceMock random_; + Grpc::MockAsyncClient* async_client_; + NiceMock async_stream_; + std::unique_ptr grpc_mux_; + NiceMock> callbacks_; + NiceMock local_info_; + Stats::IsolatedStoreImpl stats_; + Envoy::Config::RateLimitSettings rate_limit_settings_; + Stats::Gauge& control_plane_connected_state_; +}; + +class NewGrpcMuxImplTest : public NewGrpcMuxImplTestBase { +public: + Event::SimulatedTimeSystem time_system_; +}; + +// TODO(fredlas) #8478 will delete this. +TEST_F(NewGrpcMuxImplTest, JustForCoverageTodoDelete) { + setup(); + EXPECT_TRUE(grpc_mux_->isDelta()); +} + +// Test that we simply ignore a message for an unknown type_url, with no ill effects. +TEST_F(NewGrpcMuxImplTest, DiscoveryResponseNonexistentSub) { + setup(); + + const std::string& type_url = Config::TypeUrl::get().ClusterLoadAssignment; + grpc_mux_->addOrUpdateWatch(type_url, nullptr, {}, callbacks_, std::chrono::milliseconds(0)); + + EXPECT_CALL(*async_client_, startRaw(_, _, _, _)).WillOnce(Return(&async_stream_)); + grpc_mux_->start(); + + { + auto unexpected_response = std::make_unique(); + unexpected_response->set_type_url(type_url); + unexpected_response->set_system_version_info("0"); + EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "0")).Times(0); + grpc_mux_->onDiscoveryResponse(std::move(unexpected_response)); + } + { + auto response = std::make_unique(); + response->set_type_url(type_url); + response->set_system_version_info("1"); + envoy::api::v2::ClusterLoadAssignment load_assignment; + load_assignment.set_cluster_name("x"); + response->add_resources()->mutable_resource()->PackFrom(load_assignment); + EXPECT_CALL(callbacks_, onConfigUpdate(_, _, "1")) + .WillOnce( + Invoke([&load_assignment]( + const Protobuf::RepeatedPtrField& added_resources, + const Protobuf::RepeatedPtrField&, const std::string&) { + EXPECT_EQ(1, added_resources.size()); + envoy::api::v2::ClusterLoadAssignment expected_assignment; + added_resources[0].resource().UnpackTo(&expected_assignment); + EXPECT_TRUE(TestUtility::protoEqual(expected_assignment, load_assignment)); + })); + grpc_mux_->onDiscoveryResponse(std::move(response)); + } +} + +} // namespace +} // namespace Config +} // namespace Envoy diff --git a/test/common/config/sotw_subscription_state_test.cc b/test/common/config/sotw_subscription_state_test.cc deleted file mode 100644 index 2e962598299ee..0000000000000 --- a/test/common/config/sotw_subscription_state_test.cc +++ /dev/null @@ -1,183 +0,0 @@ -#include "common/config/sotw_subscription_state.h" -#include "common/config/utility.h" -#include "common/stats/isolated_store_impl.h" - -#include "test/mocks/config/mocks.h" -#include "test/mocks/event/mocks.h" -#include "test/mocks/local_info/mocks.h" - -#include "gmock/gmock.h" -#include "gtest/gtest.h" - -using testing::NiceMock; -using testing::Throw; -using testing::UnorderedElementsAre; - -namespace Envoy { -namespace Config { -namespace { - -const char TypeUrl[] = "type.googleapis.com/envoy.api.v2.ClusterLoadAssignment"; - -class SotwSubscriptionStateTest : public testing::Test { -protected: - SotwSubscriptionStateTest() - : state_(TypeUrl, callbacks_, std::chrono::milliseconds(0U), dispatcher_) { - state_.updateSubscriptionInterest({"name1", "name2", "name3"}, {}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name1", "name2", "name3")); - } - - std::unique_ptr getNextDiscoveryRequestAckless() { - auto* ptr = static_cast(state_.getNextRequestAckless()); - return std::unique_ptr(ptr); - } - - UpdateAck deliverDiscoveryResponse(const std::vector& resource_names, - const std::string& version_info, const std::string& nonce) { - envoy::api::v2::DiscoveryResponse response; - response.set_version_info(version_info); - response.set_nonce(nonce); - Protobuf::RepeatedPtrField typed_resources; - for (const auto& resource_name : resource_names) { - envoy::api::v2::ClusterLoadAssignment* load_assignment = typed_resources.Add(); - load_assignment->set_cluster_name(resource_name); - response.add_resources()->PackFrom(*load_assignment); - } - EXPECT_CALL(callbacks_, onConfigUpdate(_, version_info)); - return state_.handleResponse(&response); - } - - UpdateAck deliverBadDiscoveryResponse(const std::string& version_info, const std::string& nonce) { - envoy::api::v2::DiscoveryResponse message; - message.set_version_info(version_info); - message.set_nonce(nonce); - EXPECT_CALL(callbacks_, onConfigUpdate(_, _)).WillOnce(Throw(EnvoyException("oh no"))); - return state_.handleResponse(&message); - } - - NiceMock> callbacks_; - NiceMock dispatcher_; - // We start out interested in three resources: name1, name2, and name3. - SotwSubscriptionState state_; -}; - -// Basic gaining/losing interest in resources should lead to (un)subscriptions. -TEST_F(SotwSubscriptionStateTest, SubscribeAndUnsubscribe) { - { - state_.updateSubscriptionInterest({"name4"}, {"name1"}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name2", "name3", "name4")); - } - { - state_.updateSubscriptionInterest({"name1"}, {"name3", "name4"}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name1", "name2")); - } -} - -// Unlike delta, if SotW gets multiple interest updates before being able to send a request, they -// all collapse to a single update. However, even if the updates all cancel each other out, there -// still will be a request generated. All of the following tests explore different such cases. -TEST_F(SotwSubscriptionStateTest, RemoveThenAdd) { - state_.updateSubscriptionInterest({}, {"name3"}); - state_.updateSubscriptionInterest({"name3"}, {}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name1", "name2", "name3")); -} - -TEST_F(SotwSubscriptionStateTest, AddThenRemove) { - state_.updateSubscriptionInterest({"name4"}, {}); - state_.updateSubscriptionInterest({}, {"name4"}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name1", "name2", "name3")); -} - -TEST_F(SotwSubscriptionStateTest, AddRemoveAdd) { - state_.updateSubscriptionInterest({"name4"}, {}); - state_.updateSubscriptionInterest({}, {"name4"}); - state_.updateSubscriptionInterest({"name4"}, {}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), - UnorderedElementsAre("name1", "name2", "name3", "name4")); -} - -TEST_F(SotwSubscriptionStateTest, RemoveAddRemove) { - state_.updateSubscriptionInterest({}, {"name3"}); - state_.updateSubscriptionInterest({"name3"}, {}); - state_.updateSubscriptionInterest({}, {"name3"}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name1", "name2")); -} - -TEST_F(SotwSubscriptionStateTest, BothAddAndRemove) { - state_.updateSubscriptionInterest({"name4"}, {"name1", "name2", "name3"}); - state_.updateSubscriptionInterest({"name1", "name2", "name3"}, {"name4"}); - state_.updateSubscriptionInterest({"name4"}, {"name1", "name2", "name3"}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), UnorderedElementsAre("name4")); -} - -TEST_F(SotwSubscriptionStateTest, CumulativeUpdates) { - state_.updateSubscriptionInterest({"name4"}, {}); - state_.updateSubscriptionInterest({"name5"}, {}); - auto cur_request = getNextDiscoveryRequestAckless(); - EXPECT_THAT(cur_request->resource_names(), - UnorderedElementsAre("name1", "name2", "name3", "name4", "name5")); -} - -// Verifies that a sequence of good and bad responses from the server all get the appropriate -// ACKs/NACKs from Envoy. -TEST_F(SotwSubscriptionStateTest, AckGenerated) { - // The xDS server's first response includes items for name1 and 2, but not 3. - { - UpdateAck ack = deliverDiscoveryResponse({"name1", "name2"}, "version1", "nonce1"); - EXPECT_EQ("nonce1", ack.nonce_); - EXPECT_EQ(Grpc::Status::WellKnownGrpcStatus::Ok, ack.error_detail_.code()); - } - // The next response updates 1 and 2, and adds 3. - { - UpdateAck ack = deliverDiscoveryResponse({"name1", "name2", "name3"}, "version2", "nonce2"); - EXPECT_EQ("nonce2", ack.nonce_); - EXPECT_EQ(Grpc::Status::WellKnownGrpcStatus::Ok, ack.error_detail_.code()); - } - // The next response tries but fails to update all 3, and so should produce a NACK. - { - UpdateAck ack = deliverBadDiscoveryResponse("version3", "nonce3"); - EXPECT_EQ("nonce3", ack.nonce_); - EXPECT_NE(Grpc::Status::WellKnownGrpcStatus::Ok, ack.error_detail_.code()); - } - // The last response successfully updates all 3. - { - UpdateAck ack = deliverDiscoveryResponse({"name1", "name2", "name3"}, "version4", "nonce4"); - EXPECT_EQ("nonce4", ack.nonce_); - EXPECT_EQ(Grpc::Status::WellKnownGrpcStatus::Ok, ack.error_detail_.code()); - } -} - -TEST_F(SotwSubscriptionStateTest, CheckUpdatePending) { - // Note that the test fixture ctor causes the first request to be "sent", so we start in the - // middle of a stream, with our initially interested resources having been requested already. - EXPECT_FALSE(state_.subscriptionUpdatePending()); - state_.updateSubscriptionInterest({}, {}); // no change - EXPECT_FALSE(state_.subscriptionUpdatePending()); - state_.markStreamFresh(); - EXPECT_TRUE(state_.subscriptionUpdatePending()); // no change, BUT fresh stream - state_.updateSubscriptionInterest({}, {"name3"}); // one removed - EXPECT_TRUE(state_.subscriptionUpdatePending()); - state_.updateSubscriptionInterest({"name3"}, {}); // one added - EXPECT_TRUE(state_.subscriptionUpdatePending()); -} - -TEST_F(SotwSubscriptionStateTest, handleEstablishmentFailure) { - // Although establishment failure is not supposed to cause an onConfigUpdateFailed() on the - // ultimate actual subscription callbacks, the callbacks reference held is actually to - // the WatchMap, which then calls GrpcSubscriptionImpl(s). It is the GrpcSubscriptionImpl - // that will decline to pass on an onConfigUpdateFailed(ConnectionFailure). - EXPECT_CALL(callbacks_, onConfigUpdateFailed(_, _)); - state_.handleEstablishmentFailure(); -} - -} // namespace -} // namespace Config -} // namespace Envoy diff --git a/test/common/config/subscription_impl_test.cc b/test/common/config/subscription_impl_test.cc index 66c37b7c17694..45646de9b5315 100644 --- a/test/common/config/subscription_impl_test.cc +++ b/test/common/config/subscription_impl_test.cc @@ -150,17 +150,22 @@ TEST_P(SubscriptionImplInitFetchTimeoutTest, InitialFetchTimeout) { if (GetParam() == SubscriptionType::Filesystem) { return; // initial_fetch_timeout not implemented for filesystem. } + InSequence s; expectEnableInitFetchTimeoutTimer(std::chrono::milliseconds(1000)); startSubscription({"cluster0", "cluster1"}); - statsAre(1, 0, 0, 0, 0, 0); + EXPECT_TRUE(statsAre(1, 0, 0, 0, 0, 0)); + if (GetParam() == SubscriptionType::Http) { + expectDisableInitFetchTimeoutTimer(); + } expectConfigUpdateFailed(); - expectDisableInitFetchTimeoutTimer(); + callInitFetchTimeoutCb(); EXPECT_TRUE(statsAre(1, 0, 0, 0, 1, 0)); } // Validate that initial fetch timer is disabled on config update TEST_P(SubscriptionImplInitFetchTimeoutTest, DisableInitTimeoutOnSuccess) { + InSequence s; expectEnableInitFetchTimeoutTimer(std::chrono::milliseconds(1000)); startSubscription({"cluster0", "cluster1"}); EXPECT_TRUE(statsAre(1, 0, 0, 0, 0, 0)); @@ -170,6 +175,7 @@ TEST_P(SubscriptionImplInitFetchTimeoutTest, DisableInitTimeoutOnSuccess) { // Validate that initial fetch timer is disabled on config update failed TEST_P(SubscriptionImplInitFetchTimeoutTest, DisableInitTimeoutOnFail) { + InSequence s; expectEnableInitFetchTimeoutTimer(std::chrono::milliseconds(1000)); startSubscription({"cluster0", "cluster1"}); EXPECT_TRUE(statsAre(1, 0, 0, 0, 0, 0)); diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index b7f0214339bef..fe6b5e39ab603 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -177,6 +177,54 @@ TEST_F(CdsApiImplTest, ConfigUpdateWith2ValidClusters) { cds_callbacks_->onConfigUpdate(clusters, ""); } +TEST_F(CdsApiImplTest, DeltaConfigUpdate) { + { + InSequence s; + setup(); + } + EXPECT_CALL(initialized_, ready()); + + { + Protobuf::RepeatedPtrField resources; + { + envoy::api::v2::Cluster cluster; + cluster.set_name("cluster_1"); + expectAdd("cluster_1", "v1"); + auto* resource = resources.Add(); + resource->mutable_resource()->PackFrom(cluster); + resource->set_name("cluster_1"); + resource->set_version("v1"); + } + { + envoy::api::v2::Cluster cluster; + cluster.set_name("cluster_2"); + expectAdd("cluster_2", "v1"); + auto* resource = resources.Add(); + resource->mutable_resource()->PackFrom(cluster); + resource->set_name("cluster_2"); + resource->set_version("v1"); + } + cds_callbacks_->onConfigUpdate(resources, {}, "v1"); + } + + { + Protobuf::RepeatedPtrField resources; + { + envoy::api::v2::Cluster cluster; + cluster.set_name("cluster_3"); + expectAdd("cluster_3", "v2"); + auto* resource = resources.Add(); + resource->mutable_resource()->PackFrom(cluster); + resource->set_name("cluster_3"); + resource->set_version("v2"); + } + Protobuf::RepeatedPtrField removed; + *removed.Add() = "cluster_1"; + EXPECT_CALL(cm_, removeCluster(StrEq("cluster_1"))).WillOnce(Return(true)); + cds_callbacks_->onConfigUpdate(resources, removed, "v2"); + } +} + TEST_F(CdsApiImplTest, ConfigUpdateAddsSecondClusterEvenIfFirstThrows) { { InSequence s; diff --git a/test/config/utility.cc b/test/config/utility.cc index 7538ffd9e085c..17804e22de839 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -162,6 +162,8 @@ name: envoy.squash nanos: 0 )EOF"; +// TODO(fredlas) set_node_on_first_message_only was true; the delta+SotW unification +// work restores it here. // TODO(#6327) cleaner approach to testing with static config. std::string ConfigHelper::discoveredClustersBootstrap(const std::string& api_type) { return fmt::format( @@ -179,7 +181,7 @@ std::string ConfigHelper::discoveredClustersBootstrap(const std::string& api_typ grpc_services: envoy_grpc: cluster_name: my_cds_cluster - set_node_on_first_message_only: true + set_node_on_first_message_only: false static_resources: clusters: - name: my_cds_cluster diff --git a/test/integration/ads_integration.h b/test/integration/ads_integration.h index c806cd8107e70..31956bd6bb126 100644 --- a/test/integration/ads_integration.h +++ b/test/integration/ads_integration.h @@ -12,6 +12,8 @@ #include "test/common/grpc/grpc_client_integration.h" #include "test/integration/http_integration.h" +// TODO(fredlas) set_node_on_first_message_only was true; the delta+SotW unification +// work restores it here. namespace Envoy { static std::string AdsIntegrationConfig(const std::string& api_type) { // Note: do not use CONSTRUCT_ON_FIRST_USE here! @@ -23,7 +25,7 @@ static std::string AdsIntegrationConfig(const std::string& api_type) { ads: {{}} ads_config: api_type: {} - set_node_on_first_message_only: true + set_node_on_first_message_only: false static_resources: clusters: name: dummy_cluster diff --git a/test/integration/ads_integration_test.cc b/test/integration/ads_integration_test.cc index 6842ae9514707..56e7b33be962b 100644 --- a/test/integration/ads_integration_test.cc +++ b/test/integration/ads_integration_test.cc @@ -55,7 +55,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest( - Config::TypeUrl::get().Cluster, "", {}, {}, {}, false, + Config::TypeUrl::get().Cluster, "", {}, {}, {}, true, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().Cluster))); sendDiscoveryResponse(Config::TypeUrl::get().Cluster, @@ -70,7 +70,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "1", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "", - {"cluster_0"}, {}, {}, false, + {"cluster_0"}, {}, {}, true, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().ClusterLoadAssignment))); @@ -85,7 +85,7 @@ TEST_P(AdsIntegrationTest, Failure) { {buildRouteConfig("listener_0", "route_config_0")}, {}, "1"); EXPECT_TRUE(compareDiscoveryRequest( - Config::TypeUrl::get().Listener, "", {}, {}, {}, false, + Config::TypeUrl::get().Listener, "", {}, {}, {}, true, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().Listener))); sendDiscoveryResponse( @@ -100,7 +100,7 @@ TEST_P(AdsIntegrationTest, Failure) { EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Listener, "1", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().RouteConfiguration, "", - {"route_config_0"}, {}, {}, false, + {"route_config_0"}, {}, {}, true, Grpc::Status::WellKnownGrpcStatus::Internal, fmt::format("does not match the message-wide type URL {}", Config::TypeUrl::get().RouteConfiguration))); @@ -364,11 +364,12 @@ TEST_P(AdsIntegrationTest, CdsPausedDuringWarming) { test_server_->server().clusterManager().adsMux()->paused(Config::TypeUrl::get().Cluster)); // CDS is resumed and EDS response was acknowledged. - // - // Envoy will ACK both Cluster messages. Since they arrived while CDS was paused, they aren't - // sent until CDS is unpaused. Since version 3 has already arrived by the time the version 2 - // ACK goes out, they're both acknowledging version 3. - EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "3", {}, {}, {})); + if (sotw_or_delta_ == Grpc::SotwOrDelta::Delta) { + // Envoy will ACK both Cluster messages. Since they arrived while CDS was paused, they aren't + // sent until CDS is unpaused. Since version 3 has already arrived by the time the version 2 + // ACK goes out, they're both acknowledging version 3. + EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "3", {}, {}, {})); + } EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "3", {}, {}, {})); EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().ClusterLoadAssignment, "2", {"warming_cluster_2", "warming_cluster_1"}, {}, {})); diff --git a/test/integration/integration.cc b/test/integration/integration.cc index 40d4a48cef532..79c2cd7ae0eac 100644 --- a/test/integration/integration.cc +++ b/test/integration/integration.cc @@ -568,27 +568,11 @@ AssertionResult BaseIntegrationTest::compareDiscoveryRequest( expect_node, expected_error_code, expected_error_substring); } else { return compareDeltaDiscoveryRequest(expected_type_url, expected_resource_names_added, - expected_resource_names_removed, expect_node, - expected_error_code, expected_error_substring); + expected_resource_names_removed, expected_error_code, + expected_error_substring); } } -AssertionResult compareSets(const std::set& set1, const std::set& set2, - absl::string_view name) { - if (set1 == set2) { - return AssertionSuccess(); - } - auto failure = AssertionFailure() << name << " field not as expected.\nExpected: {"; - for (const auto& x : set1) { - failure << x << ", "; - } - failure << "}\nActual: {"; - for (const auto& x : set2) { - failure << x << ", "; - } - return failure << "}"; -} - AssertionResult BaseIntegrationTest::compareSotwDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, const std::vector& expected_resource_names, bool expect_node, @@ -600,58 +584,64 @@ AssertionResult BaseIntegrationTest::compareSotwDiscoveryRequest( EXPECT_TRUE(discovery_request.has_node()); EXPECT_FALSE(discovery_request.node().id().empty()); EXPECT_FALSE(discovery_request.node().cluster().empty()); + } else { + EXPECT_FALSE(discovery_request.has_node()); } if (expected_type_url != discovery_request.type_url()) { return AssertionFailure() << fmt::format("type_url {} does not match expected {}", discovery_request.type_url(), expected_type_url); } - - // Sort to ignore ordering. - std::set expected_sub{expected_resource_names.begin(), - expected_resource_names.end()}; - std::set actual_sub{discovery_request.resource_names().cbegin(), - discovery_request.resource_names().cend()}; - auto sub_result = compareSets(expected_sub, actual_sub, "resource_names"); - if (!sub_result) { - return sub_result; + if (!(expected_error_code == discovery_request.error_detail().code())) { + return AssertionFailure() << fmt::format("error_code {} does not match expected {}", + discovery_request.error_detail().code(), + expected_error_code); + } + EXPECT_TRUE( + IsSubstring("", "", expected_error_substring, discovery_request.error_detail().message())); + const std::vector resource_names(discovery_request.resource_names().cbegin(), + discovery_request.resource_names().cend()); + if (expected_resource_names != resource_names) { + return AssertionFailure() << fmt::format( + "resources {} do not match expected {} in {}", absl::StrJoin(resource_names, ","), + absl::StrJoin(expected_resource_names, ","), discovery_request.DebugString()); } if (expected_version != discovery_request.version_info()) { return AssertionFailure() << fmt::format("version {} does not match expected {} in {}", discovery_request.version_info(), expected_version, discovery_request.DebugString()); } - if (discovery_request.error_detail().code() != expected_error_code) { - return AssertionFailure() << fmt::format( - "error code {} does not match expected {}. (Error message is {}).", - discovery_request.error_detail().code(), expected_error_code, - discovery_request.error_detail().message()); + return AssertionSuccess(); +} + +AssertionResult compareSets(const std::set& set1, const std::set& set2, + absl::string_view name) { + if (set1 == set2) { + return AssertionSuccess(); } - if (expected_error_code != Grpc::Status::WellKnownGrpcStatus::Ok && - discovery_request.error_detail().message().find(expected_error_substring) == - std::string::npos) { - return AssertionFailure() << "\"" << expected_error_substring - << "\" is not a substring of actual error message \"" - << discovery_request.error_detail().message() << "\""; + auto failure = AssertionFailure() << name << " field not as expected.\nExpected: {"; + for (const auto& x : set1) { + failure << x << ", "; } - return AssertionSuccess(); + failure << "}\nActual: {"; + for (const auto& x : set2) { + failure << x << ", "; + } + return failure << "}"; } AssertionResult BaseIntegrationTest::compareDeltaDiscoveryRequest( const std::string& expected_type_url, const std::vector& expected_resource_subscriptions, const std::vector& expected_resource_unsubscriptions, FakeStreamPtr& xds_stream, - bool expect_node, const Protobuf::int32 expected_error_code, - const std::string& expected_error_substring) { + const Protobuf::int32 expected_error_code, const std::string& expected_error_substring) { envoy::api::v2::DeltaDiscoveryRequest request; VERIFY_ASSERTION(xds_stream->waitForGrpcMessage(*dispatcher_, request)); - if (expect_node) { - EXPECT_TRUE(request.has_node()); - EXPECT_FALSE(request.node().id().empty()); - EXPECT_FALSE(request.node().cluster().empty()); + // Verify all we care about node. + if (!request.has_node() || request.node().id().empty() || request.node().cluster().empty()) { + return AssertionFailure() << "Weird node field"; } - if (request.type_url() != expected_type_url) { return AssertionFailure() << fmt::format("type_url {} does not match expected {}.", request.type_url(), expected_type_url); @@ -665,11 +655,12 @@ AssertionResult BaseIntegrationTest::compareDeltaDiscoveryRequest( request.resource_names_subscribe().end()}; std::set actual_unsub{request.resource_names_unsubscribe().begin(), request.resource_names_unsubscribe().end()}; - auto sub_result = compareSets(expected_sub, actual_sub, "resource_names_subscribe"); + auto sub_result = compareSets(expected_sub, actual_sub, "expected_resource_subscriptions"); if (!sub_result) { return sub_result; } - auto unsub_result = compareSets(expected_unsub, actual_unsub, "resource_names_unsubscribe"); + auto unsub_result = + compareSets(expected_unsub, actual_unsub, "expected_resource_unsubscriptions"); if (!unsub_result) { return unsub_result; } diff --git a/test/integration/integration.h b/test/integration/integration.h index 8c07b6499b7ce..66461a88a1c72 100644 --- a/test/integration/integration.h +++ b/test/integration/integration.h @@ -220,13 +220,15 @@ class BaseIntegrationTest : Logger::Loggable { // sending/receiving to/from the (imaginary) xDS server. You should almost always use // compareDiscoveryRequest() and sendDiscoveryResponse(), but the SotW/delta-specific versions are // available if you're writing a SotW/delta-specific test. + // TODO(fredlas) expect_node was defaulting false here; the delta+SotW unification work restores + // it. AssertionResult compareDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, const std::vector& expected_resource_names, const std::vector& expected_resource_names_added, - const std::vector& expected_resource_names_removed, bool expect_node = false, + const std::vector& expected_resource_names_removed, bool expect_node = true, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& expected_error_substring = ""); + const std::string& expected_error_message = ""); template void sendDiscoveryResponse(const std::string& type_url, const std::vector& state_of_the_world, const std::vector& added_or_updated, @@ -241,26 +243,28 @@ class BaseIntegrationTest : Logger::Loggable { AssertionResult compareDeltaDiscoveryRequest( const std::string& expected_type_url, const std::vector& expected_resource_subscriptions, - const std::vector& expected_resource_unsubscriptions, bool expect_node = false, + const std::vector& expected_resource_unsubscriptions, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& expected_error_substring = "") { + const std::string& expected_error_message = "") { return compareDeltaDiscoveryRequest(expected_type_url, expected_resource_subscriptions, - expected_resource_unsubscriptions, xds_stream_, expect_node, - expected_error_code, expected_error_substring); + expected_resource_unsubscriptions, xds_stream_, + expected_error_code, expected_error_message); } AssertionResult compareDeltaDiscoveryRequest( const std::string& expected_type_url, const std::vector& expected_resource_subscriptions, const std::vector& expected_resource_unsubscriptions, FakeStreamPtr& stream, - bool expect_node = false, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& expected_error_substring = ""); + const std::string& expected_error_message = ""); + + // TODO(fredlas) expect_node was defaulting false here; the delta+SotW unification work restores + // it. AssertionResult compareSotwDiscoveryRequest( const std::string& expected_type_url, const std::string& expected_version, - const std::vector& expected_resource_names, bool expect_node = false, + const std::vector& expected_resource_names, bool expect_node = true, const Protobuf::int32 expected_error_code = Grpc::Status::WellKnownGrpcStatus::Ok, - const std::string& expected_error_substring = ""); + const std::string& expected_error_message = ""); template void sendSotwDiscoveryResponse(const std::string& type_url, const std::vector& messages, diff --git a/test/integration/rtds_integration_test.cc b/test/integration/rtds_integration_test.cc index 3ffcc5cf54a5c..6f223f1a626dc 100644 --- a/test/integration/rtds_integration_test.cc +++ b/test/integration/rtds_integration_test.cc @@ -6,6 +6,8 @@ namespace Envoy { namespace { +// TODO(fredlas) set_node_on_first_message_only was true; the delta+SotW unification +// work restores it here. std::string tdsBootstrapConfig(absl::string_view api_type) { return fmt::format(R"EOF( static_resources: @@ -36,7 +38,7 @@ std::string tdsBootstrapConfig(absl::string_view api_type) { grpc_services: envoy_grpc: cluster_name: rtds_cluster - set_node_on_first_message_only: true + set_node_on_first_message_only: false - name: some_admin_layer admin_layer: {{}} admin: diff --git a/test/integration/scoped_rds_integration_test.cc b/test/integration/scoped_rds_integration_test.cc index 484c07239964a..3caca65ec896b 100644 --- a/test/integration/scoped_rds_integration_test.cc +++ b/test/integration/scoped_rds_integration_test.cc @@ -82,7 +82,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, scoped_routes->mutable_scoped_rds() ->mutable_scoped_rds_config_source() ->mutable_api_config_source(); - if (sotwOrDelta() == Grpc::SotwOrDelta::Delta) { + if (isDelta()) { srds_api_config_source->set_api_type(envoy::api::v2::core::ApiConfigSource::DELTA_GRPC); } else { srds_api_config_source->set_api_type(envoy::api::v2::core::ApiConfigSource::GRPC); @@ -169,7 +169,7 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, const std::vector& to_add_list, const std::vector& to_delete_list, const std::string& version) { - if (sotwOrDelta() == Grpc::SotwOrDelta::Delta) { + if (isDelta()) { sendDeltaScopedRdsResponse(to_add_list, to_delete_list, version); } else { sendSotwScopedRdsResponse(sotw_list, version); @@ -217,6 +217,8 @@ class ScopedRdsIntegrationTest : public HttpIntegrationTest, response); } + bool isDelta() { return sotwOrDelta() == Grpc::SotwOrDelta::Delta; } + const std::string srds_config_name_{"foo-scoped-routes"}; FakeUpstreamInfo scoped_rds_upstream_info_; FakeUpstreamInfo rds_upstream_info_; @@ -283,7 +285,7 @@ route_configuration_name: {} } test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_attempt", // update_attempt only increase after a response - sotwOrDelta() == Grpc::SotwOrDelta::Delta ? 1 : 2); + isDelta() ? 1 : 2); test_server_->waitForCounterGe("http.config_test.scoped_rds.foo-scoped-routes.update_success", 1); // The version gauge should be set to xxHash64("1"). test_server_->waitForGaugeEq("http.config_test.scoped_rds.foo-scoped-routes.version", diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index 19b4a3ec0d98d..4a3e3099e0c15 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -25,11 +25,27 @@ MockSubscriptionFactory::MockSubscriptionFactory() { MockSubscriptionFactory::~MockSubscriptionFactory() = default; +MockGrpcMuxWatch::MockGrpcMuxWatch() = default; +MockGrpcMuxWatch::~MockGrpcMuxWatch() { cancel(); } + MockGrpcMux::MockGrpcMux() = default; MockGrpcMux::~MockGrpcMux() = default; MockGrpcStreamCallbacks::MockGrpcStreamCallbacks() = default; MockGrpcStreamCallbacks::~MockGrpcStreamCallbacks() = default; +GrpcMuxWatchPtr MockGrpcMux::subscribe(const std::string& type_url, + const std::set& resources, + GrpcMuxCallbacks& callbacks) { + return GrpcMuxWatchPtr(subscribe_(type_url, resources, callbacks)); +} + +MockGrpcMuxCallbacks::MockGrpcMuxCallbacks() { + ON_CALL(*this, resourceName(testing::_)) + .WillByDefault(testing::Invoke(TestUtility::xdsResourceName)); +} + +MockGrpcMuxCallbacks::~MockGrpcMuxCallbacks() = default; + } // namespace Config } // namespace Envoy diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index ee4bae564f761..7fdf475521970 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -62,21 +62,53 @@ class MockSubscriptionFactory : public SubscriptionFactory { SubscriptionCallbacks* callbacks_{}; }; +class MockGrpcMuxWatch : public GrpcMuxWatch { +public: + MockGrpcMuxWatch(); + ~MockGrpcMuxWatch() override; + + MOCK_METHOD0(cancel, void()); +}; + class MockGrpcMux : public GrpcMux { public: MockGrpcMux(); ~MockGrpcMux() override; MOCK_METHOD0(start, void()); + MOCK_METHOD3(subscribe_, + GrpcMuxWatch*(const std::string& type_url, const std::set& resources, + GrpcMuxCallbacks& callbacks)); + GrpcMuxWatchPtr subscribe(const std::string& type_url, const std::set& resources, + GrpcMuxCallbacks& callbacks) override; + MOCK_METHOD1(pause, void(const std::string& type_url)); + MOCK_METHOD1(resume, void(const std::string& type_url)); + MOCK_CONST_METHOD1(paused, bool(const std::string& type_url)); + + MOCK_METHOD5(addSubscription, + void(const std::set& resources, const std::string& type_url, + SubscriptionCallbacks& callbacks, SubscriptionStats& stats, + std::chrono::milliseconds init_fetch_timeout)); + MOCK_METHOD2(updateResourceInterest, + void(const std::set& resources, const std::string& type_url)); + MOCK_METHOD5(addOrUpdateWatch, Watch*(const std::string& type_url, Watch* watch, const std::set& resources, SubscriptionCallbacks& callbacks, std::chrono::milliseconds init_fetch_timeout)); MOCK_METHOD2(removeWatch, void(const std::string& type_url, Watch* watch)); - MOCK_METHOD1(pause, void(const std::string& type_url)); - MOCK_METHOD1(resume, void(const std::string& type_url)); - MOCK_CONST_METHOD1(paused, bool(const std::string& type_url)); - MOCK_METHOD0(disableInitFetchTimeoutTimer, void()); +}; + +class MockGrpcMuxCallbacks : public GrpcMuxCallbacks { +public: + MockGrpcMuxCallbacks(); + ~MockGrpcMuxCallbacks() override; + + MOCK_METHOD2(onConfigUpdate, void(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info)); + MOCK_METHOD2(onConfigUpdateFailed, + void(Envoy::Config::ConfigUpdateFailureReason reason, const EnvoyException* e)); + MOCK_METHOD1(resourceName, std::string(const ProtobufWkt::Any& resource)); }; class MockGrpcStreamCallbacks : public GrpcStreamCallbacks { diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index ffa81364045ed..e5843f0bc64b6 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -139,7 +139,6 @@ MockClusterManager::MockClusterManager() { ON_CALL(*this, httpAsyncClientForCluster(_)).WillByDefault(ReturnRef(async_client_)); ON_CALL(*this, httpAsyncClientForCluster(_)).WillByDefault((ReturnRef(async_client_))); ON_CALL(*this, bindConfig()).WillByDefault(ReturnRef(bind_config_)); - ads_mux_ = std::make_shared>(); ON_CALL(*this, adsMux()).WillByDefault(Return(ads_mux_)); ON_CALL(*this, grpcAsyncClientManager()).WillByDefault(ReturnRef(async_client_manager_)); ON_CALL(*this, localClusterName()).WillByDefault((ReturnRef(local_cluster_name_))); diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 3447223bfcce2..76a9f05aadc17 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -312,7 +312,6 @@ absl accesslog accessor accessors -ackless acks acls addr @@ -553,7 +552,6 @@ genrule getaddrinfo getaffinity getifaddrs -getNextRequestAckless getpeername getsockname getsockopt @@ -771,7 +769,6 @@ pkey plaintext pluggable pointee -polymorphism popen pos posix @@ -915,8 +912,6 @@ sockfd sockopt sockopts somestring -SotW -SotwSubscriptionState spanid spdlog splitter @@ -955,7 +950,6 @@ submessages subnet subnets suboptimal -SubscriptionStateFactory subsecond subseconds subsequence