controller: decouple cleanup policy from deployment strategies

Deployments get cleaned up only when they are paused, they get scaled up/down,
or when the strategy that drives rollouts completes. This means that stuck
deployments that fall into none of the above categories will not get cleaned
up. Since cleanup is already safe by itself (we only delete old replica sets
that are synced by the replica set controller and have no replicas) we can
execute it for every deployment when there is no intention to rollback.

pkg/controller/deployment/deployment_controller.go

@@ -543,7 +543,26 @@ func (dc *DeploymentController) syncDeployment(key string) error {
 		return dc.syncStatusOnly(d)
 	}
 
-	err = dc.classifyReplicaSets(deployment)
+	// Why run the cleanup policy only when there is no rollback request?
+	// The thing with the cleanup policy currently is that it is far from smart because it takes into account
+	// the latest replica sets while it should instead retain the latest *working* replica sets. This means that
+	// you can have a cleanup policy of 1 but your last known working replica set may be 2 or 3 versions back
+	// in the history.
+	// Eventually we will want to find a way to recognize replica sets that have worked at some point in time
+	// (and chances are higher that they will work again as opposed to others that didn't) for candidates to
+	// automatically roll back to (#23211) and the cleanup policy should help.
+	if d.Spec.RollbackTo == nil {
+		_, oldRSs, err := dc.getAllReplicaSetsAndSyncRevision(d, false)
+		if err != nil {
+			return err
+		}
+		// So far the cleanup policy was executed once a deployment was paused, scaled up/down, or it
+		// succesfully completed deploying a replica set. Decouple it from the strategies and have it
+		// run almost unconditionally - cleanupDeployment is safe by default.
+		dc.cleanupDeployment(oldRSs, d)
+	}
+
+	err = dc.classifyReplicaSets(d)
 	if err != nil {
 		return err
 	}

pkg/controller/deployment/recreate.go

@@ -73,8 +73,6 @@ func (dc *DeploymentController) rolloutRecreate(deployment *extensions.Deploymen
 		return dc.syncRolloutStatus(allRSs, newRS, deployment)
 	}
 
-	dc.cleanupDeployment(oldRSs, deployment)
-
 	// Sync deployment status
 	return dc.syncRolloutStatus(allRSs, newRS, deployment)
 }

pkg/controller/deployment/rolling.go

@@ -55,8 +55,6 @@ func (dc *DeploymentController) rolloutRolling(deployment *extensions.Deployment
 		return dc.syncRolloutStatus(allRSs, newRS, deployment)
 	}
 
-	dc.cleanupDeployment(oldRSs, deployment)
-
 	// Sync deployment status
 	return dc.syncRolloutStatus(allRSs, newRS, deployment)
 }

pkg/controller/deployment/sync.go

@@ -58,7 +58,6 @@ func (dc *DeploymentController) sync(deployment *extensions.Deployment) error {
 		// so we can abort this resync
 		return err
 	}
-	dc.cleanupDeployment(oldRSs, deployment)
 
 	allRSs := append(oldRSs, newRS)
 	return dc.syncDeploymentStatus(allRSs, newRS, deployment)