feat(nvml-mock): serve the rendered PCI sysfs tree and a DMI identity to Go consumers

[giuliocalzo]

Summary

The mock renders a PCI sysfs tree, but only libc consumers could read it: MOCK_PCI_ROOT works through the libpcimocksys.so LD_PRELOAD shim, and Go's os package issues openat directly, so the shim never sees the open and the process reads the node's real /sys. GPU Feature Discovery and the NVIDIA DRA driver are both Go. GFD therefore resolved each mock GPU's BDF from NVML, failed to read its vendor/class, and labelled the node nvidia.com/gpu.mode=unknown.

• sys/devices and sys/bus/pci/devices are bind-mounted read-only onto the kernel paths, in both channels that serve containers: the CDI spec setup.sh generates (which is how GFD already receives the mock libnvidia-ml.so.1) and the NRI plugin's adjustment. They go together — the PCI entries are relative symlinks into ../../../devices/pciDDDD:BB, so mounting one alone yields entries that list but whose every attribute read returns ENOENT, which looks identical to no mount at all.
• The renderer mirrors the node's own product_name and product_uuid into sys/devices/virtual/dmi/id. This is mount-target compatibility, not a machine-type mock: shadowing /sys/devices replaces the directory /sys/class/dmi/id resolves into, and kind's mount-product-files.sh createContainer hook bind-mounts the node's product files there for every container. mount(8) cannot create a target on a read-only sysfs, so without the mirror every served pod died with mount: .../sys/class/dmi/id/product_uuid: mount point does not exist — on Linux only, since Docker Desktop's linuxkit VM exposes no DMI and kind's hook guards on the host kernel having it.
• The PCI render moves ahead of CDI spec generation, because a bind mount whose source is missing fails container creation for the whole pod.

There is no opt-out. A node running nvml-mock is simulating GPU hardware, and serving the tree is what makes that simulation visible to sysfs consumers; a switch that turns it half off only adds a way to end up with gpu.mode=unknown for a reason nothing reports. Workloads that need the host's real device tree on an NRI-injected node belong in nri.excludedNamespaces.

One read-only-sysfs constraint shapes the rest of the design, and it is worth stating because it rules out the obvious alternative. /sys/devices has to be mounted whole: narrowing it to the profile's root complexes needs a mountpoint the runtime cannot create, and it fails hard rather than degrading —

error mounting "/var/lib/nvml-mock/sys/devices/pci0000:00" to rootfs at "/sys/devices/pci0000:00":
make mountpoint "/sys/devices/pci0000:00": ... read-only file system

which profiles routinely trigger (gb200 renders four root complexes on a node whose /sys/devices has none). That mount hides the host's other device classes, CPU topology among them, from served containers.

Fixes #673.

Not in scope: nvidia.com/gpu.machine. Mocking the machine type turned out to have a cause of its own — kind serves its own product_name at GFD's default machine-type file in every container, so no value the renderer writes there survives — and it is tracked separately in #681.

Test plan

• TestRender_MirrorsKernelDMI, TestRender_StandsInForUnreadableDMI, TestRender_NoDMIWithoutKernelDMI — the mirrored attributes, the empty stand-in for product_uuid (mode 0400, unreadable to a non-root renderer but still needed as a mount target), and no DMI directory at all on a kernel that exposes none.
• TestAdjustMountsPCISysfsWhenStaged / ...SkipsPCISysfsMountsWhenNotStaged / ...SkipsPCIDevicesMountWithoutSysDevices — the pair is emitted together, and skipped fail-open on a node the DaemonSet has not staged yet (a bind mount with a missing source would fail container creation for the whole pod).
• e2e: the gpu-operator scenario reads the tree from inside the GFD container at the kernel paths — device count equal to the profile's GPUs, and a vendor read that only resolves through the symlink into the mounted /sys/devices.
• make test — 33 packages pass. make helm-tests — 169 tests, 15 snapshots. make lint-fix — no violations (govulncheck reports pre-existing Go stdlib advisories from the local toolchain, unrelated).
• Live on a kind cluster (gb200 profile, GPU Operator in CDI mode, rebased branch): every operator pod Running with 0 restarts, nvidia.com/gpu.mode=compute where it previously read unknown, gpu.product=NVIDIA-GB200 with 8 allocatable GPUs. Inside the gpu-feature-discovery container, /sys/bus/pci/devices lists 8 entries, the first symlinking to ../../../devices/pci0000:00/0000:0a:00.0, which reads vendor 0x10de, class 0x030200, numa_node 0 — the class being what GFD turns into mode=compute, so the label is derived from the mounted tree rather than coincidence. That host is linuxkit, so the mirror correctly writes nothing and gpu.machine stays unknown ([Bug]: nvidia.com/gpu.machine cannot be mocked under kind — the node image shadows /sys/class/dmi/id/product_name #681); the Linux legs in CI cover the kind-hook path that the mirror exists for.

[ArangoGutierrez]

Nice piece of debugging, and the writeup makes the root cause easy to follow: the LD_PRELOAD shim can't see Go's openat, so only a real mount at the kernel path works. The renderer split into renderTopology/renderDMI is clean, the fail-open guards on both the CDI and NRI sides are the right instinct, and the e2e that reads vendor through the symlink from inside the GFD container is exactly the assertion that distinguishes delivery from coincidence. I also like that renderDMI mirrors rather than mocks, and says so. The one thing I'd want settled before merge is the blast radius of the NRI mount: it is node-wide where the CDI path is opt-in by construction, and dropping nri.pciSysfsMounts in 853cea2 removed the only switch for it.

1. The gate checks that the two directories exist, not that this run rendered them, and the renderer only adds entries. Re-profiling a node (a100 and h100 share no BDFs) leaves both sets under $HOST/sys, the gate still flips on, and the union is what gets bind-mounted at /sys/bus/pci/devices. The missing prune predates this PR, but mounting the tree at the kernel path is what makes it visible, and the exact-count assertion in PCISysfsAtKernelPath would fail on such a node. (deployments/nvml-mock/scripts/setup.sh:140)
2. While you're rewriting the row above it, this one is now stale too: the renderer writes numa_node for every device, and it reaches served containers through the same mount, so the device plugin does get NUMA hints there. Still accurate for containers the mock doesn't serve, so it probably wants the same qualifier the row above gained. (deployments/nvml-mock/helm/nvml-mock/README.md:1409)

[giuliocalzo]

Thanks — all five actionable points are addressed in ffeef77 and 2c4b3dd, and I answered the blast-radius question in its thread (short version: gating on nri.deviceAnnotation would drop the mount for GFD, which reads sysfs and requests no device, so NRI-only deployments would go back to gpu.mode=unknown; the per-pod nvml-mock.nvidia.com/inject: "false" opt-out already covers the workload that wants the host's tree, and the README now says so).

On the two numbered points:

1. Fixed, in the renderer rather than in the gate. render.Render prunes before it writes, so cmd/render-pci-sysfs callers get it too and not just setup.sh. TestRender_PrunesStaleDevices renders one profile over another and asserts the previous root complex and symlink are gone. The gate itself now keys on the completion marker, which also answers "did this run render it" rather than "does a directory exist".
2. Fixed — the numa_node row gained the same qualifier, and says the served device plugin does get the hints.

make test (33 packages), make helm-tests (169 tests, 15 snapshots) and golangci-lint run ./... (0 issues) all pass locally; make lint-fix still reports the pre-existing Go stdlib advisories from my local toolchain (1.26.5 vs 1.26.6), unrelated to this branch.

[roma-glushko]

So since we are doing mounting of sys directories, we don't need to use the previous way of mocking and we can remove it? Mounting should work for both type of consumers right?

[giuliocalzo]

Sorry for the slow reply — these landed while I was answering the other thread.

For the consumers this PR is about, yes: a Go consumer in a container the mock serves now reads the real thing at the kernel path, and the shim adds nothing there. But the mount cannot replace the shim in general, because a bind mount needs a target that already exists — mount(8) cannot create one on a read-only sysfs, which is why this PR mounts /sys/devices whole instead of just the profile's root complexes.

That rules the mount out wherever the path is absent on the node:

• /sys/class/infiniband and /sys/class/infiniband_verbs do not exist on a node with no IB hardware and no ib_core, so the IB tree is reachable only through libibmocksys. Same for /dev/infiniband.
• /sys/class/dmi is absent on Docker Desktop's linuxkit VM, which is why this PR mirrors DMI into the tree rather than relying on the kernel's copy.

And it only reaches containers one of the two channels serves. The nvml-mock DaemonSet is never self-injected, so lspci in its own pod works through LD_PRELOAD + MOCK_PCI_ROOT (set in daemonset.yaml), as does anything in a pod that opted out with nvml-mock.nvidia.com/inject: "false" or lives in an excluded namespace.

So the two mechanisms answer different questions: the shim covers any libc consumer anywhere on the node, at any path; the mount covers the paths Go consumers read with raw syscalls, in the containers we serve.

[roma-glushko]

This means that we should take the real node's devices file and "expand" it with simGPU devices in order to produce /var/lib/nvml-mock/sys/devices. This way all devices should be there.

This should work even if host devices file is not static. We could fnotify watch it and mirror in our produced devices file.

[giuliocalzo]

I dug into this and filed #689 for it, because it deserves more than a paragraph here — but I do not think mirroring is the shape that works, for two reasons.

Copying gives frozen values. Sysfs attributes are kernel-backed reads, not files: scaling_cur_freq, thermal zones and every counter change between the read and the consumer's. A mirrored /sys/devices/system/cpu would look right and be stale, which is worse than a missing directory that fails loudly. Live mirroring would have to bind-mount each host entry into our tree rather than copy it — doable from a privileged DaemonSet, but then it is mounts all the way down, and inotify on sysfs does not reliably report attribute changes, so the watch would not be the part that works.

The harder problem is that no mirror of the node's tree can be right. /sys/class/net/<iface> resolves into devices/virtual/net, and what belongs there is the pod's own net namespace view — the kernel populates it per netns at mount time. The node's copy holds the host's interfaces, so mirroring it hands the container someone else's network.

So the way out is to stop shadowing /sys/devices at all. If the renderer emitted symlink targets that escape sysfs into the overlay we already mount — /sys/bus/pci/devices/0000:07:00.0 -> ../../../../opt/nvml-mock/sys/devices/pci0000:00/0000:07:00.0 — then only /sys/bus/pci/devices needs mounting, a path every node has, and CPU topology, DMI and the pod's namespaced sysfs are all left alone. That is #689; the open question is whether the DRA driver's deviceattribute still finds the PCIe root in a path that does not start with /sys/devices.

[ArangoGutierrez]

Nothing here blocks merge.

The marker protocol is the right shape. Writing sys/.rendered last and gating both channels on it makes "this render finished" distinguishable from "the directories exist", and pruneTree keeping the two mounted directories and virtual/dmi/id in place is what lets a CDI-served container survive a re-render it has no way to wait for. product_uuid as an empty stand-in and product_name mirrored rather than mocked both land, and the new render tests each name a bug they would catch: the prune test goes red on the old additive renderer, the DMI tests on a mount target that stops existing. Moving the render to step 3b checks out too, since nothing between there and the old step 10 writes under $HOST/sys, and the renderer reads the ConfigMap at /etc/nvml-mock/config.yaml rather than the copy step 5 makes. Where the protocol still has a hole is the one path that never reaches the renderer at all.

1. The Output godoc still says Render is a no-op when Topology is nil or has no root complexes; that path now prunes the tree and deletes the marker whenever Output is set. (pkg/system/mockpcisysfs/render/render.go:55)
2. MarkerRelPath is exported from render for exactly this seam, but these two path constants are re-declared here and staged with their own literals in adjust_test.go, so a rename on the renderer side would leave pciSysfsMounts statting a path that no longer exists and failing open silently, with nothing red. (pkg/nri/nvmlmock/adjust.go:50)

[ArangoGutierrez]

main.go returns at its topo == nil check before Render is ever called, and EffectiveTopology is nil whenever no device declares a pci.bus_id, which gpu.customConfig makes reachable, so this prune never runs on the production path. Rendering a100 and then such a config into the same output directory leaves all eight A100 symlinks and sys/.rendered in place, so the gate in setup.sh flips PCI_SYSFS_RENDERED=on and both channels mount the previous profile's tree at the kernel paths; calling Render unconditionally from main.go, or clearing the marker on that early return, closes it.

Also at deployments/nvml-mock/scripts/setup.sh:146.

[giuliocalzo]

Correct, and this was the hole worth having found: the prune shipped in a commit whose own entry point could not reach it on that path. Fixed in 6b827e3.

render-pci-sysfs now calls Render unconditionally — a nil topology is already "empty the tree" to the renderer, so the command has no reason to decide that for itself — and --dry-run is the only short-circuit left. main is split into a run() so the case that regressed has a test: TestRun_ClearsTreeWhenProfileDeclaresNoDevices renders a profile with a bus_id and then one without, and asserts both the devices and the marker are gone. Against the early return it fails with Should be empty, but was [L 0000:07:00.0], which is the a100-then-customConfig sequence you described.

setup.sh's marker comment now also says the renderer removes the marker for a profile that renders nothing, since that is what makes the gate at :146 correct rather than the gate itself doing anything new.

[ArangoGutierrez]

A terminating pod keeps status.phase: Running until its containers exit, deletionTimestamp is what marks it, and the objectMeta this decodes into carries only name, labels and annotations, so the Terminating half of the doc above is not enforced and a GFD rollout can still hand back the pod that is going away. The call site resolves it once with no Eventually either, unlike waitOperatorValidatorRunning and the dcgm assertion, which poll because the operator replaces its operands a reconcile after nvml-mock rolls (#602).

[giuliocalzo]

Both true — fixed in 9974d6f. My doc claimed a filter the type could not express: objectMeta now decodes deletionTimestamp, and the pod has to be phase Running and not have one, so the pod on its way out is skipped rather than exec'd into.

The call site polls too, with ReadyTimeout/PollInterval, in the same shape as waitOperatorValidatorRunning. Resolving once assumed a steady state that #602 says does not hold here: the operator replaces its operands a reconcile after nvml-mock rolls, so there is a window with no candidate at all, and skipping the terminating pod turns that window from a failed exec into an empty result — which only helps if the caller waits it out.

[giuliocalzo]

All four addressed. The two you numbered:

1. Options.Output no longer claims Render is a no-op without a topology; it says that path empties the tree and drops the marker, which is what it has done since the prune landed (047d6e8).
2. PCIDevicesRelPath and SysDevicesRelPath are exported from render now and the NRI guard takes both from there, as it already did for MarkerRelPath. adjust_test.go stages from the same constants while the mount assertions keep their literals, so renaming a path in the renderer goes red on the assertion instead of silently failing open (047d6e8).

And the two inline: the topo == nil early return is gone, with a test that reproduces the a100-then-customConfig sequence (6b827e3), and the GFD pod lookup now skips terminating pods and polls (9974d6f).

I also answered @roma-glushko's two questions from yesterday, which I had missed. The second one — mirroring the node's /sys/devices into ours — turned into #689: mirroring cannot be made correct (a container's devices/virtual/net is netns-scoped, so the node's copy is the wrong network), but pointing the PCI symlinks outside sysfs into the overlay would mean never mounting /sys/devices at all, which dissolves the blast-radius question in your first review rather than trading it off. Not for this PR — it needs verifying against deviceattribute first.

make test, golangci-lint run ./... (0 issues) and make helm-tests (169 tests, 15 snapshots) pass locally.

[roma-glushko]

I also answered @roma-glushko's two questions from yesterday, which I had missed. The second one — mirroring the node's /sys/devices into ours — turned into #689: mirroring cannot be made correct (a container's devices/virtual/net is netns-scoped, so the node's copy is the wrong network), but pointing the PCI symlinks outside sysfs into the overlay would mean never mounting /sys/devices at all, which dissolves the blast-radius question in your first review rather than trading it off. Not for this PR — it needs verifying against deviceattribute first.

@giuliocalzo make sense to move this question out of scope, but I would think a bit more about what we can do there.