diff --git a/api/envoy/config/core/v3/health_check.proto b/api/envoy/config/core/v3/health_check.proto index 304297e7c011c..81a9b6c7f1535 100644 --- a/api/envoy/config/core/v3/health_check.proto +++ b/api/envoy/config/core/v3/health_check.proto @@ -73,7 +73,7 @@ message HealthCheck { } } - // [#next-free-field: 12] + // [#next-free-field: 13] message HttpHealthCheck { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.core.HealthCheck.HttpHealthCheck"; @@ -118,6 +118,18 @@ message HealthCheck { // range are required. Only statuses in the range [100, 600) are allowed. repeated type.v3.Int64Range expected_statuses = 9; + // Specifies a list of HTTP response statuses considered retriable. If provided, responses in this range + // will count towards the configured :ref:`unhealthy_threshold `, + // but will not result in the host being considered immediately unhealthy. Ranges follow half-open semantics of + // :ref:`Int64Range `. The start and end of each range are required. + // Only statuses in the range [100, 600) are allowed. The :ref:`expected_statuses ` + // field takes precedence for any range overlaps with this field i.e. if status code 200 is both retriable and expected, a 200 response will + // be considered a successful health check. By default all responses not in + // :ref:`expected_statuses ` will result in + // the host being considered immediately unhealthy i.e. if status code 200 is expected and there are no configured retriable statuses, any + // non-200 response will result in the host being marked unhealthy. + repeated type.v3.Int64Range retriable_statuses = 12; + // Use specified application protocol for health checks. type.v3.CodecClientType codec_client_type = 10 [(validate.rules).enum = {defined_only: true}]; @@ -243,8 +255,10 @@ message HealthCheck { uint32 interval_jitter_percent = 18; // The number of unhealthy health checks required before a host is marked - // unhealthy. Note that for *http* health checking if a host responds with 503 - // this threshold is ignored and the host is considered unhealthy immediately. + // unhealthy. Note that for *http* health checking if a host responds with a code not in + // :ref:`expected_statuses ` + // or :ref:`retriable_statuses `, + // this threshold is ignored and the host is considered immediately unhealthy. google.protobuf.UInt32Value unhealthy_threshold = 4 [(validate.rules).message = {required: true}]; // The number of healthy health checks required before a host is marked diff --git a/docs/root/intro/arch_overview/upstream/health_checking.rst b/docs/root/intro/arch_overview/upstream/health_checking.rst index 267542070ef4f..0c3e7596bd01f 100644 --- a/docs/root/intro/arch_overview/upstream/health_checking.rst +++ b/docs/root/intro/arch_overview/upstream/health_checking.rst @@ -12,10 +12,10 @@ checking along with various settings (check interval, failures required before m unhealthy, successes required before marking a host healthy, etc.): * **HTTP**: During HTTP health checking Envoy will send an HTTP request to the upstream host. By - default, it expects a 200 response if the host is healthy. Expected response codes are + default, it expects a 200 response if the host is healthy. Expected and retriable response codes are :ref:`configurable `. The - upstream host can return 503 if it wants to immediately notify downstream hosts to no longer - forward traffic to it. + upstream host can return a non-expected or non-retriable status code (any non-200 code by default) if + it wants to immediately notify downstream hosts to no longer forward traffic to it. * **L3/L4**: During L3/L4 health checking, Envoy will send a configurable byte buffer to the upstream host. It expects the byte buffer to be echoed in the response if the host is to be considered healthy. Envoy also supports connect only L3/L4 health checking. diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 4ce51bfe86592..50376952d647a 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -21,6 +21,7 @@ Removed Config or Runtime New Features ------------ +* http: added support for :ref:`retriable health check status codes `. Deprecated ---------- diff --git a/source/common/upstream/health_checker_base_impl.cc b/source/common/upstream/health_checker_base_impl.cc index e93505b4d619c..da5963e1945d1 100644 --- a/source/common/upstream/health_checker_base_impl.cc +++ b/source/common/upstream/health_checker_base_impl.cc @@ -219,7 +219,7 @@ void HealthCheckerImplBase::setUnhealthyCrossThread(const HostSharedPtr& host, return; } - session->second->setUnhealthy(envoy::data::core::v3::PASSIVE); + session->second->setUnhealthy(envoy::data::core::v3::PASSIVE, /*retriable=*/false); }); } @@ -338,13 +338,14 @@ bool networkHealthCheckFailureType(envoy::data::core::v3::HealthCheckFailureType } // namespace HealthTransition HealthCheckerImplBase::ActiveHealthCheckSession::setUnhealthy( - envoy::data::core::v3::HealthCheckFailureType type) { + envoy::data::core::v3::HealthCheckFailureType type, bool retriable) { // If we are unhealthy, reset the # of healthy to zero. num_healthy_ = 0; HealthTransition changed_state = HealthTransition::Unchanged; if (!host_->healthFlagGet(Host::HealthFlag::FAILED_ACTIVE_HC)) { - if (!networkHealthCheckFailureType(type) || ++num_unhealthy_ == parent_.unhealthy_threshold_) { + if ((!networkHealthCheckFailureType(type) && !retriable) || + ++num_unhealthy_ == parent_.unhealthy_threshold_) { host_->healthFlagSet(Host::HealthFlag::FAILED_ACTIVE_HC); parent_.decHealthy(); changed_state = HealthTransition::Changed; @@ -385,8 +386,8 @@ HealthTransition HealthCheckerImplBase::ActiveHealthCheckSession::setUnhealthy( } void HealthCheckerImplBase::ActiveHealthCheckSession::handleFailure( - envoy::data::core::v3::HealthCheckFailureType type) { - HealthTransition changed_state = setUnhealthy(type); + envoy::data::core::v3::HealthCheckFailureType type, bool retriable) { + HealthTransition changed_state = setUnhealthy(type, retriable); // It's possible that the previous call caused this session to be deferred deleted. if (timeout_timer_ != nullptr) { timeout_timer_->disableTimer(); @@ -401,7 +402,7 @@ HealthTransition HealthCheckerImplBase::ActiveHealthCheckSession::clearPendingFlag(HealthTransition changed_state) { if (host_->healthFlagGet(Host::HealthFlag::PENDING_ACTIVE_HC)) { host_->healthFlagClear(Host::HealthFlag::PENDING_ACTIVE_HC); - // Even though the health value of the host might have not changed, we set this to Changed to + // Even though the health value of the host might have not changed, we set this to Changed so // that the cluster can update its list of excluded hosts. return HealthTransition::Changed; } diff --git a/source/common/upstream/health_checker_base_impl.h b/source/common/upstream/health_checker_base_impl.h index 5081aac7351d2..780e8af0777d3 100644 --- a/source/common/upstream/health_checker_base_impl.h +++ b/source/common/upstream/health_checker_base_impl.h @@ -76,7 +76,8 @@ class HealthCheckerImplBase : public HealthChecker, class ActiveHealthCheckSession : public Event::DeferredDeletable { public: ~ActiveHealthCheckSession() override; - HealthTransition setUnhealthy(envoy::data::core::v3::HealthCheckFailureType type); + HealthTransition setUnhealthy(envoy::data::core::v3::HealthCheckFailureType type, + bool retriable); void onDeferredDeleteBase(); void start() { onInitialInterval(); } @@ -85,7 +86,7 @@ class HealthCheckerImplBase : public HealthChecker, void handleSuccess(bool degraded = false); void handleDegraded(); - void handleFailure(envoy::data::core::v3::HealthCheckFailureType type); + void handleFailure(envoy::data::core::v3::HealthCheckFailureType type, bool retriable = false); HostSharedPtr host_; diff --git a/source/common/upstream/health_checker_impl.cc b/source/common/upstream/health_checker_impl.cc index 09d8ebeca47e8..6bb52b6b90ca0 100644 --- a/source/common/upstream/health_checker_impl.cc +++ b/source/common/upstream/health_checker_impl.cc @@ -138,6 +138,7 @@ HttpHealthCheckerImpl::HttpHealthCheckerImpl(const Cluster& cluster, Router::HeaderParser::configure(config.http_health_check().request_headers_to_add(), config.http_health_check().request_headers_to_remove())), http_status_checker_(config.http_health_check().expected_statuses(), + config.http_health_check().retriable_statuses(), static_cast(Http::Code::OK)), codec_client_type_(codecClientType(config.http_health_check().codec_client_type())), random_generator_(random) { @@ -148,37 +149,63 @@ HttpHealthCheckerImpl::HttpHealthCheckerImpl(const Cluster& cluster, HttpHealthCheckerImpl::HttpStatusChecker::HttpStatusChecker( const Protobuf::RepeatedPtrField& expected_statuses, + const Protobuf::RepeatedPtrField& retriable_statuses, uint64_t default_expected_status) { for (const auto& status_range : expected_statuses) { - const auto start = status_range.start(); - const auto end = status_range.end(); + const auto start = static_cast(status_range.start()); + const auto end = static_cast(status_range.end()); - if (start >= end) { - throw EnvoyException(fmt::format( - "Invalid http status range: expecting start < end, but found start={} and end={}", start, - end)); - } + validateRange(start, end, "expected"); - if (start < 100) { - throw EnvoyException(fmt::format( - "Invalid http status range: expecting start >= 100, but found start={}", start)); - } + expected_ranges_.emplace_back(std::make_pair(start, end)); + } - if (end > 600) { - throw EnvoyException( - fmt::format("Invalid http status range: expecting end <= 600, but found end={}", end)); - } + if (expected_ranges_.empty()) { + expected_ranges_.emplace_back( + std::make_pair(default_expected_status, default_expected_status + 1)); + } + + for (const auto& status_range : retriable_statuses) { + const auto start = static_cast(status_range.start()); + const auto end = static_cast(status_range.end()); + + validateRange(start, end, "retriable"); + + retriable_ranges_.emplace_back(std::make_pair(start, end)); + } +} + +void HttpHealthCheckerImpl::HttpStatusChecker::validateRange(uint64_t start, uint64_t end, + absl::string_view range_type) { + if (start >= end) { + throw EnvoyException(fmt::format("Invalid http {} status range: expecting start < " + "end, but found start={} and end={}", + range_type, start, end)); + } - ranges_.emplace_back(std::make_pair(static_cast(start), static_cast(end))); + if (start < 100) { + throw EnvoyException( + fmt::format("Invalid http {} status range: expecting start >= 100, but found start={}", + range_type, start)); } - if (ranges_.empty()) { - ranges_.emplace_back(std::make_pair(default_expected_status, default_expected_status + 1)); + if (end > 600) { + throw EnvoyException(fmt::format( + "Invalid http {} status range: expecting end <= 600, but found end={}", range_type, end)); } } -bool HttpHealthCheckerImpl::HttpStatusChecker::inRange(uint64_t http_status) const { - for (const auto& range : ranges_) { +bool HttpHealthCheckerImpl::HttpStatusChecker::inRetriableRanges(uint64_t http_status) const { + return inRanges(http_status, retriable_ranges_); +} + +bool HttpHealthCheckerImpl::HttpStatusChecker::inExpectedRanges(uint64_t http_status) const { + return inRanges(http_status, expected_ranges_); +} + +bool HttpHealthCheckerImpl::HttpStatusChecker::inRanges( + uint64_t http_status, const std::vector>& ranges) { + for (const auto& range : ranges) { if (http_status >= range.first && http_status < range.second) { return true; } @@ -331,7 +358,7 @@ HttpHealthCheckerImpl::HttpActiveHealthCheckSession::healthCheckResult() { ENVOY_CONN_LOG(debug, "hc response={} health_flags={}", *client_, response_code, HostUtility::healthFlagsToString(*host_)); - if (!parent_.http_status_checker_.inRange(response_code)) { + if (!parent_.http_status_checker_.inExpectedRanges(response_code)) { // If the HTTP response code would indicate failure AND the immediate health check // failure header is set, exclude the host from LB. // TODO(mattklein123): We could consider doing this check for any HTTP response code, but this @@ -341,7 +368,12 @@ HttpHealthCheckerImpl::HttpActiveHealthCheckSession::healthCheckResult() { if (response_headers_->EnvoyImmediateHealthCheckFail() != nullptr) { host_->healthFlagSet(Host::HealthFlag::EXCLUDED_VIA_IMMEDIATE_HC_FAIL); } - return HealthCheckResult::Failed; + + if (parent_.http_status_checker_.inRetriableRanges(response_code)) { + return HealthCheckResult::Retriable; + } else { + return HealthCheckResult::Failed; + } } const auto degraded = response_headers_->EnvoyDegraded() != nullptr; @@ -374,7 +406,10 @@ void HttpHealthCheckerImpl::HttpActiveHealthCheckSession::onResponseComplete() { handleSuccess(true); break; case HealthCheckResult::Failed: - handleFailure(envoy::data::core::v3::ACTIVE); + handleFailure(envoy::data::core::v3::ACTIVE, /*retriable=*/false); + break; + case HealthCheckResult::Retriable: + handleFailure(envoy::data::core::v3::ACTIVE, /*retriable=*/true); break; } diff --git a/source/common/upstream/health_checker_impl.h b/source/common/upstream/health_checker_impl.h index 35a564f6118b9..cb8d62a3d4a61 100644 --- a/source/common/upstream/health_checker_impl.h +++ b/source/common/upstream/health_checker_impl.h @@ -62,12 +62,19 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase { public: HttpStatusChecker( const Protobuf::RepeatedPtrField& expected_statuses, + const Protobuf::RepeatedPtrField& retriable_statuses, uint64_t default_expected_status); - bool inRange(uint64_t http_status) const; + bool inRetriableRanges(uint64_t http_status) const; + bool inExpectedRanges(uint64_t http_status) const; private: - std::vector> ranges_; + static bool inRanges(uint64_t http_status, + const std::vector>& ranges); + static void validateRange(uint64_t start, uint64_t end, absl::string_view range_type); + + std::vector> expected_ranges_; + std::vector> retriable_ranges_; }; private: @@ -78,7 +85,7 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase { ~HttpActiveHealthCheckSession() override; void onResponseComplete(); - enum class HealthCheckResult { Succeeded, Degraded, Failed }; + enum class HealthCheckResult { Succeeded, Degraded, Failed, Retriable }; HealthCheckResult healthCheckResult(); bool shouldClose() const; diff --git a/test/common/upstream/health_checker_impl_test.cc b/test/common/upstream/health_checker_impl_test.cc index cc5fa210e69b5..ba2dfe80fce2e 100644 --- a/test/common/upstream/health_checker_impl_test.cc +++ b/test/common/upstream/health_checker_impl_test.cc @@ -3135,14 +3135,17 @@ TEST(HttpStatusChecker, Default) { path: /healthcheck )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200); + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); - EXPECT_TRUE(http_status_checker.inRange(200)); - EXPECT_FALSE(http_status_checker.inRange(204)); + EXPECT_TRUE(http_status_checker.inExpectedRanges(200)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(204)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(200)); } -TEST(HttpStatusChecker, Single100) { +TEST(HttpStatusChecker, SingleExpected100) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3157,17 +3160,44 @@ TEST(HttpStatusChecker, Single100) { end: 101 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200); + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); - EXPECT_FALSE(http_status_checker.inRange(200)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(200)); - EXPECT_FALSE(http_status_checker.inRange(99)); - EXPECT_TRUE(http_status_checker.inRange(100)); - EXPECT_FALSE(http_status_checker.inRange(101)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(99)); + EXPECT_TRUE(http_status_checker.inExpectedRanges(100)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(101)); } -TEST(HttpStatusChecker, Single599) { +TEST(HttpStatusChecker, SingleRetriable100) { + const std::string yaml = R"EOF( + timeout: 1s + interval: 1s + unhealthy_threshold: 2 + healthy_threshold: 2 + http_health_check: + service_name_matcher: + prefix: locations + path: /healthcheck + retriable_statuses: + - start: 100 + end: 101 + )EOF"; + + auto conf = parseHealthCheckFromV3Yaml(yaml); + HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); + + EXPECT_FALSE(http_status_checker.inRetriableRanges(99)); + EXPECT_TRUE(http_status_checker.inRetriableRanges(100)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(101)); +} + +TEST(HttpStatusChecker, SingleExpected599) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3182,17 +3212,44 @@ TEST(HttpStatusChecker, Single599) { end: 600 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200); + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); + + EXPECT_FALSE(http_status_checker.inExpectedRanges(200)); + + EXPECT_FALSE(http_status_checker.inExpectedRanges(598)); + EXPECT_TRUE(http_status_checker.inExpectedRanges(599)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(600)); +} + +TEST(HttpStatusChecker, SingleRetriable599) { + const std::string yaml = R"EOF( + timeout: 1s + interval: 1s + unhealthy_threshold: 2 + healthy_threshold: 2 + http_health_check: + service_name_matcher: + prefix: locations + path: /healthcheck + retriable_statuses: + - start: 599 + end: 600 + )EOF"; - EXPECT_FALSE(http_status_checker.inRange(200)); + auto conf = parseHealthCheckFromV3Yaml(yaml); + HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); - EXPECT_FALSE(http_status_checker.inRange(598)); - EXPECT_TRUE(http_status_checker.inRange(599)); - EXPECT_FALSE(http_status_checker.inRange(600)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(598)); + EXPECT_TRUE(http_status_checker.inRetriableRanges(599)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(600)); } -TEST(HttpStatusChecker, Ranges_204_304) { +TEST(HttpStatusChecker, ExpectedRanges_204_304) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3209,20 +3266,52 @@ TEST(HttpStatusChecker, Ranges_204_304) { end: 305 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200); + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); + + EXPECT_FALSE(http_status_checker.inExpectedRanges(200)); + + EXPECT_FALSE(http_status_checker.inExpectedRanges(203)); + EXPECT_TRUE(http_status_checker.inExpectedRanges(204)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(205)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(303)); + EXPECT_TRUE(http_status_checker.inExpectedRanges(304)); + EXPECT_FALSE(http_status_checker.inExpectedRanges(305)); +} + +TEST(HttpStatusChecker, RetriableRanges_304_404) { + const std::string yaml = R"EOF( + timeout: 1s + interval: 1s + unhealthy_threshold: 2 + healthy_threshold: 2 + http_health_check: + service_name_matcher: + prefix: locations + path: /healthcheck + retriable_statuses: + - start: 304 + end: 305 + - start: 404 + end: 405 + )EOF"; - EXPECT_FALSE(http_status_checker.inRange(200)); + auto conf = parseHealthCheckFromV3Yaml(yaml); + HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( + conf.http_health_check().expected_statuses(), conf.http_health_check().retriable_statuses(), + 200); - EXPECT_FALSE(http_status_checker.inRange(203)); - EXPECT_TRUE(http_status_checker.inRange(204)); - EXPECT_FALSE(http_status_checker.inRange(205)); - EXPECT_FALSE(http_status_checker.inRange(303)); - EXPECT_TRUE(http_status_checker.inRange(304)); - EXPECT_FALSE(http_status_checker.inRange(305)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(303)); + EXPECT_TRUE(http_status_checker.inRetriableRanges(304)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(305)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(403)); + EXPECT_TRUE(http_status_checker.inRetriableRanges(404)); + EXPECT_FALSE(http_status_checker.inRetriableRanges(405)); } -TEST(HttpStatusChecker, Below100) { +TEST(HttpStatusChecker, ExpectedBelow100) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3237,13 +3326,40 @@ TEST(HttpStatusChecker, Below100) { end: 100 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); + EXPECT_THROW_WITH_MESSAGE( + HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), + EnvoyException, + "Invalid http expected status range: expecting start >= 100, but found start=99"); +} + +TEST(HttpStatusChecker, RetriableBelow100) { + const std::string yaml = R"EOF( + timeout: 1s + interval: 1s + unhealthy_threshold: 2 + healthy_threshold: 2 + http_health_check: + service_name_matcher: + prefix: locations + path: /healthcheck + retriable_statuses: + - start: 99 + end: 100 + )EOF"; + + auto conf = parseHealthCheckFromV3Yaml(yaml); EXPECT_THROW_WITH_MESSAGE( HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200), - EnvoyException, "Invalid http status range: expecting start >= 100, but found start=99"); + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), + EnvoyException, + "Invalid http retriable status range: expecting start >= 100, but found start=99"); } -TEST(HttpStatusChecker, Above599) { +TEST(HttpStatusChecker, ExpectedAbove599) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3258,13 +3374,16 @@ TEST(HttpStatusChecker, Above599) { end: 601 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); EXPECT_THROW_WITH_MESSAGE( HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200), - EnvoyException, "Invalid http status range: expecting end <= 600, but found end=601"); + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), + EnvoyException, + "Invalid http expected status range: expecting end <= 600, but found end=601"); } -TEST(HttpStatusChecker, InvalidRange) { +TEST(HttpStatusChecker, RetriableAbove599) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3274,19 +3393,21 @@ TEST(HttpStatusChecker, InvalidRange) { service_name_matcher: prefix: locations path: /healthchecka - expected_statuses: - - start: 200 - end: 200 + retriable_statuses: + - start: 600 + end: 601 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); EXPECT_THROW_WITH_MESSAGE( HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200), + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), EnvoyException, - "Invalid http status range: expecting start < end, but found start=200 and end=200"); + "Invalid http retriable status range: expecting end <= 600, but found end=601"); } -TEST(HttpStatusChecker, InvalidRange2) { +TEST(HttpStatusChecker, InvalidExpectedRange) { const std::string yaml = R"EOF( timeout: 1s interval: 1s @@ -3297,15 +3418,41 @@ TEST(HttpStatusChecker, InvalidRange2) { prefix: locations path: /healthchecka expected_statuses: - - start: 201 + - start: 200 end: 200 )EOF"; + auto conf = parseHealthCheckFromV3Yaml(yaml); EXPECT_THROW_WITH_MESSAGE( HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( - parseHealthCheckFromV3Yaml(yaml).http_health_check().expected_statuses(), 200), + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), EnvoyException, - "Invalid http status range: expecting start < end, but found start=201 and end=200"); + "Invalid http expected status range: expecting start < end, but found start=200 and end=200"); +} + +TEST(HttpStatusChecker, InvalidRetriableRange) { + const std::string yaml = R"EOF( + timeout: 1s + interval: 1s + unhealthy_threshold: 2 + healthy_threshold: 2 + http_health_check: + service_name_matcher: + prefix: locations + path: /healthchecka + retriable_statuses: + - start: 200 + end: 200 + )EOF"; + + auto conf = parseHealthCheckFromV3Yaml(yaml); + EXPECT_THROW_WITH_MESSAGE(HttpHealthCheckerImpl::HttpStatusChecker http_status_checker( + conf.http_health_check().expected_statuses(), + conf.http_health_check().retriable_statuses(), 200), + EnvoyException, + "Invalid http retriable status range: expecting start < end, but found " + "start=200 and end=200"); } TEST(TcpHealthCheckMatcher, loadJsonBytes) { diff --git a/test/integration/BUILD b/test/integration/BUILD index 5e631735ef7f2..ae7c693403694 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1751,6 +1751,7 @@ envoy_cc_test( "//test/common/http/http2:http2_frame", "//test/config:v2_link_hacks", "@envoy_api//envoy/config/core/v3:pkg_cc_proto", + "@envoy_api//envoy/type/v3:pkg_cc_proto", ], ) diff --git a/test/integration/health_check_integration_test.cc b/test/integration/health_check_integration_test.cc index dcd6efdee9156..ed44be1f05954 100644 --- a/test/integration/health_check_integration_test.cc +++ b/test/integration/health_check_integration_test.cc @@ -1,6 +1,7 @@ #include #include "envoy/config/core/v3/health_check.pb.h" +#include "envoy/type/v3/range.pb.h" #include "test/common/grpc/grpc_client_integration.h" #include "test/common/http/http2/http2_frame.h" @@ -170,7 +171,8 @@ class HttpHealthCheckIntegrationTestBase // Adds a HTTP active health check specifier to the given cluster, and waits for the first health // check probe to be received. - void initHttpHealthCheck(uint32_t cluster_idx) { + void initHttpHealthCheck(uint32_t cluster_idx, int unhealthy_threshold = 1, + std::unique_ptr retriable_range = nullptr) { const envoy::type::v3::CodecClientType codec_client_type = (Http::CodecType::HTTP1 == upstream_protocol_) ? envoy::type::v3::CodecClientType::HTTP1 : envoy::type::v3::CodecClientType::HTTP2; @@ -179,6 +181,12 @@ class HttpHealthCheckIntegrationTestBase auto* health_check = addHealthCheck(cluster_data.cluster_); health_check->mutable_http_health_check()->set_path("/healthcheck"); health_check->mutable_http_health_check()->set_codec_client_type(codec_client_type); + health_check->mutable_unhealthy_threshold()->set_value(unhealthy_threshold); + if (retriable_range != nullptr) { + auto* range = health_check->mutable_http_health_check()->add_retriable_statuses(); + range->set_start(retriable_range->start()); + range->set_end(retriable_range->end()); + } // Introduce the cluster using compareDiscoveryRequest / sendDiscoveryResponse. EXPECT_TRUE(compareDiscoveryRequest(Config::TypeUrl::get().Cluster, "", {}, {}, {}, true)); @@ -246,6 +254,117 @@ TEST_P(HttpHealthCheckIntegrationTest, SingleEndpointUnhealthyHttp) { EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.health_check.failure")->value()); } +// Tests that a retriable status response does not mark endpoint unhealthy until threshold is +// reached +TEST_P(HttpHealthCheckIntegrationTest, SingleEndpointUnhealthyThresholdHttp) { + const uint32_t cluster_idx = 0; + initialize(); + auto retriable_range = std::make_unique(); + retriable_range->set_start(400); + retriable_range->set_end(401); + initHttpHealthCheck(cluster_idx, 2, std::move(retriable_range)); + + // Responds with healthy status. + clusters_[cluster_idx].host_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "200"}}, false); + clusters_[cluster_idx].host_stream_->encodeData(0, true); + + // Wait for health check + test_server_->waitForCounterEq("cluster.cluster_1.health_check.attempt", 1); + test_server_->waitForCounterEq("cluster.cluster_1.health_check.success", 1); + EXPECT_EQ(0, test_server_->counter("cluster.cluster_1.health_check.failure")->value()); + test_server_->waitForGaugeEq("cluster.cluster_1.membership_healthy", 1); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_total")->value()); + + // Wait until the next attempt is made. + test_server_->waitForCounterEq("cluster.cluster_1.health_check.attempt", 2); + + // Respond with retriable status + ASSERT_TRUE(clusters_[cluster_idx].host_fake_connection_->waitForNewStream( + *dispatcher_, clusters_[cluster_idx].host_stream_)); + ASSERT_TRUE(clusters_[cluster_idx].host_stream_->waitForEndStream(*dispatcher_)); + + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getPathValue(), "/healthcheck"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getMethodValue(), "GET"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getHostValue(), + clusters_[cluster_idx].name_); + clusters_[cluster_idx].host_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "400"}}, false); + clusters_[cluster_idx].host_stream_->encodeData(0, true); + + // Wait for second health check + test_server_->waitForCounterEq("cluster.cluster_1.health_check.failure", 1); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.health_check.success")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_total")->value()); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_healthy")->value()); + + // Wait until the next attempt is made. + test_server_->waitForCounterEq("cluster.cluster_1.health_check.attempt", 3); + + // Respond with retriable status a second time, matching unhealthy threshold + ASSERT_TRUE(clusters_[cluster_idx].host_fake_connection_->waitForNewStream( + *dispatcher_, clusters_[cluster_idx].host_stream_)); + ASSERT_TRUE(clusters_[cluster_idx].host_stream_->waitForEndStream(*dispatcher_)); + + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getPathValue(), "/healthcheck"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getMethodValue(), "GET"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getHostValue(), + clusters_[cluster_idx].name_); + clusters_[cluster_idx].host_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "400"}}, false); + clusters_[cluster_idx].host_stream_->encodeData(0, true); + + // Wait for third health check + test_server_->waitForCounterEq("cluster.cluster_1.health_check.failure", 2); + EXPECT_EQ(1, test_server_->counter("cluster.cluster_1.health_check.success")->value()); + test_server_->waitForGaugeEq("cluster.cluster_1.membership_healthy", 0); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_total")->value()); + + // Wait until the next attempt is made. + test_server_->waitForCounterEq("cluster.cluster_1.health_check.attempt", 4); + + // Respond with healthy status again. + ASSERT_TRUE(clusters_[cluster_idx].host_fake_connection_->waitForNewStream( + *dispatcher_, clusters_[cluster_idx].host_stream_)); + ASSERT_TRUE(clusters_[cluster_idx].host_stream_->waitForEndStream(*dispatcher_)); + + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getPathValue(), "/healthcheck"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getMethodValue(), "GET"); + EXPECT_EQ(clusters_[cluster_idx].host_stream_->headers().getHostValue(), + clusters_[cluster_idx].name_); + clusters_[cluster_idx].host_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "200"}}, false); + clusters_[cluster_idx].host_stream_->encodeData(0, true); + + // Wait for fourth health check + test_server_->waitForCounterEq("cluster.cluster_1.health_check.success", 2); + EXPECT_EQ(2, test_server_->counter("cluster.cluster_1.health_check.failure")->value()); + test_server_->waitForGaugeEq("cluster.cluster_1.membership_healthy", 1); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_total")->value()); +} + +// Tests that expected statuses takes precedence over retriable statuses +TEST_P(HttpHealthCheckIntegrationTest, SingleEndpointExpectedAndRetriablePrecedence) { + const uint32_t cluster_idx = 0; + initialize(); + auto retriable_range = std::make_unique(); + retriable_range->set_start(200); + retriable_range->set_end(201); + initHttpHealthCheck(cluster_idx, 2, std::move(retriable_range)); + + // Responds with healthy status. + clusters_[cluster_idx].host_stream_->encodeHeaders( + Http::TestResponseHeaderMapImpl{{":status", "200"}}, false); + clusters_[cluster_idx].host_stream_->encodeData(0, true); + + // Wait for health check + test_server_->waitForCounterEq("cluster.cluster_1.health_check.attempt", 1); + test_server_->waitForCounterEq("cluster.cluster_1.health_check.success", 1); + EXPECT_EQ(0, test_server_->counter("cluster.cluster_1.health_check.failure")->value()); + test_server_->waitForGaugeEq("cluster.cluster_1.membership_healthy", 1); + EXPECT_EQ(1, test_server_->gauge("cluster.cluster_1.membership_total")->value()); +} + // Verify that immediate health check fail causes cluster exclusion. TEST_P(HttpHealthCheckIntegrationTest, SingleEndpointImmediateHealthcheckFailHttp) { const uint32_t cluster_idx = 0;