From 22ddb708a57721acaf217f86e39ed57374b2887c Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Mon, 7 Feb 2022 14:51:41 +0200 Subject: [PATCH 1/6] Added configurable upstream connect timeout Signed-off-by: Tero Saarni --- apis/projectcontour/v1alpha1/contourconfig.go | 8 ++++++++ .../v1alpha1/zz_generated.deepcopy.go | 5 +++++ changelogs/unreleased/4326-tsaarni-small.md | 1 + cmd/contour/serve.go | 6 ++++++ cmd/contour/servecontext.go | 3 +++ cmd/contour/servecontext_test.go | 9 +++++++++ examples/contour/01-contour-config.yaml | 1 + examples/contour/01-crds.yaml | 14 ++++++++++++++ examples/render/contour-deployment.yaml | 15 +++++++++++++++ examples/render/contour-gateway.yaml | 15 +++++++++++++++ examples/render/contour.yaml | 15 +++++++++++++++ internal/contourconfig/contourconfiguration.go | 8 ++++++++ internal/dag/dag.go | 6 ++++++ internal/dag/extension_processor.go | 5 +++++ internal/dag/gatewayapi_processor.go | 12 +++++++++--- internal/dag/httpproxy_processor.go | 6 ++++++ internal/dag/ingress_processor.go | 5 +++++ internal/envoy/v3/cluster.go | 9 ++++++++- internal/envoy/v3/cluster_test.go | 16 ++++++++++++++++ internal/featuretests/v3/envoy.go | 1 - internal/xdscache/v3/cluster_test.go | 2 -- pkg/config/parameters.go | 13 +++++++++++++ pkg/config/parameters_test.go | 1 + site/content/docs/main/config/api-reference.html | 16 ++++++++++++++++ site/content/docs/main/configuration.md | 1 + 25 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 changelogs/unreleased/4326-tsaarni-small.md diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index 4a0ed0f78fe..46941d904ef 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -412,6 +412,14 @@ type TimeoutParameters struct { // for more information. // +optional ConnectionShutdownGracePeriod *string `json:"connectionShutdownGracePeriod,omitempty"` + + // ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. + // If not set, a default value of 2 seconds will be used. + // + // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + // for more information. + // +optional + ConnectTimeout *string `json:"connectTimeout"` } // ClusterDNSFamilyType is the Ip family to use for resolving DNS diff --git a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go index 0a5a3b0e203..1581420cfc2 100644 --- a/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go +++ b/apis/projectcontour/v1alpha1/zz_generated.deepcopy.go @@ -806,6 +806,11 @@ func (in *TimeoutParameters) DeepCopyInto(out *TimeoutParameters) { *out = new(string) **out = **in } + if in.ConnectTimeout != nil { + in, out := &in.ConnectTimeout, &out.ConnectTimeout + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TimeoutParameters. diff --git a/changelogs/unreleased/4326-tsaarni-small.md b/changelogs/unreleased/4326-tsaarni-small.md new file mode 100644 index 00000000000..e221d7a549a --- /dev/null +++ b/changelogs/unreleased/4326-tsaarni-small.md @@ -0,0 +1 @@ +Upstream TCP connection timeout is now configurable in [configuration file](https://projectcontour.io/docs/main/configuration/#timeout-configuration) and in [`ContourConfiguration`](https://projectcontour.io/docs/main/config/api/#projectcontour.io/v1alpha1.TimeoutParameters). diff --git a/cmd/contour/serve.go b/cmd/contour/serve.go index c89fd6db1eb..f042cafe3e1 100644 --- a/cmd/contour/serve.go +++ b/cmd/contour/serve.go @@ -401,6 +401,7 @@ func (s *Server) doServe() error { headersPolicy: contourConfiguration.Policy, clientCert: clientCert, fallbackCert: fallbackCert, + connectTimeout: timeouts.ConnectTimeout, }) // Build the core Kubernetes event handler. @@ -781,6 +782,7 @@ type dagBuilderConfig struct { headersPolicy *contour_api_v1alpha1.PolicyConfig clientCert *types.NamespacedName fallbackCert *types.NamespacedName + connectTimeout time.Duration } func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder { @@ -839,12 +841,14 @@ func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder { ClientCertificate: dbc.clientCert, RequestHeadersPolicy: &requestHeadersPolicyIngress, ResponseHeadersPolicy: &responseHeadersPolicyIngress, + ConnectTimeout: dbc.connectTimeout, }, &dag.ExtensionServiceProcessor{ // Note that ExtensionService does not support ExternalName, if it does get added, // need to bring EnableExternalNameService in here too. FieldLogger: s.log.WithField("context", "ExtensionServiceProcessor"), ClientCertificate: dbc.clientCert, + ConnectTimeout: dbc.connectTimeout, }, &dag.HTTPProxyProcessor{ EnableExternalNameService: dbc.enableExternalNameService, @@ -854,6 +858,7 @@ func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder { ClientCertificate: dbc.clientCert, RequestHeadersPolicy: &requestHeadersPolicy, ResponseHeadersPolicy: &responseHeadersPolicy, + ConnectTimeout: dbc.connectTimeout, }, } @@ -861,6 +866,7 @@ func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder { dagProcessors = append(dagProcessors, &dag.GatewayAPIProcessor{ EnableExternalNameService: dbc.enableExternalNameService, FieldLogger: s.log.WithField("context", "GatewayAPIProcessor"), + ConnectTimeout: dbc.connectTimeout, }) } diff --git a/cmd/contour/servecontext.go b/cmd/contour/servecontext.go index 4f0d14cf2c7..887004894f1 100644 --- a/cmd/contour/servecontext.go +++ b/cmd/contour/servecontext.go @@ -330,6 +330,9 @@ func (ctx *serveContext) convertToContourConfigurationSpec() contour_api_v1alpha if len(ctx.Config.Timeouts.ConnectionShutdownGracePeriod) > 0 { timeoutParams.ConnectionShutdownGracePeriod = pointer.StringPtr(ctx.Config.Timeouts.ConnectionShutdownGracePeriod) } + if len(ctx.Config.Timeouts.ConnectTimeout) > 0 { + timeoutParams.ConnectTimeout = pointer.StringPtr(ctx.Config.Timeouts.ConnectTimeout) + } var dnsLookupFamily contour_api_v1alpha1.ClusterDNSFamilyType switch ctx.Config.Cluster.DNSLookupFamily { diff --git a/cmd/contour/servecontext_test.go b/cmd/contour/servecontext_test.go index 0afd69f17b2..71b542bb67a 100644 --- a/cmd/contour/servecontext_test.go +++ b/cmd/contour/servecontext_test.go @@ -512,6 +512,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -617,6 +618,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -728,6 +730,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -833,6 +836,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -943,6 +947,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -1048,6 +1053,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -1156,6 +1162,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -1271,6 +1278,7 @@ func TestConvertServeContext(t *testing.T) { }, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, @@ -1356,6 +1364,7 @@ func TestConvertServeContext(t *testing.T) { DefaultHTTPVersions: nil, Timeouts: &contour_api_v1alpha1.TimeoutParameters{ ConnectionIdleTimeout: pointer.StringPtr("60s"), + ConnectTimeout: pointer.StringPtr("2s"), }, Cluster: contour_api_v1alpha1.ClusterParameters{ DNSLookupFamily: contour_api_v1alpha1.AutoClusterDNSFamily, diff --git a/examples/contour/01-contour-config.yaml b/examples/contour/01-contour-config.yaml index 1a7a12fdb48..1e9c08f240d 100644 --- a/examples/contour/01-contour-config.yaml +++ b/examples/contour/01-contour-config.yaml @@ -106,6 +106,7 @@ data: # max-connection-duration: infinity # delayed-close-timeout: 1s # connection-shutdown-grace-period: 5s + # connect-timeout: 2s # # Envoy cluster settings. # cluster: diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 5cdd11c3059..5f52a2cac3c 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -402,6 +402,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream service. + If not set, a default value of 2 seconds will be used. \n + See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests (for HTTP/1.1) @@ -1329,6 +1336,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream + service. If not set, a default value of 2 seconds will + be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index 3367d67e831..711526df801 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -139,6 +139,7 @@ data: # max-connection-duration: infinity # delayed-close-timeout: 1s # connection-shutdown-grace-period: 5s + # connect-timeout: 2s # # Envoy cluster settings. # cluster: @@ -594,6 +595,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream service. + If not set, a default value of 2 seconds will be used. \n + See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests (for HTTP/1.1) @@ -1521,6 +1529,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream + service. If not set, a default value of 2 seconds will + be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index 6361053afe8..bf449071517 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -142,6 +142,7 @@ data: # max-connection-duration: infinity # delayed-close-timeout: 1s # connection-shutdown-grace-period: 5s + # connect-timeout: 2s # # Envoy cluster settings. # cluster: @@ -597,6 +598,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream service. + If not set, a default value of 2 seconds will be used. \n + See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests (for HTTP/1.1) @@ -1524,6 +1532,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream + service. If not set, a default value of 2 seconds will + be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index 35df5115922..d2a7a2df424 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -139,6 +139,7 @@ data: # max-connection-duration: infinity # delayed-close-timeout: 1s # connection-shutdown-grace-period: 5s + # connect-timeout: 2s # # Envoy cluster settings. # cluster: @@ -594,6 +595,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream service. + If not set, a default value of 2 seconds will be used. \n + See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests (for HTTP/1.1) @@ -1521,6 +1529,13 @@ spec: description: Timeouts holds various configurable timeouts that can be set in the config file. properties: + connectTimeout: + description: "ConnectionTimeout defines how long the proxy + should wait when establishing connection to upstream + service. If not set, a default value of 2 seconds will + be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + for more information." + type: string connectionIdleTimeout: description: "ConnectionIdleTimeout defines how long the proxy should wait while there are no active requests diff --git a/internal/contourconfig/contourconfiguration.go b/internal/contourconfig/contourconfiguration.go index 3fb2983d8f5..60856c1c823 100644 --- a/internal/contourconfig/contourconfiguration.go +++ b/internal/contourconfig/contourconfiguration.go @@ -15,6 +15,7 @@ package contourconfig import ( "fmt" + "time" contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" "github.com/projectcontour/contour/internal/timeout" @@ -27,6 +28,7 @@ type Timeouts struct { MaxConnectionDuration timeout.Setting DelayedClose timeout.Setting ConnectionShutdownGracePeriod timeout.Setting + ConnectTimeout time.Duration // Since "infinite" is not valid ConnectTimeout value, use time.Duration instead of timeout.Setting. } func ParseTimeoutPolicy(timeoutParameters *contour_api_v1alpha1.TimeoutParameters) (Timeouts, error) { @@ -72,6 +74,12 @@ func ParseTimeoutPolicy(timeoutParameters *contour_api_v1alpha1.TimeoutParameter return Timeouts{}, fmt.Errorf("failed to parse connection shutdown grace period: %s", err) } } + if timeoutParameters.ConnectTimeout != nil { + timeouts.ConnectTimeout, err = time.ParseDuration(*timeoutParameters.ConnectTimeout) + if err != nil { + return Timeouts{}, fmt.Errorf("failed to parse connect timeout: %s", err) + } + } } return timeouts, nil } diff --git a/internal/dag/dag.go b/internal/dag/dag.go index 95d869c9dde..2209e45d6c3 100644 --- a/internal/dag/dag.go +++ b/internal/dag/dag.go @@ -685,6 +685,9 @@ type Cluster struct { // ClientCertificate is the optional identifier of the TLS secret containing client certificate and // private key to be used when establishing TLS connection to upstream cluster. ClientCertificate *Secret + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } // WeightedService represents the load balancing weight of a @@ -856,6 +859,9 @@ type ExtensionCluster struct { // ClientCertificate is the optional identifier of the TLS secret containing client certificate and // private key to be used when establishing TLS connection to upstream cluster. ClientCertificate *Secret + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } func wildcardDomainHeaderMatch(fqdn string) HeaderMatchCondition { diff --git a/internal/dag/extension_processor.go b/internal/dag/extension_processor.go index aed98eccf37..cd3d4585df4 100644 --- a/internal/dag/extension_processor.go +++ b/internal/dag/extension_processor.go @@ -16,6 +16,7 @@ package dag import ( "path" "strings" + "time" contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" @@ -35,6 +36,9 @@ type ExtensionServiceProcessor struct { // secret containing client certificate and private key to be // used when establishing TLS connection to upstream cluster. ClientCertificate *types.NamespacedName + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } var _ Processor = &ExtensionServiceProcessor{} @@ -108,6 +112,7 @@ func (p *ExtensionServiceProcessor) buildExtensionService( TimeoutPolicy: tp, SNI: "", ClientCertificate: clientCertSecret, + ConnectTimeout: p.ConnectTimeout, } lbPolicy := loadBalancerPolicy(ext.Spec.LoadBalancerPolicy) diff --git a/internal/dag/gatewayapi_processor.go b/internal/dag/gatewayapi_processor.go index a632606c727..3109f3d52f2 100644 --- a/internal/dag/gatewayapi_processor.go +++ b/internal/dag/gatewayapi_processor.go @@ -18,6 +18,7 @@ import ( "net" "net/http" "strings" + "time" "github.com/projectcontour/contour/internal/errors" "github.com/projectcontour/contour/internal/k8s" @@ -52,6 +53,9 @@ type GatewayAPIProcessor struct { // This is normally disabled for security reasons. // See https://github.com/projectcontour/contour/security/advisories/GHSA-5ph6-qq5x-7jwc for details. EnableExternalNameService bool + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } // matchConditions holds match rules. @@ -720,9 +724,10 @@ func (p *GatewayAPIProcessor) computeTLSRoute(route *gatewayapi_v1alpha2.TLSRout // https://github.com/projectcontour/contour/issues/3593 service.Weighted.Weight = routeWeight proxy.Clusters = append(proxy.Clusters, &Cluster{ - Upstream: service, - SNI: service.ExternalName, - Weight: routeWeight, + Upstream: service, + SNI: service.ExternalName, + Weight: routeWeight, + ConnectTimeout: p.ConnectTimeout, }) } @@ -1072,6 +1077,7 @@ func (p *GatewayAPIProcessor) clusterRoutes(routeNamespace string, matchConditio Weight: routeWeight, Protocol: service.Protocol, RequestHeadersPolicy: headerPolicy, + ConnectTimeout: p.ConnectTimeout, }) } diff --git a/internal/dag/httpproxy_processor.go b/internal/dag/httpproxy_processor.go index a7c4d6d26d2..23df3a3918b 100644 --- a/internal/dag/httpproxy_processor.go +++ b/internal/dag/httpproxy_processor.go @@ -19,6 +19,7 @@ import ( "sort" "strconv" "strings" + "time" contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1" contour_api_v1alpha1 "github.com/projectcontour/contour/apis/projectcontour/v1alpha1" @@ -84,6 +85,9 @@ type HTTPProxyProcessor struct { // Response headers that will be set on all routes (optional). ResponseHeadersPolicy *HeadersPolicy + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } // Run translates HTTPProxies into DAG objects and @@ -694,6 +698,7 @@ func (p *HTTPProxyProcessor) computeRoutes( SNI: determineSNI(r.RequestHeadersPolicy, reqHP, s), DNSLookupFamily: string(p.DNSLookupFamily), ClientCertificate: clientCertSecret, + ConnectTimeout: p.ConnectTimeout, } if service.Mirror && r.MirrorPolicy != nil { validCond.AddError(contour_api_v1.ConditionTypeServiceError, "OnlyOneMirror", @@ -789,6 +794,7 @@ func (p *HTTPProxyProcessor) processHTTPProxyTCPProxy(validCond *contour_api_v1. LoadBalancerPolicy: lbPolicy, TCPHealthCheckPolicy: tcpHealthCheckPolicy(tcpproxy.HealthCheckPolicy), SNI: s.ExternalName, + ConnectTimeout: p.ConnectTimeout, }) } secure := p.dag.EnsureSecureVirtualHost(host) diff --git a/internal/dag/ingress_processor.go b/internal/dag/ingress_processor.go index 42a24706427..b6b6f5df2f5 100644 --- a/internal/dag/ingress_processor.go +++ b/internal/dag/ingress_processor.go @@ -17,6 +17,7 @@ import ( "regexp" "strconv" "strings" + "time" "k8s.io/apimachinery/pkg/util/intstr" @@ -49,6 +50,9 @@ type IngressProcessor struct { // Response headers that will be set on all routes (optional). ResponseHeadersPolicy *HeadersPolicy + + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. + ConnectTimeout time.Duration } // Run translates Ingresses into DAG objects and @@ -229,6 +233,7 @@ func (p *IngressProcessor) route(ingress *networking_v1.Ingress, host string, pa ClientCertificate: clientCertSecret, RequestHeadersPolicy: reqHP, ResponseHeadersPolicy: respHP, + ConnectTimeout: p.ConnectTimeout, }}, } diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index 9fee91a850f..7eb3e393774 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -31,7 +31,6 @@ import ( func clusterDefaults() *envoy_cluster_v3.Cluster { return &envoy_cluster_v3.Cluster{ - ConnectTimeout: protobuf.Duration(2 * time.Second), CommonLbConfig: ClusterCommonLBConfig(), LbPolicy: lbPolicy(dag.LoadBalancerPolicyRoundRobin), } @@ -98,6 +97,10 @@ func Cluster(c *dag.Cluster) *envoy_cluster_v3.Cluster { cluster.TypedExtensionProtocolOptions = http2ProtocolOptions() } + if c.ConnectTimeout > time.Duration(0) { + cluster.ConnectTimeout = protobuf.Duration(c.ConnectTimeout) + } + return cluster } @@ -143,6 +146,10 @@ func ExtensionCluster(ext *dag.ExtensionCluster) *envoy_cluster_v3.Cluster { cluster.TypedExtensionProtocolOptions = http2ProtocolOptions() } + if ext.ConnectTimeout > time.Duration(0) { + cluster.ConnectTimeout = protobuf.Duration(ext.ConnectTimeout) + } + return cluster } diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go index 24a2eeea2c3..6fbf484b4ff 100644 --- a/internal/envoy/v3/cluster_test.go +++ b/internal/envoy/v3/cluster_test.go @@ -508,6 +508,22 @@ func TestCluster(t *testing.T) { ), }, }, + "cluster with connect timeout set": { + cluster: &dag.Cluster{ + Upstream: service(s1), + ConnectTimeout: 2 * time.Second, + }, + want: &envoy_cluster_v3.Cluster{ + Name: "default/kuard/443/da39a3ee5e", + AltStatName: "default_kuard_443", + ClusterDiscoveryType: ClusterDiscoveryType(envoy_cluster_v3.Cluster_EDS), + EdsClusterConfig: &envoy_cluster_v3.Cluster_EdsClusterConfig{ + EdsConfig: ConfigSource("contour"), + ServiceName: "default/kuard/http", + }, + ConnectTimeout: protobuf.Duration(2 * time.Second), + }, + }, } for name, tc := range tests { diff --git a/internal/featuretests/v3/envoy.go b/internal/featuretests/v3/envoy.go index 1e16a026b9a..5d5117fa7d8 100644 --- a/internal/featuretests/v3/envoy.go +++ b/internal/featuretests/v3/envoy.go @@ -49,7 +49,6 @@ import ( func DefaultCluster(clusters ...*envoy_cluster_v3.Cluster) *envoy_cluster_v3.Cluster { // NOTE: Keep this in sync with envoy.defaultCluster(). defaults := &envoy_cluster_v3.Cluster{ - ConnectTimeout: protobuf.Duration(2 * time.Second), LbPolicy: envoy_cluster_v3.Cluster_ROUND_ROBIN, CommonLbConfig: envoy_v3.ClusterCommonLBConfig(), } diff --git a/internal/xdscache/v3/cluster_test.go b/internal/xdscache/v3/cluster_test.go index 66f04df3d20..937719f75e2 100644 --- a/internal/xdscache/v3/cluster_test.go +++ b/internal/xdscache/v3/cluster_test.go @@ -15,7 +15,6 @@ package v3 import ( "testing" - "time" envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -845,7 +844,6 @@ func serviceWithAnnotations(ns, name string, annotations map[string]string, port func cluster(c *envoy_cluster_v3.Cluster) *envoy_cluster_v3.Cluster { // NOTE: Keep this in sync with envoy.defaultCluster(). defaults := &envoy_cluster_v3.Cluster{ - ConnectTimeout: protobuf.Duration(2 * time.Second), CommonLbConfig: envoy_v3.ClusterCommonLBConfig(), LbPolicy: envoy_cluster_v3.Cluster_ROUND_ROBIN, } diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index bd2707b75c2..985a287c8f7 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -422,6 +422,14 @@ type TimeoutParameters struct { // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/http_connection_manager/v3/http_connection_manager.proto#envoy-v3-api-field-extensions-filters-network-http-connection-manager-v3-httpconnectionmanager-drain-timeout // for more information. ConnectionShutdownGracePeriod string `yaml:"connection-shutdown-grace-period,omitempty"` + + // ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. + // If not set, a default value of 2 seconds will be used. + // + // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + // for more information. + // +optional + ConnectTimeout string `yaml:"connect-timeout"` } // Validate the timeout parameters. @@ -463,6 +471,10 @@ func (t TimeoutParameters) Validate() error { return fmt.Errorf("connection shutdown grace period %q: %w", t.ConnectionShutdownGracePeriod, err) } + if err := v(t.ConnectTimeout); err != nil { + return fmt.Errorf("connect timeout %q: %w", t.ConnectTimeout, err) + } + return nil } @@ -813,6 +825,7 @@ func Defaults() Parameters { // This is chosen as a rough default to stop idle connections wasting resources, // without stopping slow connections from being terminated too quickly. ConnectionIdleTimeout: "60s", + ConnectTimeout: "2s", }, Policy: PolicyParameters{ RequestHeadersPolicy: HeadersPolicy{}, diff --git a/pkg/config/parameters_test.go b/pkg/config/parameters_test.go index a650e36f0e4..3d3bb59f880 100644 --- a/pkg/config/parameters_test.go +++ b/pkg/config/parameters_test.go @@ -80,6 +80,7 @@ leaderelection: configmap-name: leader-elect timeouts: connection-idle-timeout: 60s + connect-timeout: 2s envoy-service-namespace: projectcontour envoy-service-name: envoy default-http-versions: [] diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index dad3b22eafe..a413f5c2563 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -6078,6 +6078,22 @@

TimeoutParameters for more information.

+ + +connectTimeout +
+ +string + + + +(Optional) +

ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. +If not set, a default value of 2 seconds will be used.

+

See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout +for more information.

+ +

XDSServerConfig diff --git a/site/content/docs/main/configuration.md b/site/content/docs/main/configuration.md index c6d9576036c..6c35d5ee66c 100644 --- a/site/content/docs/main/configuration.md +++ b/site/content/docs/main/configuration.md @@ -149,6 +149,7 @@ The timeout configuration block can be used to configure various timeouts for th | max-connection-duration | string | none* | This field defines the maximum period of time after an HTTP connection has been established from the client to the proxy before it is closed by the proxy, regardless of whether there has been activity or not. Must be a [valid Go duration string][4], or omitted or set to `infinity` for no max duration. See [the Envoy documentation][10] for more information. | | delayed-close-timeout | string | `1s`* | *Note: this is an advanced setting that should not normally need to be tuned.*

This field defines how long envoy will wait, once connection close processing has been initiated, for the downstream peer to close the connection before Envoy closes the socket associated with the connection. Setting this timeout to 'infinity' will disable it. See [the Envoy documentation][13] for more information. | | connection-shutdown-grace-period | string | `5s`* | This field defines how long the proxy will wait between sending an initial GOAWAY frame and a second, final GOAWAY frame when terminating an HTTP/2 connection. During this grace period, the proxy will continue to respond to new streams. After the final GOAWAY frame has been sent, the proxy will refuse new streams. Must be a [valid Go duration string][4]. See [the Envoy documentation][11] for more information. | +| connect-timeout | string | `2s` | This field defines how long the proxy will wait for the upstream connection to be established. _This is Envoy's default setting value and is not explicitly configured by Contour._ From 807d0e653cb296fc2977bd32add1fc346a1c88d4 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Tue, 8 Feb 2022 12:33:48 +0200 Subject: [PATCH 2/6] Do not allow infinite for connect-timeout config file parameter. Signed-off-by: Tero Saarni --- pkg/config/parameters.go | 11 ++++++++--- pkg/config/parameters_test.go | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index 985a287c8f7..46600fcf255 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -429,7 +429,7 @@ type TimeoutParameters struct { // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout // for more information. // +optional - ConnectTimeout string `yaml:"connect-timeout"` + ConnectTimeout string `yaml:"connect-timeout,omitempty"` } // Validate the timeout parameters. @@ -471,8 +471,13 @@ func (t TimeoutParameters) Validate() error { return fmt.Errorf("connection shutdown grace period %q: %w", t.ConnectionShutdownGracePeriod, err) } - if err := v(t.ConnectTimeout); err != nil { - return fmt.Errorf("connect timeout %q: %w", t.ConnectTimeout, err) + // ConnectTimeout is normally implicitly set to 2s in Defaults(). + // If user sets "" then Envoy built-in default should be used. + // ConnectTimeout cannot be "infinite" so use time.ParseDuration() directly instead of v(). + if t.ConnectTimeout != "" { + if _, err := time.ParseDuration(t.ConnectTimeout); err != nil { + return fmt.Errorf("connect timeout %q: %w", t.ConnectTimeout, err) + } } return nil diff --git a/pkg/config/parameters_test.go b/pkg/config/parameters_test.go index 3d3bb59f880..ab59beb79c4 100644 --- a/pkg/config/parameters_test.go +++ b/pkg/config/parameters_test.go @@ -260,6 +260,7 @@ func TestValidateTimeoutParams(t *testing.T) { MaxConnectionDuration: "infinite", DelayedCloseTimeout: "infinite", ConnectionShutdownGracePeriod: "infinite", + ConnectTimeout: "2s", }.Validate()) assert.NoError(t, TimeoutParameters{ RequestTimeout: "infinity", @@ -268,6 +269,7 @@ func TestValidateTimeoutParams(t *testing.T) { MaxConnectionDuration: "infinity", DelayedCloseTimeout: "infinity", ConnectionShutdownGracePeriod: "infinity", + ConnectTimeout: "2s", }.Validate()) assert.Error(t, TimeoutParameters{RequestTimeout: "foo"}.Validate()) @@ -276,6 +278,7 @@ func TestValidateTimeoutParams(t *testing.T) { assert.Error(t, TimeoutParameters{MaxConnectionDuration: "boop"}.Validate()) assert.Error(t, TimeoutParameters{DelayedCloseTimeout: "bebop"}.Validate()) assert.Error(t, TimeoutParameters{ConnectionShutdownGracePeriod: "bong"}.Validate()) + assert.Error(t, TimeoutParameters{ConnectTimeout: "infinite"}.Validate()) } From 8a4825f9ad6072cd5bc1fdd0ff1ae9362a95fae1 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Thu, 10 Feb 2022 14:57:43 +0200 Subject: [PATCH 3/6] Fixed field names to ConnectTimeout Signed-off-by: Tero Saarni --- apis/projectcontour/v1alpha1/contourconfig.go | 2 +- examples/contour/01-crds.yaml | 10 +++++----- examples/render/contour-deployment.yaml | 10 +++++----- examples/render/contour-gateway.yaml | 10 +++++----- examples/render/contour.yaml | 10 +++++----- pkg/config/parameters.go | 2 +- site/content/docs/main/config/api-reference.html | 2 +- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/apis/projectcontour/v1alpha1/contourconfig.go b/apis/projectcontour/v1alpha1/contourconfig.go index 46941d904ef..b434bee1bd3 100644 --- a/apis/projectcontour/v1alpha1/contourconfig.go +++ b/apis/projectcontour/v1alpha1/contourconfig.go @@ -413,7 +413,7 @@ type TimeoutParameters struct { // +optional ConnectionShutdownGracePeriod *string `json:"connectionShutdownGracePeriod,omitempty"` - // ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. // If not set, a default value of 2 seconds will be used. // // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/examples/contour/01-crds.yaml b/examples/contour/01-crds.yaml index 5f52a2cac3c..70c18efa6f3 100644 --- a/examples/contour/01-crds.yaml +++ b/examples/contour/01-crds.yaml @@ -403,10 +403,10 @@ spec: can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy - should wait when establishing connection to upstream service. - If not set, a default value of 2 seconds will be used. \n - See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + description: "ConnectTimeout defines how long the proxy should + wait when establishing connection to upstream service. If + not set, a default value of 2 seconds will be used. \n See + https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout for more information." type: string connectionIdleTimeout: @@ -1337,7 +1337,7 @@ spec: that can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy + description: "ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. If not set, a default value of 2 seconds will be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/examples/render/contour-deployment.yaml b/examples/render/contour-deployment.yaml index 711526df801..bcd1499b725 100644 --- a/examples/render/contour-deployment.yaml +++ b/examples/render/contour-deployment.yaml @@ -596,10 +596,10 @@ spec: can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy - should wait when establishing connection to upstream service. - If not set, a default value of 2 seconds will be used. \n - See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + description: "ConnectTimeout defines how long the proxy should + wait when establishing connection to upstream service. If + not set, a default value of 2 seconds will be used. \n See + https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout for more information." type: string connectionIdleTimeout: @@ -1530,7 +1530,7 @@ spec: that can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy + description: "ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. If not set, a default value of 2 seconds will be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/examples/render/contour-gateway.yaml b/examples/render/contour-gateway.yaml index bf449071517..0ad78520b37 100644 --- a/examples/render/contour-gateway.yaml +++ b/examples/render/contour-gateway.yaml @@ -599,10 +599,10 @@ spec: can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy - should wait when establishing connection to upstream service. - If not set, a default value of 2 seconds will be used. \n - See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + description: "ConnectTimeout defines how long the proxy should + wait when establishing connection to upstream service. If + not set, a default value of 2 seconds will be used. \n See + https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout for more information." type: string connectionIdleTimeout: @@ -1533,7 +1533,7 @@ spec: that can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy + description: "ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. If not set, a default value of 2 seconds will be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/examples/render/contour.yaml b/examples/render/contour.yaml index d2a7a2df424..52a12d29696 100644 --- a/examples/render/contour.yaml +++ b/examples/render/contour.yaml @@ -596,10 +596,10 @@ spec: can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy - should wait when establishing connection to upstream service. - If not set, a default value of 2 seconds will be used. \n - See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout + description: "ConnectTimeout defines how long the proxy should + wait when establishing connection to upstream service. If + not set, a default value of 2 seconds will be used. \n See + https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout for more information." type: string connectionIdleTimeout: @@ -1530,7 +1530,7 @@ spec: that can be set in the config file. properties: connectTimeout: - description: "ConnectionTimeout defines how long the proxy + description: "ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. If not set, a default value of 2 seconds will be used. \n See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index 46600fcf255..188c52f395b 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -423,7 +423,7 @@ type TimeoutParameters struct { // for more information. ConnectionShutdownGracePeriod string `yaml:"connection-shutdown-grace-period,omitempty"` - // ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. + // ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. // If not set, a default value of 2 seconds will be used. // // See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout diff --git a/site/content/docs/main/config/api-reference.html b/site/content/docs/main/config/api-reference.html index a413f5c2563..373bf4d2035 100644 --- a/site/content/docs/main/config/api-reference.html +++ b/site/content/docs/main/config/api-reference.html @@ -6088,7 +6088,7 @@

TimeoutParameters (Optional) -

ConnectionTimeout defines how long the proxy should wait when establishing connection to upstream service. +

ConnectTimeout defines how long the proxy should wait when establishing connection to upstream service. If not set, a default value of 2 seconds will be used.

See https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/cluster/v3/cluster.proto#envoy-v3-api-field-config-cluster-v3-cluster-connect-timeout for more information.

From a26474449b4e9295b2586e12c134fc1ddee771e4 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Thu, 10 Feb 2022 17:15:27 +0200 Subject: [PATCH 4/6] The default timeout needs to be set in cluster.go for empty ContourConfig CRD to result in default timeout Signed-off-by: Tero Saarni --- internal/envoy/v3/cluster.go | 1 + internal/featuretests/v3/envoy.go | 1 + internal/xdscache/v3/cluster_test.go | 2 ++ pkg/config/parameters.go | 1 - 4 files changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/envoy/v3/cluster.go b/internal/envoy/v3/cluster.go index 7eb3e393774..2e748a9e6df 100644 --- a/internal/envoy/v3/cluster.go +++ b/internal/envoy/v3/cluster.go @@ -31,6 +31,7 @@ import ( func clusterDefaults() *envoy_cluster_v3.Cluster { return &envoy_cluster_v3.Cluster{ + ConnectTimeout: protobuf.Duration(2 * time.Second), CommonLbConfig: ClusterCommonLBConfig(), LbPolicy: lbPolicy(dag.LoadBalancerPolicyRoundRobin), } diff --git a/internal/featuretests/v3/envoy.go b/internal/featuretests/v3/envoy.go index 5d5117fa7d8..1e16a026b9a 100644 --- a/internal/featuretests/v3/envoy.go +++ b/internal/featuretests/v3/envoy.go @@ -49,6 +49,7 @@ import ( func DefaultCluster(clusters ...*envoy_cluster_v3.Cluster) *envoy_cluster_v3.Cluster { // NOTE: Keep this in sync with envoy.defaultCluster(). defaults := &envoy_cluster_v3.Cluster{ + ConnectTimeout: protobuf.Duration(2 * time.Second), LbPolicy: envoy_cluster_v3.Cluster_ROUND_ROBIN, CommonLbConfig: envoy_v3.ClusterCommonLBConfig(), } diff --git a/internal/xdscache/v3/cluster_test.go b/internal/xdscache/v3/cluster_test.go index 937719f75e2..66f04df3d20 100644 --- a/internal/xdscache/v3/cluster_test.go +++ b/internal/xdscache/v3/cluster_test.go @@ -15,6 +15,7 @@ package v3 import ( "testing" + "time" envoy_cluster_v3 "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" envoy_core_v3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -844,6 +845,7 @@ func serviceWithAnnotations(ns, name string, annotations map[string]string, port func cluster(c *envoy_cluster_v3.Cluster) *envoy_cluster_v3.Cluster { // NOTE: Keep this in sync with envoy.defaultCluster(). defaults := &envoy_cluster_v3.Cluster{ + ConnectTimeout: protobuf.Duration(2 * time.Second), CommonLbConfig: envoy_v3.ClusterCommonLBConfig(), LbPolicy: envoy_cluster_v3.Cluster_ROUND_ROBIN, } diff --git a/pkg/config/parameters.go b/pkg/config/parameters.go index 188c52f395b..f34d05af6f2 100644 --- a/pkg/config/parameters.go +++ b/pkg/config/parameters.go @@ -472,7 +472,6 @@ func (t TimeoutParameters) Validate() error { } // ConnectTimeout is normally implicitly set to 2s in Defaults(). - // If user sets "" then Envoy built-in default should be used. // ConnectTimeout cannot be "infinite" so use time.ParseDuration() directly instead of v(). if t.ConnectTimeout != "" { if _, err := time.ParseDuration(t.ConnectTimeout); err != nil { From ec030f8d27e19ee27bedff9698d5c605294e3e4c Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Fri, 11 Feb 2022 12:26:13 +0200 Subject: [PATCH 5/6] Do not use default timeout in test case Signed-off-by: Tero Saarni --- internal/envoy/v3/cluster_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go index 6fbf484b4ff..54337e026ef 100644 --- a/internal/envoy/v3/cluster_test.go +++ b/internal/envoy/v3/cluster_test.go @@ -511,7 +511,7 @@ func TestCluster(t *testing.T) { "cluster with connect timeout set": { cluster: &dag.Cluster{ Upstream: service(s1), - ConnectTimeout: 2 * time.Second, + ConnectTimeout: 10 * time.Second, }, want: &envoy_cluster_v3.Cluster{ Name: "default/kuard/443/da39a3ee5e", @@ -521,7 +521,7 @@ func TestCluster(t *testing.T) { EdsConfig: ConfigSource("contour"), ServiceName: "default/kuard/http", }, - ConnectTimeout: protobuf.Duration(2 * time.Second), + ConnectTimeout: protobuf.Duration(10 * time.Second), }, }, } From 5c7808e2076085481964839a3a2757d0073631f2 Mon Sep 17 00:00:00 2001 From: Tero Saarni Date: Mon, 14 Feb 2022 12:52:13 +0200 Subject: [PATCH 6/6] improved test coverage Signed-off-by: Tero Saarni --- internal/contourconfig/contourconfiguration_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/contourconfig/contourconfiguration_test.go b/internal/contourconfig/contourconfiguration_test.go index 92cbca8b696..148519226f4 100644 --- a/internal/contourconfig/contourconfiguration_test.go +++ b/internal/contourconfig/contourconfiguration_test.go @@ -39,6 +39,7 @@ func TestParseTimeoutPolicy(t *testing.T) { MaxConnectionDuration: timeout.DefaultSetting(), DelayedClose: timeout.DefaultSetting(), ConnectionShutdownGracePeriod: timeout.DefaultSetting(), + ConnectTimeout: 0, }, }, "timeouts not set": { @@ -50,6 +51,7 @@ func TestParseTimeoutPolicy(t *testing.T) { MaxConnectionDuration: timeout.DefaultSetting(), DelayedClose: timeout.DefaultSetting(), ConnectionShutdownGracePeriod: timeout.DefaultSetting(), + ConnectTimeout: 0, }, }, "timeouts all set": { @@ -60,6 +62,7 @@ func TestParseTimeoutPolicy(t *testing.T) { MaxConnectionDuration: pointer.String("infinity"), DelayedCloseTimeout: pointer.String("5s"), ConnectionShutdownGracePeriod: pointer.String("6s"), + ConnectTimeout: pointer.String("8s"), }, expected: contourconfig.Timeouts{ Request: timeout.DurationSetting(time.Second), @@ -68,6 +71,7 @@ func TestParseTimeoutPolicy(t *testing.T) { MaxConnectionDuration: timeout.DisabledSetting(), DelayedClose: timeout.DurationSetting(time.Second * 5), ConnectionShutdownGracePeriod: timeout.DurationSetting(time.Second * 6), + ConnectTimeout: 8 * time.Second, }, }, "request timeout invalid": { @@ -106,6 +110,12 @@ func TestParseTimeoutPolicy(t *testing.T) { }, errorMsg: "failed to parse connection shutdown grace period", }, + "connect timeout invalid": { + config: &contour_api_v1alpha1.TimeoutParameters{ + ConnectTimeout: pointer.String("infinite"), + }, + errorMsg: "failed to parse connect timeout", + }, } for name, tc := range testCases {