[Backport release-0.4] Fix: fix step depends on skip (#147)

* Fix: fix step depends on skip Signed-off-by: FogDong <[email protected]> (cherry picked from commit 800506f) * rename the util fuction Signed-off-by: FogDong <[email protected]> (cherry picked from commit 2972fc3) --------- Co-authored-by: FogDong <[email protected]>
kubevela · Mar 13, 2023 · 12bd503 · 12bd503
1 parent 432ffde
commit 12bd503
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 5 deletions.
diff --git a/controllers/workflow_test.go b/controllers/workflow_test.go
@@ -550,6 +550,21 @@ var _ = Describe("Test Workflow", func() {
 					Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)},
 				},
 			},
+			{
+				WorkflowStepBase: v1alpha1.WorkflowStepBase{
+					Name: "step3",
+					Type: "suspend",
+					If:   "false",
+				},
+			},
+			{
+				WorkflowStepBase: v1alpha1.WorkflowStepBase{
+					Name:       "step4",
+					Type:       "test-apply",
+					Properties: &runtime.RawExtension{Raw: []byte(`{"cmd":["sleep","1000"],"image":"busybox"}`)},
+					DependsOn:  []string{"step3"},
+				},
+			},
 		}
 		wr.Spec.Mode = &v1alpha1.WorkflowExecuteMode{
 			Steps: v1alpha1.WorkflowModeDAG,
@@ -563,6 +578,7 @@ var _ = Describe("Test Workflow", func() {
 		expDeployment := &appsv1.Deployment{}
 		step1Key := types.NamespacedName{Namespace: wr.Namespace, Name: "step1"}
 		step2Key := types.NamespacedName{Namespace: wr.Namespace, Name: "step2"}
+		step4Key := types.NamespacedName{Namespace: wr.Namespace, Name: "step4"}
 		Expect(k8sClient.Get(ctx, step2Key, expDeployment)).Should(utils.NotFoundMatcher{})
 
 		checkRun := &v1alpha1.WorkflowRun{}
@@ -583,6 +599,8 @@ var _ = Describe("Test Workflow", func() {
 
 		tryReconcile(reconciler, wr.Name, wr.Namespace)
 
+		Expect(k8sClient.Get(ctx, step4Key, expDeployment)).Should(utils.NotFoundMatcher{})
+
 		Expect(k8sClient.Get(ctx, wrKey, checkRun)).Should(BeNil())
 		Expect(checkRun.Status.Mode.Steps).Should(BeEquivalentTo(v1alpha1.WorkflowModeDAG))
 		Expect(checkRun.Status.Phase).Should(BeEquivalentTo(v1alpha1.WorkflowStateSucceeded))

diff --git a/pkg/executor/workflow.go b/pkg/executor/workflow.go
@@ -600,7 +600,7 @@ func (e *engine) generateRunOptions(ctx monitorContext.Context, dependsOnPhase v
 				case "always":
 					return &types.PreCheckResult{Skip: false}, nil
 				case "":
-					return &types.PreCheckResult{Skip: isUnsuccessfulStep(dependsOnPhase)}, nil
+					return &types.PreCheckResult{Skip: skipExecutionOfNextStep(dependsOnPhase, len(step.DependsOn) > 0)}, nil
 				default:
 					ifValue, err := custom.ValidateIfValue(e.wfCtx, step, e.stepStatus, options)
 					if err != nil {
@@ -775,14 +775,15 @@ func (e *engine) needStop() bool {
 }
 
 func (e *engine) findDependPhase(taskRunners []types.TaskRunner, index int, dag bool) v1alpha1.WorkflowStepPhase {
-	if dag {
+	dependsOn := len(e.stepDependsOn[taskRunners[index].Name()]) > 0
+	if dag || dependsOn {
 		return e.findDependsOnPhase(taskRunners[index].Name())
 	}
 	if index < 1 {
 		return v1alpha1.WorkflowStepPhaseSucceeded
 	}
 	for i := index - 1; i >= 0; i-- {
-		if isUnsuccessfulStep(e.stepStatus[taskRunners[i].Name()].Phase) {
+		if skipExecutionOfNextStep(e.stepStatus[taskRunners[i].Name()].Phase, dependsOn) {
 			return e.stepStatus[taskRunners[i].Name()].Phase
 		}
 	}
@@ -794,14 +795,18 @@ func (e *engine) findDependsOnPhase(name string) v1alpha1.WorkflowStepPhase {
 		if e.stepStatus[dependsOn].Phase != v1alpha1.WorkflowStepPhaseSucceeded {
 			return e.stepStatus[dependsOn].Phase
 		}
-		if result := e.findDependsOnPhase(dependsOn); isUnsuccessfulStep(result) {
+		if result := e.findDependsOnPhase(dependsOn); result != v1alpha1.WorkflowStepPhaseSucceeded {
 			return result
 		}
 	}
 	return v1alpha1.WorkflowStepPhaseSucceeded
 }
 
-func isUnsuccessfulStep(phase v1alpha1.WorkflowStepPhase) bool {
+// skipExecutionOfNextStep returns true if the next step should be skipped
+func skipExecutionOfNextStep(phase v1alpha1.WorkflowStepPhase, dependsOn bool) bool {
+	if dependsOn {
+		return phase != v1alpha1.WorkflowStepPhaseSucceeded
+	}
 	return phase != v1alpha1.WorkflowStepPhaseSucceeded && phase != v1alpha1.WorkflowStepPhaseSkipped
 }