Added configurable upstream connect timeout (#4326)

* Added configurable upstream connect timeout

Signed-off-by: Tero Saarni <tero.saarni@est.tech>

apis/projectcontour/v1alpha1/contourconfig.go

@@ -412,6 +412,14 @@ type TimeoutParameters struct {
 	// for more information.
 	// +optional
 	ConnectionShutdownGracePeriod *string `json:"connectionShutdownGracePeriod,omitempty"`
+
+	// 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.
+	// +optional
+	ConnectTimeout *string `json:"connectTimeout"`
 }
 
 // ClusterDNSFamilyType is the Ip family to use for resolving DNS

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).

cmd/contour/serve.go

@@ -399,6 +399,7 @@ func (s *Server) doServe() error {
 		headersPolicy:             contourConfiguration.Policy,
 		clientCert:                clientCert,
 		fallbackCert:              fallbackCert,
+		connectTimeout:            timeouts.ConnectTimeout,
 	})
 
 	// Build the core Kubernetes event handler.
@@ -778,6 +779,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 {
@@ -836,12 +838,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,
@@ -851,13 +855,15 @@ func (s *Server) getDAGBuilder(dbc dagBuilderConfig) *dag.Builder {
 			ClientCertificate:         dbc.clientCert,
 			RequestHeadersPolicy:      &requestHeadersPolicy,
 			ResponseHeadersPolicy:     &responseHeadersPolicy,
+			ConnectTimeout:            dbc.connectTimeout,
 		},
 	}
 
 	if dbc.gatewayAPIConfigured {
 		dagProcessors = append(dagProcessors, &dag.GatewayAPIProcessor{
 			EnableExternalNameService: dbc.enableExternalNameService,
 			FieldLogger:               s.log.WithField("context", "GatewayAPIProcessor"),
+			ConnectTimeout:            dbc.connectTimeout,
 		})
 	}
 

cmd/contour/servecontext.go

@@ -339,6 +339,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 {

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,

examples/contour/01-contour-config.yaml

@@ -102,6 +102,7 @@ data:
     #   max-connection-duration: infinity
     #   delayed-close-timeout: 1s
     #   connection-shutdown-grace-period: 5s
+    #   connect-timeout: 2s
     #
     # Envoy cluster settings.
     # cluster:

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: "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:
                         description: "ConnectionIdleTimeout defines how long the proxy
                           should wait while there are no active requests (for HTTP/1.1)
@@ -1331,6 +1338,13 @@ spec:
                         description: Timeouts holds various configurable timeouts
                           that can be set in the config file.
                         properties:
+                          connectTimeout:
+                            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:
                             description: "ConnectionIdleTimeout defines how long the
                               proxy should wait while there are no active requests

examples/render/contour-deployment.yaml

@@ -135,6 +135,7 @@ data:
     #   max-connection-duration: infinity
     #   delayed-close-timeout: 1s
     #   connection-shutdown-grace-period: 5s
+    #   connect-timeout: 2s
     #
     # Envoy cluster settings.
     # cluster:
@@ -590,6 +591,13 @@ spec:
                     description: Timeouts holds various configurable timeouts that
                       can be set in the config file.
                     properties:
+                      connectTimeout:
+                        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:
                         description: "ConnectionIdleTimeout defines how long the proxy
                           should wait while there are no active requests (for HTTP/1.1)
@@ -1519,6 +1527,13 @@ spec:
                         description: Timeouts holds various configurable timeouts
                           that can be set in the config file.
                         properties:
+                          connectTimeout:
+                            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:
                             description: "ConnectionIdleTimeout defines how long the
                               proxy should wait while there are no active requests

examples/render/contour-gateway.yaml

@@ -138,6 +138,7 @@ data:
     #   max-connection-duration: infinity
     #   delayed-close-timeout: 1s
     #   connection-shutdown-grace-period: 5s
+    #   connect-timeout: 2s
     #
     # Envoy cluster settings.
     # cluster:
@@ -593,6 +594,13 @@ spec:
                     description: Timeouts holds various configurable timeouts that
                       can be set in the config file.
                     properties:
+                      connectTimeout:
+                        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:
                         description: "ConnectionIdleTimeout defines how long the proxy
                           should wait while there are no active requests (for HTTP/1.1)
@@ -1522,6 +1530,13 @@ spec:
                         description: Timeouts holds various configurable timeouts
                           that can be set in the config file.
                         properties:
+                          connectTimeout:
+                            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:
                             description: "ConnectionIdleTimeout defines how long the
                               proxy should wait while there are no active requests

examples/render/contour.yaml

@@ -135,6 +135,7 @@ data:
     #   max-connection-duration: infinity
     #   delayed-close-timeout: 1s
     #   connection-shutdown-grace-period: 5s
+    #   connect-timeout: 2s
     #
     # Envoy cluster settings.
     # cluster:
@@ -590,6 +591,13 @@ spec:
                     description: Timeouts holds various configurable timeouts that
                       can be set in the config file.
                     properties:
+                      connectTimeout:
+                        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:
                         description: "ConnectionIdleTimeout defines how long the proxy
                           should wait while there are no active requests (for HTTP/1.1)
@@ -1519,6 +1527,13 @@ spec:
                         description: Timeouts holds various configurable timeouts
                           that can be set in the config file.
                         properties:
+                          connectTimeout:
+                            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:
                             description: "ConnectionIdleTimeout defines how long the
                               proxy should wait while there are no active requests

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
 }

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 {

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 {