ci: always ship buildkitd as an artifact and test the PR's own image - #737
Open
gilescope wants to merge 1 commit into
Open
ci: always ship buildkitd as an artifact and test the PR's own image#737gilescope wants to merge 1 commit into
gilescope wants to merge 1 commit into
Conversation
own image Signed-off-by: Giles Cope <gilescope@gmail.com>
|
| Branch | Total Count |
|---|---|
| main | 4748 |
| This PR | 4756 |
| Difference | +8 (0.17%) |
Keep up the great work migrating from Earthly to Earthbuild! 🚀
💡 Tips for finding more occurrences
Run locally to see detailed breakdown:
./.github/scripts/count-earthly.shNote that the goal is not to reach 0.
There is anticipated to be at least some occurences of earthly in the source code due to backwards compatibility with config files and language constructs.
janishorsts
reviewed
Jul 30, 2026
| fi | ||
| - name: Download artifacts (fork) | ||
| if: steps.fork-check.outputs.is_fork == 'true' | ||
| - name: Download build-earthly artifacts |
Collaborator
There was a problem hiding this comment.
Please rename earthly to earth.
Suggested change
| - name: Download build-earthly artifacts | |
| - name: Download build-earth artifacts |
janishorsts
reviewed
Jul 30, 2026
| shell: bash | ||
| run: | | ||
| echo "EARTHLY_BUILDKIT_IMAGE=ghcr.io/earthbuild/earthbuild:buildkitd-v0.8.17-fix.1" >> $GITHUB_ENV | ||
| TAG_SUFFIX="ubuntu-latest-${{inputs.BINARY}}" |
Collaborator
There was a problem hiding this comment.
The inputs.BINARY has no impact on the built BuildKit. #615 tackles the duplicated builds.
Suggested change
| TAG_SUFFIX="ubuntu-latest-${{inputs.BINARY}}" | |
| TAG_SUFFIX="ubuntu-latest" |
Collaborator
There was a problem hiding this comment.
I would actually simplify to two tags: latest and latest-tiktok.
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.
Extracted from #442 (the buildkit upgrade). Independent of the bump — this is about which buildkitd image CI tests against.
The problem
Relying on build artifacts should mean a more stable CI and less chance of being throttled (as well as 'just working' for forked PRs).
Every test job needs a
buildkitdimage, and today there are two different paths:docker loadedbuildkitd-staging-<sha>-<suffix>buildkitd-v0.8.17-fix.1(static pin)The artifact path exists because forks can't push to ghcr. Non-forks kept the static pin — which means a non-fork PR runs its entire outer test layer against the old buildkit daemon. The PR's own buildkit is only exercised inside the nested earthly-in-earthly tests, so an outer-daemon regression can pass CI unnoticed.
The change
Delete the split. Everyone uses the artifact path, and the image variable always points at the buildkitd this PR built.
build-earthly.yml: "Save artifacts for fork CI" becomes "Save buildkitd image tarball (all runs)" (unconditional) plus "Build earthly binary artifact (fork only)". Upload is unconditional. Non-fork jobs still fetch the earthly binary from ghcr — rebuilding it here would be minutes of redundant work.stage2-setup: download and load the artifact unconditionally;EARTHLY_BUILDKIT_IMAGEbecomesbuildkitd-staging-${{ github.sha }}-<suffix>.Earthly pushes straight from buildkit's internal store to ghcr without tagging the image in the host daemon, so
saveneeds the tag present first. The step doesimage inspectand only pulls if absent: fork runs hit the locally-built image, non-fork runs pay a single pull here instead of one per test job.Secondary benefit: ~67 test jobs per run stop fetching the image from ghcr and read it from in-run artifact storage instead, which doesn't rate-limit. Note this moves the bytes rather than eliminating them — the gain is reliability, not bandwidth.
Prerequisite fix included
Both
if:conditions on the build-and-push path are widened:This is load-bearing, not cosmetic.
ci-staging-deploy.ymlcallsbuild-earthly.ymlonworkflow_dispatch, which satisfies none of the three original event names — so today a manually dispatched staging deploy silently skips "Build and push +ci-release" entirely. That goes unnoticed because nothing downstream needs the image to exist. Once the tarball step is unconditional it becomes a hard failure, so the latent skip has to be fixed here. The GHCR-login step needs the same widening or the push has no credentials.The new condition covers
push,merge_group,schedule,workflow_dispatchand same-repo PRs.For review
The staging tag is built from
${{ github.sha }}, but the tag+ci-releaseactually pushes comes from earthly'sEARTHLY_GIT_HASH:So this assumes
github.sha == EARTHLY_GIT_HASH.mainalready bakes that assumption into the fork path in both files, so it isn't new — but it graduates from affecting fork PRs only to affecting every job. Worth a second pair of eyes.Relationship to #736
Content-disjoint (no retry wrappers, swap, or diagnostics here) but both PRs touch
build-earthly.yml, in three overlapping hunk ranges. The conflicts are orthogonal — #736 rewrites step bodies, this changesif:lines — but merging #736 first is preferable: if this lands first, a reviewer resolving #736'sif:conflicts could take main's side and silently revert the condition fix, quietly reintroducing theworkflow_dispatchskip.Verification
yqparses all workflows and both composite actions.actionlintclean onbuild-earthly.ymlandci-staging-deploy.yml.max_attempts,Add swap,failure-diagnostics,continue-on-error).🤖 Generated with Claude Code
https://claude.ai/code/session_01GQK1DssrULoDFoX5JU9njo