Refactor cgroup and kubelet enforceNodeAllocatable

[VannTen]

What type of PR is this?
/kind design
Documentation/Bug are probably relevant as well

What this PR does / why we need it:
#9209 introduced knobs for using https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/

However, IMO, it leaks a bunch of stuff to the user that it should not.

This is an attempt to streamline the configuration and reduce variance of the jinja templates.

See the commits message for more detailed explanation.

closes #8870 (this one is a superset)
Require #10643
Also, this should fix journactl -u kubelet not getting kubelet logs (because kubelet jumped out of the service cgroup)

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

action required
`kubelet_enforce_node_allocatable` is removed, use instead `enforce_allocatable_{pods,kube_reserved,system_reserved}`
`system_reserved` and `kube_reserved` are removed, use instead `enforce_allocatable_{kube,system}_reserved`
The kubelet and container manager service will by default run in the `runtime.slice` 
{kube,system}_reserved_cgroups and {kube,system}_reserved_cgroups_for_service are removed, use `{kube,system}_slice` to customize their slice
The system slice is no longer created by kubelet on startup
The kube slice creation is delegated to systemd
The system and kube slice are now indifferent to the cgroup driver used
For cri-o with systemd cgroup-driver, use `kube_slice` variable for conmon_cgroup instead of "system.slice" 

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[VannTen]

/cc @MrFreezeex
(as you fixed some stuff on that)
and
/cc @shelmingsong
(original author)

[k8s-ci-robot]

@VannTen: GitHub didn't allow me to request PR reviews from the following users: shelmingsong.

Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @MrFreezeex
(as you fixed some stuff on that)
and
/cc @shelmingsong
(original author)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[MrFreezeex]

/cc @MrFreezeex (as you fixed some stuff on that) and /cc @shelmingsong (original author)

From a rough look it looks nice, I will probably take a deeper look when the other PR would be merged. If you could make sure that the kubelet is not complaining at runtime about the configured cgroup slice it would be nice (that the was the issue I fixed).

[VannTen]

Forget to mention that this require #10643 (it's on top of it)

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all PRs.

This bot triages PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the PR is closed

You can:

• Mark this PR as fresh with /remove-lifecycle stale
• Close this PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[VannTen]

/remove-lifecycle stale /lifecycle frozen

[k8s-ci-robot]

@VannTen: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/remove-lifecycle stale
/lifecycle frozen

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[rptaylor]

#9692 may be relevant to consider here.

[VannTen]

I skimmed that issue, but it might indeed. IMO our current variables are not super clear, and don't distinguish clearly between reserving resources (aka, reducing allocatable) and enforcing (add hard limits on the cgroup slice for system / kubelet). This PR does not fundamentally change how it works, but it should be easier for users to navigate ; at least that's the goal.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: VannTen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [VannTen]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[VannTen]

/ok-to-test

[rptaylor]

IMO our current variables are not super clear, and don't distinguish clearly between reserving resources (aka, reducing allocatable) and enforcing (add hard limits on the cgroup slice for system / kubelet).

Yes exactly. As far as documentation goes, please note #11367 fixed the behaviour of kube_reserved which was introduced by #9209, but now the behaviour doesn't really match the variable name. Strictly speaking, resource reservation always occurs, but if kube_reserved is true, enforcement of resource limits is applied on the kubelet and container engine.

It looks like this PR renames the kube_reserved variable to enforce_allocatable_kube_reserved ? That is more clear. Thanks!

[VannTen]

Yes it does. I also put the kubelet and runtime in a dedicated cgroup **always**, because I don't see a reason for making that customizable, since what matters is the enforceNodeAllocatable config for whether or not those cgroups have hard limits.

[VannTen]

Anyone has experience on crio ?

I'm trying to figure what conmon_cgroup is supposed to match, I'm pretty sure it should not be kube_slice (aka the cgroup for kubelet + container engine) but the slice where the pods are going (kubepods.slice) but I can't find a definitive answer on that

[VannTen]

/unhold
Now that #10643 has been merged

[VannTen]

So, reading cri-o/cri-o#2248, conmon_cgroup is largely a parrallel concern to kube_reserved and friends. It's per pod, so basically it's part of PodOverhead.
I'm gonna scrape that config entirely from CRI-O, given it should not be tied to kube_reserved in any way AFAIK.
If someone wants to add a way to specifically configure that feature of CRI-O, it should be another PR

Hum. I'm not completely sure after reading some issues in cri-o repo. Let's stick to pod for cgroupfs and slice for systemd. (using kube_slice rather than 'system.slice` since this is more about the runtime that the "other node daemons"

[VannTen]

/cc @MrFreezeex
PTAL

[MrFreezeex]

Those seems to be a bit weird kube_slice and system_slice are equal respectively to rumtime.slice and system.slice so afaiu this would be the same as kube_slice and system_slice atm?

[VannTen]

Not exactly.
It would be /runtime.slice/ and /system.slice/.
This is intented to make the translation from systemd slice units to the correspond cgroup tree in /sys/fs/cgroup/

so runtime.slice -> /sys/fs/cgroup/runtime.slice/
nested-runtime.slice -> /sys/fs/cgroup/nested.slice/runtime.sliceBut I'm just seeing now that this does not work exactly that way, instead it should benested-runtime.slice /sys/fs/cgroup/nested.slice/nested-runtime.slice`

I'll go fix that

[MrFreezeex]

Ah yes indeed forgot the / but I was more wondering about the split/join which doesn't do anything here AFAIU (?)

[VannTen]

It doesn't do anything on slice immediately under the root slice (like kube.slice).

However, if a kubespray user define kube_slice: orchestrator-kube.slice the corresponding cgroup will be
/sys/fs/cgroup/orchestrator.slice/orchestrator-kube.slice/ (I had mistakenly assumed it was orchestrator.slice/kube.slice/, which the current code reflects.)

However, I need to do more research on that. It seems that maybe the *ReservedCgroup setting are interpreted differently depending on the cgroupDriver used (cgroupfs/systemd) and in systemd case the translation from slice to cgroup would be done directly by the kubelet. Not completely sure though so I've asked on slack sig-node ( https://kubernetes.slack.com/archives/C0BP8PW9G/p1729771784322639 )

[VannTen]

/retest-failed

[VannTen]

Relevant : kubernetes/kubernetes#125982