move route config provider manager to singleton manager registry

include/envoy/server/BUILD

@@ -63,7 +63,6 @@ envoy_cc_library(
         "//include/envoy/init:init_interface",
         "//include/envoy/local_info:local_info_interface",
         "//include/envoy/ratelimit:ratelimit_interface",
-        "//include/envoy/router:route_config_provider_manager_interface",
         "//include/envoy/runtime:runtime_interface",
         "//include/envoy/ssl:context_manager_interface",
         "//include/envoy/thread_local:thread_local_interface",
@@ -107,7 +106,6 @@ envoy_cc_library(
         "//include/envoy/local_info:local_info_interface",
         "//include/envoy/network:drain_decision_interface",
         "//include/envoy/ratelimit:ratelimit_interface",
-        "//include/envoy/router:route_config_provider_manager_interface",
         "//include/envoy/runtime:runtime_interface",
         "//include/envoy/singleton:manager_interface",
         "//include/envoy/thread_local:thread_local_interface",

include/envoy/server/filter_config.h

@@ -9,7 +9,6 @@
 #include "envoy/network/drain_decision.h"
 #include "envoy/network/filter.h"
 #include "envoy/ratelimit/ratelimit.h"
-#include "envoy/router/route_config_provider_manager.h"
 #include "envoy/runtime/runtime.h"
 #include "envoy/singleton/manager.h"
 #include "envoy/thread_local/thread_local.h"
@@ -117,11 +116,6 @@ class FactoryContext {
    *         used to allow runtime lockless updates to configuration, etc. across multiple threads.
    */
   virtual ThreadLocal::SlotAllocator& threadLocal() PURE;
-
-  /**
-   * @return RouteConfigProviderManager& singleton for use by the entire server.
-   */
-  virtual Router::RouteConfigProviderManager& routeConfigProviderManager() PURE;
 };
 
 enum class NetworkFilterType { Read, Write, Both };

include/envoy/server/instance.h

@@ -10,7 +10,6 @@
 #include "envoy/local_info/local_info.h"
 #include "envoy/network/listen_socket.h"
 #include "envoy/ratelimit/ratelimit.h"
-#include "envoy/router/route_config_provider_manager.h"
 #include "envoy/runtime/runtime.h"
 #include "envoy/server/admin.h"
 #include "envoy/server/drain_manager.h"
@@ -180,11 +179,6 @@ class Instance {
    * @return information about the local environment the server is running in.
    */
   virtual const LocalInfo::LocalInfo& localInfo() PURE;
-
-  /**
-   * @return the server's http route manager.
-   */
-  virtual Router::ServerRouteConfigProviderManager& routeConfigProviderManager() PURE;
 };
 
 } // namespace Server

include/envoy/singleton/instance.h

@@ -11,7 +11,7 @@ class Instance {
   virtual ~Instance() {}
 };
 
-typedef std::shared_ptr<Instance> InstancePtr;
+typedef std::shared_ptr<Instance> InstanceSharedPtr;
 
 } // namespace Singleton
 } // namespace Envoy

include/envoy/singleton/manager.h

@@ -36,10 +36,24 @@ template <const char* name_param> class RegistrationImpl : public Registration {
   std::string name() override { return name_param; }
 };
 
+/**
+ * Macro used to statically register singletons managed by the singleton manager
+ * defined in envoy/singleton/manager.h. After the NAME has been registered use the
+ * SINGLETON_MANAGER_REGISTERED_NAME macro to access the name registered with the
+ * singleton manager.
+ */
+#define SINGLETON_MANAGER_REGISTRATION(NAME)                                                       \
+  static constexpr char NAME##_singleton_name[] = #NAME "_singleton";                              \
+  static Registry::RegisterFactory<Singleton::RegistrationImpl<NAME##_singleton_name>,             \
+                                   Singleton::Registration>                                        \
+      NAME##_singleton_registered_;
+
+#define SINGLETON_MANAGER_REGISTERED_NAME(NAME) NAME##_singleton_name
+
 /**
  * Callback function used to create a singleton.
  */
-typedef std::function<InstancePtr()> SingletonFactoryCb;
+typedef std::function<InstanceSharedPtr()> SingletonFactoryCb;
 
 /**
  * A manager for all server-side singletons.
@@ -66,7 +80,7 @@ class Manager {
    *        singletons must store the shared_ptr for as long as the singleton is needed.
    * @return InstancePtr the singleton.
    */
-  virtual InstancePtr get(const std::string& name, SingletonFactoryCb) PURE;
+  virtual InstanceSharedPtr get(const std::string& name, SingletonFactoryCb) PURE;
 };
 
 typedef std::unique_ptr<Manager> ManagerPtr;

source/common/router/BUILD

@@ -64,6 +64,7 @@ envoy_cc_library(
         "//include/envoy/local_info:local_info_interface",
         "//include/envoy/router:rds_interface",
         "//include/envoy/router:route_config_provider_manager_interface",
+        "//include/envoy/singleton:instance_interface",
         "//include/envoy/thread_local:thread_local_interface",
         "//source/common/common:assert_lib",
         "//source/common/common:logger_lib",

source/common/router/rds_impl.h

@@ -10,6 +10,7 @@
 #include "envoy/local_info/local_info.h"
 #include "envoy/router/rds.h"
 #include "envoy/router/route_config_provider_manager.h"
+#include "envoy/singleton/instance.h"
 #include "envoy/thread_local/thread_local.h"
 
 #include "common/common/logger.h"
@@ -126,8 +127,8 @@ class RdsRouteConfigProviderImpl
   friend class RouteConfigProviderManagerImpl;
 };
 
-// TODO(junr03): use then singleton manager in #1410.
-class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager {
+class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager,
+                                       public Singleton::Instance {
 public:
   RouteConfigProviderManagerImpl(Runtime::Loader& runtime, Event::Dispatcher& dispatcher,
                                  Runtime::RandomGenerator& random,

source/common/singleton/manager_impl.cc

@@ -7,14 +7,14 @@
 namespace Envoy {
 namespace Singleton {
 
-InstancePtr ManagerImpl::get(const std::string& name, SingletonFactoryCb cb) {
+InstanceSharedPtr ManagerImpl::get(const std::string& name, SingletonFactoryCb cb) {
   ASSERT(run_tid_ == Thread::Thread::currentThreadId());
   if (nullptr == Registry::FactoryRegistry<Registration>::getFactory(name)) {
     PANIC(fmt::format("invalid singleton name '{}'. Make sure it is registered.", name));
   }
 
   if (nullptr == singletons_[name].lock()) {
-    InstancePtr singleton = cb();
+    InstanceSharedPtr singleton = cb();
     singletons_[name] = singleton;
     return singleton;
   } else {

source/common/singleton/manager_impl.h

@@ -19,7 +19,7 @@ class ManagerImpl : public Manager {
   ManagerImpl() : run_tid_(Thread::Thread::currentThreadId()) {}
 
   // Singleton::Manager
-  InstancePtr get(const std::string& name, SingletonFactoryCb cb) override;
+  InstanceSharedPtr get(const std::string& name, SingletonFactoryCb cb) override;
 
 private:
   std::unordered_map<std::string, std::weak_ptr<Instance>> singletons_;

source/server/config/network/BUILD

@@ -37,6 +37,7 @@ envoy_cc_library(
         "//include/envoy/filesystem:filesystem_interface",
         "//include/envoy/http:filter_interface",
         "//include/envoy/registry",
+        "//include/envoy/router:route_config_provider_manager_interface",
         "//include/envoy/server:filter_config_interface",
         "//include/envoy/server:options_interface",
         "//include/envoy/stats:stats_interface",

source/server/config/network/http_connection_manager.cc

@@ -29,26 +29,38 @@ namespace Configuration {
 
 const std::string HttpConnectionManagerConfig::DEFAULT_SERVER_STRING = "envoy";
 
-static constexpr char date_provider_singleton_name[] = "date_provider_singleton";
-static Registry::RegisterFactory<Singleton::RegistrationImpl<date_provider_singleton_name>,
-                                 Singleton::Registration>
-    date_provider_singleton_registered_;
+// Singleton registration via macro defined in envoy/singleton/manager.h
+SINGLETON_MANAGER_REGISTRATION(date_provider);
+SINGLETON_MANAGER_REGISTRATION(route_config_provider_manager);
 
 NetworkFilterFactoryCb HttpConnectionManagerFilterConfigFactory::createFilterFactory(
     const Json::Object& json_http_connection_manager, FactoryContext& context) {
   std::shared_ptr<Http::TlsCachingDateProviderImpl> date_provider =
       context.singletonManager().getTyped<Http::TlsCachingDateProviderImpl>(
-          date_provider_singleton_name, [&context] {
+          SINGLETON_MANAGER_REGISTERED_NAME(date_provider), [&context] {
             return std::make_shared<Http::TlsCachingDateProviderImpl>(context.dispatcher(),
                                                                       context.threadLocal());
           });
 
+  std::shared_ptr<Router::RouteConfigProviderManager> route_config_provider_manager =
+      context.singletonManager().getTyped<Router::RouteConfigProviderManager>(
+          SINGLETON_MANAGER_REGISTERED_NAME(route_config_provider_manager), [&context] {
+            return std::make_shared<Router::RouteConfigProviderManagerImpl>(
+                context.runtime(), context.dispatcher(), context.random(), context.localInfo(),
+                context.threadLocal());
+          });
+
   envoy::api::v2::filter::HttpConnectionManager http_connection_manager;
   Config::FilterJson::translateHttpConnectionManager(json_http_connection_manager,
                                                      http_connection_manager);
-  std::shared_ptr<HttpConnectionManagerConfig> http_config(
-      new HttpConnectionManagerConfig(http_connection_manager, context, *date_provider));
-  return [http_config, &context, date_provider](Network::FilterManager& filter_manager) -> void {
+  std::shared_ptr<HttpConnectionManagerConfig> http_config(new HttpConnectionManagerConfig(
+      http_connection_manager, context, *date_provider, *route_config_provider_manager));
+
+  // This lambda captures the shared_ptrs created above, thus preserving the
+  // reference count. Moreover, keep in mind the capture list determines
+  // destruction order.
+  return [route_config_provider_manager, http_config, &context,
+          date_provider](Network::FilterManager& filter_manager) -> void {
     filter_manager.addReadFilter(Network::ReadFilterSharedPtr{new Http::ConnectionManagerImpl(
         *http_config, context.drainDecision(), context.random(), context.httpTracer(),
         context.runtime(), context.localInfo(), context.clusterManager())});
@@ -82,21 +94,24 @@ HttpConnectionManagerConfigUtility::determineNextProtocol(Network::Connection& c
 
 HttpConnectionManagerConfig::HttpConnectionManagerConfig(
     const envoy::api::v2::filter::HttpConnectionManager& config, FactoryContext& context,
-    Http::DateProvider& date_provider)
+    Http::DateProvider& date_provider,
+    Router::RouteConfigProviderManager& route_config_provider_manager)
     : context_(context), stats_prefix_(fmt::format("http.{}.", config.stat_prefix())),
       stats_(Http::ConnectionManagerImpl::generateStats(stats_prefix_, context_.scope())),
       tracing_stats_(
           Http::ConnectionManagerImpl::generateTracingStats(stats_prefix_, context_.scope())),
       use_remote_address_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, use_remote_address, false)),
+      route_config_provider_manager_(route_config_provider_manager),
       http2_settings_(Http::Utility::parseHttp2Settings(config.http2_protocol_options())),
       http1_settings_(Http::Utility::parseHttp1Settings(config.http_protocol_options())),
       drain_timeout_(PROTOBUF_GET_MS_OR_DEFAULT(config, drain_timeout, 5000)),
       generate_request_id_(PROTOBUF_GET_WRAPPED_OR_DEFAULT(config, generate_request_id, true)),
+
       date_provider_(date_provider) {
 
   route_config_provider_ = Router::RouteConfigProviderUtil::create(
       config, context_.runtime(), context_.clusterManager(), context_.scope(), stats_prefix_,
-      context_.initManager(), context_.routeConfigProviderManager());
+      context_.initManager(), route_config_provider_manager_);
 
   switch (config.forward_client_cert_details()) {
   case envoy::api::v2::filter::HttpConnectionManager::SANITIZE:

source/server/config/network/http_connection_manager.h

@@ -7,6 +7,7 @@
 #include <string>
 
 #include "envoy/http/filter.h"
+#include "envoy/router/route_config_provider_manager.h"
 #include "envoy/server/filter_config.h"
 
 #include "common/common/logger.h"
@@ -67,7 +68,8 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
                                     public Http::ConnectionManagerConfig {
 public:
   HttpConnectionManagerConfig(const envoy::api::v2::filter::HttpConnectionManager& config,
-                              FactoryContext& context, Http::DateProvider& date_provider);
+                              FactoryContext& context, Http::DateProvider& date_provider,
+                              Router::RouteConfigProviderManager& route_config_provider_manager);
 
   // Http::FilterChainFactory
   void createFilterChain(Http::FilterChainFactoryCallbacks& callbacks) override;
@@ -134,6 +136,7 @@ class HttpConnectionManagerConfig : Logger::Loggable<Logger::Id::config>,
   const bool use_remote_address_{};
   Http::ForwardClientCertType forward_client_cert_;
   std::vector<Http::ClientCertDetailsType> set_current_client_cert_details_;
+  Router::RouteConfigProviderManager& route_config_provider_manager_;
   CodecType codec_type_;
   const Http::Http2Settings http2_settings_;
   const Http::Http1Settings http1_settings_;

source/server/config_validation/server.cc

@@ -73,8 +73,6 @@ void ValidationInstance::initialize(Options& options,
   thread_local_.registerThread(*dispatcher_, true);
   runtime_loader_ = component_factory.createRuntime(*this, initial_config);
   ssl_context_manager_.reset(new Ssl::ContextManagerImpl(*runtime_loader_));
-  route_config_provider_manager_.reset(new Router::RouteConfigProviderManagerImpl(
-      runtime(), dispatcher(), random(), localInfo(), threadLocal()));
   cluster_manager_factory_.reset(new Upstream::ValidationClusterManagerFactory(
       runtime(), stats(), threadLocal(), random(), dnsResolver(), sslContextManager(), dispatcher(),
       localInfo()));

source/server/config_validation/server.h

@@ -86,9 +86,6 @@ class ValidationInstance : Logger::Loggable<Logger::Id::main>,
   Tracing::HttpTracer& httpTracer() override { return config_->httpTracer(); }
   ThreadLocal::Instance& threadLocal() override { return thread_local_; }
   const LocalInfo::LocalInfo& localInfo() override { return *local_info_; }
-  Router::ServerRouteConfigProviderManager& routeConfigProviderManager() override {
-    return *route_config_provider_manager_;
-  }
 
   // Server::ListenerComponentFactory
   std::vector<Configuration::NetworkFilterFactoryCb>
@@ -131,7 +128,6 @@ class ValidationInstance : Logger::Loggable<Logger::Id::main>,
   AccessLog::AccessLogManagerImpl access_log_manager_;
   std::unique_ptr<Upstream::ValidationClusterManagerFactory> cluster_manager_factory_;
   InitManagerImpl init_manager_;
-  std::unique_ptr<Router::RouteConfigProviderManagerImpl> route_config_provider_manager_;
   ListenerManagerImpl listener_manager_;
 };
 

source/server/listener_manager_impl.h

@@ -222,9 +222,6 @@ class ListenerImpl : public Listener,
   Stats::Scope& scope() override { return *global_scope_; }
   Singleton::Manager& singletonManager() override { return parent_.server_.singletonManager(); }
   ThreadLocal::Instance& threadLocal() override { return parent_.server_.threadLocal(); }
-  Router::RouteConfigProviderManager& routeConfigProviderManager() override {
-    return parent_.server_.routeConfigProviderManager();
-  }
 
   // Network::DrainDecision
   bool drainClose() const override;

source/server/server.cc

@@ -201,9 +201,6 @@ void InstanceImpl::initialize(Options& options,
       runtime(), stats(), threadLocal(), random(), dnsResolver(), sslContextManager(), dispatcher(),
       localInfo()));
 
-  route_config_provider_manager_.reset(new Router::RouteConfigProviderManagerImpl(
-      runtime(), dispatcher(), random(), localInfo(), threadLocal()));
-
   // Now the configuration gets parsed. The configuration may start setting thread local data
   // per above. See MainImpl::initialize() for why we do this pointer dance.
   Configuration::MainImpl* main_config = new Configuration::MainImpl();

source/server/server.h

@@ -133,9 +133,6 @@ class InstanceImpl : Logger::Loggable<Logger::Id::main>, public Instance {
   Tracing::HttpTracer& httpTracer() override;
   ThreadLocal::Instance& threadLocal() override { return thread_local_; }
   const LocalInfo::LocalInfo& localInfo() override { return *local_info_; }
-  Router::ServerRouteConfigProviderManager& routeConfigProviderManager() override {
-    return *route_config_provider_manager_;
-  }
 
 private:
   void flushStats();

test/mocks/server/mocks.cc

@@ -102,8 +102,6 @@ MockInstance::MockInstance()
   ON_CALL(*this, drainManager()).WillByDefault(ReturnRef(drain_manager_));
   ON_CALL(*this, initManager()).WillByDefault(ReturnRef(init_manager_));
   ON_CALL(*this, listenerManager()).WillByDefault(ReturnRef(listener_manager_));
-  ON_CALL(*this, routeConfigProviderManager())
-      .WillByDefault(ReturnRef(route_config_provider_manager_));
   ON_CALL(*this, singletonManager()).WillByDefault(ReturnRef(*singleton_manager_));
 }
 
@@ -133,8 +131,6 @@ MockFactoryContext::MockFactoryContext() : singleton_manager_(new Singleton::Man
   ON_CALL(*this, server()).WillByDefault(ReturnRef(server_));
   ON_CALL(*this, singletonManager()).WillByDefault(ReturnRef(*singleton_manager_));
   ON_CALL(*this, threadLocal()).WillByDefault(ReturnRef(thread_local_));
-  ON_CALL(*this, routeConfigProviderManager())
-      .WillByDefault(ReturnRef(route_config_provider_manager_));
 }
 
 MockFactoryContext::~MockFactoryContext() {}

test/mocks/server/mocks.h

@@ -267,7 +267,6 @@ class MockInstance : public Instance {
   MOCK_METHOD0(httpTracer, Tracing::HttpTracer&());
   MOCK_METHOD0(threadLocal, ThreadLocal::Instance&());
   MOCK_METHOD0(localInfo, const LocalInfo::LocalInfo&());
-  MOCK_METHOD0(routeConfigProviderManager, Router::ServerRouteConfigProviderManager&());
 
   testing::NiceMock<ThreadLocal::MockInstance> thread_local_;
   Stats::IsolatedStoreImpl stats_store_;
@@ -288,7 +287,6 @@ class MockInstance : public Instance {
   testing::NiceMock<Runtime::MockRandomGenerator> random_;
   testing::NiceMock<LocalInfo::MockLocalInfo> local_info_;
   testing::NiceMock<Init::MockManager> init_manager_;
-  testing::NiceMock<Router::MockRouteConfigProviderManager> route_config_provider_manager_;
   testing::NiceMock<MockListenerManager> listener_manager_;
   Singleton::ManagerPtr singleton_manager_;
 };
@@ -342,11 +340,9 @@ class MockFactoryContext : public FactoryContext {
   MOCK_METHOD0(server, Instance&());
   MOCK_METHOD0(singletonManager, Singleton::Manager&());
   MOCK_METHOD0(threadLocal, ThreadLocal::Instance&());
-  MOCK_METHOD0(routeConfigProviderManager, Router::RouteConfigProviderManager&());
 
   testing::NiceMock<AccessLog::MockAccessLogManager> access_log_manager_;
   testing::NiceMock<Upstream::MockClusterManager> cluster_manager_;
-  testing::NiceMock<Router::MockRouteConfigProviderManager> route_config_provider_manager_;
   testing::NiceMock<Event::MockDispatcher> dispatcher_;
   testing::NiceMock<MockDrainManager> drain_manager_;
   testing::NiceMock<Tracing::MockHttpTracer> http_tracer_;

test/server/config/network/BUILD

@@ -31,6 +31,7 @@ envoy_cc_test(
         "//source/common/buffer:buffer_lib",
         "//source/common/config:filter_json_lib",
         "//source/common/event:dispatcher_lib",
+        "//source/common/router:rds_lib",
         "//source/server/config/http:dynamo_lib",
         "//source/server/config/network:http_connection_manager_lib",
         "//test/mocks/network:network_mocks",