Fix system container detection in kubelet on systemd #25982
Conversation
k8s-github-robot
added
size/M
|
If we are running e2e on systems with systemd, this could cause those systems to NOT report system container stats for @pmorie @ingvagabund -- can you update the RHEL CI environment? $ cat /etc/systemd/system.conf
...
DefaultCPUAccounting=true
DefaultMemoryAccounting=true
... |
derekwaynecarr
added
release-note
This is suitable for ansible playbook so all can benefit from it. Do you think it should be enabled by default? |
| // in addition, you would not get memory or cpu accounting for the runtime unless accounting was enabled on its unit (or globally). | ||
| if systemd, found := cgs["name=systemd"]; found { | ||
| if systemd != cpu { | ||
| glog.Warningf("CPUAccounting not enabled for pid: %d", pid) |
There was a problem hiding this comment.
Should we require cpu and memory accounting to be enabled for kube daemons by default?
|
@derekwaynecarr This PR LGTM. For the future though, can we require cpu and memory accounting to be enabled by default? |
|
I wasn't sure everyone knew what to do when hacking on their local host. Was worried it would cause people problems. Ideas? |
|
Basically they need to turn global accounting on when they hack from a Linux host since most people don't have accounting on in user sessions. |
|
@derekwaynecarr May be a flag to allow kubelet to run without stats? In any case, this PR is fine as-is. So I will add the LGTM label. |
vishh
added
the
lgtm
|
@derekwaynecarr Another option is to just validate metrics requirement as part of conformance testing and not change kubelet. |
k8s-github-robot
removed
the
lgtm
derekwaynecarr
added
the
lgtm
|
The node e2e boxes that run systemd (in this case it looks like coreos stable), we need to enable cpu and memory accounting by default. Will send a PR that does the same for vagrant as an example. |
… building from local sources. For distribution packages (rpms, deb, etc.) let the installation mechanism to take care of accounting. Quoting: Fix system container detection in kubelet on systemd. This fixed environments where CPU and Memory Accounting were not enabled on the unit that launched the kubelet or docker from reporting the root cgroup when monitoring usage stats for those components. From: kubernetes/kubernetes#25982
|
fyi, required node test is failing. |
|
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
|
GCE e2e build/test passed for commit 5a8851d. |
|
Automatic merge from submit-queue |
|
It seems that this PR broke node-e2e suite. Since this is blocking merge-queue, I'm going to revert this. |
|
This should not have been merged until the node e2e infrastructure was updated per: I will re-open, but merge bot should not have merged this. |
|
@derekwaynecarr it should be enough to prefix the summary with [WIP] i believe to avoid merging |
Fixes #25909
/cc @kubernetes/sig-node @kubernetes/rh-cluster-infra @vishh @dchen1107