Skip to content
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

[release/1.7] Include userns info in PodSandboxStatus #9865

Conversation

rata
Copy link
Contributor

@rata rata commented Feb 23, 2024

This is a backport of #9782. It applies cleanly, but the cherry-picks have been amended to do the change in cri/server and cri/sbserver.


We added support for userns but we weren't showing it in the
PodSandboxStatus.

Let's just show the whole nsOpts, so we don't forget in the future
either if something else inside there changes.

Please note that this will expose the content of nsOpts.TargetId that we
weren't exposing before. But that seemed like a bug to me.

@k8s-ci-robot
Copy link

Hi @rata. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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/test-infra repository.

@dmcgowan
Copy link
Member

Looks like you have the change in sbserver but the tests still in server. Is the intent to make the change in both server and sbserver or just have the tests updated in sbserver?

@rata rata force-pushed the rata/userns-update-podSandboxStatus-1.7 branch from 792dd49 to 396a853 Compare February 27, 2024 14:24
@rata
Copy link
Contributor Author

rata commented Feb 27, 2024

@dmcgowan haha, sorry! I realized about that, fixed it, and then forgot to push 🙈. Pushed now (validated locally in a cluster too :))

@rata rata force-pushed the rata/userns-update-podSandboxStatus-1.7 branch 2 times, most recently from 3240524 to 462b59c Compare February 28, 2024 02:39
We added support for userns but we weren't showing it in the
podSandboxStatus.

Let's just show the whole nsOpts, so we don't forget in the future
either if something else inside there changes.

Please note that this will expose the content of nsOpts.TargetId that we
weren't exposing before. But that seemed like a bug to me.

The cherry-pick has been amended to do the change in cri/sbserver and
cri/server.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
(cherry picked from commit 6c356a5)
@rata rata force-pushed the rata/userns-update-podSandboxStatus-1.7 branch 2 times, most recently from 9142d8d to e78f153 Compare February 28, 2024 15:15
The cherry-pick has been amended to duplicate the tests in cri/sbserver and
cri/server.

Signed-off-by: Rodrigo Campos <rodrigoca@microsoft.com>
(cherry picked from commit 06ed897)
@rata rata force-pushed the rata/userns-update-podSandboxStatus-1.7 branch from e78f153 to b57dc9f Compare February 28, 2024 18:36
@dmcgowan dmcgowan merged commit c37fe74 into containerd:release/1.7 Apr 22, 2024
54 checks passed
@dmcgowan dmcgowan added the area/cri Container Runtime Interface (CRI) label Apr 25, 2024
@dmcgowan dmcgowan changed the title [release/1.7] Include userns info in cri/server PodSandboxStatus [release/1.7] Include userns info in PodSandboxStatus Apr 25, 2024
@rata rata deleted the rata/userns-update-podSandboxStatus-1.7 branch May 3, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants