# 0026. `TalosOperation`: the backbone every upgrade kind shares, with none of their execution Status: Accepted · Date: 2026-10-25 · Design section: Upgrades Decided: `TalosOperation`: the backbone every upgrade kind shares (lock, status-held state machine, pause-on-failure, paused, maxUnavailable, hold annotation, ordering), with none of their execution ## Context `docs/design/architecture.md`'s "Upgrades" section opens with what all three upgrade kinds (Talos, Kubernetes, Ichor) share: > All three upgrade kinds share the lock, the status-held state machine, > pause-on-failure (no automatic cluster-wide rollback), `paused`, > `maxUnavailable` for workers, a per-machine hold annotation, label-based > ordering, and a progress view. This unit builds exactly that list, scaffolded with `kubebuilder create api --group ichor --version v1alpha1 --kind TalosOperation --resource --controller`, and nothing else: no Talos Upgrade API call, no Kubernetes upgrade execution, no Ichor self-upgrade, no UI, no `ichor` CLI command. Each of those is a separate, later unit. ## Decision: an injected `AdvanceMachineFunc`, not a per-kind branch `spec.kind` (`talos`/`kubernetes`/`ichor`) is stored and reported, never branched on. `TalosOperationReconciler.Advance` is a single function field -- `func(ctx, cluster, machine, op) (done bool, err error)` -- that performs one disruptive step for one machine and reports whether that step finished. `nil` (the default, until a later unit wires a real implementation into `cmd/main.go`) means "pause with `NoExecutor`," not a panic. Declared in `internal/controller`, the consumer, not in `pkg/upgrade`: ADR 0022 left exactly this kind of interface undeclared on purpose, because its consumer didn't exist yet ("Go interfaces belong to their consumer, declared where they're consumed"). This reconciler is that consumer. Each upgrade kind's own implementation -- calling Talos's Upgrade API, driving a Kubernetes upgrade, launching Ichor's installer Job -- is later, separate work that plugs into this one seam; the state machine itself never needs to know which kind it is running. ## Decision: the lock, reused exactly as built, never called directly `TalosOperationReconciler.acquireDisruptiveLock` is a second function with that exact name (`internal/controller/talosoperation_controller.go`), alongside `TalosMachineReconciler`'s own (`internal/controller/removal.go`). `TestAcquireDisruptiveLockIsTheOnlyAcquireCallSite` matches on the enclosing function's *name*, not its receiver type, so a second, independently-named-the-same function satisfies the guard exactly the way the first one does -- no change to that test was needed. It refuses the same way `refuseReboot`'s call does: recompute the live, approved control-plane count (now via `deleteguard.CountApprovedControlPlanes`, read directly rather than through the manager's cache -- see the companion fix landed just before this unit, ADR 0010's appendix), and refuse with `*transitioningError` if it is even, before ever calling `Acquire`. Unlike `reconcileRemoval`, `TalosOperation` has no `completesTransition` exception: there is no scenario where an upgrade operation is itself the step that resolves a control-plane replacement, so this call always passes `false` (implemented by simply never exposing the parameter). `pkg/clusterlock.Lock.Renew` -- already built, documented, but with no caller until removal.go's `renewWhile` gave it one -- gets a second: once per reconcile batch, after selecting which machine(s) to advance and before calling `Advance` on any of them, comfortably inside `DefaultDuration`'s five minutes for the handful of calls one reconcile pass makes. ## Decision: status holds the whole state machine `TalosOperationStatus` carries `phase` (`Pending`/`InProgress`/`Paused`/ `Completed`) and `machines` (`[]{name, phase: InProgress|Completed}`) -- nothing is kept in the reconciler's memory. A name's *absence* from `machines` means "never selected" (held, not yet its turn, or the operation finished before reaching it), not a third phase value forcing every reader to special-case it. This is what makes resumption automatic rather than a feature to build: `Reconcile` always starts by reading `op.status` fresh and recomputing which machines are still pending from it, so a killed and restarted controller (or a second, independently constructed reconciler, which is what the test for this uses to simulate one) picks up exactly where the persisted status says the operation was, with no separate recovery path -- the same shape `removal.go`'s own resumability section already documents for `clusterlock.Lock.Acquire`'s self-resumption. ## Decision: pause-on-failure stops advancing, never undoes A machine's `Advance` call returning an error sets `phase: Paused` and `failureReason`, and returns. It does not touch any other machine's status -- there is no code path that could revert an already-`Completed` entry, because nothing ever removes an entry from `machines` or demotes its phase. Once `phase == Paused` *and* `failureReason != ""` (distinguishing a failure from `spec.paused`), `Reconcile`'s very next check short-circuits before calling `Advance` again on anything: a paused-by-failure operation never retries automatically. The only way out, in this unit's scope, is whatever the operator does next (fix the underlying cause and -- once a later unit adds the mechanism -- clear the failure); nothing here guesses at a retry policy nobody asked for. `spec.paused` is the separate, operator-driven halt: it sets `phase: Paused` with a different reason (`OperatorPaused`) and, critically, *still holds the cluster lock* -- the operation is not finished, only halted, so releasing the lock would let an unrelated disruptive operation start on a cluster that is mid-upgrade in every sense except "currently calling `Advance`." ## Decision: control planes first and alone, workers within `maxUnavailable` Every reconcile computes `pendingMachines` for control planes first; as long as any control plane has not reached `Completed`, exactly one of them (not more) is the whole batch for that reconcile, and workers are never touched. Only once every control plane shows `Completed` do workers become eligible, up to `spec.maxUnavailable` (default 1) per reconcile. This is a single `if/else` on "any control plane pending", not a counter or a phase of its own -- the state already recorded per-machine is sufficient to derive it fresh every time, consistent with principle 1. ## Decision: hold and ordering are machine-level, not operation-level `ichorv1alpha1.TalosMachineUpgradeHoldAnnotation` (`ichor.maesh.dev/upgrade-hold`) and `ichorv1alpha1.TalosMachineUpgradeOrderLabel` (`ichor.maesh.dev/upgrade-order`) are new constants in `api/v1alpha1`, read off `TalosMachine` objects, not `TalosOperation` fields -- an operator marks the machine, once, and every `TalosOperation` that ever touches it honors the mark. A held machine is filtered out before it is ever a candidate at all: it never appears in `status.machines`, and does not block an operation from reaching `Completed` (completion only waits on machines that were actually candidates). The order label sorts within a tier (`sortByUpgradeOrder`, `slices.SortFunc`) by string value, a missing label sorting last, with the object's own name as the final, deterministic tiebreak -- which matters for tests more than production, since `GenerateName`-assigned object names are not something a test should ever rely on for ordering (a real bug this unit's own tests hit once, fixed by setting the label explicitly rather than depending on name order). ## Decision: no Conditions populated, on purpose, for now `TalosOperationStatus.Conditions` exists (the same `[]metav1.Condition` shape every other reconciled CRD here carries) but nothing in this unit ever calls `apimeta.SetStatusCondition` on it. `phase`, `machines` and `failureReason` already say everything this unit needs to say, and nothing in the acceptance criteria asks for a condition-shaped signal on top of them. The field stays in the schema (removing it would be a disruptive schema change for whichever later unit decides it wants one) but is inert until something actually needs it -- deletion-by-default, not speculative API surface. ## Decision: not tracked in Git, and why that is this type's own doc comment, not a mechanism `docs/design/architecture.md`'s "GitOps and Argo CD" section lists `TalosOperation`s under "What doesn't [go in Git]" alongside Secrets: it is a record of an imperative action in progress, not a declarative desired state. `TalosOperation`'s own Go doc comment says so explicitly and points at `ichor gitops export` as the thing responsible for excluding it -- that command, and its own exclusion logic, belong to a different agent's work entirely. This type cannot enforce its own absence from a Git repository; the most it can do is tell the next reader why, which is what the doc comment does. ## Consequences - `pkg/deleteguard.CountApprovedControlPlanes` (landed just before this unit, fixing a cached-read bug -- see ADR 0010's appendix) is reused directly by `TalosOperationReconciler.acquireDisruptiveLock`, via a new `APIReader client.Reader` field wired from `mgr.GetAPIReader()` in `cmd/main.go` -- the same pattern `TalosMachineReconciler` and the `TalosMachine` webhook now both use, so this new reconciler never reintroduces the trap that fix closed. - `TalosOperationReconciler` is registered as its own controller in `cmd/main.go`, not folded into `TalosClusterReconciler` or `TalosMachineReconciler`, for the same work-queue-isolation reason ADR 0024 gives for `CNIHandoverReconciler`. - Not built here, and explicitly out of scope: any call to Talos's Upgrade API, Kubernetes upgrade execution, Ichor's self-upgrade, the UI's progress view, and the `ichor` CLI's own upgrade commands. `AdvanceMachineFunc`'s only implementation in this unit is test fakes. - `IchorInstallation` and the `kind: ichor` executor landed in ADR 0059; the installer Job is a separate process, so the backbone is unchanged (the executor is idempotent per operation). - The entry points landed in ADR 0068: `OperationService`, `ichor upgrade|cni|cluster shutdown|operation`, and the operations screens. - Every pause now says why: `setPhase` writes a `Paused` condition (reason and message) for a transitioning cluster, a held lock, `spec.paused` and a failed step; `failureReason` stays reserved for failed steps because the controller, the CLI and the UI read it as "hold, never retry". ## Amendment (2026-11-04): each operation takes the cluster lock under its own identity **What was wrong.** `OperationService` refuses a second operation while one is unfinished (ADR 0068, Decision 2), but the controller did not keep two operations apart. Every `TalosOperation` took the cluster lease as `ichor-controller`, and `clusterlock.Acquire` lets the same identity back in, so two operations created some other way (`kubectl`, GitOps, a script) ran their disruptive steps at the same time, and an operation also ran alongside a machine removal or join, which share that identity. The envtest `two TalosOperations on one cluster` showed it: the second operation called `Advance` while the first was mid-reboot. **Decision.** - An operation acquires, renews and releases the lock as `ichor-controller/op//` (`operationLockIdentity`). The name, not the UID, is used because an etcd restore rolls the API back and the restored operation has to find its own lease (ADR 0089). The same operation re-enters its own lock across reconciles and controller restarts; any other operation waits. - The one that waits pauses with `ClusterLockHeld` and the message `the cluster lock is held by TalosOperation "/"; this operation starts when it finishes`, requeued every 30 seconds. A lock held by anything else keeps the old wording (`held by ""`), which is also what a machine removal or join sees when an operation holds the lock: they defer with `ClusterLockHeld` as before. - A holder that will never renew is not waited for. If the holder is an operation that no longer exists, is `Completed`, or is `Paused` with a `failureReason` (only an explicit retry moves it, and the retry takes the lock again), `acquireDisruptiveLock` releases that lease and takes it. Before this, a failed or deleted operation's lease lingered up to five minutes and the next operation had to wait it out; with per-operation identities that wait would have appeared in the "delete it, start again" flow. An operator pause (`spec.paused`) is not stopped in this sense and keeps the lock, as above. - A lease left by the old build under `ichor-controller` is indistinguishable from a machine removal or join, which still use that identity, so it is treated as held by someone else: operations wait. Nothing renews it once the old controller is gone, so it ages out within `clusterlock.DefaultDuration` (five minutes). That is the whole upgrade story: an Ichor self-upgrade that replaces the controller pod mid-operation resumes after at most five minutes. No migration code. - Not changed: the machine removal and join paths (one shared identity among machines of a cluster), and the CNI Helm Job (`ichor-controller/cni-helm`, already separate). **The webhook does not reject a second operation.** The service refuses early so a person sees why before the spec is written; the controller serializes. A webhook refusal would break GitOps, where a repository can hold several operations that are meant to run one after another. The cost is a `Paused` operation with a visible reason instead of an error at creation. **Follow-up (2026-11-04): a control-plane join and a machine removal take the lock under the machine's own identity.** The amendment above left the machine paths on the shared `ichor-controller`, so two control-plane joins, two removals, or a join and a removal on one cluster could hold the lock together (`clusterlock.Acquire` lets the same identity back in), and the later one also released the earlier one's lease when it finished. Etcd membership changes must run one at a time: a join adds a member, a removal removes one. The envtest `two control-plane TalosMachines of one cluster at a step that takes the cluster lock` showed all four pairings getting through. - A machine acquires, renews and releases the lock as `ichor-controller/machine//` (`machineLockIdentity`, from the name and not the UID, for the reason given above). The same machine re-enters its own lock across reconciles and restarts; any other machine or operation waits. The wait is `ClusterLockHeld` on the Ready condition (join) or the Removal condition (removal), worded `the cluster lock is held by TalosMachine "/"; control plane join deferred` (or `removal deferred`). A holder that is an operation reads `TalosOperation "/"`, and an operation that waits for a machine reads `... held by TalosMachine "/"; this operation starts when it finishes`. Any other holder, such as `ichor-controller/cni-helm`, keeps clusterlock's wording. - A holder that will never renew is not waited for, whoever waits (`lockHolderStopped`, one function for operations and machines). A machine holder is stopped when the machine no longer exists, or when it is not being deleted and is done with its locked step. For a join that step is handing the configuration to the node in maintenance mode, which happens inside one reconcile and ends with `status.configStoredAt`, so a machine with `configStoredAt` set, or Ready, holds nothing. For a removal the step is the removal itself, which runs until the finalizer goes, so a machine with a deletion timestamp is not stopped; once the finalizer is gone it no longer exists and its lease is. There is no failed state to add for machines: a refused or failed join or removal releases the lock before it returns (the deferred `Release`), and the next reconcile takes it again. The lease is only ever left behind by a controller that died mid-step, which these rules turn from a five-minute wait into none when the machine is gone or finished. - Upgrade: a lease the previous build left under `ichor-controller` is held by nobody this build can name, so machines wait for it as operations do, and it ages out within `clusterlock.DefaultDuration` (five minutes) because nothing renews it. No migration code. - Worker joins take no lock and still do not: a worker is not an etcd member, and only a control-plane join reaches `acquireDisruptiveLock` (covered by `joins a worker while the cluster lock is held`). The removal of a worker that was reset (ADR 0085) still takes it, as before, to serialize its drain with everything else. - The Cilium Helm Job (`ichor-controller/cni-helm`, ADR 0113) is unaffected: it was already a separate identity. A control-plane join still waits while it holds the lock, and the Job still waits for a machine or operation that holds it. - Not changed, and worth knowing: the join holds the lock only while the configuration is handed over, not while the node installs, reboots and becomes an etcd member, and nothing in the lock keeps a second join or a removal from starting after the first reconcile ends. What the lock now guarantees is that two steps never overlap; the control-plane count (the even-count refusal and the approval cap) is what paces the membership changes across time.