diff --git a/source/extensions/filters/http/health_check/health_check.cc b/source/extensions/filters/http/health_check/health_check.cc index ad7435362a2d4..f48036863048d 100644 --- a/source/extensions/filters/http/health_check/health_check.cc +++ b/source/extensions/filters/http/health_check/health_check.cc @@ -28,7 +28,7 @@ HealthCheckCacheManager::HealthCheckCacheManager(Event::Dispatcher& dispatcher, } void HealthCheckCacheManager::onTimer() { - use_cached_response_code_ = false; + use_cached_response_ = false; clear_cache_timer_->enableTimer(timeout_); } @@ -47,7 +47,7 @@ Http::FilterHeadersStatus HealthCheckFilter::decodeHeaders(Http::HeaderMap& head // If we are not in pass through mode, we always handle. Otherwise, we handle if the server is // in the failed state or if we are using caching and we should use the cached response. if (!pass_through_mode_ || context_.healthCheckFailed() || - (cache_manager_ && cache_manager_->useCachedResponseCode())) { + (cache_manager_ && cache_manager_->useCachedResponse())) { handling_ = true; } } @@ -80,8 +80,9 @@ Http::FilterTrailersStatus HealthCheckFilter::decodeTrailers(Http::HeaderMap&) { Http::FilterHeadersStatus HealthCheckFilter::encodeHeaders(Http::HeaderMap& headers, bool) { if (health_check_request_) { if (cache_manager_) { - cache_manager_->setCachedResponseCode( - static_cast(Http::Utility::getResponseStatus(headers))); + cache_manager_->setCachedResponse( + static_cast(Http::Utility::getResponseStatus(headers)), + headers.EnvoyDegraded() != nullptr); } headers.insertEnvoyUpstreamHealthCheckedCluster().value(context_.localInfo().clusterName()); @@ -96,12 +97,15 @@ Http::FilterHeadersStatus HealthCheckFilter::encodeHeaders(Http::HeaderMap& head void HealthCheckFilter::onComplete() { ASSERT(handling_); Http::Code final_status = Http::Code::OK; + bool degraded = false; if (context_.healthCheckFailed()) { callbacks_->streamInfo().setResponseFlag(StreamInfo::ResponseFlag::FailedLocalHealthCheck); final_status = Http::Code::ServiceUnavailable; } else { if (cache_manager_) { - final_status = cache_manager_->getCachedResponseCode(); + const auto status_and_degraded = cache_manager_->getCachedResponse(); + final_status = status_and_degraded.first; + degraded = status_and_degraded.second; } else if (cluster_min_healthy_percentages_ != nullptr && !cluster_min_healthy_percentages_->empty()) { // Check the status of the specified upstream cluster(s) to determine the right response. @@ -143,7 +147,13 @@ void HealthCheckFilter::onComplete() { } } - callbacks_->sendLocalReply(final_status, "", nullptr, absl::nullopt); + callbacks_->sendLocalReply(final_status, "", + [degraded](auto& headers) { + if (degraded) { + headers.insertEnvoyDegraded(); + } + }, + absl::nullopt); } } // namespace HealthCheck diff --git a/source/extensions/filters/http/health_check/health_check.h b/source/extensions/filters/http/health_check/health_check.h index a5e8b0e4b735e..28175e7816e38 100644 --- a/source/extensions/filters/http/health_check/health_check.h +++ b/source/extensions/filters/http/health_check/health_check.h @@ -27,20 +27,24 @@ class HealthCheckCacheManager { public: HealthCheckCacheManager(Event::Dispatcher& dispatcher, std::chrono::milliseconds timeout); - Http::Code getCachedResponseCode() { return last_response_code_; } - void setCachedResponseCode(Http::Code code) { + std::pair getCachedResponse() { + return {last_response_code_, last_response_degraded_}; + } + void setCachedResponse(Http::Code code, bool degraded) { last_response_code_ = code; - use_cached_response_code_ = true; + last_response_degraded_ = degraded; + use_cached_response_ = true; } - bool useCachedResponseCode() { return use_cached_response_code_; } + bool useCachedResponse() { return use_cached_response_; } private: void onTimer(); Event::TimerPtr clear_cache_timer_; const std::chrono::milliseconds timeout_; - std::atomic use_cached_response_code_{}; + std::atomic use_cached_response_{}; std::atomic last_response_code_{}; + std::atomic last_response_degraded_{}; }; typedef std::shared_ptr HealthCheckCacheManagerSharedPtr; diff --git a/test/extensions/filters/http/health_check/health_check_test.cc b/test/extensions/filters/http/health_check/health_check_test.cc index 7b05dd7b9b68f..8c13393197158 100644 --- a/test/extensions/filters/http/health_check/health_check_test.cc +++ b/test/extensions/filters/http/health_check/health_check_test.cc @@ -266,7 +266,7 @@ TEST_F(HealthCheckFilterCachingTest, CachedServiceUnavailableCallbackCalled) { EXPECT_CALL(context_, healthCheckFailed()).WillRepeatedly(Return(false)); EXPECT_CALL(callbacks_.stream_info_, healthCheck(true)); EXPECT_CALL(callbacks_.active_span_, setSampled(false)); - cache_manager_->setCachedResponseCode(Http::Code::ServiceUnavailable); + cache_manager_->setCachedResponse(Http::Code::ServiceUnavailable, false); Http::TestHeaderMapImpl health_check_response{{":status", "503"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)) @@ -287,7 +287,7 @@ TEST_F(HealthCheckFilterCachingTest, CachedOkCallbackNotCalled) { EXPECT_CALL(context_, healthCheckFailed()).WillRepeatedly(Return(false)); EXPECT_CALL(callbacks_.stream_info_, healthCheck(true)); EXPECT_CALL(callbacks_.active_span_, setSampled(false)); - cache_manager_->setCachedResponseCode(Http::Code::OK); + cache_manager_->setCachedResponse(Http::Code::OK, false); Http::TestHeaderMapImpl health_check_response{{":status", "200"}}; EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)) @@ -313,7 +313,40 @@ TEST_F(HealthCheckFilterCachingTest, All) { EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->encodeHeaders(service_response_headers, true)); - // Verify that the next request uses the cached value. + // Verify that the next request uses the cached value without setting the degraded header. + prepareFilter(true); + EXPECT_CALL(callbacks_.stream_info_, + setResponseFlag(StreamInfo::ResponseFlag::FailedLocalHealthCheck)); + EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true)) + .Times(1) + .WillRepeatedly(Invoke([&](Http::HeaderMap& headers, bool end_stream) { + filter_->encodeHeaders(headers, end_stream); + EXPECT_STREQ("cluster_name", headers.EnvoyUpstreamHealthCheckedCluster()->value().c_str()); + })); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->decodeHeaders(request_headers_, true)); + + // Fire the timer, this should result in the next request going through. + EXPECT_CALL(*cache_timer_, enableTimer(_)); + cache_timer_->callback_(); + prepareFilter(true); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true)); +} + +TEST_F(HealthCheckFilterCachingTest, DegradedHeader) { + EXPECT_CALL(callbacks_.stream_info_, healthCheck(true)).Times(3); + EXPECT_CALL(callbacks_.active_span_, setSampled(false)).Times(3); + + // Verify that the first request goes through. + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers_, true)); + Http::TestHeaderMapImpl service_response_headers{{":status", "503"}, + {"x-envoy-degraded", "true"}}; + Http::TestHeaderMapImpl health_check_response{{":status", "503"}, {"x-envoy-degraded", ""}}; + + EXPECT_EQ(Http::FilterHeadersStatus::Continue, + filter_->encodeHeaders(service_response_headers, true)); + + // Verify that the next request uses the cached value and that the x-envoy-degraded header is set. prepareFilter(true); EXPECT_CALL(callbacks_.stream_info_, setResponseFlag(StreamInfo::ResponseFlag::FailedLocalHealthCheck));