From 12bd50375efb8567777789402801bf729a53c4e6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 13 Mar 2023 16:58:02 +0800 Subject: [PATCH] [Backport release-0.4] Fix: fix step depends on skip (#147) * Fix: fix step depends on skip Signed-off-by: FogDong (cherry picked from commit 800506f55e854ce3b9f9d1a138c71884bee8fa8c) * rename the util fuction Signed-off-by: FogDong (cherry picked from commit 2972fc31ee5fd384581a21d03eafd69c95aca9ec) --------- Co-authored-by: FogDong --- controllers/workflow_test.go | 18 ++++++++++++++++++ pkg/executor/workflow.go | 15 ++++++++++----- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/controllers/workflow_test.go b/controllers/workflow_test.go index 27163cf..c938006 100644 --- 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 index 8a0ce12..465de0b 100644 --- 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 }