operator: upgrade all control plane nodes first

[3u13r]

Context

Allow #3396. Since kubelets must not communicate with a KubeAPI Server that has an older version than the kubelet itself, we need to upgrade all control planes, before upgrading the worker nodes. Control plane nodes are configured in a way that they only talk to the local KubeAPI server that matches the kubelet version.

Proposed change(s)

• Allow increasing the node budget: This is technically a full feature that I thought we already had and I needed it for the env test that tries to upgrade the worker nodes so I can verify that the control plane pending node is created but not the worker node. This should fail because of our explicit check and node the missing node budget.
• Generally improve test coverage of the nodeversion env test since we are upgrading 2 nodes in different scaling groups now.
• Don't call out to the cloud provider API to create new worker nodes if there are still control planes in the outdated or donor category, even if we would have enough node budget to do so.
• Since the operator code gen make target didn't work for me, I bumped the version (this seemingly was needed so that this plays nicely with go workspaces), reverted the make target scripts back to their original form and manually copied over the newly generated file to the cli helm embedding (we might want to automate this in the future when we use bazel for all parts of the operator).

How to test:

bazel test --test_output=all --cache_test_results=no //operators/constellation-node-operator/controllers:controllers_test

Related issue

kubernetes/kubernetes#127316

Checklist

• Run the E2E tests that are relevant to this PR's changes
  • gcp-snp, 3:2, v1.30.4 -> v1.31.1 K8s, v1.18.0 -> head of this PR: https://github.com/edgelesssys/constellation/actions/runs/11431369883
  • gcp-snp, 3:2, v1.30.4 -> v1.31.1 K8s, v1.18.0 -> head of this PR, image to upgrade to: ref/euler-feat-operator-upgrade-control-planes-first/stream/console/v2.19.0-pre.0.20241021015702-d0e8a385bb2f: https://github.com/edgelesssys/constellation/actions/runs/11455280858
• Add labels (e.g., for changelog category)

• Link to Milestone

[daniel-weisse]

Is this problematic for upgrades, since we never upgrade the installed CRDs through helm?

[3u13r]

No, I think helm has some upgrade magic build in regarding CRDs. But this time we only add one field that is optional, we are fine. Proof: https://github.com/edgelesssys/constellation/actions/runs/11455280858/job/31871905456.
Or do you have a concrete error in mind, I'm missing?

[daniel-weisse]

As far as I know, Helm simply doesn't do anything if a CRD already exists: https://helm.sh/docs/chart_best_practices/custom_resource_definitions/#method-1-let-helm-do-it-for-you

So if we change the CRDs, running helm apply won't actually apply these changes.
From what I can tell, this effectively makes the maxNodeBudget option in the nodeversion crd non-functional.
I'm assuming everything still works fine because we default to 1 if the value is not set in the CR

[3u13r]

Oh, thanks for the hint. Yes, the behavior is (hopefully) exactly the same. I think we are good for now then (also since the e2e test passed). I didn't want to advertise this feature anyway, but even if we do we should add this constraint.
In the future(tm), we likely want to do what others do (e.g., istio: https://istio.io/latest/docs/setup/upgrade/helm/#canary-upgrade-recommended).

[burgerdev]

Suggested change

	// ControlPlaneRoleLabel label used to identify control plane nodes.
	// ControlPlaneRoleLabel label used to identify control plane nodes.
	// https://kubernetes.io/docs/reference/labels-annotations-taints/#node-role-kubernetes-io-control-plane

Nit, just to document that this is canonical.

[burgerdev]

Does this need a write lock now?

[burgerdev]

I'm against making this a user-facing feature for now, because we did not discuss its semantics sufficiently. What if the user sets it to 1000 - do we replace all control-plane nodes at once? Should we have different budgets for control planes and for workers? Could this be relative? Should we rather implement a different upgrade algorithm (3533 etc)? I'm also reminded of the evolution of https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#rollingupdatedeployment-v1-apps.

Afaict this PR would be much smaller if we removed this feature and tried to find a different way to test it.

[3u13r]

do we replace all control-plane nodes at once?

Yes, I also tested it setting the budget to 5 when I had 3:2 nodes. But "at-once" only applies to the world how the operator sees it. The join service still forbids multiple control-plane nodes joining at the same time. Note that this is the scenario right after initializing a Constellation with >=3 control planes. Also replace means, adding the nodes first. So in theory you could go from 3 control planes to 6 since the operator only removes a node once the hand over is finished.

Should we have different budgets for control planes and for workers?

We might do that in the future, if a customer requires it or we think we need it.

Could this be relative?

I assume as in a percentage value. Sure, but this is more difficult/complex then setting the number of nodes.

Should we rather implement a different upgrade algorithm

I think the bug is orthogonal to this proposal, since I'm not reworking the operator replacement algorithm, which would be a large undertaking in my opinion.

I'm also reminded of the evolution of https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.30/#rollingupdatedeployment-v1-apps

I don't know the evolution, is there a summary somewhere of what happened?

Note that also the operator API is on version v1alpha1, so in my opinion, we don't have to provide any API stability guarantees between Constellation versions and we can completely change the whole upgrade process and APIs between Constellation versions.

Some of the current fields are not really user-facing in a sense that the user should directly use them. Changing the image reference requires 1. the image reference to be one of our images references 2. the measurements in the join config to match the image. Changing the k8s version requires a config map under that name and for the Constellation to upgrade correctly it has to contain the right set of components and patches.

Afaict this PR would be much smaller if we removed this feature and tried to find a different way to test it.

Then I'll have another try at the test for this, but this might take a bit of time before I get back to this.

[github-actions]

Coverage report

Package	Old	New	Trend
bootstrapper/internal/kubernetes/k8sapi	13.10%	14.00%	↗️
internal/constellation/kubecmd	62.40%	75.50%	↗️
internal/versions	9.70%	9.70%	🚧
operators/constellation-node-operator/api/v1alpha1	0.00%	0.00%	🚧
operators/constellation-node-operator/controllers	30.80%	32.00%	↗️
operators/constellation-node-operator/internal/constants	[no test files]	[no test files]	🚧
operators/constellation-node-operator/internal/controlplane	100.00%	100.00%	↔️
operators/constellation-node-operator/internal/etcd	65.80%	65.80%	↔️
operators/constellation-node-operator/internal/node	100.00%	100.00%	↔️

[msanft]

Since the operator code gen make target didn't work for me

Do you remember what the issue was there? We might want to make a ticket for it.

[msanft]

The changes seem reasonable to me both on the abstract and implementation level.
However, I lack the expertise to be a 100% confident in reviewing this.
Running some tests again:

• AWS
• Azure
• GCP

[burgerdev]

Since the operator code gen make target didn't work for me

Do you remember what the issue was there? We might want to make a ticket for it.

Fwiw, this part was already merged as #3653.

[burgerdev]

This was merged as #3663