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.

<!--start release-notes-assistant-->
<!--URL:https://code.forgejo.org/forgejo/runner-->
- bug fixes
  - [PR](https://code.forgejo.org/forgejo/runner/pulls/851): <!--number 851 --><!--line 0 --><!--description Zml4OiBjb250YWluZXIgcmVtb3ZhbCBpcyB0aGUgcnVubmVyIHJlc3BvbnNpYmlsaXR5-->fix: container removal is the runner responsibility<!--description-->
<!--end release-notes-assistant-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/851
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.code.forgejo.org>
Co-authored-by: Earl Warren <contact@earl-warren.org>
Co-committed-by: Earl Warren <contact@earl-warren.org>

act/container/container_types.go

@@ -35,9 +35,6 @@ type NewContainerInput struct {
 	ConfigOptions string
 	JobOptions    string
 
-	// Gitea specific
-	AutoRemove bool
-
 	ValidVolumes []string
 }
 

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)
 

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,

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()

act/runner/job_executor_test.go

@@ -141,6 +141,7 @@ func TestJobExecutorNewJobExecutor(t *testing.T) {
 			executedSteps: []string{
 				"startContainer",
 				"step1",
+				"stopContainer",
 				"interpolateOutputs",
 				"closeContainer",
 			},

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),

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,
 				},
 			},
 		},

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

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)

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

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),

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),