diff --git a/modules/mesh-delivery-assigner/internal/assign/wants.go b/modules/mesh-delivery-assigner/internal/assign/wants.go index 264a2e9..1e2d5ed 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants.go @@ -201,7 +201,7 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st } if !held && !out.Lifted && !m.Released && m.FP != "" { for _, node := range slices.Sorted(slices.Values(m.Nodes)) { - if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && h.Health == delivery.HealthFailed { + if h, ok := n.Haves[node][m.Name]; ok && h.FP == m.FP && failed(h) { out.Held = &delivery.Held{Node: node, Words: HeldWords(h, m.Now)} held = true break @@ -315,6 +315,14 @@ func Wants(m Module, n Nodes, wants map[string]delivery.Want, lastHealthy map[st return out } +// failed says whether a have's health check failed once the node is done moving: phase running or failed. A failed health while it +// backs up, applies or checks is a step of the move, not its outcome (on novox at 15:27:36 on 2026-10-25, "health failed, phase +// applying" held the controller's canary two seconds before the new build was healthy). Every healthy read (healthy, providedHealthy, +// the world's lastHealthy) already asks for phase running. +func failed(h delivery.Have) bool { + return h.Health == delivery.HealthFailed && (h.Phase == delivery.PhaseRunning || h.Phase == delivery.PhaseFailed) +} + // runs says whether a have is of a module the node runs: running, whatever its health. func runs(h delivery.Have) bool { return h.Phase == delivery.PhaseRunning diff --git a/modules/mesh-delivery-assigner/internal/assign/wants_test.go b/modules/mesh-delivery-assigner/internal/assign/wants_test.go index ebaa65f..be6c0ee 100644 --- a/modules/mesh-delivery-assigner/internal/assign/wants_test.go +++ b/modules/mesh-delivery-assigner/internal/assign/wants_test.go @@ -756,3 +756,29 @@ func TestAnOldFormNodeGetsItsFirstWantAtFOnlyUnderTheCanary(t *testing.T) { t.Errorf("old-form canary: g14's want = %+v, want F", w) } } + +// A failed health counts only once the node is done moving: phase running or failed, never backing-up, applying or checking. +// The live row on novox at 15:27:36 on 2026-10-25 ("health failed, phase applying" at the controller's new fingerprint) held the +// canary two seconds before the new build was healthy. +func TestAFailedHealthWhileTheNodeMovesHoldsNothing(t *testing.T) { + for _, phase := range []delivery.Phase{delivery.PhaseBackingUp, delivery.PhaseApplying, delivery.PhaseChecking} { + m := Module{Name: "mesh-controller", FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"novox"}, + Manifest: catalogue.Manifest{Module: "mesh-controller"}} + n := Nodes{Online: map[string]bool{"novox": true}, Haves: map[string]map[string]delivery.Have{ + "novox": {"mesh-controller": {FP: fNew, Phase: phase, Health: delivery.HealthFailed}}}} + before := map[string]delivery.Want{"novox": {FP: fNew, Composed: dig(fNew)}} + if out := Wants(m, n, before, map[string]string{"novox": fOld}); out.Held != nil { + t.Errorf("phase %s, health failed: held %+v, want no hold while the node moves", phase, out.Held) + } + } + for _, phase := range []delivery.Phase{delivery.PhaseRunning, delivery.PhaseFailed} { + m := Module{Name: "mesh-controller", FP: fNew, Built: true, Granted: true, Composed: composedEverywhere, Nodes: []string{"novox"}, + Manifest: catalogue.Manifest{Module: "mesh-controller"}} + n := Nodes{Online: map[string]bool{"novox": true}, Haves: map[string]map[string]delivery.Have{ + "novox": {"mesh-controller": {FP: fNew, Phase: phase, Health: delivery.HealthFailed}}}} + before := map[string]delivery.Want{"novox": {FP: fNew, Composed: dig(fNew)}} + if out := Wants(m, n, before, map[string]string{"novox": fOld}); out.Held == nil || out.Held.Node != "novox" { + t.Errorf("phase %s, health failed: held %+v, want a hold on novox", phase, out.Held) + } + } +}