From a021d3139dfe157f214849da6245877281a61939 Mon Sep 17 00:00:00 2001 From: bircni Date: Tue, 10 Nov 2026 14:25:50 +0000 Subject: [PATCH] enhance: make shared tool cache safe at any capacity (#1401) Co-authored-by: bircni --- README.md | 2 +- act/runner/run_context.go | 15 +++++++-- act/runner/run_context_test.go | 26 +++++++++++++++ act/runner/runner.go | 3 +- internal/app/run/runner.go | 5 +++ internal/app/run/slots.go | 44 +++++++++++++++++++++++++ internal/app/run/slots_test.go | 27 +++++++++++++++ internal/pkg/config/config.example.yaml | 4 +-- internal/pkg/config/config.go | 6 +--- 9 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 internal/app/run/slots.go create mode 100644 internal/app/run/slots_test.go diff --git a/README.md b/README.md index 400b5f11..6c78097f 100644 --- a/README.md +++ b/README.md @@ -181,7 +181,7 @@ Setup actions like `setup-go` install tools into `RUNNER_TOOL_CACHE`. `runner.to | Mode | Tool cache | | --- | --- | | `none` (default) | Provided by the job image. Versions it lacks are downloaded in every job. | -| `shared` | One volume for all jobs. Only safe with `runner.capacity: 1`. | +| `shared` | One volume per `runner.capacity` slot, reused by later jobs. It hides the tools later job images ship, so use images without them. | To ship tools in a job image, install them to `/opt/hostedtoolcache///` with an empty `.complete` file next to it, as the `-full` [runner images](https://gitea.com/gitea/runner-images) do: diff --git a/act/runner/run_context.go b/act/runner/run_context.go index b06964ef..d95f4d61 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -273,7 +273,7 @@ func (rc *RunContext) validVolumes() []string { volumes = append(volumes, hostEnv.Path, hostEnv.GetActPath()) } if rc.Config.SharedToolCache { - volumes = append(volumes, sharedToolCacheVolume) + volumes = append(volumes, rc.toolCacheVolume()) } if rc.Config.BindWorkdir { volumes = append(volumes, rc.Config.Workdir, rc.workdirMountRoot()) @@ -333,6 +333,13 @@ func (rc *RunContext) startDockerProxy(ctx context.Context) { rc.dockerProxy = container.NewDockerProxy(ctx, rc.jobContainerName()) } +func (rc *RunContext) toolCacheVolume() string { + if rc.Config.ToolCacheSlot == "" { + return sharedToolCacheVolume + } + return sharedToolCacheVolume + "-" + rc.Config.ToolCacheSlot +} + // toolCache returns the tool cache path the job sees, relocatable through RUNNER_TOOL_CACHE. func (rc *RunContext) toolCache(fallback string) string { return cmp.Or(rc.GetEnv()["RUNNER_TOOL_CACHE"], fallback) @@ -393,7 +400,7 @@ func (rc *RunContext) GetBindsAndMounts(ctx context.Context) ([]string, map[stri } if rc.Config.SharedToolCache { if toolCache := rc.toolCache(container.DefaultToolCache); !claimed[toolCache] { - mounts[sharedToolCacheVolume] = toolCache + mounts[rc.toolCacheVolume()] = toolCache } } if hostEnv, ok := rc.JobContainer.(*container.HostEnvironment); ok { @@ -453,6 +460,10 @@ func (rc *RunContext) startHostEnvironment() common.Executor { toolCacheParent = cacheDir } toolCache := rc.toolCache(filepath.Join(toolCacheParent, "tool_cache")) + if rc.Config.ToolCacheSlot != "" { + toolCache = filepath.Join(toolCache, rc.Config.ToolCacheSlot) + rc.Env["RUNNER_TOOL_CACHE"] = toolCache // overrides a RUNNER_TOOL_CACHE the slots would otherwise share + } if err := os.MkdirAll(toolCache, 0o777); err != nil { return err } diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 6c5248cf..d5aa046a 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -559,6 +559,12 @@ func TestRunContext_GetBindsAndMounts(t *testing.T) { rc.containerSpec.Volumes = []string{"/unapproved:" + container.DefaultToolCache} _, gotmount, _ = rc.GetBindsAndMounts(t.Context()) assert.Equal(t, container.DefaultToolCache, gotmount[sharedToolCacheVolume]) + + rc.Config.ToolCacheSlot = "1" + _, gotmount, _ = rc.GetBindsAndMounts(t.Context()) + assert.Equal(t, container.DefaultToolCache, gotmount[sharedToolCacheVolume+"-1"]) + assert.NotContains(t, gotmount, sharedToolCacheVolume) + assert.Contains(t, rc.validVolumes(), sharedToolCacheVolume+"-1") }) t.Run("DaemonMountsAboveOwnerRepo", func(t *testing.T) { @@ -1296,6 +1302,26 @@ func TestPrintStartJobContainerGroupGolden(t *testing.T) { assert.Equal(t, want, buf.String()) } +func TestStartHostEnvironmentToolCacheSlotIsSubdirOfSharedCache(t *testing.T) { + cacheDir := t.TempDir() + override := filepath.Join(cacheDir, "override") + for sharedCache, env := range map[string]map[string]string{ + filepath.Join(cacheDir, "tool_cache"): {}, + override: {"RUNNER_TOOL_CACHE": override}, + } { + rc := &RunContext{ + Config: &Config{ActionCacheDir: cacheDir, SharedToolCache: true, ToolCacheSlot: "1"}, + Env: env, + Run: &model.Run{Workflow: &model.Workflow{Name: "wf"}}, + } + require.NoError(t, rc.startHostEnvironment()(t.Context())) + t.Cleanup(func() { require.NoError(t, rc.cleanUpJobContainer(context.Background())) }) + want := filepath.Join(sharedCache, "1") + assert.Equal(t, want, rc.JobContainer.(*container.HostEnvironment).ToolCache) + assert.Equal(t, want, rc.Env["RUNNER_TOOL_CACHE"]) + } +} + func TestRunContext_cleanupFailedStart(t *testing.T) { type ctxKey string const sentinel = ctxKey("sentinel") diff --git a/act/runner/runner.go b/act/runner/runner.go index 18e60476..db9e8c80 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -84,7 +84,8 @@ type Config struct { JobLoggerLevel *logging.Level // the level of job logger ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers ContainerVolumes []string // mounted into every job, service and docker action container - SharedToolCache bool // one tool cache for all jobs instead of one per job + SharedToolCache bool // keeps the tool cache across jobs instead of one per job + ToolCacheSlot string // keeps concurrent tasks' tool caches apart, empty for slot 0 and in exec InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance ClientCertFile, ClientKeyFile string // PEM client certificate for the Gitea instance CAFile string // PEM CA certificates the Gitea instance's certificate chains to diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index ed3db27b..a6ef3137 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -79,6 +79,7 @@ type Runner struct { runningTasks sync.Map runningCount atomic.Int64 lastIdleCleanupUnixNano atomic.Int64 + slots slotAllocator now func() time.Time healthCheckLast time.Time healthCheckReady bool @@ -451,6 +452,9 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. } }() + slot := r.slots.acquire() + defer r.slots.release(slot) + workflowCallInputs := restoreCalledWorkflowContext(task) for _, line := range r.setupLines(task) { reporter.Logf("%s", line) @@ -623,6 +627,7 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. ValidVolumes: r.cfg.Container.ValidVolumes, ContainerVolumes: r.cfg.Container.Volumes, SharedToolCache: r.cfg.Runner.ToolCacheMode == config.ToolCacheModeShared, + ToolCacheSlot: toolCacheSlot(r.cfg.Runner.ToolCacheMode, slot), InsecureSkipTLS: r.cfg.Runner.Insecure, ClientCertFile: r.cfg.Runner.ClientCertFile, ClientKeyFile: r.cfg.Runner.ClientKeyFile, diff --git a/internal/app/run/slots.go b/internal/app/run/slots.go new file mode 100644 index 00000000..d4e27c25 --- /dev/null +++ b/internal/app/run/slots.go @@ -0,0 +1,44 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package run + +import ( + "slices" + "strconv" + "sync" + + "gitea.com/gitea/runner/internal/pkg/config" +) + +// slotAllocator hands out the lowest free slot, so slot numbers stay below capacity. +type slotAllocator struct { + mu sync.Mutex + used []bool +} + +func (a *slotAllocator) acquire() int { + a.mu.Lock() + defer a.mu.Unlock() + slot := slices.Index(a.used, false) + if slot < 0 { + slot = len(a.used) + a.used = append(a.used, false) + } + a.used[slot] = true + return slot +} + +func (a *slotAllocator) release(slot int) { + a.mu.Lock() + defer a.mu.Unlock() + a.used[slot] = false +} + +// toolCacheSlot leaves slot 0 unnamed, so it keeps the cache a runner had before slots existed. +func toolCacheSlot(mode string, slot int) string { + if mode != config.ToolCacheModeShared || slot == 0 { + return "" + } + return strconv.Itoa(slot) +} diff --git a/internal/app/run/slots_test.go b/internal/app/run/slots_test.go new file mode 100644 index 00000000..475f6feb --- /dev/null +++ b/internal/app/run/slots_test.go @@ -0,0 +1,27 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package run + +import ( + "testing" + + "gitea.com/gitea/runner/internal/pkg/config" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestToolCacheSlotReusesLowestFree(t *testing.T) { + var slots slotAllocator + require.Equal(t, 0, slots.acquire()) + require.Equal(t, 1, slots.acquire()) + require.Equal(t, 2, slots.acquire()) + slots.release(1) + assert.Equal(t, 1, slots.acquire()) + assert.Equal(t, 3, slots.acquire()) + + assert.Equal(t, "3", toolCacheSlot(config.ToolCacheModeShared, 3)) + assert.Empty(t, toolCacheSlot(config.ToolCacheModeShared, 0)) + assert.Empty(t, toolCacheSlot(config.ToolCacheModeNone, 3)) +} diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index 3c98b981..c2bcd54a 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -132,8 +132,8 @@ runner: # What to mount at RUNNER_TOOL_CACHE (/opt/hostedtoolcache), where setup actions install tools: # none: nothing. A docker job sees what its image ships there, a host job an empty dir, and # either way what it installs is gone when the job ends. - # shared: one tool cache every job reuses. Two jobs writing the same tool version at once - # corrupt it, so use it only with capacity 1. + # shared: one tool cache per capacity slot, reused by later jobs. It hides the tools later job + # images ship, so use images without them. #tool_cache_mode: none # Optional executable on the host, run once after each task's built-in cleanup # (post-steps, container teardown, bind-workdir removal). Additive only. diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index e6a39ade..79121801 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -193,7 +193,7 @@ type Container struct { Sweep bool `yaml:"sweep"` // Sweep removes the containers, networks and volumes a job creates through its Docker proxy when the job ends. } -// Values of Runner.ToolCacheMode: the runner mounts no tool cache, or one that every job reuses. +// Values of Runner.ToolCacheMode: the runner mounts no tool cache, or one per capacity slot that its jobs reuse. const ( ToolCacheModeNone = "none" ToolCacheModeShared = "shared" @@ -454,10 +454,6 @@ func LoadDefault(file string) (*Config, error) { } // Validate and fix invalid config combinations to prevent confusing behavior. - if cfg.Runner.ToolCacheMode == ToolCacheModeShared && cfg.Runner.Capacity > 1 { - logging.Warnf("runner.tool_cache_mode %q with capacity %d: two jobs writing the same tool version at once corrupt it", - ToolCacheModeShared, cfg.Runner.Capacity) - } if cfg.Runner.FetchTimeout > RequestTimeout { logging.Warnf("fetch_timeout (%v) exceeds the RPC timeout (%v), capping it", cfg.Runner.FetchTimeout, RequestTimeout) cfg.Runner.FetchTimeout = RequestTimeout