refactor config provider framework

Signed-off-by: Xin Zhuang <stevenzzz@google.com>

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,49 +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<MutableConfigProviderBase*>(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<void(const ConfigSharedPtr&)>& update_fn) {
-  // 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<DeltaMutableConfigProviderBase*>(*mutable_config_providers_.begin())
-               ->getConfig() == static_cast<DeltaMutableConfigProviderBase*>(
-                                    *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.
-  auto* typed_provider =
-      static_cast<DeltaMutableConfigProviderBase*>(getAnyBoundMutableConfigProvider());
-  ConfigSharedPtr config = typed_provider->getConfig();
-  typed_provider->onConfigUpdate([config, update_fn]() { update_fn(config); });
-}
-
 ConfigProviderManagerImplBase::ConfigProviderManagerImplBase(Server::Admin& admin,
                                                              const std::string& config_name) {
   config_tracker_entry_ =

source/common/router/BUILD

@@ -175,6 +175,7 @@ envoy_cc_library(
     hdrs = ["scoped_rds.h"],
     deps = [
         ":scoped_config_lib",
+        "//include/envoy/config:config_provider_interface",
         "//include/envoy/config:subscription_interface",
         "//include/envoy/stats:stats_interface",
         "//source/common/common:assert_lib",

source/common/router/scoped_config_impl.h

@@ -141,7 +141,7 @@ class ScopeKeyBuilderImpl : public ScopeKeyBuilderBase {
  * ConnectionManagerImpl::refreshCachedRoute() will call getRouterConfig() to obtain the
  * Router::ConfigConstSharedPtr to use for route selection.
  */
-class ThreadLocalScopedConfigImpl : public ScopedConfig, public ThreadLocal::ThreadLocalObject {
+class ThreadLocalScopedConfigImpl : public ScopedConfig {
 public:
   ThreadLocalScopedConfigImpl(ScopedRoutes::ScopeKeyBuilder&& scope_key_builder)
       : scope_key_builder_(std::move(scope_key_builder)) {}

source/common/router/scoped_rds.cc

@@ -76,12 +76,13 @@ 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,
     ScopedRoutesConfigProviderManager& config_provider_manager)
-    : DeltaConfigSubscriptionInstance(
-          "SRDS", manager_identifier, config_provider_manager, factory_context.timeSource(),
-          factory_context.timeSource().systemTime(), factory_context.localInfo()),
-      name_(name),
+    : DeltaConfigSubscriptionInstance("SRDS", manager_identifier, config_provider_manager,
+                                      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_))}),
       validation_visitor_(factory_context.messageValidationVisitor()) {
@@ -91,6 +92,12 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription(
           Grpc::Common::typeUrl(
               envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()),
           *scope_, *this);
+
+  initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr {
+    return std::make_shared<ThreadLocalScopedConfigImpl>(
+        envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder(
+            scope_key_builder));
+  });
 }
 
 void ScopedRdsConfigSubscription::onConfigUpdate(
@@ -124,46 +131,41 @@ void ScopedRdsConfigSubscription::onConfigUpdate(
     ScopedRouteInfoConstSharedPtr scoped_route_info =
         scoped_config_manager_.addOrUpdateRoutingScope(scoped_route, version_info);
     ENVOY_LOG(debug, "srds: add/update scoped_route '{}'", scoped_route_name);
-    applyDeltaConfigUpdate([scoped_route_info](const ConfigProvider::ConfigConstSharedPtr& config) {
+    applyConfigUpdate([scoped_route_info](const ConfigProvider::ConfigConstSharedPtr& config)
+                          -> ConfigProvider::ConfigConstSharedPtr {
       auto* thread_local_scoped_config = const_cast<ThreadLocalScopedConfigImpl*>(
           static_cast<const ThreadLocalScopedConfigImpl*>(config.get()));
       thread_local_scoped_config->addOrUpdateRoutingScope(scoped_route_info);
+      return config;
     });
   }
 
   for (const auto& scoped_route : scoped_routes_to_remove) {
     const std::string scoped_route_name = scoped_route.first;
     ENVOY_LOG(debug, "srds: remove scoped route '{}'", scoped_route_name);
     scoped_config_manager_.removeRoutingScope(scoped_route_name);
-    applyDeltaConfigUpdate([scoped_route_name](const ConfigProvider::ConfigConstSharedPtr& config) {
+    applyConfigUpdate([scoped_route_name](const ConfigProvider::ConfigConstSharedPtr& config)
+                          -> ConfigProvider::ConfigConstSharedPtr {
+      // In place update.
       auto* thread_local_scoped_config = const_cast<ThreadLocalScopedConfigImpl*>(
           static_cast<const ThreadLocalScopedConfigImpl*>(config.get()));
       thread_local_scoped_config->removeRoutingScope(scoped_route_name);
+      return config;
     });
   }
 
-  ConfigSubscriptionCommonBase::onConfigUpdate();
+  DeltaConfigSubscriptionInstance::onConfigUpdate();
   setLastConfigInfo(absl::optional<LastConfigInfo>({absl::nullopt, version_info}));
   stats_.config_reload_.inc();
 }
 
 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),
+    envoy::api::v2::core::ConfigSource rds_config_source)
+    : MutableConfigProviderCommonBase(std::move(subscription), ConfigProvider::ApiType::Delta),
       subscription_(static_cast<ScopedRdsConfigSubscription*>(
           MutableConfigProviderCommonBase::subscription_.get())),
-      rds_config_source_(std::move(rds_config_source)) {
-  initialize([scope_key_builder](Event::Dispatcher&) -> ThreadLocal::ThreadLocalObjectSharedPtr {
-    return std::make_shared<ThreadLocalScopedConfigImpl>(
-        envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder(
-            scope_key_builder));
-  });
-}
+      rds_config_source_(std::move(rds_config_source)) {}
 
 ProtobufTypes::MessagePtr ScopedRoutesConfigProviderManager::dumpConfigs() const {
   auto config_dump = std::make_unique<envoy::admin::v2alpha::ScopedRoutesConfigDump>();
@@ -207,28 +209,25 @@ 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<const ScopedRoutesConfigProviderManagerOptArg&>(optarg);
   ScopedRdsConfigSubscriptionSharedPtr subscription =
       ConfigProviderManagerImplBase::getSubscription<ScopedRdsConfigSubscription>(
           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<ScopedRdsConfigSubscription>(
-                scoped_rds_config_source, manager_identifier,
-                static_cast<const ScopedRoutesConfigProviderManagerOptArg&>(optarg)
-                    .scoped_routes_name_,
-                factory_context, stat_prefix,
+                scoped_rds_config_source, manager_identifier, typed_optarg.scoped_routes_name_,
+                typed_optarg.scope_key_builder_, factory_context, stat_prefix,
                 static_cast<ScopedRoutesConfigProviderManager&>(config_provider_manager));
           });
 
-  const auto& typed_optarg = static_cast<const ScopedRoutesConfigProviderManagerOptArg&>(optarg);
-  return std::make_unique<ScopedRdsConfigProvider>(std::move(subscription), factory_context,
-                                                   typed_optarg.rds_config_source_,
-                                                   typed_optarg.scope_key_builder_);
+  return std::make_unique<ScopedRdsConfigProvider>(std::move(subscription),
+                                                   typed_optarg.rds_config_source_);
 }
 
 ConfigProviderPtr ScopedRoutesConfigProviderManager::createStaticConfigProvider(

source/common/router/scoped_rds.h

@@ -86,6 +86,8 @@ 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,
       ScopedRoutesConfigProviderManager& config_provider_manager);
 
@@ -98,7 +100,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio
   }
 
 private:
-  // Envoy::Config::ConfigSubscriptionCommonBase
+  // Envoy::Config::DeltaConfigSubscriptionInstance
   void start() override { subscription_->start({}); }
 
   // Envoy::Config::SubscriptionCallbacks
@@ -109,7 +111,7 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio
     NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
   }
   void onConfigUpdateFailed(const EnvoyException*) override {
-    ConfigSubscriptionCommonBase::onConfigUpdateFailed();
+    DeltaConfigSubscriptionInstance::onConfigUpdateFailed();
   }
   std::string resourceName(const ProtobufWkt::Any& resource) override {
     return MessageUtil::anyConvert<envoy::api::v2::ScopedRouteConfiguration>(resource,
@@ -119,6 +121,8 @@ class ScopedRdsConfigSubscription : public Envoy::Config::DeltaConfigSubscriptio
 
   const std::string name_;
   std::unique_ptr<Envoy::Config::Subscription> subscription_;
+  const envoy::config::filter::network::http_connection_manager::v2::ScopedRoutes::ScopeKeyBuilder
+      scope_key_builder_;
   Stats::ScopePtr scope_;
   ScopedRdsStats stats_;
   ScopedConfigManager scoped_config_manager_;
@@ -129,25 +133,13 @@ using ScopedRdsConfigSubscriptionSharedPtr = std::shared_ptr<ScopedRdsConfigSubs
 
 // 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 *subscription_; }
 
-  // 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 {
-    return std::dynamic_pointer_cast<Envoy::Config::ConfigProvider::Config>(tls_->get());
-  }
-
 private:
   ScopedRdsConfigSubscription* subscription_;
   const envoy::api::v2::core::ConfigSource rds_config_source_;