fix kube_reserved so it only controls kubeReservedCgroup

[rptaylor]

What type of PR is this?

/kind bug

What this PR does / why we need it:
I believe there may have been a misapprehension in the implementation of #9209

It introduced a new kube_reserved variable with a default value of false, and made all kubeReserved declarations conditional on it. I believe this was a mistake as the documentation makes it clear that you can set kubeReserved without kubeReservedCgroup, for the purposes of reserving resources without enforcing, which is the safest default behaviour.

This MR restores the previous behaviour - kubeReserved can/should always be defined and does not depend on or require the creation of separate cgroups, or running daemons in those cgroups, or configuring kubeReservedCgroup. The important primary purpose of this is to reduce the Node Allocatable resource amount to guarantee resources for k8s daemons purely on a scheduling basis, even if their cgroup config is not modified.

Which issue(s) this PR fixes:

Fixes #9692

Special notes for your reviewer:
Please see this comment
This is just what I have been trying to figure out for the last several days. Let's review carefully to make sure it's right.

Does this PR introduce a user-facing change?:

This restores the pre-9209 behaviour of reserving a small default amount of RAM and CPU to ensure stability of k8s daemons.

[k8s-ci-robot]

Hi @rptaylor. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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-sigs/prow repository.

[rptaylor]

I confirmed that after upgrading to Kubespray 2.21 with the default kube_reserved: false , these lines get removed from /etc/kubernetes/kubelet-config.yaml

kubeReserved:
  cpu: 100m
  memory: 256Mi

This demonstrates the problem, it causes the allocatable resources reported on a node to increase from (pre 2.21)

Allocatable:
  cpu:                15900m
  ephemeral-storage:  483172476948
  hugepages-1Gi:      0
  hugepages-2Mi:      0
  memory:             61438100Ki
  pods:               110

to (post 2.21)

  Allocatable:
  cpu:                16
  ephemeral-storage:  483172476948
  hugepages-1Gi:      0
  hugepages-2Mi:      0
  memory:             61700232Ki
  pods:               110

This means that pods can consume 100% of node resources, which would cause kubelet/containerd to crash or become unresponsive.

[rptaylor]

I applied this patch on a v2.21 cluster via ansible-playbook -b --become-user=root ~/ubernetes/kubespray/cluster.yml -t defaults,node --diff and it had the expected result, showing for each node that kubeReserved is restored:

--- before: /etc/kubernetes/kubelet-config.yaml
+++ after: /home/fedora/.ansible/tmp/ansible-local-8422569_ougct4/tmp4l2hqnen/kubelet-config.v1beta1.yaml.j2
@@ -27,6 +27,9 @@
 rotateCertificates: true
 clusterDNS:
 - 169.254.25.10
+kubeReserved:
+  cpu: 200m
+  memory: 512Mi
 resolvConf: "/etc/resolv.conf"
 eventRecordQPS: 5
 shutdownGracePeriod: 60s

Afterwards the nodeAllocatable reduced as expected.

[MrFreezeex]

/ok-to-test

[MrFreezeex]

Thanks for the explanation and the fix :D
/lgtm

[rptaylor]

/assign ant31

[rptaylor]

@ant31 @ErikJiang Can you please take a look and /approve ?

/assign ErikJiang

[mzaian]

Hello @rptaylor

Thanks a lot for the fix and explaination.

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrFreezeex, mzaian, rptaylor

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 [mzaian]

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