amadan
retries: a flag-off box stays silent about historic legacy retries; honest refused-grant log; console retry test gaps
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 files changed,
+80
−10
internal/console/apiretry_test.go+10 −2internal/console/browserretry_test.go+5 −3internal/host/restart.go+1 −1internal/host/restartscope.go+38 −3internal/host/restartscope_test.go+26 −1
diff --git a/internal/console/apiretry_test.go b/internal/console/apiretry_test.go| index 66b2f39da..decc495b9 100644 |
| --- a/internal/console/apiretry_test.go |
| +++ b/internal/console/apiretry_test.go |
| @@ -32,10 +32,18 @@ func lastScoped(t *testing.T, fx *consoleFixture, app string) release.RestartReq |
| func noRetryWritten(t *testing.T, fx *consoleFixture, app string) { |
| t.Helper() |
| ctx := context.Background() |
| - if g, found, _ := release.LoadHostRetries(ctx, fx.rel, fx.sqid, app); found && len(g.Requests) != 0 { |
| + g, found, err := release.LoadHostRetries(ctx, fx.rel, fx.sqid, app) |
| + if err != nil { |
| + t.Fatalf("load the host retries: %v", err) |
| + } |
| + if found && len(g.Requests) != 0 { |
| t.Fatalf("a retry was written: %+v", g.Requests) |
| } |
| - if g, found, _ := release.LoadRestartScopes(ctx, fx.rel, fx.sqid, app); found && len(g.Requests) != 0 { |
| + g, found, err = release.LoadRestartScopes(ctx, fx.rel, fx.sqid, app) |
| + if err != nil { |
| + t.Fatalf("load the restart-scope log: %v", err) |
| + } |
| + if found && len(g.Requests) != 0 { |
| t.Fatalf("a request was written to the restart-scope log: %+v", g.Requests) |
| } |
| } |
diff --git a/internal/console/browserretry_test.go b/internal/console/browserretry_test.go| index 8b0430a9f..ad0075325 100644 |
| --- a/internal/console/browserretry_test.go |
| +++ b/internal/console/browserretry_test.go |
| @@ -142,16 +142,18 @@ func TestBrowserRetryAsksTheBoxsReportFirst(t *testing.T) { |
| name string |
| retryV int |
| epoch uint64 |
| + book string |
| want string |
| }{ |
| - {"an older box", 0, heldEpoch, browserRetryUnsupported}, |
| - {"book not saved", status.HostRetryV, 0, browserRetryUnsaved}, |
| + {"an older box", 0, heldEpoch, heldBook, browserRetryUnsupported}, |
| + {"no book", status.HostRetryV, heldEpoch, "", browserRetryUnsupported}, |
| + {"book not saved", status.HostRetryV, 0, heldBook, browserRetryUnsaved}, |
| } { |
| t.Run(tc.name, func(t *testing.T) { |
| fx := newConsoleFixture(t) |
| fx.addRoute(t, registry.Route{Host: heldHost, App: "kass", Account: fx.sqid, Kind: "instance", Status: "ready", Version: "v2", Channel: "stable"}) |
| sibling(t, fx, "kass", heldHost, "stable") |
| - writeRetryReport(t, fx, heldHost, "", tc.retryV, tc.epoch, heldBook) |
| + writeRetryReport(t, fx, heldHost, "", tc.retryV, tc.epoch, tc.book) |
| c := signInPaul(t, fx) |
| csrf := sessionToken(t, fx, c) |
| endpoint := fx.u("/apps/kass/instances/" + heldHost + "/retry") |
diff --git a/internal/host/restart.go b/internal/host/restart.go| index 4d6c35531..8f5d28afe 100644 |
| --- a/internal/host/restart.go |
| +++ b/internal/host/restart.go |
| @@ -857,7 +857,7 @@ func restartRequestLeg(ctx context.Context, routes []registry.Route, store relea |
| // the host-retry log's, and a signed retry a console built before that |
| // log left in the narrowed one, with the same checks. |
| applyRetries(routes, grants, mem) |
| - applyRetries(routes, scoped, mem) |
| + applyRetries(routes, withoutHistory(scoped, mem), mem) |
| scoped = withoutRetries(scoped) |
| if len(moved) == 0 && len(scoped) == 0 { |
| return false |
diff --git a/internal/host/restartscope.go b/internal/host/restartscope.go| index bf1945832..92bfd3a13 100644 |
| --- a/internal/host/restartscope.go |
| +++ b/internal/host/restartscope.go |
| @@ -81,6 +81,12 @@ func newRestartMemory() *restartMemory { |
| type scopedRestart struct { |
| app [2]string |
| requests []release.RestartRequest |
| + // history marks retries handed over because the box first saw the app or |
| + // lost its place in the log, not because somebody asked for them since |
| + // the last look (restartScopePass). A box with nothing that could clear |
| + // a hold reads them as history, silently, as it always has |
| + // (withoutHistory). |
| + history bool |
| } |
| // restartScopePass is the agent's comparison for narrowed requests — the twin |
| @@ -124,7 +130,7 @@ func restartScopePass(ctx context.Context, routes []registry.Route, store releas |
| prev, seen := cache[k] |
| if !seen { |
| cache[k] = newest // first sight: record, cycle nothing |
| - out = appendRetries(out, k, g.Requests) |
| + out = appendHistory(out, k, g.Requests) |
| continue |
| } |
| if prev == newest { |
| @@ -134,7 +140,7 @@ func restartScopePass(ctx context.Context, routes []registry.Route, store releas |
| cache[k] = newest |
| if !found { |
| log.Printf("agent: narrowed restarts for %s/%s: this box's position is no longer in the log — re-baselined, nothing cycled (run the verb again if something is still waiting)", k[0], k[1]) |
| - out = appendRetries(out, k, g.Requests) |
| + out = appendHistory(out, k, g.Requests) |
| continue |
| } |
| if len(pending) > 0 { |
| @@ -247,6 +253,35 @@ func appendRetries(out []scopedRestart, app [2]string, requests []release.Restar |
| return append(out, scopedRestart{app: app, requests: rs}) |
| } |
| +// appendHistory is appendRetries for retries that are history: the box first |
| +// saw the app, or lost its place in the log. |
| +func appendHistory(out []scopedRestart, app [2]string, requests []release.RestartRequest) []scopedRestart { |
| + n := len(out) |
| + out = appendRetries(out, app, requests) |
| + if len(out) > n { |
| + out[n].history = true |
| + } |
| + return out |
| +} |
| + |
| +// withoutHistory drops the retries that are history (scopedRestart.history) |
| +// when this box has nothing that could clear a hold (retryHost nil), so the |
| +// flag-off box does not log, for every legacy retry in a log it has just |
| +// met, that its rollout manager is not armed. A retry somebody asked for |
| +// since the last look is not history and still gets its line. |
| +func withoutHistory(work []scopedRestart, mem *restartMemory) []scopedRestart { |
| + if mem.retryHost != nil { |
| + return work |
| + } |
| + var out []scopedRestart |
| + for _, w := range work { |
| + if !w.history { |
| + out = append(out, w) |
| + } |
| + } |
| + return out |
| +} |
| + |
| // hostRetryPass reads each app's host-retry log (release.HostRetryKey) and |
| // returns the entries this process has not handled yet, for retryRequests |
| // and applyRetries. It rides the serial like restartScopePass, one GET per |
| @@ -323,7 +358,7 @@ func applyRetries(routes []registry.Route, work []scopedRestart, mem *restartMem |
| } |
| log.Printf("agent: retry requested for %s: its failure hold on %s is cleared and %s", rq.host, g.Build, next) |
| } else { |
| - log.Printf("agent: retry for %s: nothing held to clear for %s at epoch %d of book %q (cleared already, a newer failure since the grant, or another book's grant)", rq.host, g.Build, g.Epoch, g.Book) |
| + log.Printf("agent: retry for %s: nothing cleared for %s at epoch %d of book %q (the restriction has changed since the grant, it was another book's grant, or this box has not saved its failure book yet)", rq.host, g.Build, g.Epoch, g.Book) |
| } |
| } |
| } |
diff --git a/internal/host/restartscope_test.go b/internal/host/restartscope_test.go| index 3a5bcf780..19dedcc86 100644 |
| --- a/internal/host/restartscope_test.go |
| +++ b/internal/host/restartscope_test.go |
| @@ -849,6 +849,31 @@ func TestRestartLegDropsARetryWithNoManager(t *testing.T) { |
| } |
| } |
| +// Flag-off behaviour is unchanged: a box with no retry path that gains an |
| +// app, or first sees one, whose restart-scope log holds a legacy retry reads |
| +// it as history and says nothing about it. (A retry asked for since the last |
| +// look still gets its dropped line: TestRestartLegDropsARetryWithNoManager.) |
| +func TestRestartLegSaysNothingOfAHistoricRetryOnABoxWithNoManager(t *testing.T) { |
| + ctx, s := genKit(t) |
| + dd := t.TempDir() |
| + one := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| + both := append(slices.Clone(one), execRoute("c.example.com", "bab", "carloku")) |
| + mem := newRestartMemory() |
| + bounce := func(h string) bool { t.Errorf("%s was bounced", h); return true } |
| + restartRequestLeg(ctx, one, s, dd, bounce, nil, mem) // baseline: carloku is not here yet |
| + legacyRetry(t, ctx, s, "bab", "carloku", "c.example.com", "") |
| + |
| + var logbuf bytes.Buffer |
| + log.SetOutput(&logbuf) |
| + defer log.SetOutput(os.Stderr) |
| + if asked := restartRequestLeg(ctx, both, s, dd, bounce, nil, mem); asked { |
| + t.Fatal("a historic retry asked for a root pass") |
| + } |
| + if got := logbuf.String(); strings.Contains(got, "retry") { |
| + t.Fatalf("a box with no manager logged about a historic retry: %q", got) |
| + } |
| +} |
| + |
| // A retry names a host but is not a restart: the narrowing that resolves hosts |
| // must never address one for it, on the agent's half or the root pass's. |
| func TestScopedHostsAndRootPassStepOverARetry(t *testing.T) { |
| @@ -914,7 +939,7 @@ func TestRestartLegLogsARetryWithNothingHeld(t *testing.T) { |
| defer log.SetOutput(os.Stderr) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| got := logbuf.String() |
| - if !strings.Contains(got, "retry for a.example.com: nothing held to clear") || strings.Contains(got, "hold is cleared") { |
| + if !strings.Contains(got, "retry for a.example.com: nothing cleared") || strings.Contains(got, "hold is cleared") { |
| t.Fatalf("log = %q", got) |
| } |
| } |