fix(executor): action steps fail with the runner's null ContainerId error when the job container does not exist
- commit
- 59a9c3a500d0b798b3af5bebb7d3e044c8cfea46
- author
- brice · · 2026-10-14 05:58 UTC
- parent
- 67ae5e1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +47 -0
M internal/executor/actions_run.go +4 -0
| @@ -175,6 +175,10 @@ func (j *job) runAction(s *stepRun, cancel <-chan struct{}) stepResult { | |||
| 175 | 175 | j.registerPost(s, def) | |
| 176 | 176 | } | |
| 177 | 177 | j.setActionRepository(n) | |
| 178 | + | // ActionRunner.cs:157-164: with a job container, the container must exist. | |
| 179 | + | if err := j.containerReady(); err != nil { | |
| 180 | + | return fail(err) | |
| 181 | + | } | |
| 178 | 182 | if err := j.prepareFileCommands(s); err != nil { | |
| 179 | 183 | return fail(err) | |
| 180 | 184 | } | |
| @@ -175,6 +175,10 @@ func (j *job) runAction(s *stepRun, cancel <-chan struct{}) stepResult { | |||||
| 175 | j.registerPost(s, def) | 175 | j.registerPost(s, def) | ||
| 176 | } | 176 | } | ||
| 177 | j.setActionRepository(n) | 177 | j.setActionRepository(n) | ||
| 178 | + | // ActionRunner.cs:157-164: with a job container, the container must exist. | |||
| 179 | + | if err := j.containerReady(); err != nil { | |||
| 180 | + | return fail(err) | |||
| 181 | + | } | |||
| 178 | if err := j.prepareFileCommands(s); err != nil { | 182 | if err := j.prepareFileCommands(s); err != nil { | ||
| 179 | return fail(err) | 183 | return fail(err) | ||
| 180 | } | 184 | } | ||
M internal/executor/containers_life_test.go +20 -0
| @@ -171,3 +171,23 @@ func TestStopContainersRunsAfterAFailedStepAndPostStepsRunInTheContainer(t *test | |||
| 171 | 171 | t.Errorf("both steps run through docker exec: %v", f.execCalls(t)) | |
| 172 | 172 | } | |
| 173 | 173 | } | |
| 174 | + | ||
| 175 | + | // After a failed "Initialize containers" an always() step has no container: | |
| 176 | + | // the runner fails it before its header with a null ContainerId (recorded: | |
| 177 | + | // the ermite-dump step of ctr-errors). | |
| 178 | + | func TestStepAfterFailedInitFailsWithNullContainerId(t *testing.T) { | |
| 179 | + | f := newFakeCtr(t) | |
| 180 | + | f.set(t, "create_fail", "1") | |
| 181 | + | tree := files{".github/actions/x/action.yml": "name: x\nruns:\n using: node20\n main: i.js\n", ".github/actions/x/i.js": "x\n"} | |
| 182 | + | rec, res := ctrRun(t, f, ctrJob, " - name: run\n if: always()\n run: echo no\n - name: act\n if: always()\n uses: ./.github/actions/x\n", func(c *Config) { | |
| 183 | + | writeFiles(t, c.Layout.Workspace, tree) | |
| 184 | + | }) | |
| 185 | + | if res.Conclusion != ConclusionFailure { | |
| 186 | + | t.Fatalf("%+v", res) | |
| 187 | + | } | |
| 188 | + | for _, n := range []string{"run", "act"} { | |
| 189 | + | if got := logOf(rec.step(t, n)); got != "##[error]Value cannot be null. (Parameter 'ContainerId')" || rec.concl(n) != ConclusionFailure { | |
| 190 | + | t.Errorf("%s: %q %s", n, got, rec.concl(n)) | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | } | |
| @@ -171,3 +171,23 @@ func TestStopContainersRunsAfterAFailedStepAndPostStepsRunInTheContainer(t *test | |||||
| 171 | t.Errorf("both steps run through docker exec: %v", f.execCalls(t)) | 171 | t.Errorf("both steps run through docker exec: %v", f.execCalls(t)) | ||
| 172 | } | 172 | } | ||
| 173 | } | 173 | } | ||
| 174 | + | ||||
| 175 | + | // After a failed "Initialize containers" an always() step has no container: | |||
| 176 | + | // the runner fails it before its header with a null ContainerId (recorded: | |||
| 177 | + | // the ermite-dump step of ctr-errors). | |||
| 178 | + | func TestStepAfterFailedInitFailsWithNullContainerId(t *testing.T) { | |||
| 179 | + | f := newFakeCtr(t) | |||
| 180 | + | f.set(t, "create_fail", "1") | |||
| 181 | + | tree := files{".github/actions/x/action.yml": "name: x\nruns:\n using: node20\n main: i.js\n", ".github/actions/x/i.js": "x\n"} | |||
| 182 | + | rec, res := ctrRun(t, f, ctrJob, " - name: run\n if: always()\n run: echo no\n - name: act\n if: always()\n uses: ./.github/actions/x\n", func(c *Config) { | |||
| 183 | + | writeFiles(t, c.Layout.Workspace, tree) | |||
| 184 | + | }) | |||
| 185 | + | if res.Conclusion != ConclusionFailure { | |||
| 186 | + | t.Fatalf("%+v", res) | |||
| 187 | + | } | |||
| 188 | + | for _, n := range []string{"run", "act"} { | |||
| 189 | + | if got := logOf(rec.step(t, n)); got != "##[error]Value cannot be null. (Parameter 'ContainerId')" || rec.concl(n) != ConclusionFailure { | |||
| 190 | + | t.Errorf("%s: %q %s", n, got, rec.concl(n)) | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | } | |||
A internal/executor/zz_dump_test.go +23 -0
| @@ -0,0 +1,23 @@ | |||
| 1 | + | package executor | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "os" | |
| 5 | + | "strings" | |
| 6 | + | "testing" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | func TestZZDump(t *testing.T) { | |
| 10 | + | docker := realDocker(t) | |
| 11 | + | y, _ := os.ReadFile(os.Getenv("DUMP_YAML")) | |
| 12 | + | rec, _ := runLocal(t, string(y), os.Getenv("DUMP_JOB"), func(c *Config) { | |
| 13 | + | c.Actions.DockerPath = docker | |
| 14 | + | c.BaseEnv = append(c.BaseEnv, "XDG_RUNTIME_DIR="+os.Getenv("XDG_RUNTIME_DIR")) | |
| 15 | + | c.Actions.ExternalsDir = fakeNode(t) | |
| 16 | + | c.Runner.Environment = "github-hosted" | |
| 17 | + | }) | |
| 18 | + | var b strings.Builder | |
| 19 | + | for _, s := range rec.steps { | |
| 20 | + | b.WriteString("=== STEP " + s.Name + " " + string(s.Conclusion) + "\n" + logOf(s) + "\n") | |
| 21 | + | } | |
| 22 | + | os.WriteFile(os.Getenv("DUMP_OUT"), []byte(b.String()), 0o644) | |
| 23 | + | } | |
| @@ -0,0 +1,23 @@ | |||||
| 1 | + | package executor | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "os" | |||
| 5 | + | "strings" | |||
| 6 | + | "testing" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | func TestZZDump(t *testing.T) { | |||
| 10 | + | docker := realDocker(t) | |||
| 11 | + | y, _ := os.ReadFile(os.Getenv("DUMP_YAML")) | |||
| 12 | + | rec, _ := runLocal(t, string(y), os.Getenv("DUMP_JOB"), func(c *Config) { | |||
| 13 | + | c.Actions.DockerPath = docker | |||
| 14 | + | c.BaseEnv = append(c.BaseEnv, "XDG_RUNTIME_DIR="+os.Getenv("XDG_RUNTIME_DIR")) | |||
| 15 | + | c.Actions.ExternalsDir = fakeNode(t) | |||
| 16 | + | c.Runner.Environment = "github-hosted" | |||
| 17 | + | }) | |||
| 18 | + | var b strings.Builder | |||
| 19 | + | for _, s := range rec.steps { | |||
| 20 | + | b.WriteString("=== STEP " + s.Name + " " + string(s.Conclusion) + "\n" + logOf(s) + "\n") | |||
| 21 | + | } | |||
| 22 | + | os.WriteFile(os.Getenv("DUMP_OUT"), []byte(b.String()), 0o644) | |||
| 23 | + | } | |||