router: add envoy-ratelimited retry policy (envoyproxy#12201)

Adds new retry policy envoy-ratelimited that retries responses containing the header x-envoy-ratelimited. Signed-off-by: Martin Matusiak <[email protected]> Signed-off-by: chaoqinli <[email protected]>
chaoqin-li1123 · Aug 7, 2020 · 03b1d84 · 03b1d84
1 parent 2543df7
commit 03b1d84
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 5 deletions.
diff --git a/docs/root/configuration/http/http_filters/router_filter.rst b/docs/root/configuration/http/http_filters/router_filter.rst
@@ -94,6 +94,12 @@ connect-failure
     configuration <envoy_v3_api_field_config.route.v3.RouteAction.retry_policy>` or via
     :ref:`virtual host retry policy <envoy_v3_api_field_config.route.v3.VirtualHost.retry_policy>`.
 
+.. _config_http_filters_router_retry_policy-envoy-ratelimited:
+
+envoy-ratelimited
+  Envoy will retry if the header :ref:`x-envoy-ratelimited<config_http_filters_router_x-envoy-ratelimited>`
+  is present.
+
 retriable-4xx
   Envoy will attempt a retry if the upstream server responds with a retriable 4xx response code.
   Currently, the only response code in this category is 409.
@@ -294,9 +300,11 @@ information.
 x-envoy-ratelimited
 ^^^^^^^^^^^^^^^^^^^
 
-If this header is set by upstream, Envoy will not retry. Currently the value of the header is not
-looked at, only its presence. This header is set by :ref:`rate limit filter<config_http_filters_rate_limit>`
-when the request is rate limited.
+If this header is set by upstream, Envoy will not retry unless the retry policy
+:ref:`envoy-ratelimited<config_http_filters_router_retry_policy-envoy-ratelimited>`
+is enabled. Currently, the value of the header is not looked at, only its
+presence. This header is set by :ref:`rate limit
+filter<config_http_filters_rate_limit>` when the request is rate limited.
 
 .. _config_http_filters_router_headers_set:
 

diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -44,6 +44,9 @@ New Features
 * http: introduced new HTTP/1 and HTTP/2 codec implementations that will remove the use of exceptions for control flow due to high risk factors and instead use error statuses. The old behavior is deprecated, but can be used during the removal period by setting the runtime feature `envoy.reloadable_features.new_codec_behavior` to false. The removal period will be one month.
 * load balancer: added a :ref:`configuration<envoy_v3_api_msg_config.cluster.v3.Cluster.LeastRequestLbConfig>` option to specify the active request bias used by the least request load balancer.
 * redis: added fault injection support :ref:`fault injection for redis proxy <envoy_v3_api_field_extensions.filters.network.redis_proxy.v3.RedisProxy.faults>`, described further in :ref:`configuration documentation <config_network_filters_redis_proxy>`.
+* router: added new
+  :ref:`envoy-ratelimited<config_http_filters_router_retry_policy-envoy-ratelimited>`
+  retry policy, which allows retrying envoy's own rate limited responses.
 * stats: added optional histograms to :ref:`cluster stats <config_cluster_manager_cluster_stats_request_response_sizes>`
   that track headers and body sizes of requests and responses.
 * tap: added :ref:`generic body matcher<envoy_v3_api_msg_config.tap.v3.HttpGenericBodyMatch>` to scan http requests and responses for text or hex patterns.

diff --git a/include/envoy/router/router.h b/include/envoy/router/router.h
@@ -169,6 +169,7 @@ class RetryPolicy {
   static const uint32_t RETRY_ON_RETRIABLE_STATUS_CODES  = 0x400;
   static const uint32_t RETRY_ON_RESET                   = 0x800;
   static const uint32_t RETRY_ON_RETRIABLE_HEADERS       = 0x1000;
+  static const uint32_t RETRY_ON_ENVOY_RATE_LIMITED      = 0x2000;
   // clang-format on
 
   virtual ~RetryPolicy() = default;

diff --git a/source/common/http/headers.h b/source/common/http/headers.h
@@ -252,6 +252,7 @@ class HeaderValues {
     const std::string _5xx{"5xx"};
     const std::string GatewayError{"gateway-error"};
     const std::string ConnectFailure{"connect-failure"};
+    const std::string EnvoyRateLimited{"envoy-ratelimited"};
     const std::string RefusedStream{"refused-stream"};
     const std::string Retriable4xx{"retriable-4xx"};
     const std::string RetriableStatusCodes{"retriable-status-codes"};

diff --git a/source/common/router/retry_state_impl.cc b/source/common/router/retry_state_impl.cc
@@ -23,6 +23,7 @@ namespace Router {
 const uint32_t RetryPolicy::RETRY_ON_5XX;
 const uint32_t RetryPolicy::RETRY_ON_GATEWAY_ERROR;
 const uint32_t RetryPolicy::RETRY_ON_CONNECT_FAILURE;
+const uint32_t RetryPolicy::RETRY_ON_ENVOY_RATE_LIMITED;
 const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_4XX;
 const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_HEADERS;
 const uint32_t RetryPolicy::RETRY_ON_RETRIABLE_STATUS_CODES;
@@ -169,6 +170,8 @@ std::pair<uint32_t, bool> RetryStateImpl::parseRetryOn(absl::string_view config)
       ret |= RetryPolicy::RETRY_ON_GATEWAY_ERROR;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.ConnectFailure) {
       ret |= RetryPolicy::RETRY_ON_CONNECT_FAILURE;
+    } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.EnvoyRateLimited) {
+      ret |= RetryPolicy::RETRY_ON_ENVOY_RATE_LIMITED;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.Retriable4xx) {
       ret |= RetryPolicy::RETRY_ON_RETRIABLE_4XX;
     } else if (retry_on == Http::Headers::get().EnvoyRetryOnValues.RefusedStream) {
@@ -290,9 +293,10 @@ RetryStatus RetryStateImpl::shouldHedgeRetryPerTryTimeout(DoRetryCallback callba
 }
 
 bool RetryStateImpl::wouldRetryFromHeaders(const Http::ResponseHeaderMap& response_headers) {
-  // We never retry if the request is rate limited.
+  // A response that contains the x-envoy-ratelimited header comes from an upstream envoy.
+  // We retry these only when the envoy-ratelimited policy is in effect.
   if (response_headers.EnvoyRateLimited() != nullptr) {
-    return false;
+    return retry_on_ & RetryPolicy::RETRY_ON_ENVOY_RATE_LIMITED;
   }
 
   if (retry_on_ & RetryPolicy::RETRY_ON_5XX) {

diff --git a/test/common/router/retry_state_impl_test.cc b/test/common/router/retry_state_impl_test.cc
@@ -218,6 +218,22 @@ TEST_F(RouterRetryStateImplTest, PolicyResourceExhaustedRemoteRateLimited) {
   EXPECT_EQ(RetryStatus::No, state_->shouldRetryHeaders(response_headers, callback_));
 }
 
+TEST_F(RouterRetryStateImplTest, PolicyEnvoyRateLimitedRemoteRateLimited) {
+  Http::TestRequestHeaderMapImpl request_headers{{"x-envoy-retry-on", "envoy-ratelimited"}};
+  setup(request_headers);
+  EXPECT_TRUE(state_->enabled());
+
+  expectTimerCreateAndEnable();
+  Http::TestResponseHeaderMapImpl response_headers{{":status", "429"},
+                                                   {"x-envoy-ratelimited", "true"}};
+  EXPECT_EQ(RetryStatus::Yes, state_->shouldRetryHeaders(response_headers, callback_));
+  EXPECT_CALL(callback_ready_, ready());
+  retry_timer_->invokeCallback();
+
+  EXPECT_EQ(RetryStatus::NoRetryLimitExceeded,
+            state_->shouldRetryHeaders(response_headers, callback_));
+}
+
 TEST_F(RouterRetryStateImplTest, PolicyGatewayErrorRemote502) {
   verifyPolicyWithRemoteResponse("gateway-error" /* retry_on */, "502" /* response_status */,
                                  false /* is_grpc */);