Add support for a request-timeout annotation

Signed-off-by: Cody Maloney <[email protected]>

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

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.

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 {

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