cluster up: set docker cgroup driver on kubelet config #13964
Conversation
|
This fixes the issue on mac with the post-rebase codebase. However, just recently discovered that it breaks previous versions of the images because the --cgroup-driver flag is not recognized. I'm thinking that maybe a better approach would be to change the startup code to default to the cgroup driver used by docker. |
|
@sttts is there a reason you hardcoded systemd as the cgroup driver? It breaks us on non-systemd distros like moby (Docker for Mac/Windows) |
|
@csrwng we hardcoded ot systemd mainly because that's the primary deployment target (Fedora/CentOS/RHEL with systemd). However, given that this is causing issues, I agree that it would be better to have the startup logic for the node attempt to set this automagically. |
| if nodeCfg.KubeletArguments == nil { | ||
| nodeCfg.KubeletArguments = configapi.ExtendedArguments{} | ||
| } | ||
| nodeCfg.KubeletArguments["cgroup-driver"] = []string{cgroupDriver} |
There was a problem hiding this comment.
Is this intentional that cgroup-driver is overridden? Shouldn't we keep an existing value?
There was a problem hiding this comment.
@stts this is just setting it to whatever docker reports is its cgroupDriver instead of the hardcoded systemd value. (only for the case of cluster up)
@ncdc do you know of a way to detect whether the systemd cgroup driver is available? |
No clue, sorry |
|
Even after updating the cgroup driver, no nodes become available for me with |
|
@spadgett I added a commit with additional kubelet flags which get this to work. |
csrwng
changed the title
|
Removed the WIP in the title. This is now working for all versions. |
|
@csrwng Validated the fix. Works great! |
|
@csrwng lgtm, did you still want @derekwaynecarr to look at the flags? |
|
yes, @derekwaynecarr it'd be great if you can check that I'm not doing anything dumb. Just keep in mind that this is only for cluster up use cases. |
|
@csrwng -- the issue when debugging with you is that these VMs have a cgroup "openrc" that is not known to runc ;-) because it appears as a mounted subsystem in the cgroupfs, and we do not see changes in that subsystem (cause runc doesnt appear to know it), we think that things that did create did not actually create. we need to make kubelet ignore openrc if its found as the proper fix. |
|
@csrwng -- finding the least hacky way to ignore openrc is my first priority tomorrow. |
|
opened opencontainers/runc#1440 |
| nodeCfg.KubeletArguments = configapi.ExtendedArguments{} | ||
| } | ||
| nodeCfg.KubeletArguments["cgroup-driver"] = []string{cgroupDriver} | ||
| if cgroupDriver != "systemd" { |
There was a problem hiding this comment.
this should not be necessary with kubernetes/kubernetes#45515
can you pull that locally and verify it works for you?
if you verify, we can carry a patch in origin.
|
@derekwaynecarr Updated to include your upstream commit and not set the additional arguments. |
|
i need to address one-nit in the usptream pr, can we merge this once i have that handled? |
On kubelet versions that support the --cgroup-driver argument, set the cgroup-driver flag to the one reported by docker.
|
I have updated this with the merged version of the upstream commit |
|
Evaluated for origin merge up to 76838dd |
|
[Test]ing while waiting on the merge queue |
|
Evaluated for origin test up to 76838dd |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/1345/) (Base Commit: 2628c77) |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin/620/) (Base Commit: 9bcaa0a) (Image: devenv-rhel7_6225) |
Fixes issue which requires that cgroup driver be specified for kubelet post 1.6 kube rebase.
Fixes #14101