[WIP] [release-4.20] OCPBUGS-66135: Implement HTTPKeepAliveTimeout tuning option

go.mod

@@ -37,9 +37,9 @@ require (
 	google.golang.org/grpc v1.72.0
 	gopkg.in/fsnotify.v1 v1.4.7
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/api v0.33.0
+	k8s.io/api v0.33.2
 	k8s.io/apiextensions-apiserver v0.33.0
-	k8s.io/apimachinery v0.33.0
+	k8s.io/apimachinery v0.33.2
 	k8s.io/apiserver v0.33.0
 	k8s.io/client-go v12.0.0+incompatible
 	k8s.io/utils v0.0.0-20250502105355-0f33e8f1c979
@@ -158,3 +158,5 @@ require (
 replace github.com/imdario/mergo => github.com/imdario/mergo v0.3.5
 
 replace k8s.io/client-go => k8s.io/client-go v0.32.1
+
+replace github.com/openshift/api => github.com/alebedev87/api v0.0.0-20251127191356-47904179fe48

go.sum

@@ -52,6 +52,8 @@ github.com/IBM/go-sdk-core/v5 v5.17.4 h1:VGb9+mRrnS2HpHZFM5hy4J6ppIWnwNrw0G+tLSg
 github.com/IBM/go-sdk-core/v5 v5.17.4/go.mod h1:KsAAI7eStAWwQa4F96MLy+whYSh39JzNjklZRbN/8ns=
 github.com/IBM/networking-go-sdk v0.26.0 h1:K/geWMCgg5P0pbaVQ0eZLhim2G6yOZc8rjszbv2Kmzc=
 github.com/IBM/networking-go-sdk v0.26.0/go.mod h1:tVxXclpQs8nQJYPTr9ZPNC1voaPNQLy8iy/72oVfFtM=
+github.com/alebedev87/api v0.0.0-20251127191356-47904179fe48 h1:ubnterFOz2hDXAqMeDQGYgY0vkzxM9YjMEJE8DrCdo4=
+github.com/alebedev87/api v0.0.0-20251127191356-47904179fe48/go.mod h1:SPLf21TYPipzCO67BURkCfK6dcIIxx0oNRVWaOyRcXM=
 github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
 github.com/antlr4-go/antlr/v4 v4.13.1 h1:SqQKkuVZ+zWkMMNkjy5FZe5mr5WURWnlpmOuzYWrPrQ=
 github.com/antlr4-go/antlr/v4 v4.13.1/go.mod h1:GKmUxMtwp6ZgGwZSva4eWPC5mS6vUAmOABFgjdkM7Nw=
@@ -342,8 +344,6 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y
 github.com/onsi/gomega v1.10.5/go.mod h1:gza4q3jKQJijlu05nKWRCW/GavJumGt8aNRxWg7mt48=
 github.com/onsi/gomega v1.37.0 h1:CdEG8g0S133B4OswTDC/5XPSzE1OeP29QOioj2PID2Y=
 github.com/onsi/gomega v1.37.0/go.mod h1:8D9+Txp43QWKhM24yyOBEdpkzN8FvJyAwecBgsU4KU0=
-github.com/openshift/api v0.0.0-20250305225826-b8da3bfeaf77 h1:w6F0sEhlUB1K54Ev4EELsLo5w/xur9pFT19VtemlB4Y=
-github.com/openshift/api v0.0.0-20250305225826-b8da3bfeaf77/go.mod h1:yk60tHAmHhtVpJQo3TwVYq2zpuP70iJIFDCmeKMIzPw=
 github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8 h1:HGfbllzRcrJBSiwzNjBCs7sExLUxC5/1evnvlNGB0Cg=
 github.com/openshift/client-go v0.0.0-20240405120947-c67c8325cdd8/go.mod h1:+VvvaMSTUhOt+rBq7NwRLSNxq06hTeRCBqm0j0PQEq8=
 github.com/openshift/library-go v0.0.0-20240419113445-f1541d628746 h1:MyLp0GgPyIgeVd1scI0iUasVgd6Xpy/t04Rk+I23wRE=
@@ -641,12 +641,12 @@ gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
-k8s.io/api v0.33.0 h1:yTgZVn1XEe6opVpP1FylmNrIFWuDqe2H0V8CT5gxfIU=
-k8s.io/api v0.33.0/go.mod h1:CTO61ECK/KU7haa3qq8sarQ0biLq2ju405IZAd9zsiM=
+k8s.io/api v0.33.2 h1:YgwIS5jKfA+BZg//OQhkJNIfie/kmRsO0BmNaVSimvY=
+k8s.io/api v0.33.2/go.mod h1:fhrbphQJSM2cXzCWgqU29xLDuks4mu7ti9vveEnpSXs=
 k8s.io/apiextensions-apiserver v0.33.0 h1:d2qpYL7Mngbsc1taA4IjJPRJ9ilnsXIrndH+r9IimOs=
 k8s.io/apiextensions-apiserver v0.33.0/go.mod h1:VeJ8u9dEEN+tbETo+lFkwaaZPg6uFKLGj5vyNEwwSzc=
-k8s.io/apimachinery v0.33.0 h1:1a6kHrJxb2hs4t8EE5wuR/WxKDwGN1FKH3JvDtA0CIQ=
-k8s.io/apimachinery v0.33.0/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM=
+k8s.io/apimachinery v0.33.2 h1:IHFVhqg59mb8PJWTLi8m1mAoepkUNYmptHsV+Z1m5jY=
+k8s.io/apimachinery v0.33.2/go.mod h1:BHW0YOu7n22fFv/JkYOEfkUYNRN0fj0BlvMFWA7b+SM=
 k8s.io/apiserver v0.33.0 h1:QqcM6c+qEEjkOODHppFXRiw/cE2zP85704YrQ9YaBbc=
 k8s.io/apiserver v0.33.0/go.mod h1:EixYOit0YTxt8zrO2kBU7ixAtxFce9gKGq367nFmqI8=
 k8s.io/client-go v0.32.1 h1:otM0AxdhdBIaQh7l1Q0jQpmo7WOFIk5FFa4bg6YMdUU=

manifests/00-custom-resource-definition.yaml

@@ -1280,9 +1280,9 @@ spec:
                       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).
+                    - HAProxy's `timeout http-keep-alive` duration expires.
+                      By default this is 300 seconds, but it can be changed
+                      using httpKeepAliveTimeout tuning option.
 
                     - The client's keep-alive timeout expires, causing the
                       client to close the connection.
@@ -2230,6 +2230,44 @@ 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 a decimal number, with optional
+                      fraction and a unit suffix, e.g. "300ms", "1.5s" or "2m45s".
+                      Valid time units are "ms", "s", "m".
+                      The allowed range is from 1 millisecond to 15 minutes.
+
+                      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 (tens of 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.
+                    maxLength: 16
+                    minLength: 1
+                    type: string
+                    x-kubernetes-validations:
+                    - message: httpKeepAliveTimeout must be a valid duration string
+                        composed of an unsigned integer value, optionally followed
+                        by a decimal fraction and a unit suffix (ms, s, m)
+                      rule: self.matches('^([0-9]+(\\.[0-9]+)?(ms|s|m))+$')
+                    - message: httpKeepAliveTimeout must be less than or equal to
+                        15 minutes
+                      rule: '!self.matches(''^([0-9]+(\\.[0-9]+)?(ms|s|m))+$'') ||
+                        duration(self) <= duration(''15m'')'
+                    - message: httpKeepAliveTimeout must be greater than or equal
+                        to 1 millisecond
+                      rule: '!self.matches(''^([0-9]+(\\.[0-9]+)?(ms|s|m))+$'') ||
+                        duration(self) >= duration(''1ms'')'
                   maxConnections:
                     description: |-
                       maxConnections defines the maximum number of simultaneous

pkg/operator/controller/ingress/deployment.go

@@ -663,6 +663,9 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, i
 	if ci.Spec.TuningOptions.ConnectTimeout != nil && ci.Spec.TuningOptions.ConnectTimeout.Duration > 0*time.Second {
 		env = append(env, corev1.EnvVar{Name: "ROUTER_DEFAULT_CONNECT_TIMEOUT", Value: durationToHAProxyTimespec(ci.Spec.TuningOptions.ConnectTimeout.Duration)})
 	}
+	if ci.Spec.TuningOptions.HTTPKeepAliveTimeout != nil && ci.Spec.TuningOptions.HTTPKeepAliveTimeout.Duration > 0*time.Second {
+		env = append(env, corev1.EnvVar{Name: "ROUTER_SLOWLORIS_HTTP_KEEPALIVE", Value: durationToHAProxyTimespec(ci.Spec.TuningOptions.HTTPKeepAliveTimeout.Duration)})
+	}
 	if ci.Spec.TuningOptions.TLSInspectDelay != nil && ci.Spec.TuningOptions.TLSInspectDelay.Duration > 0*time.Second {
 		env = append(env, corev1.EnvVar{Name: "ROUTER_INSPECT_DELAY", Value: durationToHAProxyTimespec(ci.Spec.TuningOptions.TLSInspectDelay.Duration)})
 	}

pkg/operator/controller/ingress/deployment_test.go

@@ -173,6 +173,7 @@ func TestTuningOptions(t *testing.T) {
 	ic.Spec.TuningOptions.TLSInspectDelay = &metav1.Duration{Duration: 5 * time.Second}
 	ic.Spec.TuningOptions.HealthCheckInterval = &metav1.Duration{Duration: 15 * time.Second}
 	ic.Spec.TuningOptions.ReloadInterval = metav1.Duration{Duration: 30 * time.Second}
+	ic.Spec.TuningOptions.HTTPKeepAliveTimeout = &metav1.Duration{Duration: 30 * time.Second}
 
 	deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
 	if err != nil {
@@ -187,6 +188,7 @@ func TestTuningOptions(t *testing.T) {
 		{"ROUTER_DEFAULT_SERVER_FIN_TIMEOUT", true, "4s"},
 		{"ROUTER_DEFAULT_TUNNEL_TIMEOUT", true, "30m"},
 		{"ROUTER_DEFAULT_CONNECT_TIMEOUT", true, "30s"},
+		{"ROUTER_SLOWLORIS_HTTP_KEEPALIVE", true, "30s"},
 		{"ROUTER_INSPECT_DELAY", true, "5s"},
 		{RouterBackendCheckInterval, true, "15s"},
 		{RouterReloadIntervalEnvName, true, "30s"},
@@ -444,6 +446,7 @@ func Test_desiredRouterDeployment(t *testing.T) {
 		{"ROUTER_DEFAULT_SERVER_TIMEOUT", false, ""},
 		{"ROUTER_DEFAULT_TUNNEL_TIMEOUT", false, ""},
 		{"ROUTER_DEFAULT_CONNECT_TIMEOUT", false, ""},
+		{"ROUTER_SLOWLORIS_HTTP_KEEPALIVE", false, ""},
 		{"ROUTER_ERRORFILE_503", false, ""},
 		{"ROUTER_ERRORFILE_404", false, ""},
 		{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},

test/e2e/all_test.go

@@ -89,6 +89,7 @@ func TestAll(t *testing.T) {
 		t.Run("TestUnmanagedAWSEIPAllocations", TestUnmanagedAWSEIPAllocations)
 		t.Run("Test_IdleConnectionTerminationPolicyImmediate", Test_IdleConnectionTerminationPolicyImmediate)
 		t.Run("Test_IdleConnectionTerminationPolicyDeferred", Test_IdleConnectionTerminationPolicyDeferred)
+		t.Run("Test_HTTPKeepAliveTimeout", Test_HTTPKeepAliveTimeout)
 	})
 
 	t.Run("serial", func(t *testing.T) {

test/e2e/http_keep_alive_test.go

@@ -0,0 +1,198 @@
+//go:build e2e
+// +build e2e
+
+package e2e
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"strings"
+	"testing"
+	"time"
+
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apiserver/pkg/storage/names"
+
+	operatorv1 "github.com/openshift/api/operator/v1"
+	routev1 "github.com/openshift/api/route/v1"
+)
+
+// Test_HTTPKeepAliveTimeout verifies that the HTTPKeepAliveTimeout tuning option correctly closes
+// a persistent HTTP connection after the configured duration, forcing the client to establish a new one.
+func Test_HTTPKeepAliveTimeout(t *testing.T) {
+	t.Parallel()
+
+	// Use client timeout value lesser than the http keep alive timeout
+	// to make sure we avoid https://github.com/haproxy/haproxy/issues/2334.
+	httpKeepAliveTimeoutRunTest(t, 7, 3)
+	// Use the default client timeout.
+	httpKeepAliveTimeoutRunTest(t, 7, 0)
+}
+
+// httpKeepAliveTimeoutRunTest is a helper which runs a single test workflow.
+// The test workflow:
+// - Create an ingress controller with a set HTTPKeepAliveTimeout.
+// - Send multiple requests to confirm the connection reuse.
+// - Sleep for the duration of the timeout.
+// - Send a final request and verify a new connection is established.
+func httpKeepAliveTimeoutRunTest(t *testing.T, httpKATimeoutSeconds, clientTimeoutSeconds int64) {
+	t.Helper()
+
+	const (
+		webService = "web-service"
+		// Number of requests to check the connection is reused.
+		numConnReuseAttempts = 5
+	)
+
+	createIngressController := func(t *testing.T, testNs *corev1.Namespace) (*operatorv1.IngressController, error) {
+		t.Helper()
+
+		icName := types.NamespacedName{
+			Namespace: operatorNamespace,
+			Name:      testNs.Name,
+		}
+
+		t.Logf("Creating IngressController %q...", icName)
+
+		ic := newLoadBalancerController(icName, icName.Name+"."+dnsConfig.Spec.BaseDomain)
+		ic.Spec.EndpointPublishingStrategy.LoadBalancer = &operatorv1.LoadBalancerStrategy{
+			Scope:               operatorv1.ExternalLoadBalancer,
+			DNSManagementPolicy: operatorv1.ManagedLoadBalancerDNS,
+		}
+		ic.Spec.NamespaceSelector = &metav1.LabelSelector{
+			MatchLabels: testNs.Labels,
+		}
+		ic.Spec.TuningOptions.HTTPKeepAliveTimeout = &metav1.Duration{Duration: time.Duration(httpKATimeoutSeconds) * time.Second}
+		if clientTimeoutSeconds != 0 {
+			ic.Spec.TuningOptions.ClientTimeout = &metav1.Duration{Duration: time.Duration(clientTimeoutSeconds) * time.Second}
+		}
+
+		if err := kclient.Create(context.Background(), ic); err != nil {
+			return nil, fmt.Errorf("failed to create IngressController: %w", err)
+		}
+		t.Cleanup(func() {
+			t.Logf("Deleting IngressController %q...", icName)
+			assertIngressControllerDeleted(t, kclient, ic)
+		})
+
+		if err := waitForIngressControllerCondition(t, kclient, 5*time.Minute, icName, availableConditionsForIngressControllerWithLoadBalancer...); err != nil {
+			return nil, fmt.Errorf("failed to observe expected conditions: %w", err)
+		}
+
+		return ic, nil
+	}
+
+	createTestServicesAndTestRoute := func(ns *corev1.Namespace, ic *operatorv1.IngressController) (string, error) {
+		canaryImage, err := canaryImageReference(t)
+		if err != nil {
+			return "", fmt.Errorf("failed to get canary image reference: %w", err)
+		}
+
+		t.Logf("Creating test workload in namespace %q...", ns.Name)
+
+		if err := idleConnectionCreateBackendService(context.Background(), t, ns.Name, webService, canaryImage); err != nil {
+			return "", fmt.Errorf("failed to create service %s: %w", webService, err)
+		}
+
+		routeName := types.NamespacedName{Namespace: ns.Name, Name: webService}
+		route := buildRoute(routeName.Name, routeName.Namespace, webService)
+		if err := kclient.Create(context.Background(), route); err != nil {
+			return "", fmt.Errorf("failed to create route %s: %w", routeName, err)
+		}
+
+		if err := waitForRouteIngressConditions(t, kclient, routeName, ic.Name, routev1.RouteIngressCondition{
+			Type:   routev1.RouteAdmitted,
+			Status: corev1.ConditionTrue,
+		}); err != nil {
+			return "", fmt.Errorf("failed to observe route admitted condition: %w", err)
+		}
+
+		if err := kclient.Get(context.Background(), routeName, route); err != nil {
+			return "", fmt.Errorf("failed to get route %s: %w", routeName, err)
+		}
+
+		routeHost := getRouteHost(route, ic.Name)
+		if routeHost == "" {
+			return "", fmt.Errorf("route %s has no host assigned by IngressController %s", routeName, ic.Name)
+		}
+
+		return routeHost, nil
+	}
+
+	testName := names.SimpleNameGenerator.GenerateName("http-keep-alive-")
+	testNs := createNamespace(t, testName)
+
+	ic, err := createIngressController(t, testNs)
+	if err != nil {
+		t.Fatalf("Failed to create IngressController %s: %v", testName, err)
+	}
+
+	routeHost, err := createTestServicesAndTestRoute(testNs, ic)
+	if err != nil {
+		t.Fatalf("Test setup failed: %v", err)
+	}
+
+	elbAddress, err := resolveIngressControllerAddress(t, ic)
+	if err != nil {
+		t.Fatalf("Test setup failed: %v", err)
+	}
+
+	// Create an HTTP client that reuses connections by default.
+	httpClient := &http.Client{
+		Timeout: time.Minute, // full request roundtrip (client-server-client)
+		Transport: &addressCapturingRoundTripper{
+			BaseTransport: &http.Transport{
+				// Set client's idle timeout high to prevent it from closing the connection.
+				IdleConnTimeout: 600 * time.Second,
+				// Set connection pool sizes high to ensure the connection reuse.
+				MaxConnsPerHost:     100, // active + idle per URL host
+				MaxIdleConns:        100, // all idle
+				MaxIdleConnsPerHost: 100, // idle per host
+			},
+		},
+	}
+	t.Logf("Checking connectivity to test route...")
+	if err := checkRouteConnectivity(t, httpClient, elbAddress, routeHost); err != nil {
+		t.Fatalf("Failed to check the connectivity to route %q: %v", routeHost, err)
+	}
+
+	t.Logf("Checking connection reuse by sending %d requests...", numConnReuseAttempts)
+	// Send N requests to make sure the connection keep alive is working.
+	prevLocalAddr := ""
+	for i := 1; i < (numConnReuseAttempts + 1); i++ {
+		response, localAddr, err := idleConnectionFetchResponse(t, httpClient, elbAddress, routeHost)
+		if err != nil {
+			t.Errorf("Request %d failed: %v", i, err)
+		}
+		if !strings.HasPrefix(response, webService) {
+			t.Errorf("Request %d failed: wrong response: %s", i, response)
+		}
+		if prevLocalAddr == "" {
+			prevLocalAddr = localAddr
+		} else if prevLocalAddr != localAddr {
+			t.Errorf("Request %d failed: unexpected new connection", i)
+		}
+	}
+
+	// Don't send requests for longer than HTTPKeepAliveTimeout.
+	sleepDuration := time.Duration(httpKATimeoutSeconds+1) * time.Second
+	t.Logf("Going to sleep for %v...", sleepDuration)
+	time.Sleep(sleepDuration)
+
+	// Send the last request to see that a new connection was created by the client.
+	// Old connection is supposed to be reset by the router due to expired http-keep-alive.
+	t.Logf("Checking connection reset...")
+	response, localAddr, err := idleConnectionFetchResponse(t, httpClient, elbAddress, routeHost)
+	if err != nil {
+		t.Errorf("Last request failed: %v", err)
+	}
+	if !strings.HasPrefix(response, webService) {
+		t.Errorf("Last request failed: wrong response: %s", response)
+	}
+	if prevLocalAddr == localAddr {
+		t.Errorf("Last request failed: unexpected reused connection after keep alive timeout expired")
+	}
+}