Merge pull request #670 from frobware/4.8-manual-backport-pr663

[release-4.8] Bug 2017708: Change default balancing algorithm to "leastconn"

pkg/operator/controller/ingress/deployment.go

@@ -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,

pkg/operator/controller/ingress/deployment_test.go

@@ -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")

test/e2e/operator_test.go

@@ -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)
 	}
 }