From ba5940ecd51db91ae76a963a817172fa8e48e993 Mon Sep 17 00:00:00 2001
From: FogDong <fog@bentoml.com>
Date: Tue, 24 Sep 2024 23:37:57 +0800
Subject: [PATCH] fix: fix suspend judgement

Signed-off-by: FogDong <fog@bentoml.com>
---
 makefiles/dependency.mk       |  2 +-
 pkg/executor/workflow.go      | 17 +++++++++--------
 pkg/executor/workflow_test.go | 36 +++++++++++++++++++----------------
 pkg/tasks/custom/task.go      |  2 +-
 4 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/makefiles/dependency.mk b/makefiles/dependency.mk
index 15b3c48..679a7b3 100644
--- a/makefiles/dependency.mk
+++ b/makefiles/dependency.mk
@@ -51,7 +51,7 @@ goimports:
 ifeq (, $(shell which goimports))
 	@{ \
 	set -e ;\
-	go install golang.org/x/tools/cmd/goimports@latest ;\
+	go install golang.org/x/tools/cmd/goimports@v0.1.12 ;\
 	}
 GOIMPORTS=$(GOBIN)/goimports
 else
diff --git a/pkg/executor/workflow.go b/pkg/executor/workflow.go
index b448993..723ad46 100644
--- a/pkg/executor/workflow.go
+++ b/pkg/executor/workflow.go
@@ -115,12 +115,6 @@ func (w *workflowExecutor) ExecuteRunners(ctx monitorContext.Context, taskRunner
 		}
 		return v1alpha1.WorkflowStateFailed, nil
 	}
-	if checkWorkflowSuspended(status) {
-		return v1alpha1.WorkflowStateSuspending, nil
-	}
-	if allRunnersSucceeded {
-		return v1alpha1.WorkflowStateSucceeded, nil
-	}
 
 	wfCtx, err := w.makeContext(ctx, w.instance.Name)
 	if err != nil {
@@ -129,6 +123,13 @@ func (w *workflowExecutor) ExecuteRunners(ctx monitorContext.Context, taskRunner
 	}
 	w.wfCtx = wfCtx
 
+	if checkWorkflowSuspended(status) {
+		return v1alpha1.WorkflowStateSuspending, nil
+	}
+	if allRunnersSucceeded {
+		return v1alpha1.WorkflowStateSucceeded, nil
+	}
+
 	if cacheValue, ok := StepStatusCache.Load(cacheKey); ok {
 		// handle cache resource
 		if len(status.Steps) < cacheValue.(int) {
@@ -175,11 +176,11 @@ func checkWorkflowSuspended(status *v1alpha1.WorkflowRunStatus) bool {
 	// if workflow is suspended and the suspended step is still running, return false to run the suspended step
 	if status.Suspend {
 		for _, step := range status.Steps {
-			if step.Phase == v1alpha1.WorkflowStepPhaseSuspending {
+			if step.Reason == types.StatusReasonSuspend && step.Phase == v1alpha1.WorkflowStepPhaseSuspending {
 				return false
 			}
 			for _, sub := range step.SubStepsStatus {
-				if sub.Phase == v1alpha1.WorkflowStepPhaseSuspending {
+				if sub.Reason == types.StatusReasonSuspend && sub.Phase == v1alpha1.WorkflowStepPhaseSuspending {
 					return false
 				}
 			}
diff --git a/pkg/executor/workflow_test.go b/pkg/executor/workflow_test.go
index 2181340..aafc00a 100644
--- a/pkg/executor/workflow_test.go
+++ b/pkg/executor/workflow_test.go
@@ -1772,10 +1772,11 @@ var _ = Describe("Test Workflow", func() {
 				},
 			}, {
 				StepStatus: v1alpha1.StepStatus{
-					Name:  "s2",
-					ID:    "s2",
-					Type:  "suspend",
-					Phase: v1alpha1.WorkflowStepPhaseSuspending,
+					Name:   "s2",
+					ID:     "s2",
+					Type:   "suspend",
+					Reason: types.StatusReasonSuspend,
+					Phase:  v1alpha1.WorkflowStepPhaseSuspending,
 				},
 			}},
 		})).Should(BeEquivalentTo(""))
@@ -1805,10 +1806,11 @@ var _ = Describe("Test Workflow", func() {
 				},
 			}, {
 				StepStatus: v1alpha1.StepStatus{
-					Name:  "s2",
-					ID:    "s2",
-					Type:  "suspend",
-					Phase: v1alpha1.WorkflowStepPhaseSucceeded,
+					Name:   "s2",
+					ID:     "s2",
+					Type:   "suspend",
+					Reason: types.StatusReasonSuspend,
+					Phase:  v1alpha1.WorkflowStepPhaseSucceeded,
 				},
 			}, {
 				StepStatus: v1alpha1.StepStatus{
@@ -1884,10 +1886,11 @@ var _ = Describe("Test Workflow", func() {
 						Type:  "success",
 						Phase: v1alpha1.WorkflowStepPhaseSucceeded,
 					}, {
-						Name:  "s2-sub2",
-						ID:    "s2-sub2",
-						Type:  "suspend",
-						Phase: v1alpha1.WorkflowStepPhaseSuspending,
+						Name:   "s2-sub2",
+						ID:     "s2-sub2",
+						Type:   "suspend",
+						Reason: types.StatusReasonSuspend,
+						Phase:  v1alpha1.WorkflowStepPhaseSuspending,
 					},
 				},
 			}},
@@ -2234,10 +2237,11 @@ func makeRunner(step v1alpha1.WorkflowStep, subTaskRunners []types.TaskRunner) t
 				}
 			}
 			return v1alpha1.StepStatus{
-					Name:  step.Name,
-					Type:  "suspend",
-					ID:    step.Name,
-					Phase: v1alpha1.WorkflowStepPhaseSuspending,
+					Name:   step.Name,
+					Type:   "suspend",
+					ID:     step.Name,
+					Reason: types.StatusReasonSuspend,
+					Phase:  v1alpha1.WorkflowStepPhaseSuspending,
 				}, &types.Operation{
 					Suspend: true,
 				}, nil
diff --git a/pkg/tasks/custom/task.go b/pkg/tasks/custom/task.go
index 79e4138..0315768 100644
--- a/pkg/tasks/custom/task.go
+++ b/pkg/tasks/custom/task.go
@@ -415,7 +415,7 @@ func (exec *executor) Terminate(message string) {
 // Wait let workflow wait.
 func (exec *executor) Wait(message string) {
 	exec.wait = true
-	if exec.wfStatus.Phase != v1alpha1.WorkflowStepPhaseFailed {
+	if exec.wfStatus.Phase != v1alpha1.WorkflowStepPhaseFailed && exec.wfStatus.Phase != v1alpha1.WorkflowStepPhaseSuspending {
 		exec.wfStatus.Phase = v1alpha1.WorkflowStepPhaseRunning
 		exec.wfStatus.Reason = types.StatusReasonWait
 		if message != "" {