-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
sig-cl/kubeadm/1739: update KEP for support of patching kubelet config #3312
sig-cl/kubeadm/1739: update KEP for support of patching kubelet config #3312
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: neolit123 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:
Approvers can indicate their approval by writing |
discussed here: related to: /cc @pacoxu |
ee449f3
to
6d38b8b
Compare
- Generalize the KEP to not only target static pods for the control plane - Add "kubeletconfiguration" target
6d38b8b
to
c03f84d
Compare
|
||
During `init` patches for the kubelet are applied over the user provided and kubeadm defaulted | ||
`KubeletConfiguration`. During `join/upgrade` patches are applied over the downloaded | ||
from the cluster global `KubeletConfiguration`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update LGTM.
My only concern is about how to make a new user notice this option and there is a patch for some components.
- the documentation
- maybe we can add some tips in the configmap/static-pod annotation like "kubeadm.kubernetes.io/patched.component=kube-apiserver".
- some logs during kubeadm
init/join/upgrade
process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the presence of a --patches flag or the option in config is the main indicator that components are being patched. And the user technically owns / maintains these flags and the config, so they must know what they are passing to kubeadm.
the documentation
I plan to update this page:
https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/control-plane-flags/
maybe we can add some tips in the configmap annotation like "kubeadm.kubernetes.io/patched.component=kube-apiserver".
Today we do not annotate static pods whether they are modified from kubeadm defaults. There is this extra customization layer due to extraArgs, volumes and mounts that applies over the defaults. We assume the config file is the source of truth and users will be able to trace back changes to it. Patches are also a customization layer and we assume that new users should look at the node config and see there is a patches directory.
I think such an annotation is doable, but its not clear to me whether it's going to be a net positive addition for us and the users.
some logs during kubeadm init/join/upgrade process.
Yes, logs already print that a patch folder is being processed and what patches were found there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
i'm considering doing a POC PR first, before merging this update in case we see something has to be adjusted. EDIT: POC is here: |
the implementation seems OK. we just patch before writing the kubelet config to disk. /hold cancel |
One-line PR description:
control plane
kubeadm: customization with patches #1739
NONE