diff --git a/catalog/render.py b/catalog/render.py index 4795f7f..3d2db72 100644 --- a/catalog/render.py +++ b/catalog/render.py @@ -163,6 +163,15 @@ OAUTH2_COOKIE_REFRESH = "2m30s" # are added. Do not read this flag as "metrics are working". OAUTH2_PROXY_METRICS_PORT = 44180 +# --- Hardware capability: `hardware.gpu: intel-quicksync` ------------------------------ +# The ServiceDefinition declares the CAPABILITY (schemas/servicedefinition.schema.json +# `hardware.gpu`); everything below is the MECHANISM that currently satisfies it and is +# deliberately confined to this file. Swapping to the Intel GPU device plugin +# (`gpu.intel.com/i915` resource requests instead of a hostPath) is a change here and +# nowhere else -- no catalog entry moves. +DRI_VOLUME_NAME = "dri" +DRI_DEVICE_DIR = "/dev/dri" + # ============================================================================= # Defaults (renderer-contract.md "Default posture") — materialize BEFORE mapping. @@ -513,6 +522,46 @@ def r_deployment(s: dict) -> dict: "persistentVolumeClaim": {"claimName": f"{s['name']}-{vol['name']}"}, } ) + # hardware capability (schema-optional): `hardware.gpu: intel-quicksync` means this + # workload needs the host iGPU's render node for hardware video transcode. The SD says + # only that; the wiring below is the platform's current mechanism for satisfying it. + # + # `type: DirectoryOrCreate` is DELIBERATE and load-bearing -- do NOT "tighten" it to + # `Directory`. A node that comes back without the render device (rebuild, BIOS change, + # driver regression, hardware swap) must degrade the workload to SOFTWARE transcoding, + # not fail the mount: `apps-wave-0` is `wait: true` and `apps-wave-1` dependsOn it, so a + # failed hostPath mount stalls the entire app tier for the sake of a feature that is not + # load-bearing. `DirectoryOrCreate` turns that failure mode into "transcodes in + # software" instead of "the app tier does not reconcile". /dev is tmpfs, so the empty + # directory the kubelet creates on such a node leaves nothing durable behind. + # + # The mount is deliberately NOT readOnly: VA-API opens the render node read-write. + # + # `podSecurityContext.supplementalGroups` is DELIBERATELY NOT SET here, and adding it + # would be a regression, not a hardening. MEASURED in the running container on prod-2, + # 2026-09-30: the image's own cont-init hook + # `/etc/cont-init.d/45-plex-hw-transcode-and-connected-tuner` `find`s every char device + # under /dev/dri, `stat -c '%g'` each one's GID, `groupadd`s a matching group INSIDE the + # container when none exists, and `usermod -a -G` the app user into it -- every start, + # nothing hardcoded. cont-init.d completes before services.d, so the account database + # s6 reads at exec time already carries the group. The service is then started with + # `exec s6-setuidgid plex ...`, and `s6-setuidgid` REPLACES the process group list from + # /etc/group rather than appending to it: `Plex Media Server` runs with + # `Uid: 1000 Gid: 1000 Groups: 100 1000` -- root's gid 0, which PID 1 (s6-svscan) does + # have, is GONE. Anything Kubernetes puts in supplementalGroups reaches s6-svscan and is + # then discarded before the app ever starts. MOUNTING THE DEVICE IS SUFFICIENT; the + # group handling is the image's job and it does it correctly. A supplementalGroups entry + # here would be inert AND would mislead whoever debugs this next. + hw_gpu = (s.get("hardware") or {}).get("gpu") + if hw_gpu == "intel-quicksync": + volume_mounts.append({"name": DRI_VOLUME_NAME, "mountPath": DRI_DEVICE_DIR}) + volumes.append( + { + "name": DRI_VOLUME_NAME, + "hostPath": {"path": DRI_DEVICE_DIR, "type": "DirectoryOrCreate"}, + } + ) + if volume_mounts: container["volumeMounts"] = volume_mounts @@ -2530,6 +2579,83 @@ def _selftest() -> int: "an SD without podSecurityContext must not gain a pod securityContext", ) + # 6b) hardware.gpu: intel-quicksync -> a /dev/dri hostPath volume and a read-write mount + # on the app container, and NOTHING ELSE. The SD declares the CAPABILITY; these + # assertions pin the MECHANISM that currently satisfies it -- including what it + # deliberately does NOT emit -- so a change to it is a deliberate, visible edit. + hw_docs = render( + { + "name": "hw", + "source": {"image": "a:1"}, + "hardware": {"gpu": "intel-quicksync"}, + } + ) + hw_pod = next( + d[1] for d in hw_docs if d[1]["kind"] == "Deployment" + )["spec"]["template"]["spec"] + check( + { + "name": "dri", + "hostPath": {"path": "/dev/dri", "type": "DirectoryOrCreate"}, + } + in hw_pod.get("volumes", []), + "hardware.gpu=intel-quicksync must render a /dev/dri hostPath volume " + "with type DirectoryOrCreate (a GPU-less node must degrade, not fail the mount)", + ) + hw_mounts = hw_pod["containers"][0].get("volumeMounts", []) + check( + {"name": "dri", "mountPath": "/dev/dri"} in hw_mounts, + "hardware.gpu=intel-quicksync must mount /dev/dri READ-WRITE on the app container", + ) + # The device mount is the WHOLE feature: no pod securityContext is invented. The image's + # own cont-init hook adds the app user to the device's group inside the container, and + # s6-setuidgid discards any Kubernetes-supplied supplementalGroups anyway (see the + # emission site). Asserting the absence is what stops someone adding an inert one back. + check( + "securityContext" not in hw_pod, + "hardware.gpu must NOT invent a pod securityContext (supplementalGroups is inert " + "under s6-setuidgid; the image's cont-init hook handles the device group)", + ) + # Nothing privileged, and no node pinning: Kyverno restrict-host-namespaces (Enforce) + # rejects `privileged: true` in an app namespace, and placement is a separate concern + # with its own justification -- this field is about the DEVICE only. + check( + "privileged" not in hw_pod.get("securityContext", {}) + and "securityContext" not in hw_pod["containers"][0] + and "nodeSelector" not in hw_pod, + "hardware.gpu must emit no privileged securityContext and no nodeSelector", + ) + # The regression that matters: absence renders NOTHING. Mirrors the podSecurityContext + # negative above -- an SD that does not opt in must be byte-identical to before. + nohw_pod = next( + d[1] for d in render({"name": "nohw", "source": {"image": "a:1"}}) + if d[1]["kind"] == "Deployment" + )["spec"]["template"]["spec"] + check( + "volumes" not in nohw_pod + and "volumeMounts" not in nohw_pod["containers"][0] + and "securityContext" not in nohw_pod, + "an SD without `hardware` must gain no dri volume, no mount and no securityContext", + ) + # An SD that declares BOTH keeps its own podSecurityContext exactly as written -- the + # hardware field neither adds to it nor rewrites it. + hwm_pod = next( + d[1] + for d in render( + { + "name": "hwm", + "source": {"image": "a:1"}, + "hardware": {"gpu": "intel-quicksync"}, + "podSecurityContext": {"fsGroup": 1001, "supplementalGroups": [44]}, + } + ) + if d[1]["kind"] == "Deployment" + )["spec"]["template"]["spec"] + check( + hwm_pod["securityContext"] == {"fsGroup": 1001, "supplementalGroups": [44]}, + "hardware.gpu must leave the SD's own podSecurityContext untouched", + ) + # 6c) forward-auth (Option B): an exposed forward-auth app gets an oauth2-proxy sidecar that # fronts the Gateway path (fail-closed), while oidc/native/none apps are unchanged. fa_docs = render( diff --git a/decisions/2026-09-30-sd-hardware-capability.md b/decisions/2026-09-30-sd-hardware-capability.md new file mode 100644 index 0000000..dcc5562 --- /dev/null +++ b/decisions/2026-09-30-sd-hardware-capability.md @@ -0,0 +1,184 @@ +--- +date: 2026-09-30 +branch: feat/sd-hardware-capability +title: ServiceDefinition gains a `hardware` capability field — `gpu: intel-quicksync` mounts /dev/dri and nothing else; the enum is what keeps the mechanism swappable +--- + +## What changed + +A new optional top-level ServiceDefinition property, `hardware`, with one optional member, +`gpu`, whose enum today has exactly one value: `intel-quicksync`. **No catalog entry uses it.** +This PR delivers the capability; consuming it (Plex) is separate work on top. + +- `schemas/servicedefinition.schema.json` — adds `hardware` (object, `additionalProperties: false`, + `minProperties: 1`) with `gpu` (enum `["intel-quicksync"]`). Schema version bumped **1.7.0 → + 1.8.0**, changelog entry prepended to the schema `description` per that file's own convention. +- `catalog/render.py` — two module-level constants (`DRI_VOLUME_NAME`, `DRI_DEVICE_DIR`) and the + emission in `r_deployment`. +- `schemas/renderer-contract.md` — new `### hardware` field-mapping section, plus cross-field + invariant **8** in that file's numbered list. +- `schemas/README.md` — its `**Schema version:**` line said `1.0.0`; it had been stale since 1.1.0. + Synced to 1.8.0 and annotated that the schema `description` is the changelog of record. Added one + bullet to "Cross-field rules" for the new enum. That was the whole of the staleness; nothing + ballooned. + +**What the renderer emits** for `hardware.gpu: intel-quicksync`, on the app container only (never a +sidecar), and it is exactly two things: + +1. a hostPath volume `dri` → `/dev/dri`, `type: DirectoryOrCreate`; +2. a read-write `volumeMount` of `dri` at `/dev/dri` on the app container (deliberately not + `readOnly` — VA-API opens the render node read-write). + +**Mounting the device is sufficient.** It emits **no** `privileged: true` (Kyverno +`restrict-host-namespaces` is Enforce on app namespaces and it is not needed), **no** `nodeSelector` +(placement is a separate concern with its own justification), **no** resource requests (there is no +app-container resource render path), and — the one that is worth spelling out — **no +`podSecurityContext.supplementalGroups`**. + +**Why no `supplementalGroups`, when `/dev/dri/renderD128` is `root:render 0660`.** An earlier +revision of this branch merged the host `render` GID into `supplementalGroups`. **It was measured to +be a no-op and has been removed.** In the running container on prod-2, 2026-09-30: + + PID 1 s6-svscan Uid: 0 Gid: 0 Groups: 0 + pid 246 Plex Media Server Uid: 1000 Gid: 1000 Groups: 100 1000 + +`/etc/services.d/plex/run` ends in `exec s6-setuidgid plex …`, and the child's group list is exactly +the `/etc/group` account-database list — root's gid `0`, which PID 1 holds, has **vanished**. That is +a `setgroups()` that *replaces* rather than appends, so anything Kubernetes puts in +`supplementalGroups` reaches `s6-svscan` and is discarded before the app ever starts. What actually +works is the image's own `/etc/cont-init.d/45-plex-hw-transcode-and-connected-tuner`, which `find`s +every char device under `/dev/dri`, `stat -c '%g'`s its GID, `groupadd`s a matching group inside the +container when none exists, and `usermod -a -G`s the app user into it — every start, nothing +hardcoded; `cont-init.d` completes before `services.d`, so the database is correct at exec time. + +Shipping an inert `supplementalGroups` would have been worse than shipping nothing: it would read as +the mechanism to whoever debugs this next. The reasoning lives in a comment **at the emission site** +and in `renderer-contract.md`, and `render.py --selftest` now asserts the *absence* of a +`securityContext`, so it cannot be "helpfully" added back without a test failing. A pleasant +corollary: because the hook re-`stat`s the device every start, host GID drift is **self-healing**, +and no GID is pinned anywhere in this repo. + +**The design decision worth defending: the field is an enum of intent, not a device path.** The SD +says *what class of host device the workload needs*; the hostPath and the mount exist only in +`catalog/render.py`. Migrating to the Intel GPU device plugin (`gpu.intel.com/i915` resource +requests, no hostPath at all) is then a renderer change with zero catalog edits. Both the schema +`description` and the contract section say this explicitly, because the obvious "simplification" — +letting an SD name `/dev/dri` — would push a platform implementation detail into the operator-facing +contract and make that migration a catalog-wide edit. + +**`DirectoryOrCreate` is load-bearing.** A node that comes back without the render device (rebuild, +BIOS change, driver regression, hardware swap) must degrade the workload to software transcoding, +not fail the mount: `apps-wave-0` is `wait: true` and `apps-wave-1` `dependsOn` it, so a failed +hostPath mount stalls the entire app tier for the sake of a feature that is not load-bearing. +`/dev` is tmpfs, so the empty directory the kubelet creates on such a node leaves nothing durable +behind. Do not "tighten" it to `Directory`. The reasoning is recorded at the emission site. + +## What I verified + +Measured, in this worktree, on this branch: + +- **`python3 catalog/render.py --selftest` → `render.py selftest: PASS`, rc 0.** New cases in the + existing `check(...)` style: the positive (volume + mount, and *no* `securityContext`, *no* + privileged, *no* nodeSelector), the negative that matters (an SD without `hardware` gains no + volume, no volumeMount and no `securityContext` — mirroring the existing `podSecurityContext` + negative), and a pass-through case (an SD declaring both `hardware` and its own + `podSecurityContext: {fsGroup: 1001, supplementalGroups: [44]}` keeps it **exactly** as written). +- **The self-tests actually bite.** I mutated the guard (`if hw_gpu == "intel-quicksync":` → `if + True:`) in a scratch copy and re-ran: **rc 1**, failing on "an SD without `hardware` must gain no + dri volume, no mount and no securityContext". A test that cannot fail is not a test; this one can. +- **Re-render is a zero diff.** + `python3 catalog/render.py --all catalog -o apps --wave-layout --no-image-automation` then + `git diff --stat apps/` → **empty**, and `git status --porcelain apps/` → **empty** (so the two + not-onboarded by-products, `apps/wave-1/keycloak.yaml` and `apps/wave-0/vm-demo.yaml`, were + rewritten byte-identically and needed no revert). The change is inert until a service opts in. +- **Schema validity and catalog conformance.** `Draft202012Validator.check_schema` passes; all + **12** `catalog/*.yaml` validate with **0** errors. On the new field specifically: `gpu: + intel-quicksync` accepted; `gpu: /dev/dri` rejected (enum); `hardware: {gpu: …, path: …}` rejected + (`additionalProperties: false`); `hardware: {}` rejected (`minProperties: 1`). +- **Hand-checked the emitted YAML** by rendering a throwaway SD carrying `hardware`, a Longhorn + volume and its own `podSecurityContext: {runAsUser: 1000, supplementalGroups: [44]}`. Result: + `volumeMounts` = config then `{name: dri, mountPath: /dev/dri}`; `volumes` = the PVC then + `{name: dri, hostPath: {path: /dev/dri, type: DirectoryOrCreate}}`; `securityContext` = + `{runAsUser: 1000, supplementalGroups: [44]}` — i.e. **unchanged**, nothing appended. That toy SD + was never written into `catalog/`. +- **GATE: `rc 3` — UNDETERMINED, NOT green.** Run in the pinned toolbox from this worktree with + both CLAUDE.md §3 workarounds (`DEV_KUBECONFIG=/dev/null`, and an `ENGINE` shim bind-mounting the + main checkout's `.git` at its own absolute path). Diff base `origin/main @ 0fbf544d5e2f`; touched + requirements `R-DNS-03 R-GITOPS-02 R-IDP-02`. Run **four** times on this branch with an identical + result each time, most recently against tree `c132893045b1` — the final commit differs from that + run only by this paragraph. Per stage: + + | stage | result | + |---|---| + | spec-immutability | **SKIPPED** — sibling spec repo not at `/work/../home-network-k3s`. Expected from a worktree; tolerated by §3. | + | decisions-log | pass | + | lint | pass | + | l0 | pass — 19 offline cases, `0 failure(s), 0 skip(s), 0 unwired` | + | render-golden | pass — *"all 10 onboarded service golden(s) reproduce from the canonical render"* | + | l1 | pass — 3 offline checks, `0 failure(s), 0 skip(s), 0 unwired`; coverage 51/51 requirements scaffolded | + | approval-policy | pass (`event='none'`, not a pull_request event — N/A locally; it will evaluate for real on the PR) | + | gitea-settings | **SKIPPED** — `SETTINGS_API_TOKEN` unset. Expected; branch protection and runner registration are therefore UNVERIFIED by this run. | + | conformance (L2/L3) | **SKIPPED** — `CLUSTER_AVAILABLE!=true`. Phase-0 standing skip. | + + The gate's own verdict: *"Stages that did not run: spec-immutability gitea-settings"*. **No stage + failed, and no stage skipped for a reason caused by this change.** rc 3 is reported as rc 3; I am + not calling this green. +- **Blast radius of the schema change:** `hardware` is absent from every catalog entry, so + `apply_defaults` never materializes it and `s.get("hardware")` is `None` on all 12 — which is what + the zero diff above demonstrates rather than asserts. +- **Touched no sensitive path.** Changes are confined to `schemas/`, `catalog/`, `decisions/`; per + `infra/gitea/approval-policy.json` none of those are sensitive. `conformance/**` and `clusters/**` + are untouched. I did not run `--emit-waves` (it writes `clusters/base/`, and the set of files + under `apps/` did not change). + +## What I did NOT verify + +- **Whether containerd's device cgroup permits a non-privileged pod to `open()` a bind-mounted char + device. Still unverified, and it is the single largest technical risk in this workstream.** + containerd installs a per-container **device cgroup** allow-list, and whether + `/dev/dri/renderD128` arriving as a bind-mounted device node is permitted by it on this + k3s/containerd version is **unmeasured**. Settling it requires applying a pod, which is an + operator action, not an agent one. The container's PID 1 running as uid 0 helps on the DAC side + but does **not** bypass the cgroup gate. If it blocks, the mount will succeed, the image's group + hook will do its job, and the `open()` will still fail — and the fallback is the Intel GPU device + plugin, which *sets* the cgroup entry. Note that that fallback is a **renderer-only change**: no + ServiceDefinition moves, because the authoring surface is an intent enum. That is the design + justifying itself, and it is the reason to keep the enum even if the hostPath mechanism loses. +- **That a missing device degrades silently — and nothing will tell you.** This is the accepted cost + of `DirectoryOrCreate`. The workload will software-transcode and **no signal reports it**: media + apps in this catalog declare no `metricsEndpoint`, so no ServiceMonitor exists, and there is no + metric to alert on. I deliberately did **not** propose an alert, because an alert that cannot fire + is worse than none — see the 2026-09-13 correction in `renderer-contract.md` §`observability`, + where twelve monitors discovered zero targets. The only proof of hardware transcode is an operator + seeing it in the application's own dashboard. +- **That "mounting the device is sufficient" generalizes beyond this one image.** It does not, and + I have not verified that it does. The claim was measured for an image that resolves the device + group itself in `cont-init.d`. A future consumer of `hardware.gpu` that runs non-root with no + equivalent hook would still need the group solved — and the fix for that case is a **renderer** + change (per-image handling, or the device plugin), never a new SD field. I scoped the claim in + `renderer-contract.md` rather than letting it read as general; the first such consumer must + re-measure. +- **The feature now depends on a hook inside an image that floats.** The `cont-init.d` behaviour + above was read from digest `sha256:e0ab27395614…` (`plexmediaserver 1.43.4.10903`), but the + deployment runs `plexinc/pms-docker:latest` with `imagePullPolicy: Always`. **A silent pull can + change the behaviour this feature depends on, with no commit anywhere in this repo to show for + it** — and the failure would be the silent software-transcode described above. I have not fixed + that here (it is the Plex wiring's tag/digest decision, not the capability's), but it should not + be forgotten. +- **GID drift is NOT on this list any more, deliberately.** An earlier revision pinned + `INTEL_RENDER_GID = 992` and listed "a node rebuild could move it" as an open risk. The hook + re-`stat`s the device on every start, so drift is self-healing and no GID is pinned anywhere. The + risk is retired, not overlooked. +- **Anything on the live cluster.** I ran no `kubectl` and I opened no shell in any container. The + node facts (device present, `root:render` 0660, `i915` bound, no device plugin, no + `gpu.intel.com/*` advertised), the in-container measurements quoted above, and the Kyverno + posture (`restrict-host-namespaces` Enforce; no hostPath or `supplementalGroups` + restriction; no PSA labels on app namespaces) are **prior recon handed to me, not re-measured by + me**. Per CLAUDE.md §2b, treat them as second-hand until re-queried. +- **That CI exercises the self-tests. It does not.** `git grep -- --selftest ci/` returns nothing — + no gate stage invokes the renderer's self-test. They are the executable spec and house style, and + I ran them by hand, but a future regression in this code path **will not be caught by the gate**. + Wiring `--selftest` into a gate stage is a real gap and is not part of this PR (`ci/` is a + sensitive path and would drag in a human reviewer for a change unrelated to this one). +- **Runtime behaviour of any kind.** The gate proves the L0/L1 offline subset only; + `vars.CLUSTER_AVAILABLE` is unset, so L2/L3 conformance does not run in CI. diff --git a/schemas/README.md b/schemas/README.md index 6847b69..20bddc9 100644 --- a/schemas/README.md +++ b/schemas/README.md @@ -15,7 +15,7 @@ docs/13 exactly. Do not diverge from it here — escalate spec problems, don't f | `servicedefinition.schema.json` | Formal JSON Schema (draft 2020-12) for a ServiceDefinition. Enforces enums, required fields, patterns, and cross-field invariants. | | `renderer-contract.md` | Authoritative field → rendered-resource mapping, with requirement IDs. The contract other workstreams consume. | -**Schema version:** 1.0.0 (in the schema `description`). Treat any change as an interface change: +**Schema version:** 1.8.0 (in the schema `description`, which carries the full per-version changelog — this line had said `1.0.0` since 1.1.0 and is now synced). Treat any change as an interface change: propose on the task board, update dependent renderers/tests in the same change, never silently break a consumer (coordination-protocol §2). @@ -35,6 +35,9 @@ consumer (coordination-protocol §2). - `ingressRoutes[].host` must match `