Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions include/envoy/config/grpc_mux.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#pragma once

#include <vector>

#include "envoy/common/exception.h"
#include "envoy/common/pure.h"
#include "envoy/config/subscription.h"
Expand Down Expand Up @@ -63,13 +65,36 @@ class GrpcMux {
*/
virtual void pause(const std::string& type_url) PURE;

/**
* Pause discovery requests for all of given API types. This is useful when we're processing an
* update for LDS or CDS and don't want a flood of updates for RDS or EDS respectively. Discovery
* requests may later be resumed with resume().
* @param type_urls all of type URLs corresponding to xDS APIs.
*/
void pause(const std::vector<std::string>& type_urls) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need some unit tests for these vectored pause/resume methods?

for (const auto& type_url : type_urls) {
pause(type_url);
}
}

/**
* Resume discovery requests for a given API type. This will send a discovery request if one would
* have been sent during the pause.
* @param type_url type URL corresponding to xDS API e.g. type.googleapis.com/envoy.api.v2.Cluster
*/
virtual void resume(const std::string& type_url) PURE;

/**
* Resume discovery requests for all of given API types. This will send a discovery request if one
* would have been sent during the pause.
* @param type_urls all of type URLs corresponding to xDS API.
*/
void resume(const std::vector<std::string>& type_urls) {
for (const auto& type_url : type_urls) {
resume(type_url);
}
}

/**
* Retrieves the current pause state as set by pause()/resume().
* @param type_url type URL corresponding to xDS API, e.g.
Expand Down
7 changes: 0 additions & 7 deletions source/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,6 @@ envoy_cc_library(
],
)

envoy_cc_library(
name = "resources_lib",
hdrs = ["resources.h"],
deps = ["//source/common/singleton:const_singleton"],
)

envoy_cc_library(
name = "runtime_utility_lib",
srcs = ["runtime_utility.cc"],
Expand Down Expand Up @@ -317,7 +311,6 @@ envoy_cc_library(
hdrs = ["utility.h"],
deps = [
":api_type_oracle_lib",
":resources_lib",
":version_converter_lib",
"//include/envoy/config:grpc_mux_interface",
"//include/envoy/config:subscription_interface",
Expand Down
10 changes: 4 additions & 6 deletions source/common/router/scoped_rds.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "common/common/logger.h"
#include "common/common/utility.h"
#include "common/config/api_version.h"
#include "common/config/resources.h"
#include "common/config/version_converter.h"
#include "common/init/manager_impl.h"
#include "common/init/watcher_impl.h"
Expand Down Expand Up @@ -236,16 +235,16 @@ void ScopedRdsConfigSubscription::onConfigUpdate(
std::unique_ptr<Cleanup> resume_rds;
// if local init manager is initialized, the parent init manager may have gone away.
if (localInitManager().state() == Init::Manager::State::Initialized) {
const auto type_url = Grpc::Common::typeUrl(getResourceName());
noop_init_manager =
std::make_unique<Init::ManagerImpl>(fmt::format("SRDS {}:{}", name_, version_info));
// Pause RDS to not send a burst of RDS requests until we start all the new subscriptions.
// In the case if factory_context_.init_manager() is uninitialized, RDS is already paused
// either by Server init or LDS init.
if (factory_context_.clusterManager().adsMux()) {
factory_context_.clusterManager().adsMux()->pause(
Envoy::Config::TypeUrl::get().RouteConfiguration);
factory_context_.clusterManager().adsMux()->pause(type_url);
}
resume_rds = std::make_unique<Cleanup>([this, &noop_init_manager, version_info] {
resume_rds = std::make_unique<Cleanup>([this, &noop_init_manager, version_info, type_url] {
// For new RDS subscriptions created after listener warming up, we don't wait for them to
// warm up.
Init::WatcherImpl noop_watcher(
Expand All @@ -257,8 +256,7 @@ void ScopedRdsConfigSubscription::onConfigUpdate(
// Note in the case of partial acceptance, accepted RDS subscriptions should be started
// despite of any error.
if (factory_context_.clusterManager().adsMux()) {
factory_context_.clusterManager().adsMux()->resume(
Envoy::Config::TypeUrl::get().RouteConfiguration);
factory_context_.clusterManager().adsMux()->resume(type_url);
}
});
}
Expand Down
9 changes: 5 additions & 4 deletions source/common/upstream/cds_api_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
#include "common/common/cleanup.h"
#include "common/common/utility.h"
#include "common/config/api_version.h"
#include "common/config/resources.h"
#include "common/config/resource_name.h"
#include "common/config/utility.h"
#include "common/protobuf/utility.h"

Expand Down Expand Up @@ -68,9 +68,10 @@ void CdsApiImpl::onConfigUpdate(
const std::string& system_version_info) {
std::unique_ptr<Cleanup> maybe_eds_resume;
if (cm_.adsMux()) {
cm_.adsMux()->pause(Config::TypeUrl::get().ClusterLoadAssignment);
maybe_eds_resume = std::make_unique<Cleanup>(
[this] { cm_.adsMux()->resume(Config::TypeUrl::get().ClusterLoadAssignment); });
const auto type_url = Grpc::Common::typeUrl(getResourceName());
cm_.adsMux()->pause(type_url);
maybe_eds_resume =
std::make_unique<Cleanup>([this, type_url] { cm_.adsMux()->resume(type_url); });
}

ENVOY_LOG(info, "cds: add {} cluster(s), remove {} cluster(s)", added_resources.size(),
Expand Down
24 changes: 15 additions & 9 deletions source/common/upstream/cluster_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include "common/common/fmt.h"
#include "common/common/utility.h"
#include "common/config/new_grpc_mux_impl.h"
#include "common/config/resources.h"
#include "common/config/utility.h"
#include "common/config/version_converter.h"
#include "common/grpc/async_client_manager_impl.h"
Expand Down Expand Up @@ -143,16 +142,21 @@ void ClusterManagerInitHelper::maybeFinishInitialize() {
if (!secondary_init_clusters_.empty()) {
if (!started_secondary_initialize_) {
ENVOY_LOG(info, "cm init: initializing secondary clusters");
const auto target_type_urls =
Config::getAllVersionTypeUrls<envoy::config::endpoint::v3::ClusterLoadAssignment>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a regression in terms of reducing the amount of version hardcoding; before we could just do Config::TypeUrl::get().ClusterLoadAssignment(), now we need to mention v3..


// If the first CDS response doesn't have any primary cluster, ClusterLoadAssignment
// should be already paused by CdsApiImpl::onConfigUpdate(). Need to check that to
// avoid double pause ClusterLoadAssignment.
if (cm_.adsMux() == nullptr ||
cm_.adsMux()->paused(Config::TypeUrl::get().ClusterLoadAssignment)) {
if (cm_.adsMux() == nullptr) {
initializeSecondaryClusters();
} else {
cm_.adsMux()->pause(Config::TypeUrl::get().ClusterLoadAssignment);
Cleanup eds_resume(
[this] { cm_.adsMux()->resume(Config::TypeUrl::get().ClusterLoadAssignment); });
for (auto&& type_url : target_type_urls) {
if (!cm_.adsMux()->paused(type_url)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I think we're still mixing two unrelated PRs; switching to non-hardcoded type URLs and supporting multiple type URLs.

cm_.adsMux()->pause(type_url);
Cleanup eds_resume([this, type_url] { cm_.adsMux()->resume(type_url); });
}
}
initializeSecondaryClusters();
}
}
Expand Down Expand Up @@ -753,10 +757,12 @@ void ClusterManagerImpl::updateClusterCounts() {
// If we're in the middle of shutting down (ads_mux_ already gone) then this is irrelevant.
if (ads_mux_) {
const uint64_t previous_warming = cm_stats_.warming_clusters_.value();
const auto target_type_urls =
Config::getAllVersionTypeUrls<envoy::config::cluster::v3::Cluster>();
if (previous_warming == 0 && !warming_clusters_.empty()) {
ads_mux_->pause(Config::TypeUrl::get().Cluster);
ads_mux_->pause(target_type_urls);
} else if (previous_warming > 0 && warming_clusters_.empty()) {
ads_mux_->resume(Config::TypeUrl::get().Cluster);
ads_mux_->resume(target_type_urls);
}
}
cm_stats_.active_clusters_.set(active_clusters_.size());
Expand Down Expand Up @@ -1377,4 +1383,4 @@ ProdClusterManagerFactory::createCds(const envoy::config::core::v3::ConfigSource
}

} // namespace Upstream
} // namespace Envoy
} // namespace Envoy
8 changes: 4 additions & 4 deletions source/server/lds_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "common/common/assert.h"
#include "common/common/cleanup.h"
#include "common/config/api_version.h"
#include "common/config/resources.h"
#include "common/config/utility.h"
#include "common/protobuf/utility.h"

Expand Down Expand Up @@ -43,9 +42,10 @@ void LdsApiImpl::onConfigUpdate(
const std::string& system_version_info) {
std::unique_ptr<Cleanup> maybe_eds_resume;
if (cm_.adsMux()) {
cm_.adsMux()->pause(Config::TypeUrl::get().RouteConfiguration);
maybe_eds_resume = std::make_unique<Cleanup>(
[this] { cm_.adsMux()->resume(Config::TypeUrl::get().RouteConfiguration); });
const auto type_url = Grpc::Common::typeUrl(getResourceName());
cm_.adsMux()->pause(type_url);
maybe_eds_resume =
std::make_unique<Cleanup>([this, type_url] { cm_.adsMux()->resume(type_url); });
}

bool any_applied = false;
Expand Down
7 changes: 4 additions & 3 deletions source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
#include "common/common/mutex_tracer_impl.h"
#include "common/common/utility.h"
#include "common/common/version.h"
#include "common/config/resources.h"
#include "common/config/utility.h"
#include "common/http/codes.h"
#include "common/local_info/local_info_impl.h"
Expand Down Expand Up @@ -522,12 +521,14 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch
if (instance.isShutdown()) {
return;
}
const auto target_type_urls =
Config::getAllVersionTypeUrls<envoy::config::route::v3::RouteConfiguration>();

// Pause RDS to ensure that we don't send any requests until we've
// subscribed to all the RDS resources. The subscriptions happen in the init callbacks,
// so we pause RDS until we've completed all the callbacks.
if (cm.adsMux()) {
cm.adsMux()->pause(Config::TypeUrl::get().RouteConfiguration);
cm.adsMux()->pause(target_type_urls);
}

ENVOY_LOG(info, "all clusters initialized. initializing init manager");
Expand All @@ -536,7 +537,7 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch
// Now that we're execute all the init callbacks we can resume RDS
// as we've subscribed to all the statically defined RDS resources.
if (cm.adsMux()) {
cm.adsMux()->resume(Config::TypeUrl::get().RouteConfiguration);
cm.adsMux()->resume(target_type_urls);
}
});
}
Expand Down
6 changes: 3 additions & 3 deletions test/common/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ envoy_cc_test(
"//source/common/config:api_version_lib",
"//source/common/config:grpc_mux_lib",
"//source/common/config:protobuf_link_hacks",
"//source/common/config:resources_lib",
"//source/common/config:version_converter_lib",
"//source/common/protobuf",
"//source/common/stats:isolated_store_lib",
Expand All @@ -118,6 +117,7 @@ envoy_cc_test(
"//test/mocks/local_info:local_info_mocks",
"//test/mocks/runtime:runtime_mocks",
"//test/test_common:logging_lib",
"//test/test_common:resources_lib",
"//test/test_common:simulated_time_system_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/api/v2:pkg_cc_proto",
Expand All @@ -132,7 +132,6 @@ envoy_cc_test(
deps = [
"//source/common/config:new_grpc_mux_lib",
"//source/common/config:protobuf_link_hacks",
"//source/common/config:resources_lib",
"//source/common/config:version_converter_lib",
"//source/common/protobuf",
"//test/common/stats:stat_test_utility_lib",
Expand All @@ -143,6 +142,7 @@ envoy_cc_test(
"//test/mocks/local_info:local_info_mocks",
"//test/mocks/runtime:runtime_mocks",
"//test/test_common:logging_lib",
"//test/test_common:resources_lib",
"//test/test_common:simulated_time_system_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto",
Expand Down Expand Up @@ -182,13 +182,13 @@ envoy_cc_test_library(
"//source/common/config:api_version_lib",
"//source/common/config:grpc_mux_lib",
"//source/common/config:grpc_subscription_lib",
"//source/common/config:resources_lib",
"//source/common/config:version_converter_lib",
"//test/mocks/config:config_mocks",
"//test/mocks/event:event_mocks",
"//test/mocks/grpc:grpc_mocks",
"//test/mocks/local_info:local_info_mocks",
"//test/mocks/upstream:upstream_mocks",
"//test/test_common:resources_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/api/v2:pkg_cc_proto",
"@envoy_api//envoy/config/core/v3:pkg_cc_proto",
Expand Down
2 changes: 1 addition & 1 deletion test/common/config/grpc_mux_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include "common/config/api_version.h"
#include "common/config/grpc_mux_impl.h"
#include "common/config/protobuf_link_hacks.h"
#include "common/config/resources.h"
#include "common/config/utility.h"
#include "common/config/version_converter.h"
#include "common/protobuf/protobuf.h"
Expand All @@ -22,6 +21,7 @@
#include "test/mocks/local_info/mocks.h"
#include "test/mocks/runtime/mocks.h"
#include "test/test_common/logging.h"
#include "test/test_common/resources.h"
#include "test/test_common/simulated_time_system.h"
#include "test/test_common/test_time.h"
#include "test/test_common/utility.h"
Expand Down
2 changes: 1 addition & 1 deletion test/common/config/grpc_subscription_test_harness.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "common/config/api_version.h"
#include "common/config/grpc_mux_impl.h"
#include "common/config/grpc_subscription_impl.h"
#include "common/config/resources.h"
#include "common/config/version_converter.h"

#include "test/common/config/subscription_test_harness.h"
Expand All @@ -20,6 +19,7 @@
#include "test/mocks/grpc/mocks.h"
#include "test/mocks/local_info/mocks.h"
#include "test/mocks/upstream/mocks.h"
#include "test/test_common/resources.h"
#include "test/test_common/utility.h"

#include "gmock/gmock.h"
Expand Down
2 changes: 1 addition & 1 deletion test/common/config/new_grpc_mux_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#include "common/common/empty_string.h"
#include "common/config/new_grpc_mux_impl.h"
#include "common/config/protobuf_link_hacks.h"
#include "common/config/resources.h"
#include "common/config/utility.h"
#include "common/config/version_converter.h"
#include "common/protobuf/protobuf.h"
Expand All @@ -19,6 +18,7 @@
#include "test/mocks/local_info/mocks.h"
#include "test/mocks/runtime/mocks.h"
#include "test/test_common/logging.h"
#include "test/test_common/resources.h"
#include "test/test_common/simulated_time_system.h"
#include "test/test_common/test_time.h"
#include "test/test_common/utility.h"
Expand Down
2 changes: 1 addition & 1 deletion test/config/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ envoy_cc_test_library(
"utility.h",
],
deps = [
"//source/common/config:resources_lib",
"//source/common/http:utility_lib",
"//source/common/network:address_lib",
"//source/common/protobuf",
Expand All @@ -26,6 +25,7 @@ envoy_cc_test_library(
"//test/integration:server_stats_interface",
"//test/test_common:environment_lib",
"//test/test_common:network_utility_lib",
"//test/test_common:resources_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
"@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
Expand Down
2 changes: 1 addition & 1 deletion test/config/utility.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
#include "envoy/service/discovery/v3/discovery.pb.h"

#include "common/common/assert.h"
#include "common/config/resources.h"
#include "common/http/utility.h"
#include "common/protobuf/utility.h"

#include "test/config/integration/certs/client_ecdsacert_hash.h"
#include "test/config/integration/certs/clientcert_hash.h"
#include "test/test_common/environment.h"
#include "test/test_common/network_utility.h"
#include "test/test_common/resources.h"
#include "test/test_common/utility.h"

#include "absl/strings/str_replace.h"
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/clusters/aggregate/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ envoy_extension_cc_test(
extension_name = "envoy.clusters.aggregate",
deps = [
"//source/common/config:protobuf_link_hacks",
"//source/common/config:resources_lib",
"//source/common/protobuf:utility_lib",
"//source/extensions/clusters/aggregate:cluster",
"//source/extensions/filters/network/tcp_proxy:config",
Expand All @@ -65,6 +64,7 @@ envoy_extension_cc_test(
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/server:server_mocks",
"//test/test_common:network_utility_lib",
"//test/test_common:resources_lib",
"//test/test_common:utility_lib",
"@envoy_api//envoy/config/cluster/v3:pkg_cc_proto",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include "envoy/stats/scope.h"

#include "common/config/protobuf_link_hacks.h"
#include "common/config/resources.h"
#include "common/protobuf/protobuf.h"
#include "common/protobuf/utility.h"

Expand All @@ -12,6 +11,7 @@
#include "test/integration/utility.h"
#include "test/mocks/server/mocks.h"
#include "test/test_common/network_utility.h"
#include "test/test_common/resources.h"
#include "test/test_common/simulated_time_system.h"
#include "test/test_common/utility.h"

Expand Down
Loading