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

Allow setting cgroup driver for kubelet #1677

Merged
merged 1 commit into from
Sep 21, 2017

Conversation

jistr
Copy link
Contributor

@jistr jistr commented Sep 19, 2017

Red Hat family platforms run docker daemon with --exec-opt native.cgroupdriver=systemd. When kubespray tried to start kubelet
service, it failed with:

Error: failed to run Kubelet: failed to create kubelet: misconfiguration: kubelet cgroup driver: "cgroupfs" is different from docker cgroup driver: "systemd"

Setting kubelet's cgroup driver to the correct value for the platform
fixes this issue.

@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Sep 19, 2017
@mattymo
Copy link
Contributor

mattymo commented Sep 19, 2017

ci check this

@jistr
Copy link
Contributor Author

jistr commented Sep 20, 2017

The CI failure looks unrelated. "Timeout when waiting for localhost:22"

@bogdando
Copy link
Contributor

Retried the job.
Thanks for the fix, @jistr !

@@ -1,4 +1,19 @@
---
- name: gather os specific variables
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this is required? Wouldn't the roles/kubernetes/node/vars/redhat.yml override for rh family just work?

Copy link
Contributor

Choose a reason for hiding this comment

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

please disregard, this is just am existing pattern reused

bogdando
bogdando previously approved these changes Sep 20, 2017
@@ -1,4 +1,19 @@
---
- name: gather os specific variables
Copy link
Contributor

Choose a reason for hiding this comment

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

please disregard, this is just am existing pattern reused

Copy link
Contributor

@bogdando bogdando left a comment

Choose a reason for hiding this comment

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

Please postpone merging unless the failing CI job resolved. Looks unrelated, but has to be fixed first :/

@mattymo
Copy link
Contributor

mattymo commented Sep 20, 2017

@jistr I fixed master. Can you rebase your PR?

@jistr
Copy link
Contributor Author

jistr commented Sep 20, 2017

Rebased

@mattymo
Copy link
Contributor

mattymo commented Sep 20, 2017

@jistr I think maybe you are using a different docker RPM than we use. Your PR fails CI on CentOS, which shows that the Docker we install should be run with cgroupfs. Of course, if we should change which Docker to install for centos/rhel/fedora, let's do that to make this PR work.

@bogdando bogdando dismissed their stale review September 20, 2017 15:15

need to address CI issues

@jistr
Copy link
Contributor Author

jistr commented Sep 21, 2017

Indeed i think we have different docker RPMs. I have the one which comes with CentOS - docker-1.12.6-48.git0fdc778.el7.centos.x86_64.

In the CI logs i see there's enabled https://yum.dockerproject.org repos, which i don't think is really necessary and IMO it's not even the expected production scenario for CentOS. At least in the TripleO project we don't have a reason to divert from what comes with the distro -- i'd expect the distro packages to be well tested and stable.

If desired i can amend the patch to always default to cgroupfs, and only have it configurable so that we can manually override the default in TripleO. But i think it might be better to switch Kubespray CI to use CentOS packages for docker. (?)

@jistr
Copy link
Contributor Author

jistr commented Sep 21, 2017

There would still be a chicken-and-egg issue with this patch though, as if you switched the packages without merging this, the CI would break. Maybe it would be best to autodetect the correct value and make it a fact. docker info | grep 'Cgroup Driver' could do the trick

@jistr
Copy link
Contributor Author

jistr commented Sep 21, 2017

Which is to say -- i'll update the PR with autodetection.

Red Hat family platforms run docker daemon with `--exec-opt
native.cgroupdriver=systemd`. When kubespray tried to start kubelet
service, it failed with:

Error: failed to run Kubelet: failed to create kubelet: misconfiguration: kubelet cgroup driver: "cgroupfs" is different from docker cgroup driver: "systemd"

Setting kubelet's cgroup driver to the correct value for the platform
fixes this issue. The code utilizes autodetection of docker's cgroup
driver, as different RPMs for the same distro may vary in that regard.
@jistr
Copy link
Contributor Author

jistr commented Sep 21, 2017

Passed CI, and it worked locally for me too.

@bogdando bogdando merged commit 4f63625 into kubernetes-sigs:master Sep 21, 2017
@bogdando
Copy link
Contributor

@jistr would be nice to document the new kubelet_cgroup_driver option use cases please.

@jistr
Copy link
Contributor Author

jistr commented Sep 21, 2017

@bogdando I meant the option mainly as a safety, generally i like automagic to be overridable :)

I could mention that this is autodetected and overridable -- is there a place where this would fit in the docs? I tried looking at some of the files in https://github.com/kubernetes-incubator/kubespray/tree/master/docs but they seem to be fairly higher level than this info, but admittedly i didn't go through each file there.

@apevec
Copy link

apevec commented Sep 21, 2017

I have the one which comes with CentOS - docker-1.12.6-48.git0fdc778.el7.centos.x86_64.

In CentOS Extras there is also newer Docker, RPM is named docker-latest:
http://mirror.centos.org/centos/7/extras/x86_64/Packages/docker-latest-1.13.1-21.1.gitcd75c68.el7.centos.x86_64.rpm

@bogdando
Copy link
Contributor

jistr added a commit to jistr/kubespray that referenced this pull request Sep 22, 2017
This follows pull request kubernetes-sigs#1677, adding the cgroup-driver
autodetection also for kubeadm way of deploying.

Info about this and the possibility to override is added to the docs.
@jistr
Copy link
Contributor Author

jistr commented Sep 22, 2017

@bogdando Thanks! The docs are posted and i also added the option to kubeadm deployments, which i had the opportunity to test better now and i was hitting the same issue there: #1687

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants