Kubelet drop in #1299
Kubelet drop in #1299
Conversation
We can now use a proper API for customizations. See follow-up commits. Signed-off-by: Victor Toso <[email protected]>
kubevirt-bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I've verified that in k8s-1.30, removing all options set in
KUBELET_EXTRA_ARS, would result in the following diff:
@@ -54 +53,0 @@
- "kubeletCgroups": "/systemd/system.slice",
@@ -87 +86 @@
- "failSwapOn": false,
+ "failSwapOn": true,
I've moved both kubeletCgroups and failSwapOn into kubelet's drop-in
config file and verified that it works and config is set.
I assume that --cgroup-driver=systemd is already the default in centos
stream or being set somewhere else, but to avoid regressions I've
included in the config file as well.
I don't know why --runtime-cgroups is not being shown in the node's
configz but it is the only command line option for the initial 4 that
is not marked as DEPRECATED in favor of Config file alternative, so
let's keep it as command line option, together with --config-dir.
Signed-off-by: Victor Toso <[email protected]>
victortoso
force-pushed
the
kubelet-drop-in
branch
from
f72745a to
9e3057c
Compare
|
@victortoso: The following tests failed, say
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. I understand the commands that are listed here. |
|
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with /lifecycle stale |
kubevirt-bot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
kubevirt-bot
removed
the
lifecycle/stale
There was a problem hiding this comment.
Awesome stuff, looks great to me! Thank you @victortoso!
/lgtm
There was a problem hiding this comment.
Is this the right file for this logic? Should we place it somewhere more generic that would fit other k8s versions as well? For example 1.31?
There was a problem hiding this comment.
If we move it to 1.31 it'll be cloned when 1.32 is created? Not sure how this process goes, I'm happy to change it to the right place.
There was a problem hiding this comment.
/cc @dhiller @brianmcarey
| # Create drop-in config files for kubelet | ||
| # https://kubernetes.io/docs/tasks/administer-cluster/kubelet-config-file/#kubelet-conf-d | ||
| kubelet_conf_d="/etc/kubernetes/kubelet.conf.d" | ||
| mkdir -m 644 $kubelet_conf_d | ||
|
|
||
| # Set our custom initializations to kubelet | ||
| kubevirt_kubelet_conf="$kubelet_conf_d/50-kubevirt.conf" | ||
| cat <<EOF >$kubevirt_kubelet_conf | ||
| apiVersion: kubelet.config.k8s.io/v1beta1 | ||
| kind: KubeletConfiguration | ||
| cgroupDriver: systemd | ||
| failSwapOn: false | ||
| kubeletCgroups: /systemd/system.slice | ||
| EOF | ||
|
|
||
| # Set only command line options not supported by config |
There was a problem hiding this comment.
Looks great to me!
Do you think it's possible to somehow let the user provide an alternative KubeletConfiguration that will override this? I think it would give the user full control which could be very beneficial when doing complex configuration. In any case it should not block this PR, just an idea :)
There was a problem hiding this comment.
yeah, that's a possibility. My initial motivation is actually make it easier to do changes without abusing sed :)
| @@ -148,9 +148,24 @@ patch $cni_manifest_ipv6 $cni_ipv6_diff | |||
|
|
|||
| cp /tmp/local-volume.yaml /provision/local-volume.yaml | |||
|
|
|||
| # TODO use config file! this is deprecated | |||
| # Create drop-in config files for kubelet | |||
| # https://kubernetes.io/docs/tasks/administer-cluster/kubelet-config-file/#kubelet-conf-d | |||
There was a problem hiding this comment.
I see in that link that KUBELET_CONFIG_DROPIN_DIR_ALPHA has to be defined, but I don't see it being defined here. WDYT?
What this PR does / why we need it:
We are using deprecated kubelet's command line options which have config alternative options. Seems to be known since b89e9a6. This, combined with the goal of enabling k8s feature gates such as DRA in #1251, the goal of this PR is to improve kubelet's config options and use drop-in feature.
Commit logs clarify further the changes.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
This PR only focused on k8s-1.30 to validate the change. I can do here or in a new PR for k8s-1.31 too.
/cc @lyarwood
Checklist
This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.
Release note: