From c31f498570a98a35d936c705fea94c9dfc9ecfb5 Mon Sep 17 00:00:00 2001
From: Miciah Masters <miciah.masters@gmail.com>
Date: Mon, 4 Nov 2019 16:37:05 -0500
Subject: [PATCH] Add migration for affinity and deployment strategy

When updating a router deployment, check whether the deployment strategy is
changing; if it is, update the affinity policy first, and only update the
deployment strategy once all pods have the new affinity policy.

This two-step migration is necessary because an old affinity policy may
have an anti-affinity rule that prohibits colocation of pods belonging to
the same ingresscontroller, the new deployment strategy may require surge,
and the presence of pods with the old affinity policy would block a rolling
update using the new deployment strategy.

Follow-up to commit 0f6fd1c67dd719508c33e475ebda3f6e47052501.

This commit fixes bug 1766851.

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

* pkg/operator/controller/ingress/deployment.go (updateRouterDeployment):
If the deployment strategy is changing, make sure to update the affinity
policy first.
---
 pkg/operator/controller/ingress/deployment.go | 39 +++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/pkg/operator/controller/ingress/deployment.go b/pkg/operator/controller/ingress/deployment.go
index 13a318341b..915f2d9972 100644
--- a/pkg/operator/controller/ingress/deployment.go
+++ b/pkg/operator/controller/ingress/deployment.go
@@ -6,6 +6,7 @@ import (
 	"hash"
 	"hash/fnv"
 	"path/filepath"
+	"reflect"
 	"sort"
 	"strings"
 
@@ -646,6 +647,44 @@ func (r *reconciler) updateRouterDeployment(current, desired *appsv1.Deployment)
 		return nil
 	}
 
+	// It may be impossible to transition from the current state to the
+	// desired state if the current state has an affinity policy with a pod
+	// anti-affinity rule that prohibits colocation of pods belonging to the
+	// same ingresscontroller and the desired state requires using surge for
+	// rolling updates.  Thus, if the current deployment has an old affinity
+	// policy, we update the deployment in two steps:
+	//
+	// 1. Update the affinity policy (which should enable colocation of pods
+	// with different deployment hashes).
+	//
+	// 2. Once that update is rolled out, update the deployment strategy to
+	// use surge.
+	//
+	// TODO: This migration was introduced in OpenShift 4.3 and can be
+	// removed in OpenShift 4.4.
+	switch {
+	case reflect.DeepEqual(current.Spec.Strategy, updated.Spec.Strategy):
+		// Migration is complete (or was never needed), so continue with
+		// the update as usual.
+	case reflect.DeepEqual(current.Spec.Template.Spec.Affinity, updated.Spec.Template.Spec.Affinity):
+		// Migration has been started but not completed.
+		if current.Status.UpdatedReplicas != current.Status.Replicas {
+			// We cannot continue to the next step till the rollout
+			// is complete, so log and return.
+			log.Info("waiting for rollout of new affinity policy to complete", "namespace", current.Namespace, "name", current.Name)
+			return nil
+		}
+
+		// Update of the affinity policy is complete; now update the
+		// deployment strategy by continuing with the update as normal.
+		log.Info("migrating router deployment to new deployment strategy", "namespace", current.Namespace, "name", current.Name)
+	default:
+		// Initiate migration by updating affinity but keeping the old
+		// strategy.
+		updated.Spec.Strategy = current.Spec.Strategy
+		log.Info("migrating router deployment to new affinity policy", "namespace", current.Namespace, "name", current.Name)
+	}
+
 	if err := r.client.Update(context.TODO(), updated); err != nil {
 		return fmt.Errorf("failed to update router deployment %s/%s: %v", updated.Namespace, updated.Name, err)
 	}