Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 2 additions & 0 deletions RAW_RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ final version.
* Added support for :ref:`fixed stats tag values
<envoy_api_field_TagSpecifier.fixed_value>` which will be added to all metrics.
* Added `/runtime` admin endpoint to read the current runtime values.
* Extended the health check filter to support computation of the health check response
based on the percent of healthy servers is upstream clusters.
* Added `gateway-error` retry-on policy.
* Added support for building envoy with exported symbols
This change allows scripts loaded with the lua filter to load shared object libraries such as those installed via luarocks.
2 changes: 1 addition & 1 deletion bazel/repository_locations.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ REPOSITORY_LOCATIONS = dict(
urls = ["https://github.com/google/protobuf/archive/v3.5.0.tar.gz"],
),
envoy_api = dict(
commit = "0811371f1738a2e5f0cb594b500e9db3b5bd8af8",
commit = "040b29a717eb5180c4a6797bb72f5a6ce2731363",
remote = "https://github.com/envoyproxy/data-plane-api",
),
grpc_httpjson_transcoding = dict(
Expand Down
52 changes: 48 additions & 4 deletions source/server/http/health_check.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,20 @@ HealthCheckFilterConfig::createFilter(const envoy::api::v2::filter::http::Health
std::chrono::milliseconds(cache_time_ms)));
}

return [&context, pass_through_mode, cache_manager,
hc_endpoint](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(Http::StreamFilterSharedPtr{
new HealthCheckFilter(context, pass_through_mode, cache_manager, hc_endpoint)});
ClusterMinHealthyPercentagesConstSharedPtr cluster_min_healthy_percentages;
if (!pass_through_mode && !proto_config.cluster_min_healthy_percentages().empty()) {
auto cluster_to_percentage = std::make_unique<ClusterMinHealthyPercentages>();
for (const auto& item : proto_config.cluster_min_healthy_percentages()) {
cluster_to_percentage->emplace(std::make_pair(item.first, item.second.value()));
}
cluster_min_healthy_percentages = std::move(cluster_to_percentage);
}

return [&context, pass_through_mode, cache_manager, hc_endpoint,
cluster_min_healthy_percentages](Http::FilterChainFactoryCallbacks& callbacks) -> void {
callbacks.addStreamFilter(std::make_shared<HealthCheckFilter>(
context, pass_through_mode, cache_manager, hc_endpoint, cluster_min_healthy_percentages));

};
}

Expand Down Expand Up @@ -155,6 +165,40 @@ void HealthCheckFilter::onComplete() {
Http::Code final_status = Http::Code::OK;
if (cache_manager_) {
final_status = cache_manager_->getCachedResponseCode();
} 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.
auto& clusterManager = context_.clusterManager();
for (const auto& item : *cluster_min_healthy_percentages_) {
const std::string& cluster_name = item.first;
const double min_healthy_percentage = item.second;
auto* cluster = clusterManager.get(cluster_name);
if (cluster == nullptr) {
// If the cluster does not exist at all, consider the service unhealthy.
final_status = Http::Code::ServiceUnavailable;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some comments here? I guess I see how the lack of the cluster at all is sufficient ground for failure, though I wonder if there should be a stat. At the very least I would probably add a small comment.

break;
}
const auto& stats = cluster->info()->stats();
const uint64_t membership_total = stats.membership_total_.value();
if (membership_total == 0) {
// If the cluster exists but is empty, consider the service unhealty unless
// the specified minimum percent healthy for the cluster happens to be zero.
if (min_healthy_percentage == 0.0) {
continue;
} else {
final_status = Http::Code::ServiceUnavailable;
break;
}
}
// In the general case, consider the service unhealthy if fewer than the
// specified percentage of the servers in the cluster are healthy.
// TODO(brian-pane) switch to purely integer-based math here, because the
// int-to-float conversions and floating point division are slow.
if (stats.membership_healthy_.value() < membership_total * min_healthy_percentage / 100.0) {
final_status = Http::Code::ServiceUnavailable;
break;
}
}
}

if (!Http::CodeUtility::is2xx(enumToInt(final_status))) {
Expand Down
12 changes: 9 additions & 3 deletions source/server/http/health_check.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,20 @@ class HealthCheckCacheManager {

typedef std::shared_ptr<HealthCheckCacheManager> HealthCheckCacheManagerSharedPtr;

typedef std::map<std::string, double> ClusterMinHealthyPercentages;
typedef std::shared_ptr<const ClusterMinHealthyPercentages>
ClusterMinHealthyPercentagesConstSharedPtr;

/**
* Health check responder filter.
*/
class HealthCheckFilter : public Http::StreamFilter {
public:
HealthCheckFilter(Server::Configuration::FactoryContext& context, bool pass_through_mode,
HealthCheckCacheManagerSharedPtr cache_manager, const std::string& endpoint)
HealthCheckCacheManagerSharedPtr cache_manager, const std::string& endpoint,
ClusterMinHealthyPercentagesConstSharedPtr cluster_min_healthy_percentages)
: context_(context), pass_through_mode_(pass_through_mode), cache_manager_(cache_manager),
endpoint_(endpoint) {}
endpoint_(endpoint), cluster_min_healthy_percentages_(cluster_min_healthy_percentages) {}

// Http::StreamFilterBase
void onDestroy() override {}
Expand Down Expand Up @@ -107,7 +112,8 @@ class HealthCheckFilter : public Http::StreamFilter {
bool handling_{};
bool health_check_request_{};
bool pass_through_mode_{};
HealthCheckCacheManagerSharedPtr cache_manager_{};
HealthCheckCacheManagerSharedPtr cache_manager_;
const std::string endpoint_;
ClusterMinHealthyPercentagesConstSharedPtr cluster_min_healthy_percentages_;
};
} // namespace Envoy
102 changes: 100 additions & 2 deletions test/server/http/health_check_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
#include <memory>

#include "common/buffer/buffer_impl.h"
#include "common/upstream/upstream_impl.h"

#include "server/http/health_check.h"

#include "test/mocks/server/mocks.h"
#include "test/mocks/upstream/cluster_info.h"
#include "test/test_common/printers.h"
#include "test/test_common/utility.h"

Expand Down Expand Up @@ -36,8 +38,11 @@ class HealthCheckFilterTest : public testing::Test {
prepareFilter(pass_through);
}

void prepareFilter(bool pass_through) {
filter_.reset(new HealthCheckFilter(context_, pass_through, cache_manager_, "/healthcheck"));
void prepareFilter(
bool pass_through,
ClusterMinHealthyPercentagesConstSharedPtr cluster_min_healthy_percentages = nullptr) {
filter_.reset(new HealthCheckFilter(context_, pass_through, cache_manager_, "/healthcheck",
cluster_min_healthy_percentages));
filter_->setDecoderFilterCallbacks(callbacks_);
}

Expand All @@ -49,6 +54,14 @@ class HealthCheckFilterTest : public testing::Test {
NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
Http::TestHeaderMapImpl request_headers_;
Http::TestHeaderMapImpl request_headers_no_hc_;

class MockHealthCheckCluster : public NiceMock<Upstream::MockThreadLocalCluster> {
public:
MockHealthCheckCluster(uint64_t membership_total, uint64_t membership_healthy) {
info()->stats().membership_total_.set(membership_total);
info()->stats().membership_healthy_.set(membership_healthy);
}
};
};

class HealthCheckFilterNoPassThroughTest : public HealthCheckFilterTest {
Expand Down Expand Up @@ -84,6 +97,91 @@ TEST_F(HealthCheckFilterNoPassThroughTest, NotHcRequest) {
EXPECT_STREQ("true", service_response.EnvoyImmediateHealthCheckFail()->value().c_str());
}

TEST_F(HealthCheckFilterNoPassThroughTest, ComputedHealth) {
// Test non-pass-through health checks without upstream cluster minimum health specified.
prepareFilter(false);
{
Http::TestHeaderMapImpl health_check_response{{":status", "200"}};
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: .Times(1) is redundant (it's implied). You can remove. Same below.

.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}
{
Http::TestHeaderMapImpl health_check_response{{":status", "503"}};
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(true));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))
.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}

// Test non-pass-through health checks with upstream cluster minimum health specified.
prepareFilter(false, ClusterMinHealthyPercentagesConstSharedPtr(
new ClusterMinHealthyPercentages{{"www1", 50.0}, {"www2", 75.0}}));
{
// This should pass, because each upstream cluster has at least the
// minimum percentage of healthy servers.
Http::TestHeaderMapImpl health_check_response{{":status", "200"}};
MockHealthCheckCluster cluster_www1(100, 50);
MockHealthCheckCluster cluster_www2(1000, 800);
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false));
EXPECT_CALL(context_, clusterManager()).Times(1);
EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1));
EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))
.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}
{
// This should fail, because one upstream cluster has too few healthy servers.
Http::TestHeaderMapImpl health_check_response{{":status", "503"}};
MockHealthCheckCluster cluster_www1(100, 49);
MockHealthCheckCluster cluster_www2(1000, 800);
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false));
EXPECT_CALL(context_, clusterManager()).Times(1);
EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1));
EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))
.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}
{
// This should fail, because one upstream cluster has no servers at all.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test for the empty cluster but min health == 0% case.

Http::TestHeaderMapImpl health_check_response{{":status", "503"}};
MockHealthCheckCluster cluster_www1(0, 0);
MockHealthCheckCluster cluster_www2(1000, 800);
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false));
EXPECT_CALL(context_, clusterManager()).Times(1);
EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1));
EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))
.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}
// Test the cases where an upstream cluster is empty, or has no healthy servers, but
// the minimum required percent healthy is zero. The health check should return a 200.
prepareFilter(false, ClusterMinHealthyPercentagesConstSharedPtr(
new ClusterMinHealthyPercentages{{"www1", 0.0}, {"www2", 0.0}}));
{
Http::TestHeaderMapImpl health_check_response{{":status", "200"}};
MockHealthCheckCluster cluster_www1(0, 0);
MockHealthCheckCluster cluster_www2(1000, 0);
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(false));
EXPECT_CALL(context_, clusterManager()).Times(1);
EXPECT_CALL(context_.cluster_manager_, get("www1")).WillRepeatedly(Return(&cluster_www1));
EXPECT_CALL(context_.cluster_manager_, get("www2")).WillRepeatedly(Return(&cluster_www2));
EXPECT_CALL(callbacks_, encodeHeaders_(HeaderMapEqualRef(&health_check_response), true))
.Times(1);
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter_->decodeHeaders(request_headers_, true));
}
}

TEST_F(HealthCheckFilterNoPassThroughTest, HealthCheckFailedCallbackCalled) {
EXPECT_CALL(context_, healthCheckFailed()).WillOnce(Return(true));
EXPECT_CALL(callbacks_.request_info_, healthCheck(true));
Expand Down