OCPBUGS-48377: Add IdleCloseOnResponse field to IngressControllerSpec

Add an `IdleConnectionTerminationPolicy` field to control whether HAProxy keeps idle frontend connections open during a soft stop (router reload). Allow users to prevent errors in clients or load balancers that do not properly handle connection resets. This commit is based on openshift#2102 with a key in the default value. For OCP versions <= 4.18, the default is `Deferred`, aligning with older releases where HAProxy’s `idle-close-on-response` option was added unconditionally.
alebedev87 · Jan 29, 2025 · 90df916 · 90df916
1 parent 55c9ecb
commit 90df916
Show file tree

Hide file tree

Showing 12 changed files with 423 additions and 19 deletions.
diff --git a/openapi/generated_openapi/zz_generated.openapi.go b/openapi/generated_openapi/zz_generated.openapi.go
diff --git a/openapi/openapi.json b/openapi/openapi.json
@@ -28418,6 +28418,11 @@
           "description": "httpHeaders defines policy for HTTP headers.\n\nIf this field is empty, the default values are used.",
           "$ref": "#/definitions/com.github.openshift.api.operator.v1.IngressControllerHTTPHeaders"
         },
+        "idleConnectionTerminationPolicy": {
+          "description": "idleConnectionTerminationPolicy maps directly to HAProxy's idle-close-on-response option and controls whether HAProxy keeps idle frontend connections open during a soft stop (router reload).\n\nAllowed values for this field are \"Immediate\" and \"Deferred\". The default value is \"Deferred\".\n\nWhen set to \"Immediate\", idle connections are closed immediately during router reloads. This ensures immediate propagation of route changes but may impact clients sensitive to connection resets.\n\nWhen set to \"Deferred\", HAProxy will maintain idle connections during a soft reload instead of closing them immediately. These connections remain open until any of the following occurs:\n\n  - A new request is received on the connection, in which\n    case HAProxy handles it in the old process and closes\n    the connection after sending the response.\n\n  - HAProxy's `timeout http-keep-alive` duration expires\n    (300 seconds in OpenShift's configuration, not\n    configurable).\n\n  - The client's keep-alive timeout expires, causing the\n    client to close the connection.\n\nSetting Deferred can help prevent errors in clients or load balancers that do not properly handle connection resets. Additionally, this option allows you to retain the pre-2.4 HAProxy behaviour: in HAProxy version 2.2 (OpenShift versions < 4.14), maintaining idle connections during a soft reload was the default behaviour, but starting with HAProxy 2.4, the default changed to closing idle connections immediately.\n\nImportant Consideration:\n\n  - Using Deferred will result in temporary inconsistencies\n    for the first request on each persistent connection\n    after a route update and router reload. This request\n    will be processed by the old HAProxy process using its\n    old configuration. Subsequent requests will use the\n    updated configuration.\n\nOperational Considerations:\n\n  - Keeping idle connections open during reloads may lead\n    to an accumulation of old HAProxy processes if\n    connections remain idle for extended periods,\n    especially in environments where frequent reloads\n    occur.\n\n  - Consider monitoring the number of HAProxy processes in\n    the router pods when Deferred is set.\n\n  - You may need to enable or adjust the\n    `ingress.operator.openshift.io/hard-stop-after`\n    duration (configured via an annotation on the\n    IngressController resource) in environments with\n    frequent reloads to prevent resource exhaustion.",
+          "type": "string",
+          "default": "Deferred"
+        },
         "logging": {
           "description": "logging defines parameters for what should be logged where.  If this field is empty, operational logs are enabled but access logs are disabled.",
           "$ref": "#/definitions/com.github.openshift.api.operator.v1.IngressControllerLogging"

diff --git a/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml b/operator/v1/tests/ingresscontrollers.operator.openshift.io/AAA_ungated.yaml
@@ -15,6 +15,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
     - name: Should be able to create an IngressController with valid Actions
       initial: |
         apiVersion: operator.openshift.io/v1
@@ -75,6 +76,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           httpHeaders:
             actions:
               response:
@@ -496,6 +498,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           tuningOptions:
             connectTimeout: 10s
     - name: Should be able to create an IngressController with valid composite connect timeout
@@ -516,6 +519,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           tuningOptions:
             connectTimeout: 100ms300μs
     - name: Should be able to create an IngressController with valid fraction connect timeout
@@ -536,6 +540,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           tuningOptions:
             connectTimeout: 1.5m
     - name: Should not be able to create an IngressController with invalid unit connect timeout

diff --git a/operator/v1/tests/ingresscontrollers.operator.openshift.io/IngressControllerLBOpenStack.yaml b/operator/v1/tests/ingresscontrollers.operator.openshift.io/IngressControllerLBOpenStack.yaml
@@ -19,6 +19,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -42,6 +43,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -67,6 +69,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:

diff --git a/...ator/v1/tests/ingresscontrollers.operator.openshift.io/IngressControllerLBSubnetsAWS.yaml b/...ator/v1/tests/ingresscontrollers.operator.openshift.io/IngressControllerLBSubnetsAWS.yaml
@@ -30,6 +30,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -71,6 +72,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -184,6 +186,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -474,6 +477,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -531,6 +535,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:
@@ -580,6 +585,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             type: LoadBalancerService
             loadBalancer:

diff --git a/...ator/v1/tests/ingresscontrollers.operator.openshift.io/SetEIPForNLBIngressController.yaml b/...ator/v1/tests/ingresscontrollers.operator.openshift.io/SetEIPForNLBIngressController.yaml
@@ -15,6 +15,7 @@ tests:
         kind: IngressController
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
     - name: Should allow to set NLB parameters when LBType is NLB.
       initial: |
         apiVersion: operator.openshift.io/v1
@@ -43,6 +44,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -379,6 +381,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -433,6 +436,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -486,6 +490,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -534,6 +539,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -555,6 +561,7 @@ tests:
           namespace: openshift-ingress-operator
         spec:
           httpEmptyRequestsPolicy: Respond
+          idleConnectionTerminationPolicy: Deferred
           endpointPublishingStrategy:
             loadBalancer:
               dnsManagementPolicy: Managed
@@ -567,4 +574,4 @@ tests:
                     eipAllocations:
                     - eipalloc-1234567890abcdefa
                     - eipalloc-1234567890abcdefb
-            type: LoadBalancerService
+            type: LoadBalancerService
diff --git a/operator/v1/types_ingress.go b/operator/v1/types_ingress.go
@@ -258,6 +258,75 @@ type IngressControllerSpec struct {
 	//
 	// +optional
 	HTTPCompression HTTPCompressionPolicy `json:"httpCompression,omitempty"`
+
+	// idleConnectionTerminationPolicy maps directly to HAProxy's
+	// idle-close-on-response option and controls whether HAProxy
+	// keeps idle frontend connections open during a soft stop
+	// (router reload).
+	//
+	// Allowed values for this field are "Immediate" and
+	// "Deferred". The default value is "Deferred".
+	//
+	// When set to "Immediate", idle connections are closed
+	// immediately during router reloads. This ensures immediate
+	// propagation of route changes but may impact clients
+	// sensitive to connection resets.
+	//
+	// When set to "Deferred", HAProxy will maintain idle
+	// connections during a soft reload instead of closing them
+	// immediately. These connections remain open until any of the
+	// following occurs:
+	//
+	//   - A new request is received on the connection, in which
+	//     case HAProxy handles it in the old process and closes
+	//     the connection after sending the response.
+	//
+	//   - HAProxy's `timeout http-keep-alive` duration expires
+	//     (300 seconds in OpenShift's configuration, not
+	//     configurable).
+	//
+	//   - The client's keep-alive timeout expires, causing the
+	//     client to close the connection.
+	//
+	// Setting Deferred can help prevent errors in clients or load
+	// balancers that do not properly handle connection resets.
+	// Additionally, this option allows you to retain the pre-2.4
+	// HAProxy behaviour: in HAProxy version 2.2 (OpenShift
+	// versions < 4.14), maintaining idle connections during a
+	// soft reload was the default behaviour, but starting with
+	// HAProxy 2.4, the default changed to closing idle
+	// connections immediately.
+	//
+	// Important Consideration:
+	//
+	//   - Using Deferred will result in temporary inconsistencies
+	//     for the first request on each persistent connection
+	//     after a route update and router reload. This request
+	//     will be processed by the old HAProxy process using its
+	//     old configuration. Subsequent requests will use the
+	//     updated configuration.
+	//
+	// Operational Considerations:
+	//
+	//   - Keeping idle connections open during reloads may lead
+	//     to an accumulation of old HAProxy processes if
+	//     connections remain idle for extended periods,
+	//     especially in environments where frequent reloads
+	//     occur.
+	//
+	//   - Consider monitoring the number of HAProxy processes in
+	//     the router pods when Deferred is set.
+	//
+	//   - You may need to enable or adjust the
+	//     `ingress.operator.openshift.io/hard-stop-after`
+	//     duration (configured via an annotation on the
+	//     IngressController resource) in environments with
+	//     frequent reloads to prevent resource exhaustion.
+	//
+	// +optional
+	// +kubebuilder:default:="Deferred"
+	// +default="Deferred"
+	IdleConnectionTerminationPolicy IngressControllerConnectionTerminationPolicy `json:"idleConnectionTerminationPolicy,omitempty"`
 }
 
 // httpCompressionPolicy turns on compression for the specified MIME types.
@@ -2068,3 +2137,23 @@ type IngressControllerList struct {
 
 	Items []IngressController `json:"items"`
 }
+
+// IngressControllerConnectionTerminationPolicy defines the behaviour
+// for handling idle connections during a soft reload of the router.
+//
+// +kubebuilder:validation:Enum=Immediate;Deferred
+type IngressControllerConnectionTerminationPolicy string
+
+const (
+	// IngressControllerConnectionTerminationPolicyImmediate specifies
+	// that idle connections should be closed immediately during a
+	// router reload.
+	IngressControllerConnectionTerminationPolicyImmediate IngressControllerConnectionTerminationPolicy = "Immediate"
+
+	// IngressControllerConnectionTerminationPolicyDeferred
+	// specifies that idle connections should remain open until a
+	// terminating event, such as a new request, the expiration of
+	// the proxy keep-alive timeout, or the client closing the
+	// connection.
+	IngressControllerConnectionTerminationPolicyDeferred IngressControllerConnectionTerminationPolicy = "Deferred"
+)