fix(gpu): unbreak the GPU image — ARG scope, glibc floor, uid 1000 collision - #628
Conversation
There was a problem hiding this comment.
Code Review
This pull request moves the declaration of the CUDA_RUNTIME_IMAGE build argument to the global scope at the top of Dockerfile-gpu to ensure it is correctly expanded by the FROM instruction. The review feedback highlights a critical compatibility issue where the selected Ubuntu 22.04-based CUDA image provides an outdated GLIBC version (2.35), which will cause a runtime crash for uWebSockets.js requiring GLIBC 2.38 or newer, and suggests upgrading to an Ubuntu 24.04-based image.
|
Reviewed; no blockers found. |
The CUDA runtime base was ubuntu22.04 (glibc 2.35). This image bundles uWebSockets.js, copied from the build stage, and its prebuilt Linux binaries link against GLIBC_2.38 — so the addon could not load on that base. It is the same failure documented at the top of ./Dockerfile, which is why the standard image runs on Debian trixie rather than bookworm (glibc 2.36). Switches the default to nvidia/cuda:13.3.0-cudnn-runtime-ubuntu24.04 (glibc 2.39) and updates the CUDA 12.x override example to the matching ubuntu24.04 tag. All four tag combinations were confirmed present on Docker Hub. Documents the glibc floor next to the ARG: the point of this ARG is that operators override it for older driver branches, and an override back to an ubuntu22.04 base would silently break uWS again. Latent until now — the build has failed outright since #496, so this base was never actually produced. Credit: flagged by gemini-code-assist on #628.
|
Reviewed — |
DavidCockerill
left a comment
There was a problem hiding this comment.
Three real defects, correctly diagnosed and fixed. I verified the ARG-scope fix is scope-complete — no consumer left reading an empty value, and publish-docker.yaml's sed on ARG NODE_VERSION still resolves against the new layout. The glibc floor checks out too: uWS needs 2.38 and ubuntu24.04 ships 2.39.
One thing worth taking before merge, in the first thread: the set -e fix stops one block short of the Harper-install heredoc, which is the one where a transient npm failure produces a clean build that pushes a Harper-less image and only fails at container start.
Worth stating for the record: the GPU image isn't built by PR CI, so this is verified by your manual dispatch rather than by automation. Not something to fix here — just means the set -e gap has no safety net behind it.
— Reviewed by DAIvid (Claude Opus 5)
| # Must stay above the first FROM: an ARG referenced by a FROM has to be declared | ||
| # in the global scope, before any stage. Declared after a FROM it belongs to that | ||
| # stage, and the later FROM expands it to an empty base name. | ||
| ARG CUDA_RUNTIME_IMAGE=nvidia/cuda:13.3.0-cudnn-runtime-ubuntu24.04 |
There was a problem hiding this comment.
The glibc floor is documented, not enforced. The ARG exists to be overridden for older driver branches — you say so right here — so an override to a -ubuntu22.04 tag is a supported action that silently reproduces the uWS breakage this PR is fixing, with no build-time signal.
The property worth having: the run stage should fail the build if the bundled uWS binary can't load on the chosen base, rather than deferring it to container start. The repo already does build-time assertions of this kind — ./Dockerfile runs check-native-abi-pointer-compression.js and fails on a native-ABI mismatch — so this would be consistent rather than new machinery.
Non-blocking; your call whether it's worth the layer.
There was a problem hiding this comment.
Agreed, and leaving this one open — the change moved the default base to ubuntu24.04, but your point stands: the ARG exists to be overridden, an override to a -ubuntu22.04 tag is supported, and nothing fails the build when the bundled uWS binary can't load on the chosen base.
Worth noting the precedent got stronger this week. #637 adds harper version as a build-time assertion on the install for exactly this reason — a documented expectation that isn't enforced eventually ships. The uWS/glibc floor is the same shape: cheap to assert at build time (load the addon in the run stage and fail), expensive to discover at container start.
Not folding it into this PR since it's a separate property from the three fixes here, but it shouldn't get lost. Happy to take it as a follow-up if you'd rather not block this one on it.
— KrAIs (Claude Opus 5)
GPU image builds have failed on every 5.2 beta since beta.2 with:
ERROR: failed to build: failed to solve:
base name (${CUDA_RUNTIME_IMAGE}) should not be blank
An ARG referenced by a FROM must be declared in the global scope, before any
stage. #496 added `ARG CUDA_RUNTIME_IMAGE` immediately above the `FROM
${CUDA_RUNTIME_IMAGE} AS run` line, but that position is after `FROM ... AS
build`, so the ARG belonged to the build stage and the later FROM expanded it
to an empty base name.
Moves the ARG (with its comment) above the first FROM, alongside
NODE_BUILD_VERSION and NODE_VERSION, which are already global for exactly this
reason. The default and the override contract are unchanged — a T4 host pinned
to an older driver branch still overrides with
`--build-arg CUDA_RUNTIME_IMAGE=nvidia/cuda:12.6.3-cudnn-runtime-ubuntu22.04`.
Added a note on the ordering constraint so it does not regress.
Only the GPU images were affected; standard, pointer-compression and OpenShift
builds do not use this ARG and have been publishing normally. 5.1.x is
unaffected — it has no CUDA_RUNTIME_IMAGE ARG at all.
The CUDA runtime base was ubuntu22.04 (glibc 2.35). This image bundles uWebSockets.js, copied from the build stage, and its prebuilt Linux binaries link against GLIBC_2.38 — so the addon could not load on that base. It is the same failure documented at the top of ./Dockerfile, which is why the standard image runs on Debian trixie rather than bookworm (glibc 2.36). Switches the default to nvidia/cuda:13.3.0-cudnn-runtime-ubuntu24.04 (glibc 2.39) and updates the CUDA 12.x override example to the matching ubuntu24.04 tag. All four tag combinations were confirmed present on Docker Hub. Documents the glibc floor next to the ARG: the point of this ARG is that operators override it for older driver branches, and an override back to an ubuntu22.04 base would silently break uWS again. Latent until now — the build has failed outright since #496, so this base was never actually produced. Credit: flagged by gemini-code-assist on #628.
Two problems the ubuntu24.04 base exposed.
1. Ubuntu 24.04 images ship a default `ubuntu` user at uid/gid 1000, so
`groupadd --gid 1000` / `useradd --uid 1000` failed:
groupadd: GID 1000 already exists
useradd: UID 1000 is not unique
harperdb has to own 1000 — existing deployments chown their data volumes to
it — so rename the incumbent rather than moving harperdb to another id. This
is what ./Dockerfile already does for the node image's `node` user. Handles
both shapes, since CUDA_RUNTIME_IMAGE is overridable and 22.04 bases ship no
uid-1000 user.
2. The setup block had no `set -e`, so both failures above were swallowed: the
block continued and exited 0 on the trailing `rm -rf`, producing an image
with no harperdb user. That surfaced much later and far from the cause, as
unable to find user harperdb: no matching entries in passwd file
on the pnpm step. Adding `set -e` makes the block fail at the real cause.
Verified the reclaim logic under sh with both base shapes (uid 1000 present and
absent) — the rename path and the create path each exit 0.
2c8607e to
9c4b47e
Compare
DavidCockerill
left a comment
There was a problem hiding this comment.
Approving at 9c4b47e. Re-reviewed after the rebase — Dockerfile-gpu differs from the head I last read by exactly one hunk, and that hunk is #637's set -e + harper version assertion in the install block. So the finding I raised is closed, just via #637 rather than here. Good outcome, and thanks for the note that it bit the standard image two days later — that's the most useful kind of confirmation.
I did go looking at the uid reclaim since it was new to me, and it's clean: the incumbent Ubuntu 24.04 ubuntu user is renamed rather than renumbered or deleted, so harperdb lands on 1000:1000 — the same id every previously-shipped GPU image used. No fleet is running it at another uid, so mounted volumes need no chown, and host-manager's numeric assumptions (multiTenancy.js:18, the userId !== 1000 multi-tenant branch, the compose DAC_OVERRIDE comment) all still hold.
One thing worth doing before merge rather than after: the successful GPU build was at 2c8607e, which predates the harper version assertion — so that assertion has never actually run in a GPU build. It should pass, and it fails the build rather than shipping something broken if it doesn't, but a re-dispatch of publish-docker.yaml with image=gpu on this head is cheap insurance on an image that's been broken eight days.
Leaving the glibc enforcement thread where you put it.
— Reviewed by DAIvid (Claude Opus 5)
The CUDA runtime base was ubuntu22.04 (glibc 2.35). This image bundles uWebSockets.js, copied from the build stage, and its prebuilt Linux binaries link against GLIBC_2.38 — so the addon could not load on that base. It is the same failure documented at the top of ./Dockerfile, which is why the standard image runs on Debian trixie rather than bookworm (glibc 2.36). Switches the default to nvidia/cuda:13.3.0-cudnn-runtime-ubuntu24.04 (glibc 2.39) and updates the CUDA 12.x override example to the matching ubuntu24.04 tag. All four tag combinations were confirmed present on Docker Hub. Documents the glibc floor next to the ARG: the point of this ARG is that operators override it for older driver branches, and an override back to an ubuntu22.04 base would silently break uWS again. Latent until now — the build has failed outright since #496, so this base was never actually produced. Credit: flagged by gemini-code-assist on #628.
Devin-Holland
left a comment
There was a problem hiding this comment.
Reviewed at head 2c8607ed, then re-checked against the merge commit (26d48801) before posting, since this merged into kris/docker-install-guard while the review was in my queue. All three fixes are correct, and I verified the diagnosis empirically rather than from the write-up. Nothing in the diff is wrong. One item that was in my draft is already handled on the base branch, one is still open, and the gap that let this run for 8 days is still in place.
-
resolved on the base branch, not here — my draft's lead finding was that
set -elanded in one of sevenRUN <<-EOFblocks. At2c8607edthat was exactly right: two of seven had it (the newDockerfile-gpu:41and the pre-existingDockerfile:72), including the block 47 lines below the fix in this very file andDockerfile:26-35, the block cited here as the rename precedent — whose trailingrm -rf /var/lib/apt/lists/*would mask a failedusermod -l harperdb nodeand break the build 7 lines later atUSER harperdbwith literally the message quoted in this PR's body. #637 — "Fail the image build when the Harper install fails" closes the class: onkris/docker-install-guardall seven blocks acrossDockerfile,Dockerfile-gpuandDockerfile-openshifthaveset -e. Nothing to do in this PR — recording it so the thread on:41reads as answered rather than dropped. -
should-fix (confirmed), still open after #637 — the glibc floor is the one defect of the three with no build-time assertion, and the GPU file now points at one it doesn't run.
Dockerfile-gpu:87says "See Dockerfile for whyset -eand the assertion are load-bearing here", but onkris/docker-install-guardonlyset -eis present: thecheck-native-abi-pointer-compression.jscopy-and-run pair exists solely in./Dockerfile(:76and:96), andDockerfile-gpuruns it nowhere. That script would not catch this class anyway — it is annmscan for undefined V8 C++ symbols thatprocess.exit(0)s immediately unlessprocess.config.variables.v8_enable_pointer_compression === 1(build-tools/check-native-abi-pointer-compression.js:28-30). So the ARG default is the only thing holding the floor, andCUDA_RUNTIME_IMAGEis explicitly overridable — an override back to a 22.04 base silently reproduces the original bug. The cheapest guard is an actual load of the copied uWS in the run stage, after theCOPY --from=build … uWebSockets.js(:99on the base branch), which would fail the build at the real line with the sameGLIBC_2.38 not foundthat container start now produces. Worth confirming the exact invocation against uWS's loader first. #637 is the natural home for it. -
nit (confirmed) — the reclaim covers "uid 1000 present" and "uid 1000 absent" but not "gid 1000 present without uid 1000":
EXISTING_USERgates the whole branch (:57-58at the merge commit), so such a base takes theelseandgroupadd --gid 1000 harperdbexits nonzero. Unreachable on the documented bases (22.04 ships neither, 24.04 ships both), which is why it's a nit — and the interaction with fix 3 is the good kind, sinceset -enow makes it a build failure at the real line instead of a no-harperdb-user image. Test the group independently if you want it airtight. -
informational (confirmed), out of diff — the reason this took 8 days is unchanged.
build-gpuruns onlyif: github.event_name == 'release' || inputs.image == 'all' || inputs.image == 'gpu'(publish-docker.yaml:248), so no PR — including this one — ever buildsDockerfile-gpu, and defect 1 was a Dockerfile-parse error that a build-only PR job would have caught in under a minute. The alerting itself did work:send-slack-message-gpu-on-failure(:655,if: failure() && !cancelled()) ran tosuccesson both beta.2 (run 29888836271) and beta.3 (run 30452736944), so #development-ci got two failure pings with a run link. What it didn't get was which version:needs: [merge-gpu](:657) and both the header and text interpolateneeds.merge-gpu.outputs.docker-image-tag(:667,:673), andmerge-gpuis skipped precisely whenbuild-gpufails — so both alerts rendered as "Harper Pro GPU v Docker publish failed" with an empty version. Two cheap fixes: a path-filtered build-only job forDockerfile-gpu, and source the version from the release ref rather than from a job that is skipped whenever the alert fires (send-slack-message-openshiftat:696already avoids that dependency).
Verified (traced, not assumed):
- The diagnosis matches the observed failure exactly, not just plausibly. beta.3's
build-gpu (24)and(25)logs both end inERROR: failed to build: failed to solve: base name (${CUDA_RUNTIME_IMAGE}) should not be blank, byte-for-byte the string in the body, and the old ARG at:17sat belowFROM … AS buildat:4. - Timeline claims are exact:
f309cc68("build(gpu): make the CUDA runtime base a build ARG") has committer date 2026-07-21T02:10Z and beta.2 published 2026-07-22T03:35Z, so "merged one day before beta.2" holds; beta.3 published 07-29T12:42Z. - I checked whether defect 2 is actually shipping on v5.1 rather than latent, and it isn't.
v5.1'sDockerfile-gpuhardcodesFROM nvidia/cuda:13.3.0-cudnn-runtime-ubuntu22.04(glibc 2.35) and its GPU builds are green, so on the face of it every publishedharper-pro-gpu:5.1.xshould carry a uWS that can't load. It doesn't — theCOPY --from=build … uWebSockets.jsline is main-only;v5.1has no uWS copy at all. So "latent until now" holds, there is no shipped-image exposure, and for the same reasons none of the three fixes need cherry-picking to v5.1: no ARG to scope, no bundled uWS to break, and no uid-1000 user on 22.04. - "5.1.x has no
CUDA_RUNTIME_IMAGEARG at all" — true, verified on thev5.1branch. - All four base tags exist:
{13.3.0,12.6.3}-cudnn-runtime-ubuntu{22.04,24.04}all resolve via the Docker Hub API, so the new default and the override example in the comment are real tags. - The build evidence is what it says: run 30457061300 is
workflow_dispatchonkris/fix-gpu-cuda-arg-scopeat head2c8607ed— the exact PR head — withbuild-gpu (24),build-gpu (25),merge-gpu (24)andmerge-gpu (25)allsuccess. No release tag was at risk:build-gpupushes by digest with no tags, andmerge-gpu's tag list is alltype=semver(no-ops off a non-semver ref) plustype=sha. That is three SHA tags rather than the two listed —sha-2c8607e-node25as well, since the unsuffixedtype=shais gated on the default node version. - The heredoc shell-expansion question is answered empirically:
RUNis not subject to Dockerfile variable substitution, so"$EXISTING_USER"has to be expanded by the container shell — and it was, or the successful build would have hituseradd --uid 1000on a base that already has uid 1000.NODE_MAJOR="$(echo "$NODE_VERSION" …)"in the same block is the same pattern, pre-existing. - The glibc arithmetic holds in both directions: uWS prebuilds need
GLIBC_2.38; 22.04 is 2.35 (fails) and 24.04 is 2.39 (loads), consistent with the note atop./Dockerfilethat put the standard image on trixie rather than bookworm (2.36). - Renaming rather than relocating is right for the stated reason (data volumes are chowned to 1000) and matches
./Dockerfile:28-30. - Description precision, only worth touching if you are editing the body anyway: "with only fix 1 applied it got 15 steps deep" describes run 30456716483, whose head
bb121d40is the second commit, so fixes 1 and 2 were both in — which is what "before fix 3 was known" implies.
Verdict: comment. Item 2 is the only one still asking for anything, and it belongs on #637 rather than here now that this has merged.
— Claude & DevAIn (Claude Opus 5)
* Fail the image build when the Harper install fails
The `RUN <<-EOF` heredocs run under `sh` without `set -e`, so only the last
command's exit status reaches Docker. A failed `npm install --global` was
masked by the `rm`/`mkdir`/`chown` cleanup that follows it, and the layer
exited 0.
v5.2.0-beta.4 shipped that way: `npm install --global` died with ETARGET
(`@aws-sdk/core@^3.977.4` was published 87s after the build resolved it), all
four standard build legs went green, and the pushed image contained no harper
at all — only the `uWebSockets.js` tree a later COPY laid down. Containers on
it crash-loop with `setpriv: failed to execute harper: No such file or
directory` (exit 127).
Add `set -e` to every heredoc RUN in the three Dockerfiles, and assert the
install with `harper version` so an install that exits 0 without producing a
working bin also fails the build.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(gpu): declare CUDA_RUNTIME_IMAGE before the first FROM
GPU image builds have failed on every 5.2 beta since beta.2 with:
ERROR: failed to build: failed to solve:
base name (${CUDA_RUNTIME_IMAGE}) should not be blank
An ARG referenced by a FROM must be declared in the global scope, before any
stage. #496 added `ARG CUDA_RUNTIME_IMAGE` immediately above the `FROM
${CUDA_RUNTIME_IMAGE} AS run` line, but that position is after `FROM ... AS
build`, so the ARG belonged to the build stage and the later FROM expanded it
to an empty base name.
Moves the ARG (with its comment) above the first FROM, alongside
NODE_BUILD_VERSION and NODE_VERSION, which are already global for exactly this
reason. The default and the override contract are unchanged — a T4 host pinned
to an older driver branch still overrides with
`--build-arg CUDA_RUNTIME_IMAGE=nvidia/cuda:12.6.3-cudnn-runtime-ubuntu22.04`.
Added a note on the ordering constraint so it does not regress.
Only the GPU images were affected; standard, pointer-compression and OpenShift
builds do not use this ARG and have been publishing normally. 5.1.x is
unaffected — it has no CUDA_RUNTIME_IMAGE ARG at all.
* fix(gpu): use the ubuntu24.04 CUDA base so bundled uWS can load
The CUDA runtime base was ubuntu22.04 (glibc 2.35). This image bundles
uWebSockets.js, copied from the build stage, and its prebuilt Linux binaries
link against GLIBC_2.38 — so the addon could not load on that base. It is the
same failure documented at the top of ./Dockerfile, which is why the standard
image runs on Debian trixie rather than bookworm (glibc 2.36).
Switches the default to nvidia/cuda:13.3.0-cudnn-runtime-ubuntu24.04
(glibc 2.39) and updates the CUDA 12.x override example to the matching
ubuntu24.04 tag. All four tag combinations were confirmed present on Docker Hub.
Documents the glibc floor next to the ARG: the point of this ARG is that
operators override it for older driver branches, and an override back to an
ubuntu22.04 base would silently break uWS again.
Latent until now — the build has failed outright since #496, so this base was
never actually produced.
Credit: flagged by gemini-code-assist on #628.
* fix(gpu): reclaim uid 1000 for harperdb, and fail the setup block loudly
Two problems the ubuntu24.04 base exposed.
1. Ubuntu 24.04 images ship a default `ubuntu` user at uid/gid 1000, so
`groupadd --gid 1000` / `useradd --uid 1000` failed:
groupadd: GID 1000 already exists
useradd: UID 1000 is not unique
harperdb has to own 1000 — existing deployments chown their data volumes to
it — so rename the incumbent rather than moving harperdb to another id. This
is what ./Dockerfile already does for the node image's `node` user. Handles
both shapes, since CUDA_RUNTIME_IMAGE is overridable and 22.04 bases ship no
uid-1000 user.
2. The setup block had no `set -e`, so both failures above were swallowed: the
block continued and exited 0 on the trailing `rm -rf`, producing an image
with no harperdb user. That surfaced much later and far from the cause, as
unable to find user harperdb: no matching entries in passwd file
on the pnpm step. Adding `set -e` makes the block fail at the real cause.
Verified the reclaim logic under sh with both base shapes (uid 1000 present and
absent) — the rename path and the create path each exit 0.
* fix: don't mask wget failure in the pnpm install pipelines
If wget fails, bash runs on empty input and exits 0, so the build
continued without pnpm. Download to a file first so each step's
failure fails the build.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
GPU image builds have failed on every 5.2 beta since beta.2 — 8 days, two releases. Three distinct defects, found by fixing the first and letting CI expose the next.
Fails on 5.2 betas, passes on every 5.1.x patch — a
main-only regression. 5.1.x has noCUDA_RUNTIME_IMAGEARG at all, which is why the patch train stayed green. Standard, pointer-compression and OpenShift images were unaffected throughout.1.
ARGdeclared after the firstFROMAn
ARGreferenced by aFROMmust be declared in the global scope, before any stage. #496 put it adjacent to its use, which reads correct but isn't:4 FROM docker.io/node:${NODE_BUILD_VERSION} AS build ← first stage starts 17 ARG CUDA_RUNTIME_IMAGE=nvidia/cuda:13.3.0-... ← belongs to `build` 18 FROM ${CUDA_RUNTIME_IMAGE} AS run ← expands to emptyNODE_BUILD_VERSIONworks only because it sits above line 4. Introduced in f309cc6 (#496, T4 fleet), merged one day before beta.2.Moved above the first
FROM, with a note on the constraint.2. Ubuntu 22.04 base can't load the bundled uWS
Caught in review by @gemini-code-assist. This image explicitly copies uWebSockets.js in:
Its prebuilt Linux binaries link against
GLIBC_2.38; ubuntu22.04 ships glibc 2.35. That's the same failure documented at the top of./Dockerfile— the reason the standard image runs Debian trixie rather than bookworm (2.36).Default is now
13.3.0-cudnn-runtime-ubuntu24.04(glibc 2.39), and the CUDA 12.x override example moves to its ubuntu24.04 tag. All four tag combinations confirmed present on Docker Hub. The glibc floor is documented next to the ARG, because the ARG exists to be overridden for older driver branches and an override back to a 22.04 base would silently break uWS again.Latent until now — the build failed outright since #496, so this base was never actually produced.
3. uid 1000 collision, plus a setup block that swallowed it
The 24.04 base exposed this:
Ubuntu 24.04 ships a default
ubuntuuser at uid/gid 1000. harperdb must own 1000 — existing deployments chown their data volumes to it — so the incumbent is renamed rather than harperdb moved, which is exactly what./Dockerfilealready does for the node image'snodeuser. Handles bases with and without a uid-1000 user, sinceCUDA_RUNTIME_IMAGEis overridable.The more interesting half: the setup block had no
set -e, so both failures above were swallowed. The block continued and exited 0 on its trailingrm -rf, producing an image with noharperdbuser — which surfaced 15 steps later, far from the cause, as:Added
set -eso the block fails at the real cause instead of yielding a quietly broken image.Verification
Build-verified by dispatching
publish-docker.yamlwithimage=gpuon this branch — run 30457061300, success:Pushed SHA-scoped tags only —
harper-pro-gpu:sha-2c8607eand:sha-2c8607e-node24— so no release orlatesttag was touched.The intermediate run (30456716483) is useful evidence too: with only fix 1 applied it got 15 steps deep instead of failing at
FROM, confirming that fix independently before fix 3 was known.The uid-reclaim logic was also simulated under
shwith stubbedgetent/usermod/useraddfor both base shapes (uid 1000 present → rename; absent → create); both paths exit 0 underset -e.Follow-up, not in this PR
GPU images are missing for beta.2 and beta.3. If anything consumes them, they'll want a rebuild once this merges —
publish-docker.yamlacceptsworkflow_dispatchwithimage=gpu, but building those tags would need the fix cherry-picked to them, so it's probably simpler to let the next beta be the first good GPU image unless someone needs beta.3 specifically.