Kubelet drop in #1299

victortoso · 2024-10-11T11:58:44Z

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.

Design: A design document was considered and is present (link) or not required
PR: The PR description is expressive enough and will help future contributors
Code: Write code that humans can understand and Keep it simple
Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
Upgrade: Impact of this change on upgrade flows was considered and addressed if required
Testing: New code requires new unit tests. New features and bug fixes require at least on e2e test
Documentation: A user-guide update was considered and is present (link) or not required. You want a user-guide update if it's a user facing feature / API change.
Community: Announcement to kubevirt-dev was considered

Release note:

NONE

kubevirt-bot · 2025-01-09T14:56:51Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

/lifecycle stale

victortoso · 2025-01-14T09:55:13Z

/remove-lifecycle stale
🏓 @lyarwood
@iholder101 If you have the time too? :)

iholder101

Awesome stuff, looks great to me! Thank you @victortoso!

/lgtm

iholder101 · 2025-01-14T12:27:56Z

cluster-provision/k8s/1.30/k8s_provision.sh

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?

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.

/cc @dhiller @brianmcarey

I'll ping dhiller/brian in slack and see where I should put this change to make it cloned when a new k8s release is included in kubevirtci and propose it in a follow up.

... or I can do in this one too, either way is fine for me :)

So the k8s-1.32 is available now so if we want it carried on through future providers we need this change added to k8s-1.32 & k8s-1.31. The next provider (k8s-1.33) will be based off of a copy of k8s-1.32.

Thanks Brian! I'll add & test it to k8s-1.31 and k8s-1.32 later Today.

iholder101 · 2025-01-14T12:34:36Z

cluster-provision/k8s/1.30/k8s_provision.sh

+# 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


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 :)

yeah, that's a possibility. My initial motivation is actually make it easier to do changes without abusing sed :)

iholder101 · 2025-01-15T08:02:26Z

cluster-provision/k8s/1.30/k8s_provision.sh

@@ -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


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?

afaiu, that's for k8s 1.28 and 1.29, that is, you have to use --config-dir plus set the env var. The line in full:

For Kubernetes v1.28 to v1.29, you can only specify --config-dir if you also set the environment variable KUBELET_CONFIG_DROPIN_DIR_ALPHA for the kubelet process (the value of that variable does not matter).

lyarwood

/lgtm

victortoso · 2025-01-17T08:33:57Z

/retest-required

We can now use a proper API for customizations. See follow-up commits. Signed-off-by: Victor Toso <[email protected]>

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

Apply similar changes done to k8s-1.30, see commits: * "k8s-1.30: enable kubelet drop-in config files" * "k8s-1.30: Use kubelet's drop-in config files" Signed-off-by: Victor Toso <[email protected]>

Apply similar changes done to k8s-1.30, see commits: * "k8s-1.30: enable kubelet drop-in config files" * "k8s-1.30: Use kubelet's drop-in config files" With this, future k8s will have this enabled too. Signed-off-by: Victor Toso <[email protected]>

victortoso · 2025-01-24T09:29:51Z

I've added it to k8s-1.31 and k8s-1.32 too. I'm still testing locally but I've updated here to get CI check too in the meanwhile.

victortoso · 2025-01-24T11:14:35Z

/retest

brianmcarey · 2025-01-27T08:44:50Z

/test check-provision-k8s-1.32

victortoso · 2025-01-27T11:38:56Z

On check-provision-k8s-1.32

"PullImage from image service failed" err="rpc error: code = Canceled desc = copying system image from manifest list: copying config: context canceled" image="quay.io/ceph/ceph:v17"
"PullImage from image service failed" err="rpc error: code = Canceled desc = copying system image from manifest list: copying config: context canceled" image="quay.io/cephcsi/cephcsi:v3.7.0"
"PullImage from image service failed" err="rpc error: code = Canceled desc = copying system image from manifest list: copying config: context canceled" image="quay.io/openshift/origin-kube-rbac-proxy@sha256:e2def4213ec0657e72eb790ae8a115511d5b8f164a62d3568d2f1bff189917e8"

Seem unrelated, but

[reset] Stopping the kubelet service
[reset] Unmounting mounted directories in "/var/lib/kubelet"
W0127 08:59:25.908183    8789 cleanupnode.go:105] [reset] Failed to remove containers: failed to stop running pod a699510b2138d036736797290145d120e7914e43d1905416f55df74e659776d4: rpc error: code = DeadlineExceeded desc = context deadline exceeded
[reset] Deleting contents of directories: [/etc/kubernetes/manifests /var/lib/kubelet /etc/kubernetes/pki]
[reset] Deleting files: [/etc/kubernetes/admin.conf /etc/kubernetes/super-admin.conf /etc/kubernetes/kubelet.conf /etc/kubernetes/bootstrap-kubelet.conf /etc/kubernetes/controller-manager.conf /etc/kubernetes/scheduler.conf]

Picked my interested... Pod a699510b213, seems like the error is related to cleanup of netns...

"MESSAGE":"time="2025-01-27 09:05:34.696875517Z" level=warning msg="Could not restore sandbox a699510b2138d036736797290145d120e7914e43d1905416f55df74e659776d4: failed to Statfs \"/var/run/netns/25937231-3dd4-4381-bd31-ff5e0ec1a4e8\": no such file or directory""

But after it shows

level=info msg="Successfully cleaned up network for pod a699510b2138d036736797290145d120e7914e43d1905416f55df74e659776d4

(Artifacts > 202501270936_sonobuoy_47c90549-e721-4be5-b88d-70bbea36af4)

So, not sure if issue is related.

victortoso · 2025-01-28T08:39:59Z

Okay, k8s-1.32 looks good. Thanks @brianmcarey
/cc @iholder101 and @lyarwood, after this is merged it should unblock an improvement of #1345

Cheers

brianmcarey

/approve

kubevirt-bot · 2025-01-30T07:47:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brianmcarey

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~~ [brianmcarey]

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

iholder101 · 2025-01-30T07:54:51Z

/retest

victortoso · 2025-01-30T13:02:20Z

Misses only the lgtm, @lyarwood, @iholder101 if you have the time

iholder101 · 2025-01-30T13:45:29Z

Thank you @victortoso! Great job!
/lgtm

kubevirt-bot requested a review from lyarwood October 11, 2024 11:58

kubevirt-bot added dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/S labels Oct 11, 2024

dosubot bot added the kind/enhancement label Oct 11, 2024

victortoso force-pushed the kubelet-drop-in branch from f72745a to 9e3057c Compare October 11, 2024 12:00

kubevirt-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 9, 2025

kubevirt-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 14, 2025

victortoso mentioned this pull request Jan 14, 2025

Add ability to enable Kubernetes feature gates and runtimeconfig in KubeVirt CI #1345

Open

8 tasks

iholder101 reviewed Jan 14, 2025

View reviewed changes

kubevirt-bot assigned iholder101 Jan 14, 2025

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 14, 2025

kubevirt-bot requested review from brianmcarey and dhiller January 14, 2025 13:06

iholder101 reviewed Jan 15, 2025

View reviewed changes

lyarwood approved these changes Jan 16, 2025

View reviewed changes

kubevirt-bot assigned lyarwood Jan 16, 2025

victortoso added 4 commits January 24, 2025 09:56

k8s-1.30: enable kubelet drop-in config files

ca1354d

We can now use a proper API for customizations. See follow-up commits. Signed-off-by: Victor Toso <[email protected]>

k8s-1.31: enable and use kubelet's drop-in config files

1121f66

Apply similar changes done to k8s-1.30, see commits: * "k8s-1.30: enable kubelet drop-in config files" * "k8s-1.30: Use kubelet's drop-in config files" Signed-off-by: Victor Toso <[email protected]>

victortoso force-pushed the kubelet-drop-in branch from 9e3057c to 7c80681 Compare January 24, 2025 09:28

kubevirt-bot added size/M and removed lgtm Indicates that a PR is ready to be merged. size/S labels Jan 24, 2025

brianmcarey reviewed Jan 30, 2025

View reviewed changes

kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2025

kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubelet drop in #1299

Kubelet drop in #1299

victortoso commented Oct 11, 2024

kubevirt-bot commented Jan 9, 2025

victortoso commented Jan 14, 2025

iholder101 left a comment

iholder101 Jan 14, 2025

victortoso Jan 14, 2025

iholder101 Jan 14, 2025

victortoso Jan 17, 2025

victortoso Jan 17, 2025

brianmcarey Jan 17, 2025

victortoso Jan 17, 2025

iholder101 Jan 14, 2025

victortoso Jan 14, 2025

iholder101 Jan 15, 2025

victortoso Jan 16, 2025

lyarwood left a comment

victortoso commented Jan 17, 2025

victortoso commented Jan 24, 2025

victortoso commented Jan 24, 2025

brianmcarey commented Jan 27, 2025

victortoso commented Jan 27, 2025

victortoso commented Jan 28, 2025

brianmcarey left a comment

kubevirt-bot commented Jan 30, 2025

iholder101 commented Jan 30, 2025

victortoso commented Jan 30, 2025

iholder101 commented Jan 30, 2025

Kubelet drop in #1299

Are you sure you want to change the base?

Kubelet drop in #1299

Conversation

victortoso commented Oct 11, 2024

kubevirt-bot commented Jan 9, 2025

victortoso commented Jan 14, 2025

iholder101 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lyarwood left a comment

Choose a reason for hiding this comment

victortoso commented Jan 17, 2025

victortoso commented Jan 24, 2025

victortoso commented Jan 24, 2025

brianmcarey commented Jan 27, 2025

victortoso commented Jan 27, 2025

victortoso commented Jan 28, 2025

brianmcarey left a comment

Choose a reason for hiding this comment

kubevirt-bot commented Jan 30, 2025

iholder101 commented Jan 30, 2025

victortoso commented Jan 30, 2025

iholder101 commented Jan 30, 2025