fix(ci): stop the layer-cache prune from being able to fail a build - #7252
Merged
Conversation
The settle loop added in 4f65103 reads `cur="$(total)"`, and composite steps run under `bash -e -o pipefail`, where an assignment takes its command substitution's exit status. So any failing `buildctl du` aborts the step and fails the build. `echo "$(total)"` survives the same failure, which is why this was invisible in the paths that ran first. That is not hypothetical. Deleting a sticky disk out from under a running job makes buildkitd panic inside cache.(*cacheManager).DiskUsage, and it took a Build AMD64 job down that way. `grep` also exits 1 whenever du prints no Total line, so an empty cache would have done it too. A cache-hygiene step must never be able to fail a deploy, so `total()` now always returns 0. The prune's own failure was already handled — it is an `if` condition, and a failing condition does not trip `-e`.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR prevents cache-usage reporting failures from aborting Docker build jobs under Bash errexit and pipefail behavior.
Confidence Score: 5/5The PR appears safe to merge with no actionable correctness or security issues identified. The fallback prevents the command-substitution assignment from triggering Bash errexit while preserving the prune command's existing success and failure handling.
|
| Filename | Overview |
|---|---|
| .github/actions/docker-build/action.yml | Adds a deliberate success fallback to cache-usage reporting so ancillary cache hygiene cannot fail a deployment build. |
Reviews (1): Last reviewed commit: "fix(ci): stop the layer-cache prune from..." | Re-trigger Greptile
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The settle loop added in
4f651038f3readscur="$(total)". Composite steps run underbash -e -o pipefail, where an assignment takes its command substitution's exit status — so any failingbuildctl duaborts the step and fails the build.Reproduced directly:
That asymmetry is why it was invisible: the
echo "$(total)"calls run first and survive, and only the assignment inside the settle loop trips it.Not hypothetical. Deleting a sticky disk out from under a running job makes buildkitd panic inside
cache.(*cacheManager).DiskUsage, and it took aBuild AMD64job down exactly this way:grepalso exits 1 wheneverduprints noTotal:line, so a genuinely empty cache would have done it too.Fix
total()now always returns 0. Cache hygiene must never be able to fail a deploy.The prune's own failure was already handled correctly — it sits in an
ifcondition, and a failing condition does not trip-e.Verification
bash -e -o pipefail): the settle loop now completes with a totally failingdu.shellcheckclean;actionlintidentical to the staging baseline;bun run lintpasses.Type of Change
Testing
Shell-level reproduction of both the failure and the fix, as above.
Checklist