Skip to content

health filter: cache degraded health checks #5659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mattklein123 merged 3 commits into from
Jan 22, 2019
Merged

health filter: cache degraded health checks #5659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb18077
health filter: cache degraded health checks
Jan 18, 2019
5734e71
format
Jan 18, 2019
abb0bcc
rename cachedResponseCode -> cachedResponse
Jan 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions source/extensions/filters/http/health_check/health_check.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}

Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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::Code>(Http::Utility::getResponseStatus(headers)));
cache_manager_->setCachedResponse(
static_cast<Http::Code>(Http::Utility::getResponseStatus(headers)),
headers.EnvoyDegraded() != nullptr);
}

headers.insertEnvoyUpstreamHealthCheckedCluster().value(context_.localInfo().clusterName());
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
14 changes: 9 additions & 5 deletions source/extensions/filters/http/health_check/health_check.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Http::Code, bool> 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<bool> use_cached_response_code_{};
std::atomic<bool> use_cached_response_{};
std::atomic<Http::Code> last_response_code_{};
std::atomic<bool> last_response_degraded_{};
};

typedef std::shared_ptr<HealthCheckCacheManager> HealthCheckCacheManagerSharedPtr;
Expand Down
39 changes: 36 additions & 3 deletions test/extensions/filters/http/health_check/health_check_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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));
Expand Down