xds: refactor ADS related data to xDS-Manager part 2

This PR moves the xDS-Config-Tracker from the cluster-manager to the
xds-manager.
Note that in this PR we add a getter for the xDS-Config-Tracker, but
that will be removed in the future when the rest of the xDS-related
components are moved from the cluster-manager to the xds-manager.

Signed-off-by: Adi Suissa-Peleg <adip@google.com>

envoy/config/BUILD

@@ -160,6 +160,7 @@ envoy_cc_library(
     name = "xds_manager_interface",
     hdrs = ["xds_manager.h"],
     deps = [
+        ":xds_config_tracker_interface",
         "//envoy/upstream:cluster_manager_interface",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],

envoy/config/xds_manager.h

@@ -2,6 +2,7 @@
 
 #include "envoy/common/pure.h"
 #include "envoy/config/core/v3/config_source.pb.h"
+#include "envoy/config/xds_config_tracker.h"
 #include "envoy/upstream/cluster_manager.h"
 
 #include "absl/status/status.h"
@@ -26,10 +27,12 @@ class XdsManager {
   /**
    * Initializes the xDS-Manager.
    * This should be called after the cluster-manager is created.
+   * @param boostrap - the bootstrap config of Envoy.
    * @param cm - a pointer to a valid cluster manager.
    * @return Ok if the initialization was successful, or an error otherwise.
    */
-  virtual absl::Status initialize(Upstream::ClusterManager* cm) PURE;
+  virtual absl::Status initialize(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                                  Upstream::ClusterManager* cm) PURE;
 
   /**
    * Shuts down the xDS-Manager and all the configured connections to the config
@@ -46,6 +49,15 @@ class XdsManager {
    */
   virtual absl::Status
   setAdsConfigSource(const envoy::config::core::v3::ApiConfigSource& config_source) PURE;
+
+  /**
+   * Returns the XdsConfigTracker if defined by the bootstrap.
+   * The object will be initialized (if configured) after the call to initialize().
+   * TODO(adisuissa): this method will be removed once all the ADS-related objects
+   * are moved out of the cluster-manager to the xds-manager.
+   * @return the XdsConfigTracker if defined, or nullopt if not.
+   */
+  virtual OptRef<Config::XdsConfigTracker> xdsConfigTracker() PURE;
 };
 
 using XdsManagerPtr = std::unique_ptr<XdsManager>;

source/common/config/BUILD

@@ -305,6 +305,7 @@ envoy_cc_library(
     srcs = ["xds_manager_impl.cc"],
     hdrs = ["xds_manager_impl.h"],
     deps = [
+        ":utility_lib",
         "//envoy/config:xds_manager_interface",
         "//envoy/upstream:cluster_manager_interface",
         "//source/common/common:thread_lib",

source/common/config/xds_manager_impl.cc

@@ -3,13 +3,24 @@
 #include "envoy/config/core/v3/config_source.pb.validate.h"
 
 #include "source/common/common/thread.h"
+#include "source/common/config/utility.h"
 
 namespace Envoy {
 namespace Config {
 
-absl::Status XdsManagerImpl::initialize(Upstream::ClusterManager* cm) {
+absl::Status XdsManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                                        Upstream::ClusterManager* cm) {
   ASSERT(cm != nullptr);
   cm_ = cm;
+
+  if (bootstrap.has_xds_config_tracker_extension()) {
+    auto& tracker_factory = Config::Utility::getAndCheckFactory<Config::XdsConfigTrackerFactory>(
+        bootstrap.xds_config_tracker_extension());
+    xds_config_tracker_ = tracker_factory.createXdsConfigTracker(
+        bootstrap.xds_config_tracker_extension().typed_config(),
+        validation_context_.dynamicValidationVisitor(), api_, main_thread_dispatcher_);
+  }
+
   return absl::OkStatus();
 }
 

source/common/config/xds_manager_impl.h

@@ -9,20 +9,29 @@ namespace Config {
 
 class XdsManagerImpl : public XdsManager {
 public:
-  XdsManagerImpl(ProtobufMessage::ValidationContext& validation_context)
-      : validation_context_(validation_context) {}
+  XdsManagerImpl(Event::Dispatcher& main_thread_dispatcher, Api::Api& api,
+                 ProtobufMessage::ValidationContext& validation_context)
+      : main_thread_dispatcher_(main_thread_dispatcher), api_(api),
+        validation_context_(validation_context) {}
 
   // Config::ConfigSourceProvider
-  absl::Status initialize(Upstream::ClusterManager* cm) override;
+  absl::Status initialize(const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+                          Upstream::ClusterManager* cm) override;
   void shutdown() override {}
   absl::Status
   setAdsConfigSource(const envoy::config::core::v3::ApiConfigSource& config_source) override;
+  OptRef<Config::XdsConfigTracker> xdsConfigTracker() override {
+    return makeOptRefFromPtr<Config::XdsConfigTracker>(xds_config_tracker_.get());
+  }
 
 private:
   // Validates (syntactically) the config_source by doing the PGV validation.
   absl::Status validateAdsConfig(const envoy::config::core::v3::ApiConfigSource& config_source);
 
+  Event::Dispatcher& main_thread_dispatcher_;
+  Api::Api& api_;
   ProtobufMessage::ValidationContext& validation_context_;
+  Config::XdsConfigTrackerPtr xds_config_tracker_;
   // The cm_ will only be valid after the cluster-manager is initialized.
   // Note that this implies that the xDS-manager must be shut down properly
   // prior to the cluster-manager deletion.

source/common/upstream/cluster_manager_impl.cc

@@ -377,7 +377,7 @@ ClusterManagerImpl::ClusterManagerImpl(
   }
 
   // Now that the async-client manager is set, the xDS-Manager can be initialized.
-  absl::Status status = xds_manager_.initialize(this);
+  absl::Status status = xds_manager_.initialize(bootstrap, this);
   SET_AND_RETURN_IF_NOT_OK(status, creation_status);
 
   // TODO(adisuissa): refactor and move the following data members to the
@@ -391,18 +391,9 @@ ClusterManagerImpl::ClusterManagerImpl(
         validation_context.dynamicValidationVisitor(), api, main_thread_dispatcher);
   }
 
-  if (bootstrap.has_xds_config_tracker_extension()) {
-    auto& tracer_factory = Config::Utility::getAndCheckFactory<Config::XdsConfigTrackerFactory>(
-        bootstrap.xds_config_tracker_extension());
-    xds_config_tracker_ = tracer_factory.createXdsConfigTracker(
-        bootstrap.xds_config_tracker_extension().typed_config(),
-        validation_context.dynamicValidationVisitor(), api, main_thread_dispatcher);
-  }
-
   subscription_factory_ = std::make_unique<Config::SubscriptionFactoryImpl>(
       local_info, main_thread_dispatcher, *this, validation_context.dynamicValidationVisitor(), api,
-      server, makeOptRefFromPtr(xds_resources_delegate_.get()),
-      makeOptRefFromPtr(xds_config_tracker_.get()));
+      server, makeOptRefFromPtr(xds_resources_delegate_.get()), xds_manager_.xdsConfigTracker());
 }
 
 absl::Status
@@ -501,7 +492,7 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo
           factory->create(std::move(primary_client), std::move(failover_client), dispatcher_,
                           random_, *stats_.rootScope(), dyn_resources.ads_config(), local_info_,
                           std::move(custom_config_validators), std::move(backoff_strategy),
-                          makeOptRefFromPtr(xds_config_tracker_.get()), {}, use_eds_cache);
+                          xds_manager_.xdsConfigTracker(), {}, use_eds_cache);
     } else {
       absl::Status status = Config::Utility::checkTransportVersion(dyn_resources.ads_config());
       RETURN_IF_NOT_OK(status);
@@ -531,11 +522,11 @@ ClusterManagerImpl::initialize(const envoy::config::bootstrap::v3::Bootstrap& bo
       Grpc::RawAsyncClientPtr failover_client;
       RETURN_IF_NOT_OK(createClients(factory_primary_or_error.value(), factory_failover,
                                      primary_client, failover_client));
-      ads_mux_ = factory->create(
-          std::move(primary_client), std::move(failover_client), dispatcher_, random_,
-          *stats_.rootScope(), dyn_resources.ads_config(), local_info_,
-          std::move(custom_config_validators), std::move(backoff_strategy),
-          makeOptRefFromPtr(xds_config_tracker_.get()), xds_delegate_opt_ref, use_eds_cache);
+      ads_mux_ =
+          factory->create(std::move(primary_client), std::move(failover_client), dispatcher_,
+                          random_, *stats_.rootScope(), dyn_resources.ads_config(), local_info_,
+                          std::move(custom_config_validators), std::move(backoff_strategy),
+                          xds_manager_.xdsConfigTracker(), xds_delegate_opt_ref, use_eds_cache);
     }
   } else {
     ads_mux_ = std::make_unique<Config::NullGrpcMuxImpl>();

source/common/upstream/cluster_manager_impl.h

@@ -955,7 +955,6 @@ class ClusterManagerImpl : public ClusterManager,
   ClusterSet primary_clusters_;
 
   std::unique_ptr<Config::XdsResourcesDelegate> xds_resources_delegate_;
-  std::unique_ptr<Config::XdsConfigTracker> xds_config_tracker_;
 
   bool initialized_{};
   bool ads_mux_initialized_{};

source/server/config_validation/server.cc

@@ -145,7 +145,7 @@ void ValidationInstance::initialize(const Options& options,
           serverFactoryContext(), messageValidationContext().staticValidationVisitor(),
           thread_local_);
 
-  xds_manager_ = std::make_unique<Config::XdsManagerImpl>(validation_context_);
+  xds_manager_ = std::make_unique<Config::XdsManagerImpl>(*dispatcher_, *api_, validation_context_);
 
   cluster_manager_factory_ = std::make_unique<Upstream::ValidationClusterManagerFactory>(
       server_contexts_, stats(), threadLocal(), http_context_,

source/server/server.cc

@@ -753,7 +753,7 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar
 
   // Create the xDS-Manager that will be passed to the cluster manager when it
   // is initialized below.
-  xds_manager_ = std::make_unique<Config::XdsManagerImpl>(validation_context_);
+  xds_manager_ = std::make_unique<Config::XdsManagerImpl>(*dispatcher_, *api_, validation_context_);
 
   cluster_manager_factory_ = std::make_unique<Upstream::ProdClusterManagerFactory>(
       serverFactoryContext(), stats_store_, thread_local_, http_context_,

test/common/config/BUILD

@@ -277,6 +277,8 @@ envoy_cc_test(
     rbe_pool = "2core",
     deps = [
         "//source/common/config:xds_manager_lib",
+        "//test/mocks/api:api_mocks",
+        "//test/mocks/event:event_mocks",
         "//test/mocks/protobuf:protobuf_mocks",
         "//test/mocks/upstream:cluster_manager_mocks",
         "//test/test_common:status_utility_lib",

test/common/config/xds_manager_impl_test.cc

@@ -1,5 +1,7 @@
 #include "source/common/config/xds_manager_impl.h"
 
+#include "test/mocks/api/mocks.h"
+#include "test/mocks/event/mocks.h"
 #include "test/mocks/protobuf/mocks.h"
 #include "test/mocks/upstream/cluster_manager.h"
 #include "test/test_common/status_utility.h"
@@ -17,13 +19,18 @@ using testing::Return;
 
 class XdsManagerImplTest : public testing::Test {
 public:
-  XdsManagerImplTest() : xds_manager_impl_(validation_context_) {
+  XdsManagerImplTest() : xds_manager_impl_(dispatcher_, api_, validation_context_) {
     ON_CALL(validation_context_, staticValidationVisitor())
         .WillByDefault(ReturnRef(validation_visitor_));
   }
 
-  void initialize() { ASSERT_OK(xds_manager_impl_.initialize(&cm_)); }
+  void initialize() {
+    const envoy::config::bootstrap::v3::Bootstrap bootstrap;
+    ASSERT_OK(xds_manager_impl_.initialize(bootstrap, &cm_));
+  }
 
+  NiceMock<Event::MockDispatcher> dispatcher_;
+  NiceMock<Api::MockApi> api_;
   NiceMock<Upstream::MockClusterManager> cm_;
   NiceMock<ProtobufMessage::MockValidationVisitor> validation_visitor_;
   NiceMock<ProtobufMessage::MockValidationContext> validation_context_;
@@ -36,7 +43,8 @@ TEST_F(XdsManagerImplTest, ShutdownSuccessful) {
   xds_manager_impl_.shutdown();
 }
 
-// Validates that setAdsConfigSource invokes the correct method in the cm_.
+// Validates that setAdsConfigSource invokes the correct method in the
+// cluster-manager.
 TEST_F(XdsManagerImplTest, AdsConfigSourceSetterSuccess) {
   initialize();
   envoy::config::core::v3::ApiConfigSource config_source;
@@ -46,8 +54,8 @@ TEST_F(XdsManagerImplTest, AdsConfigSourceSetterSuccess) {
   EXPECT_TRUE(res.ok());
 }
 
-// Validates that setAdsConfigSource invokes the correct method in the cm_,
-// and fails if needed.
+// Validates that setAdsConfigSource invokes the correct method in the
+// cluster-manager, and fails if needed.
 TEST_F(XdsManagerImplTest, AdsConfigSourceSetterFailure) {
   initialize();
   envoy::config::core::v3::ApiConfigSource config_source;

test/mocks/config/xds_manager.cc

@@ -6,7 +6,7 @@ using testing::_;
 using testing::Return;
 
 MockXdsManager::MockXdsManager() {
-  ON_CALL(*this, initialize(_)).WillByDefault(Return(absl::OkStatus()));
+  ON_CALL(*this, initialize(_, _)).WillByDefault(Return(absl::OkStatus()));
 }
 
 } // namespace Config

test/mocks/config/xds_manager.h

@@ -12,10 +12,13 @@ class MockXdsManager : public XdsManager {
   MockXdsManager();
   ~MockXdsManager() override = default;
 
-  MOCK_METHOD(absl::Status, initialize, (Upstream::ClusterManager * cm));
+  MOCK_METHOD(absl::Status, initialize,
+              (const envoy::config::bootstrap::v3::Bootstrap& bootstrap,
+               Upstream::ClusterManager* cm));
   MOCK_METHOD(void, shutdown, ());
   MOCK_METHOD(absl::Status, setAdsConfigSource,
               (const envoy::config::core::v3::ApiConfigSource& config_source));
+  MOCK_METHOD(OptRef<Config::XdsConfigTracker>, xdsConfigTracker, ());
 };
 
 } // namespace Config