feat: sequenced rolling restart, MAIN and Raft leader last

CLAUDE.md

@@ -47,22 +47,25 @@ CI (`.github/workflows/`) runs `make lint-config`, `make lint`, `make test-unit`
 
 ## Architecture
 
-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:
+The PRD defines seven modules with two pure cores and one mock seam; an eighth (the rolling-restart decision) was added post-v1 as a third pure core. 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 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, show replication lag, 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`). Moving MAIN off a retiring instance is the one step that can lose data, so it has a precondition nothing else does: a survivor that is both reachable and reported by `SHOW REPLICATION LAG` as holding every transaction the MAIN committed, in every database. Without one, the demotion, the promotion and that instance's unregistration are all left out of the plan and the retiring MAIN keeps serving — a scale-down that pauses, not one that drops writes. Because that state plans *nothing*, an empty plan is not proof a retirement finished: `planner.Retired` is what gates shedding the pods. 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.
+7. **Rolling-restart decision** (`internal/rollout/`) — the third pure core: both roles' pods reduced to `{name, revisionHash, ready}` plus each StatefulSet's `UpdateRevision`, the `SHOW INSTANCES` view and `SHOW REPLICATION LAG` in, **exactly one** action out (`Done` / `Wait(reason)` / `Delete(pod)`). Both StatefulSets use `updateStrategy: OnDelete`, so the operator owns every pod restart and this decides which pod is next: data instances before coordinators, the observed MAIN last of its role, the Raft leader last of its. Nothing is persisted — pods already carrying the new revision *are* the ones already restarted, so a mid-roll spec revert or a Raft-driven MAIN move self-corrects. One action per pass and never a list, because every step re-gates on fresh lag. It issues no Bolt commands at all; a roll is invisible to the planner.
+8. **E2E harness** (`test/e2e/`, build tag `e2e`) — multi-node KinD with real Memgraph images.
 
-Test philosophy (from the PRD): assert external behavior, never internal call ordering or private state. Builders get golden tests, planner gets pure topology-diff cases, controller gets envtest with the HA client mocked.
+Test philosophy (from the PRD): assert external behavior, never internal call ordering or private state. Builders get golden tests, planner gets pure topology-diff cases, the rolling-restart decision gets pure cases over pod revisions and observed cluster state, controller gets envtest with the HA client mocked.
 
 ## Conventions
 
 - 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 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.
+- Workload pods: non-root uid 101 / gid 103, seccomp RuntimeDefault, all capabilities dropped, `terminationGracePeriodSeconds: 300` (a ceiling, not a delay — an instance killed mid-shutdown recovers from its WAL and lengthens the catch-up a roll waits on).
+- Both StatefulSets use `updateStrategy: OnDelete`, so **nothing but the operator ever restarts a workload pod**. A pod-template change no reconcile acts on takes effect never, which is what the `Updated` condition exists to report. `RollingUpdate` cannot express the required order (it sweeps highest ordinal to lowest, and `partition` is a descending cutoff, not a set), so a MAIN on any ordinal but 0 would be restarted mid-sweep and each such restart buys another failover.
+- The operator never promotes a MAIN outside bootstrap and the scale-down handover: a roll deletes the MAIN's pod and lets the Raft coordinators promote. That is only safe on a Memgraph reporting an unreachable MAIN as `role=main, health=down` — a release that vacates the `main` row makes `planner.Plan` believe there is no MAIN and race the failover.
 - Log messages follow Kubernetes style: capital first letter, no trailing period, past tense, object type named (see AGENTS.md for examples).

api/v1alpha1/memgraphcluster_types.go

@@ -133,6 +133,16 @@ const (
 	// --for=condition=Converged` therefore means a scale is genuinely finished,
 	// not merely accepted.
 	ConditionConverged = "Converged"
+
+	// ConditionUpdated is True when every workload pod runs the pod template the
+	// spec currently describes. Because both StatefulSets use updateStrategy
+	// OnDelete, Kubernetes replaces no pod on its own: the operator restarts them
+	// one at a time, data instances before coordinators, MAIN and the Raft leader
+	// last. It is kept apart from Converged deliberately — Converged answers
+	// "does the cluster have the declared members", this one answers "do they run
+	// the declared template", and a user looking at a False condition needs to
+	// know which of the two is happening.
+	ConditionUpdated = "Updated"
 )
 
 // Condition reasons reported on MemgraphCluster status. Reasons are CamelCase
@@ -198,6 +208,23 @@ const (
 	// survivors are down, or too far behind to catch up.
 	ReasonNoCaughtUpSurvivor = "NoCaughtUpSurvivor"
 
+	// ReasonRollingRestartInProgress is set while the operator is restarting pods
+	// to bring them onto the pod template the spec currently describes. The
+	// message names the pod being restarted and why it is that one's turn, because
+	// the order is the whole safety argument: every data instance except MAIN
+	// first, then MAIN, then the coordinators with the Raft leader last.
+	ReasonRollingRestartInProgress = "RollingRestartInProgress"
+
+	// ReasonWaitingForCatchUp is set while a rolling restart waits for the data
+	// instance it restarted last to hold every transaction the MAIN has committed
+	// again. Until it does, restarting the next pod would leave recent writes on
+	// the MAIN alone.
+	ReasonWaitingForCatchUp = "WaitingForCatchUp"
+
+	// ReasonAllPodsUpdated is set when every workload pod runs the pod template
+	// the spec currently describes.
+	ReasonAllPodsUpdated = "AllPodsUpdated"
+
 	// ReasonMainElected is set when a data instance is observed as MAIN.
 	ReasonMainElected = "MainElected"
 

charts/memgraph-operator/rbac/manager-rules.yaml

@@ -13,6 +13,15 @@
 # Generated from the +kubebuilder:rbac markers in the controller sources.
 # Regenerate with 'make chart-sync'; do not edit by hand. To widen or tighten
 # the permissions, edit the markers -- 'make chart-verify' fails if the two drift.
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:

cmd/main.go

@@ -25,10 +25,14 @@ import (
 	// to ensure that exec-entrypoint and run can make use of them.
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
+	corev1 "k8s.io/api/core/v1"
+	klabels "k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/cache"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/healthz"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
@@ -38,6 +42,7 @@ import (
 	memgraphcomv1alpha1 "github.com/memgraph/kubernetes-operator/api/v1alpha1"
 	"github.com/memgraph/kubernetes-operator/internal/controller"
 	"github.com/memgraph/kubernetes-operator/internal/memgraph"
+	"github.com/memgraph/kubernetes-operator/internal/resources"
 	// +kubebuilder:scaffold:imports
 )
 
@@ -160,8 +165,21 @@ func main() {
 		Metrics:                metricsServerOptions,
 		WebhookServer:          webhookServer,
 		HealthProbeBindAddress: probeAddr,
-		LeaderElection:         enableLeaderElection,
-		LeaderElectionID:       "a5adec69.memgraph.com",
+		// Pods are cached, because the rolling restart needs each one's
+		// controller-revision-hash and readiness on every pass — but only this
+		// operator's own pods are. Watching every pod in the cluster to find them
+		// would cost memory proportional to somebody else's workload.
+		Cache: cache.Options{
+			ByObject: map[client.Object]cache.ByObject{
+				&corev1.Pod{}: {
+					Label: klabels.SelectorFromSet(klabels.Set{
+						resources.ManagedByLabel: resources.ManagedByValue,
+					}),
+				},
+			},
+		},
+		LeaderElection:   enableLeaderElection,
+		LeaderElectionID: "a5adec69.memgraph.com",
 		// LeaderElectionReleaseOnCancel defines if the leader should step down voluntarily
 		// when the Manager ends. This requires the binary to immediately end when the
 		// Manager is stopped, otherwise, this setting is unsafe. Setting this significantly

config/rbac/role.yaml

@@ -4,6 +4,15 @@ kind: ClusterRole
 metadata:
   name: manager-role
 rules:
+- apiGroups:
+  - ""
+  resources:
+  - pods
+  verbs:
+  - delete
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:

internal/controller/fake_memgraph_test.go

@@ -211,7 +211,7 @@ func (c *fakeClient) ShowReplicationLag(context.Context) ([]memgraph.Replication
 		lag = append(lag, memgraph.ReplicationLag{
 			Instance: instance.Name,
 			Databases: []memgraph.DatabaseLag{{
-				Database:       "memgraph",
+				Database:       memgraphDbName,
 				CommittedTxns:  100 - behind,
 				TxnsBehindMain: behind,
 			}},