Set the kubelet `--cgroup-driver` flag conditionally at kubeadm init time for Docker #844

luxas · 2018-05-22T18:18:51Z

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

PR:
kubernetes/kubernetes#64347

The text was updated successfully, but these errors were encountered:

neolit123 · 2018-05-22T18:24:35Z

i really wanted to see a solution where they make the default kubelet --cgroup-driver mode auto, by default instead of cgroupfs. looking at the source code of the kubelet that does that validation to match the drivers it seemed quite possible, but maybe there is a catch.

this would avoid kubeadm setting the driver explicitly, given the kubelet knows what driver to use.

luxas · 2018-05-22T18:28:25Z

WDYT @kubernetes/sig-node-feature-requests about the suggestion above? cc @mtaufen

yujuhong · 2018-05-22T20:03:54Z

/cc @filbranden

filbranden · 2018-05-22T20:18:31Z

@neolit123 I meant to work on something like that... But talking to @Random-Liu we noticed it was hard to support that since the Runtime (CRI) doesn't need to be up when kubelet starts and we would like to avoid blocking on the Runtime for kubelet startup...

We discussed a few alternative proposals, but we have nothing to show yet.

One possible alternative is to make the CRI the slave and kubelet the master for that attribute. (If kubelet sends the Runtime a systemd cgroup path, then Runtime also uses the systemd cgroup driver and vice-versa.) We might need some changes in libcontainer to do that... It should be feasible.

neolit123 · 2018-05-22T20:45:19Z

@filbranden

One possible alternative is to make the CRI the slave and kubelet the master for that attribute. (If kubelet sends the Runtime a systemd cgroup path, then Runtime also uses the systemd cgroup driver and vice-versa.) We might need some changes in libcontainer to do that... It should be feasible.

it's a complicated change and a long-term one, but i guess it would work!

I meant to work on something like that... But talking to @Random-Liu we noticed it was hard to support that since the Runtime (CRI) doesn't need to be up when kubelet starts and we would like to avoid blocking on the Runtime for kubelet startup...

We discussed a few alternative proposals, but we have nothing to show yet.

you have probably considered this already, but wouldn't the following be the easiest short-term solution that doesn't modify the current kubelet / CRI (master-on-slave) setup:

add the auto parameter to kubelet's cgroup-driver argument
if the CRI is running take it's cgroup driver
if the CRI isn't running use cgroupfs

filbranden · 2018-05-22T20:53:40Z

The current way the code is laid out makes it tricky...

Even with docker-shim, it turns out there is code that depends on the cgroup driver setting that runs before the first connection to docker-shim is made... So it's actually quite a big refactor. That's when I started talking to @Random-Liu about this and we looked at the other shortcomings of the approach.

Agreed, having CRI runtimes do both cgroup drivers (and have the request from kubelet decide) is a long term project, it's gonna take a while...

neolit123 · 2018-05-22T21:18:53Z

@filbranden
understood, i was almost sure that there is a catch here, otherwise auto would have easily made it as the default option. thank you for the detailed explaination!

luxas · 2018-05-22T21:25:21Z

@neolit123 let's go with this approach for the moment then
@filbranden do you have an issue in k/k to track the auto feature so we can track the progress there and eventually switch over?

neolit123 · 2018-05-22T21:26:07Z

@luxas what do you have in mind for the detection of the CRI driver at kubeadm runtime?

something like this would fetch it:

docker info | grep -i cgroupfs

but it's a command line and only for docker...

luxas · 2018-05-22T21:29:17Z

We expect the CRI to be running at the time kubeadm is running. Can you query the CRI runtime for that information for the general case or just query docker info or similar for the special case?

neolit123 · 2018-05-22T21:50:38Z

Can you query the CRI runtime for that information for the general case

should that be done with crictl?

or just query docker info or similar for the special case?

could you elaborate on general case vs special case.
i'm sorry for the silly questions.

filbranden · 2018-05-22T22:24:02Z

do you have an issue in k/k to track the auto feature so we can track the progress there and eventually switch over?

No I don't, I never really filed one, I was trying to get a commit at least to a prototype stage before doing that, and given I found it wasn't really feasible, I didn't move forward...

Can you query the CRI runtime for that information for the general case?

That's also part of the problem... CRI doesn't really have an official way to expose that either...

IIRC, for containerd, if you query it using the Status CRI call (crictl status) and set Verbose: true then you get a string containing a JSON that has the cgroup driver somewhere in there...

When I checked cri-o, it doesn't have anything like that...

So if you want to query it, you might need to add some more support in CRI to export that... (Perhaps the Status request is the appropriate place... just might need an explicit proto field to export that, and update existing implementations.)

filbranden · 2018-05-22T22:27:32Z

You might find this interesting too!

https://github.com/filbranden/community/blob/009f067c6bc106d062a416d0a1537de5ffa8c023/contributors/design-proposals/node/cgroup-driver-kubelet-and-runtime.md

Cheers,
Filipe

neolit123 · 2018-05-22T22:50:07Z

@filbranden

thanks for the details! i will have a look at this tomorrow:

https://github.com/filbranden/community/blob/009f067c6bc106d062a416d0a1537de5ffa8c023/contributors/design-proposals/node/cgroup-driver-kubelet-and-runtime.md

That's also part of the problem... CRI doesn't really have an official way to expose that either...
When I checked cri-o, it doesn't have anything like that...

So if you want to query it, you might need to add some more support in CRI to export that... (Perhaps the Status request is the appropriate place... just might need an explicit proto field to export that, and update existing implementations.)

oh, well. i'm not exactly sure how we can proceed with this for 1.11.
@luxas what do you think about the outlined complication?

luxas · 2018-05-23T06:56:48Z

hmm, let's do this only for docker then. So if the CRISocket is dockershim, check docker ps output or whatever to determine what to use. We really need this for docker in the general case, otherwise all our rpm-based installations fail.
@neolit123 seems legit to do this?

stealthybox · 2018-05-23T08:38:03Z

available since 1.13

$ docker system info --format '{{.CgroupDriver}}'
cgroupfs

neolit123 · 2018-05-23T10:23:52Z

@luxas
seems good.

@stealthybox
thanks for the shortcut.
i will parse the docker info output as i don't want to deprecate 1.11 and 1.12 because of that.

neolit123 · 2018-05-30T13:35:37Z

NOTE: keeping this issue for the rest of the CRI.

luxas · 2018-05-30T15:55:24Z

@neolit123 Would you be okay with keeping this in the v1.11 milestone to track what's needed for v1.11 and then open a new to track the rest of the CRI work in order to not change purpose midterm?

neolit123 · 2018-05-30T15:57:47Z

@luxas ok!

@luxas

Automatic merge from submit-queue (batch tested with PRs 64338, 64219, 64486, 64495, 64347). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. kubeadm: conditionally set the kubelet cgroup driver for Docker **What this PR does / why we need it**: Add a new utility file - util/cgroupdriver.go. Currently it only contains the public function GetCgroupDriverDocker(). The function uses 'docker info' to obtain the cgroup driver for Docker. On a later stage this file can contain more methods for different CRI. Use GetCgroupDriverDocker() in phases/kubelet/flags.go to conditionally set the 'cgroup-driver' argument. On error print a warning and don't set the argument value. Add unit tests in cgroupdriver_test.go. **Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*: Fixes kubernetes/kubeadm#844 **Special notes for your reviewer**: NONE **Release note**: ```release-note kubeadm: conditionally set the kubelet cgroup driver for Docker ``` /area kubeadm /kind enhancement @kubernetes/sig-cluster-lifecycle-pr-reviews @luxas

luxas added this to the v1.11 milestone May 22, 2018

luxas added kind/enhancement priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. area/releasing area/upgrades labels May 22, 2018

k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. kind/feature Categorizes issue or PR as related to a new feature. labels May 22, 2018

luxas mentioned this issue May 22, 2018

Make it possible to configure the kubelet via the v1alpha2 Config file #847

Closed

luxas assigned neolit123 May 22, 2018

neolit123 mentioned this issue May 25, 2018

kubeadm: conditionally set the kubelet cgroup driver for Docker kubernetes/kubernetes#64347

Merged

luxas added the lifecycle/active Indicates that an issue or PR is actively being worked on by a contributor. label May 29, 2018

neolit123 removed this from the v1.11 milestone May 30, 2018

neolit123 added this to the v1.12 milestone May 30, 2018

neolit123 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/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels May 30, 2018

neolit123 modified the milestones: v1.12, v1.11 May 30, 2018

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 30, 2018

neolit123 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 Docker May 30, 2018

neolit123 mentioned this issue May 30, 2018

remove the docker cgroup driver detection #874

Closed

3 tasks

k8s-github-robot closed this as completed in kubernetes/kubernetes#64347 May 31, 2018

github-actions bot mentioned this issue Dec 14, 2020

add support for detecting the cgroup driver for CRI other than pacoxu/kubernetes#874

Open

afbjorklund mentioned this issue Jan 18, 2021

default the "cgroupDriver" setting of the kubelet to "systemd" #2376

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set the kubelet `--cgroup-driver` flag conditionally at kubeadm init time for Docker #844

Set the kubelet `--cgroup-driver` flag conditionally at kubeadm init time for Docker #844

luxas commented May 22, 2018 •

edited by neolit123

Loading

neolit123 commented May 22, 2018

luxas commented May 22, 2018

yujuhong commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 22, 2018

neolit123 commented May 22, 2018 •

edited

Loading

filbranden commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 23, 2018

stealthybox commented May 23, 2018 •

edited

Loading

neolit123 commented May 23, 2018

neolit123 commented May 30, 2018

luxas commented May 30, 2018

neolit123 commented May 30, 2018

Set the kubelet --cgroup-driver flag conditionally at kubeadm init time for Docker #844

Set the kubelet --cgroup-driver flag conditionally at kubeadm init time for Docker #844

Comments

luxas commented May 22, 2018 • edited by neolit123 Loading

neolit123 commented May 22, 2018

luxas commented May 22, 2018

yujuhong commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 22, 2018

neolit123 commented May 22, 2018 • edited Loading

filbranden commented May 22, 2018

filbranden commented May 22, 2018

neolit123 commented May 22, 2018

luxas commented May 23, 2018

stealthybox commented May 23, 2018 • edited Loading

neolit123 commented May 23, 2018

neolit123 commented May 30, 2018

luxas commented May 30, 2018

neolit123 commented May 30, 2018

Set the kubelet `--cgroup-driver` flag conditionally at kubeadm init time for Docker #844

Set the kubelet `--cgroup-driver` flag conditionally at kubeadm init time for Docker #844

luxas commented May 22, 2018 •

edited by neolit123

Loading

neolit123 commented May 22, 2018 •

edited

Loading

stealthybox commented May 23, 2018 •

edited

Loading