From 20239c3292842c925356b3730c2be410b25aa9af Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Fri, 11 Oct 2019 10:40:38 -0400 Subject: [PATCH 1/7] add temporary isDelta getter Signed-off-by: Fred Douglas --- include/envoy/config/grpc_mux.h | 7 +++++++ source/common/config/grpc_mux_impl.h | 4 ++++ source/common/config/new_grpc_mux_impl.h | 3 +++ 3 files changed, 14 insertions(+) diff --git a/include/envoy/config/grpc_mux.h b/include/envoy/config/grpc_mux.h index 1742ea60e0601..8d65e28d58faf 100644 --- a/include/envoy/config/grpc_mux.h +++ b/include/envoy/config/grpc_mux.h @@ -113,6 +113,13 @@ class GrpcMux { */ virtual void resume(const std::string& type_url) PURE; + // TODO(fredlas) PR #8478 will remove this. + /** + * Whether this GrpcMux is delta. + * @return bool whether this GrpcMux is delta. + */ + virtual bool isDelta() const PURE; + // For delta virtual Watch* addOrUpdateWatch(const std::string& type_url, Watch* watch, const std::set& resources, diff --git a/source/common/config/grpc_mux_impl.h b/source/common/config/grpc_mux_impl.h index 83821a91504a6..3f55178f4e1e2 100644 --- a/source/common/config/grpc_mux_impl.h +++ b/source/common/config/grpc_mux_impl.h @@ -36,6 +36,8 @@ class GrpcMuxImpl : public GrpcMux, GrpcMuxCallbacks& callbacks) override; // GrpcMux + // TODO(fredlas) PR #8478 will remove this. + bool isDelta() const override { return false; } void pause(const std::string& type_url) override; void resume(const std::string& type_url) override; bool paused(const std::string& type_url) const override; @@ -133,6 +135,8 @@ class NullGrpcMuxImpl : public GrpcMux, GrpcStreamCallbacks Date: Fri, 11 Oct 2019 11:05:40 -0400 Subject: [PATCH 2/7] replace is_delta with isDelta Signed-off-by: Fred Douglas --- source/common/config/subscription_factory_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index 8e8fbc4e088f8..38930ba3d6608 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -22,7 +22,7 @@ SubscriptionFactoryImpl::SubscriptionFactoryImpl( SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, - Stats::Scope& scope, SubscriptionCallbacks& callbacks, bool is_delta) { + Stats::Scope& scope, SubscriptionCallbacks& callbacks, bool) { Config::Utility::checkLocalInfo(type_url, local_info_); std::unique_ptr result; SubscriptionStats stats = Utility::generateStats(scope); @@ -78,7 +78,7 @@ SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( break; } case envoy::api::v2::core::ConfigSource::kAds: { - if (is_delta) { + if (cm_.adsMux()->isDelta()) { result = std::make_unique( cm_.adsMux(), type_url, callbacks, stats, Utility::configSourceInitialFetchTimeout(config), true); From 6c408b06e906b160870520cf3d833be389be3d63 Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Fri, 11 Oct 2019 14:26:00 -0400 Subject: [PATCH 3/7] remove is_delta Signed-off-by: Fred Douglas --- include/envoy/config/subscription_factory.h | 2 +- .../router/route_config_provider_manager.h | 2 +- include/envoy/server/listener_manager.h | 6 ++-- include/envoy/upstream/cluster_manager.h | 4 +-- .../config/subscription_factory_impl.cc | 2 +- .../common/config/subscription_factory_impl.h | 4 +-- source/common/router/rds_impl.cc | 12 +++---- source/common/router/rds_impl.h | 8 ++--- source/common/router/scoped_rds.cc | 4 +-- source/common/router/vhds.cc | 2 +- source/common/runtime/runtime_impl.cc | 2 +- source/common/secret/sds_api.cc | 2 +- source/common/upstream/cds_api_impl.cc | 13 +++----- source/common/upstream/cds_api_impl.h | 9 +++--- .../common/upstream/cluster_manager_impl.cc | 32 ++----------------- source/common/upstream/cluster_manager_impl.h | 6 +--- source/common/upstream/eds.cc | 7 ++-- source/common/upstream/eds.h | 2 +- .../network/http_connection_manager/config.cc | 3 +- .../config_validation/cluster_manager.cc | 4 +-- .../config_validation/cluster_manager.h | 2 +- source/server/config_validation/server.h | 9 +++--- source/server/lds_api.cc | 4 +-- source/server/lds_api.h | 2 +- source/server/listener_manager_impl.h | 10 +++--- source/server/server.cc | 8 +---- test/common/upstream/cds_api_impl_test.cc | 4 +-- test/mocks/config/mocks.cc | 18 +++++------ test/mocks/config/mocks.h | 4 +-- test/mocks/router/mocks.h | 4 +-- test/mocks/server/mocks.h | 11 +++---- test/mocks/upstream/mocks.h | 5 ++- 32 files changed, 76 insertions(+), 131 deletions(-) diff --git a/include/envoy/config/subscription_factory.h b/include/envoy/config/subscription_factory.h index 5268ac7f2abe2..b1733afda6c46 100644 --- a/include/envoy/config/subscription_factory.h +++ b/include/envoy/config/subscription_factory.h @@ -24,7 +24,7 @@ class SubscriptionFactory { virtual SubscriptionPtr subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, Stats::Scope& scope, - SubscriptionCallbacks& callbacks, bool is_delta) PURE; + SubscriptionCallbacks& callbacks) PURE; }; } // namespace Config diff --git a/include/envoy/router/route_config_provider_manager.h b/include/envoy/router/route_config_provider_manager.h index accb99c85f240..9912aa4603564 100644 --- a/include/envoy/router/route_config_provider_manager.h +++ b/include/envoy/router/route_config_provider_manager.h @@ -39,7 +39,7 @@ class RouteConfigProviderManager { virtual RouteConfigProviderPtr createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - Init::Manager& init_manager, bool is_delta) PURE; + Init::Manager& init_manager) PURE; /** * Get a RouteConfigSharedPtr for a statically defined route. Ownership is as described for diff --git a/include/envoy/server/listener_manager.h b/include/envoy/server/listener_manager.h index 54d4b4a9bbcb1..ce2a015decf32 100644 --- a/include/envoy/server/listener_manager.h +++ b/include/envoy/server/listener_manager.h @@ -39,8 +39,7 @@ class ListenerComponentFactory { * @return an LDS API provider. * @param lds_config supplies the management server configuration. */ - virtual LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config, - bool is_delta) PURE; + virtual LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) PURE; /** * Creates a socket. @@ -129,8 +128,7 @@ class ListenerManager { * pieces of the server existing. * @param lds_config supplies the management server configuration. */ - virtual void createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config, - bool is_delta) PURE; + virtual void createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) PURE; /** * @return std::vector> a list of the currently diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 132c36c054b6a..e77b8bbaaee99 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -223,8 +223,6 @@ class ClusterManager { virtual Config::SubscriptionFactory& subscriptionFactory() PURE; virtual std::size_t warmingClusterCount() const PURE; - - virtual bool xdsIsDelta() const PURE; }; using ClusterManagerPtr = std::unique_ptr; @@ -298,7 +296,7 @@ class ClusterManagerFactory { /** * Create a CDS API provider from configuration proto. */ - virtual CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, + virtual CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm) PURE; /** diff --git a/source/common/config/subscription_factory_impl.cc b/source/common/config/subscription_factory_impl.cc index 38930ba3d6608..9217b612b18d6 100644 --- a/source/common/config/subscription_factory_impl.cc +++ b/source/common/config/subscription_factory_impl.cc @@ -22,7 +22,7 @@ SubscriptionFactoryImpl::SubscriptionFactoryImpl( SubscriptionPtr SubscriptionFactoryImpl::subscriptionFromConfigSource( const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, - Stats::Scope& scope, SubscriptionCallbacks& callbacks, bool) { + Stats::Scope& scope, SubscriptionCallbacks& callbacks) { Config::Utility::checkLocalInfo(type_url, local_info_); std::unique_ptr result; SubscriptionStats stats = Utility::generateStats(scope); diff --git a/source/common/config/subscription_factory_impl.h b/source/common/config/subscription_factory_impl.h index 43cec2139649c..3a7f0ba1e31c8 100644 --- a/source/common/config/subscription_factory_impl.h +++ b/source/common/config/subscription_factory_impl.h @@ -16,12 +16,10 @@ class SubscriptionFactoryImpl : public SubscriptionFactory { Upstream::ClusterManager& cm, Runtime::RandomGenerator& random, ProtobufMessage::ValidationVisitor& validation_visitor, Api::Api& api); - // TODO(fredlas) remove is_delta once delta and SotW are unified // Config::SubscriptionFactory SubscriptionPtr subscriptionFromConfigSource(const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, Stats::Scope& scope, - SubscriptionCallbacks& callbacks, - bool is_delta) override; + SubscriptionCallbacks& callbacks) override; private: const LocalInfo::LocalInfo& local_info_; diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index af64085ddd195..c9fd1b3002556 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -23,7 +23,7 @@ RouteConfigProviderPtr RouteConfigProviderUtil::create( const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& config, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - RouteConfigProviderManager& route_config_provider_manager, bool is_delta) { + RouteConfigProviderManager& route_config_provider_manager) { switch (config.route_specifier_case()) { case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: kRouteConfig: @@ -31,7 +31,7 @@ RouteConfigProviderPtr RouteConfigProviderUtil::create( factory_context); case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager::kRds: return route_config_provider_manager.createRdsRouteConfigProvider( - config.rds(), factory_context, stat_prefix, factory_context.initManager(), is_delta); + config.rds(), factory_context, stat_prefix, factory_context.initManager()); default: NOT_REACHED_GCOVR_EXCL_LINE; } @@ -56,7 +56,7 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager, bool is_delta) + Envoy::Router::RouteConfigProviderManagerImpl& route_config_provider_manager) : route_config_name_(rds.route_config_name()), factory_context_(factory_context), init_target_(fmt::format("RdsRouteConfigSubscription {}", route_config_name_), [this]() { subscription_->start({route_config_name_}); }), @@ -69,7 +69,7 @@ RdsRouteConfigSubscription::RdsRouteConfigSubscription( factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( rds.config_source(), Grpc::Common::typeUrl(envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name()), - *scope_, *this, is_delta); + *scope_, *this); config_update_info_ = std::make_unique( factory_context.timeSource(), factory_context.messageValidationVisitor()); @@ -225,7 +225,7 @@ RouteConfigProviderManagerImpl::RouteConfigProviderManagerImpl(Server::Admin& ad Router::RouteConfigProviderPtr RouteConfigProviderManagerImpl::createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - Init::Manager& init_manager, bool is_delta) { + Init::Manager& init_manager) { // RdsRouteConfigSubscriptions are unique based on their serialized RDS config. const uint64_t manager_identifier = MessageUtil::hash(rds); @@ -237,7 +237,7 @@ Router::RouteConfigProviderPtr RouteConfigProviderManagerImpl::createRdsRouteCon // around it. However, since this is not a performance critical path we err on the side // of simplicity. subscription.reset(new RdsRouteConfigSubscription(rds, manager_identifier, factory_context, - stat_prefix, *this, is_delta)); + stat_prefix, *this)); init_manager.add(subscription->init_target_); route_config_subscriptions_.insert({manager_identifier, subscription}); } else { diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index caf2340f084e5..418512bb6e50a 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -48,7 +48,7 @@ class RouteConfigProviderUtil { create(const envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager& config, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - RouteConfigProviderManager& route_config_provider_manager, bool is_delta); + RouteConfigProviderManager& route_config_provider_manager); }; class RouteConfigProviderManagerImpl; @@ -132,8 +132,8 @@ class RdsRouteConfigSubscription : Envoy::Config::SubscriptionCallbacks, RdsRouteConfigSubscription( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, const uint64_t manager_identifier, Server::Configuration::FactoryContext& factory_context, - const std::string& stat_prefix, RouteConfigProviderManagerImpl& route_config_provider_manager, - bool is_delta); + const std::string& stat_prefix, + RouteConfigProviderManagerImpl& route_config_provider_manager); bool validateUpdateSize(int num_resources); @@ -207,7 +207,7 @@ class RouteConfigProviderManagerImpl : public RouteConfigProviderManager, RouteConfigProviderPtr createRdsRouteConfigProvider( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Server::Configuration::FactoryContext& factory_context, const std::string& stat_prefix, - Init::Manager& init_manager, bool is_delta) override; + Init::Manager& init_manager) override; RouteConfigProviderPtr createStaticRouteConfigProvider(const envoy::api::v2::RouteConfiguration& route_config, diff --git a/source/common/router/scoped_rds.cc b/source/common/router/scoped_rds.cc index d7ae71b134bcf..ae77d119a5843 100644 --- a/source/common/router/scoped_rds.cc +++ b/source/common/router/scoped_rds.cc @@ -98,7 +98,7 @@ ScopedRdsConfigSubscription::ScopedRdsConfigSubscription( scoped_rds.scoped_rds_config_source(), Grpc::Common::typeUrl( envoy::api::v2::ScopedRouteConfiguration().GetDescriptor()->full_name()), - *scope_, *this, true); + *scope_, *this); initialize([scope_key_builder]() -> Envoy::Config::ConfigProvider::ConfigConstSharedPtr { return std::make_shared( @@ -115,7 +115,7 @@ ScopedRdsConfigSubscription::RdsRouteConfigProviderHelper::RdsRouteConfigProvide route_provider_(static_cast( parent_.route_config_provider_manager_ .createRdsRouteConfigProvider(rds, parent_.factory_context_, parent_.stat_prefix_, - init_manager, true) + init_manager) .release())), rds_update_callback_handle_(route_provider_->subscription().addUpdateCallback([this]() { // Subscribe to RDS update. diff --git a/source/common/router/vhds.cc b/source/common/router/vhds.cc index de67108a9e26c..8e715aef04dce 100644 --- a/source/common/router/vhds.cc +++ b/source/common/router/vhds.cc @@ -42,7 +42,7 @@ VhdsSubscription::VhdsSubscription(RouteConfigUpdatePtr& config_update_info, factory_context.clusterManager().subscriptionFactory().subscriptionFromConfigSource( config_update_info_->routeConfiguration().vhds().config_source(), Grpc::Common::typeUrl(envoy::api::v2::route::VirtualHost().GetDescriptor()->full_name()), - *scope_, *this, /*is_delta=*/true); + *scope_, *this); } void VhdsSubscription::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, diff --git a/source/common/runtime/runtime_impl.cc b/source/common/runtime/runtime_impl.cc index c01d3e8ce97be..9da583bdb8db2 100644 --- a/source/common/runtime/runtime_impl.cc +++ b/source/common/runtime/runtime_impl.cc @@ -563,7 +563,7 @@ void RtdsSubscription::start() { subscription_ = parent_.cm_->subscriptionFactory().subscriptionFromConfigSource( config_source_, Grpc::Common::typeUrl(envoy::service::discovery::v2::Runtime().GetDescriptor()->full_name()), - store_, *this, /*is_delta=*/false); + store_, *this); subscription_->start({resource_name_}); } diff --git a/source/common/secret/sds_api.cc b/source/common/secret/sds_api.cc index 3e3478f0beb7b..4514f5ce07922 100644 --- a/source/common/secret/sds_api.cc +++ b/source/common/secret/sds_api.cc @@ -79,7 +79,7 @@ void SdsApi::initialize() { subscription_ = subscription_factory_.subscriptionFromConfigSource( sds_config_, Grpc::Common::typeUrl(envoy::api::v2::auth::Secret().GetDescriptor()->full_name()), stats_, - *this, /*is_delta=*/false); + *this); subscription_->start({sds_config_name_}); } diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index 3269d58a2c3f5..82521e3e08915 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -15,22 +15,19 @@ namespace Envoy { namespace Upstream { -// TODO(fredlas) the is_delta argument can be removed upon delta+SotW ADS Envoy code unification. It -// is only actually needed to choose the grpc_method, which is irrelevant if ADS is used. -CdsApiPtr CdsApiImpl::create(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, +CdsApiPtr CdsApiImpl::create(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor) { - return CdsApiPtr{new CdsApiImpl(cds_config, is_delta, cm, scope, validation_visitor)}; + return CdsApiPtr{new CdsApiImpl(cds_config, cm, scope, validation_visitor)}; } -CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, - ClusterManager& cm, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor) +CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, + Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor) : cm_(cm), scope_(scope.createScope("cluster_manager.cds.")), validation_visitor_(validation_visitor) { subscription_ = cm_.subscriptionFactory().subscriptionFromConfigSource( cds_config, Grpc::Common::typeUrl(envoy::api::v2::Cluster().GetDescriptor()->full_name()), - *scope_, *this, is_delta); + *scope_, *this); } void CdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 66bb7e4c3df8b..b17d4bbc9989d 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -22,8 +22,8 @@ class CdsApiImpl : public CdsApi, Config::SubscriptionCallbacks, Logger::Loggable { public: - static CdsApiPtr create(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, - ClusterManager& cm, Stats::Scope& scope, + static CdsApiPtr create(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, + Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor); // Upstream::CdsApi @@ -46,9 +46,8 @@ class CdsApiImpl : public CdsApi, return MessageUtil::anyConvert(resource).name(); } - CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, - ClusterManager& cm, Stats::Scope& scope, - ProtobufMessage::ValidationVisitor& validation_visitor); + CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm, + Stats::Scope& scope, ProtobufMessage::ValidationVisitor& validation_visitor); void runInitializeCallbackIfAny(); ClusterManager& cm_; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index a08dc584f3446..752f7782d61aa 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -221,32 +221,7 @@ ClusterManagerImpl::ClusterManagerImpl( } } - // TODO(fredlas) HACK to support - // loadCluster->clusterFromProto->ClusterFactoryImplBase::create->EdsClusterFactory::createClusterImpl(), - // which wants to call xdsIsDelta() on us. So, we need to get our xds_is_delta_ defined before - // then. Once SotW and delta are unified, that is_delta bool will be gone from everywhere, and the - // xds_is_delta_ variable can be removed. const auto& dyn_resources = bootstrap.dynamic_resources(); - if (dyn_resources.has_ads_config()) { - xds_is_delta_ = - dyn_resources.ads_config().api_type() == envoy::api::v2::core::ApiConfigSource::DELTA_GRPC; - } else if (dyn_resources.has_cds_config()) { - const auto& cds_config = dyn_resources.cds_config(); - xds_is_delta_ = - cds_config.api_config_source().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC || - (dyn_resources.has_ads_config() && dyn_resources.ads_config().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC); - } else if (dyn_resources.has_lds_config()) { - const auto& lds_config = dyn_resources.lds_config(); - xds_is_delta_ = - lds_config.api_config_source().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC || - (dyn_resources.has_ads_config() && dyn_resources.ads_config().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC); - } else { - xds_is_delta_ = false; - } // Cluster loading happens in two phases: first all the primary clusters are loaded, and then all // the secondary clusters are loaded. As it currently stands all non-EDS clusters are primary and @@ -326,7 +301,7 @@ ClusterManagerImpl::ClusterManagerImpl( // We can now potentially create the CDS API once the backing cluster exists. if (dyn_resources.has_cds_config()) { - cds_api_ = factory_.createCds(dyn_resources.cds_config(), xds_is_delta_, *this); + cds_api_ = factory_.createCds(dyn_resources.cds_config(), *this); init_helper_.setCds(cds_api_.get()); } else { init_helper_.setCds(nullptr); @@ -1366,10 +1341,9 @@ std::pair ProdClusterManagerFactor } CdsApiPtr ProdClusterManagerFactory::createCds(const envoy::api::v2::core::ConfigSource& cds_config, - bool is_delta, ClusterManager& cm) { + ClusterManager& cm) { // TODO(htuch): Differentiate static vs. dynamic validation visitors. - return CdsApiImpl::create(cds_config, is_delta, cm, stats_, - validation_context_.dynamicValidationVisitor()); + return CdsApiImpl::create(cds_config, cm, stats_, validation_context_.dynamicValidationVisitor()); } } // namespace Upstream diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index 068ab7dafe4ca..b1567a5c94cfb 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -68,7 +68,7 @@ class ProdClusterManagerFactory : public ClusterManagerFactory { std::pair clusterFromProto(const envoy::api::v2::Cluster& cluster, ClusterManager& cm, Outlier::EventLoggerSharedPtr outlier_event_logger, bool added_via_api) override; - CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, + CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm) override; Secret::SecretManager& secretManager() override { return secret_manager_; } @@ -238,9 +238,6 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggablefull_name()), - info_->statsScope(), *this, is_delta); + info_->statsScope(), *this); } void EdsClusterImpl::startPreInit() { subscription_->start({cluster_name_}); } @@ -269,8 +269,7 @@ EdsClusterFactory::createClusterImpl( return std::make_pair( std::make_unique(cluster, context.runtime(), socket_factory_context, - std::move(stats_scope), context.addedViaApi(), - context.clusterManager().xdsIsDelta()), + std::move(stats_scope), context.addedViaApi()), nullptr); } diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 3f0a74b8dfca6..0df5f4c844736 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -24,7 +24,7 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba public: EdsClusterImpl(const envoy::api::v2::Cluster& cluster, Runtime::Loader& runtime, Server::Configuration::TransportSocketFactoryContext& factory_context, - Stats::ScopePtr&& stats_scope, bool added_via_api, bool is_delta); + Stats::ScopePtr&& stats_scope, bool added_via_api); // Upstream::Cluster InitializePhase initializePhase() const override { return InitializePhase::Secondary; } diff --git a/source/extensions/filters/network/http_connection_manager/config.cc b/source/extensions/filters/network/http_connection_manager/config.cc index aa47daf32a279..767b93b26d918 100644 --- a/source/extensions/filters/network/http_connection_manager/config.cc +++ b/source/extensions/filters/network/http_connection_manager/config.cc @@ -195,8 +195,7 @@ HttpConnectionManagerConfig::HttpConnectionManagerConfig( case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: kRouteConfig: route_config_provider_ = Router::RouteConfigProviderUtil::create( - config, context_, stats_prefix_, route_config_provider_manager_, - context_.clusterManager().xdsIsDelta()); + config, context_, stats_prefix_, route_config_provider_manager_); break; case envoy::config::filter::network::http_connection_manager::v2::HttpConnectionManager:: kScopedRoutes: diff --git a/source/server/config_validation/cluster_manager.cc b/source/server/config_validation/cluster_manager.cc index 0f014a1741ea1..cf20a6c0e2217 100644 --- a/source/server/config_validation/cluster_manager.cc +++ b/source/server/config_validation/cluster_manager.cc @@ -14,9 +14,9 @@ ClusterManagerPtr ValidationClusterManagerFactory::clusterManagerFromProto( CdsApiPtr ValidationClusterManagerFactory::createCds(const envoy::api::v2::core::ConfigSource& cds_config, - bool is_delta, ClusterManager& cm) { + ClusterManager& cm) { // Create the CdsApiImpl... - ProdClusterManagerFactory::createCds(cds_config, is_delta, cm); + ProdClusterManagerFactory::createCds(cds_config, cm); // ... and then throw it away, so that we don't actually connect to it. return nullptr; } diff --git a/source/server/config_validation/cluster_manager.h b/source/server/config_validation/cluster_manager.h index b62387c3341d6..0fe33294d1904 100644 --- a/source/server/config_validation/cluster_manager.h +++ b/source/server/config_validation/cluster_manager.h @@ -38,7 +38,7 @@ class ValidationClusterManagerFactory : public ProdClusterManagerFactory { // Delegates to ProdClusterManagerFactory::createCds, but discards the result and returns nullptr // unconditionally. - CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, bool is_delta, + CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm) override; private: diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 58ad9affadc38..d447c58229406 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -109,11 +109,10 @@ class ValidationInstance : Logger::Loggable, } // Server::ListenerComponentFactory - LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config, - bool is_delta) override { - return std::make_unique( - lds_config, clusterManager(), initManager(), stats(), listenerManager(), - messageValidationContext().dynamicValidationVisitor(), is_delta); + LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { + return std::make_unique(lds_config, clusterManager(), initManager(), stats(), + listenerManager(), + messageValidationContext().dynamicValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index ca5cb3ed187d5..5254fdbac3dc0 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -17,13 +17,13 @@ namespace Server { LdsApiImpl::LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, Upstream::ClusterManager& cm, Init::Manager& init_manager, Stats::Scope& scope, ListenerManager& lm, - ProtobufMessage::ValidationVisitor& validation_visitor, bool is_delta) + ProtobufMessage::ValidationVisitor& validation_visitor) : listener_manager_(lm), scope_(scope.createScope("listener_manager.lds.")), cm_(cm), init_target_("LDS", [this]() { subscription_->start({}); }), validation_visitor_(validation_visitor) { subscription_ = cm.subscriptionFactory().subscriptionFromConfigSource( lds_config, Grpc::Common::typeUrl(envoy::api::v2::Listener().GetDescriptor()->full_name()), - *scope_, *this, is_delta); + *scope_, *this); init_manager.add(init_target_); } diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 2ef9209542b2c..24ca66cfe73f2 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -24,7 +24,7 @@ class LdsApiImpl : public LdsApi, public: LdsApiImpl(const envoy::api::v2::core::ConfigSource& lds_config, Upstream::ClusterManager& cm, Init::Manager& init_manager, Stats::Scope& scope, ListenerManager& lm, - ProtobufMessage::ValidationVisitor& validation_visitor, bool is_delta); + ProtobufMessage::ValidationVisitor& validation_visitor); // Server::LdsApi std::string versionInfo() const override { return system_version_info_; } diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h index ca00bc11082e8..599f471ea7fe3 100644 --- a/source/server/listener_manager_impl.h +++ b/source/server/listener_manager_impl.h @@ -59,12 +59,10 @@ class ProdListenerComponentFactory : public ListenerComponentFactory, Configuration::ListenerFactoryContext& context); // Server::ListenerComponentFactory - LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config, - bool is_delta) override { + LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { return std::make_unique( lds_config, server_.clusterManager(), server_.initManager(), server_.stats(), - server_.listenerManager(), server_.messageValidationContext().dynamicValidationVisitor(), - is_delta); + server_.listenerManager(), server_.messageValidationContext().dynamicValidationVisitor()); } std::vector createNetworkFilterFactoryList( const Protobuf::RepeatedPtrField& filters, @@ -129,9 +127,9 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable> listeners() override; uint64_t numConnections() override; diff --git a/source/server/server.cc b/source/server/server.cc index 8aba537de9b2d..aa5ccb8aef286 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -400,13 +400,7 @@ void InstanceImpl::initialize(const Options& options, // 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()) { - const bool is_delta = - bootstrap_.dynamic_resources().lds_config().api_config_source().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC || - (bootstrap_.dynamic_resources().has_ads_config() && - bootstrap_.dynamic_resources().ads_config().api_type() == - envoy::api::v2::core::ApiConfigSource::DELTA_GRPC); - listener_manager_->createLdsApi(bootstrap_.dynamic_resources().lds_config(), is_delta); + listener_manager_->createLdsApi(bootstrap_.dynamic_resources().lds_config()); } // We have to defer RTDS initialization until after the cluster manager is diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index 6c0138cbaf04d..228aaeba9d79e 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -32,9 +32,9 @@ MATCHER_P(WithName, expectedName, "") { return arg.name() == expectedName; } class CdsApiImplTest : public testing::Test { protected: - void setup(bool is_delta = false) { + void setup() { envoy::api::v2::core::ConfigSource cds_config; - cds_ = CdsApiImpl::create(cds_config, is_delta, cm_, store_, validation_visitor_); + cds_ = CdsApiImpl::create(cds_config, cm_, store_, validation_visitor_); cds_->setInitializedCb([this]() -> void { initialized_.ready(); }); EXPECT_CALL(*cm_.subscription_factory_.subscription_, start(_)); diff --git a/test/mocks/config/mocks.cc b/test/mocks/config/mocks.cc index 7d2846e44a6d4..4a3e3099e0c15 100644 --- a/test/mocks/config/mocks.cc +++ b/test/mocks/config/mocks.cc @@ -10,15 +10,15 @@ namespace Envoy { namespace Config { MockSubscriptionFactory::MockSubscriptionFactory() { - ON_CALL(*this, subscriptionFromConfigSource(_, _, _, _, _)) - .WillByDefault(testing::Invoke( - [this](const envoy::api::v2::core::ConfigSource&, absl::string_view, Stats::Scope&, - SubscriptionCallbacks& callbacks, bool) -> SubscriptionPtr { - auto ret = std::make_unique>(); - subscription_ = ret.get(); - callbacks_ = &callbacks; - return ret; - })); + ON_CALL(*this, subscriptionFromConfigSource(_, _, _, _)) + .WillByDefault(testing::Invoke([this](const envoy::api::v2::core::ConfigSource&, + absl::string_view, Stats::Scope&, + SubscriptionCallbacks& callbacks) -> SubscriptionPtr { + auto ret = std::make_unique>(); + subscription_ = ret.get(); + callbacks_ = &callbacks; + return ret; + })); ON_CALL(*this, messageValidationVisitor()) .WillByDefault(testing::ReturnRef(ProtobufMessage::getStrictValidationVisitor())); } diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 7b88cea67c880..7fdf475521970 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -52,10 +52,10 @@ class MockSubscriptionFactory : public SubscriptionFactory { MockSubscriptionFactory(); ~MockSubscriptionFactory() override; - MOCK_METHOD5(subscriptionFromConfigSource, + MOCK_METHOD4(subscriptionFromConfigSource, SubscriptionPtr(const envoy::api::v2::core::ConfigSource& config, absl::string_view type_url, Stats::Scope& scope, - SubscriptionCallbacks& callbacks, bool is_delta)); + SubscriptionCallbacks& callbacks)); MOCK_METHOD0(messageValidationVisitor, ProtobufMessage::ValidationVisitor&()); MockSubscription* subscription_{}; diff --git a/test/mocks/router/mocks.h b/test/mocks/router/mocks.h index f355354fa54dd..3447c03a72dd1 100644 --- a/test/mocks/router/mocks.h +++ b/test/mocks/router/mocks.h @@ -412,11 +412,11 @@ class MockRouteConfigProviderManager : public RouteConfigProviderManager { MockRouteConfigProviderManager(); ~MockRouteConfigProviderManager() override; - MOCK_METHOD5(createRdsRouteConfigProvider, + MOCK_METHOD4(createRdsRouteConfigProvider, RouteConfigProviderPtr( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, Server::Configuration::FactoryContext& factory_context, - const std::string& stat_prefix, Init::Manager& init_manager, bool is_delta)); + const std::string& stat_prefix, Init::Manager& init_manager)); MOCK_METHOD2(createStaticRouteConfigProvider, RouteConfigProviderPtr(const envoy::api::v2::RouteConfiguration& route_config, Server::Configuration::FactoryContext& factory_context)); diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 1487bc0b88529..bcbc232bd2408 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -234,13 +234,11 @@ class MockListenerComponentFactory : public ListenerComponentFactory { DrainManagerPtr createDrainManager(envoy::api::v2::Listener::DrainType drain_type) override { return DrainManagerPtr{createDrainManager_(drain_type)}; } - LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config, - bool is_delta) override { - return LdsApiPtr{createLdsApi_(lds_config, is_delta)}; + LdsApiPtr createLdsApi(const envoy::api::v2::core::ConfigSource& lds_config) override { + return LdsApiPtr{createLdsApi_(lds_config)}; } - MOCK_METHOD2(createLdsApi_, - LdsApi*(const envoy::api::v2::core::ConfigSource& lds_config, bool is_delta)); + MOCK_METHOD1(createLdsApi_, LdsApi*(const envoy::api::v2::core::ConfigSource& lds_config)); MOCK_METHOD2(createNetworkFilterFactoryList, std::vector( const Protobuf::RepeatedPtrField& filters, @@ -271,8 +269,7 @@ class MockListenerManager : public ListenerManager { MOCK_METHOD3(addOrUpdateListener, bool(const envoy::api::v2::Listener& config, const std::string& version_info, bool modifiable)); - MOCK_METHOD2(createLdsApi, - void(const envoy::api::v2::core::ConfigSource& lds_config, bool is_delta)); + MOCK_METHOD1(createLdsApi, void(const envoy::api::v2::core::ConfigSource& lds_config)); MOCK_METHOD0(listeners, std::vector>()); MOCK_METHOD0(numConnections, uint64_t()); MOCK_METHOD1(removeListener, bool(const std::string& listener_name)); diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 7b6b22f8cf784..e2469f6efb6e7 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -269,8 +269,8 @@ class MockClusterManagerFactory : public ClusterManagerFactory { const envoy::api::v2::Cluster& cluster, ClusterManager& cm, Outlier::EventLoggerSharedPtr outlier_event_logger, bool added_via_api)); - MOCK_METHOD3(createCds, CdsApiPtr(const envoy::api::v2::core::ConfigSource& cds_config, - bool is_delta, ClusterManager& cm)); + MOCK_METHOD2(createCds, + CdsApiPtr(const envoy::api::v2::core::ConfigSource& cds_config, ClusterManager& cm)); private: NiceMock secret_manager_; @@ -328,7 +328,6 @@ class MockClusterManager : public ClusterManager { MOCK_METHOD1(addThreadLocalClusterUpdateCallbacks_, ClusterUpdateCallbacksHandle*(ClusterUpdateCallbacks& callbacks)); MOCK_CONST_METHOD0(warmingClusterCount, std::size_t()); - MOCK_CONST_METHOD0(xdsIsDelta, bool()); MOCK_METHOD0(subscriptionFactory, Config::SubscriptionFactory&()); NiceMock conn_pool_; From 1fb26a6cace28e7ff1e807ac78d95caa86bea40f Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Fri, 11 Oct 2019 16:03:34 -0400 Subject: [PATCH 4/7] ahhhh shoulda known i would need to run a test/... Signed-off-by: Fred Douglas --- .../config/subscription_factory_impl_test.cc | 2 +- test/common/router/rds_impl_test.cc | 14 +++++++------- test/common/router/scoped_rds_test.cc | 6 +++--- test/common/runtime/runtime_impl_test.cc | 4 ++-- test/common/upstream/cds_api_impl_test.cc | 2 +- test/common/upstream/cluster_manager_impl_test.cc | 2 +- test/common/upstream/eds_test.cc | 4 ++-- test/server/lds_api_test.cc | 2 +- test/server/listener_manager_impl_test.cc | 4 ++-- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/test/common/config/subscription_factory_impl_test.cc b/test/common/config/subscription_factory_impl_test.cc index 2426c10d83951..403ad4d5f91f9 100644 --- a/test/common/config/subscription_factory_impl_test.cc +++ b/test/common/config/subscription_factory_impl_test.cc @@ -38,7 +38,7 @@ class SubscriptionFactoryTest : public testing::Test { return SubscriptionFactoryImpl(local_info_, dispatcher_, cm_, random_, validation_visitor_, *api_) .subscriptionFromConfigSource(config, Config::TypeUrl::get().ClusterLoadAssignment, - stats_store_, callbacks_, false); + stats_store_, callbacks_); } Upstream::MockClusterManager cm_; diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 942c7bdb69657..564b1c01531a6 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -89,9 +89,9 @@ stat_prefix: foo )EOF"; EXPECT_CALL(factory_context_.init_manager_, add(_)); - rds_ = RouteConfigProviderUtil::create(parseHttpConnectionManagerFromYaml(config_yaml), - factory_context_, "foo.", - *route_config_provider_manager_, false); + rds_ = + RouteConfigProviderUtil::create(parseHttpConnectionManagerFromYaml(config_yaml), + factory_context_, "foo.", *route_config_provider_manager_); rds_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; EXPECT_CALL(*factory_context_.cluster_manager_.subscription_factory_.subscription_, start(_)); factory_context_.init_manager_.initialize(init_watcher_); @@ -121,7 +121,7 @@ stat_prefix: foo EXPECT_THROW(RouteConfigProviderUtil::create(parseHttpConnectionManagerFromYaml(config_yaml), factory_context_, "foo.", - *route_config_provider_manager_, false), + *route_config_provider_manager_), EnvoyException); } @@ -259,7 +259,7 @@ class RouteConfigProviderManagerImplTest : public RdsTestBase { rds_.set_route_config_name("foo_route_config"); rds_.mutable_config_source()->set_path("foo_path"); provider_ = route_config_provider_manager_->createRdsRouteConfigProvider( - rds_, factory_context_, "foo_prefix.", factory_context_.initManager(), false); + rds_, factory_context_, "foo_prefix.", factory_context_.initManager()); rds_callbacks_ = factory_context_.cluster_manager_.subscription_factory_.callbacks_; } @@ -411,7 +411,7 @@ name: foo_route_config "1"); RouteConfigProviderPtr provider2 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds_, factory_context_, "foo_prefix", factory_context_.initManager(), false); + rds_, factory_context_, "foo_prefix", factory_context_.initManager()); // provider2 should have route config immediately after create EXPECT_TRUE(provider2->configInfo().has_value()); @@ -425,7 +425,7 @@ name: foo_route_config rds2.set_route_config_name("foo_route_config"); rds2.mutable_config_source()->set_path("bar_path"); RouteConfigProviderPtr provider3 = route_config_provider_manager_->createRdsRouteConfigProvider( - rds2, factory_context_, "foo_prefix", factory_context_.initManager(), false); + rds2, factory_context_, "foo_prefix", factory_context_.initManager()); EXPECT_NE(provider3, provider_); factory_context_.cluster_manager_.subscription_factory_.callbacks_->onConfigUpdate(route_configs, "provider3"); diff --git a/test/common/router/scoped_rds_test.cc b/test/common/router/scoped_rds_test.cc index d8a3fcd5ff23d..8ac915cd11f14 100644 --- a/test/common/router/scoped_rds_test.cc +++ b/test/common/router/scoped_rds_test.cc @@ -104,18 +104,18 @@ class ScopedRdsTest : public ScopedRoutesTestBase { // To build the map from RDS route_config_name to the RDS subscription, we need to get the // route_config_name by mocking start() on the Config::Subscription. EXPECT_CALL(factory_context_.cluster_manager_.subscription_factory_, - subscriptionFromConfigSource(_, _, _, _, _)) + subscriptionFromConfigSource(_, _, _, _)) .Times(AnyNumber()); EXPECT_CALL(factory_context_.cluster_manager_.subscription_factory_, subscriptionFromConfigSource( _, Eq(Grpc::Common::typeUrl( envoy::api::v2::RouteConfiguration().GetDescriptor()->full_name())), - _, _, _)) + _, _)) .Times(AnyNumber()) .WillRepeatedly(Invoke([this](const envoy::api::v2::core::ConfigSource&, absl::string_view, Stats::Scope&, - Envoy::Config::SubscriptionCallbacks& callbacks, bool) { + Envoy::Config::SubscriptionCallbacks& callbacks) { auto ret = std::make_unique>(); rds_subscription_by_config_subscription_[ret.get()] = &callbacks; EXPECT_CALL(*ret, start(_)) diff --git a/test/common/runtime/runtime_impl_test.cc b/test/common/runtime/runtime_impl_test.cc index 0a9350e47971d..6e66970c2a7b4 100644 --- a/test/common/runtime/runtime_impl_test.cc +++ b/test/common/runtime/runtime_impl_test.cc @@ -814,10 +814,10 @@ class RtdsLoaderImplTest : public LoaderImplTest { EXPECT_CALL(init_manager_, add(_)).WillRepeatedly(Invoke([this](const Init::Target& target) { init_target_handles_.emplace_back(target.createHandle("test")); })); - ON_CALL(cm_.subscription_factory_, subscriptionFromConfigSource(_, _, _, _, _)) + ON_CALL(cm_.subscription_factory_, subscriptionFromConfigSource(_, _, _, _)) .WillByDefault(testing::Invoke( [this](const envoy::api::v2::core::ConfigSource&, absl::string_view, Stats::Scope&, - Config::SubscriptionCallbacks& callbacks, bool) -> Config::SubscriptionPtr { + Config::SubscriptionCallbacks& callbacks) -> Config::SubscriptionPtr { auto ret = std::make_unique>(); rtds_subscriptions_.push_back(ret.get()); rtds_callbacks_.push_back(&callbacks); diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index 228aaeba9d79e..fe6b5e39ab603 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -180,7 +180,7 @@ TEST_F(CdsApiImplTest, ConfigUpdateWith2ValidClusters) { TEST_F(CdsApiImplTest, DeltaConfigUpdate) { { InSequence s; - setup(true); + setup(); } EXPECT_CALL(initialized_, ready()); diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 26388b5811f6a..615824ac11d2a 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -102,7 +102,7 @@ class TestClusterManagerFactory : public ClusterManagerFactory { return std::make_pair(result.first, ThreadAwareLoadBalancerPtr(result.second)); } - CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource&, bool, ClusterManager&) override { + CdsApiPtr createCds(const envoy::api::v2::core::ConfigSource&, ClusterManager&) override { return CdsApiPtr{createCds_()}; } diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 1e520b8151636..9247c9d84eb86 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -75,8 +75,8 @@ class EdsTest : public testing::Test { Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, random_, stats_, singleton_manager_, tls_, validation_visitor_, *api_); - cluster_.reset(new EdsClusterImpl(eds_cluster_, runtime_, factory_context, std::move(scope), - false, false)); + cluster_.reset( + new EdsClusterImpl(eds_cluster_, runtime_, factory_context, std::move(scope), false)); EXPECT_EQ(Cluster::InitializePhase::Secondary, cluster_->initializePhase()); eds_callbacks_ = cm_.subscription_factory_.callbacks_; } diff --git a/test/server/lds_api_test.cc b/test/server/lds_api_test.cc index a15788822fef0..ed90b013ad94a 100644 --- a/test/server/lds_api_test.cc +++ b/test/server/lds_api_test.cc @@ -36,7 +36,7 @@ class LdsApiTest : public testing::Test { envoy::api::v2::core::ConfigSource lds_config; EXPECT_CALL(init_manager_, add(_)); lds_ = std::make_unique(lds_config, cluster_manager_, init_manager_, store_, - listener_manager_, validation_visitor_, false); + listener_manager_, validation_visitor_); EXPECT_CALL(*cluster_manager_.subscription_factory_.subscription_, start(_)); init_target_handle_->initialize(init_watcher_); lds_callbacks_ = cluster_manager_.subscription_factory_.callbacks_; diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index dd4170eb1544c..615fd0add191c 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -591,9 +591,9 @@ TEST_F(ListenerManagerImplTest, AddOrUpdateListener) { InSequence s; auto* lds_api = new MockLdsApi(); - EXPECT_CALL(listener_factory_, createLdsApi_(_, _)).WillOnce(Return(lds_api)); + EXPECT_CALL(listener_factory_, createLdsApi_(_)).WillOnce(Return(lds_api)); envoy::api::v2::core::ConfigSource lds_config; - manager_->createLdsApi(lds_config, false); + manager_->createLdsApi(lds_config); EXPECT_CALL(*lds_api, versionInfo()).WillOnce(Return("")); checkConfigDump(R"EOF( From 3fee2f0097f8e00324324454f0d885638429aa01 Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Mon, 14 Oct 2019 10:38:39 -0400 Subject: [PATCH 5/7] Kick CI Signed-off-by: Fred Douglas From 4286c1ce6ae93151b010dad8ef3a5fcaeb988e83 Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Fri, 18 Oct 2019 12:24:36 -0400 Subject: [PATCH 6/7] test coverage for two return const value functions lol the coverage requirement is really on a knife edge... Signed-off-by: Fred Douglas --- .../config/api_type_db.generated.pb_text | 34 +++++++++++++++++++ test/common/config/grpc_mux_impl_test.cc | 8 +++++ test/common/config/new_grpc_mux_impl_test.cc | 6 ++++ 3 files changed, 48 insertions(+) diff --git a/source/common/config/api_type_db.generated.pb_text b/source/common/config/api_type_db.generated.pb_text index dd963044eb702..d473694baa490 100644 --- a/source/common/config/api_type_db.generated.pb_text +++ b/source/common/config/api_type_db.generated.pb_text @@ -4039,6 +4039,14 @@ types { next_version_type_name: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.FilterConfig" } } +types { + key: "envoy.config.filter.http.dynamic_forward_proxy.v2alpha.PerRouteConfig" + value { + qualified_package: "envoy.config.filter.http.dynamic_forward_proxy.v2alpha" + proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v2alpha/dynamic_forward_proxy.proto" + next_version_type_name: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.PerRouteConfig" + } +} types { key: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.FilterConfig" value { @@ -4046,6 +4054,13 @@ types { proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v3alpha/dynamic_forward_proxy.proto" } } +types { + key: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.PerRouteConfig" + value { + qualified_package: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha" + proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v3alpha/dynamic_forward_proxy.proto" + } +} types { key: "envoy.config.filter.http.ext_authz.v2.AuthorizationRequest" value { @@ -7517,6 +7532,14 @@ types { proto_path: "envoy/service/trace/v3alpha/trace_service.proto" } } +types { + key: "envoy.type.CodecClientType" + value { + qualified_package: "envoy.type" + proto_path: "envoy/type/http.proto" + next_version_type_name: "envoy.type.v3alpha.CodecClientType" + } +} types { key: "envoy.type.DoubleRange" value { @@ -7723,6 +7746,13 @@ types { proto_path: "envoy/type/matcher/v3alpha/value.proto" } } +types { + key: "envoy.type.v3alpha.CodecClientType" + value { + qualified_package: "envoy.type.v3alpha" + proto_path: "envoy/type/v3alpha/http.proto" + } +} types { key: "envoy.type.v3alpha.DoubleRange" value { @@ -8380,6 +8410,10 @@ next_version_proto_paths { key: "envoy/service/trace/v2/trace_service.proto" value: "envoy/service/trace/v3alpha/trace_service.proto" } +next_version_proto_paths { + key: "envoy/type/http.proto" + value: "envoy/type/v3alpha/http.proto" +} next_version_proto_paths { key: "envoy/type/http_status.proto" value: "envoy/type/v3alpha/http_status.proto" diff --git a/test/common/config/grpc_mux_impl_test.cc b/test/common/config/grpc_mux_impl_test.cc index d45169d387bdc..d498dc5efaa76 100644 --- a/test/common/config/grpc_mux_impl_test.cc +++ b/test/common/config/grpc_mux_impl_test.cc @@ -105,6 +105,14 @@ class GrpcMuxImplTest : public GrpcMuxImplTestBase { Event::SimulatedTimeSystem time_system_; }; +// TODO(fredlas) #8478 will delete this. +TEST_F(GrpcMuxImplTest, JustForCoverageTodoDelete) { + setup(); + NullGrpcMuxImpl fake; + EXPECT_FALSE(grpc_mux_->isDelta()); + EXPECT_FALSE(fake.isDelta()); +} + // Validate behavior when multiple type URL watches are maintained, watches are created/destroyed // (via RAII). TEST_F(GrpcMuxImplTest, MultipleTypeUrlStreams) { diff --git a/test/common/config/new_grpc_mux_impl_test.cc b/test/common/config/new_grpc_mux_impl_test.cc index 68047881d54b6..57b1373dab22b 100644 --- a/test/common/config/new_grpc_mux_impl_test.cc +++ b/test/common/config/new_grpc_mux_impl_test.cc @@ -68,6 +68,12 @@ class NewGrpcMuxImplTest : public NewGrpcMuxImplTestBase { Event::SimulatedTimeSystem time_system_; }; +// TODO(fredlas) #8478 will delete this. +TEST_F(NewGrpcMuxImplTest, JustForCoverageTodoDelete) { + setup(); + EXPECT_TRUE(grpc_mux_->isDelta()); +} + // Test that we simply ignore a message for an unknown type_url, with no ill effects. TEST_F(NewGrpcMuxImplTest, DiscoveryResponseNonexistentSub) { setup(); From 174fb4fda9e26f816d4fb6f1accf18835c4e99b6 Mon Sep 17 00:00:00 2001 From: Fred Douglas Date: Mon, 21 Oct 2019 15:43:59 -0400 Subject: [PATCH 7/7] restore exactly the api_type_db copy currently living in master Signed-off-by: Fred Douglas --- .../config/api_type_db.generated.pb_text | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/source/common/config/api_type_db.generated.pb_text b/source/common/config/api_type_db.generated.pb_text index dd963044eb702..94afe82b5a15f 100644 --- a/source/common/config/api_type_db.generated.pb_text +++ b/source/common/config/api_type_db.generated.pb_text @@ -1921,6 +1921,14 @@ types { next_version_type_name: "envoy.api.v3alpha.route.RouteMatch.GrpcRouteMatchOptions" } } +types { + key: "envoy.api.v2.route.RouteMatch.TlsContextMatchOptions" + value { + qualified_package: "envoy.api.v2.route" + proto_path: "envoy/api/v2/route/route.proto" + next_version_type_name: "envoy.api.v3alpha.route.RouteMatch.TlsContextMatchOptions" + } +} types { key: "envoy.api.v2.route.Tracing" value { @@ -3198,6 +3206,13 @@ types { proto_path: "envoy/api/v3alpha/route/route.proto" } } +types { + key: "envoy.api.v3alpha.route.RouteMatch.TlsContextMatchOptions" + value { + qualified_package: "envoy.api.v3alpha.route" + proto_path: "envoy/api/v3alpha/route/route.proto" + } +} types { key: "envoy.api.v3alpha.route.Tracing" value { @@ -4039,6 +4054,14 @@ types { next_version_type_name: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.FilterConfig" } } +types { + key: "envoy.config.filter.http.dynamic_forward_proxy.v2alpha.PerRouteConfig" + value { + qualified_package: "envoy.config.filter.http.dynamic_forward_proxy.v2alpha" + proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v2alpha/dynamic_forward_proxy.proto" + next_version_type_name: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.PerRouteConfig" + } +} types { key: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.FilterConfig" value { @@ -4046,6 +4069,13 @@ types { proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v3alpha/dynamic_forward_proxy.proto" } } +types { + key: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha.PerRouteConfig" + value { + qualified_package: "envoy.config.filter.http.dynamic_forward_proxy.v3alpha" + proto_path: "envoy/config/filter/http/dynamic_forward_proxy/v3alpha/dynamic_forward_proxy.proto" + } +} types { key: "envoy.config.filter.http.ext_authz.v2.AuthorizationRequest" value { @@ -4203,6 +4233,13 @@ types { proto_path: "envoy/config/filter/http/grpc_http1_reverse_bridge/v2alpha1/config.proto" } } +types { + key: "envoy.config.filter.http.grpc_http1_reverse_bridge.v2alpha1.FilterConfigPerRoute" + value { + qualified_package: "envoy.config.filter.http.grpc_http1_reverse_bridge.v2alpha1" + proto_path: "envoy/config/filter/http/grpc_http1_reverse_bridge/v2alpha1/config.proto" + } +} types { key: "envoy.config.filter.http.gzip.v2.Gzip" value { @@ -7517,6 +7554,14 @@ types { proto_path: "envoy/service/trace/v3alpha/trace_service.proto" } } +types { + key: "envoy.type.CodecClientType" + value { + qualified_package: "envoy.type" + proto_path: "envoy/type/http.proto" + next_version_type_name: "envoy.type.v3alpha.CodecClientType" + } +} types { key: "envoy.type.DoubleRange" value { @@ -7723,6 +7768,13 @@ types { proto_path: "envoy/type/matcher/v3alpha/value.proto" } } +types { + key: "envoy.type.v3alpha.CodecClientType" + value { + qualified_package: "envoy.type.v3alpha" + proto_path: "envoy/type/v3alpha/http.proto" + } +} types { key: "envoy.type.v3alpha.DoubleRange" value { @@ -8380,6 +8432,10 @@ next_version_proto_paths { key: "envoy/service/trace/v2/trace_service.proto" value: "envoy/service/trace/v3alpha/trace_service.proto" } +next_version_proto_paths { + key: "envoy/type/http.proto" + value: "envoy/type/v3alpha/http.proto" +} next_version_proto_paths { key: "envoy/type/http_status.proto" value: "envoy/type/v3alpha/http_status.proto"