Add support for configuring Envoys route idle_timeout (#14340) (#15611)

* Add idleTimeout Co-authored-by: James Oulman <oulman@users.noreply.github.com> Co-authored-by: Jeff Boruszak <104028618+boruszak@users.noreply.github.com> Co-authored-by: Dhia Ayachi <dhia@hashicorp.com>
hashicorp · Nov 29, 2022 · 7c0eec4 · 7c0eec4
1 parent c5dd81e
commit 7c0eec4
Show file tree

Hide file tree

Showing 19 changed files with 144 additions and 1 deletion.
diff --git a/.changelog/14340.txt b/.changelog/14340.txt
@@ -0,0 +1,4 @@
+```release-note:feature
+connect: Add local_idle_timeout_ms to allow configuring the Envoy route idle timeout on local_app
+connect: Add IdleTimeout to service-router to allow configuring the Envoy route idle timeout
+```
diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go
@@ -4108,6 +4108,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 									  "namespace"               : "leek",
 									  "prefix_rewrite"          : "/alternate",
 									  "request_timeout"         : "99s",
+									  "idle_timeout"            : "99s",
 									  "num_retries"             : 12345,
 									  "retry_on_connect_failure": true,
 									  "retry_on_status_codes"   : [401, 209]
@@ -4196,6 +4197,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 								  namespace             = "leek"
 								  prefix_rewrite         = "/alternate"
 								  request_timeout        = "99s"
+								  idle_timeout           = "99s"
 								  num_retries            = 12345
 								  retry_on_connect_failure = true
 								  retry_on_status_codes    = [401, 209]
@@ -4285,6 +4287,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 								Partition:             acl.DefaultPartitionName,
 								PrefixRewrite:         "/alternate",
 								RequestTimeout:        99 * time.Second,
+								IdleTimeout:           99 * time.Second,
 								NumRetries:            12345,
 								RetryOnConnectFailure: true,
 								RetryOnStatusCodes:    []uint32{401, 209},

diff --git a/agent/proxycfg/testing_upstreams.go b/agent/proxycfg/testing_upstreams.go
@@ -682,6 +682,15 @@ func setupTestVariationDiscoveryChain(
 							RequestTimeout: 33 * time.Second,
 						},
 					},
+					{
+						Match: httpMatch(&structs.ServiceRouteHTTPMatch{
+							PathPrefix: "/idle-timeout",
+						}),
+						Destination: &structs.ServiceRouteDestination{
+							Service:     "idle-timeout",
+							IdleTimeout: 33 * time.Second,
+						},
+					},
 					{
 						Match: httpMatch(&structs.ServiceRouteHTTPMatch{
 							PathPrefix: "/retry-connect",

diff --git a/agent/structs/config_entry_discoverychain.go b/agent/structs/config_entry_discoverychain.go
@@ -426,6 +426,10 @@ type ServiceRouteDestination struct {
 	// downstream request (and retries) to be processed.
 	RequestTimeout time.Duration `json:",omitempty" alias:"request_timeout"`
 
+	// IdleTimeout is The total amount of time permitted for the request stream
+	// to be idle
+	IdleTimeout time.Duration `json:",omitempty" alias:"idle_timeout"`
+
 	// NumRetries is the number of times to retry the request when a retryable
 	// result occurs. This seems fairly proxy agnostic.
 	NumRetries uint32 `json:",omitempty" alias:"num_retries"`
@@ -452,22 +456,29 @@ func (e *ServiceRouteDestination) MarshalJSON() ([]byte, error) {
 	type Alias ServiceRouteDestination
 	exported := &struct {
 		RequestTimeout string `json:",omitempty"`
+		IdleTimeout    string `json:",omitempty"`
 		*Alias
 	}{
 		RequestTimeout: e.RequestTimeout.String(),
+		IdleTimeout:    e.IdleTimeout.String(),
 		Alias:          (*Alias)(e),
 	}
 	if e.RequestTimeout == 0 {
 		exported.RequestTimeout = ""
 	}
 
+	if e.IdleTimeout == 0 {
+		exported.IdleTimeout = ""
+	}
+
 	return json.Marshal(exported)
 }
 
 func (e *ServiceRouteDestination) UnmarshalJSON(data []byte) error {
 	type Alias ServiceRouteDestination
 	aux := &struct {
 		RequestTimeout string
+		IdleTimeout    string
 		*Alias
 	}{
 		Alias: (*Alias)(e),
@@ -481,6 +492,11 @@ func (e *ServiceRouteDestination) UnmarshalJSON(data []byte) error {
 			return err
 		}
 	}
+	if aux.IdleTimeout != "" {
+		if e.IdleTimeout, err = time.ParseDuration(aux.IdleTimeout); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

diff --git a/agent/structs/config_entry_test.go b/agent/structs/config_entry_test.go
@@ -603,6 +603,7 @@ func TestDecodeConfigEntry(t *testing.T) {
 						  namespace             = "leek"
 						  prefix_rewrite         = "/alternate"
 						  request_timeout        = "99s"
+						  idle_timeout           = "99s"
 						  num_retries            = 12345
 						  retry_on_connect_failure = true
 						  retry_on_status_codes    = [401, 209]
@@ -704,6 +705,7 @@ func TestDecodeConfigEntry(t *testing.T) {
 						  Namespace             = "leek"
 						  PrefixRewrite         = "/alternate"
 						  RequestTimeout        = "99s"
+						  IdleTimeout           = "99s"
 						  NumRetries            = 12345
 						  RetryOnConnectFailure = true
 						  RetryOnStatusCodes    = [401, 209]
@@ -805,6 +807,7 @@ func TestDecodeConfigEntry(t *testing.T) {
 							Namespace:             "leek",
 							PrefixRewrite:         "/alternate",
 							RequestTimeout:        99 * time.Second,
+							IdleTimeout:           99 * time.Second,
 							NumRetries:            12345,
 							RetryOnConnectFailure: true,
 							RetryOnStatusCodes:    []uint32{401, 209},

diff --git a/agent/xds/config.go b/agent/xds/config.go
@@ -49,6 +49,11 @@ type ProxyConfig struct {
 	// respected (15s)
 	LocalRequestTimeoutMs *int `mapstructure:"local_request_timeout_ms"`
 
+	// LocalIdleTimeoutMs is the number of milliseconds to timeout HTTP streams
+	// to the local app instance. If not set, no value is set, Envoy defaults are
+	// respected (300s)
+	LocalIdleTimeoutMs *int `mapstructure:"local_idle_timeout_ms"`
+
 	// Protocol describes the service's protocol. Valid values are "tcp",
 	// "http" and "grpc". Anything else is treated as tcp. This enables
 	// protocol aware features like per-request metrics and connection

diff --git a/agent/xds/config_test.go b/agent/xds/config_test.go
@@ -157,6 +157,39 @@ func TestParseProxyConfig(t *testing.T) {
 				Protocol:              "tcp",
 			},
 		},
+		{
+			name: "local idle timeout override, float ",
+			input: map[string]interface{}{
+				"local_idle_timeout_ms": float64(1000.0),
+			},
+			want: ProxyConfig{
+				LocalConnectTimeoutMs: 5000,
+				LocalIdleTimeoutMs:    intPointer(1000),
+				Protocol:              "tcp",
+			},
+		},
+		{
+			name: "local idle timeout override, int ",
+			input: map[string]interface{}{
+				"local_idle_timeout_ms": 1000,
+			},
+			want: ProxyConfig{
+				LocalConnectTimeoutMs: 5000,
+				LocalIdleTimeoutMs:    intPointer(1000),
+				Protocol:              "tcp",
+			},
+		},
+		{
+			name: "local idle timeout override, string",
+			input: map[string]interface{}{
+				"local_idle_timeout_ms": "1000",
+			},
+			want: ProxyConfig{
+				LocalConnectTimeoutMs: 5000,
+				LocalIdleTimeoutMs:    intPointer(1000),
+				Protocol:              "tcp",
+			},
+		},
 		{
 			name: "balance inbound connections override, string",
 			input: map[string]interface{}{

diff --git a/agent/xds/listeners.go b/agent/xds/listeners.go
@@ -1275,6 +1275,7 @@ func (s *ResourceGenerator) makeInboundListener(cfgSnap *proxycfg.ConfigSnapshot
 		routeName:        name,
 		cluster:          LocalAppClusterName,
 		requestTimeoutMs: cfg.LocalRequestTimeoutMs,
+		idleTimeoutMs:    cfg.LocalIdleTimeoutMs,
 		tracing:          tracing,
 	}
 	if useHTTPFilter {
@@ -2114,6 +2115,7 @@ type listenerFilterOpts struct {
 	statPrefix           string
 	routePath            string
 	requestTimeoutMs     *int
+	idleTimeoutMs        *int
 	ingressGateway       bool
 	httpAuthzFilter      *envoy_http_v3.HttpFilter
 	forwardClientDetails bool
@@ -2260,6 +2262,11 @@ func makeHTTPFilter(opts listenerFilterOpts) (*envoy_listener_v3.Filter, error)
 			r.Timeout = durationpb.New(time.Duration(*opts.requestTimeoutMs) * time.Millisecond)
 		}
 
+		if opts.idleTimeoutMs != nil {
+			r := route.GetRoute()
+			r.IdleTimeout = durationpb.New(time.Duration(*opts.idleTimeoutMs) * time.Millisecond)
+		}
+
 		// If a path is provided, do not match on a catch-all prefix
 		if opts.routePath != "" {
 			route.Match.PathSpecifier = &envoy_route_v3.RouteMatch_Path{Path: opts.routePath}

diff --git a/agent/xds/listeners_test.go b/agent/xds/listeners_test.go
@@ -230,6 +230,7 @@ func TestListenersFromSnapshot(t *testing.T) {
 					ns.Proxy.Config["protocol"] = "http"
 					ns.Proxy.Config["local_connect_timeout_ms"] = 1234
 					ns.Proxy.Config["local_request_timeout_ms"] = 2345
+					ns.Proxy.Config["local_idle_timeout_ms"] = 3456
 				}, nil)
 			},
 		},

diff --git a/agent/xds/routes.go b/agent/xds/routes.go
@@ -577,6 +577,10 @@ func (s *ResourceGenerator) makeUpstreamRouteForDiscoveryChain(
 					routeAction.Route.Timeout = durationpb.New(destination.RequestTimeout)
 				}
 
+				if destination.IdleTimeout > 0 {
+					routeAction.Route.IdleTimeout = durationpb.New(destination.IdleTimeout)
+				}
+
 				if destination.HasRetryFeatures() {
 					routeAction.Route.RetryPolicy = getRetryPolicyForDestination(destination)
 				}

diff --git a/agent/xds/testdata/listeners/http-listener-with-timeouts.latest.golden b/agent/xds/testdata/listeners/http-listener-with-timeouts.latest.golden
@@ -83,7 +83,8 @@
                           },
                           "route": {
                             "cluster": "local_app",
-                            "timeout": "2.345s"
+                            "timeout": "2.345s",
+                            "idleTimeout": "3.456s"
                           }
                         }
                       ]

diff --git a/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.latest.golden b/agent/xds/testdata/routes/connect-proxy-with-chain-and-router.latest.golden
@@ -274,6 +274,15 @@
                 "timeout": "33s"
               }
             },
+            {
+              "match": {
+                "prefix": "/idle-timeout"
+              },
+              "route": {
+                "cluster": "idle-timeout.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
+                "idleTimeout": "33s"
+              }
+            },
             {
               "match": {
                 "prefix": "/retry-connect"

diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.latest.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router-header-manip.latest.golden
@@ -275,6 +275,15 @@
                 "timeout": "33s"
               }
             },
+            {
+              "match": {
+                "prefix": "/idle-timeout"
+              },
+              "route": {
+                "cluster": "idle-timeout.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
+                "idleTimeout": "33s"
+              }
+            },
             {
               "match": {
                 "prefix": "/retry-connect"

diff --git a/agent/xds/testdata/routes/ingress-with-chain-and-router.latest.golden b/agent/xds/testdata/routes/ingress-with-chain-and-router.latest.golden
@@ -275,6 +275,15 @@
                 "timeout": "33s"
               }
             },
+	    {
+              "match": {
+                "prefix": "/idle-timeout"
+              },
+              "route": {
+                "cluster": "idle-timeout.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
+                "idleTimeout": "33s"
+              }
+            },
             {
               "match": {
                 "prefix": "/retry-connect"

diff --git a/api/config_entry_discoverychain.go b/api/config_entry_discoverychain.go
@@ -69,6 +69,7 @@ type ServiceRouteDestination struct {
 	Partition             string               `json:",omitempty"`
 	PrefixRewrite         string               `json:",omitempty" alias:"prefix_rewrite"`
 	RequestTimeout        time.Duration        `json:",omitempty" alias:"request_timeout"`
+	IdleTimeout           time.Duration        `json:",omitempty" alias:"idle_timeout"`
 	NumRetries            uint32               `json:",omitempty" alias:"num_retries"`
 	RetryOnConnectFailure bool                 `json:",omitempty" alias:"retry_on_connect_failure"`
 	RetryOnStatusCodes    []uint32             `json:",omitempty" alias:"retry_on_status_codes"`
@@ -81,14 +82,19 @@ func (e *ServiceRouteDestination) MarshalJSON() ([]byte, error) {
 	type Alias ServiceRouteDestination
 	exported := &struct {
 		RequestTimeout string `json:",omitempty"`
+		IdleTimeout    string `json:",omitempty"`
 		*Alias
 	}{
 		RequestTimeout: e.RequestTimeout.String(),
+		IdleTimeout:    e.IdleTimeout.String(),
 		Alias:          (*Alias)(e),
 	}
 	if e.RequestTimeout == 0 {
 		exported.RequestTimeout = ""
 	}
+	if e.IdleTimeout == 0 {
+		exported.IdleTimeout = ""
+	}
 
 	return json.Marshal(exported)
 }
@@ -97,6 +103,7 @@ func (e *ServiceRouteDestination) UnmarshalJSON(data []byte) error {
 	type Alias ServiceRouteDestination
 	aux := &struct {
 		RequestTimeout string
+		IdleTimeout    string
 		*Alias
 	}{
 		Alias: (*Alias)(e),
@@ -110,6 +117,11 @@ func (e *ServiceRouteDestination) UnmarshalJSON(data []byte) error {
 			return err
 		}
 	}
+	if aux.IdleTimeout != "" {
+		if e.IdleTimeout, err = time.ParseDuration(aux.IdleTimeout); err != nil {
+			return err
+		}
+	}
 	return nil
 }
 

diff --git a/api/config_entry_test.go b/api/config_entry_test.go
@@ -612,6 +612,7 @@ func TestDecodeConfigEntry(t *testing.T) {
 						  "Namespace": "leek",
 						  "PrefixRewrite": "/alternate",
 						  "RequestTimeout": "99s",
+						  "IdleTimeout": "99s",
 						  "NumRetries": 12345,
 						  "RetryOnConnectFailure": true,
 						  "RetryOnStatusCodes": [401, 209]
@@ -696,6 +697,7 @@ func TestDecodeConfigEntry(t *testing.T) {
 							Namespace:             "leek",
 							PrefixRewrite:         "/alternate",
 							RequestTimeout:        99 * time.Second,
+							IdleTimeout:           99 * time.Second,
 							NumRetries:            12345,
 							RetryOnConnectFailure: true,
 							RetryOnStatusCodes:    []uint32{401, 209},

diff --git a/command/helpers/helpers_test.go b/command/helpers/helpers_test.go
@@ -821,6 +821,7 @@ func TestParseConfigEntry(t *testing.T) {
 						  partition                = "chard"
 						  prefix_rewrite           = "/alternate"
 						  request_timeout          = "99s"
+						  idle_timeout             = "99s"
 						  num_retries              = 12345
 						  retry_on_connect_failure = true
 						  retry_on_status_codes    = [401, 209]
@@ -906,6 +907,7 @@ func TestParseConfigEntry(t *testing.T) {
 						  Partition             = "chard"
 						  PrefixRewrite         = "/alternate"
 						  RequestTimeout        = "99s"
+						  IdleTimeout           = "99s"
 						  NumRetries            = 12345
 						  RetryOnConnectFailure = true
 						  RetryOnStatusCodes    = [401, 209]
@@ -992,6 +994,7 @@ func TestParseConfigEntry(t *testing.T) {
 							"partition": "chard",
 							"prefix_rewrite": "/alternate",
 							"request_timeout": "99s",
+							"idle_timeout": "99s",
 							"num_retries": 12345,
 							"retry_on_connect_failure": true,
 							"retry_on_status_codes": [
@@ -1085,6 +1088,7 @@ func TestParseConfigEntry(t *testing.T) {
 							"Partition": "chard",
 							"PrefixRewrite": "/alternate",
 							"RequestTimeout": "99s",
+							"IdleTimeout": "99s",
 							"NumRetries": 12345,
 							"RetryOnConnectFailure": true,
 							"RetryOnStatusCodes": [
@@ -1177,6 +1181,7 @@ func TestParseConfigEntry(t *testing.T) {
 							Partition:             "chard",
 							PrefixRewrite:         "/alternate",
 							RequestTimeout:        99 * time.Second,
+							IdleTimeout:           99 * time.Second,
 							NumRetries:            12345,
 							RetryOnConnectFailure: true,
 							RetryOnStatusCodes:    []uint32{401, 209},

diff --git a/website/content/docs/connect/config-entries/service-router.mdx b/website/content/docs/connect/config-entries/service-router.mdx
@@ -696,6 +696,12 @@ spec:
       description:
         'The total amount of time permitted for the entire downstream request (and retries) to be processed.',
     },
+    {
+      name: 'IdleTimeout',
+      type: 'duration: 0',
+      description:
+        'The total amount of time permitted for the request stream to be idle',
+    },
     {
       name: 'NumRetries',
       type: 'int: 0',