feat: coordinator scale-down with leadership-safe Raft removal

CLAUDE.md

@@ -49,10 +49,10 @@ CI (`.github/workflows/`) runs `make lint-config`, `make lint`, `make test-unit`
 
 The PRD defines seven modules with two pure cores and one mock seam. Keep this separation — it's what makes the logic testable without a cluster:
 
-1. **API types** (`api/v1alpha1/`) — spec/status with kubebuilder + CEL markers. Replica counts (coordinators, data instances) are **mutable**, bounded by schema floors alone (coordinators ≥ 3 and odd, data instances ≥ 1) — enforced by the CRD, not a webhook. Raising a count grows the cluster; lowering one is accepted but not yet carried out (see `specs/operator-mvp/issues/15`, `16`). Defaults are declared as CRD schema defaults *and* mirrored as Go constants in `memgraphcluster_types.go` so resource builders behave correctly on specs that never passed admission (unit tests).
+1. **API types** (`api/v1alpha1/`) — spec/status with kubebuilder + CEL markers. Replica counts (coordinators, data instances) are **mutable in both directions**, bounded by schema floors alone (coordinators ≥ 3 and odd, data instances ≥ 1) — enforced by the CRD, not a webhook. Defaults are declared as CRD schema defaults *and* mirrored as Go constants in `memgraphcluster_types.go` so resource builders behave correctly on specs that never passed admission (unit tests).
 2. **Resource builders** — pure functions: spec in, desired Kubernetes objects out (one StatefulSet per role + headless Services; per-pod identity such as coordinator ID and advertised addresses derived from pod ordinals). No API calls, no side effects. Tested golden-style.
-3. **Memgraph HA client** — a narrow Go interface (show instances, register instance, add coordinator, set main) over the Bolt driver. Everything above depends on the interface, never the driver — this is the mock seam.
-4. **Registration planner** — pure diff: declared topology + observed `SHOW INSTANCES` in, ordered registration commands out (empty when converged). Reconciliation semantics live here: read-before-write, idempotent, re-issue only missing registrations. A MAIN is promoted only when the cluster has none — at bootstrap, and after the planner itself demoted a data instance that is retiring; a MAIN that is staying is never overridden, because failover belongs to the Raft coordinators. A lowered `dataInstances` count is the one removal the planner drives: `DEMOTE INSTANCE` the retiring MAIN, promote a survivor, `UNREGISTER INSTANCE` the retiring members, and only then does the controller shed their pods (see `specs/operator-mvp/issues/15`).
+3. **Memgraph HA client** — a narrow Go interface (show instances, register instance, add coordinator, set main, demote/unregister instance, remove coordinator, yield leadership) over the Bolt driver. Everything above depends on the interface, never the driver — this is the mock seam.
+4. **Registration planner** — pure diff: declared topology + observed `SHOW INSTANCES` in, ordered registration commands out (empty when converged). Reconciliation semantics live here: read-before-write, idempotent, re-issue only missing registrations. A MAIN is promoted only when the cluster has none — at bootstrap, and after the planner itself demoted a data instance that is retiring; a MAIN that is staying is never overridden, because failover belongs to the Raft coordinators. A lowered count is the one removal the planner drives: `DEMOTE INSTANCE` the retiring MAIN, promote a survivor, `UNREGISTER INSTANCE` the retiring data instances, `REMOVE COORDINATOR` the retiring coordinators, and only then does the controller shed their pods (see `specs/operator-mvp/issues/15`, `16`). One command breaks the pure-diff mould: `YIELD LEADERSHIP`, needed because Raft refuses to remove its own leader. It names no successor, so it is always a plan's **last** command and terminal — the controller requeues and re-observes under whichever coordinator won the election.
 5. **Controller** (`internal/controller/`) — fetch CR, server-side-apply builder output, gate on pod readiness, run planner against the HA client, write status/conditions.
 6. **Operator install chart** (`charts/memgraph-operator/`) — lives in this repo, cross-published to `memgraph.github.io/helm-charts` at release. Its `crds/` and `rbac/manager-rules.yaml` are **generated** (`make chart-sync`, verified by `make chart-verify`): the manager's ClusterRole comes from the `+kubebuilder:rbac` markers, so tightening or widening the controller's permissions means editing the markers, never the chart. The e2e suite installs the operator through this chart, so every scenario runs under the RBAC users get. The chart's `version` and its `appVersion` (the operator image tag) move **independently**: tag `v<version>` releases the operator, `chart-<version>` releases the chart alone — see `docs/releasing.md`.
 7. **E2E harness** (`test/e2e/`, build tag `e2e`) — multi-node KinD with real Memgraph images.
@@ -63,6 +63,6 @@ Test philosophy (from the PRD): assert external behavior, never internal call or
 
 - Spec knob names mirror the HA Helm chart's vocabulary where the concept carries over (e.g. the `secrets.name` / `secrets.licenseKey` / `secrets.organizationKey` block) — check the chart before inventing a name.
 - No secret material in spec or status; secrets are consumed by reference only.
-- Storage is never deleted by the operator: no finalizer-based cleanup; PVC retention (deletion *and* scale-down) maps to the StatefulSet PVC retention policy (default `Retain`). The only cluster members the operator removes are the data instances a lowered `dataInstances` count retires; coordinators are never removed (`REMOVE COORDINATOR` arrives with `specs/operator-mvp/issues/16`).
+- Storage is never deleted by the operator: no finalizer-based cleanup; PVC retention (deletion *and* scale-down) maps to the StatefulSet PVC retention policy (default `Retain`). The only cluster members the operator removes are the ones a lowered replica count retires; a coordinator removed from Raft keeps running and keeps its state on purpose, which is what makes re-growing onto a retained volume safe.
 - Workload pods: non-root uid 101 / gid 103, seccomp RuntimeDefault, all capabilities dropped.
 - Log messages follow Kubernetes style: capital first letter, no trailing period, past tense, object type named (see AGENTS.md for examples).

README.md

@@ -175,7 +175,7 @@ The MVP is deliberately "provision, bootstrap, observe". It does:
 - bootstrap HA: add the coordinators, register the data instances, and promote the initial MAIN once;
 - re-register continuously: every reconcile compares `SHOW INSTANCES` on the coordinator leader against the declared topology and issues only the missing registrations, so an instance that loses its registration state (say, after being rescheduled onto a fresh node) rejoins without human action;
 - **grow a live cluster**: raise `coordinators` or `dataInstances` (both in one edit if you like, in any step size) and the added pods are provisioned and registered by the same diff that restores a lost registration — no manual `ADD COORDINATOR` or `REGISTER INSTANCE`;
-- **shrink the data instances**: lower `dataInstances` and the instances above the new count are retired — MAIN moved off them if one of them holds it, then `UNREGISTER INSTANCE`, and only then are their pods shed, so the coordinators never expect an instance whose pod is gone;
+- **shrink a live cluster**: lower `dataInstances` or `coordinators` and the members above the new count are retired before their pods are shed — a data instance has MAIN moved off it if it holds it and is then `UNREGISTER INSTANCE`d, a coordinator is `REMOVE COORDINATOR`ed out of the Raft cluster — so the coordinators never expect an instance whose pod is gone, and no removed member's pod outlives its vote;
 - report the observed MAIN, the registered member counts, and the readiness and convergence conditions on the resource's status.
 
 Scaling is one edit, and `Converged` tells you when it is finished:
@@ -185,17 +185,17 @@ kubectl patch mgc memgraph -n memgraph --type=merge -p '{"spec":{"coordinators":
 kubectl wait --namespace memgraph --for=condition=Converged memgraphcluster/memgraph --timeout=10m
 ```
 
-A scale-down reports `Converged=False` with reason `RetirementInProgress`, naming the instances on their way out, until their pods are gone. Two things to know about it:
+Both counts have a floor the schema enforces at creation and on every update: `coordinators` must stay odd and at or above three, `dataInstances` at or above one. A scale-down reports `Converged=False` with reason `RetirementInProgress`, naming the members on their way out, until their pods are gone. Three things to know about it:
 
-- **A retiring pod that cannot become ready blocks its own removal.** The operator only touches the cluster when every pod of both StatefulSets is ready, and until the shrink is applied the retiring pods still belong to the data StatefulSet. So an instance that is stuck (crash-looping, unschedulable, wedged in a snapshot restore) keeps its own retirement waiting, and the resource reports `WorkloadsNotReady` rather than the operator writing to a cluster whose state it only half knows. Fix the pod, or delete it if it is genuinely unrecoverable, and the retirement continues.
-- **The claims of a retired instance follow `spec.storage.retentionPolicy`**, the same knob that decides what happens to storage when the cluster is deleted — `Retain` (the default) keeps them, so a shrink made by accident loses no data, and re-raising the count reattaches them.
+- **A retiring pod that cannot become ready blocks its own removal.** The operator only touches the cluster when every pod of both StatefulSets is ready, and until the shrink is applied the retiring pods still belong to their StatefulSet. So a member that is stuck (crash-looping, unschedulable, wedged in a snapshot restore) keeps its own retirement waiting, and the resource reports `WorkloadsNotReady` rather than the operator writing to a cluster whose state it only half knows. Fix the pod, or delete it if it is genuinely unrecoverable, and the retirement continues.
+- **The claims of a retired member follow `spec.storage.retentionPolicy`**, the same knob that decides what happens to storage when the cluster is deleted — `Retain` (the default) keeps them, so a shrink made by accident loses no data, and re-raising the count reattaches them. A coordinator removed from Raft keeps running and keeps its state on purpose, which is what makes re-growing onto a retained volume safe: it is in the same position as one whose pod crashed and stayed down, and a later `ADD COORDINATOR` brings it back in.
+- **A coordinator shrink may have to wait for a Raft election.** Raft refuses to remove its own leader, and a StatefulSet sheds only its highest ordinals, so a leader sitting in the retiring range is asked to `YIELD LEADERSHIP` first — which cannot name a successor. The resource reports `Converged=False` with reason `LeadershipTransferInProgress` while that is pending, and the operator asks again if the election happens to pick another retiring coordinator.
 
 What it does not do yet:
 
-- **Scaling the coordinators down.** `coordinators` must stay odd and at or above three, `dataInstances` at or above one — all enforced at creation and on every update. Lowering `coordinators` is accepted by admission but not carried out: dropping a coordinator means removing a Raft member, which the operator does not do yet, so it holds the StatefulSet at its current size and reports `Converged=False` with reason `ScaleInProgress` until the count is raised back.
 - **Failover.** The operator promotes a MAIN only when the cluster has none: once at bootstrap, and once more when it demotes an instance that is retiring. It never overrides a MAIN that is staying — leadership belongs to the Raft coordinators, so two control systems never fight over which instance is MAIN.
 - **Other day-2 operations**: orchestrated or rolling version upgrades, backup and restore, storage-mode changes.
-- **Removing coordinators**: there is no `REMOVE COORDINATOR`, and no finalizer-based storage cleanup — deleting storage is left entirely to the StatefulSet's own retention policy.
+- **Deleting storage**: the operator owns no finalizer and runs no cleanup of its own — deleting a volume is left entirely to the StatefulSet's own retention policy.
 - **External access** of any kind — no LoadBalancer, NodePort, ingress or gateway. Access is in-cluster (or `kubectl port-forward`) only; the approach is expected to change, so it was deliberately deferred rather than shipped and broken later.
 - **TLS**, for Bolt or intra-cluster traffic.
 - **Bolt authentication** — the operator connects to the coordinators unauthenticated, so clusters must not enable auth yet.

api/v1alpha1/memgraphcluster_types.go

@@ -163,18 +163,20 @@ const (
 	// declared topology.
 	ReasonAllInstancesRegistered = "AllInstancesRegistered"
 
-	// ReasonScaleInProgress is set when registration has converged but a
-	// StatefulSet still runs a different number of replicas than the spec
-	// declares, so the declared topology is not fully realized yet.
-	ReasonScaleInProgress = "ScaleInProgress"
-
-	// ReasonRetirementInProgress is set while a lowered dataInstances count is
-	// being carried out: the instances beyond the declared count are still
-	// members of the cluster, or their pods are still being shed. The message
-	// names them, so a scale-down that stalls says which instance it is waiting
-	// on.
+	// ReasonRetirementInProgress is set while a lowered count of either role is
+	// being carried out: the members beyond the declared count are still part of
+	// the cluster, or their pods are still being shed. The message names them, so
+	// a scale-down that stalls says which member it is waiting on.
 	ReasonRetirementInProgress = "RetirementInProgress"
 
+	// ReasonLeadershipTransferInProgress is set while a lowered coordinators
+	// count is waiting on Raft leadership to move: Raft refuses to remove its own
+	// leader, so a retiring coordinator holding leadership is asked to yield it
+	// first. YIELD LEADERSHIP cannot name a successor, so the operator re-observes
+	// the cluster under whichever coordinator won the election and may have to ask
+	// again — which is exactly what this reason means when it persists.
+	ReasonLeadershipTransferInProgress = "LeadershipTransferInProgress"
+
 	// ReasonMainElected is set when a data instance is observed as MAIN.
 	ReasonMainElected = "MainElected"
 

config/samples/v1alpha1_memgraphcluster.yaml

@@ -11,14 +11,15 @@ spec:
   # odd so the Raft quorum cannot split, and at least three — a quorum of one
   # cannot survive losing itself.
   #
-  # Lowering dataInstances shrinks the cluster: the instances above the new count
-  # are retired (MAIN moved off them, then UNREGISTER INSTANCE) before their pods
-  # are shed, reported as Converged=False with reason RetirementInProgress. Note
-  # that the operator only touches the cluster while every pod is ready, so a
-  # retiring pod that cannot become ready blocks its own removal. Lowering
-  # coordinators is not supported yet: it is accepted at admission but the
-  # operator holds the StatefulSet at its current size and reports
-  # Converged=False with reason ScaleInProgress.
+  # Lowering either count shrinks the cluster: the members above the new count are
+  # retired before their pods are shed — a data instance has MAIN moved off it and
+  # is then UNREGISTER INSTANCEd, a coordinator is REMOVE COORDINATORed out of the
+  # Raft cluster — reported as Converged=False with reason RetirementInProgress.
+  # Two notes. The operator only touches the cluster while every pod is ready, so
+  # a retiring pod that cannot become ready blocks its own removal. And Raft
+  # refuses to remove its own leader, so a leader in the retiring range is asked
+  # to YIELD LEADERSHIP first, reported as LeadershipTransferInProgress while that
+  # is pending.
   coordinators: 3
   dataInstances: 2
   # repository carries the registry host and image path only — the version

examples/minimal-cluster.yaml

@@ -14,8 +14,9 @@ spec:
   # Raise either count later to grow the cluster: the operator provisions the
   # new pods and registers them, no manual registration involved. The
   # coordinator count must be odd so the Raft quorum cannot split, and at least
-  # three. Lowering dataInstances retires the instances above the new count —
-  # unregistered before their pods go; lowering coordinators is not supported yet.
+  # three. Lower either count and the members above it are retired first — a data
+  # instance unregistered, a coordinator removed from the Raft cluster — before
+  # their pods go.
   coordinators: 3
   dataInstances: 2
   image:

internal/controller/fake_memgraph_test.go

@@ -88,6 +88,23 @@ func (f *fakeMemgraph) setInstances(instances []memgraph.Instance) {
 	f.instances = slices.Clone(instances)
 }
 
+// setLeader moves Raft leadership onto the named coordinator, which is how a spec
+// parks it where a scale-down cannot remove it: on an ordinal the shrink retires.
+func (f *fakeMemgraph) setLeader(name string) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	for i, instance := range f.instances {
+		if !strings.HasPrefix(instance.Name, "coordinator_") {
+			continue
+		}
+		role := memgraph.RoleFollower
+		if instance.Name == name {
+			role = memgraph.RoleLeader
+		}
+		f.instances[i].Role = role
+	}
+}
+
 // setStaleView makes the coordinator at the given Bolt address answer
 // SHOW INSTANCES with its own view instead of the cluster's.
 func (f *fakeMemgraph) setStaleView(address string, instances []memgraph.Instance) {
@@ -240,6 +257,57 @@ func (c *fakeClient) UnregisterInstance(_ context.Context, name string) error {
 	})
 }
 
+// RemoveCoordinator drops the coordinator with the given Raft ID from the cluster
+// view and — as Raft does — refuses the current leader, so a plan that aims a
+// removal at the leader fails the suite loudly instead of quietly working.
+func (c *fakeClient) RemoveCoordinator(_ context.Context, id int32) error {
+	name := fmt.Sprintf("coordinator_%d", id)
+	return c.execute(fmt.Sprintf("REMOVE COORDINATOR %d", id), func() error {
+		for i, instance := range c.cluster.instances {
+			if instance.Name != name {
+				continue
+			}
+			if instance.IsLeader() {
+				return fmt.Errorf("fake memgraph: %s is the leader", name)
+			}
+			c.cluster.instances = slices.Delete(c.cluster.instances, i, i+1)
+			return nil
+		}
+		return fmt.Errorf("fake memgraph: coordinator %s is not a member", name)
+	})
+}
+
+// YieldLeadership moves leadership off the coordinator serving this connection to
+// the lowest-numbered remaining member, standing in for the election NuRaft runs.
+// A test cannot rely on which coordinator wins — that is the point of the command
+// — only on leadership having moved, which is what the operator has to converge
+// around.
+func (c *fakeClient) YieldLeadership(context.Context) error {
+	self, err := c.selfName()
+	if err != nil {
+		return err
+	}
+	return c.execute("YIELD LEADERSHIP", func() error {
+		successor := -1
+		for i, instance := range c.cluster.instances {
+			if strings.HasPrefix(instance.Name, "coordinator_") && instance.Name != self {
+				successor = i
+				break
+			}
+		}
+		if successor < 0 {
+			return fmt.Errorf("fake memgraph: %s is the only coordinator, so leadership cannot be yielded", self)
+		}
+		for i, instance := range c.cluster.instances {
+			if instance.Name == self {
+				c.cluster.instances[i].Role = memgraph.RoleFollower
+			}
+		}
+		c.cluster.instances[successor].Role = memgraph.RoleLeader
+		return nil
+	})
+}
+
 func (c *fakeClient) Close(context.Context) error {
 	c.cluster.mu.Lock()
 	defer c.cluster.mu.Unlock()