Skip to content

Commit

Permalink
only support request timeout for now
Browse files Browse the repository at this point in the history 
Signed-off-by: Steve Kriss <stephen.kriss@gmail.com>
  • Loading branch information
@skriss
skriss committed Dec 5, 2023
1 parent 027e489 commit 805debf
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 108 deletions.
93 changes: 28 additions & 65 deletions internal/dag/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4159,55 +4159,50 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
),
},

"Route rule with timeouts": {
"HTTPRoute rule with request timeout": {
gatewayclass: validClass,
gateway: gatewayHTTPAllNamespaces,
objs: []any{
kuardService,
makeHTTPRoute("5s", "5s"),
makeHTTPRoute("5s", ""),
},
want: listeners(
&Listener{
Name: "http-80",
VirtualHosts: virtualhosts(
virtualhost("test.projectcontour.io", timeoutsHTTPProute("/", makeHTTPRouteTimeouts("5s", "5s"), service(kuardService))),
virtualhost("test.projectcontour.io",
&Route{
PathMatchCondition: prefixString("/"),
Clusters: clustersWeight(service(kuardService)),
TimeoutPolicy: RouteTimeoutPolicy{
ResponseTimeout: timeout.DurationSetting(5 * time.Second),
},
},
),
),
},
),
},
"Route rule with timeouts request only": {
"HTTPRoute rule with request and backendRequest timeout": {
gatewayclass: validClass,
gateway: gatewayHTTPAllNamespaces,
objs: []any{
kuardService,
makeHTTPRoute("5s", ""),
makeHTTPRoute("5s", "5s"),
},
want: listeners(
&Listener{
Name: "http-80",
VirtualHosts: virtualhosts(
virtualhost("test.projectcontour.io", timeoutsHTTPProute("/", makeHTTPRouteTimeouts("5s", ""), service(kuardService))),
),
},
),
want: listeners(),
},
"Route rule with timeouts backendRequest only": {

"HTTPRoute rule with backendRequest timeout only": {
gatewayclass: validClass,
gateway: gatewayHTTPAllNamespaces,
objs: []any{
kuardService,
makeHTTPRoute("", "5s"),
},
want: listeners(
&Listener{
Name: "http-80",
VirtualHosts: virtualhosts(
virtualhost("test.projectcontour.io", timeoutsHTTPProute("/", makeHTTPRouteTimeouts("", ""), service(kuardService))),
),
},
),
want: listeners(),
},
"Route rule with timeouts request invalid": {
"HTTPRoute rule with invalid request timeout": {
gatewayclass: validClass,
gateway: gatewayHTTPAllNamespaces,
objs: []any{
Expand All @@ -4216,7 +4211,6 @@ func TestDAGInsertGatewayAPI(t *testing.T) {
},
want: listeners(),
},
// END

"different weights for multiple forwardTos": {
gatewayclass: validClass,
Expand Down Expand Up @@ -15994,44 +15988,6 @@ func prefixroute(prefix string, first *Service, rest ...*Service) *Route {
}
}

func httpRouteTimeoutToPolicy(routeTimeout *gatewayapi_v1.HTTPRouteTimeouts) (rtp *RouteTimeoutPolicy, rp *RetryPolicy, err error) {
rtp = &RouteTimeoutPolicy{}
if routeTimeout.Request != nil {
reqTimeout, err2 := timeout.Parse(string(*routeTimeout.Request))
if err2 != nil {
return nil, nil, fmt.Errorf("HTTPRoute.Spec.Rules.Timeouts: Invalid value for Request is specified")
}
rtp.ResponseTimeout = reqTimeout
}

br := &contour_api_v1.RetryPolicy{}
if routeTimeout.BackendRequest != nil {
br.PerTryTimeout = string(*routeTimeout.BackendRequest)
}

rp = retryPolicy(br)
if rp.PerTryTimeout.Duration() > rtp.ResponseTimeout.Duration() {
rp.PerTryTimeout = rtp.ResponseTimeout
}

rp.RetryOn = ""
return

}

func timeoutsHTTPProute(prefix string, timeouts *gatewayapi_v1.HTTPRouteTimeouts, first *Service, rest ...*Service) *Route {
services := append([]*Service{first}, rest...)

rtp, rp, _ := httpRouteTimeoutToPolicy(timeouts)

return &Route{
PathMatchCondition: prefixString(prefix),
Clusters: clustersWeight(services...),
TimeoutPolicy: *rtp,
RetryPolicy: rp,
}
}

func prefixrouteHTTPRoute(prefix string, first *Service, rest ...*Service) *Route {
services := append([]*Service{first}, rest...)
return &Route{
Expand Down Expand Up @@ -16371,11 +16327,18 @@ func withMirror(r *Route, mirrors []*Service, weight int64) *Route {
}

func makeHTTPRouteTimeouts(request, backendRequest string) *gatewayapi_v1.HTTPRouteTimeouts {
return &gatewayapi_v1.HTTPRouteTimeouts{
Request: ref.To(gatewayapi_v1.Duration(request)),
BackendRequest: ref.To(gatewayapi_v1.Duration(backendRequest)),
httpRouteTimeouts := &gatewayapi_v1.HTTPRouteTimeouts{}

if request != "" {
httpRouteTimeouts.Request = ref.To(gatewayapi_v1.Duration(request))
}
if backendRequest != "" {
httpRouteTimeouts.BackendRequest = ref.To(gatewayapi_v1.Duration(backendRequest))
}

return httpRouteTimeouts
}

func makeHTTPRoute(request, backendRequest string) *gatewayapi_v1beta1.HTTPRoute {
return &gatewayapi_v1beta1.HTTPRoute{
ObjectMeta: metav1.ObjectMeta{
Expand Down
57 changes: 21 additions & 36 deletions internal/dag/gatewayapi_processor.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,18 @@
package dag

import (
"errors"
"fmt"
"net/http"
"regexp"
"strings"
"time"

contour_api_v1 "github.com/projectcontour/contour/apis/projectcontour/v1"
"github.com/projectcontour/contour/internal/gatewayapi"
"github.com/projectcontour/contour/internal/k8s"
"github.com/projectcontour/contour/internal/ref"
"github.com/projectcontour/contour/internal/status"
"github.com/projectcontour/contour/internal/timeout"

"github.com/sirupsen/logrus"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -1107,36 +1108,27 @@ func (p *GatewayAPIProcessor) resolveRouteRefs(route any, routeAccessor *status.
// *NOTE: the Gateway API does not yet support retries, so do we.
// please refer to https://github.com/projectcontour/contour/pull/5940#discussion_r1385198654
// and related discussions for more information. for more information.
func parseHTTPRouteTimeouts(timeout *gatewayapi_v1.HTTPRouteTimeouts) (*RouteTimeoutPolicy, *RetryPolicy, error) {
if timeout == nil || (timeout.BackendRequest == nil && timeout.Request == nil) {
return nil, nil, nil
func parseHTTPRouteTimeouts(httpRouteTimeouts *gatewayapi_v1.HTTPRouteTimeouts) (*RouteTimeoutPolicy, error) {
if httpRouteTimeouts == nil {
return nil, nil
}

tr := &contour_api_v1.TimeoutPolicy{}
if timeout.Request != nil {
tr.Response = string(*timeout.Request)
}

rtp, _, err := timeoutPolicy(tr, 0)
if err != nil {
return nil, nil, fmt.Errorf("HTTPRoute.Spec.Rules.Timeouts: Invalid value for Request is specified")
if httpRouteTimeouts.BackendRequest != nil {
return nil, errors.New("HTTPRoute.Spec.Rules.Timeouts.BackendRequest is not supported")
}

br := &contour_api_v1.RetryPolicy{}
if timeout.BackendRequest != nil {
br.PerTryTimeout = string(*timeout.BackendRequest)
if httpRouteTimeouts.Request == nil {
return nil, nil
}

rp := retryPolicy(br)
if rp.PerTryTimeout.Duration() > rtp.ResponseTimeout.Duration() {
rp.PerTryTimeout = rtp.ResponseTimeout
requestTimeout, err := timeout.Parse(string(*httpRouteTimeouts.Request))
if err != nil {
return nil, fmt.Errorf("invalid HTTPRoute.Spec.Rules.Timeouts.Request: %v", err)
}

// retry disabled
rp.RetryOn = ""
return &rtp, rp, nil

return &RouteTimeoutPolicy{
ResponseTimeout: requestTimeout,
}, nil
}

func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1beta1.HTTPRoute, routeAccessor *status.RouteParentStatusUpdate, listener *listenerInfo, hosts sets.Set[string]) bool {
var programmed bool
for ruleIndex, rule := range route.Spec.Rules {
Expand Down Expand Up @@ -1179,19 +1171,17 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be

// Process rule-level filters.
var (
err error
redirect *Redirect
urlRewriteHostname string

retryPolicy *RetryPolicy
err error
redirect *Redirect
urlRewriteHostname string
mirrorPolicies []*MirrorPolicy
requestHeaderPolicy *HeadersPolicy
responseHeaderPolicy *HeadersPolicy
pathRewritePolicy *PathRewritePolicy
requestTimeoutPolicy *RouteTimeoutPolicy
)

requestTimeoutPolicy, retryPolicy, err = parseHTTPRouteTimeouts(rule.Timeouts)
requestTimeoutPolicy, err = parseHTTPRouteTimeouts(rule.Timeouts)
if err != nil {
routeAccessor.AddCondition(gatewayapi_v1beta1.RouteConditionAccepted, metav1.ConditionFalse, gatewayapi_v1beta1.RouteReasonUnsupportedValue, err.Error())
continue
Expand Down Expand Up @@ -1414,8 +1404,7 @@ func (p *GatewayAPIProcessor) computeHTTPRouteForListener(route *gatewayapi_v1be
responseHeaderPolicy,
mirrorPolicies,
pathRewritePolicy,
requestTimeoutPolicy,
retryPolicy)
requestTimeoutPolicy)
}

// Add each route to the relevant vhost(s)/svhosts(s).
Expand Down Expand Up @@ -1556,13 +1545,11 @@ func (p *GatewayAPIProcessor) computeGRPCRouteForListener(route *gatewayapi_v1al
KindGRPCRoute,
route.Namespace,
route.Name,

requestHeaderPolicy,
responseHeaderPolicy,
mirrorPolicies,
nil,
nil,
nil,
)

// Add each route to the relevant vhost(s)/svhosts(s).
Expand Down Expand Up @@ -2146,7 +2133,6 @@ func (p *GatewayAPIProcessor) clusterRoutes(
mirrorPolicies []*MirrorPolicy,
pathRewritePolicy *PathRewritePolicy,
timeoutPolicy *RouteTimeoutPolicy,
retryPolicy *RetryPolicy,
) []*Route {

var routes []*Route
Expand All @@ -2170,7 +2156,6 @@ func (p *GatewayAPIProcessor) clusterRoutes(
MirrorPolicies: mirrorPolicies,
Priority: priority,
PathRewritePolicy: pathRewritePolicy,
RetryPolicy: retryPolicy,
}
if timeoutPolicy != nil {
route.TimeoutPolicy = *timeoutPolicy
Expand Down
14 changes: 7 additions & 7 deletions internal/dag/status_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9222,7 +9222,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1),
})

run(t, "route rule with timeouts", testcase{
run(t, "route rule with timeouts.request and timeouts.backendRequest specified", testcase{
objs: []any{
kuardService,
&gatewayapi_v1beta1.HTTPRoute{
Expand Down Expand Up @@ -9256,15 +9256,15 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"),
Conditions: []metav1.Condition{
routeResolvedRefsCondition(),
routeAcceptedHTTPRouteCondition(),
routeAcceptedFalse(gatewayapi_v1.RouteReasonUnsupportedValue, "HTTPRoute.Spec.Rules.Timeouts.BackendRequest is not supported"),
},
},
},
}},
wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1),
wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0),
})

run(t, "route rule with timeouts.request empty", testcase{
run(t, "route rule with only timeouts.backendRequest specified", testcase{
objs: []any{
kuardService,
&gatewayapi_v1beta1.HTTPRoute{
Expand Down Expand Up @@ -9297,12 +9297,12 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"),
Conditions: []metav1.Condition{
routeResolvedRefsCondition(),
routeAcceptedHTTPRouteCondition(),
routeAcceptedFalse(gatewayapi_v1.RouteReasonUnsupportedValue, "HTTPRoute.Spec.Rules.Timeouts.BackendRequest is not supported"),
},
},
},
}},
wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 1),
wantGatewayStatusUpdate: validGatewayStatusUpdate("http", gatewayapi_v1.HTTPProtocolType, 0),
})

run(t, "timeouts with invalid request for httproute", testcase{
Expand Down Expand Up @@ -9335,7 +9335,7 @@ func TestGatewayAPIHTTPRouteDAGStatus(t *testing.T) {
ParentRef: gatewayapi.GatewayParentRef("projectcontour", "contour"),
Conditions: []metav1.Condition{
routeResolvedRefsCondition(),
routeAcceptedFalse(gatewayapi_v1.RouteReasonUnsupportedValue, "HTTPRoute.Spec.Rules.Timeouts: Invalid value for Request is specified"),
routeAcceptedFalse(gatewayapi_v1.RouteReasonUnsupportedValue, "invalid HTTPRoute.Spec.Rules.Timeouts.Request: unable to parse timeout string \"invalid\": time: invalid duration \"invalid\""),
},
},
},
Expand Down

0 comments on commit 805debf

Please sign in to comment.