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

Add support for CRI-O user namespaces #8268

Merged
merged 2 commits into from
Dec 20, 2021

Conversation

nmasse-itix
Copy link
Contributor

@nmasse-itix nmasse-itix commented Dec 3, 2021

This Pull Request is about adding support for user namespaces for CRI-O.

Description

Support for user namespaces in CRI-O has been introduced with cri-o/cri-o#3944 and refined in cri-o/cri-o#4281.

As explained in cri-o/cri-o#4281, there is a configuration option in crio.conf to enable this feature.

add allowed_annotations option to runtime handler structure, which allows admins
to gate which runtime classes interpret the annotation io.kubernetes.cri-o.userns-mode.
In doing so, also drop the experimental allow_userns_annotation option.

This PR does two things:

  • it modifies /etc/crio/crio.conf to add the allowed_annotations directive.
  • it adds an entry to /etc/subuid and /etc/subgid files.

This behavior is controlled by the crio_runtimes[*].allowed_annotations variable (default value: empty) and the crio_remap_enable variable (default value: false).

The default values introduce no change to existing deployments.

Documentation

cri-o.md has been updated.

Testing

In your group_vars/all/crio.yaml, add:

crio_runtimes:
  - name: runc
    path: /usr/bin/runc
    type: oci
    root: /run/runc
    allowed_annotations:
    - "io.kubernetes.cri-o.userns-mode"

crio_remap_enable: true

Then, create a pod like this:

apiVersion: v1
kind: Pod
metadata:
  generateName: test-
  labels:
    test-pod: "true"
  annotations:
    io.kubernetes.cri-o.userns-mode: "auto:keep-id=true"
spec:
  containers:
  - name: alpine
    image: docker.io/library/alpine:latest
    command:
    - /bin/sleep
    - "3600"
    securityContext:
      runAsUser: 1000
      runAsGroup: 1000
  restartPolicy: OnFailure

On the Kubernetes node running this pod, use the lsns command to list user namespaces.
In the following example, I created two pods that have their own namespace and their own uid/gid space.

[root@kube-cicd ~]# lsns -t user
        NS TYPE  NPROCS   PID USER       COMMAND
4026531837 user     180     1 root       /usr/lib/systemd/systemd --switched-root --system --
4026532579 user       1 70662 2130772967 /bin/sleep 3600
4026532658 user       1 72674 2130838503 /bin/sleep 3600

Cleanup with:

kubectl delete pod -l test-pod=true

Tested configurations

  • Kubernetes 1.22
  • CentOS Stream 8
  • CRI-O 1.21 and 1.22

Caveats

There is packaging glitch with CRI-O 1.22 (containers/common#789) which should be fixed later. In the meantime, I had to comment the [machine] section in /usr/share/containers/containers.conf.

Also, with CRI-O 1.22, I could not manage to have the "map-to-root" feature working (io.kubernetes.cri-o.userns-mode: "auto:map-to-root=true") while it was working on CRI-O 1.21

[cri-o] Add support for cri-o user namespaces

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Dec 3, 2021
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
  • If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
  • Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

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. I understand the commands that are listed here.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 3, 2021

CLA Signed

The committers are authorized under a signed CLA.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Dec 3, 2021
@k8s-ci-robot
Copy link
Contributor

Welcome @nmasse-itix!

It looks like this is your first PR to kubernetes-sigs/kubespray 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubespray has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

@k8s-ci-robot
Copy link
Contributor

Hi @nmasse-itix. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 3, 2021
@cristicalin
Copy link
Contributor

cristicalin commented Dec 3, 2021

Hi @nmasse-itix , awsome work!

Please sign the CLA so we can accept this contribution: https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/20677499/43613404/8268/#/?version=2

Also, please address the linting issues identified by the CI.

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 3, 2021
@nmasse-itix
Copy link
Contributor Author

Thanks for the warm welcome !

I signed the CLA a couple of minutes ago and I fixed the issues pointed by yamllint.

@floryut
Copy link
Member

floryut commented Dec 7, 2021

/check-cla

@nmasse-itix hum strange I think I saw your name somewhere while working for Credit Mutuel / EID with Openshift or 3scale or another RH product 😉

@nmasse-itix
Copy link
Contributor Author

Definitely possible... 😀

@nmasse-itix
Copy link
Contributor Author

The CI tests are failing because of a timeout with packer. And the CLA is now signed.

/retest
/check-cla

@floryut
Copy link
Member

floryut commented Dec 8, 2021

The CI tests are failing because of a timeout with packer. And the CLA is now signed.

/retest /check-cla

I've retried the debian10 job, weird that the CLA bot isn't picking up your email signing

@floryut
Copy link
Member

floryut commented Dec 8, 2021

@nmasse-itix Ok I think you need to rebase master, as you are encountering the calico checksum ninja change done by the calico team and fixed by @cristicalin a few days ago.

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 8, 2021
@nmasse-itix nmasse-itix changed the base branch from master to release-2.17 December 8, 2021 15:19
@nmasse-itix nmasse-itix changed the base branch from release-2.17 to master December 8, 2021 15:19
@nmasse-itix nmasse-itix changed the base branch from master to release-2.17 December 8, 2021 15:19
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2021
@nmasse-itix nmasse-itix changed the base branch from release-2.17 to master December 8, 2021 15:20
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 8, 2021
@nmasse-itix
Copy link
Contributor Author

My rebase went wrong. Let me try to fix this.
/hold

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 8, 2021
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 20, 2021
@nmasse-itix
Copy link
Contributor Author

Rebase should be ok now.
/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 20, 2021
@cristicalin
Copy link
Contributor

@nmasse-itix thanks for this.

There is still the matter of the linuxfoundation CLA , would you mind signing it so we can accept the patch set ?

@nmasse-itix
Copy link
Contributor Author

I signed the CLA on December, the 7th.

CLA SignedThe committers are authorized under a signed CLA.

* white_check_mark  Nicolas MASSE ([31b9898](https://github.com/kubernetes-sigs/kubespray/commit/31b989826da963adb21221667696817be3a41f60))

But somehow the bot does not want to refresh the status...

@cristicalin
Copy link
Contributor

That is a different CLA, one which you had already signed. Please check this: https://git.k8s.io/community/CLA.md#the-contributor-license-agreement

@nmasse-itix
Copy link
Contributor Author

ok, sorry for the misunderstanding.

How can I ask the bot to recheck ? /check-cla ?

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 20, 2021
@nmasse-itix
Copy link
Contributor Author

yes, that's much better now !

@cristicalin
Copy link
Contributor

👍

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 20, 2021
Copy link
Member

@floryut floryut left a comment

Choose a reason for hiding this comment

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

@nmasse-itix Thank you, great contrib 👍

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: floryut, nmasse-itix

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2021
@floryut floryut added kind/container-managers Containers section in the release note and removed kind/feature Categorizes issue or PR as related to a new feature. labels Dec 20, 2021
@k8s-ci-robot k8s-ci-robot merged commit f01f7c5 into kubernetes-sigs:master Dec 20, 2021
@floryut floryut mentioned this pull request Dec 21, 2021
sakuraiyuta pushed a commit to sakuraiyuta/kubespray that referenced this pull request Apr 16, 2022
* add support for cri-o user namespaces

* comply with yamllint rules
LuckySB pushed a commit to southbridgeio/kubespray that referenced this pull request Jun 28, 2023
* add support for cri-o user namespaces

* comply with yamllint rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/container-managers Containers section in the release note lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants