From a47ad11ba25a38e2605a20617c3aed02f0a7fc31 Mon Sep 17 00:00:00 2001 From: Andrey Lebedev Date: Fri, 24 Oct 2025 10:47:00 +0200 Subject: [PATCH] OCPBUGS-61858: Add HTTPKeepAliveTimeout to IngressController API This commit introduces `HTTPKeepAliveTimeout` tuning option to the IngressController API, allowing customers to configure `timeout http-keep-alive`. In OCP versions prior to 4.16, this timeout was not respected (see https://github.com/haproxy/haproxy/issues/2334). This addition brings the ability to adjust the behavior to match pre-4.16 configurations. --- .../generated_openapi/zz_generated.openapi.go | 6 ++ .../AAA_ungated.yaml | 85 +++++++++++++++++++ operator/v1/types_ingress.go | 25 ++++++ ..._50_ingress_00_ingresscontrollers.crd.yaml | 23 +++++ operator/v1/zz_generated.deepcopy.go | 5 ++ .../AAA_ungated.yaml | 23 +++++ .../v1/zz_generated.swagger_doc_generated.go | 1 + 7 files changed, 168 insertions(+) diff --git a/openapi/generated_openapi/zz_generated.openapi.go b/openapi/generated_openapi/zz_generated.openapi.go index 6bcb9038af6..9fa85e0ed81 100644 --- a/openapi/generated_openapi/zz_generated.openapi.go +++ b/openapi/generated_openapi/zz_generated.openapi.go @@ -52782,6 +52782,12 @@ func schema_openshift_api_operator_v1_IngressControllerTuningOptions(ref common. Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"), }, }, + "httpKeepAliveTimeout": { + SchemaProps: spec.SchemaProps{ + Description: "httpKeepAliveTimeout defines the maximum allowed time to wait for a new HTTP request to appear on a connection from the client to the router.\n\nThis field expects an unsigned duration string of decimal numbers, each with optional fraction and a unit suffix, e.g. \"300ms\", \"1.5h\" or \"2h45m\". Valid time units are \"ns\", \"us\" (or \"µs\" U+00B5 or \"μs\" U+03BC), \"ms\", \"s\", \"m\", \"h\".\n\nWhen omitted, this means the user has no opinion and the platform is left to choose a reasonable default. This default is subject to change over time. The current default is 300s.\n\nLow values (milliseconds or less) can cause clients to close and reopen connections for each request, leading to reduced connection sharing. For HTTP/2, special care should be taken with low values. A few seconds is a reasonable starting point to avoid holding idle connections open while still allowing subsequent requests to reuse the connection.\n\nHigh values (minutes or more) favor connection reuse but may cause idle connections to linger longer.", + Ref: ref("k8s.io/apimachinery/pkg/apis/meta/v1.Duration"), + }, + }, "tlsInspectDelay": { SchemaProps: spec.SchemaProps{ Description: "tlsInspectDelay defines how long the router can hold data to find a matching route.\n\nSetting this too short can cause the router to fall back to the default certificate for edge-terminated or reencrypt routes even when a better matching certificate could be used.\n\nIf unset, the default inspect delay is 5s", diff --git a/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml b/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml index b7f6317ae93..298f68a1b37 100644 --- a/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml +++ b/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml @@ -563,6 +563,91 @@ tests: tuningOptions: connectTimeout: "4 s" expectedError: "IngressController.operator.openshift.io \"default\" is invalid: spec.tuningOptions.connectTimeout: Invalid value: \"4 s\": spec.tuningOptions.connectTimeout in body should match '^(0|([0-9]+(\\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$'" + - name: Should be able to create an IngressController with valid nominal httpKeepAlive timeout + initial: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + tuningOptions: + httpKeepAliveTimeout: 10s + expected: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + httpEmptyRequestsPolicy: Respond + idleConnectionTerminationPolicy: Immediate + tuningOptions: + httpKeepAliveTimeout: 10s + - name: Should be able to create an IngressController with valid composite httpKeepAlive timeout + initial: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + tuningOptions: + httpKeepAliveTimeout: 100ms300μs + expected: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + httpEmptyRequestsPolicy: Respond + idleConnectionTerminationPolicy: Immediate + tuningOptions: + httpKeepAliveTimeout: 100ms300μs + - name: Should be able to create an IngressController with valid fraction httpKeepAlive timeout + initial: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + tuningOptions: + httpKeepAliveTimeout: 1.5m + expected: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + httpEmptyRequestsPolicy: Respond + idleConnectionTerminationPolicy: Immediate + tuningOptions: + httpKeepAliveTimeout: 1.5m + - name: Should not be able to create an IngressController with invalid unit httpKeepAlive timeout + initial: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + tuningOptions: + httpKeepAliveTimeout: 3d + expectedError: "IngressController.operator.openshift.io \"default\" is invalid: spec.tuningOptions.httpKeepAliveTimeout: Invalid value: \"3d\": spec.tuningOptions.httpKeepAliveTimeout in body should match '^(0|([0-9]+(\\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$'" + - name: Should not be able to create an IngressController with invalid space httpKeepAlive timeout + initial: | + apiVersion: operator.openshift.io/v1 + kind: IngressController + metadata: + name: default + namespace: openshift-ingress-operator + spec: + tuningOptions: + httpKeepAliveTimeout: "4 s" + expectedError: "IngressController.operator.openshift.io \"default\" is invalid: spec.tuningOptions.httpKeepAliveTimeout: Invalid value: \"4 s\": spec.tuningOptions.httpKeepAliveTimeout in body should match '^(0|([0-9]+(\\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$'" - name: Should be able to create an IngressController with valid domain initial: | apiVersion: operator.openshift.io/v1 diff --git a/operator/v1/types_ingress.go b/operator/v1/types_ingress.go index 46b906518d9..ddb343594d4 100644 --- a/operator/v1/types_ingress.go +++ b/operator/v1/types_ingress.go @@ -1884,6 +1884,31 @@ type IngressControllerTuningOptions struct { // +optional ConnectTimeout *metav1.Duration `json:"connectTimeout,omitempty"` + // httpKeepAliveTimeout defines the maximum allowed time to wait for + // a new HTTP request to appear on a connection from the client to the router. + // + // This field expects an unsigned duration string of decimal numbers, each with optional + // fraction and a unit suffix, e.g. "300ms", "1.5h" or "2h45m". + // Valid time units are "ns", "us" (or "µs" U+00B5 or "μs" U+03BC), "ms", "s", "m", "h". + // + // When omitted, this means the user has no opinion and the platform is left + // to choose a reasonable default. This default is subject to change over time. + // The current default is 300s. + // + // Low values (milliseconds or less) can cause clients to close and reopen connections + // for each request, leading to reduced connection sharing. + // For HTTP/2, special care should be taken with low values. + // A few seconds is a reasonable starting point to avoid holding idle connections open + // while still allowing subsequent requests to reuse the connection. + // + // High values (minutes or more) favor connection reuse but may cause idle + // connections to linger longer. + // + // +kubebuilder:validation:Pattern=^(0|([0-9]+(\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$ + // +kubebuilder:validation:Type:=string + // +optional + HTTPKeepAliveTimeout *metav1.Duration `json:"httpKeepAliveTimeout,omitempty"` + // tlsInspectDelay defines how long the router can hold data to find a // matching route. // diff --git a/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml b/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml index 25c51d79566..c416382474c 100644 --- a/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml +++ b/operator/v1/zz_generated.crd-manifests/0000_50_ingress_00_ingresscontrollers.crd.yaml @@ -2250,6 +2250,29 @@ spec: 2147483647ms (24.85 days). Both are subject to change over time. pattern: ^(0|([0-9]+(\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$ type: string + httpKeepAliveTimeout: + description: |- + httpKeepAliveTimeout defines the maximum allowed time to wait for + a new HTTP request to appear on a connection from the client to the router. + + This field expects an unsigned duration string of decimal numbers, each with optional + fraction and a unit suffix, e.g. "300ms", "1.5h" or "2h45m". + Valid time units are "ns", "us" (or "µs" U+00B5 or "μs" U+03BC), "ms", "s", "m", "h". + + When omitted, this means the user has no opinion and the platform is left + to choose a reasonable default. This default is subject to change over time. + The current default is 300s. + + Low values (milliseconds or less) can cause clients to close and reopen connections + for each request, leading to reduced connection sharing. + For HTTP/2, special care should be taken with low values. + A few seconds is a reasonable starting point to avoid holding idle connections open + while still allowing subsequent requests to reuse the connection. + + High values (minutes or more) favor connection reuse but may cause idle + connections to linger longer. + pattern: ^(0|([0-9]+(\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$ + type: string maxConnections: description: |- maxConnections defines the maximum number of simultaneous diff --git a/operator/v1/zz_generated.deepcopy.go b/operator/v1/zz_generated.deepcopy.go index fd83694c23f..3bc6b81de46 100644 --- a/operator/v1/zz_generated.deepcopy.go +++ b/operator/v1/zz_generated.deepcopy.go @@ -2564,6 +2564,11 @@ func (in *IngressControllerTuningOptions) DeepCopyInto(out *IngressControllerTun *out = new(metav1.Duration) **out = **in } + if in.HTTPKeepAliveTimeout != nil { + in, out := &in.HTTPKeepAliveTimeout, &out.HTTPKeepAliveTimeout + *out = new(metav1.Duration) + **out = **in + } if in.TLSInspectDelay != nil { in, out := &in.TLSInspectDelay, &out.TLSInspectDelay *out = new(metav1.Duration) diff --git a/operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml b/operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml index c271527c418..c8fdc51135a 100644 --- a/operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml +++ b/operator/v1/zz_generated.featuregated-crd-manifests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml @@ -2233,6 +2233,29 @@ spec: 2147483647ms (24.85 days). Both are subject to change over time. pattern: ^(0|([0-9]+(\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$ type: string + httpKeepAliveTimeout: + description: |- + httpKeepAliveTimeout defines the maximum allowed time to wait for + a new HTTP request to appear on a connection from the client to the router. + + This field expects an unsigned duration string of decimal numbers, each with optional + fraction and a unit suffix, e.g. "300ms", "1.5h" or "2h45m". + Valid time units are "ns", "us" (or "µs" U+00B5 or "μs" U+03BC), "ms", "s", "m", "h". + + When omitted, this means the user has no opinion and the platform is left + to choose a reasonable default. This default is subject to change over time. + The current default is 300s. + + Low values (milliseconds or less) can cause clients to close and reopen connections + for each request, leading to reduced connection sharing. + For HTTP/2, special care should be taken with low values. + A few seconds is a reasonable starting point to avoid holding idle connections open + while still allowing subsequent requests to reuse the connection. + + High values (minutes or more) favor connection reuse but may cause idle + connections to linger longer. + pattern: ^(0|([0-9]+(\.[0-9]+)?(ns|us|µs|μs|ms|s|m|h))+)$ + type: string maxConnections: description: |- maxConnections defines the maximum number of simultaneous diff --git a/operator/v1/zz_generated.swagger_doc_generated.go b/operator/v1/zz_generated.swagger_doc_generated.go index d3475d9024b..b6ae252f300 100644 --- a/operator/v1/zz_generated.swagger_doc_generated.go +++ b/operator/v1/zz_generated.swagger_doc_generated.go @@ -1115,6 +1115,7 @@ var map_IngressControllerTuningOptions = map[string]string{ "serverFinTimeout": "serverFinTimeout defines how long a connection will be held open while waiting for the server/backend response to the client closing the connection.\n\nIf unset, the default timeout is 1s", "tunnelTimeout": "tunnelTimeout defines how long a tunnel connection (including websockets) will be held open while the tunnel is idle.\n\nIf unset, the default timeout is 1h", "connectTimeout": "connectTimeout defines the maximum time to wait for a connection attempt to a server/backend to succeed.\n\nThis field expects an unsigned duration string of decimal numbers, each with optional fraction and a unit suffix, e.g. \"300ms\", \"1.5h\" or \"2h45m\". Valid time units are \"ns\", \"us\" (or \"µs\" U+00B5 or \"μs\" U+03BC), \"ms\", \"s\", \"m\", \"h\".\n\nWhen omitted, this means the user has no opinion and the platform is left to choose a reasonable default. This default is subject to change over time. The current default is 5s.", + "httpKeepAliveTimeout": "httpKeepAliveTimeout defines the maximum allowed time to wait for a new HTTP request to appear on a connection from the client to the router.\n\nThis field expects an unsigned duration string of decimal numbers, each with optional fraction and a unit suffix, e.g. \"300ms\", \"1.5h\" or \"2h45m\". Valid time units are \"ns\", \"us\" (or \"µs\" U+00B5 or \"μs\" U+03BC), \"ms\", \"s\", \"m\", \"h\".\n\nWhen omitted, this means the user has no opinion and the platform is left to choose a reasonable default. This default is subject to change over time. The current default is 300s.\n\nLow values (milliseconds or less) can cause clients to close and reopen connections for each request, leading to reduced connection sharing. For HTTP/2, special care should be taken with low values. A few seconds is a reasonable starting point to avoid holding idle connections open while still allowing subsequent requests to reuse the connection.\n\nHigh values (minutes or more) favor connection reuse but may cause idle connections to linger longer.", "tlsInspectDelay": "tlsInspectDelay defines how long the router can hold data to find a matching route.\n\nSetting this too short can cause the router to fall back to the default certificate for edge-terminated or reencrypt routes even when a better matching certificate could be used.\n\nIf unset, the default inspect delay is 5s", "healthCheckInterval": "healthCheckInterval defines how long the router waits between two consecutive health checks on its configured backends. This value is applied globally as a default for all routes, but may be overridden per-route by the route annotation \"router.openshift.io/haproxy.health.check.interval\".\n\nExpects an unsigned duration string of decimal numbers, each with optional fraction and a unit suffix, eg \"300ms\", \"1.5h\" or \"2h45m\". Valid time units are \"ns\", \"us\" (or \"µs\" U+00B5 or \"μs\" U+03BC), \"ms\", \"s\", \"m\", \"h\".\n\nSetting this to less than 5s can cause excess traffic due to too frequent TCP health checks and accompanying SYN packet storms. Alternatively, setting this too high can result in increased latency, due to backend servers that are no longer available, but haven't yet been detected as such.\n\nAn empty or zero healthCheckInterval means no opinion and IngressController chooses a default, which is subject to change over time. Currently the default healthCheckInterval value is 5s.\n\nCurrently the minimum allowed value is 1s and the maximum allowed value is 2147483647ms (24.85 days). Both are subject to change over time.", "maxConnections": "maxConnections defines the maximum number of simultaneous connections that can be established per HAProxy process. Increasing this value allows each ingress controller pod to handle more connections but at the cost of additional system resources being consumed.\n\nPermitted values are: empty, 0, -1, and the range 2000-2000000.\n\nIf this field is empty or 0, the IngressController will use the default value of 50000, but the default is subject to change in future releases.\n\nIf the value is -1 then HAProxy will dynamically compute a maximum value based on the available ulimits in the running container. Selecting -1 (i.e., auto) will result in a large value being computed (~520000 on OpenShift >=4.10 clusters) and therefore each HAProxy process will incur significant memory usage compared to the current default of 50000.\n\nSetting a value that is greater than the current operating system limit will prevent the HAProxy process from starting.\n\nIf you choose a discrete value (e.g., 750000) and the router pod is migrated to a new node, there's no guarantee that that new node has identical ulimits configured. In such a scenario the pod would fail to start. If you have nodes with different ulimits configured (e.g., different tuned profiles) and you choose a discrete value then the guidance is to use -1 and let the value be computed dynamically at runtime.\n\nYou can monitor memory usage for router containers with the following metric: 'container_memory_working_set_bytes{container=\"router\",namespace=\"openshift-ingress\"}'.\n\nYou can monitor memory usage of individual HAProxy processes in router containers with the following metric: 'container_memory_working_set_bytes{container=\"router\",namespace=\"openshift-ingress\"}/container_processes{container=\"router\",namespace=\"openshift-ingress\"}'.",