fix(controllers): stop exempting the bootstrap seed from self-heal

controllers/etcdmember_controller.go

@@ -1053,8 +1053,14 @@ func (r *EtcdMemberReconciler) updateStatus(ctx context.Context, member *lll.Etc
 		// outage never cascades into mass deletion. Covers memory members
 		// too: the pod-loss check needs the Pod gone, but a wedged member's
 		// Pod stays alive under the same UID.
-		if !member.Spec.Bootstrap &&
-			etcdContainerStuck(pod) &&
+		//
+		// Covers the bootstrap seed too. Only the bootstrap *window* needs
+		// protecting, and the quorum gate already does it: ReadyMembers stays 0
+		// until clusterID latches, so nothing passes at any replica count (which
+		// also permanently protects a 1-replica cluster's only member). Gating
+		// on spec.bootstrap instead made the exemption outlive the window and
+		// stranded corrupt seeds forever. Phase, not identity.
+		if etcdContainerStuck(pod) &&
 			r.clusterHasQuorumWithout(ctx, member) {
 			log.Info("etcd member is persistently crash-looping while the rest of the cluster is healthy; deleting it for replacement",
 				"restartThreshold", dataLossRestartThreshold)

controllers/etcdmember_controller_test.go

@@ -1027,9 +1027,11 @@ func TestUpdateStatus_KeepsStuckMemberWithoutQuorum(t *testing.T) {
 	}
 }
 
-// TestUpdateStatus_KeepsStuckBootstrapMember: the bootstrap seed is never
-// self-healed by deletion — there is nothing to replace it from yet.
-func TestUpdateStatus_KeepsStuckBootstrapMember(t *testing.T) {
+// TestUpdateStatus_ReplacesStuckSeedAfterBootstrap: the bootstrap seed enjoys
+// no lifelong exemption. Once the cluster is formed it is an ordinary voter, so
+// a crash-looping seed backed by a healthy majority is replaced like any other
+// member. Exempting it on identity used to strand it crash-looping forever.
+func TestUpdateStatus_ReplacesStuckSeedAfterBootstrap(t *testing.T) {
 	ctx := context.Background()
 	cluster := &lll.EtcdCluster{
 		ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"},
@@ -1040,15 +1042,81 @@ func TestUpdateStatus_KeepsStuckBootstrapMember(t *testing.T) {
 		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Bootstrap: true, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
 	}
 	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-0", "ns"))
+	// Cluster is formed and the other two members are ready → quorum without
+	// the seed, exactly as for any non-seed member.
 	clusterWithReady(t, c, "test", "ns", 2)
 
 	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)}
 	if _, err := r.updateStatus(ctx, member); err != nil {
 		t.Fatalf("updateStatus: %v", err)
 	}
 
+	err := c.Get(ctx, types.NamespacedName{Name: "test-0", Namespace: "ns"}, &lll.EtcdMember{})
+	if !apierrors.IsNotFound(err) {
+		t.Fatalf("expected a formed cluster's seed to be self-healed like any other member; Get err = %v", err)
+	}
+}
+
+// TestUpdateStatus_KeepsStuckSeedDuringBootstrap: the bootstrap *window* is
+// what must be protected, and the quorum gate alone protects it. Before
+// clusterID is latched the cluster controller never runs updateStatus, so
+// ReadyMembers is 0 and no member — seed or otherwise — can pass the gate.
+// Deleting the seed here would destroy the only copy of a cluster that no other
+// member has joined yet.
+func TestUpdateStatus_KeepsStuckSeedDuringBootstrap(t *testing.T) {
+	ctx := context.Background()
+	cluster := &lll.EtcdCluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"},
+		Spec:       lll.EtcdClusterSpec{Replicas: ptrInt32(3)},
+	}
+	member := &lll.EtcdMember{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-0", Namespace: "ns", Labels: memberLabels("test", "test-0")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Bootstrap: true, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
+	}
+	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-0", "ns"))
+	// Mid-bootstrap: clusterID unlatched, nothing has ever been counted ready.
+	clusterWithReady(t, c, "test", "ns", 0)
+
+	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)}
+	if _, err := r.updateStatus(ctx, member); err != nil {
+		t.Fatalf("updateStatus: %v", err)
+	}
+
+	if err := c.Get(ctx, types.NamespacedName{Name: "test-0", Namespace: "ns"}, &lll.EtcdMember{}); err != nil {
+		t.Fatalf("seed must NOT be self-deleted while the cluster is still forming; Get err = %v", err)
+	}
+}
+
+// TestUpdateStatus_KeepsStuckSoleMember: a single-member cluster's only member
+// can never be self-healed, seed or not — there is no majority to survive its
+// removal, so the quorum gate holds for the life of the cluster. Total loss is
+// reported rather than healed.
+func TestUpdateStatus_KeepsStuckSoleMember(t *testing.T) {
+	ctx := context.Background()
+	cluster := &lll.EtcdCluster{
+		ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"},
+		Spec:       lll.EtcdClusterSpec{Replicas: ptrInt32(1)},
+	}
+	member := &lll.EtcdMember{
+		ObjectMeta: metav1.ObjectMeta{Name: "test-0", Namespace: "ns", Labels: memberLabels("test", "test-0")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Bootstrap: true, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
+	}
+	// The worst case for the gate: ReadyMembers has not yet been decremented
+	// for the member that just started crash-looping, and the member's own
+	// status still says Ready. Subtracting it is what keeps readyOthers at 0.
+	member.Status.Conditions = []metav1.Condition{{
+		Type: lll.MemberReady, Status: metav1.ConditionTrue, Reason: "Ready", LastTransitionTime: metav1.Now(),
+	}}
+	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-0", "ns"))
+	clusterWithReady(t, c, "test", "ns", 1) // stale-high: still counts test-0
+
+	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)}
+	if _, err := r.updateStatus(ctx, member); err != nil {
+		t.Fatalf("updateStatus: %v", err)
+	}
+
 	if err := c.Get(ctx, types.NamespacedName{Name: "test-0", Namespace: "ns"}, &lll.EtcdMember{}); err != nil {
-		t.Fatalf("bootstrap member must NOT be self-deleted; Get err = %v", err)
+		t.Fatalf("the only member of a 1-replica cluster must NOT be self-deleted; Get err = %v", err)
 	}
 }
 

docs/concepts.md

@@ -88,7 +88,9 @@ The cluster forms from a single seed member. Multi-seed bootstrap (multiple memb
 
 **Discovery** is the bridge between "seed pod is up" and "operator knows the cluster ID". The cluster controller calls `MemberList` against the seed's client URL, validates the response (exactly one member, matching the seed's name or peer URL), and latches `status.clusterID`. Once latched, discovery is never run again.
 
-The seed is identified by `spec.bootstrap=true`. Member names being random precludes a name-based lookup, and trusting list order (`members[0]`) silently anchors discovery to the wrong member when scale-up CRs land in front of the seed. Once `clusterID` is set, the operator never re-reads `spec.bootstrap` for any decision — the seed is, from that point on, just a regular member.
+The seed is identified by `spec.bootstrap=true`. Member names being random precludes a name-based lookup, and trusting list order (`members[0]`) silently anchors discovery to the wrong member when scale-up CRs land in front of the seed. Once `clusterID` is set, no *membership* decision re-reads `spec.bootstrap` — the seed is, from that point on, just a regular member. It is not scheduled differently, not weighted in quorum, and not exempt from [crash-loop self-heal](#crash-loop-self-heal). A cluster running with no `spec.bootstrap=true` member at all is a normal, supported state: it is what every cluster adopted by `cmd/etcd-migrate` starts out as, and what any cluster becomes once its seed is replaced.
+
+One reader does survive past latch: `buildPod` still derives `--initial-cluster-state` from `spec.bootstrap`, so a *re-created* seed Pod is handed `=new` rather than `=existing` for the life of the cluster. etcd consults that flag only when the data dir is empty, so it is inert on every ordinary restart. It is not inert if the seed's data dir is ever lost while the PVC binding survives — see [Known gap: a seed that re-bootstraps](#known-gap-a-seed-that-re-bootstraps).
 
 If the seed's pod hasn't been created yet (between Create and Pod-up), the controller surfaces `Progressing=True/WaitingForSeed` rather than dialing a nonexistent endpoint and burning the reconcile budget.
 
@@ -163,9 +165,27 @@ The member controller detects this and replaces the member:
 
 - **Trigger.** The etcd container is not ready and has restarted at least `dataLossRestartThreshold` (5) times. `OOMKilled` is excluded (whether it's the current or the last termination) — that's a resource problem re-creating the member would not fix — and a Pod that is itself being deleted (drain/eviction/manual restart) is never treated as stuck.
 - **Quorum gate.** The operator deletes the member only when the *rest* of the cluster still has quorum, so a cluster-wide outage (many members crashing at once) never cascades into mass deletion. The count is read from `Status.ReadyMembers`, which the cluster controller maintains and which can lag; if the stuck member is still counted ready, the gate subtracts it. As a second line of defence the finalizer's `MemberRemove` is itself quorum-gated, so even a stale-high count cannot delete data below quorum.
+- **No seed exemption.** The trigger is the member's *state*, never its identity, so the bootstrap seed is replaced on the same terms as anyone else. Only the bootstrap *window* needs protecting, and the quorum gate delivers that for free: `updateStatus` does not run until `clusterID` is latched, so `ReadyMembers` is 0 throughout bootstrap and nothing can pass the gate. Phase expires; identity does not.
 - **Replacement.** Deleting the `EtcdMember` runs the finalizer's clean `MemberRemove`, any member-owned PVC is GC'd (discarding the corrupt data dir; memory members have none), and the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID — not a same-ID rejoin.
 - **Latency.** `CrashLoopBackOff` caps its backoff at 5 minutes, so reaching 5 restarts takes on the order of **tens of minutes**, not the ~5s of the memory Pod-loss path. A deliberately-deleted-and-replaced member during this window is expected operator behavior, not a fault. A slow restore or slow learner join on the *replacement* can itself trip the threshold and be replaced again; this is quorum-gated and self-limiting, but expect it on a struggling cluster.
 
+### Known gap: a seed that re-bootstraps
+
+Crash-loop self-heal keys on a Pod that will not become ready. There is one seed-specific data-loss shape it therefore cannot see, and it is worth knowing about because it is *quieter* than the crash-loop it resembles.
+
+`buildPod` sets `--initial-cluster-state=new` for `spec.bootstrap=true` members and `=existing` for everyone else, and `spec.bootstrap` is never cleared — so the seed's Pod is built with `=new` for the whole life of the cluster, not just for its first boot. Its `--initial-cluster` is likewise frozen at the value written during bootstrap, which lists only itself.
+
+etcd reads both flags only when the data dir is empty. So on an ordinary restart, or on a restart onto a *corrupt* data dir, nothing changes: a corrupt dir makes etcd fail to boot, the Pod crash-loops, and self-heal replaces the member as described above. The gap is the case where the seed's data dir comes back **empty** rather than corrupt, with the PVC binding intact — a re-provisioned or wiped volume, a PV restored blank, node-local storage lost on reimage. Then:
+
+- a non-seed member gets `=existing` against a stale `--initial-cluster` and fails loudly (`member count is unequal`), crash-loops, and is self-healed;
+- the **seed** gets `=new` against an `--initial-cluster` naming only itself, which is a complete and internally consistent bootstrap instruction. etcd does not error. It forms a fresh one-member cluster on the empty dir and reports healthy.
+
+The Pod then becomes *ready*, so neither self-heal trigger fires — not the `Status.PodUID` check (the Pod was never lost) and not the crash-loop check (`etcdContainerStuck` requires not-ready). Meanwhile `<cluster>-client` selects every member Pod with no role filter, so a share of client traffic lands on a member serving an empty keyspace, and writes routed there are invisible to the real cluster.
+
+Note also that etcd derives both the cluster ID and the member ID from the initial peer-URL set plus `--initial-cluster-token`, all of which are unchanged here — so the re-bootstrapped seed is expected to come back up under the *same* cluster ID as the cluster it lost, rather than being rejected on an ID mismatch. That reasoning is from etcd's ID derivation, not from an observed incident.
+
+Closing this means decoupling "which member bootstrapped the cluster" from "which member should boot with `=new`" — the same phase-versus-identity split that removed the self-heal exemption. It is tracked separately from that fix.
+
 ### What is missing from memory clusters today
 
 Two things are not auto-defaulted and matter for production memory clusters — both tracked in [issue #16](https://github.com/lllamnyp/etcd-operator/issues/16):

docs/operations.md

@@ -484,10 +484,11 @@ kubectl get etcdmember.etcd-operator.cozystack.io -n default -w
 
 The `Status.PodUID` mechanism above keys on the Pod disappearing. It cannot see a member whose Pod is *alive* but whose etcd can never start: the Pod keeps its UID while the etcd container crash-loops inside it. There is a separate, second trigger for that state, covering both storage media.
 
-If a non-bootstrap member's etcd cannot start — because its frozen `--initial-cluster` went stale while the cluster membership moved on (`error validating peerURLs ... member count is unequal`), either a PVC member whose data dir was lost (e.g. a volume lost on node failure) or a replacement learner of any medium whose membership changed between Pod creation and its first successful boot — it crash-loops forever with no recovery path of its own. The operator detects this and replaces it:
+If a member's etcd cannot start — because its frozen `--initial-cluster` went stale while the cluster membership moved on (`error validating peerURLs ... member count is unequal`), either a PVC member whose data dir was lost (e.g. a volume lost on node failure) or a replacement learner of any medium whose membership changed between Pod creation and its first successful boot — it crash-loops forever with no recovery path of its own. The operator detects this and replaces it:
 
 - **Detection**: the etcd container is not ready and has restarted at least 5 times (`dataLossRestartThreshold`), excluding `OOMKilled` (a resource problem, not a lost data dir — raising `spec.resources.limits.memory` is the fix there, not replacement). A Pod that is being deleted (drain, eviction, manual restart) is never treated as stuck.
 - **Quorum gate**: the operator deletes the member only when the *rest* of the cluster still has quorum, so a cluster-wide outage never cascades into mass deletion. The gate reads `Status.ReadyMembers` (maintained by the cluster controller, and possibly lagging) and subtracts the stuck member if it is still counted; the finalizer's `MemberRemove` is independently quorum-gated as a backstop.
+- **The seed is included**: having formed the cluster is an origin fact, not a permanent exemption. Only the bootstrap *window* is protected, and the quorum gate does it alone — before `clusterID` is latched nothing has ever been counted ready, so `ReadyMembers` is 0 and no member can pass. That same arithmetic permanently protects a 1-replica cluster's only member.
 - **Replacement**: the `EtcdMember` CR is deleted → finalizer `MemberRemove` → any member-owned `data-<member>` PVC is GC'd (discarding the corrupt data dir; memory members have none) → the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID.
 
 **Detection latency is much longer than the Pod-loss path.** `CrashLoopBackOff` caps backoff at 5 minutes, so reaching 5 restarts takes **tens of minutes**, not ~5 s. Budget for that before concluding the operator is misbehaving — a member that vanishes and is replaced by a fresh-named one after a long crash-loop is the operator working as designed, not flapping. Note also that a replacement which is itself slow to come up (slow restore, slow learner join) can trip the same threshold and be replaced again; this is quorum-gated and harmless to the cluster, but expect repeated replacement on a genuinely unhealthy member.

test/e2e/member_selfheal_test.go

@@ -74,7 +74,13 @@ func TestPVCMemberCrashLoopSelfHeal(t *testing.T) {
 	if len(original) != 3 {
 		t.Fatalf("expected 3 members, got %d: %v", len(original), original)
 	}
-	victim := original[0]
+	// Target the bootstrap seed on purpose. It used to be exempt from self-heal
+	// for the life of the cluster, so a corrupt seed crash-looped forever; the
+	// exemption now expires with the bootstrap window. Picking it deliberately
+	// also makes the victim deterministic — member names are apiserver-assigned
+	// random suffixes, so indexing into a name-sorted list chose the seed about
+	// a third of the time and turned this into a coin-flip test.
+	victim := selfHealSeedMember(ctx, t)
 	victimPVC := "data-" + victim
 	t.Logf("corrupting data dir of victim member %q (pvc %q)", victim, victimPVC)
 
@@ -99,7 +105,10 @@ func TestPVCMemberCrashLoopSelfHeal(t *testing.T) {
 			if err != nil {
 				return err
 			}
-			return fmt.Errorf("victim %q still present (crash-loop not yet past threshold)", victim)
+			return fmt.Errorf("victim %q still present; self-heal has not deleted it "+
+				"(either the crash-loop is not yet past the restart threshold, or a gate is "+
+				"rejecting it — check the member's restartCount against dataLossRestartThreshold "+
+				"and the cluster's readyMembers against the quorum gate)", victim)
 		})
 
 	// The corrupt member's PVC must be GC'd (owner-ref), discarding the bad
@@ -168,6 +177,30 @@ func selfHealMembers(ctx context.Context, t *testing.T) []string {
 	return names
 }
 
+// selfHealSeedMember returns the name of the cluster's bootstrap seed — the one
+// member with spec.bootstrap=true. Asserts the single-seed invariant the cluster
+// controller relies on (it locates the seed by that field and assumes exactly
+// one), so a wrongly-shaped cluster fails loudly here rather than silently
+// selecting some other member.
+func selfHealSeedMember(ctx context.Context, t *testing.T) string {
+	t.Helper()
+	list := &etcdv1alpha2.EtcdMemberList{}
+	if err := kube.List(ctx, list, client.InNamespace(selfHealNamespace),
+		client.MatchingLabels{"etcd-operator.cozystack.io/cluster": selfHealCluster}); err != nil {
+		t.Fatalf("list members: %v", err)
+	}
+	var seeds []string
+	for i := range list.Items {
+		if list.Items[i].Spec.Bootstrap {
+			seeds = append(seeds, list.Items[i].Name)
+		}
+	}
+	if len(seeds) != 1 {
+		t.Fatalf("expected exactly one spec.bootstrap=true member, got %d: %v", len(seeds), seeds)
+	}
+	return seeds[0]
+}
+
 // selfHealMembersErr is the error-tolerant form for use inside waitFor (a list
 // error returns an empty slice; the caller's own assertions then retry).
 func selfHealMembersErr(ctx context.Context) []string {