fix: deprecate skip-current-step command (#1156)

Signed-off-by: hari rongali <[email protected]>

pkg/kubectl-argo-rollouts/cmd/promote/promote.go

@@ -21,10 +21,7 @@ const (
 	%[1]s promote guestbook
 
 	# Fully promote a rollout to desired version, skipping analysis, pauses, and steps
-	%[1]s promote guestbook --full
-	
-	# Skip currently running canary step of a rollout
-	%[1]s promote guestbook --skip-current-step`
+	%[1]s promote guestbook --full`
 
 	promoteUsage = `Promote a rollout
 
@@ -33,11 +30,13 @@ To skip analysis, pauses and steps entirely, use '--full' to fully promote the r
 )
 
 const (
-	setCurrentStepIndex                 = `{"status":{"currentStepIndex":%d}}`
-	unpausePatch                        = `{"spec":{"paused":false}}`
-	clearPauseConditionsPatch           = `{"status":{"pauseConditions":null}}`
-	unpauseAndClearPauseConditionsPatch = `{"spec":{"paused":false},"status":{"pauseConditions":null}}`
-	promoteFullPatch                    = `{"status":{"promoteFull":true}}`
+	setCurrentStepIndex                         = `{"status":{"currentStepIndex":%d}}`
+	unpausePatch                                = `{"spec":{"paused":false}}`
+	clearPauseConditionsPatch                   = `{"status":{"pauseConditions":null}}`
+	unpauseAndClearPauseConditionsPatch         = `{"spec":{"paused":false},"status":{"pauseConditions":null}}`
+	promoteFullPatch                            = `{"status":{"promoteFull":true}}`
+	clearPauseConditionsPatchWithStep           = `{"status":{"pauseConditions":null, "currentStepIndex":%d}}`
+	unpauseAndClearPauseConditionsPatchWithStep = `{"spec":{"paused":false},"status":{"pauseConditions":null, "currentStepIndex":%d}}`
 
 	useBothSkipFlagsError         = "Cannot use skip-current-step and skip-all-steps flags at the same time"
 	skipFlagsWithBlueGreenError   = "Cannot skip steps of a bluegreen rollout. Run without a flags"
@@ -81,6 +80,8 @@ func NewCmdPromote(o *options.ArgoRolloutsOptions) *cobra.Command {
 	}
 	cmd.Flags().BoolVarP(&skipCurrentStep, "skip-current-step", "c", false, "Skip currently running canary step")
 	cmd.Flags().BoolVarP(&skipAllSteps, "skip-all-steps", "a", false, "Skip remaining steps")
+	cmd.Flags().MarkDeprecated("skip-current-step", "use without the flag instead ex: promote ROLLOUT_NAME")
+	cmd.Flags().MarkShorthandDeprecated("c", "use without the flag instead ex: promote ROLLOUT_NAME")
 	cmd.Flags().MarkDeprecated("skip-all-steps", "use --full instead")
 	cmd.Flags().MarkShorthandDeprecated("a", "use --full instead")
 	cmd.Flags().BoolVar(&full, "full", false, "Perform a full promotion, skipping analysis, pauses, and steps")
@@ -157,6 +158,17 @@ func getPatches(rollout *v1alpha1.Rollout, skipCurrentStep, skipAllStep, full bo
 			statusPatch = []byte(clearPauseConditionsPatch)
 		}
 		unifiedPatch = []byte(unpauseAndClearPauseConditionsPatch)
+
+		if rollout.Spec.Strategy.Canary != nil && !rollout.Status.ControllerPause {
+			_, index := replicasetutil.GetCurrentCanaryStep(rollout)
+			// At this point, the controller knows that the rollout is a canary with steps and GetCurrentCanaryStep returns 0 if
+			// the index is not set in the rollout
+			if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
+				*index++
+			}
+			statusPatch = []byte(fmt.Sprintf(clearPauseConditionsPatchWithStep, *index))
+			unifiedPatch = []byte(fmt.Sprintf(unpauseAndClearPauseConditionsPatchWithStep, *index))
+		}
 	}
 	return specPatch, statusPatch, unifiedPatch
 }

pkg/kubectl-argo-rollouts/cmd/promote/promote_test.go

@@ -236,7 +236,7 @@ func TestPromoteCmdSuccessDoNotGoPastLastStep(t *testing.T) {
 
 	cmd := NewCmdPromote(o)
 	cmd.PersistentPreRunE = o.PersistentPreRunE
-	cmd.SetArgs([]string{"guestbook", "-c"})
+	cmd.SetArgs([]string{"guestbook"})
 
 	err := cmd.Execute()
 	assert.Nil(t, err)

rollout/pause.go

@@ -185,7 +185,10 @@ func (pCtx *pauseContext) CompletedCanaryPauseStep(pause v1alpha1.RolloutPause)
 	rollout := pCtx.rollout
 	pauseCondition := getPauseCondition(rollout, v1alpha1.PauseReasonCanaryPauseStep)
 
-	if pause.Duration != nil {
+	if rollout.Status.ControllerPause && pauseCondition == nil {
+		pCtx.log.Info("Rollout has been unpaused")
+		return true
+	} else if pause.Duration != nil {
 		now := metav1.Now()
 		if pauseCondition != nil {
 			expiredTime := pauseCondition.StartTime.Add(time.Duration(pause.DurationSeconds()) * time.Second)
@@ -194,9 +197,6 @@ func (pCtx *pauseContext) CompletedCanaryPauseStep(pause v1alpha1.RolloutPause)
 				return true
 			}
 		}
-	} else if rollout.Status.ControllerPause && pauseCondition == nil {
-		pCtx.log.Info("Rollout has been unpaused")
-		return true
 	}
 	return false
 }

test/e2e/canary_test.go

@@ -34,15 +34,15 @@ func (s *CanarySuite) TestCanarySetCanaryScale() {
 - pause: {}
 - setCanaryScale:
     weight: 25
-- pause: {}
+- pause: {duration: 100s}
 - setWeight: 50
-- pause: {}
+- pause: {duration: 10s}
 - setCanaryScale:
     replicas: 3
 - pause: {}
 - setCanaryScale:
     matchTrafficWeight: true
-- pause: {}
+- pause: {duration: 5s}
 `
 	s.Given().
 		RolloutTemplate("@functional/nginx-template.yaml", "set-canary-scale").