From 0d3bcfb16093af65e4b808c52e043821340adb3a Mon Sep 17 00:00:00 2001 From: Ashish Jaiswal Date: Thu, 9 Jul 2026 16:56:16 +0530 Subject: [PATCH] feat(cluster): day-2 sync for hcloud instance types and control-plane replicas Editing machineType or controlPlane.hcloud.replicas in general.yaml had no effect on a running cluster. Only `bootstrap` re-renders values-capi-cluster.yaml; `cluster sync` refused every non-bare-metal provider, and `cluster upgrade` only patched the Kubernetes version and the machine image. `cluster sync` now handles Hetzner: re-render values-capi-cluster.yaml (plus the verbatim general.yaml copy), show an inline unified diff, open the PR, sync the capi-cluster ArgoCD app, and wait for the control plane to converge. Nothing here deletes and recreates a MachineTemplate, which is what the design in docs/TODO.md originally called for. ClusterAPI decides a Machine is up to date by comparing the *name* of the template it was cloned from against the name its owner references -- never the template's contents. Recreating a template under its old name therefore rolls nothing, and the new instance type applies silently to machines created later. The capi-cluster chart instead names templates after a hash of their spec, so a machineType change rotates the name and CAPI rolls normally, through plain GitOps. Requires the matching kubeaid chart change, opted into per-cluster via cluster.machineTemplateRotation. `cluster upgrade` skips UpdateMachineTemplate on rotation-enabled clusters -- the fixed-name object it deletes does not exist there -- and syncs the whole app so the newly-named template lands. Both commands now confirm which live cluster they are about to touch. Nothing in kubeaid-cli ever selects a kubeconfig context: every client is built with clientcmd.BuildConfigFromFlags("", path) and silently follows whatever current-context the kubeconfig carries. The gate prints the cluster name from general.yaml, the resolved context, API server and kubeconfig path, plus a loud MISMATCH line when they disagree. Declining, a missing kubeconfig or no TTY all abort. planCapiValuesSync then refuses two changes outright, because both fail silently: - a rotating change while machineTemplateRotation is false (nothing would roll) - a rotating change combined with a replicas change (asks CAPI to both add and replace control-plane members, walking etcd through membership changes with no quorum to lose) and warns when a rolling change would run against fewer than 3 replicas. For netbird-obmondo-com (1 x cpx32 -> 3 x cpx22) that forces the correct order: scale to 3 first on the existing type, where the template spec and therefore its name are untouched and the new members simply join etcd; then re-type, where the rotation rolls the three machines one at a time with quorum held throughout. --- cmd/kubeaid-core/root/cluster/sync/sync.go | 11 +- .../root/config/templates/general.yaml | 17 + docs/TODO.md | 90 ++---- docs/config-reference.md | 9 +- ...7-09-machinetemplate-rotation-day2-sync.md | 157 ++++++++++ go.mod | 2 +- pkg/config/general.go | 18 ++ pkg/constants/templates.go | 6 +- pkg/core/capi_values_sync_plan.go | 260 ++++++++++++++++ pkg/core/capi_values_sync_plan_test.go | 241 +++++++++++++++ pkg/core/live_cluster_context.go | 140 +++++++++ pkg/core/live_cluster_context_test.go | 152 +++++++++ pkg/core/setup_kubeaid_config.go | 14 + pkg/core/sync_cluster_capi.go | 291 ++++++++++++++++++ pkg/core/sync_cluster_kubeone.go | 13 + .../argocd-apps/values-capi-cluster.yaml.tmpl | 6 + pkg/core/upgrade_cluster.go | 30 ++ 17 files changed, 1386 insertions(+), 71 deletions(-) create mode 100644 docs/specs/2026-07-09-machinetemplate-rotation-day2-sync.md create mode 100644 pkg/core/capi_values_sync_plan.go create mode 100644 pkg/core/capi_values_sync_plan_test.go create mode 100644 pkg/core/live_cluster_context.go create mode 100644 pkg/core/live_cluster_context_test.go create mode 100644 pkg/core/sync_cluster_capi.go diff --git a/cmd/kubeaid-core/root/cluster/sync/sync.go b/cmd/kubeaid-core/root/cluster/sync/sync.go index 8c30c75c..a37a1a13 100644 --- a/cmd/kubeaid-core/root/cluster/sync/sync.go +++ b/cmd/kubeaid-core/root/cluster/sync/sync.go @@ -31,9 +31,18 @@ var SyncCmd = &cobra.Command{ SkipPRWorkflow: skipPRWorkflow, }) + // ArgoCD reconciles whatever is merged into kubeaid-config, but only `bootstrap` + // ever re-renders values-capi-cluster.yaml from general.yaml. So a machineType or + // control-plane replicas edit never reaches a provisioned ClusterAPI cluster on its + // own : re-render it, and let ArgoCD carry it the rest of the way. + case constants.CloudProviderHetzner: + core.SyncCAPICluster(cmd.Context(), core.SyncCAPIClusterArgs{ + SkipPRWorkflow: skipPRWorkflow, + }) + default: assert.Assert(cmd.Context(), false, fmt.Sprintf( - "'cluster sync' is only needed for the Bare Metal (KubeOne) provider - on %s, merged kubeaid-config changes get reconciled by ArgoCD", + "'cluster sync' does not support %s yet - re-run 'cluster bootstrap' to re-render values-capi-cluster.yaml", globals.CloudProviderName, )) } diff --git a/cmd/kubeaid-core/root/config/templates/general.yaml b/cmd/kubeaid-core/root/config/templates/general.yaml index 03d92641..79f91858 100644 --- a/cmd/kubeaid-core/root/config/templates/general.yaml +++ b/cmd/kubeaid-core/root/config/templates/general.yaml @@ -58,6 +58,23 @@ cluster: # Suitable Kubernetes API configurations will be done for you automatically. And they can be # changed using the apiSever struct field. enableAuditLogging: True + # MachineTemplateRotation names the capi-cluster chart's HCloud MachineTemplates + # after a hash of their spec, so that changing a machineType rotates the name. + # + # ClusterAPI decides whether a Machine is up to date by comparing the name of + # the template it was cloned from against the name its owner references — it + # never compares the template's contents. Without rotation, a machineType change + # therefore never rolls the cluster: it applies silently to machines created + # later, leaving a control plane with mixed instance types. + # + # Enabling this on an existing cluster renames the template even when its spec + # is unchanged, which rolls the control plane once. Scale to at least 3 + # control-plane replicas first, so etcd keeps quorum while the machines are + # replaced one at a time. + # + # Hetzner HCloud only — bare-metal workers are Machine objects, not + # MachineDeployments, and their templates keep the fixed legacy names. + machineTemplateRotation: False # ACMEEmail is the contact email used to register with the ACME # CA (Let's Encrypt) when cert-manager's ClusterIssuer is # rendered. Required when cluster.keycloak.mode=managed (the diff --git a/docs/TODO.md b/docs/TODO.md index 01093d8e..bc4e0cf7 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -168,67 +168,29 @@ kubeaid-cli with the offending path, same shape as the parser's existing `validate` errors. Defer until we hit the next case from a different field; the regions one is fixed at source. -### Day-2 `cluster sync` for cloud instance-type changes (AWS / Azure / hcloud) - -Changing a machine `instanceType` (control-plane or a node group) in -general.yaml has no day-2 propagation for a running cloud cluster. -`cluster sync` hard-refuses every non-bare-metal provider -(`cmd/kubeaid-core/root/cluster/sync/sync.go` — "only needed for the Bare -Metal (KubeOne) provider"), and `cluster upgrade` only patches -`.global.kubernetes.version` + the machine image -(`pkg/core/upgrade_cluster.go`, `pkg/cloud/*/cluster_upgrade.go`) — it never -re-renders `instanceType`. Only `bootstrap` re-renders the full -`values-capi-cluster.yaml`. So today you re-run bootstrap or hand-edit the -rendered values file. - -Goal: extend `cluster sync` to AWS/Azure/hcloud so an `instanceType` change -in general.yaml reconciles onto the live cluster — with a kubeone-style -inline diff + double approval, because this runs on live clusters and rolls -nodes. - -Flow (reuses existing primitives): - -1. Re-render config from general.yaml → new `values-capi-cluster.yaml` (+ the - verbatim `kubeaid-cli.general.yaml` copy) via `getTemplateValues` + - `createOrUpdateNonSecretFiles` (`pkg/core/setup_kubeaid_config.go`). -2. **Inline diff** of the re-rendered files → confirm (approval #1). NEW — - there is no inline diff/plan today; the only diff is the forge PR diff. -3. Create PR → `git.WaitUntilPRMerged` (approval #2 = the merge). -4. **Delete+recreate** the affected MachineTemplate(s) with the new - `instanceType`. Required because MachineTemplate specs are immutable and - the capi-cluster chart (kubeaid repo) names them with FIXED names - (`-control-plane`, `-md-0`, `-system` / - `-user`) — so a plain ArgoCD sync of a new instanceType is rejected by the - API server, and CAPI wouldn't roll anyway (the template ref name is - unchanged). Generalize the existing `CloudProvider.UpdateMachineTemplate` - (`pkg/cloud/cloud_provider.go:29`; AWS/Azure impls in - `pkg/cloud/*/cluster_upgrade.go`) from *image* → *instanceType*. hcloud - needs an impl too. This is exactly what `cluster upgrade` already does for - the image. -5. Sync the `capi-cluster` ArgoCD app - (`SyncArgoCDApp(constants.ArgoCDAppCapiCluster)`). -6. **Watch** the KubeadmControlPlane (and MachineDeployments) roll — reuse - `renderCAPIStatusTable` / the machine-wait logic (`pkg/utils/kubernetes/ - clusterapi.go`). A CP-template change makes the KCP auto-roll; an - MD-template change needs `clusterctl RolloutRestart` (as upgrade does). - -Also add the same inline-diff + double-approval gate to `cluster upgrade` — -it has the PR-merge gate today but no inline diff. - -Open scoping questions (decide before building): - -- Both control-plane and worker instance types (CP change → KCP roll, node - group change → that MachineDeployment rolls)? Assume yes. -- v1 = instance-type only, or also fold in the *mutable* changes (replicas, - add/remove node groups) that sync cleanly with NO template recreate? Those - are the easy, fully-GitOps path (PR + sync + watch, no delete+recreate); - instance-type is the one field needing the recreate. - -Alternative to the CLI-driven delete+recreate: make the capi-cluster chart -name MachineTemplates by a spec hash (`-md-0-`) so a new instanceType -→ new template name → ArgoCD creates it and CAPI rolls naturally (pure -GitOps, no client-go surgery). That's a kubeaid-repo chart change affecting -every cluster's rollout behaviour, so it's a separate PR — noted as the -cleaner long-term end-state if we're willing to touch the chart. - -Design captured 2026-07-08 from a brainstorming session; not yet built. +### Day-2 `cluster sync` for cloud instance-type changes (AWS / Azure) + +hcloud is done — see `docs/specs/2026-07-09-machinetemplate-rotation-day2-sync.md`. + +The delete+recreate plan this section used to carry was wrong, and worth recording +why: ClusterAPI decides a Machine is up to date by comparing the *name* of the +template it was cloned from against the name its owner references +(`matchesTemplateClonedFrom` for KCP, `MachineTemplateUpToDate` for MachineDeployments). +It never compares the template's contents. Recreating a template under its old name +therefore rolls nothing, and the new instanceType applies silently to machines +created later. `cluster upgrade` only appeared to work because the Kubernetes version +change was doing the rolling. + +The fix shipped for hcloud is to name MachineTemplates after a hash of their spec, so +a changed instanceType rotates the name and CAPI rolls normally — pure GitOps, no +client-go surgery. It is gated on `cluster.machineTemplateRotation` because adopting +it renames the template once, which costs one control-plane roll. + +Remaining: AWS and Azure name their MachineTemplates the same fixed way +(`charts/aws/templates/AWSMachineTemplate.yaml:4`, +`charts/azure/templates/AzureMachineTemplate.yaml:4`) and have the same silent no-op. +`cluster sync` still refuses both providers. Porting the `_helpers.tpl` name/spec +helpers is mechanical; the CLI side already generalises (`planCapiValuesSync` reads +only the rendered values, and would need each provider's values paths added). + +Also still open: `cluster upgrade` has the PR-merge gate but no inline diff. diff --git a/docs/config-reference.md b/docs/config-reference.md index 42d70ebc..ed59b17b 100644 --- a/docs/config-reference.md +++ b/docs/config-reference.md @@ -206,11 +206,11 @@ NOTE : Generally, refer to the KubeadmControlPlane CRD instead of the correspond |-------|------|---------|-------------| | vmSize | `string` | | | | diskSizeGB | `uint32` | | | -| minSize | `uint` | | Minimum number of replicas in the nodegroup.
| -| maxSize | `uint` | | Maximum number of replicas in the nodegroup.
| | name | `string` | | Nodegroup name.
| | labels | `map[string]string` | [] | Labels that you want to be propagated to each node in the nodegroup.

Each label should meet one of the following criterias to propagate to each of the nodes :

1. Has node-role.kubernetes.io as prefix.
2. Belongs to node-restriction.kubernetes.io domain.
3. Belongs to node.cluster.x-k8s.io domain.

REFER : https://cluster-api.sigs.k8s.io/developer/architecture/controllers/metadata-propagation#machine.
| | taints | []`k8s.io/api/core/v1.Taint` | [] | Taints that you want to be propagated to each node in the nodegroup.
| +| minSize | `uint` | | Minimum number of replicas in the nodegroup.
| +| maxSize | `uint` | | Maximum number of replicas in the nodegroup.
| ## AzureConfig @@ -353,6 +353,7 @@ REFER : https://docs.kubermatic.com/kubeone/v1.13/references/kubeone-cluster-v1b | name | `string` | | Name of the Kubernetes cluster.

We don't allow using dots in the cluster name, since it can cause issues with tools like
ClusterAPI and Cilium : which use the cluster name to generate other configurations.
| | k8sVersion | `string` | | Kubernetes version (>= 1.30.0).
| | enableAuditLogging | `bool` | True | Whether you would like to enable Kubernetes Audit Logging out of the box.
Suitable Kubernetes API configurations will be done for you automatically. And they can be
changed using the apiSever struct field.
| +| machineTemplateRotation | `bool` | False | MachineTemplateRotation names the capi-cluster chart's HCloud MachineTemplates
after a hash of their spec, so that changing a machineType rotates the name.

ClusterAPI decides whether a Machine is up to date by comparing the name of
the template it was cloned from against the name its owner references — it
never compares the template's contents. Without rotation, a machineType change
therefore never rolls the cluster: it applies silently to machines created
later, leaving a control plane with mixed instance types.

Enabling this on an existing cluster renames the template even when its spec
is unchanged, which rolls the control plane once. Scale to at least 3
control-plane replicas first, so etcd keeps quorum while the machines are
replaced one at a time.

Hetzner HCloud only — bare-metal workers are Machine objects, not
MachineDeployments, and their templates keep the fixed legacy names.
| | acmeEmail | `string` | | ACMEEmail is the contact email used to register with the ACME
CA (Let's Encrypt) when cert-manager's ClusterIssuer is
rendered. Required when cluster.keycloak.mode=managed (the
keycloakx and netbird-mgmt Ingresses both need TLS certs);
optional otherwise. Used as Issuer.spec.acme.email.
| | acmeDNS01 | [`ACMEDNS01Config`](#acmedns01config) | | ACMEDNS01 switches the rendered ClusterIssuer's solver from
the HTTP-01 default to DNS-01. Required for the split-horizon
mesh pattern: NetBird-exposed services use real public DNS
names (e.g. argocd.staging.acme.com) that only resolve inside
the mesh — Let's Encrypt can never reach them over HTTP, but
proves ownership via a TXT record on the public zone instead.
Requires cluster.acmeEmail plus the provider credential in
secrets.yaml (acme.cloudflareApiToken).
| | apiServer | [`APIServerConfig`](#apiserverconfig) | | Configuration options for the Kubernetes API server.
| @@ -456,11 +457,11 @@ We enforce the user to use SSH, for authenticating to the Git server.

| Field | Type | Default | Description | |-------|------|---------|-------------| | machineType | `string` | | HCloud machine type.
You can browse all available HCloud machine types here : https://hetzner.com/cloud.
| -| minSize | `uint` | | Minimum number of replicas in the nodegroup.
| -| maxSize | `uint` | | Maximum number of replicas in the nodegroup.
| | name | `string` | | Nodegroup name.
| | labels | `map[string]string` | [] | Labels that you want to be propagated to each node in the nodegroup.

Each label should meet one of the following criterias to propagate to each of the nodes :

1. Has node-role.kubernetes.io as prefix.
2. Belongs to node-restriction.kubernetes.io domain.
3. Belongs to node.cluster.x-k8s.io domain.

REFER : https://cluster-api.sigs.k8s.io/developer/architecture/controllers/metadata-propagation#machine.
| | taints | []`k8s.io/api/core/v1.Taint` | [] | Taints that you want to be propagated to each node in the nodegroup.
| +| minSize | `uint` | | Minimum number of replicas in the nodegroup.
| +| maxSize | `uint` | | Maximum number of replicas in the nodegroup.
| ## HCloudConfig diff --git a/docs/specs/2026-07-09-machinetemplate-rotation-day2-sync.md b/docs/specs/2026-07-09-machinetemplate-rotation-day2-sync.md new file mode 100644 index 00000000..8e41353f --- /dev/null +++ b/docs/specs/2026-07-09-machinetemplate-rotation-day2-sync.md @@ -0,0 +1,157 @@ +# Day-2 `cluster sync` for HCloud instance types, via MachineTemplate name rotation + +Status: implemented (kubeaid-cli + kubeaid `capi-cluster` chart). +Supersedes the "Day-2 `cluster sync` for cloud instance-type changes" plan in +`docs/TODO.md`, which proposed deleting and recreating the MachineTemplate. + +## The problem + +Editing `cloud.hetzner.controlPlane.hcloud.machineType` (or `replicas`) in +general.yaml had no effect on a running cluster. Only `bootstrap` re-renders +`values-capi-cluster.yaml`; `cluster sync` refused every non-bare-metal provider, +and `cluster upgrade` only patched `.global.kubernetes.version` plus the machine +image. + +## Why delete + recreate was the wrong fix + +The original plan was to delete the immutable MachineTemplate and recreate it +with the new `instanceType`, since its spec cannot be updated in place. + +That does not work, and fails silently. ClusterAPI decides whether a Machine is +up to date by comparing the **name** of the template it was cloned from against +the name its owner currently references. It never compares the template's +contents: + +- `controlplane/kubeadm/internal/filters.go:143` — `matchesTemplateClonedFrom` + compares the `cluster.x-k8s.io/cloned-from-name` annotation on the infra machine + against `kcp.Spec.MachineTemplate.Spec.InfrastructureRef.Name`. +- `internal/controllers/machinedeployment/mdutil/util.go:408` — + `MachineTemplateUpToDate` compares `Spec.InfrastructureRef` (kind + name). + +(both `sigs.k8s.io/cluster-api v1.11.10`, the version this repo pins) + +So recreating `-control-plane` with a different `type:` leaves +`clonedFromName` matching. Every Machine is judged up to date, **nothing rolls**, +and the new instance type applies only to machines created later — a control +plane with mixed instance types and no error reported anywhere. + +The existing `cluster upgrade` gets away with the same delete+recreate for the +machine *image* only because the accompanying Kubernetes version change is what +triggers the rollout; the new image just rides along on it. + +Worse, deleting a template that a KubeadmControlPlane references opens a window +where the controller cannot clone new machines at all. + +## The fix: rotate the name + +CAPI's own diagnostic — *"Infrastructure template on KCP rotated from X to Y"* — +names the sanctioned mechanism. Name the template after a hash of its spec: + + -control-plane- + -- + +Then the two day-2 changes fall out with the right semantics, through plain +GitOps, with no client-go surgery and no window where the template is missing: + +| change | template name | what ClusterAPI does | +| --- | --- | --- | +| `replicas` 1 → 3 | unchanged | KubeadmControlPlane scales out. **New members join etcd; nothing is replaced.** | +| `machineType` cpx32 → cpx22 | rotates | Rolling replacement: surge one machine, wait for it to join, remove an old one. | +| node-group `machineType` | that group's rotates | Only that MachineDeployment rolls. | + +ArgoCD creates the newly-named template and repoints the owner's +`infrastructureRef` at it in the same sync. + +### Chart changes (kubeaid) + +`argocd-helm-charts/capi-cluster/charts/hetzner/`: + +- `_helpers.tpl` gains `hetzner.hcloud{ControlPlane,NodeGroup}MachineTemplate{Spec,Name}`. + The **spec** helpers are the single definition of each template's + `spec.template.spec`; `HCloudMachineTemplate.yaml` renders them and the **name** + helpers hash them. One definition, so the hash cannot drift from the spec it names. +- `KubeadmControlPlane.yaml` and `MachineDeployment.yaml` take their + `infrastructureRef.name` from the same name helpers. + +Gated on `global.machineTemplateRotation`, default `false`. Enabling it renames +the template even when the spec is unchanged, which rolls the control plane once +— existing clusters must opt in deliberately. + +Rotated templates carry `argocd.argoproj.io/sync-options: Prune=false`. A +superseded template is still referenced by the MachineSet being scaled down; +pruning it mid-rollout would strand that MachineSet if it had to replace a +Machine. Stale templates hold no resources and can be deleted once every Machine +has moved off them. + +Bare-metal templates keep their fixed names: bare-metal workers are `Machine` +objects, not MachineDeployments, so nothing rotates. + +### CLI changes (kubeaid-cli) + +- `cluster.machineTemplateRotation` in general.yaml → `global.machineTemplateRotation` + in the rendered values. +- `cluster sync` now handles Hetzner: re-render `values-capi-cluster.yaml` (plus the + verbatim `kubeaid-cli.general.yaml` copy), show an inline unified diff, open the PR, + sync the `capi-cluster` ArgoCD app, and wait on `WaitForControlPlaneRolloutComplete`. +- `cluster upgrade` skips `UpdateMachineTemplate` on rotation-enabled clusters — under + rotation there is no `-control-plane` object to delete — and syncs the whole + app instead, so the newly-named template lands. + +## Gates + +`cluster sync` and `cluster upgrade` both mutate a live cluster, and nothing in +kubeaid-cli ever selects a kubeconfig context: every client is built with +`clientcmd.BuildConfigFromFlags("", path)`, silently following whatever +`current-context` the kubeconfig carries. + +So both commands now resolve that context and make the operator confirm it — +cluster name from general.yaml, context name, API server, kubeconfig path, and a +loud `MISMATCH` line when the context's cluster name differs from general.yaml's. +Declining, a missing kubeconfig, or no TTY all abort. A command that rolls +control-plane machines should stop, not guess. + +`planCapiValuesSync` then inspects the diff and **refuses** two changes outright, +because both fail silently: + +1. **A rotating change while `machineTemplateRotation` is false.** Nothing would + roll (see above). Told to enable rotation first. +2. **A rotating change combined with a replicas change.** That asks ClusterAPI to + both add members and replace them; starting from one control-plane node it + walks etcd through a sequence of membership changes with no quorum to lose. + Told to split it into two syncs. + +And **warns** when a rolling change would run against fewer than 3 control-plane +replicas: a rolling replacement surges one machine, waits for it to join etcd, then +removes an old one — below three replicas every intermediate membership is a single +node away from losing quorum. + +## Applying this to `netbird-obmondo-com` (1 × cpx32 → 3 × cpx22) + +The refusals above force the correct order. Two separate syncs: + +**Phase 1 — scale out.** Set `replicas: 3`, leave `machineType: cpx32`, leave +`machineTemplateRotation: false`. The template spec is untouched, so its name is +untouched: the KubeadmControlPlane simply scales, and two new control-plane +members join etcd. Nothing is replaced. Wait for all three to be Ready. + +**Phase 2 — re-type.** Set `machineTemplateRotation: true` and +`machineType: cpx22`. The template rotates, and ClusterAPI rolls the three +control-plane machines one at a time with quorum held throughout. + +Doing both in one sync is refused. + +Before phase 2, confirm `cpx22` is actually large enough for a control plane +running etcd — this is a downsize. `GetVMSpecs` resolves the type against the live +Hetzner API, so an unknown type fails at render time rather than silently. + +## Follow-ups + +- AWS and Azure name their MachineTemplates the same fixed way + (`charts/aws/templates/AWSMachineTemplate.yaml:4`, + `charts/azure/templates/AzureMachineTemplate.yaml:4`), so they have the same + silent no-op. `cluster sync` still refuses them. Porting the helpers is + mechanical. +- `cluster upgrade` has a PR-merge gate but still no inline diff. +- The `capi-cluster` chart's helm-unittest suites are not wired into CI + (no `helm unittest` invocation anywhere in the kubeaid repo), so + `tests/machine-template-rotation_test.yaml` only runs when someone runs it. diff --git a/go.mod b/go.mod index bd1d5a00..b34e0fd8 100644 --- a/go.mod +++ b/go.mod @@ -39,6 +39,7 @@ require ( github.com/mattn/go-runewidth v0.0.19 github.com/mikefarah/yq/v4 v4.50.1 github.com/muesli/termenv v0.16.0 + github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 github.com/sagikazarmark/slog-shim v0.1.0 github.com/schollz/progressbar/v3 v3.19.0 github.com/siderolabs/talos/pkg/machinery v1.10.5 @@ -361,7 +362,6 @@ require ( github.com/pjbgf/sha1cd v0.5.0 // indirect github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect github.com/pkg/errors v0.9.1 // indirect - github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/prometheus/client_golang v1.23.2 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.67.4 // indirect diff --git a/pkg/config/general.go b/pkg/config/general.go index 994069e0..f10003de 100644 --- a/pkg/config/general.go +++ b/pkg/config/general.go @@ -143,6 +143,24 @@ type ( // changed using the apiSever struct field. EnableAuditLogging bool `yaml:"enableAuditLogging" default:"True"` + // MachineTemplateRotation names the capi-cluster chart's HCloud MachineTemplates + // after a hash of their spec, so that changing a machineType rotates the name. + // + // ClusterAPI decides whether a Machine is up to date by comparing the name of + // the template it was cloned from against the name its owner references — it + // never compares the template's contents. Without rotation, a machineType change + // therefore never rolls the cluster: it applies silently to machines created + // later, leaving a control plane with mixed instance types. + // + // Enabling this on an existing cluster renames the template even when its spec + // is unchanged, which rolls the control plane once. Scale to at least 3 + // control-plane replicas first, so etcd keeps quorum while the machines are + // replaced one at a time. + // + // Hetzner HCloud only — bare-metal workers are Machine objects, not + // MachineDeployments, and their templates keep the fixed legacy names. + MachineTemplateRotation bool `yaml:"machineTemplateRotation" default:"False"` + // ACMEEmail is the contact email used to register with the ACME // CA (Let's Encrypt) when cert-manager's ClusterIssuer is // rendered. Required when cluster.keycloak.mode=managed (the diff --git a/pkg/constants/templates.go b/pkg/constants/templates.go index 4a54f11d..6542a2d8 100644 --- a/pkg/constants/templates.go +++ b/pkg/constants/templates.go @@ -52,10 +52,14 @@ var ( "argocd-apps/templates/cluster-api-operator.yaml.tmpl", "argocd-apps/values-cluster-api-operator.yaml.tmpl", "argocd-apps/templates/capi-cluster.yaml.tmpl", - "argocd-apps/values-capi-cluster.yaml.tmpl", + TemplateNameCapiClusterValues, } ) +// Rendered by `cluster sync` on its own, to reconcile a running cloud cluster onto +// general.yaml without re-rendering every other kubeaid-config file. +const TemplateNameCapiClusterValues = "argocd-apps/values-capi-cluster.yaml.tmpl" + // AWS specific template names. var ( AWSSpecificNonSecretTemplateNames = []string{ diff --git a/pkg/core/capi_values_sync_plan.go b/pkg/core/capi_values_sync_plan.go new file mode 100644 index 00000000..927d85aa --- /dev/null +++ b/pkg/core/capi_values_sync_plan.go @@ -0,0 +1,260 @@ +// Copyright 2026 Obmondo +// SPDX-License-Identifier: AGPL3 + +package core + +import ( + "fmt" + "sort" + "strings" + + "sigs.k8s.io/yaml" +) + +// capiValuesFacts is the subset of a rendered values-capi-cluster.yaml that decides +// whether a change can be reconciled onto a running cluster, and how it will land. +// +// Only HCloud is modelled. Bare-metal workers are Machine objects rather than +// MachineDeployments, so they never rotate a template. +type capiValuesFacts struct { + // MachineTemplateRotation names MachineTemplates after a hash of their spec. + MachineTemplateRotation bool + + // ImageName and the machine types below feed that hash. Changing any of them + // rotates a template name, which is the only thing ClusterAPI reacts to. + ImageName string + ControlPlaneType string + ControlPlaneReplicas int + NodeGroupTypes map[string]string +} + +// capiValuesSyncPlan describes what a values-capi-cluster.yaml change will do to a +// running cluster. +type capiValuesSyncPlan struct { + // RollingChanges rotate a MachineTemplate name, so ClusterAPI replaces machines. + RollingChanges []string + + // ReplicaChange scales the control plane. New members join etcd; nothing is + // replaced. Empty when the replica count is unchanged. + ReplicaChange string + + // RotationNewlyEnabled means the templates are about to be renamed for the first + // time. The rename alone rolls the control plane, even with an unchanged spec. + RotationNewlyEnabled bool + + // RotationEnabled is the state after the change. + RotationEnabled bool + + // ControlPlaneReplicasAfter is the replica count the control plane will roll with. + ControlPlaneReplicasAfter int +} + +// rolls reports whether ClusterAPI will actually replace machines. +// +// A rewritten template spec only rolls when rotation is on to rename the template — with +// the fixed legacy name ClusterAPI sees no change at all. That case is a refusal, not a +// roll, and must not be described to the operator as one. +func (p *capiValuesSyncPlan) rolls() bool { + if !p.RotationEnabled { + return false + } + + return len(p.RollingChanges) > 0 || p.RotationNewlyEnabled +} + +// Refusals lists the reasons this change must not be applied as a single sync. +// +// Each is a change that would either do nothing at all, or roll the control plane in +// a way that risks etcd quorum. They are hard stops rather than warnings because both +// failure modes are silent: ClusterAPI reports success in the first case, and a lost +// quorum is not recoverable by re-running the command. +func (p *capiValuesSyncPlan) Refusals() []string { + var refusals []string + + // A rotating change with rotation off never reaches the cluster. ClusterAPI decides + // a Machine is up to date by comparing the name of the template it was cloned from + // against the name its owner references — never the template's contents. With the + // fixed legacy name, the changed spec applies only to machines created later. + if len(p.RollingChanges) > 0 && !p.RotationEnabled { + refusals = append(refusals, fmt.Sprintf( + "This change rewrites a MachineTemplate spec :\n\n%s\n\n"+ + "but cluster.machineTemplateRotation is false, so the template keeps its fixed\n"+ + "name. ClusterAPI compares only a template's name when deciding whether a Machine\n"+ + "is up to date, so nothing would roll : the new spec would apply silently to\n"+ + "machines created later, leaving the cluster with mixed instance types.\n\n"+ + "Set cluster.machineTemplateRotation: true in general.yaml first. Note that\n"+ + "enabling it renames the template on its own, which rolls the control plane once.", + indentLines(p.RollingChanges, " - "), + )) + } + + // Scaling and rolling at once means ClusterAPI both adds members and replaces them. + // Starting from a single control-plane node that walks etcd through a sequence of + // membership changes with no quorum to lose. + if p.rolls() && len(p.ReplicaChange) > 0 { + refusals = append(refusals, fmt.Sprintf( + "This change both scales the control plane (%s) and rolls it :\n\n%s\n\n"+ + "Split it into two syncs. Scale out first — the MachineTemplate spec is untouched,\n"+ + "so the new members simply join etcd and nothing is replaced. Once they are Ready,\n"+ + "apply the rolling change against a control plane that has quorum to spare.", + p.ReplicaChange, indentLines(p.rollReasons(), " - "), + )) + } + + return refusals +} + +// Warnings lists things worth an explicit second look, but which are safe to proceed with. +func (p *capiValuesSyncPlan) Warnings() []string { + if !p.rolls() { + return nil + } + + // A rolling replacement surges one machine, waits for it to join, then removes an old + // one. Below three replicas every intermediate etcd membership is one node away from + // losing quorum. + if p.ControlPlaneReplicasAfter < 3 { + return []string{fmt.Sprintf( + "The control plane will be rolled with %d replica(s). ClusterAPI adds one machine,\n"+ + "waits for it to join etcd, then removes an old one — with fewer than 3 replicas\n"+ + "every step of that sequence is a single node away from losing quorum.\n"+ + "Scaling to 3 first is strongly recommended.", + p.ControlPlaneReplicasAfter, + )} + } + + return nil +} + +// rollReasons describes every reason machines will be replaced. +func (p *capiValuesSyncPlan) rollReasons() []string { + reasons := make([]string, len(p.RollingChanges), len(p.RollingChanges)+1) + copy(reasons, p.RollingChanges) + + if p.RotationNewlyEnabled { + reasons = append(reasons, "machineTemplateRotation enabled (renames the template, which rolls it once)") + } + + return reasons +} + +// planCapiValuesSync compares two rendered values-capi-cluster.yaml files and works out +// how ClusterAPI will react to the change. +func planCapiValuesSync(before, after []byte) (*capiValuesSyncPlan, error) { + oldFacts, err := parseCapiValuesFacts(before) + if err != nil { + return nil, fmt.Errorf("parsing the current values-capi-cluster.yaml: %w", err) + } + + newFacts, err := parseCapiValuesFacts(after) + if err != nil { + return nil, fmt.Errorf("parsing the re-rendered values-capi-cluster.yaml: %w", err) + } + + plan := &capiValuesSyncPlan{ + RotationEnabled: newFacts.MachineTemplateRotation, + RotationNewlyEnabled: newFacts.MachineTemplateRotation && !oldFacts.MachineTemplateRotation, + ControlPlaneReplicasAfter: newFacts.ControlPlaneReplicas, + } + + if oldFacts.ImageName != newFacts.ImageName { + plan.RollingChanges = append(plan.RollingChanges, fmt.Sprintf( + "hcloud.imageName : %s -> %s", oldFacts.ImageName, newFacts.ImageName, + )) + } + + if oldFacts.ControlPlaneType != newFacts.ControlPlaneType { + plan.RollingChanges = append(plan.RollingChanges, fmt.Sprintf( + "controlPlane.hcloud.machineType : %s -> %s", + oldFacts.ControlPlaneType, newFacts.ControlPlaneType, + )) + } + + for _, name := range sortedKeys(newFacts.NodeGroupTypes) { + oldType, existed := oldFacts.NodeGroupTypes[name] + if !existed || oldType == newFacts.NodeGroupTypes[name] { + continue + } + + plan.RollingChanges = append(plan.RollingChanges, fmt.Sprintf( + "nodeGroups.hcloud[%s].machineType : %s -> %s", + name, oldType, newFacts.NodeGroupTypes[name], + )) + } + + if oldFacts.ControlPlaneReplicas != newFacts.ControlPlaneReplicas { + plan.ReplicaChange = fmt.Sprintf("%d -> %d", + oldFacts.ControlPlaneReplicas, newFacts.ControlPlaneReplicas, + ) + } + + return plan, nil +} + +// parseCapiValuesFacts pulls the rotation-relevant fields out of a rendered +// values-capi-cluster.yaml. Missing fields are left at their zero value: a +// non-Hetzner or bare-metal cluster simply yields no rolling changes. +func parseCapiValuesFacts(values []byte) (*capiValuesFacts, error) { + var root struct { + Global struct { + MachineTemplateRotation bool `json:"machineTemplateRotation"` + } `json:"global"` + + Hetzner struct { + HCloud struct { + ImageName string `json:"imageName"` + } `json:"hcloud"` + + ControlPlane struct { + HCloud struct { + MachineType string `json:"machineType"` + Replicas int `json:"replicas"` + } `json:"hcloud"` + } `json:"controlPlane"` + + NodeGroups struct { + HCloud []struct { + Name string `json:"name"` + MachineType string `json:"machineType"` + } `json:"hcloud"` + } `json:"nodeGroups"` + } `json:"hetzner"` + } + + if err := yaml.Unmarshal(values, &root); err != nil { + return nil, err + } + + facts := &capiValuesFacts{ + MachineTemplateRotation: root.Global.MachineTemplateRotation, + ImageName: root.Hetzner.HCloud.ImageName, + ControlPlaneType: root.Hetzner.ControlPlane.HCloud.MachineType, + ControlPlaneReplicas: root.Hetzner.ControlPlane.HCloud.Replicas, + NodeGroupTypes: map[string]string{}, + } + + for _, nodeGroup := range root.Hetzner.NodeGroups.HCloud { + facts.NodeGroupTypes[nodeGroup.Name] = nodeGroup.MachineType + } + + return facts, nil +} + +func sortedKeys(m map[string]string) []string { + keys := make([]string, 0, len(m)) + for key := range m { + keys = append(keys, key) + } + sort.Strings(keys) + + return keys +} + +func indentLines(lines []string, prefix string) string { + prefixed := make([]string, 0, len(lines)) + for _, line := range lines { + prefixed = append(prefixed, prefix+line) + } + + return strings.Join(prefixed, "\n") +} diff --git a/pkg/core/capi_values_sync_plan_test.go b/pkg/core/capi_values_sync_plan_test.go new file mode 100644 index 00000000..7a8dcf62 --- /dev/null +++ b/pkg/core/capi_values_sync_plan_test.go @@ -0,0 +1,241 @@ +// Copyright 2026 Obmondo +// SPDX-License-Identifier: AGPL3 + +package core + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// capiValues renders a minimal values-capi-cluster.yaml carrying only the fields +// planCapiValuesSync reads. +type capiValues struct { + rotation bool + imageName string + cpType string + cpReplicas int + nodeGroups map[string]string +} + +func (v capiValues) render() []byte { + imageName := v.imageName + if len(imageName) == 0 { + imageName = "ubuntu-24.04" + } + + cpType := v.cpType + if len(cpType) == 0 { + cpType = "cpx32" + } + + cpReplicas := v.cpReplicas + if cpReplicas == 0 { + cpReplicas = 1 + } + + var nodeGroups strings.Builder + for _, name := range sortedKeys(v.nodeGroups) { + fmt.Fprintf(&nodeGroups, "\n - name: %s\n machineType: %s", name, v.nodeGroups[name]) + } + if nodeGroups.Len() == 0 { + nodeGroups.WriteString(" []") + } + + return fmt.Appendf(nil, `global: + clusterName: demo + machineTemplateRotation: %t +hetzner: + mode: hcloud + hcloud: + imageName: %s + controlPlane: + hcloud: + machineType: %s + replicas: %d + nodeGroups: + hcloud:%s +`, v.rotation, imageName, cpType, cpReplicas, nodeGroups.String()) +} + +func TestPlanCapiValuesSync(t *testing.T) { + t.Parallel() + + baseline := capiValues{cpType: "cpx32", cpReplicas: 1, nodeGroups: map[string]string{"md-0": "cpx41"}} + + testCases := []struct { + name string + + before, after capiValues + + wantRolls bool + wantRollingChanges []string + wantReplicaChange string + wantRefusalContains string + wantWarningContains string + }{ + { + name: "no change is not a roll and not a scale", + before: baseline, + after: baseline, + }, + { + name: "replicas only scales out, rotation irrelevant", + before: baseline, + after: capiValues{cpType: "cpx32", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantReplicaChange: "1 -> 3", + wantRolls: false, + }, + { + name: "machineType change without rotation is refused as a silent no-op", + before: baseline, + after: capiValues{cpType: "cpx22", cpReplicas: 1, nodeGroups: baseline.nodeGroups}, + wantRollingChanges: []string{ + "controlPlane.hcloud.machineType : cpx32 -> cpx22", + }, + // Nothing rolls: the template keeps its fixed name, so ClusterAPI never + // notices. That is precisely why the change is refused. + wantRolls: false, + wantRefusalContains: "nothing would roll", + }, + { + name: "machineType change with rotation rolls, and warns below 3 replicas", + before: capiValues{rotation: true, cpType: "cpx32", cpReplicas: 1, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, cpType: "cpx22", cpReplicas: 1, nodeGroups: baseline.nodeGroups}, + wantRollingChanges: []string{ + "controlPlane.hcloud.machineType : cpx32 -> cpx22", + }, + wantRolls: true, + wantWarningContains: "losing quorum", + }, + { + name: "machineType change with rotation at 3 replicas rolls without warning", + before: capiValues{rotation: true, cpType: "cpx32", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, cpType: "cpx22", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantRolls: true, + }, + { + name: "rolling and scaling together is refused", + before: capiValues{rotation: true, cpType: "cpx32", cpReplicas: 1, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, cpType: "cpx22", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantRolls: true, + wantReplicaChange: "1 -> 3", + wantRefusalContains: "Split it into two syncs", + }, + { + name: "enabling rotation alone rolls, and refuses to also scale", + before: capiValues{cpType: "cpx32", cpReplicas: 1, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, cpType: "cpx32", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantRolls: true, + wantReplicaChange: "1 -> 3", + wantRefusalContains: "Split it into two syncs", + }, + { + name: "enabling rotation at 3 replicas rolls once, cleanly", + before: capiValues{cpType: "cpx32", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, cpType: "cpx32", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantRolls: true, + }, + { + name: "imageName change rotates the template", + before: capiValues{rotation: true, imageName: "ubuntu-24.04", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + after: capiValues{rotation: true, imageName: "ubuntu-26.04", cpReplicas: 3, nodeGroups: baseline.nodeGroups}, + wantRollingChanges: []string{ + "hcloud.imageName : ubuntu-24.04 -> ubuntu-26.04", + }, + wantRolls: true, + }, + { + name: "node-group machineType change rotates only that node group", + before: capiValues{rotation: true, cpReplicas: 3, nodeGroups: map[string]string{"md-0": "cpx41", "md-1": "cpx31"}}, + after: capiValues{rotation: true, cpReplicas: 3, nodeGroups: map[string]string{"md-0": "cpx51", "md-1": "cpx31"}}, + wantRollingChanges: []string{ + "nodeGroups.hcloud[md-0].machineType : cpx41 -> cpx51", + }, + wantRolls: true, + }, + { + name: "a newly added node group is not a rolling change", + before: capiValues{rotation: true, cpReplicas: 3, nodeGroups: map[string]string{"md-0": "cpx41"}}, + after: capiValues{rotation: true, cpReplicas: 3, nodeGroups: map[string]string{"md-0": "cpx41", "md-new": "cpx31"}}, + wantRolls: false, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + t.Parallel() + + plan, err := planCapiValuesSync(testCase.before.render(), testCase.after.render()) + require.NoError(t, err) + + assert.Equal(t, testCase.wantRolls, plan.rolls(), "whether ClusterAPI replaces machines") + assert.Equal(t, testCase.wantReplicaChange, plan.ReplicaChange, "control-plane scale") + + if testCase.wantRollingChanges != nil { + assert.Equal(t, testCase.wantRollingChanges, plan.RollingChanges) + } + + refusals := strings.Join(plan.Refusals(), "\n") + if len(testCase.wantRefusalContains) == 0 { + assert.Empty(t, plan.Refusals(), "expected the change to be allowed") + } else { + assert.Contains(t, refusals, testCase.wantRefusalContains) + } + + warnings := strings.Join(plan.Warnings(), "\n") + if len(testCase.wantWarningContains) == 0 { + assert.Empty(t, plan.Warnings(), "expected no warning") + } else { + assert.Contains(t, warnings, testCase.wantWarningContains) + } + }) + } +} + +// The netbird-obmondo-com change, staged the way the refusals force you to stage it. +func TestPlanCapiValuesSyncNetbirdTwoPhase(t *testing.T) { + t.Parallel() + + oneReplicaCPX32 := capiValues{cpType: "cpx32", cpReplicas: 1} + threeReplicaCPX32 := capiValues{cpType: "cpx32", cpReplicas: 3} + threeReplicaCPX22 := capiValues{rotation: true, cpType: "cpx22", cpReplicas: 3} + + t.Run("phase 1 : scale 1 -> 3 on the existing type, members join etcd", func(t *testing.T) { + t.Parallel() + + plan, err := planCapiValuesSync(oneReplicaCPX32.render(), threeReplicaCPX32.render()) + require.NoError(t, err) + + assert.False(t, plan.rolls(), "a pure scale-out must not replace any machine") + assert.Equal(t, "1 -> 3", plan.ReplicaChange) + assert.Empty(t, plan.Refusals()) + assert.Empty(t, plan.Warnings()) + }) + + t.Run("phase 2 : enable rotation and re-type, rolling with quorum to spare", func(t *testing.T) { + t.Parallel() + + plan, err := planCapiValuesSync(threeReplicaCPX32.render(), threeReplicaCPX22.render()) + require.NoError(t, err) + + assert.True(t, plan.rolls()) + assert.True(t, plan.RotationNewlyEnabled) + assert.Empty(t, plan.ReplicaChange, "replicas must already be settled before re-typing") + assert.Empty(t, plan.Refusals()) + assert.Empty(t, plan.Warnings(), "3 replicas is enough to roll safely") + }) + + t.Run("doing both at once is refused", func(t *testing.T) { + t.Parallel() + + plan, err := planCapiValuesSync(oneReplicaCPX32.render(), threeReplicaCPX22.render()) + require.NoError(t, err) + + assert.Contains(t, strings.Join(plan.Refusals(), "\n"), "Split it into two syncs") + }) +} diff --git a/pkg/core/live_cluster_context.go b/pkg/core/live_cluster_context.go new file mode 100644 index 00000000..8ced6781 --- /dev/null +++ b/pkg/core/live_cluster_context.go @@ -0,0 +1,140 @@ +// Copyright 2026 Obmondo +// SPDX-License-Identifier: AGPL3 + +package core + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/huh" + "k8s.io/client-go/tools/clientcmd" + + "github.com/Obmondo/kubeaid-cli/pkg/config" + "github.com/Obmondo/kubeaid-cli/pkg/utils/progress" +) + +// liveClusterContext identifies the cluster a day-2 command is about to act on. +// +// Nothing in kubeaid-cli ever selects a kubeconfig context : every client is built with +// clientcmd.BuildConfigFromFlags("", path), which silently follows whatever +// current-context the kubeconfig happens to carry. Commands that mutate a running +// cluster therefore have to show the operator which cluster that resolved to, rather +// than assume it is the one general.yaml describes. +type liveClusterContext struct { + KubeconfigPath string + ContextName string + ClusterName string + Server string +} + +// describeLiveClusterContext resolves the kubeconfig's current-context into the cluster +// it points at. +func describeLiveClusterContext(kubeconfigPath string) (*liveClusterContext, error) { + apiConfig, err := clientcmd.LoadFromFile(kubeconfigPath) + if err != nil { + return nil, fmt.Errorf("loading kubeconfig %q : %w", kubeconfigPath, err) + } + + if len(apiConfig.CurrentContext) == 0 { + return nil, fmt.Errorf("kubeconfig %q has no current-context set", kubeconfigPath) + } + + kubeContext, found := apiConfig.Contexts[apiConfig.CurrentContext] + if !found { + return nil, fmt.Errorf("kubeconfig %q sets current-context to %q, which it doesn't define", + kubeconfigPath, apiConfig.CurrentContext, + ) + } + + live := &liveClusterContext{ + KubeconfigPath: kubeconfigPath, + ContextName: apiConfig.CurrentContext, + ClusterName: kubeContext.Cluster, + } + + if cluster, found := apiConfig.Clusters[kubeContext.Cluster]; found { + live.Server = cluster.Server + } + + return live, nil +} + +// matchesConfiguredCluster reports whether the resolved context names the same cluster +// general.yaml does. A mismatch is not proof of a wrong target — a kubeconfig may name +// its cluster anything — but it is the signal that matters most when it is wrong. +func (l *liveClusterContext) matchesConfiguredCluster() bool { + return l.ClusterName == config.ParsedGeneralConfig.Cluster.Name +} + +// summary renders the context as an aligned block for the confirmation prompt. +func (l *liveClusterContext) summary(action string) string { + lines := []string{ + fmt.Sprintf(" about to : %s", action), + fmt.Sprintf(" cluster : %s (from general.yaml)", config.ParsedGeneralConfig.Cluster.Name), + fmt.Sprintf(" context : %s", l.ContextName), + fmt.Sprintf(" api server : %s", l.Server), + fmt.Sprintf(" kubeconfig : %s", l.KubeconfigPath), + } + + if !l.matchesConfiguredCluster() { + lines = append(lines, "", fmt.Sprintf( + " MISMATCH : the kubeconfig's current-context points at cluster %q,\n"+ + " but general.yaml describes %q. Check you are not about to\n"+ + " reconcile one cluster's config onto another.", + l.ClusterName, config.ParsedGeneralConfig.Cluster.Name, + )) + } + + return strings.Join(lines, "\n") +} + +// confirmLiveClusterContext makes the operator name the cluster before anything touches it. +// +// bar may be nil, for callers that run outside a progress section. +// +// Returns an error when the operator declines, when the kubeconfig can't be resolved, or +// when there is no terminal to ask on. Never proceeds by default : an unattended run of a +// command that rolls control-plane machines should stop, not guess. +func confirmLiveClusterContext(bar *progress.Bar, + kubeconfigPath string, + action string, +) error { + live, err := describeLiveClusterContext(kubeconfigPath) + if err != nil { + return err + } + + proceed := false + + if bar != nil { + bar.Pause() + defer bar.Resume() + } + + title := "Confirm the target cluster" + if !live.matchesConfiguredCluster() { + title = "Confirm the target cluster (name mismatch)" + } + + if err := huh.NewForm( + huh.NewGroup( + huh.NewNote(). + Title(title). + Description(live.summary(action)), + huh.NewConfirm(). + Title("Operate on this cluster?"). + Affirmative("Yes, this is the right cluster"). + Negative("No, abort"). + Value(&proceed), + ), + ).Run(); err != nil { + return fmt.Errorf("confirming the target cluster %q : %w", live.ContextName, err) + } + + if !proceed { + return fmt.Errorf("aborted : declined to operate on cluster %q", live.ContextName) + } + + return nil +} diff --git a/pkg/core/live_cluster_context_test.go b/pkg/core/live_cluster_context_test.go new file mode 100644 index 00000000..85adf97f --- /dev/null +++ b/pkg/core/live_cluster_context_test.go @@ -0,0 +1,152 @@ +// Copyright 2026 Obmondo +// SPDX-License-Identifier: AGPL3 + +package core + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/yaml" + + "github.com/Obmondo/kubeaid-cli/pkg/config" +) + +// writeKubeconfig drops a kubeconfig on disk and returns its path. +func writeKubeconfig(t *testing.T, contents string) string { + t.Helper() + + kubeconfigPath := filepath.Join(t.TempDir(), "kubeconfig.yaml") + require.NoError(t, os.WriteFile(kubeconfigPath, []byte(contents), 0o600)) + + return kubeconfigPath +} + +const validKubeconfig = `apiVersion: v1 +kind: Config +current-context: netbird-acme-com +clusters: + - name: netbird-acme-com + cluster: + server: https://api.vpn.acme.com:6443 +contexts: + - name: netbird-acme-com + context: + cluster: netbird-acme-com + user: netbird +users: + - name: netbird + user: {} +` + +func TestDescribeLiveClusterContext(t *testing.T) { + t.Parallel() + + t.Run("resolves the current-context into its cluster and server", func(t *testing.T) { + t.Parallel() + + live, err := describeLiveClusterContext(writeKubeconfig(t, validKubeconfig)) + require.NoError(t, err) + + assert.Equal(t, "netbird-acme-com", live.ContextName) + assert.Equal(t, "netbird-acme-com", live.ClusterName) + assert.Equal(t, "https://api.vpn.acme.com:6443", live.Server) + }) + + t.Run("fails when the kubeconfig is missing", func(t *testing.T) { + t.Parallel() + + _, err := describeLiveClusterContext(filepath.Join(t.TempDir(), "absent.yaml")) + require.Error(t, err) + }) + + t.Run("fails when no current-context is set", func(t *testing.T) { + t.Parallel() + + // Without this, every client would silently fall back to an empty context. + _, err := describeLiveClusterContext(writeKubeconfig(t, `apiVersion: v1 +kind: Config +clusters: [] +contexts: [] +users: [] +`)) + require.ErrorContains(t, err, "no current-context") + }) + + t.Run("fails when current-context names a context that is not defined", func(t *testing.T) { + t.Parallel() + + _, err := describeLiveClusterContext(writeKubeconfig(t, `apiVersion: v1 +kind: Config +current-context: ghost +clusters: [] +contexts: [] +users: [] +`)) + require.ErrorContains(t, err, "doesn't define") + }) +} + +func TestLiveClusterContextMatchesConfiguredCluster(t *testing.T) { + // Mutates the config package global, so it cannot run in parallel with other tests + // that read it. + original := config.ParsedGeneralConfig + t.Cleanup(func() { config.ParsedGeneralConfig = original }) + + live, err := describeLiveClusterContext(writeKubeconfig(t, validKubeconfig)) + require.NoError(t, err) + + config.ParsedGeneralConfig.Cluster.Name = "netbird-acme-com" + assert.True(t, live.matchesConfiguredCluster()) + assert.NotContains(t, live.summary("sync"), "MISMATCH") + + // The case the gate exists for: a kubeconfig left pointing at another cluster. + config.ParsedGeneralConfig.Cluster.Name = "kbm-acme-com" + assert.False(t, live.matchesConfiguredCluster()) + + summary := live.summary("sync") + assert.Contains(t, summary, "MISMATCH") + assert.Contains(t, summary, "netbird-acme-com") + assert.Contains(t, summary, "kbm-acme-com") +} + +// The chart gates template rotation on global.machineTemplateRotation, so the rendered +// values file has to carry it. Without this line a cluster silently keeps the fixed +// template names and no instance-type change ever rolls. +func TestCapiClusterValuesRendersMachineTemplateRotation(t *testing.T) { + t.Parallel() + + const tmplPath = "templates/argocd-apps/values-capi-cluster.yaml.tmpl" + + globalOf := func(t *testing.T, out string) map[string]any { + t.Helper() + + var parsed map[string]any + require.NoError(t, yaml.Unmarshal([]byte(out), &parsed), "rendered values must be valid YAML:\n%s", out) + + global, ok := parsed["global"].(map[string]any) + require.True(t, ok, "global block must be present") + + return global + } + + t.Run("defaults to false", func(t *testing.T) { + t.Parallel() + + out := renderEmbeddedTemplate(t, tmplPath, capiClusterTV(false)) + assert.Equal(t, false, globalOf(t, out)["machineTemplateRotation"]) + }) + + t.Run("renders true when enabled in general.yaml", func(t *testing.T) { + t.Parallel() + + tv := capiClusterTV(false) + tv.MachineTemplateRotation = true + + out := renderEmbeddedTemplate(t, tmplPath, tv) + assert.Equal(t, true, globalOf(t, out)["machineTemplateRotation"]) + }) +} diff --git a/pkg/core/setup_kubeaid_config.go b/pkg/core/setup_kubeaid_config.go index f413c367..faf16b77 100644 --- a/pkg/core/setup_kubeaid_config.go +++ b/pkg/core/setup_kubeaid_config.go @@ -250,6 +250,20 @@ func createOrUpdateKubeOneConfigFile(ctx context.Context, templateValues *Templa createFileFromTemplate(ctx, destinationFilePath, constants.KubeOneConfigTemlateName, templateValues) } +// Creates / updates the values-capi-cluster.yaml file, which the capi-cluster ArgoCD App +// renders the cluster's ClusterAPI resources from. Rendered on its own by `cluster sync`, +// so a day-2 reconcile touches the ClusterAPI values and nothing else. +func createOrUpdateCapiClusterValuesFile(ctx context.Context, + templateValues *TemplateValues, + clusterDir string, +) { + destinationFilePath := path.Join( + clusterDir, + strings.TrimSuffix(constants.TemplateNameCapiClusterValues, ".tmpl"), + ) + createFileFromTemplate(ctx, destinationFilePath, constants.TemplateNameCapiClusterValues, templateValues) +} + // Creates / updates the cluster's kubeaid-cli.general.yaml copy in the KubeAid Config // repository - the source of truth the KubeOne manifest derives from, kept next to it so a // day-2 PR always carries both. diff --git a/pkg/core/sync_cluster_capi.go b/pkg/core/sync_cluster_capi.go new file mode 100644 index 00000000..4db97fe0 --- /dev/null +++ b/pkg/core/sync_cluster_capi.go @@ -0,0 +1,291 @@ +// Copyright 2026 Obmondo +// SPDX-License-Identifier: AGPL3 + +package core + +import ( + "context" + "fmt" + "os" + "path" + "strings" + "time" + + "github.com/charmbracelet/huh" + goGit "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/transport" + "github.com/pmezard/go-difflib/difflib" + + "github.com/Obmondo/kubeaid-cli/pkg/config" + "github.com/Obmondo/kubeaid-cli/pkg/constants" + "github.com/Obmondo/kubeaid-cli/pkg/globals" + "github.com/Obmondo/kubeaid-cli/pkg/utils" + "github.com/Obmondo/kubeaid-cli/pkg/utils/assert" + "github.com/Obmondo/kubeaid-cli/pkg/utils/git" + "github.com/Obmondo/kubeaid-cli/pkg/utils/kubernetes" + "github.com/Obmondo/kubeaid-cli/pkg/utils/progress" +) + +type SyncCAPIClusterArgs struct { + SkipPRWorkflow bool +} + +// SyncCAPICluster reconciles a running ClusterAPI cluster onto general.yaml, without a +// Kubernetes version change. +// +// Only `bootstrap` re-renders values-capi-cluster.yaml today, so a machineType or replicas +// edit in general.yaml never reaches a provisioned cluster. This re-renders that one file, +// shows the operator the diff, and lets ArgoCD apply it. +// +// Nothing here deletes or recreates a MachineTemplate. ClusterAPI decides a Machine is out +// of date by comparing the *name* of the template it was cloned from against the name its +// owner references, so a template rewritten under its old name is invisible to it. The +// capi-cluster chart instead names HCloud MachineTemplates after a hash of their spec +// (global.machineTemplateRotation), which turns an instance-type change into a new template +// name and a normal rolling replacement — entirely through GitOps. +func SyncCAPICluster(ctx context.Context, args SyncCAPIClusterArgs) { + bar := progress.New("Syncing cluster with general.yaml") + defer bar.Finish() + + ctx = progress.WithBar(ctx, bar) + + // Resolve which live cluster we are about to reconcile, and make the operator say so. + // This runs before the repo is even cloned : the whole command is a no-op if the answer + // is "wrong cluster". + bar.Describe("Confirming the target cluster") + + kubeconfigPath := resolveLiveClusterKubeconfig(ctx) + + err := confirmLiveClusterContext(bar, kubeconfigPath, + "reconcile the cluster's ClusterAPI resources onto general.yaml", + ) + assert.AssertErrNil(ctx, err, "Refusing to sync") + + bar.Substep(fmt.Sprintf("Targeting cluster %s", config.ParsedGeneralConfig.Cluster.Name)) + + // Clone kubeaid-config, so we can re-render into it. + bar.Describe("Re-rendering values-capi-cluster.yaml from general.yaml") + + gitAuthMethod := git.GetGitAuthMethod(ctx) + + repo := git.CloneRepo(ctx, config.ParsedGeneralConfig.Forks.KubeaidConfigFork.URL, gitAuthMethod) + bar.Substep("Cloned kubeaid-config repo") + + clusterDir := utils.GetClusterDir() + + capiValuesPath := path.Join(clusterDir, + strings.TrimSuffix(constants.TemplateNameCapiClusterValues, ".tmpl"), + ) + + before, err := os.ReadFile(capiValuesPath) + assert.AssertErrNil(ctx, err, "Failed reading the cluster's current values-capi-cluster.yaml") + + // Re-render values-capi-cluster.yaml, plus the verbatim general.yaml copy that sits + // beside it, so the PR always carries the source of truth alongside what it produced. + templateValues := getTemplateValues(ctx) + createOrUpdateCapiClusterValuesFile(ctx, templateValues, clusterDir) + createOrUpdateGeneralConfigFile(ctx, templateValues, clusterDir) + + after, err := os.ReadFile(capiValuesPath) + assert.AssertErrNil(ctx, err, "Failed reading the re-rendered values-capi-cluster.yaml") + + diff, err := unifiedDiff("values-capi-cluster.yaml", before, after) + assert.AssertErrNil(ctx, err, "Failed diffing values-capi-cluster.yaml") + + if len(diff) == 0 { + bar.Substep("values-capi-cluster.yaml already matches general.yaml — nothing to sync") + return + } + + // Work out how ClusterAPI will react, and refuse the changes that would silently do + // nothing or endanger etcd quorum. + plan, err := planCapiValuesSync(before, after) + assert.AssertErrNil(ctx, err, "Failed inspecting the values-capi-cluster.yaml change") + + if refusals := plan.Refusals(); len(refusals) > 0 { + assert.Assert(ctx, false, strings.Join(refusals, "\n\n")) + } + + // Approval #1 : the inline diff, before anything leaves this machine. + if !confirmCapiValuesDiff(bar, diff, plan) { + assert.Assert(ctx, false, "Aborted : declined the values-capi-cluster.yaml change") + } + + // Approval #2 : merging the PR. + bar.Describe("Pushing the change to kubeaid-config") + + if !pushCapiClusterValuesChanges(ctx, repo, gitAuthMethod, args.SkipPRWorkflow) { + bar.Substep("kubeaid-config already up to date — nothing to sync") + return + } + + // Let ArgoCD apply the merged values, then watch ClusterAPI act on them. + applyCapiClusterValues(ctx, plan) +} + +// resolveLiveClusterKubeconfig points KUBECONFIG at whichever cluster currently holds the +// ClusterAPI resources : the main cluster once `clusterctl move` has run, the management +// cluster before that. +func resolveLiveClusterKubeconfig(ctx context.Context) string { + utils.MustSetEnv(constants.EnvNameKubeconfig, constants.OutputPathMainClusterKubeconfig) + + if !kubernetes.IsClusterctlMoveExecuted(ctx) { + mgmtKubeconfig, err := kubernetes.GetManagementClusterKubeconfigPath(ctx) + assert.AssertErrNil(ctx, err, "Failed getting management cluster kubeconfig path") + + utils.MustSetEnv(constants.EnvNameKubeconfig, mgmtKubeconfig) + } + + return utils.MustGetEnv(constants.EnvNameKubeconfig) +} + +// unifiedDiff renders a `diff -u` style patch, or "" when the two versions are identical. +func unifiedDiff(name string, before, after []byte) (string, error) { + if string(before) == string(after) { + return "", nil + } + + return difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ + A: difflib.SplitLines(string(before)), + B: difflib.SplitLines(string(after)), + FromFile: name + " (live)", + ToFile: name + " (general.yaml)", + Context: 3, + }) +} + +// confirmCapiValuesDiff shows the operator exactly what will change, and what ClusterAPI +// will do about it, before a PR exists. +func confirmCapiValuesDiff(bar *progress.Bar, diff string, plan *capiValuesSyncPlan) bool { + description := diff + + if reasons := plan.rollReasons(); len(reasons) > 0 { + description += fmt.Sprintf( + "\nThis rotates a MachineTemplate name, so ClusterAPI will REPLACE machines :\n\n%s\n\n"+ + "Each replacement adds a machine, waits for it to join, then removes an old one.\n"+ + "The control plane will roll with %d replica(s).", + indentLines(reasons, " - "), plan.ControlPlaneReplicasAfter, + ) + } + + if len(plan.ReplicaChange) > 0 { + description += fmt.Sprintf( + "\nThe control plane scales : %s. The MachineTemplate spec is unchanged, so the new\n"+ + "members simply join etcd — no existing machine is replaced.", + plan.ReplicaChange, + ) + } + + for _, warning := range plan.Warnings() { + description += "\n\nWARNING : " + warning + } + + proceed := false + + bar.Pause() + defer bar.Resume() + + if err := huh.NewForm( + huh.NewGroup( + huh.NewNote(). + Title("values-capi-cluster.yaml will change"). + Description(description), + huh.NewConfirm(). + Title("Open a kubeaid-config PR with this change?"). + Affirmative("Yes, push it"). + Negative("No, abort"). + Value(&proceed), + ), + ).Run(); err != nil { + return false + } + + return proceed +} + +// pushCapiClusterValuesChanges commits the re-rendered files and waits for the PR to merge. +// Returns false when the worktree was already clean. +func pushCapiClusterValuesChanges(ctx context.Context, + repo *goGit.Repository, + gitAuthMethod transport.AuthMethod, + skipPRWorkflow bool, +) bool { + bar := progress.FromCtx(ctx) + + workTree, err := repo.Worktree() + assert.AssertErrNil(ctx, err, "Failed getting kubeaid-config repo worktree") + + defaultBranchName := git.GetDefaultBranchName(ctx, gitAuthMethod, repo) + + targetBranchName := defaultBranchName + if !skipPRWorkflow { + newBranchName := fmt.Sprintf("kubeaid-%s-%d", + config.ParsedGeneralConfig.Cluster.Name, time.Now().Unix(), + ) + git.CreateAndCheckoutToBranch(ctx, repo, newBranchName, workTree, gitAuthMethod) + + targetBranchName = newBranchName + } + + commitMessage := fmt.Sprintf("(cluster/%s) : synced values-capi-cluster.yaml with general.yaml", + config.ParsedGeneralConfig.Cluster.Name, + ) + + commitHash := git.AddCommitAndPushChanges(ctx, + repo, workTree, targetBranchName, gitAuthMethod, + config.ParsedGeneralConfig.Cluster.Name, commitMessage, defaultBranchName, + ) + if commitHash.IsZero() { + return false + } + + if !skipPRWorkflow { + releasePRWait := bar.InProgress("Waiting for you to merge the PR") + git.WaitUntilPRMerged(ctx, repo, defaultBranchName, commitHash, gitAuthMethod, targetBranchName) + releasePRWait() + + bar.Substep("Confirmed PR merged") + } + + return true +} + +// applyCapiClusterValues syncs the capi-cluster ArgoCD App and waits for ClusterAPI to +// settle on the new values. +func applyCapiClusterValues(ctx context.Context, plan *capiValuesSyncPlan) { + bar := progress.FromCtx(ctx) + bar.Describe("Applying the change to the cluster") + + clusterClient, err := kubernetes.CreateKubernetesClient(ctx, utils.MustGetEnv(constants.EnvNameKubeconfig)) + assert.AssertErrNil(ctx, err, "Failed constructing Kubernetes cluster client") + + { + argoCDClient, argoCDErr := kubernetes.NewArgoCDClient(ctx, clusterClient) + assert.AssertErrNil(ctx, argoCDErr, "Failed creating ArgoCD client") + + globals.ArgoCDApplicationClientCloser, globals.ArgoCDApplicationClient = argoCDClient.NewApplicationClientOrDie() + defer globals.ArgoCDApplicationClientCloser.Close() + } + + // Sync the whole app rather than named resources : a rotating change creates a + // differently-named MachineTemplate, so there is no stable resource list to scope to. + err = kubernetes.SyncArgoCDApp(ctx, constants.ArgoCDAppCapiCluster, nil) + assert.AssertErrNil(ctx, err, "Failed syncing the capi-cluster ArgoCD app") + + bar.Substep("Synced the capi-cluster ArgoCD app") + + // A pure scale-out is complete once the new control-plane machines are Running; a + // rotating change additionally has to retire the old ones. Both are covered by waiting + // for the KubeadmControlPlane to report every replica updated and no surge in flight. + bar.Describe("Waiting for ClusterAPI to converge") + + err = kubernetes.WaitForControlPlaneRolloutComplete(ctx, clusterClient) + assert.AssertErrNil(ctx, err, "Failed waiting for the control plane to converge") + + if plan.rolls() { + bar.Substep("Control plane rolled onto the new MachineTemplate") + return + } + + bar.Substep(fmt.Sprintf("Control plane scaled to %d replica(s)", plan.ControlPlaneReplicasAfter)) +} diff --git a/pkg/core/sync_cluster_kubeone.go b/pkg/core/sync_cluster_kubeone.go index 51250cbc..f9a98a16 100644 --- a/pkg/core/sync_cluster_kubeone.go +++ b/pkg/core/sync_cluster_kubeone.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "log/slog" + "os" "sort" "strconv" "strings" @@ -17,6 +18,7 @@ import ( kubeonessh "k8c.io/kubeone/pkg/ssh" "github.com/Obmondo/kubeaid-cli/pkg/config" + "github.com/Obmondo/kubeaid-cli/pkg/constants" "github.com/Obmondo/kubeaid-cli/pkg/utils/assert" "github.com/Obmondo/kubeaid-cli/pkg/utils/git" "github.com/Obmondo/kubeaid-cli/pkg/utils/progress" @@ -46,6 +48,17 @@ func SyncClusterUsingKubeOne(ctx context.Context, args SyncKubeOneClusterArgs) { ctx = progress.WithBar(ctx, bar) bar.Describe("Syncing cluster with general.yaml") + // This reconciles a live cluster : cordons, drains and restarts kubelet on its nodes. + // Nothing in kubeaid-cli selects a kubeconfig context, so make the operator confirm which + // cluster the current-context resolved to. Skipped when there is no kubeconfig yet — then + // there is no live cluster to act on, and the manifest is the only source of truth. + if _, err := os.Stat(constants.OutputPathMainClusterKubeconfig); err == nil { + confirmErr := confirmLiveClusterContext(bar, constants.OutputPathMainClusterKubeconfig, + "reconcile the cluster's KubeOne manifest and kubelet flags onto general.yaml", + ) + assert.AssertErrNil(ctx, confirmErr, "Refusing to sync") + } + gitAuthMethod := git.GetGitAuthMethod(ctx) repo := git.CloneRepo(ctx, config.ParsedGeneralConfig.Forks.KubeaidConfigFork.URL, gitAuthMethod) bar.Substep("Cloned kubeaid-config repo") diff --git a/pkg/core/templates/argocd-apps/values-capi-cluster.yaml.tmpl b/pkg/core/templates/argocd-apps/values-capi-cluster.yaml.tmpl index 6f086fc5..676afe55 100644 --- a/pkg/core/templates/argocd-apps/values-capi-cluster.yaml.tmpl +++ b/pkg/core/templates/argocd-apps/values-capi-cluster.yaml.tmpl @@ -5,6 +5,12 @@ {{- $preCreatedLB := (and .HetznerConfig (not .HCloudSingleNodePublic) (or (eq .ClusterConfig.Type "vpn") (and (eq .ClusterConfig.Type "workload") .HetznerConfig.HCloudVPNCluster))) -}} global: clusterName: {{ .ClusterConfig.Name }} + {{- /* Names HCloud MachineTemplates after a hash of their spec, so a changed + machineType rotates the name and ClusterAPI rolls the machines. CAPI + compares only the template's name, never its contents, so with the fixed + legacy name an instance-type change never rolls. Changing only replicas + leaves the name alone, and the control plane scales out instead. */}} + machineTemplateRotation: {{ .ClusterConfig.MachineTemplateRotation }} {{- if .ObmondoConfig }} customerid: {{ .ObmondoConfig.CustomerID }} {{- end }} diff --git a/pkg/core/upgrade_cluster.go b/pkg/core/upgrade_cluster.go index 2cbd3969..147f9b17 100644 --- a/pkg/core/upgrade_cluster.go +++ b/pkg/core/upgrade_cluster.go @@ -49,6 +49,14 @@ func UpgradeCluster(ctx context.Context, args UpgradeClusterArgs) { utils.MustSetEnv(constants.EnvNameKubeconfig, mgmtKubeconfig) } + // Rolling the control plane is not undoable : make the operator name the cluster the + // kubeconfig's current-context resolved to, before any machine is replaced. + confirmErr := confirmLiveClusterContext(nil, + utils.MustGetEnv(constants.EnvNameKubeconfig), + "upgrade the cluster, replacing its machines", + ) + assert.AssertErrNil(ctx, confirmErr, "Refusing to upgrade") + // Construct the Kubernetes cluster client. clusterClient, err := kubernetes.CreateKubernetesClient(ctx, utils.MustGetEnv(constants.EnvNameKubeconfig), @@ -211,6 +219,18 @@ func upgradeControlPlane(ctx context.Context, machineTemplateName = kubeadmControlPlaneName ) + // Under MachineTemplate name rotation the chart names the template after a hash of its + // spec, so a new image already renders a differently-named object. There is nothing to + // delete and recreate — `-control-plane` does not exist. Syncing the whole app + // creates the new template and repoints the KubeadmControlPlane at it, which is what + // makes ClusterAPI roll. + if config.ParsedGeneralConfig.Cluster.MachineTemplateRotation { + syncErr := kubernetes.SyncArgoCDApp(ctx, constants.ArgoCDAppCapiCluster, nil) + assert.AssertErrNil(ctx, syncErr, "Failed syncing capi-cluster ArgoCD app for control plane upgrade") + + return + } + // When the user wants an OS upgrade, // make necessary updates in the corresponding infrastructure specific MachineTemplate resource // (like in AWSMachineTemplate when dealing with AWS), by deleting and recreating it. Since it's @@ -266,6 +286,16 @@ func upgradeNodeGroup(ctx context.Context, machineTemplateName = machineDeploymentName ) + // Under MachineTemplate name rotation the template is named after a hash of its spec, so + // `-` does not exist to be deleted. Syncing the whole app creates the + // newly-named template and repoints the MachineDeployment at it, which rolls the nodes. + if config.ParsedGeneralConfig.Cluster.MachineTemplateRotation { + syncErr := kubernetes.SyncArgoCDApp(ctx, constants.ArgoCDAppCapiCluster, nil) + assert.AssertErrNil(ctx, syncErr, "Failed syncing capi-cluster ArgoCD app for node group upgrade") + + return + } + // When the user wants to do an OS upgrade, // make necessary updates in the corresponding infrastructure specific MachineTemplate resource // (like in AWSMachineTemplate when dealing with AWS), by deleting and recreating it. Since it's