cmd/mesh-controller/backups_before_send_integration_test.go

@@ -4,6 +4,7 @@ package main
 
 import (
 	"encoding/json"
+	"os"
 	"strings"
 	"sync"
 	"testing"
@@ -217,3 +218,91 @@ func TestADeliveryThatMovesABackedUpModuleWaitsForItsBackupAndAFailedOneHoldsItA
 		t.Fatalf("the condition stays: %+v", said)
 	}
 }
+
+// Issue 1086: a plan cut from a merge asked its first tier at the cut, so a module keeping data was sent without the
+// backup the plan promised. The cut now takes the backups first (askFirstTierAtCut, batches.go), and the walk's own
+// step asks the tier once they are done.
+func TestAPlanCutFromAMergeOfABackedUpModuleAsksItsBackupAndNotItsTierUntilItIsDone(t *testing.T) {
+	open := aMesh(t)
+	ctx := t.Context()
+	inv := open.inventory
+	settings := t.TempDir() + "/merge-window.json"
+	if err := os.WriteFile(settings, []byte(`{"merge-window": "90s", "merge-window-at-most": "10m"}`), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv(mergeWindowFileVar, settings)
+	everyHeadWaits(t)
+	asked := asksRecorded(t)
+	js, err := broker.Dial(testbus.URL(t))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(js.Close)
+	before := handActConn
+	handActConn = js.Conn()
+	t.Cleanup(func() { handActConn = before })
+
+	if _, err := inv.SeedSeats(ctx, catalogue.DefaultSeats()); err != nil {
+		t.Fatal(err)
+	}
+	register(t, open, catalogue.Manifest{Module: "keeper", Version: "1",
+		Claims: []catalogue.Claim{{Name: catalogue.BackupSeat, Scope: catalogue.ScopeNode, Serves: []string{"backed-up", "now", "restore"}}}})
+	if err := inv.RegisterModule(ctx, catalogue.Manifest{Module: "mesh-controller", Version: "1"},
+		inventory.Source{Repository: "novox/mesh-controller", Seat: "git", Ref: "main", BuiltFrom: "c0", Head: "c0"}); err != nil {
+		t.Fatal(err)
+	}
+	notes, err := catalogue.ParseManifest([]byte(`{"module":"notes","version":"1","data":{"own":[` +
+		`{"id":"files","path":"${dir:files}","class":"valuable","why":"people's notes"}]},` +
+		`"resources":[{"id":"files","type":"directory"}]}`))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.RegisterModule(ctx, notes, inventory.Source{Repository: "novox/notes", Seat: "git", Ref: "main",
+		BuiltFrom: "c0", Head: "c0"}); err != nil {
+		t.Fatal(err)
+	}
+	for _, m := range []string{"keeper", "notes"} {
+		if _, err := assign(ctx, open, "anchor", m); err != nil {
+			t.Fatal(err)
+		}
+	}
+	seat := &aBackupSeatOnTheBus{}
+	seat.serve(t, js.Conn(), "anchor")
+	wasWithin, wasEvery := backupsWithin, backupsPollEvery
+	backupsWithin, backupsPollEvery = time.Minute, 10*time.Millisecond
+	t.Cleanup(func() { backupsWithin, backupsPollEvery = wasWithin, wasEvery })
+
+	// 1. A merge of notes is heard and its window closes: the batch is cut through cutBatch.
+	hear(t, open, repoMerge("notes", "c-notes", t0), t0.Add(time.Second))
+	cutAt(t, open, t0.Add(time.Second+90*time.Second))
+	ws, _ := walks(t, open)
+	if len(ws) != 1 {
+		t.Fatalf("%d walk(s)", len(ws))
+	}
+	// 2. Right after the cut no tier is asked, the record has anchor/notes pending, the seat heard one `now`.
+	cut, err := inv.PlanByID(ctx, ws[0].ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(*asked) != 0 || askedInTier(cut) {
+		t.Fatalf("the first tier was asked at the cut: %v %+v", *asked, cut.Modules)
+	}
+	if cut.Backups == nil || cut.Backups.State != inventory.BackupPending || len(cut.Backups.Asked) != 1 ||
+		cut.Backups.Asked[0].Machine != "anchor" || cut.Backups.Asked[0].Module != "notes" {
+		t.Fatalf("the backups of the cut: %+v", cut.Backups)
+	}
+	if seat.asks() != 1 {
+		t.Fatalf("the seat heard %d `now`", seat.asks())
+	}
+	// 3. The backup answers done: the walk's own step asks the first tier, once, and asks no second backup.
+	seat.answers("notes", true, "")
+	advancePlans(ctx, open)
+	advancePlans(ctx, open)
+	after, err := inv.PlanByID(ctx, ws[0].ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(*asked) != 1 || after.Backups.State != inventory.BackupDone || seat.asks() != 1 {
+		t.Fatalf("builds=%v backups=%+v asks=%d", *asked, after.Backups, seat.asks())
+	}
+}

cmd/mesh-controller/batches.go

@@ -1224,7 +1224,9 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [
 		return nil
 	}
 	// Written and its first tier asked as one act on the plans (novox/hq issue 213), under the caller's hold.
-	if err := askTier(ctx, inv, &plan, plan.Tiers[plan.Tier]); err != nil {
+	if err := askFirstTierAtCut(&plan,
+		func(p *inventory.Plan) (bool, error) { return backupsFirst(ctx, inv, p) },
+		func() error { return askTier(ctx, inv, &plan, plan.Tiers[plan.Tier]) }); err != nil {
 		return err
 	}
 	if err := inv.SavePlan(ctx, &plan); err != nil {
@@ -1234,6 +1236,23 @@ func planBatch(ctx context.Context, open *stores, batch *inventory.Plan, carry [
 	return nil
 }
 
+// askFirstTierAtCut asks a freshly cut plan's first tier, after the backups of the data it moves (novox/hq ADR 0404,
+// issue 1086): the cut asks the tier itself, so advanceOnce, which asks the backups before a first tier, finds it
+// asked and never takes them. A plan whose backups wait, failed or could not be worked out asks nothing; it is saved
+// unasked and the walk's own step goes on from its record.
+func askFirstTierAtCut(p *inventory.Plan, backups func(*inventory.Plan) (bool, error), ask func() error) error {
+	proceed, err := backups(p)
+	if err != nil {
+		p.Note = backupsNote + "not asked, the backups of the plan could not be worked out: " + err.Error() +
+			"; nothing is asked or sent, and the plan tries again"
+		return nil
+	}
+	if !proceed {
+		return nil
+	}
+	return ask()
+}
+
 // placedBy writes on each module of a plan the edges of its order that put it after another module of the plan, with the
 // rule that drew each (novox/hq ADR 0419 clause 5).
 func placedBy(plan *inventory.Plan, ordered []inventory.Edge) {

cmd/mesh-controller/plan_backups_test.go

@@ -237,3 +237,40 @@ func TestTheBackupHoldersAreEveryMachinesAndAFailedReadIsNotNoHolder(t *testing.
 		t.Fatalf("a failed read of the holders must fail the step: %v", err)
 	}
 }
+
+// Issue 1086: the cut of a plan asked its first tier itself, so gitea was sent to novox with no backup asked.
+func giteaOnNovox(t *testing.T) inventory.Entry {
+	return aModuleWith(t, "gitea", []string{"novox"},
+		`{"id":"data","path":"/var/lib/gitea/data","class":"valuable","why":"repositories"}`)
+}
+
+func TestACutPlanAsksTheBackupOfGiteaOnNovoxBeforeItAsksItsFirstTier(t *testing.T) {
+	seat := &aFakeBackupSeat{}
+	seat.install(t)
+	facts := backupFactsFrom([]inventory.Entry{giteaOnNovox(t)}, holders("novox", "ace"))
+	p := aPlanOf("gitea", "mesh-delivery")
+	tierAsked := false
+	err := askFirstTierAtCut(p,
+		func(p *inventory.Plan) (bool, error) { return stepBackups(t.Context(), p, facts, backupT0) },
+		func() error { tierAsked = true; return nil })
+	if err != nil || tierAsked {
+		t.Fatalf("the first tier was asked before the backup: asked=%v err=%v", tierAsked, err)
+	}
+	if strings.Join(seat.asked, " ") != "novox/gitea" || p.Backups == nil || p.Backups.State != inventory.BackupPending {
+		t.Fatalf("asked %v, record %+v", seat.asked, p.Backups)
+	}
+	if !strings.HasPrefix(p.Note, backupsNote) {
+		t.Fatalf("note %q", p.Note)
+	}
+}
+
+func TestACutPlanWhoseBackupsCouldNotBeWorkedOutAsksNothingAndSaysSo(t *testing.T) {
+	p := aPlanOf("gitea")
+	tierAsked := false
+	err := askFirstTierAtCut(p,
+		func(*inventory.Plan) (bool, error) { return false, fmt.Errorf("the catalogue could not be read") },
+		func() error { tierAsked = true; return nil })
+	if err != nil || tierAsked || !strings.Contains(p.Note, "could not be worked out") {
+		t.Fatalf("asked=%v err=%v note %q", tierAsked, err, p.Note)
+	}
+}