[ci] Build images once per PR and promote by tag instead of rebuilding by VietND96 · Pull Request #3229 · SeleniumHQ/docker-selenium

VietND96 · 2026-09-05T12:30:27Z

The problem, measured

Every job in the PR suite builds the images it needs, from scratch.

Workflow	Matrix jobs
`docker-test.yml`	18
`k8s-scaling-test.yml`	9
`helm-chart-test.yml`	8
`k8s-dynamic-grid-test.yml`	2
Total	37

It is not only the four entries with build-all: true. Every test target names image targets as Make prerequisites — test_video: video hub chrome firefox edge chromium — so all 37 build something. Recent PR runs took 75–88 minutes.

deploy.yml then built them a 38th time, so the artefact released was a rebuild of the tested source rather than the tested artefact.

The design

Build once, publish to GHCR, have everything else pull.

Tag	Meaning
`ghcr.io/<owner>/<image>:pr-<N>`	One set per pull request
`ghcr.io/<owner>/<image>:trunk-<sha>`	One set per trunk run
`ghcr.io/<owner>/<image>:main`	The last trunk set whose tests passed

build-images.yml decides what each run actually needs:

Mode	When	Result
`reuse-main`	Nothing image-affecting changed	Nothing is built at all
`build-and-push`	Image content changed	Built once, multi-arch; 37 jobs pull
`build-in-job`	Fork PR	Exactly today's behaviour

Image-affecting means the 21 directories holding a Dockerfile, or the Makefile (build args and pinned versions). That rule is deliberately coarse — an unnecessary build costs minutes, a missed one ships an untested image. Measured against the last 18 merges: 12 would rebuild, 6 would reuse :main.

Fork safety

Forks are detected by comparing head repository to this one, and never receive credentials. pull_request_target is deliberately not used — it would hand a registry write token to untrusted code. 3 of the last 60 PRs came from forks; they keep building in-job, unchanged.

How tests consume the images

Test jobs pull and retag to the local names the compose files already expect, then set SKIP_BUILD=true. That turns every image-build target into a no-op via one guarded block at the end of the Makefile.

The 48 build recipes and 59 test targets are untouched. The block sits last because GNU Make lets the last definition of a target win — I had it at the top first and SKIP_BUILD silently did nothing, which is exactly the sort of thing that would have looked green and scanned nothing.

gen_certs, prepare_resources and update_go are deliberately not skipped: they produce working-tree files the tests read, not images.

make -n test_video                    -> 8 docker buildx build lines, 0 warnings
make -n test_video SKIP_BUILD=true    -> 0 docker buildx build lines
make -n build                         -> 30 build lines, unchanged

Promotion, not rebuilding

:main is moved only after the full suite passes, by retagging the run-scoped trunk-<sha> manifest — so :main always denotes a tested tree and the digest never changes.

deploy.yml pulls :main and promotes it, falling back to building when :main does not exist — which it will not, on the first release after this lands.

Cleanup

pr-* tags are deleted when the PR closes, plus a weekly sweep. The script only removes tags matching pr-<digits>, and only when every tag on that manifest matches, so a shared or release tag cannot be caught by it. Its image list is asserted to match the Makefile's CI_IMAGES.

What is verified, and what is not

Verified locally:

SKIP_BUILD contract, all three cases above.
All five decision branches of the decide job, run as shell against real commits: fork, trunk push, forced, docs-only commit (reuse-main), NodeChrome/ commit (build-and-push).
All 27 workflow files parse; mbake validate passes; the two image lists match exactly.
Build and pull steps are mutually exclusive in all four test workflows.

Not verified, and it needs a real run: none of this has executed in Actions. The first PR after merge is the real test. Two things I would watch specifically:

The first run cannot use :main — it does not exist yet, so the decide job will choose build-and-push. That is handled, but it means the speedup only shows from the second PR onward.
GHCR package visibility. The packages must allow the repository's GITHUB_TOKEN to pull. If :pr-N pushes succeed but test jobs fail to pull, that is the cause, and it is a package settings change rather than a workflow one.

I would merge this when you can watch a PR run through it, rather than at the end of a day.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

Every job in the PR suite built the images it needed, from scratch. Not just the four entries with build-all: every test target names image targets as Make prerequisites - `test_video: video hub chrome firefox edge chromium` - so all of them built something. That is 18 docker-test jobs, 9 k8s-scaling, 8 helm-chart and 2 k8s-dynamic-grid: 37 builds of the same images per pull request. Recent runs took 75-88 minutes. deploy.yml then built them a 38th time, so the artefact released was a rebuild of the tested source rather than the tested artefact. Build once, publish to GHCR, and have everything else pull. ghcr.io/<owner>/<image>:pr-<N> one set per pull request ghcr.io/<owner>/<image>:trunk-<sha> one set per trunk run ghcr.io/<owner>/<image>:main the last trunk set whose tests passed build-images.yml decides what a run needs: reuse-main nothing image-affecting changed, so run against :main and build nothing at all. Of the last 18 merges, 6 qualify. build-and-push build once, multi-arch, push the tag, every job pulls it build-in-job fork pull requests, which have no registry credentials, keep today's behaviour exactly Image-affecting means the 21 directories holding a Dockerfile, or the Makefile, which carries the build arguments and pinned versions. That last rule is deliberately coarse: an unnecessary build costs minutes, a missed one ships an untested image. Forks are detected by comparing head repository to this one, and never receive credentials. pull_request_target is not used - it would hand a registry write token to untrusted code. Test jobs pull and retag to the local names the compose files already expect, then set SKIP_BUILD=true. That turns every image-build target into a no-op via a single guarded block at the end of the Makefile, so the 48 build recipes and the 59 test targets are untouched and `make build` and `make test` behave exactly as before when the variable is unset. The block sits last because GNU Make lets the last definition of a target win. gen_certs, prepare_resources and update_go are deliberately not skipped: they produce working-tree files the tests read. :main is moved only after the full suite passes, by retagging the run-scoped trunk tag, so it always denotes a tested tree and the digest never changes. deploy.yml pulls :main and promotes it, falling back to building when :main does not exist yet - which it will not, on the first release after this lands. pr-* tags are deleted when the pull request closes, and swept weekly for anything that missed. The cleanup only ever removes tags matching pr-<digits>, and only when every tag on that manifest matches, so a shared or release tag cannot be caught by it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

qodo-code-review · 2026-09-05T12:30:33Z

ⓘ Qodo reviews are paused because the subscription is no longer active. Ask your workspace admin to reactivate the subscription to resume reviews. Manage billing

The first run of this branch failed before any job started: "This run likely failed because of a workflow file issue." Inserting the ci-* inputs put a second `with:` block in the helm-chart-test and k8s-dynamic-grid-test call sites, because their existing `with:` came after `secrets: inherit` rather than immediately after `uses:`: uses: ./.github/workflows/helm-chart-test.yml with: # inserted ci-mode: ... secrets: inherit with: # original, still there release: ... PyYAML's safe_load accepts duplicate mapping keys and silently keeps the last, so my validation passed while GitHub rejected the file - the same class of false-pass as the grep quoting bug: a check that could not see the defect it was meant to catch. Merge into the existing blocks, and validate with a loader that rejects duplicate keys instead of one that hides them. Also give docker-test `secrets: inherit`, so every called workflow can reach the registry the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

`make lint_format_scripts` runs the formatters and fails if anything changed, so the committed file has to already be in canonical form. Also simplifies promote_ci_images: its `if ... ; \` continuation was something mbake kept re-indenting, so the gate could never have gone green. A plain `test -n ... || (echo; exit 1)` guard on its own line is stable, and reads better. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

A merge rebuilt everything the pull request had already built and tested. Key the images on their own content instead, and the merge can reuse them. The tag becomes src-<hash>, where the hash covers every path that can change what ends up in an image: the 21 directories holding a Dockerfile, plus the Makefile, hashed by blob id. This works where matching on a pull request number would not. SeleniumHQ squash-merges, so the pull request commit is not an ancestor of trunk and is unreachable once the branch is deleted. The content hash is identical on both sides regardless. Verified against #3228: its head and its squash merge on trunk both hash to fd9b32eb0465, while a Makefile tag bump hashes to 36fcdfcb808a. So the decision collapses to one question - do images for this content already exist? yes reuse them. On a pull request that means an earlier run or another branch already built this content. On trunk it means the pull request just did, which is the promotion case: no rebuild, tests run against the same images, and promote-main retags them to :main. no build once and push. This also replaces the changed-path heuristic, which only approximated the same question and rebuilt whenever the Makefile was touched at all. Pull request builds additionally tag pr-<N>, so cleanup-pr-images.yml still has something to delete on close. The src-* tag is shared across pull requests and deliberately outlives any one of them, so cleanup never removes it. One note on the verification: the first attempt at checking the hash ran in zsh, which does not word-split unquoted variables, so `git ls-tree -- $PATHS` matched nothing and every commit hashed to the SHA-256 of an empty string. Re-run under bash, which is what CI uses, it gives the real answer above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

…eased Promoting a pull request's images to :main and then to a release was wrong, and would have shipped images built against the wrong Selenium core. get-latest-upstream resolves a different core depending on how it is called: release: false the latest Nightly upstream release (prerelease == true) release: true the latest stable release Pull requests build against nightly, deliberately - that is how an upcoming core gets tested. deploy.yml builds against stable. My build-images job asked for 'false', so every image it produced carried a nightly core. Promoting one of those to :main and releasing it would have published a stable release built on a nightly Selenium. The content hash made it worse rather than catching it: it covered the source files only, so the same tree built against a nightly core and a stable core produced the *same* src-<hash> tag. The two would have been interchangeable. So the core is now part of the key. The hash covers the image-affecting paths and the resolved BASE_VERSION, and decide resolves the core before hashing: pull request nightly core, as before trunk, release stable core Identical source now hashes to src-d61948d6378d against a 4.49.0-SNAPSHOT core and src-b9fa526b0f36 against 4.48.0, so the two can never collide and a pull request build can never be promoted as a release. That gives the intended flow. A merge no longer reuses the pull request's images: its stable-core hash is new, so trunk builds, tests, and promotes that to :main. deploy.yml now checks before trusting :main. It recomputes the hash for the release core and only promotes when :main resolves to the same digest as src-<hash>; a stale :main, from trunk moving on or the core changing underneath, falls through to a rebuild rather than releasing the wrong bits. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

The build failed downloading the Selenium core. get-latest-upstream writes three variables: BASE_RELEASE=<upstream tag> nightly | selenium-4.48.0 BASE_VERSION=<version> 4.49.0-SNAPSHOT | 4.48.0 VERSION=<version> and Base/Dockerfile builds its download URL from both of them: .../releases/download/${RELEASE}/selenium-server-${VERSION}.jar Passing the resolved core from the decide job to the build job, I carried only BASE_VERSION. BASE_RELEASE therefore fell back to the Makefile default selenium-4.48.0 while BASE_VERSION was the nightly 4.49.0-SNAPSHOT, so the build asked for a jar that does not exist: selenium-4.48.0 + 4.48.0 -> 200 nightly + 4.49.0-SNAPSHOT -> 200 selenium-4.48.0 + 4.49.0-SNAPSHOT -> 404 <- what it asked for Carry all three. Splitting a resolved pair across jobs and reassembling half of it was the mistake; the comment at that step now says why all three have to travel together. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

The multi-arch build was heading past an hour. Building linux/amd64 and linux/arm64 on one amd64 runner means QEMU-emulating every arm64 layer, and for 26 images that is most of the run. Build each architecture on its own native runner, in parallel, and merge the results into a manifest list. The repo already uses ubuntu-24.04-arm for its arm64 tests, so the runners were there. Two native builds cost roughly one amd64 build rather than one plus an emulated one. build (amd64, ubuntu-24.04) -> src-<hash>-amd64 build (arm64, ubuntu-24.04-arm) -> src-<hash>-arm64 merge -> src-<hash> (manifest list) Edge and Chrome for Testing are amd64-only. The Makefile already skips them when PLATFORMS carries no linux/amd64, so the arm64 job never builds them; two changes follow from that: push_ci_images skips images this build did not produce, rather than failing on `docker tag` for one that was correctly never built. It still fails if it pushed nothing at all, so a genuinely empty build cannot pass quietly. merge_ci_images assembles the list from whichever architecture tags exist, so Edge and Chrome for Testing get a single-architecture list instead of an error. Test jobs pull src-<hash> and Docker selects the right architecture, so nothing in the test workflows changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

push ghcr.io/SeleniumHQ/base:src-331808bba75d-arm64 Error parsing reference: ... repository name (SeleniumHQ/base) must be lowercase github.repository_owner is SeleniumHQ, and a registry reference has to be lowercase. Everywhere else in this repo already folds it: ghcr.io/$(echo "${{ github.repository_owner }}" | tr '[:upper:]' '[:lower:]') I used that in deploy.yml and then wrote the new workflow's registry as a top-level `env:` instead, where no shell runs and GitHub expressions have no lower() to reach for. Derive it once in the decide job, which already publishes `registry` as an output, and let build, merge and every test workflow read it from there. The top-level env is gone, so there is no second definition to drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

push ghcr.io/seleniumhq/base:src-331808bba75d-arm64 unexpected status from POST to https://ghcr.io/v2/seleniumhq/base/blobs/uploads/: 403 Forbidden The login succeeded and the push did not. GITHUB_TOKEN has no write access to these packages: they are org-level, pre-existing, and every workflow in this repo that publishes to GHCR authenticates with a PAT instead - echo "${{ secrets.SELENIUM_CI_TOKEN }}" | docker login ghcr.io \ -u "${{ secrets.SELENIUM_CI_USERNAME }}" --password-stdin deploy.yml, nightly.yml, build-ffmpeg.yml and all four release-*-versions.yml do it that way. I used GITHUB_TOKEN in the new steps and did not check what the working ones used. Pulls need it too: ghcr.io/seleniumhq/base returns 401 unauthenticated, so the packages are private and a read is no more anonymous than a write. All eight logins now use the PAT, falling back to GITHUB_TOKEN so nothing hard-fails if it is ever unset. Fork pull requests are unaffected - they build in-job and never touch the registry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

The merge job failed: no architecture tags for video. push_ci_images looked for $(NAME)/<image>:$(TAG_VERSION) for all 26 images, but video is not built with the grid tag. It is built as $(NAME)/video:$(FFMPEG_TAG_VERSION)-$(BUILD_DATE) ffmpeg-8.1-20260905 and the compose files read it from VIDEO_TAG, which the test targets set to the same thing. So `docker image inspect` never found it, the skip-if-absent guard I added for the amd64-only images swallowed it, and the failure only surfaced two jobs later when merge had nothing to assemble. The guard was right for Edge and Chrome for Testing on arm64 and wrong here; it turned a missing image into silence. It now runs after the tag is resolved correctly, so it only skips images this architecture genuinely does not build. Both push and pull resolve the tag per image. Audited the other 25: video is the only one that differs, and ffmpeg is handled too so it is right if it is ever added to CI_IMAGES. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

pull ghcr.io/seleniumhq/node-chrome-for-testing:src-de73f4c3321a Error response from daemon: no matching manifest for linux/arm64/v8 in the manifest list entries Edge and Chrome for Testing are amd64-only. The Makefile already skips them when PLATFORMS carries no linux/amd64, so the arm64 build never produces them and their manifest list has no arm64 entry - confirmed on the published images: selenium/node-chrome amd64, arm64 selenium/node-chrome-for-testing amd64 pull_ci_images asked for all 26 regardless, so the arm64 test jobs failed on the first one that does not exist for them. Forcing --platform would be the wrong fix: it would pull an amd64 image onto an arm64 runner, which cannot run it. These images should not be pulled there at all, and the arm64 tests do not use them. So inspect the manifest first and skip only images with no entry for this architecture, reporting them. An image missing from the registry entirely stays a hard error - that means the build or the merge went wrong, and conflating the two is what let the missing video image surface two jobs late. The pull is also explicitly --platform linux/<arch> now, so it can never resolve to the wrong architecture by accident. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EGLHB3pjY3mGZBu4TyUsrm

VietND96 and others added 10 commits September 5, 2026 19:33

Navigation Menu

Uh oh!

Uh oh!

[ci] Build images once per PR and promote by tag instead of rebuilding - #3229

[ci] Build images once per PR and promote by tag instead of rebuilding#3229
VietND96 wants to merge 11 commits into
trunkfrom
ci-build-once-promote

VietND96 commented Sep 5, 2026

Uh oh!

qodo-code-review Bot commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

VietND96 commented Sep 5, 2026

The problem, measured

The design

Fork safety

How tests consume the images

Promotion, not rebuilding

Cleanup

What is verified, and what is not

Uh oh!

qodo-code-review Bot commented Sep 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant