build: Isolate Docker release caches - #1156
Conversation
The release script clears the Go build cache between target platforms. Host cache mounts let this cleanup race with host Go commands. Keep both Go caches in the container's writable layer. Trust the fixed checkout path without changing host Git config.
Fresh module and build caches use about 3.2 GB in the release container. Recommend 5 GB of free Docker storage for packaging overhead. Explain why per-target cleanup and --rm keep this use temporary.
|
/gateway review |
There was a problem hiding this comment.
Gateway review — 2 findings
🔴 0 Blocker · 🟠 0 Major · 🟡 2 Minor · 🔵 0 Nit
Summary
The change removes the two host bind mounts for GOCACHE/GOMODCACHE from DOCKER_RELEASE_ARGS and relies on the image-provided /tmp/build/.cache and /tmp/build/.modcache, which the builder Dockerfile already creates and chmod -R 777s before WORKDIR. That closes the described race — go clean -cache inside the container can no longer unlink entries a host go command is recreating — and it removes the last reason for docker-release to shell out to a host go env.
Reproducibility is unaffected: the in-container cache paths are identical to what the mounts previously targeted, so -trimpath/-buildid= output does not change, and module integrity is still pinned by go.sum. The GIT_CONFIG_COUNT/GIT_CONFIG_KEY_0/GIT_CONFIG_VALUE_0 triple is the right shape for this: with --user $(id -u):$(id -g) and no HOME set in the golang image, a git config --global write inside the container would target an unwritable /.gitconfig, and the env form sidesteps that without touching host config. Only the checkout's own top-level path is trusted, which is the path git actually ownership-checks.
Two operational follow-ups, both minor; nothing blocking.
Bot commands
/gateway re-review— re-run after pushing changes (maintainers)/gateway dismiss <id>— silence a finding (maintainers)/gateway explain <id>— elaborate on a finding (anyone)
| # the build cache between platforms, so mounting the host cache would delete | ||
| # host entries and race with Go commands running outside the container. | ||
| # Trust only the checkout's fixed container path without changing host config. | ||
| DOCKER_RELEASE_ARGS = --rm $(DOCKER_RELEASE_USER_ARGS) \ |
There was a problem hiding this comment.
🟡 F1 (Minor) — Release CI may still cache now-unused Go cache paths · Makefile:191
Dropping the two mounts means every release run starts from an empty module cache and a cold build cache, so the container re-downloads the full module graph each time. docs/release.md states that .github/workflows/release.yaml runs make docker-release; that workflow is not in the provided context, so if it carries an actions/cache step keyed on the host GOCACHE/GOMODCACHE paths those mounts used to expose, that cache is now dead weight on every release and should be removed alongside this change.
| the container and are discarded with it. The release build never reads, | ||
| writes, or clears the host's Go caches. | ||
|
|
||
| Allow at least 5 GB of free space in Docker's storage for these temporary |
There was a problem hiding this comment.
🟡 F2 (Minor) — 5 GB guidance unverified by a full-platform release · docs/release.md:38
The new disk-space figure and the "clears its build cache between target platforms" behavior only get exercised on a multi-platform run, but the PR's test notes cover sys=linux-amd64 only. One full make docker-release across all six BUILD_SYSTEM targets would confirm the container-local caches actually stay inside the documented budget before that number ships as guidance.
|
🤖 gateway audit metadata for this PR — auto-generated, please don't edit. |
The problem
make docker-releasebind-mounts the host Go build and module caches into the release container. The release script runsgo clean -cachebetween target platforms. A host Go command can recreate a cache entry while the container removes it, which aborts the release withunlinkat ... directory not empty. The cleanup can also clear the user's normal host build cache.Docker Desktop can also report the mounted checkout as unsafe, which forces callers to replace
DOCKER_RELEASE_ARGSwith host-specific Git configuration.The fix
Keep both Go caches in the container's writable layer. The source checkout remains the only host bind mount because the build must write release artifacts back to the caller.
Pass a scoped Git configuration that trusts only
/tmp/build/wavelength. This handles Docker Desktop ownership mapping without changing host Git configuration.Document the cache boundary in the release guide.
What does not change
The pinned builder image, release script, artifacts, reproducibility flags, platform list, and host user mapping are unchanged. Docker still discards the container and its caches after the build.
Tests
make fmt-changedmake lint-changed-localmake doc-checkmake commitmsg-lint range="origin/main..HEAD"SKIP_VERSION_CHECK=1 make docker-release tag=isolation-test sys=linux-amd64from a clean normal clone. The release completed through cache cleanup, archive generation, and manifest hashing without host cache mounts.