Feat: add if in workflow (#3941)

* Feat: add if in workflow struct Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Feat: implement the if in workflow Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Feat: support dependency and skip for suspend step Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Fix: fix the rebase from sub steps Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Fix: fix the lint Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Feat: support if in sub steps Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Feat: add tests in application controller Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Fix: fix the lint Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Test: add more tests in discover and custom Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Lint: fix lint Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Tests: add more tests in application controller Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Fix: change failed after retries into reason Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * Fix: fix the terminate cli Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * fix lint Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * remove the terminate workflow to pkg and add feature gates Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * resolve comments Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * nit fix Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com> * make finish condition more clear Signed-off-by: FogDong <dongtianxin.tx@alibaba-inc.com>
2026-05-16 14:27:00 +00:00 · 2022-05-27 22:01:14 +08:00
parent fd024bc3e2
commit fcfb1012d6
31 changed files with 1442 additions and 273 deletions
--- a/pkg/workflow/tasks/custom/task.go
+++ b/pkg/workflow/tasks/custom/task.go
@@ -24,6 +24,7 @@ import (

 	"cuelang.org/go/cue"
 	"github.com/pkg/errors"
+	"k8s.io/apiserver/pkg/util/feature"

 	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
@@ -32,6 +33,7 @@ import (
 	"github.com/oam-dev/kubevela/pkg/cue/model/value"
 	"github.com/oam-dev/kubevela/pkg/cue/packages"
 	"github.com/oam-dev/kubevela/pkg/cue/process"
+	"github.com/oam-dev/kubevela/pkg/features"
 	monitorContext "github.com/oam-dev/kubevela/pkg/monitor/context"
 	wfContext "github.com/oam-dev/kubevela/pkg/workflow/context"
 	"github.com/oam-dev/kubevela/pkg/workflow/hooks"
@@ -47,6 +49,8 @@ var (
 const (
 	// StatusReasonWait is the reason of the workflow progress condition which is Wait.
 	StatusReasonWait = "Wait"
+	// StatusReasonSkip is the reason of the workflow progress condition which is Skip.
+	StatusReasonSkip = "Skip"
 	// StatusReasonRendering is the reason of the workflow progress condition which is Rendering.
 	StatusReasonRendering = "Rendering"
 	// StatusReasonExecute is the reason of the workflow progress condition which is Execute.
@@ -59,6 +63,8 @@ const (
 	StatusReasonParameter = "ProcessParameter"
 	// StatusReasonOutput is the reason of the workflow progress condition which is Output.
 	StatusReasonOutput = "Output"
+	// StatusReasonFailedAfterRetries is the reason of the workflow progress condition which is FailedAfterRetries.
+	StatusReasonFailedAfterRetries = "FailedAfterRetries"
 )

 // LoadTaskTemplate gets the workflowStep definition from cluster and resolve it.
@@ -85,7 +91,8 @@ func (t *TaskLoader) GetTaskGenerator(ctx context.Context, name string) (wfTypes
 type taskRunner struct {
 	name         string
 	run          func(ctx wfContext.Context, options *wfTypes.TaskRunOptions) (common.StepStatus, *wfTypes.Operation, error)
-	checkPending func(ctx wfContext.Context) bool
+	checkPending func(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool
+	skip         func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool)
 }

 // Name return step name.
@@ -99,10 +106,15 @@ func (tr *taskRunner) Run(ctx wfContext.Context, options *wfTypes.TaskRunOptions
 }

 // Pending check task should be executed or not.
-func (tr *taskRunner) Pending(ctx wfContext.Context) bool {
-	return tr.checkPending(ctx)
+func (tr *taskRunner) Pending(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool {
+	return tr.checkPending(ctx, stepStatus)
 }

+func (tr *taskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	return tr.skip(dependsOnPhase, stepStatus)
+}
+
+// nolint:gocyclo
 func (t *TaskLoader) makeTaskGenerator(templ string) (wfTypes.TaskGenerator, error) {
 	return func(wfStep v1beta1.WorkflowStep, genOpt *wfTypes.GeneratorOptions) (wfTypes.TaskRunner, error) {

@@ -141,18 +153,21 @@ func (t *TaskLoader) makeTaskGenerator(templ string) (wfTypes.TaskGenerator, err

 		tRunner := new(taskRunner)
 		tRunner.name = wfStep.Name
-		tRunner.checkPending = func(ctx wfContext.Context) bool {
-			for _, depend := range wfStep.DependsOn {
-				if _, err := ctx.GetVar(hooks.ReadyComponent, depend); err != nil {
-					return true
-				}
+		tRunner.checkPending = func(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool {
+			return CheckPending(ctx, wfStep, stepStatus)
+		}
+		tRunner.skip = func(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+			if feature.DefaultMutableFeatureGate.Enabled(features.EnableSuspendOnFailure) {
+				return exec.status(), false
 			}
-			for _, input := range wfStep.Inputs {
-				if _, err := ctx.GetVar(strings.Split(input.From, ".")...); err != nil {
-					return true
-				}
+			skip := SkipTaskRunner(&SkipOptions{
+				If:             wfStep.If,
+				DependsOnPhase: dependsOnPhase,
+			})
+			if skip {
+				exec.Skip("")
 			}
-			return false
+			return exec.status(), skip
 		}
 		tRunner.run = func(ctx wfContext.Context, options *wfTypes.TaskRunOptions) (common.StepStatus, *wfTypes.Operation, error) {
 			if options.GetTracer == nil {
@@ -261,6 +276,7 @@ type executor struct {
 	terminated         bool
 	failedAfterRetries bool
 	wait               bool
+	skip               bool

 	tracer monitorContext.Context
 }
@@ -289,6 +305,13 @@ func (exec *executor) Wait(message string) {
 	exec.wfStatus.Message = message
 }

+func (exec *executor) Skip(message string) {
+	exec.skip = true
+	exec.wfStatus.Phase = common.WorkflowStepPhaseSkipped
+	exec.wfStatus.Reason = StatusReasonSkip
+	exec.wfStatus.Message = message
+}
+
 func (exec *executor) err(ctx wfContext.Context, err error, reason string) {
 	exec.wait = true
 	exec.wfStatus.Phase = common.WorkflowStepPhaseFailed
@@ -302,6 +325,7 @@ func (exec *executor) checkErrorTimes(ctx wfContext.Context) {
 	if times >= MaxWorkflowStepErrorRetryTimes {
 		exec.wait = false
 		exec.failedAfterRetries = true
+		exec.wfStatus.Reason = StatusReasonFailedAfterRetries
 	}
 }

@@ -441,3 +465,58 @@ func NewTaskLoader(lt LoadTaskTemplate, pkgDiscover *packages.PackageDiscover, h
 		logLevel: logLevel,
 	}
 }
+
+// SkipOptions is the options of skip task runner
+type SkipOptions struct {
+	If             string
+	DependsOnPhase common.WorkflowStepPhase
+}
+
+// SkipTaskRunner will decide whether to skip task runner.
+func SkipTaskRunner(options *SkipOptions) bool {
+	switch options.If {
+	case "always":
+		return false
+	case "":
+		return options.DependsOnPhase != common.WorkflowStepPhaseSucceeded
+	default:
+		// TODO:(fog) support more if cases
+		return false
+	}
+}
+
+// CheckPending checks whether to pending task run
+func CheckPending(ctx wfContext.Context, step v1beta1.WorkflowStep, stepStatus map[string]common.StepStatus) bool {
+	for _, depend := range step.DependsOn {
+		if status, ok := stepStatus[depend]; ok {
+			if !IsStepFinish(status.Phase, status.Reason) {
+				return true
+			}
+		} else {
+			return true
+		}
+	}
+	for _, input := range step.Inputs {
+		if _, err := ctx.GetVar(strings.Split(input.From, ".")...); err != nil {
+			return true
+		}
+	}
+	return false
+}
+
+// IsStepFinish will decide whether step is finish.
+func IsStepFinish(phase common.WorkflowStepPhase, reason string) bool {
+	if feature.DefaultMutableFeatureGate.Enabled(features.EnableSuspendOnFailure) {
+		return phase == common.WorkflowStepPhaseSucceeded
+	}
+	switch phase {
+	case common.WorkflowStepPhaseFailed:
+		return reason == StatusReasonTerminate || reason == StatusReasonFailedAfterRetries
+	case common.WorkflowStepPhaseSkipped:
+		return true
+	case common.WorkflowStepPhaseSucceeded:
+		return true
+	default:
+		return false
+	}
+}
--- a/pkg/workflow/tasks/custom/task_test.go
+++ b/pkg/workflow/tasks/custom/task_test.go
@@ -36,7 +36,6 @@ import (
 	"github.com/oam-dev/kubevela/pkg/cue/model/value"
 	"github.com/oam-dev/kubevela/pkg/cue/process"
 	wfContext "github.com/oam-dev/kubevela/pkg/workflow/context"
-	"github.com/oam-dev/kubevela/pkg/workflow/hooks"
 	"github.com/oam-dev/kubevela/pkg/workflow/providers"
 	"github.com/oam-dev/kubevela/pkg/workflow/types"
 )
@@ -270,6 +269,7 @@ close({
 			r.Equal(operation.Waiting, false)
 			r.Equal(operation.FailedAfterRetries, true)
 			r.Equal(status.Phase, common.WorkflowStepPhaseFailed)
+			r.Equal(status.Reason, StatusReasonFailedAfterRetries)
 		default:
 			r.Equal(operation.Waiting, true)
 			r.Equal(status.Phase, common.WorkflowStepPhaseFailed)
@@ -438,14 +438,14 @@ func TestPendingInputCheck(t *testing.T) {
 	r.NoError(err)
 	run, err := gen(step, &types.GeneratorOptions{})
 	r.NoError(err)
-	r.Equal(run.Pending(wfCtx), true)
+	r.Equal(run.Pending(wfCtx, nil), true)
 	score, err := value.NewValue(`
 100
 `, nil, "")
 	r.NoError(err)
 	err = wfCtx.SetVar(score, "score")
 	r.NoError(err)
-	r.Equal(run.Pending(wfCtx), false)
+	r.Equal(run.Pending(wfCtx, nil), false)
 }

 func TestPendingDependsOnCheck(t *testing.T) {
@@ -473,12 +473,49 @@ func TestPendingDependsOnCheck(t *testing.T) {
 	r.NoError(err)
 	run, err := gen(step, &types.GeneratorOptions{})
 	r.NoError(err)
-	r.Equal(run.Pending(wfCtx), true)
-	ready, err := value.NewValue("true", nil, "")
+	r.Equal(run.Pending(wfCtx, nil), true)
+	ss := map[string]common.StepStatus{
+		"depend": {
+			Phase: common.WorkflowStepPhaseSucceeded,
+		},
+	}
+	r.Equal(run.Pending(wfCtx, ss), false)
+}
+
+func TestSkip(t *testing.T) {
+	r := require.New(t)
+	discover := providers.NewProviders()
+	discover.Register("test", map[string]providers.Handler{
+		"ok": func(ctx wfContext.Context, v *value.Value, act types.Action) error {
+			return nil
+		},
+	})
+	step := v1beta1.WorkflowStep{
+		Name: "skip",
+		Type: "ok",
+	}
+	pCtx := process.NewContext(process.ContextData{
+		AppName:         "myapp",
+		CompName:        "mycomp",
+		Namespace:       "default",
+		AppRevisionName: "myapp-v1",
+	})
+	tasksLoader := NewTaskLoader(mockLoadTemplate, nil, discover, 0, pCtx)
+	gen, err := tasksLoader.GetTaskGenerator(context.Background(), step.Type)
 	r.NoError(err)
-	err = wfCtx.SetVar(ready, hooks.ReadyComponent, "depend")
+	runner, err := gen(step, &types.GeneratorOptions{})
 	r.NoError(err)
-	r.Equal(run.Pending(wfCtx), false)
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailed, nil)
+	r.Equal(skip, true)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	r.NoError(err)
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailed, nil)
+	r.Equal(skip, false)
 }

 func newWorkflowContextForTest(t *testing.T) wfContext.Context {
--- a/pkg/workflow/tasks/discover.go
+++ b/pkg/workflow/tasks/discover.go
@@ -22,6 +22,7 @@ import (
 	builtintime "time"

 	"github.com/pkg/errors"
+	"k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"

@@ -29,6 +30,7 @@ import (
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/pkg/cue/packages"
 	"github.com/oam-dev/kubevela/pkg/cue/process"
+	"github.com/oam-dev/kubevela/pkg/features"
 	monitorContext "github.com/oam-dev/kubevela/pkg/monitor/context"
 	"github.com/oam-dev/kubevela/pkg/oam/discoverymapper"
 	"github.com/oam-dev/kubevela/pkg/velaql/providers/query"
@@ -73,7 +75,7 @@ func (td *taskDiscover) GetTaskGenerator(ctx context.Context, name string) (type
 func suspend(step v1beta1.WorkflowStep, opt *types.GeneratorOptions) (types.TaskRunner, error) {
 	tr := &suspendTaskRunner{
 		id:   opt.ID,
-		name: step.Name,
+		step: step,
 		wait: false,
 	}

@@ -92,6 +94,7 @@ func StepGroup(step v1beta1.WorkflowStep, opt *types.GeneratorOptions) (types.Ta
 	return &stepGroupTaskRunner{
 		id:             opt.ID,
 		name:           step.Name,
+		step:           step,
 		subTaskRunners: opt.SubTaskRunners,
 	}, nil
 }
@@ -119,40 +122,64 @@ func NewTaskDiscoverFromRevision(ctx monitorContext.Context, providerHandlers pr
 }

 type suspendTaskRunner struct {
-	id   string
-	name string
-	wait bool
+	id    string
+	step  v1beta1.WorkflowStep
+	wait  bool
+	phase common.WorkflowStepPhase
 }

 // Name return suspend step name.
 func (tr *suspendTaskRunner) Name() string {
-	return tr.name
+	return tr.step.Name
 }

 // Run make workflow suspend.
 func (tr *suspendTaskRunner) Run(ctx wfContext.Context, options *types.TaskRunOptions) (common.StepStatus, *types.Operation, error) {
+	if tr.wait {
+		tr.phase = common.WorkflowStepPhaseRunning
+	} else {
+		tr.phase = common.WorkflowStepPhaseSucceeded
+	}
 	stepStatus := common.StepStatus{
 		ID:    tr.id,
-		Name:  tr.name,
+		Name:  tr.step.Name,
 		Type:  types.WorkflowStepTypeSuspend,
-		Phase: common.WorkflowStepPhaseSucceeded,
-	}
-
-	if tr.wait {
-		stepStatus.Phase = common.WorkflowStepPhaseRunning
+		Phase: tr.phase,
 	}

 	return stepStatus, &types.Operation{Suspend: true}, nil
 }

 // Pending check task should be executed or not.
-func (tr *suspendTaskRunner) Pending(ctx wfContext.Context) bool {
-	return false
+func (tr *suspendTaskRunner) Pending(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool {
+	return custom.CheckPending(ctx, tr.step, stepStatus)
+}
+
+func (tr *suspendTaskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	status := common.StepStatus{
+		ID:    tr.id,
+		Name:  tr.step.Name,
+		Type:  types.WorkflowStepTypeSuspend,
+		Phase: tr.phase,
+	}
+	if feature.DefaultMutableFeatureGate.Enabled(features.EnableSuspendOnFailure) {
+		return status, false
+	}
+	skip := custom.SkipTaskRunner(&custom.SkipOptions{
+		If:             tr.step.If,
+		DependsOnPhase: dependsOnPhase,
+	})
+	if skip {
+		status.Phase = common.WorkflowStepPhaseSkipped
+		status.Reason = custom.StatusReasonSkip
+	}
+	return status, skip
 }

 type stepGroupTaskRunner struct {
 	id             string
 	name           string
+	step           v1beta1.WorkflowStep
 	subTaskRunners []types.TaskRunner
 }

@@ -161,12 +188,43 @@ func (tr *stepGroupTaskRunner) Name() string {
 	return tr.name
 }

+// Pending check task should be executed or not.
+func (tr *stepGroupTaskRunner) Pending(ctx wfContext.Context, stepStatus map[string]common.StepStatus) bool {
+	return custom.CheckPending(ctx, tr.step, stepStatus)
+}
+
+func (tr *stepGroupTaskRunner) Skip(dependsOnPhase common.WorkflowStepPhase, stepStatus map[string]common.StepStatus) (common.StepStatus, bool) {
+	status := common.StepStatus{
+		ID:   tr.id,
+		Name: tr.step.Name,
+		Type: types.WorkflowStepTypeStepGroup,
+	}
+	if feature.DefaultMutableFeatureGate.Enabled(features.EnableSuspendOnFailure) {
+		return status, false
+	}
+	skip := custom.SkipTaskRunner(&custom.SkipOptions{
+		If:             tr.step.If,
+		DependsOnPhase: dependsOnPhase,
+	})
+	if skip {
+		status.Phase = common.WorkflowStepPhaseSkipped
+		status.Reason = custom.StatusReasonSkip
+		stepStatus[tr.step.Name] = common.StepStatus{
+			ID:    tr.id,
+			Phase: status.Phase,
+		}
+		// return false here to set all the sub steps to skipped
+		return status, false
+	}
+	return status, skip
+}
+
 // Run make workflow step group.
 func (tr *stepGroupTaskRunner) Run(ctx wfContext.Context, options *types.TaskRunOptions) (common.StepStatus, *types.Operation, error) {
 	e := options.Engine
 	if len(tr.subTaskRunners) > 0 {
-		// set sub steps to dag mode for now
 		e.SetParentRunner(tr.name)
+		// set sub steps to dag mode for now
 		if err := e.Run(tr.subTaskRunners, true); err != nil {
 			return common.StepStatus{
 				ID:    tr.id,
@@ -178,34 +236,39 @@ func (tr *stepGroupTaskRunner) Run(ctx wfContext.Context, options *types.TaskRun
 		e.SetParentRunner("")
 	}
 	stepStatus := e.GetStepStatus(tr.name)
-	var phase common.WorkflowStepPhase
-	subStepPhases := make(map[common.WorkflowStepPhase]int)
+	status := common.StepStatus{
+		ID:   tr.id,
+		Name: tr.name,
+		Type: types.WorkflowStepTypeStepGroup,
+	}
+
+	subStepCounts := make(map[string]int)
 	for _, subStepsStatus := range stepStatus.SubStepsStatus {
-		subStepPhases[subStepsStatus.Phase]++
+		subStepCounts[string(subStepsStatus.Phase)]++
+		subStepCounts[subStepsStatus.Reason]++
 	}
 	switch {
 	case len(stepStatus.SubStepsStatus) < len(tr.subTaskRunners):
-		phase = common.WorkflowStepPhaseRunning
-	case subStepPhases[common.WorkflowStepPhaseRunning] > 0:
-		phase = common.WorkflowStepPhaseRunning
-	case subStepPhases[common.WorkflowStepPhaseStopped] > 0:
-		phase = common.WorkflowStepPhaseStopped
-	case subStepPhases[common.WorkflowStepPhaseFailed] > 0:
-		phase = common.WorkflowStepPhaseFailed
+		status.Phase = common.WorkflowStepPhaseRunning
+	case subStepCounts[string(common.WorkflowStepPhaseRunning)] > 0:
+		status.Phase = common.WorkflowStepPhaseRunning
+	case subStepCounts[string(common.WorkflowStepPhaseStopped)] > 0:
+		status.Phase = common.WorkflowStepPhaseStopped
+	case subStepCounts[string(common.WorkflowStepPhaseFailed)] > 0:
+		status.Phase = common.WorkflowStepPhaseFailed
+		switch {
+		case subStepCounts[custom.StatusReasonFailedAfterRetries] > 0:
+			status.Reason = custom.StatusReasonFailedAfterRetries
+		case subStepCounts[custom.StatusReasonTerminate] > 0:
+			status.Reason = custom.StatusReasonTerminate
+		}
+	case subStepCounts[string(common.WorkflowStepPhaseSkipped)] > 0:
+		status.Phase = common.WorkflowStepPhaseSkipped
+		status.Reason = custom.StatusReasonSkip
 	default:
-		phase = common.WorkflowStepPhaseSucceeded
+		status.Phase = common.WorkflowStepPhaseSucceeded
 	}
-	return common.StepStatus{
-		ID:    tr.id,
-		Name:  tr.name,
-		Type:  types.WorkflowStepTypeStepGroup,
-		Phase: phase,
-	}, e.GetOperation(), nil
-}
-
-// Pending check task should be executed or not.
-func (tr *stepGroupTaskRunner) Pending(ctx wfContext.Context) bool {
-	return false
+	return status, e.GetOperation(), nil
 }

 // NewViewTaskDiscover will create a client for load task generator.
--- a/pkg/workflow/tasks/discover_test.go
+++ b/pkg/workflow/tasks/discover_test.go
@@ -25,6 +25,7 @@ import (

 	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+
 	"github.com/oam-dev/kubevela/pkg/cue/process"
 	"github.com/oam-dev/kubevela/pkg/workflow/tasks/custom"
 	"github.com/oam-dev/kubevela/pkg/workflow/types"
@@ -82,10 +83,36 @@ func TestSuspendStep(t *testing.T) {
 	}
 	gen, err := discover.GetTaskGenerator(context.Background(), "suspend")
 	r.NoError(err)
-	runner, err := gen(v1beta1.WorkflowStep{Name: "test"}, &types.GeneratorOptions{ID: "124"})
+	runner, err := gen(v1beta1.WorkflowStep{
+		Name:      "test",
+		DependsOn: []string{"depend"},
+	}, &types.GeneratorOptions{ID: "124"})
 	r.NoError(err)
 	r.Equal(runner.Name(), "test")
-	r.Equal(runner.Pending(nil), false)
+
+	// test pending
+	r.Equal(runner.Pending(nil, nil), true)
+	ss := map[string]common.StepStatus{
+		"depend": {
+			Phase: common.WorkflowStepPhaseSucceeded,
+		},
+	}
+	r.Equal(runner.Pending(nil, ss), false)
+
+	// test skip
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailed, nil)
+	r.Equal(skip, true)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, custom.StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	r.NoError(err)
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailed, nil)
+	r.Equal(skip, false)
+
+	// test run
 	status, act, err := runner.Run(nil, nil)
 	r.NoError(err)
 	r.Equal(act.Suspend, true)
@@ -127,11 +154,38 @@ func TestStepGroupStep(t *testing.T) {
 	r.NoError(err)
 	gen, err := discover.GetTaskGenerator(context.Background(), "stepGroup")
 	r.NoError(err)
-	runner, err := gen(v1beta1.WorkflowStep{Name: "test"}, &types.GeneratorOptions{ID: "124", SubTaskRunners: []types.TaskRunner{subRunner}})
+	runner, err := gen(v1beta1.WorkflowStep{
+		Name:      "test",
+		DependsOn: []string{"depend"},
+	}, &types.GeneratorOptions{ID: "124", SubTaskRunners: []types.TaskRunner{subRunner}})
 	r.NoError(err)
 	r.Equal(runner.Name(), "test")
-	r.Equal(runner.Pending(nil), false)

+	// test pending
+	r.Equal(runner.Pending(nil, nil), true)
+	ss := map[string]common.StepStatus{
+		"depend": {
+			Phase: common.WorkflowStepPhaseSucceeded,
+		},
+	}
+	r.Equal(runner.Pending(nil, ss), false)
+
+	// test skip
+	stepStatus := make(map[string]common.StepStatus)
+	status, skip := runner.Skip(common.WorkflowStepPhaseFailed, stepStatus)
+	r.Equal(skip, false)
+	r.Equal(stepStatus["test"].Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Phase, common.WorkflowStepPhaseSkipped)
+	r.Equal(status.Reason, custom.StatusReasonSkip)
+	runner2, err := gen(v1beta1.WorkflowStep{
+		If:   "always",
+		Name: "test",
+	}, &types.GeneratorOptions{ID: "124"})
+	r.NoError(err)
+	_, skip = runner2.Skip(common.WorkflowStepPhaseFailed, stepStatus)
+	r.Equal(skip, false)
+
+	// test run
 	testCases := []struct {
 		name          string
 		engine        *testEngine