OCPBUGS-43745: Add support for IdleCloseTerminationPolicy

go.mod

@@ -157,6 +157,6 @@ require (
 // github.com/operator-framework/operator-sdk.
 replace (
 	bitbucket.org/ww/goautoneg => github.com/munnerz/goautoneg v0.0.0-20120707110453-a547fc61f48d
-	github.com/openshift/api => github.com/openshift/api v0.0.0-20241018083007-4f6053f954b0
+	github.com/openshift/api => github.com/frobware/api v0.0.0-20241209160021-000e4bc7faac
 	k8s.io/client-go => k8s.io/client-go v0.31.1
 )

go.sum

@@ -341,6 +341,8 @@ github.com/fatih/structtag v1.1.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4
 github.com/florianl/go-nfqueue v1.3.2 h1:8DPzhKJHywpHJAE/4ktgcqveCL7qmMLsEsVD68C4x4I=
 github.com/florianl/go-nfqueue v1.3.2/go.mod h1:eSnAor2YCfMCVYrVNEhkLGN/r1L+J4uDjc0EUy0tfq4=
 github.com/fortytw2/leaktest v1.3.0/go.mod h1:jDsjWgpAGjm2CA7WthBh/CdZYEPF31XHquHwclZch5g=
+github.com/frobware/api v0.0.0-20241209160021-000e4bc7faac h1:LK11h8uaXrmIQxk2Dlikqie7zA87zrnO+lpIy6qdne0=
+github.com/frobware/api v0.0.0-20241209160021-000e4bc7faac/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
 github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
 github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ=
 github.com/fsnotify/fsnotify v1.7.0 h1:8JEhPFa5W2WU7YfeZzPNqzMP6Lwt7L2715Ggo0nosvA=
@@ -993,8 +995,6 @@ github.com/opencontainers/runtime-spec v0.1.2-0.20190507144316-5b71a03e2700/go.m
 github.com/opencontainers/runtime-spec v0.1.2-0.20190618234442-a950415649c7/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-spec v1.0.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
 github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mod h1:r3f7wjNzSs2extwzU3Y+6pKfobzPh+kKFJ3ofN+3nfs=
-github.com/openshift/api v0.0.0-20241018083007-4f6053f954b0 h1:9CBNaPGycU2dDzq0XoRIqxH0vHZezKDfbINx8e5zH0I=
-github.com/openshift/api v0.0.0-20241018083007-4f6053f954b0/go.mod h1:Shkl4HanLwDiiBzakv+con/aMGnVE2MAGvoKp5oyYUo=
 github.com/openshift/build-machinery-go v0.0.0-20200211121458-5e3d6e570160/go.mod h1:1CkcsT3aVebzRBzVTSbiKSkJMsC/CASqxesfqEMfJEc=
 github.com/openshift/client-go v0.0.0-20200116152001-92a2713fa240/go.mod h1:4riOwdj99Hd/q+iAcJZfNCsQQQMwURnZV6RL4WHYS5w=
 github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8 h1:HGfbllzRcrJBSiwzNjBCs7sExLUxC5/1evnvlNGB0Cg=

manifests/00-custom-resource-definition.yaml

@@ -1255,6 +1255,76 @@ spec:
                         type: string
                     type: object
                 type: object
+              idleConnectionTerminationPolicy:
+                default: Immediate
+                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).
+
+                  Allowed values for this field are "Immediate" and
+                  "Deferred". The default value is "Immediate".
+
+                  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.
+                enum:
+                - Immediate
+                - Deferred
+                type: string
               logging:
                 description: |-
                   logging defines parameters for what should be logged where.  If this
@@ -2089,7 +2159,7 @@ spec:
                     type: string
                   connectTimeout:
                     description: |-
-                      ConnectTimeout defines the maximum time to wait for
+                      connectTimeout defines the maximum time to wait for
                       a connection attempt to a server/backend to succeed.
 
                       This field expects an unsigned duration string of decimal numbers, each with optional

pkg/operator/controller/ingress/deployment.go

@@ -1185,6 +1185,13 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
 		)
 	}
 
+	if ci.Spec.IdleConnectionTerminationPolicy == operatorv1.IngressControllerConnectionTerminationPolicyDeferred {
+		env = append(env, corev1.EnvVar{
+			Name:  "ROUTER_IDLE_CLOSE_ON_RESPONSE",
+			Value: "true",
+		})
+	}
+
 	// TODO: The only connections from the router that may need the cluster-wide proxy are those for downloading CRLs,
 	// which, as of writing this, will always be http. If https becomes necessary, the router will need to mount the
 	// trusted CA bundle that cluster-network-operator generates. The process for adding that is described here:

pkg/operator/controller/ingress/deployment_test.go

@@ -2564,3 +2564,54 @@ func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
 
 	checkDeploymentHasEnvSorted(t, deployment)
 }
+
+// Test_IdleConnectionTerminationPolicy validates that the ingress
+// controller correctly sets the ROUTER_IDLE_CLOSE_ON_RESPONSE
+// environment variable based on the IngressController's
+// IdleConnectionTerminationPolicy field.
+func Test_IdleConnectionTerminationPolicy(t *testing.T) {
+	ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)
+
+	for _, tc := range []struct {
+		name                string
+		policy              operatorv1.IngressControllerConnectionTerminationPolicy
+		expectEnvVarPresent bool
+		expectedEnvVarValue string
+	}{{
+		name:                "IdleConnectionTerminationPolicy is Deferred",
+		policy:              operatorv1.IngressControllerConnectionTerminationPolicyDeferred,
+		expectEnvVarPresent: true,
+		expectedEnvVarValue: "true",
+	}, {
+		name:                "IdleConnectionTerminationPolicy is not set",
+		policy:              "",
+		expectEnvVarPresent: false,
+		expectedEnvVarValue: "",
+	}, {
+		name:                "IdleConnectionTerminationPolicy is Immediate (default)",
+		policy:              operatorv1.IngressControllerConnectionTerminationPolicyImmediate,
+		expectEnvVarPresent: false,
+		expectedEnvVarValue: "",
+	}} {
+		t.Run(tc.name, func(t *testing.T) {
+			ic.Spec.IdleConnectionTerminationPolicy = tc.policy
+
+			deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
+			if err != nil {
+				t.Fatalf("failed to generate desired router Deployment: %v", err)
+			}
+
+			expectedEnv := []envData{{
+				name:          "ROUTER_IDLE_CLOSE_ON_RESPONSE",
+				expectPresent: tc.expectEnvVarPresent,
+				expectedValue: tc.expectedEnvVarValue,
+			}}
+
+			if err := checkDeploymentEnvironment(t, deployment, expectedEnv); err != nil {
+				t.Errorf("environment variable check failed: %v", err)
+			}
+
+			checkDeploymentHasEnvSorted(t, deployment)
+		})
+	}
+}

test/e2e/all_test.go

@@ -88,6 +88,7 @@ func TestAll(t *testing.T) {
 		t.Run("TestUnmanagedAWSLBSubnets", TestUnmanagedAWSLBSubnets)
 		t.Run("TestAWSEIPAllocationsForNLB", TestAWSEIPAllocationsForNLB)
 		t.Run("TestUnmanagedAWSEIPAllocations", TestUnmanagedAWSEIPAllocations)
+		t.Run("Test_IdleConnectionTerminationPolicy", Test_IdleConnectionTerminationPolicy)
 	})
 
 	t.Run("serial", func(t *testing.T) {