feat(profiles): auto-inject variant jupyterlab image for image-variant: gpu profiles

[tylerpotts]

What

Closes #230

Profiles in jupyterhub.custom.profiles can now be marked image-variant: <name>:

- slug: gpu
  display_name: "GPU Access"
  image-variant: gpu
  access: yaml
  groups: [gpu-access]
  kubespawner_override:
    # no image needed — the -gpu image is injected automatically
    node_selector: {node.kubernetes.io/instance-type: g4dn.xlarge}
    extra_resource_limits: {nvidia.com/gpu: 1}

At hub startup, 01-spawner.py injects kubespawner_override.image = <singleuser.image.name>-<variant>:<singleuser.image.tag>, so GPU profiles track pack updates exactly like CPU profiles instead of pinning a -gpu SHA in the deployer overlay. gpu is the only variant published today; the key is a string so a future -rocm/arm64 build needs no chart change.

How

• Both jupyterlab images are built by the same build-images.yaml run from the same commit, so they always share the same sha-<short> tag.
• _resolve_image_variants() in 01-spawner.py runs once at load, reading singleuser.image.name/.tag and custom.image-variants from z2jh. Precedence: explicit kubespawner_override.image > custom.image-variants.<name> > derived > nothing (warn, fall back to CPU default). The image-variant key is stripped whatever its value.
• jupyterhub.custom.image-variants: {} is the per-variant full-ref override map — the escape hatch for mirrored/airgapped registries.
• Warns when a variant profile also declares profile_options.image (the selected choice's image replaces the injected one at spawn time) and logs the injected ref per profile.
• scripts/bump_image_tags.py already bumps singleuser.image.tag each release, so the derived ref stays current with zero script changes.
• No Helm-side change: the derivation needs the variant name, which only the profile knows.

Testing

• tests/unit/test_spawner_profiles.py: derivation, generic variant names, override map, explicit-image precedence, key stripping (incl. empty variant), empty-base fallback + warning, profile_options.image warning (+ negative), input non-mutation, load-time wiring from z2jh keys, load-time log naming the injected ref (13 tests).
• Full unit suite: 141 passed. helm lint / helm template (incl. --set nebariapp.enabled=false as CI does) and ruff check config/ clean.

Docs

• docs/src/content/docs/server-profiles.md: "The GPU image, without hardcoding a SHA" section, mirrored-registry caution, amd64-only note, profile_options.image and explicit-pin caveats.
• docs/src/content/docs/values-reference.md: image-variants row.
• values.yaml: image-variants placeholder + updated GPU profile example.
• docs/superpowers/specs/2026-08-24-gpu-profile-image-design.md: design, why the string key, rejected alternatives.

[github-actions]

Docs preview for feat/gpu-profile-image-230:
https://feat-gpu-profile-image-230.data-science-pack.pages.dev

[viniciusdc]

Nice work — this is the right fix for #230, and the derivation holds up. I checked scripts/bump_image_tags.py:27-30 and TAG_TARGETS does bump singleuser.image.tag, so the derived ref really does track releases with no script change. The suite reproduces too: 137 passed in a CI-equivalent venv, no skips.

One blocker, two questions, and a few smaller things.

Blocker

profile_options.image.choices silently defeats the injection — config/jupyterhub/01-spawner.py:445

A gpu: true profile that also declares an image profile option won't get the GPU image. KubeSpawner applies the choice override after the profile-level one and it replaces rather than merges (_load_profile at spawner.py:3642, profile override at :3658, choices at :3694, and the docstring at :1807-1814 spells out the precedence). Reproduced against the pinned 7.0.0 — profile-level gpu-lab:sha-1 in, cpu-lab:sha-1 out.

The spec says deployer-defined profiles rarely carry them, but both shipped profiles do (values.yaml:523-531 and :541-549), and that's the shape someone copies to build a GPU profile. So you end up with a GPU node requesting nvidia.com/gpu: 1 and running the CPU image, silently — and jhub-apps' Create App reads type.kubespawner_override.image, so it displays the GPU one. Split brain.

Injecting into the choices too would fix it properly, but a log.warning when a gpu: true profile declares profile_options.image, plus a doc sentence, is fine by me.

Questions

Should this be image-variant: gpu rather than a boolean? — values.yaml:495

We already have two naming axes (-base, -gpu), so a -rocm or an ARM build isn't hypothetical. Once gpu: true ships in values.yaml we can't drop it without a deprecation cycle, and we'd end up carrying gpu: true and variant: rocm. The string key is the same amount of code today.

The counter-argument is that gpu: true reads better in an overlay and is more discoverable, which I do buy. What made you pick the boolean? Asking mainly because the spec has no ### Rejected alternatives or ### Why this approach section — the umask spec has both, and this is exactly the call that belongs there.

"always valid" isn't quite true — docs/src/content/docs/server-profiles.md:173-176

It holds for the chart default, but anyone mirroring jupyterhub.singleuser.image.name into ECR or an airgapped registry gets <mirror>-gpu:<tag>, which almost certainly doesn't exist. helm upgrade and helm template both succeed, and the first GPU spawn dies in ImagePullBackOff — no signal until a user hits it.

custom.gpu-image is exactly the escape hatch for this, so that knob earns its place IMO. But neither this page nor values.yaml:393-398 says "if you mirror singleuser.image, set gpu-image too". Also worth a line that jupyterlab-gpu is amd64-only (build-images.yaml:67) while the CPU image is multi-arch.

Suggestions

gpu: false isn't stripped — config/jupyterhub/01-spawner.py:460-461

The strip sits inside if profile.get("gpu"), so gpu: false stays on the profile — but the docstring at :454-456, server-profiles.md:180, and values.yaml:502 all say it's stripped either way. Not a spawn bug, I checked: _load_profile, _get_initialized_profile_list and get_options_form are all happy with gpu: False present, no warnings. So it's just the three docs being wrong.

Hoisting the strip out of the if makes them true and removes the case entirely. I tried it (if "gpu" in profile: outer, if is_gpu: inner) and the suite still passes 137.

The new helper orphans the nebiImage doc comment — templates/_helpers.tpl:161-178

The new comment and define got wedged between the "Nebi image reference" comment at :161-164 and the nebiImage define at :178 it documents, so nebiImage has no doc now and the GPU helper looks like it carries a two-paragraph one. Moving the new block below nebiImage's {{- end -}} fixes it. While you're there — there's a real precedence chain here (explicit kubespawner_override.image > custom.gpu-image > derived > nothing), and every other helper with one carries the numbered "Order of precedence" list (:12, :29, :67, :94).

The injection is invisible in hub logs — config/jupyterhub/01-spawner.py:445

We log nss-wrapper detail at debug (:1007, :1015, :1067) and profile visibility at info (:437), so a derived image ref that gets pulled into a user pod is above that bar. The log.info("profiles: loaded %d profile(s)...") at :475 only prints slugs, so kubectl logs deploy/hub can't answer which image the GPU profile got or where it came from. Adding the injected ref to that line would cover it.

Same place: a log.warning when gpu-image derives empty would help. That case silently lands a CPU image on a GPU node, and raising would break hub startup and therefore login, so a warning is the right level.

Test docstrings, and one doc clause — tests/unit/test_spawner_profiles.py

Four of the seven new tests have no docstring (:286, :295, :309, :330), and none of the seven carries an assert message. test_chart_derived.py:153 is the model — it has both.

Separately, worth one clause in the docs: _bump_profile_list matches startswith(JUPYTERLAB_IMAGE + ":") (:79, :93), so an explicitly pinned -gpu ref never gets bumped. "An explicit kubespawner_override.image always wins" also means "and stays frozen forever", which is the exact pain #230 is about. Worth saying so nobody pins as a workaround.

Follow-ups, not this PR

• Teach bump_image_tags.py to bump -jupyterlab-gpu: refs so an explicit pin isn't a one-way door.
• Write down the build invariant. build-images.yaml runs jupyterlab and jupyterlab-gpu as independent workflow calls with no needs:, and the only thing keeping the derived ref real is bump-image-tags.yaml:31 gating on workflow_run.conclusion == 'success'. A continue-on-error, a paths: divergence, or the manual workflow_dispatch path breaks it silently and nothing records that.
• Name the gpu strip next to _PROFILE_GATING_KEYS (:334) with a line on why load-time stripping is correct here, so a future custom key knows which mechanism to use.

Checked and explicitly not raising

The hub does roll on a values change (both paths change a z2jh checksum annotation); tag: 1.2 is rejected by z2jh's schema before printf sees it; the bump_image_tags.py prefix mismatch is deliberate — the trailing colon is what stops it rewriting a -gpu ref into the CPU one; no test isolation leakage from the z2jh.get_config swap.

One worth flagging because it's easy to get backwards: the if and $img.name $img.tag guard in the helper is not dead code. z2jh's schema requires name/tag to be present strings but allows empty ones, so --set jupyterhub.singleuser.image.name="" is schema-valid and renders. With the guard you get "gpu-image": ""; without it, "-gpu:sha-5dfee5e", a garbage ref. The same reasoning makes the Python if gpu_image and ... guard reachable. Keep both, and keep test_gpu_profile_without_derived_image_falls_back_to_default.

Nothing security-relevant here either — gpu and custom.gpu-image are both deployer-authored values, and the deployer can already write kubespawner_override.image directly, which is what #230 is asking to stop doing. The new knob is strictly less expressive than what they already control.

[tylerpotts]

Thanks for the thorough review — addressed in da477a7 and c6f2201.

Blocker: profile_options.image defeats the injection. Went with the warning + docs rather than injecting into the choices: choices always carry an explicit image (that is what they are for), so rewriting them would overwrite explicit deployer values and contradict "explicit wins". _resolve_image_variants now logs a warning naming the profile when image-variant coexists with profile_options.image; server-profiles.md and the values.yaml comment both say not to copy the shipped profiles' image option onto a variant profile. Tests: test_variant_profile_with_image_choices_warns (+ a negative test so the warning stays specific).

image-variant: gpu instead of a boolean. Agreed and switched (c6f2201). The key is a string so a future -rocm/arm64 build needs no chart change. Consequences:

• Derivation is now <singleuser.image.name>-<variant>:<singleuser.image.tag> in 01-spawner.py, reading singleuser.image.name/.tag from z2jh. It had to move out of Helm: only the profile knows the variant name, so _CHART_DERIVED["gpu-image"] and the gpuJupyterlabImage helper are gone (and with them the and $img.name $img.tag guard you flagged — the equivalent base_name and base_tag check lives in Python and is covered by test_variant_without_base_image_falls_back_to_default).
• custom.gpu-image becomes custom.image-variants: {}, a per-variant full-ref override map (image-variants: {gpu: <mirror>/lab-gpu:sha-...}), so the mirrored-registry escape hatch generalises too.
• Precedence: explicit kubespawner_override.image > custom.image-variants.<name> > derived > nothing (warn, CPU default). Key stripped whatever its value, including image-variant: "".
• The spec now has ### Why this approach / ### Rejected alternatives, arguing for the string key and listing the boolean and the Helm derivation as rejected.

"always valid". Reworded to "exists on quay.io/nebari for every release", and added a :::caution[Mirrored registries] block: the derivation only rewrites the name, nothing validates the ref at helm upgrade, first GPU spawn dies in ImagePullBackOff, so mirror the -gpu image too or map it in custom.image-variants. Also notes the -gpu image is linux/amd64 only. Same note in the values.yaml comment.

Suggestions

• The marker key is stripped whatever its value (strip hoisted out of the truthiness check); docs were already claiming this, now they are true. Covered in test_variant_key_is_stripped_before_kubespawner.
• _helpers.tpl: nebiImage has its doc comment back (the GPU helper that displaced it no longer exists).
• Logging: per-profile info with the injected ref, and a warning when the ref cannot be derived and no override is set. Tests: test_load_log_names_the_injected_variant_image, test_variant_without_base_image_falls_back_to_default.
• All variant tests have docstrings and assert messages.
• Docs + values.yaml say an explicit pin is never bumped by bump_image_tags.py and to prefer the key over pinning.
• Took the third follow-up in this PR since it is a comment: _PROFILE_GATING_KEYS now explains per-user strip vs load-time strip and which a future key should use.

Follow-ups (left out as you suggested): teaching bump_image_tags.py to bump -gpu refs, and recording the build invariant between the two build-images.yaml jobs. Both noted in the spec's out-of-scope list.

PR title/body updated to match. Suite: 141 passed; helm lint/helm template with nebariapp.enabled=false and ruff check config/ clean.

[tylerpotts]

This has been tested on Hetzner and is functional

[viniciusdc]

Thanks @tylerpotts, and sorry for the slow turnaround on this one — you'd addressed it five days ago and I left it sitting.

The pivot answered four of my findings in a single move: precedence is documented in the docstring, the key is stripped whatever its value, both degraded paths warn instead of raising, and the injected ref gets logged. And the profile_options.image resolution landed better than I expected — the warning fires at import, not at spawn, so an operator sees it in kubectl logs deploy/hub before any user reaches a GPU node, and server-profiles.md:212-213 names the hazard at exactly the spot someone would copy a shipped profile. I'm satisfied with warn+docs there.

Also worth saying plainly: my previous review closes with "keep both, and keep test_gpu_profile_without_derived_image_falls_back_to_default". After the pivot neither referent exists — that review is written entirely against gpu: true and the Helm helper, so anyone reading it now gets a wrong picture.

One blocker, and it's a small one.

A mistyped custom.image-variants takes the hub down

01-spawner.py:491. If a deployer writes image-variants: gpu instead of a map, nobody can log in — the hub CrashLoops at config load, and fixing it needs a values edit plus a rollout.

(overrides or {}) only rescues falsy values, so a truthy non-mapping reaches .get at module import (:516 is top-level). Nothing upstream types it — z2jh 4.4.0 has "custom": {"type":"object","additionalProperties":true} and we ship no schema of our own — so it clears both gates and lands in the Secret verbatim:

helm template ... -f bad.yaml   ->  exit=0 (coalesce warning only)
helm lint ...                   ->  1 chart(s) linted, 0 chart(s) failed
Secret/hub data["values.yaml"]  ->  custom.image-variants = 'notamap'
import 01-spawner.py            ->  AttributeError: 'str' object has no attribute 'get'

That's the one thing :470-471 says this function must never do. To be fair on weight: no shipped default triggers it and the key has zero adopters, so it's blocking-but-trivial rather than a design problem.

        if overrides and not isinstance(overrides, dict):
            log.warning(
                "profiles: custom.image-variants is %s, expected a mapping of "
                "variant -> image ref — ignoring it", type(overrides).__name__,
            )
            overrides = {}

Keep the overrides and — without it, bare image-variants: (YAML null) trips the warning for a perfectly fine value. And if isinstance(image, str) and image: at :493 would stop the sibling case: {gpu: 123} currently injects image: 123 and surfaces as a traitlets error at spawn instead. I ran the guard against all three bad shapes plus null plus a good dict — 141 still pass.

On whether the map should exist at all: I went in expecting to argue for deleting it, and the argument doesn't hold. The -gpu image is a separate repository (build-images.yaml:41-46 and :61-69), so mirroring is per-repo. Pull-through proxies preserve the name and derive fine, but copy-based mirroring — skopeo copy, crane copy, the dominant airgap flow — takes an explicit destination per invocation, and nothing makes an org that flattens to platform/jupyterlab then choose platform/jupyterlab-gpu. Keep it.

A map key typo silently discards the deployer's override

01-spawner.py:491. Write image-variants: {gpus: ...} against an image-variant: gpu profile and the entry never matches — the derived ref is used instead, and :496 logs it at info as a normal successful injection. The deployer's explicit choice is overridden by a default with no warning anywhere, which is the one failure mode a mirrored-registry deployer is most likely to hit.

That's also the map's own failure mode, and values.yaml:393-406 is the one place the house style says to record it. A loop after :513 warning on override keys no profile claimed would cover it, or a sentence in the comment block if you'd rather keep the code as-is.

Both profile_options.image warning tests pass if the warning names the wrong profile

tests/unit/test_spawner_profiles.py:381 and :414. That warning is the only signal a deployer gets that their variant profile is silently spawning the CPU image, and the tests guarding it wouldn't notice the profile name vanishing from it.

Both fixtures use a profile whose slug is "gpu" and whose variant is "gpu", so "gpu" in w is satisfied by the variant token alone. Substituting "WRONG-PROFILE" for name at :502 and :509 gives 30 passed, no failures — I re-ran it to be sure. :382's own assert message says it expects "a warning naming the profile", which is what made me look.

Giving the fixture a slug distinct from the variant ({"slug": "gpu-large", "image-variant": "gpu"}) and asserting "gpu-large" in w fixes both, and covers the slug or display_name fallback at :483 that nothing exercises today.

Separately, test_variant_profile_without_image_choices_does_not_warn:432 asserts on unscoped caplog.records, and _load() at :422 runs the whole module import inside the recording window — so any future load-time warning anywhere in the file turns this test red for an unrelated reason. caplog.clear() right after :422.

Smaller things

• The spec's reason for the pivot will not survive being checked. 2026-08-24-gpu-profile-image-design.md:75-78 says Helm "does not see the profile list the z2jh subchart consumes". It does — jupyterhub is a subchart (Chart.yaml:9-12) and we already do index .Values.jupyterhub.custom "<key>" at _helpers.tpl:182, 213, 228, 237, so a range over the profiles is mechanically possible. Someone will re-derive that and conclude the whole rejection was sloppy. The real reason is better: _CHART_DERIVED's precedence trick is scalar-only — "" means "not set" (hub-config.yaml:63-71), and there's no equivalent sentinel for a map, so you can't tell "deployer set {}" from "chart baked {}". Suggested swap: "Helm can read the profile list, but _CHART_DERIVED carries scalars only — its ""-means-unset sentinel has no map equivalent, so a variant map cannot round-trip through it."
• :468's "Returns new dicts" is false for most profiles, and that's the sentence a future caller will trust before mutating a result and corrupting z2jh's cached config dicts hub-wide. Non-variant profiles are appended by reference at :481 — deliberate, and test_non_variant_profile_is_untouched:355 pins it as "byte-for-byte", so the code is right and the docstring is wrong. Something like "Variant profiles are rebuilt as new dicts; the rest pass through by reference. Nothing is mutated — the input list and its dicts are left untouched." Related: image-variant: with an empty value takes :486 and produces the same outcome as precedence case 4 with no log at all, while case 4 warns at :498-503. log.warning("profiles: %r has an empty image-variant — no image injected", name) before :487 would close the gap, since the numbered list promises four outcomes and this is a fifth.
• server-profiles.md:181 — "Like the gating keys, image-variant is stripped…" is accurate, but the comparison invites the reader to expect gating-key behaviour on a typo, and a misspelled variant doesn't fail closed, it ImagePullBackOffs at spawn. I think the asymmetry is right (an access typo is a silent permissive security failure; this one is loud and inert, and the derived ref is already in the startup log) — just worth a clause in the existing :::caution saying so.
• tests/unit/test_chart_derived.py:151 has a stray trailing blank line from reverting the gpu-image tests. CI can't catch it — lint.yaml:33 is ruff check config/ only.

Checked and not raising

141 pass at head, reproduced in a CI-pinned venv, and isolation is clean (30 + 111 = 141 in every ordering). I went looking for coverage theater in 13 tests for a 60-line function and there isn't any — every one uniquely kills a mutant, including the three I'd have bet on cutting; test_variant_name_is_generic is the only thing standing between you and a hardcoded -gpu.

The hub rolls on a profiles change — checksum/secret covers all of custom (z2jh deployment.yaml:31), which also settles an open question from an earlier round. Renders clean both ways, the new key survives into the Secret, the kubespawner replace-not-merge ordering is confirmed against 7.0.0's source, and bump_image_tags.py genuinely never touches a -gpu ref. Nothing exploitable — every input is deployer-authored values, and crafted variants produce invalid refs rather than redirecting anywhere.

On the spec being rewritten rather than carrying a > Post-review update blockquote: that convention is for a spec already on main. This one is new in this PR, so a blockquote would permanently record a within-PR iteration and leave a ## Design section describing code that never shipped. Demoting both dead designs into ### Rejected alternatives with why they died is the better call.

Finally, #241 and this PR only collide on the values.yaml NOTE block, and both texts are additive — whoever lands second keeps both paragraphs. Variant profiles pass #241's guarded image-ref tests already; it was written anticipating this.