fix(controllers): replace a persistently crash-looping member

Makefile

@@ -80,7 +80,7 @@ test: manifests generate fmt vet envtest ## Run tests.
 
 .PHONY: test-e2e
 test-e2e: ## Run the e2e suite against the current kubeconfig context (expects cert-manager, Kamaji and the operator installed; see hack/e2e.sh).
-	go test -tags e2e -count=1 ./test/e2e/ -v -timeout 30m
+	go test -tags e2e -count=1 ./test/e2e/ -v -timeout 45m
 
 .PHONY: e2e
 e2e: ## Provision a kind cluster with cert-manager and Kamaji, deploy the operator, run the e2e suite. KEEP_CLUSTER=1 keeps the cluster for debugging.

README.md

@@ -32,7 +32,7 @@ The full design rationale is in [docs/concepts.md](docs/concepts.md).
 
 ## What's not supported (yet)
 
-No multi-user / per-tenant RBAC inside etcd — single-user `root` auth is available via `spec.auth.enabled` (BYO credentials Secret; see [docs/concepts.md](docs/concepts.md#authentication)), but every authenticated client is `root`. No in-place version upgrades (changing `spec.version` only affects newly-created members). No PVC resizing — see [#2](https://github.com/lllamnyp/etcd-operator/issues/2). No automatic broken-member replacement for PVC-backed clusters (memory-backed members do auto-replace on Pod loss; `status.brokenMembers` reads 0 in practice — see [docs/concepts.md](docs/concepts.md#storage)). One-shot snapshots and restore-on-bootstrap are supported (see above), but there is no *scheduled* snapshot CRD. No defragmentation scheduling. PodAntiAffinity is supported via `spec.affinity` but not applied by default (defaulting tracked in [#16](https://github.com/lllamnyp/etcd-operator/issues/16)). See the [issue tracker](https://github.com/lllamnyp/etcd-operator/issues) for the running follow-up list.
+No multi-user / per-tenant RBAC inside etcd — single-user `root` auth is available via `spec.auth.enabled` (BYO credentials Secret; see [docs/concepts.md](docs/concepts.md#authentication)), but every authenticated client is `root`. No in-place version upgrades (changing `spec.version` only affects newly-created members). No PVC resizing — see [#2](https://github.com/lllamnyp/etcd-operator/issues/2). PVC-backed members auto-replace only on a *persistent crash-loop* (a lost or corrupt data dir whose etcd cannot boot) — quorum-gated, and far slower than the seconds-fast Pod-loss path memory-backed members get (tens of minutes, at the CrashLoopBackOff cap); a member that is merely slow or flapping is left alone. `status.brokenMembers` still reads 0 in practice — see [docs/concepts.md](docs/concepts.md#storage). One-shot snapshots and restore-on-bootstrap are supported (see above), but there is no *scheduled* snapshot CRD. No defragmentation scheduling. PodAntiAffinity is supported via `spec.affinity` but not applied by default (defaulting tracked in [#16](https://github.com/lllamnyp/etcd-operator/issues/16)). See the [issue tracker](https://github.com/lllamnyp/etcd-operator/issues) for the running follow-up list.
 
 ## Quick start
 

controllers/etcdmember_controller.go

@@ -898,9 +898,91 @@ func restoreInitContainer(member *lll.EtcdMember, peerAddr, operatorImage string
 	}, vols
 }
 
+// dataLossRestartThreshold is how many times the etcd container must have
+// restarted before we treat a non-bootstrap PVC member as unrecoverable and
+// replace it. High enough to ride out transient join churn and slow restores;
+// CrashLoopBackOff caps its backoff at 5m, so this many restarts means a
+// member that has been unable to start for several minutes.
+const dataLossRestartThreshold = 5
+
+// etcdContainerStuck reports whether the pod's etcd container is persistently
+// failing to start: not ready and restarted at least dataLossRestartThreshold
+// times, excluding OOMKilled (a resource problem that re-creating the member
+// would not fix). This is the signature of an unrecoverable member — most
+// commonly a data dir lost while the cluster membership moved on, so the
+// member's frozen --initial-cluster no longer matches the live cluster and
+// etcd refuses to boot ("error validating peerURLs ... member count is
+// unequal"). etcd ignores --initial-cluster for an initialised data dir, so a
+// healthy member that merely restarts is unaffected.
+//
+// A Pod already being deleted (DeletionTimestamp set — manual restart, node
+// drain, eviction) is never "stuck": its containers are terminating on the way
+// to a clean reschedule, and treating that window as unrecoverable would
+// trigger a needless member replacement. OOMKilled is excluded whether it is
+// the last termination or the current one — a just-OOMKilled container sits in
+// State.Terminated before it transitions to Waiting/CrashLoopBackOff.
+func etcdContainerStuck(pod *corev1.Pod) bool {
+	if pod.DeletionTimestamp != nil {
+		return false
+	}
+	for _, cs := range pod.Status.ContainerStatuses {
+		if cs.Name != "etcd" {
+			continue
+		}
+		if cs.Ready || cs.RestartCount < dataLossRestartThreshold {
+			return false
+		}
+		if t := cs.State.Terminated; t != nil && t.Reason == "OOMKilled" {
+			return false
+		}
+		if t := cs.LastTerminationState.Terminated; t != nil && t.Reason == "OOMKilled" {
+			return false
+		}
+		return true
+	}
+	return false
+}
+
+// clusterHasQuorumWithout reports whether the member's cluster still has quorum
+// among its OTHER members — i.e. losing this one is a minority failure, so
+// replacing it cannot break the cluster. Used to gate self-heal: we never
+// delete a member during a cluster-wide outage (where many members crash at
+// once), only an isolated stuck member backed by a healthy majority.
+//
+// ReadyMembers is the cluster controller's count and can lag: a member that has
+// just started crash-looping may still be counted ready until that controller
+// reconciles. So if THIS member's own status still records it as ready, subtract
+// it — otherwise a second concurrent failure could read a stale-high count, pass
+// the gate, and let one member be deleted when quorum is in fact already gone.
+func (r *EtcdMemberReconciler) clusterHasQuorumWithout(ctx context.Context, member *lll.EtcdMember) bool {
+	cluster, err := r.clusterFor(ctx, member)
+	if err != nil {
+		return false
+	}
+	desired := 0
+	if cluster.Status.Observed != nil {
+		desired = int(cluster.Status.Observed.Replicas)
+	}
+	if desired == 0 && cluster.Spec.Replicas != nil {
+		desired = int(*cluster.Spec.Replicas)
+	}
+	if desired == 0 {
+		return false
+	}
+	readyOthers := int(cluster.Status.ReadyMembers)
+	for _, cond := range member.Status.Conditions {
+		if cond.Type == lll.MemberReady && cond.Status == metav1.ConditionTrue {
+			readyOthers--
+			break
+		}
+	}
+	return readyOthers >= desired/2+1
+}
+
 // ── Status ───────────────────────────────────────────────────────────────
 
 func (r *EtcdMemberReconciler) updateStatus(ctx context.Context, member *lll.EtcdMember) (ctrl.Result, error) {
+	log := log.FromContext(ctx)
 	pod := &corev1.Pod{}
 	if err := r.Get(ctx, types.NamespacedName{Namespace: member.Namespace, Name: member.Name}, pod); err != nil {
 		if errors.IsNotFound(err) {
@@ -950,6 +1032,26 @@ func (r *EtcdMemberReconciler) updateStatus(ctx context.Context, member *lll.Etc
 
 	switch {
 	case !podReady:
+		// Self-heal an unrecoverable member. A non-bootstrap PVC member whose
+		// etcd cannot start — classically because its data dir was lost while
+		// the cluster membership moved on, leaving its frozen --initial-cluster
+		// stale (etcd: "member count is unequal") — crash-loops forever on its
+		// own. Replace it: delete the CR so the finalizer does a clean
+		// MemberRemove and the cluster controller gap-fills a fresh member with
+		// a current --initial-cluster. Gate on the rest of the cluster having
+		// quorum so a cluster-wide outage never cascades into mass deletion
+		// (the finalizer's MemberRemove is quorum-gated too — belt and braces).
+		if !member.Spec.Bootstrap &&
+			member.Spec.Storage.Medium != lll.StorageMediumMemory &&
+			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)
+			if err := r.Delete(ctx, member); err != nil {
+				return ctrl.Result{}, err
+			}
+			return ctrl.Result{}, nil
+		}
 		if setMemberCondition(member, lll.MemberReady, metav1.ConditionFalse, "PodNotReady",
 			fmt.Sprintf("pod phase: %s", pod.Status.Phase)) {
 			changed = true

controllers/etcdmember_controller_test.go

@@ -26,6 +26,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	lll "github.com/cozystack/etcd-operator/api/v1alpha2"
 )
@@ -691,6 +692,198 @@ func TestUpdateStatus_PopulatesMemberIDAndFlipsReady(t *testing.T) {
 	}
 }
 
+// TestEtcdContainerStuck pins the self-heal detection: an etcd container is
+// "stuck" only when it is not ready, has restarted at least the threshold, and
+// was not OOMKilled — and never while the Pod is itself being deleted.
+func TestEtcdContainerStuck(t *testing.T) {
+	mk := func(name string, ready bool, restarts int32, lastReason string) *corev1.Pod {
+		cs := corev1.ContainerStatus{Name: name, Ready: ready, RestartCount: restarts}
+		if lastReason != "" {
+			cs.LastTerminationState.Terminated = &corev1.ContainerStateTerminated{Reason: lastReason, ExitCode: 1}
+		}
+		return &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{cs}}}
+	}
+	// currentlyOOMKilled: the etcd container is over the restart threshold and
+	// sits in State.Terminated=OOMKilled right now (not yet backed off into
+	// Waiting/CrashLoopBackOff), so LastTerminationState is empty.
+	currentlyOOMKilled := &corev1.Pod{Status: corev1.PodStatus{ContainerStatuses: []corev1.ContainerStatus{{
+		Name: "etcd", Ready: false, RestartCount: dataLossRestartThreshold + 4,
+		State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{Reason: "OOMKilled", ExitCode: 137}},
+	}}}}
+	// deletingPod: a stuck-looking container, but the Pod is terminating
+	// (DeletionTimestamp set) — a drain/eviction/restart, not an unrecoverable
+	// member.
+	now := metav1.Now()
+	deletingPod := mk("etcd", false, dataLossRestartThreshold+4, "Error")
+	deletingPod.DeletionTimestamp = &now
+
+	cases := []struct {
+		name string
+		pod  *corev1.Pod
+		want bool
+	}{
+		{"stuck: not ready, at threshold, Error exit", mk("etcd", false, dataLossRestartThreshold, "Error"), true},
+		{"stuck: no last-termination recorded yet", mk("etcd", false, dataLossRestartThreshold+1, ""), true},
+		{"ready", mk("etcd", true, dataLossRestartThreshold+4, "Error"), false},
+		{"below restart threshold", mk("etcd", false, dataLossRestartThreshold-1, "Error"), false},
+		{"OOMKilled (last termination) is excluded", mk("etcd", false, dataLossRestartThreshold+4, "OOMKilled"), false},
+		{"OOMKilled (current state) is excluded", currentlyOOMKilled, false},
+		{"pod being deleted is never stuck", deletingPod, false},
+		{"no etcd container", mk("other", false, dataLossRestartThreshold+4, "Error"), false},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := etcdContainerStuck(tc.pod); got != tc.want {
+				t.Fatalf("etcdContainerStuck = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+// crashLoopPod builds a Pod whose etcd container is persistently crash-looping
+// (not ready, restarted past the threshold with an Error exit) — the data-loss
+// signature.
+func crashLoopPod(name, ns string) *corev1.Pod {
+	return &corev1.Pod{
+		ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: ns},
+		Status: corev1.PodStatus{
+			Phase:      corev1.PodRunning,
+			Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionFalse}},
+			ContainerStatuses: []corev1.ContainerStatus{{
+				Name:         "etcd",
+				Ready:        false,
+				RestartCount: dataLossRestartThreshold + 2,
+				LastTerminationState: corev1.ContainerState{
+					Terminated: &corev1.ContainerStateTerminated{Reason: "Error", ExitCode: 1},
+				},
+			}},
+		},
+	}
+}
+
+// clusterWithReady builds a 3-replica EtcdCluster and persists ready as its
+// status.readyMembers (status is a subresource on the fake client).
+func clusterWithReady(t *testing.T, c client.Client, name, ns string, ready int32) {
+	t.Helper()
+	got := mustGet(t, c, name, ns, &lll.EtcdCluster{})
+	got.Status.ReadyMembers = ready
+	if err := c.Status().Update(context.Background(), got); err != nil {
+		t.Fatalf("seed cluster status: %v", err)
+	}
+}
+
+// TestUpdateStatus_ReplacesStuckMember: a persistently crash-looping
+// non-bootstrap PVC member is deleted for replacement when the rest of the
+// cluster still has quorum.
+func TestUpdateStatus_ReplacesStuckMember(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-1", Namespace: "ns", Labels: memberLabels("test", "test-1")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
+	}
+	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-1", "ns"))
+	clusterWithReady(t, c, "test", "ns", 2) // 2/3 ready → quorum without test-1
+
+	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-1", Namespace: "ns"}, &lll.EtcdMember{})
+	if !apierrors.IsNotFound(err) {
+		t.Fatalf("expected member deleted for replacement; Get err = %v", err)
+	}
+}
+
+// TestUpdateStatus_KeepsStuckMemberWithoutQuorum: the same crash-looping member
+// is NOT deleted when the rest of the cluster lacks quorum — self-heal must
+// never cascade a cluster-wide outage into mass deletion.
+func TestUpdateStatus_KeepsStuckMemberWithoutQuorum(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-1", Namespace: "ns", Labels: memberLabels("test", "test-1")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
+	}
+	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-1", "ns"))
+	clusterWithReady(t, c, "test", "ns", 1) // only 1/3 ready → no quorum
+
+	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-1", Namespace: "ns"}, &lll.EtcdMember{}); err != nil {
+		t.Fatalf("member must NOT be deleted without quorum; Get err = %v", err)
+	}
+}
+
+// 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) {
+	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"))
+	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)
+	}
+
+	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)
+	}
+}
+
+// TestUpdateStatus_KeepsStuckMemberWhenStaleReadyCountIncludesIt: a member that
+// just started crash-looping can still be counted in the cluster controller's
+// lagging ReadyMembers and still record MemberReady=True in its own status. The
+// quorum gate must subtract this member, so a stale-high count can't green-light
+// a deletion that would actually drop the cluster below quorum.
+func TestUpdateStatus_KeepsStuckMemberWhenStaleReadyCountIncludesIt(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-1", Namespace: "ns", Labels: memberLabels("test", "test-1")},
+		Spec:       lll.EtcdMemberSpec{ClusterName: "test", Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, InitialCluster: "x", ClusterToken: "test"},
+	}
+	// Stale: this member is still listed ready in its own status, mirroring a
+	// ReadyMembers count that has not yet been decremented for it.
+	member.Status.Conditions = []metav1.Condition{{
+		Type: lll.MemberReady, Status: metav1.ConditionTrue, Reason: "Ready", LastTransitionTime: metav1.Now(),
+	}}
+	c, _ := newTestClient(t, cluster, member, crashLoopPod("test-1", "ns"))
+	clusterWithReady(t, c, "test", "ns", 2) // stale-high: still counts test-1
+
+	r := &EtcdMemberReconciler{Client: c, Scheme: testScheme(t)}
+	if _, err := r.updateStatus(ctx, member); err != nil {
+		t.Fatalf("updateStatus: %v", err)
+	}
+
+	// 2 ready minus this still-counted member = 1 < quorum(2) → must be kept.
+	if err := c.Get(ctx, types.NamespacedName{Name: "test-1", Namespace: "ns"}, &lll.EtcdMember{}); err != nil {
+		t.Fatalf("member must NOT be deleted while it is still double-counted in ReadyMembers; Get err = %v", err)
+	}
+}
+
 // TestRemoveMemberFromEtcd_LastMemberIsNoOp: if no other members exist (the
 // cluster is being torn down or this is genuinely the last member), the
 // finalizer can't reach a peer to call MemberRemove. Don't block — return