diff --git a/controllers/etcdcluster_controller.go b/controllers/etcdcluster_controller.go index 32ab7632..b810d365 100644 --- a/controllers/etcdcluster_controller.go +++ b/controllers/etcdcluster_controller.go @@ -214,6 +214,42 @@ func (r *EtcdClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) desired := cluster.Status.Observed.Replicas + // ── Total member loss ───────────────────────────────────────────── + // A latched ClusterID with no EtcdMember CRs left at all means the data + // plane is gone — something removed every member, and each member's PVC + // is controller-owned by it, so the data went with them. There is no + // recovery this controller can perform: rebuilding would mean adopting a + // new identity on an empty data dir, and scaleUp cannot help either, + // because MemberAdd needs a live member to dial and there is none. + // + // Without this branch the reconcile falls through to scaleUp and spins on + // "etcdclient: no available endpoints" every 10s forever, while the + // status keeps advertising its last pre-loss values — an empty cluster + // reporting Available=True/QuorumHealthy. Park in a terminal condition + // instead: name what happened, drive Available false, and leave the + // destructive call (restore a snapshot, or delete and recreate) to a + // human, as every other unrecoverable state in this controller does. + // + // Scope: this fires for a converged cluster, whose ProgressDeadline has + // been cleared — the state the endless spin was observed in. A cluster + // that loses every member mid-reconcile still has its deadline armed and + // is handled above by handleDeadlineExceeded, which parks it under the + // generic DeadlineExceeded reason. Both are terminal and visible; only + // the specificity of the reason differs, and reusing the existing arm + // keeps this change from altering deadline behaviour. + // + // Both member sets must be empty. `active` alone is not enough: a member + // being deleted (finalizer still running MemberRemove) is filtered out of + // `active` while its CR is very much still there, so a scale-down of the + // last member — or a self-heal replacement — would otherwise be declared + // a total loss mid-flight. Requiring the raw list to be empty too leaves + // those cases to the in-flight-deletion wait below, which is where they + // belong. + if cluster.Status.ClusterID != "" && desired > 0 && + len(active) == 0 && len(memberList.Items) == 0 { + return r.handleAllMembersLost(ctx, cluster) + } + // ── Bootstrap ────────────────────────────────────────────────────── // Bootstrap creates a single-member etcd cluster (member -0 only) with // --initial-cluster-state=new. Once ClusterID is latched, scale-up adds @@ -1951,6 +1987,58 @@ func (r *EtcdClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { // that happens we sit in DeadlineExceeded. // // We never auto-pivot during bootstrap, and never silently in steady state. +// handleAllMembersLost parks a cluster whose EtcdMember CRs have all been +// removed while its ClusterID is still latched. +// +// This is a diagnosis, not a repair. The controller deliberately performs no +// recovery: every option — re-bootstrapping onto a fresh data dir, or +// re-running the one-time spec.bootstrap.restore — either discards data or +// silently rolls it back to a snapshot, and both would run unattended against +// a cluster whose members may have been removed by mistake. Nothing here +// mutates ClusterID, ClusterToken, AuthEnabled, Pods or PVCs, so a surviving +// PVC (owner-ref GC skipped, e.g. --cascade=orphan) can still be recovered by +// hand, and the evidence of what happened stays intact. +// +// ReadyMembers is zeroed alongside the conditions: it is the scale +// subresource's status path, and leaving the pre-loss count there would keep +// anything reading it — HPA-style tooling, dashboards, GitOps health — seeing +// a healthy cluster. +// +// Idempotent and parking, matching handleDeadlineExceeded: write only when +// something actually changed, then return without a requeue and let the watch +// wake the controller when a human intervenes. +func (r *EtcdClusterReconciler) handleAllMembersLost( + ctx context.Context, + cluster *lll.EtcdCluster, +) (ctrl.Result, error) { + log := log.FromContext(ctx) + + changed := setClusterCondition(cluster, lll.ClusterAvailable, metav1.ConditionFalse, "AllMembersLost", + fmt.Sprintf("all %d members are gone; no EtcdMember remains to serve or to add a member against. "+ + "Recover by restoring from a snapshot, or delete the cluster and recreate it", + cluster.Status.Observed.Replicas)) + changed = setClusterCondition(cluster, lll.ClusterProgressing, metav1.ConditionFalse, "AllMembersLost", + "every member was lost; manual recovery required") || changed + changed = setClusterCondition(cluster, lll.ClusterDegraded, metav1.ConditionTrue, "AllMembersLost", + "no members remain") || changed + if cluster.Status.ReadyMembers != 0 { + cluster.Status.ReadyMembers = 0 + changed = true + } + + if changed { + log.Error(nil, "cluster lost every member; manual recovery required", + "clusterID", cluster.Status.ClusterID, "desired", cluster.Status.Observed.Replicas) + if err := r.Status().Update(ctx, cluster); err != nil { + if errors.IsConflict(err) { + return ctrl.Result{Requeue: true}, nil + } + return ctrl.Result{}, err + } + } + return ctrl.Result{}, nil +} + func (r *EtcdClusterReconciler) handleDeadlineExceeded( ctx context.Context, cluster *lll.EtcdCluster, diff --git a/controllers/etcdcluster_controller_test.go b/controllers/etcdcluster_controller_test.go index dcf8bc38..91b652d4 100644 --- a/controllers/etcdcluster_controller_test.go +++ b/controllers/etcdcluster_controller_test.go @@ -12,6 +12,7 @@ package controllers import ( "context" + "crypto/tls" "errors" "fmt" "reflect" @@ -36,6 +37,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" lll "github.com/cozystack/etcd-operator/api/v1alpha2" ) @@ -961,6 +963,278 @@ func TestScaleUp_WaitsForInFlightDeletion(t *testing.T) { } } +// findCond returns the named condition, or nil when it is absent. +func findCond(conds []metav1.Condition, condType string) *metav1.Condition { + for i := range conds { + if conds[i].Type == condType { + return &conds[i] + } + } + return nil +} + +// factoryFailingOnEmptyEndpoints mirrors production: the real factory cannot +// build a client without at least one endpoint to dial. Tests that must not +// reach etcd use this so the assertion means "never dialed", not "dialed a +// fake that happens to answer". +func factoryFailingOnEmptyEndpoints(c EtcdClusterClient) EtcdClientFactory { + return func(_ context.Context, endpoints []string, _ *tls.Config, _, _ string) (EtcdClusterClient, error) { + if len(endpoints) == 0 { + return nil, errors.New("etcdclient: no available endpoints") + } + return c, nil + } +} + +// allMembersLostCluster builds a cluster that has latched a ClusterID and then +// lost every EtcdMember — the state this suite exercises. +func allMembersLostCluster(t *testing.T, deadline *metav1.Time) *lll.EtcdCluster { + t.Helper() + return &lll.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "test", Namespace: "ns"}, + Spec: lll.EtcdClusterSpec{ + Replicas: ptrInt32(3), + Version: "3.5.17", + Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + Status: lll.EtcdClusterStatus{ + ClusterToken: "ns-test-x", + ClusterID: "deadbeef", + AuthEnabled: true, + ReadyMembers: 3, + Observed: &lll.ObservedClusterSpec{ + Replicas: 3, Version: "3.5.17", Storage: lll.StorageSpec{Size: quickQty(t, "1Gi")}, + }, + ProgressDeadline: deadline, + Conditions: []metav1.Condition{ + {Type: lll.ClusterAvailable, Status: metav1.ConditionTrue, Reason: "QuorumHealthy", + LastTransitionTime: metav1.Now()}, + }, + }, + } +} + +// TestAllMembersLost_ParksTerminalWithoutDialing covers a cluster whose +// EtcdMember CRs have all been removed while status.clusterID is still +// latched. +// +// Before this branch existed the reconcile fell through to scaleUp, whose +// endpoints derive from the (empty) member set, so the client factory failed +// with "no available endpoints" and the reconcile requeued every 10s forever +// — while Available stayed at its last pre-loss value, advertising a healthy +// quorum for a cluster with nothing running. +// +// The controller cannot repair this state (rebuilding means a new identity on +// an empty data dir), so it parks in a terminal condition and leaves the +// destructive call to a human. Asserted here: Available goes false with a +// reason naming the event, ReadyMembers is zeroed, etcd is never dialed, and +// — critically — nothing is mutated or created. +func TestAllMembersLost_ParksTerminalWithoutDialing(t *testing.T) { + ctx := context.Background() + // ProgressDeadline nil: a converged cluster clears it, which is the + // state this branch is normally reached in. + cluster := allMembersLostCluster(t, nil) + c, _ := newTestClient(t, cluster) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryFailingOnEmptyEndpoints(fe)} + + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if res.RequeueAfter != 0 || res.Requeue { + t.Fatalf("terminal state must park, not requeue; got %+v", res) + } + + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err != nil { + t.Fatalf("Get: %v", err) + } + avail := findCond(got.Status.Conditions, lll.ClusterAvailable) + if avail == nil || avail.Status != metav1.ConditionFalse || avail.Reason != "AllMembersLost" { + t.Fatalf("Available must go False/AllMembersLost; got %+v", avail) + } + if got.Status.ReadyMembers != 0 { + t.Fatalf("ReadyMembers must be zeroed; got %d", got.Status.ReadyMembers) + } + // Nothing destructive, and nothing that changes the cluster's identity: + // a surviving PVC must stay recoverable by hand, and the evidence of + // what happened must stay intact. + if got.Status.ClusterID != "deadbeef" { + t.Fatalf("ClusterID must not be discarded; got %q", got.Status.ClusterID) + } + if got.Status.ClusterToken != "ns-test-x" { + t.Fatalf("ClusterToken must not be rotated; got %q", got.Status.ClusterToken) + } + if !got.Status.AuthEnabled { + t.Fatalf("AuthEnabled must not be cleared") + } + members := &lll.EtcdMemberList{} + if err := c.List(ctx, members, client.InNamespace("ns")); err != nil { + t.Fatalf("List: %v", err) + } + if len(members.Items) != 0 { + t.Fatalf("no member may be created on this path; got %d", len(members.Items)) + } + if len(fe.addCalls) > 0 { + t.Fatalf("etcd must never be dialed with zero members; got %v", fe.addCalls) + } +} + +// TestAllMembersLost_ArmedDeadlineStillParks pins the scope boundary. A +// cluster that loses every member *before* converging still has its +// ProgressDeadline armed and is claimed by handleDeadlineExceeded, which parks +// it under the generic DeadlineExceeded reason rather than AllMembersLost. +// +// That is deliberate — routing it here instead would change deadline +// behaviour, which this fix has no reason to touch. What matters, and what +// this asserts, is the property the bug was about: whichever arm claims the +// cluster, the reconcile parks instead of spinning on scaleUp forever, and +// Available does not survive as True. +func TestAllMembersLost_ArmedDeadlineStillParks(t *testing.T) { + ctx := context.Background() + expired := metav1.NewTime(metav1.Now().Add(-time.Hour)) + cluster := allMembersLostCluster(t, &expired) + c, _ := newTestClient(t, cluster) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryFailingOnEmptyEndpoints(fe)} + + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if res.RequeueAfter != 0 || res.Requeue { + t.Fatalf("must park rather than spin; got %+v", res) + } + + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err != nil { + t.Fatalf("Get: %v", err) + } + avail := findCond(got.Status.Conditions, lll.ClusterAvailable) + if avail == nil || avail.Status != metav1.ConditionFalse { + t.Fatalf("Available must not survive as True through total member loss; got %+v", avail) + } + if len(fe.addCalls) > 0 { + t.Fatalf("etcd must never be dialed with zero members; got %v", fe.addCalls) + } +} + +// TestAllMembersLost_NotTriggeredByDormantMember: a paused-then-resumed +// cluster has current==0 with a dormant CR parked alongside. That member still +// owns its PVC and its data — waking it is the correct recovery, and reporting +// the cluster as unrecoverable would be wrong. +func TestAllMembersLost_NotTriggeredByDormantMember(t *testing.T) { + ctx := context.Background() + cluster := allMembersLostCluster(t, nil) + dormant := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-dormant", Namespace: "ns", Labels: memberLabels("test", "test-dormant"), + }, + Spec: lll.EtcdMemberSpec{ClusterName: "test", InitialCluster: "x", Bootstrap: true, Dormant: true}, + } + if err := controllerutil.SetControllerReference(cluster, dormant, testScheme(t)); err != nil { + t.Fatalf("SetControllerReference: %v", err) + } + c, _ := newTestClient(t, cluster, dormant) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryFailingOnEmptyEndpoints(fe)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } + + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err != nil { + t.Fatalf("Get: %v", err) + } + if avail := findCond(got.Status.Conditions, lll.ClusterAvailable); avail != nil && avail.Reason == "AllMembersLost" { + t.Fatalf("a dormant member is recoverable; must not report AllMembersLost") + } + gotMember := &lll.EtcdMember{} + if err := c.Get(ctx, types.NamespacedName{Name: "test-dormant", Namespace: "ns"}, gotMember); err != nil { + t.Fatalf("Get member: %v", err) + } + if gotMember.Spec.Dormant { + t.Fatalf("dormant member should have been woken instead") + } +} + +// TestAllMembersLost_NotTriggeredWhileAMemberIsTerminating: the last member is +// being deleted and its finalizer is still running MemberRemove. It is +// filtered out of the active set, but its CR is still there and the deletion +// is expected to complete — scaling down to zero, or replacing a member, must +// not be mistaken for a total loss. The in-flight-deletion wait owns this +// state; parking here would strand a cluster mid-operation. +func TestAllMembersLost_NotTriggeredWhileAMemberIsTerminating(t *testing.T) { + ctx := context.Background() + now := metav1.Now() + cluster := allMembersLostCluster(t, nil) + terminating := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-going", Namespace: "ns", Labels: memberLabels("test", "test-going"), + DeletionTimestamp: &now, Finalizers: []string{MemberFinalizer}, + }, + Spec: lll.EtcdMemberSpec{ClusterName: "test", InitialCluster: "x"}, + } + if err := controllerutil.SetControllerReference(cluster, terminating, testScheme(t)); err != nil { + t.Fatalf("SetControllerReference: %v", err) + } + c, _ := newTestClient(t, cluster, terminating) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryFailingOnEmptyEndpoints(fe)} + + res, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}) + if err != nil { + t.Fatalf("Reconcile: %v", err) + } + if res.RequeueAfter == 0 { + t.Fatalf("expected the in-flight-deletion wait to requeue; got %+v", res) + } + + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err != nil { + t.Fatalf("Get: %v", err) + } + if avail := findCond(got.Status.Conditions, lll.ClusterAvailable); avail != nil && avail.Reason == "AllMembersLost" { + t.Fatalf("a member still terminating is not a total loss; must not park") + } +} + +// TestAllMembersLost_NotTriggeredWhileAMemberSurvives: one member left, not +// Ready (e.g. crash-looping). The data plane is degraded, not gone — the +// existing wait-for-Ready path owns this, and a terminal condition here would +// park a cluster that can still recover on its own. +func TestAllMembersLost_NotTriggeredWhileAMemberSurvives(t *testing.T) { + ctx := context.Background() + cluster := allMembersLostCluster(t, nil) + survivor := &lll.EtcdMember{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-alive", Namespace: "ns", Labels: memberLabels("test", "test-alive"), + }, + Spec: lll.EtcdMemberSpec{ClusterName: "test", InitialCluster: "x"}, + Status: lll.EtcdMemberStatus{PodName: "test-alive", MemberID: "abc"}, + } + if err := controllerutil.SetControllerReference(cluster, survivor, testScheme(t)); err != nil { + t.Fatalf("SetControllerReference: %v", err) + } + c, _ := newTestClient(t, cluster, survivor) + fe := newFakeEtcd(0xdeadbeef) + r := &EtcdClusterReconciler{Client: c, Scheme: testScheme(t), EtcdClientFactory: factoryFailingOnEmptyEndpoints(fe)} + + if _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "test", Namespace: "ns"}}); err != nil { + t.Fatalf("Reconcile: %v", err) + } + + got := &lll.EtcdCluster{} + if err := c.Get(ctx, types.NamespacedName{Name: "test", Namespace: "ns"}, got); err != nil { + t.Fatalf("Get: %v", err) + } + if avail := findCond(got.Status.Conditions, lll.ClusterAvailable); avail != nil && avail.Reason == "AllMembersLost" { + t.Fatalf("a surviving member means the cluster is degraded, not lost") + } +} + // TestScaleDown_PicksMostRecentlyCreatedVictim: with apiserver-assigned // names there is no ordinal to scale down by; victim selection sorts by // CreationTimestamp (newest first). This naturally retires the most diff --git a/docs/concepts.md b/docs/concepts.md index 6cd535ba..bcfb28bf 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -417,6 +417,7 @@ The cluster surfaces three conditions: `Available`, `Progressing`, `Degraded`. T | `False` | `ClusterUnreachable` | Discovery couldn't dial etcd (DNS failure, network partition, etcd not yet listening). | | `False` | `BootstrapFailed` | Deadline expired before `clusterID` was latched. Terminal — recovery is delete and recreate. | | `False` | `DeadlineExceeded` | Deadline expired after bootstrap. Terminal — recovery is to edit spec. | +| `False` | `AllMembersLost` | Every `EtcdMember` is gone while `clusterID` is still latched. Terminal — the operator performs no automatic recovery; restore from a snapshot, or delete and recreate. | `Progressing` distinguishes "actively reconciling" from "we hit a wall": @@ -428,9 +429,9 @@ The cluster surfaces three conditions: `Available`, `Progressing`, `Degraded`. T | `True` | `RetryAfterDeadline` | Deadline-exceeded recovery: user edited spec after a steady-state deadline. | | `False` | `Reconciled` | At steady state with the current `observed`. | | `False` | `Paused` | Same as Available; emitted when `desired==0`. | -| `False` | `BootstrapFailed` / `DeadlineExceeded` | Terminal states; see Available. | +| `False` | `BootstrapFailed` / `DeadlineExceeded` / `AllMembersLost` | Terminal states; see Available. | -`Degraded` is `True` whenever `Available=True/QuorumAvailable` (partial outage) or `Available=False/QuorumLost`. `False` in healthy or paused states. In other words, `Degraded` means "the cluster is not delivering its full intended capacity right now"; reading `Degraded` alone tells an alerting layer whether to page someone. +`Degraded` is `True` whenever `Available=True/QuorumAvailable` (partial outage), `Available=False/QuorumLost`, or `Available=False/AllMembersLost` (every member gone — terminal, see above). `False` in healthy or paused states. In other words, `Degraded` means "the cluster is not delivering its full intended capacity right now"; reading `Degraded` alone tells an alerting layer whether to page someone. All conditions carry `observedGeneration` so consumers can tell whether a condition reflects the latest spec. Status writes are gated on "did anything actually change" — the operator does not bump `resourceVersion` every 30 s just because of the periodic reconcile. diff --git a/docs/operations.md b/docs/operations.md index 48649db8..a9409432 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -179,6 +179,27 @@ kubectl edit etcdcluster.etcd-operator.cozystack.io -n The next reconcile notices `spec != observed`, treats your edit as the intervention signal, snapshots the new spec into `observed`, sets a fresh deadline, and resumes. +### `Available=False/AllMembersLost` + +Terminal. Every `EtcdMember` of the cluster is gone while `status.clusterID` is still latched — the data plane no longer exists. Each member's PVC is controller-owned by its `EtcdMember`, so unless owner-ref GC was skipped (`--cascade=orphan`, a CRD replacement, restored-from-backup CRs) the data went with them. + +The operator deliberately performs **no** automatic recovery here, and nothing on this path mutates `clusterID`, `clusterToken`, `authEnabled`, Pods or PVCs. Rebuilding would mean bringing the cluster back under a new identity on an empty data dir — or, for a cluster created via `spec.bootstrap.restore`, silently re-running that one-time restore and rolling the data back to the snapshot it was created from. Both are decisions for a human, and doing either automatically would also destroy the evidence of what removed the members. + +First find out what happened — several clusters entering this state at once points at a systemic cause (a GC misfire, a GitOps prune, a CRD replacement, a cleanup script) that will repeat: + +```sh +kubectl get etcdmember.etcd-operator.cozystack.io -n # expected: none +kubectl get pvc -n -l etcd-operator.cozystack.io/cluster= +kubectl get pods -n -l etcd-operator.cozystack.io/cluster= +kubectl get events -n --sort-by=.lastTimestamp | tail -50 +``` + +Then pick a recovery: + +- **PVCs (or Pods) survived** — the data is still there. Do not delete the cluster: work from the surviving volumes, and stop the old Pods before recreating anything so two etcd clusters never serve the same Service. +- **A snapshot exists** — restore it into a freshly created cluster (see [Restoring a cluster from a snapshot](#restoring-a-cluster-from-a-snapshot)). `spec.bootstrap` is immutable post-create, so this means creating a new `EtcdCluster`, not editing this one. +- **Neither** — the data is gone. Delete the `EtcdCluster` and recreate it to get an empty cluster back. + ### `Progressing=True/WaitingForSeed` The seed `EtcdMember` CR exists but the member controller hasn't yet created its Pod — this is the gap between the cluster controller creating the CR and the member controller's next reconcile pass. `kubectl describe pod` is **not** useful here: there is no Pod yet, so it returns "not found" and obscures the actual state. Inspect the CR and the namespace's events instead: