Skip to content

Conversation

@fuweid
Copy link
Member

@fuweid fuweid commented Dec 5, 2024

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

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>
@dosubot dosubot bot added the area/cri Container Runtime Interface (CRI) label Dec 5, 2024
@fuweid fuweid added the cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch label Dec 6, 2024
Comment on lines +40 to +41
// 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].
Copy link
Member

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?)

Copy link
Member Author

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.

@fuweid fuweid added this pull request to the merge queue Dec 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@fuweid fuweid added this pull request to the merge queue Dec 12, 2024
Merged via the queue into containerd:main with commit 62232e9 Dec 12, 2024
59 checks passed
@fuweid fuweid deleted the fix-11091 branch December 12, 2024 03:34
@fuweid
Copy link
Member Author

fuweid commented Dec 12, 2024

/cherrypick release/2.0

@k8s-infra-cherrypick-robot

@fuweid: new pull request created: #11151

Details

In response to this:

/cherrypick release/2.0

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.

@dmcgowan dmcgowan added cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch and removed cherry-pick/2.0.x Change to be cherry picked to release/2.0 branch labels Dec 12, 2024
@rata
Copy link
Contributor

rata commented Jan 21, 2025

@fuweid thanks for solving this while I was away! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cri Container Runtime Interface (CRI) cherry-picked/2.0.x PR commits are cherry picked into the release/2.0 branch size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants