diff --git a/include/envoy/router/route_config_provider_manager.h b/include/envoy/router/route_config_provider_manager.h index ffd9922bd1a7..9912aa460356 100644 --- a/include/envoy/router/route_config_provider_manager.h +++ b/include/envoy/router/route_config_provider_manager.h @@ -33,10 +33,13 @@ class RouteConfigProviderManager { * @param rds supplies the proto configuration of an RDS-configured RouteConfigProvider. * @param factory_context is the context to use for the route config provider. * @param stat_prefix supplies the stat_prefix to use for the provider stats. + * @param init_manager the Init::Manager used to coordinate initialization of a the underlying RDS + * subscription. */ virtual RouteConfigProviderPtr createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, - Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix) PURE; + Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, + Init::Manager& init_manager) PURE; /** * Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for diff --git a/source/common/config/config_provider_impl.cc b/source/common/config/config_provider_impl.cc index 61a0f1d7c7e1..8deac8cae62e 100644 --- a/source/common/config/config_provider_impl.cc +++ b/source/common/config/config_provider_impl.cc @@ -23,24 +23,6 @@ ConfigSubscriptionCommonBase::~ConfigSubscriptionCommonBase() { init_target_.ready(); config_provider_manager_.unbindSubscription(manager_identifier_); } - -void ConfigSubscriptionCommonBase::bindConfigProvider(MutableConfigProviderCommonBase* provider) { - // All config providers bound to a ConfigSubscriptionCommonBase must be of the same concrete - // type; this is assumed by ConfigSubscriptionInstance::checkAndApplyConfigUpdate() and is - // verified by the assertion below. NOTE: an inlined statement ASSERT() triggers a potentially - // evaluated expression warning from clang due to `typeid(**mutable_config_providers_.begin())`. - // To avoid this, we use a lambda to separate the first mutable provider dereference from the - // typeid() statement. - ASSERT([&]() { - if (!mutable_config_providers_.empty()) { - const auto& first_provider = **mutable_config_providers_.begin(); - return typeid(*provider) == typeid(first_provider); - } - return true; - }()); - mutable_config_providers_.insert(provider); -} - bool ConfigSubscriptionInstance::checkAndApplyConfigUpdate(const Protobuf::Message& config_proto, const std::string& config_name, const std::string& version_info) { @@ -55,58 +37,12 @@ bool ConfigSubscriptionInstance::checkAndApplyConfigUpdate(const Protobuf::Messa config_info_ = {new_hash, version_info}; ENVOY_LOG(debug, "{}: loading new configuration: config_name={} hash={}", name_, config_name, new_hash); - - ASSERT(!mutable_config_providers_.empty()); - ConfigProvider::ConfigConstSharedPtr new_config; - for (auto* provider : mutable_config_providers_) { - // All bound mutable config providers must be of the same type (see the ASSERT... in - // bindConfigProvider()). - // This makes it safe to call any of the provider's onConfigProtoUpdate() to get a new config - // impl, which can then be passed to all providers. - auto* typed_provider = static_cast(provider); - if (new_config == nullptr) { - if ((new_config = typed_provider->onConfigProtoUpdate(config_proto)) == nullptr) { - return false; - } - } - typed_provider->onConfigUpdate(new_config); - } - + auto new_config_impl = onConfigProtoUpdate(config_proto); + applyConfigUpdate([new_config_impl](ConfigProvider::ConfigConstSharedPtr) + -> ConfigProvider::ConfigConstSharedPtr { return new_config_impl; }); return true; } -void DeltaConfigSubscriptionInstance::applyDeltaConfigUpdate( - const std::function& update_fn, Event::PostCb complete_cb) { - // The Config implementation is assumed to be shared across the config providers bound to this - // subscription, therefore, simply propagating the update to all worker threads for a single bound - // provider will be sufficient. - if (mutable_config_providers_.size() > 1) { - ASSERT(static_cast(*mutable_config_providers_.begin()) - ->getConfig() == static_cast( - *std::next(mutable_config_providers_.begin())) - ->getConfig()); - } - - // TODO(AndresGuedez): currently, the caller has to compute the differences in resources between - // DS API config updates and passes a granular update_fn() that adds/modifies/removes resources as - // needed. Such logic could be generalized as part of this framework such that this function owns - // the diffing and issues the corresponding call to add/modify/remove a resource according to a - // vector of functions passed by the caller. - // For now each config provider has its own copy of config, we need to propagate the update to - // every provider. - for (auto* provider : mutable_config_providers_) { - auto* typed_provider = static_cast(provider); - typed_provider->onConfigUpdate( - [update_fn, typed_provider]() { - // Note: this lambda is run on every worker thread, getting the config from within the - // lambda ensures us getting the per-worker config. - ConfigSharedPtr config = typed_provider->getConfig(); - update_fn(config); - }, - complete_cb); - } -} - ConfigProviderManagerImplBase::ConfigProviderManagerImplBase(Server::Admin& admin, const std::string& config_name) { config_tracker_entry_ = diff --git a/source/common/config/config_provider_impl.h b/source/common/config/config_provider_impl.h index 29c4df358ec2..d60b4cd83e80 100644 --- a/source/common/config/config_provider_impl.h +++ b/source/common/config/config_provider_impl.h @@ -20,11 +20,11 @@ namespace Envoy { namespace Config { // This file provides a set of base classes, (ImmutableConfigProviderBase, -// MutableConfigProviderCommonBase, MutableConfigProviderBase, DeltaMutableConfigProviderBase, -// ConfigProviderManagerImplBase, ConfigSubscriptionCommonBase, ConfigSubscriptionInstance, -// DeltaConfigSubscriptionInstance), conforming to the ConfigProvider/ConfigProviderManager -// interfaces, which in tandem provide a framework for implementing statically defined (i.e., -// immutable) and dynamic (mutable via subscriptions) configuration for Envoy. +// MutableConfigProviderCommonBase, ConfigProviderManagerImplBase, ConfigSubscriptionCommonBase, +// ConfigSubscriptionInstance, DeltaConfigSubscriptionInstance), conforming to the +// ConfigProvider/ConfigProviderManager interfaces, which in tandem provide a framework for +// implementing statically defined (i.e., immutable) and dynamic (mutable via subscriptions) +// configuration for Envoy. // // The mutability property applies to the ConfigProvider itself and _not_ the underlying config // proto, which is always immutable. MutableConfigProviderCommonBase objects receive config proto @@ -58,11 +58,12 @@ namespace Config { // interface. // // For mutable (xDS) providers: -// 1) According to the API type, create a class derived from MutableConfigProviderBase or -// DeltaMutableConfigProviderBase and implement the required interface. -// 2) According to the API type, create a class derived from ConfigSubscriptionInstance or -// DeltaConfigSubscriptionInstance; this is the entity responsible for owning and managing the -// Envoy::Config::Subscription that provides the underlying config subscription. +// 1) According to the API type, create a class derived from MutableConfigProviderCommonBase and +// implement the required interface. +// 2) According to the API type, create a class derived from +// ConfigSubscriptionInstance or DeltaConfigSubscriptionInstance; this is the entity responsible +// for owning and managing the Envoy::Config::Subscription that provides the +// underlying config subscription, and the Config implemention shared by associated providers. // a) For a ConfigProvider::ApiType::Full subscription instance (i.e., a // ConfigSubscriptionInstance child): // - When subscription callbacks (onConfigUpdate, onConfigUpdateFailed) are issued by the @@ -78,8 +79,8 @@ namespace Config { // - When subscription callbacks (onConfigUpdate, onConfigUpdateFailed) are issued by the // underlying subscription, the corresponding ConfigSubscriptionInstance functions must be called // as well. -// - On a successful config update, applyConfigUpdate() should be called to propagate the config -// updates to all bound config providers and worker threads. +// - On a successful config update, applyConfigUpdate() should be called to propagate the +// config updates to all bound config providers and worker threads. class ConfigProviderManagerImplBase; @@ -130,12 +131,13 @@ class ImmutableConfigProviderBase : public ConfigProvider { class MutableConfigProviderCommonBase; /** - * Provides common DS API subscription functionality required by the ConfigProvider::ApiType - * specific base classes (see ConfigSubscriptionInstance and DeltaConfigSubscriptionInstance). + * Provides common DS API subscription functionality required by the ConfigProvider::ApiType. * - * To do so, this class keeps track of a set of MutableConfigProviderCommonBase instances associated - * with an underlying subscription; providers are bound/unbound as needed as they are created and - * destroyed. + * This class can not be instantiated directly; instead, it provides the foundation for + * config subscription implementations which derive from it. + * + * A subscription is supposed to be co-owned by config providers with the same config source, it's + * designed to be created/destructed on admin thread only. * * xDS config providers and subscriptions are split to avoid lifetime issues with arguments * required by the config providers. An example is the Server::Configuration::FactoryContext, which @@ -143,10 +145,10 @@ class MutableConfigProviderCommonBase; * in use (see #3960). This split enables single ownership of the config providers, while enabling * shared ownership of the underlying subscription. * - * This class can not be instantiated directly; instead, it provides the foundation for - * config subscription implementations which derive from it. */ -class ConfigSubscriptionCommonBase : protected Logger::Loggable { +class ConfigSubscriptionCommonBase + : protected Logger::Loggable, + public std::enable_shared_from_this { public: struct LastConfigInfo { absl::optional last_config_hash_; @@ -166,6 +168,10 @@ class ConfigSubscriptionCommonBase : protected Logger::Loggable& configInfo() const { return config_info_; } + ConfigProvider::ConfigConstSharedPtr getConfig() const { + return tls_->getTyped().config_; + } + /** * Must be called by derived classes when the onConfigUpdate() callback associated with the * underlying subscription is issued. @@ -184,25 +190,45 @@ class ConfigSubscriptionCommonBase : protected Logger::Loggable& update_fn, + Event::PostCb complete_cb = []() {}) { + // It is safe to call shared_from_this here as this is in main thread, and destruction of a + // ConfigSubscriptionCommonBase owner (i.e., a provider) happens in main thread as well. + auto shared_this = shared_from_this(); + tls_->runOnAllThreads( + [this, update_fn]() { + tls_->getTyped().config_ = update_fn(this->getConfig()); + }, + /*Make sure this subscription will not be teared down during the update propagation.*/ + [shared_this, complete_cb]() { complete_cb(); }); } void setLastUpdated() { last_updated_ = time_source_.systemTime(); } @@ -212,16 +238,12 @@ class ConfigSubscriptionCommonBase : protected Logger::Loggable mutable_config_providers_; absl::optional config_info_; + // This slot holds a Config implementation in each thread, which is shared between + // bound providers. + ThreadLocal::SlotPtr tls_; private: - void bindConfigProvider(MutableConfigProviderCommonBase* provider); - - void unbindConfigProvider(MutableConfigProviderCommonBase* provider) { - mutable_config_providers_.erase(provider); - } - Init::TargetImpl init_target_; const uint64_t manager_identifier_; ConfigProviderManagerImplBase& config_provider_manager_; @@ -235,28 +257,34 @@ class ConfigSubscriptionCommonBase : protected Logger::Loggables and // instead centralizing lifetime management in the ConfigProviderManagerImplBase with explicit // reference counting would be more maintainable. - friend class MutableConfigProviderCommonBase; - friend class MutableConfigProviderBase; - friend class DeltaMutableConfigProviderBase; friend class ConfigProviderManagerImplBase; - friend class MockMutableConfigProviderBase; }; using ConfigSubscriptionCommonBaseSharedPtr = std::shared_ptr; /** * Provides common subscription functionality required by ConfigProvider::ApiType::Full DS APIs. + * A single Config instance is shared across all providers and all workers associated with this + * subscription. */ class ConfigSubscriptionInstance : public ConfigSubscriptionCommonBase { -protected: +public: ConfigSubscriptionInstance(const std::string& name, const uint64_t manager_identifier, ConfigProviderManagerImplBase& config_provider_manager, - TimeSource& time_source, const SystemTime& last_updated, - const LocalInfo::LocalInfo& local_info) - : ConfigSubscriptionCommonBase(name, manager_identifier, config_provider_manager, time_source, - last_updated, local_info) {} + Server::Configuration::FactoryContext& factory_context) + : ConfigSubscriptionCommonBase(name, manager_identifier, config_provider_manager, + factory_context) {} - ~ConfigSubscriptionInstance() override = default; + /** + * Must be called by the derived class' constructor. + * @param initial_config supplies an initial Envoy::Config::ConfigProvider::Config associated with + * the underlying subscription, shared across all providers and workers. + */ + void initialize(const ConfigProvider::ConfigConstSharedPtr& initial_config) { + tls_->set([initial_config](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { + return std::make_shared(initial_config); + }); + } /** * Determines whether a configuration proto is a new update, and if so, propagates it to all @@ -268,46 +296,52 @@ class ConfigSubscriptionInstance : public ConfigSubscriptionCommonBase { */ bool checkAndApplyConfigUpdate(const Protobuf::Message& config_proto, const std::string& config_name, const std::string& version_info); -}; -using ConfigSharedPtr = std::shared_ptr; +protected: + /** + * Called when a new config proto is received via an xDS subscription. + * On successful validation of the config, must return a shared_ptr to a ConfigProvider::Config + * implementation that will be propagated to all mutable config providers sharing the + * subscription. + * Note that this function is called _once_ across all shared config providers per xDS + * subscription config update. + * @param config_proto supplies the configuration proto. + * @return ConfigConstSharedPtr the ConfigProvider::Config to share with other providers. + */ + virtual ConfigProvider::ConfigConstSharedPtr + onConfigProtoUpdate(const Protobuf::Message& config_proto) PURE; +}; /** * Provides common subscription functionality required by ConfigProvider::ApiType::Delta DS APIs. */ class DeltaConfigSubscriptionInstance : public ConfigSubscriptionCommonBase { protected: - DeltaConfigSubscriptionInstance(const std::string& name, const uint64_t manager_identifier, - ConfigProviderManagerImplBase& config_provider_manager, - TimeSource& time_source, const SystemTime& last_updated, - const LocalInfo::LocalInfo& local_info) - : ConfigSubscriptionCommonBase(name, manager_identifier, config_provider_manager, time_source, - last_updated, local_info) {} - + using ConfigSubscriptionCommonBase::ConfigSubscriptionCommonBase; ~DeltaConfigSubscriptionInstance() override = default; /** - * Propagates a config update to the config providers and worker threads associated with the - * subscription. - * - * @param update_fn the callback to run on each worker thread. - * @param complete_cb the callback to run on each worker thread. NOTE it's called each time a - * registered provider's update propagation finishes. + * Must be called by the derived class' constructor. + * @param init_cb supplies an initial Envoy::Config::ConfigProvider::Config associated with the + * underlying subscription for each worker thread. */ - void applyDeltaConfigUpdate(const std::function& update_fn, - Event::PostCb complete_cb = Event::PostCb()); + void initialize(const std::function& init_cb) { + tls_->set([init_cb](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { + return std::make_shared(init_cb()); + }); + } }; /** * Provides generic functionality required by the ConfigProvider::ApiType specific dynamic config - * providers (see MutableConfigProviderBase and DeltaMutableConfigProviderBase). + * providers. * * This class can not be instantiated directly; instead, it provides the foundation for * dynamic config provider implementations which derive from it. */ class MutableConfigProviderCommonBase : public ConfigProvider { public: - ~MutableConfigProviderCommonBase() override { subscription_->unbindConfigProvider(this); } + ~MutableConfigProviderCommonBase() override = default; // Envoy::Config::ConfigProvider SystemTime lastUpdated() const override { return subscription_->lastUpdated(); } @@ -315,132 +349,16 @@ class MutableConfigProviderCommonBase : public ConfigProvider { protected: MutableConfigProviderCommonBase(ConfigSubscriptionCommonBaseSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, ApiType api_type) - : tls_(factory_context.threadLocal().allocateSlot()), subscription_(subscription), - api_type_(api_type) {} - - ThreadLocal::SlotPtr tls_; - ConfigSubscriptionCommonBaseSharedPtr subscription_; - -private: - ApiType api_type_; -}; + : subscription_(subscription), api_type_(api_type) {} -/** - * Provides common mutable (dynamic) config provider functionality required by - * ConfigProvider::ApiType::Full DS APIs. - */ -class MutableConfigProviderBase : public MutableConfigProviderCommonBase { -public: // Envoy::Config::ConfigProvider - // NOTE: This is being promoted to public for internal uses to avoid an unnecessary dynamic_cast - // in the public API (ConfigProvider::config()). - ConfigConstSharedPtr getConfig() const override { - return tls_->getTyped().config_; - } - - /** - * Called when a new config proto is received via an xDS subscription. - * On successful validation of the config, must return a shared_ptr to a ConfigProvider::Config - * implementation that will be propagated to all mutable config providers sharing the - * subscription. - * Note that this function is called _once_ across all shared config providers per xDS - * subscription config update. - * @param config_proto supplies the configuration proto. - * @return ConfigConstSharedPtr the ConfigProvider::Config to share with other providers. - */ - virtual ConfigConstSharedPtr onConfigProtoUpdate(const Protobuf::Message& config_proto) PURE; - - /** - * Must be called by the derived class' constructor. - * @param initial_config supplies an initial Envoy::Config::ConfigProvider::Config associated with - * the underlying subscription. - */ - void initialize(const ConfigConstSharedPtr& initial_config) { - subscription_->bindConfigProvider(this); - tls_->set([initial_config](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { - return std::make_shared(initial_config); - }); - } - - /** - * Propagates a newly instantiated Envoy::Config::ConfigProvider::Config to all workers. - * @param config supplies the newly instantiated config. - */ - void onConfigUpdate(const ConfigConstSharedPtr& config) { - if (getConfig() == config) { - return; - } - tls_->runOnAllThreads( - [this, config]() -> void { tls_->getTyped().config_ = config; }); - } - -protected: - MutableConfigProviderBase(ConfigSubscriptionCommonBaseSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, - ApiType api_type) - : MutableConfigProviderCommonBase(std::move(subscription), factory_context, api_type) {} + ConfigConstSharedPtr getConfig() const override { return subscription_->getConfig(); } - ~MutableConfigProviderBase() override = default; + ConfigSubscriptionCommonBaseSharedPtr subscription_; private: - struct ThreadLocalConfig : public ThreadLocal::ThreadLocalObject { - ThreadLocalConfig(ConfigProvider::ConfigConstSharedPtr initial_config) - : config_(std::move(initial_config)) {} - - ConfigProvider::ConfigConstSharedPtr config_; - }; -}; - -/** - * Provides common mutable (dynamic) config provider functionality required by - * ConfigProvider::ApiType::Delta DS APIs. - */ -class DeltaMutableConfigProviderBase : public MutableConfigProviderCommonBase { -public: - // Envoy::Config::ConfigProvider - // This promotes getConfig() to public so that internal uses can avoid an unnecessary dynamic_cast - // in the public API (ConfigProvider::config()). - ConfigConstSharedPtr getConfig() const override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } - - /** - * Non-const overload for use within the framework. - * @return ConfigSharedPtr the config implementation associated with the provider. - */ - virtual ConfigSharedPtr getConfig() PURE; - - /** - * Propagates a delta config update to all workers. - * @param update_cb the callback to run on each worker. - * @param complete_cb the callback to run in main thread after the update propagation is done on - * every worker thread. - */ - void onConfigUpdate(Envoy::Event::PostCb update_cb, Event::PostCb complete_cb) { - if (complete_cb) { - tls_->runOnAllThreads(std::move(update_cb), std::move(complete_cb)); - } else { - tls_->runOnAllThreads(std::move(update_cb)); - } - } - -protected: - DeltaMutableConfigProviderBase(ConfigSubscriptionCommonBaseSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, - ApiType api_type) - : MutableConfigProviderCommonBase(std::move(subscription), factory_context, api_type) {} - - ~DeltaMutableConfigProviderBase() override = default; - - /** - * Must be called by the derived class' constructor. - * @param initializeCb supplies the initialization callback to be issued for each worker - * thread. - */ - void initialize(ThreadLocal::Slot::InitializeCb initializeCb) { - subscription_->bindConfigProvider(this); - tls_->set(std::move(initializeCb)); - } + ApiType api_type_; }; /** diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 3a499d27fae0..d6b5ee2bf957 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -169,12 +169,15 @@ envoy_cc_library( hdrs = ["scoped_rds.h"], deps = [ ":scoped_config_lib", + "//include/envoy/config:config_provider_interface", "//include/envoy/config:subscription_interface", "//include/envoy/router:route_config_provider_manager_interface", "//include/envoy/stats:stats_interface", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", "//source/common/config:config_provider_lib", + "//source/common/init:manager_lib", + "//source/common/init:watcher_lib", "@envoy_api//envoy/admin/v2alpha:config_dump_cc", "@envoy_api//envoy/api/v2:srds_cc", ], diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 781ce9db1521..eb40e8d5e9e1 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -30,8 +30,8 @@ RouteConfigProviderPtr RouteConfigProviderUtil::create( return route_config_provider_manager.createStaticRouteConfigProvider(config.route_config(), factory_context); case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::kRds: - return route_config_provider_manager.createRdsRouteConfigProvider(config.rds(), factory_context, - stat_prefix); + return route_config_provider_manager.createRdsRouteConfigProvider( + config.rds(), factory_context, stat_prefix, factory_context.initManager()); default: NOT_REACHED_GCOVR_EXCL_LINE; } @@ -207,8 +207,8 @@ RouteConfigProviderManagerImpl::RouteConfigProviderManagerImpl(Server::Admin& ad Router::RouteConfigProviderPtr RouteConfigProviderManagerImpl::createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, - Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix) { - + Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, + Init::Manager& init_manager) { // RdsRouteConfigSubscriptions are unique based on their serialized RDS config. const uint64_t manager_identifier = MessageUtil::hash(rds); @@ -221,9 +221,7 @@ Router::RouteConfigProviderPtr RouteConfigProviderManagerImpl::createRdsRouteCon // of simplicity. subscription.reset(new RdsRouteConfigSubscription(rds, manager_identifier, factory_context, stat_prefix, *this)); - - factory_context.initManager().add(subscription->init_target_); - + init_manager.add(subscription->init_target_); route_config_subscriptions_.insert({manager_identifier, subscription}); } else { // Because the RouteConfigProviderManager's weak_ptrs only get cleaned up diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index ee311b63a163..09baa58fc96d 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -194,8 +194,8 @@ class RouteConfigProviderManagerImpl : public RouteConfigProviderManager, // RouteConfigProviderManager RouteConfigProviderPtr createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, - Server::Configuration::FactoryContext& factory_context, - const std::string& stat_prefix) override; + Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, + Init::Manager& init_manager) override; RouteConfigProviderPtr createStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config, diff --git a/source/common/router/scoped_config_impl.cc b/source/common/router/scoped_config_impl.cc index d324db781198..d0a27686d8f2 100644 --- a/source/common/router/scoped_config_impl.cc +++ b/source/common/router/scoped_config_impl.cc @@ -98,13 +98,13 @@ ScopeKeyBuilderImpl::computeScopeKey(const Http::HeaderMap& headers) const { return std::make_unique(std::move(key)); } -void ThreadLocalScopedConfigImpl::addOrUpdateRoutingScope( +void ScopedConfigImpl::addOrUpdateRoutingScope( const ScopedRouteInfoConstSharedPtr& scoped_route_info) { scoped_route_info_by_name_.try_emplace(scoped_route_info->scopeName(), scoped_route_info); scoped_route_info_by_key_.try_emplace(scoped_route_info->scopeKey().hash(), scoped_route_info); } -void ThreadLocalScopedConfigImpl::removeRoutingScope(const std::string& scope_name) { +void ScopedConfigImpl::removeRoutingScope(const std::string& scope_name) { const auto iter = scoped_route_info_by_name_.find(scope_name); if (iter != scoped_route_info_by_name_.end()) { ASSERT(scoped_route_info_by_key_.count(iter->second->scopeKey().hash()) == 1); @@ -114,7 +114,7 @@ void ThreadLocalScopedConfigImpl::removeRoutingScope(const std::string& scope_na } Router::ConfigConstSharedPtr -ThreadLocalScopedConfigImpl::getRouteConfig(const Http::HeaderMap& headers) const { +ScopedConfigImpl::getRouteConfig(const Http::HeaderMap& headers) const { std::unique_ptr scope_key = scope_key_builder_.computeScopeKey(headers); if (scope_key == nullptr) { return nullptr; diff --git a/source/common/router/scoped_config_impl.h b/source/common/router/scoped_config_impl.h index 8e5fff1582f8..23cdfee0d76e 100644 --- a/source/common/router/scoped_config_impl.h +++ b/source/common/router/scoped_config_impl.h @@ -167,7 +167,6 @@ class ScopedRouteInfo { } } } - ~ScopedRouteInfo() = default; Router::ConfigConstSharedPtr routeConfig() const { return route_provider_->config(); } const ScopeKey& scopeKey() const { return scope_key_; } @@ -190,9 +189,9 @@ using ScopedRouteMap = std::map; * ConnectionManagerImpl::refreshCachedRoute() will call getRouterConfig() to obtain the * Router::ConfigConstSharedPtr to use for route selection. */ -class ThreadLocalScopedConfigImpl : public ScopedConfig, public ThreadLocal::ThreadLocalObject { +class ScopedConfigImpl : public ScopedConfig { public: - ThreadLocalScopedConfigImpl(ScopedRoutes::ScopeKeyBuilder&& scope_key_builder) + ScopedConfigImpl(ScopedRoutes::ScopeKeyBuilder&& scope_key_builder) : scope_key_builder_(std::move(scope_key_builder)) {} void addOrUpdateRoutingScope(const ScopedRouteInfoConstSharedPtr& scoped_route_info); diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index 3be47136056b..2a850df370ec 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -8,6 +8,9 @@ #include "common/common/assert.h" #include "common/common/logger.h" #include "common/common/utility.h" +#include "common/config/resources.h" +#include "common/init/manager_impl.h" +#include "common/init/watcher_impl.h" // Types are deeply nested under Envoy::Config::ConfigProvider; use 'using-directives' across all // ConfigProvider related types for consistency. @@ -67,7 +70,7 @@ InlineScopedRoutesConfigProvider::InlineScopedRoutesConfigProvider( ConfigProviderInstanceType::Inline, ConfigProvider::ApiType::Delta), name_(std::move(name)), - config_(std::make_shared(std::move(scope_key_builder))), + config_(std::make_shared(std::move(scope_key_builder))), config_protos_(std::make_move_iterator(config_protos.begin()), std::make_move_iterator(config_protos.end())), rds_config_source_(std::move(rds_config_source)) {} @@ -75,24 +78,32 @@ InlineScopedRoutesConfigProvider::InlineScopedRoutesConfigProvider( ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( const envoy::config::filter::network::http_connection_manager::v2::ScopedRds& scoped_rds, const uint64_t manager_identifier, const std::string& name, + const envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes:: + ScopeKeyBuilder& scope_key_builder, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, envoy::api::v2::core::ConfigSource rds_config_source, + RouteConfigProviderManager& route_config_provider_manager, ScopedRoutesConfigProviderManager& config_provider_manager) - : DeltaConfigSubscriptionInstance( - "SRDS", manager_identifier, config_provider_manager, factory_context.timeSource(), - factory_context.timeSource().systemTime(), factory_context.localInfo()), - factory_context_(factory_context), name_(name), + : DeltaConfigSubscriptionInstance("SRDS", manager_identifier, config_provider_manager, + factory_context), + factory_context_(factory_context), name_(name), scope_key_builder_(scope_key_builder), scope_(factory_context.scope().createScope(stat_prefix + "scoped_rds." + name + ".")), stats_({ALL_SCOPED_RDS_STATS(POOL_COUNTER(*scope_))}), rds_config_source_(std::move(rds_config_source)), validation_visitor_(factory_context.messageValidationVisitor()), stat_prefix_(stat_prefix), - srds_config_provider_manager_(config_provider_manager) { + route_config_provider_manager_(route_config_provider_manager) { subscription_ = factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( scoped_rds.scoped_rds_config_source(), Grpc::Common::typeUrl( envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), *scope_, *this); + + initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr { + return std::make_shared( + envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder( + scope_key_builder)); + }); } void ScopedRdsConfigSubscription::onConfigUpdate( @@ -104,6 +115,16 @@ void ScopedRdsConfigSubscription::onConfigUpdate( absl::flat_hash_set unique_resource_names; envoy::config::filter::network::http_connection_manager::v2::Rds rds; rds.mutable_config_source()->MergeFrom(rds_config_source_); + + std::unique_ptr overriding_init_manager; + if (factory_context_.initManager().state() == Init::Manager::State::Initialized) { + // Pause RDS to not send a burst of RDS requests until we start all the new subscriptions. + factory_context_.clusterManager().adsMux().pause( + Envoy::Config::TypeUrl::get().RouteConfiguration); + overriding_init_manager = + std::make_unique(fmt::format("SRDS {}:{}", name_, version_info)); + } + for (const auto& resource : added_resources) { envoy::api::v2::ScopedRouteConfiguration scoped_route_config; try { @@ -116,8 +137,11 @@ void ScopedRdsConfigSubscription::onConfigUpdate( } rds.set_route_config_name(scoped_route_config.route_configuration_name()); ScopedRouteInfoConstSharedPtr scoped_route_info = std::make_shared( - std::move(scoped_route_config), srds_config_provider_manager_.createRouteConfigProvider( - factory_context_, rds, stat_prefix_)); + std::move(scoped_route_config), + route_config_provider_manager_.createRdsRouteConfigProvider( + rds, factory_context_, stat_prefix_, + (overriding_init_manager == nullptr ? factory_context_.initManager() + : *overriding_init_manager))); // Detect if there is key conflict between two scopes. auto iter = scope_name_by_hash_.find(scoped_route_info->scopeKey().hash()); if (iter != scope_name_by_hash_.end() && iter->second != scoped_route_info->scopeName()) { @@ -127,30 +151,46 @@ void ScopedRdsConfigSubscription::onConfigUpdate( } scope_name_by_hash_[scoped_route_info->scopeKey().hash()] = scoped_route_info->scopeName(); scoped_route_map_[scoped_route_info->scopeName()] = scoped_route_info; - applyDeltaConfigUpdate( - [scoped_route_info](const ConfigProvider::ConfigConstSharedPtr& config) { - auto* thread_local_scoped_config = const_cast( - static_cast(config.get())); + applyConfigUpdate([scoped_route_info](ConfigProvider::ConfigConstSharedPtr config) + -> ConfigProvider::ConfigConstSharedPtr { + auto* thread_local_scoped_config = + const_cast(static_cast(config.get())); - thread_local_scoped_config->addOrUpdateRoutingScope(scoped_route_info); - }); + thread_local_scoped_config->addOrUpdateRoutingScope(scoped_route_info); + return config; + }); any_applied = true; ENVOY_LOG(debug, "srds: add/update scoped_route '{}'", scoped_route_info->scopeName()); } catch (const EnvoyException& e) { exception_msgs.emplace_back(fmt::format("{}", e.what())); } } + if (overriding_init_manager != nullptr) { + // For new RDS subscriptions created after listener warming up, we don't wait for them to warm + // up. + Init::WatcherImpl noop_watcher( + // Note: we just throw it away. + fmt::format("SRDS ConfigUpdate watcher {}:{}", name_, version_info), + []() { ENVOY_LOG_MISC(trace, ""); }); + overriding_init_manager->initialize(noop_watcher); + // New subscriptions should be created, now lift the floodgate. + factory_context_.clusterManager().adsMux().resume( + Envoy::Config::TypeUrl::get().RouteConfiguration); + } + for (const auto& scope_name : removed_resources) { auto iter = scoped_route_map_.find(scope_name); if (iter != scoped_route_map_.end()) { ScopedRouteInfoConstSharedPtr to_be_deleted = iter->second; scope_name_by_hash_.erase(iter->second->scopeKey().hash()); scoped_route_map_.erase(iter); - applyDeltaConfigUpdate( - [scope_name](const ConfigProvider::ConfigConstSharedPtr& config) { - auto* thread_local_scoped_config = const_cast( - static_cast(config.get())); + applyConfigUpdate( + [scope_name]( + ConfigProvider::ConfigConstSharedPtr config) -> ConfigProvider::ConfigConstSharedPtr { + auto* thread_local_scoped_config = + const_cast(static_cast(config.get())); thread_local_scoped_config->removeRoutingScope(scope_name); + return config; }, // We need to delete the associated RouteConfigProvider in main thread. [to_be_deleted]() { /*to_be_deleted is destructed in main thread.*/ }); @@ -212,27 +252,14 @@ void ScopedRdsConfigSubscription::onConfigUpdate( *to_remove_repeated.Add() = scoped_route.first; } onConfigUpdate(to_add_repeated, to_remove_repeated, version_info); -} +} // namespace Router ScopedRdsConfigProvider::ScopedRdsConfigProvider( ScopedRdsConfigSubscriptionSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, - envoy::api::v2::core::ConfigSource rds_config_source, - const envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes:: - ScopeKeyBuilder& scope_key_builder) - : DeltaMutableConfigProviderBase(std::move(subscription), factory_context, - ConfigProvider::ApiType::Delta), - rds_config_source_(std::move(rds_config_source)) { - initialize([scope_key_builder](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { - return std::make_shared( - envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder( - scope_key_builder)); - }); -} + envoy::api::v2::core::ConfigSource rds_config_source) + : MutableConfigProviderCommonBase(std::move(subscription), ConfigProvider::ApiType::Delta), + rds_config_source_(std::move(rds_config_source)) {} -Envoy::Config::ConfigSharedPtr ScopedRdsConfigProvider::getConfig() { - return std::dynamic_pointer_cast(tls_->get()); -} ProtobufTypes::MessagePtr ScopedRoutesConfigProviderManager::dumpConfigs() const { auto config_dump = std::make_unique(); for (const auto& element : configSubscriptions()) { @@ -274,30 +301,28 @@ ConfigProviderPtr ScopedRoutesConfigProviderManager::createXdsConfigProvider( const Protobuf::Message& config_source_proto, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, const ConfigProviderManager::OptionalArg& optarg) { + const auto& typed_optarg = static_cast(optarg); ScopedRdsConfigSubscriptionSharedPtr subscription = ConfigProviderManagerImplBase::getSubscription( config_source_proto, factory_context.initManager(), [&config_source_proto, &factory_context, &stat_prefix, - &optarg](const uint64_t manager_identifier, - ConfigProviderManagerImplBase& config_provider_manager) + &typed_optarg](const uint64_t manager_identifier, + ConfigProviderManagerImplBase& config_provider_manager) -> Envoy::Config::ConfigSubscriptionCommonBaseSharedPtr { const auto& scoped_rds_config_source = dynamic_cast< const envoy::config::filter::network::http_connection_manager::v2::ScopedRds&>( config_source_proto); return std::make_shared( - scoped_rds_config_source, manager_identifier, - static_cast(optarg) - .scoped_routes_name_, - factory_context, stat_prefix, - static_cast(optarg) - .rds_config_source_, + scoped_rds_config_source, manager_identifier, typed_optarg.scoped_routes_name_, + typed_optarg.scope_key_builder_, factory_context, stat_prefix, + typed_optarg.rds_config_source_, + static_cast(config_provider_manager) + .route_config_provider_manager(), static_cast(config_provider_manager)); }); - const auto& typed_optarg = static_cast(optarg); - return std::make_unique(std::move(subscription), factory_context, - typed_optarg.rds_config_source_, - typed_optarg.scope_key_builder_); + return std::make_unique(std::move(subscription), + typed_optarg.rds_config_source_); } ConfigProviderPtr ScopedRoutesConfigProviderManager::createStaticConfigProvider( diff --git a/source/common/router/scoped_rds.h b/source/common/router/scoped_rds.h index 45948d0ffbed..3ee10c05bd63 100644 --- a/source/common/router/scoped_rds.h +++ b/source/common/router/scoped_rds.h @@ -88,8 +88,11 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio ScopedRdsConfigSubscription( const envoy::config::filter::network::http_connection_manager::v2::ScopedRds& scoped_rds, const uint64_t manager_identifier, const std::string& name, + const envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes:: + ScopeKeyBuilder& scope_key_builder, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, envoy::api::v2::core::ConfigSource rds_config_source, + RouteConfigProviderManager& route_config_provider_manager, ScopedRoutesConfigProviderManager& config_provider_manager); ~ScopedRdsConfigSubscription() override = default; @@ -99,7 +102,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio const ScopedRouteMap& scopedRouteMap() const { return scoped_route_map_; } private: - // Envoy::Config::ConfigSubscriptionCommonBase + // Envoy::Config::DeltaConfigSubscriptionInstance void start() override { subscription_->start({}); } // Envoy::Config::SubscriptionCallbacks @@ -109,7 +112,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio const Protobuf::RepeatedPtrField& removed_resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException*) override { - ConfigSubscriptionCommonBase::onConfigUpdateFailed(); + DeltaConfigSubscriptionInstance::onConfigUpdateFailed(); } std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource, @@ -125,37 +128,29 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio Server::Configuration::FactoryContext& factory_context_; const std::string name_; std::unique_ptr subscription_; + const envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder + scope_key_builder_; Stats::ScopePtr scope_; ScopedRdsStats stats_; const envoy::api::v2::core::ConfigSource rds_config_source_; ProtobufMessage::ValidationVisitor& validation_visitor_; const std::string stat_prefix_; - ScopedRoutesConfigProviderManager& srds_config_provider_manager_; + RouteConfigProviderManager& route_config_provider_manager_; }; using ScopedRdsConfigSubscriptionSharedPtr = std::shared_ptr; // A ConfigProvider for scoped RDS that dynamically fetches scoped routing configuration via a // subscription. -class ScopedRdsConfigProvider : public Envoy::Config::DeltaMutableConfigProviderBase { +class ScopedRdsConfigProvider : public Envoy::Config::MutableConfigProviderCommonBase { public: ScopedRdsConfigProvider(ScopedRdsConfigSubscriptionSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, - envoy::api::v2::core::ConfigSource rds_config_source, - const envoy::config::filter::network::http_connection_manager::v2:: - ScopedRoutes::ScopeKeyBuilder& scope_key_builder); + envoy::api::v2::core::ConfigSource rds_config_source); ScopedRdsConfigSubscription& subscription() { return *static_cast(subscription_.get()); } - // getConfig() is overloaded (const/non-const only). Make all base getConfig()s visible to avoid - // compiler warnings. - using DeltaMutableConfigProviderBase::getConfig; - - // Envoy::Config::DeltaMutableConfigProviderBase - Envoy::Config::ConfigSharedPtr getConfig() override; - private: const envoy::api::v2::core::ConfigSource rds_config_source_; }; @@ -197,14 +192,6 @@ class ScopedRoutesConfigProviderManager : public Envoy::Config::ConfigProviderMa return route_config_provider_manager_; } - std::shared_ptr createRouteConfigProvider( - Server::Configuration::FactoryContext& factory_context, - const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, - const std::string& stat_name) { - return route_config_provider_manager_.createRdsRouteConfigProvider(rds, factory_context, - stat_name); - } - private: RouteConfigProviderManager& route_config_provider_manager_; }; diff --git a/test/common/config/config_provider_impl_test.cc b/test/common/config/config_provider_impl_test.cc index 8dcf22c39527..4a852615552a 100644 --- a/test/common/config/config_provider_impl_test.cc +++ b/test/common/config/config_provider_impl_test.cc @@ -4,7 +4,6 @@ #include "common/protobuf/utility.h" #include "test/common/config/dummy_config.pb.h" -#include "test/mocks/config/mocks.h" #include "test/mocks/server/mocks.h" #include "test/test_common/simulated_time_system.h" #include "test/test_common/utility.h" @@ -20,6 +19,22 @@ using testing::InSequence; class DummyConfigProviderManager; +class DummyConfig : public Envoy::Config::ConfigProvider::Config { +public: + DummyConfig() {} + DummyConfig(const test::common::config::DummyConfig& config_proto) { + protos_.push_back(config_proto); + } + void addProto(const test::common::config::DummyConfig& config_proto) { + protos_.push_back(config_proto); + } + + uint32_t numProtos() const { return protos_.size(); } + +private: + std::vector protos_; +}; + class StaticDummyConfigProvider : public ImmutableConfigProviderBase { public: StaticDummyConfigProvider(const test::common::config::DummyConfig& config_proto, @@ -48,12 +63,18 @@ class DummyConfigSubscription : public ConfigSubscriptionInstance, DummyConfigSubscription(const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, DummyConfigProviderManager& config_provider_manager); - ~DummyConfigSubscription() override = default; // Envoy::Config::ConfigSubscriptionCommonBase void start() override {} + // Envoy::Config::ConfigSubscriptionInstance + ConfigProvider::ConfigConstSharedPtr + onConfigProtoUpdate(const Protobuf::Message& config_proto) override { + return std::make_shared( + static_cast(config_proto)); + } + // Envoy::Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override { @@ -84,33 +105,17 @@ class DummyConfigSubscription : public ConfigSubscriptionInstance, }; using DummyConfigSubscriptionSharedPtr = std::shared_ptr; -class DummyConfig : public ConfigProvider::Config { +class DummyDynamicConfigProvider : public MutableConfigProviderCommonBase { public: - DummyConfig(const test::common::config::DummyConfig&) {} -}; - -class DummyDynamicConfigProvider : public MutableConfigProviderBase { -public: - DummyDynamicConfigProvider(DummyConfigSubscriptionSharedPtr&& subscription, - const ConfigConstSharedPtr& initial_config, - Server::Configuration::FactoryContext& factory_context) - : MutableConfigProviderBase(std::move(subscription), factory_context, ApiType::Full), + DummyDynamicConfigProvider(DummyConfigSubscriptionSharedPtr&& subscription) + : MutableConfigProviderCommonBase(std::move(subscription), ApiType::Full), subscription_(static_cast( - MutableConfigProviderCommonBase::subscription_.get())) { - initialize(initial_config); - } + MutableConfigProviderCommonBase::subscription_.get())) {} ~DummyDynamicConfigProvider() override = default; DummyConfigSubscription& subscription() { return *subscription_; } - // Envoy::Config::MutableConfigProviderBase - ConfigProvider::ConfigConstSharedPtr - onConfigProtoUpdate(const Protobuf::Message& config) override { - return std::make_shared( - static_cast(config)); - } - // Envoy::Config::ConfigProvider const Protobuf::Message* getConfigProto() const override { if (!subscription_->config_proto().has_value()) { @@ -177,14 +182,7 @@ class DummyConfigProviderManager : public ConfigProviderManagerImplBase { static_cast(config_provider_manager)); }); - ConfigProvider::ConfigConstSharedPtr initial_config; - const auto* provider = static_cast( - subscription->getAnyBoundMutableConfigProvider()); - if (provider) { - initial_config = provider->getConfig(); - } - return std::make_unique(std::move(subscription), initial_config, - factory_context); + return std::make_unique(std::move(subscription)); } // Envoy::Config::ConfigProviderManager @@ -204,6 +202,15 @@ class DummyConfigProviderManager : public ConfigProviderManagerImplBase { } }; +DummyConfigSubscription::DummyConfigSubscription( + const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, + DummyConfigProviderManager& config_provider_manager) + : ConfigSubscriptionInstance("DummyDS", manager_identifier, config_provider_manager, + factory_context) { + // Returns a null value. + initialize(nullptr); +} + StaticDummyConfigProvider::StaticDummyConfigProvider( const test::common::config::DummyConfig& config_proto, Server::Configuration::FactoryContext& factory_context, @@ -212,13 +219,6 @@ StaticDummyConfigProvider::StaticDummyConfigProvider( ConfigProviderInstanceType::Static, ApiType::Full), config_(std::make_shared(config_proto)), config_proto_(config_proto) {} -DummyConfigSubscription::DummyConfigSubscription( - const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, - DummyConfigProviderManager& config_provider_manager) - : ConfigSubscriptionInstance( - "DummyDS", manager_identifier, config_provider_manager, factory_context.timeSource(), - factory_context.timeSource().systemTime(), factory_context.localInfo()) {} - class ConfigProviderImplTest : public testing::Test { public: void initialize() { @@ -318,7 +318,7 @@ TEST_F(ConfigProviderImplTest, SharedOwnership) { .size()); } -// A ConfigProviderManager that returns a mock ConfigProvider. +// A ConfigProviderManager that returns a dummy ConfigProvider. class DummyConfigProviderManagerMockConfigProvider : public DummyConfigProviderManager { public: DummyConfigProviderManagerMockConfigProvider(Server::Admin& admin) @@ -338,15 +338,14 @@ class DummyConfigProviderManagerMockConfigProvider : public DummyConfigProviderM manager_identifier, factory_context, static_cast(config_provider_manager)); }); - return std::make_unique(std::move(subscription), nullptr, - factory_context); + return std::make_unique(std::move(subscription)); } }; // Test that duplicate config updates will not trigger creation of a new ConfigProvider::Config. TEST_F(ConfigProviderImplTest, DuplicateConfigProto) { InSequence sequence; - // This provider manager returns a MockMutableConfigProviderBase. + // This provider manager returns a DummyDynamicConfigProvider. auto provider_manager = std::make_unique(factory_context_.admin_); envoy::api::v2::core::ApiConfigSource config_source_proto; @@ -354,18 +353,22 @@ TEST_F(ConfigProviderImplTest, DuplicateConfigProto) { ConfigProviderPtr provider = provider_manager->createXdsConfigProvider( config_source_proto, factory_context_, "dummy_prefix", ConfigProviderManager::NullOptionalArg()); - auto* typed_provider = static_cast(provider.get()); + auto* typed_provider = static_cast(provider.get()); DummyConfigSubscription& subscription = static_cast(typed_provider->subscription()); + EXPECT_EQ(subscription.getConfig(), nullptr); // First time issuing a configUpdate(). A new ConfigProvider::Config should be created. - EXPECT_CALL(*typed_provider, onConfigProtoUpdate(_)).Times(1); Protobuf::RepeatedPtrField untyped_dummy_configs; untyped_dummy_configs.Add()->PackFrom(parseDummyConfigFromYaml("a: a dynamic dummy config")); subscription.onConfigUpdate(untyped_dummy_configs, "1"); + EXPECT_NE(subscription.getConfig(), nullptr); + auto config_ptr = subscription.getConfig(); + EXPECT_EQ(typed_provider->config().get(), config_ptr.get()); // Second time issuing the configUpdate(), this time with a duplicate proto. A new // ConfigProvider::Config _should not_ be created. - EXPECT_CALL(*typed_provider, onConfigProtoUpdate(_)).Times(0); - subscription.onConfigUpdate(untyped_dummy_configs, "1"); + subscription.onConfigUpdate(untyped_dummy_configs, "2"); + EXPECT_EQ(config_ptr, subscription.getConfig()); + EXPECT_EQ(typed_provider->config().get(), config_ptr.get()); } // An empty config provider tests on base class' constructor. @@ -518,7 +521,31 @@ class DeltaDummyConfigSubscription : public DeltaConfigSubscriptionInstance, // Envoy::Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, - const std::string& version_info) override; + const std::string& version_info) override { + if (resources.empty()) { + return; + } + + // For simplicity, there is no logic here to track updates and/or removals to the existing + // config proto set (i.e., this is append only). Real xDS APIs will need to track additions, + // updates and removals to the config set and apply the diffs to the underlying config + // implementations. + for (const auto& resource_any : resources) { + auto dummy_config = TestUtility::anyConvert(resource_any); + proto_map_[version_info] = dummy_config; + // Propagate the new config proto to all worker threads. + applyConfigUpdate([&dummy_config](ConfigProvider::ConfigConstSharedPtr prev_config) + -> ConfigProvider::ConfigConstSharedPtr { + auto* config = const_cast(static_cast(prev_config.get())); + // Per above, append only for now. + config->addProto(dummy_config); + return prev_config; + }); + } + + ConfigSubscriptionCommonBase::onConfigUpdate(); + setLastConfigInfo(absl::optional({absl::nullopt, version_info})); + } void onConfigUpdate(const Protobuf::RepeatedPtrField&, const Protobuf::RepeatedPtrField&, const std::string&) override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; @@ -537,32 +564,12 @@ class DeltaDummyConfigSubscription : public DeltaConfigSubscriptionInstance, }; using DeltaDummyConfigSubscriptionSharedPtr = std::shared_ptr; -class ThreadLocalDummyConfig : public ThreadLocal::ThreadLocalObject, - public Envoy::Config::ConfigProvider::Config { -public: - void addProto(const test::common::config::DummyConfig& config_proto) { - protos_.push_back(config_proto); - } - - uint32_t numProtos() const { return protos_.size(); } - -private: - std::vector protos_; -}; - -class DeltaDummyDynamicConfigProvider : public Envoy::Config::DeltaMutableConfigProviderBase { +class DeltaDummyDynamicConfigProvider : public Envoy::Config::MutableConfigProviderCommonBase { public: - DeltaDummyDynamicConfigProvider(DeltaDummyConfigSubscriptionSharedPtr&& subscription, - Server::Configuration::FactoryContext& factory_context, - std::shared_ptr dummy_config) - : DeltaMutableConfigProviderBase(std::move(subscription), factory_context, - ConfigProvider::ApiType::Delta), + DeltaDummyDynamicConfigProvider(DeltaDummyConfigSubscriptionSharedPtr&& subscription) + : MutableConfigProviderCommonBase(std::move(subscription), ConfigProvider::ApiType::Delta), subscription_(static_cast( - MutableConfigProviderCommonBase::subscription_.get())) { - initialize([&dummy_config](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr { - return (dummy_config != nullptr) ? dummy_config : std::make_shared(); - }); - } + MutableConfigProviderCommonBase::subscription_.get())) {} DeltaDummyConfigSubscription& subscription() { return *subscription_; } @@ -574,23 +581,12 @@ class DeltaDummyDynamicConfigProvider : public Envoy::Config::DeltaMutableConfig } return proto_vector; } + std::string getConfigVersion() const override { return (subscription_->configInfo().has_value()) ? subscription_->configInfo().value().last_config_version_ : ""; } - ConfigConstSharedPtr getConfig() const override { - return std::dynamic_pointer_cast(tls_->get()); - } - - // Envoy::Config::DeltaMutableConfigProviderBase - ConfigSharedPtr getConfig() override { - return std::dynamic_pointer_cast(tls_->get()); - } - - std::shared_ptr getThreadLocalDummyConfig() { - return std::dynamic_pointer_cast(tls_->get()); - } private: DeltaDummyConfigSubscription* subscription_; @@ -632,6 +628,7 @@ class DeltaDummyConfigProviderManager : public ConfigProviderManagerImplBase { const std::string&, const Envoy::Config::ConfigProviderManager::OptionalArg&) override { DeltaDummyConfigSubscriptionSharedPtr subscription = + getSubscription( config_source_proto, factory_context.initManager(), [&factory_context](const uint64_t manager_identifier, @@ -642,44 +639,17 @@ class DeltaDummyConfigProviderManager : public ConfigProviderManagerImplBase { static_cast(config_provider_manager)); }); - auto* existing_provider = static_cast( - subscription->getAnyBoundMutableConfigProvider()); - return std::make_unique( - std::move(subscription), factory_context, - (existing_provider != nullptr) ? existing_provider->getThreadLocalDummyConfig() : nullptr); + return std::make_unique(std::move(subscription)); } }; DeltaDummyConfigSubscription::DeltaDummyConfigSubscription( const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, DeltaDummyConfigProviderManager& config_provider_manager) - : DeltaConfigSubscriptionInstance( - "Dummy", manager_identifier, config_provider_manager, factory_context.timeSource(), - factory_context.timeSource().systemTime(), factory_context.localInfo()) {} - -void DeltaDummyConfigSubscription::onConfigUpdate( - const Protobuf::RepeatedPtrField& resources, - const std::string& version_info) { - if (resources.empty()) { - return; - } - - // For simplicity, there is no logic here to track updates and/or removals to the existing config - // proto set (i.e., this is append only). Real xDS APIs will need to track additions, updates and - // removals to the config set and apply the diffs to the underlying config implementations. - for (const auto& resource_any : resources) { - auto dummy_config = TestUtility::anyConvert(resource_any); - proto_map_[version_info] = dummy_config; - // Propagate the new config proto to all worker threads. - applyDeltaConfigUpdate([&dummy_config](const ConfigSharedPtr& config) { - auto* thread_local_dummy_config = static_cast(config.get()); - // Per above, append only for now. - thread_local_dummy_config->addProto(dummy_config); - }); - } - - ConfigSubscriptionCommonBase::onConfigUpdate(); - setLastConfigInfo(absl::optional({absl::nullopt, version_info})); + : DeltaConfigSubscriptionInstance("Dummy", manager_identifier, config_provider_manager, + factory_context) { + initialize( + []() -> ConfigProvider::ConfigConstSharedPtr { return std::make_shared(); }); } class DeltaConfigProviderImplTest : public testing::Test { @@ -721,7 +691,7 @@ TEST_F(DeltaConfigProviderImplTest, MultipleDeltaSubscriptions) { config_source_proto, factory_context_, "dummy_prefix", ConfigProviderManager::NullOptionalArg()); - // Providers, config implementations (i.e., the ThreadLocalDummyConfig) and config protos are + // Providers, config implementations (i.e., the DummyConfig) and config protos are // expected to be shared for a given subscription. EXPECT_EQ(&dynamic_cast(*provider1).subscription(), &dynamic_cast(*provider2).subscription()); @@ -729,18 +699,20 @@ TEST_F(DeltaConfigProviderImplTest, MultipleDeltaSubscriptions) { EXPECT_EQ( provider1->configProtoInfoVector().value().config_protos_, provider2->configProtoInfoVector().value().config_protos_); - EXPECT_EQ(provider1->config().get(), - provider2->config().get()); + EXPECT_EQ(provider1->config().get(), + provider2->config().get()); // Validate that the config protos are propagated to the thread local config implementation. - EXPECT_EQ(provider1->config()->numProtos(), 2); + EXPECT_EQ(provider1->config()->numProtos(), 2); // Issue a second config update to validate that having multiple providers bound to the // subscription causes a single update to the underlying shared config implementation. subscription.onConfigUpdate(untyped_dummy_configs, "2"); - // NOTE: the two providers share the same config, each config update propagation would add 2 - // protos to the same config's proto vector, so the expectation is 6 here. - EXPECT_EQ(provider1->config()->numProtos(), 6); - EXPECT_EQ(provider1->configProtoInfoVector().value().version_, + // NOTE: the config implementation is append only and _does not_ track updates/removals to the + // config proto set, so the expectation is to double the size of the set. + EXPECT_EQ(provider1->config().get(), + provider2->config().get()); + EXPECT_EQ(provider1->config()->numProtos(), 4); + EXPECT_EQ(provider2->configProtoInfoVector().value().version_, "2"); } diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index daa93436b370..34b68537c6f4 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -265,8 +265,8 @@ class RouteConfigProviderManagerImplTest : public RdsTestBase { // Get a RouteConfigProvider. This one should create an entry in the RouteConfigProviderManager. rds_.set_route_config_name("foo_route_config"); rds_.mutable_config_source()->set_path("foo_path"); - provider_ = route_config_provider_manager_->createRdsRouteConfigProvider(rds_, factory_context_, - "foo_prefix."); + provider_ = route_config_provider_manager_->createRdsRouteConfigProvider( + rds_, factory_context_, "foo_prefix.", factory_context_.initManager()); rds_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; } @@ -416,7 +416,7 @@ name: foo_route_config "1"); RouteConfigProviderPtr provider2 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds_, factory_context_, "foo_prefix"); + rds_, factory_context_, "foo_prefix", factory_context_.initManager()); // provider2 should have route config immediately after create EXPECT_TRUE(provider2->configInfo().has_value()); @@ -430,7 +430,7 @@ name: foo_route_config rds2.set_route_config_name("foo_route_config"); rds2.mutable_config_source()->set_path("bar_path"); RouteConfigProviderPtr provider3 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds2, factory_context_, "foo_prefix"); + rds2, factory_context_, "foo_prefix", factory_context_.initManager()); EXPECT_NE(provider3, provider_); factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(route_configs, "provider3"); diff --git a/test/common/router/scoped_config_impl_test.cc b/test/common/router/scoped_config_impl_test.cc index b4bfe4f434a3..5e0f20ce644a 100644 --- a/test/common/router/scoped_config_impl_test.cc +++ b/test/common/router/scoped_config_impl_test.cc @@ -394,7 +394,7 @@ TEST_F(ScopedRouteInfoDeathTest, AssertFailure) { "RouteConfigProvider's name 'bar_route' doesn't match route_configuration_name 'foo_route'."); } -class ThreadLocalScopedConfigImplTest : public testing::Test { +class ScopedConfigImplTest : public testing::Test { public: void SetUp() override { std::string yaml_plain = R"EOF( @@ -445,12 +445,11 @@ class ThreadLocalScopedConfigImplTest : public testing::Test { std::shared_ptr scope_info_a_; std::shared_ptr scope_info_b_; ScopedRoutes::ScopeKeyBuilder key_builder_config_; - std::unique_ptr scoped_config_impl_; + std::unique_ptr scoped_config_impl_; }; -TEST_F(ThreadLocalScopedConfigImplTest, PickRoute) { - scoped_config_impl_ = - std::make_unique(std::move(key_builder_config_)); +TEST_F(ScopedConfigImplTest, PickRoute) { + scoped_config_impl_ = std::make_unique(std::move(key_builder_config_)); scoped_config_impl_->addOrUpdateRoutingScope(scope_info_a_); scoped_config_impl_->addOrUpdateRoutingScope(scope_info_b_); @@ -476,9 +475,8 @@ TEST_F(ThreadLocalScopedConfigImplTest, PickRoute) { EXPECT_EQ(route_config, nullptr); } -TEST_F(ThreadLocalScopedConfigImplTest, Update) { - scoped_config_impl_ = - std::make_unique(std::move(key_builder_config_)); +TEST_F(ScopedConfigImplTest, Update) { + scoped_config_impl_ = std::make_unique(std::move(key_builder_config_)); TestHeaderMapImpl headers{ {"foo_header", ",,key=value,bar=foo,"}, diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index 323100f264fb..a5b767f8d873 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -53,11 +53,11 @@ class ScopedRoutesTestBase : public testing::Test { EXPECT_CALL(factory_context_.admin_.config_tracker_, add_("route_scopes", _)); config_provider_manager_ = std::make_unique( factory_context_.admin_, route_config_provider_manager_); - EXPECT_CALL(route_config_provider_manager_, createRdsRouteConfigProvider(_, _, _)) + EXPECT_CALL(route_config_provider_manager_, createRdsRouteConfigProvider(_, _, _, _)) .WillRepeatedly(Invoke( [this](const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, - Server::Configuration::FactoryContext&, - const std::string&) -> RouteConfigProviderPtr { + Server::Configuration::FactoryContext&, const std::string&, + Init::Manager&) -> RouteConfigProviderPtr { auto iter = cached_route_configs_.find(rds.route_config_name()); if (iter == cached_route_configs_.end()) { cached_route_configs_[rds.route_config_name()] = std::make_shared(); diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index c5bee0ccb920..4a3e3099e0c1 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -47,12 +47,5 @@ MockGrpcMuxCallbacks::MockGrpcMuxCallbacks() { MockGrpcMuxCallbacks::~MockGrpcMuxCallbacks() = default; -MockMutableConfigProviderBase::MockMutableConfigProviderBase( - std::shared_ptr&& subscription, - ConfigProvider::ConfigConstSharedPtr, Server::Configuration::FactoryContext& factory_context) - : MutableConfigProviderBase(std::move(subscription), factory_context, ApiType::Full) { - subscription_->bindConfigProvider(this); -} - } // namespace Config } // namespace Envoy diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 8396fff4fc31..fbcd624a1e1a 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -109,20 +109,6 @@ class MockGrpcStreamCallbacks : public GrpcStreamCallbacks&& subscription, - ConfigProvider::ConfigConstSharedPtr initial_config, - Server::Configuration::FactoryContext& factory_context); - - MOCK_CONST_METHOD0(getConfig, ConfigConstSharedPtr()); - MOCK_METHOD1(onConfigProtoUpdate, ConfigConstSharedPtr(const Protobuf::Message& config_proto)); - MOCK_METHOD1(initialize, void(const ConfigConstSharedPtr& initial_config)); - MOCK_METHOD1(onConfigUpdate, void(const ConfigConstSharedPtr& config)); - - ConfigSubscriptionCommonBase& subscription() { return *subscription_.get(); } -}; - class MockConfigProviderManager : public ConfigProviderManager { public: MockConfigProviderManager() = default; diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index 0c9113c13788..ffd1d375f86f 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -400,11 +400,11 @@ class MockRouteConfigProviderManager : public RouteConfigProviderManager { MockRouteConfigProviderManager(); ~MockRouteConfigProviderManager(); - MOCK_METHOD3(createRdsRouteConfigProvider, + MOCK_METHOD4(createRdsRouteConfigProvider, RouteConfigProviderPtr( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Server::Configuration::FactoryContext& factory_context, - const std::string& stat_prefix)); + const std::string& stat_prefix, Init::Manager& init_manager)); MOCK_METHOD2(createStaticRouteConfigProvider, RouteConfigProviderPtr(const envoy::api::v2::RouteConfiguration& route_config, Server::Configuration::FactoryContext& factory_context));