diff --git a/cmd/mesh-controller/plan_backups.go b/cmd/mesh-controller/plan_backups.go index 81a8fbfa..a38cd0b9 100644 --- a/cmd/mesh-controller/plan_backups.go +++ b/cmd/mesh-controller/plan_backups.go @@ -131,8 +131,8 @@ func loadBackupFacts(ctx context.Context, inv *inventory.Inventory) (backupFacts // stepBackups takes one step of a plan's backups and says whether the plan may go on to ask its first tier. It // asks what is not asked, reads how the asked ones went, and records it all in p.Backups; it waits (false, nil) // while any is pending, and stays stopped (false, nil) once one failed, which part B holds and says. A plan whose -// moved modules have no backed-up item keeps no record and goes on at once. An error is an ask that could not be -// made or a read the store refused: the step is tried again, the asks already made kept. +// moved modules have no backed-up item keeps no record and goes on at once. A holder that cannot be asked is tried again +// within the bound and then failed; an error is left for what the step itself cannot do. func stepBackups(ctx context.Context, p *inventory.Plan, facts backupFacts, now time.Time) (bool, error) { proceed, err := stepBackupsOnce(ctx, p, facts, now) if err == nil { @@ -164,16 +164,25 @@ func stepBackupsOnce(ctx context.Context, p *inventory.Plan, facts backupFacts, } if a.AskedAt == nil { asked := now + if a.TriedAt == nil { + a.TriedAt = &asked + } if err := askBackupNow(ctx, a.Machine, a.Module); err != nil { var unasked *backupNotAsked if errors.As(err, &unasked) { - return false, err + // A holder off the bus is tried again within the bound, by the look and never by the + // "tried again" error path; past it the backup is failed and the plan held. + a.Words = "could not be asked: " + err.Error() + if now.Sub(*a.TriedAt) >= backupsWithin { + a.State = inventory.BackupFailed + } + continue } a.State, a.Words = inventory.BackupFailed, err.Error() - a.AskedAt = &asked + a.AskedAt, a.TriedAt = &asked, nil continue } - a.AskedAt = &asked + a.AskedAt, a.TriedAt, a.Words = &asked, nil, "" continue } if _, read := nights[a.Machine]; !read { @@ -355,7 +364,7 @@ func askBackupsAgain(p *inventory.Plan, by, why string) ([]inventory.PlanBackup, continue } again = append(again, *a) - a.State, a.AskedAt, a.Words = inventory.BackupPending, nil, "" + a.State, a.AskedAt, a.TriedAt, a.Words = inventory.BackupPending, nil, nil, "" } p.Backups.State = backupsStateOf(p.Backups.Asked) p.Note = backupsNote + "asked again by " + by + ": " + why diff --git a/cmd/mesh-controller/plan_backups_hold_test.go b/cmd/mesh-controller/plan_backups_hold_test.go index 41e71296..6294e204 100644 --- a/cmd/mesh-controller/plan_backups_hold_test.go +++ b/cmd/mesh-controller/plan_backups_hold_test.go @@ -1,6 +1,8 @@ package main import ( + "context" + "fmt" "strings" "testing" "time" @@ -115,3 +117,38 @@ func TestAHeldPlanRaisesOneConditionWithTheBackupsWords(t *testing.T) { t.Fatalf("%+v", o) } } + +// A holder off the bus cannot be asked: the plan waits within the bound, retried by the look and not by the error +// path, and at the bound the backup is failed with the reason, so the hold and the condition take over. +func TestABackupHolderThatCannotBeAskedIsFailedAtTheBoundWithTheReason(t *testing.T) { + seat := &aFakeBackupSeat{} + seat.install(t) + was := askBackupNow + t.Cleanup(func() { askBackupNow = was }) + calls := 0 + askBackupNow = func(_ context.Context, machine, module string) error { + calls++ + return &backupNotAsked{fmt.Errorf("%s's backup holder was not asked: no responders", machine)} + } + entries := []inventory.Entry{aModuleWith(t, "mail", []string{"ace"}, anItem("a", ""))} + facts := backupFactsFrom(entries, holders("ace")) + p := aPlanOf("mail") + if proceed, err := stepBackups(t.Context(), p, facts, backupT0); proceed || err != nil || p.Backups.State != inventory.BackupPending { + t.Fatalf("proceed=%v err=%v %+v", proceed, err, p.Backups) + } + if proceed, err := stepBackups(t.Context(), p, facts, backupT0.Add(backupsWithin/2)); proceed || err != nil || + p.Backups.State != inventory.BackupPending || calls != 2 || p.Backups.Asked[0].AskedAt != nil { + t.Fatalf("within the bound: proceed=%v err=%v calls=%d %+v", proceed, err, calls, p.Backups) + } + if proceed, err := stepBackups(t.Context(), p, facts, backupT0.Add(backupsWithin)); proceed || err != nil { + t.Fatalf("proceed=%v err=%v", proceed, err) + } + a := p.Backups.Asked[0] + if p.Backups.State != inventory.BackupFailed || a.State != inventory.BackupFailed || + !strings.Contains(a.Words, "could not be asked: ") || !strings.Contains(a.Words, "no responders") { + t.Fatalf("%+v", p.Backups) + } + if strings.Contains(p.Note, "tried again") || !strings.Contains(p.Note, "held") { + t.Fatalf("note %q", p.Note) + } +} diff --git a/internal/inventory/plans.go b/internal/inventory/plans.go index d2e15402..68b02bda 100644 --- a/internal/inventory/plans.go +++ b/internal/inventory/plans.go @@ -100,6 +100,9 @@ type PlanBackup struct { State string `json:"state"` // AskedAt is when the holder was asked; nil while it is not asked yet. AskedAt *time.Time `json:"asked_at,omitempty"` + // TriedAt is when the controller first tried to ask, kept while the holder cannot be reached: the bound of a + // backup that is never asked counts from it. Nil once asked. + TriedAt *time.Time `json:"tried_at,omitempty"` // Words are the backup's own: the holder's error for a failed one, "did not answer in ten minutes" for a silent one. Words string `json:"words,omitempty"` }