Skip to content

Commit

Permalink
fix(conditions): add foreach property on conditions
Browse files Browse the repository at this point in the history 
Signed-off-by: Adrien Barreau <adrien.barreau@ovhcloud.com>
  • Loading branch information
@deathiop @rclsilver
deathiop authored and rclsilver committed Aug 29, 2023
1 parent f76bdce commit 7f80fba
Show file tree
Hide file tree
Showing 10 changed files with 331 additions and 75 deletions.
46 changes: 46 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,52 @@ It can be declared in the template like this:
foreach_strategy: "sequence"
```

When writing `skip` conditions on loops, an additional property `foreach` can be added. It can have two values:
- `children`: default value. If no value is set, this value is used. The condition will be run on every iteration of the foreach loop;
- `parent`: the condition will be run on the step itself before creating its children.

For example:

```yaml
foreach: '{{.step.aPreviousStep.output.ids | toJson}}'
action:
type: echo
configuration:
output:
url: '{{ .iterator }}'
conditions:
- type: skip
foreach: children # <- this line can be omitted
if:
- value: '{{ .iterator }}'
operator: EQ
expected: '{{ .step.something.output.dontTouchId }}'
then:
this: PRUNE
```

will be run on every children and skip the child by pruning it the condition is true. And

```yaml
foreach: '{{.step.aPreviousStep.output.ids | toJson}}'
action:
type: echo
configuration:
output:
url: '{{ .iterator }}'
conditions:
- type: skip
foreach: parent
if:
- value: '{{ step.previousCheck.output.result }}'
operator: EQ
expected: 'already_done'
then:
this: PRUNE
```

will be run before creating any children, by pruning the parent.

#### Resources <a name="resources"></a>

Resources are a way to restrict the concurrency factor of operations, to control the throughput and avoid dangerous behavior (e.g. flooding the targets).
Expand Down
18 changes: 16 additions & 2 deletions engine/engine.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,13 @@ func runAvailableSteps(dbp zesty.DBProvider, modifiedSteps map[string]bool, res
// prepare step
s.Name = name
if s.ForEach != "" { // loop step
// run "skip" step conditions in step in in todo or to_retry
switch s.State {
case step.StateTODO, step.StateToRetry:
step.PreRun(s, res.Values, resolutionStateSetter(res, preRunModifiedSteps), executedSteps)
_ = commit(dbp, res, nil)
}

switch s.State {
case step.StateTODO:
expanded++
Expand Down Expand Up @@ -751,12 +758,19 @@ func expandStep(s *step.Step, res *resolution.Resolution) {
// to be shared between multiple steps
dependencies := make([]string, len(s.Dependencies))
customStates := make([]string, len(s.CustomStates))
conditions := make([]*condition.Condition, len(s.Conditions))
resources := make([]string, len(s.Resources))
conditions := []*condition.Condition{}

copy(dependencies, s.Dependencies)
copy(customStates, s.CustomStates)
copy(conditions, s.Conditions)
copy(resources, s.Resources)
for _, c := range s.Conditions {
// Only copy skip conditions that are flagged with foreach: children
if c.Type == condition.SKIP && c.ForEach != condition.ForEachChildren {
continue
}
conditions = append(conditions, c)
}

res.Steps[childStepName] = &step.Step{
Name: childStepName,
Expand Down
171 changes: 100 additions & 71 deletions engine/engine_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,6 @@ func templateFromYAML(dbp zesty.DBProvider, filename string) (*tasktemplate.Task
return tasktemplate.LoadFromName(dbp, tmpl.Name)
}

type lintingAndValidationTest struct {
NilResolution bool
NilError bool
}

func TestSimpleTemplate(t *testing.T) {
input := map[string]interface{}{
"foo": "bar",
Expand Down Expand Up @@ -333,41 +328,48 @@ func TestStepMaxRetries(t *testing.T) {
}

func TestLintingAndValidation(t *testing.T) {
expectedResult := map[string]lintingAndValidationTest{
"lintingError.yaml": {true, false},
"lintingRootKey.yaml": {true, false},
"lintingReservedStep.yaml": {true, false},
"customStates.yaml": {true, false},
"forbiddenStateImpact.yaml": {true, false},
"stepDetailsLintingError.yaml": {true, false},
"circularDependencies.yaml": {true, false},
"selfDependency.yaml": {true, false},
"orphanDependencies.yaml": {true, false},
"functionEchoHelloWorldError.yaml": {true, false},

"lintingInfiniteOk.yaml": {false, true},
"lintingObject.yaml": {false, true},
"allowedStateImpact.yaml": {false, true},
"functionEchoHelloWorld.yaml": {false, true},
"functionCustomState.yaml": {false, true},
"functionPreHook.yaml": {false, true},
"functionEchoTemplatedOutput.yaml": {false, true},
expectedResult := map[string]struct {
nilResolution bool
errstr string
}{
"lintingError.yaml": {true, `Variable notfound does not exist`},
"lintingRootKey.yaml": {true, `Variable grault does not exist`},
"lintingReservedStep.yaml": {true, `'this' step name is reserved`},
"customStates.yaml": {true, `Custom state "SERVER_ERROR" is not allowed as it's a reserved state`},
"forbiddenStateImpact.yaml": {true, `Step condition cannot impact the state of step stepTwo, only those who belong to the dependency chain are allowed`},
"stepDetailsLintingError.yaml": {true, `Wrong step key: stepNotFound`},
"circularDependencies.yaml": {true, `Invalid: circular dependency [stepOne stepThree stepTwo] <-> step`}, // Last step name is random
"selfDependency.yaml": {true, `Invalid: circular dependency [stepOne] <-> stepOne`},
"orphanDependencies.yaml": {true, `Invalid dependency, no step with that name: "stepTwo"`},
"functionEchoHelloWorldError.yaml": {true, `Invalid executor action: missing function_args "name"`},
"conditionForeachSkipOnly.yaml": {true, `Step condition can set foreach on a skip condition`},
"conditionForeachInvalid.yaml": {true, `Unknown condition foreach: invalid`},
"conditionForeachStepNotForeach.yaml": {true, `Step condition cannot set foreach on a non-foreach step`},

"lintingInfiniteOk.yaml": {false, ""},
"lintingObject.yaml": {false, ""},
"allowedStateImpact.yaml": {false, ""},
"functionEchoHelloWorld.yaml": {false, ""},
"functionCustomState.yaml": {false, ""},
"functionPreHook.yaml": {false, ""},
"functionEchoTemplatedOutput.yaml": {false, ""},
}

for template, testCase := range expectedResult {
t.Run(template, func(t *testing.T) {
res, err := createResolution(template, map[string]interface{}{}, nil)

if testCase.NilResolution {
if testCase.nilResolution {
assert.Nil(t, res)
} else {
assert.NotNil(t, res)
}

if testCase.NilError {
if testCase.errstr == "" {
assert.Nil(t, err)
} else {
assert.NotNil(t, err)
require.NotNil(t, err)
assert.Contains(t, err.Error(), testCase.errstr)
}
})
}
Expand Down Expand Up @@ -711,7 +713,7 @@ func TestForeach(t *testing.T) {
}

func TestForeachWithChainedIterations(t *testing.T) {
_, require := td.AssertRequire(t)
assert, require := td.AssertRequire(t)
res, err := createResolution("foreach.yaml", map[string]interface{}{
"list": []interface{}{"a", "b", "c", "d", "e"},
}, nil)
Expand All @@ -733,6 +735,7 @@ func TestForeachWithChainedIterations(t *testing.T) {
Then: map[string]string{
"this": "SERVER_ERROR",
},
ForEach: condition.ForEachChildren,
},
)
res.Steps["generateItems"].ForEachStrategy = "sequence"
Expand All @@ -744,24 +747,24 @@ func TestForeachWithChainedIterations(t *testing.T) {
require.Nil(err)
require.Cmp(res.State, resolution.StateError)

td.Cmp(t, res.Steps["emptyLoop"].State, step.StateDone) // running on empty collection is ok
td.Cmp(t, res.Steps["concatItems"].State, step.StateTODO)
td.Cmp(t, res.Steps["finalStep"].State, step.StateTODO)
td.Cmp(t, res.Steps["bStep"].State, "B")
td.Cmp(t, res.Steps["generateItems-0"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-1"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-2"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-3"].State, step.StateServerError)
td.Cmp(t, res.Steps["generateItems-4"].State, step.StateTODO)
td.CmpLen(t, res.Steps["generateItems-0"].Dependencies, 0)
td.Cmp(t, res.Steps["generateItems-1"].Dependencies, []string{"generateItems-0"})
td.Cmp(t, res.Steps["generateItems-2"].Dependencies, []string{"generateItems-1"})
td.Cmp(t, res.Steps["generateItems-3"].Dependencies, []string{"generateItems-2"})
td.Cmp(t, res.Steps["generateItems-4"].Dependencies, []string{"generateItems-3"})
assert.Cmp(res.Steps["emptyLoop"].State, step.StateDone) // running on empty collection is ok
assert.Cmp(res.Steps["concatItems"].State, step.StateTODO)
assert.Cmp(res.Steps["finalStep"].State, step.StateTODO)
assert.Cmp(res.Steps["bStep"].State, "B")
assert.Cmp(res.Steps["generateItems-0"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-1"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-2"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-3"].State, step.StateServerError)
assert.Cmp(res.Steps["generateItems-4"].State, step.StateTODO)
assert.Len(res.Steps["generateItems-0"].Dependencies, 0)
assert.Cmp(res.Steps["generateItems-1"].Dependencies, []string{"generateItems-0"})
assert.Cmp(res.Steps["generateItems-2"].Dependencies, []string{"generateItems-1"})
assert.Cmp(res.Steps["generateItems-3"].Dependencies, []string{"generateItems-2"})
assert.Cmp(res.Steps["generateItems-4"].Dependencies, []string{"generateItems-3"})
}

func TestForeachWithChainedIterationsWithDepOnParent(t *testing.T) {
_, require := td.AssertRequire(t)
assert, require := td.AssertRequire(t)
res, err := createResolution("foreach.yaml", map[string]interface{}{
"list": []interface{}{"a", "b", "c", "d", "e"},
}, nil)
Expand All @@ -784,6 +787,7 @@ func TestForeachWithChainedIterationsWithDepOnParent(t *testing.T) {
Then: map[string]string{
"this": "SERVER_ERROR",
},
ForEach: condition.ForEachChildren,
},
)
res.Steps["generateItems"].ForEachStrategy = "sequence"
Expand All @@ -795,39 +799,64 @@ func TestForeachWithChainedIterationsWithDepOnParent(t *testing.T) {
require.Nil(err)
require.Cmp(res.State, resolution.StateError)

td.Cmp(t, res.Steps["emptyLoop"].State, step.StateDone) // running on empty collection is ok
td.Cmp(t, res.Steps["concatItems"].State, step.StateTODO)
td.Cmp(t, res.Steps["finalStep"].State, step.StateTODO)
td.Cmp(t, res.Steps["bStep"].State, "B")
td.Cmp(t, res.Steps["generateItems-0"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-1"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-2"].State, step.StateDone)
td.Cmp(t, res.Steps["generateItems-3"].State, step.StateServerError)
td.Cmp(t, res.Steps["generateItems-4"].State, step.StateTODO)
td.Cmp(t, res.Steps["generateItems"].Dependencies, []string{"emptyLoop", "generateItems-0:ANY", "generateItems-1:ANY", "generateItems-2:ANY", "generateItems-3:ANY", "generateItems-4:ANY"})
td.Cmp(t, res.Steps["generateItems-0"].Dependencies, []string{"emptyLoop"})
td.Cmp(t, res.Steps["generateItems-1"].Dependencies, []string{"emptyLoop", "generateItems-0"})
td.Cmp(t, res.Steps["generateItems-2"].Dependencies, []string{"emptyLoop", "generateItems-1"})
td.Cmp(t, res.Steps["generateItems-3"].Dependencies, []string{"emptyLoop", "generateItems-2"})
td.Cmp(t, res.Steps["generateItems-4"].Dependencies, []string{"emptyLoop", "generateItems-3"})
assert.Cmp(res.Steps["emptyLoop"].State, step.StateDone) // running on empty collection is ok
assert.Cmp(res.Steps["concatItems"].State, step.StateTODO)
assert.Cmp(res.Steps["finalStep"].State, step.StateTODO)
assert.Cmp(res.Steps["bStep"].State, "B")
assert.Cmp(res.Steps["generateItems-0"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-1"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-2"].State, step.StateDone)
assert.Cmp(res.Steps["generateItems-3"].State, step.StateServerError)
assert.Cmp(res.Steps["generateItems-4"].State, step.StateTODO)
assert.Cmp(res.Steps["generateItems"].Dependencies, []string{"emptyLoop", "generateItems-0:ANY", "generateItems-1:ANY", "generateItems-2:ANY", "generateItems-3:ANY", "generateItems-4:ANY"})
assert.Cmp(res.Steps["generateItems-0"].Dependencies, []string{"emptyLoop"})
assert.Cmp(res.Steps["generateItems-1"].Dependencies, []string{"emptyLoop", "generateItems-0"})
assert.Cmp(res.Steps["generateItems-2"].Dependencies, []string{"emptyLoop", "generateItems-1"})
assert.Cmp(res.Steps["generateItems-3"].Dependencies, []string{"emptyLoop", "generateItems-2"})
assert.Cmp(res.Steps["generateItems-4"].Dependencies, []string{"emptyLoop", "generateItems-3"})
}

func TestForeachWithPreRun(t *testing.T) {
input := map[string]interface{}{}
res, err := createResolution("foreachAndPreRun.yaml", input, nil)
require.Nilf(t, err, "expecting nil error, got %s", err)
require.NotNil(t, res)
for _, switchToToRetry := range []bool{false, true} {
t.Run(fmt.Sprintf("%s-%t", t.Name(), switchToToRetry), func(t *testing.T) {
input := map[string]interface{}{}
res, err := createResolution("foreachAndPreRun.yaml", input, nil)
require.Nilf(t, err, "expecting nil error, got %s", err)
require.NotNil(t, res)

if switchToToRetry {
for _, st := range []string{"stepForeachPrune", "stepDepOnForeachPrune", "stepForeachPruneParentTask", "stepDepOnForeachPruneParentTask"} {
res.Steps[st].State = step.StateToRetry
}
require.NoError(t, updateResolution(res))
}

res, err = runResolution(res)
res, err = runResolution(res)

require.Nilf(t, err, "got error %s", err)
require.NotNil(t, res)
assert.Equal(t, resolution.StateDone, res.State)
for _, st := range []string{"stepForeachNoDep", "stepSkippedNoDep", "stepNoDep", "stepForeachWithDep", "stepSkippedWithDep"} {
assert.Equal(t, step.StateDone, res.Steps[st].State)
}
for _, st := range []string{"stepDep", "stepDep2"} {
assert.Equal(t, step.StatePrune, res.Steps[st].State)
require.Nilf(t, err, "got error %s", err)
require.NotNil(t, res)
assert.Equal(t, resolution.StateDone, res.State)
for _, st := range []string{"stepForeachNoDep", "stepSkippedNoDep", "stepNoDep", "stepForeachWithDep", "stepSkippedWithDep"} {
assert.Equal(t, step.StateDone, res.Steps[st].State)
}
for _, st := range []string{"stepDep", "stepDep2"} {
assert.Equal(t, step.StatePrune, res.Steps[st].State)
}

// skip prune on a foreach step's children means:
// - foreach children are set to prune
// - the foreach step itself is set to done
// - the dependencies are not pruned
assert.Equal(t, step.StateDone, res.Steps["stepForeachPrune"].State)
assert.Equal(t, step.StateDone, res.Steps["stepDepOnForeachPrune"].State)

// skip prune on a foreach step itself means:
// - foreach children are not generated
// - the foreach step itself is set to prune
// - the dependencies are pruned
assert.Equal(t, step.StatePrune, res.Steps["stepForeachPruneParentTask"].State)
assert.Equal(t, step.StatePrune, res.Steps["stepDepOnForeachPruneParentTask"].State)
})
}
}

Expand Down
33 changes: 33 additions & 0 deletions engine/step/condition/stepcondition.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package condition
import (
"fmt"

"github.com/juju/errors"
"github.com/ovh/utask/engine/values"
)

Expand All @@ -13,15 +14,47 @@ const (
CHECK = "check"
)

const (
// ForEachChildren executes the condition on the children of a foreach step.
// The children are created, the condition is copied in them, then run.
// This is the default value.
ForEachChildren = "children"
// ForEachParent executes the condition on the foreach step itself.
ForEachParent = "parent"
)

// Condition defines a condition to be evaluated before or after a step's action
type Condition struct {
Type string `json:"type"`
If []*Assert `json:"if"`
Then map[string]string `json:"then"`
Final bool `json:"final"`
ForEach string `json:"foreach"`
Message string `json:"message"`
}

// Valid asserts that a condition's definition is valid
// ie. the type and foreach are among the accepted values listed above
func (c *Condition) Valid() error {
if c == nil {
return nil
}

switch c.Type {
case SKIP, CHECK:
default:
return errors.BadRequestf("Unknown condition type: %s", c.Type)
}

switch c.ForEach {
case "", ForEachChildren, ForEachParent:
default:
return errors.BadRequestf("Unknown condition foreach: %s", c.ForEach)
}

return nil
}

// Eval runs the condition against a set of values, evaluating the underlying Condition
func (sc *Condition) Eval(v *values.Values, item interface{}, stepName string) error {
for _, c := range sc.If {
Expand Down
Loading

0 comments on commit 7f80fba

Please sign in to comment.