Merge pull request openshift#185 from alebedev87/recreate-strategy

OCPBUGS-3630: deployment: enforce recreate strategy to avoid conflicts during upgrades

hack/golangci-lint.sh

@@ -1,7 +1,7 @@
 #!/bin/sh
 set -e
 
-GOLANGCI_VERSION="1.50.0"
+GOLANGCI_VERSION="1.51.2"
 
 OUTPUT_PATH=${1:-./bin/golangci-lint}
 
@@ -10,10 +10,10 @@ GOARCH=$(go env GOARCH)
 
 case $GOOS in
   linux)
-    CHECKSUM="b4b329efcd913082c87d0e9606711ecb57415b5e6ddf233fde9e76c69d9b4e8b"
+    CHECKSUM="4de479eb9d9bc29da51aec1834e7c255b333723d38dbd56781c68e5dddc6a90b"
     ;;
   darwin)
-    CHECKSUM="7ab306b91b0f2bb741cc0a4c86f29f69506eb7b505f47e91b0e74365e4c28c4e"
+    CHECKSUM="0549cbaa2df451cf3a2011a9d73a9cb127784d26749d9cd14c9f4818af104d44"
     ;;
     *)
     echo "Unsupported OS $GOOS"

pkg/operator/controller/externaldns/deployment.go

@@ -231,6 +231,9 @@ func desiredExternalDNSDeployment(cfg *deploymentConfig) (*appsv1.Deployment, er
 			Selector: &metav1.LabelSelector{
 				MatchLabels: matchLbl,
 			},
+			Strategy: appsv1.DeploymentStrategy{
+				Type: appsv1.RecreateDeploymentStrategyType,
+			},
 			Template: corev1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      matchLbl,
@@ -332,11 +335,37 @@ func (r *reconciler) updateExternalDNSDeployment(ctx context.Context, current, d
 // externalDNSDeploymentChanged evaluates whether or not a deployment update is necessary.
 // Returns a boolean if an update is necessary, and the deployment resource to update to.
 func externalDNSDeploymentChanged(current, expected *appsv1.Deployment) (bool, *appsv1.Deployment) {
+	changed := false
 	updated := current.DeepCopy()
-	annotationChanged := externalDNSAnnotationsChanged(current, expected, updated)
-	containersChanged := externalDNSContainersChanged(current, expected, updated)
-	volumesChanged := externalDNSVolumesChanged(current, expected, updated)
-	return annotationChanged || containersChanged || volumesChanged, updated
+
+	if expected.Spec.Replicas != nil {
+		expReplicas := *expected.Spec.Replicas
+		if current.Spec.Replicas == nil {
+			updated.Spec.Replicas = &expReplicas
+			changed = true
+		} else if *expected.Spec.Replicas != *current.Spec.Replicas {
+			updated.Spec.Replicas = &expReplicas
+			changed = true
+		}
+	}
+
+	if externalDNSDeploymentStrategyChanged(current, expected, updated) {
+		changed = true
+	}
+
+	if externalDNSAnnotationsChanged(current, expected, updated) {
+		changed = true
+	}
+
+	if externalDNSContainersChanged(current, expected, updated) {
+		changed = true
+	}
+
+	if externalDNSVolumesChanged(current, expected, updated) {
+		changed = true
+	}
+
+	return changed, updated
 }
 
 // externalDNSAnnotationsChanged returns true if any annotation from the podspec differs from the expected.```
@@ -448,6 +477,28 @@ func externalDNSVolumesChanged(current, expected, updated *appsv1.Deployment) bo
 	return changed
 }
 
+// externalDNSDeploymentStrategyChanged returns true if the current update strategy differs from expected.
+func externalDNSDeploymentStrategyChanged(current, expected, updated *appsv1.Deployment) bool {
+	changed := false
+
+	// reset the strategy type if it differs
+	if expected.Spec.Strategy.Type != "" &&
+		expected.Spec.Strategy.Type != current.Spec.Strategy.Type {
+		updated.Spec.Strategy.Type = expected.Spec.Strategy.Type
+		changed = true
+	}
+
+	// if the expected strategy is Recreate: make sure no rolling update params are present
+	if expected.Spec.Strategy.Type == appsv1.RecreateDeploymentStrategyType {
+		if current.Spec.Strategy.RollingUpdate != nil {
+			updated.Spec.Strategy.RollingUpdate = nil
+			changed = true
+		}
+	}
+
+	return changed
+}
+
 // volumeMountsChanged checks that the current volume mounts have all expected ones,
 // returns true if the current volume mounts had to be changed to match the expected.
 func volumeMountsChanged(current, expected, updated []corev1.VolumeMount) (bool, []corev1.VolumeMount) {