Cluster-api: KubeadmControlPlane Spec should be fully mutable

[rudoi]

User Story

As an operator I would like to be able to modify a KubeadmControlPlane in the following ways:

1. change .spec.version
2. change .spec.infrastructureTemplate
3. change .spec.kubeadmConfigSpec

Each of these changes should produce a result (upgrade kubernetes, change a control plane instance type, modify a kube-apiserver argument).

Detailed Description

We have some plans for the KubeadmControlPlane involving upgrade. An upgrade, in summary, means making new machines one by one, removing etcd members, checking for the kubeadm configmap, etc.

I think, if possible, we should just consider this control plane Machine replacement. This gets us a handful of other use cases for free, like modifying cluster configuration, changing infrastructure configuration, etc. The Machines behind the KubeadmControlPlane remain immutable but the control plane itself becomes flexible.

To highlight a specific use case, we have some Cluster API clusters that were not provisioned with TopologyManager, and we really need to enable that functionality. I was assuming initially that the upgrade capabilities in v1a3 would help us cover that by providing for Machine replacement. So, for example, we can get both an upgrade and new apiserver arguments simultaneously by changing the spec of a KubeadmControlPlane object.

I think we should consider this before implementing a Machine replacement flow that is _specific_ to upgrades.

Also, sorry I didn't know if this should be a feature or a proposal.

/kind feature

[ncdc]

1 and 2 should already be in the works or implemented.

3 is a bit trickier (and currently disallowed via webhook validation). Once kubeadm init has executed, we're only doing kubeadm joins going foward, so any changes to the ClusterConfiguration or InitConfiguration in the KubeadmConfigSpec are not evaluated. The JoinConfiguration is something we could consider allowing to be mutated, so future control plane Machines could take advantage of whatever changes are in there. The topology manager, for example, appears to be a flag you set on the kubelet, so you probably could modify JoinConfiguration.NodeRegistrationOptions.KubeletExtraArgs to enable this (if we allowed mutations).

There's also a use case where someone might need to adjust the apiserver flags when upgrading to a newer minor version (switching from in-tree AWS EBS to external CSI for EBS, for example). Currently we'd need to adjust the ConfigMap that kubeadm stores in kube-system that contains the InitConfiguration and ClusterConfiguration. cc @fabriziopandini @randomvariable

I am 💯 in favor of figuring out how to do this sort of stuff. I'd also like to make sure we take our time with the design, so I'd ask that we do this in v0.3.x or possibly v0.4.0, depending on timing & what the changes look like. WDYT?

[rudoi]

Yeah!

No issues with taking our time. I was confident that there were some mechanics in the Kubeadm provider/Kubeadm itself that may complicate this, but I wanted to make sure I called it out as a use case.

I can take the lead on a proposal/design/etc if that would help.

[fejta-bot]

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.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[sethp-nr]

/remove-lifecycle stale

I think the third item is still a valid feature request. It seems likely to tug on / help define #2769 .

[dhawal55]

@ncdc If I understand correctly, it is possible to update apiserver flags and other configuration by updating the kubeadm-config ConfigMap in kube-system and manually rolling the nodes.

[ncdc]

@dhawal55 I believe that will work

[dhawal55]

@ncdc Thank you. I tested it and it works. Do you know if progress is made in this regard and are you targeting a specific release milestone for this change?

[ncdc]

@dhawal55 I am not aware of any updates. Would you have time to prepare a PR for it?

[Arvinderpal]

+1 We (airshipctl community) are building out clusters using CAPI and need finer control over the kubeadm configuration parameters. We also make use of the latest/cutting-edge features in K and would like to upgrade our existing clusters as those features get incorporated into kubeadm.

One option here is to introduce a run-time flag to KCP (e.g. --enable-expert-mode) that bypasses validation checks, and would essentially do the following:

allowedPaths := [][]string{
  {"spec", "*"},
  ...

See https://github.com/kubernetes-sigs/cluster-api/issues/3014 which brings mutability to PreKubeadmCommands, PostKubeadmCommands and Files. This functionality has recently been merged.

Also, related to this discussion is https://github.com/kubernetes-sigs/cluster-api/issues/1584 -- which wants to open up KubeletConfiguration and KubeProxyConfiguration.

[ncdc]

We should try to find a solution that works for everyone, and I'd prefer to avoid adding "expert mode" flags to enable something like this.

Additionally, we have to add logic that copies changes made to the KCP spec to the kubeadm ConfigMap that lives inside the workload cluster - it's not sufficient to solely set allowedPaths to *.

@Arvinderpal would you be able to enumerate which specific fields you'd like to manipulate, and we can consider them on a case by case basis?

Thanks!

[detiber]

Part of the issue with updating the kubeadm configmap will be that the version of the kubeadm config stored there will be dependent on the version of kubeadm that was used to create it, and unless that version is < v1.15 I suspect it will be a v1beta2 configuration rather than the v1beta1 configuration that we have embedded in our types.

[mariusgrigoriu]

Wouldn't the strategy of taking on individual fields make this a recurring issue and put cluster api always behind whatever capabilities come out in kubeadm? How might we allow the whole config file to be regenerated?

[detiber]

Wouldn't the strategy of taking on individual fields make this a recurring issue and put cluster api always behind whatever capabilities come out in kubeadm? How might we allow the whole config file to be regenerated?

Potentially yes, but we also need to be able to target a least common denominator configuration format to support the minimum k8s version that we expect to support in cluster-api.

Treating the kubeadm config as an opaque blob leads to a few different issues:

• We can't validate the config for users nor provide any type of feedback on why it may be failing to bootstrap/join a cluster
• We can't handle any automation around upgrade related tasks for the cluster
• We fully externalize the complexity to the user to provide a kubeadm config that works for a given kubernetes version they are requesting, rather than providing any assurances on the Cluster API side as to which versions of kubernetes/kubeadm are supported

[Arvinderpal]

@Arvinderpal would you be able to enumerate which specific fields you'd like to manipulate, and we can consider them on a case by case basis?

Can we start with joinConfiguration.nodeRegistration.kubeletExtraArgs and also expose (and allow mutation of) KubeletConfiguration and KubeProxyConfiguration (https://github.com/kubernetes-sigs/cluster-api/issues/1584)?

More than happy to do the leg work in implementing these changes, if we can reach some consensus here.

[Arvinderpal]

Part of the issue with updating the kubeadm configmap will be that the version of the kubeadm config stored there will be dependent on the version of kubeadm that was used to create it, and unless that version is < v1.15 I suspect it will be a v1beta2 configuration rather than the v1beta1 configuration that we have embedded in our types.

Can we assume v1beta1 for now?

There is a separate issue for v1beta2 here: https://github.com/kubernetes-sigs/cluster-api/issues/3150

[detiber]

Can we assume v1beta1 for now?

For mutating the stored kubeadm config in the workload cluster, unfortunately not. It already is v1beta2 in the general case today and the config map handling we have today around it just happens to get away with there being high similarity between the versions.

There is a separate issue for v1beta2 here: #3150

That issue is more around being able to consume the v1beta2 config and leveraging the v1beta2 config for bootstrapping nodes.

[ncdc]

I would be in favor of joinConfiguration.nodeRegistration.kubeletExtraArgs, as I believe that is self-contained (does not have to be copied to the ConfigMap). Does anyone foresee any issues here (other than the possibility that you might specify args that prevent the kubelet from running correctly)?

We should probably continue to use #1584 for fleshing out the component config ideas.

[ncdc]

For v1beta1 vs v1beta2 in the ConfigMap, we are currently working with the data as *unstructured.Unstructured, which means we don't specifically to have convert to/from versioned kubeadm go types, but @detiber is correct that we have been lucky up til now that things are generally compatible in this space between v1beta1 and v1beta2. We may need to start adding logic that checks the apiVersion of the ClusterConfiguration in the ConfigMap, and then alter our behavior based on version.

[fejta-bot]

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.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[vincepri]

/lifecycle frozen

[vincepri]

/milestone v0.4.0

[vincepri]

/remove-lifecycle frozen
/help
/priority important-soon

[hh]

/cc

[detiber]

I believe @shysank is currently looking into this
/assign @shysank
/lifecycle active
/remove-help

[k8s-ci-robot]

@detiber: GitHub didn't allow me to assign the following users: shysank.

Note that only kubernetes-sigs members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

I believe @shysank is currently looking into this
/assign @shysank
/lifecycle active
/remove-help

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/test-infra repository.

[shysank]

@vincepri @detiber should we open a new issue to track the remaining immutable kcp specs or just reopen this one?

[detiber]

@shysank I'm fine either way

[shysank]

/reopen

[k8s-ci-robot]

@shysank: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to this:

/reopen

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/test-infra repository.

[shysank]

@rudoi @detiber Would you mind re-opening since I don't have access?

[detiber]

/reopen

[k8s-ci-robot]

@detiber: Reopened this issue.

In response to this:

/reopen

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/test-infra repository.