From fa3ed07e05af5970cbf51fa929488aac82c439de Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 14:56:25 +0900 Subject: [PATCH 1/8] Support `defer:` for deferring steps in a runbook --- defer.go | 3 + defer_test.go | 79 ++++++++++++++ include.go | 1 + operator.go | 152 +++++++++++++++++++-------- operator_test.go | 3 +- step.go | 1 + testdata/book/custom_runner_http.yml | 2 +- testdata/book/defer.yml | 31 ++++++ testdata/book/defer_included.yml | 13 +++ 9 files changed, 241 insertions(+), 44 deletions(-) create mode 100644 defer.go create mode 100644 defer_test.go create mode 100644 testdata/book/defer.yml create mode 100644 testdata/book/defer_included.yml diff --git a/defer.go b/defer.go new file mode 100644 index 00000000..34cf9e95 --- /dev/null +++ b/defer.go @@ -0,0 +1,3 @@ +package runn + +const deferSectionKey = "defer" diff --git a/defer_test.go b/defer_test.go new file mode 100644 index 00000000..f7d2aecd --- /dev/null +++ b/defer_test.go @@ -0,0 +1,79 @@ +package runn + +import ( + "context" + "testing" +) + +func TestDeferRun(t *testing.T) { + book := "testdata/book/defer.yml" + ctx := context.Background() + o, err := New(Book(book)) + if err != nil { + t.Fatal(err) + } + if err := o.Run(ctx); err == nil { + t.Fatal("expected error") + } + if want := 8; len(o.store.steps) != want { + t.Errorf("o.store.steps got %v, want %v", len(o.store.steps), want) + } + r := o.Result() + if want := 8; len(r.StepResults) != want { + t.Errorf("r.StepResults got %v, want %v", len(r.StepResults), want) + } + + t.Run("main steps", func(t *testing.T) { + wantResults := []struct { + desc string + skipped bool + err bool + }{ + {"step 1", false, false}, + {"include step", false, false}, + {"step 2", false, false}, + {"step 3", false, true}, + {"step 4", true, false}, + {"defererd step c", false, false}, + {"defererd step b", false, true}, + {"defererd step a", false, false}, + } + for i, want := range wantResults { + got := r.StepResults[i] + if got.Desc != want.desc { + t.Errorf("got %v, want %v", got.Desc, want.desc) + } + if got.Skipped != want.skipped { + t.Errorf("got %v, want %v", got.Skipped, want.skipped) + } + if (got.Err == nil) == want.err { + t.Errorf("got %v, want %v", got.Err, want.err) + } + } + }) + + t.Run("include steps", func(t *testing.T) { + wantResults := []struct { + desc string + skipped bool + err bool + }{ + {"included step 1", false, false}, + {"included step 2", false, false}, + {"included defererd step d", false, false}, + } + + for i, want := range wantResults { + got := r.StepResults[1].IncludedRunResults[0].StepResults[i] + if got.Desc != want.desc { + t.Errorf("got %v, want %v", got.Desc, want.desc) + } + if got.Skipped != want.skipped { + t.Errorf("got %v, want %v", got.Skipped, want.skipped) + } + if (got.Err == nil) == want.err { + t.Errorf("got %v, want %v", got.Err, want.err) + } + } + }) +} diff --git a/include.go b/include.go index 195acebe..20c1fa5f 100644 --- a/include.go +++ b/include.go @@ -215,6 +215,7 @@ func (o *operator) newNestedOperator(parent *step, opts ...Option) (*operator, e oo.store.runNIndex = o.store.runNIndex oo.dbg = o.dbg oo.nm = o.nm + oo.deferred = o.deferred return oo, nil } diff --git a/operator.go b/operator.go index 8886f275..234a8fed 100644 --- a/operator.go +++ b/operator.go @@ -40,6 +40,16 @@ type need struct { op *operator } +type deferredOpAndStep struct { + op *operator + idx int + step *step +} + +type deferredOpAndSteps struct { + steps []*deferredOpAndStep +} + type operator struct { id string httpRunners map[string]*httpRunner @@ -49,6 +59,7 @@ type operator struct { sshRunners map[string]*sshRunner includeRunners map[string]*includeRunner steps []*step + deferred *deferredOpAndSteps store *store desc string needs map[string]*need // Map of `needs:` in runbook. key is the operator.bookPath. @@ -439,6 +450,7 @@ func New(opts ...Option) (*operator, error) { cdpRunners: map[string]*cdpRunner{}, sshRunners: map[string]*sshRunner{}, includeRunners: map[string]*includeRunner{}, + deferred: &deferredOpAndSteps{}, store: st, useMap: bk.useMap, desc: bk.desc, @@ -670,10 +682,10 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { if op.t != nil { op.t.Helper() } - step := newStep(idx, key, op, s) + st := newStep(idx, key, op, s) // if section if v, ok := s[ifSectionKey]; ok { - step.ifCond, ok = v.(string) + st.ifCond, ok = v.(string) if !ok { return fmt.Errorf("invalid if condition: %v", v) } @@ -681,33 +693,41 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { } // desc section if v, ok := s[descSectionKey]; ok { - step.desc, ok = v.(string) + st.desc, ok = v.(string) if !ok { return fmt.Errorf("invalid desc: %v", v) } delete(s, descSectionKey) } + // defer section + if v, ok := s[deferSectionKey]; ok { + st.deferred, ok = v.(bool) + if !ok { + return fmt.Errorf("invalid defer: %v", v) + } + delete(s, deferSectionKey) + } // loop section if v, ok := s[loopSectionKey]; ok { r, err := newLoop(v) if err != nil { return fmt.Errorf("invalid loop: %w\n%v", err, v) } - step.loop = r + st.loop = r delete(s, loopSectionKey) } // test runner if v, ok := s[testRunnerKey]; ok { - step.testRunner = newTestRunner() + st.testRunner = newTestRunner() switch vv := v.(type) { case bool: if vv { - step.testCond = "true" + st.testCond = "true" } else { - step.testCond = "false" + st.testCond = "false" } case string: - step.testCond = vv + st.testCond = vv default: return fmt.Errorf("invalid test condition: %v", v) } @@ -715,10 +735,10 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { } // dump runner if v, ok := s[dumpRunnerKey]; ok { - step.dumpRunner = newDumpRunner() + st.dumpRunner = newDumpRunner() switch vv := v.(type) { case string: - step.dumpRequest = &dumpRequest{ + st.dumpRequest = &dumpRequest{ expr: vv, } case map[string]any: @@ -738,7 +758,7 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { if !ok { disableMask = false } - step.dumpRequest = &dumpRequest{ + st.dumpRequest = &dumpRequest{ expr: cast.ToString(expr), out: cast.ToString(out), disableTrailingNewline: cast.ToBool(disableNL), @@ -751,105 +771,105 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { } // bind runner if v, ok := s[bindRunnerKey]; ok { - step.bindRunner = newBindRunner() + st.bindRunner = newBindRunner() cond, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid bind condition: %v", v) } - step.bindCond = cond + st.bindCond = cond delete(s, bindRunnerKey) } k, v, ok := pop(s) if ok { - step.runnerKey = k + st.runnerKey = k switch { case k == includeRunnerKey: ir, err := newIncludeRunner() if err != nil { return err } - step.includeRunner = ir + st.includeRunner = ir c, err := parseIncludeConfig(v) if err != nil { return err } - c.step = step - step.includeConfig = c + c.step = st + st.includeConfig = c case k == execRunnerKey: - step.execRunner = newExecRunner() + st.execRunner = newExecRunner() vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid exec command: %v", v) } - step.execCommand = vv + st.execCommand = vv case k == runnerRunnerKey: - step.runnerRunner = newRunnerRunner() + st.runnerRunner = newRunnerRunner() vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid runner runner: %v", v) } - step.runnerDefinition = vv + st.runnerDefinition = vv op.hasRunnerRunner = true default: detected := false h, ok := op.httpRunners[k] if ok { - step.httpRunner = h + st.httpRunner = h vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid http request: %v", v) } - step.httpRequest = vv + st.httpRequest = vv detected = true } db, ok := op.dbRunners[k] if ok && !detected { - step.dbRunner = db + st.dbRunner = db vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid db query: %v", v) } - step.dbQuery = vv + st.dbQuery = vv detected = true } gc, ok := op.grpcRunners[k] if ok && !detected { - step.grpcRunner = gc + st.grpcRunner = gc vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid gRPC request: %v", v) } - step.grpcRequest = vv + st.grpcRequest = vv detected = true } cc, ok := op.cdpRunners[k] if ok && !detected { - step.cdpRunner = cc + st.cdpRunner = cc vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid CDP actions: %v", v) } - step.cdpActions = vv + st.cdpActions = vv detected = true } sc, ok := op.sshRunners[k] if ok && !detected { - step.sshRunner = sc + st.sshRunner = sc vv, ok := v.(map[string]any) if !ok { return fmt.Errorf("invalid SSH command: %v", v) } - step.sshCommand = vv + st.sshCommand = vv detected = true } ic, ok := op.includeRunners[k] if ok && !detected { - step.includeRunner = ic + st.includeRunner = ic c := &includeConfig{ - step: step, + step: st, } - step.includeConfig = c + st.includeConfig = c detected = true } @@ -861,11 +881,12 @@ func (op *operator) appendStep(idx int, key string, s map[string]any) error { if !ok { return fmt.Errorf("invalid runner values: %v", v) } - step.runnerValues = vv + st.runnerValues = vv } } } - op.steps = append(op.steps, step) + + op.steps = append(op.steps, st) return nil } @@ -1192,25 +1213,35 @@ func (op *operator) runInternal(ctx context.Context) (rerr error) { // steps failed := false force := op.force - for i, s := range op.steps { + var deferred []*deferredOpAndStep + + idx := -1 + for _, s := range op.steps { + if s.deferred { + d := &deferredOpAndStep{op: op, step: s} + deferred = append([]*deferredOpAndStep{d}, deferred...) + op.deferred.steps = append([]*deferredOpAndStep{d}, op.deferred.steps...) + continue + } + idx++ if failed && !force { s.setResult(errStepSkipped) - op.recordNotRun(i) + op.recordNotRun(idx) if err := op.recordResultToLatest(resultSkipped); err != nil { return err } continue } - err := op.runStep(ctx, i, s) + err := op.runStep(ctx, idx, s) s.setResult(err) switch { case errors.Is(errStepSkipped, err): - op.recordNotRun(i) + op.recordNotRun(idx) if err := op.recordResultToLatest(resultSkipped); err != nil { return err } case err != nil: - op.recordNotRun(i) + op.recordNotRun(idx) if err := op.recordResultToLatest(resultFailure); err != nil { return err } @@ -1223,6 +1254,35 @@ func (op *operator) runInternal(ctx context.Context) (rerr error) { } } + // set index for deferred steps + for _, d := range deferred { + idx++ + d.idx = idx + } + + // deferred steps + if op.included { + return + } + + for _, os := range op.deferred.steps { + err := os.op.runStep(ctx, os.idx, os.step) + os.step.setResult(err) + switch { + case err != nil: + os.op.recordNotRun(os.idx) + if err := os.op.recordResultToLatest(resultFailure); err != nil { + return err + } + rerr = errors.Join(rerr, err) + failed = true + default: + if err := os.op.recordResultToLatest(resultSuccess); err != nil { + return err + } + } + } + return } @@ -1335,8 +1395,18 @@ func (op *operator) toOperatorN() *operatorN { func (op *operator) StepResults() []*StepResult { var results []*StepResult for _, s := range op.steps { + if lo.ContainsBy(op.deferred.steps, func(op *deferredOpAndStep) bool { + return s.runbookID() == op.step.runbookID() + }) { + continue + } results = append(results, s.result) } + for _, os := range op.deferred.steps { + if op.id == os.op.id { + results = append(results, os.step.result) + } + } return results } diff --git a/operator_test.go b/operator_test.go index 97a6c290..9182f610 100644 --- a/operator_test.go +++ b/operator_test.go @@ -209,7 +209,6 @@ func TestRun(t *testing.T) { ctx := context.Background() t.Setenv("DEBUG", "false") for _, tt := range tests { - tt := tt t.Run(tt.book, func(t *testing.T) { _, dsn := testutil.SQLite(t) t.Setenv("TEST_DB_DSN", dsn) @@ -984,7 +983,7 @@ func TestShard(t *testing.T) { cmp.AllowUnexported(allow...), cmpopts.IgnoreUnexported(ignore...), cmpopts.IgnoreFields(stopw.Span{}, "ID"), - cmpopts.IgnoreFields(operator{}, "id", "concurrency", "mu", "dbg", "needs", "nm", "maskRule", "stdout", "stderr"), + cmpopts.IgnoreFields(operator{}, "id", "concurrency", "mu", "dbg", "needs", "nm", "maskRule", "stdout", "stderr", "deferred"), cmpopts.IgnoreFields(cdpRunner{}, "ctx", "cancel", "opts", "mu", "operatorID"), cmpopts.IgnoreFields(sshRunner{}, "client", "sess", "stdin", "stdout", "stderr", "operatorID"), cmpopts.IgnoreFields(grpcRunner{}, "mu", "operatorID"), diff --git a/step.go b/step.go index c6c513dc..7880e2fe 100644 --- a/step.go +++ b/step.go @@ -11,6 +11,7 @@ type step struct { runnerKey string desc string ifCond string + deferred bool // deferred step runs after all other steps like defer in Go loop *Loop // loopIndex - Index of the loop is dynamically recorded at runtime loopIndex *int diff --git a/testdata/book/custom_runner_http.yml b/testdata/book/custom_runner_http.yml index d323f0d5..0369622d 100644 --- a/testdata/book/custom_runner_http.yml +++ b/testdata/book/custom_runner_http.yml @@ -1,4 +1,4 @@ -desc: HTTP runner with defferent syntax +desc: HTTP runner with different syntax runners: req: endpoint: '{{ parent.nodes.url }}' diff --git a/testdata/book/defer.yml b/testdata/book/defer.yml new file mode 100644 index 00000000..d6f6de6b --- /dev/null +++ b/testdata/book/defer.yml @@ -0,0 +1,31 @@ +desc: Test for defer +steps: + - + desc: step 1 + test: len(steps) == 0 + - + defer: true + desc: defererd step a + test: len(steps) == 7 + - + desc: include step + include: + path: defer_included.yml + - + defer: true + desc: defererd step b + test: false + - + defer: true + desc: defererd step c + test: len(steps) == 5 + - + desc: step 2 + test: len(steps) == 2 + - + desc: step 3 + test: false + - + desc: step 4 + test: true + diff --git a/testdata/book/defer_included.yml b/testdata/book/defer_included.yml new file mode 100644 index 00000000..3485d076 --- /dev/null +++ b/testdata/book/defer_included.yml @@ -0,0 +1,13 @@ +desc: Test for defer (included) +steps: + - + desc: included step 1 + test: len(steps) == 0 + - + defer: true + desc: included defererd step d + test: len(steps) == 2 + - + desc: included step 2 + test: len(steps) == 1 + From 7b83818db0f30b3c1c929ad757cbd1294094e582 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 15:10:28 +0900 Subject: [PATCH 2/8] Fix lint warn --- operator.go | 1 - 1 file changed, 1 deletion(-) diff --git a/operator.go b/operator.go index 234a8fed..2afc833d 100644 --- a/operator.go +++ b/operator.go @@ -1275,7 +1275,6 @@ func (op *operator) runInternal(ctx context.Context) (rerr error) { return err } rerr = errors.Join(rerr, err) - failed = true default: if err := os.op.recordResultToLatest(resultSuccess); err != nil { return err From 4374d2b6c8243079e900a0232324790572b2659b Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 15:49:16 +0900 Subject: [PATCH 3/8] To avoid confusion, do not allow the `defer` marked step to refer to the store with the `previous` key. --- bind.go | 8 ++++++-- cdp.go | 2 +- db.go | 2 +- dbg.go | 12 +++++++++--- dump.go | 8 ++++++-- exec.go | 2 +- grpc.go | 4 ++-- http.go | 2 +- include.go | 12 +++++++----- operator.go | 20 +++++++++++++------- operator_test.go | 2 +- parse.go | 22 +++++++++++----------- parse_test.go | 2 +- runner_runner.go | 2 +- ssh.go | 2 +- step.go | 2 +- test.go | 8 ++++++-- 17 files changed, 69 insertions(+), 43 deletions(-) diff --git a/bind.go b/bind.go index 1792ae90..55c4f783 100644 --- a/bind.go +++ b/bind.go @@ -25,9 +25,13 @@ func (rnr *bindRunner) Run(ctx context.Context, s *step, first bool) error { store := o.store.toMap() store[storeRootKeyIncluded] = o.included if first { - store[storeRootKeyPrevious] = o.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.latest() + } } else { - store[storeRootKeyPrevious] = o.store.previous() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.previous() + } store[storeRootKeyCurrent] = o.store.latest() } keys := lo.Keys(cond) diff --git a/cdp.go b/cdp.go index ed05c186..9d205795 100644 --- a/cdp.go +++ b/cdp.go @@ -88,7 +88,7 @@ func (rnr *cdpRunner) Renew() error { func (rnr *cdpRunner) Run(ctx context.Context, s *step) error { o := s.parent - cas, err := parseCDPActions(s.cdpActions, o.expandBeforeRecord) + cas, err := parseCDPActions(s.cdpActions, s, o.expandBeforeRecord) if err != nil { return fmt.Errorf("failed to parse: %w", err) } diff --git a/db.go b/db.go index 4a9e5997..d7173416 100644 --- a/db.go +++ b/db.go @@ -84,7 +84,7 @@ func normalizeDSN(dsn string) string { func (rnr *dbRunner) Run(ctx context.Context, s *step) error { o := s.parent - e, err := o.expandBeforeRecord(s.dbQuery) + e, err := o.expandBeforeRecord(s.dbQuery, s) if err != nil { return err } diff --git a/dbg.go b/dbg.go index 4076b6ed..cebbc85e 100644 --- a/dbg.go +++ b/dbg.go @@ -95,7 +95,9 @@ func (c *completer) do(d prompt.Document) ([]prompt.Suggest, pstrings.RuneNumber // print store := c.step.parent.store.toMap() store[storeRootKeyIncluded] = c.step.parent.included - store[storeRootKeyPrevious] = c.step.parent.store.latest() + if !c.step.deferred { + store[storeRootKeyPrevious] = c.step.parent.store.latest() + } keys := storeKeys(store) for _, k := range keys { if strings.HasPrefix(k, w) { @@ -193,7 +195,9 @@ L: } store := s.parent.store.toMapForDbg() store[storeRootKeyIncluded] = s.parent.included - store[storeRootKeyPrevious] = s.parent.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = s.parent.store.latest() + } e, err := Eval(cmd[1], store) if err != nil { _, _ = fmt.Fprintf(os.Stderr, "%s\n", err.Error()) @@ -244,7 +248,9 @@ L: case "variables", "v": store := s.parent.store.toMapForDbg() store[storeRootKeyIncluded] = s.parent.included - store[storeRootKeyPrevious] = s.parent.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = s.parent.store.latest() + } keys := lo.Keys(store) sort.Strings(keys) for _, k := range keys { diff --git a/dump.go b/dump.go index cae113a5..07bb065b 100644 --- a/dump.go +++ b/dump.go @@ -34,9 +34,13 @@ func (rnr *dumpRunner) Run(ctx context.Context, s *step, first bool) error { store := o.store.toMap() store[storeRootKeyIncluded] = o.included if first { - store[storeRootKeyPrevious] = o.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.latest() + } } else { - store[storeRootKeyPrevious] = o.store.previous() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.previous() + } store[storeRootKeyCurrent] = o.store.latest() } if r.out == "" { diff --git a/exec.go b/exec.go index 8a3c1d4f..f4a9deec 100644 --- a/exec.go +++ b/exec.go @@ -48,7 +48,7 @@ func (rnr *execRunner) Run(ctx context.Context, s *step) error { } globalScopes.mu.RUnlock() o := s.parent - e, err := o.expandBeforeRecord(s.execCommand) + e, err := o.expandBeforeRecord(s.execCommand, s) if err != nil { return err } diff --git a/grpc.go b/grpc.go index 6b070a95..01c87ad3 100644 --- a/grpc.go +++ b/grpc.go @@ -131,7 +131,7 @@ func (rnr *grpcRunner) Close() error { func (rnr *grpcRunner) Run(ctx context.Context, s *step) error { o := s.parent - req, err := parseGrpcRequest(s.grpcRequest, o.expandBeforeRecord) + req, err := parseGrpcRequest(s.grpcRequest, s, o.expandBeforeRecord) if err != nil { return err } @@ -732,7 +732,7 @@ func setHeaders(ctx context.Context, h metadata.MD) context.Context { func (rnr *grpcRunner) setMessage(req proto.Message, message map[string]any, s *step) error { o := s.parent // Lazy expand due to the possibility of computing variables between multiple messages. - e, err := o.expandBeforeRecord(message) + e, err := o.expandBeforeRecord(message, s) if err != nil { return err } diff --git a/http.go b/http.go index 4bcaf74e..80a75b27 100644 --- a/http.go +++ b/http.go @@ -374,7 +374,7 @@ func isLocalhost(domain string) (bool, error) { func (rnr *httpRunner) Run(ctx context.Context, s *step) error { o := s.parent - e, err := o.expandBeforeRecord(s.httpRequest) + e, err := o.expandBeforeRecord(s.httpRequest, s) if err != nil { return err } diff --git a/include.go b/include.go index 20c1fa5f..8898da55 100644 --- a/include.go +++ b/include.go @@ -78,7 +78,9 @@ func (rnr *includeRunner) Run(ctx context.Context, s *step) error { // Store before record store := o.store.toMap() store[storeRootKeyIncluded] = o.included - store[storeRootKeyPrevious] = o.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.latest() + } nodes, err := s.expandNodes() if err != nil { @@ -96,7 +98,7 @@ func (rnr *includeRunner) Run(ctx context.Context, s *step) error { switch ov := v.(type) { case string: var vv any - vv, err = o.expandBeforeRecord(ov) + vv, err = o.expandBeforeRecord(ov, s) if err != nil { return err } @@ -106,7 +108,7 @@ func (rnr *includeRunner) Run(ctx context.Context, s *step) error { } params[k] = evv case map[string]any, []any: - vv, err := o.expandBeforeRecord(ov) + vv, err := o.expandBeforeRecord(ov, s) if err != nil { return err } @@ -133,7 +135,7 @@ func (rnr *includeRunner) Run(ctx context.Context, s *step) error { switch ov := v.(type) { case string: var vv any - vv, err = o.expandBeforeRecord(ov) + vv, err = o.expandBeforeRecord(ov, s) if err != nil { return err } @@ -143,7 +145,7 @@ func (rnr *includeRunner) Run(ctx context.Context, s *step) error { } oo.store.vars[k] = evv case map[string]any, []any: - vv, err := o.expandBeforeRecord(ov) + vv, err := o.expandBeforeRecord(ov, s) if err != nil { return err } diff --git a/operator.go b/operator.go index 2afc833d..edca4fb8 100644 --- a/operator.go +++ b/operator.go @@ -175,7 +175,7 @@ func (op *operator) runStep(ctx context.Context, idx int, s *step) error { op.Debugln("") } if s.ifCond != "" { - tf, err := op.expandCondBeforeRecord(s.ifCond) + tf, err := op.expandCondBeforeRecord(s.ifCond, s) if err != nil { return err } @@ -345,7 +345,9 @@ func (op *operator) runStep(ctx context.Context, idx int, s *step) error { if s.loop.Until != "" { store := op.store.toMap() store[storeRootKeyIncluded] = op.included - store[storeRootKeyPrevious] = op.store.previous() + if !s.deferred { + store[storeRootKeyPrevious] = op.store.previous() + } store[storeRootKeyCurrent] = op.store.latest() tf, err := EvalWithTrace(s.loop.Until, store) if err != nil { @@ -1180,7 +1182,7 @@ func (op *operator) runInternal(ctx context.Context) (rerr error) { // if if op.ifCond != "" { - tf, err := op.expandCondBeforeRecord(op.ifCond) + tf, err := op.expandCondBeforeRecord(op.ifCond, &step{}) if err != nil { rerr = err return @@ -1313,18 +1315,22 @@ func (op *operator) stepName(i int) string { } // expandBeforeRecord - expand before the runner records the result. -func (op *operator) expandBeforeRecord(in any) (any, error) { +func (op *operator) expandBeforeRecord(in any, s *step) (any, error) { store := op.store.toMap() store[storeRootKeyIncluded] = op.included - store[storeRootKeyPrevious] = op.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = op.store.latest() + } return EvalExpand(in, store) } // expandCondBeforeRecord - expand condition before the runner records the result. -func (op *operator) expandCondBeforeRecord(ifCond string) (bool, error) { +func (op *operator) expandCondBeforeRecord(ifCond string, s *step) (bool, error) { store := op.store.toMap() store[storeRootKeyIncluded] = op.included - store[storeRootKeyPrevious] = op.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = op.store.latest() + } return EvalCond(ifCond, store) } diff --git a/operator_test.go b/operator_test.go index 9182f610..dda11212 100644 --- a/operator_test.go +++ b/operator_test.go @@ -147,7 +147,7 @@ func TestExpand(t *testing.T) { o.store.steps = tt.steps o.store.vars = tt.vars - got, err := o.expandBeforeRecord(tt.in) + got, err := o.expandBeforeRecord(tt.in, &step{}) if err != nil { t.Fatal(err) } diff --git a/parse.go b/parse.go index b2ec914c..88690938 100644 --- a/parse.go +++ b/parse.go @@ -144,7 +144,7 @@ func parseDBQuery(v map[string]any) (*dbQuery, error) { return q, nil } -func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcRequest, error) { +func parseGrpcRequest(v map[string]any, s *step, expand func(any, *step) (any, error)) (*grpcRequest, error) { v = trimDelimiter(v) req := &grpcRequest{ headers: metadata.MD{}, @@ -157,7 +157,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq return nil, fmt.Errorf("invalid request: %s", string(part)) } for k, vv := range v { - pe, err := expand(k) + pe, err := expand(k, s) if err != nil { return nil, err } @@ -177,7 +177,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq } hm, ok := vvv["headers"] if ok { - hme, err := expand(hm) + hme, err := expand(hm, s) if err != nil { return nil, err } @@ -204,7 +204,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq } tm, ok := vvv["timeout"] if ok { - tme, err := expand(tm) + tme, err := expand(tm, s) if err != nil { return nil, err } @@ -223,7 +223,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq ms, ok := mm.(string) if ok { // Only for string, variable expansion is acceptable. - mm, err = expand(ms) + mm, err = expand(ms, s) if err != nil { return nil, err } @@ -242,7 +242,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq ms, ok := mm.(string) if ok { // Only for string, variable expansion is acceptable. - mm, err = expand(ms) + mm, err = expand(ms, s) if err != nil { return nil, err } @@ -255,7 +255,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq // Only for string, variable expansion is acceptable. mms, ok := mm.(string) if ok { - mm, err = expand(mms) + mm, err = expand(mms, s) if err != nil { return nil, err } @@ -295,7 +295,7 @@ func parseGrpcRequest(v map[string]any, expand func(any) (any, error)) (*grpcReq return req, nil } -func parseCDPActions(v map[string]any, expand func(any) (any, error)) (CDPActions, error) { +func parseCDPActions(v map[string]any, s *step, expand func(any, *step) (any, error)) (CDPActions, error) { v = trimDelimiter(v) cas := CDPActions{} part, err := yaml.Marshal(v) @@ -317,7 +317,7 @@ func parseCDPActions(v map[string]any, expand func(any) (any, error)) (CDPAction ca := CDPAction{ Args: map[string]any{}, } - v, err := expand(v) + v, err := expand(v, s) if err != nil { return nil, err } @@ -352,14 +352,14 @@ func parseCDPActions(v map[string]any, expand func(any) (any, error)) (CDPAction return cas, nil } -func parseSSHCommand(v map[string]any, expand func(any) (any, error)) (*sshCommand, error) { +func parseSSHCommand(v map[string]any, s *step, expand func(any, *step) (any, error)) (*sshCommand, error) { var err error part, err := yaml.Marshal(v) if err != nil { return nil, err } v = trimDelimiter(v) - vv, err := expand(v) + vv, err := expand(v, s) if err != nil { return nil, err } diff --git a/parse_test.go b/parse_test.go index f1015f8d..32ebd61e 100644 --- a/parse_test.go +++ b/parse_test.go @@ -378,7 +378,7 @@ my.custom.server.Service/Method: if err := yaml.Unmarshal([]byte(tt.in), &v); err != nil { t.Fatal(err) } - got, err := parseGrpcRequest(v, o.expandBeforeRecord) + got, err := parseGrpcRequest(v, &step{}, o.expandBeforeRecord) if err != nil { if !tt.wantErr { t.Error(err) diff --git a/runner_runner.go b/runner_runner.go index 643d7dd1..4ed310c1 100644 --- a/runner_runner.go +++ b/runner_runner.go @@ -27,7 +27,7 @@ func (rnr *runnerRunner) Run(ctx context.Context, s *step) error { default: return fmt.Errorf("only one runner can be defined: %v", s.runnerDefinition) } - e, err := o.expandBeforeRecord(s.runnerDefinition) + e, err := o.expandBeforeRecord(s.runnerDefinition, s) if err != nil { return err } diff --git a/ssh.go b/ssh.go index 59865f67..32a3f478 100644 --- a/ssh.go +++ b/ssh.go @@ -194,7 +194,7 @@ func (rnr *sshRunner) Close() error { func (rnr *sshRunner) Run(ctx context.Context, s *step) error { o := s.parent - cmd, err := parseSSHCommand(s.sshCommand, o.expandBeforeRecord) + cmd, err := parseSSHCommand(s.sshCommand, s, o.expandBeforeRecord) if err != nil { return fmt.Errorf("invalid ssh command: %w", err) } diff --git a/step.go b/step.go index 7880e2fe..e97c29e6 100644 --- a/step.go +++ b/step.go @@ -60,7 +60,7 @@ func (s *step) expandNodes() (map[string]any, error) { return s.nodes, nil } o := s.parent - nodes, err := o.expandBeforeRecord(s.rawStep) + nodes, err := o.expandBeforeRecord(s.rawStep, s) if err != nil { return nil, err } diff --git a/test.go b/test.go index cb10eced..895a12b4 100644 --- a/test.go +++ b/test.go @@ -38,9 +38,13 @@ func (rnr *testRunner) Run(ctx context.Context, s *step, first bool) error { store := exprtrace.EvalEnv(o.store.toMap()) store[storeRootKeyIncluded] = o.included if first { - store[storeRootKeyPrevious] = o.store.latest() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.latest() + } } else { - store[storeRootKeyPrevious] = o.store.previous() + if !s.deferred { + store[storeRootKeyPrevious] = o.store.previous() + } store[storeRootKeyCurrent] = o.store.latest() } if err := rnr.run(ctx, cond, store, s, first); err != nil { From ae91ea93b433bf40292043722864aa3e4cbadf31 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 16:46:21 +0900 Subject: [PATCH 4/8] Add test for map --- defer_test.go | 144 ++++++++++++++++++++---------------- testdata/book/defer_map.yml | 31 ++++++++ 2 files changed, 111 insertions(+), 64 deletions(-) create mode 100644 testdata/book/defer_map.yml diff --git a/defer_test.go b/defer_test.go index f7d2aecd..c7547e6d 100644 --- a/defer_test.go +++ b/defer_test.go @@ -6,74 +6,90 @@ import ( ) func TestDeferRun(t *testing.T) { - book := "testdata/book/defer.yml" - ctx := context.Background() - o, err := New(Book(book)) - if err != nil { - t.Fatal(err) + tests := []struct { + book string + }{ + {"testdata/book/defer.yml"}, + {"testdata/book/defer_map.yml"}, } - if err := o.Run(ctx); err == nil { - t.Fatal("expected error") - } - if want := 8; len(o.store.steps) != want { - t.Errorf("o.store.steps got %v, want %v", len(o.store.steps), want) - } - r := o.Result() - if want := 8; len(r.StepResults) != want { - t.Errorf("r.StepResults got %v, want %v", len(r.StepResults), want) - } - - t.Run("main steps", func(t *testing.T) { - wantResults := []struct { - desc string - skipped bool - err bool - }{ - {"step 1", false, false}, - {"include step", false, false}, - {"step 2", false, false}, - {"step 3", false, true}, - {"step 4", true, false}, - {"defererd step c", false, false}, - {"defererd step b", false, true}, - {"defererd step a", false, false}, - } - for i, want := range wantResults { - got := r.StepResults[i] - if got.Desc != want.desc { - t.Errorf("got %v, want %v", got.Desc, want.desc) - } - if got.Skipped != want.skipped { - t.Errorf("got %v, want %v", got.Skipped, want.skipped) + for _, tt := range tests { + t.Run(tt.book, func(t *testing.T) { + ctx := context.Background() + o, err := New(Book(tt.book)) + if err != nil { + t.Fatal(err) } - if (got.Err == nil) == want.err { - t.Errorf("got %v, want %v", got.Err, want.err) + if err := o.Run(ctx); err == nil { + t.Fatal("expected error") } - } - }) - - t.Run("include steps", func(t *testing.T) { - wantResults := []struct { - desc string - skipped bool - err bool - }{ - {"included step 1", false, false}, - {"included step 2", false, false}, - {"included defererd step d", false, false}, - } - for i, want := range wantResults { - got := r.StepResults[1].IncludedRunResults[0].StepResults[i] - if got.Desc != want.desc { - t.Errorf("got %v, want %v", got.Desc, want.desc) + if o.useMap { + if want := 8; len(o.store.stepMap) != want { + t.Errorf("o.store.steps got %v, want %v", len(o.store.steps), want) + } + } else { + if want := 8; len(o.store.steps) != want { + t.Errorf("o.store.steps got %v, want %v", len(o.store.steps), want) + } } - if got.Skipped != want.skipped { - t.Errorf("got %v, want %v", got.Skipped, want.skipped) + r := o.Result() + if want := 8; len(r.StepResults) != want { + t.Errorf("r.StepResults got %v, want %v", len(r.StepResults), want) } - if (got.Err == nil) == want.err { - t.Errorf("got %v, want %v", got.Err, want.err) - } - } - }) + + t.Run("main steps", func(t *testing.T) { + wantResults := []struct { + desc string + skipped bool + err bool + }{ + {"step 1", false, false}, + {"include step", false, false}, + {"step 2", false, false}, + {"step 3", false, true}, + {"step 4", true, false}, + {"defererd step c", false, false}, + {"defererd step b", false, true}, + {"defererd step a", false, false}, + } + for i, want := range wantResults { + got := r.StepResults[i] + if got.Desc != want.desc { + t.Errorf("got %v, want %v", got.Desc, want.desc) + } + if got.Skipped != want.skipped { + t.Errorf("got %v, want %v", got.Skipped, want.skipped) + } + if (got.Err == nil) == want.err { + t.Errorf("got %v, want %v", got.Err, want.err) + } + } + }) + + t.Run("include steps", func(t *testing.T) { + wantResults := []struct { + desc string + skipped bool + err bool + }{ + {"included step 1", false, false}, + {"included step 2", false, false}, + {"included defererd step d", false, false}, + } + + for i, want := range wantResults { + got := r.StepResults[1].IncludedRunResults[0].StepResults[i] + if got.Desc != want.desc { + t.Errorf("got %v, want %v", got.Desc, want.desc) + } + if got.Skipped != want.skipped { + t.Errorf("got %v, want %v", got.Skipped, want.skipped) + } + if (got.Err == nil) == want.err { + t.Errorf("got %v, want %v", got.Err, want.err) + } + } + }) + }) + } } diff --git a/testdata/book/defer_map.yml b/testdata/book/defer_map.yml new file mode 100644 index 00000000..88242e61 --- /dev/null +++ b/testdata/book/defer_map.yml @@ -0,0 +1,31 @@ +desc: Test for defer +steps: + step1: + desc: step 1 + test: len(steps) == 0 + step2: + defer: true + desc: defererd step a + test: len(steps) == 7 + step3: + desc: include step + include: + path: defer_included.yml + step4: + defer: true + desc: defererd step b + test: false + step5: + defer: true + desc: defererd step c + test: len(steps) == 5 + step6: + desc: step 2 + test: len(steps) == 2 + step7: + desc: step 3 + test: false + step8: + desc: step 4 + test: true + From 55b223bb57ee9ef019704697dc508558949df390 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 17:12:11 +0900 Subject: [PATCH 5/8] Update README --- README.md | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cf90a2d5..f8068f4a 100644 --- a/README.md +++ b/README.md @@ -652,7 +652,7 @@ steps: ### `steps[*].loop:` `steps..loop:` -Loop settings for steps. +Loop setting for step. #### Simple loop step @@ -711,6 +711,28 @@ steps: [...] ``` +### `steps[*].defer:` `steps..defer:` + +Deferring setting for step. + +```yaml +steps: + - + defer: true + req: + /cart: + delete: + body: null +[...] +``` + +The step marked defer behaves as follows. + +- If `defer: true` is set, run of the step is deferred until finish of the runbook. +- Steps marked with `defer` are always run even if the running of intermediate steps fails. +- If there are multiple steps marked with `defer`, they are run in LIFO order. + - Also, the included steps are added to run sequence of the parent runbook's deferred steps. + ## Variables to be stored runn can use variables and functions when running step. From 96a1ddfc69212d0e15dc2a5ad8be4b6fe62b5920 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Tue, 17 Dec 2024 21:39:23 +0900 Subject: [PATCH 6/8] Add docs/designs/defer.md --- README.md | 2 +- docs/designs/defer.md | 133 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 docs/designs/defer.md diff --git a/README.md b/README.md index f8068f4a..1f974ce2 100644 --- a/README.md +++ b/README.md @@ -726,7 +726,7 @@ steps: [...] ``` -The step marked defer behaves as follows. +The step marked `defer` behaves as follows. - If `defer: true` is set, run of the step is deferred until finish of the runbook. - Steps marked with `defer` are always run even if the running of intermediate steps fails. diff --git a/docs/designs/defer.md b/docs/designs/defer.md new file mode 100644 index 00000000..54cf3ccb --- /dev/null +++ b/docs/designs/defer.md @@ -0,0 +1,133 @@ +# Behavior of `defer:` + +Authors: @k1low + +Status: Add continuously + +## Objective + +This document describes the `defer:` behavior in runn. + +## Backgroud + +Allow post-processing of the runbook to be described using `defer:` + +## Behavior of `defer:` + +As the name suggests, `defer:` is inspired by the [defer](https://go.dev/blog/defer-panic-and-recover) of the Go programming language. + +Behavior is also similar to the defer of the Go programming language, but some differences. + +The order of run of steps not marked `defer:` are as follows. + +```yaml +# main.yml +steps: + - desc: step 1 + test: true + - desc: step 2 + test: true + - desc: step 3 + test: true + - desc: step 4 + include: + path: include.yml + - desc: step 5 + test: true +``` + +```yaml +# include.yml +steps: + - desc: included step 1 + test: true + - desc: included step 2 + test: true + - desc: included step 3 + test: true +``` + +``` mermaid +flowchart TB + subgraph "(main.yml)" + A[step 1] + B[step 2] + C[step 3] + G[step 5] + end + + subgraph "step 4 (include.yml)" + D[included step 1] + E[included step 2] + F[included step 3] + end + + A --> B + B --> C + C --> D + D --> E + E --> F + F --> G +``` + +The steps with `defer:` are run as follows. + +```yaml +# main.yml +steps: + - desc: step 1 + test: true + - desc: step 2 + defer: true + test: true + - desc: step 3 + defer: true + test: true + - desc: step 4 + include: + path: include.yml + - desc: step 5 + test: true +``` + +```yaml +# include.yml +steps: + - desc: included step 1 + test: true + - desc: included step 2 + defer: true + test: true + - desc: included step 3 + test: true +``` + +``` mermaid +flowchart TB + subgraph "(main.yml)" + A[step 1] + B["step 2 (defer: true)"] + C["step 3 (defer: true)"] + G[step 5] + end + + subgraph "step 4 (include.yml)" + D[included step 1] + E["included step 2 (defer: true)"] + F[included step 3] + end + + A --> D + D --> F + F --> G + G --> E + E --> C + C --> B +``` + +The step marked `defer` behaves as follows. + +- If `defer: true` is set, run of the step is deferred until finish of the runbook. +- Steps marked with `defer` are always run even if the running of intermediate steps fails. +- If there are multiple steps marked with `defer`, they are run in LIFO order. + - Also, the included steps are added to run sequence of the parent runbook's deferred steps. From 23a54b7b8392d2519460250d6aa88fbe929c02f2 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Thu, 19 Dec 2024 09:36:55 +0900 Subject: [PATCH 7/8] Fix lint warn --- store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/store.go b/store.go index b359015d..e3a1f134 100644 --- a/store.go +++ b/store.go @@ -296,7 +296,7 @@ func (s *store) toMap() map[string]any { } // toMapForIncludeRunner - returns a map for include runner. -// toMap without s.parentVars and s.needsVars and runn +// toMap without s.parentVars and s.needsVars. func (s *store) toMapForIncludeRunner() map[string]any { store := map[string]any{} store[storeRootKeyEnv] = envMap() From e647346ddc7a214867cc0a4a173bfb25d70cc675 Mon Sep 17 00:00:00 2001 From: k1LoW Date: Thu, 19 Dec 2024 09:52:09 +0900 Subject: [PATCH 8/8] Add experiment mark --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1f974ce2..7bbddd40 100644 --- a/README.md +++ b/README.md @@ -711,7 +711,7 @@ steps: [...] ``` -### `steps[*].defer:` `steps..defer:` +### `steps[*].defer:` `steps..defer:` [THIS IS EXPERIMENT] Deferring setting for step.