Skip to content

ci: always ship buildkitd as an artifact and test the PR's own image - #737

Merged
gilescope merged 10 commits into
mainfrom
giles-buildkitd-artifact
Aug 12, 2026
Merged

ci: always ship buildkitd as an artifact and test the PR's own image#737
gilescope merged 10 commits into
mainfrom
giles-buildkitd-artifact

Conversation

@gilescope

@gilescope gilescope commented Jul 30, 2026

Copy link
Copy Markdown

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 buildkitd image, and today there are two different paths:

image source tag
fork PR GHA artifact, docker loaded buildkitd-staging-<sha>-<suffix>
everything else pulled from ghcr per job 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_IMAGE becomes buildkitd-staging-${{ github.sha }}-<suffix>.

Earthly pushes straight from buildkit's internal store to ghcr without tagging the image in the host daemon, so save needs the tag present first. The step does image inspect and 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:

-if: github.event_name == 'push' || github.event_name == 'merge_group' || <same-repo PR>
+if: github.event_name != 'pull_request' || <same-repo PR>

This is load-bearing, not cosmetic. ci-staging-deploy.yml calls build-earthly.yml on workflow_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_dispatch and same-repo PRs.

For review

The staging tag is built from ${{ github.sha }}, but the tag +ci-release actually pushes comes from earthly's EARTHLY_GIT_HASH:

./buildkitd+buildkitd --TAG=${EARTHLY_GIT_HASH}-${TAG_SUFFIX} --DOCKERHUB_BUILDKIT_IMG="buildkitd-staging"

So this assumes github.sha == EARTHLY_GIT_HASH. main already 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 changes if: lines — but merging #736 first is preferable: if this lands first, a reviewer resolving #736's if: conflicts could take main's side and silently revert the condition fix, quietly reintroducing the workflow_dispatch skip.

Verification

🤖 Generated with Claude Code

https://claude.ai/code/session_01GQK1DssrULoDFoX5JU9njo

  own image

Signed-off-by: Giles Cope <gilescope@gmail.com>
@gilescope gilescope added the ai-assisted Authored with AI assistance label Jul 30, 2026
@github-actions

github-actions Bot commented Jul 30, 2026

Copy link
Copy Markdown

🎉 Are we earthbuild yet?

Great progress! You've reduced "earthly" occurrences by 20 (0.49%)

📈 Overall Progress

Branch Total Count
main 4050
This PR 4030
Difference -20 (0.49%)

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.sh

Note 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.

@gilescope
gilescope marked this pull request as ready for review July 30, 2026 07:04
@gilescope
gilescope requested a review from a team as a code owner July 30, 2026 07:04
@gilescope
gilescope requested review from kmannislands and removed request for a team July 30, 2026 07:04
@gilescope gilescope mentioned this pull request Jul 30, 2026
12 tasks
Comment thread .github/actions/stage2-setup/action.yml Outdated
Comment thread .github/actions/stage2-setup/action.yml Outdated
gilescope and others added 5 commits August 9, 2026 15:41
Co-authored-by: Janis Horsts <janis.horsts@gmail.com>
Co-authored-by: Janis Horsts <janis.horsts@gmail.com>
Conflict in .github/actions/stage2-setup/action.yml: main gated the
"Set fixed buildkitd image for Docker 29+" step on BINARY == 'docker'
(#615). This branch replaces that step's body with the PR's own staging
image, so the docker-only guard no longer applies -- podman jobs must
exercise the PR's buildkitd too. Kept this branch's step, dropped the guard.

Also realigned the new "Save buildkitd image tarball" step's TAG_SUFFIX
with #615's removal of the -${BINARY} suffix; git merged it clean but the
tag it built no longer existed.

Assisted-by: Claude:claude-opus-5 claude-code
Signed-off-by: Giles Cope <gilescope@gmail.com>
Per review on #737: the tag suffix carried a runner dimension
(ubuntu-latest[-${BINARY}]) that never varied -- #615 already dropped
${BINARY}, leaving 'ubuntu-' as dead width in every staging tag and
artifact name. Two tags is the whole truth: latest and latest-ticktock.

Producer and consumer moved together, since a mismatch is only
observable as a 404 on the ghcr pull at test time:
  - ci.yml, build-earthly.yml       TAG_SUFFIX + upload-artifact name
  - stage2-setup/action.yml         download-artifact name, TAG, EARTHLY_BUILDKIT_IMAGE
  - ci-staging-deploy.yml           ${GITHUB_SHA}-<suffix> upgrade/mv pairs

build-earthly.yml now hardcodes the suffix rather than deriving it from
RUNS_ON, so RUNS_ON is a runner label only. That removes the silent
coupling to stage2-setup, which had 'ubuntu-latest' hardcoded on the
consumer side while the producer templated it.

runs-on: values are untouched.

Assisted-by: Claude:claude-opus-5 claude-code
Signed-off-by: Giles Cope <gilescope@gmail.com>
#615 moved the live CI build out of build-earthly.yml and into ci.yml's
fast-check-and-build, where the save+upload steps are still gated on
fork PRs. This branch made stage2-setup download unconditionally, so
after the merge every non-fork run (push, merge_group, same-repo PR)
had a consumer with no producer -- download-artifact v8 fails hard on a
missing artifact, which is every reusable-* test job.

Split each variant into an always-run tarball step and a fork-only
binary build, mirroring build-earthly.yml:
  - non-fork: image lives only in ghcr, so pull-if-absent before save
  - fork: --image already put it in the local daemon, inspect hits
  - binary stays fork-only; non-fork gets it via earth upgrade

Assisted-by: Claude:claude-opus-5 claude-code
Signed-off-by: Giles Cope <gilescope@gmail.com>
@gilescope
gilescope enabled auto-merge (squash) August 9, 2026 15:09
The earthly-count check flagged +7 lines. All seven were prose this
branch introduced -- comments and one step display name -- where
'earthly' meant the tool, not an identifier.

Left alone, because there the word IS the identifier and a rename would
make the comment wrong: DEFAULT_INSTALLATION_NAME=earthly, the
earthly-buildkitd container name tests assert on, +earthly/earthly,
./earthly upgrade, ~/.earthly, EARTHLY_BUILDKIT_IMAGE, and the
earthly-build-* artifact names.

Also drops a reference to build-earthly from stage2-setup, which #615
made stale -- the artifact it downloads now comes from ci.yml.

Net delta against main: 0.

Assisted-by: Claude:claude-opus-5 claude-code
Signed-off-by: Giles Cope <gilescope@gmail.com>
@gilescope
gilescope requested a review from janishorsts August 10, 2026 05:32
Comment thread .github/actions/stage2-setup/action.yml Outdated
Comment thread .github/workflows/build-earthly.yml Outdated
Comment thread .github/actions/stage2-setup/action.yml Outdated
Signed-off-by: Giles Cope <gilescope@gmail.com>
@gilescope
gilescope requested a review from janishorsts August 11, 2026 07:07
Signed-off-by: Giles Cope <gilescope@gmail.com>
@gilescope
gilescope merged commit eb545a2 into main Aug 12, 2026
92 of 93 checks passed
@gilescope
gilescope deleted the giles-buildkitd-artifact branch August 12, 2026 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-assisted Authored with AI assistance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants