Skip to content

Backport of Add ServiceResolver RequestTimeout for route timeouts to make TerminatingGateway upstream timeouts configurable into release/1.15.x #16520

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.

Sign up for GitHub

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

Merged
andrewstucki merged 4 commits into from
Mar 3, 2023
Merged

Backport of Add ServiceResolver RequestTimeout for route timeouts to make TerminatingGateway upstream timeouts configurable into release/1.15.x #16520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0ef9c2
backport of commit 29768f27aad29fcd8120daf080b06b33c9f22373
Mar 1, 2023
57b1145
backport of commit 1f26c2a6b4a8f9879cbf4dd6056a94625b753728
Mar 1, 2023
1e5c6e5
Add RequestTimeout field
Mar 2, 2023
189408c
Add changelog entry
Mar 2, 2023
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
3 changes: 3 additions & 0 deletions .changelog/16495.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
mesh: Add ServiceResolver RequestTimeout for route timeouts to make request timeouts configurable
```
1 change: 1 addition & 0 deletions agent/consul/discoverychain/compile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,7 @@ RESOLVE_AGAIN:
Default: resolver.IsDefault(),
Target: target.ID,
ConnectTimeout: connectTimeout,
RequestTimeout: resolver.RequestTimeout,
},
LoadBalancer: resolver.LoadBalancer,
}
Expand Down
6 changes: 6 additions & 0 deletions agent/proxycfg/testing_ingress_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,11 +700,13 @@ func TestConfigSnapshotIngress_HTTPMultipleServices(t testing.T) *ConfigSnapshot
Kind: structs.ServiceResolver,
Name: "foo",
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "bar",
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
}

Expand Down Expand Up @@ -855,11 +857,13 @@ func TestConfigSnapshotIngress_GRPCMultipleServices(t testing.T) *ConfigSnapshot
Kind: structs.ServiceResolver,
Name: "foo",
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "bar",
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
}

Expand Down Expand Up @@ -1213,12 +1217,14 @@ func TestConfigSnapshotIngressGatewayWithChain(
Name: "web",
EnterpriseMeta: *webEntMeta,
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Name: "foo",
EnterpriseMeta: *fooEntMeta,
ConnectTimeout: 22 * time.Second,
RequestTimeout: 22 * time.Second,
},
}

Expand Down
2 changes: 2 additions & 0 deletions agent/proxycfg/testing_mesh_gateway.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func TestConfigSnapshotMeshGateway(t testing.T, variant string, nsFn func(ns *st
Kind: structs.ServiceResolver,
Name: "bar",
ConnectTimeout: 10 * time.Second,
RequestTimeout: 10 * time.Second,
Subsets: map[string]structs.ServiceResolverSubset{
"v1": {
Filter: "Service.Meta.Version == 1",
Expand Down Expand Up @@ -687,6 +688,7 @@ func TestConfigSnapshotPeeredMeshGateway(t testing.T, variant string, nsFn func(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ServiceResolverConfigEntry{
Kind: structs.ServiceResolver,
Expand Down
12 changes: 12 additions & 0 deletions agent/proxycfg/testing_upstreams.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
)
case "external-sni":
Expand All @@ -263,6 +264,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
)
case "failover":
Expand All @@ -271,6 +273,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Service: "fail",
Expand All @@ -293,6 +296,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Datacenters: []string{"dc2"},
Expand All @@ -306,6 +310,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Targets: []structs.ServiceResolverFailoverTarget{
Expand All @@ -321,6 +326,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Redirect: &structs.ServiceResolverRedirect{
Peer: "cluster-01",
},
Expand All @@ -341,6 +347,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Datacenters: []string{"dc2", "dc3"},
Expand All @@ -363,6 +370,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Datacenters: []string{"dc2"},
Expand All @@ -385,6 +393,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
Failover: map[string]structs.ServiceResolverFailover{
"*": {
Datacenters: []string{"dc2", "dc3"},
Expand Down Expand Up @@ -446,6 +455,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
Expand Down Expand Up @@ -497,6 +507,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
Expand Down Expand Up @@ -528,6 +539,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
Expand Down
21 changes: 21 additions & 0 deletions agent/structs/config_entry_discoverychain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -857,6 +857,11 @@ type ServiceResolverConfigEntry struct {
// to this service.
ConnectTimeout time.Duration `json:",omitempty" alias:"connect_timeout"`

// RequestTimeout is the timeout for an HTTP request to complete before
// the connection is automatically terminated. If unspecified, defaults
// to 15 seconds.
RequestTimeout time.Duration `json:",omitempty" alias:"request_timeout"`

// LoadBalancer determines the load balancing policy and configuration for services
// issuing requests to this upstream service.
LoadBalancer *LoadBalancer `json:",omitempty" alias:"load_balancer"`
Expand All @@ -870,14 +875,19 @@ func (e *ServiceResolverConfigEntry) MarshalJSON() ([]byte, error) {
type Alias ServiceResolverConfigEntry
exported := &struct {
ConnectTimeout string `json:",omitempty"`
RequestTimeout string `json:",omitempty"`
*Alias
}{
ConnectTimeout: e.ConnectTimeout.String(),
RequestTimeout: e.RequestTimeout.String(),
Alias: (*Alias)(e),
}
if e.ConnectTimeout == 0 {
exported.ConnectTimeout = ""
}
if e.RequestTimeout == 0 {
exported.RequestTimeout = ""
}

return json.Marshal(exported)
}
Expand All @@ -886,6 +896,7 @@ func (e *ServiceResolverConfigEntry) UnmarshalJSON(data []byte) error {
type Alias ServiceResolverConfigEntry
aux := &struct {
ConnectTimeout string
RequestTimeout string
*Alias
}{
Alias: (*Alias)(e),
Expand All @@ -899,6 +910,11 @@ func (e *ServiceResolverConfigEntry) UnmarshalJSON(data []byte) error {
return err
}
}
if aux.RequestTimeout != "" {
if e.RequestTimeout, err = time.ParseDuration(aux.RequestTimeout); err != nil {
return err
}
}
return nil
}

Expand All @@ -919,6 +935,7 @@ func (e *ServiceResolverConfigEntry) IsDefault() bool {
e.Redirect == nil &&
len(e.Failover) == 0 &&
e.ConnectTimeout == 0 &&
e.RequestTimeout == 0 &&
e.LoadBalancer == nil
}

Expand Down Expand Up @@ -1117,6 +1134,10 @@ func (e *ServiceResolverConfigEntry) Validate() error {
return fmt.Errorf("Bad ConnectTimeout '%s', must be >= 0", e.ConnectTimeout)
}

if e.RequestTimeout < 0 {
return fmt.Errorf("Bad RequestTimeout '%s', must be >= 0", e.RequestTimeout)
}

if e.LoadBalancer != nil {
lb := e.LoadBalancer

Expand Down
1 change: 1 addition & 0 deletions agent/structs/discovery_chain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ func (s *DiscoveryGraphNode) MapKey() string {
type DiscoveryResolver struct {
Default bool `json:",omitempty"`
ConnectTimeout time.Duration `json:",omitempty"`
RequestTimeout time.Duration `json:",omitempty"`
Target string `json:",omitempty"`
Failover *DiscoveryFailover `json:",omitempty"`
}
Expand Down
13 changes: 10 additions & 3 deletions agent/xds/routes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func (s *ResourceGenerator) makeRoutes(
if resolver.LoadBalancer != nil {
lb = resolver.LoadBalancer
}
route, err := makeNamedDefaultRouteWithLB(clusterName, lb, autoHostRewrite)
route, err := makeNamedDefaultRouteWithLB(clusterName, lb, resolver.RequestTimeout, autoHostRewrite)
if err != nil {
s.Logger.Error("failed to make route", "cluster", clusterName, "error", err)
return nil, err
Expand All @@ -228,7 +228,7 @@ func (s *ResourceGenerator) makeRoutes(
// If there is a service-resolver for this service then also setup routes for each subset
for name := range resolver.Subsets {
clusterName = connect.ServiceSNI(svc.Name, name, svc.NamespaceOrDefault(), svc.PartitionOrDefault(), cfgSnap.Datacenter, cfgSnap.Roots.TrustDomain)
route, err := makeNamedDefaultRouteWithLB(clusterName, lb, true)
route, err := makeNamedDefaultRouteWithLB(clusterName, lb, resolver.RequestTimeout, true)
if err != nil {
s.Logger.Error("failed to make route", "cluster", clusterName, "error", err)
return nil, err
Expand Down Expand Up @@ -282,7 +282,7 @@ func (s *ResourceGenerator) routesForMeshGateway(cfgSnap *proxycfg.ConfigSnapsho
return resources, nil
}

func makeNamedDefaultRouteWithLB(clusterName string, lb *structs.LoadBalancer, autoHostRewrite bool) (*envoy_route_v3.RouteConfiguration, error) {
func makeNamedDefaultRouteWithLB(clusterName string, lb *structs.LoadBalancer, timeout time.Duration, autoHostRewrite bool) (*envoy_route_v3.RouteConfiguration, error) {
action := makeRouteActionFromName(clusterName)

if err := injectLBToRouteAction(lb, action.Route); err != nil {
Expand All @@ -296,6 +296,10 @@ func makeNamedDefaultRouteWithLB(clusterName string, lb *structs.LoadBalancer, a
}
}

if timeout != 0 {
action.Route.Timeout = durationpb.New(timeout)
}

return &envoy_route_v3.RouteConfiguration{
Name: clusterName,
VirtualHosts: []*envoy_route_v3.VirtualHost{
Expand Down Expand Up @@ -637,6 +641,9 @@ func (s *ResourceGenerator) makeUpstreamRouteForDiscoveryChain(
return nil, fmt.Errorf("failed to apply load balancer configuration to route action: %v", err)
}

if startNode.Resolver.RequestTimeout > 0 {
routeAction.Route.Timeout = durationpb.New(startNode.Resolver.RequestTimeout)
}
defaultRoute := &envoy_route_v3.Route{
Match: makeDefaultRouteMatch(),
Action: routeAction,
Expand Down
31 changes: 16 additions & 15 deletions ...t/xds/testdata/routes/connect-proxy-with-chain-and-failover-to-cluster-peer.latest.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
{
"versionInfo": "00000001",
"resources": [
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"virtualHosts": [
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"virtualHosts": [
{
"name": "db",
"domains": [
"name": "db",
"domains": [
"*"
],
"routes": [
"routes": [
{
"match": {
"prefix": "/"
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
}
],
"validateClusters": true
"validateClusters": true
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"nonce": "00000001"
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"nonce": "00000001"
}
3 changes: 2 additions & 1 deletion agent/xds/testdata/routes/connect-proxy-with-chain-and-overrides.latest.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
"prefix": "/"
},
"route": {
"cluster": "78ebd528~db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "78ebd528~db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]
Expand Down
31 changes: 16 additions & 15 deletions ...t/xds/testdata/routes/connect-proxy-with-chain-and-redirect-to-cluster-peer.latest.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,31 @@
{
"versionInfo": "00000001",
"resources": [
"versionInfo": "00000001",
"resources": [
{
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"virtualHosts": [
"@type": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"name": "db",
"virtualHosts": [
{
"name": "db",
"domains": [
"name": "db",
"domains": [
"*"
],
"routes": [
"routes": [
{
"match": {
"prefix": "/"
"match": {
"prefix": "/"
},
"route": {
"cluster": "db.default.cluster-01.external.peer1.domain"
"route": {
"cluster": "db.default.cluster-01.external.peer1.domain",
"timeout": "33s"
}
}
]
}
],
"validateClusters": true
"validateClusters": true
}
],
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"nonce": "00000001"
"typeUrl": "type.googleapis.com/envoy.config.route.v3.RouteConfiguration",
"nonce": "00000001"
}
Loading