Skip to content

Making //test/integration:hds_integration_test test faster and more robust #4164

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
htuch merged 6 commits into from
Aug 22, 2018
Merged

Making //test/integration:hds_integration_test test faster and more robust #4164

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1d1d77a
Making hds_integration test faster and more robust
markatou Aug 15, 2018
b7d94d4
Addressing comments
markatou Aug 16, 2018
6040189
Addressing comments
markatou Aug 16, 2018
02687d9
Merge remote-tracking branch 'upstream/master' into less_fragile
markatou Aug 21, 2018
a851f9d
Addressing comments
markatou Aug 21, 2018
a4dd166
Addressing comments
markatou Aug 22, 2018
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
73 changes: 57 additions & 16 deletions test/integration/hds_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ class HdsIntegrationTest : public HttpIntegrationTest,
// one HTTP health_check
envoy::service::discovery::v2::HealthCheckSpecifier makeHttpHealthCheckSpecifier() {
envoy::service::discovery::v2::HealthCheckSpecifier server_health_check_specifier_;
server_health_check_specifier_.mutable_interval()->set_seconds(1);
server_health_check_specifier_.mutable_interval()->set_nanos(100000000); // 0.1 seconds

auto* health_check = server_health_check_specifier_.add_cluster_health_checks();

Expand All @@ -130,8 +130,8 @@ class HdsIntegrationTest : public HttpIntegrationTest,
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_zone("some_zone");
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_sub_zone("crete");

health_check->add_health_checks()->mutable_timeout()->set_seconds(2);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(1);
health_check->add_health_checks()->mutable_timeout()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_unhealthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_healthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_grpc_health_check();
Expand All @@ -145,7 +145,7 @@ class HdsIntegrationTest : public HttpIntegrationTest,
// one TCP health_check
envoy::service::discovery::v2::HealthCheckSpecifier makeTcpHealthCheckSpecifier() {
envoy::service::discovery::v2::HealthCheckSpecifier server_health_check_specifier_;
server_health_check_specifier_.mutable_interval()->set_seconds(1);
server_health_check_specifier_.mutable_interval()->set_nanos(100000000); // 0.1 seconds

auto* health_check = server_health_check_specifier_.add_cluster_health_checks();

Expand All @@ -157,8 +157,8 @@ class HdsIntegrationTest : public HttpIntegrationTest,
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_zone("some_zone");
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_sub_zone("crete");

health_check->add_health_checks()->mutable_timeout()->set_seconds(2);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(1);
health_check->add_health_checks()->mutable_timeout()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_unhealthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_healthy_threshold()->set_value(2);
auto* tcp_hc = health_check->mutable_health_checks(0)->mutable_tcp_health_check();
Expand All @@ -180,9 +180,9 @@ class HdsIntegrationTest : public HttpIntegrationTest,
}

// Checks if the cluster counters are correct
void checkCounters(int requests, int response_s, int successes, int failures) {
void checkCounters(int requests, int responses, int successes, int failures) {
EXPECT_EQ(requests, test_server_->counter("hds_delegate.requests")->value());
EXPECT_EQ(response_s, test_server_->counter("hds_delegate.responses")->value());
EXPECT_LE(responses, test_server_->counter("hds_delegate.responses")->value());
EXPECT_EQ(successes, test_server_->counter("cluster.anna.health_check.success")->value());
EXPECT_EQ(failures, test_server_->counter("cluster.anna.health_check.failure")->value());
}
Expand All @@ -201,6 +201,7 @@ class HdsIntegrationTest : public HttpIntegrationTest,
FakeHttpConnectionPtr host2_fake_connection_;
FakeRawConnectionPtr host_fake_raw_connection_;

int MaxTimeout = 100;

@htuch htuch Aug 16, 2018

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: static constexpr int

envoy::service::discovery::v2::HealthCheckRequest envoy_msg_;
envoy::service::discovery::v2::HealthCheckRequestOrEndpointHealthResponse response_;
envoy::service::discovery::v2::HealthCheckSpecifier server_health_check_specifier_;
Expand Down Expand Up @@ -234,6 +235,9 @@ TEST_P(HdsIntegrationTest, SingleEndpointHealthyHttp) {
host_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false);
host_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoint's response
test_server_->waitForCounterGe("cluster.anna.health_check.success", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand All @@ -253,14 +257,16 @@ TEST_P(HdsIntegrationTest, SingleEndpointHealthyHttp) {
TEST_P(HdsIntegrationTest, SingleEndpointTimeoutHttp) {
initialize();
server_health_check_specifier_ = makeHttpHealthCheckSpecifier();

server_health_check_specifier_.mutable_cluster_health_checks(0)
->mutable_health_checks(0)
->mutable_timeout()
->set_seconds(0);
server_health_check_specifier_.mutable_cluster_health_checks(0)
->mutable_health_checks(0)
->mutable_timeout()
->set_nanos(800000000);
->set_nanos(100000000); // 0.1 seconds

// Server <--> Envoy
waitForHdsStream();
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, envoy_msg_));
Expand All @@ -275,6 +281,9 @@ TEST_P(HdsIntegrationTest, SingleEndpointTimeoutHttp) {

// Endpoint doesn't repond to the health check

// Make sure the cluster processed the endpoint's (lack of) response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -312,6 +321,9 @@ TEST_P(HdsIntegrationTest, SingleEndpointUnhealthyHttp) {
host_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "404"}}, false);
host_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoint's response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -346,7 +358,8 @@ TEST_P(HdsIntegrationTest, SingleEndpointTimeoutTcp) {
server_health_check_specifier_.mutable_cluster_health_checks(0)
->mutable_health_checks(0)
->mutable_timeout()
->set_nanos(800000000);
->set_nanos(100000000); // 0.1 seconds

hds_stream_->startGrpcStream();
hds_stream_->sendGrpcMessage(server_health_check_specifier_);
test_server_->waitForCounterGe("hds_delegate.requests", ++hds_requests_);
Expand All @@ -355,10 +368,13 @@ TEST_P(HdsIntegrationTest, SingleEndpointTimeoutTcp) {
ASSERT_TRUE(host_upstream_->waitForRawConnection(host_fake_raw_connection_));
ASSERT_TRUE(
host_fake_raw_connection_->waitForData(FakeRawConnection::waitForInexactMatch("Ping")));

host_upstream_->set_allow_unexpected_disconnects(true);

// No response from the endpoint

// Make sure the cluster processed the endpoint's (lack of) response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -396,6 +412,9 @@ TEST_P(HdsIntegrationTest, SingleEndpointHealthyTcp) {
RELEASE_ASSERT(result, result.message());
host_upstream_->set_allow_unexpected_disconnects(true);

// Make sure the cluster processed the endpoint's response
test_server_->waitForCounterGe("cluster.anna.health_check.success", 1);

@htuch htuch Aug 16, 2018

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.

It's a bit unclear to me what the win with some of the waits is. We will need to wait for the gRPC response from Envoy to the HDS server below, and those messages are sent independently of what we're doing here wait wise.


// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -433,6 +452,9 @@ TEST_P(HdsIntegrationTest, SingleEndpointUnhealthyTcp) {
RELEASE_ASSERT(result, result.message());
host_upstream_->set_allow_unexpected_disconnects(true);

// TODO(lilika): TcpHealthchecker is not incrementing the stats in failure
// so we can't wait for the cluster to process the endpoint's response

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -476,6 +498,10 @@ TEST_P(HdsIntegrationTest, TwoEndpointsSameLocality) {
host2_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false);
host2_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoints' response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);
test_server_->waitForCounterGe("cluster.anna.health_check.success", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -527,6 +553,10 @@ TEST_P(HdsIntegrationTest, TwoEndpointsDifferentLocality) {
host2_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false);
host2_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoints' response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);
test_server_->waitForCounterGe("cluster.anna.health_check.success", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -561,8 +591,8 @@ TEST_P(HdsIntegrationTest, TwoEndpointsDifferentClusters) {
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_zone("peculiar_zone");
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_sub_zone("paris");

health_check->add_health_checks()->mutable_timeout()->set_seconds(2);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(1);
health_check->add_health_checks()->mutable_timeout()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_unhealthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_healthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_grpc_health_check();
Expand All @@ -587,6 +617,10 @@ TEST_P(HdsIntegrationTest, TwoEndpointsDifferentClusters) {
host2_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false);
host2_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoints' response
test_server_->waitForCounterGe("cluster.anna.health_check.failure", 1);
test_server_->waitForCounterGe("cluster.cat.health_check.success", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down Expand Up @@ -628,6 +662,9 @@ TEST_P(HdsIntegrationTest, TestUpdateMessage) {
host_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "200"}}, false);
host_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoint's response
test_server_->waitForCounterGe("cluster.anna.health_check.success", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand All @@ -641,7 +678,7 @@ TEST_P(HdsIntegrationTest, TestUpdateMessage) {

// New HealthCheckSpecifier message
envoy::service::discovery::v2::HealthCheckSpecifier new_message;
new_message.mutable_interval()->set_seconds(1);
new_message.mutable_interval()->set_nanos(100000000); // 0.1 seconds

auto* health_check = new_message.add_cluster_health_checks();

Expand All @@ -654,8 +691,8 @@ TEST_P(HdsIntegrationTest, TestUpdateMessage) {
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_zone("peculiar_zone");
health_check->mutable_locality_endpoints(0)->mutable_locality()->set_sub_zone("paris");

health_check->add_health_checks()->mutable_timeout()->set_seconds(2);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(1);
health_check->add_health_checks()->mutable_timeout()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_interval()->set_seconds(MaxTimeout);
health_check->mutable_health_checks(0)->mutable_unhealthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_healthy_threshold()->set_value(2);
health_check->mutable_health_checks(0)->mutable_grpc_health_check();
Expand All @@ -671,10 +708,14 @@ TEST_P(HdsIntegrationTest, TestUpdateMessage) {
ASSERT_TRUE(host2_fake_connection_->waitForNewStream(*dispatcher_, host2_stream_));
ASSERT_TRUE(host2_stream_->waitForEndStream(*dispatcher_));
host2_upstream_->set_allow_unexpected_disconnects(true);

// Endpoint responds to the health check
host2_stream_->encodeHeaders(Http::TestHeaderMapImpl{{":status", "404"}}, false);
host2_stream_->encodeData(1024, true);

// Make sure the cluster processed the endpoint's response
test_server_->waitForCounterGe("cluster.cat.health_check.failure", 1);

// Envoy reports back to server
ASSERT_TRUE(hds_stream_->waitForGrpcMessage(*dispatcher_, response_));

Expand Down