fix: scope Allocate env map to each container request - #196
Open
lexfrei wants to merge 1 commit into
Open
Conversation
lexfrei
marked this pull request as ready for review
July 20, 2026 01:19
AllocateRequest.ContainerRequests is a repeated field, but the env map holding the allocated PCI addresses was created once per Allocate call and reused for every entry in it, so each entry's addresses were appended to those of the entries processed before it. For requests naming devices 11 and 22, the second response's PCI_RESOURCE_NVIDIA_COM_* is emitted as "11,22" — it names a device belonging to a different entry. Upstream kubelet sends exactly one ContainerAllocateRequest per call and reads only ContainerResponses[0], so no released kubelet reaches this path. The repeated field is part of the public device plugin API and nothing in its contract forbids batching, so handle it correctly instead of depending on the caller sending a single entry. Move the map inside the per-container loop so a response advertises only the devices requested in its own entry. The vGPU plugin already scopes its env map this way. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <f@lex.la>
lexfrei
force-pushed
the
fix/allocate-envlist-per-container
branch
from
September 1, 2026 13:17
4c65c51 to
d66b8f3
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
1 participant
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.
envListwas created once perAllocate()call and reused for every entry inreqs.ContainerRequests, so each entry's PCI addresses were appended to those of the entries processed before it. For requests naming devices11and22, the second response'sPCI_RESOURCE_NVIDIA_COM_*is emitted as11,22— it names a device belonging to a different entry.Moving the declaration inside the per-container loop scopes it correctly.
GenericVGpuDevicePlugin.Allocate()already scopes its env map this way and is unaffected, so the generic plugin was the outlier. The declaration predates #191, which changed what entersdevAddrsbut not the scope ofenvList.This is latent in practice: upstream kubelet sends a single
ContainerAllocateRequestper call and reads onlyContainerResponses[0], so no released kubelet hits this path. The repeated field is part of the public device plugin API and nothing in its contract forbids batching, so the plugin should handle it correctly rather than depending on the caller sending one entry.Testing
New spec in
generic_device_plugin_test.gosends two container requests and asserts each response carries only its own device address and only its own/dev/vfio/<group>node. Verified it fails without the fix (entry 1 gets11,22) and passes with it; the device-node assertions additionally catch hoistingdeviceSpecs/seenDeviceSpecsout of the loop. Full suite 43/43, no newgo vetorgolangci-lintfindings against master.