From 077afdc7da8499ee1184daa8281dc4cdd876c16d Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Thu, 26 Dec 2019 21:25:49 +0800 Subject: [PATCH 01/11] Implement the EGDS protocol Signed-off-by: leilei.gll Signed-off-by: jingyi.lyq --- api/envoy/api/v2/egds.proto | 46 ++ api/envoy/api/v2/endpoint.proto | 27 ++ api/envoy/config/endpoint/v3/endpoint.proto | 31 ++ docs/root/intro/version_history.rst | 1 + generated_api_shadow/envoy/api/v2/egds.proto | 46 ++ .../envoy/api/v2/endpoint.proto | 27 ++ .../envoy/config/endpoint/v3/endpoint.proto | 32 ++ source/common/config/resources.h | 1 + source/common/upstream/BUILD | 89 ++++ source/common/upstream/eds.cc | 97 +++- source/common/upstream/eds.h | 22 +- source/common/upstream/egds_api_impl.cc | 132 ++++++ source/common/upstream/egds_api_impl.h | 83 ++++ source/common/upstream/egds_cluster_mapper.cc | 177 ++++++++ source/common/upstream/egds_cluster_mapper.h | 111 +++++ source/common/upstream/endpoint_group.cc | 16 + source/common/upstream/endpoint_group.h | 38 ++ .../common/upstream/endpoint_group_monitor.h | 34 ++ .../common/upstream/endpoint_groups_manager.h | 25 ++ .../upstream/endpoint_groups_manager_impl.cc | 54 +++ .../upstream/endpoint_groups_manager_impl.h | 35 ++ source/common/upstream/upstream_impl.cc | 16 + source/common/upstream/upstream_impl.h | 13 +- test/common/upstream/BUILD | 51 +++ test/common/upstream/egds_api_impl_test.cc | 266 +++++++++++ .../upstream/egds_cluster_mapper_test.cc | 223 ++++++++++ .../endpoint_groups_manager_impl_test.cc | 118 +++++ test/config/utility.cc | 35 ++ test/config/utility.h | 18 + test/integration/BUILD | 19 + test/integration/egds_integration_test.cc | 418 ++++++++++++++++++ test/mocks/upstream/BUILD | 3 + test/mocks/upstream/mocks.cc | 6 + test/mocks/upstream/mocks.h | 42 ++ 34 files changed, 2349 insertions(+), 3 deletions(-) create mode 100644 api/envoy/api/v2/egds.proto create mode 100644 generated_api_shadow/envoy/api/v2/egds.proto create mode 100644 source/common/upstream/egds_api_impl.cc create mode 100644 source/common/upstream/egds_api_impl.h create mode 100644 source/common/upstream/egds_cluster_mapper.cc create mode 100644 source/common/upstream/egds_cluster_mapper.h create mode 100644 source/common/upstream/endpoint_group.cc create mode 100644 source/common/upstream/endpoint_group.h create mode 100644 source/common/upstream/endpoint_group_monitor.h create mode 100644 source/common/upstream/endpoint_groups_manager.h create mode 100644 source/common/upstream/endpoint_groups_manager_impl.cc create mode 100644 source/common/upstream/endpoint_groups_manager_impl.h create mode 100644 test/common/upstream/egds_api_impl_test.cc create mode 100644 test/common/upstream/egds_cluster_mapper_test.cc create mode 100644 test/common/upstream/endpoint_groups_manager_impl_test.cc create mode 100644 test/integration/egds_integration_test.cc diff --git a/api/envoy/api/v2/egds.proto b/api/envoy/api/v2/egds.proto new file mode 100644 index 000000000000..fbd3a9e69367 --- /dev/null +++ b/api/envoy/api/v2/egds.proto @@ -0,0 +1,46 @@ +syntax = "proto3"; + +package envoy.api.v2; + +import "envoy/api/v2/core/config_source.proto"; +import "envoy/api/v2/discovery.proto"; +import "envoy/api/v2/endpoint/endpoint_components.proto"; + +import "google/api/annotations.proto"; +import "google/protobuf/duration.proto"; +import "google/protobuf/wrappers.proto"; + +import "envoy/annotations/resource.proto"; +import "udpa/annotations/migrate.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.api.v2"; +option java_outer_classname = "EgdsProto"; +option java_multiple_files = true; +option java_generic_services = true; +option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; + +// [#protodoc-title: EGDS] +// Endpoint discovery :ref:`architecture overview ` + +service EndpointGroupDiscoveryService { + option (envoy.annotations.resource).type = "envoy.api.v2.EndpointGroup"; + + // The resource_names field in DiscoveryRequest specifies a list of clusters + // to subscribe to updates for. + rpc StreamEndpointGroups(stream DiscoveryRequest) returns (stream DiscoveryResponse) { + } + + rpc DeltaEndpointGroups(stream DeltaDiscoveryRequest) returns (stream DeltaDiscoveryResponse) { + } + + rpc FetchEndpointGroups(DiscoveryRequest) returns (DiscoveryResponse) { + option (google.api.http).post = "/v2/discovery:endpoint_groups"; + option (google.api.http).body = "*"; + } +} + +// [#not-implemented-hide:] Not configuration. Workaround c++ protobuf issue with importing +// services: https://github.com/google/protobuf/issues/4221 and protoxform to upgrade the file. +message EgdsDummy { +} diff --git a/api/envoy/api/v2/endpoint.proto b/api/envoy/api/v2/endpoint.proto index d800c6d19e5a..3ae53b168596 100644 --- a/api/envoy/api/v2/endpoint.proto +++ b/api/envoy/api/v2/endpoint.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package envoy.api.v2; +import "envoy/api/v2/core/config_source.proto"; import "envoy/api/v2/endpoint/endpoint_components.proto"; import "envoy/type/percent.proto"; @@ -114,4 +115,30 @@ message ClusterLoadAssignment { // Load balancing policy settings. Policy policy = 4; + + // References endpoint groups by name. + // Both endpoints and EGDS references will be used when present + repeated Egds endpoint_groups = 6; +} + +// New xDS payload. The size of endpoints is fully arbitrary. +// Also the EGDS requests to Pilot can be merged into one to boost efficiency +message EndpointGroup { + string name = 1 [(validate.rules).string = {min_bytes: 1}]; + + // List of endpoints to load balance to. + repeated endpoint.LocalityLbEndpoints endpoints = 2; + + // Map of named endpoints that can be referenced in LocalityLbEndpoints. + // [#not-implemented-hide:] + map named_endpoints = 3; +} + +// The EGDS request configuration. +message Egds { + // Configuration for the source of EGDS updates for this Cluster. + core.ConfigSource config_source = 1; + + // the endpoint group resource name. + string endpoint_group_name = 2; } diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index 127b2f9b6709..71882455525a 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -122,4 +122,35 @@ message ClusterLoadAssignment { // Load balancing policy settings. Policy policy = 4; + + // References endpoint groups by name. + // Both endpoints and EGDS references will be used when present + repeated Egds endpoint_groups = 6; +} + +// New xDS payload. The size of endpoints is fully arbitrary. +// Also the EGDS requests to Pilot can be merged into one to boost efficiency +message EndpointGroup { + option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; + + string name = 1 [(validate.rules).string = {min_bytes: 1}]; + + // List of endpoints to load balance to. + repeated LocalityLbEndpoints endpoints = 2; + + // Map of named endpoints that can be referenced in LocalityLbEndpoints. + // [#not-implemented-hide:] + map named_endpoints = 3; +} + +// The EGDS request configuration. +message Egds { + option (udpa.annotations.versioning).previous_message_type = + "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; + + // Configuration for the source of EGDS updates for this Cluster. + core.v3.ConfigSource config_source = 1; + + // the endpoint group resource name. + string endpoint_group_name = 2; } diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index d8b23a2d5cc6..4415bb3b5573 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -29,6 +29,7 @@ Version history restart Envoy. The behavior will not switch until the connection pools are recreated. The new circuit breaker behavior is described :ref:`here `. * upstream: changed load distribution algorithm when all priorities enter :ref:`panic mode`. +* upstream: added support for the EGDS protocol 1.13.0 (January 20, 2020) ========================= diff --git a/generated_api_shadow/envoy/api/v2/egds.proto b/generated_api_shadow/envoy/api/v2/egds.proto new file mode 100644 index 000000000000..c43b2c8137ef --- /dev/null +++ b/generated_api_shadow/envoy/api/v2/egds.proto @@ -0,0 +1,46 @@ +syntax = "proto3"; + +package envoy.api.v2; + +import "envoy/api/v2/discovery.proto"; +import "envoy/api/v2/endpoint/endpoint_components.proto"; +import "envoy/api/v2/core/config_source.proto"; + +import "google/api/annotations.proto"; +import "google/protobuf/duration.proto"; +import "google/protobuf/wrappers.proto"; + +import "envoy/annotations/resource.proto"; +import "udpa/annotations/migrate.proto"; +import "validate/validate.proto"; + +option java_package = "io.envoyproxy.envoy.api.v2"; +option java_outer_classname = "EgdsProto"; +option java_multiple_files = true; +option java_generic_services = true; +option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; + +// [#protodoc-title: EGDS] +// Endpoint discovery :ref:`architecture overview ` + +service EndpointGroupDiscoveryService { + option (envoy.annotations.resource).type = "envoy.api.v2.EndpointGroup"; + + // The resource_names field in DiscoveryRequest specifies a list of clusters + // to subscribe to updates for. + rpc StreamEndpointGroups(stream DiscoveryRequest) returns (stream DiscoveryResponse) { + } + + rpc DeltaEndpointGroups(stream DeltaDiscoveryRequest) returns (stream DeltaDiscoveryResponse) { + } + + rpc FetchEndpointGroups(DiscoveryRequest) returns (DiscoveryResponse) { + option (google.api.http).post = "/v2/discovery:endpoint_groups"; + option (google.api.http).body = "*"; + } +} + +// [#not-implemented-hide:] Not configuration. Workaround c++ protobuf issue with importing +// services: https://github.com/google/protobuf/issues/4221 and protoxform to upgrade the file. +message EgdsDummy { +} diff --git a/generated_api_shadow/envoy/api/v2/endpoint.proto b/generated_api_shadow/envoy/api/v2/endpoint.proto index d800c6d19e5a..3ae53b168596 100644 --- a/generated_api_shadow/envoy/api/v2/endpoint.proto +++ b/generated_api_shadow/envoy/api/v2/endpoint.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package envoy.api.v2; +import "envoy/api/v2/core/config_source.proto"; import "envoy/api/v2/endpoint/endpoint_components.proto"; import "envoy/type/percent.proto"; @@ -114,4 +115,30 @@ message ClusterLoadAssignment { // Load balancing policy settings. Policy policy = 4; + + // References endpoint groups by name. + // Both endpoints and EGDS references will be used when present + repeated Egds endpoint_groups = 6; +} + +// New xDS payload. The size of endpoints is fully arbitrary. +// Also the EGDS requests to Pilot can be merged into one to boost efficiency +message EndpointGroup { + string name = 1 [(validate.rules).string = {min_bytes: 1}]; + + // List of endpoints to load balance to. + repeated endpoint.LocalityLbEndpoints endpoints = 2; + + // Map of named endpoints that can be referenced in LocalityLbEndpoints. + // [#not-implemented-hide:] + map named_endpoints = 3; +} + +// The EGDS request configuration. +message Egds { + // Configuration for the source of EGDS updates for this Cluster. + core.ConfigSource config_source = 1; + + // the endpoint group resource name. + string endpoint_group_name = 2; } diff --git a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto index 127b2f9b6709..ce52089c9b37 100644 --- a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto +++ b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package envoy.config.endpoint.v3; +import "envoy/config/core/v3/config_source.proto"; import "envoy/config/endpoint/v3/endpoint_components.proto"; import "envoy/type/v3/percent.proto"; @@ -122,4 +123,35 @@ message ClusterLoadAssignment { // Load balancing policy settings. Policy policy = 4; + + // References endpoint groups by name. + // Both endpoints and EGDS references will be used when present + repeated Egds endpoint_groups = 6; +} + +// New xDS payload. The size of endpoints is fully arbitrary. +// Also the EGDS requests to Pilot can be merged into one to boost efficiency +message EndpointGroup { + option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; + + string name = 1 [(validate.rules).string = {min_bytes: 1}]; + + // List of endpoints to load balance to. + repeated LocalityLbEndpoints endpoints = 2; + + // Map of named endpoints that can be referenced in LocalityLbEndpoints. + // [#not-implemented-hide:] + map named_endpoints = 3; +} + +// The EGDS request configuration. +message Egds { + option (udpa.annotations.versioning).previous_message_type = + "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; + + // Configuration for the source of EGDS updates for this Cluster. + core.v3.ConfigSource config_source = 1; + + // the endpoint group resource name. + string endpoint_group_name = 2; } diff --git a/source/common/config/resources.h b/source/common/config/resources.h index 7af6a74e4f85..39b2f58662e6 100644 --- a/source/common/config/resources.h +++ b/source/common/config/resources.h @@ -21,6 +21,7 @@ class TypeUrlValues { const std::string ScopedRouteConfiguration{ "type.googleapis.com/envoy.api.v2.ScopedRouteConfiguration"}; const std::string Runtime{"type.googleapis.com/envoy.service.discovery.v2.Runtime"}; + const std::string EndpointGroup{"type.googleapis.com/envoy.api.v2.EndpointGroup"}; }; using TypeUrl = ConstSingleton; diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index f79cea3e6944..5f39be711a0d 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -342,6 +342,9 @@ envoy_cc_library( hdrs = ["eds.h"], deps = [ ":cluster_factory_lib", + ":egds_api_impl_lib", + ":egds_cluster_mapper_lib", + ":endpoint_groups_manager_lib", ":upstream_includes", "//include/envoy/config:grpc_mux_interface", "//include/envoy/config:subscription_factory_interface", @@ -568,3 +571,89 @@ envoy_cc_library( "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) + +envoy_cc_library( + name = "endpoint_group_monitor_interface", + hdrs = ["endpoint_group_monitor.h"], + deps = [ + "//include/envoy/upstream:upstream_interface", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "egds_cluster_mapper_lib", + srcs = ["egds_cluster_mapper.cc"], + hdrs = ["egds_cluster_mapper.h"], + deps = [ + ":endpoint_group_monitor_interface", + ":upstream_includes", + "//include/envoy/local_info:local_info_interface", + "//include/envoy/upstream:upstream_interface", + "//source/common/common:minimal_logger_lib", + "//source/common/network:resolver_lib", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "endpoint_groups_manager_interface", + hdrs = ["endpoint_groups_manager.h"], + deps = [ + "//include/envoy/upstream:upstream_interface", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "endpoint_groups_manager_lib", + srcs = ["endpoint_groups_manager_impl.cc"], + hdrs = ["endpoint_groups_manager_impl.h"], + deps = [ + ":endpoint_group_lib", + ":endpoint_group_monitor_interface", + ":endpoint_groups_manager_interface", + "//include/envoy/upstream:upstream_interface", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "endpoint_group_lib", + srcs = ["endpoint_group.cc"], + hdrs = ["endpoint_group.h"], + deps = [ + ":endpoint_group_monitor_interface", + "//include/envoy/upstream:upstream_interface", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_library( + name = "egds_api_impl_lib", + srcs = ["egds_api_impl.cc"], + hdrs = ["egds_api_impl.h"], + deps = [ + ":egds_cluster_mapper_lib", + ":endpoint_group_monitor_interface", + ":endpoint_groups_manager_lib", + ":upstream_includes", + "//include/envoy/config:grpc_mux_interface", + "//include/envoy/config:subscription_factory_interface", + "//include/envoy/config:subscription_interface", + "//include/envoy/stats:stats_interface", + "//include/envoy/upstream:locality_lib", + "//include/envoy/upstream:upstream_interface", + "//source/common/common:minimal_logger_lib", + "//source/common/config:metadata_lib", + "//source/common/config:subscription_factory_lib", + "//source/common/config:utility_lib", + "//source/common/grpc:common_lib", + "//source/common/network:address_lib", + "//source/common/network:utility_lib", + "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", + ], +) diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index dfe6ac4d059d..92eab7f024fa 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -9,9 +9,12 @@ #include "envoy/service/discovery/v3/discovery.pb.h" #include "common/common/assert.h" +#include "common/common/cleanup.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/upstream/endpoint_groups_manager_impl.h" namespace Envoy { namespace Upstream { @@ -26,7 +29,9 @@ EdsClusterImpl::EdsClusterImpl( cluster_name_(cluster.eds_cluster_config().service_name().empty() ? cluster.name() : cluster.eds_cluster_config().service_name()), - validation_visitor_(factory_context.messageValidationVisitor()) { + validation_visitor_(factory_context.messageValidationVisitor()), + subscription_factory_(factory_context.clusterManager().subscriptionFactory()), + cm_(factory_context.clusterManager()) { Event::Dispatcher& dispatcher = factory_context.dispatcher(); assignment_timeout_ = dispatcher.createTimer([this]() -> void { onAssignmentTimeout(); }); const auto& eds_config = cluster.eds_cluster_config().eds_config(); @@ -114,6 +119,14 @@ void EdsClusterImpl::onConfigUpdate(const Protobuf::RepeatedPtrField maybe_egds_resume; + if (cm_.adsMux()) { + cm_.adsMux()->pause(Config::TypeUrl::get().EndpointGroup); + maybe_egds_resume = std::make_unique( + [this] { cm_.adsMux()->resume(Config::TypeUrl::get().EndpointGroup); }); + } + auto cluster_load_assignment = MessageUtil::anyConvertAndValidate( resources[0], validation_visitor_); @@ -138,6 +151,23 @@ void EdsClusterImpl::onConfigUpdate(const Protobuf::RepeatedPtrFieldenableTimer(std::chrono::milliseconds(stale_after_ms)); } + // Check if EGDS resource is included. + if (!cluster_load_assignment.endpoint_groups().empty()) { + endpoint_group_manager_ = std::make_unique(); + egds_cluster_mapper_ = std::make_unique( + dynamic_cast(*endpoint_group_manager_), + dynamic_cast(*this), *this, cluster_load_assignment, + local_info_); + egds_api_ = std::make_unique(*endpoint_group_manager_, validation_visitor_, + subscription_factory_, info_->statsScope(), + cluster_load_assignment.endpoint_groups()); + egds_api_->initialize([&] { + // We need to allow server startup to continue, even if we have a bad config. + this->onPreInitComplete(); + }); + return; + } + BatchUpdateHelper helper(*this, cluster_load_assignment); priority_set_.batchHostUpdate(helper); } @@ -153,6 +183,71 @@ void EdsClusterImpl::onConfigUpdate( onConfigUpdate(unwrapped_resource, resources[0].version()); } +void EdsClusterImpl::initializeCluster( + const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment) { + BatchUpdateHelper helper(*this, cluster_load_assignment); + priority_set_.batchHostUpdate(helper); +} + +void EdsClusterImpl::updateHosts(uint32_t priority, const HostVector& hosts_added, + const HostVector& hosts_removed, + PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor) { + ENVOY_LOG(debug, "EG update hosts, add {}, removed {}", hosts_added.size(), hosts_removed.size()); + + // Start update the hosts changes in the Endpoint Group to the cluster store. + const auto& host_set = priority_set_.getOrCreateHostSet(priority, overprovisioning_factor); + HostVectorSharedPtr current_hosts_copy(new HostVector(host_set.hosts())); + + // Update the removed host. + for (auto it = hosts_removed.begin(); it != hosts_removed.end(); ++it) { + const auto& address = (*it)->address()->asString(); + + ENVOY_LOG(debug, "EG removed host {}", address); + + const auto existing_itr = std::find_if(current_hosts_copy->begin(), current_hosts_copy->end(), + [address](const HostSharedPtr current) { + return current->address()->asString() == address; + }); + ASSERT(existing_itr != current_hosts_copy->end()); + current_hosts_copy->erase(existing_itr); + + ASSERT(all_hosts_.find(address) != all_hosts_.end()); + all_hosts_.erase(address); + } + + // Update the added host. + current_hosts_copy->insert(current_hosts_copy->end(), hosts_added.begin(), hosts_added.end()); + for (auto it = hosts_added.begin(); it != hosts_added.end(); ++it) { + const auto& address = (*it)->address()->asString(); + ENVOY_LOG(debug, "EG added host {}", address); + + ASSERT(all_hosts_.find(address) == all_hosts_.end()); + all_hosts_[address] = *it; + } + + // Update the added locality weights. + for (const auto& pair : new_locality_weights_map) { + if (!locality_weights_map_[priority].count(pair.first)) { + locality_weights_map_[priority].insert({pair.first, pair.second}); + } + } + + ASSERT(std::all_of(current_hosts_copy->begin(), current_hosts_copy->end(), + [&](const auto& host) { return host->priority() == priority; })); + ENVOY_LOG(debug, + "EDS hosts or locality weights changed for cluster: {} current hosts {} priority {}", + info_->name(), host_set.hosts().size(), host_set.priority()); + priority_state_manager.updateClusterPrioritySet(priority, std::move(current_hosts_copy), + hosts_added, hosts_removed, absl::nullopt, + overprovisioning_factor); +} + +void EdsClusterImpl::batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) { + priority_set_.batchHostUpdate(callback); +} + bool EdsClusterImpl::validateUpdateSize(int num_resources) { if (num_resources == 0) { ENVOY_LOG(debug, "Missing ClusterLoadAssignment for {} in onConfigUpdate()", cluster_name_); diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 79470a5259db..7c1971953ae9 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -13,6 +13,9 @@ #include "envoy/upstream/locality.h" #include "common/upstream/cluster_factory_impl.h" +#include "common/upstream/egds_api_impl.h" +#include "common/upstream/egds_cluster_mapper.h" +#include "common/upstream/endpoint_groups_manager.h" #include "common/upstream/upstream_impl.h" #include "extensions/clusters/well_known_names.h" @@ -23,7 +26,9 @@ namespace Upstream { /** * Cluster implementation that reads host information from the Endpoint Discovery Service. */ -class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallbacks { +class EdsClusterImpl : public BaseDynamicClusterImpl, + Config::SubscriptionCallbacks, + EgdsClusterMapper::Delegate { public: EdsClusterImpl(const envoy::config::cluster::v3::Cluster& cluster, Runtime::Loader& runtime, Server::Configuration::TransportSocketFactoryContextImpl& factory_context, @@ -54,6 +59,15 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba std::unordered_map& updated_hosts); bool validateUpdateSize(int num_resources); + // EgdsClusterMapper::Delegate + void initializeCluster( + const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment) override; + void updateHosts(uint32_t priority, const HostVector& hosts_added, + const HostVector& hosts_removed, PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor = absl::nullopt) override; + void batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) override; + // ClusterImplBase void reloadHealthyHostsHelper(const HostSharedPtr& host) override; void startPreInit() override; @@ -82,6 +96,12 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, Config::SubscriptionCallba Event::TimerPtr assignment_timeout_; ProtobufMessage::ValidationVisitor& validation_visitor_; InitializePhase initialize_phase_; + + Config::SubscriptionFactory& subscription_factory_; + Upstream::ClusterManager& cm_; + EgdsApiPtr egds_api_; + EgdsClusterMapperPtr egds_cluster_mapper_; + EndpointGroupsManagerPtr endpoint_group_manager_; }; class EdsClusterFactory : public ClusterFactoryImplBase { diff --git a/source/common/upstream/egds_api_impl.cc b/source/common/upstream/egds_api_impl.cc new file mode 100644 index 000000000000..bba1d6cf8bae --- /dev/null +++ b/source/common/upstream/egds_api_impl.cc @@ -0,0 +1,132 @@ +#include "common/upstream/egds_api_impl.h" + +#include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.validate.h" +#include "envoy/service/discovery/v3/discovery.pb.h" + +#include "common/common/utility.h" +#include "common/grpc/common.h" +#include "common/protobuf/utility.h" + +namespace Envoy { +namespace Upstream { + +EgdsApiImpl::EgdsApiImpl( + EndpointGroupsManager& endpoint_group_mananger, + ProtobufMessage::ValidationVisitor& validation_visitor, + Config::SubscriptionFactory& subscription_factory, Stats::Scope& scope, + const Protobuf::RepeatedPtrField& egds_list) + : endpoint_group_mananger_(endpoint_group_mananger), validation_visitor_(validation_visitor), + scope_(scope.createScope("egds.")), stats_({ALL_EGDS_STATS(POOL_COUNTER(*scope_))}) { + std::unordered_map, MessageUtil, + MessageUtil> + config_source_map; + for (auto& egds : egds_list) { + if (config_source_map.count(egds.config_source())) { + config_source_map[egds.config_source()].insert(egds.endpoint_group_name()); + } else { + config_source_map[egds.config_source()] = {egds.endpoint_group_name()}; + } + } + + for (auto& pair : config_source_map) { + auto subscription = subscription_factory.subscriptionFromConfigSource( + pair.first, + Grpc::Common::typeUrl( + envoy::config::endpoint::v3::EndpointGroup().GetDescriptor()->full_name()), + *scope_, *this); + subscription_map_.emplace(std::move(subscription), std::move(pair.second)); + } +} + +void EgdsApiImpl::initialize(std::function initialization_fail_callback) { + initialization_fail_callback_ = initialization_fail_callback; + for (const auto& pair : subscription_map_) { + pair.first->start(pair.second); + } +} + +void EgdsApiImpl::onConfigUpdate(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) { + if (resources.empty()) { + stats_.update_empty_.inc(); + ENVOY_LOG(debug, "egds: empty resource"); + return; + } + + Protobuf::RepeatedPtrField to_remove_repeated; + Protobuf::RepeatedPtrField to_add_repeated; + for (const auto& endpoint_group_blob : resources) { + auto endpoint_group = + MessageUtil::anyConvert(endpoint_group_blob); + if (endpoint_group.endpoints().empty() && endpoint_group.named_endpoints().empty()) { + *to_remove_repeated.Add() = endpoint_group.name(); + continue; + } + + auto* to_add = to_add_repeated.Add(); + to_add->set_name(endpoint_group.name()); + to_add->set_version(version_info); + to_add->mutable_resource()->PackFrom(endpoint_group); + } + + onConfigUpdate(to_add_repeated, to_remove_repeated, version_info); +} + +void EgdsApiImpl::onConfigUpdate( + const Protobuf::RepeatedPtrField& added_resources, + const Protobuf::RepeatedPtrField& removed_resources, + const std::string& version_info) { + std::vector exception_msgs; + std::unordered_set endpoint_group_names; + bool any_applied = false; + for (const auto& resource : added_resources) { + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + try { + endpoint_group = + MessageUtil::anyConvert(resource.resource()); + MessageUtil::validate(endpoint_group, validation_visitor_); + if (!endpoint_group_names.insert(endpoint_group.name()).second) { + throw EnvoyException( + fmt::format("duplicate endpoint group {} found", endpoint_group.name())); + } + + if (endpoint_group_mananger_.addOrUpdateEndpointGroup(endpoint_group, resource.version())) { + any_applied = true; + ENVOY_LOG(info, "egds: update endpoint group '{}'", endpoint_group.name()); + } else { + ENVOY_LOG(debug, "egds: update endpoint group '{}' skipped", endpoint_group.name()); + } + } catch (const EnvoyException& e) { + exception_msgs.push_back(fmt::format("{}: {}", endpoint_group.name(), e.what())); + } + } + + for (const auto& resource_name : removed_resources) { + if (endpoint_group_mananger_.clearEndpointGroup(resource_name, version_info)) { + any_applied = true; + ENVOY_LOG(info, "egds: remove endpoint group '{}'", resource_name); + } + } + + if (any_applied) { + system_version_info_ = version_info; + } + + if (!exception_msgs.empty()) { + throw EnvoyException(fmt::format("Error adding/updating endpoint group(s) {}", + absl::StrJoin(exception_msgs, ", "))); + } + + stats_.config_reload_.inc(); +} + +void EgdsApiImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, + const EnvoyException*) { + ASSERT(Envoy::Config::ConfigUpdateFailureReason::ConnectionFailure != reason); + initialization_fail_callback_(); +} + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/egds_api_impl.h b/source/common/upstream/egds_api_impl.h new file mode 100644 index 000000000000..a040ce33953f --- /dev/null +++ b/source/common/upstream/egds_api_impl.h @@ -0,0 +1,83 @@ +#pragma once + +#include +#include +#include + +#include "envoy/config/core/v3/base.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/config/subscription.h" +#include "envoy/config/subscription_factory.h" +#include "envoy/service/discovery/v3/discovery.pb.h" +#include "envoy/stats/scope.h" + +#include "common/common/logger.h" +#include "common/upstream/egds_cluster_mapper.h" +#include "common/upstream/endpoint_groups_manager.h" + +namespace Envoy { +namespace Upstream { + +class EgdsApi { +public: + virtual ~EgdsApi() = default; + + virtual void initialize(std::function initialization_fail_callback) PURE; + virtual const std::string versionInfo() const PURE; +}; + +using EgdsApiPtr = std::unique_ptr; + +/** + * All SRDS stats. @see stats_macros.h + */ +// clang-format off +#define ALL_EGDS_STATS(COUNTER) \ + COUNTER(config_reload) \ + COUNTER(update_empty) + +// clang-format on + +struct EgdsStats { + ALL_EGDS_STATS(GENERATE_COUNTER_STRUCT) +}; + +class EgdsApiImpl : public Config::SubscriptionCallbacks, + public EgdsApi, + Logger::Loggable { +public: + EgdsApiImpl(EndpointGroupsManager& endpoint_group_mananger, + ProtobufMessage::ValidationVisitor& validation_visitor, + Config::SubscriptionFactory& subscription_factory, Stats::Scope& scope, + const Protobuf::RepeatedPtrField& egds_list); + ~EgdsApiImpl() override = default; + + // EgdsApi + void initialize(std::function initialization_fail_callback) override; + const std::string versionInfo() const override { return system_version_info_; } + + // Config::SubscriptionCallbacks + void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, + const std::string& version_info) override; + void onConfigUpdate( + const Protobuf::RepeatedPtrField& added_resources, + const Protobuf::RepeatedPtrField& removed_resources, + const std::string& version_info) override; + void onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason reason, + const EnvoyException* e) override; + std::string resourceName(const ProtobufWkt::Any& resource) override { + return MessageUtil::anyConvert(resource).name(); + } + +private: + EndpointGroupsManager& endpoint_group_mananger_; + ProtobufMessage::ValidationVisitor& validation_visitor_; + std::unordered_map> subscription_map_; + std::string system_version_info_; + Stats::ScopePtr scope_; + EgdsStats stats_; + std::function initialization_fail_callback_; +}; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/egds_cluster_mapper.cc b/source/common/upstream/egds_cluster_mapper.cc new file mode 100644 index 000000000000..3aa01cb4ea56 --- /dev/null +++ b/source/common/upstream/egds_cluster_mapper.cc @@ -0,0 +1,177 @@ +#include "common/upstream/egds_cluster_mapper.h" + +#include "common/network/resolver_impl.h" + +namespace Envoy { +namespace Upstream { + +EgdsClusterMapper::EgdsClusterMapper( + EndpointGroupMonitorManager& monitor_manager, Delegate& delegate, + BaseDynamicClusterImpl& cluster, + const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment, + const LocalInfo::LocalInfo& local_info) + : monitor_manager_(monitor_manager), delegate_(delegate), cluster_(cluster), + origin_cluster_load_assignment_(cluster_load_assignment), local_info_(local_info) { + for (const auto& config : cluster_load_assignment.endpoint_groups()) { + addResource(config.endpoint_group_name()); + } + + origin_cluster_load_assignment_.clear_endpoints(); + origin_cluster_load_assignment_.clear_named_endpoints(); + origin_cluster_load_assignment_.clear_endpoint_groups(); +} + +std::set EgdsClusterMapper::egds_resource_names() const { + std::set names; + std::transform(active_monitors_.begin(), active_monitors_.end(), + std::inserter(names, names.end()), [](auto pair) { return pair.first; }); + return names; +} + +bool EgdsClusterMapper::resourceExists(absl::string_view name) const { + return active_monitors_.find(name) != active_monitors_.end(); +} + +void EgdsClusterMapper::addResource(absl::string_view name) { + if (active_monitors_.find(name) == active_monitors_.end()) { + active_monitors_.emplace(name, std::make_shared(*this)); + monitor_manager_.addMonitor(active_monitors_[name], name); + } +} + +void EgdsClusterMapper::removeResource(absl::string_view name) { + if (active_monitors_.find(name) != active_monitors_.end()) { + monitor_manager_.removeMonitor(active_monitors_[name], name); + active_monitors_.erase(name); + } +} + +void EgdsClusterMapper::ActiveEndpointGroupMonitor::update( + const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info) { + if (group_.has_value() && Protobuf::util::MessageDifferencer::Equivalent(group_.value(), group)) { + ENVOY_LOG(info, "Resources {} are unchanged and do not need to be updated, version: {} ", + group.name(), version_info); + return; + } + + group_ = group; + version_ = version_info.data(); + + // When the complete data of the cluster is initialized, subsequent updates to each EG are + // incrementally calculated. + if (parent_.cluster_initialized_) { + BatchUpdateHelper helper(*this); + parent_.delegate_.batchHostUpdateForEndpointGroup(helper); + return; + } + + if (parent_.clusterDataIsReady()) { + // Initialize the cluster after all EG data is complete and the initialization is performed only + // once. + parent_.initializeCluster(); + } + + // Initialize the Endpoint Group, using the initial data as the baseline for subsequent + // calculations. Initialization does not trigger "updateHosts" notification. + initializeEndpointGroup(); +} + +void EgdsClusterMapper::ActiveEndpointGroupMonitor::initializeEndpointGroup() { + BatchUpdateHelper helper(*this, false); + EmptyBatchUpdateScope empty_update; + helper.batchUpdate(empty_update); +} + +bool EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper:: + calculateUpdatedHostsPerLocality(const uint32_t priority, const HostVector& new_hosts, + std::unordered_map& updated_hosts, + PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor) { + auto& host_set = parent_.priority_set_.getOrCreateMutableHostSet(priority); + ENVOY_LOG(debug, + "compute the updated host in the '{}' endpoint-group, added hosts count '{}', existed " + "hosts count '{}'", + parent_.group_.value().name(), new_hosts.size(), host_set.mutableHosts().size()); + HostVector hosts_added; + HostVector hosts_removed; + const bool hosts_updated = parent_.parent_.cluster_.updateDynamicHostList( + new_hosts, host_set.mutableHosts(), hosts_added, hosts_removed, updated_hosts, + parent_.all_hosts_); + if (hosts_updated && notify_hosts_updated_) { + parent_.parent_.delegate_.updateHosts(priority, hosts_added, hosts_removed, + priority_state_manager, new_locality_weights_map, + overprovisioning_factor); + } + + return hosts_updated; +} + +void EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper::batchUpdate( + PrioritySet::HostUpdateCb& host_update_cb) { + std::unordered_map updated_hosts; + PriorityStateManager priority_state_manager(parent_.parent_.cluster_, parent_.parent_.local_info_, + &host_update_cb); + for (const auto& locality_lb_endpoint : parent_.group_.value().endpoints()) { + priority_state_manager.initializePriorityFor(locality_lb_endpoint); + + for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { + priority_state_manager.registerHostForPriority( + "", Network::Address::resolveProtoAddress(lb_endpoint.endpoint().address()), + locality_lb_endpoint, lb_endpoint); + } + } + + // Track whether we rebuilt any LB structures. + bool endpoint_group_rebuilt = false; + + const uint32_t overprovisioning_factor = + PROTOBUF_GET_WRAPPED_OR_DEFAULT(parent_.parent_.origin_cluster_load_assignment_.policy(), + overprovisioning_factor, kDefaultOverProvisioningFactor); + + // Loop over all priorities that exist in the new configuration. + auto& priority_state = priority_state_manager.priorityState(); + for (size_t i = 0; i < priority_state.size(); ++i) { + if (priority_state[i].first != nullptr) { + endpoint_group_rebuilt |= calculateUpdatedHostsPerLocality( + i, *priority_state[i].first, updated_hosts, priority_state_manager, + priority_state[i].second, overprovisioning_factor); + } + } + + parent_.all_hosts_ = std::move(updated_hosts); + + if (endpoint_group_rebuilt) { + // TODO(leilei.gll) Add stats. + } + + ENVOY_LOG(debug, "EG resource '{}' completes batch updates", parent_.group_.value().name()); +} + +bool EgdsClusterMapper::clusterDataIsReady() { + for (const auto& pair : active_monitors_) { + if (!pair.second->group_.has_value()) { + return false; + } + } + + return (cluster_initialized_ = true); +} + +void EgdsClusterMapper::initializeCluster() { + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment( + origin_cluster_load_assignment_); + for (const auto& pair : active_monitors_) { + cluster_load_assignment.mutable_endpoints()->MergeFrom(pair.second->group_.value().endpoints()); + cluster_load_assignment.mutable_named_endpoints()->insert( + pair.second->group_.value().named_endpoints().begin(), + pair.second->group_.value().named_endpoints().end()); + } + + ENVOY_LOG(info, "EGDS cluster {} updated, endpoint count {}", + cluster_load_assignment.cluster_name(), cluster_load_assignment.endpoints_size()); + delegate_.initializeCluster(cluster_load_assignment); +} + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/egds_cluster_mapper.h b/source/common/upstream/egds_cluster_mapper.h new file mode 100644 index 000000000000..feb26c36dca6 --- /dev/null +++ b/source/common/upstream/egds_cluster_mapper.h @@ -0,0 +1,111 @@ +#pragma once + +#include +#include + +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/local_info/local_info.h" +#include "envoy/upstream/upstream.h" + +#include "common/common/logger.h" +#include "common/upstream/endpoint_group_monitor.h" +#include "common/upstream/upstream_impl.h" + +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Upstream { + +class EdsClusterImpl; + +class EgdsClusterMapper : Logger::Loggable { +public: + class Delegate { + public: + virtual ~Delegate() {} + + virtual void initializeCluster( + const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment) PURE; + + virtual void updateHosts(uint32_t priority, const HostVector& hosts_added, + const HostVector& hosts_removed, + PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor = absl::nullopt) PURE; + + virtual void batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) PURE; + }; + + EgdsClusterMapper( + EndpointGroupMonitorManager& monitor_manager, Delegate& delegate, + BaseDynamicClusterImpl& cluster, + const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment, + const LocalInfo::LocalInfo& local_info); + + bool resourceExists(absl::string_view name) const; + void addResource(absl::string_view name); + void removeResource(absl::string_view name); + std::set egds_resource_names() const; + +private: + struct ActiveEndpointGroupMonitor : public EndpointGroupMonitor { + ActiveEndpointGroupMonitor(EgdsClusterMapper& parent) : parent_(parent) {} + ~ActiveEndpointGroupMonitor() = default; + + // Upstream::EndpointGroupMonitor + void update(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info); + + void initializeEndpointGroup(); + + class BatchUpdateHelper : public PrioritySet::BatchUpdateCb { + public: + BatchUpdateHelper(ActiveEndpointGroupMonitor& parent, bool notify_hosts_updated = true) + : parent_(parent), notify_hosts_updated_(notify_hosts_updated) {} + + // Upstream::PrioritySet::BatchUpdateCb + void batchUpdate(PrioritySet::HostUpdateCb& host_update_cb) override; + + bool calculateUpdatedHostsPerLocality( + const uint32_t priority, const HostVector& new_hosts, + std::unordered_map& updated_hosts, + PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor); + + private: + ActiveEndpointGroupMonitor& parent_; + const bool notify_hosts_updated_; + }; + + EgdsClusterMapper& parent_; + absl::optional group_; + absl::optional version_; + PrioritySetImpl priority_set_; + HostMap all_hosts_; + }; + using ActiveEndpointGroupMonitorSharedPtr = std::shared_ptr; + + class EmptyBatchUpdateScope : public PrioritySet::HostUpdateCb { + public: + void updateHosts(uint32_t, PrioritySet::UpdateHostsParams&&, LocalityWeightsConstSharedPtr, + const HostVector&, const HostVector&, absl::optional) override {} + }; + + bool clusterDataIsReady(); + void initializeCluster(); + + absl::flat_hash_map active_monitors_; + EndpointGroupMonitorManager& monitor_manager_; + Delegate& delegate_; + BaseDynamicClusterImpl& cluster_; + bool cluster_initialized_{false}; + envoy::config::endpoint::v3::ClusterLoadAssignment origin_cluster_load_assignment_; + const LocalInfo::LocalInfo& local_info_; +}; + +using EgdsClusterMapperPtr = std::unique_ptr; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/endpoint_group.cc b/source/common/upstream/endpoint_group.cc new file mode 100644 index 000000000000..73ed20bcccf7 --- /dev/null +++ b/source/common/upstream/endpoint_group.cc @@ -0,0 +1,16 @@ +#include "common/upstream/endpoint_group.h" + +namespace Envoy { +namespace Upstream { + +bool ActiveEndpointGroup::update(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) { + for (auto& monitor : monitors_) { + monitor->update(group, version_info); + } + + return true; +} + +} // namespace Upstream +} // namespace Envoy \ No newline at end of file diff --git a/source/common/upstream/endpoint_group.h b/source/common/upstream/endpoint_group.h new file mode 100644 index 000000000000..072394b48c80 --- /dev/null +++ b/source/common/upstream/endpoint_group.h @@ -0,0 +1,38 @@ +#pragma once + +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/upstream/upstream.h" + +#include "common/upstream/endpoint_group_monitor.h" + +namespace Envoy { +namespace Upstream { + +class ActiveEndpointGroup { +public: + ActiveEndpointGroup(EndpointGroupMonitorSharedPtr monitor) : monitors_({monitor}) {} + ActiveEndpointGroup() {} + + bool update(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info); + + void addMonitor(EndpointGroupMonitorSharedPtr monitor) { + if (!monitors_.count(monitor)) { + monitors_.insert(monitor); + } + } + void removeMonitor(EndpointGroupMonitorSharedPtr monitor) { + ASSERT(monitors_.count(monitor)); + monitors_.erase(monitor); + } + + bool findMonitor(EndpointGroupMonitorSharedPtr monitor) const { return monitors_.count(monitor); } + +private: + MonitorSet monitors_; +}; + +using ActiveEndpointGroupSharedPtr = std::shared_ptr; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/endpoint_group_monitor.h b/source/common/upstream/endpoint_group_monitor.h new file mode 100644 index 000000000000..97b01f1d6fc3 --- /dev/null +++ b/source/common/upstream/endpoint_group_monitor.h @@ -0,0 +1,34 @@ +#pragma once + +#include +#include + +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/upstream/upstream.h" + +#include "absl/container/flat_hash_set.h" + +namespace Envoy { +namespace Upstream { + +class EndpointGroupMonitor { +public: + virtual ~EndpointGroupMonitor() = default; + + virtual void update(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) PURE; +}; + +using EndpointGroupMonitorSharedPtr = std::shared_ptr; +using MonitorSet = absl::flat_hash_set; + +class EndpointGroupMonitorManager { +public: + virtual ~EndpointGroupMonitorManager() = default; + virtual void addMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) PURE; + virtual void removeMonitor(EndpointGroupMonitorSharedPtr monitor, + absl::string_view group_name) PURE; +}; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/endpoint_groups_manager.h b/source/common/upstream/endpoint_groups_manager.h new file mode 100644 index 000000000000..36b7a363a81c --- /dev/null +++ b/source/common/upstream/endpoint_groups_manager.h @@ -0,0 +1,25 @@ +#pragma once + +#include + +#include "envoy/config/endpoint/v3/endpoint.pb.h" + +#include "absl/strings/string_view.h" + +namespace Envoy { +namespace Upstream { + +class EndpointGroupsManager { +public: + virtual ~EndpointGroupsManager() = default; + + virtual bool addOrUpdateEndpointGroup(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) PURE; + virtual bool removeEndpointGroup(absl::string_view name) PURE; + virtual bool clearEndpointGroup(absl::string_view name, absl::string_view version_info) PURE; +}; + +using EndpointGroupsManagerPtr = std::unique_ptr; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/endpoint_groups_manager_impl.cc b/source/common/upstream/endpoint_groups_manager_impl.cc new file mode 100644 index 000000000000..14c39d0746eb --- /dev/null +++ b/source/common/upstream/endpoint_groups_manager_impl.cc @@ -0,0 +1,54 @@ +#include "common/upstream/endpoint_groups_manager_impl.h" + +namespace Envoy { +namespace Upstream { + +bool EndpointGroupsManagerImpl::addOrUpdateEndpointGroup( + const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info) { + if (active_groups_.find(group.name()) == active_groups_.end()) { + active_groups_.emplace(group.name(), std::make_shared()); + } + + return active_groups_[group.name()]->update(group, version_info); +} + +bool EndpointGroupsManagerImpl::clearEndpointGroup(absl::string_view name, + absl::string_view version_info) { + ASSERT(active_groups_.find(name) != active_groups_.end()); + + // Clear all endpoints with an empty EndpointGroup. + envoy::config::endpoint::v3::EndpointGroup empty_group; + empty_group.set_name(name.data()); + return active_groups_[name]->update(empty_group, version_info); +} + +bool EndpointGroupsManagerImpl::removeEndpointGroup(absl::string_view name) { + ASSERT(active_groups_.find(name) != active_groups_.end()); + return active_groups_.erase(name); +} + +void EndpointGroupsManagerImpl::addMonitor(EndpointGroupMonitorSharedPtr monitor, + absl::string_view group_name) { + if (active_groups_.find(group_name) == active_groups_.end()) { + active_groups_.emplace(group_name, std::make_shared(monitor)); + return; + } + + active_groups_[group_name]->addMonitor(monitor); +} + +void EndpointGroupsManagerImpl::removeMonitor(EndpointGroupMonitorSharedPtr monitor, + absl::string_view group_name) { + if (active_groups_.find(group_name) != active_groups_.end()) { + active_groups_[group_name]->removeMonitor(monitor); + } +} + +bool EndpointGroupsManagerImpl::findMonitor(EndpointGroupMonitorSharedPtr monitor, + absl::string_view group_name) const { + return active_groups_.count(group_name) && + active_groups_.at(group_name.data())->findMonitor(monitor); +} + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/endpoint_groups_manager_impl.h b/source/common/upstream/endpoint_groups_manager_impl.h new file mode 100644 index 000000000000..4f0e64d78475 --- /dev/null +++ b/source/common/upstream/endpoint_groups_manager_impl.h @@ -0,0 +1,35 @@ +#pragma once + +#include + +#include "envoy/config/endpoint/v3/endpoint.pb.h" + +#include "common/upstream/endpoint_group.h" +#include "common/upstream/endpoint_group_monitor.h" +#include "common/upstream/endpoint_groups_manager.h" + +namespace Envoy { +namespace Upstream { + +class EndpointGroupsManagerImpl : public EndpointGroupsManager, public EndpointGroupMonitorManager { +public: + ~EndpointGroupsManagerImpl() override = default; + + // Upstream::EndpointGroupsManager + bool addOrUpdateEndpointGroup(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) override; + bool removeEndpointGroup(absl::string_view name) override; + bool clearEndpointGroup(absl::string_view name, absl::string_view version_info) override; + + // Upstream::EndpointGroupMonitorManager + void addMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) override; + void removeMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) override; + + bool findMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) const; + +private: + absl::flat_hash_map active_groups_; +}; + +} // namespace Upstream +} // namespace Envoy diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 320045334421..a22445f0cb03 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -537,6 +537,22 @@ PrioritySetImpl::getOrCreateHostSet(uint32_t priority, return *host_sets_[priority]; } +HostSetImpl& +PrioritySetImpl::getOrCreateMutableHostSet(uint32_t priority, + absl::optional overprovisioning_factor) { + if (host_sets_.size() < priority + 1) { + for (size_t i = host_sets_.size(); i <= priority; ++i) { + HostSetImplPtr host_set = createHostSet(i, overprovisioning_factor); + host_set->addPriorityUpdateCb([this](uint32_t priority, const HostVector& hosts_added, + const HostVector& hosts_removed) { + runReferenceUpdateCallbacks(priority, hosts_added, hosts_removed); + }); + host_sets_.push_back(std::move(host_set)); + } + } + return dynamic_cast(*host_sets_[priority]); +} + void PrioritySetImpl::updateHosts(uint32_t priority, UpdateHostsParams&& update_hosts_params, LocalityWeightsConstSharedPtr locality_weights, const HostVector& hosts_added, const HostVector& hosts_removed, diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index fb65fc79a71a..0ef31b637c36 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -100,6 +100,10 @@ class HostDescriptionImpl : virtual public HostDescription, metadata_ = std::make_shared(new_metadata); } +#if defined(ALIMESH) + void resetCluster(lusterInfoConstSharedPtr cluster) { cluster_ = cluster; } +#endif + const ClusterInfo& cluster() const override { return *cluster_; } HealthCheckHostMonitor& healthChecker() const override { if (health_checker_) { @@ -355,6 +359,8 @@ class HostSetImpl : public HostSet { const HostVector& hosts_removed, absl::optional overprovisioning_factor = absl::nullopt); + HostVector& mutableHosts() { return *const_cast(hosts_.get()); } + protected: virtual void runUpdateCallbacks(const HostVector& hosts_added, const HostVector& hosts_removed) { member_update_cb_helper_.runCallbacks(priority_, hosts_added, hosts_removed); @@ -445,6 +451,11 @@ class PrioritySetImpl : public PrioritySet { getOrCreateHostSet(uint32_t priority, absl::optional overprovisioning_factor = absl::nullopt); + // Get the host set for this priority level, creating it if necessary. + HostSetImpl& + getOrCreateMutableHostSet(uint32_t priority, + absl::optional overprovisioning_factor = absl::nullopt); + void updateHosts(uint32_t priority, UpdateHostsParams&& update_hosts_params, LocalityWeightsConstSharedPtr locality_weights, const HostVector& hosts_added, const HostVector& hosts_removed, @@ -825,7 +836,7 @@ using PriorityStateManagerPtr = std::unique_ptr; * Base for all dynamic cluster types. */ class BaseDynamicClusterImpl : public ClusterImplBase { -protected: +public: using ClusterImplBase::ClusterImplBase; /** diff --git a/test/common/upstream/BUILD b/test/common/upstream/BUILD index c53b8368e9ed..c2a18de2de50 100644 --- a/test/common/upstream/BUILD +++ b/test/common/upstream/BUILD @@ -511,3 +511,54 @@ envoy_cc_test( "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", ], ) + +envoy_cc_test( + name = "egds_api_impl_test", + srcs = ["egds_api_impl_test.cc"], + deps = [ + ":utility_lib", + "//source/common/config:utility_lib", + "//source/common/protobuf:utility_lib", + "//source/common/upstream:cds_api_lib", + "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", + ], +) + +envoy_cc_test( + name = "endpoint_groups_manager_impl_test", + srcs = ["endpoint_groups_manager_impl_test.cc"], + deps = [ + "//source/common/upstream:endpoint_groups_manager_lib", + "//source/common/upstream:upstream_lib", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + ], +) + +envoy_cc_test( + name = "egds_cluster_mapper_test", + srcs = ["egds_cluster_mapper_test.cc"], + deps = [ + ":utility_lib", + "//source/common/upstream:egds_cluster_mapper_lib", + "//source/common/upstream:endpoint_group_monitor_interface", + "//source/common/upstream:endpoint_groups_manager_lib", + "//source/common/upstream:upstream_lib", + "//source/extensions/transport_sockets/raw_buffer:config", + "//source/server:transport_socket_config_lib", + "//test/mocks/local_info:local_info_mocks", + "//test/mocks/protobuf:protobuf_mocks", + "//test/mocks/runtime:runtime_mocks", + "//test/mocks/server:server_mocks", + "//test/mocks/ssl:ssl_mocks", + "//test/mocks/upstream:upstream_mocks", + "//test/test_common:utility_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", + ], +) diff --git a/test/common/upstream/egds_api_impl_test.cc b/test/common/upstream/egds_api_impl_test.cc new file mode 100644 index 000000000000..aecbf34c580f --- /dev/null +++ b/test/common/upstream/egds_api_impl_test.cc @@ -0,0 +1,266 @@ +#include +#include +#include +#include + +#include "envoy/config/core/v3/config_source.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/config/endpoint/v3/endpoint_components.pb.h" +#include "envoy/service/discovery/v3/discovery.pb.h" + +#include "common/config/utility.h" +#include "common/protobuf/utility.h" +#include "common/upstream/egds_api_impl.h" + +#include "test/common/upstream/utility.h" +#include "test/mocks/protobuf/mocks.h" +#include "test/mocks/upstream/mocks.h" +#include "test/test_common/printers.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +using testing::_; +using testing::InSequence; +using testing::Return; +using testing::StrEq; +using testing::Throw; + +namespace Envoy { +namespace Upstream { +namespace { + +inline envoy::config::endpoint::v3::ClusterLoadAssignment +parseClusterLoadAssignment(const std::string& yaml_config) { + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; + TestUtility::loadFromYaml(yaml_config, cluster_load_assignment); + return cluster_load_assignment; +} + +class EgdsApiImplTest : public testing::Test { +protected: + EgdsApiImplTest() { resetClusterLoadAssignment(); } + void resetClusterLoadAssignment() { + resetClusterLoadAssignment(R"EOF( + cluster_name: cluster_0 + endpoint_groups: + - config_source: + path: egds path + endpoint_group_name: group_name + )EOF"); + } + + void resetMultiGroupClusterLoadAssignment() { + resetClusterLoadAssignment(R"EOF( + cluster_name: cluster_0 + endpoint_groups: + - config_source: + path: egds path + endpoint_group_name: group_name_1 + - config_source: + path: egds path + endpoint_group_name: group_name_2 + )EOF"); + } + + void resetClusterLoadAssignment(const std::string& yaml_config) { + cluster_load_assignment_ = parseClusterLoadAssignment(yaml_config); + auto scope = + stats_.createScope(fmt::format("cluster.{}", cluster_load_assignment_.cluster_name())); + egds_api_ = + std::make_unique(eg_manager_, validation_visitor_, subscription_factory_, + *scope, cluster_load_assignment_.endpoint_groups()); + egds_callbacks_ = subscription_factory_.callbacks_; + } + + void initialize() { + EXPECT_CALL(*subscription_factory_.subscription_, start(_)); + egds_api_->initialize([&] { this->initialization_fail_ = true; }); + } + + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment_; + NiceMock delegate_; + NiceMock egds_monitor_manager_; + NiceMock eg_manager_; + NiceMock egds_monitor_; + NiceMock subscription_factory_; + NiceMock validation_visitor_; + Stats::IsolatedStoreImpl stats_; + EgdsApiPtr egds_api_; + Config::SubscriptionCallbacks* egds_callbacks_{}; + bool initialization_fail_{false}; +}; + +TEST_F(EgdsApiImplTest, OnConfigUpdateEmpty) { + initialize(); + EXPECT_NE(nullptr, egds_callbacks_); + egds_callbacks_->onConfigUpdate({}, ""); + EXPECT_EQ(1UL, stats_.counter("cluster.cluster_0.egds.update_empty").value()); +} + +TEST_F(EgdsApiImplTest, onConfigUpdateWrongName) { + initialize(); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name("wrong name"); + auto* endpoints = endpoint_group.add_endpoints(); + auto* endpoint = endpoints->add_lb_endpoints(); + + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); + + Protobuf::RepeatedPtrField resources; + resources.Add()->PackFrom(endpoint_group); + // TODO(jingyi.lyq) should throw exception when group not be subscribed + // EXPECT_THROW(egds_callbacks_->onConfigUpdate(resources, ""), EnvoyException); +} + +TEST_F(EgdsApiImplTest, onConfigUpdateDuplicateEnpointGroup) { + initialize(); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name("group_name"); + Protobuf::RepeatedPtrField resources; + auto* endpoints = endpoint_group.add_endpoints(); + auto* endpoint = endpoints->add_lb_endpoints(); + + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); + resources.Add()->PackFrom(endpoint_group); + resources.Add()->PackFrom(endpoint_group); + EXPECT_THROW(egds_callbacks_->onConfigUpdate(resources, ""), EnvoyException); +} + +TEST_F(EgdsApiImplTest, onConfigUpdateSuccess) { + initialize(); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name("group_name"); + Protobuf::RepeatedPtrField resources; + auto* endpoints = endpoint_group.add_endpoints(); + auto* endpoint = endpoints->add_lb_endpoints(); + + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); + resources.Add()->PackFrom(endpoint_group); + + EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)); + egds_callbacks_->onConfigUpdate(resources, ""); +} + +TEST_F(EgdsApiImplTest, onConfigUpdateReplace) { + initialize(); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name("group_name"); + Protobuf::RepeatedPtrField resources; + auto* endpoints = endpoint_group.add_endpoints(); + auto* endpoint = endpoints->add_lb_endpoints(); + + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); + resources.Add()->PackFrom(endpoint_group); + egds_callbacks_->onConfigUpdate(resources, ""); + + EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)); + egds_callbacks_->onConfigUpdate(resources, ""); +} + +TEST_F(EgdsApiImplTest, onConfigUpdateRemove) { + initialize(); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name("group_name"); + Protobuf::RepeatedPtrField resources; + auto* endpoints = endpoint_group.add_endpoints(); + auto* endpoint = endpoints->add_lb_endpoints(); + + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("1.2.3.4"); + endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); + resources.Add()->PackFrom(endpoint_group); + egds_callbacks_->onConfigUpdate(resources, ""); + + Protobuf::RepeatedPtrField new_resources; + endpoint_group.clear_endpoints(); + new_resources.Add()->PackFrom(endpoint_group); + egds_callbacks_->onConfigUpdate(resources, ""); + + EXPECT_CALL(eg_manager_, clearEndpointGroup(_, _)); + egds_callbacks_->onConfigUpdate(new_resources, ""); +} + +TEST_F(EgdsApiImplTest, multiConfigSource) { + // Two config sources of the same type. + { + const std::string yaml_config = R"EOF( + cluster_name: cluster_0 + endpoint_groups: + - config_source: + path: egds path + endpoint_group_name: group_name_1 + - config_source: + path: egds path + endpoint_group_name: group_name_2 + )EOF"; + cluster_load_assignment_ = parseClusterLoadAssignment(yaml_config); + auto scope = + stats_.createScope(fmt::format("cluster.{}", cluster_load_assignment_.cluster_name())); + std::unordered_map subscription_map; + ON_CALL(subscription_factory_, subscriptionFromConfigSource(_, _, _, _)) + .WillByDefault(testing::Invoke( + [&](const envoy::config::core::v3::ConfigSource&, absl::string_view, Stats::Scope&, + Config::SubscriptionCallbacks& callbacks) -> Config::SubscriptionPtr { + auto ret = std::make_unique>(); + subscription_map.emplace(ret.get(), &callbacks); + return ret; + })); + egds_api_ = + std::make_unique(eg_manager_, validation_visitor_, subscription_factory_, + *scope, cluster_load_assignment_.endpoint_groups()); + EXPECT_EQ(1, subscription_map.size()); + } + + // Two config sources of different types. + { + const std::string yaml_config = R"EOF( + cluster_name: cluster_0 + endpoint_groups: + - config_source: + path: egds path + endpoint_group_name: group_name_1 + - config_source: + api_config_source: + api_type: DELTA_GRPC + grpc_services: + envoy_grpc: + cluster_name: xds_cluster + endpoint_group_name: group_name_2 + )EOF"; + cluster_load_assignment_ = parseClusterLoadAssignment(yaml_config); + auto scope = + stats_.createScope(fmt::format("cluster.{}", cluster_load_assignment_.cluster_name())); + std::unordered_map subscription_map; + ON_CALL(subscription_factory_, subscriptionFromConfigSource(_, _, _, _)) + .WillByDefault(testing::Invoke( + [&](const envoy::config::core::v3::ConfigSource&, absl::string_view, Stats::Scope&, + Config::SubscriptionCallbacks& callbacks) -> Config::SubscriptionPtr { + auto ret = std::make_unique>(); + subscription_map.emplace(ret.get(), &callbacks); + return ret; + })); + egds_api_ = + std::make_unique(eg_manager_, validation_visitor_, subscription_factory_, + *scope, cluster_load_assignment_.endpoint_groups()); + EXPECT_EQ(2, subscription_map.size()); + + uint8_t i = 1; + for (const auto& pair : subscription_map) { + EXPECT_CALL(*pair.first, start(_)) + .WillOnce(Invoke([&](const std::set resources) -> void { + EXPECT_TRUE(resources.count(fmt::format("group_name_{}", i++))); + EXPECT_EQ(1, resources.size()); + })); + } + egds_api_->initialize([&] { this->initialization_fail_ = true; }); + } +} + +} // namespace +} // namespace Upstream +} // namespace Envoy \ No newline at end of file diff --git a/test/common/upstream/egds_cluster_mapper_test.cc b/test/common/upstream/egds_cluster_mapper_test.cc new file mode 100644 index 000000000000..8c0e69746e48 --- /dev/null +++ b/test/common/upstream/egds_cluster_mapper_test.cc @@ -0,0 +1,223 @@ +#include "envoy/api/v2/eds.pb.h" +#include "envoy/api/v2/egds.pb.h" +#include "envoy/stats/scope.h" + +#include "common/config/utility.h" +#include "common/singleton/manager_impl.h" +#include "common/upstream/eds.h" +#include "common/upstream/egds_cluster_mapper.h" + +#include "server/transport_socket_config_impl.h" + +#include "test/common/upstream/utility.h" +#include "test/mocks/local_info/mocks.h" +#include "test/mocks/protobuf/mocks.h" +#include "test/mocks/runtime/mocks.h" +#include "test/mocks/server/mocks.h" +#include "test/mocks/ssl/mocks.h" +#include "test/mocks/upstream/mocks.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Upstream { +namespace { + +class EgdsClusterMapperTest : public testing::Test { +protected: + EgdsClusterMapperTest() : api_(Api::createApiForTest(stats_)) { resetCluster(); } + + void resetCluster() { + resetCluster(R"EOF( + name: name + connect_timeout: 0.25s + type: EDS + lb_policy: ROUND_ROBIN + eds_cluster_config: + service_name: fare + eds_config: + api_config_source: + api_type: REST + cluster_names: + - eds + refresh_delay: 1s + )EOF", + Cluster::InitializePhase::Secondary); + } + + void resetCluster(const std::string& yaml_config, Cluster::InitializePhase initialize_phase) { + local_info_.node_.mutable_locality()->set_zone("us-east-1a"); + eds_cluster_ = parseClusterFromV2Yaml(yaml_config); + Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( + "cluster.{}.", + eds_cluster_.alt_stat_name().empty() ? eds_cluster_.name() : eds_cluster_.alt_stat_name())); + Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( + admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, random_, stats_, + singleton_manager_, tls_, validation_visitor_, *api_); + cluster_.reset( + new EdsClusterImpl(eds_cluster_, runtime_, factory_context, std::move(scope), false)); + EXPECT_EQ(initialize_phase, cluster_->initializePhase()); + eds_callbacks_ = cm_.subscription_factory_.callbacks_; + } + + Stats::IsolatedStoreImpl stats_; + Ssl::MockContextManager ssl_context_manager_; + envoy::config::cluster::v3::Cluster eds_cluster_; + NiceMock cm_; + NiceMock dispatcher_; + std::shared_ptr cluster_; + Config::SubscriptionCallbacks* eds_callbacks_{}; + NiceMock random_; + NiceMock runtime_; + NiceMock local_info_; + NiceMock admin_; + Singleton::ManagerImpl singleton_manager_{Thread::threadFactoryForTest()}; + NiceMock tls_; + NiceMock validation_visitor_; + Api::ApiPtr api_; +}; + +TEST_F(EgdsClusterMapperTest, Basic) { + MockEndpointGroupMonitorManager mock_manager; + MockEgdsClusterMapperDelegate mock_delegate; + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; + // LocalInfo::MockLocalInfo mock_local_info; + EgdsClusterMapper mapper(mock_manager, mock_delegate, *cluster_, cluster_load_assignment, + local_info_); + + const std::vector egds_resource_names = {"test1", "test2", "test3"}; + EndpointGroupMonitorSharedPtr active_monitor_test1, active_monitor_test2, active_monitor_test3; + + { + EXPECT_FALSE(mapper.resourceExists(egds_resource_names.at(0))); + EXPECT_CALL(mock_manager, addMonitor(_, _)) + .WillOnce(Invoke( + [&](EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) -> void { + active_monitor_test1 = monitor; + EXPECT_STREQ(egds_resource_names.at(0).c_str(), group_name.data()); + })); + mapper.addResource(egds_resource_names.at(0)); + EXPECT_TRUE(mapper.resourceExists(egds_resource_names.at(0))); + } + + { + EXPECT_CALL(mock_manager, addMonitor(_, _)) + .WillOnce(Invoke( + [&](EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) -> void { + active_monitor_test2 = monitor; + EXPECT_STREQ(egds_resource_names.at(1).c_str(), group_name.data()); + })); + mapper.addResource(egds_resource_names.at(1)); + EXPECT_TRUE(mapper.resourceExists(egds_resource_names.at(1))); + } + + { + EXPECT_CALL(mock_manager, addMonitor(_, _)) + .WillOnce(Invoke( + [&](EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) -> void { + active_monitor_test3 = monitor; + EXPECT_STREQ(egds_resource_names.at(2).c_str(), group_name.data()); + })); + mapper.addResource(egds_resource_names.at(2)); + EXPECT_TRUE(mapper.resourceExists(egds_resource_names.at(2))); + } + + auto egds_names = mapper.egds_resource_names(); + for (const auto& name : egds_resource_names) { + EXPECT_TRUE(egds_names.count(name)); + } + + { + // The onUpdate interface is not called because the other two dependent resources are not + // retrieved, test_data_2 and test_data_3 isn't ready. + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_1"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("1.2.3.4"); + socket_address->set_port_value(1234); + active_monitor_test1->update(group_data, version); + } + + { + // The onUpdate interface is not called because the other dependent resources are not + // retrieved, test_data_3 isn't ready. + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_2"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("2.3.4.5"); + socket_address->set_port_value(2345); + active_monitor_test2->update(group_data, version); + } + + { + // The onUpdate interface is called because the dependent resources has been + // retrieved. + EXPECT_CALL(mock_delegate, initializeCluster(_)) + .WillOnce(Invoke( + [&](const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment) + -> void { EXPECT_EQ(3, cluster_load_assignment.endpoints_size()); })); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_3"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("3.4.5.6"); + socket_address->set_port_value(3456); + active_monitor_test3->update(group_data, version); + } + + { + // The onUpdate interface is called because the dependent resources has been + // retrieved. + PrioritySet::BatchUpdateCb* batch_update_callback; + EXPECT_CALL(mock_delegate, batchHostUpdateForEndpointGroup(_)) + .WillOnce(Invoke([&](PrioritySet::BatchUpdateCb& callback) -> void { + batch_update_callback = &callback; + })); + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_1"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("4.5.6.7"); + socket_address->set_port_value(4567); + active_monitor_test1->update(group_data, version); + } + + EXPECT_CALL(mock_manager, removeMonitor(_, _)).Times(1); + mapper.removeResource(egds_resource_names.at(0)); + EXPECT_FALSE(mapper.resourceExists(egds_resource_names.at(0))); + + EXPECT_CALL(mock_manager, removeMonitor(_, _)).Times(1); + mapper.removeResource(egds_resource_names.at(2)); + EXPECT_FALSE(mapper.resourceExists(egds_resource_names.at(2))); + + // Adding duplicate resources. + EXPECT_CALL(mock_manager, addMonitor(_, _)).Times(0); + mapper.addResource(egds_resource_names.at(1)); +} + +} // namespace +} // namespace Upstream +} // namespace Envoy diff --git a/test/common/upstream/endpoint_groups_manager_impl_test.cc b/test/common/upstream/endpoint_groups_manager_impl_test.cc new file mode 100644 index 000000000000..6b43534490fe --- /dev/null +++ b/test/common/upstream/endpoint_groups_manager_impl_test.cc @@ -0,0 +1,118 @@ +#include "envoy/config/endpoint/v3/endpoint.pb.h" + +#include "common/upstream/endpoint_groups_manager_impl.h" + +#include "test/mocks/upstream/mocks.h" +#include "test/test_common/utility.h" + +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace Envoy { +namespace Upstream { +namespace { + +TEST(EndpointGroupsManagerImplTest, Basic) { + EndpointGroupsManagerImpl manager; + + auto monitor = std::make_shared(); + const std::string group_name("test1"); + EXPECT_FALSE(manager.findMonitor(monitor, group_name)); + manager.addMonitor(monitor, group_name); + EXPECT_TRUE(manager.findMonitor(monitor, group_name)); + + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name(group_name); + const std::string version("1.0"); + EXPECT_CALL(*monitor, update(_, _)) + .WillOnce(Invoke([&](const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) -> void { + EXPECT_EQ(group_data.name(), group.name()); + EXPECT_STREQ(version.c_str(), version_info.data()); + })) + .WillOnce(Invoke([&](const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info) -> void { + EXPECT_EQ(group_data.name(), group.name()); + EXPECT_TRUE(group.endpoints().empty()); + EXPECT_STREQ(version.c_str(), version_info.data()); + })); + manager.addOrUpdateEndpointGroup(group_data, version); + + // Clear the endpoint group resource. + manager.clearEndpointGroup(group_name, version); + // The monitor does not deleted. + EXPECT_TRUE(manager.findMonitor(monitor, group_name)); + + // Remove the endpoint group resource. + manager.removeEndpointGroup(group_name); + // The monitor does not exist because the resource has been deleted. + EXPECT_FALSE(manager.findMonitor(monitor, group_name)); +} + +TEST(EndpointGroupsManagerImplTest, MultiEndpointGroupResource) { + EndpointGroupsManagerImpl manager; + + const std::string test1_group("test1"); + auto test1_monitor = std::make_shared(); + EXPECT_FALSE(manager.findMonitor(test1_monitor, test1_group)); + manager.addMonitor(test1_monitor, test1_group); + EXPECT_TRUE(manager.findMonitor(test1_monitor, test1_group)); + + const std::string test2_group("test2"); + auto test2_monitor = std::make_shared(); + EXPECT_FALSE(manager.findMonitor(test2_monitor, test2_group)); + manager.addMonitor(test2_monitor, test2_group); + EXPECT_TRUE(manager.findMonitor(test2_monitor, test2_group)); + + const std::string test3_group("test3"); + auto test3_monitor = std::make_shared(); + EXPECT_FALSE(manager.findMonitor(test3_monitor, test3_group)); + manager.addMonitor(test3_monitor, test3_group); + EXPECT_TRUE(manager.findMonitor(test3_monitor, test3_group)); + + // Listen on resource test2. + auto test_monitor = std::make_shared(); + manager.addMonitor(test_monitor, test2_group); + EXPECT_TRUE(manager.findMonitor(test_monitor, test2_group)); + + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup test_group_data; + test_group_data.set_name(test1_group); + EXPECT_CALL(*test1_monitor, update(_, _)).Times(1); + EXPECT_CALL(*test2_monitor, update(_, _)).Times(0); + EXPECT_CALL(*test3_monitor, update(_, _)).Times(0); + manager.addOrUpdateEndpointGroup(test_group_data, version); + + test_group_data.set_name(test2_group); + EXPECT_CALL(*test2_monitor, update(_, _)).Times(1); + EXPECT_CALL(*test_monitor, update(_, _)).Times(1); + EXPECT_CALL(*test1_monitor, update(_, _)).Times(0); + EXPECT_CALL(*test3_monitor, update(_, _)).Times(0); + manager.addOrUpdateEndpointGroup(test_group_data, version); + + // Remove the test3_monitor. + test_group_data.set_name(test3_group); + manager.removeMonitor(test3_monitor, test3_group); + EXPECT_CALL(*test3_monitor, update(_, _)).Times(0); + manager.addOrUpdateEndpointGroup(test_group_data, version); + + // Remove the test2_monitor. + test_group_data.set_name(test2_group); + manager.removeMonitor(test2_monitor, test2_group); + EXPECT_CALL(*test2_monitor, update(_, _)).Times(0); + EXPECT_CALL(*test_monitor, update(_, _)).Times(1); + manager.addOrUpdateEndpointGroup(test_group_data, version); + + // Remove the endpoint group resource. + manager.removeEndpointGroup(test1_group); + // The monitor does not exist because the resource has been deleted. + EXPECT_FALSE(manager.findMonitor(test1_monitor, test1_group)); + + manager.removeEndpointGroup(test2_group); + EXPECT_FALSE(manager.findMonitor(test2_monitor, test2_group)); + EXPECT_FALSE(manager.findMonitor(test3_monitor, test3_group)); +} + +} // namespace +} // namespace Upstream +} // namespace Envoy diff --git a/test/config/utility.cc b/test/config/utility.cc index 4c9a0f2b89a6..eea308806af1 100644 --- a/test/config/utility.cc +++ b/test/config/utility.cc @@ -893,4 +893,39 @@ void EdsHelper::setEdsAndWait( update_successes_ == server_stats.counter("cluster.cluster_0.update_success")->value(), ""); } +EgdsHelper::EgdsHelper() + : egds_path_(TestEnvironment::writeStringToFileForTest("egds.pb_text", "")) { + // cluster.cluster_0.update_success will be incremented on the initial + // load when Envoy comes up. + ++update_successes_; +} + +void EgdsHelper::setEgds( + const std::vector& endpoint_groups) { + // Write to file the DiscoveryResponse and trigger inotify watch. + envoy::service::discovery::v3::DiscoveryResponse egds_response; + egds_response.set_version_info(std::to_string(egds_version_++)); + egds_response.set_type_url(Config::TypeUrl::get().EndpointGroup); + for (const auto& group : endpoint_groups) { + egds_response.add_resources()->PackFrom(group); + } + // Past the initial write, need move semantics to trigger inotify move event that the + // FilesystemSubscriptionImpl is subscribed to. + std::string path = + TestEnvironment::writeStringToFileForTest("egds.update.pb_text", egds_response.DebugString()); + TestEnvironment::renameFile(path, egds_path_); +} + +void EgdsHelper::setEgdsAndWait( + const std::vector& endpoint_groups, + IntegrationTestServerStats& server_stats) { + setEgds(endpoint_groups); + // Make sure Envoy has consumed the update now that it is running. + ++update_successes_; + server_stats.waitForCounterGe("cluster.cluster_0.egds.update_success", update_successes_); + RELEASE_ASSERT(update_successes_ == + server_stats.counter("cluster.cluster_0.egds.update_success")->value(), + ""); +} + } // namespace Envoy diff --git a/test/config/utility.h b/test/config/utility.h index e47601919866..f7c7b8746bfa 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -249,4 +249,22 @@ class EdsHelper { uint32_t update_successes_{}; }; +// Common code for tests that deliver EGDS update via the filesystem. +class EgdsHelper { +public: + EgdsHelper(); + + // Set EDS contents on filesystem and wait for Envoy to pick this up. + void setEgds(const std::vector& endpoint_groups); + void + setEgdsAndWait(const std::vector& endpoint_groups, + IntegrationTestServerStats& server_stats); + const std::string& egds_path() const { return egds_path_; } + +private: + const std::string egds_path_; + uint32_t egds_version_{}; + uint32_t update_successes_{}; +}; + } // namespace Envoy diff --git a/test/integration/BUILD b/test/integration/BUILD index fa47c2657914..32b2f12e3ee7 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1029,3 +1029,22 @@ envoy_cc_test( "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", ], ) + +envoy_cc_test( + name = "egds_integration_test", + srcs = ["egds_integration_test.cc"], + deps = [ + ":http_integration_lib", + "//source/common/runtime:runtime_lib", + "//source/common/upstream:load_balancer_lib", + "//test/config:utility_lib", + "//test/integration/filters:eds_ready_filter_config_lib", + "//test/test_common:network_utility_lib", + "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto", + "@envoy_api//envoy/config/cluster/v3:pkg_cc_proto", + "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", + "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto", + "@envoy_api//envoy/type/v3:pkg_cc_proto", + ], +) diff --git a/test/integration/egds_integration_test.cc b/test/integration/egds_integration_test.cc new file mode 100644 index 000000000000..703ad9259b62 --- /dev/null +++ b/test/integration/egds_integration_test.cc @@ -0,0 +1,418 @@ +#include + +#include "envoy/config/bootstrap/v3/bootstrap.pb.h" +#include "envoy/config/cluster/v3/cluster.pb.h" +#include "envoy/config/core/v3/health_check.pb.h" +#include "envoy/config/endpoint/v3/endpoint.pb.h" +#include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.pb.h" +#include "envoy/type/v3/http.pb.h" + +#include "common/runtime/runtime_impl.h" +#include "common/upstream/load_balancer_impl.h" + +#include "test/config/utility.h" +#include "test/integration/http_integration.h" +#include "test/test_common/network_utility.h" + +#include "gtest/gtest.h" + +namespace Envoy { +namespace { + +// Integration test for EGDS features. EGDS is consumed via filesystem +// subscription. +class EgdsIntegrationTest : public testing::TestWithParam, + public HttpIntegrationTest { +public: + EgdsIntegrationTest() + : HttpIntegrationTest(Http::CodecClient::Type::HTTP1, GetParam()), + codec_client_type_(envoy::type::v3::HTTP1), + endpoint_group_names_({"group1", "group2", "group3", "group4"}) {} + + void addShardEgds(absl::string_view group_name, uint32_t total_endpoints, + uint32_t healthy_endpoints, uint32_t degraded_endpoints, + uint32_t& current_upstream_index, + std::vector& output_endpoint_groups, + bool remaining_unhealthy = true) { + ASSERT(total_endpoints >= healthy_endpoints + degraded_endpoints); + envoy::config::endpoint::v3::EndpointGroup endpoint_group; + endpoint_group.set_name(group_name.data()); + auto* locality_lb_endpoints = endpoint_group.add_endpoints(); + for (uint32_t i = 0; i < total_endpoints; ++i) { + ASSERT(current_upstream_index <= 4); + auto* endpoint = locality_lb_endpoints->add_lb_endpoints(); + setUpstreamAddress(current_upstream_index++, *endpoint); + // First N endpoints are degraded, next M are healthy and the remaining endpoints are + // unhealthy or unknown depending on remaining_unhealthy. + if (i < degraded_endpoints) { + endpoint->set_health_status(::envoy::config::core::v3::HealthStatus::DEGRADED); + } else if (i >= healthy_endpoints + degraded_endpoints) { + endpoint->set_health_status(remaining_unhealthy + ? ::envoy::config::core::v3::HealthStatus::UNHEALTHY + : ::envoy::config::core::v3::HealthStatus::UNKNOWN); + } + } + + output_endpoint_groups.emplace_back(std::move(endpoint_group)); + } + + void + addEmptyShardEgds(std::vector& output_endpoint_groups, + bool remaining_unhealthy = true) { + uint32_t current_upstream_index = 0; + for (const auto& name : endpoint_group_names_) { + addShardEgds(name, 0, 0, 0, current_upstream_index, output_endpoint_groups, + remaining_unhealthy); + } + } + + inline bool canShard(uint32_t total_endpoints, uint32_t group_count) { + return total_endpoints >= group_count; + } + + void setEndpoints(uint32_t total_endpoints, uint32_t healthy_endpoints, + uint32_t degraded_endpoints, bool remaining_unhealthy = true, + bool await_update = true) { + std::vector endpoint_groups; + uint32_t current_upstream_index = 0; + const uint32_t group_names_size = endpoint_group_names_.size(); + if (0 == total_endpoints) { + ASSERT(0 == healthy_endpoints && 0 == degraded_endpoints); + addEmptyShardEgds(endpoint_groups, remaining_unhealthy); + } else { + if (canShard(total_endpoints, group_names_size)) { + uint32_t shard_total_endpoints = total_endpoints / group_names_size; + uint32_t shard_healthy_endpoints = healthy_endpoints / group_names_size; + uint32_t shard_degraded_endpoints = degraded_endpoints / group_names_size; + + for (uint32_t i = 0; i < group_names_size; i++) { + // Place the remaining endpoints into the last shard. + if (i == group_names_size - 1) { + shard_total_endpoints = total_endpoints - shard_total_endpoints * i; + shard_healthy_endpoints = healthy_endpoints - shard_healthy_endpoints * i; + shard_degraded_endpoints = degraded_endpoints - shard_degraded_endpoints * i; + } + addShardEgds(endpoint_group_names_.at(i), shard_total_endpoints, shard_healthy_endpoints, + shard_degraded_endpoints, current_upstream_index, endpoint_groups, + remaining_unhealthy); + } + } else { + ENVOY_LOG(info, "the number of endpoints is less than the number of groups, and only the " + "first group one is set"); + addShardEgds(endpoint_group_names_.at(0), total_endpoints, healthy_endpoints, + degraded_endpoints, current_upstream_index, endpoint_groups, + remaining_unhealthy); + } + } + + if (await_update) { + egds_helper_.setEgdsAndWait(endpoint_groups, *test_server_); + } else { + egds_helper_.setEgds(endpoint_groups); + } + } + + void setEds(absl::optional overprovisioning_factor = absl::nullopt, + bool await_update = true) { + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; + cluster_load_assignment.set_cluster_name("cluster_0"); + if (overprovisioning_factor.has_value()) { + cluster_load_assignment.mutable_policy()->mutable_overprovisioning_factor()->set_value( + overprovisioning_factor.value()); + } + + for (const auto& name : endpoint_group_names_) { + auto* group = cluster_load_assignment.add_endpoint_groups(); + group->set_endpoint_group_name(name); + group->mutable_config_source()->set_path(egds_helper_.egds_path()); + } + + if (await_update) { + eds_helper_.setEdsAndWait({cluster_load_assignment}, *test_server_); + } else { + eds_helper_.setEds({cluster_load_assignment}); + } + } + + void initializeTest(bool http_active_hc) { + setUpstreamCount(4); + if (codec_client_type_ == envoy::type::v3::HTTP2) { + setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + } + config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + // Switch predefined cluster_0 to CDS filesystem sourcing. + bootstrap.mutable_dynamic_resources()->mutable_cds_config()->set_path(cds_helper_.cds_path()); + bootstrap.mutable_static_resources()->clear_clusters(); + }); + + // Set validate_clusters to false to allow us to reference a CDS cluster. + config_helper_.addConfigModifier( + [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager& + hcm) { hcm.mutable_route_config()->mutable_validate_clusters()->set_value(false); }); + + cluster_.mutable_connect_timeout()->CopyFrom( + Protobuf::util::TimeUtil::MillisecondsToDuration(100)); + cluster_.set_name("cluster_0"); + // cluster_.mutable_hosts()->Clear(); + cluster_.set_type(envoy::config::cluster::v3::Cluster::EDS); + auto* eds_cluster_config = cluster_.mutable_eds_cluster_config(); + eds_cluster_config->mutable_eds_config()->set_path(eds_helper_.eds_path()); + if (http_active_hc) { + auto* health_check = cluster_.add_health_checks(); + health_check->mutable_timeout()->set_seconds(30); + // TODO(mattklein123): Consider using simulated time here. + health_check->mutable_interval()->CopyFrom( + Protobuf::util::TimeUtil::MillisecondsToDuration(100)); + health_check->mutable_no_traffic_interval()->CopyFrom( + Protobuf::util::TimeUtil::MillisecondsToDuration(100)); + health_check->mutable_unhealthy_threshold()->set_value(1); + health_check->mutable_healthy_threshold()->set_value(1); + health_check->mutable_http_health_check()->set_path("/healthcheck"); + health_check->mutable_http_health_check()->set_codec_client_type(codec_client_type_); + } + + setEndpoints(0, 0, 0, false, false); + setEds(absl::nullopt, false); + cds_helper_.setCds({cluster_}); + initialize(); + + test_server_->waitForGaugeEq("cluster_manager.warming_clusters", 0); + } + + envoy::type::v3::CodecClientType codec_client_type_{}; + std::vector endpoint_group_names_; + EdsHelper eds_helper_; + CdsHelper cds_helper_; + EgdsHelper egds_helper_; + envoy::config::cluster::v3::Cluster cluster_; +}; + +INSTANTIATE_TEST_SUITE_P(IpVersions, EgdsIntegrationTest, + testing::ValuesIn(TestEnvironment::getIpVersionsForTest())); + +// Verifies that a new cluster can we warmed when using HTTP/2 health checking. Regression test +// of the issue detailed in issue #6951. +TEST_P(EgdsIntegrationTest, Http2HcClusterRewarming) { + codec_client_type_ = envoy::type::v3::HTTP2; + initializeTest(true); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + fake_upstreams_[1]->set_allow_unexpected_disconnects(true); + fake_upstreams_[2]->set_allow_unexpected_disconnects(true); + fake_upstreams_[3]->set_allow_unexpected_disconnects(true); + uint32_t total_endpoints = 4; + setEndpoints(total_endpoints, 0, 0, false); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + + // Wait for the first HC and verify the host is healthy. This should warm the initial cluster. + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); + test_server_->waitForGaugeEq("cluster.cluster_0.membership_healthy", 1); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + + // Trigger a CDS update. This should cause a new cluster to require warming, blocked on the host + // being health checked. + cluster_.mutable_circuit_breakers()->add_thresholds()->mutable_max_connections()->set_value(100); + cds_helper_.setCds({cluster_}); + test_server_->waitForGaugeEq("cluster_manager.warming_clusters", 1); + EXPECT_EQ(1, test_server_->gauge("cluster_manager.warming_clusters")->value()); + + // We need to do a bunch of work to get a hold of second hc connection. + FakeHttpConnectionPtr fake_upstream_connection; + auto result = fake_upstreams_[0]->waitForHttpConnection( + *dispatcher_, fake_upstream_connection, TestUtility::DefaultTimeout, max_request_headers_kb_); + RELEASE_ASSERT(result, result.message()); + + FakeStreamPtr upstream_request; + result = fake_upstream_connection->waitForNewStream(*dispatcher_, upstream_request); + RELEASE_ASSERT(result, result.message()); + // Wait for the stream to be completely received. + result = upstream_request->waitForEndStream(*dispatcher_); + RELEASE_ASSERT(result, result.message()); + + // Respond with a health check. This will cause the previous cluster to be destroyed inline as + // part of processing the response. + upstream_request->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, true); + test_server_->waitForGaugeEq("cluster_manager.warming_clusters", 0); + EXPECT_EQ(0, test_server_->gauge("cluster_manager.warming_clusters")->value()); + + // Since the second connection is not managed by the integration test base we need to close it + // ourselves. + result = fake_upstream_connection->close(); + RELEASE_ASSERT(result, result.message()); + result = fake_upstream_connection->waitForDisconnect(); + RELEASE_ASSERT(result, result.message()); + fake_upstream_connection.reset(); +} + +// Verify that a host stabilized via active health checking which is first removed from EGDS and +// then fails health checking is removed. +TEST_P(EgdsIntegrationTest, RemoveAfterHcFail) { + initializeTest(true); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + fake_upstreams_[1]->set_allow_unexpected_disconnects(true); + fake_upstreams_[2]->set_allow_unexpected_disconnects(true); + fake_upstreams_[3]->set_allow_unexpected_disconnects(true); + uint32_t total_endpoints = 4; + setEndpoints(total_endpoints, 0, 0, false); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + + // Wait for the first HC and verify the host is healthy. + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); + test_server_->waitForGaugeEq("cluster.cluster_0.membership_healthy", 1); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + + // Clear out the host and verify the host is still healthy. + setEndpoints(0, 0, 0); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + + // Fail HC and verify the host is gone. + waitForNextUpstreamRequest(); + upstream_request_->encodeHeaders( + Http::TestHeaderMapImpl{{":status", "503"}, {"connection", "close"}}, true); + test_server_->waitForGaugeEq("cluster.cluster_0.membership_healthy", 0); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_total")->value()); +} + +// Verifies that endpoints are ignored until health checked when configured to. +TEST_P(EgdsIntegrationTest, EndpointWarmingSuccessfulHc) { + cluster_.mutable_common_lb_config()->set_ignore_new_hosts_until_first_hc(true); + + // Endpoints are initially excluded. + initializeTest(true); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + fake_upstreams_[1]->set_allow_unexpected_disconnects(true); + fake_upstreams_[2]->set_allow_unexpected_disconnects(true); + fake_upstreams_[3]->set_allow_unexpected_disconnects(true); + uint32_t total_endpoints = 4; + setEndpoints(total_endpoints, 0, 0, false); + + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_excluded")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + + // Wait for the first HC and verify the host is healthy and that it is no longer being excluded. + // The other endpoint should still be excluded. + waitForNextUpstreamRequest(0); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, true); + test_server_->waitForGaugeEq("cluster.cluster_0.membership_excluded", 0); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); +} + +// Verifies that endpoints are ignored until health checked when configured to when the first +// health check fails. +TEST_P(EgdsIntegrationTest, EndpointWarmingFailedHc) { + cluster_.mutable_common_lb_config()->set_ignore_new_hosts_until_first_hc(true); + + // Endpoints are initially excluded. + initializeTest(true); + fake_upstreams_[0]->set_allow_unexpected_disconnects(true); + fake_upstreams_[1]->set_allow_unexpected_disconnects(true); + fake_upstreams_[2]->set_allow_unexpected_disconnects(true); + fake_upstreams_[3]->set_allow_unexpected_disconnects(true); + uint32_t total_endpoints = 4; + setEndpoints(total_endpoints, 0, 0, false); + + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_excluded")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + + // Wait for the first HC and verify the host is healthy and that it is no longer being excluded. + // The other endpoint should still be excluded. + waitForNextUpstreamRequest(0); + upstream_request_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "503"}}, true); + test_server_->waitForGaugeEq("cluster.cluster_0.membership_excluded", 0); + EXPECT_EQ(total_endpoints, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); +} + +// Validate that health status updates are consumed from EGDS. +TEST_P(EgdsIntegrationTest, HealthUpdate) { + initializeTest(false); + // Initial state, no cluster members. + EXPECT_EQ(0, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + // 2/2 healthy endpoints. + setEndpoints(2, 2, 0); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + // Drop to 0/2 healthy endpoints. + setEndpoints(2, 0, 0); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + // Increase to 1/2 healthy endpoints. + setEndpoints(2, 1, 0); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + // Add host and modify health to 2/3 healthy endpoints. + setEndpoints(3, 2, 0); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(3, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + // Modify health to 2/3 healthy and 1/3 degraded. + setEndpoints(3, 2, 1); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_0.membership_change")->value()); + EXPECT_EQ(3, test_server_->gauge("cluster.cluster_0.membership_total")->value()); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_0.membership_degraded")->value()); +} + +// Validate that overprovisioning_factor update are picked up by Envoy. +TEST_P(EgdsIntegrationTest, OverprovisioningFactorUpdate) { + initializeTest(false); + // Default overprovisioning factor. + setEndpoints(4, 4, 0); + auto get_and_compare = [this](const uint32_t expected_factor) { + const auto& cluster_map = test_server_->server().clusterManager().clusters(); + EXPECT_EQ(1, cluster_map.size()); + EXPECT_EQ(1, cluster_map.count("cluster_0")); + const auto& cluster_ref = cluster_map.find("cluster_0")->second; + const auto& hostset_per_priority = cluster_ref.get().prioritySet().hostSetsPerPriority(); + EXPECT_EQ(1, hostset_per_priority.size()); + const Envoy::Upstream::HostSetPtr& host_set = hostset_per_priority[0]; + EXPECT_EQ(expected_factor, host_set->overprovisioningFactor()); + }; + get_and_compare(Envoy::Upstream::kDefaultOverProvisioningFactor); + + // Use new overprovisioning factor 200. + setEndpoints(4, 4, 0); + setEds(200); + get_and_compare(200); +} + +TEST_P(EgdsIntegrationTest, StatsReadyFilter) { + config_helper_.addFilter("name: eds-ready-filter"); + initializeTest(false); + + // Initial state: no healthy endpoints + EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest( + lookupPort("http"), "GET", "/cluster1", "", downstream_protocol_, version_, "foo.com"); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("500", response->headers().Status()->value().getStringView()); + EXPECT_EQ("EDS not ready", response->body()); + + cleanupUpstreamAndDownstream(); + + // 2/2 healthy endpoints. + setEndpoints(2, 2, 0); + EXPECT_EQ(2, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); + response = IntegrationUtil::makeSingleRequest(lookupPort("http"), "GET", "/cluster1", "", + downstream_protocol_, version_, "foo.com"); + ASSERT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); + EXPECT_EQ("EDS is ready", response->body()); + + cleanupUpstreamAndDownstream(); +} + +} // namespace +} // namespace Envoy diff --git a/test/mocks/upstream/BUILD b/test/mocks/upstream/BUILD index 6d3be18e174b..ab922f60963e 100644 --- a/test/mocks/upstream/BUILD +++ b/test/mocks/upstream/BUILD @@ -77,6 +77,9 @@ envoy_cc_mock( "//include/envoy/upstream:load_balancer_interface", "//include/envoy/upstream:upstream_interface", "//source/common/upstream:cluster_factory_lib", + "//source/common/upstream:egds_cluster_mapper_lib", + "//source/common/upstream:endpoint_group_monitor_interface", + "//source/common/upstream:endpoint_groups_manager_interface", "//source/common/upstream:health_discovery_service_lib", "//source/common/upstream:upstream_lib", "//test/mocks/config:config_mocks", diff --git a/test/mocks/upstream/mocks.cc b/test/mocks/upstream/mocks.cc index e5843f0bc64b..12370188d269 100644 --- a/test/mocks/upstream/mocks.cc +++ b/test/mocks/upstream/mocks.cc @@ -177,5 +177,11 @@ MockRetryHostPredicate::~MockRetryHostPredicate() = default; MockClusterManagerFactory::MockClusterManagerFactory() = default; MockClusterManagerFactory::~MockClusterManagerFactory() = default; +MockEndpointGroupsManager::MockEndpointGroupsManager() = default; +MockEndpointGroupsManager::~MockEndpointGroupsManager() = default; + +MockEndpointGroupMonitor::MockEndpointGroupMonitor() = default; +MockEndpointGroupMonitor::~MockEndpointGroupMonitor() = default; + } // namespace Upstream } // namespace Envoy diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 9a8ca01b00f8..12325447c9f2 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -17,6 +17,9 @@ #include "envoy/upstream/upstream.h" #include "common/common/callback_impl.h" +#include "common/upstream/egds_cluster_mapper.h" +#include "common/upstream/endpoint_group_monitor.h" +#include "common/upstream/endpoint_groups_manager.h" #include "common/upstream/health_discovery_service.h" #include "common/upstream/load_balancer_impl.h" #include "common/upstream/upstream_impl.h" @@ -428,5 +431,44 @@ class TestRetryHostPredicateFactory : public RetryHostPredicateFactory { return ProtobufTypes::MessagePtr{new Envoy::ProtobufWkt::Struct()}; } }; + +class MockEndpointGroupMonitor : public EndpointGroupMonitor { +public: + MockEndpointGroupMonitor(); + ~MockEndpointGroupMonitor() override; + + MOCK_METHOD2(update, void(const envoy::config::endpoint::v3::EndpointGroup&, absl::string_view)); +}; + +class MockEndpointGroupsManager : public EndpointGroupsManager { +public: + MockEndpointGroupsManager(); + ~MockEndpointGroupsManager() override; + + MOCK_METHOD2(addOrUpdateEndpointGroup, + bool(const envoy::config::endpoint::v3::EndpointGroup&, absl::string_view)); + MOCK_METHOD2(clearEndpointGroup, bool(absl::string_view, absl::string_view)); + MOCK_METHOD1(removeEndpointGroup, bool(absl::string_view)); +}; + +class MockEgdsClusterMapperDelegate : public EgdsClusterMapper::Delegate { +public: + ~MockEgdsClusterMapperDelegate() override = default; + + MOCK_METHOD1(initializeCluster, void(const envoy::config::endpoint::v3::ClusterLoadAssignment&)); + MOCK_METHOD1(batchHostUpdateForEndpointGroup, void(PrioritySet::BatchUpdateCb&)); + MOCK_METHOD6(updateHosts, + void(uint32_t priority, const HostVector&, const HostVector&, PriorityStateManager&, + LocalityWeightsMap&, absl::optional)); +}; + +class MockEndpointGroupMonitorManager : public EndpointGroupMonitorManager { +public: + ~MockEndpointGroupMonitorManager() override = default; + + MOCK_METHOD2(addMonitor, void(EndpointGroupMonitorSharedPtr, absl::string_view)); + MOCK_METHOD2(removeMonitor, void(EndpointGroupMonitorSharedPtr, absl::string_view)); +}; + } // namespace Upstream } // namespace Envoy From b309be2468203b556d680f4f42caecae74c85cf5 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Thu, 13 Feb 2020 11:58:04 +0800 Subject: [PATCH 02/11] Add missing dependencies Signed-off-by: leilei.gll --- api/envoy/config/endpoint/v3/endpoint.proto | 1 + 1 file changed, 1 insertion(+) diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index 71882455525a..ce52089c9b37 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -2,6 +2,7 @@ syntax = "proto3"; package envoy.config.endpoint.v3; +import "envoy/config/core/v3/config_source.proto"; import "envoy/config/endpoint/v3/endpoint_components.proto"; import "envoy/type/v3/percent.proto"; From 2d9446ca8b46bd809a1f3e9f9867c50a68a0e06f Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Thu, 13 Feb 2020 14:38:31 +0800 Subject: [PATCH 03/11] Fix formatting error Signed-off-by: leilei.gll --- api/envoy/config/endpoint/v3/endpoint.proto | 1 - 1 file changed, 1 deletion(-) diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index ce52089c9b37..0bfed7d5f250 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -125,7 +125,6 @@ message ClusterLoadAssignment { Policy policy = 4; // References endpoint groups by name. - // Both endpoints and EGDS references will be used when present repeated Egds endpoint_groups = 6; } From dfdfa1c57f9317166f4d90f74626a198a88059dc Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Thu, 13 Feb 2020 16:52:31 +0800 Subject: [PATCH 04/11] Fix formatting error Signed-off-by: leilei.gll --- api/envoy/api/v2/endpoint.proto | 7 +++---- api/envoy/config/endpoint/v3/endpoint.proto | 6 +++--- generated_api_shadow/envoy/api/v2/endpoint.proto | 7 +++---- .../envoy/config/endpoint/v3/endpoint.proto | 7 +++---- source/common/upstream/eds.cc | 2 +- source/common/upstream/egds_cluster_mapper.cc | 2 +- test/config/utility.h | 2 +- test/integration/egds_integration_test.cc | 6 +++--- 8 files changed, 18 insertions(+), 21 deletions(-) diff --git a/api/envoy/api/v2/endpoint.proto b/api/envoy/api/v2/endpoint.proto index 3ae53b168596..7fa2c808e975 100644 --- a/api/envoy/api/v2/endpoint.proto +++ b/api/envoy/api/v2/endpoint.proto @@ -117,12 +117,11 @@ message ClusterLoadAssignment { Policy policy = 4; // References endpoint groups by name. - // Both endpoints and EGDS references will be used when present repeated Egds endpoint_groups = 6; } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the EGDS requests to Pilot can be merged into one to boost efficiency +// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; @@ -134,9 +133,9 @@ message EndpointGroup { map named_endpoints = 3; } -// The EGDS request configuration. +// The EndpointGroup request configuration. message Egds { - // Configuration for the source of EGDS updates for this Cluster. + // Configuration for the source of EndpointGroup updates for this Cluster. core.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index 0bfed7d5f250..90a8218d710c 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -129,7 +129,7 @@ message ClusterLoadAssignment { } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the EGDS requests to Pilot can be merged into one to boost efficiency +// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; @@ -143,12 +143,12 @@ message EndpointGroup { map named_endpoints = 3; } -// The EGDS request configuration. +// The EndpointGroup request configuration. message Egds { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; - // Configuration for the source of EGDS updates for this Cluster. + // Configuration for the source of EndpointGroup updates for this Cluster. core.v3.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/generated_api_shadow/envoy/api/v2/endpoint.proto b/generated_api_shadow/envoy/api/v2/endpoint.proto index 3ae53b168596..7fa2c808e975 100644 --- a/generated_api_shadow/envoy/api/v2/endpoint.proto +++ b/generated_api_shadow/envoy/api/v2/endpoint.proto @@ -117,12 +117,11 @@ message ClusterLoadAssignment { Policy policy = 4; // References endpoint groups by name. - // Both endpoints and EGDS references will be used when present repeated Egds endpoint_groups = 6; } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the EGDS requests to Pilot can be merged into one to boost efficiency +// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; @@ -134,9 +133,9 @@ message EndpointGroup { map named_endpoints = 3; } -// The EGDS request configuration. +// The EndpointGroup request configuration. message Egds { - // Configuration for the source of EGDS updates for this Cluster. + // Configuration for the source of EndpointGroup updates for this Cluster. core.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto index ce52089c9b37..90a8218d710c 100644 --- a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto +++ b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto @@ -125,12 +125,11 @@ message ClusterLoadAssignment { Policy policy = 4; // References endpoint groups by name. - // Both endpoints and EGDS references will be used when present repeated Egds endpoint_groups = 6; } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the EGDS requests to Pilot can be merged into one to boost efficiency +// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; @@ -144,12 +143,12 @@ message EndpointGroup { map named_endpoints = 3; } -// The EGDS request configuration. +// The EndpointGroup request configuration. message Egds { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; - // Configuration for the source of EGDS updates for this Cluster. + // Configuration for the source of EndpointGroup updates for this Cluster. core.v3.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 92eab7f024fa..29dac9c09241 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -151,7 +151,7 @@ void EdsClusterImpl::onConfigUpdate(const Protobuf::RepeatedPtrFieldenableTimer(std::chrono::milliseconds(stale_after_ms)); } - // Check if EGDS resource is included. + // Check if Egds resource is included. if (!cluster_load_assignment.endpoint_groups().empty()) { endpoint_group_manager_ = std::make_unique(); egds_cluster_mapper_ = std::make_unique( diff --git a/source/common/upstream/egds_cluster_mapper.cc b/source/common/upstream/egds_cluster_mapper.cc index 3aa01cb4ea56..ffda4d5d5ff1 100644 --- a/source/common/upstream/egds_cluster_mapper.cc +++ b/source/common/upstream/egds_cluster_mapper.cc @@ -168,7 +168,7 @@ void EgdsClusterMapper::initializeCluster() { pair.second->group_.value().named_endpoints().end()); } - ENVOY_LOG(info, "EGDS cluster {} updated, endpoint count {}", + ENVOY_LOG(info, "Egds cluster {} updated, endpoint count {}", cluster_load_assignment.cluster_name(), cluster_load_assignment.endpoints_size()); delegate_.initializeCluster(cluster_load_assignment); } diff --git a/test/config/utility.h b/test/config/utility.h index f7c7b8746bfa..73271f3a6433 100644 --- a/test/config/utility.h +++ b/test/config/utility.h @@ -249,7 +249,7 @@ class EdsHelper { uint32_t update_successes_{}; }; -// Common code for tests that deliver EGDS update via the filesystem. +// Common code for tests that deliver Egds update via the filesystem. class EgdsHelper { public: EgdsHelper(); diff --git a/test/integration/egds_integration_test.cc b/test/integration/egds_integration_test.cc index 703ad9259b62..47faa8afc5d7 100644 --- a/test/integration/egds_integration_test.cc +++ b/test/integration/egds_integration_test.cc @@ -19,7 +19,7 @@ namespace Envoy { namespace { -// Integration test for EGDS features. EGDS is consumed via filesystem +// Integration test for Egds features. Egds is consumed via filesystem // subscription. class EgdsIntegrationTest : public testing::TestWithParam, public HttpIntegrationTest { @@ -245,7 +245,7 @@ TEST_P(EgdsIntegrationTest, Http2HcClusterRewarming) { fake_upstream_connection.reset(); } -// Verify that a host stabilized via active health checking which is first removed from EGDS and +// Verify that a host stabilized via active health checking which is first removed from Egds and // then fails health checking is removed. TEST_P(EgdsIntegrationTest, RemoveAfterHcFail) { initializeTest(true); @@ -330,7 +330,7 @@ TEST_P(EgdsIntegrationTest, EndpointWarmingFailedHc) { EXPECT_EQ(0, test_server_->gauge("cluster.cluster_0.membership_healthy")->value()); } -// Validate that health status updates are consumed from EGDS. +// Validate that health status updates are consumed from Egds. TEST_P(EgdsIntegrationTest, HealthUpdate) { initializeTest(false); // Initial state, no cluster members. From 71d337d2605631ade0160193f8c1d3121464c1d0 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Mon, 17 Feb 2020 15:55:43 +0800 Subject: [PATCH 05/11] Optimize the code Signed-off-by: leilei.gll --- source/common/upstream/eds.cc | 4 - source/common/upstream/eds.h | 1 - source/common/upstream/egds_api_impl.cc | 19 ++-- source/common/upstream/egds_cluster_mapper.cc | 102 +++++++++++------- source/common/upstream/egds_cluster_mapper.h | 56 +++++----- source/common/upstream/endpoint_group.cc | 10 ++ source/common/upstream/endpoint_group.h | 3 + .../common/upstream/endpoint_group_monitor.h | 2 + .../common/upstream/endpoint_groups_manager.h | 5 + .../upstream/endpoint_groups_manager_impl.cc | 22 ++++ .../upstream/endpoint_groups_manager_impl.h | 4 + source/common/upstream/upstream_impl.cc | 10 +- .../upstream/egds_cluster_mapper_test.cc | 34 +++--- test/mocks/upstream/mocks.h | 3 +- 14 files changed, 173 insertions(+), 102 deletions(-) diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 29dac9c09241..20aa83010dc5 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -244,10 +244,6 @@ void EdsClusterImpl::updateHosts(uint32_t priority, const HostVector& hosts_adde overprovisioning_factor); } -void EdsClusterImpl::batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) { - priority_set_.batchHostUpdate(callback); -} - bool EdsClusterImpl::validateUpdateSize(int num_resources) { if (num_resources == 0) { ENVOY_LOG(debug, "Missing ClusterLoadAssignment for {} in onConfigUpdate()", cluster_name_); diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 7c1971953ae9..1c0e5ec156c6 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -66,7 +66,6 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, const HostVector& hosts_removed, PriorityStateManager& priority_state_manager, LocalityWeightsMap& new_locality_weights_map, absl::optional overprovisioning_factor = absl::nullopt) override; - void batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) override; // ClusterImplBase void reloadHealthyHostsHelper(const HostSharedPtr& host) override; diff --git a/source/common/upstream/egds_api_impl.cc b/source/common/upstream/egds_api_impl.cc index bba1d6cf8bae..68c98a266e08 100644 --- a/source/common/upstream/egds_api_impl.cc +++ b/source/common/upstream/egds_api_impl.cc @@ -81,6 +81,8 @@ void EgdsApiImpl::onConfigUpdate( std::vector exception_msgs; std::unordered_set endpoint_group_names; bool any_applied = false; + std::vector added; + std::vector removed; for (const auto& resource : added_resources) { envoy::config::endpoint::v3::EndpointGroup endpoint_group; try { @@ -92,24 +94,17 @@ void EgdsApiImpl::onConfigUpdate( fmt::format("duplicate endpoint group {} found", endpoint_group.name())); } - if (endpoint_group_mananger_.addOrUpdateEndpointGroup(endpoint_group, resource.version())) { - any_applied = true; - ENVOY_LOG(info, "egds: update endpoint group '{}'", endpoint_group.name()); - } else { - ENVOY_LOG(debug, "egds: update endpoint group '{}' skipped", endpoint_group.name()); - } + ENVOY_LOG(debug, "egds: added endpoint group '{}'", endpoint_group.name()); + added.emplace_back(endpoint_group); } catch (const EnvoyException& e) { exception_msgs.push_back(fmt::format("{}: {}", endpoint_group.name(), e.what())); } } - for (const auto& resource_name : removed_resources) { - if (endpoint_group_mananger_.clearEndpointGroup(resource_name, version_info)) { - any_applied = true; - ENVOY_LOG(info, "egds: remove endpoint group '{}'", resource_name); - } - } + ENVOY_LOG(debug, "egds: removed endpoint group '{}'", removed_resources.size()); + removed.insert(removed.end(), removed_resources.begin(), removed_resources.end()); + any_applied = endpoint_group_mananger_.batchUpdateEndpointGroup(added, removed, version_info); if (any_applied) { system_version_info_ = version_info; } diff --git a/source/common/upstream/egds_cluster_mapper.cc b/source/common/upstream/egds_cluster_mapper.cc index ffda4d5d5ff1..de348c1deb1b 100644 --- a/source/common/upstream/egds_cluster_mapper.cc +++ b/source/common/upstream/egds_cluster_mapper.cc @@ -48,6 +48,12 @@ void EgdsClusterMapper::removeResource(absl::string_view name) { void EgdsClusterMapper::ActiveEndpointGroupMonitor::update( const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info) { + batchUpdate(group, version_info, true); +} + +void EgdsClusterMapper::ActiveEndpointGroupMonitor::batchUpdate( + const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info, + bool all_endpoint_groups_updated) { if (group_.has_value() && Protobuf::util::MessageDifferencer::Equivalent(group_.value(), group)) { ENVOY_LOG(info, "Resources {} are unchanged and do not need to be updated, version: {} ", group.name(), version_info); @@ -60,59 +66,38 @@ void EgdsClusterMapper::ActiveEndpointGroupMonitor::update( // When the complete data of the cluster is initialized, subsequent updates to each EG are // incrementally calculated. if (parent_.cluster_initialized_) { - BatchUpdateHelper helper(*this); - parent_.delegate_.batchHostUpdateForEndpointGroup(helper); + parent_.addUpdatedActiveMonitor(shared_from_this()); + if (all_endpoint_groups_updated) { + // This is the last updated endpoint group, trigger a batch update. + parent_.batchHostUpdate(); + return; + } + return; } + // Initialize the Endpoint Group, using the initial data as the baseline for subsequent + // calculations. Initialization does not trigger "updateHosts" notification. + initializeEndpointGroup(); + if (parent_.clusterDataIsReady()) { // Initialize the cluster after all EG data is complete and the initialization is performed only // once. parent_.initializeCluster(); } - - // Initialize the Endpoint Group, using the initial data as the baseline for subsequent - // calculations. Initialization does not trigger "updateHosts" notification. - initializeEndpointGroup(); } void EgdsClusterMapper::ActiveEndpointGroupMonitor::initializeEndpointGroup() { - BatchUpdateHelper helper(*this, false); EmptyBatchUpdateScope empty_update; - helper.batchUpdate(empty_update); + doBatchUpdate(empty_update); } -bool EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper:: - calculateUpdatedHostsPerLocality(const uint32_t priority, const HostVector& new_hosts, - std::unordered_map& updated_hosts, - PriorityStateManager& priority_state_manager, - LocalityWeightsMap& new_locality_weights_map, - absl::optional overprovisioning_factor) { - auto& host_set = parent_.priority_set_.getOrCreateMutableHostSet(priority); - ENVOY_LOG(debug, - "compute the updated host in the '{}' endpoint-group, added hosts count '{}', existed " - "hosts count '{}'", - parent_.group_.value().name(), new_hosts.size(), host_set.mutableHosts().size()); - HostVector hosts_added; - HostVector hosts_removed; - const bool hosts_updated = parent_.parent_.cluster_.updateDynamicHostList( - new_hosts, host_set.mutableHosts(), hosts_added, hosts_removed, updated_hosts, - parent_.all_hosts_); - if (hosts_updated && notify_hosts_updated_) { - parent_.parent_.delegate_.updateHosts(priority, hosts_added, hosts_removed, - priority_state_manager, new_locality_weights_map, - overprovisioning_factor); - } - - return hosts_updated; -} - -void EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper::batchUpdate( +void EgdsClusterMapper::ActiveEndpointGroupMonitor::doBatchUpdate( PrioritySet::HostUpdateCb& host_update_cb) { std::unordered_map updated_hosts; - PriorityStateManager priority_state_manager(parent_.parent_.cluster_, parent_.parent_.local_info_, + PriorityStateManager priority_state_manager(parent_.cluster_, parent_.local_info_, &host_update_cb); - for (const auto& locality_lb_endpoint : parent_.group_.value().endpoints()) { + for (const auto& locality_lb_endpoint : group_.value().endpoints()) { priority_state_manager.initializePriorityFor(locality_lb_endpoint); for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { @@ -126,7 +111,7 @@ void EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper::batchUpda bool endpoint_group_rebuilt = false; const uint32_t overprovisioning_factor = - PROTOBUF_GET_WRAPPED_OR_DEFAULT(parent_.parent_.origin_cluster_load_assignment_.policy(), + PROTOBUF_GET_WRAPPED_OR_DEFAULT(parent_.origin_cluster_load_assignment_.policy(), overprovisioning_factor, kDefaultOverProvisioningFactor); // Loop over all priorities that exist in the new configuration. @@ -139,13 +124,35 @@ void EgdsClusterMapper::ActiveEndpointGroupMonitor::BatchUpdateHelper::batchUpda } } - parent_.all_hosts_ = std::move(updated_hosts); + all_hosts_ = std::move(updated_hosts); if (endpoint_group_rebuilt) { // TODO(leilei.gll) Add stats. } - ENVOY_LOG(debug, "EG resource '{}' completes batch updates", parent_.group_.value().name()); + ENVOY_LOG(debug, "Edgs resource '{}' completes batch updates", group_.value().name()); +} + +bool EgdsClusterMapper::ActiveEndpointGroupMonitor::calculateUpdatedHostsPerLocality( + const uint32_t priority, const HostVector& new_hosts, + std::unordered_map& updated_hosts, + PriorityStateManager& priority_state_manager, LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor) { + auto& host_set = priority_set_.getOrCreateMutableHostSet(priority); + ENVOY_LOG(debug, + "compute the updated host in the '{}' endpoint-group, added hosts count '{}', existed " + "hosts count '{}'", + group_.value().name(), new_hosts.size(), host_set.mutableHosts().size()); + HostVector hosts_added; + HostVector hosts_removed; + const bool hosts_updated = parent_.cluster_.updateDynamicHostList( + new_hosts, host_set.mutableHosts(), hosts_added, hosts_removed, updated_hosts, all_hosts_); + if (hosts_updated && parent_.cluster_initialized_) { + parent_.delegate_.updateHosts(priority, hosts_added, hosts_removed, priority_state_manager, + new_locality_weights_map, overprovisioning_factor); + } + + return hosts_updated; } bool EgdsClusterMapper::clusterDataIsReady() { @@ -168,10 +175,25 @@ void EgdsClusterMapper::initializeCluster() { pair.second->group_.value().named_endpoints().end()); } - ENVOY_LOG(info, "Egds cluster {} updated, endpoint count {}", + ENVOY_LOG(debug, "Egds cluster {} updated, endpoint count {}", cluster_load_assignment.cluster_name(), cluster_load_assignment.endpoints_size()); delegate_.initializeCluster(cluster_load_assignment); } +void EgdsClusterMapper::BatchUpdateHelper::batchUpdate(PrioritySet::HostUpdateCb& host_update_cb) { + for (auto& monitor : updated_monitor_) { + monitor->doBatchUpdate(host_update_cb); + } +} + +void EgdsClusterMapper::batchHostUpdate() { + cluster_.prioritySet().batchHostUpdate(batch_update_helper_); + batch_update_helper_.clear(); +} + +void EgdsClusterMapper::addUpdatedActiveMonitor(ActiveEndpointGroupMonitorSharedPtr monitor) { + batch_update_helper_.addUpdatedMonitor(monitor); +} + } // namespace Upstream } // namespace Envoy diff --git a/source/common/upstream/egds_cluster_mapper.h b/source/common/upstream/egds_cluster_mapper.h index feb26c36dca6..17064287b918 100644 --- a/source/common/upstream/egds_cluster_mapper.h +++ b/source/common/upstream/egds_cluster_mapper.h @@ -33,8 +33,6 @@ class EgdsClusterMapper : Logger::Loggable { PriorityStateManager& priority_state_manager, LocalityWeightsMap& new_locality_weights_map, absl::optional overprovisioning_factor = absl::nullopt) PURE; - - virtual void batchHostUpdateForEndpointGroup(PrioritySet::BatchUpdateCb& callback) PURE; }; EgdsClusterMapper( @@ -49,35 +47,27 @@ class EgdsClusterMapper : Logger::Loggable { std::set egds_resource_names() const; private: - struct ActiveEndpointGroupMonitor : public EndpointGroupMonitor { + struct ActiveEndpointGroupMonitor + : public EndpointGroupMonitor, + public std::enable_shared_from_this { ActiveEndpointGroupMonitor(EgdsClusterMapper& parent) : parent_(parent) {} ~ActiveEndpointGroupMonitor() = default; // Upstream::EndpointGroupMonitor void update(const envoy::config::endpoint::v3::EndpointGroup& group, - absl::string_view version_info); - - void initializeEndpointGroup(); + absl::string_view version_info) override; + void batchUpdate(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info, bool all_endpoint_groups_updated) override; - class BatchUpdateHelper : public PrioritySet::BatchUpdateCb { - public: - BatchUpdateHelper(ActiveEndpointGroupMonitor& parent, bool notify_hosts_updated = true) - : parent_(parent), notify_hosts_updated_(notify_hosts_updated) {} + void doBatchUpdate(PrioritySet::HostUpdateCb& host_update_cb); - // Upstream::PrioritySet::BatchUpdateCb - void batchUpdate(PrioritySet::HostUpdateCb& host_update_cb) override; - - bool calculateUpdatedHostsPerLocality( - const uint32_t priority, const HostVector& new_hosts, - std::unordered_map& updated_hosts, - PriorityStateManager& priority_state_manager, - LocalityWeightsMap& new_locality_weights_map, - absl::optional overprovisioning_factor); - - private: - ActiveEndpointGroupMonitor& parent_; - const bool notify_hosts_updated_; - }; + void initializeEndpointGroup(); + bool + calculateUpdatedHostsPerLocality(const uint32_t priority, const HostVector& new_hosts, + std::unordered_map& updated_hosts, + PriorityStateManager& priority_state_manager, + LocalityWeightsMap& new_locality_weights_map, + absl::optional overprovisioning_factor); EgdsClusterMapper& parent_; absl::optional group_; @@ -93,8 +83,25 @@ class EgdsClusterMapper : Logger::Loggable { const HostVector&, const HostVector&, absl::optional) override {} }; + class BatchUpdateHelper : public PrioritySet::BatchUpdateCb { + public: + // Upstream::PrioritySet::BatchUpdateCb + void batchUpdate(PrioritySet::HostUpdateCb& host_update_cb) override; + + void addUpdatedMonitor(ActiveEndpointGroupMonitorSharedPtr monitor) { + updated_monitor_.push_back(monitor); + } + + void clear() { updated_monitor_.clear(); } + + private: + std::deque updated_monitor_; + }; + bool clusterDataIsReady(); void initializeCluster(); + void batchHostUpdate(); + void addUpdatedActiveMonitor(ActiveEndpointGroupMonitorSharedPtr monitor); absl::flat_hash_map active_monitors_; EndpointGroupMonitorManager& monitor_manager_; @@ -103,6 +110,7 @@ class EgdsClusterMapper : Logger::Loggable { bool cluster_initialized_{false}; envoy::config::endpoint::v3::ClusterLoadAssignment origin_cluster_load_assignment_; const LocalInfo::LocalInfo& local_info_; + BatchUpdateHelper batch_update_helper_; }; using EgdsClusterMapperPtr = std::unique_ptr; diff --git a/source/common/upstream/endpoint_group.cc b/source/common/upstream/endpoint_group.cc index 73ed20bcccf7..de3456948637 100644 --- a/source/common/upstream/endpoint_group.cc +++ b/source/common/upstream/endpoint_group.cc @@ -12,5 +12,15 @@ bool ActiveEndpointGroup::update(const envoy::config::endpoint::v3::EndpointGrou return true; } +bool ActiveEndpointGroup::batchUpdate(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info, + bool all_endpoint_groups_updated) { + for (auto& monitor : monitors_) { + monitor->batchUpdate(group, version_info, all_endpoint_groups_updated); + } + + return true; +} + } // namespace Upstream } // namespace Envoy \ No newline at end of file diff --git a/source/common/upstream/endpoint_group.h b/source/common/upstream/endpoint_group.h index 072394b48c80..d50008f7edc2 100644 --- a/source/common/upstream/endpoint_group.h +++ b/source/common/upstream/endpoint_group.h @@ -16,6 +16,9 @@ class ActiveEndpointGroup { bool update(const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info); + bool batchUpdate(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info, bool all_endpoint_groups_updated); + void addMonitor(EndpointGroupMonitorSharedPtr monitor) { if (!monitors_.count(monitor)) { monitors_.insert(monitor); diff --git a/source/common/upstream/endpoint_group_monitor.h b/source/common/upstream/endpoint_group_monitor.h index 97b01f1d6fc3..df1015c87efb 100644 --- a/source/common/upstream/endpoint_group_monitor.h +++ b/source/common/upstream/endpoint_group_monitor.h @@ -17,6 +17,8 @@ class EndpointGroupMonitor { virtual void update(const envoy::config::endpoint::v3::EndpointGroup& group, absl::string_view version_info) PURE; + virtual void batchUpdate(const envoy::config::endpoint::v3::EndpointGroup& group, + absl::string_view version_info, bool all_endpoint_groups_updated) PURE; }; using EndpointGroupMonitorSharedPtr = std::shared_ptr; diff --git a/source/common/upstream/endpoint_groups_manager.h b/source/common/upstream/endpoint_groups_manager.h index 36b7a363a81c..48630593b3f4 100644 --- a/source/common/upstream/endpoint_groups_manager.h +++ b/source/common/upstream/endpoint_groups_manager.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "envoy/config/endpoint/v3/endpoint.pb.h" @@ -17,6 +18,10 @@ class EndpointGroupsManager { absl::string_view version_info) PURE; virtual bool removeEndpointGroup(absl::string_view name) PURE; virtual bool clearEndpointGroup(absl::string_view name, absl::string_view version_info) PURE; + virtual bool + batchUpdateEndpointGroup(const std::vector& added, + const std::vector removed, + absl::string_view version_info) PURE; }; using EndpointGroupsManagerPtr = std::unique_ptr; diff --git a/source/common/upstream/endpoint_groups_manager_impl.cc b/source/common/upstream/endpoint_groups_manager_impl.cc index 14c39d0746eb..dbbcc1deaa70 100644 --- a/source/common/upstream/endpoint_groups_manager_impl.cc +++ b/source/common/upstream/endpoint_groups_manager_impl.cc @@ -27,6 +27,28 @@ bool EndpointGroupsManagerImpl::removeEndpointGroup(absl::string_view name) { return active_groups_.erase(name); } +bool EndpointGroupsManagerImpl::batchUpdateEndpointGroup( + const std::vector& added, + const std::vector removed, absl::string_view version_info) { + std::vector total(added); + for (auto& name : removed) { + envoy::config::endpoint::v3::EndpointGroup empty_group; + empty_group.set_name(name.data()); + total.emplace_back(empty_group); + } + + for (size_t i = 0; i < total.size(); i++) { + const auto& group = total[i]; + if (active_groups_.find(group.name()) == active_groups_.end()) { + active_groups_.emplace(group.name(), std::make_shared()); + } + + active_groups_[group.name()]->batchUpdate(group, version_info, i == total.size() - 1); + } + + return true; +} + void EndpointGroupsManagerImpl::addMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) { if (active_groups_.find(group_name) == active_groups_.end()) { diff --git a/source/common/upstream/endpoint_groups_manager_impl.h b/source/common/upstream/endpoint_groups_manager_impl.h index 4f0e64d78475..b2f9482c3886 100644 --- a/source/common/upstream/endpoint_groups_manager_impl.h +++ b/source/common/upstream/endpoint_groups_manager_impl.h @@ -20,6 +20,10 @@ class EndpointGroupsManagerImpl : public EndpointGroupsManager, public EndpointG absl::string_view version_info) override; bool removeEndpointGroup(absl::string_view name) override; bool clearEndpointGroup(absl::string_view name, absl::string_view version_info) override; + bool + batchUpdateEndpointGroup(const std::vector& added, + const std::vector removed, + absl::string_view version_info) override; // Upstream::EndpointGroupMonitorManager void addMonitor(EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) override; diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index a22445f0cb03..e1437f4c3d76 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -585,9 +585,13 @@ void PrioritySetImpl::BatchUpdateScope::updateHosts( uint32_t priority, PrioritySet::UpdateHostsParams&& update_hosts_params, LocalityWeightsConstSharedPtr locality_weights, const HostVector& hosts_added, const HostVector& hosts_removed, absl::optional overprovisioning_factor) { - // We assume that each call updates a different priority. - ASSERT(priorities_.find(priority) == priorities_.end()); - priorities_.insert(priority); + // The updateHosts function is called for each EGDS update, and multiple EGDS may contain the same + // priority, so the ASSERT check here does not apply. + // ASSERT(priorities_.find(priority) == priorities_.end()); + + if (!priorities_.count(priority)) { + priorities_.insert(priority); + } for (const auto& host : hosts_added) { all_hosts_added_.insert(host); diff --git a/test/common/upstream/egds_cluster_mapper_test.cc b/test/common/upstream/egds_cluster_mapper_test.cc index 8c0e69746e48..ce506880a6e7 100644 --- a/test/common/upstream/egds_cluster_mapper_test.cc +++ b/test/common/upstream/egds_cluster_mapper_test.cc @@ -186,23 +186,23 @@ TEST_F(EgdsClusterMapperTest, Basic) { { // The onUpdate interface is called because the dependent resources has been // retrieved. - PrioritySet::BatchUpdateCb* batch_update_callback; - EXPECT_CALL(mock_delegate, batchHostUpdateForEndpointGroup(_)) - .WillOnce(Invoke([&](PrioritySet::BatchUpdateCb& callback) -> void { - batch_update_callback = &callback; - })); - EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); - const std::string version("1.0"); - envoy::config::endpoint::v3::EndpointGroup group_data; - group_data.set_name("test_data_1"); - auto* endpoints = group_data.add_endpoints(); - auto* socket_address = endpoints->add_lb_endpoints() - ->mutable_endpoint() - ->mutable_address() - ->mutable_socket_address(); - socket_address->set_address("4.5.6.7"); - socket_address->set_port_value(4567); - active_monitor_test1->update(group_data, version); + // PrioritySet::BatchUpdateCb* batch_update_callback; + // EXPECT_CALL(mock_delegate, batchHostUpdateForEndpointGroup(_)) + // .WillOnce(Invoke([&](PrioritySet::BatchUpdateCb& callback) -> void { + // batch_update_callback = &callback; + // })); + // EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + // const std::string version("1.0"); + // envoy::config::endpoint::v3::EndpointGroup group_data; + // group_data.set_name("test_data_1"); + // auto* endpoints = group_data.add_endpoints(); + // auto* socket_address = endpoints->add_lb_endpoints() + // ->mutable_endpoint() + // ->mutable_address() + // ->mutable_socket_address(); + // socket_address->set_address("4.5.6.7"); + // socket_address->set_port_value(4567); + // active_monitor_test1->update(group_data, version); } EXPECT_CALL(mock_manager, removeMonitor(_, _)).Times(1); diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 12325447c9f2..47b43f31a558 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -438,6 +438,8 @@ class MockEndpointGroupMonitor : public EndpointGroupMonitor { ~MockEndpointGroupMonitor() override; MOCK_METHOD2(update, void(const envoy::config::endpoint::v3::EndpointGroup&, absl::string_view)); + MOCK_METHOD3(batchUpdate, + void(const envoy::config::endpoint::v3::EndpointGroup&, absl::string_view, bool)); }; class MockEndpointGroupsManager : public EndpointGroupsManager { @@ -456,7 +458,6 @@ class MockEgdsClusterMapperDelegate : public EgdsClusterMapper::Delegate { ~MockEgdsClusterMapperDelegate() override = default; MOCK_METHOD1(initializeCluster, void(const envoy::config::endpoint::v3::ClusterLoadAssignment&)); - MOCK_METHOD1(batchHostUpdateForEndpointGroup, void(PrioritySet::BatchUpdateCb&)); MOCK_METHOD6(updateHosts, void(uint32_t priority, const HostVector&, const HostVector&, PriorityStateManager&, LocalityWeightsMap&, absl::optional)); From f22ade3a6e67491943133253cba33d4148050994 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Thu, 20 Feb 2020 14:31:33 +0800 Subject: [PATCH 06/11] EGDS messages are compatible with V2/V3 versions Signed-off-by: leilei.gll --- source/common/upstream/BUILD | 1 + source/common/upstream/egds_api_impl.cc | 22 ++++++++++++++++++---- source/common/upstream/egds_api_impl.h | 2 ++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index 5f39be711a0d..ba9c4a70489d 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -645,6 +645,7 @@ envoy_cc_library( "//include/envoy/upstream:locality_lib", "//include/envoy/upstream:upstream_interface", "//source/common/common:minimal_logger_lib", + "//source/common/config:api_version_lib", "//source/common/config:metadata_lib", "//source/common/config:subscription_factory_lib", "//source/common/config:utility_lib", diff --git a/source/common/upstream/egds_api_impl.cc b/source/common/upstream/egds_api_impl.cc index 68c98a266e08..ea1340e7393f 100644 --- a/source/common/upstream/egds_api_impl.cc +++ b/source/common/upstream/egds_api_impl.cc @@ -1,11 +1,13 @@ #include "common/upstream/egds_api_impl.h" +#include "envoy/api/v2/endpoint.pb.h" #include "envoy/config/core/v3/config_source.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.h" #include "envoy/config/endpoint/v3/endpoint.pb.validate.h" #include "envoy/service/discovery/v3/discovery.pb.h" #include "common/common/utility.h" +#include "common/config/api_version.h" #include "common/grpc/common.h" #include "common/protobuf/utility.h" @@ -32,10 +34,7 @@ EgdsApiImpl::EgdsApiImpl( for (auto& pair : config_source_map) { auto subscription = subscription_factory.subscriptionFromConfigSource( - pair.first, - Grpc::Common::typeUrl( - envoy::config::endpoint::v3::EndpointGroup().GetDescriptor()->full_name()), - *scope_, *this); + pair.first, loadTypeUrl(pair.first.resource_api_version()), *scope_, *this); subscription_map_.emplace(std::move(subscription), std::move(pair.second)); } } @@ -123,5 +122,20 @@ void EgdsApiImpl::onConfigUpdateFailed(Envoy::Config::ConfigUpdateFailureReason initialization_fail_callback_(); } +std::string EgdsApiImpl::loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version) { + switch (resource_api_version) { + // automatically set api version as V2 + case envoy::config::core::v3::ApiVersion::AUTO: + case envoy::config::core::v3::ApiVersion::V2: + return Grpc::Common::typeUrl( + API_NO_BOOST(envoy::api::v2::EndpointGroup().GetDescriptor()->full_name())); + case envoy::config::core::v3::ApiVersion::V3: + return Grpc::Common::typeUrl( + API_NO_BOOST(envoy::config::endpoint::v3::EndpointGroup().GetDescriptor()->full_name())); + default: + NOT_REACHED_GCOVR_EXCL_LINE; + } +} + } // namespace Upstream } // namespace Envoy diff --git a/source/common/upstream/egds_api_impl.h b/source/common/upstream/egds_api_impl.h index a040ce33953f..7f5c79686b22 100644 --- a/source/common/upstream/egds_api_impl.h +++ b/source/common/upstream/egds_api_impl.h @@ -70,6 +70,8 @@ class EgdsApiImpl : public Config::SubscriptionCallbacks, } private: + std::string loadTypeUrl(envoy::config::core::v3::ApiVersion resource_api_version); + EndpointGroupsManager& endpoint_group_mananger_; ProtobufMessage::ValidationVisitor& validation_visitor_; std::unordered_map> subscription_map_; From 8e64ac98fc494026e1d72a54fd550c0abe34a65b Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Fri, 21 Feb 2020 10:36:13 +0800 Subject: [PATCH 07/11] Support for reload health hosting mechanisms Signed-off-by: leilei.gll --- source/common/upstream/eds.cc | 4 + source/common/upstream/eds.h | 2 + source/common/upstream/egds_cluster_mapper.cc | 29 +++- source/common/upstream/egds_cluster_mapper.h | 10 +- test/common/upstream/egds_api_impl_test.cc | 27 +++- .../upstream/egds_cluster_mapper_test.cc | 135 +++++++++++++++--- test/mocks/upstream/mocks.h | 3 + 7 files changed, 186 insertions(+), 24 deletions(-) diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 20aa83010dc5..a2e30fa5cc4e 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -310,6 +310,10 @@ void EdsClusterImpl::reloadHealthyHostsHelper(const HostSharedPtr& host) { prioritySet().updateHosts(priority, HostSetImpl::partitionHosts(hosts_copy, hosts_per_locality_copy), host_set->localityWeights(), {}, hosts_to_remove, absl::nullopt); + + if (egds_cluster_mapper_ && host_to_exclude) { + egds_cluster_mapper_->reloadHealthyHostsHelper(priority, host_to_exclude); + } } if (host_to_exclude != nullptr) { diff --git a/source/common/upstream/eds.h b/source/common/upstream/eds.h index 1c0e5ec156c6..bd6cc823c58a 100644 --- a/source/common/upstream/eds.h +++ b/source/common/upstream/eds.h @@ -37,6 +37,8 @@ class EdsClusterImpl : public BaseDynamicClusterImpl, // Upstream::Cluster InitializePhase initializePhase() const override { return initialize_phase_; } + const HostMap& all_hosts() const { return all_hosts_; } + private: // Config::SubscriptionCallbacks void onConfigUpdate(const Protobuf::RepeatedPtrField& resources, diff --git a/source/common/upstream/egds_cluster_mapper.cc b/source/common/upstream/egds_cluster_mapper.cc index de348c1deb1b..80537f85d215 100644 --- a/source/common/upstream/egds_cluster_mapper.cc +++ b/source/common/upstream/egds_cluster_mapper.cc @@ -140,7 +140,7 @@ bool EgdsClusterMapper::ActiveEndpointGroupMonitor::calculateUpdatedHostsPerLoca absl::optional overprovisioning_factor) { auto& host_set = priority_set_.getOrCreateMutableHostSet(priority); ENVOY_LOG(debug, - "compute the updated host in the '{}' endpoint-group, added hosts count '{}', existed " + "compute the updated host in the '{}' endpoint-group, new hosts count '{}', existed " "hosts count '{}'", group_.value().name(), new_hosts.size(), host_set.mutableHosts().size()); HostVector hosts_added; @@ -155,6 +155,26 @@ bool EgdsClusterMapper::ActiveEndpointGroupMonitor::calculateUpdatedHostsPerLoca return hosts_updated; } +void EgdsClusterMapper::ActiveEndpointGroupMonitor::reloadHealthyHostsHelper( + const uint32_t priority, const HostSharedPtr& host_to_exclude) { + ASSERT(host_to_exclude->healthFlagGet(Host::HealthFlag::FAILED_ACTIVE_HC) && + host_to_exclude->healthFlagGet(Host::HealthFlag::PENDING_DYNAMIC_REMOVAL)); + if (priority_set_.hostSetsPerPriority().size() < priority + 1) { + // The priority queue does not exist. + return; + } + + auto& hosts = priority_set_.getOrCreateMutableHostSet(priority).mutableHosts(); + const auto& address = host_to_exclude->address()->asString(); + const auto existing_itr = + std::find_if(hosts.begin(), hosts.end(), [address](const HostSharedPtr current) { + return current->address()->asString() == address; + }); + if (existing_itr != hosts.end()) { + hosts.erase(existing_itr); + } +} + bool EgdsClusterMapper::clusterDataIsReady() { for (const auto& pair : active_monitors_) { if (!pair.second->group_.has_value()) { @@ -195,5 +215,12 @@ void EgdsClusterMapper::addUpdatedActiveMonitor(ActiveEndpointGroupMonitorShared batch_update_helper_.addUpdatedMonitor(monitor); } +void EgdsClusterMapper::reloadHealthyHostsHelper(const uint32_t priority, + const HostSharedPtr& host_to_exclude) { + for (auto& pair : active_monitors_) { + pair.second->reloadHealthyHostsHelper(priority, host_to_exclude); + } +} + } // namespace Upstream } // namespace Envoy diff --git a/source/common/upstream/egds_cluster_mapper.h b/source/common/upstream/egds_cluster_mapper.h index 17064287b918..58bccf59e046 100644 --- a/source/common/upstream/egds_cluster_mapper.h +++ b/source/common/upstream/egds_cluster_mapper.h @@ -17,8 +17,6 @@ namespace Envoy { namespace Upstream { -class EdsClusterImpl; - class EgdsClusterMapper : Logger::Loggable { public: class Delegate { @@ -46,6 +44,12 @@ class EgdsClusterMapper : Logger::Loggable { void removeResource(absl::string_view name); std::set egds_resource_names() const; + void reloadHealthyHostsHelper(const uint32_t priority, const HostSharedPtr& host_to_exclude); + + const PrioritySet& getPrioritySetInMonitorForTest(const std::string& name) const { + return active_monitors_.at(name)->priority_set_; + } + private: struct ActiveEndpointGroupMonitor : public EndpointGroupMonitor, @@ -69,6 +73,8 @@ class EgdsClusterMapper : Logger::Loggable { LocalityWeightsMap& new_locality_weights_map, absl::optional overprovisioning_factor); + void reloadHealthyHostsHelper(const uint32_t priority, const HostSharedPtr& host_to_exclude); + EgdsClusterMapper& parent_; absl::optional group_; absl::optional version_; diff --git a/test/common/upstream/egds_api_impl_test.cc b/test/common/upstream/egds_api_impl_test.cc index aecbf34c580f..ccda854fc0c7 100644 --- a/test/common/upstream/egds_api_impl_test.cc +++ b/test/common/upstream/egds_api_impl_test.cc @@ -142,7 +142,14 @@ TEST_F(EgdsApiImplTest, onConfigUpdateSuccess) { endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80); resources.Add()->PackFrom(endpoint_group); - EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)); + EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)).Times(0); + EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) + .WillOnce(Invoke([&](const std::vector& added, + const std::vector removed, absl::string_view) -> bool { + EXPECT_EQ(1, added.size()); + EXPECT_EQ(0, removed.size()); + return true; + })); egds_callbacks_->onConfigUpdate(resources, ""); } @@ -159,7 +166,14 @@ TEST_F(EgdsApiImplTest, onConfigUpdateReplace) { resources.Add()->PackFrom(endpoint_group); egds_callbacks_->onConfigUpdate(resources, ""); - EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)); + EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)).Times(0); + EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) + .WillOnce(Invoke([&](const std::vector& added, + const std::vector removed, absl::string_view) -> bool { + EXPECT_EQ(1, added.size()); + EXPECT_EQ(0, removed.size()); + return true; + })); egds_callbacks_->onConfigUpdate(resources, ""); } @@ -181,7 +195,14 @@ TEST_F(EgdsApiImplTest, onConfigUpdateRemove) { new_resources.Add()->PackFrom(endpoint_group); egds_callbacks_->onConfigUpdate(resources, ""); - EXPECT_CALL(eg_manager_, clearEndpointGroup(_, _)); + EXPECT_CALL(eg_manager_, clearEndpointGroup(_, _)).Times(0); + EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) + .WillOnce(Invoke([&](const std::vector& added, + const std::vector removed, absl::string_view) -> bool { + EXPECT_EQ(0, added.size()); + EXPECT_EQ(1, removed.size()); + return true; + })); egds_callbacks_->onConfigUpdate(new_resources, ""); } diff --git a/test/common/upstream/egds_cluster_mapper_test.cc b/test/common/upstream/egds_cluster_mapper_test.cc index ce506880a6e7..807cfd04ad19 100644 --- a/test/common/upstream/egds_cluster_mapper_test.cc +++ b/test/common/upstream/egds_cluster_mapper_test.cc @@ -15,12 +15,16 @@ #include "test/mocks/runtime/mocks.h" #include "test/mocks/server/mocks.h" #include "test/mocks/ssl/mocks.h" +#include "test/mocks/upstream/host.h" #include "test/mocks/upstream/mocks.h" #include "test/test_common/utility.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +using testing::_; +using testing::Return; + namespace Envoy { namespace Upstream { namespace { @@ -83,7 +87,6 @@ TEST_F(EgdsClusterMapperTest, Basic) { MockEndpointGroupMonitorManager mock_manager; MockEgdsClusterMapperDelegate mock_delegate; envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; - // LocalInfo::MockLocalInfo mock_local_info; EgdsClusterMapper mapper(mock_manager, mock_delegate, *cluster_, cluster_load_assignment, local_info_); @@ -186,23 +189,33 @@ TEST_F(EgdsClusterMapperTest, Basic) { { // The onUpdate interface is called because the dependent resources has been // retrieved. - // PrioritySet::BatchUpdateCb* batch_update_callback; - // EXPECT_CALL(mock_delegate, batchHostUpdateForEndpointGroup(_)) - // .WillOnce(Invoke([&](PrioritySet::BatchUpdateCb& callback) -> void { - // batch_update_callback = &callback; - // })); - // EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); - // const std::string version("1.0"); - // envoy::config::endpoint::v3::EndpointGroup group_data; - // group_data.set_name("test_data_1"); - // auto* endpoints = group_data.add_endpoints(); - // auto* socket_address = endpoints->add_lb_endpoints() - // ->mutable_endpoint() - // ->mutable_address() - // ->mutable_socket_address(); - // socket_address->set_address("4.5.6.7"); - // socket_address->set_port_value(4567); - // active_monitor_test1->update(group_data, version); + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + EXPECT_CALL(mock_delegate, updateHosts(_, _, _, _, _, _)) + .WillOnce(Invoke([&](uint32_t priority, const HostVector& hosts_added, + const HostVector& hosts_removed, + PriorityStateManager&, + LocalityWeightsMap&, + absl::optional) -> void { + EXPECT_EQ(0, priority); + EXPECT_EQ("4.5.6.7:4567", hosts_added.at(0)->address()->asString()); + EXPECT_EQ("1.2.3.4:1234", hosts_removed.at(0)->address()->asString()); + })); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_1"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("4.5.6.7"); + socket_address->set_port_value(4567); + active_monitor_test1->update(group_data, version); + + const auto& hosts_set = mapper.getPrioritySetInMonitorForTest("test1").hostSetsPerPriority()[0]; + const auto& hosts = hosts_set->hosts(); + EXPECT_EQ(1, hosts.size()); + EXPECT_EQ("4.5.6.7:4567", hosts.at(0)->address()->asString()); } EXPECT_CALL(mock_manager, removeMonitor(_, _)).Times(1); @@ -218,6 +231,92 @@ TEST_F(EgdsClusterMapperTest, Basic) { mapper.addResource(egds_resource_names.at(1)); } +TEST_F(EgdsClusterMapperTest, RemoveInactiveHosts) { + MockEndpointGroupMonitorManager mock_manager; + MockEgdsClusterMapperDelegate mock_delegate; + envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment; + EgdsClusterMapper mapper(mock_manager, mock_delegate, *cluster_, cluster_load_assignment, + local_info_); + + const std::vector egds_resource_names = {"test1", "test2"}; + EndpointGroupMonitorSharedPtr active_monitor_test1, active_monitor_test2; + + EXPECT_CALL(mock_manager, addMonitor(_, _)) + .WillOnce( + Invoke([&](EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) -> void { + active_monitor_test1 = monitor; + EXPECT_STREQ(egds_resource_names.at(0).c_str(), group_name.data()); + })); + mapper.addResource(egds_resource_names.at(0)); + + EXPECT_CALL(mock_manager, addMonitor(_, _)) + .WillOnce( + Invoke([&](EndpointGroupMonitorSharedPtr monitor, absl::string_view group_name) -> void { + active_monitor_test2 = monitor; + EXPECT_STREQ(egds_resource_names.at(1).c_str(), group_name.data()); + })); + mapper.addResource(egds_resource_names.at(1)); + + { + // The onUpdate interface is not called because the other two dependent resources are not + // retrieved, test_data_2 and test_data_3 isn't ready. + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_1"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("1.2.3.4"); + socket_address->set_port_value(1234); + active_monitor_test1->update(group_data, version); + } + + { + // The onUpdate interface is not called because the other dependent resources are not + // retrieved, test_data_3 isn't ready. + EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(1); + const std::string version("1.0"); + envoy::config::endpoint::v3::EndpointGroup group_data; + group_data.set_name("test_data_2"); + auto* endpoints = group_data.add_endpoints(); + auto* socket_address = endpoints->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address->set_address("2.3.4.5"); + socket_address->set_port_value(2345); + + auto* endpoints_2 = group_data.add_endpoints(); + auto* socket_address_2 = endpoints_2->add_lb_endpoints() + ->mutable_endpoint() + ->mutable_address() + ->mutable_socket_address(); + socket_address_2->set_address("8.3.4.5"); + socket_address_2->set_port_value(2345); + + active_monitor_test2->update(group_data, version); + } + + std::shared_ptr mock_host(new NiceMock()); + auto address_ptr = Network::Utility::resolveUrl("tcp://2.3.4.5:2345"); + ON_CALL(*mock_host, address()).WillByDefault(Return(address_ptr)); + ON_CALL(*mock_host, healthFlagGet(_)).WillByDefault(Return(true)); + mapper.reloadHealthyHostsHelper(0, mock_host); + + const auto& hosts_set = mapper.getPrioritySetInMonitorForTest("test2").hostSetsPerPriority()[0]; + const auto& hosts = hosts_set->hosts(); + EXPECT_EQ(1, hosts.size()); + + const auto existing_itr = + std::find_if(hosts.begin(), hosts.end(), [address_ptr](const HostSharedPtr current) { + return current->address()->asString() == address_ptr->asString(); + }); + EXPECT_TRUE(existing_itr == hosts.end()); +} + } // namespace } // namespace Upstream } // namespace Envoy diff --git a/test/mocks/upstream/mocks.h b/test/mocks/upstream/mocks.h index 47b43f31a558..ae6d58e81dc2 100644 --- a/test/mocks/upstream/mocks.h +++ b/test/mocks/upstream/mocks.h @@ -451,6 +451,9 @@ class MockEndpointGroupsManager : public EndpointGroupsManager { bool(const envoy::config::endpoint::v3::EndpointGroup&, absl::string_view)); MOCK_METHOD2(clearEndpointGroup, bool(absl::string_view, absl::string_view)); MOCK_METHOD1(removeEndpointGroup, bool(absl::string_view)); + MOCK_METHOD3(batchUpdateEndpointGroup, + bool(const std::vector&, + const std::vector, absl::string_view)); }; class MockEgdsClusterMapperDelegate : public EgdsClusterMapper::Delegate { From 2b838de4afea0e4b237ce60cf001bb7d50af97b1 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Fri, 21 Feb 2020 10:46:00 +0800 Subject: [PATCH 08/11] Adjust some comments Signed-off-by: leilei.gll --- api/envoy/api/v2/endpoint.proto | 1 - api/envoy/config/endpoint/v3/endpoint.proto | 1 - generated_api_shadow/envoy/api/v2/endpoint.proto | 1 - generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto | 1 - 4 files changed, 4 deletions(-) diff --git a/api/envoy/api/v2/endpoint.proto b/api/envoy/api/v2/endpoint.proto index 7fa2c808e975..211742370285 100644 --- a/api/envoy/api/v2/endpoint.proto +++ b/api/envoy/api/v2/endpoint.proto @@ -121,7 +121,6 @@ message ClusterLoadAssignment { } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index 90a8218d710c..a9481702c34a 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -129,7 +129,6 @@ message ClusterLoadAssignment { } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; diff --git a/generated_api_shadow/envoy/api/v2/endpoint.proto b/generated_api_shadow/envoy/api/v2/endpoint.proto index 7fa2c808e975..211742370285 100644 --- a/generated_api_shadow/envoy/api/v2/endpoint.proto +++ b/generated_api_shadow/envoy/api/v2/endpoint.proto @@ -121,7 +121,6 @@ message ClusterLoadAssignment { } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; diff --git a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto index 90a8218d710c..a9481702c34a 100644 --- a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto +++ b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto @@ -129,7 +129,6 @@ message ClusterLoadAssignment { } // New xDS payload. The size of endpoints is fully arbitrary. -// Also the Egds requests to Pilot can be merged into one to boost efficiency message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; From 1e0b06bab202c620a48754395ef8ddbff389671f Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Tue, 25 Feb 2020 16:50:18 +0800 Subject: [PATCH 09/11] Format the code and adjust some comments Signed-off-by: leilei.gll --- api/envoy/api/v2/egds.proto | 4 ++-- api/envoy/api/v2/endpoint.proto | 4 ++-- api/envoy/config/endpoint/v3/endpoint.proto | 4 ++-- generated_api_shadow/envoy/api/v2/egds.proto | 4 ++-- generated_api_shadow/envoy/api/v2/endpoint.proto | 4 ++-- .../envoy/config/endpoint/v3/endpoint.proto | 4 ++-- source/common/upstream/BUILD | 1 + test/common/upstream/egds_api_impl_test.cc | 6 +++--- test/common/upstream/egds_cluster_mapper_test.cc | 6 ++---- 9 files changed, 18 insertions(+), 19 deletions(-) diff --git a/api/envoy/api/v2/egds.proto b/api/envoy/api/v2/egds.proto index fbd3a9e69367..01852fd8aecb 100644 --- a/api/envoy/api/v2/egds.proto +++ b/api/envoy/api/v2/egds.proto @@ -21,12 +21,12 @@ option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; // [#protodoc-title: EGDS] -// Endpoint discovery :ref:`architecture overview ` +// Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { option (envoy.annotations.resource).type = "envoy.api.v2.EndpointGroup"; - // The resource_names field in DiscoveryRequest specifies a list of clusters + // The resource_names field in DiscoveryRequest specifies a list of endpoint groups // to subscribe to updates for. rpc StreamEndpointGroups(stream DiscoveryRequest) returns (stream DiscoveryResponse) { } diff --git a/api/envoy/api/v2/endpoint.proto b/api/envoy/api/v2/endpoint.proto index 211742370285..23b32535bbeb 100644 --- a/api/envoy/api/v2/endpoint.proto +++ b/api/envoy/api/v2/endpoint.proto @@ -120,7 +120,7 @@ message ClusterLoadAssignment { repeated Egds endpoint_groups = 6; } -// New xDS payload. The size of endpoints is fully arbitrary. +// The endpoint group configuration. message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; @@ -134,7 +134,7 @@ message EndpointGroup { // The EndpointGroup request configuration. message Egds { - // Configuration for the source of EndpointGroup updates for this Cluster. + // Configuration for the source of EndpointGroup. core.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/api/envoy/config/endpoint/v3/endpoint.proto b/api/envoy/config/endpoint/v3/endpoint.proto index a9481702c34a..c2fa98edface 100644 --- a/api/envoy/config/endpoint/v3/endpoint.proto +++ b/api/envoy/config/endpoint/v3/endpoint.proto @@ -128,7 +128,7 @@ message ClusterLoadAssignment { repeated Egds endpoint_groups = 6; } -// New xDS payload. The size of endpoints is fully arbitrary. +// The endpoint group configuration. message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; @@ -147,7 +147,7 @@ message Egds { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; - // Configuration for the source of EndpointGroup updates for this Cluster. + // Configuration for the source of EndpointGroup. core.v3.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/generated_api_shadow/envoy/api/v2/egds.proto b/generated_api_shadow/envoy/api/v2/egds.proto index c43b2c8137ef..09fc7b2626ec 100644 --- a/generated_api_shadow/envoy/api/v2/egds.proto +++ b/generated_api_shadow/envoy/api/v2/egds.proto @@ -21,12 +21,12 @@ option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; // [#protodoc-title: EGDS] -// Endpoint discovery :ref:`architecture overview ` +// Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { option (envoy.annotations.resource).type = "envoy.api.v2.EndpointGroup"; - // The resource_names field in DiscoveryRequest specifies a list of clusters + // The resource_names field in DiscoveryRequest specifies a list of endpoint groups // to subscribe to updates for. rpc StreamEndpointGroups(stream DiscoveryRequest) returns (stream DiscoveryResponse) { } diff --git a/generated_api_shadow/envoy/api/v2/endpoint.proto b/generated_api_shadow/envoy/api/v2/endpoint.proto index 211742370285..23b32535bbeb 100644 --- a/generated_api_shadow/envoy/api/v2/endpoint.proto +++ b/generated_api_shadow/envoy/api/v2/endpoint.proto @@ -120,7 +120,7 @@ message ClusterLoadAssignment { repeated Egds endpoint_groups = 6; } -// New xDS payload. The size of endpoints is fully arbitrary. +// The endpoint group configuration. message EndpointGroup { string name = 1 [(validate.rules).string = {min_bytes: 1}]; @@ -134,7 +134,7 @@ message EndpointGroup { // The EndpointGroup request configuration. message Egds { - // Configuration for the source of EndpointGroup updates for this Cluster. + // Configuration for the source of EndpointGroup. core.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto index a9481702c34a..c2fa98edface 100644 --- a/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto +++ b/generated_api_shadow/envoy/config/endpoint/v3/endpoint.proto @@ -128,7 +128,7 @@ message ClusterLoadAssignment { repeated Egds endpoint_groups = 6; } -// New xDS payload. The size of endpoints is fully arbitrary. +// The endpoint group configuration. message EndpointGroup { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.EndpointGroup"; @@ -147,7 +147,7 @@ message Egds { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.ClusterLoadAssignment.EgdsConfig"; - // Configuration for the source of EndpointGroup updates for this Cluster. + // Configuration for the source of EndpointGroup. core.v3.ConfigSource config_source = 1; // the endpoint group resource name. diff --git a/source/common/upstream/BUILD b/source/common/upstream/BUILD index ba9c4a70489d..3b35ed31b1b4 100644 --- a/source/common/upstream/BUILD +++ b/source/common/upstream/BUILD @@ -653,6 +653,7 @@ envoy_cc_library( "//source/common/network:address_lib", "//source/common/network:utility_lib", "//source/common/protobuf:utility_lib", + "@envoy_api//envoy/api/v2:pkg_cc_proto", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", "@envoy_api//envoy/config/endpoint/v3:pkg_cc_proto", "@envoy_api//envoy/service/discovery/v3:pkg_cc_proto", diff --git a/test/common/upstream/egds_api_impl_test.cc b/test/common/upstream/egds_api_impl_test.cc index ccda854fc0c7..3c52bcca5bac 100644 --- a/test/common/upstream/egds_api_impl_test.cc +++ b/test/common/upstream/egds_api_impl_test.cc @@ -145,7 +145,7 @@ TEST_F(EgdsApiImplTest, onConfigUpdateSuccess) { EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)).Times(0); EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) .WillOnce(Invoke([&](const std::vector& added, - const std::vector removed, absl::string_view) -> bool { + const std::vector removed, absl::string_view) -> bool { EXPECT_EQ(1, added.size()); EXPECT_EQ(0, removed.size()); return true; @@ -169,7 +169,7 @@ TEST_F(EgdsApiImplTest, onConfigUpdateReplace) { EXPECT_CALL(eg_manager_, addOrUpdateEndpointGroup(_, _)).Times(0); EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) .WillOnce(Invoke([&](const std::vector& added, - const std::vector removed, absl::string_view) -> bool { + const std::vector removed, absl::string_view) -> bool { EXPECT_EQ(1, added.size()); EXPECT_EQ(0, removed.size()); return true; @@ -198,7 +198,7 @@ TEST_F(EgdsApiImplTest, onConfigUpdateRemove) { EXPECT_CALL(eg_manager_, clearEndpointGroup(_, _)).Times(0); EXPECT_CALL(eg_manager_, batchUpdateEndpointGroup(_, _, _)) .WillOnce(Invoke([&](const std::vector& added, - const std::vector removed, absl::string_view) -> bool { + const std::vector removed, absl::string_view) -> bool { EXPECT_EQ(0, added.size()); EXPECT_EQ(1, removed.size()); return true; diff --git a/test/common/upstream/egds_cluster_mapper_test.cc b/test/common/upstream/egds_cluster_mapper_test.cc index 807cfd04ad19..834ac8cf0a0e 100644 --- a/test/common/upstream/egds_cluster_mapper_test.cc +++ b/test/common/upstream/egds_cluster_mapper_test.cc @@ -192,10 +192,8 @@ TEST_F(EgdsClusterMapperTest, Basic) { EXPECT_CALL(mock_delegate, initializeCluster(_)).Times(0); EXPECT_CALL(mock_delegate, updateHosts(_, _, _, _, _, _)) .WillOnce(Invoke([&](uint32_t priority, const HostVector& hosts_added, - const HostVector& hosts_removed, - PriorityStateManager&, - LocalityWeightsMap&, - absl::optional) -> void { + const HostVector& hosts_removed, PriorityStateManager&, + LocalityWeightsMap&, absl::optional) -> void { EXPECT_EQ(0, priority); EXPECT_EQ("4.5.6.7:4567", hosts_added.at(0)->address()->asString()); EXPECT_EQ("1.2.3.4:1234", hosts_removed.at(0)->address()->asString()); From 51180a1312d629bf3bba95c4c28bffb734447ef3 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Tue, 25 Feb 2020 17:39:46 +0800 Subject: [PATCH 10/11] Fix spelling errors Signed-off-by: leilei.gll --- api/envoy/api/v2/egds.proto | 2 +- generated_api_shadow/envoy/api/v2/egds.proto | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/envoy/api/v2/egds.proto b/api/envoy/api/v2/egds.proto index 01852fd8aecb..ae9d2ffe6034 100644 --- a/api/envoy/api/v2/egds.proto +++ b/api/envoy/api/v2/egds.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; -// [#protodoc-title: EGDS] +// [#protodoc-title: EG-DS] // Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { diff --git a/generated_api_shadow/envoy/api/v2/egds.proto b/generated_api_shadow/envoy/api/v2/egds.proto index 09fc7b2626ec..1cb50c2f5724 100644 --- a/generated_api_shadow/envoy/api/v2/egds.proto +++ b/generated_api_shadow/envoy/api/v2/egds.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; -// [#protodoc-title: EGDS] +// [#protodoc-title: EG-DS] // Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { From d37fd08f621cf78e47beb0801f70e48133008490 Mon Sep 17 00:00:00 2001 From: "leilei.gll" Date: Tue, 25 Feb 2020 22:41:22 +0800 Subject: [PATCH 11/11] Add egds to the spelling dictionary Signed-off-by: leilei.gll --- api/envoy/api/v2/egds.proto | 2 +- generated_api_shadow/envoy/api/v2/egds.proto | 2 +- tools/spelling_dictionary.txt | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/api/envoy/api/v2/egds.proto b/api/envoy/api/v2/egds.proto index ae9d2ffe6034..01852fd8aecb 100644 --- a/api/envoy/api/v2/egds.proto +++ b/api/envoy/api/v2/egds.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; -// [#protodoc-title: EG-DS] +// [#protodoc-title: EGDS] // Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { diff --git a/generated_api_shadow/envoy/api/v2/egds.proto b/generated_api_shadow/envoy/api/v2/egds.proto index 1cb50c2f5724..09fc7b2626ec 100644 --- a/generated_api_shadow/envoy/api/v2/egds.proto +++ b/generated_api_shadow/envoy/api/v2/egds.proto @@ -20,7 +20,7 @@ option java_multiple_files = true; option java_generic_services = true; option (udpa.annotations.file_migrate).move_to_package = "envoy.service.endpoint.v3"; -// [#protodoc-title: EG-DS] +// [#protodoc-title: EGDS] // Endpoint discovery :ref:`architecture overview ` service EndpointGroupDiscoveryService { diff --git a/tools/spelling_dictionary.txt b/tools/spelling_dictionary.txt index 217468c6db3c..d036c4130f27 100644 --- a/tools/spelling_dictionary.txt +++ b/tools/spelling_dictionary.txt @@ -76,6 +76,7 @@ ECMP ECONNREFUSED EDESTRUCTION EDF +EGDS EINVAL ELB ENOTFOUND