Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added configurable upstream connect timeout #4326

Merged
merged 6 commits into from
Feb 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apis/projectcontour/v1alpha1/contourconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions apis/projectcontour/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions changelogs/unreleased/4326-tsaarni-small.md
Original file line number Diff line number Diff line change
@@ -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).
6 changes: 6 additions & 0 deletions cmd/contour/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -854,13 +858,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,
})
}

Expand Down
3 changes: 3 additions & 0 deletions cmd/contour/servecontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
9 changes: 9 additions & 0 deletions cmd/contour/servecontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions examples/contour/01-contour-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
14 changes: 14 additions & 0 deletions examples/contour/01-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -1329,6 +1336,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
Expand Down
15 changes: 15 additions & 0 deletions examples/render/contour-deployment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -594,6 +595,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)
Expand Down Expand Up @@ -1521,6 +1529,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
Expand Down
15 changes: 15 additions & 0 deletions examples/render/contour-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -597,6 +598,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)
Expand Down Expand Up @@ -1524,6 +1532,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
Expand Down
15 changes: 15 additions & 0 deletions examples/render/contour.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -594,6 +595,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)
Expand Down Expand Up @@ -1521,6 +1529,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
Expand Down
8 changes: 8 additions & 0 deletions internal/contourconfig/contourconfiguration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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) {
Expand Down Expand Up @@ -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
}
10 changes: 10 additions & 0 deletions internal/contourconfig/contourconfiguration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func TestParseTimeoutPolicy(t *testing.T) {
MaxConnectionDuration: timeout.DefaultSetting(),
DelayedClose: timeout.DefaultSetting(),
ConnectionShutdownGracePeriod: timeout.DefaultSetting(),
ConnectTimeout: 0,
},
},
"timeouts not set": {
Expand All @@ -50,6 +51,7 @@ func TestParseTimeoutPolicy(t *testing.T) {
MaxConnectionDuration: timeout.DefaultSetting(),
DelayedClose: timeout.DefaultSetting(),
ConnectionShutdownGracePeriod: timeout.DefaultSetting(),
ConnectTimeout: 0,
},
},
"timeouts all set": {
Expand All @@ -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),
Expand All @@ -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": {
Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions internal/dag/dag.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Loading