Skip to content

Commit

Permalink
Explicitly set DeploymentStrategy for SingleReplica
Browse files Browse the repository at this point in the history
Currently, the code does not set deployment.Spec.Strategy before
breaking out in the SingleReplica case. This results in the
kube-apiserver defaulting the Strategy. On the next reconcile pass, this
code will attempt to unset the defaulted values leading to a hot
reconciliation loop.

To avoid this, explicitly set the Strategy to the kube default before
the break out of the switch.

Fixes https://issues.redhat.com/browse/OCPBUGS-38871
  • Loading branch information
sjenning committed Aug 29, 2024
1 parent 26f0181 commit 0eb116c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 3 deletions.
10 changes: 9 additions & 1 deletion pkg/operator/controller/ingress/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand All @@ -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{
Expand Down
9 changes: 7 additions & 2 deletions pkg/operator/controller/ingress/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down

0 comments on commit 0eb116c

Please sign in to comment.