fix: stop rebuilding cells against a fault that is not in the cell

api/v1alpha1/conditions.go

@@ -85,6 +85,7 @@ const (
 	ReasonNodeNameCollision          = "NodeNameCollision"
 	ReasonBootstrapCredentialSuspect = "BootstrapCredentialSuspect"
 	ReasonCellReplacementExhausted   = "CellReplacementExhausted"
+	ReasonFaultNotInTheCell          = "FaultNotInTheCell"
 	ReasonWaitingForAllocations      = "WaitingForAllocations"
 	ReasonDrainTimedOut              = "DrainTimedOut"
 

charts/gpucellpool/templates/monitoring/prometheusrule.yaml

@@ -60,7 +60,11 @@ spec:
             description: >-
               A failed cell is replaced behind a backoff, so this firing repeatedly
               means replacement is not fixing it — usually a bad image, an expired
-              join credential, or a GPU that never surfaces in the guest.
+              join credential, or a GPU that never surfaces in the guest. If it stays
+              Failed with no replacement attempts at all, check Progressing: on
+              FaultNotInTheCell the pool has deliberately stopped rebuilding because
+              nothing advertises a GPU pool-wide, and the fault is in the workload
+              cluster.
             runbook_url: https://github.com/kubeswift-io/gpucellpool/blob/main/docs/runbook.md
 
         - alert: GPUCellPoolCellFlapping

docs/observability.md

@@ -85,7 +85,7 @@ conditions are states the operator reports deliberately rather than faults.
 |---|---|---|
 | `GPUCellPoolWorkloadClusterUnreachable` | unreachable for 10m — every destructive path is frozen meanwhile | transport |
 | `GPUCellPoolCellsNotReady` | short of Ready cells for 30m (a cell takes ~5 min, so this is stuck, not starting) | cells |
-| `GPUCellPoolCellFailed` | a Failed cell persists for 15m — replacement is not fixing it | cells |
+| `GPUCellPoolCellFailed` | a Failed cell persists for 15m — replacement is not fixing it, or the pool has stopped replacing (`Progressing=FaultNotInTheCell`) because the fault is pool-wide | cells |
 | `GPUCellPoolCellFlapping` | sustained phase churn | cells |
 | `GPUCellPoolNoFreePhysicalGPU` | the pool wants a cell **and** no device is free | outer |
 | `GPUCellPoolSharedGPUExhausted` | healthy cells whose GPU memory is entirely allocated | inner |

docs/runbook.md

@@ -107,6 +107,19 @@ swiftctl ssh <cell> -n <ns> -- 'nvidia-smi -L; cat /run/gpu-cell-preflight'
 Do **not** expect the pool to call the cell Ready meanwhile. A running VM whose GPU
 never surfaced is exactly the failure this design refuses to paper over.
 
+After `spec.capacity.readyTimeout` the cell goes `Failed`. What happens next depends
+on whether the fault is the cell's:
+
+- **One cell advertises nothing, others advertise fine** → the cell is replaced,
+  with backoff, up to five attempts per index (`CellReplacementExhausted`).
+- **No cell node advertises anything** → the pool **stops** and reports
+  `Progressing=False/FaultNotInTheCell`. Causes 2–4 above are workload-cluster
+  faults, so a rebuilt VM would fail identically — rebuilding would just cost a GPU
+  allocation, a root-disk clone, a boot and a join per attempt, and destroy the
+  evidence. The failed cell's VM is left up for you to inspect. Fix the provider
+  (`CapacityProviderReady` names the cause) and the cell is replaced on the next
+  pass.
+
 ### `allocatable nvidia.com/gpu` says 10 and I have one GPU
 
 That is HAMi's `deviceSplitCount` inflation, not a bug and not a device count. The

internal/controller/controller.go

@@ -234,6 +234,7 @@ func (r *GPUCellPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 		WorkloadReachable: reachable,
 		EverReady:         everReady(&pool, cells),
 		DrainPreference:   scale.DrainCandidates,
+		ProviderUsable:    health.Ready,
 	})
 
 	for _, idx := range plan.Create {
@@ -260,7 +261,7 @@ func (r *GPUCellPoolReconciler) Reconcile(ctx context.Context, req ctrl.Request)
 	// Replace failed cells whose backoff has expired: delete the guest and let
 	// the next pass recreate the index.
 	for i := range cells {
-		if ShouldReplace(cells[i], r.now()) {
+		if ShouldReplace(cells[i], r.now(), health.Ready) {
 			if err := r.deleteCell(ctx, &pool, prov, cells[i], false); err != nil {
 				return ctrl.Result{}, err
 			}

internal/controller/membership.go

@@ -64,6 +64,12 @@ type MembershipInput struct {
 	// burst of failures" from "this pool has never worked".
 	EverReady bool
 
+	// ProviderUsable is the pool-wide capacity verdict: false means no cell node
+	// anywhere advertises a GPU, which makes a cell rebuild pointless. It is also
+	// false while the workload cluster is unreachable — where not acting is the
+	// rule, not an accident.
+	ProviderUsable bool
+
 	// DrainPreference is an ordered list of cell names to remove first when
 	// shrinking. The autoscaler sets it to IDLE cells only; empty falls back to
 	// highest-index-first, which is right for an operator-driven scale-down where
@@ -181,6 +187,15 @@ func PlanMembership(in MembershipInput) MembershipPlan {
 	// Replacement: a failed index is retired from the live set, so the create
 	// path below refills it — but only once its backoff has expired.
 	for _, c := range failed {
+		if RebuildWouldNotHelp(c, in.ProviderUsable) {
+			plan.Stalled = true
+			plan.Reason = cellsv1alpha1.ReasonFaultNotInTheCell
+			plan.Message = "not replacing cell " + c.Name +
+				": its Node joined and advertises no GPU, and no cell node advertises one — " +
+				"the fault is in the workload cluster, so a rebuilt VM would fail the same way. " +
+				"Fix the capacity provider (see CapacityProviderReady) and the cell is replaced then"
+			return plan
+		}
 		if c.FailureCount >= MaxFailuresPerIndex {
 			plan.Stalled = true
 			plan.Reason = cellsv1alpha1.ReasonCellReplacementExhausted
@@ -255,19 +270,45 @@ func PlanMembership(in MembershipInput) MembershipPlan {
 
 // ShouldReplace reports whether a failed cell's backoff has expired, so the
 // reconciler can delete it and let the next pass recreate the index.
-func ShouldReplace(c cellsv1alpha1.CellStatus, now time.Time) bool {
+//
+// providerUsable is the pool-wide capacity verdict: false means NO cell node
+// anywhere advertises a GPU. See RebuildWouldNotHelp for why that vetoes a
+// replacement.
+func ShouldReplace(c cellsv1alpha1.CellStatus, now time.Time, providerUsable bool) bool {
 	if c.Phase != cellsv1alpha1.CellPhaseFailed {
 		return false
 	}
 	if c.FailureCount >= MaxFailuresPerIndex {
 		return false
 	}
+	if RebuildWouldNotHelp(c, providerUsable) {
+		return false
+	}
 	if c.LastTransitionTime == nil {
 		return true
 	}
 	return now.Sub(c.LastTransitionTime.Time) >= Backoff(c.FailureCount)
 }
 
+// RebuildWouldNotHelp reports whether replacing a failed cell cannot possibly fix
+// it, because the fault is not in the cell.
+//
+// The shape: the cell's Node joined and went Ready, it advertises no GPU, and no
+// cell node ANYWHERE advertises one either. That is a workload-cluster fault — HAMi
+// missing, its DaemonSet not tolerating the pool's taints, a broken inner CNI — and
+// a fresh VM will reach exactly the same place. Rebuilding costs a GPU allocation, a
+// full root-disk clone, a boot and a join per attempt, up to MaxFailuresPerIndex per
+// index, and destroys the evidence each time. Observed on hardware: HAMi could not
+// register, and the pool's answer was to rebuild the VM.
+//
+// A single broken cell in an otherwise healthy pool is NOT this: the provider
+// reports usable as soon as any node advertises a device, so that cell is still
+// replaced. And with no cells at all the provider is usable by definition, so a pool
+// can never wedge itself out of ever creating one.
+func RebuildWouldNotHelp(c cellsv1alpha1.CellStatus, providerUsable bool) bool {
+	return !providerUsable && c.NodeReady && c.CapacityDevices == 0
+}
+
 func minDuration(a, b time.Duration) time.Duration {
 	if a == 0 || (b > 0 && b < a) {
 		return b

internal/controller/membership_test.go

@@ -185,22 +185,62 @@ func TestBackoffGrowsAndCaps(t *testing.T) {
 func TestShouldReplaceHonoursBackoff(t *testing.T) {
 	failedAt := metav1.Time{Time: now.Add(-10 * time.Second)}
 	c := cellsv1alpha1.CellStatus{Phase: cellsv1alpha1.CellPhaseFailed, FailureCount: 1, LastTransitionTime: &failedAt}
-	if ShouldReplace(c, now) {
+	if ShouldReplace(c, now, true) {
 		t.Error("replaced before the 30s backoff expired")
 	}
 	c.LastTransitionTime = &metav1.Time{Time: now.Add(-31 * time.Second)}
-	if !ShouldReplace(c, now) {
+	if !ShouldReplace(c, now, true) {
 		t.Error("did not replace after the backoff expired")
 	}
 	c.FailureCount = MaxFailuresPerIndex
-	if ShouldReplace(c, now) {
+	if ShouldReplace(c, now, true) {
 		t.Error("replaced an exhausted index")
 	}
-	if ShouldReplace(cellsv1alpha1.CellStatus{Phase: cellsv1alpha1.CellPhaseReady}, now) {
+	if ShouldReplace(cellsv1alpha1.CellStatus{Phase: cellsv1alpha1.CellPhaseReady}, now, true) {
 		t.Error("replaced a healthy cell")
 	}
 }
 
+// TestShouldNotRebuildAgainstAPoolWideFault: a cell whose Node joined and
+// advertises nothing, in a pool where nothing advertises anything, is not a broken
+// cell — it is a broken workload cluster. Rebuilding it costs a GPU allocation, a
+// full root-disk clone, a boot and a join per attempt and cannot succeed. Measured
+// on hardware: HAMi could not register and the pool's answer was to rebuild the VM.
+func TestShouldNotRebuildAgainstAPoolWideFault(t *testing.T) {
+	failedAt := metav1.Time{Time: now.Add(-10 * time.Minute)}
+	joined := cellsv1alpha1.CellStatus{
+		Name: "cells-0", Phase: cellsv1alpha1.CellPhaseFailed, FailureCount: 1,
+		LastTransitionTime: &failedAt, NodeReady: true, CapacityDevices: 0,
+	}
+
+	if ShouldReplace(joined, now, false) {
+		t.Error("rebuilt a cell against a pool-wide provider fault")
+	}
+	if !ShouldReplace(joined, now, true) {
+		t.Error("a single cell advertising nothing in an otherwise healthy pool must still be replaced")
+	}
+
+	// A cell that never joined tells us nothing about the provider, so the veto
+	// must not catch it — that failure really may be the cell's.
+	neverJoined := joined
+	neverJoined.NodeReady = false
+	if !ShouldReplace(neverJoined, now, false) {
+		t.Error("a cell that never joined was not replaced; its fault is not the provider's")
+	}
+
+	// And the stall is reported, not silent.
+	plan := PlanMembership(MembershipInput{
+		Desired: 1, Cells: []cellsv1alpha1.CellStatus{joined}, Now: now,
+		WorkloadReachable: true, EverReady: true, ProviderUsable: false,
+	})
+	if !plan.Stalled || plan.Reason != cellsv1alpha1.ReasonFaultNotInTheCell {
+		t.Errorf("plan = %+v, want a stall naming the pool-wide fault", plan)
+	}
+	if len(plan.Create) != 0 || len(plan.Drain) != 0 {
+		t.Errorf("plan acted on a fault it cannot fix: %+v", plan)
+	}
+}
+
 func TestPlanRequeuesWhenABackoffIsPending(t *testing.T) {
 	failedAt := metav1.Time{Time: now.Add(-10 * time.Second)}
 	in := input(1, cellsv1alpha1.CellStatus{