Add ServiceResolver RequestTimeout for route timeouts to make TerminatingGateway upstream timeouts configurable (#16495)

* Leverage ServiceResolver ConnectTimeout for route timeouts to make TerminatingGateway upstream timeouts configurable

* Regenerate golden files

* Add RequestTimeout field

* Add changelog entry
This commit is contained in:
Andrew Stucki 2023-03-03 09:37:12 -05:00 committed by GitHub
parent f1d16adda8
commit 2916821b55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
24 changed files with 1289 additions and 1194 deletions

3
.changelog/16495.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:improvement
mesh: Add ServiceResolver RequestTimeout for route timeouts to make request timeouts configurable
```

View File

@ -978,6 +978,7 @@ RESOLVE_AGAIN:
Default: resolver.IsDefault(),
Target: target.ID,
ConnectTimeout: connectTimeout,
RequestTimeout: resolver.RequestTimeout,
},
LoadBalancer: resolver.LoadBalancer,
}

View File

@ -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,
},
}
@ -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,
},
}
@ -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,
},
}

View File

@ -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",
@ -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,

View File

@ -250,6 +250,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
)
case "external-sni":
@ -263,6 +264,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
)
case "failover":
@ -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",
@ -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"},
@ -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{
@ -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",
},
@ -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"},
@ -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"},
@ -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"},
@ -446,6 +455,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
@ -497,6 +507,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,
@ -528,6 +539,7 @@ func setupTestVariationDiscoveryChain(
Kind: structs.ServiceResolver,
Name: "db",
ConnectTimeout: 33 * time.Second,
RequestTimeout: 33 * time.Second,
},
&structs.ProxyConfigEntry{
Kind: structs.ProxyDefaults,

View File

@ -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"`
@ -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)
}
@ -886,6 +896,7 @@ func (e *ServiceResolverConfigEntry) UnmarshalJSON(data []byte) error {
type Alias ServiceResolverConfigEntry
aux := &struct {
ConnectTimeout string
RequestTimeout string
*Alias
}{
Alias: (*Alias)(e),
@ -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
}
@ -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
}
@ -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

View File

@ -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"`
}

View File

@ -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
@ -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
@ -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 {
@ -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{
@ -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,

View File

@ -16,7 +16,8 @@
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]

View File

@ -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"
}
}
]

View File

@ -16,7 +16,8 @@
"prefix": "/"
},
"route": {
"cluster": "db.default.cluster-01.external.peer1.domain"
"cluster": "db.default.cluster-01.external.peer1.domain",
"timeout": "33s"
}
}
]

View File

@ -16,7 +16,8 @@
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]

View File

@ -16,7 +16,8 @@
"prefix": "/"
},
"route": {
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "33s"
}
}
]

View File

@ -19,7 +19,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -36,7 +37,8 @@
"prefix": "/"
},
"route": {
"cluster": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -60,7 +60,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -77,7 +78,8 @@
"prefix": "/"
},
"route": {
"cluster": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "bar.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -17,7 +17,8 @@
"prefix": "/"
},
"route": {
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -34,7 +35,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -17,7 +17,8 @@
"prefix": "/"
},
"route": {
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -34,7 +35,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -17,7 +17,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -41,7 +42,8 @@
"prefix": "/"
},
"route": {
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -17,7 +17,8 @@
"prefix": "/"
},
"route": {
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "foo.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]
@ -41,7 +42,8 @@
"prefix": "/"
},
"route": {
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul"
"cluster": "web.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"timeout": "22s"
}
}
]

View File

@ -167,6 +167,7 @@ type ServiceResolverConfigEntry struct {
Redirect *ServiceResolverRedirect `json:",omitempty"`
Failover map[string]ServiceResolverFailover `json:",omitempty"`
ConnectTimeout time.Duration `json:",omitempty" alias:"connect_timeout"`
RequestTimeout time.Duration `json:",omitempty" alias:"request_timeout"`
// LoadBalancer determines the load balancing policy and configuration for services
// issuing requests to this upstream service.

View File

@ -1343,6 +1343,7 @@ func ServiceResolverToStructs(s *ServiceResolver, t *structs.ServiceResolverConf
}
}
t.ConnectTimeout = structs.DurationFromProto(s.ConnectTimeout)
t.RequestTimeout = structs.DurationFromProto(s.RequestTimeout)
if s.LoadBalancer != nil {
var x structs.LoadBalancer
LoadBalancerToStructs(s.LoadBalancer, &x)
@ -1385,6 +1386,7 @@ func ServiceResolverFromStructs(t *structs.ServiceResolverConfigEntry, s *Servic
}
}
s.ConnectTimeout = structs.DurationToProto(t.ConnectTimeout)
s.RequestTimeout = structs.DurationToProto(t.RequestTimeout)
if t.LoadBalancer != nil {
var x LoadBalancer
LoadBalancerFromStructs(t.LoadBalancer, &x)

File diff suppressed because it is too large Load Diff

View File

@ -121,6 +121,8 @@ message ServiceResolver {
google.protobuf.Duration ConnectTimeout = 5;
LoadBalancer LoadBalancer = 6;
map<string, string> Meta = 7;
// mog: func-to=structs.DurationFromProto func-from=structs.DurationToProto
google.protobuf.Duration RequestTimeout = 8;
}
// mog annotation:

View File

@ -440,6 +440,12 @@ spec:
description:
'The timeout for establishing new network connections to this service. The default unit of time is `ns`.',
},
{
name: 'RequestTimeout',
type: 'duration: 0s',
description:
'The timeout for receiving an HTTP response from this service before the connection is terminated. If unspecified or 0, the default of 15s is used. The default unit of time is `ns`.',
},
{
name: 'DefaultSubset',
type: 'string: ""',