From 9e166f39d86069cdd1ae8cab2546aa0f461d1401 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Fri, 19 Jan 2018 12:27:35 -0800 Subject: [PATCH] Add support for a request-timeout annotation Signed-off-by: Cody Maloney --- docs/README.md | 1 + docs/annotations.md | 15 ++++++ internal/contour/translator_test.go | 72 +++++++++++++++++++++++++++++ internal/contour/virtualhost.go | 51 ++++++++++++++++++-- 4 files changed, 136 insertions(+), 3 deletions(-) create mode 100644 docs/annotations.md diff --git a/docs/README.md b/docs/README.md index 0de6d9450a4..58d7dafd51b 100644 --- a/docs/README.md +++ b/docs/README.md @@ -8,5 +8,6 @@ The root-level README can get you started. Here you can dig into the details. * [About Contour and Envoy](about.md) * [Image tagging policy](tagging.md) * [Architecture](architecture.md) +* [Supported Annotations](annotations.md) For more about how we're thinking of Contour's future, check out [the design docs](../design/). \ No newline at end of file diff --git a/docs/annotations.md b/docs/annotations.md new file mode 100644 index 00000000000..6c816d59e10 --- /dev/null +++ b/docs/annotations.md @@ -0,0 +1,15 @@ +# Annotations + +Contour supports a couple of standard kubernetes ingress annotations, as well as some contour-specific ones. Below is a listing of each supported annotation and a brief description + + +## Standard Ingress Annotations + + - `kubernetes.io/ingress.class`: The ingress class which should interpret and serve the ingress. If this isn't set, then all ingress controllers will serve the ingress. If specified as `kubernetes.io/ingress.class: contour` then contour will serve the ingress. If it has any other value, contour will ignore the ingress definition. + - `ingress.kubernetes.io/force-ssl-redirect`: Marks the ingress to envoy as requiring TLS/SSL by setting the [envoy virtual host option require_tls](https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/route/route.proto.html#envoy-api-field-route-virtualhost-require-tls) + - `kubernetes.io/allow-http`: Instructs contour to not create an envoy http route for the virtual host at all. The ingress will only exist for HTTPS requests. This should be given the value `"true"` to cause envoy to mark the endpoint as http only. All other values are ignored. + + +## Contour Specific Ingress Annotations + + - `contour.heptio.com/request-timeout`: Set the [envoy HTTP route timeout](https://www.envoyproxy.io/docs/envoy/latest/api-v2/api/v2/route/route.proto.html#envoy-api-field-route-routeaction-timeout) to the given value, specified as a [golang duration](https://golang.org/pkg/time/#ParseDuration). By default envoy has a 15 second timeout for a backend service to respond. Set this to `infinity` to specify envoy should never timeout the connection to the backend. Note the value `0s` / zero has special semantics to envoy. diff --git a/internal/contour/translator_test.go b/internal/contour/translator_test.go index d8fac189cf5..5e84e111ece 100644 --- a/internal/contour/translator_test.go +++ b/internal/contour/translator_test.go @@ -455,6 +455,9 @@ func TestTranslatorRemoveEndpoints(t *testing.T) { } func TestTranslatorAddIngress(t *testing.T) { + duration20seconds := time.Duration(20 * time.Second) + duration0seconds := time.Duration(0) + tests := []struct { name string setup func(*Translator) @@ -943,6 +946,75 @@ func TestTranslatorAddIngress(t *testing.T) { }}, }}, ingress_https: []*v2.VirtualHost{}, + }, { + name: "explicitly set upstream timeout to seconds", + ing: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "correct", + Namespace: "default", + Annotations: map[string]string{ + "contour.heptio.com/request-timeout": "20s", + }, + }, + Spec: v1beta1.IngressSpec{ + Backend: backend("backend", intstr.FromInt(80)), + }, + }, + ingress_http: []*v2.VirtualHost{{ + Name: "*", + Domains: []string{"*"}, + Routes: []*v2.Route{{ + Match: prefixmatch("/"), // match all + Action: clusteractiontimeout("default/backend/80", &duration20seconds), + }}, + }}, + ingress_https: []*v2.VirtualHost{}, + }, { + name: "explicitly set upstream timeout to infinite", + ing: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "correct", + Namespace: "default", + Annotations: map[string]string{ + "contour.heptio.com/request-timeout": "infinity", + }, + }, + Spec: v1beta1.IngressSpec{ + Backend: backend("backend", intstr.FromInt(80)), + }, + }, + ingress_http: []*v2.VirtualHost{{ + Name: "*", + Domains: []string{"*"}, + Routes: []*v2.Route{{ + Match: prefixmatch("/"), // match all + Action: clusteractiontimeout("default/backend/80", &duration0seconds), + }}, + }}, + ingress_https: []*v2.VirtualHost{}, + }, { + name: "explicitly set upstream timeout to an invalid duration", + ing: &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "correct", + Namespace: "default", + Annotations: map[string]string{ + "contour.heptio.com/request-timeout": "300jiosadf", + }, + }, + Spec: v1beta1.IngressSpec{ + Backend: backend("backend", intstr.FromInt(80)), + }, + }, + ingress_http: []*v2.VirtualHost{{ + Name: "*", + Domains: []string{"*"}, + Routes: []*v2.Route{{ + Match: prefixmatch("/"), // match all + Action: clusteractiontimeout("default/backend/80", &duration0seconds), + }}, + }}, + ingress_https: []*v2.VirtualHost{}, }} for _, tc := range tests { diff --git a/internal/contour/virtualhost.go b/internal/contour/virtualhost.go index a831224a8b6..d7a12125b77 100644 --- a/internal/contour/virtualhost.go +++ b/internal/contour/virtualhost.go @@ -16,6 +16,7 @@ package contour import ( "sort" "strings" + "time" v2 "github.com/envoyproxy/go-control-plane/api" "k8s.io/api/extensions/v1beta1" @@ -28,6 +29,35 @@ type VirtualHostCache struct { Cond } +func getRequestTimeout(annotations map[string]string) *time.Duration { + timeoutStr, ok := annotations["contour.heptio.com/request-timeout"] + // Error or unspecified is interpreted as no timeout specified, use envoy defaults + if !ok { + return nil + } + + if timeoutStr == "" { + return nil + } + + // Interpret "infinity" explicitly as an infinite timeout, which envoy config + // expects as a timeout of 0. This could be specified with the duration string + // "0s" but want to give an explicit out for operators. + infiniteTimeout := time.Duration(0) + if timeoutStr == "infinity" { + return &infiniteTimeout + } + + timeoutParsed, err := time.ParseDuration(timeoutStr) + if err != nil { + // TODO(cmalonty) plumb a logger in here so we can log this error. + // Assuming infinite duration is going to surprise people less for + // a not-parseable duration than a implicit 15 second one. + return &infiniteTimeout + } + return &timeoutParsed +} + // recomputevhost recomputes the ingress_http (HTTP) and ingress_https (HTTPS) record // from the vhost from list of ingresses supplied. func (v *VirtualHostCache) recomputevhost(vhost string, ingresses map[metadata]*v1beta1.Ingress) { @@ -46,9 +76,12 @@ func (v *VirtualHostCache) recomputevhost(vhost string, ingresses map[metadata]* // TODO(dfc) plumb a logger in here so we can log this error. continue } + + timeout := getRequestTimeout(ing.Annotations) + for _, p := range rule.IngressRuleValue.HTTP.Paths { m := pathToRouteMatch(p) - a := clusteraction(ingressBackendToClusterName(ing, &p.Backend)) + a := clusteractiontimeout(ingressBackendToClusterName(ing, &p.Backend), timeout) vv.Routes = append(vv.Routes, &v2.Route{Match: m, Action: a}) } } @@ -71,10 +104,12 @@ func (v *VirtualHostCache) recomputevhost(vhost string, ingresses map[metadata]* // set blanket 301 redirect vv.RequireTls = v2.VirtualHost_ALL } + timeout := getRequestTimeout(i.Annotations) + if i.Spec.Backend != nil && len(ingresses) == 1 { vv.Routes = []*v2.Route{{ Match: prefixmatch("/"), // match all - Action: clusteraction(ingressBackendToClusterName(i, i.Spec.Backend)), + Action: clusteractiontimeout(ingressBackendToClusterName(i, i.Spec.Backend), timeout), }} continue } @@ -88,7 +123,7 @@ func (v *VirtualHostCache) recomputevhost(vhost string, ingresses map[metadata]* } for _, p := range rule.IngressRuleValue.HTTP.Paths { m := pathToRouteMatch(p) - a := clusteraction(ingressBackendToClusterName(i, &p.Backend)) + a := clusteractiontimeout(ingressBackendToClusterName(i, &p.Backend), timeout) vv.Routes = append(vv.Routes, &v2.Route{Match: m, Action: a}) } } @@ -197,6 +232,16 @@ func clusteraction(cluster string) *v2.Route_Route { } } +// clusteractiontimeout returns a Route_Route action for the supplied cluster. +func clusteractiontimeout(cluster string, timeout *time.Duration) *v2.Route_Route { + // TODO(cmaloney): Pull timeout off of the backend cluster annotation + // and use it over the value retrieved from the ingress annotation if + // specified. + c := clusteraction(cluster) + c.Route.Timeout = timeout + return c +} + func virtualhost(hostname string) *v2.VirtualHost { return &v2.VirtualHost{ Name: hashname(60, hostname),