Merge pull request 'A build waits out a registry held still, and a retry asks every failed build of the tier (issue 457)' (#225) from fix/457-a-build-waits-out-a-registry-pause into main

2026-10-11 09:17:09 +00:00
parent 3b6b54a501 83ce19b9c0
commit 02c61b983c
7 changed files with 419 additions and 26 deletions
@@ -288,13 +288,24 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) {
 				"those walks answer them; a newer merge, or `rebuild <module>`, builds again", p.ID)
 		}
 	}
+	// Settled from the build records before anything is judged (novox/hq issue 457): a build of the tier
+	// that failed after the plan did is as failed as the one that failed it. What toRetry says is the
+	// failed set every step below works from.
+	var failed []string
+	if p.Tier < len(p.Tiers) {
+		recorded, byID, err := recordsOfAsked(ctx, inv, &p, p.Tiers[p.Tier])
+		if err != nil {
+			return "", err
+		}
+		failed = toRetry(&p, recorded, byID)
+	}
 	if err := retryRefusal(p, plans); err != nil {
 		return "", err
 	}
-	if again := unjudgedAtGate(p); len(failedIn(p)) == 0 && len(again) > 0 {
+	if again := unjudgedAtGate(p); len(failed) == 0 && len(again) > 0 {
 		return retryTierWhole(ctx, open, &p, again)
 	}
-	if len(failedIn(p)) == 0 {
+	if len(failed) == 0 {
 		return retryRollouts(ctx, open, &p)
 	}
 	entries, err := inv.Catalogued(ctx)
@@ -305,7 +316,6 @@ func retryPlan(ctx context.Context, open *stores, id string) (string, error) {
 	for _, e := range entries {
 		byName[e.Manifest.Module] = e
 	}
-	failed := failedIn(p)
 	var asked []string
 	for _, m := range failed {
 		askModule(ctx, &p, m, byName)
@@ -525,3 +535,16 @@ func retryTierWhole(ctx context.Context, open *stores, p *inventory.Plan, again
 func sendAgain(s *inventory.PlanModule) {
 	s.First, s.FirstAt, s.Gate, s.GatedBy, s.Previous, s.Why = nil, nil, nil, "", "", ""
 }
+
+// toRetry is the modules a retry asks again: every one of the tier that failed, the plan's state
+// settled from the build records first (novox/hq issue 457). A plan fails on the first failure in its
+// tier, and an outcome arriving after that finds no open plan to answer — it is kept only in the build
+// records. Read from the plan alone, a retry asked only the build that failed first, and the records
+// then failed the plan again on the next: each failed build of a tier took a retry of its own. What
+// still runs is left asked, and its outcome is the plan's once the retry sets it building.
+func toRetry(p *inventory.Plan, recorded map[string][]inventory.Build, byID map[string]inventory.Build) []string {
+	if p.Tier < len(p.Tiers) {
+		settleFromRecords(p, p.Tiers[p.Tier], recorded, byID)
+	}
+	return failedIn(*p)
+}
@@ -0,0 +1,57 @@
+package main
+
+import (
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/inventory"
+)
+
+// A retry asks every failed build of the tier, not one (novox/hq issue 457). Seen 2026-10-11: gitea
+// and plex both failed in tier 0 while the registry was held still; gitea's failure failed the plan,
+// and plex's, arriving after, found no open plan and was kept only in the build records. The first
+// retry asked gitea alone, the records then failed the plan again on plex, and a second retry asked
+// plex.
+func TestARetryAsksEveryFailedBuildOfTheTier(t *testing.T) {
+	asked := time.Date(2026, 10, 11, 1, 29, 0, 0, time.UTC)
+	failedAt := asked.Add(2 * time.Minute)
+	p := inventory.Plan{ID: "plan-457", State: inventory.PlanFailed, Tier: 0,
+		Tiers: [][]string{{"gitea", "plex"}}, Note: "gitea failed to build in tier 0",
+		Modules: map[string]*inventory.PlanModule{
+			"gitea": {State: "failed", AskedAt: &asked, Build: "build-gitea", Why: "cannot reach the registry"},
+			"plex":  {State: "asked", AskedAt: &asked, Build: "build-plex"},
+		}}
+	byID := map[string]inventory.Build{
+		"build-plex": {ID: "build-plex", Module: "plex", At: failedAt, Failed: "cannot reach the registry"},
+	}
+
+	if got := toRetry(&p, nil, byID); !reflect.DeepEqual(got, []string{"gitea", "plex"}) {
+		t.Fatalf("a retry of a tier where gitea and plex failed asks %v", got)
+	}
+}
+
+// A build of the tier still running when the plan failed is not asked again: its outcome is the plan's
+// once the retry sets it building, and one that is recorded built is taken as built.
+func TestARetryLeavesABuildThatRunsOrWorked(t *testing.T) {
+	asked := time.Date(2026, 10, 11, 1, 29, 0, 0, time.UTC)
+	builtAt := asked.Add(3 * time.Minute)
+	p := inventory.Plan{ID: "plan-457", State: inventory.PlanFailed, Tier: 0,
+		Tiers: [][]string{{"a", "b", "c"}},
+		Modules: map[string]*inventory.PlanModule{
+			"a": {State: "failed", AskedAt: &asked, Build: "build-a", Why: "broken"},
+			"b": {State: "asked", AskedAt: &asked, Build: "build-b"},
+			"c": {State: "asked", AskedAt: &asked, Build: "build-c"},
+		}}
+	byID := map[string]inventory.Build{"build-c": {ID: "build-c", Module: "c", At: builtAt, Commit: "c0ffee"}}
+
+	if got := toRetry(&p, nil, byID); !reflect.DeepEqual(got, []string{"a"}) {
+		t.Fatalf("asks %v, want a alone", got)
+	}
+	if s := p.Modules["c"]; s.State != "built" || s.Commit != "c0ffee" {
+		t.Errorf("c, recorded built, is %+v", s)
+	}
+	if s := p.Modules["b"]; s.State != "asked" {
+		t.Errorf("b, still building, is %+v", s)
+	}
+}
@@ -649,26 +649,9 @@ func advanceOnce(ctx context.Context, open *stores, p *inventory.Plan,
 	// the controller in its first tier: the build that produced the new one is recorded, and the
 	// plan never hears it. The record is the fact; a build recorded after the ask is that tier's
 	// outcome, whoever was listening.
-	recorded := map[string][]inventory.Build{}
-	byID := map[string]inventory.Build{}
-	for _, m := range tier {
-		if s := p.Modules[m]; s != nil && s.State == "asked" {
-			builds, err := inv.Builds(ctx, m, 5)
-			if err != nil {
-				return false, err
-			}
-			recorded[m] = builds
-			// Its own ask's record, by id — found even when the outcome named no module (ADR 0219).
-			if s.Build != "" {
-				b, found, err := inv.BuildByID(ctx, s.Build)
-				if err != nil {
-					return false, err
-				}
-				if found {
-					byID[s.Build] = b
-				}
-			}
-		}
+	recorded, byID, err := recordsOfAsked(ctx, inv, p, tier)
+	if err != nil {
+		return false, err
 	}
 	if settleFromRecords(p, tier, recorded, byID) {
 		return true, nil
@@ -1776,6 +1759,34 @@ func splitList(s string) []string {
 	return out
 }

+// recordsOfAsked is what settleFromRecords reads: for every module of the tier still `asked`, its last
+// builds and the record of its own ask, by id.
+func recordsOfAsked(ctx context.Context, inv *inventory.Inventory, p *inventory.Plan, tier []string) (
+	map[string][]inventory.Build, map[string]inventory.Build, error) {
+	recorded := map[string][]inventory.Build{}
+	byID := map[string]inventory.Build{}
+	for _, m := range tier {
+		if s := p.Modules[m]; s != nil && s.State == "asked" {
+			builds, err := inv.Builds(ctx, m, 5)
+			if err != nil {
+				return nil, nil, err
+			}
+			recorded[m] = builds
+			// Its own ask's record, by id — found even when the outcome named no module (ADR 0219).
+			if s.Build != "" {
+				b, found, err := inv.BuildByID(ctx, s.Build)
+				if err != nil {
+					return nil, nil, err
+				}
+				if found {
+					byID[s.Build] = b
+				}
+			}
+		}
+	}
+	return recorded, byID, nil
+}
+
 // settleFromRecords marks every module of the tier still `asked` built — or failed — from a build
 // recorded after it was asked, and says whether it changed anything (novox/hq 04-ISSUES/214).
 // Newest first, as Builds answers: the first record after the ask is the outcome of that ask.