diff --git a/controllers/etcdmember_controller.go b/controllers/etcdmember_controller.go index ffb2567e..6129bb85 100644 --- a/controllers/etcdmember_controller.go +++ b/controllers/etcdmember_controller.go @@ -909,7 +909,7 @@ func restoreInitContainer(member *lll.EtcdMember, peerAddr, operatorImage string } // dataLossRestartThreshold is how many times the etcd container must have -// restarted before we treat a non-bootstrap PVC member as unrecoverable and +// restarted before we treat a non-bootstrap 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. @@ -1046,17 +1046,14 @@ 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). + // Self-heal: etcd that can never start (classically a stale frozen + // --initial-cluster: "member count is unequal") crash-loops forever. + // Delete the CR — the finalizer MemberRemoves it and the cluster + // controller gap-fills a replacement. Quorum-gated so a cluster-wide + // 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 && - 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", diff --git a/controllers/etcdmember_controller_test.go b/controllers/etcdmember_controller_test.go index 56096f8a..04783b94 100644 --- a/controllers/etcdmember_controller_test.go +++ b/controllers/etcdmember_controller_test.go @@ -974,6 +974,33 @@ func TestUpdateStatus_ReplacesStuckMember(t *testing.T) { } } +// TestUpdateStatus_ReplacesStuckMemoryMember: wedged-learner regression — a +// crash-looping memory member keeps its Pod (and UID) alive, so only the +// crashloop self-heal can replace it. +func TestUpdateStatus_ReplacesStuckMemoryMember(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"), Medium: lll.StorageMediumMemory}, 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 stuck memory 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. diff --git a/docs/concepts.md b/docs/concepts.md index 30fcad82..8eade560 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -127,8 +127,8 @@ Each member's data dir is configured via `spec.storage`, a struct with `size`, ` | `spec.storage.medium` | Backend | Lifetime | Pod loss → | |---|---|---|---| -| `""` (default) | PVC; `spec.storage.storageClassName` if set, else the namespace default; `ReadWriteOnce` | Survives Pod restart, eviction, node failure (re-attached to new Pod). | Same Pod / new Pod re-uses existing data dir; etcd rejoins with the same member ID and `ClusterID`. **Exception:** if the data dir is lost or corrupt so etcd cannot boot and the member crash-loops past the threshold, the operator deletes and replaces it with a *fresh* member ID (quorum-gated) — see [Crash-loop self-heal](#crash-loop-self-heal-pvc-members) below. | -| `"Memory"` | `emptyDir{medium: Memory}` with `sizeLimit: spec.storage.size` | Bound to the Pod. Container restart preserves tmpfs; Pod deletion / eviction / node failure destroys it. | Operator detects Pod loss via recorded `Status.PodUID`, self-deletes the `EtcdMember`, finalizer calls `MemberRemove`, scale-up gap-fill creates a replacement with a fresh member ID. | +| `""` (default) | PVC; `spec.storage.storageClassName` if set, else the namespace default; `ReadWriteOnce` | Survives Pod restart, eviction, node failure (re-attached to new Pod). | Same Pod / new Pod re-uses existing data dir; etcd rejoins with the same member ID and `ClusterID`. **Exception:** if the data dir is lost or corrupt so etcd cannot boot and the member crash-loops past the threshold, the operator deletes and replaces it with a *fresh* member ID (quorum-gated) — see [Crash-loop self-heal](#crash-loop-self-heal) below. | +| `"Memory"` | `emptyDir{medium: Memory}` with `sizeLimit: spec.storage.size` | Bound to the Pod. Container restart preserves tmpfs; Pod deletion / eviction / node failure destroys it. | Operator detects Pod loss via recorded `Status.PodUID`, self-deletes the `EtcdMember`, finalizer calls `MemberRemove`, scale-up gap-fill creates a replacement with a fresh member ID. A member whose Pod is *alive* but whose etcd can never start (e.g. a learner baked with a stale `--initial-cluster`) is instead caught by [Crash-loop self-heal](#crash-loop-self-heal). | `spec.storage.storageClassName` mirrors the corev1 PVC field of the same name: **nil** uses the namespace's default `StorageClass`, the **empty string** explicitly disables dynamic provisioning (a pre-provisioned PV must already match), any other value names a specific `StorageClass`. It's immutable post-create — `PersistentVolumeClaim.spec.storageClassName` is itself immutable, so there is no in-place change a controller could honour without rolling every PVC. Ignored when `medium=Memory` (no PVC is created). @@ -153,17 +153,17 @@ The member controller self-deletes the `EtcdMember`. The existing finalizer runs If quorum is already lost across multiple simultaneous failures, `MemberRemove` will fail and the dying members stay in `Terminating` until quorum returns. That is the correct outcome: the cluster is dead and the user has to recreate it. The operator does not try to be clever about restoring a quorum from inconsistent half-states. -`Status.BrokenMembers` stays at 0 in normal operation, including across a memory pod-loss + auto-replacement cycle. The `isBroken` predicate is implemented for memory members (lost-Pod state), but the member controller intercepts the loss and self-deletes the member in the same reconcile pass — by the time the cluster controller computes the count, the lost member is already `Terminating` and excluded from the running set. The field exists as a future hook for broken-member detection policies that don't immediately tear the member down. PVC crash-loop self-heal (next section) does **not** flow through `isBroken`/`BrokenMembers` — it triggers off the Pod's container restart count directly, so `BrokenMembers` stays 0 across that cycle too. +`Status.BrokenMembers` stays at 0 in normal operation, including across a memory pod-loss + auto-replacement cycle. The `isBroken` predicate is implemented for memory members (lost-Pod state), but the member controller intercepts the loss and self-deletes the member in the same reconcile pass — by the time the cluster controller computes the count, the lost member is already `Terminating` and excluded from the running set. The field exists as a future hook for broken-member detection policies that don't immediately tear the member down. Crash-loop self-heal (next section) does **not** flow through `isBroken`/`BrokenMembers` — it triggers off the Pod's container restart count directly, so `BrokenMembers` stays 0 across that cycle too. -### Crash-loop self-heal (PVC members) +### Crash-loop self-heal -A PVC-backed member normally survives Pod loss: the PVC re-attaches and etcd rejoins with the same member ID (see the [storage table](#storage)). But that assumes the data dir is intact. If the data dir is lost or corrupt — classically a volume lost on node failure while the cluster membership moved on, leaving the member's *frozen* `--initial-cluster` stale — etcd refuses to boot (`error validating peerURLs ... member count is unequal`) and the Pod crash-loops forever. "Pod lost ≠ data lost" for a PVC, so the memory-style `Status.PodUID` loss check never fires here. +A member can end up with a Pod that is alive but whose etcd can never start. For a PVC member the classic cause is a data dir lost or corrupt (a volume lost on node failure) while the cluster membership moved on, leaving the member's *frozen* `--initial-cluster` stale — etcd refuses to boot (`error validating peerURLs ... member count is unequal`) and the Pod crash-loops forever. For a memory member the same wedge hits a *replacement learner*: its `--initial-cluster` is baked into the immutable Pod spec at creation, so if membership changes again before its first successful boot, every restart re-fails the same validation. In both cases the Pod itself never dies, so the memory-style `Status.PodUID` loss check never fires — this restart-count trigger is the only recovery path. 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. -- **Replacement.** Deleting the `EtcdMember` runs the finalizer's clean `MemberRemove`, the member-owned PVC is GC'd (discarding the corrupt data dir), 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. +- **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. ### What is missing from memory clusters today diff --git a/docs/operations.md b/docs/operations.md index 90e6d08b..192bb9ed 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -480,15 +480,15 @@ kubectl get etcdmember.etcd-operator.cozystack.io -n default -w # appear, and READY=3 restore within a minute or so. ``` -### PVC member crash-loop replacement +### Member crash-loop replacement -The `Status.PodUID` mechanism above keys on the Pod disappearing — the right signal for a memory-backed member, where Pod loss *is* data loss. A PVC-backed member is different: the PVC survives Pod restarts, so a lost Pod re-attaches the same data dir and etcd rejoins with the same member ID. There is a separate, second trigger for the case where the data dir itself is gone or corrupt. +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 PVC member's etcd cannot start — classically because its data dir was lost (e.g. a volume lost on node failure) while the cluster membership moved on, leaving its frozen `--initial-cluster` stale (`error validating peerURLs ... member count is unequal`) — it crash-loops forever with no recovery path of its own. The operator detects this and replaces it: +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: - **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. -- **Replacement**: the `EtcdMember` CR is deleted → finalizer `MemberRemove` → the member-owned `data-` PVC is GC'd (discarding the corrupt data dir) → the cluster controller gap-fills a fresh `GenerateName` member with a current `--initial-cluster` and a **new** etcd member ID. +- **Replacement**: the `EtcdMember` CR is deleted → finalizer `MemberRemove` → any member-owned `data-` 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.