From 7e78fb78180925e522f1a2cfe0c455e23363bb67 Mon Sep 17 00:00:00 2001 From: James Oulman Date: Tue, 29 Nov 2022 17:43:15 -0500 Subject: [PATCH] Add support for configuring Envoys route idle_timeout (#14340) * Add idleTimeout Co-authored-by: Jeff Boruszak <104028618+boruszak@users.noreply.github.com> Co-authored-by: Dhia Ayachi --- .changelog/14340.txt | 4 +++ agent/config/runtime_test.go | 3 ++ agent/proxycfg/testing_upstreams.go | 9 +++++ agent/structs/config_entry_discoverychain.go | 16 +++++++++ agent/structs/config_entry_test.go | 3 ++ agent/xds/config.go | 5 +++ agent/xds/config_test.go | 33 +++++++++++++++++++ agent/xds/listeners.go | 7 ++++ agent/xds/listeners_test.go | 1 + agent/xds/routes.go | 4 +++ .../http-listener-with-timeouts.latest.golden | 3 +- ...-proxy-with-chain-and-router.latest.golden | 9 +++++ ...hain-and-router-header-manip.latest.golden | 9 +++++ ...ngress-with-chain-and-router.latest.golden | 9 +++++ api/config_entry_discoverychain.go | 12 +++++++ api/config_entry_test.go | 2 ++ command/helpers/helpers_test.go | 5 +++ .../connect/config-entries/service-router.mdx | 6 ++++ .../content/docs/connect/proxies/envoy.mdx | 5 +++ 19 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 .changelog/14340.txt diff --git a/.changelog/14340.txt b/.changelog/14340.txt new file mode 100644 index 0000000000..f871e55428 --- /dev/null +++ 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 index 92fc452045..576873bfa0 100644 --- a/agent/config/runtime_test.go +++ b/agent/config/runtime_test.go @@ -4107,6 +4107,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] @@ -4195,6 +4196,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] @@ -4284,6 +4286,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 index bbf541f404..f35053cb43 100644 --- 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 index 5c58f1d394..2444e90101 100644 --- 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,15 +456,21 @@ 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) } @@ -468,6 +478,7 @@ 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 index 887f1d68fd..4b161a825a 100644 --- 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 index 1d75e0a907..648c847419 100644 --- 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 index 574449f1ee..89bd7a6c0a 100644 --- 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 index 577e3f51b5..e876b2dd1f 100644 --- 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 index 5cff623bb3..309dfaab5d 100644 --- 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 index 0ca73f6fd2..966daf983f 100644 --- 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 index a70876af29..a3c58e2ab8 100644 --- 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 index adc6611352..d34194b932 100644 --- 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 index eaca45d8ef..1fa06838f8 100644 --- 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 index a34ea1f68f..17c6603584 100644 --- 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 index 734f9454e4..ff92125dcd 100644 --- 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 index 376ad6182a..0344620810 100644 --- 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 index d4f426b364..4edc58349d 100644 --- 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 index 5f3722f51e..a1358a25f9 100644 --- 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', diff --git a/website/content/docs/connect/proxies/envoy.mdx b/website/content/docs/connect/proxies/envoy.mdx index 6ce8f66a04..c3981540a7 100644 --- a/website/content/docs/connect/proxies/envoy.mdx +++ b/website/content/docs/connect/proxies/envoy.mdx @@ -330,6 +330,11 @@ defaults that are inherited by all services. specified, inherits the Envoy default for route timeouts (15s). A value of 0 will disable request timeouts. +- `local_idle_timeout_ms` - In milliseconds, the idle timeout for HTTP requests + to the local application instance. Applies to HTTP based protocols only. If not + specified, inherits the Envoy default for route idle timeouts (15s). A value of 0 + disables request timeouts. + - `max_inbound_connections` - The maximum number of concurrent inbound connections to the local application instance. If not specified, inherits the Envoy default (1024).