From 773bb92aa3f121face65bb1fc909f45410e52a0c Mon Sep 17 00:00:00 2001 From: Earl Warren Date: Thu, 14 Aug 2025 07:47:50 +0000 Subject: [PATCH] fix: container removal is the runner responsibility (#851) If a container is configured for implicit removal in the docker/podman server, it will race against the explicit removal performed by the runner. - bug fixes - [PR](https://code.forgejo.org/forgejo/runner/pulls/851): fix: container removal is the runner responsibility Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/851 Reviewed-by: Mathieu Fenniak Co-authored-by: Earl Warren Co-committed-by: Earl Warren --- act/container/container_types.go | 3 --- act/container/docker_run.go | 1 - act/runner/action.go | 1 - act/runner/job_executor.go | 2 +- act/runner/job_executor_test.go | 1 + act/runner/run_context.go | 2 -- act/runner/run_context_test.go | 3 --- act/runner/runner.go | 1 - act/runner/runner_test.go | 1 - act/runner/step_docker.go | 1 - internal/app/cmd/exec.go | 1 - internal/app/run/runner.go | 1 - 12 files changed, 2 insertions(+), 16 deletions(-) diff --git a/act/container/container_types.go b/act/container/container_types.go index 8fe651f3..eb524cef 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -35,9 +35,6 @@ type NewContainerInput struct { ConfigOptions string JobOptions string - // Gitea specific - AutoRemove bool - ValidVolumes []string } diff --git a/act/container/docker_run.go b/act/container/docker_run.go index b64703c0..4e218ecf 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -577,7 +577,6 @@ func (cr *containerReference) create(capAdd, capDrop []string) common.Executor { Privileged: input.Privileged, UsernsMode: container.UsernsMode(input.UsernsMode), PortBindings: input.PortBindings, - AutoRemove: input.AutoRemove, } logger.Debugf("Common container.HostConfig ==> %+v", hostConfig) diff --git a/act/runner/action.go b/act/runner/action.go index 3f50bccc..8ca5f510 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -451,7 +451,6 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - AutoRemove: rc.Config.AutoRemove, ValidVolumes: validVolumes, ConfigOptions: rc.Config.ContainerOptions, diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index f9ab36a0..848e56f8 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -119,7 +119,7 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo setJobOutputs(ctx, rc) var err error - if rc.Config.AutoRemove || jobError == nil { + { // Separate timeout for cleanup tasks; logger is cleared so that cleanup logs go to runner, not job ctx, cancel := context.WithTimeout(context.Background(), cleanupTimeout) defer cancel() diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 5e113d83..b9c434a1 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -141,6 +141,7 @@ func TestJobExecutorNewJobExecutor(t *testing.T) { executedSteps: []string{ "startContainer", "step1", + "stopContainer", "interpolateOutputs", "closeContainer", }, diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 219adea4..67ceefa4 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -501,7 +501,6 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error { Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - AutoRemove: rc.Config.AutoRemove, NetworkMode: networkName, NetworkAliases: []string{sanitizeNetworkAlias(ctx, serviceID)}, ExposedPorts: exposedPorts, @@ -569,7 +568,6 @@ func (rc *RunContext) prepareJobContainer(ctx context.Context) error { Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - AutoRemove: rc.Config.AutoRemove, ValidVolumes: validVolumes, JobOptions: rc.options(ctx), diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 1a6aa66c..09506d47 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -735,7 +735,6 @@ jobs: PortBindings: nil, ConfigOptions: "", JobOptions: "", - AutoRemove: false, ValidVolumes: []string{ "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB", "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855_JOB-env", @@ -763,7 +762,6 @@ jobs: PortBindings: nat.PortMap{}, ConfigOptions: "", JobOptions: "", - AutoRemove: false, }, { Name: "WORKFLOW-e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca49599-c233cf913e1d0c90cc1404ee09917e625f9cb82156ca3d7cb10b729d563728ea", @@ -786,7 +784,6 @@ jobs: PortBindings: nat.PortMap{}, ConfigOptions: "", JobOptions: "", - AutoRemove: false, }, }, }, diff --git a/act/runner/runner.go b/act/runner/runner.go index 19aea849..26d30c78 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -52,7 +52,6 @@ type Config struct { GitHubInstance string // GitHub instance to use, default "github.com" ContainerCapAdd []string // list of kernel capabilities to add to the containers ContainerCapDrop []string // list of kernel capabilities to remove from the containers - AutoRemove bool // controls if the container is automatically removed upon workflow completion ArtifactServerPath string // the path where the artifact server stores uploads ArtifactServerAddr string // the address the artifact server binds to ArtifactServerPort string // the port the artifact server binds to diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 3811811b..8f2ab0db 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -191,7 +191,6 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config Matrix: cfg.Matrix, JobLoggerLevel: cfg.JobLoggerLevel, ActionCache: cfg.ActionCache, - AutoRemove: true, } runner, err := New(runnerConfig) diff --git a/act/runner/step_docker.go b/act/runner/step_docker.go index db86ad35..055098a8 100644 --- a/act/runner/step_docker.go +++ b/act/runner/step_docker.go @@ -129,7 +129,6 @@ func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, e Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - AutoRemove: rc.Config.AutoRemove, ValidVolumes: validVolumes, }) return stepContainer diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index 8c197711..8559b960 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -394,7 +394,6 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command ContainerCapAdd: execArgs.containerCapAdd, ContainerCapDrop: execArgs.containerCapDrop, ContainerOptions: execArgs.containerOptions, - AutoRemove: true, NoSkipCheckout: execArgs.noSkipCheckout, // PresetGitHubContext: preset, // EventJSON: string(eventJSON), diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index e8d91822..3749ee50 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -314,7 +314,6 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. Env: runEnvs, Secrets: task.Secrets, GitHubInstance: strings.TrimSuffix(r.client.Address(), "/"), - AutoRemove: true, NoSkipCheckout: true, PresetGitHubContext: preset, EventJSON: string(eventJSON),