From d8d4aa3e03513283af18e82d9d9a6995db1afc65 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Fri, 18 Jan 2019 00:19:57 -0500 Subject: [PATCH 1/8] upstream: add support for setting degraded through EDS Adds a DEGRADED value that can be set on a host through EDS/LoadAssignment, allowing for a host to be marked degraded without the need for active health checking. Moves the mapping of EDS flag to health flag to inside registerHostForPriority, which means that we're now consistently setting the EDS health flag for EDS/STATIC/STRICT_DNS/LOGICAL_DNS. Simplifies the check for whether the health flag value of a host has changed during EDS updates. Adds tests for the EDS mapping as well as tests to verify that we're honoring the EDS flag for non-EDS cluster types. Signed-off-by: Snow Pettersen --- api/envoy/api/v2/core/health_check.proto | 3 + include/envoy/upstream/upstream.h | 4 +- source/common/upstream/eds.cc | 2 +- source/common/upstream/logical_dns_cluster.cc | 2 +- source/common/upstream/upstream_impl.cc | 85 ++++++++++++------- source/common/upstream/upstream_impl.h | 9 +- test/common/upstream/eds_test.cc | 32 +++++++ test/common/upstream/upstream_impl_test.cc | 65 +++++++++++++- 8 files changed, 159 insertions(+), 43 deletions(-) diff --git a/api/envoy/api/v2/core/health_check.proto b/api/envoy/api/v2/core/health_check.proto index 24082c1f0253e..4c061a45bc167 100644 --- a/api/envoy/api/v2/core/health_check.proto +++ b/api/envoy/api/v2/core/health_check.proto @@ -252,4 +252,7 @@ enum HealthStatus { // Health check timed out. This is part of HDS and is interpreted by Envoy as // *UNHEALTHY*. TIMEOUT = 4; + + // Degraded. + DEGRADED = 5; } diff --git a/include/envoy/upstream/upstream.h b/include/envoy/upstream/upstream.h index 7f846bea2c13e..c9f7863a5a46c 100644 --- a/include/envoy/upstream/upstream.h +++ b/include/envoy/upstream/upstream.h @@ -48,7 +48,9 @@ class Host : virtual public HostDescription { /* The host is currently marked as unhealthy by EDS. */ \ m(FAILED_EDS_HEALTH, 0x04) \ /* The host is currently marked as degraded through active health checking. */ \ - m(DEGRADED_ACTIVE_HC, 0x08) + m(DEGRADED_ACTIVE_HC, 0x08) \ + /* The host is currently marked as degraded by EDS. */ \ + m(DEGRADED_EDS_HEALTH, 0x10) // clang-format on #define DECLARE_ENUM(name, value) name = value, diff --git a/source/common/upstream/eds.cc b/source/common/upstream/eds.cc index 5fee9360c262a..95d8eb19cb666 100644 --- a/source/common/upstream/eds.cc +++ b/source/common/upstream/eds.cc @@ -75,7 +75,7 @@ void EdsClusterImpl::onConfigUpdate(const ResourceVector& resources, const std:: for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { priority_state_manager.registerHostForPriority( "", resolveProtoAddress(lb_endpoint.endpoint().address()), locality_lb_endpoint, - lb_endpoint, Host::HealthFlag::FAILED_EDS_HEALTH); + lb_endpoint); } } diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc index e98ba6fc5c79f..9231ba9625583 100644 --- a/source/common/upstream/logical_dns_cluster.cc +++ b/source/common/upstream/logical_dns_cluster.cc @@ -113,7 +113,7 @@ void LogicalDnsCluster::startResolve() { PriorityStateManager priority_state_manager(*this, local_info_); priority_state_manager.initializePriorityFor(locality_lb_endpoint); priority_state_manager.registerHostForPriority(logical_host_, locality_lb_endpoint, - lbEndpoint(), absl::nullopt); + lbEndpoint()); const uint32_t priority = locality_lb_endpoint.priority(); priority_state_manager.updateClusterPrioritySet( diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index d9600d30fa140..9ab43a42d1aa2 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -170,6 +170,46 @@ parseExtensionProtocolOptions(const envoy::api::v2::Cluster& config) { return options; } +// Updates the health flags for an existing host to match the new host. +// @param updated_host the new host to read health flag values from. +// @param existing_host the host to update. +// @param flag the health flag to update. +// @return bool whether the flag update caused the host health to change. +bool updateHealthFlags(const Host& updated_host, Host& existing_host, Host::HealthFlag flag) { + // Check if the health flag has changed. + if (existing_host.healthFlagGet(flag) != updated_host.healthFlagGet(flag)) { + // Keep track of the previous health value of the host. + const auto previous_health = existing_host.health(); + + if (updated_host.healthFlagGet(flag)) { + existing_host.healthFlagSet(flag); + } else { + existing_host.healthFlagClear(flag); + } + + // Rebuild if changing the flag affected the host health. + return previous_health != existing_host.health(); + } + + return false; +} + +void setEdsHealthFlag(Host& host, envoy::api::v2::core::HealthStatus health_status) { + switch (health_status) { + case envoy::api::v2::core::HealthStatus::UNHEALTHY: + case envoy::api::v2::core::HealthStatus::DRAINING: + case envoy::api::v2::core::HealthStatus::TIMEOUT: + host.healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); + break; + case envoy::api::v2::core::HealthStatus::DEGRADED: + host.healthFlagSet(Host::HealthFlag::DEGRADED_EDS_HEALTH); + break; + default:; + break; + // No health flags should be set. + } +} + } // namespace Host::CreateConnectionData HostImpl::createConnection( @@ -861,32 +901,24 @@ void PriorityStateManager::initializePriorityFor( void PriorityStateManager::registerHostForPriority( const std::string& hostname, Network::Address::InstanceConstSharedPtr address, const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint, - const absl::optional health_checker_flag) { + const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint) { const HostSharedPtr host( new HostImpl(parent_.info(), hostname, address, lb_endpoint.metadata(), lb_endpoint.load_balancing_weight().value(), locality_lb_endpoint.locality(), lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority())); - registerHostForPriority(host, locality_lb_endpoint, lb_endpoint, health_checker_flag); + registerHostForPriority(host, locality_lb_endpoint, lb_endpoint); } void PriorityStateManager::registerHostForPriority( const HostSharedPtr& host, const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint, - const absl::optional health_checker_flag) { + const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint) { const uint32_t priority = locality_lb_endpoint.priority(); // Should be called after initializePriorityFor. ASSERT(priority_state_[priority].first); priority_state_[priority].first->emplace_back(host); - if (health_checker_flag.has_value()) { - const auto& health_status = lb_endpoint.health_status(); - if (health_status == envoy::api::v2::core::HealthStatus::UNHEALTHY || - health_status == envoy::api::v2::core::HealthStatus::DRAINING || - health_status == envoy::api::v2::core::HealthStatus::TIMEOUT) { - priority_state_[priority].first->back()->healthFlagSet(health_checker_flag.value()); - } - } + + setEdsHealthFlag(*priority_state_[priority].first->back(), lb_endpoint.health_status()); } void PriorityStateManager::updateClusterPrioritySet( @@ -980,7 +1012,7 @@ StaticClusterImpl::StaticClusterImpl( for (const auto& lb_endpoint : locality_lb_endpoint.lb_endpoints()) { priority_state_manager_->registerHostForPriority( "", resolveProtoAddress(lb_endpoint.endpoint().address()), locality_lb_endpoint, - lb_endpoint, absl::nullopt); + lb_endpoint); } } } @@ -1067,24 +1099,10 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const HostVector& new_hosts, max_host_weight = host->weight(); } - if (existing_host->second->healthFlagGet(Host::HealthFlag::FAILED_EDS_HEALTH) != - host->healthFlagGet(Host::HealthFlag::FAILED_EDS_HEALTH)) { - // TODO(snowp): To accommodate degraded, this bit should be checking for any changes - // to the health flag, not just healthy vs not healthy. - const bool previously_healthy = existing_host->second->health() == Host::Health::Healthy; - if (host->healthFlagGet(Host::HealthFlag::FAILED_EDS_HEALTH)) { - existing_host->second->healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); - // If the host was previously healthy and we're now unhealthy, we need to - // rebuild. - hosts_changed |= previously_healthy; - } else { - existing_host->second->healthFlagClear(Host::HealthFlag::FAILED_EDS_HEALTH); - // If the host was previously unhealthy and now healthy, we need to - // rebuild. - hosts_changed |= - !previously_healthy && existing_host->second->health() == Host::Health::Healthy; - } - } + hosts_changed |= + updateHealthFlags(*host, *existing_host->second, Host::HealthFlag::FAILED_EDS_HEALTH); + hosts_changed |= + updateHealthFlags(*host, *existing_host->second, Host::HealthFlag::DEGRADED_EDS_HEALTH); // Did metadata change? const bool metadata_changed = !Protobuf::util::MessageDifferencer::Equivalent( @@ -1258,7 +1276,7 @@ void StrictDnsClusterImpl::updateAllHosts(const HostVector& hosts_added, for (const HostSharedPtr& host : target->hosts_) { if (target->locality_lb_endpoint_.priority() == current_priority) { priority_state_manager.registerHostForPriority(host, target->locality_lb_endpoint_, - target->lb_endpoint_, absl::nullopt); + target->lb_endpoint_); } } } @@ -1308,6 +1326,7 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { lb_endpoint_.metadata(), lb_endpoint_.load_balancing_weight().value(), locality_lb_endpoint_.locality(), lb_endpoint_.endpoint().health_check_config(), locality_lb_endpoint_.priority())); + setEdsHealthFlag(*new_hosts.back(), lb_endpoint_.health_status()); } HostVector hosts_added; diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 2f6f7c85adb5b..30cceaf8fad5c 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -206,7 +206,8 @@ class HostImpl : public HostDescriptionImpl, } // Only possible option at this point is that the host is degraded. - ASSERT(health_flags_ == static_cast(HealthFlag::DEGRADED_ACTIVE_HC)); + ASSERT(healthFlagGet(HealthFlag::DEGRADED_ACTIVE_HC) || + healthFlagGet(HealthFlag::DEGRADED_EDS_HEALTH)); return Host::Health::Degraded; } @@ -665,14 +666,12 @@ class PriorityStateManager : protected Logger::Loggable { registerHostForPriority(const std::string& hostname, Network::Address::InstanceConstSharedPtr address, const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint, - const absl::optional health_checker_flag); + const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint); void registerHostForPriority(const HostSharedPtr& host, const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint, - const absl::optional health_checker_flag); + const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint); void updateClusterPrioritySet(const uint32_t priority, HostVectorSharedPtr&& current_hosts, diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 79ebc8b6fa4c6..06bef24adcd4c 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -332,6 +332,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { {envoy::api::v2::core::HealthStatus::UNHEALTHY, Host::Health::Unhealthy}, {envoy::api::v2::core::HealthStatus::DRAINING, Host::Health::Unhealthy}, {envoy::api::v2::core::HealthStatus::TIMEOUT, Host::Health::Unhealthy}, + {envoy::api::v2::core::HealthStatus::DEGRADED, Host::Health::Degraded}, }; int port = 80; @@ -416,6 +417,37 @@ TEST_F(EdsTest, EndpointHealthStatus) { hosts[0]->healthFlagClear(Host::HealthFlag::FAILED_ACTIVE_HC); EXPECT_EQ(Host::Health::Healthy, hosts[0]->health()); } + + const auto rebuild_conter = stats_.counter("cluster.name.update_no_rebuild").value(); + // Now mark host 0 degraded via EDS, it should be degraded. + endpoints->mutable_lb_endpoints(0)->set_health_status( + envoy::api::v2::core::HealthStatus::DEGRADED); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); + { + auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); + EXPECT_EQ(Host::Health::Degraded, hosts[0]->health()); + } + + std::cerr << cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size() << std::endl; + + // We should rebuild the cluster since we went from healthy -> degraded. + EXPECT_EQ(rebuild_conter, stats_.counter("cluster.name.update_no_rebuild").value()); + + // Now mark the host as having been degraded through active hc. + cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0]->healthFlagSet( + Host::HealthFlag::DEGRADED_ACTIVE_HC); + + // Now mark host 0 healthy via EDS, it should still be degraded. + endpoints->mutable_lb_endpoints(0)->set_health_status( + envoy::api::v2::core::HealthStatus::HEALTHY); + VERBOSE_EXPECT_NO_THROW(cluster_->onConfigUpdate(resources, "")); + { + auto& hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); + EXPECT_EQ(Host::Health::Degraded, hosts[0]->health()); + } + + // Since the host health didn't change, expect no rebuild. + EXPECT_EQ(rebuild_conter + 1, stats_.counter("cluster.name.update_no_rebuild").value()); } // Validate that onConfigUpdate() removes endpoints that are marked as healthy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 999c412eb4b03..f3dbdf14088ca 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -463,6 +463,7 @@ TEST(StrictDnsClusterImplTest, LoadAssignmentBasic) { port_value: 11001 health_check_config: port_value: 8000 + health_status: DEGRADED - endpoint: address: socket_address: @@ -532,6 +533,10 @@ TEST(StrictDnsClusterImplTest, LoadAssignmentBasic) { EXPECT_EQ("localhost1", cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[0]->hostname()); EXPECT_EQ("localhost1", cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[1]->hostname()); EXPECT_EQ(100, cluster.prioritySet().hostSetsPerPriority()[0]->overprovisioningFactor()); + EXPECT_EQ(Host::Health::Degraded, + cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[0]->health()); + EXPECT_EQ(Host::Health::Degraded, + cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[1]->health()); // This is the first time we receveived an update for localhost1, we expect to rebuild. EXPECT_EQ(0UL, stats.counter("cluster.name.update_no_rebuild").value()); @@ -590,7 +595,8 @@ TEST(StrictDnsClusterImplTest, LoadAssignmentBasic) { std::list({"127.0.0.3:11001", "10.0.0.1:11002"}), ContainerEq(hostListToAddresses(cluster.prioritySet().hostSetsPerPriority()[0]->hosts()))); - EXPECT_EQ(2UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); + EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); + EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->degradedHosts().size()); EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->hostsPerLocality().get().size()); EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHostsPerLocality().get().size()); @@ -604,7 +610,8 @@ TEST(StrictDnsClusterImplTest, LoadAssignmentBasic) { EXPECT_THAT(std::list({"127.0.0.3:11001", "10.0.0.1:11002", "10.0.0.1:11002"}), ContainerEq(hostListToAddresses(hosts))); - EXPECT_EQ(3UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); + EXPECT_EQ(2UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHosts().size()); + EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->degradedHosts().size()); EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->hostsPerLocality().get().size()); EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->healthyHostsPerLocality().get().size()); @@ -860,6 +867,18 @@ TEST(HostImplTest, HealthFlags) { // If the degraded flag is the only thing set, host is degraded. host->healthFlagClear(Host::HealthFlag::FAILED_ACTIVE_HC); EXPECT_EQ(Host::Health::Degraded, host->health()); + + // If the EDS and active degraded flag is set, host is degraded. + host->healthFlagSet(Host::HealthFlag::DEGRADED_EDS_HEALTH); + EXPECT_EQ(Host::Health::Degraded, host->health()); + + // If only the EDS degraded is set, host is degraded. + host->healthFlagClear(Host::HealthFlag::DEGRADED_ACTIVE_HC); + EXPECT_EQ(Host::Health::Degraded, host->health()); + + // If EDS and failed active hc is set, host is unhealthy. + host->healthFlagSet(Host::HealthFlag::FAILED_ACTIVE_HC); + EXPECT_EQ(Host::Health::Unhealthy, host->health()); } TEST(StaticClusterImplTest, InitialHosts) { @@ -1088,6 +1107,48 @@ TEST(StaticClusterImplTest, LoadAssignmentLocality) { EXPECT_FALSE(cluster.info()->addedViaApi()); } +TEST(StaticClusterImplTest, LoadAssignmentEdsHealth) { + Stats::IsolatedStoreImpl stats; + Ssl::MockContextManager ssl_context_manager; + NiceMock dispatcher; + NiceMock runtime; + NiceMock local_info; + NiceMock random; + const std::string yaml = R"EOF( + name: staticcluster + connect_timeout: 0.25s + type: STATIC + lb_policy: ROUND_ROBIN + load_assignment: + policy: + overprovisioning_factor: 100 + endpoints: + - lb_endpoints: + - endpoint: + address: + socket_address: + address: 10.0.0.1 + port_value: 443 + health_check_config: + port_value: 8000 + health_status: DEGRADED + )EOF"; + + NiceMock cm; + envoy::api::v2::Cluster cluster_config = parseClusterFromV2Yaml(yaml); + Envoy::Stats::ScopePtr scope = stats.createScope(fmt::format( + "cluster.{}.", cluster_config.alt_stat_name().empty() ? cluster_config.name() + : cluster_config.alt_stat_name())); + Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( + ssl_context_manager, *scope, cm, local_info, dispatcher, random, stats); + StaticClusterImpl cluster(cluster_config, runtime, factory_context, std::move(scope), false); + cluster.initialize([] {}); + + EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->degradedHosts().size()); + EXPECT_EQ(Host::Health::Degraded, + cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[0]->health()); +} + TEST(StaticClusterImplTest, AltStatName) { Stats::IsolatedStoreImpl stats; Ssl::MockContextManager ssl_context_manager; From 5f6fe9a60b6ee52ad8e6aaaf3ef5b41bb622f5ff Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Thu, 17 Jan 2019 22:51:01 -0800 Subject: [PATCH 2/8] add admin output for new flag Signed-off-by: Snow Pettersen --- source/server/http/admin.cc | 12 ++++++++---- test/server/http/admin_test.cc | 4 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 19d1e52cdb1da..a6ebba76ada20 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -168,10 +168,14 @@ void setHealthFlag(Upstream::Host::HealthFlag flag, const Upstream::Host& host, host.healthFlagGet(Upstream::Host::HealthFlag::FAILED_OUTLIER_CHECK)); break; case Upstream::Host::HealthFlag::FAILED_EDS_HEALTH: - health_status.set_eds_health_status( - host.healthFlagGet(Upstream::Host::HealthFlag::FAILED_EDS_HEALTH) - ? envoy::api::v2::core::HealthStatus::UNHEALTHY - : envoy::api::v2::core::HealthStatus::HEALTHY); + case Upstream::Host::HealthFlag::DEGRADED_EDS_HEALTH: + if (host.healthFlagGet(Upstream::Host::HealthFlag::FAILED_EDS_HEALTH)) { + health_status.set_eds_health_status(envoy::api::v2::core::HealthStatus::UNHEALTHY); + } else if (host.healthFlagGet(Upstream::Host::HealthFlag::DEGRADED_EDS_HEALTH)) { + health_status.set_eds_health_status(envoy::api::v2::core::HealthStatus::DEGRADED); + } else { + health_status.set_eds_health_status(envoy::api::v2::core::HealthStatus::HEALTHY); + } break; case Upstream::Host::HealthFlag::DEGRADED_ACTIVE_HC: health_status.set_failed_active_degraded_check( diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index ff5b80e104e8d..883fdcf9d51fd 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -1033,6 +1033,8 @@ TEST_P(AdminInstanceTest, ClustersJson) { .WillByDefault(Return(false)); ON_CALL(*host, healthFlagGet(Upstream::Host::HealthFlag::DEGRADED_ACTIVE_HC)) .WillByDefault(Return(true)); + ON_CALL(*host, healthFlagGet(Upstream::Host::HealthFlag::DEGRADED_EDS_HEALTH)) + .WillByDefault(Return(true)); ON_CALL(host->outlier_detector_, successRate()).WillByDefault(Return(43.2)); ON_CALL(*host, weight()).WillByDefault(Return(5)); @@ -1089,7 +1091,7 @@ TEST_P(AdminInstanceTest, ClustersJson) { }, ], "health_status": { - "eds_health_status": "HEALTHY", + "eds_health_status": "DEGRADED", "failed_active_health_check": true, "failed_outlier_check": true, "failed_active_degraded_check": true From c980745944e99791558cec79787ff97980bdca93 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 22 Jan 2019 18:41:50 -0500 Subject: [PATCH 3/8] PR feedback naming changes, remove std::cerrs Signed-off-by: Snow Pettersen --- source/common/upstream/upstream_impl.cc | 6 +++--- test/common/upstream/eds_test.cc | 8 +++----- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 9ab43a42d1aa2..c417f43b579ee 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -175,7 +175,7 @@ parseExtensionProtocolOptions(const envoy::api::v2::Cluster& config) { // @param existing_host the host to update. // @param flag the health flag to update. // @return bool whether the flag update caused the host health to change. -bool updateHealthFlags(const Host& updated_host, Host& existing_host, Host::HealthFlag flag) { +bool updateHealthFlag(const Host& updated_host, Host& existing_host, Host::HealthFlag flag) { // Check if the health flag has changed. if (existing_host.healthFlagGet(flag) != updated_host.healthFlagGet(flag)) { // Keep track of the previous health value of the host. @@ -1100,9 +1100,9 @@ bool BaseDynamicClusterImpl::updateDynamicHostList(const HostVector& new_hosts, } hosts_changed |= - updateHealthFlags(*host, *existing_host->second, Host::HealthFlag::FAILED_EDS_HEALTH); + updateHealthFlag(*host, *existing_host->second, Host::HealthFlag::FAILED_EDS_HEALTH); hosts_changed |= - updateHealthFlags(*host, *existing_host->second, Host::HealthFlag::DEGRADED_EDS_HEALTH); + updateHealthFlag(*host, *existing_host->second, Host::HealthFlag::DEGRADED_EDS_HEALTH); // Did metadata change? const bool metadata_changed = !Protobuf::util::MessageDifferencer::Equivalent( diff --git a/test/common/upstream/eds_test.cc b/test/common/upstream/eds_test.cc index 06bef24adcd4c..a3675f0a75d22 100644 --- a/test/common/upstream/eds_test.cc +++ b/test/common/upstream/eds_test.cc @@ -418,7 +418,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { EXPECT_EQ(Host::Health::Healthy, hosts[0]->health()); } - const auto rebuild_conter = stats_.counter("cluster.name.update_no_rebuild").value(); + const auto rebuild_container = stats_.counter("cluster.name.update_no_rebuild").value(); // Now mark host 0 degraded via EDS, it should be degraded. endpoints->mutable_lb_endpoints(0)->set_health_status( envoy::api::v2::core::HealthStatus::DEGRADED); @@ -428,10 +428,8 @@ TEST_F(EdsTest, EndpointHealthStatus) { EXPECT_EQ(Host::Health::Degraded, hosts[0]->health()); } - std::cerr << cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size() << std::endl; - // We should rebuild the cluster since we went from healthy -> degraded. - EXPECT_EQ(rebuild_conter, stats_.counter("cluster.name.update_no_rebuild").value()); + EXPECT_EQ(rebuild_container, stats_.counter("cluster.name.update_no_rebuild").value()); // Now mark the host as having been degraded through active hc. cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0]->healthFlagSet( @@ -447,7 +445,7 @@ TEST_F(EdsTest, EndpointHealthStatus) { } // Since the host health didn't change, expect no rebuild. - EXPECT_EQ(rebuild_conter + 1, stats_.counter("cluster.name.update_no_rebuild").value()); + EXPECT_EQ(rebuild_container + 1, stats_.counter("cluster.name.update_no_rebuild").value()); } // Validate that onConfigUpdate() removes endpoints that are marked as healthy From 0071ab847e5bbf057a04777b8aed3b9209837de5 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 29 Jan 2019 19:21:44 -0500 Subject: [PATCH 4/8] clean up merge Signed-off-by: Snow Pettersen --- test/common/upstream/upstream_impl_test.cc | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 9685407eb8e20..15f6726dcf673 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1074,13 +1074,7 @@ TEST_F(StaticClusterImplTest, LoadAssignmentLocality) { EXPECT_FALSE(cluster.info()->addedViaApi()); } -TEST(StaticClusterImplTest, LoadAssignmentEdsHealth) { - Stats::IsolatedStoreImpl stats; - Ssl::MockContextManager ssl_context_manager; - NiceMock dispatcher; - NiceMock runtime; - NiceMock local_info; - NiceMock random; +TEST_F(StaticClusterImplTest, LoadAssignmentEdsHealth) { const std::string yaml = R"EOF( name: staticcluster connect_timeout: 0.25s @@ -1107,7 +1101,8 @@ TEST(StaticClusterImplTest, LoadAssignmentEdsHealth) { "cluster.{}.", cluster_config.alt_stat_name().empty() ? cluster_config.name() : cluster_config.alt_stat_name())); Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( - ssl_context_manager, *scope, cm, local_info, dispatcher, random, stats); + admin, ssl_context_manager, *scope, cm, local_info, dispatcher, random, stats, + singleton_manager, tls); StaticClusterImpl cluster(cluster_config, runtime, factory_context, std::move(scope), false); cluster.initialize([] {}); @@ -1116,14 +1111,6 @@ TEST(StaticClusterImplTest, LoadAssignmentEdsHealth) { cluster.prioritySet().hostSetsPerPriority()[0]->hosts()[0]->health()); } -TEST(StaticClusterImplTest, AltStatName) { - Stats::IsolatedStoreImpl stats; - Ssl::MockContextManager ssl_context_manager; - NiceMock runtime; - NiceMock local_info; - NiceMock random; - NiceMock dispatcher; - TEST_F(StaticClusterImplTest, AltStatName) { const std::string yaml = R"EOF( name: staticcluster From aa99b0eff5a61d36864a889acffe71a62fec2467 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 4 Feb 2019 18:28:11 -0500 Subject: [PATCH 5/8] specify FALLTHRU and add integration test coverage Signed-off-by: Snow Pettersen --- source/common/upstream/upstream_impl.cc | 2 ++ test/integration/eds_integration_test.cc | 28 +++++++++++++++++------- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 1224dfa3f279d..3444adde9704c 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -197,7 +197,9 @@ bool updateHealthFlag(const Host& updated_host, Host& existing_host, Host::Healt void setEdsHealthFlag(Host& host, envoy::api::v2::core::HealthStatus health_status) { switch (health_status) { case envoy::api::v2::core::HealthStatus::UNHEALTHY: + FALLTHRU; case envoy::api::v2::core::HealthStatus::DRAINING: + FALLTHRU; case envoy::api::v2::core::HealthStatus::TIMEOUT: host.healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); break; diff --git a/test/integration/eds_integration_test.cc b/test/integration/eds_integration_test.cc index ebfacb27906d1..fde1061852e73 100644 --- a/test/integration/eds_integration_test.cc +++ b/test/integration/eds_integration_test.cc @@ -22,7 +22,9 @@ class EdsIntegrationTest : public HttpIntegrationTest, // We need to supply the endpoints via EDS to provide health status. Use a // filesystem delivery to simplify test mechanics. void setEndpoints(uint32_t total_endpoints, uint32_t healthy_endpoints, + uint32_t degraded_endpoints, absl::optional overprovisioning_factor = absl::nullopt) { + ASSERT(total_endpoints >= healthy_endpoints + degraded_endpoints); envoy::api::v2::ClusterLoadAssignment cluster_load_assignment; cluster_load_assignment.set_cluster_name("cluster_0"); if (overprovisioning_factor.has_value()) { @@ -34,7 +36,11 @@ class EdsIntegrationTest : public HttpIntegrationTest, for (uint32_t i = 0; i < total_endpoints; ++i) { auto* endpoint = locality_lb_endpoints->add_lb_endpoints(); setUpstreamAddress(i, *endpoint); - if (i >= healthy_endpoints) { + // First N endpoints are degraded, next M are healthy and the remaining endpoints are + // unhealthy. + if (i < degraded_endpoints) { + endpoint->set_health_status(envoy::api::v2::core::HealthStatus::DEGRADED); + } else if (i >= healthy_endpoints + degraded_endpoints) { endpoint->set_health_status(envoy::api::v2::core::HealthStatus::UNHEALTHY); } } @@ -52,7 +58,7 @@ class EdsIntegrationTest : public HttpIntegrationTest, eds_cluster_config->mutable_eds_config()->set_path(eds_helper_.eds_path()); }); HttpIntegrationTest::initialize(); - setEndpoints(0, 0); + setEndpoints(0, 0, 0); } EdsHelper eds_helper_; @@ -69,32 +75,38 @@ TEST_P(EdsIntegrationTest, HealthUpdate) { 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); + 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); + 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); + 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); + 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(EdsIntegrationTest, OverprovisioningFactorUpdate) { initialize(); // Default overprovisioning factor. - setEndpoints(4, 4); + 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()); @@ -108,7 +120,7 @@ TEST_P(EdsIntegrationTest, OverprovisioningFactorUpdate) { get_and_compare(Envoy::Upstream::kDefaultOverProvisioningFactor); // Use new overprovisioning factor 200. - setEndpoints(4, 4, 200); + setEndpoints(4, 4, 0, 200); get_and_compare(200); } From 198eb6e5870d57d365c41bae95555df6f47ce454 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Mon, 4 Feb 2019 20:06:57 -0500 Subject: [PATCH 6/8] fix bad merge Signed-off-by: Snow Pettersen --- test/common/upstream/upstream_impl_test.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 9f99c7a815915..19fda26ea74d7 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -1082,13 +1082,13 @@ TEST_F(StaticClusterImplTest, LoadAssignmentEdsHealth) { NiceMock cm; envoy::api::v2::Cluster cluster_config = parseClusterFromV2Yaml(yaml); - Envoy::Stats::ScopePtr scope = stats.createScope(fmt::format( + Envoy::Stats::ScopePtr scope = stats_.createScope(fmt::format( "cluster.{}.", cluster_config.alt_stat_name().empty() ? cluster_config.name() : cluster_config.alt_stat_name())); Envoy::Server::Configuration::TransportSocketFactoryContextImpl factory_context( - admin, ssl_context_manager, *scope, cm, local_info, dispatcher, random, stats, - singleton_manager, tls); - StaticClusterImpl cluster(cluster_config, runtime, factory_context, std::move(scope), false); + admin_, ssl_context_manager_, *scope, cm_, local_info_, dispatcher_, random_, stats_, + singleton_manager_, tls_, *api_); + StaticClusterImpl cluster(cluster_config, runtime_, factory_context, std::move(scope), false); cluster.initialize([] {}); EXPECT_EQ(1UL, cluster.prioritySet().hostSetsPerPriority()[0]->degradedHosts().size()); From 3626ea78615efc66db1f1eeaf97bd8ead538b317 Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Tue, 5 Feb 2019 15:11:47 -0500 Subject: [PATCH 7/8] move setEdsFlag to HostImpl ctor, add test description Signed-off-by: Snow Pettersen --- .../upstream/health_discovery_service.cc | 11 ++--- source/common/upstream/logical_dns_cluster.cc | 3 +- source/common/upstream/logical_dns_cluster.h | 2 +- .../common/upstream/original_dst_cluster.cc | 11 ++--- source/common/upstream/upstream_impl.cc | 42 +++++++++---------- source/common/upstream/upstream_impl.h | 12 +++--- test/common/upstream/upstream_impl_test.cc | 4 +- test/common/upstream/utility.h | 9 ++-- 8 files changed, 49 insertions(+), 45 deletions(-) diff --git a/source/common/upstream/health_discovery_service.cc b/source/common/upstream/health_discovery_service.cc index 624f5b8e86243..ae44def4a796f 100644 --- a/source/common/upstream/health_discovery_service.cc +++ b/source/common/upstream/health_discovery_service.cc @@ -204,11 +204,12 @@ HdsCluster::HdsCluster(Server::Admin& admin, Runtime::Loader& runtime, dispatcher, random, singleton_manager, tls, api}); for (const auto& host : cluster.hosts()) { - initial_hosts_->emplace_back(new HostImpl( - info_, "", Network::Address::resolveProtoAddress(host), - envoy::api::v2::core::Metadata::default_instance(), 1, - envoy::api::v2::core::Locality().default_instance(), - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), 0)); + initial_hosts_->emplace_back( + new HostImpl(info_, "", Network::Address::resolveProtoAddress(host), + envoy::api::v2::core::Metadata::default_instance(), 1, + envoy::api::v2::core::Locality().default_instance(), + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)); } initialize([] {}); } diff --git a/source/common/upstream/logical_dns_cluster.cc b/source/common/upstream/logical_dns_cluster.cc index 9231ba9625583..c5bcc8bbb67df 100644 --- a/source/common/upstream/logical_dns_cluster.cc +++ b/source/common/upstream/logical_dns_cluster.cc @@ -112,8 +112,7 @@ void LogicalDnsCluster::startResolve() { const auto& locality_lb_endpoint = localityLbEndpoint(); PriorityStateManager priority_state_manager(*this, local_info_); priority_state_manager.initializePriorityFor(locality_lb_endpoint); - priority_state_manager.registerHostForPriority(logical_host_, locality_lb_endpoint, - lbEndpoint()); + priority_state_manager.registerHostForPriority(logical_host_, locality_lb_endpoint); const uint32_t priority = locality_lb_endpoint.priority(); priority_state_manager.updateClusterPrioritySet( diff --git a/source/common/upstream/logical_dns_cluster.h b/source/common/upstream/logical_dns_cluster.h index 90479696c4428..b75b7858da03b 100644 --- a/source/common/upstream/logical_dns_cluster.h +++ b/source/common/upstream/logical_dns_cluster.h @@ -47,7 +47,7 @@ class LogicalDnsCluster : public ClusterImplBase { parent.lbEndpoint().load_balancing_weight().value(), parent.localityLbEndpoint().locality(), parent.lbEndpoint().endpoint().health_check_config(), - parent.localityLbEndpoint().priority()), + parent.localityLbEndpoint().priority(), parent.lbEndpoint().health_status()), parent_(parent) {} // Upstream::Host diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc index 2e4232095059e..1b54ca9f4faef 100644 --- a/source/common/upstream/original_dst_cluster.cc +++ b/source/common/upstream/original_dst_cluster.cc @@ -77,11 +77,12 @@ HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerCont Network::Address::InstanceConstSharedPtr host_ip_port( Network::Utility::copyInternetAddressAndPort(*dst_ip)); // Create a host we can use immediately. - host.reset(new HostImpl( - info_, info_->name() + dst_addr.asString(), std::move(host_ip_port), - envoy::api::v2::core::Metadata::default_instance(), 1, - envoy::api::v2::core::Locality().default_instance(), - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), 0)); + host.reset( + new HostImpl(info_, info_->name() + dst_addr.asString(), std::move(host_ip_port), + envoy::api::v2::core::Metadata::default_instance(), 1, + envoy::api::v2::core::Locality().default_instance(), + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), + 0, envoy::api::v2::core::HealthStatus::UNKNOWN)); ENVOY_LOG(debug, "Created host {}.", host->address()->asString()); // Add the new host to the map. We just failed to find it in diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index c72791406f414..e232be55eb270 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -194,17 +194,26 @@ bool updateHealthFlag(const Host& updated_host, Host& existing_host, Host::Healt return false; } -void setEdsHealthFlag(Host& host, envoy::api::v2::core::HealthStatus health_status) { +} // namespace + +Host::CreateConnectionData HostImpl::createConnection( + Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, + Network::TransportSocketOptionsSharedPtr transport_socket_options) const { + return {createConnection(dispatcher, *cluster_, address_, options, transport_socket_options), + shared_from_this()}; +} + +void HostImpl::setEdsHealthFlag(envoy::api::v2::core::HealthStatus health_status) { switch (health_status) { case envoy::api::v2::core::HealthStatus::UNHEALTHY: FALLTHRU; case envoy::api::v2::core::HealthStatus::DRAINING: FALLTHRU; case envoy::api::v2::core::HealthStatus::TIMEOUT: - host.healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); + healthFlagSet(Host::HealthFlag::FAILED_EDS_HEALTH); break; case envoy::api::v2::core::HealthStatus::DEGRADED: - host.healthFlagSet(Host::HealthFlag::DEGRADED_EDS_HEALTH); + healthFlagSet(Host::HealthFlag::DEGRADED_EDS_HEALTH); break; default:; break; @@ -212,15 +221,6 @@ void setEdsHealthFlag(Host& host, envoy::api::v2::core::HealthStatus health_stat } } -} // namespace - -Host::CreateConnectionData HostImpl::createConnection( - Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, - Network::TransportSocketOptionsSharedPtr transport_socket_options) const { - return {createConnection(dispatcher, *cluster_, address_, options, transport_socket_options), - shared_from_this()}; -} - Host::CreateConnectionData HostImpl::createHealthCheckConnection(Event::Dispatcher& dispatcher) const { return {createConnection(dispatcher, *cluster_, healthCheckAddress(), nullptr, nullptr), @@ -909,20 +909,18 @@ void PriorityStateManager::registerHostForPriority( const HostSharedPtr host( new HostImpl(parent_.info(), hostname, address, lb_endpoint.metadata(), lb_endpoint.load_balancing_weight().value(), locality_lb_endpoint.locality(), - lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority())); - registerHostForPriority(host, locality_lb_endpoint, lb_endpoint); + lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority(), + lb_endpoint.health_status())); + registerHostForPriority(host, locality_lb_endpoint); } void PriorityStateManager::registerHostForPriority( const HostSharedPtr& host, - const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint) { + const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint) { const uint32_t priority = locality_lb_endpoint.priority(); // Should be called after initializePriorityFor. ASSERT(priority_state_[priority].first); priority_state_[priority].first->emplace_back(host); - - setEdsHealthFlag(*priority_state_[priority].first->back(), lb_endpoint.health_status()); } void PriorityStateManager::updateClusterPrioritySet( @@ -1279,8 +1277,7 @@ void StrictDnsClusterImpl::updateAllHosts(const HostVector& hosts_added, priority_state_manager.initializePriorityFor(target->locality_lb_endpoint_); for (const HostSharedPtr& host : target->hosts_) { if (target->locality_lb_endpoint_.priority() == current_priority) { - priority_state_manager.registerHostForPriority(host, target->locality_lb_endpoint_, - target->lb_endpoint_); + priority_state_manager.registerHostForPriority(host, target->locality_lb_endpoint_); } } } @@ -1329,8 +1326,7 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { parent_.info_, dns_address_, Network::Utility::getAddressWithPort(*address, port_), lb_endpoint_.metadata(), lb_endpoint_.load_balancing_weight().value(), locality_lb_endpoint_.locality(), lb_endpoint_.endpoint().health_check_config(), - locality_lb_endpoint_.priority())); - setEdsHealthFlag(*new_hosts.back(), lb_endpoint_.health_status()); + locality_lb_endpoint_.priority(), lb_endpoint_.health_status())); } HostVector hosts_added; @@ -1358,4 +1354,4 @@ void StrictDnsClusterImpl::ResolveTarget::startResolve() { } } // namespace Upstream -} // namespace Envoy \ No newline at end of file +} // namespace Envoy diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index 4522e6e63d322..ba40c584a4aa0 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -162,10 +162,11 @@ class HostImpl : public HostDescriptionImpl, const envoy::api::v2::core::Metadata& metadata, uint32_t initial_weight, const envoy::api::v2::core::Locality& locality, const envoy::api::v2::endpoint::Endpoint::HealthCheckConfig& health_check_config, - uint32_t priority) + uint32_t priority, const envoy::api::v2::core::HealthStatus health_status) : HostDescriptionImpl(cluster, hostname, address, metadata, locality, health_check_config, priority), used_(true) { + setEdsHealthFlag(health_status); weight(initial_weight); } @@ -224,6 +225,8 @@ class HostImpl : public HostDescriptionImpl, Network::TransportSocketOptionsSharedPtr transport_socket_options); private: + void setEdsHealthFlag(envoy::api::v2::core::HealthStatus health_status); + std::atomic health_flags_{}; ActiveHealthFailureType active_health_failure_type_{}; std::atomic weight_; @@ -669,10 +672,9 @@ class PriorityStateManager : protected Logger::Loggable { const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint); - void - registerHostForPriority(const HostSharedPtr& host, - const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint, - const envoy::api::v2::endpoint::LbEndpoint& lb_endpoint); + void registerHostForPriority( + const HostSharedPtr& host, + const envoy::api::v2::endpoint::LocalityLbEndpoints& locality_lb_endpoint); void updateClusterPrioritySet(const uint32_t priority, HostVectorSharedPtr&& current_hosts, diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index 19fda26ea74d7..358ce56188502 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -823,7 +823,8 @@ TEST(HostImplTest, HostnameCanaryAndLocality) { locality.set_sub_zone("world"); HostImpl host(cluster.info_, "lyft.com", Network::Utility::resolveUrl("tcp://10.0.0.1:1234"), metadata, 1, locality, - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 1); + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 1, + envoy::api::v2::core::HealthStatus::UNKNOWN); EXPECT_EQ(cluster.info_.get(), &host.cluster()); EXPECT_EQ("lyft.com", host.hostname()); EXPECT_TRUE(host.canary()); @@ -1059,6 +1060,7 @@ TEST_F(StaticClusterImplTest, LoadAssignmentLocality) { EXPECT_FALSE(cluster.info()->addedViaApi()); } +// Validates that setting an EDS health value through LoadAssignment is honored for static clusters. TEST_F(StaticClusterImplTest, LoadAssignmentEdsHealth) { const std::string yaml = R"EOF( name: staticcluster diff --git a/test/common/upstream/utility.h b/test/common/upstream/utility.h index be3373ced54b6..7eec5f8d21954 100644 --- a/test/common/upstream/utility.h +++ b/test/common/upstream/utility.h @@ -79,7 +79,8 @@ inline HostSharedPtr makeTestHost(ClusterInfoConstSharedPtr cluster, const std:: return HostSharedPtr{new HostImpl( cluster, "", Network::Utility::resolveUrl(url), envoy::api::v2::core::Metadata::default_instance(), weight, envoy::api::v2::core::Locality(), - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0)}; + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)}; } inline HostSharedPtr makeTestHost(ClusterInfoConstSharedPtr cluster, const std::string& url, @@ -88,7 +89,8 @@ inline HostSharedPtr makeTestHost(ClusterInfoConstSharedPtr cluster, const std:: return HostSharedPtr{ new HostImpl(cluster, "", Network::Utility::resolveUrl(url), metadata, weight, envoy::api::v2::core::Locality(), - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0)}; + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)}; } inline HostSharedPtr @@ -97,7 +99,8 @@ makeTestHost(ClusterInfoConstSharedPtr cluster, const std::string& url, uint32_t weight = 1) { return HostSharedPtr{new HostImpl(cluster, "", Network::Utility::resolveUrl(url), envoy::api::v2::core::Metadata::default_instance(), weight, - envoy::api::v2::core::Locality(), health_check_config, 0)}; + envoy::api::v2::core::Locality(), health_check_config, 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)}; } inline HostDescriptionConstSharedPtr makeTestHostDescription(ClusterInfoConstSharedPtr cluster, From 44015a1b7617a17b948aab49c013bcc0c0df932b Mon Sep 17 00:00:00 2001 From: Snow Pettersen Date: Wed, 6 Feb 2019 11:03:52 -0500 Subject: [PATCH 8/8] fix load balancer simulation Signed-off-by: Snow Pettersen --- test/common/upstream/load_balancer_simulation_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/common/upstream/load_balancer_simulation_test.cc b/test/common/upstream/load_balancer_simulation_test.cc index 14267d825b685..a02c9674395c7 100644 --- a/test/common/upstream/load_balancer_simulation_test.cc +++ b/test/common/upstream/load_balancer_simulation_test.cc @@ -29,7 +29,8 @@ static HostSharedPtr newTestHost(Upstream::ClusterInfoConstSharedPtr cluster, return HostSharedPtr{ new HostImpl(cluster, "", Network::Utility::resolveUrl(url), envoy::api::v2::core::Metadata::default_instance(), weight, locality, - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0)}; + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig::default_instance(), 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)}; } // Simulate weighted LR load balancer.