Fix honoring autoPromotionSeconds (#360)

rollout/analysis.go

@@ -114,7 +114,7 @@ func (c *RolloutController) reconcileBackgroundAnalysisRun(roCtx *canaryContext)
 		return nil, nil
 	}
 
-	if roCtx.PauseContext().GetPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
+	if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
 		return currentAr, nil
 	}
 
@@ -160,7 +160,7 @@ func (c *RolloutController) reconcileStepBasedAnalysisRun(roCtx *canaryContext)
 	step, index := replicasetutil.GetCurrentCanaryStep(rollout)
 	currentAr := analysisutil.FilterAnalysisRunsByName(currentArs, rollout.Status.Canary.CurrentStepAnalysisRun)
 
-	if roCtx.PauseContext().GetPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
+	if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
 		return currentAr, nil
 	}
 

rollout/bluegreen.go

@@ -154,7 +154,7 @@ func (c *RolloutController) reconcileBlueGreenPause(activeSvc, previewSvc *corev
 		return false
 	}
 
-	cond := roCtx.PauseContext().GetPauseCondition(v1alpha1.PauseReasonBlueGreenPause)
+	cond := getPauseCondition(rollout, v1alpha1.PauseReasonBlueGreenPause)
 	// If the rollout is not paused and the active service is not point at the newRS, we should pause the rollout.
 	if cond == nil && !rollout.Status.ControllerPause && !rollout.Status.BlueGreen.ScaleUpPreviewCheckPoint && activeSvc.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey] != newRSPodHash {
 		roCtx.PauseContext().AddPauseCondition(v1alpha1.PauseReasonBlueGreenPause)
@@ -168,6 +168,7 @@ func (c *RolloutController) reconcileBlueGreenPause(activeSvc, previewSvc *corev
 		now := metav1.Now()
 		if now.After(switchDeadline) {
 			roCtx.PauseContext().RemovePauseCondition(v1alpha1.PauseReasonBlueGreenPause)
+			return false
 		}
 
 	}

rollout/bluegreen_test.go

@@ -326,11 +326,17 @@ func TestBlueGreenHandlePause(t *testing.T) {
 
 		expectedPatchWithoutSubs := `{
 			"status": {
+				"blueGreen": {
+					"activeSelector": "%s"
+				},
 				"pauseConditions": null,
-				"controllerPause": null
+				"controllerPause": null,
+				"selector": "foo=bar,rollouts-pod-template-hash=%s"
 			}
 		}`
-		expectedPatch := calculatePatch(r2, expectedPatchWithoutSubs)
+		expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, rs2PodHash))
+		f.expectPatchServiceAction(activeSvc, rs2PodHash)
+		f.expectPatchReplicaSetAction(rs1)
 		patchRolloutIndex := f.expectPatchRolloutActionWithPatch(r2, expectedPatch)
 		f.run(getKey(r2, t))
 

rollout/canary.go

@@ -132,7 +132,7 @@ func (c *RolloutController) reconcileCanaryPause(roCtx *canaryContext) bool {
 	if currentStep.Pause == nil {
 		return false
 	}
-	cond := roCtx.PauseContext().GetPauseCondition(v1alpha1.PauseReasonCanaryPauseStep)
+	cond := getPauseCondition(rollout, v1alpha1.PauseReasonCanaryPauseStep)
 	if cond == nil {
 		// When the pause condition is null, that means the rollout is in an not paused state.
 		// As a result,, the controller needs to detect whether a rollout was unpaused or the

rollout/controller.go

@@ -284,7 +284,7 @@ func (c *RolloutController) syncHandler(key string) error {
 		return err
 	}
 
-	if len(rollout.Status.PauseConditions) > 0 || r.Spec.Paused || isScalingEvent {
+	if getPauseCondition(r, v1alpha1.PauseReasonInconclusiveAnalysis) != nil || r.Spec.Paused || isScalingEvent {
 		return c.syncReplicasOnly(r, rsList, isScalingEvent)
 	}
 

rollout/experiment.go

@@ -144,7 +144,7 @@ func (c *RolloutController) reconcileExperiments(roCtx *canaryContext) error {
 		return c.cancelExperiments(roCtx, allExs)
 	}
 
-	if roCtx.PauseContext().GetPauseCondition(v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
+	if getPauseCondition(rollout, v1alpha1.PauseReasonInconclusiveAnalysis) != nil {
 		return nil
 	}
 

rollout/pause.go

@@ -105,9 +105,9 @@ func (pCtx *pauseContext) CalculatePauseStatus(newStatus *v1alpha1.RolloutStatus
 	newStatus.PauseConditions = newPauseConditions
 }
 
-func (pCtx *pauseContext) GetPauseCondition(reason v1alpha1.PauseReason) *v1alpha1.PauseCondition {
-	for i := range pCtx.rollout.Status.PauseConditions {
-		cond := pCtx.rollout.Status.PauseConditions[i]
+func getPauseCondition(rollout *v1alpha1.Rollout, reason v1alpha1.PauseReason) *v1alpha1.PauseCondition {
+	for i := range rollout.Status.PauseConditions {
+		cond := rollout.Status.PauseConditions[i]
 		if cond.Reason == reason {
 			return &cond
 		}
@@ -117,7 +117,7 @@ func (pCtx *pauseContext) GetPauseCondition(reason v1alpha1.PauseReason) *v1alph
 
 func (pCtx *pauseContext) CompletedPauseStep(pause v1alpha1.RolloutPause) bool {
 	rollout := pCtx.rollout
-	pauseCondition := pCtx.GetPauseCondition(v1alpha1.PauseReasonCanaryPauseStep)
+	pauseCondition := getPauseCondition(rollout, v1alpha1.PauseReasonCanaryPauseStep)
 
 	if pause.Duration != nil {
 		now := metav1.Now()

rollout/sync.go

@@ -210,8 +210,7 @@ func (c *RolloutController) getNewReplicaSet(rollout *v1alpha1.Rollout, rsList,
 // syncReplicasOnly is responsible for reconciling rollouts on scaling events.
 func (c *RolloutController) syncReplicasOnly(r *v1alpha1.Rollout, rsList []*appsv1.ReplicaSet, isScaling bool) error {
 	logCtx := logutil.WithRollout(r)
-	isControllerPaused := len(r.Status.PauseConditions) > 0
-	logCtx.Infof("Syncing replicas only (controllerPaused: %v, userPaused %v, isScaling: %v)", isControllerPaused, r.Spec.Paused, isScaling)
+	logCtx.Infof("Syncing replicas only (userPaused %v, isScaling: %v)", r.Spec.Paused, isScaling)
 	newRS, oldRSs, err := c.getAllReplicaSetsAndSyncRevision(r, rsList, false)
 	if err != nil {
 		return err
@@ -228,7 +227,6 @@ func (c *RolloutController) syncReplicasOnly(r *v1alpha1.Rollout, rsList []*apps
 			// so we can abort this resync
 			return err
 		}
-		c.reconcileBlueGreenPause(previewSvc, activeSvc, roCtx)
 		return c.syncRolloutStatusBlueGreen(previewSvc, activeSvc, roCtx)
 	}
 	// The controller wants to use the rolloutCanary method to reconcile the rolllout if the rollout is not paused.