-
Notifications
You must be signed in to change notification settings - Fork 5.5k
router: add missing unit tests for x-envoy-attempt-count on upstream requests #10368
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -242,6 +242,10 @@ class RouterTestBase : public testing::Test { | |
| StreamInfo::FilterState::LifeSpan::DownstreamRequest); | ||
| } | ||
|
|
||
| void setIncludeAttemptCount(bool include) { | ||
| ON_CALL(callbacks_.route_->route_entry_, includeAttemptCount()).WillByDefault(Return(include)); | ||
| } | ||
|
|
||
| void enableHedgeOnPerTryTimeout() { | ||
| callbacks_.route_->route_entry_.hedge_policy_.hedge_on_per_try_timeout_ = true; | ||
| callbacks_.route_->route_entry_.hedge_policy_.additional_request_chance_ = | ||
|
|
@@ -868,6 +872,121 @@ TEST_F(RouterTest, EnvoyUpstreamServiceTime) { | |
| EXPECT_TRUE(verifyHostUpstreamStats(1, 0)); | ||
| } | ||
|
|
||
| // Validate that x-envoy-attempt-count is added to request headers when option is true. | ||
| TEST_F(RouterTest, EnvoyAttemptCountInRequest) { | ||
| setIncludeAttemptCount(true); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. optional: do you want to name this to Request even though it doesn't reflect the config option until v4?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to keep the name change in #10325. So I will update this or that other PR depending on merge order. |
||
|
|
||
| EXPECT_CALL(cm_.conn_pool_, newStream(_, _)).WillOnce(Return(&cancellable_)); | ||
| expectResponseTimerCreate(); | ||
|
|
||
| Http::TestRequestHeaderMapImpl headers; | ||
| HttpTestUtility::addDefaultHeaders(headers); | ||
| router_.decodeHeaders(headers, true); | ||
|
junr03 marked this conversation as resolved.
Outdated
|
||
|
|
||
| EXPECT_EQ(1, atoi(std::string(headers.EnvoyAttemptCount()->value().getStringView()).c_str())); | ||
|
|
||
| // When the router filter gets reset we should cancel the pool request. | ||
| EXPECT_CALL(cancellable_, cancel()); | ||
| router_.onDestroy(); | ||
| EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); | ||
| } | ||
|
|
||
| // Validate that x-envoy-attempt-count is overwritten by the router on request headers, if the | ||
| // header is sent from the downstream and the option is set to true. | ||
| TEST_F(RouterTest, EnvoyAttemptCountInRequestOverwritten) { | ||
| setIncludeAttemptCount(true); | ||
|
|
||
| EXPECT_CALL(cm_.conn_pool_, newStream(_, _)).WillOnce(Return(&cancellable_)); | ||
| expectResponseTimerCreate(); | ||
|
|
||
| Http::TestRequestHeaderMapImpl headers; | ||
| HttpTestUtility::addDefaultHeaders(headers); | ||
| headers.setEnvoyAttemptCount(123); | ||
| router_.decodeHeaders(headers, true); | ||
|
|
||
| EXPECT_EQ(1, atoi(std::string(headers.EnvoyAttemptCount()->value().getStringView()).c_str())); | ||
|
|
||
| // When the router filter gets reset we should cancel the pool request. | ||
| EXPECT_CALL(cancellable_, cancel()); | ||
| router_.onDestroy(); | ||
| EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); | ||
| } | ||
|
|
||
| // Validate that x-envoy-attempt-count is not overwritten by the router on request headers, if the | ||
| // header is sent from the downstream and the option is set to false. | ||
| TEST_F(RouterTest, EnvoyAttemptCountInRequestNotOverwritten) { | ||
| EXPECT_CALL(cm_.conn_pool_, newStream(_, _)).WillOnce(Return(&cancellable_)); | ||
| expectResponseTimerCreate(); | ||
|
|
||
| Http::TestRequestHeaderMapImpl headers; | ||
| HttpTestUtility::addDefaultHeaders(headers); | ||
| headers.setEnvoyAttemptCount(123); | ||
| router_.decodeHeaders(headers, true); | ||
|
|
||
| EXPECT_EQ(123, atoi(std::string(headers.EnvoyAttemptCount()->value().getStringView()).c_str())); | ||
|
|
||
| // When the router filter gets reset we should cancel the pool request. | ||
| EXPECT_CALL(cancellable_, cancel()); | ||
| router_.onDestroy(); | ||
| EXPECT_TRUE(verifyHostUpstreamStats(0, 0)); | ||
| } | ||
|
|
||
| TEST_F(RouterTest, EnvoyAttemptCountInRequestUpdatedInRetries) { | ||
| setIncludeAttemptCount(true); | ||
|
|
||
| NiceMock<Http::MockRequestEncoder> encoder1; | ||
| Http::ResponseDecoder* response_decoder = nullptr; | ||
| EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) | ||
| .WillOnce(Invoke( | ||
| [&](Http::ResponseDecoder& decoder, | ||
| Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { | ||
| response_decoder = &decoder; | ||
| callbacks.onPoolReady(encoder1, cm_.conn_pool_.host_, upstream_stream_info_); | ||
| return nullptr; | ||
| })); | ||
| expectResponseTimerCreate(); | ||
|
|
||
| Http::TestRequestHeaderMapImpl headers{{"x-envoy-retry-on", "5xx"}, {"x-envoy-internal", "true"}}; | ||
| HttpTestUtility::addDefaultHeaders(headers); | ||
| router_.decodeHeaders(headers, true); | ||
|
|
||
| // Initial request has 1 attempt. | ||
| EXPECT_EQ(1, atoi(std::string(headers.EnvoyAttemptCount()->value().getStringView()).c_str())); | ||
|
|
||
| // 5xx response. | ||
| router_.retry_state_->expectHeadersRetry(); | ||
| Http::ResponseHeaderMapPtr response_headers1( | ||
| new Http::TestResponseHeaderMapImpl{{":status", "503"}}); | ||
| EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(503)); | ||
| response_decoder->decodeHeaders(std::move(response_headers1), true); | ||
| EXPECT_TRUE(verifyHostUpstreamStats(0, 1)); | ||
|
|
||
| // We expect the 5xx response to kick off a new request. | ||
| EXPECT_CALL(encoder1.stream_, resetStream(_)).Times(0); | ||
| NiceMock<Http::MockRequestEncoder> encoder2; | ||
| EXPECT_CALL(cm_.conn_pool_, newStream(_, _)) | ||
| .WillOnce(Invoke( | ||
| [&](Http::ResponseDecoder& decoder, | ||
| Http::ConnectionPool::Callbacks& callbacks) -> Http::ConnectionPool::Cancellable* { | ||
| response_decoder = &decoder; | ||
| callbacks.onPoolReady(encoder2, cm_.conn_pool_.host_, upstream_stream_info_); | ||
| return nullptr; | ||
| })); | ||
| router_.retry_state_->callback_(); | ||
|
|
||
| // The retry should cause the header to increase to 2. | ||
| EXPECT_EQ(2, atoi(std::string(headers.EnvoyAttemptCount()->value().getStringView()).c_str())); | ||
|
|
||
| // Normal response. | ||
| EXPECT_CALL(*router_.retry_state_, shouldRetryHeaders(_, _)).WillOnce(Return(RetryStatus::No)); | ||
| EXPECT_CALL(cm_.conn_pool_.host_->health_checker_, setUnhealthy()).Times(0); | ||
| Http::ResponseHeaderMapPtr response_headers2( | ||
| new Http::TestResponseHeaderMapImpl{{":status", "200"}}); | ||
| EXPECT_CALL(cm_.conn_pool_.host_->outlier_detector_, putHttpResponseCode(200)); | ||
| response_decoder->decodeHeaders(std::move(response_headers2), true); | ||
| EXPECT_TRUE(verifyHostUpstreamStats(1, 1)); | ||
| } | ||
|
|
||
| // Validate that the cluster is appended to the response when configured. | ||
| void RouterTestBase::testAppendCluster(absl::optional<Http::LowerCaseString> cluster_header_name) { | ||
| auto debug_config = std::make_unique<DebugConfig>( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.