Machine pool nodes are not rolled during update

[calvix]

Issue

The machine pool nodes are not rolled if the KubeadmConfig is changed and the nodes needs to be manually rolled.

Details

I noticed during the thunder update, as there were many changes to both cluster-aws and other stuff, it looked like it rolled nodes but the changes for proxy were not rolled as well, once I had deleted the nodes manually they came up with proper config.

We should first confirm if the scenario is reproducible, and if it is we need to find a solution or be aware of the issue.

How to try to reproduce:

• trigger a cluster update that includes a roll of worker nodes
• once the update is triggered make a change to a configuration that would trigger yet another change causing and roll of worker nodes
• observe if the second change is applied to workers or not

[fiunchinho]

I think it's this bug kubernetes-sigs/cluster-api-provider-aws#4071

[alex-dabija]

I've encountered a similar situation when other machine pool settings were updated. I ended up manually rolling the nodes from the AWS console.

[primeroz]

FYI CAPz has the a very similar issues with MachinePool Roll updates , https://github.com/giantswarm/giantswarm/issues/25188 , reason why we reverted to MachineDeployments

[AndiDog]

Here's why this happens: upstream comment. I'll try to figure out implementation ideas for the fix.

[AndiDog]

In kubernetes-sigs/cluster-api-provider-aws#4071 (comment), I figured that cluster-api changes are also needed to speed up time until nodes get refreshed.

Therefore I started working on making our fork https://github.com/giantswarm/cluster-api ready to work with. First change to be upstream: kubernetes-sigs/cluster-api#8586 so we can build only the production components controller images of CAPI, but avoid building not required components (clusterctl, test stuff). Once merged, our fork should not differ from upstream apart from our CircleCI config.

[AndiDog]

First part of the fix in CAPA: kubernetes-sigs/cluster-api-provider-aws#4245

[T-Kukawka]

@AndiDog do u know if there was any decision made?

[AndiDog]

Next step is to discuss the issue in the CAPI office hours, since in the CAPA meeting we noted that it's generic across providers and CAPI needs to be adapted at best.

[AndiDog]

Continuing the discussion with CAPI maintainers in the new issue kubernetes-sigs/cluster-api#8858 and Slack thread.

[AndiDog]

I'm writing down a proposed solution which requires some contract from CAPI to infra providers (e.g. CAPA) so they can get a checksum of the user data without the bootstrap token – or in other terms, a checksum of KubeadmConfig.spec since we want to roll node on changes to that. That should then trigger some further discussion with upstream maintainers, since it's more actionable. The proposal will go into kubernetes-sigs/cluster-api#8858 very soon, so I won't repeat it here.

In the meantime, I want to try a workaround: CAPA always rolls out the new launch template if AWSMachinePool.spec.additionalTags changes (see if needsUpdate || tagsChanged || ... in code). If we put a checksum of KubeadmConfig.spec in such a tag, we would therefore enforce rolling nodes. The timing issue in CAPI still remains, meaning that the user data (alias bootstrap data secret) only gets updated every 5-15 minutes because CAPI only updates when the bootstrap token TTL is about to expire. That's however much easier to fix and does not need any discussion, since we've already discussed this as clear bug in the upstream meetings. I have a half-ready change that was already working for me.

[AndiDog]

The proposed workaround, i.e. adding a hash of the machine pool's KubeadmConfig.spec as tag value into AWSMachinePool.spec.additionalTags, forces node rollout just fine. However, due to CAPI not immediately updating the bootstrap secret (kubernetes-sigs/cluster-api#8858), the new nodes start with an old version of the launch template. Only the next rollout, if 10 minutes later, would use the new version, since CAPI by then has updated the secret. So my workaround won't work until that is fixed, and I'll concentrate on that first. But even then, I have the feeling that a race condition could occur – what if CAPA's AWSMachinePool controller first triggers a node rollout and CAPI takes a few seconds longer to update the bootstrap secret? Well, we'll give it a try.

[AndiDog]

Proposed solution in the CAPI issue: kubernetes-sigs/cluster-api#8858 (comment)

[AndiDog]

My first proposal would lead to quite some contract changes between CAPI and bootstrap providers, so not sure if it's feasible or desired.

Therefore, to get a quick turnaround, I tried the "swap the MachinePool -> KubeadmConfig object reference" workaround. Unfortunately, it doesn't work and creates miserable problems. But it's a use case that seemingly can be fixed more easily, so there's a chance that I can go forward with maintainers to patch CAPI and CAPA. Details in kubernetes-sigs/cluster-api#8858 (comment).

[JosephSalisbury]

from wg capa migration sync: critical for adidas poc (as we need to do manual work to upgrade the cluster)

[AndiDog]

I have a meeting with upstream this Friday to see how we can go on. None of the workarounds worked until now because of CAPI/CAPA bugs or shortcomings.

[AndiDog]

Working and tested solution in kubernetes-sigs/cluster-api-provider-aws#4619 (combined with newer CAPI version), so moving this to blocked until we progress on the upstream PRs

[AndiDog]

We're almost done here. CAPI/CAPA forks and cluster-aws have the feature. Waiting for the upstream PR (targeting CAPA v2.4.0 because it's a new feature) kubernetes-sigs/cluster-api-provider-aws#4619 to be merged before closing this issue. There's also still a small docs PR open: giantswarm/docs#2028.

[AndiDog]

Upstream PR is merged now