fix(executor): bound every docker command, clean up on an independent budget, record cleanup identifiers first and add a job-level fallback (GO-001, GO-002, GO-003, GO-007)

commit: 493a3ece4775a7981bc4826b16efc38692751765
author: brice · 8d ago · 2026-10-14 06:29 UTC
parent: 8e03ed6
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

9 files · +560 -68

M internal/executor/actions_types.go +3 -0


@@ -46,6 +46,9 @@ type ActionsConfig struct {
46	46		// (HandlerFactory.cs, NodeUtil.DetermineActionsNodeVersion): ermite has no
47	47		// feature flags, so the default is pinned here.
48	48		Node24ByDefault bool
	49	+	// Containers are the time limits of the docker commands of job containers
	50	+	// and services; the zero value means the defaults.
	51	+	Containers ContainerTimeouts
49	52		// Backoff, when set, replaces the random 10 to 30 s wait between download
50	53		// attempts (tests).
51	54		Backoff func() time.Duration


@@ -46,6 +46,9 @@ type ActionsConfig struct {
46	// (HandlerFactory.cs, NodeUtil.DetermineActionsNodeVersion): ermite has no	46		// (HandlerFactory.cs, NodeUtil.DetermineActionsNodeVersion): ermite has no
47	// feature flags, so the default is pinned here.	47		// feature flags, so the default is pinned here.
48	Node24ByDefault bool	48		Node24ByDefault bool
		49	+	// Containers are the time limits of the docker commands of job containers
		50	+	// and services; the zero value means the defaults.
		51	+	Containers ContainerTimeouts
49	// Backoff, when set, replaces the random 10 to 30 s wait between download	52		// Backoff, when set, replaces the random 10 to 30 s wait between download
50	// attempts (tests).	53		// attempts (tests).
51	Backoff func() time.Duration	54		Backoff func() time.Duration

A internal/executor/containers_cleanup_test.go +203 -0

@@ -0,0 +1,203 @@
	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	+	func fastTimeouts(c *Config) {
	14	+	c.Actions.Containers = ContainerTimeouts{Command: 400 * time.Millisecond, Pull: 400 * time.Millisecond, CleanupCommand: 400 * time.Millisecond, CleanupBudget: 3 * time.Second}
	15	+	}
	16	+
	17	+	func names(l [][]string) string {
	18	+	var v []string
	19	+	for _, c := range l {
	20	+	v = append(v, c[0])
	21	+	}
	22	+	return strings.Join(v, " ")
	23	+	}
	24	+
	25	+	// GO-001: a docker command that stalls is stopped by its own limit, the job
	26	+	// does not wait for its timeout, and the cleanup still finds the container by
	27	+	// the name chosen before the create.
	28	+	func TestStalledCreateIsBoundedAndCleanedByName(t *testing.T) {
	29	+	f := newFakeCtr(t)
	30	+	f.set(t, "hang_create", "1")
	31	+	start := time.Now()
	32	+	rec, res := ctrRun(t, f, ctrJob, " - run: echo never\n", fastTimeouts)
	33	+	if res.Conclusion != ConclusionFailure \|\| time.Since(start) > 20*time.Second {
	34	+	t.Fatalf("%+v after %s", res, time.Since(start))
	35	+	}
	36	+	log := logOf(rec.step(t, "Initialize containers"))
	37	+	if !strings.Contains(log, "##[error]The docker command timed out after 400ms: create --name ") {
	38	+	t.Errorf("init:\n%s", log)
	39	+	}
	40	+	var rm []string
	41	+	for _, c := range f.calls(t) {
	42	+	if c[0] == "rm" {
	43	+	rm = c
	44	+	}
	45	+	}
	46	+	if len(rm) < 3 \|\| !strings.Contains(rm[2], "_ubuntu2404_") \|\| strings.Contains(rm[2], strings.Repeat("0", 20)) {
	47	+	t.Errorf("rm by name expected: %v", rm)
	48	+	}
	49	+	if got := logOf(rec.step(t, "Stop containers")); !strings.Contains(got, "Remove container network: github_network_") {
	50	+	t.Errorf("stop:\n%s", got)
	51	+	}
	52	+	}
	53	+
	54	+	// GO-001: the cancellation of the job reaches every docker call of the init.
	55	+	func TestCancelDuringInitStopsTheCommandAndCleans(t *testing.T) {
	56	+	for _, verb := range []string{"version", "inspect", "port", "start", "ps"} {
	57	+	f := newFakeCtr(t)
	58	+	f.set(t, "hang_"+verb, "1")
	59	+	cancel := NewCanceller()
	60	+	go func() { time.Sleep(500 * time.Millisecond); cancel.Cancel(CancelUser) }()
	61	+	start := time.Now()
	62	+	rec, res := ctrRunC(t, f, cancel, " container: ubuntu:24.04\n services:\n s:\n image: redis:7\n", " - run: echo never\n", func(c *Config) {
	63	+	c.Actions.Containers.Command = time.Minute
	64	+	})
	65	+	if res.Conclusion != ConclusionCancelled \|\| time.Since(start) > 15*time.Second {
	66	+	t.Fatalf("%s: %+v after %s %v", verb, res, time.Since(start), rec.names())
	67	+	}
	68	+	if got := rec.concl("Stop containers"); got != ConclusionSuccess {
	69	+	t.Errorf("%s: Stop containers %s", verb, got)
	70	+	}
	71	+	}
	72	+	}
	73	+
	74	+	// GO-002: one removal that stalls does not keep the others and the network from
	75	+	// being removed; the logs of a service never prevent its removal; the
	76	+	// failures are reported by the step.
	77	+	func TestCleanupAttemptsEverythingWithItsOwnBudget(t *testing.T) {
	78	+	f := newFakeCtr(t)
	79	+	f.set(t, "hang_logs", "1")
	80	+	f.set(t, "hang_for", "rm --force "+strings.Repeat("0", 63)+"1") // the rm of the job container (first id)
	81	+	cancel := NewCanceller()
	82	+	rec, res := ctrRunC(t, f, cancel, " container: ubuntu:24.04\n services:\n a:\n image: redis:7\n b:\n image: redis:7\n", " - run: echo ok\n", fastTimeouts)
	83	+	if res.Conclusion == ConclusionSuccess {
	84	+	t.Fatalf("a failed cleanup must be visible: %+v", res)
	85	+	}
	86	+	stop := logOf(rec.step(t, "Stop containers"))
	87	+	for _, w := range []string{"Stop and remove container: ", "Print service container logs: ", "Remove container network: github_network_", "##[error]The docker command timed out after 400ms: rm --force ", "##[warning]Docker logs failed: The docker command timed out"} {
	88	+	if !strings.Contains(stop, w) {
	89	+	t.Errorf("stop lacks %q:\n%s", w, stop)
	90	+	}
	91	+	}
	92	+	rms, nets := 0, 0
	93	+	for _, c := range f.calls(t) {
	94	+	if c[0] == "rm" {
	95	+	rms++
	96	+	}
	97	+	if c[0] == "network" && len(c) > 1 && c[1] == "rm" {
	98	+	nets++
	99	+	}
	100	+	}
	101	+	// 3 removals, and the fallback of the job tries the one that timed out again.
	102	+	if rms < 3 \|\| rms > 4 \|\| nets != 1 {
	103	+	t.Errorf("rm=%d network rm=%d (want 3 or 4 and 1):\n%s", rms, nets, stop)
	104	+	}
	105	+	if rec.concl("Stop containers") != ConclusionFailure {
	106	+	t.Errorf("Stop containers: %s", rec.concl("Stop containers"))
	107	+	}
	108	+	}
	109	+
	110	+	// GO-003: a shutdown skips the queued post steps (StepsRunner); the job-level
	111	+	// fallback removes the containers and the network all the same, and does not
	112	+	// run twice what "Stop containers" did.
	113	+	func TestShutdownStillCleansUpThroughTheFallback(t *testing.T) {
	114	+	f := newFakeCtr(t)
	115	+	f.set(t, "hang_exec", "1")
	116	+	cancel := NewCanceller()
	117	+	go func() { time.Sleep(600 * time.Millisecond); cancel.Cancel(CancelShutdown) }()
	118	+	rec, res := ctrRunC(t, f, cancel, ctrJob, " - name: long\n run: sleep 60\n", nil)
	119	+	_ = res
	120	+	if rec.concl("Stop containers") == ConclusionSuccess {
	121	+	t.Skip("the runner ran Stop containers on shutdown")
	122	+	}
	123	+	got := names(f.calls(t))
	124	+	if !strings.HasSuffix(got, "rm network") {
	125	+	t.Errorf("fallback did not remove the container and the network: %s", got)
	126	+	}
	127	+	}
	128	+
	129	+	func TestFallbackDoesNothingWhenStopContainersDidItAll(t *testing.T) {
	130	+	f := newFakeCtr(t)
	131	+	_, res := ctrRun(t, f, ctrJob, " - run: echo ok\n", nil)
	132	+	if res.Conclusion != ConclusionSuccess {
	133	+	t.Fatalf("%+v", res)
	134	+	}
	135	+	n := 0
	136	+	for _, c := range f.calls(t) {
	137	+	if c[0] == "rm" \|\| (c[0] == "network" && len(c) > 1 && c[1] == "rm") {
	138	+	n++
	139	+	}
	140	+	}
	141	+	if n != 2 {
	142	+	t.Errorf("%d removals, want exactly 2 (container, network): %s", n, names(f.calls(t)))
	143	+	}
	144	+	}
	145	+
	146	+	// GO-003: a panic of the job still cleans up.
	147	+	func TestPanicStillCleansUp(t *testing.T) {
	148	+	f := newFakeCtr(t)
	149	+	defer func() {
	150	+	_ = recover()
	151	+	got := names(f.calls(t))
	152	+	if !strings.Contains(got, "rm") \|\| !strings.HasSuffix(got, "rm network") {
	153	+	t.Errorf("calls after the panic: %s", got)
	154	+	}
	155	+	}()
	156	+	ctrRun(t, f, ctrJob, " - run: echo ok\n", func(c *Config) {
	157	+	c.Reporter = &panicOnStep{Reporter: c.Reporter, name: "Run echo ok"}
	158	+	})
	159	+	}
	160	+
	161	+	type panicOnStep struct {
	162	+	Reporter
	163	+	name string
	164	+	}
	165	+
	166	+	func (p *panicOnStep) StepStarted(n int, name string, at time.Time) {
	167	+	if name == p.name {
	168	+	panic("boom")
	169	+	}
	170	+	p.Reporter.StepStarted(n, name, at)
	171	+	}
	172	+
	173	+	// GO-007: the credentials directory is removed even when the pull fails; a
	174	+	// removal that fails is reported and left to the fallback.
	175	+	func TestRegistryConfigRemovedAfterFailedPullAndRemovalFailureIsReported(t *testing.T) {
	176	+	f := newFakeCtr(t)
	177	+	f.set(t, "pull_fail", "9")
	178	+	var temp string
	179	+	rec, _ := ctrRun(t, f, " container:\n image: ghcr.io/o/i:1\n credentials:\n username: u\n password: pw-secret\n", " - run: echo\n", func(c *Config) { temp = c.Layout.Temp })
	180	+	if m, _ := filepath.Glob(filepath.Join(temp, ".docker_*")); len(m) != 0 {
	181	+	t.Errorf("config left after a failed pull: %v", m)
	182	+	}
	183	+	_ = rec
	184	+
	185	+	if os.Geteuid() == 0 {
	186	+	t.Skip("root removes anything")
	187	+	}
	188	+	f2 := newFakeCtr(t)
	189	+	f2.set(t, "lock_cfg", "1")
	190	+	rec2, res2 := ctrRun(t, f2, " container:\n image: ghcr.io/o/i:1\n credentials:\n username: u\n password: pw-secret\n", " - run: echo\n", func(c *Config) { temp = c.Layout.Temp })
	191	+	defer func() {
	192	+	m, _ := filepath.Glob(filepath.Join(temp, ".docker_*", "sub"))
	193	+	for _, d := range m {
	194	+	_ = os.Chmod(d, 0o755)
	195	+	}
	196	+	}()
	197	+	if res2.Conclusion != ConclusionFailure {
	198	+	t.Fatalf("%+v", res2)
	199	+	}
	200	+	if got := logOf(rec2.step(t, "Initialize containers")); !strings.Contains(got, "##[error]Failed to remove directory containing Docker client credentials:") {
	201	+	t.Errorf("init:\n%s", got)
	202	+	}
	203	+	}

@@ -0,0 +1,203 @@
	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	+	func fastTimeouts(c *Config) {
	14	+	c.Actions.Containers = ContainerTimeouts{Command: 400 * time.Millisecond, Pull: 400 * time.Millisecond, CleanupCommand: 400 * time.Millisecond, CleanupBudget: 3 * time.Second}
	15	+	}
	16	+
	17	+	func names(l [][]string) string {
	18	+	var v []string
	19	+	for _, c := range l {
	20	+	v = append(v, c[0])
	21	+	}
	22	+	return strings.Join(v, " ")
	23	+	}
	24	+
	25	+	// GO-001: a docker command that stalls is stopped by its own limit, the job
	26	+	// does not wait for its timeout, and the cleanup still finds the container by
	27	+	// the name chosen before the create.
	28	+	func TestStalledCreateIsBoundedAndCleanedByName(t *testing.T) {
	29	+	f := newFakeCtr(t)
	30	+	f.set(t, "hang_create", "1")
	31	+	start := time.Now()
	32	+	rec, res := ctrRun(t, f, ctrJob, " - run: echo never\n", fastTimeouts)
	33	+	if res.Conclusion != ConclusionFailure \|\| time.Since(start) > 20*time.Second {
	34	+	t.Fatalf("%+v after %s", res, time.Since(start))
	35	+	}
	36	+	log := logOf(rec.step(t, "Initialize containers"))
	37	+	if !strings.Contains(log, "##[error]The docker command timed out after 400ms: create --name ") {
	38	+	t.Errorf("init:\n%s", log)
	39	+	}
	40	+	var rm []string
	41	+	for _, c := range f.calls(t) {
	42	+	if c[0] == "rm" {
	43	+	rm = c
	44	+	}
	45	+	}
	46	+	if len(rm) < 3 \|\| !strings.Contains(rm[2], "_ubuntu2404_") \|\| strings.Contains(rm[2], strings.Repeat("0", 20)) {
	47	+	t.Errorf("rm by name expected: %v", rm)
	48	+	}
	49	+	if got := logOf(rec.step(t, "Stop containers")); !strings.Contains(got, "Remove container network: github_network_") {
	50	+	t.Errorf("stop:\n%s", got)
	51	+	}
	52	+	}
	53	+
	54	+	// GO-001: the cancellation of the job reaches every docker call of the init.
	55	+	func TestCancelDuringInitStopsTheCommandAndCleans(t *testing.T) {
	56	+	for _, verb := range []string{"version", "inspect", "port", "start", "ps"} {
	57	+	f := newFakeCtr(t)
	58	+	f.set(t, "hang_"+verb, "1")
	59	+	cancel := NewCanceller()
	60	+	go func() { time.Sleep(500 * time.Millisecond); cancel.Cancel(CancelUser) }()
	61	+	start := time.Now()
	62	+	rec, res := ctrRunC(t, f, cancel, " container: ubuntu:24.04\n services:\n s:\n image: redis:7\n", " - run: echo never\n", func(c *Config) {
	63	+	c.Actions.Containers.Command = time.Minute
	64	+	})
	65	+	if res.Conclusion != ConclusionCancelled \|\| time.Since(start) > 15*time.Second {
	66	+	t.Fatalf("%s: %+v after %s %v", verb, res, time.Since(start), rec.names())
	67	+	}
	68	+	if got := rec.concl("Stop containers"); got != ConclusionSuccess {
	69	+	t.Errorf("%s: Stop containers %s", verb, got)
	70	+	}
	71	+	}
	72	+	}
	73	+
	74	+	// GO-002: one removal that stalls does not keep the others and the network from
	75	+	// being removed; the logs of a service never prevent its removal; the
	76	+	// failures are reported by the step.
	77	+	func TestCleanupAttemptsEverythingWithItsOwnBudget(t *testing.T) {
	78	+	f := newFakeCtr(t)
	79	+	f.set(t, "hang_logs", "1")
	80	+	f.set(t, "hang_for", "rm --force "+strings.Repeat("0", 63)+"1") // the rm of the job container (first id)
	81	+	cancel := NewCanceller()
	82	+	rec, res := ctrRunC(t, f, cancel, " container: ubuntu:24.04\n services:\n a:\n image: redis:7\n b:\n image: redis:7\n", " - run: echo ok\n", fastTimeouts)
	83	+	if res.Conclusion == ConclusionSuccess {
	84	+	t.Fatalf("a failed cleanup must be visible: %+v", res)
	85	+	}
	86	+	stop := logOf(rec.step(t, "Stop containers"))
	87	+	for _, w := range []string{"Stop and remove container: ", "Print service container logs: ", "Remove container network: github_network_", "##[error]The docker command timed out after 400ms: rm --force ", "##[warning]Docker logs failed: The docker command timed out"} {
	88	+	if !strings.Contains(stop, w) {
	89	+	t.Errorf("stop lacks %q:\n%s", w, stop)
	90	+	}
	91	+	}
	92	+	rms, nets := 0, 0
	93	+	for _, c := range f.calls(t) {
	94	+	if c[0] == "rm" {
	95	+	rms++
	96	+	}
	97	+	if c[0] == "network" && len(c) > 1 && c[1] == "rm" {
	98	+	nets++
	99	+	}
	100	+	}
	101	+	// 3 removals, and the fallback of the job tries the one that timed out again.
	102	+	if rms < 3 \|\| rms > 4 \|\| nets != 1 {
	103	+	t.Errorf("rm=%d network rm=%d (want 3 or 4 and 1):\n%s", rms, nets, stop)
	104	+	}
	105	+	if rec.concl("Stop containers") != ConclusionFailure {
	106	+	t.Errorf("Stop containers: %s", rec.concl("Stop containers"))
	107	+	}
	108	+	}
	109	+
	110	+	// GO-003: a shutdown skips the queued post steps (StepsRunner); the job-level
	111	+	// fallback removes the containers and the network all the same, and does not
	112	+	// run twice what "Stop containers" did.
	113	+	func TestShutdownStillCleansUpThroughTheFallback(t *testing.T) {
	114	+	f := newFakeCtr(t)
	115	+	f.set(t, "hang_exec", "1")
	116	+	cancel := NewCanceller()
	117	+	go func() { time.Sleep(600 * time.Millisecond); cancel.Cancel(CancelShutdown) }()
	118	+	rec, res := ctrRunC(t, f, cancel, ctrJob, " - name: long\n run: sleep 60\n", nil)
	119	+	_ = res
	120	+	if rec.concl("Stop containers") == ConclusionSuccess {
	121	+	t.Skip("the runner ran Stop containers on shutdown")
	122	+	}
	123	+	got := names(f.calls(t))
	124	+	if !strings.HasSuffix(got, "rm network") {
	125	+	t.Errorf("fallback did not remove the container and the network: %s", got)
	126	+	}
	127	+	}
	128	+
	129	+	func TestFallbackDoesNothingWhenStopContainersDidItAll(t *testing.T) {
	130	+	f := newFakeCtr(t)
	131	+	_, res := ctrRun(t, f, ctrJob, " - run: echo ok\n", nil)
	132	+	if res.Conclusion != ConclusionSuccess {
	133	+	t.Fatalf("%+v", res)
	134	+	}
	135	+	n := 0
	136	+	for _, c := range f.calls(t) {
	137	+	if c[0] == "rm" \|\| (c[0] == "network" && len(c) > 1 && c[1] == "rm") {
	138	+	n++
	139	+	}
	140	+	}
	141	+	if n != 2 {
	142	+	t.Errorf("%d removals, want exactly 2 (container, network): %s", n, names(f.calls(t)))
	143	+	}
	144	+	}
	145	+
	146	+	// GO-003: a panic of the job still cleans up.
	147	+	func TestPanicStillCleansUp(t *testing.T) {
	148	+	f := newFakeCtr(t)
	149	+	defer func() {
	150	+	_ = recover()
	151	+	got := names(f.calls(t))
	152	+	if !strings.Contains(got, "rm") \|\| !strings.HasSuffix(got, "rm network") {
	153	+	t.Errorf("calls after the panic: %s", got)
	154	+	}
	155	+	}()
	156	+	ctrRun(t, f, ctrJob, " - run: echo ok\n", func(c *Config) {
	157	+	c.Reporter = &panicOnStep{Reporter: c.Reporter, name: "Run echo ok"}
	158	+	})
	159	+	}
	160	+
	161	+	type panicOnStep struct {
	162	+	Reporter
	163	+	name string
	164	+	}
	165	+
	166	+	func (p *panicOnStep) StepStarted(n int, name string, at time.Time) {
	167	+	if name == p.name {
	168	+	panic("boom")
	169	+	}
	170	+	p.Reporter.StepStarted(n, name, at)
	171	+	}
	172	+
	173	+	// GO-007: the credentials directory is removed even when the pull fails; a
	174	+	// removal that fails is reported and left to the fallback.
	175	+	func TestRegistryConfigRemovedAfterFailedPullAndRemovalFailureIsReported(t *testing.T) {
	176	+	f := newFakeCtr(t)
	177	+	f.set(t, "pull_fail", "9")
	178	+	var temp string
	179	+	rec, _ := ctrRun(t, f, " container:\n image: ghcr.io/o/i:1\n credentials:\n username: u\n password: pw-secret\n", " - run: echo\n", func(c *Config) { temp = c.Layout.Temp })
	180	+	if m, _ := filepath.Glob(filepath.Join(temp, ".docker_*")); len(m) != 0 {
	181	+	t.Errorf("config left after a failed pull: %v", m)
	182	+	}
	183	+	_ = rec
	184	+
	185	+	if os.Geteuid() == 0 {
	186	+	t.Skip("root removes anything")
	187	+	}
	188	+	f2 := newFakeCtr(t)
	189	+	f2.set(t, "lock_cfg", "1")
	190	+	rec2, res2 := ctrRun(t, f2, " container:\n image: ghcr.io/o/i:1\n credentials:\n username: u\n password: pw-secret\n", " - run: echo\n", func(c *Config) { temp = c.Layout.Temp })
	191	+	defer func() {
	192	+	m, _ := filepath.Glob(filepath.Join(temp, ".docker_*", "sub"))
	193	+	for _, d := range m {
	194	+	_ = os.Chmod(d, 0o755)
	195	+	}
	196	+	}()
	197	+	if res2.Conclusion != ConclusionFailure {
	198	+	t.Fatalf("%+v", res2)
	199	+	}
	200	+	if got := logOf(rec2.step(t, "Initialize containers")); !strings.Contains(got, "##[error]Failed to remove directory containing Docker client credentials:") {
	201	+	t.Errorf("init:\n%s", got)
	202	+	}
	203	+	}

M internal/executor/containers_docker.go +139 -18


@@ -3,7 +3,11 @@
3	3		package executor
4	4
5	5		import (
	6	+	"fmt"
6	7		"strings"
	8	+	"sync"
	9	+	"sync/atomic"
	10	+	"time"
7	11		)
8	12
9	13		// Helpers for the docker commands of the container steps. The runner builds a

@@ -105,41 +109,158 @@ func (j *job) dockerArgString(c dockerCmd) string {
105	109		return strings.TrimPrefix(c.line, j.dockerPath()+" ")
106	110		}
107	111
108		-	// dockerList is the overload of ExecuteDockerCommandAsync that returns the
109		-	// output (DockerCommandManager.cs:462-492): non-empty lines of stdout are kept
110		-	// and logged, non-empty lines of stderr are logged, and a non-zero exit is an
111		-	// error.
112		-	func (j job) dockerList(s stepRun, c dockerCmd, cancel <-chan struct{}) ([]string, error) {
113		-	j.out(s.num, "##[command]"+c.line)
114		-	var lines []string
115		-	res, err := runProcess(procSpec{
	112	+	// exitError is ProcessExitCodeException: docker ran and said no.
	113	+	type exitError struct {
	114	+	code int
	115	+	msg string
	116	+	}
	117	+
	118	+	func (e *exitError) Error() string { return e.msg }
	119	+
	120	+	// Time limits of the docker commands of the container steps. The runner has
	121	+	// none (its commands end with the job); a sandbox whose daemon stalls must not
	122	+	// hold the job until its timeout, nor the cleanup forever.
	123	+	type ContainerTimeouts struct {
	124	+	// Command bounds one docker command (default 2 minutes); Pull bounds a
	125	+	// docker pull (30 minutes).
	126	+	Command, Pull time.Duration
	127	+	// CleanupCommand bounds one command of "Stop containers" (30 seconds) and
	128	+	// CleanupBudget the whole cleanup (3 minutes), whatever happens to the job.
	129	+	CleanupCommand, CleanupBudget time.Duration
	130	+	}
	131	+
	132	+	func (j *job) timeouts() ContainerTimeouts {
	133	+	t := j.cfg.Actions.Containers
	134	+	def := func(v *time.Duration, d time.Duration) {
	135	+	if *v <= 0 {
	136	+	*v = d
	137	+	}
	138	+	}
	139	+	def(&t.Command, 2*time.Minute)
	140	+	def(&t.Pull, 30*time.Minute)
	141	+	def(&t.CleanupCommand, 30*time.Second)
	142	+	def(&t.CleanupBudget, 3*time.Minute)
	143	+	return t
	144	+	}
	145	+
	146	+	// cmdTimeoutError is a docker command stopped by its time limit.
	147	+	type cmdTimeoutError struct {
	148	+	cmd string
	149	+	d time.Duration
	150	+	}
	151	+
	152	+	func (e *cmdTimeoutError) Error() string {
	153	+	return fmt.Sprintf("The docker command timed out after %s: %s", e.d, e.cmd)
	154	+	}
	155	+
	156	+	// bound returns a channel that closes when cancel does (nil never) or after d,
	157	+	// and a function that tells whether it was the time, with the one that
	158	+	// releases the timer.
	159	+	func bound(cancel <-chan struct{}, d time.Duration) (ch <-chan struct{}, timedOut func() bool, stop func()) {
	160	+	out := make(chan struct{})
	161	+	var once sync.Once
	162	+	var fired atomic.Bool
	163	+	closeOut := func() { once.Do(func() { close(out) }) }
	164	+	t := time.AfterFunc(d, func() { fired.Store(true); closeOut() })
	165	+	done := make(chan struct{})
	166	+	if cancel != nil {
	167	+	go func() {
	168	+	select {
	169	+	case <-cancel:
	170	+	closeOut()
	171	+	case <-done:
	172	+	}
	173	+	}()
	174	+	}
	175	+	return out, fired.Load, func() { t.Stop(); close(done) }
	176	+	}
	177	+
	178	+	// dockerOpts are the options of dockerDo.
	179	+	type dockerOpts struct {
	180	+	// s is the step whose log gets the command and its output; nil runs the
	181	+	// command silently (the fallback cleanup).
	182	+	s *stepRun
	183	+	// cancel stops the command (the escalation of runProcess); always set.
	184	+	cancel <-chan struct{}
	185	+	// timeout bounds the command; 0 means the command limit.
	186	+	timeout time.Duration
	187	+	// quiet keeps the echoed command and the output out of the log.
	188	+	quiet bool
	189	+	// all logs the empty lines too (the overloads that return a code).
	190	+	all bool
	191	+	// errToOut also collects the lines of stderr.
	192	+	errToOut bool
	193	+	}
	194	+
	195	+	// do runs the command and returns the non-empty lines of its stdout (and
	196	+	// stderr with errToOut), its exit code and an error for what is not an exit
	197	+	// code: a start failure, a cancellation, a time limit.
	198	+	func (j *job) dockerDo(c dockerCmd, o dockerOpts) (lines []string, code int, err error) {
	199	+	log := o.s != nil && !o.quiet
	200	+	if log {
	201	+	j.out(o.s.num, "##[command]"+c.line)
	202	+	}
	203	+	d := o.timeout
	204	+	if d <= 0 {
	205	+	d = j.timeouts().Command
	206	+	}
	207	+	cancel, timedOut, stop := bound(o.cancel, d)
	208	+	defer stop()
	209	+	var mu sync.Mutex
	210	+	res, perr := runProcess(procSpec{
116	211		Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: append(j.processEnv(newOrdered()), c.env...), Credential: j.cfg.Credential,
117	212		OnLine: func(l string) {
	213	+	mu.Lock()
118	214		if l != "" {
119	215		lines = append(lines, l)
120		-	j.write(s.num, l)
	216	+	}
	217	+	mu.Unlock()
	218	+	if log && (o.all \|\| l != "") {
	219	+	j.write(o.s.num, l)
121	220		}
122	221		},
123	222		OnErrLine: func(l string) {
124		-	if l != "" {
125		-	j.write(s.num, l)
	223	+	if o.errToOut && l != "" {
	224	+	mu.Lock()
	225	+	lines = append(lines, l)
	226	+	mu.Unlock()
	227	+	}
	228	+	if log && (o.all \|\| l != "") {
	229	+	j.write(o.s.num, l)
126	230		}
127	231		},
128	232		Cancel: cancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
129	233		})
130		-	if err != nil {
131		-	return nil, err
	234	+	mu.Lock()
	235	+	defer mu.Unlock()
	236	+	switch {
	237	+	case perr != nil:
	238	+	return lines, 0, perr
	239	+	case res.Cancelled && timedOut():
	240	+	return lines, res.ExitCode, &cmdTimeoutError{cmd: j.dockerArgString(c), d: d}
	241	+	case res.Cancelled:
	242	+	return lines, res.ExitCode, errCancelled
132	243		}
133		-	if res.Cancelled {
134		-	return lines, errCancelled
	244	+	return lines, res.ExitCode, nil
	245	+	}
	246	+
	247	+	// dockerList is the overload of ExecuteDockerCommandAsync that returns the
	248	+	// output (DockerCommandManager.cs:462-492): non-empty lines of stdout are kept
	249	+	// and logged, non-empty lines of stderr are logged, and a non-zero exit is an
	250	+	// error.
	251	+	func (j job) dockerList(s stepRun, c dockerCmd, cancel <-chan struct{}) ([]string, error) {
	252	+	lines, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: cancel})
	253	+	if err != nil {
	254	+	return lines, err
135	255		}
136		-	if res.ExitCode != 0 {
137		-	return lines, ghErrorf("Exit code %d returned from process: file name '%s', arguments '%s'.", res.ExitCode, j.dockerPath(), j.dockerArgString(c))
	256	+	if code != 0 {
	257	+	return lines, &exitError{code: code, msg: fmt.Sprintf("Exit code %d returned from process: file name '%s', arguments '%s'.", code, j.dockerPath(), j.dockerArgString(c))}
138	258		}
139	259		return lines, nil
140	260		}
141	261
142	262		// dockerCode is the overload that logs every line and returns the exit code.
143	263		func (j job) dockerCode(s stepRun, c dockerCmd, cancel <-chan struct{}) (int, error) {
144		-	return j.runDockerCmd(s, c, dirOrCwd(j.githubValue("workspace")), nil, cancel)
	264	+	_, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: cancel, all: true})
	265	+	return code, err
145	266		}


@@ -3,7 +3,11 @@
3		package executor	3		package executor
4			4
5		import (	5		import (
			6	+	"fmt"
6		"strings"	7		"strings"
			8	+	"sync"
			9	+	"sync/atomic"
			10	+	"time"
7		)	11		)
8			12
9		// Helpers for the docker commands of the container steps. The runner builds a	13		// Helpers for the docker commands of the container steps. The runner builds a

@@ -105,41 +109,158 @@ func (j *job) dockerArgString(c dockerCmd) string {
105		return strings.TrimPrefix(c.line, j.dockerPath()+" ")	109		return strings.TrimPrefix(c.line, j.dockerPath()+" ")
106		}	110		}
107			111
108	-	// dockerList is the overload of ExecuteDockerCommandAsync that returns the	112	+	// exitError is ProcessExitCodeException: docker ran and said no.
109	-	// output (DockerCommandManager.cs:462-492): non-empty lines of stdout are kept	113	+	type exitError struct {
110	-	// and logged, non-empty lines of stderr are logged, and a non-zero exit is an	114	+	code int
111	-	// error.	115	+	msg string
112	-	func (j job) dockerList(s stepRun, c dockerCmd, cancel <-chan struct{}) ([]string, error) {	116	+	}
113	-	j.out(s.num, "##[command]"+c.line)	117	+
114	-	var lines []string	118	+	func (e *exitError) Error() string { return e.msg }
115	-	res, err := runProcess(procSpec{	119	+
			120	+	// Time limits of the docker commands of the container steps. The runner has
			121	+	// none (its commands end with the job); a sandbox whose daemon stalls must not
			122	+	// hold the job until its timeout, nor the cleanup forever.
			123	+	type ContainerTimeouts struct {
			124	+	// Command bounds one docker command (default 2 minutes); Pull bounds a
			125	+	// docker pull (30 minutes).
			126	+	Command, Pull time.Duration
			127	+	// CleanupCommand bounds one command of "Stop containers" (30 seconds) and
			128	+	// CleanupBudget the whole cleanup (3 minutes), whatever happens to the job.
			129	+	CleanupCommand, CleanupBudget time.Duration
			130	+	}
			131	+
			132	+	func (j *job) timeouts() ContainerTimeouts {
			133	+	t := j.cfg.Actions.Containers
			134	+	def := func(v *time.Duration, d time.Duration) {
			135	+	if *v <= 0 {
			136	+	*v = d
			137	+	}
			138	+	}
			139	+	def(&t.Command, 2*time.Minute)
			140	+	def(&t.Pull, 30*time.Minute)
			141	+	def(&t.CleanupCommand, 30*time.Second)
			142	+	def(&t.CleanupBudget, 3*time.Minute)
			143	+	return t
			144	+	}
			145	+
			146	+	// cmdTimeoutError is a docker command stopped by its time limit.
			147	+	type cmdTimeoutError struct {
			148	+	cmd string
			149	+	d time.Duration
			150	+	}
			151	+
			152	+	func (e *cmdTimeoutError) Error() string {
			153	+	return fmt.Sprintf("The docker command timed out after %s: %s", e.d, e.cmd)
			154	+	}
			155	+
			156	+	// bound returns a channel that closes when cancel does (nil never) or after d,
			157	+	// and a function that tells whether it was the time, with the one that
			158	+	// releases the timer.
			159	+	func bound(cancel <-chan struct{}, d time.Duration) (ch <-chan struct{}, timedOut func() bool, stop func()) {
			160	+	out := make(chan struct{})
			161	+	var once sync.Once
			162	+	var fired atomic.Bool
			163	+	closeOut := func() { once.Do(func() { close(out) }) }
			164	+	t := time.AfterFunc(d, func() { fired.Store(true); closeOut() })
			165	+	done := make(chan struct{})
			166	+	if cancel != nil {
			167	+	go func() {
			168	+	select {
			169	+	case <-cancel:
			170	+	closeOut()
			171	+	case <-done:
			172	+	}
			173	+	}()
			174	+	}
			175	+	return out, fired.Load, func() { t.Stop(); close(done) }
			176	+	}
			177	+
			178	+	// dockerOpts are the options of dockerDo.
			179	+	type dockerOpts struct {
			180	+	// s is the step whose log gets the command and its output; nil runs the
			181	+	// command silently (the fallback cleanup).
			182	+	s *stepRun
			183	+	// cancel stops the command (the escalation of runProcess); always set.
			184	+	cancel <-chan struct{}
			185	+	// timeout bounds the command; 0 means the command limit.
			186	+	timeout time.Duration
			187	+	// quiet keeps the echoed command and the output out of the log.
			188	+	quiet bool
			189	+	// all logs the empty lines too (the overloads that return a code).
			190	+	all bool
			191	+	// errToOut also collects the lines of stderr.
			192	+	errToOut bool
			193	+	}
			194	+
			195	+	// do runs the command and returns the non-empty lines of its stdout (and
			196	+	// stderr with errToOut), its exit code and an error for what is not an exit
			197	+	// code: a start failure, a cancellation, a time limit.
			198	+	func (j *job) dockerDo(c dockerCmd, o dockerOpts) (lines []string, code int, err error) {
			199	+	log := o.s != nil && !o.quiet
			200	+	if log {
			201	+	j.out(o.s.num, "##[command]"+c.line)
			202	+	}
			203	+	d := o.timeout
			204	+	if d <= 0 {
			205	+	d = j.timeouts().Command
			206	+	}
			207	+	cancel, timedOut, stop := bound(o.cancel, d)
			208	+	defer stop()
			209	+	var mu sync.Mutex
			210	+	res, perr := runProcess(procSpec{
116		Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: append(j.processEnv(newOrdered()), c.env...), Credential: j.cfg.Credential,	211		Argv: c.argv, Dir: dirOrCwd(j.githubValue("workspace")), Env: append(j.processEnv(newOrdered()), c.env...), Credential: j.cfg.Credential,
117		OnLine: func(l string) {	212		OnLine: func(l string) {
			213	+	mu.Lock()
118		if l != "" {	214		if l != "" {
119		lines = append(lines, l)	215		lines = append(lines, l)
120	-	j.write(s.num, l)	216	+	}
			217	+	mu.Unlock()
			218	+	if log && (o.all \|\| l != "") {
			219	+	j.write(o.s.num, l)
121		}	220		}
122		},	221		},
123		OnErrLine: func(l string) {	222		OnErrLine: func(l string) {
124	-	if l != "" {	223	+	if o.errToOut && l != "" {
125	-	j.write(s.num, l)	224	+	mu.Lock()
			225	+	lines = append(lines, l)
			226	+	mu.Unlock()
			227	+	}
			228	+	if log && (o.all \|\| l != "") {
			229	+	j.write(o.s.num, l)
126		}	230		}
127		},	231		},
128		Cancel: cancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,	232		Cancel: cancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
129		})	233		})
130	-	if err != nil {	234	+	mu.Lock()
131	-	return nil, err	235	+	defer mu.Unlock()
			236	+	switch {
			237	+	case perr != nil:
			238	+	return lines, 0, perr
			239	+	case res.Cancelled && timedOut():
			240	+	return lines, res.ExitCode, &cmdTimeoutError{cmd: j.dockerArgString(c), d: d}
			241	+	case res.Cancelled:
			242	+	return lines, res.ExitCode, errCancelled
132		}	243		}
133	-	if res.Cancelled {	244	+	return lines, res.ExitCode, nil
134	-	return lines, errCancelled	245	+	}
			246	+
			247	+	// dockerList is the overload of ExecuteDockerCommandAsync that returns the
			248	+	// output (DockerCommandManager.cs:462-492): non-empty lines of stdout are kept
			249	+	// and logged, non-empty lines of stderr are logged, and a non-zero exit is an
			250	+	// error.
			251	+	func (j job) dockerList(s stepRun, c dockerCmd, cancel <-chan struct{}) ([]string, error) {
			252	+	lines, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: cancel})
			253	+	if err != nil {
			254	+	return lines, err
135		}	255		}
136	-	if res.ExitCode != 0 {	256	+	if code != 0 {
137	-	return lines, ghErrorf("Exit code %d returned from process: file name '%s', arguments '%s'.", res.ExitCode, j.dockerPath(), j.dockerArgString(c))	257	+	return lines, &exitError{code: code, msg: fmt.Sprintf("Exit code %d returned from process: file name '%s', arguments '%s'.", code, j.dockerPath(), j.dockerArgString(c))}
138		}	258		}
139		return lines, nil	259		return lines, nil
140		}	260		}
141			261
142		// dockerCode is the overload that logs every line and returns the exit code.	262		// dockerCode is the overload that logs every line and returns the exit code.
143		func (j job) dockerCode(s stepRun, c dockerCmd, cancel <-chan struct{}) (int, error) {	263		func (j job) dockerCode(s stepRun, c dockerCmd, cancel <-chan struct{}) (int, error) {
144	-	return j.runDockerCmd(s, c, dirOrCwd(j.githubValue("workspace")), nil, cancel)	264	+	_, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: cancel, all: true})
			265	+	return code, err
145		}	266		}

M internal/executor/containers_fake_test.go +6 -2


@@ -29,12 +29,16 @@ func newFakeCtr(t testing.T) fakeCtr {
29	29		script := `#!/bin/sh
30	30		D=` + d + `
31	31		{ echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; echo "ENV:CVAR=$CVAR:EMPTYV=${EMPTYV-unset}:PATH=$PATH"; } >> $D/calls
32		-	[ "$1" = "--config" ] && { cat > $D/login_stdin_$$ 2>/dev/null </dev/null; shift 2; }
	32	+	[ "$1" = "--config" ] && { CFG=$2; shift 2; }
	33	+	hang() { trap 'exit 130' INT TERM; while :; do sleep 0.05; done; }
	34	+	[ -f $D/hang_$1 ] && hang
	35	+	if [ -f $D/hang_for ]; then case "$" in "$(cat $D/hang_for)"*) hang ;; esac; fi
33	36		case "$1" in
34	37		version) echo "'1.48'" ;;
35	38		ps) case "$" in --quiet) ;; status=running) id=$(echo "$" \| sed 's/.id=$[^ ]$.*/\1/'); echo "$id Up Less than a second" ;; esac ;;
36	39		network) ;;
37		-	login) cat > $D/login_stdin ;;
	40	+	login) cat > $D/login_stdin
	41	+	if [ -f $D/lock_cfg ]; then mkdir -p $CFG/sub; : > $CFG/sub/f; chmod 500 $CFG/sub; fi ;;
38	42		pull) if [ -f $D/pull_fail ]; then n=$(cat $D/pull_fail); if [ "$n" -gt 0 ]; then echo $((n-1)) > $D/pull_fail; echo "pull denied"; exit 1; fi; fi; echo "pulled $2" ;;
39	43		create) [ -f $D/create_fail ] && { echo "unknown flag"; exit 125; }
40	44		[ -f $D/create_out ] && { cat $D/create_out; exit 0; }

 	script := `#!/bin/sh
 D=` + d + `
 { echo "--"; for a in "$@"; do printf "%s\n" "$a"; done; echo "ENV:CVAR=$CVAR:EMPTYV=${EMPTYV-unset}:PATH=$PATH"; } >> $D/calls
-[ "$1" = "--config" ] && { cat > $D/login_stdin_$$ 2>/dev/null </dev/null; shift 2; }
+[ "$1" = "--config" ] && { CFG=$2; shift 2; }
+hang() { trap 'exit 130' INT TERM; while :; do sleep 0.05; done; }
+[ -f $D/hang_$1 ] && hang
+if [ -f $D/hang_for ]; then case "$*" in *"$(cat $D/hang_for)"*) hang ;; esac; fi
 case "$1" in
 version) echo "'1.48'" ;;
 ps) case "$*" in *--quiet*) ;; *status=running*) id=$(echo "$*" | sed 's/.*id=\([^ ]*\).*/\1/'); echo "$id Up Less than a second" ;; esac ;;
 network) ;;
-login) cat > $D/login_stdin ;;
+login) cat > $D/login_stdin
+  if [ -f $D/lock_cfg ]; then mkdir -p $CFG/sub; : > $CFG/sub/f; chmod 500 $CFG/sub; fi ;;
 pull) if [ -f $D/pull_fail ]; then n=$(cat $D/pull_fail); if [ "$n" -gt 0 ]; then echo $((n-1)) > $D/pull_fail; echo "pull denied"; exit 1; fi; fi; echo "pulled $2" ;;
 create) [ -f $D/create_fail ] && { echo "unknown flag"; exit 125; }
   [ -f $D/create_out ] && { cat $D/create_out; exit 0; }

M internal/executor/containers_init.go +41 -4


@@ -10,6 +10,7 @@ import (
10	10		"regexp"
11	11		"strconv"
12	12		"strings"
	13	+	"sync"
13	14		"time"
14	15		)
15	16

@@ -19,8 +20,23 @@ type containerSet struct {
19	20		job *container
20	21		services []*container
21	22		network string
	23	+
	24	+	// What the cleanup needs, recorded BEFORE the command that creates it, so
	25	+	// that a command whose outcome is not known (a timeout, a cancellation, a
	26	+	// lost connection) can still be cleaned: the names are chosen here, not by
	27	+	// docker. mu guards them; cleanupMu serialises the cleanups ("Stop
	28	+	// containers" and the fallback of the job).
	29	+	mu sync.Mutex
	30	+	cleanupMu sync.Mutex
	31	+	networkTry string // network name, set before docker network create
	32	+	netRemoved bool // the network is gone
	33	+	dirs []string // docker --config directories that could not be removed
22	34		}
23	35
	36	+	func (cs *containerSet) tryNetwork(name string) { cs.mu.Lock(); cs.networkTry = name; cs.mu.Unlock() }
	37	+
	38	+	func (cs *containerSet) addDir(d string) { cs.mu.Lock(); cs.dirs = append(cs.dirs, d); cs.mu.Unlock() }
	39	+
24	40		func (cs containerSet) all() []container {
25	41		var l []*container
26	42		if cs.job != nil {

@@ -83,7 +99,7 @@ func (j job) startContainers(s stepRun, cancel <-chan struct{}) error {
83	99		j.out(num, "##[group]Clean up resources from previous jobs")
84	100		label := j.dockerLabel()
85	101		ps := j.newDockerCmd("ps", raw("--all", "--quiet", "--no-trunc"), quotedOpt("--filter", "label="+label))
86		-	stale, err := j.dockerList(s, ps, nil)
	102	+	stale, err := j.dockerList(s, ps, cancel)
87	103		if err != nil {
88	104		return err
89	105		}

@@ -108,6 +124,7 @@ func (j job) startContainers(s stepRun, cancel <-chan struct{}) error {
108	124
109	125		j.out(num, "##[group]Create local container network")
110	126		network := "github_network_" + strings.ReplaceAll(newImageTag(""), ":", "")
	127	+	cs.tryNetwork(network)
111	128		code, err = j.dockerCode(s, j.newDockerCmd("network", raw("create", "--label", label, network)), cancel)
112	129		if err != nil {
113	130		return err

@@ -137,7 +154,7 @@ func (j job) checkDockerVersion(s stepRun, cancel <-chan struct{}) error {
137	154		num := s.num
138	155		j.out(num, "##[group]Checking docker version")
139	156		ver := func(which string) (string, error) {
140		-	l, err := j.dockerList(s, j.newDockerCmd("version", raw("--format", "'{{."+which+".APIVersion}}'")), nil)
	157	+	l, err := j.dockerList(s, j.newDockerCmd("version", raw("--format", "'{{."+which+".APIVersion}}'")), cancel)
141	158		if err != nil {
142	159		return "", err
143	160		}

@@ -199,9 +216,24 @@ func (j *job) pullCmd(image, config string) dockerCmd {
199	216		return dockerCmd{line: p + " --config " + config + " pull " + image, argv: []string{p, "--config", config, "pull", image}}
200	217		}
201	218
	219	+	// removeConfigDir is ContainerRegistryLogout (:457-470): the credentials
	220	+	// directory goes, and a failure is the runner's error. A directory that
	221	+	// cannot be removed is kept for the cleanup of the job, which tries again.
	222	+	func (j *job) removeConfigDir(dir string) error {
	223	+	if dir == "" {
	224	+	return nil
	225	+	}
	226	+	if err := os.RemoveAll(dir); err != nil {
	227	+	j.cs.addDir(dir)
	228	+	return ghErrorf("Failed to remove directory containing Docker client credentials: %v", err)
	229	+	}
	230	+	return nil
	231	+	}
	232	+
202	233		// registryLogin is ContainerRegistryLogin (:394-455): the credentials go to a
203	234		// private docker config directory, the password through stdin. It returns the
204		-	// directory ("" without credentials); the caller removes it.
	235	+	// directory ("" without credentials); the caller removes it with
	236	+	// removeConfigDir, whatever happens.
205	237		func (j job) registryLogin(s stepRun, c *container, cancel <-chan struct{}) (string, error) {
206	238		if c.credUser == "" \|\| c.credPass == "" {
207	239		return "", nil

@@ -221,15 +253,20 @@ func (j job) registryLogin(s stepRun, c *container, cancel <-chan struct{}) (s
221	253		args, line = append(args, "-u", c.credUser, "--password-stdin"), line+" -u "+c.credUser+" --password-stdin"
222	254		for try := 1; ; try++ {
223	255		j.out(s.num, "##[command]"+j.dockerPath()+" "+line)
	256	+	bc, timedOut, stop := bound(cancel, j.timeouts().Command)
224	257		res, err := runProcess(procSpec{
225	258		Argv: args, Dir: dirOrCwd(j.githubValue("workspace")), Env: j.processEnv(newOrdered()), Credential: j.cfg.Credential,
226	259		OnLine: func(string) {}, Stdin: c.credPass + "\n",
227		-	Cancel: cancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
	260	+	Cancel: bc, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
228	261		})
	262	+	stop()
229	263		if err != nil {
230	264		return dir, err
231	265		}
232	266		if res.Cancelled {
	267	+	if timedOut() {
	268	+	return dir, &cmdTimeoutError{cmd: "login", d: j.timeouts().Command}
	269	+	}
233	270		return dir, errCancelled
234	271		}
235	272		if res.ExitCode == 0 {

 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 )
 	job      *container
 	services []*container
 	network  string
+	// What the cleanup needs, recorded BEFORE the command that creates it, so
+	// that a command whose outcome is not known (a timeout, a cancellation, a
+	// lost connection) can still be cleaned: the names are chosen here, not by
+	// docker. mu guards them; cleanupMu serialises the cleanups ("Stop
+	// containers" and the fallback of the job).
+	mu         sync.Mutex
+	cleanupMu  sync.Mutex
+	networkTry string   // network name, set before docker network create
+	netRemoved bool     // the network is gone
+	dirs       []string // docker --config directories that could not be removed
 }
+func (cs *containerSet) tryNetwork(name string) { cs.mu.Lock(); cs.networkTry = name; cs.mu.Unlock() }
+func (cs *containerSet) addDir(d string) { cs.mu.Lock(); cs.dirs = append(cs.dirs, d); cs.mu.Unlock() }
 func (cs *containerSet) all() []*container {
 	var l []*container
 	if cs.job != nil {
 	j.out(num, "##[group]Clean up resources from previous jobs")
 	label := j.dockerLabel()
 	ps := j.newDockerCmd("ps", raw("--all", "--quiet", "--no-trunc"), quotedOpt("--filter", "label="+label))
-	stale, err := j.dockerList(s, ps, nil)
+	stale, err := j.dockerList(s, ps, cancel)
 	if err != nil {
 		return err
 	}
 	j.out(num, "##[group]Create local container network")
 	network := "github_network_" + strings.ReplaceAll(newImageTag(""), ":", "")
+	cs.tryNetwork(network)
 	code, err = j.dockerCode(s, j.newDockerCmd("network", raw("create", "--label", label, network)), cancel)
 	if err != nil {
 		return err
 	num := s.num
 	j.out(num, "##[group]Checking docker version")
 	ver := func(which string) (string, error) {
-		l, err := j.dockerList(s, j.newDockerCmd("version", raw("--format", "'{{."+which+".APIVersion}}'")), nil)
+		l, err := j.dockerList(s, j.newDockerCmd("version", raw("--format", "'{{."+which+".APIVersion}}'")), cancel)
 		if err != nil {
 			return "", err
 		}
 	return dockerCmd{line: p + " --config " + config + " pull " + image, argv: []string{p, "--config", config, "pull", image}}
 }
+// removeConfigDir is ContainerRegistryLogout (:457-470): the credentials
+// directory goes, and a failure is the runner's error. A directory that
+// cannot be removed is kept for the cleanup of the job, which tries again.
+func (j *job) removeConfigDir(dir string) error {
+	if dir == "" {
+		return nil
+	}
+	if err := os.RemoveAll(dir); err != nil {
+		j.cs.addDir(dir)
+		return ghErrorf("Failed to remove directory containing Docker client credentials: %v", err)
+	}
+	return nil
+}
 // registryLogin is ContainerRegistryLogin (:394-455): the credentials go to a
 // private docker config directory, the password through stdin. It returns the
-// directory ("" without credentials); the caller removes it.
+// directory ("" without credentials); the caller removes it with
+// removeConfigDir, whatever happens.
 func (j *job) registryLogin(s *stepRun, c *container, cancel <-chan struct{}) (string, error) {
 	if c.credUser == "" || c.credPass == "" {
 		return "", nil
 	args, line = append(args, "-u", c.credUser, "--password-stdin"), line+" -u "+c.credUser+" --password-stdin"
 	for try := 1; ; try++ {
 		j.out(s.num, "##[command]"+j.dockerPath()+" "+line)
+		bc, timedOut, stop := bound(cancel, j.timeouts().Command)
 		res, err := runProcess(procSpec{
 			Argv: args, Dir: dirOrCwd(j.githubValue("workspace")), Env: j.processEnv(newOrdered()), Credential: j.cfg.Credential,
 			OnLine: func(string) {}, Stdin: c.credPass + "\n",
-			Cancel: cancel, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
+			Cancel: bc, TermAfter: j.cfg.TermAfter, KillAfter: j.cfg.KillAfter, WaitDelay: j.cfg.WaitDelay, Signal: j.cfg.Signal,
 		})
+		stop()
 		if err != nil {
 			return dir, err
 		}
 		if res.Cancelled {
+			if timedOut() {
+				return dir, &cmdTimeoutError{cmd: "login", d: j.timeouts().Command}
+			}
 			return dir, errCancelled
 		}
 		if res.ExitCode == 0 {

M internal/executor/containers_spec.go +1 -0


@@ -65,6 +65,7 @@ type container struct {
65	65		workDir string
66	66		id, network, runtimePath string
67	67		failedInit bool
	68	+	createTried, removed bool
68	69		maps []pathMap // path translation, first match wins
69	70		portMappings []portMapping
70	71		}


@@ -65,6 +65,7 @@ type container struct {
65	workDir string	65		workDir string
66	id, network, runtimePath string	66		id, network, runtimePath string
67	failedInit bool	67		failedInit bool
		68	+	createTried, removed bool
68	maps []pathMap // path translation, first match wins	69		maps []pathMap // path translation, first match wins
69	portMappings []portMapping	70		portMappings []portMapping
70	}	71		}

M internal/executor/containers_start.go +24 -13


@@ -3,6 +3,7 @@
3	3		package executor
4	4
5	5		import (
	6	+	"errors"
6	7		"fmt"
7	8		"os"
8	9		"path/filepath"

@@ -94,14 +95,19 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
94	95		}
95	96		cfg, err := j.registryLogin(s, c, cancel)
96	97		if err != nil {
97		-	if cfg != "" {
98		-	_ = os.RemoveAll(cfg)
	98	+	if rerr := j.removeConfigDir(cfg); rerr != nil {
	99	+	j.errorf(num, "%s", rerr.Error())
99	100		}
100	101		return err
101	102		}
102	103		code, err := j.pullWithRetry(s, c.image, cfg, cancel)
103		-	if cfg != "" {
104		-	_ = os.RemoveAll(cfg)
	104	+	// Credentials are removed after the pull, whatever it did (:171-173).
	105	+	if rerr := j.removeConfigDir(cfg); rerr != nil {
	106	+	if err == nil {
	107	+	err = rerr
	108	+	} else {
	109	+	j.errorf(num, "%s", rerr.Error())
	110	+	}
105	111		}
106	112		if err != nil {
107	113		return err

@@ -114,8 +120,13 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
114	120		return err
115	121		}
116	122		}
117		-	out, err := j.dockerList(s, j.createCmd(c), nil)
	123	+	c.createTried = true // the container may exist from here on, named c.name
	124	+	out, err := j.dockerList(s, j.createCmd(c), cancel)
118	125		if err != nil {
	126	+	var ee *exitError
	127	+	if errors.As(err, &ee) {
	128	+	c.createTried = false // docker said no: there is nothing to remove
	129	+	}
119	130		return err
120	131		}
121	132		if len(out) == 0 \|\| out[0] == "" {

@@ -134,9 +145,9 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
134	145		if code != 0 {
135	146		return ghErrorf("Docker start fail with exit code %d", code)
136	147		}
137		-	j.checkRunning(s, c)
	148	+	j.checkRunning(s, c, cancel)
138	149		if !c.isJob {
139		-	ports, err := j.dockerList(s, j.newDockerCmd("port", raw(c.id)), nil)
	150	+	ports, err := j.dockerList(s, j.newDockerCmd("port", raw(c.id)), cancel)
140	151		if err != nil {
141	152		return err
142	153		}

@@ -145,7 +156,7 @@ func (j job) startContainer(s stepRun, c *container, cancel <-chan struct{}) e
145	156		j.state.SetJobService(c.alias, c.id, c.network, c.portMappings)
146	157		j.mu.Unlock()
147	158		} else {
148		-	env, err := j.dockerList(s, j.newDockerCmd("inspect", quotedOpt("--format", "{{range .Config.Env}}{{println .}}{{end}}"), raw(c.id)), nil)
	159	+	env, err := j.dockerList(s, j.newDockerCmd("inspect", quotedOpt("--format", "{{range .Config.Env}}{{println .}}{{end}}"), raw(c.id)), cancel)
149	160		if err != nil {
150	161		return err
151	162		}

@@ -164,7 +175,7 @@ func (j job) pullWithRetry(s stepRun, image, cfg string, cancel <-chan struct{
164	175		code := 0
165	176		for try := 1; try <= 3; try++ {
166	177		var err error
167		-	code, err = j.dockerCode(s, j.pullCmd(image, cfg), cancel)
	178	+	_, code, err = j.dockerDo(j.pullCmd(image, cfg), dockerOpts{s: s, cancel: cancel, all: true, timeout: j.timeouts().Pull})
168	179		if err != nil {
169	180		return code, err
170	181		}

@@ -183,9 +194,9 @@ func (j job) pullWithRetry(s stepRun, image, cfg string, cancel <-chan struct{
183	194		}
184	195
185	196		// checkRunning is the best-effort check of :193-213.
186		-	func (j job) checkRunning(s stepRun, c *container) {
	197	+	func (j job) checkRunning(s stepRun, c *container, cancel <-chan struct{}) {
187	198		fmtOpt := quotedOpt("--format", "{{.ID}} {{.Status}}")
188		-	out, err := j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--filter", "status=running"), raw("--no-trunc"), fmtOpt), nil)
	199	+	out, err := j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--filter", "status=running"), raw("--no-trunc"), fmtOpt), cancel)
189	200		if err != nil {
190	201		return
191	202		}

@@ -197,8 +208,8 @@ func (j job) checkRunning(s stepRun, c *container) {
197	208		break
198	209		}
199	210		}
200		-	_, _ = j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--no-trunc"), fmtOpt), nil)
201		-	if code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), nil); err == nil && code != 0 {
	211	+	_, _ = j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--no-trunc"), fmtOpt), cancel)
	212	+	if code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), cancel); err == nil && code != 0 {
202	213		j.tagged(s.num, "##[warning]", fmt.Sprintf("Docker logs fail with exit code %d", code))
203	214		}
204	215		j.tagged(s.num, "##[warning]", fmt.Sprintf("Docker container %s is not in running state.", c.id))

 package executor
 import (
+	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
 	}
 	cfg, err := j.registryLogin(s, c, cancel)
 	if err != nil {
-		if cfg != "" {
+		if rerr := j.removeConfigDir(cfg); rerr != nil {
-			_ = os.RemoveAll(cfg)
+			j.errorf(num, "%s", rerr.Error())
 		}
 		return err
 	}
 	code, err := j.pullWithRetry(s, c.image, cfg, cancel)
-	if cfg != "" {
+	// Credentials are removed after the pull, whatever it did (:171-173).
-		_ = os.RemoveAll(cfg)
+	if rerr := j.removeConfigDir(cfg); rerr != nil {
+		if err == nil {
+			err = rerr
+		} else {
+			j.errorf(num, "%s", rerr.Error())
+		}
 	}
 	if err != nil {
 		return err
 			return err
 		}
 	}
-	out, err := j.dockerList(s, j.createCmd(c), nil)
+	c.createTried = true // the container may exist from here on, named c.name
+	out, err := j.dockerList(s, j.createCmd(c), cancel)
 	if err != nil {
+		var ee *exitError
+		if errors.As(err, &ee) {
+			c.createTried = false // docker said no: there is nothing to remove
+		}
 		return err
 	}
 	if len(out) == 0 || out[0] == "" {
 	if code != 0 {
 		return ghErrorf("Docker start fail with exit code %d", code)
 	}
-	j.checkRunning(s, c)
+	j.checkRunning(s, c, cancel)
 	if !c.isJob {
-		ports, err := j.dockerList(s, j.newDockerCmd("port", raw(c.id)), nil)
+		ports, err := j.dockerList(s, j.newDockerCmd("port", raw(c.id)), cancel)
 		if err != nil {
 			return err
 		}
 		j.state.SetJobService(c.alias, c.id, c.network, c.portMappings)
 		j.mu.Unlock()
 	} else {
-		env, err := j.dockerList(s, j.newDockerCmd("inspect", quotedOpt("--format", "{{range .Config.Env}}{{println .}}{{end}}"), raw(c.id)), nil)
+		env, err := j.dockerList(s, j.newDockerCmd("inspect", quotedOpt("--format", "{{range .Config.Env}}{{println .}}{{end}}"), raw(c.id)), cancel)
 		if err != nil {
 			return err
 		}
 	code := 0
 	for try := 1; try <= 3; try++ {
 		var err error
-		code, err = j.dockerCode(s, j.pullCmd(image, cfg), cancel)
+		_, code, err = j.dockerDo(j.pullCmd(image, cfg), dockerOpts{s: s, cancel: cancel, all: true, timeout: j.timeouts().Pull})
 		if err != nil {
 			return code, err
 		}
 }
 // checkRunning is the best-effort check of :193-213.
-func (j *job) checkRunning(s *stepRun, c *container) {
+func (j *job) checkRunning(s *stepRun, c *container, cancel <-chan struct{}) {
 	fmtOpt := quotedOpt("--format", "{{.ID}} {{.Status}}")
-	out, err := j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--filter", "status=running"), raw("--no-trunc"), fmtOpt), nil)
+	out, err := j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--filter", "status=running"), raw("--no-trunc"), fmtOpt), cancel)
 	if err != nil {
 		return
 	}
 			break
 		}
 	}
-	_, _ = j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--no-trunc"), fmtOpt), nil)
+	_, _ = j.dockerList(s, j.newDockerCmd("ps", raw("--all"), raw("--filter", "id="+c.id), raw("--no-trunc"), fmtOpt), cancel)
-	if code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), nil); err == nil && code != 0 {
+	if code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), cancel); err == nil && code != 0 {
 		j.tagged(s.num, "##[warning]", fmt.Sprintf("Docker logs fail with exit code %d", code))
 	}
 	j.tagged(s.num, "##[warning]", fmt.Sprintf("Docker container %s is not in running state.", c.id))

M internal/executor/containers_stop.go +140 -31


@@ -6,6 +6,7 @@ import (
6	6		"fmt"
7	7		"math"
8	8		"math/rand/v2"
	9	+	"os"
9	10		"strings"
10	11		"time"
11	12		)

@@ -93,46 +94,154 @@ func (j job) healthchecks(s stepRun, all []*container, cancel <-chan struct{})
93	94
94	95		// stopContainers is "Stop containers" (StopContainersAsync, :105-124 and
95	96		// StopContainerAsync, :328-351). It is registered with the condition always(),
96		-	// so it runs after a failure, a cancellation and a timeout alike.
97		-	func (j job) stopContainers(s stepRun, cancel <-chan struct{}) stepResult {
98		-	num := s.num
	97	+	// so it runs after a failure, a cancellation and a timeout alike; its commands
	98	+	// run on a budget of their own (they do not end with the job's cancellation,
	99	+	// and a stalled daemon does not hold them forever).
	100	+	func (j job) stopContainers(s stepRun, _ <-chan struct{}) stepResult {
	101	+	if j.cs == nil {
	102	+	return resSucceeded
	103	+	}
	104	+	errs := j.cleanupContainers(s)
	105	+	for _, e := range errs {
	106	+	j.errorf(s.num, "%s", e.Error())
	107	+	}
	108	+	if len(errs) > 0 {
	109	+	return resFailed
	110	+	}
	111	+	return resSucceeded
	112	+	}
	113	+
	114	+	// fallbackCleanup is the cleanup of the job when "Stop containers" did not (or
	115	+	// not completely) run: a panic, a runner shutdown that skips the queued post
	116	+	// steps, a budget that ran out. It does only what is left; with nothing left
	117	+	// it runs no docker command. Its messages go to the diagnostics of the runner,
	118	+	// the job log being closed.
	119	+	func (j *job) fallbackCleanup() {
	120	+	if j.cs == nil \|\| !j.cs.pending() {
	121	+	return
	122	+	}
	123	+	for _, e := range j.cleanupContainers(nil) {
	124	+	j.cfg.Log.Warn().Err(e).Msg("fallback cleanup of the job containers")
	125	+	}
	126	+	}
	127	+
	128	+	// pending reports whether something created by the job is still to be removed.
	129	+	func (cs *containerSet) pending() bool {
	130	+	cs.mu.Lock()
	131	+	defer cs.mu.Unlock()
	132	+	if cs.networkTry != "" && !cs.netRemoved \|\| len(cs.dirs) > 0 {
	133	+	return true
	134	+	}
	135	+	for _, c := range cs.all() {
	136	+	if c.createTried && !c.removed {
	137	+	return true
	138	+	}
	139	+	}
	140	+	return false
	141	+	}
	142	+
	143	+	// cleanupContainers removes what the job created: every container, then the
	144	+	// network, then the credentials directories left behind. It attempts all of
	145	+	// them whatever fails, within CleanupBudget, each command within
	146	+	// CleanupCommand, and returns the errors. Logs of a service are best effort and
	147	+	// never stand in the way of its removal. s, when set, gets the runner's lines.
	148	+	// It is idempotent: what is gone is not tried again.
	149	+	func (j job) cleanupContainers(s stepRun) (errs []error) {
99	150		cs := j.cs
	151	+	cs.cleanupMu.Lock()
	152	+	defer cs.cleanupMu.Unlock()
	153	+	t := j.timeouts()
	154	+	budget, budgetOut, stop := bound(nil, t.CleanupBudget)
	155	+	defer stop()
	156	+	out := func(line string) {
	157	+	if s != nil {
	158	+	j.out(s.num, line)
	159	+	}
	160	+	}
	161	+	run := func(c dockerCmd) (int, error) {
	162	+	select {
	163	+	case <-budget:
	164	+	if budgetOut() {
	165	+	return 0, ghErrorf("The cleanup of the containers ran out of time (%s)", t.CleanupBudget)
	166	+	}
	167	+	default:
	168	+	}
	169	+	_, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: budget, all: true, timeout: t.CleanupCommand})
	170	+	return code, err
	171	+	}
100	172		for _, c := range cs.all() {
101		-	if c.id == "" {
	173	+	if !c.createTried \|\| c.removed {
102	174		continue
103	175		}
104		-	if !c.isJob && !c.failedInit {
105		-	j.out(num, "Print service container logs: "+c.name)
106		-	code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), cancel)
107		-	if err != nil {
108		-	return j.containerResult(s, err)
109		-	}
110		-	if code != 0 {
111		-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker logs fail with exit code %d", code))
112		-	}
	176	+	target := c.id
	177	+	if target == "" {
	178	+	// The create is not known to have finished: the name was ours.
	179	+	target = c.name
113	180		}
114		-	j.out(num, "Stop and remove container: "+c.name)
115		-	code, err := j.dockerCode(s, j.newDockerCmd("rm", raw("--force", c.id)), cancel)
116		-	if err != nil {
117		-	return j.containerResult(s, err)
	181	+	if c.id != "" && !c.isJob && !c.failedInit {
	182	+	out("Print service container logs: " + c.name)
	183	+	code, err := run(j.newDockerCmd("logs", raw("--details", c.id)))
	184	+	switch {
	185	+	case err != nil:
	186	+	// Best effort, as the runner's warning for a failed logs: it
	187	+	// neither fails the step nor stops the removal.
	188	+	j.warnTo(s, "Docker logs failed: "+err.Error())
	189	+	case code != 0:
	190	+	j.warnTo(s, fmt.Sprintf("Docker logs fail with exit code %d", code))
	191	+	}
118	192		}
119		-	if code != 0 {
120		-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker rm fail with exit code %d", code))
	193	+	out("Stop and remove container: " + c.name)
	194	+	code, err := run(j.newDockerCmd("rm", raw("--force", target)))
	195	+	switch {
	196	+	case err != nil:
	197	+	errs = append(errs, err)
	198	+	case code != 0:
	199	+	j.warnTo(s, fmt.Sprintf("Docker rm fail with exit code %d", code))
	200	+	if c.id == "" {
	201	+	c.removed = true // a name that docker does not know is nothing to remove
	202	+	}
	203	+	default:
	204	+	c.removed = true
121	205		}
122	206		}
123		-	// The network of the first container; without one the runner throws
124		-	// (ArgUtil.NotNull, ContainerOperationProvider.cs:272).
125		-	if len(cs.all()) > 0 && cs.all()[0].network == "" {
126		-	return j.containerResult(s, ghErrorf("Value cannot be null. (Parameter 'network')"))
	207	+	cs.mu.Lock()
	208	+	network, gone := cs.networkTry, cs.netRemoved
	209	+	cs.mu.Unlock()
	210	+	if network != "" && !gone {
	211	+	out("Remove container network: " + network)
	212	+	code, err := run(j.newDockerCmd("network", raw("rm", network)))
	213	+	switch {
	214	+	case err != nil:
	215	+	errs = append(errs, err)
	216	+	case code != 0:
	217	+	j.warnTo(s, fmt.Sprintf("Docker network rm failed with exit code %d", code))
	218	+	cs.mu.Lock()
	219	+	cs.netRemoved = true // the runner warns and moves on; docker knows no such network (or refuses)
	220	+	cs.mu.Unlock()
	221	+	default:
	222	+	cs.mu.Lock()
	223	+	cs.netRemoved = true
	224	+	cs.mu.Unlock()
	225	+	}
127	226		}
128		-	network := cs.all()[0].network
129		-	j.out(num, "Remove container network: "+network)
130		-	code, err := j.dockerCode(s, j.newDockerCmd("network", raw("rm", network)), cancel)
131		-	if err != nil {
132		-	return j.containerResult(s, err)
	227	+	cs.mu.Lock()
	228	+	dirs := cs.dirs
	229	+	cs.dirs = nil
	230	+	cs.mu.Unlock()
	231	+	for _, d := range dirs {
	232	+	if err := os.RemoveAll(d); err != nil {
	233	+	cs.addDir(d)
	234	+	errs = append(errs, ghErrorf("Failed to remove directory containing Docker client credentials: %v", err))
	235	+	}
133	236		}
134		-	if code != 0 {
135		-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker network rm failed with exit code %d", code))
	237	+	return errs
	238	+	}
	239	+
	240	+	// warnTo logs a warning to the step, or to the diagnostics without one.
	241	+	func (j job) warnTo(s stepRun, msg string) {
	242	+	if s != nil {
	243	+	j.tagged(s.num, "##[warning]", msg)
	244	+	return
136	245		}
137		-	return resSucceeded
	246	+	j.cfg.Log.Warn().Msg(msg)
138	247		}


@@ -6,6 +6,7 @@ import (
6		"fmt"	6		"fmt"
7		"math"	7		"math"
8		"math/rand/v2"	8		"math/rand/v2"
			9	+	"os"
9		"strings"	10		"strings"
10		"time"	11		"time"
11		)	12		)

@@ -93,46 +94,154 @@ func (j job) healthchecks(s stepRun, all []*container, cancel <-chan struct{})
93			94
94		// stopContainers is "Stop containers" (StopContainersAsync, :105-124 and	95		// stopContainers is "Stop containers" (StopContainersAsync, :105-124 and
95		// StopContainerAsync, :328-351). It is registered with the condition always(),	96		// StopContainerAsync, :328-351). It is registered with the condition always(),
96	-	// so it runs after a failure, a cancellation and a timeout alike.	97	+	// so it runs after a failure, a cancellation and a timeout alike; its commands
97	-	func (j job) stopContainers(s stepRun, cancel <-chan struct{}) stepResult {	98	+	// run on a budget of their own (they do not end with the job's cancellation,
98	-	num := s.num	99	+	// and a stalled daemon does not hold them forever).
			100	+	func (j job) stopContainers(s stepRun, _ <-chan struct{}) stepResult {
			101	+	if j.cs == nil {
			102	+	return resSucceeded
			103	+	}
			104	+	errs := j.cleanupContainers(s)
			105	+	for _, e := range errs {
			106	+	j.errorf(s.num, "%s", e.Error())
			107	+	}
			108	+	if len(errs) > 0 {
			109	+	return resFailed
			110	+	}
			111	+	return resSucceeded
			112	+	}
			113	+
			114	+	// fallbackCleanup is the cleanup of the job when "Stop containers" did not (or
			115	+	// not completely) run: a panic, a runner shutdown that skips the queued post
			116	+	// steps, a budget that ran out. It does only what is left; with nothing left
			117	+	// it runs no docker command. Its messages go to the diagnostics of the runner,
			118	+	// the job log being closed.
			119	+	func (j *job) fallbackCleanup() {
			120	+	if j.cs == nil \|\| !j.cs.pending() {
			121	+	return
			122	+	}
			123	+	for _, e := range j.cleanupContainers(nil) {
			124	+	j.cfg.Log.Warn().Err(e).Msg("fallback cleanup of the job containers")
			125	+	}
			126	+	}
			127	+
			128	+	// pending reports whether something created by the job is still to be removed.
			129	+	func (cs *containerSet) pending() bool {
			130	+	cs.mu.Lock()
			131	+	defer cs.mu.Unlock()
			132	+	if cs.networkTry != "" && !cs.netRemoved \|\| len(cs.dirs) > 0 {
			133	+	return true
			134	+	}
			135	+	for _, c := range cs.all() {
			136	+	if c.createTried && !c.removed {
			137	+	return true
			138	+	}
			139	+	}
			140	+	return false
			141	+	}
			142	+
			143	+	// cleanupContainers removes what the job created: every container, then the
			144	+	// network, then the credentials directories left behind. It attempts all of
			145	+	// them whatever fails, within CleanupBudget, each command within
			146	+	// CleanupCommand, and returns the errors. Logs of a service are best effort and
			147	+	// never stand in the way of its removal. s, when set, gets the runner's lines.
			148	+	// It is idempotent: what is gone is not tried again.
			149	+	func (j job) cleanupContainers(s stepRun) (errs []error) {
99		cs := j.cs	150		cs := j.cs
			151	+	cs.cleanupMu.Lock()
			152	+	defer cs.cleanupMu.Unlock()
			153	+	t := j.timeouts()
			154	+	budget, budgetOut, stop := bound(nil, t.CleanupBudget)
			155	+	defer stop()
			156	+	out := func(line string) {
			157	+	if s != nil {
			158	+	j.out(s.num, line)
			159	+	}
			160	+	}
			161	+	run := func(c dockerCmd) (int, error) {
			162	+	select {
			163	+	case <-budget:
			164	+	if budgetOut() {
			165	+	return 0, ghErrorf("The cleanup of the containers ran out of time (%s)", t.CleanupBudget)
			166	+	}
			167	+	default:
			168	+	}
			169	+	_, code, err := j.dockerDo(c, dockerOpts{s: s, cancel: budget, all: true, timeout: t.CleanupCommand})
			170	+	return code, err
			171	+	}
100		for _, c := range cs.all() {	172		for _, c := range cs.all() {
101	-	if c.id == "" {	173	+	if !c.createTried \|\| c.removed {
102		continue	174		continue
103		}	175		}
104	-	if !c.isJob && !c.failedInit {	176	+	target := c.id
105	-	j.out(num, "Print service container logs: "+c.name)	177	+	if target == "" {
106	-	code, err := j.dockerCode(s, j.newDockerCmd("logs", raw("--details", c.id)), cancel)	178	+	// The create is not known to have finished: the name was ours.
107	-	if err != nil {	179	+	target = c.name
108	-	return j.containerResult(s, err)
109	-	}
110	-	if code != 0 {
111	-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker logs fail with exit code %d", code))
112	-	}
113		}	180		}
114	-	j.out(num, "Stop and remove container: "+c.name)	181	+	if c.id != "" && !c.isJob && !c.failedInit {
115	-	code, err := j.dockerCode(s, j.newDockerCmd("rm", raw("--force", c.id)), cancel)	182	+	out("Print service container logs: " + c.name)
116	-	if err != nil {	183	+	code, err := run(j.newDockerCmd("logs", raw("--details", c.id)))
117	-	return j.containerResult(s, err)	184	+	switch {
			185	+	case err != nil:
			186	+	// Best effort, as the runner's warning for a failed logs: it
			187	+	// neither fails the step nor stops the removal.
			188	+	j.warnTo(s, "Docker logs failed: "+err.Error())
			189	+	case code != 0:
			190	+	j.warnTo(s, fmt.Sprintf("Docker logs fail with exit code %d", code))
			191	+	}
118		}	192		}
119	-	if code != 0 {	193	+	out("Stop and remove container: " + c.name)
120	-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker rm fail with exit code %d", code))	194	+	code, err := run(j.newDockerCmd("rm", raw("--force", target)))
			195	+	switch {
			196	+	case err != nil:
			197	+	errs = append(errs, err)
			198	+	case code != 0:
			199	+	j.warnTo(s, fmt.Sprintf("Docker rm fail with exit code %d", code))
			200	+	if c.id == "" {
			201	+	c.removed = true // a name that docker does not know is nothing to remove
			202	+	}
			203	+	default:
			204	+	c.removed = true
121		}	205		}
122		}	206		}
123	-	// The network of the first container; without one the runner throws	207	+	cs.mu.Lock()
124	-	// (ArgUtil.NotNull, ContainerOperationProvider.cs:272).	208	+	network, gone := cs.networkTry, cs.netRemoved
125	-	if len(cs.all()) > 0 && cs.all()[0].network == "" {	209	+	cs.mu.Unlock()
126	-	return j.containerResult(s, ghErrorf("Value cannot be null. (Parameter 'network')"))	210	+	if network != "" && !gone {
			211	+	out("Remove container network: " + network)
			212	+	code, err := run(j.newDockerCmd("network", raw("rm", network)))
			213	+	switch {
			214	+	case err != nil:
			215	+	errs = append(errs, err)
			216	+	case code != 0:
			217	+	j.warnTo(s, fmt.Sprintf("Docker network rm failed with exit code %d", code))
			218	+	cs.mu.Lock()
			219	+	cs.netRemoved = true // the runner warns and moves on; docker knows no such network (or refuses)
			220	+	cs.mu.Unlock()
			221	+	default:
			222	+	cs.mu.Lock()
			223	+	cs.netRemoved = true
			224	+	cs.mu.Unlock()
			225	+	}
127		}	226		}
128	-	network := cs.all()[0].network	227	+	cs.mu.Lock()
129	-	j.out(num, "Remove container network: "+network)	228	+	dirs := cs.dirs
130	-	code, err := j.dockerCode(s, j.newDockerCmd("network", raw("rm", network)), cancel)	229	+	cs.dirs = nil
131	-	if err != nil {	230	+	cs.mu.Unlock()
132	-	return j.containerResult(s, err)	231	+	for _, d := range dirs {
			232	+	if err := os.RemoveAll(d); err != nil {
			233	+	cs.addDir(d)
			234	+	errs = append(errs, ghErrorf("Failed to remove directory containing Docker client credentials: %v", err))
			235	+	}
133		}	236		}
134	-	if code != 0 {	237	+	return errs
135	-	j.tagged(num, "##[warning]", fmt.Sprintf("Docker network rm failed with exit code %d", code))	238	+	}
			239	+
			240	+	// warnTo logs a warning to the step, or to the diagnostics without one.
			241	+	func (j job) warnTo(s stepRun, msg string) {
			242	+	if s != nil {
			243	+	j.tagged(s.num, "##[warning]", msg)
			244	+	return
136		}	245		}
137	-	return resSucceeded	246	+	j.cfg.Log.Warn().Msg(msg)
138		}	247		}

M internal/executor/job.go +3 -0


@@ -213,6 +213,9 @@ func Run(cancel *Canceller, cfg Config) Result {
213	213		}
214	214
215	215		func (j *job) run() Result {
	216	+	// Whatever happens below (a panic, a shutdown that skips the post steps),
	217	+	// the containers, the network and the credentials directories are removed.
	218	+	defer j.fallbackCleanup()
216	219		limit := j.cfg.Deadline
217	220		if limit <= 0 {
218	221		limit = time.Duration(j.timeoutMinutes()) * time.Minute


@@ -213,6 +213,9 @@ func Run(cancel *Canceller, cfg Config) Result {
213	}	213		}
214		214
215	func (j *job) run() Result {	215		func (j *job) run() Result {
		216	+	// Whatever happens below (a panic, a shutdown that skips the post steps),
		217	+	// the containers, the network and the credentials directories are removed.
		218	+	defer j.fallbackCleanup()
216	limit := j.cfg.Deadline	219		limit := j.cfg.Deadline
217	if limit <= 0 {	220		if limit <= 0 {
218	limit = time.Duration(j.timeoutMinutes()) * time.Minute	221		limit = time.Duration(j.timeoutMinutes()) * time.Minute