Commit b1a50405 authored by Miciah Masters's avatar Miciah Masters Committed by Andrew McDermott
Browse files

Change default balancing algorithm to "leastconn"

Configure OpenShift router to use the "leastconn" balancing algorithm for
non-passthrough routes by default.

This was the default algorithm for non-passthrough routes before it was
changed to "random" in OpenShift 4.8.  The "random" algorithm is expected
to provide better behavior across reloads or multiple router pod replicas.
However, it incurs significant memory overhead for each backend that uses
it, and so we need to change the default back to "leastconn" in order to
avoid excessive memory usage until we can fix the issue in HAProxy.

This commit fixes bug 2007581.

https://bugzilla.redhat.com/show_bug.cgi?id=2007581

* pkg/operator/controller/ingress/deployment.go (desiredRouterDeployment):
Change the default balancing algorithm for non-passthrough routes to
"leastconn", and allow an override to set "random".
* pkg/operator/controller/ingress/deployment_test.go
(TestDesiredRouterDeployment): Update to expect "leastconn".
* test/e2e/operator_test.go
(TestLoadBalancingAlgorithmUnsupportedConfigOverride): Update to expect
"leastconn" as the default setting and verify that the override allows
setting "random".  Add a check that passthrough routes use "source".
parent 3257f07e
......@@ -418,14 +418,19 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
}
}
// For non-TLS, edge-terminated, and reencrypt routes, use the "random"
// balancing algorithm by default, but allow an unsupported config
// override to override it. For passthrough routes, use the "source"
// balancing algorithm in order to provide some session-affinity.
loadBalancingAlgorithm := "random"
// For non-TLS, edge-terminated, and reencrypt routes, use the
// "leastconn" balancing algorithm by default, but allow an unsupported
// config override to override it. For passthrough routes, use the
// "source" balancing algorithm in order to provide some
// session-affinity. This code at one time used "random" as the default
// for non-passthrough routes, but we changed it to "leastconn" upon
// discovering that "random" incurs significant memory overhead for each
// backend that uses it. See
// <https://bugzilla.redhat.com/show_bug.cgi?id=2007581>.
loadBalancingAlgorithm := "leastconn"
switch unsupportedConfigOverrides.LoadBalancingAlgorithm {
case "leastconn":
loadBalancingAlgorithm = "leastconn"
case "random":
loadBalancingAlgorithm = "random"
}
env = append(env, corev1.EnvVar{
Name: RouterLoadBalancingAlgorithmEnvName,
......
......@@ -218,7 +218,7 @@ func TestDesiredRouterDeployment(t *testing.T) {
deployment.Spec.Template.Spec.Tolerations)
}
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "random")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "leastconn")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_TCP_BALANCE_SCHEME", true, "source")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_USE_PROXY_PROTOCOL", false, "")
......@@ -348,7 +348,7 @@ func TestDesiredRouterDeployment(t *testing.T) {
var expectedReplicas int32 = 8
ci.Spec.Replicas = &expectedReplicas
ci.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
Raw: []byte(`{"loadBalancingAlgorithm":"leastconn"}`),
Raw: []byte(`{"loadBalancingAlgorithm":"random","dynamicConfigManager":"false","maxConnections":-1,"reloadInterval":15}`),
}
ci.Status.Domain = "example.com"
ci.Status.EndpointPublishingStrategy.Type = operatorv1.LoadBalancerServiceStrategyType
......@@ -378,7 +378,7 @@ func TestDesiredRouterDeployment(t *testing.T) {
t.Errorf("expected empty startup probe host, got %q", deployment.Spec.Template.Spec.Containers[0].StartupProbe.Handler.HTTPGet.Host)
}
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "leastconn")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "random")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_TCP_BALANCE_SCHEME", true, "source")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_USE_PROXY_PROTOCOL", true, "true")
......@@ -411,7 +411,7 @@ func TestDesiredRouterDeployment(t *testing.T) {
checkDeploymentHasEnvVar(t, deployment, "ROUTER_IP_V4_V6_MODE", true, "v4v6")
// Any value for loadBalancingAlgorithm other than "leastconn" should be
// Any value for loadBalancingAlgorithm other than "random" should be
// ignored.
ci.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
Raw: []byte(`{"loadBalancingAlgorithm":"source"}`),
......@@ -448,7 +448,7 @@ func TestDesiredRouterDeployment(t *testing.T) {
t.Errorf("expected empty startup probe host, got %q", deployment.Spec.Template.Spec.Containers[0].StartupProbe.Handler.HTTPGet.Host)
}
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "random")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_LOAD_BALANCE_ALGORITHM", true, "leastconn")
checkDeploymentHasEnvVar(t, deployment, "ROUTER_TCP_BALANCE_SCHEME", true, "source")
checkDeploymentDoesNotHaveEnvVar(t, deployment, "ROUTER_USE_PROXY_PROTOCOL")
......
......@@ -1938,9 +1938,11 @@ func TestUniqueIdHeader(t *testing.T) {
}
// TestLoadBalancingAlgorithmUnsupportedConfigOverride verifies that the
// operator configures router pod replicas to use the "leastconn" load-balancing
// algorithm if the ingresscontroller is so configured using an unsupported
// config override.
// operator configures router pod replicas to use the "random" load-balancing
// algorithm for non-passthrough routes if the ingresscontroller is so
// configured using an unsupported config override. The test also verifies that
// the operator always configures router pod replicas to use the "source"
// algorithm for passthrough routes irrespective of the override.
func TestLoadBalancingAlgorithmUnsupportedConfigOverride(t *testing.T) {
icName := types.NamespacedName{Namespace: operatorNamespace, Name: "leastconn"}
domain := icName.Name + "." + dnsConfig.Spec.BaseDomain
......@@ -1961,20 +1963,26 @@ func TestLoadBalancingAlgorithmUnsupportedConfigOverride(t *testing.T) {
if err := kclient.Get(context.TODO(), controller.RouterDeploymentName(ic), deployment); err != nil {
t.Fatalf("failed to get ingresscontroller deployment: %v", err)
}
expectedAlgorithm := "random"
expectedAlgorithm := "leastconn"
if err := waitForDeploymentEnvVar(t, kclient, deployment, 30*time.Second, "ROUTER_LOAD_BALANCE_ALGORITHM", expectedAlgorithm); err != nil {
t.Fatalf("expected initial deployment to use the %q algorithm: %v", expectedAlgorithm, err)
t.Fatalf("expected initial deployment to have ROUTER_LOAD_BALANCE_ALGORITHM=%s: %v", expectedAlgorithm, err)
}
if err := waitForDeploymentEnvVar(t, kclient, deployment, 30*time.Second, "ROUTER_TCP_BALANCE_SCHEME", "source"); err != nil {
t.Fatalf("expected initial deployment to have ROUTER_TCP_BALANCE_SCHEME=source: %v", err)
}
ic.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
Raw: []byte(`{"loadBalancingAlgorithm":"leastconn"}`),
Raw: []byte(`{"loadBalancingAlgorithm":"random"}`),
}
if err := kclient.Update(context.TODO(), ic); err != nil {
t.Fatalf("failed to update ingresscontroller: %v", err)
}
expectedAlgorithm = "leastconn"
expectedAlgorithm = "random"
if err := waitForDeploymentEnvVar(t, kclient, deployment, 1*time.Minute, "ROUTER_LOAD_BALANCE_ALGORITHM", expectedAlgorithm); err != nil {
t.Fatalf("expected updated deployment to use the %q algorithm: %v", expectedAlgorithm, err)
t.Fatalf("expected updated deployment to have ROUTER_LOAD_BALANCE_ALGORITHM=%s: %v", expectedAlgorithm, err)
}
if err := waitForDeploymentEnvVar(t, kclient, deployment, 30*time.Second, "ROUTER_TCP_BALANCE_SCHEME", "source"); err != nil {
t.Fatalf("expected updated deployment to have ROUTER_TCP_BALANCE_SCHEME=source: %v", err)
}
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment