diff --git a/act/container/host_environment.go b/act/container/host_environment.go index 8a88e473..8aa3f8ce 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -31,18 +31,15 @@ import ( ) type HostEnvironment struct { - Path string - TmpDir string - ToolCache string - Workdir string - // CleanWorkdir means teardown owns Workdir and may delete it. Leave false - // when Workdir points at a caller-owned checkout (e.g. `act` local mode). - CleanWorkdir bool - ActPath string - Root string - TrackingID string // RUNNER_TRACKING_ID of the job, a step with another value keeps its leftovers - StdOut io.Writer - AllocatePTY bool // allocate a pseudo-TTY for each step's process + Path string + TmpDir string + ToolCache string + Workdir string + ActPath string + Root string + TrackingID string // RUNNER_TRACKING_ID of the job, a step with another value keeps its leftovers + StdOut io.Writer + AllocatePTY bool // allocate a pseudo-TTY for each step's process // procGroup owns every process the job's steps start. Atomic: Remove may read // it while a step is still starting. @@ -436,24 +433,14 @@ func (e *HostEnvironment) Remove() common.Executor { rmCtx, rmCancel := context.WithTimeout(context.Background(), hostCleanupTimeout) defer rmCancel() - var errs []error - if err := removePathWithRetry(rmCtx, cmp.Or(e.Root, e.Path)); err != nil { + err := removePathWithRetry(rmCtx, cmp.Or(e.Root, e.Path)) + if err != nil { logger.Warnf("failed to remove host misc state %s: %v", cmp.Or(e.Root, e.Path), err) - errs = append(errs, err) } - if e.CleanWorkdir { - if err := removePathWithRetry(rmCtx, e.Workdir); err != nil { - logger.Warnf("failed to remove host workspace %s: %v", e.Workdir, err) - errs = append(errs, err) - } + if errors.Is(err, context.DeadlineExceeded) { + return nil // a stalled delete must not fail job completion } - for _, err := range errs { - if !errors.Is(err, context.DeadlineExceeded) { - return errors.Join(errs...) - } - } - // Teardown timed out; warned above. Do not fail job completion over it. - return nil + return err } } diff --git a/act/container/host_environment_test.go b/act/container/host_environment_test.go index 67bb8dc8..49ae0d46 100644 --- a/act/container/host_environment_test.go +++ b/act/container/host_environment_test.go @@ -158,27 +158,12 @@ func TestHostEnvironmentAllocatePTY(t *testing.T) { } func TestHostEnvironmentRemoveWorkdir(t *testing.T) { - for name, owned := range map[string]bool{"preserves caller workdir": false, "cleans owned workdir": true} { - t.Run(name, func(t *testing.T) { - miscRoot, workdir := t.TempDir(), t.TempDir() - e := &HostEnvironment{ - Path: filepath.Join(miscRoot, "hostexecutor"), - Workdir: workdir, - CleanWorkdir: owned, - Root: miscRoot, - } - require.NoError(t, os.MkdirAll(e.Path, 0o700)) - require.NoError(t, os.MkdirAll(filepath.Join(miscRoot, "tmp"), 0o700)) - require.NoError(t, e.Remove()(t.Context())) - assert.NoDirExists(t, miscRoot) - _, err := os.Stat(workdir) - if owned { - assert.ErrorIs(t, err, os.ErrNotExist) - } else { - require.NoError(t, err) - } - }) - } + miscRoot, workdir := t.TempDir(), t.TempDir() + e := &HostEnvironment{Path: filepath.Join(miscRoot, "hostexecutor"), Workdir: workdir, Root: miscRoot} + require.NoError(t, os.MkdirAll(e.Path, 0o700)) + require.NoError(t, e.Remove()(t.Context())) + assert.NoDirExists(t, miscRoot) + assert.DirExists(t, workdir) } func TestRemoveAllWithContextDoesNotHangOnStuckDelete(t *testing.T) { diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 8575b72c..daba5dcf 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -468,16 +468,15 @@ func (rc *RunContext) startHostEnvironment() common.Executor { return err } rc.JobContainer = &container.HostEnvironment{ - Path: path, - TmpDir: runnerTmp, - ToolCache: toolCache, - Workdir: rc.Config.Workdir, - CleanWorkdir: rc.Config.CleanWorkdir, - ActPath: actPath, - Root: miscpath, - TrackingID: rc.runnerTrackingID(), - StdOut: logWriter, - AllocatePTY: rc.Config.AllocatePTY, + Path: path, + TmpDir: runnerTmp, + ToolCache: toolCache, + Workdir: rc.Config.Workdir, + ActPath: actPath, + Root: miscpath, + TrackingID: rc.runnerTrackingID(), + StdOut: logWriter, + AllocatePTY: rc.Config.AllocatePTY, } rc.cleanUpJobContainer = rc.JobContainer.Remove() rc.setRunnerContextEnv(ctx) diff --git a/act/runner/runner.go b/act/runner/runner.go index 1a1cd261..6ede6459 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -70,7 +70,6 @@ type Config struct { EventJSON string // the content of JSON file to use for event.json in containers, overrides EventPath ContainerNamePrefix string // the prefix of container name ContainerMaxLifetime time.Duration // the max lifetime of job containers - CleanWorkdir bool // remove host executor workdir on teardown DefaultActionInstance string // the default actions web site // DefaultActionInstanceIsSelfHosted reports whether DefaultActionInstance is this // self-hosted Gitea (DEFAULT_ACTIONS_URL=self). It gates token trust: only then may the diff --git a/docs/post-task-script.md b/docs/post-task-script.md index cca3f4bb..deb80c8b 100644 --- a/docs/post-task-script.md +++ b/docs/post-task-script.md @@ -9,13 +9,13 @@ It is configured under `runner.post_task_script` in the runner YAML config (see For each task, execution order is: 1. Workflow runs (steps, actions, containers). -2. In-job cleanup (action `post:` steps, container stop/remove). +2. In-job cleanup (action `post:` steps, container stop/remove, host job directory removal). 3. Job outputs are reported to Gitea. 4. Bind-workdir workspace removal, when `container.bind_workdir` is enabled. 5. **Post-task script** (this hook). 6. Final task acknowledgement to Gitea (`reporter.Close()`). -The script is **additive**: it does not replace any built-in cleanup. When `container.bind_workdir` is enabled, the task workspace directory has usually already been deleted before the script starts. `GITEA_WORKSPACE` is still set to the path the job used, for reference. +The script is **additive**: it does not replace any built-in cleanup. The workspace of host jobs, and of container jobs with `container.bind_workdir`, is deleted before the script starts. ## Runner stays offline until the script finishes @@ -75,7 +75,7 @@ The script receives `runner.envs` / `runner.env_file` values plus: | `GITEA_TASK_ID` | Numeric task ID. | | `GITEA_RUN_ID` | Workflow run ID, when provided by the server. | | `GITEA_REPOSITORY` | Repository slug (`owner/name`). | -| `GITEA_WORKSPACE` | Workspace path used for the job (may already be deleted). | +| `GITEA_WORKSPACE` | Workspace path of the job. | | `GITEA_JOB_RESULT` | `success`, `failure`, `cancelled`, `skipped`, or `unknown`. | The script environment is **not** a full copy of the job container environment. System variables such as `PATH` are only present if you define them in `runner.envs` or `runner.env_file`. diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index a6ef3137..13c8f8cc 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -603,7 +603,6 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. EventJSON: string(eventJSON), ContainerNamePrefix: fmt.Sprintf("GITEA-ACTIONS-TASK-%d", task.Id), ContainerMaxLifetime: maxLifetime, - CleanWorkdir: true, ContainerNetworkMode: docker_container.NetworkMode(r.cfg.Container.Network), CacheContainer: cacheContainer, ContainerNetworkCreateOptions: container.NewDockerNetworkCreateExecutorInput{