Kubeadm feature flags should be mutable about cluster-api HOT 12 CLOSED

Sounds good. If not strictly required, I would like to avoid having handling for specific kubeadm feature gates in CAPI (eventually the feature gate will go away anyway)

from cluster-api.

/assign

from cluster-api.

If we change the webhook to make the field mutable, what is then the current rollout behavior without further changes?

I don't know if a rollout will be triggered by the change. I'll check the code and maybe also test it with our fork.
In our case with the EtcdLearnerMode we don't even need a rollout. The next rollout will enable the feature gate and it is also only necessary for new nodes.

from cluster-api.

Sounds good to me.

Also it is not necessary to rollout the KCP immediately after the change.

If we change the webhook to make the field mutable, what is then the current rollout behavior without further changes?

from cluster-api.

cc @fabriziopandini maybe you have some thoughts on this 🙂

from cluster-api.

@sbueringer so I just tested the mutation of the kubeadm feature gates by deleting the validation webhook capi-kubeadm-control-plane-validating-webhook-configuration.
After adding a kubeadm feature gate the KCP controller will trigger a rolling update immediately. So I think this is exactly what we are trying to achieve, right?

Example

Patch does not work

❯ k -n c42pc004 get kcp
NAME                     CLUSTER    INITIALIZED   API SERVER AVAILABLE   REPLICAS   READY   UPDATED   UNAVAILABLE   AGE   VERSION
c42pc004-control-plane   c42pc004   true          true                   1          1       1         0             13h   v1.27.9
❯ kubectl -n c42pc004 patch kcp c42pc004-control-plane --type='merge' -p '{"spec":{"kubeadmConfigSpec":{"clusterConfiguration":{"featureGates":{"EtcdLearnerMode": true}}}}}'
The KubeadmControlPlane "c42pc004-control-plane" is invalid: spec.kubeadmConfigSpec.clusterConfiguration.featureGates.EtcdLearnerMode: Forbidden: cannot be modified

Remove the validation webhook

❯ k delete validatingwebhookconfigurations capi-kubeadm-control-plane-validating-webhook-configuration
validatingwebhookconfiguration.admissionregistration.k8s.io "capi-kubeadm-control-plane-validating-webhook-configuration" deleted

Patch the KCP

❯ kubectl -n c42pc004 patch kcp c42pc004-control-plane --type='merge' -p '{"spec":{"kubeadmConfigSpec":{"clusterConfiguration":{"featureGates":{"EtcdLearnerMode": true}}}}}'
kubeadmcontrolplane.controlplane.cluster.x-k8s.io/c42pc004-control-plane patched
❯ k -n c42pc004 get kcp
NAME                     CLUSTER    INITIALIZED   API SERVER AVAILABLE   REPLICAS   READY   UPDATED   UNAVAILABLE   AGE   VERSION
c42pc004-control-plane   c42pc004   true          true                   2          1       1         1             13h   v1.27.9

And to verify that the feature gate is working:

Jan 24 17:05:34 c42pc004-control-plane-7646d458b6-fjkcn cloud-init[658]: [2024-01-24 17:05:34] I0124 17:05:34.688405    3391 etcd.go:416] [etcd] Adding etcd member as learner: 68747470733a2f2f3139322e3136382e302e37393a32333830
Jan 24 17:05:34 c42pc004-control-plane-7646d458b6-fjkcn cloud-init[658]: [2024-01-24 17:05:34] I0124 17:05:34.781981    3391 etcd.go:506] [etcd] Promoting a learner as a voting member: 8c3d38ccb8d86480

from cluster-api.

trying to understand what are the implications.

does this mean that the KCP controller will persist an FG in kube-system/kubeadm-config.ClusterConfiguration ?
on upgrade in a later version if kubeadm join then downloads the ClusterConfiguration and finds a legacy FG, we need to make sure it does not fail. i forgot what's the existing behavior in kubeadm / need to check.

from cluster-api.

Hm not sure. The proposed change here is mostly about changing the webhook. Not sure what the implications are regarding the ConfigMap

from cluster-api.

kube-system/kubeadm-config.ClusterConfiguration is a versioned API.

thus when converting to internal type a validation will trigger:
https://github.com/kubernetes/kubernetes/blob/0e945e5cec50028bf5ad00c19a710dfc50ddec9f/cmd/kubeadm/app/apis/kubeadm/validation/validation.go#L612

i don't recall instances where in kubeadm we manually removed a FG from the ClusterConfiguration CM, during upgrade.
which means we likely left this action to the user.

in CAPI, on kubeadm join the CM is downloaded, converted to internal type and kubeadm join can fail. this means the CAPI admin must manually remove a legacy FG from kubeadm's ClusterConfiguration if they want the cluster to allow upgrade or machine scale.

from cluster-api.

/triage approved

+1 for the change
As @neolit123 noted out, if we make some field mutable and they are expected to be persisted in the kubeadm config map (or in some other kubeadm managed resource), we should make sure this happens as well.

We already have several examples of this in https://github.com/fabriziopandini/cluster-api/blob/67c222eaaddbee7c48f5514d9f1d86c3c086069d/controlplane/kubeadm/internal/controllers/upgrade.go#L57-L123 (as well as the framework for managing kubeadm config versions)

from cluster-api.

@fabriziopandini: The label(s) triage/approved cannot be applied, because the repository doesn't have them.

from cluster-api.

/triage accepted

from cluster-api.