fix(node-agent): serve the mock PCI tree at the kernel paths - #742
Open
giuliocalzo wants to merge 9 commits into
Open
fix(node-agent): serve the mock PCI tree at the kernel paths#742giuliocalzo wants to merge 9 commits into
giuliocalzo wants to merge 9 commits into
Conversation
giuliocalzo
marked this pull request as draft
August 27, 2026 12:02
Greptile SummaryThe change serves the simulated PCI and machine-type data at paths used by Go consumers.
Reviews (8): Last reviewed commit: "Merge branch 'main' into feat/issue-673-..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
giuliocalzo
force-pushed
the
feat/issue-673-agent-pci-sysfs
branch
from
August 27, 2026 12:55
1f38fba to
05df40c
Compare
giuliocalzo
marked this pull request as ready for review
August 27, 2026 12:59
roma-glushko
previously approved these changes
Aug 27, 2026
giuliocalzo
dismissed stale reviews from roma-glushko and ArangoGutierrez
via
August 27, 2026 15:01
02e1033
The pcibus simulator renders the tree into the agent's overlay, where libpcisysfs.so redirects reads of /sys/bus/pci/devices to it. 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 resolves each GPU's BDF from NVML and then reads its class from sysfs, so it labelled every mock node nvidia.com/gpu.mode=unknown; the DRA driver leaves dra.k8s.io/pcieRoot off its ResourceSlices for the same reason. The cdi simulator now bind-mounts the rendered tree read-only at the kernel paths in the nvidia.com CDI spec, which is the channel that already delivers the mock libnvidia-ml.so.1 to the operands. Both entries go together: the lookup directory holds relative symlinks into ../../../devices/pciDDDD:BB, so serving it alone yields entries that list and whose every attribute read returns ENOENT. They are emitted only when the state declares a topology, matching the condition pcibus renders on, because a mount with a missing source fails container creation for the whole pod. /sys/devices is served whole — narrowing it to a profile's root complexes needs mountpoints the runtime cannot create on a read-only sysfs, and profiles routinely declare roots the node does not have. A served container therefore no longer sees the host's other device classes; NVIDIA#689 tracks that. Shadowing that directory also takes /sys/class/dmi/id with it, so pcibus reproduces the attributes kind's createContainer hook bind-mounts, or every served pod dies on a missing mount target. That is target compatibility only, not a machine-type mock (NVIDIA#681), and needs the node's /sys on the agent. Renders now converge on an empty topology instead of returning early, so a config that declares no bus_id cannot leave the previous profile's GPUs mounted at the kernel paths. Fixes NVIDIA#673 Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
nvidia.com/gpu.machine read unknown on every mock node. GFD derives it from --machine-type-file, and its default /sys/class/dmi/id/product_name is a path no mock can own under kind: the node image writes "kind" there and re-binds it into every container after the container's own mounts are set up, while hosts without DMI have no such path at all. The agent writes the machine type to driver/config/machine-type, which the CDI config mount already serves at /etc/nvml-mock, and the chart's GPU Operator values point GFD_MACHINE_TYPE_FILE there. CDI's own env edits are not a usable channel: the runtime applies the spec's mounts but drops its env, so nothing in containerEdits.env reaches a served container today. The value is the profile's GPU product name for want of a platform name in the profiles, so the label reads NVIDIA-GB300-NVL, matching gpu.product, rather than the NVIDIA-GB300-NVL72 a real compute tray reports. Also compacts the comments added for NVIDIA#673, which had grown to restate the issue rather than record intent. Fixes NVIDIA#681 Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Nothing in that container reads it. It is there so a `kubectl exec` can compare the node's own sysfs against the tree the agent renders beside it, which is the first thing worth checking when a consumer resolves the wrong devices. Read-only, like the node-agent's copy. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
…le's pcie_topology is compiled from the profile whole while GPU_COUNT truncates the device list, so a capped node rendered BDFs with no NVML device behind them. Every attribute of those entries is plausible — vendor falls back to 0x10de and the class to a 3D controller — which was harmless only while nothing could read the tree, and stopped being harmless when it started being served at the kernel paths. Reconcile the layout against the devices instead, in one place both the renderer and the mount gate read: declared BDFs no device claims are dropped along with any root they leave empty, and a device no root claims is adopted by the first, since an unplaced GPU cannot be resolved at all where a misplaced one only misreports its pcieRoot. HasPCITopology now answers from that same reconciliation, so the predicate cannot drift from what gets rendered. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
Review feedback: pointing GFD at the mock's machine-type file is simulation logic, not something to ask an operator's values file for. The NRI plugin already injects the overlay's config path the same way, so add GFD_MACHINE_TYPE_FILE beside it as a default a workload can still override. The overlays keep the override, because they drive the CDI path where the runtime applies the spec's mounts and drops its env. Also drop the /host/sys mount from the mock container, which was a debugging aid nothing there reads, and stop the DMI comments reading as if kind were the only deployment target: the reproduction keeps DMI attributes readable on any cluster, and only the mount-target failure is kind's. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The dropped containerEdits.env is now NVIDIA#747, so the operator-values overrides and the docs say which defect they are working around. The spec's env block carries the warning too: its values are correct but the channel does nothing, so nothing new should be built on it. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
A device whose BDF the profile's pcie_topology omits was appended to the first declared root, which handed the DRA driver and the device plugin a specific PCIe root and NUMA node for a GPU the profile never placed. Locality is the thing that tree gets read for, and a confident wrong answer there is worse than none: it cannot be re-derived, and it silently misgroups GPUs. Such a device now renders under the root its own address implies, joining a declared root only where the address names it, and otherwise reporting numa_node -1 — what Linux writes for a device it has no proximity information for, so consumers read "unknown" in the encoding they already handle. It is still rendered, because a GPU missing from the tree is one no consumer can resolve from the BDF NVML hands it, which is the failure this path exists to fix. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
The comments said "the runtime" drops containerEdits.env, which reads as CDI env being useless everywhere and contradicts a passing e2e assertion: the nri-cdi-inject leg proves NVML_MOCK_DEVICE_SOURCE arrives from the NRI spec's containerEdits when containerd resolves it natively, and MEP-0002 records the same measured on stock kind. What drops env is the toolkit resolving nvidia.com/gpu, so say that — it points whoever picks up NVIDIA#747 at the right component instead of the runtime that demonstrably works. Signed-off-by: Giulio Calzolari <gcalzolari@nvidia.com>
giuliocalzo
force-pushed
the
feat/issue-673-agent-pci-sysfs
branch
from
August 28, 2026 08:24
6d246d9 to
4d84de7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What This PR Does
Makes the simulated PCI tree and machine type visible to Go consumers, so a mock node labels itself the way a real one does.
nvidia.com/gpuCDI spec bind-mountssys/devicesandsys/bus/pci/devicesread-only, as a pair. Fixesnvidia.com/gpu.mode=unknown./sys/devicesreplaces what/sys/class/dmi/idresolves into, so without this a container readsENOENTwhere the node has values — and underkind, whose node image bind-mounts its product files into every container, the pod fails to start outright.gpu.countcaps the device list without touchingpcie_topology, so a capped node used to render PCI entries that read as GPUs with no NVML device behind them.nvidia.com/gpu.machine=unknown.Trade-off:
/sys/devicesis served whole, so a served container no longer sees the host's other device classes, CPU topology among them. Narrowing it needs mountpoints the runtime cannot create on a read-only sysfs; #689 tracks removing it.Known gap: the CDI path still needs
GFD_MACHINE_TYPE_FILEin the operator's values, because the runtime applies the spec's mounts but drops its env — filed as #747. NRI needs no override.Why
Fixes #673.
pcibusrendered the tree into the agent's overlay, reachable only through thelibpcisysfs.soLD_PRELOADshim. That covers libc consumers likelspci, but Go bypasses it —os.Openissuesopenatdirectly — so GPU Feature Discovery and the NVIDIA DRA driver read the node's real/sysand found no mock GPUs. GFD resolved each GPU's BDF from NVML, failed to read its class, and labelled the nodenvidia.com/gpu.mode=unknown; the DRA driver omitteddra.k8s.io/pcieRootfor the same reason.Fixes #681.
nvidia.com/gpu.machinereadunknownfor a separate reason: GFD's default source for it is a DMI path no mock can own underkind, and one that does not exist at all on hosts without DMI.Checklist
git commit -s)go test -v -race ./...) — plusmake helm-tests(175 tests, 15 snapshots) andgo vetincluding under the e2e build tagmake lint-fix) —golangci-lintreports 0 issues; the target'sgovulncheckstage fails on Go 1.26.5 stdlib advisories fixed in 1.26.6, pre-existing and reached only through code this PR does not touchdocs/helm-chart.mdVerified with
tilt ci -- --gpu-operator --gpu-profile gb300onkind: both workers reachgpu.mode=computeandgpu.machine=NVIDIA-GB300-NVL, and all four GPUs resolve at the kernel paths from inside a served container. Not verified locally:kind's DMI mount-target hook, which cannot fire on Docker Desktop (no DMI) and is covered by Linux CI; anddra.k8s.io/pcieRoot, which needs a--drarun.