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

remove the docker cgroup driver detection #874

Closed
3 tasks done
neolit123 opened this issue May 30, 2018 · 22 comments
Closed
3 tasks done

remove the docker cgroup driver detection #874

neolit123 opened this issue May 30, 2018 · 22 comments
Assignees
Labels
area/UX lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.

Comments

@neolit123
Copy link
Member

neolit123 commented May 30, 2018

old issue was tracking cgroup driver detection for docker and other CRs

this is targeting 1.22.

remove the docker cgroupdriver detection in kubeadm:


old description:

this was already done for docker:
#844

When kubernetes/kubernetes#63887 merges, we should set the kubelet's --cgroup-driver flag to either systemd or cgroupfs conditionally at kubeadm init time, in cmd/kubeadm/app/phases/kubelet/flags.go.
This instead of doing it statically in the deb/rpm packages.
xref related issue: #822

we need to detect the driver for other CRI as well.

Docker PR:
kubernetes/kubernetes#64347

Document outlining some of the previous proposals and issues for handling this in the kubelet:
https://github.com/filbranden/community/blob/009f067c6bc106d062a416d0a1537de5ffa8c023/contributors/design-proposals/node/cgroup-driver-kubelet-and-runtime.md

@neolit123 neolit123 added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. area/UX labels May 30, 2018
@neolit123 neolit123 added this to the v1.12 milestone May 30, 2018
@luxas luxas added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels May 30, 2018
@neolit123 neolit123 self-assigned this May 30, 2018
@timothysc timothysc changed the title Set the kubelet --cgroup-driver flag conditionally at kubeadm init time Set the kubelet --cgroup-driver flag conditionally at kubeadm init time for other CRIs Jul 3, 2018
@timothysc timothysc removed this from the v1.12 milestone Jul 3, 2018
@timothysc timothysc added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jul 3, 2018
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 1, 2018
@neolit123
Copy link
Member Author

/lifecycle frozen
until there is a way to ingrate detection for other CRI.

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 1, 2018
@timothysc
Copy link
Member

@neolit123 @kad - any update on this one ?
If @rosti is updating the CRI detection we should address this as well for 1.13.
Thoughts?

@neolit123
Copy link
Member Author

AFAIK, it's not possible to detect a cgroup driver using crictl, yet.

for docker we probe the output of the CLI. other CRIs don't allow that TMK and we need to parse configs.

ideally this should be in the kubelet:
https://github.com/filbranden/kubernetes-community/blob/009f067c6bc106d062a416d0a1537de5ffa8c023/contributors/design-proposals/node/cgroup-driver-kubelet-and-runtime.md

cc @Random-Liu @filbranden @bart0sh

@kad
Copy link
Member

kad commented Oct 12, 2018

I agree with @neolit123 here. There is no at the moment good way to reliably detect correct driver.
I would probably go the way of parsing cgroupfs and trying to guess based on slice names... but that will be just good enough guess and not a 100% solution.

@timothysc timothysc added this to the Next milestone Jan 4, 2019
@timothysc
Copy link
Member

@neolit123 want to re-eval for 1.14 cycle?

@neolit123
Copy link
Member Author

neolit123 commented Jan 5, 2019

@timothysc i don't see us finding a good solution for this in the next few cycles.

EDIT: we now have this in the docs:
https://kubernetes.io/docs/setup/cri/#cgroup-drivers

some stats:
according to our survey 30% of the user base are using non-docker.

  • 1/3 of those are using CRI-O and are safe because it defaults to cgroupfs - the same as the kubelet.
    so if they change the driver for some reason, they are on they own. but we can still try to detect it and parse the crio.conf.
  • 1/3 are using containerd. no idea how to detect what is uses.
  • 1/3 are using other runtimes. we can't handle all of them.

@rosti
Copy link

rosti commented Apr 12, 2019

We may want to try to tackle this in 1.15.

As part of the manual testing that I did for #1495, I found out that the instructions for CRI-O on Ubuntu here aren't up to date.
First, I had some problem installing the 1.11 package and I installed CRI-O 1.13 instead. In it, the cgroups driver was set to systemd. As kubelet is defaulted to cgroupfs, it didn't work out of the box, requiring me to manually override it via kubeletExtraArgs.

We should verify that, but it seems to me, that most packages targeting systemd based Linux distros are moving to defaulting the cgroup driver to systemd. If that's true we may want to try the same.

The other option is to continue adding detection code for the most popular CRIs out there and document a way of overriding it in the kubelet (via the config).

@neolit123
Copy link
Member Author

neolit123 commented Apr 12, 2019

As part of the manual testing that I did for #1495, I found out that the instructions for CRI-O on Ubuntu here aren't up to date.

EDIT: the docs there say this for contained, but nothing for cri-o:

To use the systemd cgroup driver, set plugins.cri.systemd_cgroup = true in /etc/containerd/config.toml.
When using kubeadm, manually configure the cgroup driver for kubelet as well.

this does not explain how to do it very well, but the link is here:
https://kubernetes.io/docs/setup/independent/#configure-cgroup-driver-used-by-kubelet-on-master-node

(not using the config).

it didn't work out of the box, requiring me to manually override it via kubeletExtraArgs.

the question is how to automatically detect it. should we parse e.g. /etc/containerd/config.toml for cotainerd? is it safe long term? how about cri-o?

We should verify that, but it seems to me, that most packages targeting systemd based Linux distros are moving to defaulting the cgroup driver to systemd. If that's true we may want to try the same.

it's rather sparse. docker defaults to cgroupfs and the kubelet has no plans to do the detection anytime soon or to move from it's default of cgroupfs.

The other option is to continue adding detection code for the most popular CRIs out there and document a way of overriding it in the kubelet (via the config).

the manual setup is documented here:
https://kubernetes.io/docs/setup/independent/#configure-cgroup-driver-used-by-kubelet-on-master-node

the auto-detection for containerd and cri-o is under question - e.g. "what is the best way".

@timothysc timothysc modified the milestones: Next, v1.15 Apr 30, 2019
@timothysc timothysc added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed priority/backlog Higher priority than priority/awaiting-more-evidence. labels Apr 30, 2019
@timothysc timothysc modified the milestones: v1.15, Next May 3, 2019
@chenzhiwei
Copy link

For CRI-O, I asked this question in crio, and I think in the meantime we can get it from /etc/crio/crio.conf, the section is cgroup_manager = "systemd".

@chenzhiwei
Copy link

According to cri-o/cri-o#2416 (comment)

> sudo curl -s --unix-socket /var/run/crio/crio.sock http://localhost/info | jq
{
  "storage_driver": "btrfs",
  "storage_root": "/var/lib/containers/storage",
  "cgroup_driver": "cgroupfs",
  "default_id_mappings": {
    "uids": [
      {
        "container_id": 0,
        "host_id": 0,
        "size": 4294967295
      }
    ],
    "gids": [
      {
        "container_id": 0,
        "host_id": 0,
        "size": 4294967295
      }
    ]
  }
}

@neolit123
Copy link
Member Author

@chenzhiwei
nice, this is something we can look at for 1.16.
but we shouldn't use curl/jq and possibly try net.Dial("unix", " /var/run/crio/crio.sock") + encoding/json
instead.

also, is there something similar for containerd?

@chenzhiwei
Copy link

@neolit123 I will work on this after my PR kubernetes/kubernetes#77059 is merged.

Will try to check with containerd team about this, I can't find it on google or containerd website.

@villasenor
Copy link

Now that we are encouraging the use of fully CRI-compliant runtimes, it might be time to implement this. Even if the Dockershim is maintained and provides a CRI-compliant interface, making it easy to use CRI-O or containerd during kubeadm init should be a priority. The current documentation on setting the cgroupDriver during init is sparse, so I can try to work on that, but would be good to have a helping hand during init programmatically as well.

@neolit123
Copy link
Member Author

neolit123 commented Dec 7, 2020

this is still problematic - doable but not maintainable as a layer in kubeadm. different container runtimes place their configuration in different locations, behind different tools and endpoints. so unless we have something consistent like curl -s --unix-socket <some socket> http://localhost/info i'm leaning towards not managing this in kubeadm.

crictl info exists nowadays cri-o/cri-o#2414 (comment)
but i see it only prints conditions and the CgroupManager type is not always present.

instead we could always default the kubelet configuration field (cgroupDriver) to systemd and tell users to setup their container runtime to the same value, because that is the one they should be using regardless (when running the kubelet under system):

https://kubernetes.io/docs/setup/production-environment/container-runtimes/#cgroup-drivers

@villasenor
Copy link

@neolit123 I agree, pushing for the default to be systemd for the cgroupDriver in the kubelet and kubeadm, adding an optional flag/struct option for override to cgroupfs in kubeadm, and adding a pre-flight check during kubeadm init (if override isn't set) might be the best solution here. What are the next steps to get this going?

@neolit123 neolit123 modified the milestones: Next, v1.21 Dec 7, 2020
@neolit123
Copy link
Member Author

adding an optional flag/struct option for override to cgroupfs

this is already possible via the KubeletConfiguration field during "kubeadm init".

and adding a pre-flight check during kubeadm init

a preflight check to tell the user that "cgroupfs" is not recommended if they used the override?

What are the next steps to get this going?

1.20 is in code freeze, but we can try making these changes in 1.21.

@villasenor
Copy link

Sorry, new to this and probably could have been more clear!

this is already possible via the KubeletConfiguration field during "kubeadm init".

Right, yes!

a preflight check to tell the user that "cgroupfs" is not recommended if they used the override?

No, I meant a pre-flight check that ensures users are using systemd. If the user is not using systemd, we can say something like "It looks like you're not using the systemd cgroupDriver (default). Please switch to using systemd as your cgroupDriver, or override with a KubeletConfiguration field to use cgroupfs"

1.20 is in code freeze, but we can try making these changes in 1.21.

Sounds good! Hopefully this helps some people going forward :)

@neolit123
Copy link
Member Author

changing OP and description

/retitle remove the docker cgroup driver detection

@k8s-ci-robot k8s-ci-robot changed the title Set the kubelet --cgroup-driver flag conditionally at kubeadm init time for other CRIs remove the docker cgroup driver detection Jan 8, 2021
@neolit123
Copy link
Member Author

the OP now includes details on how you can help us with removing this detection.

@neolit123 neolit123 added lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. and removed help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. labels Mar 9, 2021
@neolit123 neolit123 modified the milestones: v1.21, v1.22 Mar 9, 2021
@neolit123 neolit123 added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels May 18, 2021
@neolit123
Copy link
Member Author

this work is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/UX lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants