fix(controllers): report total member loss instead of spinning

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,

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