Merge pull request #1134 from sjenning/set-deploy-strat-single-replica

OCPBUGS-38871: ingress: deployment: explicitly set DeploymentStrategy in SingleReplica case
openshift · Aug 29, 2024 · 8252ac4 · 8252ac4
2 parents aea6fec + 0eb116c
commit 8252ac4
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/pkg/operator/controller/ingress/deployment.go b/pkg/operator/controller/ingress/deployment.go
@@ -309,7 +309,16 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
 		// because we need the rule on multi-node clusters, even if
 		// desiredReplicas is 1, so that a rolling update schedules
 		// the new pod to the same node as the old pod.
+		pointerTo := func(ios intstr.IntOrString) *intstr.IntOrString { return &ios }
 		if singleReplica(ingressConfig, infraConfig) {
+			// We must explicitly set deployment strategy to the kube defaults to avoid a reconcile loop
+			deployment.Spec.Strategy = appsv1.DeploymentStrategy{
+				Type: appsv1.RollingUpdateDeploymentStrategyType,
+				RollingUpdate: &appsv1.RollingUpdateDeployment{
+					MaxUnavailable: pointerTo(intstr.FromString("25%")),
+					MaxSurge:       pointerTo(intstr.FromString("25%")),
+				},
+			}
 			break
 		}
 
@@ -327,7 +336,6 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
 		if desiredReplicas >= 4 {
 			maxUnavailable = "25%"
 		}
-		pointerTo := func(ios intstr.IntOrString) *intstr.IntOrString { return &ios }
 		deployment.Spec.Strategy = appsv1.DeploymentStrategy{
 			Type: appsv1.RollingUpdateDeploymentStrategyType,
 			RollingUpdate: &appsv1.RollingUpdateDeployment{

diff --git a/pkg/operator/controller/ingress/deployment_test.go b/pkg/operator/controller/ingress/deployment_test.go
@@ -1034,8 +1034,13 @@ func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
 		t.Errorf("expected no pod affinity, got %+v", *deployment.Spec.Template.Spec.Affinity)
 	}
 
-	if deployment.Spec.Strategy.Type != "" || deployment.Spec.Strategy.RollingUpdate != nil {
-		t.Errorf("expected default deployment strategy, got %s", deployment.Spec.Strategy.Type)
+	if deployment.Spec.Strategy.Type != appsv1.RollingUpdateDeploymentStrategyType ||
+		deployment.Spec.Strategy.RollingUpdate == nil ||
+		deployment.Spec.Strategy.RollingUpdate.MaxUnavailable == nil ||
+		deployment.Spec.Strategy.RollingUpdate.MaxSurge == nil ||
+		*deployment.Spec.Strategy.RollingUpdate.MaxUnavailable != intstr.FromString("25%") ||
+		*deployment.Spec.Strategy.RollingUpdate.MaxSurge != intstr.FromString("25%") {
+		t.Errorf("expected default deployment strategy, got %+v", deployment.Spec.Strategy)
 	}
 }