fix(executor): stop the image setup of a local composite action with the step
- commit
- dd71bec30e83d31469e0c053411e67230a7ad9f8
- author
- brice · · 2026-10-14 02:47 UTC
- parent
- 9f6323c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +98 -4
A internal/executor/actions_cancel_test.go +61 -0
| @@ -0,0 +1,61 @@ | |||
| 1 | + | // SPDX-License-Identifier: MIT | |
| 2 | + | ||
| 3 | + | package executor | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "os" | |
| 7 | + | "path/filepath" | |
| 8 | + | "strings" | |
| 9 | + | "testing" | |
| 10 | + | "time" | |
| 11 | + | ) | |
| 12 | + | ||
| 13 | + | // GO-006: the image setup of a local composite action (pull, retry sleep) stops | |
| 14 | + | // with the step. | |
| 15 | + | func TestLocalCompositeSetupIsCancellable(t *testing.T) { | |
| 16 | + | needTools(t, "bash") | |
| 17 | + | dir := t.TempDir() | |
| 18 | + | pulls := filepath.Join(dir, "pulls") | |
| 19 | + | script := "#!/bin/sh\necho x >> " + pulls + "\necho ready\nexit 1\n" | |
| 20 | + | if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil { | |
| 21 | + | t.Fatal(err) | |
| 22 | + | } | |
| 23 | + | c := NewCanceller() | |
| 24 | + | started := make(chan struct{}) | |
| 25 | + | go func() { | |
| 26 | + | <-started | |
| 27 | + | c.Cancel(CancelUser) | |
| 28 | + | }() | |
| 29 | + | home := t.TempDir() | |
| 30 | + | layout := NewLayout(home, "hello", home+"/tc") | |
| 31 | + | writeFiles(t, layout.Workspace, files{"c/action.yml": "name: c\nruns:\n using: composite\n steps:\n - uses: docker://alpine:3\n"}) | |
| 32 | + | rec := newRecorder() | |
| 33 | + | rep := &signalReporter{recorder: rec, started: started, match: "ready"} | |
| 34 | + | done := make(chan Result, 1) | |
| 35 | + | go func() { | |
| 36 | + | done <- Run(c, Config{ | |
| 37 | + | WorkflowYAML: []byte(wfHead + " steps:\n - uses: ./c\n - if: always()\n run: echo cleanup-ran\n"), | |
| 38 | + | JobKey: "j", ContextsJSON: testGitHubContext("j"), Layout: &layout, Reporter: rep, | |
| 39 | + | BaseEnv: []string{"PATH=" + os.Getenv("PATH"), "HOME=" + home}, | |
| 40 | + | TermAfter: 200 * time.Millisecond, KillAfter: 200 * time.Millisecond, WaitDelay: 500 * time.Millisecond, | |
| 41 | + | Actions: ActionsConfig{DockerPath: filepath.Join(dir, "docker"), Backoff: func() time.Duration { return time.Hour }}, | |
| 42 | + | }) | |
| 43 | + | }() | |
| 44 | + | select { | |
| 45 | + | case res := <-done: | |
| 46 | + | if res.Conclusion != ConclusionCancelled { | |
| 47 | + | t.Fatalf("%+v", res) | |
| 48 | + | } | |
| 49 | + | case <-time.After(20 * time.Second): | |
| 50 | + | t.Fatal("the retry sleep of the pull ignored the cancellation") | |
| 51 | + | } | |
| 52 | + | if b, _ := os.ReadFile(pulls); strings.Count(string(b), "x") != 1 { | |
| 53 | + | t.Errorf("no retry after the cancellation: %q", b) | |
| 54 | + | } | |
| 55 | + | if got := rec.step(t, "Run ./c").Conclusion; got != ConclusionCancelled { | |
| 56 | + | t.Errorf("step %s", got) | |
| 57 | + | } | |
| 58 | + | if got := rec.step(t, "Run echo cleanup-ran").Conclusion; got != ConclusionSuccess { | |
| 59 | + | t.Errorf("always() step %s", got) | |
| 60 | + | } | |
| 61 | + | } | |
| @@ -0,0 +1,61 @@ | |||||
| 1 | + | // SPDX-License-Identifier: MIT | |||
| 2 | + | ||||
| 3 | + | package executor | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "os" | |||
| 7 | + | "path/filepath" | |||
| 8 | + | "strings" | |||
| 9 | + | "testing" | |||
| 10 | + | "time" | |||
| 11 | + | ) | |||
| 12 | + | ||||
| 13 | + | // GO-006: the image setup of a local composite action (pull, retry sleep) stops | |||
| 14 | + | // with the step. | |||
| 15 | + | func TestLocalCompositeSetupIsCancellable(t *testing.T) { | |||
| 16 | + | needTools(t, "bash") | |||
| 17 | + | dir := t.TempDir() | |||
| 18 | + | pulls := filepath.Join(dir, "pulls") | |||
| 19 | + | script := "#!/bin/sh\necho x >> " + pulls + "\necho ready\nexit 1\n" | |||
| 20 | + | if err := os.WriteFile(filepath.Join(dir, "docker"), []byte(script), 0o755); err != nil { | |||
| 21 | + | t.Fatal(err) | |||
| 22 | + | } | |||
| 23 | + | c := NewCanceller() | |||
| 24 | + | started := make(chan struct{}) | |||
| 25 | + | go func() { | |||
| 26 | + | <-started | |||
| 27 | + | c.Cancel(CancelUser) | |||
| 28 | + | }() | |||
| 29 | + | home := t.TempDir() | |||
| 30 | + | layout := NewLayout(home, "hello", home+"/tc") | |||
| 31 | + | writeFiles(t, layout.Workspace, files{"c/action.yml": "name: c\nruns:\n using: composite\n steps:\n - uses: docker://alpine:3\n"}) | |||
| 32 | + | rec := newRecorder() | |||
| 33 | + | rep := &signalReporter{recorder: rec, started: started, match: "ready"} | |||
| 34 | + | done := make(chan Result, 1) | |||
| 35 | + | go func() { | |||
| 36 | + | done <- Run(c, Config{ | |||
| 37 | + | WorkflowYAML: []byte(wfHead + " steps:\n - uses: ./c\n - if: always()\n run: echo cleanup-ran\n"), | |||
| 38 | + | JobKey: "j", ContextsJSON: testGitHubContext("j"), Layout: &layout, Reporter: rep, | |||
| 39 | + | BaseEnv: []string{"PATH=" + os.Getenv("PATH"), "HOME=" + home}, | |||
| 40 | + | TermAfter: 200 * time.Millisecond, KillAfter: 200 * time.Millisecond, WaitDelay: 500 * time.Millisecond, | |||
| 41 | + | Actions: ActionsConfig{DockerPath: filepath.Join(dir, "docker"), Backoff: func() time.Duration { return time.Hour }}, | |||
| 42 | + | }) | |||
| 43 | + | }() | |||
| 44 | + | select { | |||
| 45 | + | case res := <-done: | |||
| 46 | + | if res.Conclusion != ConclusionCancelled { | |||
| 47 | + | t.Fatalf("%+v", res) | |||
| 48 | + | } | |||
| 49 | + | case <-time.After(20 * time.Second): | |||
| 50 | + | t.Fatal("the retry sleep of the pull ignored the cancellation") | |||
| 51 | + | } | |||
| 52 | + | if b, _ := os.ReadFile(pulls); strings.Count(string(b), "x") != 1 { | |||
| 53 | + | t.Errorf("no retry after the cancellation: %q", b) | |||
| 54 | + | } | |||
| 55 | + | if got := rec.step(t, "Run ./c").Conclusion; got != ConclusionCancelled { | |||
| 56 | + | t.Errorf("step %s", got) | |||
| 57 | + | } | |||
| 58 | + | if got := rec.step(t, "Run echo cleanup-ran").Conclusion; got != ConclusionSuccess { | |||
| 59 | + | t.Errorf("always() step %s", got) | |||
| 60 | + | } | |||
| 61 | + | } | |||
M internal/executor/actions_composite.go +33 -3
| @@ -3,6 +3,8 @@ | |||
| 3 | 3 | package executor | |
| 4 | 4 | ||
| 5 | 5 | import ( | |
| 6 | + | "context" | |
| 7 | + | ||
| 6 | 8 | "bnema.dev/ermite-actions/expr" | |
| 7 | 9 | ) | |
| 8 | 10 | ||
| @@ -21,11 +23,19 @@ func compositeGitHubKeys() []string { | |||
| 21 | 23 | // composite action (ActionRunner.cs:77-96): its steps are prepared when the | |
| 22 | 24 | // step runs, in the log of the step, and the pulls and builds they need run | |
| 23 | 25 | // first as embedded steps. | |
| 24 | - | func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | |
| 26 | + | func (j *job) prepareLocalComposite(s *stepRun, def *actionDef, cancel <-chan struct{}) error { | |
| 25 | 27 | n := s.n | |
| 26 | 28 | if n.children != nil { | |
| 27 | 29 | return nil // pre and post stages reuse the nodes of the main stage | |
| 28 | 30 | } | |
| 31 | + | // Depth restarts at 1 for the steps of a local composite action, as in the | |
| 32 | + | // runner (ActionRunner.cs:77-96 calls PrepareActionsAsync with depth 1), so a | |
| 33 | + | // chain of local composites has no depth limit here | |
| 34 | + | // (TestLocalCompositeChainHasNoDepthLimit). Whether github.com limits it is | |
| 35 | + | // to be settled by the recording of ermite-conformance | |
| 36 | + | // corpus/act-composite-depth. Every level that needs a resolution is counted | |
| 37 | + | // by the host's per-job budget (host/resolve.go), so the server is safe | |
| 38 | + | // whatever is decided. | |
| 29 | 39 | kids, err := newNodes(def.act.Runs.Steps, n, n.scope) | |
| 30 | 40 | if err != nil { | |
| 31 | 41 | return err | |
| @@ -39,14 +49,30 @@ func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | |||
| 39 | 49 | saved := *j.actions | |
| 40 | 50 | j.actions.pullOrder, j.actions.buildOrder = nil, nil | |
| 41 | 51 | j.actions.pullUsers, j.actions.buildUsers = map[string][]*node{}, map[string][]*node{} | |
| 42 | - | if err := j.prepare(j.ctx(), s.num, kids, 1, n); err != nil { | |
| 52 | + | // The preparation stops with the step: its context ends when the step is | |
| 53 | + | // cancelled (and with the job, whose context it derives from). | |
| 54 | + | ctx, stop := context.WithCancel(j.ctx()) | |
| 55 | + | defer stop() | |
| 56 | + | if cancel != nil { | |
| 57 | + | go func() { | |
| 58 | + | select { | |
| 59 | + | case <-cancel: | |
| 60 | + | stop() | |
| 61 | + | case <-ctx.Done(): | |
| 62 | + | } | |
| 63 | + | }() | |
| 64 | + | } | |
| 65 | + | if err := j.prepare(ctx, s.num, kids, 1, n); err != nil { | |
| 43 | 66 | return err | |
| 44 | 67 | } | |
| 45 | 68 | setups := j.setupSteps() | |
| 46 | 69 | j.actions.pullOrder, j.actions.buildOrder = saved.pullOrder, saved.buildOrder | |
| 47 | 70 | j.actions.pullUsers, j.actions.buildUsers = saved.pullUsers, saved.buildUsers | |
| 48 | 71 | for _, q := range setups { | |
| 49 | - | if r := q.setup(s, nil); r != resSucceeded { | |
| 72 | + | if r := q.setup(s, cancel); r != resSucceeded { | |
| 73 | + | if r == resCanceled { | |
| 74 | + | return errSetupCancelled | |
| 75 | + | } | |
| 50 | 76 | return errSetupFailed | |
| 51 | 77 | } | |
| 52 | 78 | } | |
| @@ -55,6 +81,10 @@ func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | |||
| 55 | 81 | ||
| 56 | 82 | // errSetupFailed ends a composite whose image setup failed; the cause is in the | |
| 57 | 83 | // log already. | |
| 84 | + | // errSetupCancelled: the step was cancelled while its images were prepared; the | |
| 85 | + | // cancellation is logged by the docker command already. | |
| 86 | + | var errSetupCancelled = ghErrorf("The operation was canceled.") | |
| 87 | + | ||
| 58 | 88 | var errSetupFailed = ghErrorf("A container setup step of the composite action failed.") | |
| 59 | 89 | ||
| 60 | 90 | // runComposite is CompositeActionHandler.RunAsync (CompositeActionHandler.cs: | |
| @@ -3,6 +3,8 @@ | |||||
| 3 | package executor | 3 | package executor | ||
| 4 | 4 | ||||
| 5 | import ( | 5 | import ( | ||
| 6 | + | "context" | |||
| 7 | + | ||||
| 6 | "bnema.dev/ermite-actions/expr" | 8 | "bnema.dev/ermite-actions/expr" | ||
| 7 | ) | 9 | ) | ||
| 8 | 10 | ||||
| @@ -21,11 +23,19 @@ func compositeGitHubKeys() []string { | |||||
| 21 | // composite action (ActionRunner.cs:77-96): its steps are prepared when the | 23 | // composite action (ActionRunner.cs:77-96): its steps are prepared when the | ||
| 22 | // step runs, in the log of the step, and the pulls and builds they need run | 24 | // step runs, in the log of the step, and the pulls and builds they need run | ||
| 23 | // first as embedded steps. | 25 | // first as embedded steps. | ||
| 24 | - | func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | 26 | + | func (j *job) prepareLocalComposite(s *stepRun, def *actionDef, cancel <-chan struct{}) error { |
| 25 | n := s.n | 27 | n := s.n | ||
| 26 | if n.children != nil { | 28 | if n.children != nil { | ||
| 27 | return nil // pre and post stages reuse the nodes of the main stage | 29 | return nil // pre and post stages reuse the nodes of the main stage | ||
| 28 | } | 30 | } | ||
| 31 | + | // Depth restarts at 1 for the steps of a local composite action, as in the | |||
| 32 | + | // runner (ActionRunner.cs:77-96 calls PrepareActionsAsync with depth 1), so a | |||
| 33 | + | // chain of local composites has no depth limit here | |||
| 34 | + | // (TestLocalCompositeChainHasNoDepthLimit). Whether github.com limits it is | |||
| 35 | + | // to be settled by the recording of ermite-conformance | |||
| 36 | + | // corpus/act-composite-depth. Every level that needs a resolution is counted | |||
| 37 | + | // by the host's per-job budget (host/resolve.go), so the server is safe | |||
| 38 | + | // whatever is decided. | |||
| 29 | kids, err := newNodes(def.act.Runs.Steps, n, n.scope) | 39 | kids, err := newNodes(def.act.Runs.Steps, n, n.scope) | ||
| 30 | if err != nil { | 40 | if err != nil { | ||
| 31 | return err | 41 | return err | ||
| @@ -39,14 +49,30 @@ func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | |||||
| 39 | saved := *j.actions | 49 | saved := *j.actions | ||
| 40 | j.actions.pullOrder, j.actions.buildOrder = nil, nil | 50 | j.actions.pullOrder, j.actions.buildOrder = nil, nil | ||
| 41 | j.actions.pullUsers, j.actions.buildUsers = map[string][]*node{}, map[string][]*node{} | 51 | j.actions.pullUsers, j.actions.buildUsers = map[string][]*node{}, map[string][]*node{} | ||
| 42 | - | if err := j.prepare(j.ctx(), s.num, kids, 1, n); err != nil { | 52 | + | // The preparation stops with the step: its context ends when the step is |
| 53 | + | // cancelled (and with the job, whose context it derives from). | |||
| 54 | + | ctx, stop := context.WithCancel(j.ctx()) | |||
| 55 | + | defer stop() | |||
| 56 | + | if cancel != nil { | |||
| 57 | + | go func() { | |||
| 58 | + | select { | |||
| 59 | + | case <-cancel: | |||
| 60 | + | stop() | |||
| 61 | + | case <-ctx.Done(): | |||
| 62 | + | } | |||
| 63 | + | }() | |||
| 64 | + | } | |||
| 65 | + | if err := j.prepare(ctx, s.num, kids, 1, n); err != nil { | |||
| 43 | return err | 66 | return err | ||
| 44 | } | 67 | } | ||
| 45 | setups := j.setupSteps() | 68 | setups := j.setupSteps() | ||
| 46 | j.actions.pullOrder, j.actions.buildOrder = saved.pullOrder, saved.buildOrder | 69 | j.actions.pullOrder, j.actions.buildOrder = saved.pullOrder, saved.buildOrder | ||
| 47 | j.actions.pullUsers, j.actions.buildUsers = saved.pullUsers, saved.buildUsers | 70 | j.actions.pullUsers, j.actions.buildUsers = saved.pullUsers, saved.buildUsers | ||
| 48 | for _, q := range setups { | 71 | for _, q := range setups { | ||
| 49 | - | if r := q.setup(s, nil); r != resSucceeded { | 72 | + | if r := q.setup(s, cancel); r != resSucceeded { |
| 73 | + | if r == resCanceled { | |||
| 74 | + | return errSetupCancelled | |||
| 75 | + | } | |||
| 50 | return errSetupFailed | 76 | return errSetupFailed | ||
| 51 | } | 77 | } | ||
| 52 | } | 78 | } | ||
| @@ -55,6 +81,10 @@ func (j *job) prepareLocalComposite(s *stepRun, def *actionDef) error { | |||||
| 55 | 81 | ||||
| 56 | // errSetupFailed ends a composite whose image setup failed; the cause is in the | 82 | // errSetupFailed ends a composite whose image setup failed; the cause is in the | ||
| 57 | // log already. | 83 | // log already. | ||
| 84 | + | // errSetupCancelled: the step was cancelled while its images were prepared; the | |||
| 85 | + | // cancellation is logged by the docker command already. | |||
| 86 | + | var errSetupCancelled = ghErrorf("The operation was canceled.") | |||
| 87 | + | ||||
| 58 | var errSetupFailed = ghErrorf("A container setup step of the composite action failed.") | 88 | var errSetupFailed = ghErrorf("A container setup step of the composite action failed.") | ||
| 59 | 89 | ||||
| 60 | // runComposite is CompositeActionHandler.RunAsync (CompositeActionHandler.cs: | 90 | // runComposite is CompositeActionHandler.RunAsync (CompositeActionHandler.cs: | ||
M internal/executor/actions_run.go +4 -1
| @@ -153,7 +153,10 @@ func (j *job) runAction(s *stepRun, cancel <-chan struct{}) stepResult { | |||
| 153 | 153 | local := n.uses.Kind == action.UsesLocal | |
| 154 | 154 | // A local composite action is expanded when it runs (ActionRunner.cs:77-96). | |
| 155 | 155 | if def.kind == kindComposite && s.stage == stageMain && local { | |
| 156 | - | if err := j.prepareLocalComposite(s, def); err != nil { | |
| 156 | + | if err := j.prepareLocalComposite(s, def, cancel); err != nil { | |
| 157 | + | if err == errSetupCancelled { | |
| 158 | + | return resCanceled // logged by the command that was stopped | |
| 159 | + | } | |
| 157 | 160 | return fail(err) | |
| 158 | 161 | } | |
| 159 | 162 | } | |
| @@ -153,7 +153,10 @@ func (j *job) runAction(s *stepRun, cancel <-chan struct{}) stepResult { | |||||
| 153 | local := n.uses.Kind == action.UsesLocal | 153 | local := n.uses.Kind == action.UsesLocal | ||
| 154 | // A local composite action is expanded when it runs (ActionRunner.cs:77-96). | 154 | // A local composite action is expanded when it runs (ActionRunner.cs:77-96). | ||
| 155 | if def.kind == kindComposite && s.stage == stageMain && local { | 155 | if def.kind == kindComposite && s.stage == stageMain && local { | ||
| 156 | - | if err := j.prepareLocalComposite(s, def); err != nil { | 156 | + | if err := j.prepareLocalComposite(s, def, cancel); err != nil { |
| 157 | + | if err == errSetupCancelled { | |||
| 158 | + | return resCanceled // logged by the command that was stopped | |||
| 159 | + | } | |||
| 157 | return fail(err) | 160 | return fail(err) | ||
| 158 | } | 161 | } | ||
| 159 | } | 162 | } | ||