fix: analysis step should be ignored after promote in case if result …

…was inconclusive

Signed-off-by: pashakostohrys <[email protected]>

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

@@ -34,6 +34,7 @@ const (
 	setCurrentStepIndex                         = `{"status":{"currentStepIndex":%d}}`
 	unpausePatch                                = `{"spec":{"paused":false}}`
 	clearPauseConditionsPatch                   = `{"status":{"pauseConditions":null}}`
+	clearPauseConditionsAndControllerPausePatch = `{"status":{"pauseConditions":null, "controllerPause":false, "currentStepIndex":%d}}`
 	unpauseAndClearPauseConditionsPatch         = `{"spec":{"paused":false},"status":{"pauseConditions":null}}`
 	promoteFullPatch                            = `{"status":{"promoteFull":true}}`
 	clearPauseConditionsPatchWithStep           = `{"status":{"pauseConditions":null, "currentStepIndex":%d}}`
@@ -133,6 +134,10 @@ func PromoteRollout(rolloutIf clientset.RolloutInterface, name string, skipCurre
 	return ro, nil
 }
 
+func isInconclusive(rollout *v1alpha1.Rollout) bool {
+	return rollout.Spec.Strategy.Canary != nil && rollout.Status.Canary.CurrentStepAnalysisRunStatus != nil && rollout.Status.Canary.CurrentStepAnalysisRunStatus.Status == v1alpha1.AnalysisPhaseInconclusive
+}
+
 func getPatches(rollout *v1alpha1.Rollout, skipCurrentStep, skipAllStep, full bool) ([]byte, []byte, []byte) {
 	var specPatch, statusPatch, unifiedPatch []byte
 	switch {
@@ -160,7 +165,18 @@ func getPatches(rollout *v1alpha1.Rollout, skipCurrentStep, skipAllStep, full bo
 		if rollout.Spec.Paused {
 			specPatch = []byte(unpausePatch)
 		}
-		if len(rollout.Status.PauseConditions) > 0 {
+		// in case if canary rollout in inconclusive state, we want to unset controller pause , clean pause conditions and increment step index
+		// so that rollout can proceed to next step
+		// without such patch, rollout will be stuck in inconclusive state in case if next step is pause step
+		if isInconclusive(rollout) && len(rollout.Status.PauseConditions) > 0 && rollout.Status.ControllerPause {
+			_, index := replicasetutil.GetCurrentCanaryStep(rollout)
+			if index != nil {
+				if *index < int32(len(rollout.Spec.Strategy.Canary.Steps)) {
+					*index++
+				}
+				statusPatch = []byte(fmt.Sprintf(clearPauseConditionsAndControllerPausePatch, *index))
+			}
+		} else if len(rollout.Status.PauseConditions) > 0 {
 			statusPatch = []byte(clearPauseConditionsPatch)
 		} else if rollout.Spec.Strategy.Canary != nil {
 			// we only want to clear pause conditions, or increment step index (never both)

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

@@ -490,3 +490,69 @@ func TestPromoteCmdAlreadyFullyPromoted(t *testing.T) {
 	assert.Equal(t, stdout, "rollout 'guestbook' fully promoted\n")
 	assert.Empty(t, stderr)
 }
+
+func TestPromoteInconclusiveStep(t *testing.T) {
+	ro := v1alpha1.Rollout{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "guestbook",
+			Namespace: metav1.NamespaceDefault,
+		},
+		Spec: v1alpha1.RolloutSpec{
+			Paused: true,
+			Strategy: v1alpha1.RolloutStrategy{
+				Canary: &v1alpha1.CanaryStrategy{
+					Steps: []v1alpha1.CanaryStep{
+						{
+							SetWeight: pointer.Int32Ptr(1),
+						},
+						{
+							SetWeight: pointer.Int32Ptr(2),
+						},
+					},
+				},
+			},
+		},
+		Status: v1alpha1.RolloutStatus{
+			PauseConditions: []v1alpha1.PauseCondition{{
+				Reason: v1alpha1.PauseReasonCanaryPauseStep,
+			}},
+			ControllerPause: true,
+			Canary: v1alpha1.CanaryStatus{
+				CurrentStepAnalysisRunStatus: &v1alpha1.RolloutAnalysisRunStatus{
+					Status: v1alpha1.AnalysisPhaseInconclusive,
+				},
+			},
+		},
+	}
+
+	tf, o := options.NewFakeArgoRolloutsOptions(&ro)
+	defer tf.Cleanup()
+	fakeClient := o.RolloutsClient.(*fakeroclient.Clientset)
+	fakeClient.PrependReactor("patch", "*", func(action kubetesting.Action) (handled bool, ret runtime.Object, err error) {
+		if patchAction, ok := action.(kubetesting.PatchAction); ok {
+			patchRo := v1alpha1.Rollout{}
+			err := json.Unmarshal(patchAction.GetPatch(), &patchRo)
+			if err != nil {
+				panic(err)
+			}
+			ro.Status.CurrentStepIndex = patchRo.Status.CurrentStepIndex
+			ro.Status.ControllerPause = patchRo.Status.ControllerPause
+			ro.Status.PauseConditions = patchRo.Status.PauseConditions
+		}
+		return true, &ro, nil
+	})
+
+	cmd := NewCmdPromote(o)
+	cmd.PersistentPreRunE = o.PersistentPreRunE
+	cmd.SetArgs([]string{"guestbook"})
+
+	err := cmd.Execute()
+	assert.Nil(t, err)
+	assert.Equal(t, false, ro.Status.ControllerPause)
+	assert.Empty(t, ro.Status.PauseConditions)
+
+	stdout := o.Out.(*bytes.Buffer).String()
+	stderr := o.ErrOut.(*bytes.Buffer).String()
+	assert.Equal(t, stdout, "rollout 'guestbook' promoted\n")
+	assert.Empty(t, stderr)
+}