Prefer runtime options for PluginInfo request #11442
Conversation
Previously, PluginInfo was called with task options as the primary value, resulting in opts.BinaryName being omitted. Consequently, the containerd-shim-runc-v2 fell back to the system's runc binary in the PATH rather than the explicitly specified one. This change inverts the option fallback by preferring runtime options over task options, ensuring the correct binary is used for the PluginInfo request. Closes: containerd#11169 Signed-off-by: Jose Fernandez <[email protected]> Reviewed-by: Erikson Tung <[email protected]>
|
Hi @jfernandez. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/ok-to-test |
|
/retest |
AkihiroSuda
added
the
cherry-pick/2.0.x
|
Would it be possible to have an integration test? |
Could you give me a code hint about where you'd drop the integration test for this? Or should I create a new file? |
|
@AkihiroSuda, I attempted to write an integration test, but I'm facing a challenge. To verify the behavior, I need to set up the following in the test:
I'm not entirely sure how to accomplish steps 1 and 2. If you have any suggestions for a simpler way to test this, please let me know. Otherwise, I would prefer to proceed with the PR as is, and then circle back to add better test coverage to this part of the codebase. |
|
/cherry-pick release/2.0 |
|
@AkihiroSuda: new pull request created: #11446 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
thaJeztah
added
cherry-picked/2.0.x
Previously, PluginInfo was called with task options as the primary value, resulting in opts.BinaryName being omitted. Consequently, the containerd-shim-runc-v2 fell back to the system's runc binary in the PATH rather than the explicitly specified one. This change inverts the option fallback by preferring runtime options over task options, ensuring the correct binary is used for the PluginInfo request.
At Netflix, we maintain runc in both /usr/bin (which lacks support for user namespaces) and at a custom path. This issue surfaced when enabling user namespaces and encountering the error:
Although the
featurescommand for runc at our custom path confirmed that mountExtensions.idmap was enabled, the issue arose because opts.BinaryName was blank in the shim code (see:https://github.com/containerd/containerd/blob/main/cmd/containerd-shim-runc-v2/manager/manager_linux.go#L345), triggering a fallback to the default runc binary at /usr/bin.
Closes: #11169