Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Kubelet drop in #1299

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Kubelet drop in #1299

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5c65ec7
k8s-1.30: enable kubelet drop-in config files
victortoso Oct 11, 2024
9e3057c
k8s-1.30: Use kubelet's drop-in config files
victortoso Oct 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions cluster-provision/k8s/1.30/k8s_provision.sh
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @dhiller @brianmcarey

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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
Comment on lines +151 to +166
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

cat <<EOT >/etc/sysconfig/kubelet
KUBELET_EXTRA_ARGS=--cgroup-driver=systemd --runtime-cgroups=/systemd/system.slice --fail-swap-on=false --kubelet-cgroups=/systemd/system.slice
KUBELET_EXTRA_ARGS=--runtime-cgroups=/systemd/system.slice --config-dir=$kubelet_conf_d
EOT

# Enable userfaultfd for centos9 to support post-copy live migration.
Expand Down