From 1ed2606b23a944e95285d32545b8b4e285df8927 Mon Sep 17 00:00:00 2001 From: Adi Suissa-Peleg Date: Wed, 22 Jan 2025 20:56:19 +0000 Subject: [PATCH] xds: refactor ADS related data to xDS-Manager part 1 Signed-off-by: Adi Suissa-Peleg --- envoy/config/BUILD | 1 + envoy/config/xds_manager.h | 15 ++++++++ envoy/server/BUILD | 1 + envoy/server/factory_context.h | 6 ++++ source/common/config/xds_manager_impl.cc | 8 ++++- source/common/config/xds_manager_impl.h | 13 ++++--- .../common/upstream/cluster_manager_impl.cc | 13 +++++-- source/common/upstream/cluster_manager_impl.h | 21 +++++++----- .../config_validation/cluster_manager.cc | 2 +- source/server/config_validation/server.cc | 3 +- source/server/config_validation/server.h | 4 +-- source/server/server.cc | 9 +++-- source/server/server.h | 1 + test/common/config/xds_manager_impl_test.cc | 13 ++++++- test/common/upstream/BUILD | 1 + .../upstream/cluster_manager_impl_test.cc | 34 +++++++++---------- .../deferred_cluster_initialization_test.cc | 6 ++-- test/common/upstream/test_cluster_manager.h | 10 +++--- .../clusters/aggregate/cluster_update_test.cc | 9 +++-- test/mocks/config/BUILD | 3 +- test/mocks/config/xds_manager.cc | 13 +++++++ test/mocks/config/xds_manager.h | 6 +++- test/mocks/server/BUILD | 1 + test/mocks/server/instance.cc | 1 + test/mocks/server/instance.h | 4 ++- test/mocks/server/server_factory_context.cc | 1 + test/mocks/server/server_factory_context.h | 4 +++ 27 files changed, 147 insertions(+), 56 deletions(-) create mode 100644 test/mocks/config/xds_manager.cc diff --git a/envoy/config/BUILD b/envoy/config/BUILD index 2553bd91aabf..9301f228fd0c 100644 --- a/envoy/config/BUILD +++ b/envoy/config/BUILD @@ -160,6 +160,7 @@ envoy_cc_library( name = "xds_manager_interface", hdrs = ["xds_manager.h"], deps = [ + "//envoy/upstream:cluster_manager_interface", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", ], ) diff --git a/envoy/config/xds_manager.h b/envoy/config/xds_manager.h index 8497ef7bf2a5..c9253246596b 100644 --- a/envoy/config/xds_manager.h +++ b/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/upstream/cluster_manager.h" #include "absl/status/status.h" @@ -22,6 +23,20 @@ class XdsManager { public: virtual ~XdsManager() = default; + /** + * Initializes the xDS-Manager. + * This should be called after the cluster-manager is created. + * @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; + + /** + * Shuts down the xDS-Manager and all the configured connections to the config + * servers. + */ + virtual void shutdown() PURE; + /** * Set the ADS ConfigSource Envoy should use that will replace the current ADS * server. diff --git a/envoy/server/BUILD b/envoy/server/BUILD index ebd6ffb9bb6f..6e41fa28291e 100644 --- a/envoy/server/BUILD +++ b/envoy/server/BUILD @@ -173,6 +173,7 @@ envoy_cc_library( "//envoy/api:api_interface", "//envoy/config:typed_config_interface", "//envoy/config:typed_metadata_interface", + "//envoy/config:xds_manager_interface", "//envoy/grpc:context_interface", "//envoy/http:codes_interface", "//envoy/http:context_interface", diff --git a/envoy/server/factory_context.h b/envoy/server/factory_context.h index 6a4d77f3ad19..8a0559d8d0d8 100644 --- a/envoy/server/factory_context.h +++ b/envoy/server/factory_context.h @@ -9,6 +9,7 @@ #include "envoy/config/core/v3/base.pb.h" #include "envoy/config/typed_config.h" #include "envoy/config/typed_metadata.h" +#include "envoy/config/xds_manager.h" #include "envoy/grpc/context.h" #include "envoy/http/codes.h" #include "envoy/http/context.h" @@ -121,6 +122,11 @@ class CommonFactoryContext { */ virtual Upstream::ClusterManager& clusterManager() PURE; + /** + * @return Config::XdsManager& singleton for use by the entire server. + */ + virtual Config::XdsManager& xdsManager() PURE; + /** * @return const Http::HttpServerPropertiesCacheManager& instance for use by the entire server. */ diff --git a/source/common/config/xds_manager_impl.cc b/source/common/config/xds_manager_impl.cc index 98ded16d7c94..916a63f39c4e 100644 --- a/source/common/config/xds_manager_impl.cc +++ b/source/common/config/xds_manager_impl.cc @@ -7,12 +7,18 @@ namespace Envoy { namespace Config { +absl::Status XdsManagerImpl::initialize(Upstream::ClusterManager* cm) { + ASSERT(cm != nullptr); + cm_ = cm; + return absl::OkStatus(); +} + absl::Status XdsManagerImpl::setAdsConfigSource(const envoy::config::core::v3::ApiConfigSource& config_source) { ASSERT_IS_MAIN_OR_TEST_THREAD(); RETURN_IF_NOT_OK(validateAdsConfig(config_source)); - return cm_.replaceAdsMux(config_source); + return cm_->replaceAdsMux(config_source); } absl::Status diff --git a/source/common/config/xds_manager_impl.h b/source/common/config/xds_manager_impl.h index e8378b0f7b7c..cea12b6d4922 100644 --- a/source/common/config/xds_manager_impl.h +++ b/source/common/config/xds_manager_impl.h @@ -1,7 +1,6 @@ #pragma once #include "envoy/config/xds_manager.h" -#include "envoy/upstream/cluster_manager.h" #include "source/common/common/thread.h" @@ -10,11 +9,12 @@ namespace Config { class XdsManagerImpl : public XdsManager { public: - XdsManagerImpl(Upstream::ClusterManager& cm, - ProtobufMessage::ValidationContext& validation_context) - : cm_(cm), validation_context_(validation_context) {} + XdsManagerImpl(ProtobufMessage::ValidationContext& validation_context) + : validation_context_(validation_context) {} // Config::ConfigSourceProvider + absl::Status initialize(Upstream::ClusterManager* cm) override; + void shutdown() override {} absl::Status setAdsConfigSource(const envoy::config::core::v3::ApiConfigSource& config_source) override; @@ -22,8 +22,11 @@ class XdsManagerImpl : public XdsManager { // Validates (syntactically) the config_source by doing the PGV validation. absl::Status validateAdsConfig(const envoy::config::core::v3::ApiConfigSource& config_source); - Upstream::ClusterManager& cm_; ProtobufMessage::ValidationContext& validation_context_; + // 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. + Upstream::ClusterManager* cm_; }; } // namespace Config diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 34f444f444f0..75393c0eca2c 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -325,9 +325,10 @@ ClusterManagerImpl::ClusterManagerImpl( AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, OptRef admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, Grpc::Context& grpc_context, - Router::Context& router_context, Server::Instance& server, absl::Status& creation_status) + Router::Context& router_context, Server::Instance& server, Config::XdsManager& xds_manager, + absl::Status& creation_status) : server_(server), factory_(factory), runtime_(runtime), stats_(stats), tls_(tls), - random_(api.randomGenerator()), + xds_manager_(xds_manager), random_(api.randomGenerator()), deferred_cluster_creation_(bootstrap.cluster_manager().enable_deferred_cluster_creation()), bind_config_(bootstrap.cluster_manager().has_upstream_bind_config() ? absl::make_optional(bootstrap.cluster_manager().upstream_bind_config()) @@ -375,6 +376,12 @@ ClusterManagerImpl::ClusterManagerImpl( local_cluster_name_ = cm_config.local_cluster_name(); } + // Now that the async-client manager is set, the xDS-Manager can be initialized. + absl::Status status = xds_manager_.initialize(this); + SET_AND_RETURN_IF_NOT_OK(status, creation_status); + + // TODO(adisuissa): refactor and move the following data members to the + // xDS-manager class. // Initialize the XdsResourceDelegate extension, if set on the bootstrap config. if (bootstrap.has_xds_delegate_extension()) { auto& factory = Config::Utility::getAndCheckFactory( @@ -2337,7 +2344,7 @@ absl::StatusOr ProdClusterManagerFactory::clusterManagerFromP bootstrap, *this, context_, stats_, tls_, context_.runtime(), context_.localInfo(), context_.accessLogManager(), context_.mainThreadDispatcher(), context_.admin(), context_.messageValidationContext(), context_.api(), http_context_, context_.grpcContext(), - context_.routerContext(), server_, creation_status)}; + context_.routerContext(), server_, context_.xdsManager(), creation_status)}; RETURN_IF_NOT_OK(creation_status); return cluster_manager_impl; } diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 4cbcdd577b03..1b5ad6ffd432 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -296,6 +296,7 @@ class ClusterManagerImpl : public ClusterManager, // Make sure we destroy all potential outgoing connections before this returns. cds_api_.reset(); ads_mux_.reset(); + xds_manager_.shutdown(); active_clusters_.clear(); warming_clusters_.clear(); updateClusterCounts(); @@ -381,14 +382,17 @@ class ClusterManagerImpl : public ClusterManager, protected: // ClusterManagerImpl's constructor should not be invoked directly; create instances from the // clusterManagerFromProto() static method. The init() method must be called after construction. - ClusterManagerImpl( - const envoy::config::bootstrap::v3::Bootstrap& bootstrap, ClusterManagerFactory& factory, - Server::Configuration::CommonFactoryContext& context, Stats::Store& stats, - ThreadLocal::Instance& tls, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, - AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, - OptRef admin, ProtobufMessage::ValidationContext& validation_context, - Api::Api& api, Http::Context& http_context, Grpc::Context& grpc_context, - Router::Context& router_context, Server::Instance& server, absl::Status& creation_status); + ClusterManagerImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstrap, + ClusterManagerFactory& factory, + Server::Configuration::CommonFactoryContext& context, Stats::Store& stats, + ThreadLocal::Instance& tls, Runtime::Loader& runtime, + const LocalInfo::LocalInfo& local_info, + AccessLog::AccessLogManager& log_manager, + Event::Dispatcher& main_thread_dispatcher, OptRef admin, + ProtobufMessage::ValidationContext& validation_context, Api::Api& api, + Http::Context& http_context, Grpc::Context& grpc_context, + Router::Context& router_context, Server::Instance& server, + Config::XdsManager& xds_manager, absl::Status& creation_status); virtual void postThreadLocalRemoveHosts(const Cluster& cluster, const HostVector& hosts_removed); @@ -913,6 +917,7 @@ class ClusterManagerImpl : public ClusterManager, ThreadLocal::TypedSlot tls_; // Contains information about ongoing on-demand cluster discoveries. ClusterCreationsMap pending_cluster_creations_; + Config::XdsManager& xds_manager_; Random::RandomGenerator& random_; ClusterMap warming_clusters_; const bool deferred_cluster_creation_; diff --git a/source/server/config_validation/cluster_manager.cc b/source/server/config_validation/cluster_manager.cc index 2d7794d0c8ec..061ef1ff6633 100644 --- a/source/server/config_validation/cluster_manager.cc +++ b/source/server/config_validation/cluster_manager.cc @@ -15,7 +15,7 @@ absl::StatusOr ValidationClusterManagerFactory::clusterManage bootstrap, *this, context_, stats_, tls_, context_.runtime(), context_.localInfo(), context_.accessLogManager(), context_.mainThreadDispatcher(), context_.admin(), context_.messageValidationContext(), context_.api(), http_context_, context_.grpcContext(), - context_.routerContext(), server_, creation_status)}; + context_.routerContext(), server_, context_.xdsManager(), creation_status)}; RETURN_IF_NOT_OK(creation_status); return cluster_manager; } diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 9830dd180a44..8dcabf9e4bca 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -145,6 +145,8 @@ void ValidationInstance::initialize(const Options& options, serverFactoryContext(), messageValidationContext().staticValidationVisitor(), thread_local_); + xds_manager_ = std::make_unique(validation_context_); + cluster_manager_factory_ = std::make_unique( server_contexts_, stats(), threadLocal(), http_context_, [this]() -> Network::DnsResolverSharedPtr { return this->dnsResolver(); }, @@ -152,7 +154,6 @@ void ValidationInstance::initialize(const Options& options, THROW_IF_NOT_OK(config_.initialize(bootstrap_, *this, *cluster_manager_factory_)); THROW_IF_NOT_OK(runtime().initialize(clusterManager())); clusterManager().setInitializedCb([this]() -> void { init_manager_.initialize(init_watcher_); }); - xds_manager_ = std::make_unique(clusterManager(), validation_context_); } void ValidationInstance::shutdown() { diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 03f53be8c86a..e7a3fb2071e6 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -78,6 +78,7 @@ class ValidationInstance final : Logger::Loggable, } Api::Api& api() override { return *api_; } Upstream::ClusterManager& clusterManager() override { return *config_.clusterManager(); } + Config::XdsManager& xdsManager() override { return *xds_manager_; } const Upstream::ClusterManager& clusterManager() const override { return *config_.clusterManager(); } @@ -133,7 +134,6 @@ class ValidationInstance final : Logger::Loggable, http_context_.setDefaultTracingConfig(tracing_config); } void setSinkPredicates(std::unique_ptr&&) override {} - Config::XdsManager& xdsManager() override { return *xds_manager_; } // Server::WorkerFactory WorkerPtr createWorker(uint32_t, OverloadManager&, OverloadManager&, @@ -185,8 +185,8 @@ class ValidationInstance final : Logger::Loggable, LocalInfo::LocalInfoPtr local_info_; AccessLog::AccessLogManagerImpl access_log_manager_; std::unique_ptr http_server_properties_cache_manager_; - std::unique_ptr cluster_manager_factory_; Config::XdsManagerPtr xds_manager_; + std::unique_ptr cluster_manager_factory_; std::unique_ptr listener_manager_; std::unique_ptr overload_manager_; std::unique_ptr null_overload_manager_; diff --git a/source/server/server.cc b/source/server/server.cc index ced062a4212d..45baa06e9d39 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -751,6 +751,10 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar serverFactoryContext(), messageValidationContext().staticValidationVisitor(), thread_local_); + // Create the xDS-Manager that will be passed to the cluster manager when it + // is initialized below. + xds_manager_ = std::make_unique(validation_context_); + cluster_manager_factory_ = std::make_unique( serverFactoryContext(), stats_store_, thread_local_, http_context_, [this]() -> Network::DnsResolverSharedPtr { return this->getOrCreateDnsResolver(); }, @@ -762,11 +766,6 @@ absl::Status InstanceBase::initializeOrThrow(Network::Address::InstanceConstShar // cluster_manager_factory_ is available. RETURN_IF_NOT_OK(config_.initialize(bootstrap_, *this, *cluster_manager_factory_)); - // Create the xDS-Manager and pass the cluster manager that was created above. - ASSERT(config_.clusterManager()); - xds_manager_ = - std::make_unique(*config_.clusterManager(), validation_context_); - // Instruct the listener manager to create the LDS provider if needed. This must be done later // because various items do not yet exist when the listener manager is created. if (bootstrap_.dynamic_resources().has_lds_config() || diff --git a/source/server/server.h b/source/server/server.h index 8fef9d981db7..f472047667ee 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -178,6 +178,7 @@ class ServerFactoryContextImpl : public Configuration::ServerFactoryContext, // Configuration::ServerFactoryContext Upstream::ClusterManager& clusterManager() override { return server_.clusterManager(); } + Config::XdsManager& xdsManager() override { return server_.xdsManager(); } Http::HttpServerPropertiesCacheManager& httpServerPropertiesCacheManager() override { return server_.httpServerPropertiesCacheManager(); } diff --git a/test/common/config/xds_manager_impl_test.cc b/test/common/config/xds_manager_impl_test.cc index 183bca2dc2cf..9e839142118a 100644 --- a/test/common/config/xds_manager_impl_test.cc +++ b/test/common/config/xds_manager_impl_test.cc @@ -17,19 +17,28 @@ using testing::Return; class XdsManagerImplTest : public testing::Test { public: - XdsManagerImplTest() : xds_manager_impl_(cm_, validation_context_) { + XdsManagerImplTest() : xds_manager_impl_(validation_context_) { ON_CALL(validation_context_, staticValidationVisitor()) .WillByDefault(ReturnRef(validation_visitor_)); } + void initialize() { ASSERT_OK(xds_manager_impl_.initialize(&cm_)); } + NiceMock cm_; NiceMock validation_visitor_; NiceMock validation_context_; XdsManagerImpl xds_manager_impl_; }; +// Validates that a call to shutdown succeeds. +TEST_F(XdsManagerImplTest, ShutdownSuccessful) { + initialize(); + xds_manager_impl_.shutdown(); +} + // Validates that setAdsConfigSource invokes the correct method in the cm_. TEST_F(XdsManagerImplTest, AdsConfigSourceSetterSuccess) { + initialize(); envoy::config::core::v3::ApiConfigSource config_source; config_source.set_api_type(envoy::config::core::v3::ApiConfigSource::GRPC); EXPECT_CALL(cm_, replaceAdsMux(ProtoEq(config_source))).WillOnce(Return(absl::OkStatus())); @@ -40,6 +49,7 @@ TEST_F(XdsManagerImplTest, AdsConfigSourceSetterSuccess) { // Validates that setAdsConfigSource invokes the correct method in the cm_, // and fails if needed. TEST_F(XdsManagerImplTest, AdsConfigSourceSetterFailure) { + initialize(); envoy::config::core::v3::ApiConfigSource config_source; config_source.set_api_type(envoy::config::core::v3::ApiConfigSource::GRPC); EXPECT_CALL(cm_, replaceAdsMux(ProtoEq(config_source))) @@ -51,6 +61,7 @@ TEST_F(XdsManagerImplTest, AdsConfigSourceSetterFailure) { // Validates that setAdsConfigSource validation failure is detected. TEST_F(XdsManagerImplTest, AdsConfigSourceSetterInvalidConfig) { + initialize(); envoy::config::core::v3::ApiConfigSource config_source; config_source.set_api_type(envoy::config::core::v3::ApiConfigSource::GRPC); // Add an empty gRPC service (without EnvoyGrpc/GoogleGrpc) which should be diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index 8f77860fbf06..cc7dfbff0efe 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -541,6 +541,7 @@ envoy_cc_test_library( "//test/integration/clusters:custom_static_cluster", "//test/mocks/access_log:access_log_mocks", "//test/mocks/api:api_mocks", + "//test/mocks/config:xds_manager_mocks", "//test/mocks/http:http_mocks", "//test/mocks/local_info:local_info_mocks", "//test/mocks/network:network_mocks", diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index a6995b621561..59bf8f92eb41 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -118,10 +118,11 @@ class MockedUpdatedClusterManagerImpl : public TestClusterManagerImpl { Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, MockLocalClusterUpdate& local_cluster_update, MockLocalHostsRemoved& local_hosts_removed, Http::Context& http_context, Grpc::Context& grpc_context, Router::Context& router_context, - Server::Instance& server, absl::Status& creation_status) + Server::Instance& server, Config::XdsManager& xds_manager, absl::Status& creation_status) : TestClusterManagerImpl(bootstrap, factory, factory_context, stats, tls, runtime, local_info, log_manager, main_thread_dispatcher, admin, validation_context, api, - http_context, grpc_context, router_context, server, creation_status), + http_context, grpc_context, router_context, server, xds_manager, + creation_status), local_cluster_update_(local_cluster_update), local_hosts_removed_(local_hosts_removed) {} protected: @@ -198,19 +199,17 @@ class FakeConfigValidatorFactory : public Config::ConfigValidatorFactory { // solves the problem outlined in https://github.com/envoyproxy/envoy/issues/27702. class UpdateOverrideClusterManagerImpl : public TestClusterManagerImpl { public: - UpdateOverrideClusterManagerImpl(const Bootstrap& bootstrap, ClusterManagerFactory& factory, - Server::Configuration::CommonFactoryContext& factory_context, - Stats::Store& stats, ThreadLocal::Instance& tls, - Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, - AccessLog::AccessLogManager& log_manager, - Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin, - ProtobufMessage::ValidationContext& validation_context, - Api::Api& api, Http::Context& http_context, - Grpc::Context& grpc_context, Router::Context& router_context, - Server::Instance& server, absl::Status& creation_status) + UpdateOverrideClusterManagerImpl( + const Bootstrap& bootstrap, ClusterManagerFactory& factory, + Server::Configuration::CommonFactoryContext& factory_context, Stats::Store& stats, + ThreadLocal::Instance& tls, Runtime::Loader& runtime, const LocalInfo::LocalInfo& local_info, + AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, + Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, + Http::Context& http_context, Grpc::Context& grpc_context, Router::Context& router_context, + Server::Instance& server, Config::XdsManager& xds_manager, absl::Status& creation_status) : TestClusterManagerImpl(bootstrap, factory, factory_context, stats, tls, runtime, local_info, log_manager, main_thread_dispatcher, admin, validation_context, api, - http_context, grpc_context, router_context, server, + http_context, grpc_context, router_context, server, xds_manager, creation_status) {} protected: @@ -252,8 +251,8 @@ class ClusterManagerImplTest : public testing::Test { cluster_manager_ = TestClusterManagerImpl::createAndInit( bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, - validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, - server_); + validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_, + xds_manager_); cluster_manager_->setPrimaryClustersInitializedCb([this, bootstrap]() { THROW_IF_NOT_OK(cluster_manager_->initializeSecondaryClusters(bootstrap)); }); @@ -321,7 +320,7 @@ class ClusterManagerImplTest : public testing::Test { bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, validation_context_, *factory_.api_, local_cluster_update_, local_hosts_removed_, - http_context_, grpc_context_, router_context_, server_, creation_status); + http_context_, grpc_context_, router_context_, server_, xds_manager_, creation_status); THROW_IF_NOT_OK(creation_status); THROW_IF_NOT_OK(cluster_manager_->initialize(bootstrap)); } @@ -332,7 +331,7 @@ class ClusterManagerImplTest : public testing::Test { bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_, - creation_status); + xds_manager_, creation_status); THROW_IF_NOT_OK(creation_status); THROW_IF_NOT_OK(cluster_manager_->initialize(bootstrap)); } @@ -379,6 +378,7 @@ class ClusterManagerImplTest : public testing::Test { Event::SimulatedTimeSystem time_system_; NiceMock factory_; NiceMock validation_context_; + NiceMock xds_manager_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; NiceMock admin_; diff --git a/test/common/upstream/deferred_cluster_initialization_test.cc b/test/common/upstream/deferred_cluster_initialization_test.cc index ee517026d216..f1bee589850f 100644 --- a/test/common/upstream/deferred_cluster_initialization_test.cc +++ b/test/common/upstream/deferred_cluster_initialization_test.cc @@ -11,6 +11,7 @@ #include "test/common/upstream/test_cluster_manager.h" #include "test/mocks/config/mocks.h" +#include "test/mocks/config/xds_manager.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/server/instance.h" #include "test/test_common/simulated_time_system.h" @@ -65,8 +66,8 @@ class DeferredClusterInitializationTest : public testing::TestWithParam { cluster_manager_ = TestClusterManagerImpl::createAndInit( bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, - validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, - server_); + validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_, + xds_manager_); cluster_manager_->setPrimaryClustersInitializedCb([this, bootstrap]() { THROW_IF_NOT_OK(cluster_manager_->initializeSecondaryClusters(bootstrap)); }); @@ -112,6 +113,7 @@ class DeferredClusterInitializationTest : public testing::TestWithParam { NiceMock factory_; NiceMock validation_context_; + NiceMock xds_manager_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; NiceMock admin_; diff --git a/test/common/upstream/test_cluster_manager.h b/test/common/upstream/test_cluster_manager.h index 5732bd101c79..2b6dcb410e2a 100644 --- a/test/common/upstream/test_cluster_manager.h +++ b/test/common/upstream/test_cluster_manager.h @@ -28,6 +28,7 @@ #include "test/integration/clusters/custom_static_cluster.h" #include "test/mocks/access_log/mocks.h" #include "test/mocks/api/mocks.h" +#include "test/mocks/config/xds_manager.h" #include "test/mocks/http/mocks.h" #include "test/mocks/local_info/mocks.h" #include "test/mocks/network/mocks.h" @@ -172,12 +173,12 @@ class TestClusterManagerImpl : public ClusterManagerImpl { AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, Grpc::Context& grpc_context, Router::Context& router_context, - Server::Instance& server) { + Server::Instance& server, Config::XdsManager& xds_manager) { absl::Status creation_status = absl::OkStatus(); auto cluster_manager = std::unique_ptr{new TestClusterManagerImpl( bootstrap, factory, context, stats, tls, runtime, local_info, log_manager, main_thread_dispatcher, admin, validation_context, api, http_context, grpc_context, - router_context, server, creation_status)}; + router_context, server, xds_manager, creation_status)}; THROW_IF_NOT_OK(creation_status); THROW_IF_NOT_OK(cluster_manager->initialize(bootstrap)); return cluster_manager; @@ -217,10 +218,11 @@ class TestClusterManagerImpl : public ClusterManagerImpl { AccessLog::AccessLogManager& log_manager, Event::Dispatcher& main_thread_dispatcher, Server::Admin& admin, ProtobufMessage::ValidationContext& validation_context, Api::Api& api, Http::Context& http_context, Grpc::Context& grpc_context, Router::Context& router_context, - Server::Instance& server, absl::Status& creation_status) + Server::Instance& server, Config::XdsManager& xds_manager, absl::Status& creation_status) : ClusterManagerImpl(bootstrap, factory, context, stats, tls, runtime, local_info, log_manager, main_thread_dispatcher, admin, validation_context, api, - http_context, grpc_context, router_context, server, creation_status) {} + http_context, grpc_context, router_context, server, xds_manager, + creation_status) {} }; } // namespace Upstream diff --git a/test/extensions/clusters/aggregate/cluster_update_test.cc b/test/extensions/clusters/aggregate/cluster_update_test.cc index 1f4a5b742daf..05cbaed5004c 100644 --- a/test/extensions/clusters/aggregate/cluster_update_test.cc +++ b/test/extensions/clusters/aggregate/cluster_update_test.cc @@ -10,6 +10,7 @@ #include "test/common/upstream/test_cluster_manager.h" #include "test/common/upstream/utility.h" +#include "test/mocks/config/xds_manager.h" #include "test/mocks/protobuf/mocks.h" #include "test/mocks/server/admin.h" #include "test/mocks/server/instance.h" @@ -45,8 +46,8 @@ class AggregateClusterUpdateTest : public Event::TestUsingSimulatedTime, cluster_manager_ = Upstream::TestClusterManagerImpl::createAndInit( bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, - validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, - server_); + validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_, + xds_manager_); ASSERT_TRUE(cluster_manager_->initializeSecondaryClusters(bootstrap).ok()); EXPECT_EQ(cluster_manager_->activeClusters().size(), 1); cluster_ = cluster_manager_->getThreadLocalCluster("aggregate_cluster"); @@ -58,6 +59,7 @@ class AggregateClusterUpdateTest : public Event::TestUsingSimulatedTime, Upstream::ThreadLocalCluster* cluster_; NiceMock validation_context_; + NiceMock xds_manager_; std::unique_ptr cluster_manager_; AccessLog::MockAccessLogManager log_manager_; Http::ContextImpl http_context_; @@ -282,7 +284,8 @@ TEST_P(AggregateClusterUpdateTest, InitializeAggregateClusterAfterOtherClusters) cluster_manager_ = Upstream::TestClusterManagerImpl::createAndInit( bootstrap, factory_, factory_.server_context_, factory_.stats_, factory_.tls_, factory_.runtime_, factory_.local_info_, log_manager_, factory_.dispatcher_, admin_, - validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_); + validation_context_, *factory_.api_, http_context_, grpc_context_, router_context_, server_, + xds_manager_); ASSERT_TRUE(cluster_manager_->initializeSecondaryClusters(bootstrap).ok()); EXPECT_EQ(cluster_manager_->activeClusters().size(), 2); cluster_ = cluster_manager_->getThreadLocalCluster("aggregate_cluster"); diff --git a/test/mocks/config/BUILD b/test/mocks/config/BUILD index 341786e8ae62..75410c465c3a 100644 --- a/test/mocks/config/BUILD +++ b/test/mocks/config/BUILD @@ -48,7 +48,8 @@ envoy_cc_mock( envoy_cc_mock( name = "xds_manager_mocks", - hdrs = ["custom_config_validators.h"], + srcs = ["xds_manager.cc"], + hdrs = ["xds_manager.h"], deps = [ "//envoy/config:xds_manager_interface", ], diff --git a/test/mocks/config/xds_manager.cc b/test/mocks/config/xds_manager.cc new file mode 100644 index 000000000000..3cf0332db16d --- /dev/null +++ b/test/mocks/config/xds_manager.cc @@ -0,0 +1,13 @@ +#include "test/mocks/config/xds_manager.h" + +namespace Envoy { +namespace Config { +using testing::_; +using testing::Return; + +MockXdsManager::MockXdsManager() { + ON_CALL(*this, initialize(_)).WillByDefault(Return(absl::OkStatus())); +} + +} // namespace Config +} // namespace Envoy diff --git a/test/mocks/config/xds_manager.h b/test/mocks/config/xds_manager.h index 1002d6e7efb4..9a61111245c0 100644 --- a/test/mocks/config/xds_manager.h +++ b/test/mocks/config/xds_manager.h @@ -1,5 +1,7 @@ #pragma once +#include "envoy/config/xds_manager.h" + #include "gmock/gmock.h" namespace Envoy { @@ -7,9 +9,11 @@ namespace Config { class MockXdsManager : public XdsManager { public: - MockXdsManager() = default; + MockXdsManager(); ~MockXdsManager() override = default; + MOCK_METHOD(absl::Status, initialize, (Upstream::ClusterManager * cm)); + MOCK_METHOD(void, shutdown, ()); MOCK_METHOD(absl::Status, setAdsConfigSource, (const envoy::config::core::v3::ApiConfigSource& config_source)); }; diff --git a/test/mocks/server/BUILD b/test/mocks/server/BUILD index 13ff682a65e5..8c8f1d562b44 100644 --- a/test/mocks/server/BUILD +++ b/test/mocks/server/BUILD @@ -223,6 +223,7 @@ envoy_cc_mock( "//source/common/tls:context_lib", "//test/mocks/access_log:access_log_mocks", "//test/mocks/api:api_mocks", + "//test/mocks/config:xds_manager_mocks", "//test/mocks/http:http_mocks", "//test/mocks/http:http_server_properties_cache_mocks", "//test/mocks/init:init_mocks", diff --git a/test/mocks/server/instance.cc b/test/mocks/server/instance.cc index 8d3b8e3afac9..461fcd610f55 100644 --- a/test/mocks/server/instance.cc +++ b/test/mocks/server/instance.cc @@ -31,6 +31,7 @@ MockInstance::MockInstance() ON_CALL(*this, api()).WillByDefault(ReturnRef(api_)); ON_CALL(*this, admin()).WillByDefault(Return(OptRef{admin_})); ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); + ON_CALL(*this, xdsManager()).WillByDefault(ReturnRef(xds_manager_)); ON_CALL(*this, httpServerPropertiesCacheManager()) .WillByDefault(ReturnRef(http_server_properties_cache_manager_)); ON_CALL(*this, sslContextManager()).WillByDefault(ReturnRef(ssl_context_manager_)); diff --git a/test/mocks/server/instance.h b/test/mocks/server/instance.h index b9bc7768b12c..97006a5aa8c3 100644 --- a/test/mocks/server/instance.h +++ b/test/mocks/server/instance.h @@ -2,6 +2,7 @@ #include "envoy/server/instance.h" +#include "test/mocks/config/xds_manager.h" #include "test/mocks/http/http_server_properties_cache.h" #include "test/mocks/server/server_factory_context.h" #include "test/mocks/server/transport_socket_factory_context.h" @@ -22,6 +23,7 @@ class MockInstance : public Instance { MOCK_METHOD(void, run, ()); MOCK_METHOD(Api::Api&, api, ()); MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); + MOCK_METHOD(Config::XdsManager&, xdsManager, ()); MOCK_METHOD(const Upstream::ClusterManager&, clusterManager, (), (const)); MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Ssl::ContextManager&, sslContextManager, ()); @@ -63,7 +65,6 @@ class MockInstance : public Instance { MOCK_METHOD(Configuration::TransportSocketFactoryContext&, transportSocketFactoryContext, ()); MOCK_METHOD(bool, enableReusePortDefault, ()); MOCK_METHOD(void, setSinkPredicates, (std::unique_ptr &&)); - MOCK_METHOD(Config::XdsManager&, xdsManager, ()); void setDefaultTracingConfig(const envoy::config::trace::v3::Tracing& tracing_config) override { http_context_.setDefaultTracingConfig(tracing_config); @@ -82,6 +83,7 @@ class MockInstance : public Instance { Event::GlobalTimeSystem time_system_; std::unique_ptr secret_manager_; testing::NiceMock cluster_manager_; + testing::NiceMock xds_manager_; testing::NiceMock http_server_properties_cache_manager_; Thread::MutexBasicLockable access_log_lock_; diff --git a/test/mocks/server/server_factory_context.cc b/test/mocks/server/server_factory_context.cc index 80fe4b390425..25a9bf28a676 100644 --- a/test/mocks/server/server_factory_context.cc +++ b/test/mocks/server/server_factory_context.cc @@ -11,6 +11,7 @@ MockServerFactoryContext::MockServerFactoryContext() : singleton_manager_(new Singleton::ManagerImpl()), http_context_(store_.symbolTable()), grpc_context_(store_.symbolTable()), router_context_(store_.symbolTable()) { ON_CALL(*this, clusterManager()).WillByDefault(ReturnRef(cluster_manager_)); + ON_CALL(*this, xdsManager()).WillByDefault(ReturnRef(xds_manager_)); ON_CALL(*this, httpServerPropertiesCacheManager()) .WillByDefault(ReturnRef(http_server_properties_cache_manager_)); ON_CALL(*this, mainThreadDispatcher()).WillByDefault(ReturnRef(dispatcher_)); diff --git a/test/mocks/server/server_factory_context.h b/test/mocks/server/server_factory_context.h index fc4c69deed00..b84f94c804c8 100644 --- a/test/mocks/server/server_factory_context.h +++ b/test/mocks/server/server_factory_context.h @@ -11,6 +11,7 @@ #include "test/mocks/access_log/mocks.h" #include "test/mocks/api/mocks.h" +#include "test/mocks/config/xds_manager.h" #include "test/mocks/event/mocks.h" #include "test/mocks/http/http_server_properties_cache.h" #include "test/mocks/http/mocks.h" @@ -57,6 +58,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { ~MockServerFactoryContext() override; MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); + MOCK_METHOD(Config::XdsManager&, xdsManager, ()); MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Event::Dispatcher&, mainThreadDispatcher, ()); MOCK_METHOD(const Server::Options&, options, ()); @@ -89,6 +91,7 @@ class MockServerFactoryContext : public virtual ServerFactoryContext { MOCK_METHOD(bool, shouldBypassOverloadManager, (), (const)); MOCK_METHOD(bool, healthCheckFailed, (), (const)); + testing::NiceMock xds_manager_; testing::NiceMock cluster_manager_; testing::NiceMock dispatcher_; testing::NiceMock drain_manager_; @@ -141,6 +144,7 @@ class StatelessMockServerFactoryContext : public virtual ServerFactoryContext { ~StatelessMockServerFactoryContext() override = default; MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ()); + MOCK_METHOD(Config::XdsManager&, xdsManager, ()); MOCK_METHOD(Http::HttpServerPropertiesCacheManager&, httpServerPropertiesCacheManager, ()); MOCK_METHOD(Event::Dispatcher&, mainThreadDispatcher, ()); MOCK_METHOD(const Server::Options&, options, ());