Health checks: Add retriable http health check statuses.

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 <envoy_v3_api_field_config.core.v3.HealthCheck.unhealthy_threshold>`,
+    // but will not result in the host being considered immediately unhealthy. Ranges follow half-open semantics of
+    // :ref:`Int64Range <envoy_v3_api_msg_type.v3.Int64Range>`. The start and end of each range are required.
+    // Only statuses in the range [100, 600) are allowed. The :ref:`expected_statuses <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.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 <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.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 <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.expected_statuses>`
+  // or :ref:`retriable_statuses <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.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

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 <envoy_v3_api_msg_config.core.v3.HealthCheck.HttpHealthCheck>`. 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.

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 <envoy_v3_api_field_config.core.v3.HealthCheck.HttpHealthCheck.retriable_statuses>`.
 
 Deprecated
 ----------

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;
   }

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_;
 

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<uint64_t>(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<envoy::type::v3::Int64Range>& expected_statuses,
+    const Protobuf::RepeatedPtrField<envoy::type::v3::Int64Range>& 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<uint64_t>(status_range.start());
+    const auto end = static_cast<uint64_t>(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<uint64_t>(status_range.start());
+    const auto end = static_cast<uint64_t>(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<uint64_t>(start), static_cast<uint64_t>(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<std::pair<uint64_t, uint64_t>>& 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;
   }
 

source/common/upstream/health_checker_impl.h

@@ -62,12 +62,19 @@ class HttpHealthCheckerImpl : public HealthCheckerImplBase {
   public:
     HttpStatusChecker(
         const Protobuf::RepeatedPtrField<envoy::type::v3::Int64Range>& expected_statuses,
+        const Protobuf::RepeatedPtrField<envoy::type::v3::Int64Range>& 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<std::pair<uint64_t, uint64_t>> ranges_;
+    static bool inRanges(uint64_t http_status,
+                         const std::vector<std::pair<uint64_t, uint64_t>>& ranges);
+    static void validateRange(uint64_t start, uint64_t end, absl::string_view range_type);
+
+    std::vector<std::pair<uint64_t, uint64_t>> expected_ranges_;
+    std::vector<std::pair<uint64_t, uint64_t>> 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;