diff --git a/api/envoy/admin/v2/config_dump.proto b/api/envoy/admin/v2/config_dump.proto deleted file mode 100644 index 45994d812e53..000000000000 --- a/api/envoy/admin/v2/config_dump.proto +++ /dev/null @@ -1,35 +0,0 @@ -syntax = "proto3"; - -package envoy.admin.v2; - -import "google/protobuf/any.proto"; -import "envoy/api/v2/rds.proto"; - -import "gogoproto/gogo.proto"; - -// [#protodoc-title: ConfigDump] -// [#proto-status: draft] - -// The /config_dump admin endpoint uses this wrapper message to maintain and serve arbitrary -// configuration information from any component in Envoy. -// TODO(jsedgwick) In the future, we may want to formalize this further with an RPC for config_dump, -// and perhaps even with an RPC per config type. That strategy across all endpoints will allow for -// more flexibility w.r.t. protocol, serialization, parameters, etc. -message ConfigDump { - // This map is serialized and dumped in its entirety at the /config_dump endpoint. - // - // Keys should be a short descriptor of the config object they map to. For example, Envoy's HTTP - // routing subsystem might use "routes" as the key for its config, for which it uses the message - // RouteConfigDump as defined below. In the future, the key will also be used to filter the output - // of the /config_dump endpoint. - map configs = 1 [(gogoproto.nullable) = false]; -} - -// Envoy's RDS implementation fills this message with all currently loaded routes, as described by -// their RouteConnfiguration objects. Static routes configured in the bootstrap configuration are -// separated from those configured dynamically via RDS. This message is available at the -// /config_dump admin endpoint. -message RouteConfigDump { - repeated envoy.api.v2.RouteConfiguration static_route_configs = 1 [(gogoproto.nullable) = false]; - repeated envoy.api.v2.RouteConfiguration dynamic_route_configs = 2 [(gogoproto.nullable) = false]; -} diff --git a/api/envoy/admin/v2/BUILD b/api/envoy/admin/v2alpha/BUILD similarity index 70% rename from api/envoy/admin/v2/BUILD rename to api/envoy/admin/v2alpha/BUILD index 0e1ab5f23665..9d2875da2a44 100644 --- a/api/envoy/admin/v2/BUILD +++ b/api/envoy/admin/v2alpha/BUILD @@ -7,6 +7,9 @@ api_proto_library( srcs = ["config_dump.proto"], visibility = ["//visibility:public"], deps = [ + "//envoy/api/v2:cds", + "//envoy/api/v2:lds", "//envoy/api/v2:rds", + "//envoy/config/bootstrap/v2:bootstrap", ], ) diff --git a/api/envoy/admin/v2alpha/config_dump.proto b/api/envoy/admin/v2alpha/config_dump.proto new file mode 100644 index 000000000000..0dabb167f61b --- /dev/null +++ b/api/envoy/admin/v2alpha/config_dump.proto @@ -0,0 +1,118 @@ +syntax = "proto3"; + +package envoy.admin.v2alpha; + +import "envoy/api/v2/cds.proto"; +import "envoy/api/v2/lds.proto"; +import "envoy/api/v2/rds.proto"; +import "envoy/config/bootstrap/v2/bootstrap.proto"; + +import "google/protobuf/any.proto"; + +import "gogoproto/gogo.proto"; + +// [#protodoc-title: ConfigDump] + +// The /config_dump admin endpoint uses this wrapper message to maintain and serve arbitrary +// configuration information from any component in Envoy. +// TODO(jsedgwick) In the future, we may want to formalize this further with an RPC for config_dump, +// and perhaps even with an RPC per config type. That strategy across all endpoints will allow for +// more flexibility w.r.t. protocol, serialization, parameters, etc. +message ConfigDump { + // This map is serialized and dumped in its entirety at the /config_dump endpoint. + // + // Keys should be a short descriptor of the config object they map to. For example, Envoy's HTTP + // routing subsystem might use "routes" as the key for its config, for which it uses the message + // RouteConfigDump as defined below. In the future, the key will also be used to filter the output + // of the /config_dump endpoint. + map configs = 1 [(gogoproto.nullable) = false]; +} + +// This message describes the bootstrap configuration that Envoy was started with. This includes +// any CLI overrides that were merged. +message BootstrapConfigDump { + envoy.config.bootstrap.v2.Bootstrap bootstrap = 1 [(gogoproto.nullable) = false]; +} + +// Envoy's listener manager fills this message with all currently known listeners. +message ListenersConfigDump { + // This is the top level, "transactional," version information in the last processed LDS + // discovery response. If there are only static bootstrap listeners, this field will be "". + string version_info = 1; + + // Describes a dynamically loaded cluster via the LDS API. + message DynamicListener { + // This is the top level, "transactional," version information in the last processed LDS + // discovery response. If there are only static bootstrap listeners, this field will be "". + string version_info = 1; + + // The listener config. + envoy.api.v2.Listener listener = 2; + } + + // The statically loaded listener configs. + repeated envoy.api.v2.Listener static_listeners = 2 [(gogoproto.nullable) = false]; + + // The dynamically loaded active listeners These are listeners that are available to service + // data plane traffic. + repeated DynamicListener dynamic_active_listeners = 3 [(gogoproto.nullable) = false]; + + // The dynamically loaded warming listeners These are listeners that are currently undergoing + // warming in preparation to service data plane traffic. + repeated DynamicListener dynamic_warming_listeners = 4 [(gogoproto.nullable) = false]; + + // The dynamically loaded draining listeners These are listeners that are currently undergoing + // draining in preparation to stop servicing data plane traffic. + repeated DynamicListener dynamic_draining_listeners = 5 [(gogoproto.nullable) = false]; +} + +// Envoy's cluster manager fills this message with all currently known clusters. +message ClustersConfigDump { + // This is the top level, "transactional," version information in the last processed CDS + // discovery response. If there are only static bootstrap clusters, this field will be "". + string version_info = 1; + + // Describes a dynamically loaded cluster via the CDS API. + message DynamicCluster { + // This is the per-resource version information. This version is currently taken from the + // transactional version_info field at the time that the cluster was loaded. In the future, + // discrete per cluster versions may be supported by the API. + string version_info = 1; + + // The cluster config. + envoy.api.v2.Cluster cluster = 2; + } + + // The statically loaded cluster configs. + repeated envoy.api.v2.Cluster static_clusters = 2 [(gogoproto.nullable) = false]; + + // The dynamically loaded active clusters. These are clusters that are available to service + // data plane traffic. + repeated DynamicCluster dynamic_active_clusters = 3 [(gogoproto.nullable) = false]; + + // The dynamically loaded warming clusters. These are clusters that are currently undergoing + // warming in preparation to service data plane traffic. + repeated DynamicCluster dynamic_warming_clusters = 4 [(gogoproto.nullable) = false]; +} + +// Envoy's RDS implementation fills this message with all currently loaded routes, as described by +// their RouteConfiguration objects. Static routes configured in the bootstrap configuration are +// separated from those configured dynamically via RDS. +message RoutesConfigDump { + message DynamicRouteConfig { + // This is the per-resource version information. This version is currently taken from the + // transactional version_info field in the RDS response at the time that the route was loaded. + // Note that *RoutesConfigDump* does not contain a top-level transactional *version_info* + // field as every RDS subscription is discrete. + string version_info = 1; + + // The route config. + envoy.api.v2.RouteConfiguration route_config = 2; + } + + // The statically loaded route configs. + repeated envoy.api.v2.RouteConfiguration static_route_configs = 2 [(gogoproto.nullable) = false]; + + // The dynamically loaded route configs. + repeated DynamicRouteConfig dynamic_route_configs = 3 [(gogoproto.nullable) = false]; +} diff --git a/include/envoy/config/subscription.h b/include/envoy/config/subscription.h index 70caeb5f2244..e338c4ac7c42 100644 --- a/include/envoy/config/subscription.h +++ b/include/envoy/config/subscription.h @@ -21,11 +21,14 @@ template class SubscriptionCallbacks { /** * Called when a configuration update is received. * @param resources vector of fetched resources corresponding to the configuration update. + * @param version_info supplies the transactional/top-level version information as supplied by + * the xDS discovery response. * @throw EnvoyException with reason if the configuration is rejected. Otherwise the configuration * is accepted. Accepted configurations have their version_info reflected in subsequent * requests. */ - virtual void onConfigUpdate(const ResourceVector& resources) PURE; + virtual void onConfigUpdate(const ResourceVector& resources, + const std::string& version_info) PURE; /** * Called when either the Subscription is unable to fetch a config update or when onConfigUpdate @@ -65,16 +68,6 @@ template class Subscription { * @param resources vector of resource names to fetch. */ virtual void updateResources(const std::vector& resources) PURE; - - /** - * @return std::string version info from last accepted onConfigUpdate. - * - * TODO(dnoe): This would ideally return by reference, but this causes a - * problem due to incompatible string implementations returned by - * protobuf generated code. Revisit when string implementations - * are converged. - */ - virtual const std::string versionInfo() const PURE; }; /** diff --git a/include/envoy/router/rds.h b/include/envoy/router/rds.h index 9ac1c4d0ebbb..08ed4dd28ba4 100644 --- a/include/envoy/router/rds.h +++ b/include/envoy/router/rds.h @@ -13,31 +13,31 @@ namespace Router { */ class RouteConfigProvider { public: + struct ConfigInfo { + // A reference to the currently loaded route configuration. Do not hold this reference beyond + // the caller's scope. + const envoy::api::v2::RouteConfiguration& config_; + + // The discovery version that supplied this route. This will be set to "" in the case of + // static clusters. + std::string version_; + }; + virtual ~RouteConfigProvider() {} /** * @return Router::ConfigConstSharedPtr a route configuration for use during a single request. The - * returned - * config may be different on a subsequent call, so a new config should be acquired for - * each request flow. + * returned config may be different on a subsequent call, so a new config should be acquired for + * each request flow. */ virtual Router::ConfigConstSharedPtr config() PURE; /** - * @return envoy::api::v2::RouteConfiguration the underlying RouteConfiguration object associated - * with this provider. - */ - virtual const envoy::api::v2::RouteConfiguration& configAsProto() const PURE; - - /** - * @return const std::string version info from last accepted config. - * - * TODO(dnoe): This would ideally return by reference, but this causes a - * problem due to incompatible string implementations returned by - * protobuf generated code. Revisit when string implementations - * are converged. + * @return the configuration information for the currently loaded route configuration. Note that + * if the provider has not yet performed an initial configuration load, no information will be + * returned. */ - virtual const std::string versionInfo() const PURE; + virtual absl::optional configInfo() const PURE; }; typedef std::shared_ptr RouteConfigProviderSharedPtr; diff --git a/include/envoy/upstream/cluster_manager.h b/include/envoy/upstream/cluster_manager.h index 459718aeaf65..c1ffe8ac582d 100644 --- a/include/envoy/upstream/cluster_manager.h +++ b/include/envoy/upstream/cluster_manager.h @@ -165,14 +165,6 @@ class ClusterManager { */ virtual Grpc::AsyncClientManager& grpcAsyncClientManager() PURE; - /** - * Return the current version info string for dynamic clusters, if CDS is setup. - * - * @return std::string the current version info string for dynamic clusters, - * or "static" if CDS is not in use. - */ - virtual const std::string versionInfo() const PURE; - /** * Return the local cluster name, if it was configured. * @@ -213,16 +205,6 @@ class CdsApi { * server. If the initial load fails, the callback will also be called. */ virtual void setInitializedCb(std::function callback) PURE; - - /** - * @return std::string last accepted version from fetch. - * - * TODO(dnoe): This would ideally return by reference, but this causes a - * problem due to incompatible string implementations returned by - * protobuf generated code. Revisit when string implementations - * are converged. - */ - virtual const std::string versionInfo() const PURE; }; typedef std::unique_ptr CdsApiPtr; diff --git a/source/common/config/filesystem_subscription_impl.h b/source/common/config/filesystem_subscription_impl.h index 8a13369829fa..ee0564449660 100644 --- a/source/common/config/filesystem_subscription_impl.h +++ b/source/common/config/filesystem_subscription_impl.h @@ -52,8 +52,6 @@ class FilesystemSubscriptionImpl : public Config::Subscription, stats_.update_attempt_.inc(); } - const std::string versionInfo() const override { return version_info_; } - private: void refresh() { ENVOY_LOG(debug, "Filesystem config refresh for {}", path_); @@ -64,13 +62,11 @@ class FilesystemSubscriptionImpl : public Config::Subscription, MessageUtil::loadFromFile(path_, message); const auto typed_resources = Config::Utility::getTypedResources(message); config_update_available = true; - callbacks_->onConfigUpdate(typed_resources); - version_info_ = message.version_info(); - stats_.version_.set(HashUtil::xxHash64(version_info_)); + callbacks_->onConfigUpdate(typed_resources, message.version_info()); + stats_.version_.set(HashUtil::xxHash64(message.version_info())); stats_.update_success_.inc(); ENVOY_LOG(debug, "Filesystem config update accepted for {}: {}", path_, message.DebugString()); - // TODO(htuch): Add some notion of current version for every API in stats/admin. } catch (const EnvoyException& e) { if (config_update_available) { ENVOY_LOG(warn, "Filesystem config update rejected: {}", e.what()); @@ -85,7 +81,6 @@ class FilesystemSubscriptionImpl : public Config::Subscription, bool started_{}; const std::string path_; - std::string version_info_; std::unique_ptr watcher_; SubscriptionCallbacks* callbacks_{}; SubscriptionStats stats_; diff --git a/source/common/config/grpc_mux_impl.cc b/source/common/config/grpc_mux_impl.cc index 304a8648aabf..3d8e7432109a 100644 --- a/source/common/config/grpc_mux_impl.cc +++ b/source/common/config/grpc_mux_impl.cc @@ -195,6 +195,8 @@ void GrpcMuxImpl::onReceiveMessage(std::unique_ptrcallbacks_.onConfigUpdate(found_resources, message->version_info()); } + // TODO(mattklein123): In the future if we start tracking per-resource versions, we would do + // that tracking here. api_state_[type_url].request_.set_version_info(message->version_info()); } catch (const EnvoyException& e) { ENVOY_LOG(warn, "gRPC config for {} update rejected: {}", message->type_url(), e.what()); diff --git a/source/common/config/grpc_mux_subscription_impl.h b/source/common/config/grpc_mux_subscription_impl.h index 2e85871e5a48..e45263ad52b6 100644 --- a/source/common/config/grpc_mux_subscription_impl.h +++ b/source/common/config/grpc_mux_subscription_impl.h @@ -41,8 +41,6 @@ class GrpcMuxSubscriptionImpl : public Subscription, stats_.update_attempt_.inc(); } - const std::string versionInfo() const override { return version_info_; } - // Config::GrpcMuxCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, const std::string& version_info) override { @@ -50,11 +48,14 @@ class GrpcMuxSubscriptionImpl : public Subscription, std::transform(resources.cbegin(), resources.cend(), Protobuf::RepeatedPtrFieldBackInserter(&typed_resources), MessageUtil::anyConvert); - callbacks_->onConfigUpdate(typed_resources); + // TODO(mattklein123): In the future if we start tracking per-resource versions, we need to + // supply those versions to onConfigUpdate() along with the top-level/transactional + // version_info. This way, both types of verisons can be tracked and exposed for debugging by + // the configuration update targets. + callbacks_->onConfigUpdate(typed_resources, version_info); stats_.update_success_.inc(); stats_.update_attempt_.inc(); - version_info_ = version_info; - stats_.version_.set(HashUtil::xxHash64(version_info_)); + stats_.version_.set(HashUtil::xxHash64(version_info)); ENVOY_LOG(debug, "gRPC config for {} accepted with {} resources: {}", type_url_, resources.size(), RepeatedPtrUtil::debugString(typed_resources)); } @@ -82,7 +83,6 @@ class GrpcMuxSubscriptionImpl : public Subscription, const std::string type_url_; SubscriptionCallbacks* callbacks_{}; GrpcMuxWatchPtr watch_{}; - std::string version_info_; }; } // namespace Config diff --git a/source/common/config/grpc_subscription_impl.h b/source/common/config/grpc_subscription_impl.h index 1ab8bde6b1e9..4b07f0223990 100644 --- a/source/common/config/grpc_subscription_impl.h +++ b/source/common/config/grpc_subscription_impl.h @@ -32,8 +32,6 @@ class GrpcSubscriptionImpl : public Config::Subscription { grpc_mux_subscription_.updateResources(resources); } - const std::string versionInfo() const override { return grpc_mux_subscription_.versionInfo(); } - GrpcMuxImpl& grpcMux() { return grpc_mux_; } private: diff --git a/source/common/config/http_subscription_impl.h b/source/common/config/http_subscription_impl.h index 72cb3e649934..5e79c14ea5bf 100644 --- a/source/common/config/http_subscription_impl.h +++ b/source/common/config/http_subscription_impl.h @@ -60,8 +60,6 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, request_.mutable_resource_names()->Swap(&resources_vector); } - const std::string versionInfo() const override { return request_.version_info(); } - // Http::RestApiFetcher void createRequest(Http::Message& request) override { ENVOY_LOG(debug, "Sending REST request for {}", path_); @@ -81,7 +79,7 @@ class HttpSubscriptionImpl : public Http::RestApiFetcher, } const auto typed_resources = Config::Utility::getTypedResources(message); try { - callbacks_->onConfigUpdate(typed_resources); + callbacks_->onConfigUpdate(typed_resources, message.version_info()); request_.set_version_info(message.version_info()); stats_.version_.set(HashUtil::xxHash64(request_.version_info())); stats_.update_success_.inc(); diff --git a/source/common/router/BUILD b/source/common/router/BUILD index 34cccd13e421..266df626eab8 100644 --- a/source/common/router/BUILD +++ b/source/common/router/BUILD @@ -77,7 +77,7 @@ envoy_cc_library( "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", "//source/common/protobuf:utility_lib", - "@envoy_api//envoy/admin/v2:config_dump_cc", + "@envoy_api//envoy/admin/v2alpha:config_dump_cc", "@envoy_api//envoy/api/v2:rds_cc", "@envoy_api//envoy/config/filter/network/http_connection_manager/v2:http_connection_manager_cc", ], diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index 55e694bad2b2..2f14219b53ce 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -5,7 +5,7 @@ #include #include -#include "envoy/admin/v2/config_dump.pb.h" +#include "envoy/admin/v2alpha/config_dump.pb.h" #include "envoy/api/v2/rds.pb.validate.h" #include "envoy/api/v2/route/route.pb.validate.h" @@ -93,7 +93,8 @@ Router::ConfigConstSharedPtr RdsRouteConfigProviderImpl::config() { return tls_->getTyped().config_; } -void RdsRouteConfigProviderImpl::onConfigUpdate(const ResourceVector& resources) { +void RdsRouteConfigProviderImpl::onConfigUpdate(const ResourceVector& resources, + const std::string& version_info) { if (resources.empty()) { ENVOY_LOG(debug, "Missing RouteConfiguration for {} in onConfigUpdate()", route_config_name_); stats_.update_empty_.inc(); @@ -111,10 +112,9 @@ void RdsRouteConfigProviderImpl::onConfigUpdate(const ResourceVector& resources) route_config_name_, route_config.name())); } const uint64_t new_hash = MessageUtil::hash(route_config); - if (new_hash != last_config_hash_ || !initialized_) { + if (!config_info_ || new_hash != config_info_.value().last_config_hash_) { ConfigConstSharedPtr new_config(new ConfigImpl(route_config, runtime_, cm_, false)); - initialized_ = true; - last_config_hash_ = new_hash; + config_info_ = {new_hash, version_info}; stats_.config_reload_.inc(); ENVOY_LOG(debug, "rds: loading new configuration: config_name={} hash={}", route_config_name_, new_hash); @@ -173,11 +173,15 @@ std::vector RouteConfigProviderManagerImpl::getStaticRouteConfigProviders() { std::vector providers_strong; // Collect non-expired providers. - std::transform(static_route_config_providers_.begin(), static_route_config_providers_.end(), - providers_strong.begin(), [](auto&& weak) { return weak.lock(); }); + for (const auto& weak_provider : static_route_config_providers_) { + const auto strong_provider = weak_provider.lock(); + if (strong_provider != nullptr) { + providers_strong.push_back(strong_provider); + } + } // Replace our stored list of weak_ptrs with the filtered list. - static_route_config_providers_.assign(providers_strong.begin(), providers_strong.begin()); + static_route_config_providers_.assign(providers_strong.begin(), providers_strong.end()); return providers_strong; }; @@ -226,15 +230,23 @@ RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getStaticRouteConfi } ProtobufTypes::MessagePtr RouteConfigProviderManagerImpl::dumpRouteConfigs() { - auto config_dump = std::make_unique(); - auto* const dynamic_configs = config_dump->mutable_dynamic_route_configs(); + auto config_dump = std::make_unique(); + for (const auto& provider : getRdsRouteConfigProviders()) { - dynamic_configs->Add()->MergeFrom(provider->configAsProto()); + auto config_info = provider->configInfo(); + if (config_info) { + auto* dynamic_config = config_dump->mutable_dynamic_route_configs()->Add(); + dynamic_config->set_version_info(config_info.value().version_); + dynamic_config->mutable_route_config()->MergeFrom(config_info.value().config_); + } } - auto* const static_configs = config_dump->mutable_static_route_configs(); + for (const auto& provider : getStaticRouteConfigProviders()) { - static_configs->Add()->MergeFrom(provider->configAsProto()); + ASSERT(provider->configInfo()); + config_dump->mutable_static_route_configs()->Add()->MergeFrom( + provider->configInfo().value().config_); } + return ProtobufTypes::MessagePtr{std::move(config_dump)}; } diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index cb6e3d3397d8..ea8502060d32 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -51,9 +51,8 @@ class StaticRouteConfigProviderImpl : public RouteConfigProvider { // Router::RouteConfigProvider Router::ConfigConstSharedPtr config() override { return config_; } - const std::string versionInfo() const override { CONSTRUCT_ON_FIRST_USE(std::string, "static"); } - const envoy::api::v2::RouteConfiguration& configAsProto() const override { - return route_config_proto_; + absl::optional configInfo() const override { + return ConfigInfo{route_config_proto_, ""}; } private: @@ -100,13 +99,16 @@ class RdsRouteConfigProviderImpl // Router::RouteConfigProvider Router::ConfigConstSharedPtr config() override; - const envoy::api::v2::RouteConfiguration& configAsProto() const override { - return route_config_proto_; + absl::optional configInfo() const override { + if (!config_info_) { + return {}; + } else { + return ConfigInfo{route_config_proto_, config_info_.value().last_config_version_}; + } } - const std::string versionInfo() const override { return subscription_->versionInfo(); } // Config::SubscriptionCallbacks - void onConfigUpdate(const ResourceVector& resources) override; + void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException* e) override; std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); @@ -119,6 +121,11 @@ class RdsRouteConfigProviderImpl ConfigConstSharedPtr config_; }; + struct LastConfigInfo { + uint64_t last_config_hash_; + std::string last_config_version_; + }; + RdsRouteConfigProviderImpl( const envoy::config::filter::network::http_connection_manager::v2::Rds& rds, const std::string& manager_identifier, Runtime::Loader& runtime, Upstream::ClusterManager& cm, @@ -136,8 +143,7 @@ class RdsRouteConfigProviderImpl ThreadLocal::SlotPtr tls_; std::string config_source_; const std::string route_config_name_; - bool initialized_{}; - uint64_t last_config_hash_{}; + absl::optional config_info_; Stats::ScopePtr scope_; RdsStats stats_; std::function initialize_callback_; diff --git a/source/common/router/rds_subscription.cc b/source/common/router/rds_subscription.cc index 9f50d5b8cefa..fcfebd95e872 100644 --- a/source/common/router/rds_subscription.cc +++ b/source/common/router/rds_subscription.cc @@ -47,10 +47,9 @@ void RdsSubscription::parseResponse(const Http::Message& response) { Protobuf::RepeatedPtrField resources; Envoy::Config::RdsJson::translateRouteConfiguration(*response_json, *resources.Add()); resources[0].set_name(route_config_name_); - callbacks_->onConfigUpdate(resources); std::pair hash = Envoy::Config::Utility::computeHashedVersion(response_body); - version_info_ = hash.first; + callbacks_->onConfigUpdate(resources, hash.first); stats_.version_.set(hash.second); stats_.update_success_.inc(); } diff --git a/source/common/router/rds_subscription.h b/source/common/router/rds_subscription.h index 445899dd5296..d27217af5616 100644 --- a/source/common/router/rds_subscription.h +++ b/source/common/router/rds_subscription.h @@ -45,8 +45,6 @@ class RdsSubscription : public Http::RestApiFetcher, NOT_IMPLEMENTED; } - const std::string versionInfo() const override { return version_info_; } - // Http::RestApiFetcher void createRequest(Http::Message& request) override; void parseResponse(const Http::Message& response) override; @@ -54,7 +52,6 @@ class RdsSubscription : public Http::RestApiFetcher, void onFetchFailure(const EnvoyException* e) override; std::string route_config_name_; - std::string version_info_; const LocalInfo::LocalInfo& local_info_; Envoy::Config::SubscriptionCallbacks* callbacks_ = nullptr; Envoy::Config::SubscriptionStats stats_; diff --git a/source/common/upstream/cds_api_impl.cc b/source/common/upstream/cds_api_impl.cc index db39fe11c472..abd759bcaaed 100644 --- a/source/common/upstream/cds_api_impl.cc +++ b/source/common/upstream/cds_api_impl.cc @@ -44,7 +44,7 @@ CdsApiImpl::CdsApiImpl(const envoy::api::v2::core::ConfigSource& cds_config, "envoy.api.v2.ClusterDiscoveryService.StreamClusters"); } -void CdsApiImpl::onConfigUpdate(const ResourceVector& resources) { +void CdsApiImpl::onConfigUpdate(const ResourceVector& resources, const std::string&) { cm_.adsMux().pause(Config::TypeUrl::get().ClusterLoadAssignment); Cleanup eds_resume([this] { cm_.adsMux().resume(Config::TypeUrl::get().ClusterLoadAssignment); }); for (const auto& cluster : resources) { diff --git a/source/common/upstream/cds_api_impl.h b/source/common/upstream/cds_api_impl.h index 273cd1270ce7..a9ad213ea136 100644 --- a/source/common/upstream/cds_api_impl.h +++ b/source/common/upstream/cds_api_impl.h @@ -31,10 +31,9 @@ class CdsApiImpl : public CdsApi, void setInitializedCb(std::function callback) override { initialize_callback_ = callback; } - const std::string versionInfo() const override { return subscription_->versionInfo(); } // Config::SubscriptionCallbacks - void onConfigUpdate(const ResourceVector& resources) override; + void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException* e) override; std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); diff --git a/source/common/upstream/cds_subscription.cc b/source/common/upstream/cds_subscription.cc index 942417a59f29..d3287a88e5d2 100644 --- a/source/common/upstream/cds_subscription.cc +++ b/source/common/upstream/cds_subscription.cc @@ -49,10 +49,9 @@ void CdsSubscription::parseResponse(const Http::Message& response) { Config::CdsJson::translateCluster(*cluster, eds_config_, *resources.Add()); } - callbacks_->onConfigUpdate(resources); std::pair hash = Envoy::Config::Utility::computeHashedVersion(response_body); - version_info_ = hash.first; + callbacks_->onConfigUpdate(resources, hash.first); stats_.version_.set(hash.second); stats_.update_success_.inc(); } diff --git a/source/common/upstream/cds_subscription.h b/source/common/upstream/cds_subscription.h index ea61f0fce7ee..8e4f1454612d 100644 --- a/source/common/upstream/cds_subscription.h +++ b/source/common/upstream/cds_subscription.h @@ -45,15 +45,12 @@ class CdsSubscription : public Http::RestApiFetcher, NOT_IMPLEMENTED; } - const std::string versionInfo() const override { return version_info_; } - // Http::RestApiFetcher void createRequest(Http::Message& request) override; void parseResponse(const Http::Message& response) override; void onFetchComplete() override; void onFetchFailure(const EnvoyException* e) override; - std::string version_info_; const LocalInfo::LocalInfo& local_info_; Config::SubscriptionCallbacks* callbacks_ = nullptr; Config::SubscriptionStats stats_; diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 84c40ae2a52c..91a80de4fcee 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -600,13 +600,6 @@ Http::AsyncClient& ClusterManagerImpl::httpAsyncClientForCluster(const std::stri } } -const std::string ClusterManagerImpl::versionInfo() const { - if (cds_api_) { - return cds_api_->versionInfo(); - } - return "static"; -} - ClusterUpdateCallbacksHandlePtr ClusterManagerImpl::addThreadLocalClusterUpdateCallbacks(ClusterUpdateCallbacks& cb) { ThreadLocalClusterManagerImpl& cluster_manager = tls_->getTyped(); diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index acdfaec3548d..573a1886dc72 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -186,7 +186,6 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggablestart({cluster_name_}, *this); } -void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources) { +void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std::string&) { typedef std::unique_ptr HostListPtr; std::vector> priority_state(1); if (resources.empty()) { diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 6921ff2a6c15..e17437ebf7aa 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -23,13 +23,11 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random, bool added_via_api); - const std::string versionInfo() const { return subscription_->versionInfo(); } - // Upstream::Cluster InitializePhase initializePhase() const override { return InitializePhase::Secondary; } // Config::SubscriptionCallbacks - void onConfigUpdate(const ResourceVector& resources) override; + void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException* e) override; std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).cluster_name(); diff --git a/source/common/upstream/sds_subscription.cc b/source/common/upstream/sds_subscription.cc index 0523ff547d9d..c9d4fef52538 100644 --- a/source/common/upstream/sds_subscription.cc +++ b/source/common/upstream/sds_subscription.cc @@ -73,10 +73,9 @@ void SdsSubscription::parseResponse(const Http::Message& response) { locality_lb_endpoints->mutable_lb_endpoints()->Swap(&it.second); } - callbacks_->onConfigUpdate(resources); std::pair hash = Envoy::Config::Utility::computeHashedVersion(response_body); - version_info_ = hash.first; + callbacks_->onConfigUpdate(resources, hash.first); stats_.version_.set(hash.second); stats_.update_success_.inc(); } diff --git a/source/common/upstream/sds_subscription.h b/source/common/upstream/sds_subscription.h index 799dcd70efd4..4a6d6c78ef57 100644 --- a/source/common/upstream/sds_subscription.h +++ b/source/common/upstream/sds_subscription.h @@ -25,9 +25,6 @@ class SdsSubscription : public Http::RestApiFetcher, ClusterManager& cm, Event::Dispatcher& dispatcher, Runtime::RandomGenerator& random); - // Config::Subscription - const std::string versionInfo() const override { return version_info_; } - private: // Config::Subscription void @@ -55,7 +52,6 @@ class SdsSubscription : public Http::RestApiFetcher, void onFetchFailure(const EnvoyException* e) override; std::string cluster_name_; - std::string version_info_; Config::SubscriptionCallbacks* callbacks_ = nullptr; ClusterStats& stats_; }; diff --git a/source/server/http/BUILD b/source/server/http/BUILD index 0bf8c8216f29..17b8ab8f0c18 100644 --- a/source/server/http/BUILD +++ b/source/server/http/BUILD @@ -52,7 +52,7 @@ envoy_cc_library( "//source/common/stats:stats_lib", "//source/common/upstream:host_utility_lib", "//source/extensions/access_loggers/file:file_access_log_lib", - "@envoy_api//envoy/admin/v2:config_dump_cc", + "@envoy_api//envoy/admin/v2alpha:config_dump_cc", ], ) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 8cde3d04a1b3..eed7324c89e8 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -9,7 +9,7 @@ #include #include -#include "envoy/admin/v2/config_dump.pb.h" +#include "envoy/admin/v2alpha/config_dump.pb.h" #include "envoy/filesystem/filesystem.h" #include "envoy/runtime/runtime.h" #include "envoy/server/hot_restart.h" @@ -224,8 +224,6 @@ void AdminImpl::addCircuitSettings(const std::string& cluster_name, const std::s Http::Code AdminImpl::handlerClusters(absl::string_view, Http::HeaderMap&, Buffer::Instance& response) { - response.add(fmt::format("version_info::{}\n", server_.clusterManager().versionInfo())); - for (auto& cluster : server_.clusterManager().clusters()) { addOutlierInfo(cluster.second.get().info()->name(), cluster.second.get().outlierDetector(), response); @@ -282,7 +280,7 @@ Http::Code AdminImpl::handlerClusters(absl::string_view, Http::HeaderMap&, // TODO(jsedgwick) Use query params to list available dumps, selectively dump, etc Http::Code AdminImpl::handlerConfigDump(absl::string_view, Http::HeaderMap&, Buffer::Instance& response) const { - envoy::admin::v2::ConfigDump dump; + envoy::admin::v2alpha::ConfigDump dump; auto& config_dump_map = *(dump.mutable_configs()); for (const auto& key_callback_pair : config_tracker_.getCallbacksMap()) { ProtobufTypes::MessagePtr message = key_callback_pair.second(); diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 0196b2688d21..42ffd9153de8 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -115,10 +115,7 @@ class AdminImpl : public Admin, // Router::RouteConfigProvider Router::ConfigConstSharedPtr config() override { return config_; } - const envoy::api::v2::RouteConfiguration& configAsProto() const override { - return envoy::api::v2::RouteConfiguration::default_instance(); - } - const std::string versionInfo() const override { CONSTRUCT_ON_FIRST_USE(std::string, ""); } + absl::optional configInfo() const override { return {}; } Router::ConfigConstSharedPtr config_; }; diff --git a/source/server/lds_api.cc b/source/server/lds_api.cc index d611b0fe853a..9820c8a7b75a 100644 --- a/source/server/lds_api.cc +++ b/source/server/lds_api.cc @@ -40,7 +40,7 @@ void LdsApi::initialize(std::function callback) { subscription_->start({}, *this); } -void LdsApi::onConfigUpdate(const ResourceVector& resources) { +void LdsApi::onConfigUpdate(const ResourceVector& resources, const std::string&) { cm_.adsMux().pause(Config::TypeUrl::get().RouteConfiguration); Cleanup rds_resume([this] { cm_.adsMux().resume(Config::TypeUrl::get().RouteConfiguration); }); for (const auto& listener : resources) { diff --git a/source/server/lds_api.h b/source/server/lds_api.h index 016a0d9c1bd3..adf866d66380 100644 --- a/source/server/lds_api.h +++ b/source/server/lds_api.h @@ -24,13 +24,11 @@ class LdsApi : public Init::Target, Init::Manager& init_manager, const LocalInfo::LocalInfo& local_info, Stats::Scope& scope, ListenerManager& lm); - const std::string versionInfo() const { return subscription_->versionInfo(); } - // Init::Target void initialize(std::function callback) override; // Config::SubscriptionCallbacks - void onConfigUpdate(const ResourceVector& resources) override; + void onConfigUpdate(const ResourceVector& resources, const std::string& version_info) override; void onConfigUpdateFailed(const EnvoyException* e) override; std::string resourceName(const ProtobufWkt::Any& resource) override { return MessageUtil::anyConvert(resource).name(); diff --git a/source/server/lds_subscription.cc b/source/server/lds_subscription.cc index a1601d9410f0..a825ddbff02b 100644 --- a/source/server/lds_subscription.cc +++ b/source/server/lds_subscription.cc @@ -51,10 +51,9 @@ void LdsSubscription::parseResponse(const Http::Message& response) { Config::LdsJson::translateListener(*json_listener, *resources.Add()); } - callbacks_->onConfigUpdate(resources); std::pair hash = Envoy::Config::Utility::computeHashedVersion(response_body); - version_info_ = hash.first; + callbacks_->onConfigUpdate(resources, hash.first); stats_.version_.set(hash.second); stats_.update_success_.inc(); } diff --git a/source/server/lds_subscription.h b/source/server/lds_subscription.h index 40d4ace39bcf..2dd39d873c69 100644 --- a/source/server/lds_subscription.h +++ b/source/server/lds_subscription.h @@ -42,15 +42,12 @@ class LdsSubscription : public Http::RestApiFetcher, NOT_IMPLEMENTED; } - const std::string versionInfo() const override { return version_info_; } - // Http::RestApiFetcher void createRequest(Http::Message& request) override; void parseResponse(const Http::Message& response) override; void onFetchComplete() override; void onFetchFailure(const EnvoyException* e) override; - std::string version_info_; const LocalInfo::LocalInfo& local_info_; Config::SubscriptionCallbacks* callbacks_ = nullptr; Config::SubscriptionStats stats_; diff --git a/test/common/config/filesystem_subscription_test_harness.h b/test/common/config/filesystem_subscription_test_harness.h index 9f970e9d3087..16a42f054380 100644 --- a/test/common/config/filesystem_subscription_test_harness.h +++ b/test/common/config/filesystem_subscription_test_harness.h @@ -71,9 +71,11 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { envoy::api::v2::DiscoveryResponse response_pb; EXPECT_TRUE(Protobuf::util::JsonStringToMessage(file_json, &response_pb).ok()); EXPECT_CALL(callbacks_, - onConfigUpdate(RepeatedProtoEq( - Config::Utility::getTypedResources( - response_pb)))) + onConfigUpdate( + RepeatedProtoEq( + Config::Utility::getTypedResources( + response_pb)), + version)) .WillOnce(ThrowOnRejectedConfig(accept)); if (accept) { version_ = version; @@ -81,7 +83,6 @@ class FilesystemSubscriptionTestHarness : public SubscriptionTestHarness { EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); } updateFile(file_json); - EXPECT_EQ(version_, subscription_.versionInfo()); } void verifyStats(uint32_t attempt, uint32_t success, uint32_t rejected, uint32_t failure, diff --git a/test/common/config/grpc_subscription_test_harness.h b/test/common/config/grpc_subscription_test_harness.h index b7a22f1db15e..c1fa1694fe5e 100644 --- a/test/common/config/grpc_subscription_test_harness.h +++ b/test/common/config/grpc_subscription_test_harness.h @@ -100,7 +100,7 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { response->add_resources()->PackFrom(*load_assignment); } } - EXPECT_CALL(callbacks_, onConfigUpdate(RepeatedProtoEq(typed_resources))) + EXPECT_CALL(callbacks_, onConfigUpdate(RepeatedProtoEq(typed_resources), version)) .WillOnce(ThrowOnRejectedConfig(accept)); if (accept) { expectSendMessage(last_cluster_names_, version); @@ -111,7 +111,6 @@ class GrpcSubscriptionTestHarness : public SubscriptionTestHarness { "bad config"); } subscription_->grpcMux().onReceiveMessage(std::move(response)); - EXPECT_EQ(version_, subscription_->versionInfo()); Mock::VerifyAndClearExpectations(&async_stream_); } diff --git a/test/common/config/http_subscription_test_harness.h b/test/common/config/http_subscription_test_harness.h index 0d95a1299f57..65b3d0aaa6b8 100644 --- a/test/common/config/http_subscription_test_harness.h +++ b/test/common/config/http_subscription_test_harness.h @@ -108,9 +108,11 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { Http::MessagePtr message{new Http::ResponseMessageImpl(std::move(response_headers))}; message->body().reset(new Buffer::OwnedImpl(response_json)); EXPECT_CALL(callbacks_, - onConfigUpdate(RepeatedProtoEq( - Config::Utility::getTypedResources( - response_pb)))) + onConfigUpdate( + RepeatedProtoEq( + Config::Utility::getTypedResources( + response_pb)), + version)) .WillOnce(ThrowOnRejectedConfig(accept)); if (!accept) { EXPECT_CALL(callbacks_, onConfigUpdateFailed(_)); @@ -121,7 +123,6 @@ class HttpSubscriptionTestHarness : public SubscriptionTestHarness { if (accept) { version_ = version; } - EXPECT_EQ(version_, subscription_->versionInfo()); request_in_progress_ = false; timerTick(); } diff --git a/test/common/http/conn_manager_impl_test.cc b/test/common/http/conn_manager_impl_test.cc index 8173714c57dc..d56771c0da06 100644 --- a/test/common/http/conn_manager_impl_test.cc +++ b/test/common/http/conn_manager_impl_test.cc @@ -61,10 +61,7 @@ class HttpConnectionManagerImplTest : public Test, public ConnectionManagerConfi struct RouteConfigProvider : public Router::RouteConfigProvider { // Router::RouteConfigProvider Router::ConfigConstSharedPtr config() override { return route_config_; } - const std::string versionInfo() const override { CONSTRUCT_ON_FIRST_USE(std::string, ""); } - const envoy::api::v2::RouteConfiguration& configAsProto() const override { - return envoy::api::v2::RouteConfiguration::default_instance(); - } + absl::optional configInfo() const override { return {}; } std::shared_ptr route_config_{new NiceMock()}; }; diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 708aa7b71d8c..1fd88213b4d0 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -1,8 +1,8 @@ #include #include -#include "envoy/admin/v2/config_dump.pb.h" -#include "envoy/admin/v2/config_dump.pb.validate.h" +#include "envoy/admin/v2alpha/config_dump.pb.h" +#include "envoy/admin/v2alpha/config_dump.pb.validate.h" #include "common/config/filter_json.h" #include "common/config/utility.h" @@ -44,9 +44,43 @@ parseHttpConnectionManagerFromJson(const std::string& json_string) { return http_connection_manager; } -class RdsImplTest : public testing::Test { +class RdsTestBase : public testing::Test { public: - RdsImplTest() : request_(&cm_.async_client_) { + RdsTestBase() : request_(&cm_.async_client_) {} + + void expectRequest() { + EXPECT_CALL(cm_, httpAsyncClientForCluster("foo_cluster")); + EXPECT_CALL(cm_.async_client_, send_(_, _, _)) + .WillOnce(Invoke( + [&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, + const absl::optional&) -> Http::AsyncClient::Request* { + EXPECT_EQ((Http::TestHeaderMapImpl{ + {":method", "GET"}, + {":path", "/v1/routes/foo_route_config/cluster_name/node_name"}, + {":authority", "foo_cluster"}}), + request->headers()); + callbacks_ = &callbacks; + return &request_; + })); + } + + NiceMock runtime_; + NiceMock cm_; + Event::MockDispatcher dispatcher_; + NiceMock random_; + NiceMock local_info_; + Stats::IsolatedStoreImpl store_; + NiceMock tls_; + NiceMock init_manager_; + NiceMock admin_; + Http::MockAsyncClientRequest request_; + Http::AsyncClient::Callbacks* callbacks_{}; + Event::MockTimer* interval_timer_{}; +}; + +class RdsImplTest : public RdsTestBase { +public: + RdsImplTest() { EXPECT_CALL(admin_.config_tracker, addReturnsRaw("routes", _)) .WillOnce(Return(new Server::MockConfigTracker::MockEntryOwner())); route_config_provider_manager_.reset(new RouteConfigProviderManagerImpl( @@ -84,41 +118,12 @@ class RdsImplTest : public testing::Test { runtime_, cm_, store_, "foo.", init_manager_, *route_config_provider_manager_); expectRequest(); - EXPECT_EQ("", rds_->versionInfo()); init_manager_.initialize(); } - void expectRequest() { - EXPECT_CALL(cm_, httpAsyncClientForCluster("foo_cluster")); - EXPECT_CALL(cm_.async_client_, send_(_, _, _)) - .WillOnce(Invoke( - [&](Http::MessagePtr& request, Http::AsyncClient::Callbacks& callbacks, - const absl::optional&) -> Http::AsyncClient::Request* { - EXPECT_EQ((Http::TestHeaderMapImpl{ - {":method", "GET"}, - {":path", "/v1/routes/foo_route_config/cluster_name/node_name"}, - {":authority", "foo_cluster"}}), - request->headers()); - callbacks_ = &callbacks; - return &request_; - })); - } - - NiceMock runtime_; - NiceMock cm_; - Event::MockDispatcher dispatcher_; - NiceMock random_; - NiceMock local_info_; - Stats::IsolatedStoreImpl store_; - NiceMock tls_; - NiceMock init_manager_; - Http::MockAsyncClientRequest request_; NiceMock server_; - NiceMock admin_; std::unique_ptr route_config_provider_manager_; RouteConfigProviderSharedPtr rds_; - Event::MockTimer* interval_timer_{}; - Http::AsyncClient::Callbacks* callbacks_{}; }; TEST_F(RdsImplTest, RdsAndStatic) { @@ -208,7 +213,6 @@ TEST_F(RdsImplTest, Basic) { // Make sure the initial empty route table works. EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); - EXPECT_EQ("", rds_->versionInfo()); EXPECT_EQ(0UL, store_.gauge("foo.rds.foo_route_config.version").value()); // Initial request. @@ -226,7 +230,6 @@ TEST_F(RdsImplTest, Basic) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); - EXPECT_EQ("hash_15ed54077da94d8b", rds_->versionInfo()); EXPECT_EQ(1580011435426663819U, store_.gauge("foo.rds.foo_route_config.version").value()); expectRequest(); @@ -280,7 +283,6 @@ TEST_F(RdsImplTest, Basic) { EXPECT_CALL(cm_, get("bar")).Times(0); EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_7a3f97b327d08382", rds_->versionInfo()); EXPECT_EQ("foo", rds_->config() ->route(Http::TestHeaderMapImpl{{":authority", "foo"}, {":path", "/foo"}}, 0) ->routeEntry() @@ -347,7 +349,7 @@ TEST_F(RdsImplTest, FailureArray) { EXPECT_EQ(1UL, store_.counter("foo.rds.foo_route_config.update_failure").value()); } -class RouteConfigProviderManagerImplTest : public testing::Test { +class RouteConfigProviderManagerImplTest : public RdsTestBase { public: void setup() { std::string config_json = R"EOF( @@ -369,6 +371,7 @@ class RouteConfigProviderManagerImplTest : public testing::Test { EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); EXPECT_CALL(*cluster.info_, type()); + interval_timer_ = new Event::MockTimer(&dispatcher_); provider_ = route_config_provider_manager_->getRdsRouteConfigProvider( rds_, cm_, store_, "foo_prefix.", init_manager_); } @@ -383,15 +386,6 @@ class RouteConfigProviderManagerImplTest : public testing::Test { } ~RouteConfigProviderManagerImplTest() { tls_.shutdownThread(); } - NiceMock runtime_; - NiceMock cm_; - NiceMock dispatcher_; - NiceMock random_; - NiceMock local_info_; - Stats::IsolatedStoreImpl store_; - NiceMock tls_; - NiceMock init_manager_; - NiceMock admin_; NiceMock config_tracker_; Server::ConfigTracker::Cb config_tracker_callback_; envoy::config::filter::network::http_connection_manager::v2::Rds rds_; @@ -400,16 +394,96 @@ class RouteConfigProviderManagerImplTest : public testing::Test { RouteConfigProviderSharedPtr provider_; }; +envoy::api::v2::RouteConfiguration parseRouteConfigurationFromV2Yaml(const std::string& yaml) { + envoy::api::v2::RouteConfiguration route_config; + MessageUtil::loadFromYaml(yaml, route_config); + return route_config; +} + TEST_F(RouteConfigProviderManagerImplTest, ConfigDump) { - setup(); auto message_ptr = config_tracker_callback_(); - EXPECT_NE(nullptr, message_ptr); const auto& route_config_dump = - MessageUtil::downcastAndValidate(*message_ptr); - EXPECT_EQ(0, route_config_dump.static_route_configs_size()); - EXPECT_EQ(1, route_config_dump.dynamic_route_configs_size()); - EXPECT_TRUE(Protobuf::util::MessageDifferencer::Equivalent( - provider_->configAsProto(), route_config_dump.dynamic_route_configs(0))); + MessageUtil::downcastAndValidate( + *message_ptr); + + // No routes at all. + envoy::admin::v2alpha::RoutesConfigDump expected_route_config_dump; + MessageUtil::loadFromYaml(R"EOF( +static_route_configs: +dynamic_route_configs: +)EOF", + expected_route_config_dump); + EXPECT_EQ(expected_route_config_dump.DebugString(), route_config_dump.DebugString()); + + std::string config_yaml = R"EOF( +name: foo +virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } +)EOF"; + + // Only static route. + RouteConfigProviderSharedPtr static_config = + route_config_provider_manager_->getStaticRouteConfigProvider( + parseRouteConfigurationFromV2Yaml(config_yaml), runtime_, cm_); + message_ptr = config_tracker_callback_(); + const auto& route_config_dump2 = + MessageUtil::downcastAndValidate( + *message_ptr); + MessageUtil::loadFromYaml(R"EOF( +static_route_configs: + - name: foo + virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } +dynamic_route_configs: +)EOF", + expected_route_config_dump); + EXPECT_EQ(expected_route_config_dump.DebugString(), route_config_dump2.DebugString()); + + // Static + dynamic. + setup(); + expectRequest(); + init_manager_.initialize(); + const std::string response1_json = R"EOF( + { + "virtual_hosts": [] + } + )EOF"; + + Http::MessagePtr message(new Http::ResponseMessageImpl( + Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "200"}}})); + message->body().reset(new Buffer::OwnedImpl(response1_json)); + EXPECT_CALL(init_manager_.initialized_, ready()); + EXPECT_CALL(*interval_timer_, enableTimer(_)); + callbacks_->onSuccess(std::move(message)); + message_ptr = config_tracker_callback_(); + const auto& route_config_dump3 = + MessageUtil::downcastAndValidate( + *message_ptr); + MessageUtil::loadFromYaml(R"EOF( +static_route_configs: + - name: foo + virtual_hosts: + - name: bar + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { cluster: baz } +dynamic_route_configs: + - version_info: "hash_15ed54077da94d8b" + route_config: + name: foo_route_config + virtual_hosts: +)EOF", + expected_route_config_dump); + EXPECT_EQ(expected_route_config_dump.DebugString(), route_config_dump3.DebugString()); } TEST_F(RouteConfigProviderManagerImplTest, Basic) { @@ -448,6 +522,7 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { EXPECT_CALL(cluster, info()).Times(2); EXPECT_CALL(*cluster.info_, addedViaApi()); EXPECT_CALL(*cluster.info_, type()); + new Event::MockTimer(&dispatcher_); RouteConfigProviderSharedPtr provider3 = route_config_provider_manager_->getRdsRouteConfigProvider(rds2, cm_, store_, "foo_prefix", init_manager_); @@ -486,7 +561,7 @@ TEST_F(RouteConfigProviderManagerImplTest, ValidateFail) { auto* route_config = route_configs.Add(); route_config->set_name("foo_route_config"); route_config->mutable_virtual_hosts()->Add(); - EXPECT_THROW(provider_impl.onConfigUpdate(route_configs), ProtoValidationException); + EXPECT_THROW(provider_impl.onConfigUpdate(route_configs, ""), ProtoValidationException); } TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateEmpty) { @@ -494,7 +569,7 @@ TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateEmpty) { init_manager_.initialize(); auto& provider_impl = dynamic_cast(*provider_.get()); EXPECT_CALL(init_manager_.initialized_, ready()); - provider_impl.onConfigUpdate({}); + provider_impl.onConfigUpdate({}, ""); EXPECT_EQ(1UL, store_.counter("foo_prefix.rds.foo_route_config.update_empty").value()); } @@ -506,7 +581,7 @@ TEST_F(RouteConfigProviderManagerImplTest, onConfigUpdateWrongSize) { route_configs.Add(); route_configs.Add(); EXPECT_CALL(init_manager_.initialized_, ready()); - EXPECT_THROW_WITH_MESSAGE(provider_impl.onConfigUpdate(route_configs), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(provider_impl.onConfigUpdate(route_configs, ""), EnvoyException, "Unexpected RDS resource length: 2"); } diff --git a/test/common/upstream/cds_api_impl_test.cc b/test/common/upstream/cds_api_impl_test.cc index e85bb462f9a8..a2b65c58f9df 100644 --- a/test/common/upstream/cds_api_impl_test.cc +++ b/test/common/upstream/cds_api_impl_test.cc @@ -118,7 +118,7 @@ TEST_F(CdsApiImplTest, ValidateFail) { Protobuf::RepeatedPtrField clusters; clusters.Add(); - EXPECT_THROW(dynamic_cast(cds_.get())->onConfigUpdate(clusters), + EXPECT_THROW(dynamic_cast(cds_.get())->onConfigUpdate(clusters, ""), ProtoValidationException); EXPECT_CALL(request_, cancel()); } @@ -161,10 +161,8 @@ TEST_F(CdsApiImplTest, Basic) { expectAdd("cluster2"); EXPECT_CALL(initialized_, ready()); EXPECT_CALL(*interval_timer_, enableTimer(_)); - EXPECT_EQ("", cds_->versionInfo()); EXPECT_EQ(0UL, store_.gauge("cluster_manager.cds.version").value()); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ(Config::Utility::computeHashedVersion(response1_json).first, cds_->versionInfo()); EXPECT_EQ(4054905652974790809U, store_.gauge("cluster_manager.cds.version").value()); expectRequest(); @@ -187,7 +185,6 @@ TEST_F(CdsApiImplTest, Basic) { EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_attempt").value()); EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_success").value()); - EXPECT_EQ(Config::Utility::computeHashedVersion(response2_json).first, cds_->versionInfo()); EXPECT_EQ(1872764556139482420U, store_.gauge("cluster_manager.cds.version").value()); } @@ -217,7 +214,6 @@ TEST_F(CdsApiImplTest, Failure) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); - EXPECT_EQ("", cds_->versionInfo()); EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_attempt").value()); EXPECT_EQ(2UL, store_.counter("cluster_manager.cds.update_failure").value()); EXPECT_EQ(0UL, store_.gauge("cluster_manager.cds.version").value()); @@ -241,7 +237,6 @@ TEST_F(CdsApiImplTest, FailureArray) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("", cds_->versionInfo()); EXPECT_EQ(1UL, store_.counter("cluster_manager.cds.update_attempt").value()); EXPECT_EQ(1UL, store_.counter("cluster_manager.cds.update_failure").value()); EXPECT_EQ(0UL, store_.gauge("cluster_manager.cds.version").value()); diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 504adccf3d4c..1ce30879236d 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -174,37 +174,6 @@ TEST_F(ClusterManagerImplTest, MultipleProtocolCluster) { create(parseBootstrapFromV2Yaml(yaml)); } -TEST_F(ClusterManagerImplTest, VersionInfoStatic) { - const std::string json = - fmt::sprintf("{%s}", clustersJson({defaultStaticClusterJson("cluster_0")})); - - create(parseBootstrapFromJson(json)); - EXPECT_EQ("static", cluster_manager_->versionInfo()); -} - -TEST_F(ClusterManagerImplTest, VersionInfoDynamic) { - const std::string json = fmt::sprintf( - R"EOF( - { - "cds": {"cluster": %s}, - %s - } - )EOF", - defaultStaticClusterJson("cds_cluster"), - clustersJson({defaultStaticClusterJson("cluster_0")})); - - // Setup cds api in order to control returned version info string. - MockCdsApi* cds = new MockCdsApi(); - EXPECT_CALL(factory_, createCds_()).WillOnce(Return(cds)); - EXPECT_CALL(*cds, setInitializedCb(_)); - EXPECT_CALL(*cds, initialize()); - - create(parseBootstrapFromJson(json)); - - EXPECT_CALL(*cds, versionInfo()).WillOnce(Return("version")); - EXPECT_EQ("version", cluster_manager_->versionInfo()); -} - TEST_F(ClusterManagerImplTest, OutlierEventLog) { const std::string json = R"EOF( { diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 2818ad23b43e..29aeb62ccb45 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -68,7 +68,7 @@ class EdsTest : public testing::Test { TEST_F(EdsTest, ValidateFail) { Protobuf::RepeatedPtrField resources; resources.Add(); - EXPECT_THROW(cluster_->onConfigUpdate(resources), ProtoValidationException); + EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), ProtoValidationException); } // Validate that onConfigUpdate() with unexpected cluster names rejects config. @@ -78,7 +78,7 @@ TEST_F(EdsTest, OnConfigUpdateWrongName) { cluster_load_assignment->set_cluster_name("wrong name"); bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - EXPECT_THROW(cluster_->onConfigUpdate(resources), EnvoyException); + EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), EnvoyException); cluster_->onConfigUpdateFailed(nullptr); EXPECT_TRUE(initialized); } @@ -87,7 +87,7 @@ TEST_F(EdsTest, OnConfigUpdateWrongName) { TEST_F(EdsTest, OnConfigUpdateEmpty) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - cluster_->onConfigUpdate({}); + cluster_->onConfigUpdate({}, ""); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_empty").value()); EXPECT_TRUE(initialized); } @@ -101,7 +101,7 @@ TEST_F(EdsTest, OnConfigUpdateWrongSize) { cluster_load_assignment->set_cluster_name("fare"); cluster_load_assignment = resources.Add(); cluster_load_assignment->set_cluster_name("fare"); - EXPECT_THROW(cluster_->onConfigUpdate(resources), EnvoyException); + EXPECT_THROW(cluster_->onConfigUpdate(resources, ""), EnvoyException); cluster_->onConfigUpdateFailed(nullptr); EXPECT_TRUE(initialized); } @@ -113,7 +113,7 @@ TEST_F(EdsTest, OnConfigUpdateSuccess) { cluster_load_assignment->set_cluster_name("fare"); bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); } @@ -137,7 +137,7 @@ TEST_F(EdsTest, NoServiceNameOnSuccessConfigUpdate) { cluster_load_assignment->set_cluster_name("name"); bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); } @@ -168,7 +168,7 @@ TEST_F(EdsTest, EndpointMetadata) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value()); @@ -196,7 +196,7 @@ TEST_F(EdsTest, EndpointMetadata) { EXPECT_TRUE(hosts[1]->canary()); // We don't rebuild with the exact same config. - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); } @@ -229,7 +229,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); @@ -244,7 +244,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { // to unhealthy, check we have the expected change in status. endpoints->mutable_lb_endpoints(0)->set_health_status( envoy::api::v2::core::HealthStatus::UNHEALTHY); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_EQ(hosts.size(), health_status_expected.size()); @@ -259,7 +259,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { // to healthy, check we have the expected change in status. endpoints->mutable_lb_endpoints(health_status_expected.size() - 1) ->set_health_status(envoy::api::v2::core::HealthStatus::HEALTHY); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_EQ(hosts.size(), health_status_expected.size()); @@ -277,7 +277,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); hosts[0]->healthFlagSet(Host::HealthFlag::FAILED_ACTIVE_HC); } - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_FALSE(hosts[0]->healthy()); @@ -287,7 +287,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { // active health check failure. endpoints->mutable_lb_endpoints(0)->set_health_status( envoy::api::v2::core::HealthStatus::HEALTHY); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); EXPECT_FALSE(hosts[0]->healthy()); @@ -332,7 +332,7 @@ TEST_F(EdsTest, EndpointLocality) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); @@ -371,7 +371,7 @@ TEST_F(EdsTest, EndpointLocalityWeightsIgnored) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); EXPECT_EQ(nullptr, cluster_->prioritySet().hostSetsPerPriority()[0]->localityWeights()); @@ -448,7 +448,7 @@ TEST_F(EdsTest, EndpointLocalityWeights) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); const auto& locality_weights = @@ -489,7 +489,7 @@ TEST_F(EdsTest, EndpointHostsPerLocality) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); { @@ -508,7 +508,7 @@ TEST_F(EdsTest, EndpointHostsPerLocality) { add_hosts_to_locality("oceania", "koala", "eucalyptus", 3); add_hosts_to_locality("general", "koala", "ingsoc", 5); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& hosts_per_locality = cluster_->prioritySet().hostSetsPerPriority()[0]->hostsPerLocality(); @@ -554,7 +554,7 @@ TEST_F(EdsTest, EndpointHostsPerPriority) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); ASSERT_EQ(2, cluster_->prioritySet().hostSetsPerPriority().size()); @@ -567,7 +567,7 @@ TEST_F(EdsTest, EndpointHostsPerPriority) { add_hosts_to_priority(0, 2); add_hosts_to_priority(3, 5); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); ASSERT_EQ(4, cluster_->prioritySet().hostSetsPerPriority().size()); EXPECT_EQ(4, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); @@ -579,7 +579,7 @@ TEST_F(EdsTest, EndpointHostsPerPriority) { // levels are affected. cluster_load_assignment->clear_endpoints(); add_hosts_to_priority(3, 4); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); ASSERT_EQ(4, cluster_->prioritySet().hostSetsPerPriority().size()); EXPECT_EQ(4, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(1, cluster_->prioritySet().hostSetsPerPriority()[1]->hosts().size()); @@ -614,13 +614,13 @@ TEST_F(EdsTest, NoPriorityForLocalCluster) { add_hosts_to_priority(1, 1); bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources, ""), EnvoyException, "Unexpected non-zero priority for local cluster 'fare'."); // Try an update which only has endpoints with P=0. This should go through. cluster_load_assignment->clear_endpoints(); add_hosts_to_priority(0, 2); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); } // Set up an EDS config with multiple priorities and localities and make sure @@ -658,7 +658,7 @@ TEST_F(EdsTest, PriorityAndLocality) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); { @@ -685,7 +685,7 @@ TEST_F(EdsTest, PriorityAndLocality) { add_hosts_to_locality_and_priority("oceania", "koala", "eucalyptus", 0, 3); add_hosts_to_locality_and_priority("general", "koala", "ingsoc", 1, 5); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); { auto& first_hosts_per_locality = @@ -769,7 +769,7 @@ TEST_F(EdsTest, PriorityAndLocalityWeighted) { bool initialized = false; cluster_->initialize([&initialized] { initialized = true; }); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_TRUE(initialized); EXPECT_EQ(0UL, stats_.counter("cluster.name.update_no_rebuild").value()); @@ -803,13 +803,13 @@ TEST_F(EdsTest, PriorityAndLocalityWeighted) { // This should noop (regression test for earlier bug where we would still // rebuild). - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); // Adjust locality weights, validate that we observe an update. cluster_load_assignment->mutable_endpoints(0)->mutable_load_balancing_weight()->set_value(60); cluster_load_assignment->mutable_endpoints(1)->mutable_load_balancing_weight()->set_value(40); - VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources)); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); EXPECT_EQ(1UL, stats_.counter("cluster.name.update_no_rebuild").value()); } @@ -826,7 +826,7 @@ TEST_F(EdsTest, MalformedIP) { endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); cluster_->initialize([] {}); - EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(cluster_->onConfigUpdate(resources, ""), EnvoyException, "malformed IP address: foo.bar.com. Consider setting resolver_name or " "setting cluster type to 'STRICT_DNS' or 'LOGICAL_DNS'"); } diff --git a/test/common/upstream/sds_test.cc b/test/common/upstream/sds_test.cc index 5ce7303c12c4..cd01ae7539cc 100644 --- a/test/common/upstream/sds_test.cc +++ b/test/common/upstream/sds_test.cc @@ -154,7 +154,6 @@ TEST_F(SdsTest, NoHealthChecker) { EXPECT_CALL(membership_updated_, ready()).Times(2); EXPECT_CALL(*timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_5f3725fa79001155", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(13UL, cluster_->info()->stats().membership_healthy_.value()); @@ -193,7 +192,6 @@ TEST_F(SdsTest, NoHealthChecker) { EXPECT_CALL(membership_updated_, ready()); EXPECT_CALL(*timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_2d1af371bb73a287", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(canary_host, findHost("10.0.16.43")); EXPECT_TRUE(canary_host->canary()); @@ -244,7 +242,6 @@ TEST_F(SdsTest, NoHealthChecker) { message.reset(new Http::ResponseMessageImpl( Http::HeaderMapPtr{new Http::TestHeaderMapImpl{{":status", "503"}}})); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_2d1af371bb73a287", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(50U, canary_host->weight()); EXPECT_EQ(50UL, cluster_->info()->stats().max_host_weight_.value()); @@ -283,7 +280,6 @@ TEST_F(SdsTest, HealthChecker) { EXPECT_CALL(*health_checker, addHostCheckCompleteCb(_)); EXPECT_CALL(*timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_5f3725fa79001155", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(0UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(0UL, cluster_->info()->stats().membership_healthy_.value()); @@ -334,7 +330,6 @@ TEST_F(SdsTest, HealthChecker) { Host::HealthFlag::FAILED_ACTIVE_HC); health_checker->runCallbacks(cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0], HealthTransition::Changed); - EXPECT_EQ("hash_5f3725fa79001155", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(13UL, cluster_->info()->stats().membership_healthy_.value()); EXPECT_EQ( @@ -361,7 +356,6 @@ TEST_F(SdsTest, HealthChecker) { message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd( TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_2.json")))); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_398882981ada7c10", cluster_->versionInfo()); EXPECT_EQ(14UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(13UL, cluster_->info()->stats().membership_healthy_.value()); @@ -391,7 +385,6 @@ TEST_F(SdsTest, HealthChecker) { message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd( TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_2.json")))); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_398882981ada7c10", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(12UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(12UL, cluster_->info()->stats().membership_healthy_.value()); @@ -420,7 +413,6 @@ TEST_F(SdsTest, HealthChecker) { message->body().reset(new Buffer::OwnedImpl(Filesystem::fileReadToEnd( TestEnvironment::runfilesPath("test/common/upstream/test_data/sds_response_3.json")))); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("hash_b9f8f43f828a3b25", cluster_->versionInfo()); EXPECT_EQ(13UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(12UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); EXPECT_EQ(12UL, cluster_->info()->stats().membership_healthy_.value()); @@ -456,7 +448,6 @@ TEST_F(SdsTest, Failure) { EXPECT_CALL(*timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("", cluster_->versionInfo()); EXPECT_EQ(1UL, cluster_->info()->stats().update_failure_.value()); } @@ -475,7 +466,6 @@ TEST_F(SdsTest, FailureArray) { EXPECT_CALL(*timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ("", cluster_->versionInfo()); EXPECT_EQ(1UL, cluster_->info()->stats().update_failure_.value()); } diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 164526729faf..fbd6c5670695 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -209,7 +209,6 @@ TEST_P(IntegrationAdminTest, Admin) { EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); EXPECT_THAT(response->body(), testing::HasSubstr("added_via_api")); - EXPECT_THAT(response->body(), testing::HasSubstr("version_info::static\n")); EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/cpuprofiler", "", diff --git a/test/mocks/config/mocks.h b/test/mocks/config/mocks.h index 83dd4b76bebd..99014dad6def 100644 --- a/test/mocks/config/mocks.h +++ b/test/mocks/config/mocks.h @@ -26,9 +26,9 @@ class MockSubscriptionCallbacks : public SubscriptionCallbacks { } template static std::string resourceName_(const T& resource) { return resource.name(); } - MOCK_METHOD1_T( - onConfigUpdate, - void(const typename SubscriptionCallbacks::ResourceVector& resources)); + MOCK_METHOD2_T(onConfigUpdate, + void(const typename SubscriptionCallbacks::ResourceVector& resources, + const std::string& version_info)); MOCK_METHOD1_T(onConfigUpdateFailed, void(const EnvoyException* e)); MOCK_METHOD1_T(resourceName, std::string(const ProtobufWkt::Any& resource)); }; @@ -38,8 +38,6 @@ template class MockSubscription : public Subscription& resources, SubscriptionCallbacks& callbacks)); MOCK_METHOD1_T(updateResources, void(const std::vector& resources)); - - MOCK_CONST_METHOD0_T(versionInfo, const std::string()); }; class MockGrpcMuxWatch : public GrpcMuxWatch { diff --git a/test/server/lds_api_test.cc b/test/server/lds_api_test.cc index c8bbb0ad42cd..1cb7d01b1c3e 100644 --- a/test/server/lds_api_test.cc +++ b/test/server/lds_api_test.cc @@ -120,7 +120,7 @@ TEST_F(LdsApiTest, ValidateFail) { Protobuf::RepeatedPtrField listeners; listeners.Add(); - EXPECT_THROW(lds_->onConfigUpdate(listeners), ProtoValidationException); + EXPECT_THROW(lds_->onConfigUpdate(listeners, ""), ProtoValidationException); EXPECT_CALL(request_, cancel()); } @@ -167,7 +167,7 @@ TEST_F(LdsApiTest, MisconfiguredListenerNameIsPresentInException) { EXPECT_CALL(listener_manager_, addOrUpdateListener(_, true)) .WillOnce(Throw(EnvoyException("something is wrong"))); - EXPECT_THROW_WITH_MESSAGE(lds_->onConfigUpdate(listeners), EnvoyException, + EXPECT_THROW_WITH_MESSAGE(lds_->onConfigUpdate(listeners, ""), EnvoyException, "Error adding/updating listener invalid-listener: something is wrong"); EXPECT_CALL(request_, cancel()); } @@ -231,7 +231,6 @@ TEST_F(LdsApiTest, Basic) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ(Config::Utility::computeHashedVersion(response1_json).first, lds_->versionInfo()); EXPECT_EQ(15400115654359694268U, store_.gauge("listener_manager.lds.version").value()); expectRequest(); interval_timer_->callback_(); @@ -263,7 +262,6 @@ TEST_F(LdsApiTest, Basic) { EXPECT_CALL(listener_manager_, removeListener("listener2")).WillOnce(Return(true)); EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ(Config::Utility::computeHashedVersion(response2_json).first, lds_->versionInfo()); EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_success").value()); @@ -293,7 +291,6 @@ TEST_F(LdsApiTest, TlsConfigWithoutCaCert) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onSuccess(std::move(message)); - EXPECT_EQ(Config::Utility::computeHashedVersion(response1_json).first, lds_->versionInfo()); expectRequest(); interval_timer_->callback_(); @@ -342,7 +339,6 @@ TEST_F(LdsApiTest, TlsConfigWithoutCaCert) { expectAdd("listener-8080", true); EXPECT_CALL(*interval_timer_, enableTimer(_)); EXPECT_NO_THROW(callbacks_->onSuccess(std::move(message))); - EXPECT_EQ(Config::Utility::computeHashedVersion(response2_json).first, lds_->versionInfo()); } TEST_F(LdsApiTest, Failure) { @@ -369,7 +365,6 @@ TEST_F(LdsApiTest, Failure) { EXPECT_CALL(*interval_timer_, enableTimer(_)); callbacks_->onFailure(Http::AsyncClient::FailureReason::Reset); - EXPECT_EQ("", lds_->versionInfo()); EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_attempt").value()); EXPECT_EQ(2UL, store_.counter("listener_manager.lds.update_failure").value());