Skip to content

adaptation: fix silently dropped plugin pids limit updates - #339

Merged
mikebrow merged 1 commit into
containerd:mainfrom
pandaWall:main
Oct 29, 2026
Merged

mikebrow merged 1 commit into
containerd:mainfrom
pandaWall:main

Conversation

@pandaWall

Copy link
Copy Markdown
Contributor

adaptation: fix silently dropped plugin pids limit updates

updateResources() merges each resource field of a plugin's ContainerUpdate into a copy of the container's current resources and writes the merged copy back to both the request and the reply. Every field takes its new value from the plugin's update, u.Linux.Resources, except the pids limit. That block was copied from the create-path adjustResources(), where the variable of the same name holds the plugin's resources, so in updateResources() it ended up reading the merged copy instead of the update. Consequently:

  • if the update request carried no pids limit, the block was skipped entirely: the plugin's pids limit was silently dropped, no ownership of the field was recorded, and two plugins setting the same limit went undetected instead of conflicting, while

  • if the request carried one, the old value was silently kept, the plugin was still recorded as the owner, and any other plugin touching the pids limit then failed the whole request with a spurious conflict error.

Read the pids limit from the plugin's update, like all the other fields, and add regression tests covering pids limit updates relayed via UpdateContainer, the Update section of CreateContainer responses, and the conflict when two plugins set the pids limit.

adaptation: fix silently dropped plugin pids limit updates

updateResources() merges each resource field of a plugin's
ContainerUpdate into a copy of the container's current resources and
writes the merged copy back to both the request and the reply. Every
field takes its new value from the plugin's update, u.Linux.Resources,
except the pids limit. That block was copied from the create-path
adjustResources(), where the variable of the same name holds the
plugin's resources, so in updateResources() it ended up reading the
merged copy instead of the update. Consequently:

- if the update request carried no pids limit, the block was skipped
entirely: the plugin's pids limit was silently dropped, no
ownership of the field was recorded, and two plugins setting the
same limit went undetected instead of conflicting, while

- if the request carried one, the old value was silently kept, the
plugin was still recorded as the owner, and any other plugin
touching the pids limit then failed the whole request with a
spurious conflict error.

Read the pids limit from the plugin's update, like all the other
fields, and add regression tests covering pids limit updates relayed
via UpdateContainer, the Update section of CreateContainer responses,
and the conflict when two plugins set the pids limit.

Signed-off-by: pandaWall <mangosteen922@126.com>

@klihub klihub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for spotting and fixing this. LTGM.

@mikebrow mikebrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mikebrow
mikebrow merged commit 6ea9f20 into containerd:main Oct 29, 2026
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants