-
Notifications
You must be signed in to change notification settings - Fork 3.8k
internal/cri: should not apply IoOwner options if it's not user namespace #11104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
When kubelet enables UserNamespaceSupport feature gate, kubelet always uses non-empty UsernsOptions to setup pods. In this case, the gVisor shim is unable to parse runc.Option so that it will be unable to start container. This change is to avoid adding IoOwner options if the UsernsOptions is for node level. Since gVisor hasn't feature subcommand yet, CRI status will report that gVisor runtime doesn't support user namespace. So it's kind of workaround to avoid compatible issue. REF: containerd#11091 Signed-off-by: Wei Fu <fuweid89@gmail.com>
| // However, gVisor runtime doesn't support runc.Options and no idea why | ||
| // adding options could breaks the sig-node conformance case [when querying /stats/summary should report resource usage through the stats api]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my understanding, would there be something that could (should?) also be addressed by the gVisor maintainers? (if so, did we raise it with them?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also be addressed by the gVisor maintainers?
After check gVisor code (like https://github.com/google/gvisor/blob/b78f2ee7c4c393990a84298dd0f200e927b18dab/pkg/shim/runsc/service.go#L213), I think gVisor project should address this compatible issue, even if it's kind of painful.
The main issue seems to be that containerd lacks abstract layer for configuring new task in shim. In containerd client SDK, new task always uses runc option by default. For the out-of-tree runtimes, it's unlikely to support them in CRI plugin if there is no common layer for them. It's weird to check runtime name, since it's not reliable. So, I use runc option only if pod enables pod-level user namespace. It's workaround.
Other option is that we can automatically adjust pipe owner. But it's plan B.
if so, did we raise it with them?
The issue reporter seems not to be in the gVisor team. I can tag gVisor team in that issue #11091.
|
/cherrypick release/2.0 |
|
@fuweid: new pull request created: #11151 DetailsIn 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. |
|
@fuweid thanks for solving this while I was away! :) |
When kubelet enables UserNamespaceSupport feature gate, kubelet always uses non-empty UsernsOptions to setup pods. In this case, the gVisor shim is unable to parse runc.Option so that it will be unable to start container.
This change is to avoid adding IoOwner options if the UsernsOptions is for node level. Since gVisor hasn't feature subcommand yet, CRI status will report that gVisor runtime doesn't support user namespace. So it's kind of workaround to avoid compatible issue.
REF: #11091