api: add Route.TimeoutPolicy.Idle

Fixes #944

Add the ability to specify a per route idle timeout policy. Envoy's
definition of idle, and exactly between who the timeout applies is less
clear than it should be, but at least end users can control this
timeout, whatever it means.

Also, move response timeout tests from e2e to featuretests, add idle timeout tests for HTTPProxy as those are the only types that support them.

Signed-off-by: Dave Cheney <dave@cheney.net>

apis/projectcontour/v1alpha1/httpproxy.go

@@ -159,7 +159,11 @@ type HealthCheck struct {
 type TimeoutPolicy struct {
 	// Timeout for receiving a response from the server after processing a request from client.
 	// If not supplied the timeout duration is undefined.
-	Response string `json:"Response"`
+	Response string `json:"response"`
+
+	// Timeout after which if there are no active requests, the connection between Envoy and the
+	// backend will be closed.
+	Idle string `json:"idle"`
 }
 
 // RetryPolicy define the attributes associated with retrying policy

internal/dag/dag.go

@@ -115,6 +115,9 @@ type TimeoutPolicy struct {
 	// A timeout of zero implies "use envoy's default"
 	// A timeout of -1 represents "infinity"
 	ResponseTimeout time.Duration
+
+	// IdleTimeout is the timeout applied to idle connections.
+	IdleTimeout time.Duration
 }
 
 // RetryPolicy defines the retry / number / timeout options

internal/dag/policy.go

@@ -50,6 +50,7 @@ func timeoutPolicy(tp *projcontour.TimeoutPolicy) *TimeoutPolicy {
 	}
 	return &TimeoutPolicy{
 		ResponseTimeout: parseTimeout(tp.Response),
+		IdleTimeout:     parseTimeout(tp.Idle),
 	}
 }
 

internal/dag/policy_test.go

@@ -177,6 +177,14 @@ func TestTimeoutPolicy(t *testing.T) {
 				ResponseTimeout: -1,
 			},
 		},
+		"idle timeout": {
+			tp: &projcontour.TimeoutPolicy{
+				Idle: "900s",
+			},
+			want: &TimeoutPolicy{
+				IdleTimeout: 900 * time.Second,
+			},
+		},
 	}
 
 	for name, tc := range tests {

internal/e2e/rds_test.go

@@ -2158,135 +2158,6 @@ func TestRouteRetryIngressRoute(t *testing.T) {
 	), nil)
 }
 
-// issue 815, support for timeoutpolicy in IngressRoute
-func TestRouteTimeoutPolicyIngressRoute(t *testing.T) {
-	const (
-		durationInfinite  = time.Duration(0)
-		duration10Minutes = 10 * time.Minute
-	)
-
-	rh, cc, done := setup(t)
-	defer done()
-
-	s1 := &v1.Service{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "backend",
-			Namespace: "default",
-		},
-		Spec: v1.ServiceSpec{
-			Ports: []v1.ServicePort{{
-				Protocol:   "TCP",
-				Port:       80,
-				TargetPort: intstr.FromInt(8080),
-			}},
-		},
-	}
-	rh.OnAdd(s1)
-
-	// i1 is an _invalid_ timeout, which we interpret as _infinite_.
-	i1 := &ingressroutev1.IngressRoute{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "simple",
-			Namespace: "default",
-		},
-		Spec: ingressroutev1.IngressRouteSpec{
-			VirtualHost: &projcontour.VirtualHost{Fqdn: "test2.test.com"},
-			Routes: []ingressroutev1.Route{{
-				Match: "/",
-				Services: []ingressroutev1.Service{{
-					Name: "backend",
-					Port: 80,
-				}},
-			}},
-		},
-	}
-	rh.OnAdd(i1)
-	assertRDS(t, cc, "1", virtualhosts(
-		envoy.VirtualHost("test2.test.com",
-			envoy.Route(envoy.RoutePrefix("/"), routecluster("default/backend/80/da39a3ee5e")),
-		),
-	), nil)
-
-	// i2 adds an _invalid_ timeout, which we interpret as _infinite_.
-	i2 := &ingressroutev1.IngressRoute{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "simple",
-			Namespace: "default",
-		},
-		Spec: ingressroutev1.IngressRouteSpec{
-			VirtualHost: &projcontour.VirtualHost{Fqdn: "test2.test.com"},
-			Routes: []ingressroutev1.Route{{
-				Match: "/",
-				TimeoutPolicy: &ingressroutev1.TimeoutPolicy{
-					Request: "600",
-				},
-				Services: []ingressroutev1.Service{{
-					Name: "backend",
-					Port: 80,
-				}},
-			}},
-		},
-	}
-	rh.OnUpdate(i1, i2)
-	assertRDS(t, cc, "2", virtualhosts(
-		envoy.VirtualHost("test2.test.com",
-			envoy.Route(envoy.RoutePrefix("/"), clustertimeout("default/backend/80/da39a3ee5e", durationInfinite)),
-		),
-	), nil)
-	// i3 corrects i2 to use a proper duration
-	i3 := &ingressroutev1.IngressRoute{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "simple",
-			Namespace: "default",
-		},
-		Spec: ingressroutev1.IngressRouteSpec{
-			VirtualHost: &projcontour.VirtualHost{Fqdn: "test2.test.com"},
-			Routes: []ingressroutev1.Route{{
-				Match: "/",
-				TimeoutPolicy: &ingressroutev1.TimeoutPolicy{
-					Request: "600s", // 10 * time.Minute
-				},
-				Services: []ingressroutev1.Service{{
-					Name: "backend",
-					Port: 80,
-				}},
-			}},
-		},
-	}
-	rh.OnUpdate(i2, i3)
-	assertRDS(t, cc, "3", virtualhosts(
-		envoy.VirtualHost("test2.test.com",
-			envoy.Route(envoy.RoutePrefix("/"), clustertimeout("default/backend/80/da39a3ee5e", duration10Minutes)),
-		),
-	), nil)
-	// i4 updates i3 to explicitly request infinite timeout
-	i4 := &ingressroutev1.IngressRoute{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "simple",
-			Namespace: "default",
-		},
-		Spec: ingressroutev1.IngressRouteSpec{
-			VirtualHost: &projcontour.VirtualHost{Fqdn: "test2.test.com"},
-			Routes: []ingressroutev1.Route{{
-				Match: "/",
-				TimeoutPolicy: &ingressroutev1.TimeoutPolicy{
-					Request: "infinity",
-				},
-				Services: []ingressroutev1.Service{{
-					Name: "backend",
-					Port: 80,
-				}},
-			}},
-		},
-	}
-	rh.OnUpdate(i3, i4)
-	assertRDS(t, cc, "4", virtualhosts(
-		envoy.VirtualHost("test2.test.com",
-			envoy.Route(envoy.RoutePrefix("/"), clustertimeout("default/backend/80/da39a3ee5e", durationInfinite)),
-		),
-	), nil)
-}
-
 func TestRouteWithSessionAffinity(t *testing.T) {
 	rh, cc, done := setup(t)
 	defer done()

internal/envoy/route.go

@@ -15,6 +15,7 @@ package envoy
 import (
 	"fmt"
 	"sort"
+	"time"
 
 	v2 "github.com/envoyproxy/go-control-plane/envoy/api/v2"
 	envoy_api_v2_core "github.com/envoyproxy/go-control-plane/envoy/api/v2/core"
@@ -44,6 +45,7 @@ func RouteRoute(r *dag.Route) *envoy_api_v2_route.Route_Route {
 	ra := envoy_api_v2_route.RouteAction{
 		RetryPolicy:   retryPolicy(r),
 		Timeout:       responseTimeout(r),
+		IdleTimeout:   idleTimeout(r),
 		PrefixRewrite: r.PrefixRewrite,
 		HashPolicy:    hashPolicy(r),
 	}
@@ -94,17 +96,33 @@ func responseTimeout(r *dag.Route) *duration.Duration {
 	if r.TimeoutPolicy == nil {
 		return nil
 	}
+	return timeout(r.TimeoutPolicy.ResponseTimeout)
+}
+
+func idleTimeout(r *dag.Route) *duration.Duration {
+	if r.TimeoutPolicy == nil {
+		return nil
+	}
+	return timeout(r.TimeoutPolicy.IdleTimeout)
+}
 
-	switch r.TimeoutPolicy.ResponseTimeout {
-	case 0:
+// timeout interprets a time.Duration with respect to
+// Envoy's timeout logic. Zero durations are interpreted
+// as nil, therefore remaining unset. Negative durations
+// are interpreted as infinity, which is represented as
+// an explicit value of 0. Positive durations behave as
+// expected.
+func timeout(d time.Duration) *duration.Duration {
+	switch {
+	case d == 0:
 		// no timeout specified
 		return nil
-	case -1:
+	case d < 0:
 		// infinite timeout, set timeout value to a pointer to zero which tells
 		// envoy "infinite timeout"
 		return protobuf.Duration(0)
 	default:
-		return protobuf.Duration(r.TimeoutPolicy.ResponseTimeout)
+		return protobuf.Duration(d)
 	}
 }
 

internal/envoy/route_test.go

@@ -269,6 +269,38 @@ func TestRouteRoute(t *testing.T) {
 				},
 			},
 		},
+		"idle timeout 10m": {
+			route: &dag.Route{
+				TimeoutPolicy: &dag.TimeoutPolicy{
+					IdleTimeout: 10 * time.Minute,
+				},
+				Clusters: []*dag.Cluster{c1},
+			},
+			want: &envoy_api_v2_route.Route_Route{
+				Route: &envoy_api_v2_route.RouteAction{
+					ClusterSpecifier: &envoy_api_v2_route.RouteAction_Cluster{
+						Cluster: "default/kuard/8080/da39a3ee5e",
+					},
+					IdleTimeout: protobuf.Duration(600 * time.Second),
+				},
+			},
+		},
+		"idle timeout infinity": {
+			route: &dag.Route{
+				TimeoutPolicy: &dag.TimeoutPolicy{
+					IdleTimeout: -1,
+				},
+				Clusters: []*dag.Cluster{c1},
+			},
+			want: &envoy_api_v2_route.Route_Route{
+				Route: &envoy_api_v2_route.RouteAction{
+					ClusterSpecifier: &envoy_api_v2_route.RouteAction_Cluster{
+						Cluster: "default/kuard/8080/da39a3ee5e",
+					},
+					IdleTimeout: protobuf.Duration(0),
+				},
+			},
+		},
 		"single service w/ session affinity": {
 			route: &dag.Route{
 				Clusters: []*dag.Cluster{c2},

internal/featuretests/envoy.go

@@ -16,10 +16,13 @@ package featuretests
 // envoy helpers
 
 import (
+	"time"
+
 	envoy_api_v2_route "github.com/envoyproxy/go-control-plane/envoy/api/v2/route"
+	"github.com/projectcontour/contour/internal/protobuf"
 )
 
-func routecluster(cluster string) *envoy_api_v2_route.Route_Route {
+func routeCluster(cluster string) *envoy_api_v2_route.Route_Route {
 	return &envoy_api_v2_route.Route_Route{
 		Route: &envoy_api_v2_route.RouteAction{
 			ClusterSpecifier: &envoy_api_v2_route.RouteAction_Cluster{
@@ -28,3 +31,13 @@ func routecluster(cluster string) *envoy_api_v2_route.Route_Route {
 		},
 	}
 }
+
+func withResponseTimeout(route *envoy_api_v2_route.Route_Route, timeout time.Duration) *envoy_api_v2_route.Route_Route {
+	route.Route.Timeout = protobuf.Duration(timeout)
+	return route
+}
+
+func withIdleTimeout(route *envoy_api_v2_route.Route_Route, timeout time.Duration) *envoy_api_v2_route.Route_Route {
+	route.Route.IdleTimeout = protobuf.Duration(timeout)
+	return route
+}

internal/featuretests/httpproxy.go

@@ -0,0 +1,26 @@
+// Copyright © 2019 VMware
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package featuretests
+
+// HTTPProxy helpers
+
+import (
+	projcontour "github.com/projectcontour/contour/apis/projectcontour/v1alpha1"
+)
+
+func prefixCondition(prefix string) []projcontour.Condition {
+	return []projcontour.Condition{{
+		Prefix: prefix,
+	}}
+}