merge PR envoyproxy#7704, add init_manager injection when creating Rd…

…sRouteConfigProvider Signed-off-by: Xin Zhuang <stevenzzz@google.com>
stevenzzzz · Jul 25, 2019 · c354a1e · c354a1e
1 parent fdd0a74
commit c354a1e
Show file tree

Hide file tree

Showing 17 changed files with 321 additions and 503 deletions.
diff --git 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
@@ -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<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, 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<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.
-  // 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<DeltaMutableConfigProviderBase*>(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_ =