Fix: fix step depends on skip

Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>
kubevela · Mar 13, 2023 · 018ad30 · 018ad30
1 parent 8ea00c6
commit 018ad30
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
@@ -538,6 +538,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,
@@ -551,6 +566,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{}
@@ -571,6 +587,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
@@ -597,7 +597,8 @@ 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
+					fmt.Println("=====", step.Name, dependsOnPhase, step.DependsOn)
+					return &types.PreCheckResult{Skip: isUnsuccessfulStep(dependsOnPhase, len(step.DependsOn) > 0)}, nil
 				default:
 					ifValue, err := custom.ValidateIfValue(e.wfCtx, step, e.stepStatus, options)
 					if err != nil {
@@ -781,14 +782,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 isUnsuccessfulStep(e.stepStatus[taskRunners[i].Name()].Phase, dependsOn) {
 			return e.stepStatus[taskRunners[i].Name()].Phase
 		}
 	}
@@ -800,14 +802,17 @@ 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 {
+func isUnsuccessfulStep(phase v1alpha1.WorkflowStepPhase, dependsOn bool) bool {
+	if dependsOn {
+		return phase != v1alpha1.WorkflowStepPhaseSucceeded
+	}
 	return phase != v1alpha1.WorkflowStepPhaseSucceeded && phase != v1alpha1.WorkflowStepPhaseSkipped
 }