Skip to content

Commit

Permalink
allow to override default route timeout (#3016)
Browse files Browse the repository at this point in the history
Co-authored-by: Stavros Kontopoulos <[email protected]>
  • Loading branch information
openshift-cherrypick-robot and skonto authored Nov 14, 2024
1 parent 1f34b3b commit bbe85e1
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,8 @@ spec:
fieldPath: metadata.namespace
- name: KUBERNETES_MIN_VERSION
value: "v1.0.0"
- name: ROUTE_HAPROXY_TIMEOUT
value: "600"
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
Expand Down
14 changes: 13 additions & 1 deletion serving/ingress/pkg/reconciler/ingress/resources/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"crypto/sha256"
"errors"
"fmt"
"os"
"strconv"
"strings"

socommon "github.com/openshift-knative/serverless-operator/pkg/common"
Expand All @@ -27,11 +29,13 @@ const (

OpenShiftIngressLabelKey = socommon.ServingDownstreamDomain + "/ingressName"
OpenShiftIngressNamespaceLabelKey = socommon.ServingDownstreamDomain + "/ingressNamespace"

HAProxyTimeoutEnv = "ROUTE_HAPROXY_TIMEOUT"
)

// DefaultTimeout is set by DefaultMaxRevisionTimeoutSeconds. So, the OpenShift Route's timeout
// should not have any effect on Knative services by default.
var DefaultTimeout = fmt.Sprintf("%vs", config.DefaultMaxRevisionTimeoutSeconds)
var DefaultTimeout = getDefaultHAProxyTimeout()

// ErrNoValidLoadbalancerDomain indicates that the current ingress does not have a DomainInternal field, or
// said field does not contain a value we can work with.
Expand Down Expand Up @@ -182,3 +186,11 @@ func routeName(uid, host string) string {
func hashHost(host string) string {
return fmt.Sprintf("%x", sha256.Sum256([]byte(host)))[0:6]
}

func getDefaultHAProxyTimeout() string {
timeout := os.Getenv(HAProxyTimeoutEnv)
if _, err := strconv.ParseInt(timeout, 10, 64); err == nil {
return fmt.Sprintf("%vs", timeout)
}
return fmt.Sprintf("%vs", config.DefaultMaxRevisionTimeoutSeconds)
}
48 changes: 48 additions & 0 deletions serving/ingress/pkg/reconciler/ingress/resources/route_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ func TestMakeRoute(t *testing.T) {
ingress *networkingv1alpha1.Ingress
want []*routev1.Route
wantErr error
timeout string
}{
{
name: "no rules",
Expand Down Expand Up @@ -85,6 +86,44 @@ func TestMakeRoute(t *testing.T) {
WildcardPolicy: routev1.WildcardPolicyNone,
},
}},
}, {
name: "valid, default timeout modified by env var",
ingress: ingress(withRules(
rule(withHosts([]string{localDomain, externalDomain}))),
),
want: []*routev1.Route{{
ObjectMeta: metav1.ObjectMeta{
Labels: map[string]string{
networking.IngressLabelKey: "ingress",
serving.RouteLabelKey: "route1",
serving.RouteNamespaceLabelKey: "default",
OpenShiftIngressLabelKey: "ingress",
OpenShiftIngressNamespaceLabelKey: "default",
},
Annotations: map[string]string{
TimeoutAnnotation: "900s",
},
Namespace: lbNamespace,
Name: routeName0,
},
Spec: routev1.RouteSpec{
Host: externalDomain,
To: routev1.RouteTargetReference{
Kind: "Service",
Name: lbService,
Weight: ptr.Int32(100),
},
Port: &routev1.RoutePort{
TargetPort: intstr.FromString(HTTPPort),
},
TLS: &routev1.TLSConfig{
Termination: routev1.TLSTerminationEdge,
InsecureEdgeTerminationPolicy: routev1.InsecureEdgeTerminationPolicyAllow,
},
WildcardPolicy: routev1.WildcardPolicyNone,
},
}},
timeout: "900",
},
{
name: "valid but disabled",
Expand Down Expand Up @@ -375,13 +414,22 @@ func TestMakeRoute(t *testing.T) {

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if test.timeout != "" {
t.Setenv(HAProxyTimeoutEnv, test.timeout)
DefaultTimeout = getDefaultHAProxyTimeout()
defer func() {
t.Setenv(HAProxyTimeoutEnv, "")
DefaultTimeout = getDefaultHAProxyTimeout()
}()
}
routes, err := MakeRoutes(test.ingress)
if test.want != nil && !cmp.Equal(routes, test.want) {
t.Errorf("got = %v, want: %v, diff: %s", routes, test.want, cmp.Diff(routes, test.want))
}
if !errors.Is(err, test.wantErr) {
t.Errorf("got = %v, want: %v", err, test.wantErr)
}

})
}
}
Expand Down
2 changes: 2 additions & 0 deletions templates/csv.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,8 @@ spec:
fieldPath: metadata.namespace
- name: KUBERNETES_MIN_VERSION
value: "v1.0.0"
- name: ROUTE_HAPROXY_TIMEOUT
value: "600"
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
Expand Down

0 comments on commit bbe85e1

Please sign in to comment.