zdd-readiness: final review round 1, retry grants name the failure book they count in
Finding 3 (Astra, High): epochs are a counter every book starts alike, so a grant for one book's restriction could clear another's at the same number. The book now has a persistent random identity (failureFile.Book, 16 bytes hex), published beside HeldEpoch as HostRollout.HeldBook (additive); the console's retry doors carry it and sign it (HostRetryGrant.Book); RetryHostAt clears only for a grant naming the box's own book, and an empty book never clears. Every open advances the sequence by 1<<20 so epochs handed out unsaved before a crash are never reissued; an armed box saves that before arming and refuses managed starts when it cannot. A declined box keeps its never-writes-on-open rule: it makes no new restriction epochs outside reconciliation, which already flushes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
22 files changed,
+389
−125
instances_e2e_test.go+5 −5internal/console/apiinstances.go+6 −4internal/console/apiretry.go+24 −15internal/console/apiretry_test.go+13 −9internal/console/browserretry.go+3 −3internal/console/browserretry_test.go+2 −2internal/host/restart.go+2 −2internal/host/restartscope.go+4 −3internal/host/restartscope_test.go+40 −27internal/host/rollout.go+14 −8internal/host/rollout_test.go+47 −0internal/host/rolloutdeclined.go+17 −5internal/host/rolloutdeclined_test.go+6 −5internal/release/hostretry.go+11 −5internal/release/hostretry_test.go+8 −7internal/rollout/failures.go+72 −6internal/rollout/finalreview1_test.go+82 −1internal/rollout/manager.go+12 −8internal/rollout/retrygrant_test.go+12 −9internal/rollout/sim_test.go+1 −1internal/rollout/status.go+2 −0internal/status/snapshot.go+6 −0
diff --git a/instances_e2e_test.go b/instances_e2e_test.go| index 51ea76704..dae1ecf85 100644 |
| --- a/instances_e2e_test.go |
| +++ b/instances_e2e_test.go |
| @@ -293,7 +293,7 @@ func TestE2EInstancesThroughConsole(t *testing.T) { |
| box := status.CollectBox(nil, status.Probes{MachineID: "e2e-box"}, time.Now()) |
| box.Metrics = &status.BoxMetrics{Rollout: &status.RolloutReport{Hosts: map[string]status.HostRollout{ |
| haHost: {Account: release.OpsAccount, App: "titogo", Installed: "v2", Serving: "v1", Good: "v1", |
| - Awake: true, HeldBy: "v2", HeldEpoch: 9, Reason: status.ReasonHealth}, |
| + Awake: true, HeldBy: "v2", HeldEpoch: 9, HeldBook: "e2e-book", Reason: status.ReasonHealth}, |
| }}} |
| if err := status.WriteBox(context.Background(), relStore, box); err != nil { |
| t.Fatal(err) |
| @@ -312,10 +312,10 @@ func TestE2EInstancesThroughConsole(t *testing.T) { |
| t.Fatalf("last request = %+v", last) |
| } |
| // The entry carries the console's root-signed grant for the held build |
| - // at the epoch the box reported (Task 8, spec §11): the box clears |
| - // nothing on an entry without one. |
| - if g, err := release.VerifyHostRetry(pub, last.Scope.Signed, release.OpsAccount, "titogo", haHost, time.Now()); err != nil || g.Build != "v2" || g.Epoch != 9 { |
| - t.Fatalf("signed grant = %+v, %v; want v2 at epoch 9", g, err) |
| + // at the epoch and book the box reported (Task 8, spec §11; final review |
| + // round 1, finding 3): the box clears nothing on an entry without one. |
| + if g, err := release.VerifyHostRetry(pub, last.Scope.Signed, release.OpsAccount, "titogo", haHost, time.Now()); err != nil || g.Build != "v2" || g.Epoch != 9 || g.Book != "e2e-book" { |
| + t.Fatalf("signed grant = %+v, %v; want v2 at epoch 9 of book e2e-book", g, err) |
| } |
| // The whole flow ran through the console: nothing here may have touched |
diff --git a/internal/console/apiinstances.go b/internal/console/apiinstances.go| index 5bc77a030..d8417a545 100644 |
| --- a/internal/console/apiinstances.go |
| +++ b/internal/console/apiinstances.go |
| @@ -371,9 +371,11 @@ type rolloutView struct { |
| Unavailable string `json:"unavailable,omitempty"` |
| account, app string |
| // heldEpoch is the box's epoch for HeldBy's restriction |
| - // (status.HostRollout.HeldEpoch). It is not shown: the retry doors sign |
| - // it into their grant (retryHostBuild). |
| + // (status.HostRollout.HeldEpoch), and heldBook the failure book it |
| + // counts in (HeldBook, final review round 1, finding 3). Neither is |
| + // shown: the retry doors sign both into their grant (retryHostBuild). |
| heldEpoch uint64 |
| + heldBook string |
| // refused marks a view built from an Unavailable box, which has no |
| // account or app to check (hosts are unique; the caller filtered by app). |
| refused bool |
| @@ -588,7 +590,7 @@ func (j *boxJoin) foldRollout(b status.Box, override bool) { |
| } |
| for host, hr := range ro.Hosts { |
| put(host, rolloutView{Box: b.Label, Installed: hr.Installed, Good: hr.Good, Attempt: hr.Attempt, AttemptBuild: hr.AttemptBuild, |
| - HeldBy: hr.HeldBy, Reason: hr.Reason, Unavailable: ro.Unavailable, account: hr.Account, app: hr.App, heldEpoch: hr.HeldEpoch}) |
| + HeldBy: hr.HeldBy, Reason: hr.Reason, Unavailable: ro.Unavailable, account: hr.Account, app: hr.App, heldEpoch: hr.HeldEpoch, heldBook: hr.HeldBook}) |
| } |
| return |
| } |
| @@ -609,7 +611,7 @@ func (j *boxJoin) foldRollout(b status.Box, override bool) { |
| Box: b.Label, Installed: hr.Installed, Serving: hr.Serving, Good: hr.Good, Awake: hr.Awake, |
| Attempt: hr.Attempt, AttemptBuild: hr.AttemptBuild, HeldBy: hr.HeldBy, Reason: hr.Reason, |
| ConfigPending: hr.ConfigPending, Transient: hr.Transient, SchedulerIdle: a.Queued == 0 && a.Running == 0 && a.Settling == 0, |
| - account: hr.Account, app: hr.App, heldEpoch: hr.HeldEpoch, |
| + account: hr.Account, app: hr.App, heldEpoch: hr.HeldEpoch, heldBook: hr.HeldBook, |
| }) |
| } |
| } |
diff --git a/internal/console/apiretry.go b/internal/console/apiretry.go| index 9ac29877c..3d4086d75 100644 |
| --- a/internal/console/apiretry.go |
| +++ b/internal/console/apiretry.go |
| @@ -44,7 +44,8 @@ var ( |
| ) |
| // retryHostBuild is the build holding host back, and the epoch of that |
| -// restriction (plan review finding 13), as its box reports them. The record |
| +// restriction and the failure book it counts in (plan review finding 13; |
| +// final review round 1, finding 3), as its box reports them. The record |
| // must exist in this account and app (the instance family's 404), and the |
| // report must be this app's own (boxJoin.rollout), read through the same |
| // loader as the list and the instance page (loadRolloutJoin). A box that |
| @@ -53,33 +54,40 @@ var ( |
| // host held by the failure book it still honours: that box verifies and |
| // applies a signed retry too (Task 10, plan review #5). A refusing box, or |
| // one whose code this console does not know, is never retried here. A box |
| -// older than the epoch reports 0, which is signed as it is and which no |
| -// box accepts. |
| -func (s *Server) retryHostBuild(ctx context.Context, acct *Account, app, host string) (string, uint64, error) { |
| +// older than the epoch reports 0, and one older than the book reports "": |
| +// each is signed as it is, and no box accepts it. |
| +func (s *Server) retryHostBuild(ctx context.Context, acct *Account, app, host string) (heldRestriction, error) { |
| if _, err := provision.GetRecord(ctx, s.cfg.Releases, acct.Sqid, app, host); err != nil { |
| if errors.Is(err, os.ErrNotExist) { |
| - return "", 0, errRetryNoRecord |
| + return heldRestriction{}, errRetryNoRecord |
| } |
| - return "", 0, err |
| + return heldRestriction{}, err |
| } |
| join, _ := s.loadRolloutJoin(ctx, acct.Sqid, time.Now(), nil) |
| ro := join.rollout(host, acct.Sqid, app) |
| if ro == nil || ro.HeldBy == "" || (ro.Unavailable != "" && !rolloutwords.UnavailableDeclines(ro.Unavailable)) { |
| - return "", 0, errRetryNotHeld |
| + return heldRestriction{}, errRetryNotHeld |
| } |
| - return ro.HeldBy, ro.heldEpoch, nil |
| + return heldRestriction{build: ro.HeldBy, book: ro.heldBook, epoch: ro.heldEpoch}, nil |
| } |
| -// requestHostRetry signs the retry grant for build at epoch with the root |
| -// key, appends it as a retry request, by the same append the |
| -// narrowed-restart door uses, and announces it on the serial. With no key |
| +// heldRestriction is what a retry grant names: the build holding the host |
| +// back, and that restriction's epoch and book as its box reported them. |
| +type heldRestriction struct { |
| + build, book string |
| + epoch uint64 |
| +} |
| + |
| +// requestHostRetry signs the retry grant for held (its build, book and |
| +// epoch) with the root key, appends it as a retry request, by the same |
| +// append the narrowed-restart door uses, and announces it on the serial. With no key |
| // it writes nothing (errRetryNoKey): an unsigned entry is one every box |
| // drops. |
| -func (s *Server) requestHostRetry(ctx context.Context, acct, app, host, build string, epoch uint64) (release.RestartRequest, error) { |
| +func (s *Server) requestHostRetry(ctx context.Context, acct, app, host string, held heldRestriction) (release.RestartRequest, error) { |
| if s.cfg.Key == nil { |
| return release.RestartRequest{}, errRetryNoKey |
| } |
| - signed, err := release.SignHostRetry(release.RootSigner(s.cfg.Key), acct, app, host, build, epoch, time.Now()) |
| + signed, err := release.SignHostRetry(release.RootSigner(s.cfg.Key), acct, app, host, held.build, held.book, held.epoch, time.Now()) |
| if err != nil { |
| return release.RestartRequest{}, err |
| } |
| @@ -128,7 +136,7 @@ func (s *Server) handleAPIInstanceRetry(w http.ResponseWriter, r *http.Request) |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| - build, epoch, err := s.retryHostBuild(ctx, acct, app, host) |
| + held, err := s.retryHostBuild(ctx, acct, app, host) |
| switch { |
| case errors.Is(err, errRetryNoRecord): |
| http.NotFound(w, r) |
| @@ -141,7 +149,8 @@ func (s *Server) handleAPIInstanceRetry(w http.ResponseWriter, r *http.Request) |
| jsonError(w, http.StatusBadGateway, "could not read the instance") |
| return |
| } |
| - rr, err := s.requestHostRetry(ctx, acct.Sqid, app, host, build, epoch) |
| + build := held.build |
| + rr, err := s.requestHostRetry(ctx, acct.Sqid, app, host, held) |
| if err != nil { |
| log.Printf("console: api instance retry %s/%s/%s: %v", acct.Sqid, app, host, err) |
| jsonError(w, http.StatusBadGateway, apiRetryStore) |
diff --git a/internal/console/apiretry_test.go b/internal/console/apiretry_test.go| index dd118a30e..3f39b4979 100644 |
| --- a/internal/console/apiretry_test.go |
| +++ b/internal/console/apiretry_test.go |
| @@ -23,14 +23,18 @@ func lastScoped(t *testing.T, fx *consoleFixture, app string) release.RestartReq |
| return g.Requests[len(g.Requests)-1] |
| } |
| -// heldEpoch is the restriction epoch writeHeldBox reports. |
| -const heldEpoch = 17 |
| +// heldEpoch is the restriction epoch writeHeldBox reports, and heldBook the |
| +// failure book it counts in (final review round 1, finding 3). |
| +const ( |
| + heldEpoch = 17 |
| + heldBook = "0123456789abcdef0123456789abcdef" |
| +) |
| func writeHeldBox(t *testing.T, fx *consoleFixture, host, app, build string) { |
| t.Helper() |
| box := status.CollectBox(nil, status.Probes{MachineID: "box1"}, time.Now()) |
| box.Metrics = &status.BoxMetrics{Rollout: &status.RolloutReport{Hosts: map[string]status.HostRollout{ |
| - host: {Account: fx.sqid, App: app, Installed: build, Serving: "v1", Good: "v1", Awake: true, HeldBy: build, HeldEpoch: heldEpoch, Reason: status.ReasonHealth}, |
| + host: {Account: fx.sqid, App: app, Installed: build, Serving: "v1", Good: "v1", Awake: true, HeldBy: build, HeldEpoch: heldEpoch, HeldBook: heldBook, Reason: status.ReasonHealth}, |
| }}} |
| if err := status.WriteBox(context.Background(), fx.rel, box); err != nil { |
| t.Fatal(err) |
| @@ -63,8 +67,8 @@ func TestAPIInstanceRetryWritesARetryRequest(t *testing.T) { |
| // The entry carries the root's grant for the held build at the box's |
| // epoch (spec §11, plan review finding 13). |
| g, err := release.VerifyHostRetry(fx.pub, r.Scope.Signed, fx.sqid, "kass", host, time.Now()) |
| - if err != nil || g.Build != "v2" || g.Epoch != heldEpoch { |
| - t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d", g, err, heldEpoch) |
| + if err != nil || g.Build != "v2" || g.Epoch != heldEpoch || g.Book != heldBook { |
| + t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d of book %s", g, err, heldEpoch, heldBook) |
| } |
| if n := serial(t, fx); n != 1 { |
| t.Fatalf("serial = %d", n) |
| @@ -124,7 +128,7 @@ func TestAPIInstanceRetryOfADeclinedBoxsHold(t *testing.T) { |
| ro := &status.RolloutReport{Unavailable: code, FailureBook: "honoured"} |
| if held { |
| ro.Hosts = map[string]status.HostRollout{host: {Account: fx.sqid, App: "kass", Installed: "v2", Good: "v1", |
| - Attempt: status.AttemptDown, AttemptBuild: "v2", HeldBy: "v2", HeldEpoch: heldEpoch}} |
| + Attempt: status.AttemptDown, AttemptBuild: "v2", HeldBy: "v2", HeldEpoch: heldEpoch, HeldBook: heldBook}} |
| } |
| box := status.CollectBox(nil, status.Probes{MachineID: "box1"}, time.Now()) |
| box.Metrics = &status.BoxMetrics{RouteVersions: map[string]string{host: "v2"}, Rollout: ro} |
| @@ -165,8 +169,8 @@ func TestAPIInstanceRetryOfADeclinedBoxsHold(t *testing.T) { |
| } |
| r := lastScoped(t, fx, "kass") |
| g, err := release.VerifyHostRetry(fx.pub, r.Scope.Signed, fx.sqid, "kass", host, time.Now()) |
| - if err != nil || g.Build != "v2" || g.Epoch != heldEpoch { |
| - t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d", g, err, heldEpoch) |
| + if err != nil || g.Build != "v2" || g.Epoch != heldEpoch || g.Book != heldBook { |
| + t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d of book %s", g, err, heldEpoch, heldBook) |
| } |
| }) |
| } |
| @@ -176,7 +180,7 @@ func TestAPIInstanceRetryOfADeclinedBoxsHold(t *testing.T) { |
| box := status.CollectBox(nil, status.Probes{MachineID: "box1"}, time.Now()) |
| box.Metrics = &status.BoxMetrics{RouteVersions: map[string]string{host: "v2"}, Rollout: &status.RolloutReport{ |
| Unavailable: "no-cgroup-delegation", FailureBook: "honoured", |
| - Hosts: map[string]status.HostRollout{host: {Account: fx.sqid, App: "otherapp", Installed: "v2", HeldBy: "v2", HeldEpoch: heldEpoch}}}} |
| + Hosts: map[string]status.HostRollout{host: {Account: fx.sqid, App: "otherapp", Installed: "v2", HeldBy: "v2", HeldEpoch: heldEpoch, HeldBook: heldBook}}}} |
| if err := status.WriteBox(context.Background(), fx.rel, box); err != nil { |
| t.Fatal(err) |
| } |
diff --git a/internal/console/browserretry.go b/internal/console/browserretry.go| index 0e7beb084..1c2c14a67 100644 |
| --- a/internal/console/browserretry.go |
| +++ b/internal/console/browserretry.go |
| @@ -58,7 +58,7 @@ func (s *Server) handleInstanceRetry(w http.ResponseWriter, r *http.Request) { |
| } |
| ctx, cancel := context.WithTimeout(r.Context(), 15*time.Second) |
| defer cancel() |
| - build, epoch, err := s.retryHostBuild(ctx, acct, app, host) |
| + held, err := s.retryHostBuild(ctx, acct, app, host) |
| switch { |
| case errors.Is(err, errRetryNoRecord): |
| http.NotFound(w, r) |
| @@ -71,7 +71,7 @@ func (s *Server) handleInstanceRetry(w http.ResponseWriter, r *http.Request) { |
| flash("err", swapCopy.RetryFailed) |
| return |
| } |
| - fill := strings.NewReplacer("{host}", host, "{build}", build) |
| + fill := strings.NewReplacer("{host}", host, "{build}", held.build) |
| if !confirmed(r) { |
| s.renderConfirm(w, r, addr, raw, acct, role, app, &ConfirmView{ |
| Action: r.URL.Path, Fields: [][2]string{{"csrf", raw}}, CancelURL: back, |
| @@ -84,7 +84,7 @@ func (s *Server) handleInstanceRetry(w http.ResponseWriter, r *http.Request) { |
| flash("err", swapCopy.RetryLimited) |
| return |
| } |
| - if _, err := s.requestHostRetry(ctx, acct.Sqid, app, host, build, epoch); err != nil { |
| + if _, err := s.requestHostRetry(ctx, acct.Sqid, app, host, held); err != nil { |
| log.Printf("console: browser retry %s/%s/%s: %v", acct.Sqid, app, host, err) |
| flash("err", swapCopy.RetryFailed) |
| return |
diff --git a/internal/console/browserretry_test.go b/internal/console/browserretry_test.go| index a8a5f248c..4942ee739 100644 |
| --- a/internal/console/browserretry_test.go |
| +++ b/internal/console/browserretry_test.go |
| @@ -102,8 +102,8 @@ func TestBrowserRetryConfirmsThenWrites(t *testing.T) { |
| t.Fatalf("stored = %+v", r) |
| } |
| g, err := release.VerifyHostRetry(fx.pub, r.Scope.Signed, fx.sqid, "kass", heldHost, time.Now()) |
| - if err != nil || g.Build != "v2" || g.Epoch != heldEpoch { |
| - t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d", g, err, heldEpoch) |
| + if err != nil || g.Build != "v2" || g.Epoch != heldEpoch || g.Book != heldBook { |
| + t.Fatalf("signed grant = %+v, %v; want v2 at epoch %d of book %s", g, err, heldEpoch, heldBook) |
| } |
| } |
diff --git a/internal/host/restart.go b/internal/host/restart.go| index 9ad6a94f5..68b641d05 100644 |
| --- a/internal/host/restart.go |
| +++ b/internal/host/restart.go |
| @@ -865,14 +865,14 @@ func restartRequestLeg(ctx context.Context, routes []registry.Route, store relea |
| log.Printf("agent: retry requested for %s, dropped: %s", rq.host, why) |
| continue |
| } |
| - if mem.retryHost(rq.host, g.Build, g.Epoch) { |
| + if mem.retryHost(rq.host, g.Build, g.Book, g.Epoch) { |
| next := "its next attempt runs as a canary" |
| if armedRollout() == nil { |
| next = "it starts the build as today" // a declined box runs no canary |
| } |
| 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 (cleared already, or a newer failure since the grant)", rq.host, g.Build, g.Epoch) |
| + 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) |
| } |
| } |
| scoped = withoutRetries(scoped) |
diff --git a/internal/host/restartscope.go b/internal/host/restartscope.go| index 63913b1fb..919f539d5 100644 |
| --- a/internal/host/restartscope.go |
| +++ b/internal/host/restartscope.go |
| @@ -54,13 +54,14 @@ type restartMemory struct { |
| scope map[[2]string]string // app -> last applied request id, or release.RestartScopeNone |
| // retryHost clears one host's failure hold, for a RestartScopeRetry |
| - // request whose root-signed grant verified: the grant's build and the |
| - // restriction's epoch it was issued for (rolloutRetryHost). It is the |
| + // request whose root-signed grant verified: the grant's build, and the |
| + // restriction's book and epoch it was issued for (rolloutRetryHost; |
| + // final review round 1, finding 3). It is the |
| // rollout manager's (rollout.Manager.RetryHost) when armed, and a |
| // declined box's that honours its failure book (failureHolds.retryHost, |
| // Task 10). nil otherwise: there is no hold to clear, and the request is |
| // dropped. |
| - retryHost func(host, build string, epoch uint64) bool |
| + retryHost func(host, build, book string, epoch uint64) bool |
| // pub is the deployment root's public key (CARLOS_RELEASE_PUBKEY), which |
| // every retry's grant is verified against (spec §11). Nil drops every |
| // retry: a box that cannot tell a grant from a forgery clears nothing. |
diff --git a/internal/host/restartscope_test.go b/internal/host/restartscope_test.go| index af232d197..cecdb2a8d 100644 |
| --- a/internal/host/restartscope_test.go |
| +++ b/internal/host/restartscope_test.go |
| @@ -584,29 +584,29 @@ func saveMarks(t *testing.T, path string, m *restartMarks) { |
| } |
| // A retry request clears one host's hold through the rollout manager, cycles |
| -// nothing, and asks root for no restart pass. The grant's build and epoch are |
| -// what the manager is handed, once. |
| +// nothing, and asks root for no restart pass. The grant's build, book and |
| +// epoch are what the manager is handed, once. |
| func TestRestartLegHandsARetryToTheRolloutManager(t *testing.T) { |
| ctx, s := genKit(t) |
| dd := t.TempDir() |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven"), execRoute("b.example.com", "bab", "eleven")} |
| mem, priv := retryMemory(t) |
| var retried []string |
| - mem.retryHost = func(h, b string, e uint64) bool { |
| - retried = append(retried, fmt.Sprintf("%s %s %d", h, b, e)) |
| + mem.retryHost = func(h, b, bk string, e uint64) bool { |
| + retried = append(retried, fmt.Sprintf("%s %s %s %d", h, b, bk, e)) |
| return true |
| } |
| bounce := func(h string) bool { t.Errorf("%s was bounced for a retry", h); return true } |
| if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem); asked { |
| t.Fatal("the baselining leg asked for a root pass") |
| } |
| - askRetry(t, ctx, s, priv, "bab", "eleven", "b.example.com", "v2", 5) |
| - askRetry(t, ctx, s, priv, "bab", "eleven", "elsewhere.example.com", "v2", 5) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", "b.example.com", "v2", "bk", 5) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", "elsewhere.example.com", "v2", "bk", 5) |
| if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem); asked { |
| t.Fatal("a retry-only move asked for a root restart pass") |
| } |
| - if !slices.Equal(retried, []string{"b.example.com v2 5"}) { |
| - t.Fatalf("retried %v, want only this box's b.example.com, for v2 at epoch 5", retried) |
| + if !slices.Equal(retried, []string{"b.example.com v2 bk 5"}) { |
| + t.Fatalf("retried %v, want only this box's b.example.com, for v2 at epoch 5 of book bk", retried) |
| } |
| } |
| @@ -625,9 +625,9 @@ func retryMemory(t *testing.T) (*restartMemory, ed25519.PrivateKey) { |
| // askRetry is the console's signed `carlos instances retry <host>` landing |
| // in the bucket (internal/console requestHostRetry). |
| -func askRetry(t *testing.T, ctx context.Context, s release.Store, priv ed25519.PrivateKey, acct, app, host, build string, epoch uint64) string { |
| +func askRetry(t *testing.T, ctx context.Context, s release.Store, priv ed25519.PrivateKey, acct, app, host, build, book string, epoch uint64) string { |
| t.Helper() |
| - wire, err := release.SignHostRetry(release.RootSigner(priv), acct, app, host, build, epoch, time.Now()) |
| + wire, err := release.SignHostRetry(release.RootSigner(priv), acct, app, host, build, book, epoch, time.Now()) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -654,15 +654,18 @@ func TestRestartLegDropsARetryThatIsNotTheRootsGrant(t *testing.T) { |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven"), execRoute("b.example.com", "bab", "eleven")} |
| mem, priv := retryMemory(t) |
| _, other, _ := ed25519.GenerateKey(nil) |
| - mem.retryHost = func(h, b string, e uint64) bool { t.Errorf("retryHost(%s, %s, %d) was called", h, b, e); return true } |
| + mem.retryHost = func(h, b, _ string, e uint64) bool { |
| + t.Errorf("retryHost(%s, %s, %d) was called", h, b, e) |
| + return true |
| + } |
| bounce := func(h string) bool { t.Errorf("%s was bounced", h); return true } |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) // baseline |
| askScoped(t, ctx, s, "bab", "eleven", release.RestartScopeRetry, "a.example.com") |
| - askRetry(t, ctx, s, other, "bab", "eleven", "a.example.com", "v2", 5) |
| - forB, _ := release.SignHostRetry(release.RootSigner(priv), "bab", "eleven", "b.example.com", "v2", 5, time.Now()) |
| + askRetry(t, ctx, s, other, "bab", "eleven", "a.example.com", "v2", "bk", 5) |
| + forB, _ := release.SignHostRetry(release.RootSigner(priv), "bab", "eleven", "b.example.com", "v2", "bk", 5, time.Now()) |
| askSignedRetry(t, ctx, s, "bab", "eleven", "a.example.com", forB) |
| - forShop, _ := release.SignHostRetry(release.RootSigner(priv), "bab", "shop", "a.example.com", "v2", 5, time.Now()) |
| + forShop, _ := release.SignHostRetry(release.RootSigner(priv), "bab", "shop", "a.example.com", "v2", "bk", 5, time.Now()) |
| askSignedRetry(t, ctx, s, "bab", "eleven", "a.example.com", forShop) |
| var logbuf bytes.Buffer |
| @@ -679,7 +682,7 @@ func TestRestartLegDropsARetryThatIsNotTheRootsGrant(t *testing.T) { |
| // A box with no root key cannot tell a grant from a forgery. |
| logbuf.Reset() |
| mem.pub = nil |
| - askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", 5) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", "bk", 5) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| if !strings.Contains(logbuf.String(), "agent: retry requested for a.example.com, dropped: this box has no root public key") { |
| t.Fatalf("log = %q, want the keyless drop named", logbuf.String()) |
| @@ -688,9 +691,9 @@ func TestRestartLegDropsARetryThatIsNotTheRootsGrant(t *testing.T) { |
| // The grants against a real failure book (plan review findings 5 and 13): a |
| // grant whose epoch predates a second failure of the same build on the same |
| -// host, a grant for another build, and a used grant replayed after the book |
| -// is reopened each clear nothing; the grant for the current restriction |
| -// clears it. |
| +// host, a grant for another build, a grant for another book (final review |
| +// round 1, finding 3), and a used grant replayed after the book is reopened |
| +// each clear nothing; the grant for the current restriction clears it. |
| func TestRestartLegRetryGrantsAgainstTheBook(t *testing.T) { |
| ctx, s := genKit(t) |
| dd, bookDir := t.TempDir(), t.TempDir() |
| @@ -702,8 +705,8 @@ func TestRestartLegRetryGrantsAgainstTheBook(t *testing.T) { |
| } |
| mem, priv := retryMemory(t) |
| var cleared int |
| - mem.retryHost = func(h, b string, e uint64) bool { |
| - rec, mark, _, _ := book.RetryHostAt("bab", "eleven", b, h, e) |
| + mem.retryHost = func(h, b, bk string, e uint64) bool { |
| + rec, mark, _, _ := book.RetryHostAt("bab", "eleven", b, h, bk, e) |
| if rec || mark { |
| cleared++ |
| } |
| @@ -715,7 +718,7 @@ func TestRestartLegRetryGrantsAgainstTheBook(t *testing.T) { |
| book.Record(rollout.FailureRecord{Account: "bab", App: "eleven", Build: "v2", Host: host}) |
| first := book.RestrictionEpoch("bab", "eleven", "v2", host) |
| - stale := askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", first) |
| + stale := askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", book.ID(), first) |
| // The same build fails again on the same host before the box reads it. |
| book.Record(rollout.FailureRecord{Account: "bab", App: "eleven", Build: "v2", Host: host, HeldDown: []string{host}}) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| @@ -724,13 +727,23 @@ func TestRestartLegRetryGrantsAgainstTheBook(t *testing.T) { |
| } |
| now := book.RestrictionEpoch("bab", "eleven", "v2", host) |
| - askRetry(t, ctx, s, priv, "bab", "eleven", host, "v9", now) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", host, "v9", book.ID(), now) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| if cleared != 0 || !held() { |
| t.Fatal("a grant for another build cleared v2's hold") |
| } |
| - used := askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", now) |
| + // Final review round 1, finding 3: the current epoch signed for another |
| + // book (another box's, or this host's before a relocation) clears |
| + // nothing, and nor does one that names no book (an older console). |
| + askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", "another-book", now) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", "", now) |
| + restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| + if cleared != 0 || !held() { |
| + t.Fatal("a grant at the current epoch for another book, or for none, cleared v2's hold") |
| + } |
| + |
| + used := askRetry(t, ctx, s, priv, "bab", "eleven", host, "v2", book.ID(), now) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| if cleared != 1 || held() { |
| t.Fatalf("the current grant: %d clears, held %v; want one clear", cleared, held()) |
| @@ -819,10 +832,10 @@ func TestRestartLegSeparatesARetryFromARestartInOneWindow(t *testing.T) { |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven"), execRoute("b.example.com", "bab", "eleven")} |
| mem, priv := retryMemory(t) |
| var retried, bounced []string |
| - mem.retryHost = func(h, _ string, _ uint64) bool { retried = append(retried, h); return true } |
| + mem.retryHost = func(h, _, _ string, _ uint64) bool { retried = append(retried, h); return true } |
| bounce := func(h string) bool { bounced = append(bounced, h); return true } |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) // baseline |
| - askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", 5) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", "bk", 5) |
| askScoped(t, ctx, s, "bab", "eleven", release.RestartScopeHost, "b.example.com") |
| if asked := restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem); !asked { |
| t.Fatal("the restart in the window did not ask for a root pass") |
| @@ -839,10 +852,10 @@ func TestRestartLegLogsARetryWithNothingHeld(t *testing.T) { |
| dd := t.TempDir() |
| routes := []registry.Route{execRoute("a.example.com", "bab", "eleven")} |
| mem, priv := retryMemory(t) |
| - mem.retryHost = func(string, string, uint64) bool { return false } |
| + mem.retryHost = func(string, string, string, uint64) bool { return false } |
| bounce := func(h string) bool { t.Errorf("%s was bounced", h); return true } |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| - askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", 5) |
| + askRetry(t, ctx, s, priv, "bab", "eleven", "a.example.com", "v2", "bk", 5) |
| var logbuf bytes.Buffer |
| log.SetOutput(&logbuf) |
| defer log.SetOutput(os.Stderr) |
diff --git a/internal/host/rollout.go b/internal/host/rollout.go| index 634eccd3d..126b668cf 100644 |
| --- a/internal/host/rollout.go |
| +++ b/internal/host/rollout.go |
| @@ -336,15 +336,21 @@ func armRollout(ctx context.Context, getenv func(string) string, svc *activator. |
| if n := book.Reconciled(); n > 0 { |
| // Executions an earlier agent began and never saw recorded (Task 5, |
| // spec §8.4): their intents are failure records now, saved before |
| - // anything runs. A box that cannot save them must not run either: |
| - // its next restart would find the intents again, and nothing it |
| - // recorded meanwhile would be kept. |
| + // anything runs (the Flush below). A box that cannot save them must |
| + // not run either: its next restart would find the intents again, and |
| + // nothing it recorded meanwhile would be kept. |
| log.Printf("agent: %s=overlap: %d execution(s) an earlier agent left unrecorded are now failure records", rollout.SwapEnv, n) |
| - if err := book.Flush(); err != nil { |
| - log.Printf("agent: %s=overlap: save the failure book after reconciling it: %v", rollout.SwapEnv, err) |
| - refuseManaged(svc, rolloutBookUnreadable) |
| - return nil |
| - } |
| + } |
| + // Every open owes a save before anything runs (final review round 1, |
| + // finding 3): the open moved the book's sequence past every epoch an |
| + // earlier run may have handed out unsaved, and gave a new book its |
| + // identity. Unsaved, a crash would hand those epochs out again, so a |
| + // grant signed for one restriction could clear another; a box that |
| + // cannot save refuses managed starts, as it does for reconciliation. |
| + if err := book.Flush(); err != nil { |
| + log.Printf("agent: %s=overlap: save the failure book after opening it: %v", rollout.SwapEnv, err) |
| + refuseManaged(svc, rolloutBookUnreadable) |
| + return nil |
| } |
| platform := runtime.GOOS + "-" + runtime.GOARCH |
| m := rollout.NewManager(ctx, rollout.Deps{ |
diff --git a/internal/host/rollout_test.go b/internal/host/rollout_test.go| index 1157f55b2..d65507bc0 100644 |
| --- a/internal/host/rollout_test.go |
| +++ b/internal/host/rollout_test.go |
| @@ -3,6 +3,7 @@ package host |
| import ( |
| "bytes" |
| "context" |
| + "encoding/json" |
| "errors" |
| "log" |
| "os" |
| @@ -380,6 +381,52 @@ func TestAnArmedBoxThatCannotSaveItsReconciledBookRefusesManagedStarts(t *testin |
| } |
| } |
| +// Final review round 1, finding 3: every open of the book owes a save before |
| +// the scheduler arms, intents or none. The open moves the book's sequence |
| +// past every epoch an earlier run may have handed out unsaved; a box that |
| +// cannot save that could hand the same epochs out again after a crash, so |
| +// it refuses managed starts, as for an unsaved reconciliation. A box that |
| +// can save it arms, with the advance and the book's identity on disk. |
| +func TestAnArmedBoxSavesItsBookOnOpenOrRefusesManagedStarts(t *testing.T) { |
| + if os.Geteuid() == 0 { |
| + t.Skip("running as root: a read-only directory does not fail a save") |
| + } |
| + _, pub, err := release.GenerateKey() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + store := func(context.Context) (release.Store, error) { return release.NewDirStore(t.TempDir()), nil } |
| + |
| + svc, reg, dd := rolloutFixture(t, true) |
| + if err := os.Chmod(dd, 0o500); err != nil { |
| + t.Fatal(err) |
| + } |
| + locked := dd |
| + t.Cleanup(func() { os.Chmod(locked, 0o700) }) |
| + if m := armRollout(context.Background(), overlapEnv(pub), svc, reg, dd, nil, store); m != nil || armedRollout() != nil { |
| + t.Fatal("armed over a book whose open it could not save") |
| + } |
| + if rolloutUnavailable() != rolloutBookUnreadable { |
| + t.Fatalf("unavailable %q, want %q", rolloutUnavailable(), rolloutBookUnreadable) |
| + } |
| + |
| + svc, reg, dd = rolloutFixture(t, true) |
| + if m := armRollout(context.Background(), overlapEnv(pub), svc, reg, dd, nil, store); m == nil { |
| + t.Fatal("did not arm over a writable data dir") |
| + } |
| + raw, err := os.ReadFile(filepath.Join(dd, rollout.FailureFile)) |
| + if err != nil { |
| + t.Fatalf("the open's advance was not saved before arming: %v", err) |
| + } |
| + var f struct { |
| + Seq uint64 `json:"seq"` |
| + Book string `json:"book"` |
| + } |
| + if err := json.Unmarshal(raw, &f); err != nil || f.Seq == 0 || len(f.Book) != 32 { |
| + t.Fatalf("saved book %s (%v): want the advanced sequence and a 16-byte identity", raw, err) |
| + } |
| +} |
| + |
| // The unreadable-book refusal is one approved text, shared with the |
| // scheduler's refusal of a wake whose intent it could not write (Task 5). |
| func TestTheUnreadableBookRefusalIsShared(t *testing.T) { |
diff --git a/internal/host/rolloutdeclined.go b/internal/host/rolloutdeclined.go| index 3d10ebe55..8933705e5 100644 |
| --- a/internal/host/rolloutdeclined.go |
| +++ b/internal/host/rolloutdeclined.go |
| @@ -100,6 +100,14 @@ func honourDeclinedFailures(svc *activator.Service, reg *registry.Registry, dd, |
| if recs == 0 && marks == 0 { |
| return |
| } |
| + // The open moved the book's sequence past every epoch an armed run may |
| + // have handed out unsaved (final review round 1, finding 3). A declined |
| + // box does not save that here, unlike an armed one: it never writes its |
| + // book except to apply a retry or a pointer (bookSnapshot in its |
| + // tests), and the values those take are clears, never a restriction's |
| + // epoch that status publishes; the only restrictions it makes are |
| + // reconciliation's, saved above. Its first such save carries the |
| + // advance. |
| h := &failureHolds{book: book, get: reg.Get, list: reg.List, optDir: optDir} |
| declinedHolds.Store(h) |
| svc.Planner = h.plan |
| @@ -246,7 +254,8 @@ func (h *failureHolds) report() map[string]status.HostRollout { |
| out = map[string]status.HostRollout{} |
| } |
| out[rt.Host] = status.HostRollout{Account: acct, App: rt.App, Installed: rt.Version, Good: rt.GoodVersion, |
| - Attempt: attempt, AttemptBuild: hd.by, HeldBy: hd.by, HeldEpoch: h.book.RestrictionEpoch(acct, rt.App, hd.by, rt.Host)} |
| + Attempt: attempt, AttemptBuild: hd.by, HeldBy: hd.by, HeldEpoch: h.book.RestrictionEpoch(acct, rt.App, hd.by, rt.Host), |
| + HeldBook: h.book.ID()} // the book the epoch counts in (final review round 1, finding 3) |
| } |
| return out |
| } |
| @@ -254,23 +263,26 @@ func (h *failureHolds) report() map[string]status.HostRollout { |
| // retryHost is restartMemory.retryHost on a declined box (Task 10, ruling |
| // 194): a `carlos instances retry` whose root-signed grant the restart leg |
| // has verified, applied to this box's book exactly as the armed box applies |
| -// it (RetryHostAt): while build's restriction of host is still at epoch, it |
| +// it (RetryHostAt): while build's restriction of host is still at epoch, in |
| +// this box's book (book, final review round 1, finding 3), it |
| // clears build's record for host and host's marker when the marker names |
| // build, and saves. A grant for another build, or from before the |
| // restriction last changed, clears nothing. There is no scheduler to run a |
| // canary: the next wake, or the converge leg, starts the build as today. |
| -func (h *failureHolds) retryHost(host, build string, epoch uint64) bool { |
| +func (h *failureHolds) retryHost(host, build, book string, epoch uint64) bool { |
| rt, err := h.get(host) |
| if err != nil || rt == nil || !rollout.Managed(*rt) || build == "" { |
| return false |
| } |
| acct := release.RouteAccount(*rt) |
| - rec, mark, now, err := h.book.RetryHostAt(acct, rt.App, build, host, epoch) |
| + rec, mark, now, err := h.book.RetryHostAt(acct, rt.App, build, host, book, epoch) |
| if err != nil { |
| log.Printf("agent: retry %s on %s: save the failure book: %v (cleared in memory)", build, host, err) |
| } |
| if !rec && !mark && now != epoch { |
| log.Printf("agent: retry %s on %s: the grant is for epoch %d, the restriction's is %d, so nothing was cleared", build, host, epoch, now) |
| + } else if !rec && !mark && book != h.book.ID() { |
| + log.Printf("agent: retry %s on %s: the grant is for book %q, not this box's, so nothing was cleared", build, host, book) |
| } |
| if rec || mark { |
| nudgeStatus() |
| @@ -310,7 +322,7 @@ func (h *failureHolds) pointerLoop(ctx context.Context, read rollout.PointerRead |
| // rolloutRetryHost is restartMemory.retryHost for this box: the armed |
| // manager's, a declined box's that honours its book (failureHolds.retryHost, |
| // Task 10), or nil, which drops every retry because nothing is held. |
| -func rolloutRetryHost() func(host, build string, epoch uint64) bool { |
| +func rolloutRetryHost() func(host, build, book string, epoch uint64) bool { |
| if m := armedRollout(); m != nil { |
| return m.RetryHost |
| } |
diff --git a/internal/host/rolloutdeclined_test.go b/internal/host/rolloutdeclined_test.go| index b78156fe4..dfe790875 100644 |
| --- a/internal/host/rolloutdeclined_test.go |
| +++ b/internal/host/rolloutdeclined_test.go |
| @@ -870,8 +870,9 @@ func TestDeclinedBoxProcessesAVerifiedSignedRetry(t *testing.T) { |
| t.Fatalf("plan before the retry = %+v, want %q", p, held) |
| } |
| hr, ok := declinedHostReport(t, host) |
| - if !ok || hr.Account != acct || hr.App != "blog" || hr.Installed != "v2" || hr.HeldBy != "v2" || hr.HeldEpoch != epoch || hr.Attempt != status.AttemptDown { |
| - t.Fatalf("published %+v (%v), want v2 holding %s down at epoch %d", hr, ok, host, epoch) |
| + if !ok || hr.Account != acct || hr.App != "blog" || hr.Installed != "v2" || hr.HeldBy != "v2" || hr.HeldEpoch != epoch || hr.Attempt != status.AttemptDown || |
| + hr.HeldBook == "" || hr.HeldBook != heldHolds().book.ID() { |
| + t.Fatalf("published %+v (%v), want v2 holding %s down at epoch %d of the box's book", hr, ok, host, epoch) |
| } |
| mem, priv := retryMemory(t) |
| @@ -886,13 +887,13 @@ func TestDeclinedBoxProcessesAVerifiedSignedRetry(t *testing.T) { |
| } |
| bounce := func(h string) bool { t.Errorf("%s was bounced for a retry", h); return true } |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) // baseline |
| - askRetry(t, ctx, s, priv, acct, "blog", host, "v9", epoch) |
| - askRetry(t, ctx, s, priv, acct, "blog", host, "v2", stale) |
| + askRetry(t, ctx, s, priv, acct, "blog", host, "v9", hr.HeldBook, epoch) |
| + askRetry(t, ctx, s, priv, acct, "blog", host, "v2", hr.HeldBook, stale) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| if p := svc.Planner(ctx, rt); p.Refuse != held { |
| t.Fatalf("plan after a grant for another build and a stale grant = %+v, want %q still", p, held) |
| } |
| - askRetry(t, ctx, s, priv, acct, "blog", host, "v2", epoch) |
| + askRetry(t, ctx, s, priv, acct, "blog", host, "v2", hr.HeldBook, epoch) |
| restartRequestLeg(ctx, routes, s, dd, bounce, nil, mem) |
| if p := svc.Planner(ctx, rt); !isTodaysWake(p) { |
| t.Fatalf("plan after the signed retry = %+v, want today's wake onto v2", p) |
diff --git a/internal/release/hostretry.go b/internal/release/hostretry.go| index dd3abf188..9ac7cc634 100644 |
| --- a/internal/release/hostretry.go |
| +++ b/internal/release/hostretry.go |
| @@ -31,7 +31,13 @@ type HostRetryGrant struct { |
| // Epoch is the restriction's epoch the console read from the box's |
| // report. Zero is an older box that reported none: the console still |
| // signs it, and no box accepts it. |
| - Epoch uint64 `json:"epoch"` |
| + Epoch uint64 `json:"epoch"` |
| + // Book is the identity of the failure book Epoch counts in, as the box |
| + // reported it (status.HostRollout.HeldBook): epochs are a counter every |
| + // book starts alike, so a grant binds the restriction only with both |
| + // (final review round 1, finding 3). Empty is a console or a box older |
| + // than the identity: the console still signs it, and no box accepts it. |
| + Book string `json:"book"` |
| IssuedAt string `json:"issued_at"` // RFC 3339, UTC, whole seconds |
| } |
| @@ -48,8 +54,8 @@ var ErrHostRetryGrant = errors.New("host retry grant: not valid for this request |
| // SignHostRetry signs a grant with the deployment root. A scoped release |
| // key is refused (SignRootEnvelope): a channel delegate has no say over |
| // another instance's failure hold. |
| -func SignHostRetry(sg Signer, acct, app, host, build string, epoch uint64, now time.Time) (string, error) { |
| - g := HostRetryGrant{V: 1, Account: acct, App: app, Host: host, Build: build, Epoch: epoch, |
| +func SignHostRetry(sg Signer, acct, app, host, build, book string, epoch uint64, now time.Time) (string, error) { |
| + g := HostRetryGrant{V: 1, Account: acct, App: app, Host: host, Build: build, Epoch: epoch, Book: book, |
| IssuedAt: now.UTC().Truncate(time.Second).Format(time.RFC3339)} |
| wire, err := SignRootEnvelope(sg, g) |
| return string(wire), err |
| @@ -58,8 +64,8 @@ func SignHostRetry(sg Signer, acct, app, host, build string, epoch uint64, now t |
| // VerifyHostRetry checks wire against the root key and the entry it rode in: |
| // the same account, app and host, a build and a non-zero epoch, issued no |
| // later than now plus a minute of skew and no earlier than now minus |
| -// HostRetryGrantTTL. Whether the epoch is still the restriction's is the |
| -// failure book's check, made where the clear is. |
| +// HostRetryGrantTTL. Whether the epoch is still the restriction's, and the |
| +// book the box's, is the failure book's check, made where the clear is. |
| func VerifyHostRetry(pub ed25519.PublicKey, wire, acct, app, host string, now time.Time) (HostRetryGrant, error) { |
| var g HostRetryGrant |
| if err := VerifyRootEnvelope(pub, []byte(wire), &g); err != nil { |
diff --git a/internal/release/hostretry_test.go b/internal/release/hostretry_test.go| index a07c5c699..55fa6e846 100644 |
| --- a/internal/release/hostretry_test.go |
| +++ b/internal/release/hostretry_test.go |
| @@ -22,7 +22,7 @@ func hostRetryKey(t *testing.T) (ed25519.PublicKey, ed25519.PrivateKey) { |
| func TestHostRetryGrantRoundTrip(t *testing.T) { |
| pub, priv := hostRetryKey(t) |
| now := time.Date(2026, 10, 11, 9, 30, 15, 500, time.UTC) |
| - wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", 42, now) |
| + wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", "book-1", 42, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -30,7 +30,7 @@ func TestHostRetryGrantRoundTrip(t *testing.T) { |
| if err != nil { |
| t.Fatalf("verify = %v", err) |
| } |
| - want := HostRetryGrant{V: 1, Account: "bab", App: "blog", Host: "a.example.com", Build: "v2", Epoch: 42, IssuedAt: "2026-10-11T09:30:15Z"} |
| + want := HostRetryGrant{V: 1, Account: "bab", App: "blog", Host: "a.example.com", Build: "v2", Epoch: 42, Book: "book-1", IssuedAt: "2026-10-11T09:30:15Z"} |
| if g != want { |
| t.Fatalf("grant = %+v, want %+v", g, want) |
| } |
| @@ -47,7 +47,7 @@ func TestHostRetryGrantRefusals(t *testing.T) { |
| pub, priv := hostRetryKey(t) |
| otherPub, _ := hostRetryKey(t) |
| now := time.Date(2026, 10, 11, 9, 30, 0, 0, time.UTC) |
| - wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", 7, now) |
| + wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", "book-1", 7, now) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -68,6 +68,7 @@ func TestHostRetryGrantRefusals(t *testing.T) { |
| {"empty", pub, "", "bab", "blog", "a.example.com", now}, |
| {"tampered payload", pub, tamperHostRetry(t, wire, func(g map[string]any) { g["build"] = "v3" }), "bab", "blog", "a.example.com", now}, |
| {"tampered epoch", pub, tamperHostRetry(t, wire, func(g map[string]any) { g["epoch"] = 8 }), "bab", "blog", "a.example.com", now}, |
| + {"tampered book", pub, tamperHostRetry(t, wire, func(g map[string]any) { g["book"] = "book-2" }), "bab", "blog", "a.example.com", now}, |
| } |
| for _, c := range cases { |
| if g, err := VerifyHostRetry(c.pub, c.wire, c.acct, c.app, c.host, c.at); err == nil { |
| @@ -85,14 +86,14 @@ func TestHostRetryGrantRefusals(t *testing.T) { |
| func TestHostRetryGrantRefusesAZeroEpoch(t *testing.T) { |
| pub, priv := hostRetryKey(t) |
| now := time.Now() |
| - wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", 0, now) |
| + wire, err := SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "v2", "book-1", 0, now) |
| if err != nil { |
| t.Fatalf("the console signs a zero epoch: %v", err) |
| } |
| if _, err := VerifyHostRetry(pub, wire, "bab", "blog", "a.example.com", now); !errors.Is(err, ErrHostRetryGrant) { |
| t.Fatalf("zero epoch = %v, want ErrHostRetryGrant", err) |
| } |
| - wire, _ = SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "", 3, now) |
| + wire, _ = SignHostRetry(RootSigner(priv), "bab", "blog", "a.example.com", "", "book-1", 3, now) |
| if _, err := VerifyHostRetry(pub, wire, "bab", "blog", "a.example.com", now); err == nil { |
| t.Fatal("a grant naming no build verified") |
| } |
| @@ -101,10 +102,10 @@ func TestHostRetryGrantRefusesAZeroEpoch(t *testing.T) { |
| func TestHostRetryGrantRefusesAScopedSigner(t *testing.T) { |
| pub, priv := hostRetryKey(t) |
| scoped := Signer{key: priv, scope: &Grant{Name: "ci"}, grant: &signedGrant{}, root: pub} |
| - if _, err := SignHostRetry(scoped, "bab", "blog", "a.example.com", "v2", 1, time.Now()); !errors.Is(err, ErrRootEnvelope) { |
| + if _, err := SignHostRetry(scoped, "bab", "blog", "a.example.com", "v2", "book-1", 1, time.Now()); !errors.Is(err, ErrRootEnvelope) { |
| t.Fatalf("scoped signer = %v, want ErrRootEnvelope", err) |
| } |
| - if _, err := SignHostRetry(Signer{}, "bab", "blog", "a.example.com", "v2", 1, time.Now()); err == nil { |
| + if _, err := SignHostRetry(Signer{}, "bab", "blog", "a.example.com", "v2", "book-1", 1, time.Now()); err == nil { |
| t.Fatal("a signer with no key signed a retry") |
| } |
| } |
diff --git a/internal/rollout/failures.go b/internal/rollout/failures.go| index 35a3d5b0e..15fc421ff 100644 |
| --- a/internal/rollout/failures.go |
| +++ b/internal/rollout/failures.go |
| @@ -1,6 +1,8 @@ |
| package rollout |
| import ( |
| + "crypto/rand" |
| + "encoding/hex" |
| "encoding/json" |
| "errors" |
| "fmt" |
| @@ -219,6 +221,30 @@ type failureFile struct { |
| Seq uint64 `json:"seq,omitempty"` |
| // Intents are the executions in flight by host (Intent), schema 2. |
| Intents map[string]Intent `json:"intents,omitempty"` |
| + // Book is the book's identity (FailureBook.id), additive like HoldRan: |
| + // 16 random bytes, hex, made when the book is first created or first |
| + // opened without one, and kept for its life. |
| + Book string `json:"book"` |
| +} |
| + |
| +// reopenAdvance is how far OpenFailureBook moves the sequence past the one |
| +// on disk (final review round 1, finding 3): epochs handed out after the |
| +// last save that landed live only in memory, and status may have published |
| +// them, so a reopen must not hand the same values out again. The caller |
| +// saves the advance before it runs anything (Flush); an armed box whose save |
| +// fails refuses managed starts. A run would need over a million book events |
| +// between two saves that land to reach it. |
| +const reopenAdvance = 1 << 20 |
| + |
| +// newBookID is a book's identity (FailureBook.id). crypto/rand does not fail |
| +// on the platforms the agent runs on; if it ever did, the clock's |
| +// nanoseconds still tell two books apart. |
| +func newBookID() string { |
| + var b [16]byte |
| + if _, err := rand.Read(b[:]); err != nil { |
| + return fmt.Sprintf("t%x", time.Now().UnixNano()) |
| + } |
| + return hex.EncodeToString(b[:]) |
| } |
| // FailureBook is the box's failure records. An empty path keeps them in |
| @@ -266,6 +292,29 @@ type FailureBook struct { |
| // survive an agent restart, so neither need these. Prune drops the |
| // entries of builds no route installs, with their records. |
| cleared map[[4]string]uint64 |
| + // id is the book's identity, saved with it (failureFile.Book). Status |
| + // publishes it beside every epoch (HostRollout.HeldBook), the console |
| + // signs it into its retry grant, and RetryHostAt clears only for a grant |
| + // naming it (final review round 1, finding 3, Astra): epochs are a |
| + // counter every book starts alike, so two books (a host relocated, a box |
| + // rebuilt) can give one value to two restrictions, and without the |
| + // identity a grant for one would clear the other. A memory-only book |
| + // makes one on first use (idLocked). |
| + id string |
| +} |
| + |
| +// ID is the book's identity (FailureBook.id). |
| +func (b *FailureBook) ID() string { |
| + b.mu.Lock() |
| + defer b.mu.Unlock() |
| + return b.idLocked() |
| +} |
| + |
| +func (b *FailureBook) idLocked() string { |
| + if b.id == "" { |
| + b.id = newBookID() |
| + } |
| + return b.id |
| } |
| func (b *FailureBook) clock() time.Time { |
| @@ -289,10 +338,18 @@ func cloneRec(r FailureRecord) FailureRecord { |
| // held instance straight back onto the build that failed. Intents left in |
| // the file become failure records (reconcileLocked, Reconciled), which the |
| // caller saves with Flush. |
| +// |
| +// Every open moves the sequence reopenAdvance past the file's (final review |
| +// round 1, finding 3): an epoch handed out after the last save that landed |
| +// was never written, and must not be handed out again for another |
| +// restriction. A book with no identity on disk (a new one, or a file an |
| +// earlier agent wrote) gets one. Both are owed a save (dirty), which the |
| +// caller makes with Flush before it runs anything. |
| func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| b := &FailureBook{path: filepath.Join(dataDir, FailureFile)} |
| raw, err := os.ReadFile(b.path) |
| if errors.Is(err, os.ErrNotExist) { |
| + b.id, b.seq, b.dirty = newBookID(), reopenAdvance, true |
| return b, nil |
| } |
| if err != nil { |
| @@ -310,6 +367,10 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| b.held = f.HoldRan |
| b.seq = f.Seq |
| b.intents = f.Intents |
| + b.id = f.Book |
| + if b.id == "" { |
| + b.id = newBookID() |
| + } |
| for _, h := range b.held { |
| // A marker's sequence is saved with the book's, so this changes |
| // nothing in a file this book wrote; it keeps a hand-edited one from |
| @@ -321,6 +382,8 @@ func OpenFailureBook(dataDir string) (*FailureBook, error) { |
| b.seq = max(b.seq, e) |
| } |
| } |
| + b.seq += reopenAdvance |
| + b.dirty = true |
| b.mu.Lock() |
| b.epochOlderLocked() |
| b.reconcileLocked() |
| @@ -481,7 +544,7 @@ func (b *FailureBook) saveLocked() error { |
| if b.path == "" { |
| return nil |
| } |
| - raw, err := json.MarshalIndent(failureFile{Schema: failureSchema, Records: b.recs, HoldRan: b.held, Seq: b.seq, Intents: b.intents}, "", " ") |
| + raw, err := json.MarshalIndent(failureFile{Schema: failureSchema, Records: b.recs, HoldRan: b.held, Seq: b.seq, Intents: b.intents, Book: b.idLocked()}, "", " ") |
| if err == nil { |
| err = release.WriteFileAtomic(b.path, raw, 0o644) |
| } |
| @@ -859,18 +922,21 @@ func (b *FailureBook) restrictionEpochLocked(acct, app, build, host string) uint |
| } |
| // RetryHostAt is a signed `carlos instances retry <host>` grant (spec §8.4, |
| -// §11; HostRetryGrant) applied to the book: when epoch is build's |
| -// restriction of host now (RestrictionEpoch), it clears build's record for |
| +// §11; HostRetryGrant) applied to the book: when the grant names this book |
| +// (book, the book's ID) and epoch is build's restriction of host now |
| +// (RestrictionEpoch), it clears build's record for |
| // host, when that record holds host (RetryHost), and host's hold-ran |
| // marker, when the marker names build (plan review finding 5). A grant from |
| // before the restriction last changed, a zero epoch, or one for a build that |
| -// restricts nothing clears nothing. The check and the clears are one hold of |
| +// restricts nothing clears nothing. Nor does a grant for another book, or |
| +// one that names no book (a console or a box from before the identity): |
| +// its epoch could be another book's (final review round 1, finding 3). The check and the clears are one hold of |
| // the lock. now is the epoch at the check, for the caller's log. |
| -func (b *FailureBook) RetryHostAt(acct, app, build, host string, epoch uint64) (rec, mark bool, now uint64, err error) { |
| +func (b *FailureBook) RetryHostAt(acct, app, build, host, book string, epoch uint64) (rec, mark bool, now uint64, err error) { |
| b.mu.Lock() |
| defer b.mu.Unlock() |
| now = b.restrictionEpochLocked(acct, app, build, host) |
| - if epoch == 0 || now != epoch { |
| + if book == "" || book != b.idLocked() || epoch == 0 || now != epoch { |
| return false, false, now, b.retryLocked() |
| } |
| if m, ok := b.held[host]; ok && m.Account == acct && m.App == app && m.Build == build { |
diff --git a/internal/rollout/finalreview1_test.go b/internal/rollout/finalreview1_test.go| index f28b27b00..38319c089 100644 |
| --- a/internal/rollout/finalreview1_test.go |
| +++ b/internal/rollout/finalreview1_test.go |
| @@ -94,7 +94,7 @@ func TestAnExecutionAfterARestartIsANewExecution(t *testing.T) { |
| // The first execution's grant and a rollback signed between the two runs |
| // clear nothing of the second's marker. |
| - if rec, mark, _, _ := h.book.RetryHostAt("bab", "blog", "v2", "s", staleGrant); rec || mark { |
| + if rec, mark, _, _ := h.book.RetryHostAt("bab", "blog", "v2", "s", h.book.ID(), staleGrant); rec || mark { |
| t.Fatal("a retry grant signed for the first execution cleared the second's marker") |
| } |
| if ok, _ := h.book.ClearHoldRanOnPointer("s", "bab", "blog", t0.Add(30*time.Second)); ok { |
| @@ -126,3 +126,84 @@ func TestExecutionIdentitiesCarryTheManagersIncarnation(t *testing.T) { |
| t.Fatalf("the same manager handed out %q twice", a1) |
| } |
| } |
| + |
| +// grant is a console's host retry grant as the box's status lets it sign |
| +// one: the restriction's book and epoch. |
| +type grant struct { |
| + book string |
| + epoch uint64 |
| +} |
| + |
| +func grantFor(b *FailureBook, build, host string) grant { |
| + return grant{book: b.ID(), epoch: b.RestrictionEpoch("bab", "blog", build, host)} |
| +} |
| + |
| +func applyGrant(b *FailureBook, build, host string, g grant) (rec, mark bool) { |
| + rec, mark, _, _ = b.RetryHostAt("bab", "blog", build, host, g.book, g.epoch) |
| + return rec, mark |
| +} |
| + |
| +// Final review round 1, finding 3 (Astra): two independent books (a host |
| +// relocated, or a box rebuilt) count from the same place, so the same |
| +// numeric epoch names different restrictions in each. A grant signed for |
| +// one book's restriction clears nothing in the other's. |
| +func TestARetryGrantForOneBookClearsNothingInAnother(t *testing.T) { |
| + one, other := &FailureBook{}, &FailureBook{} |
| + for _, b := range []*FailureBook{one, other} { |
| + b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Mode: string(activator.ModeHold), |
| + HeldDown: []string{"h"}, MarkHoldRan: true, HoldRanExec: "a1", HoldRanAt: t0}) |
| + } |
| + g := grantFor(one, "v2", "h") |
| + if g.epoch == 0 || g.epoch != other.RestrictionEpoch("bab", "blog", "v2", "h") { |
| + t.Fatalf("the two books' epochs differ (%d, %d): the case needs equal counters", g.epoch, other.RestrictionEpoch("bab", "blog", "v2", "h")) |
| + } |
| + if rec, mark := applyGrant(other, "v2", "h", g); rec || mark { |
| + t.Fatalf("a grant for one book cleared record %v, marker %v in another", rec, mark) |
| + } |
| + if !other.HeldDown("bab", "blog", "v2", "h") { |
| + t.Fatal("the other book's hold-down was lifted") |
| + } |
| + if rec, mark := applyGrant(one, "v2", "h", g); !rec || !mark { |
| + t.Fatalf("the grant cleared record %v, marker %v in its own book; want both", rec, mark) |
| + } |
| +} |
| + |
| +// Finding 3's failed-save half: epochs handed out after a save failed live |
| +// only in memory, and status may already have published them. An agent |
| +// crash then a reopen must never hand the same values out again for |
| +// another restriction, or the grant the console signed for the first |
| +// would clear the second. |
| +func TestAReopenAfterUnsavedEpochsNeverReissuesThem(t *testing.T) { |
| + dd := t.TempDir() |
| + b, err := OpenFailureBook(dd) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := b.Flush(); err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h"}); err != nil { |
| + t.Fatal(err) |
| + } |
| + readOnly(t, dd) |
| + if _, err := b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h", HeldDown: []string{"h"}}); err == nil { |
| + t.Skip("the save did not fail") |
| + } |
| + published := grantFor(b, "v3", "h") // status publishes it; the console signs it |
| + writable(t, dd) |
| + |
| + b2, err := OpenFailureBook(dd) // the agent crashed: v3's record never reached the disk |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := b2.Flush(); err != nil { |
| + t.Fatal(err) |
| + } |
| + b2.Record(FailureRecord{Account: "bab", App: "blog", Build: "v3", Host: "h", HeldDown: []string{"h"}}) |
| + if e := b2.RestrictionEpoch("bab", "blog", "v3", "h"); e <= published.epoch { |
| + t.Fatalf("the reopened book handed out epoch %d, not past the unsaved %d", e, published.epoch) |
| + } |
| + if rec, mark := applyGrant(b2, "v3", "h", published); rec || mark { |
| + t.Fatal("the grant for the unsaved restriction cleared the new one") |
| + } |
| +} |
diff --git a/internal/rollout/manager.go b/internal/rollout/manager.go| index ebd926869..f53062248 100644 |
| --- a/internal/rollout/manager.go |
| +++ b/internal/rollout/manager.go |
| @@ -802,12 +802,14 @@ func (m *Manager) retryBuild(k appKey, build, host string) { |
| // RetryHost is the box half of `carlos instances retry <host>` (spec §8.4, |
| // §11): a grant the console signed (release.HostRetryGrant), which the |
| // caller has verified, naming the build whose restriction of host it lifts |
| -// and that restriction's epoch as the box last reported it (plan review |
| -// findings 5 and 13). The book clears build's record for host, when one |
| -// holds it, and host's hold-ran marker, when the marker names build, and |
| -// only while the restriction's epoch is still epoch (RetryHostAt): a grant |
| -// from before a later failure of the same build, or one replayed after it |
| -// was spent, clears nothing. True means either was cleared. |
| +// and that restriction's epoch and book as the box last reported them |
| +// (plan review findings 5 and 13; final review round 1, finding 3). The |
| +// book clears build's record for host, when one holds it, and host's |
| +// hold-ran marker, when the marker names build, and only while the |
| +// restriction's epoch is still epoch and the grant names this box's book |
| +// (RetryHostAt): a grant from before a later failure of the same build, one |
| +// replayed after it was spent, or one for another book's restriction at the |
| +// same epoch, clears nothing. True means either was cleared. |
| // |
| // When the record was cleared and host's slot wants build, its next attempt |
| // is a canary. The book entry is cleared and the host's retry flag bound in |
| @@ -822,7 +824,7 @@ func (m *Manager) retryBuild(k appKey, build, host string) { |
| // |
| // A grant naming another build than the slot's (the marker's build B while |
| // the slot wants A) clears B's restriction and leaves A's queue alone. |
| -func (m *Manager) RetryHost(host, build string, epoch uint64) bool { |
| +func (m *Manager) RetryHost(host, build, book string, epoch uint64) bool { |
| m.mu.Lock() |
| k, ok := m.hostApp[host] |
| m.mu.Unlock() |
| @@ -835,7 +837,7 @@ func (m *Manager) RetryHost(host, build string, epoch uint64) bool { |
| if h := s.q.hosts[host]; h != nil { |
| want = h.wantBuild |
| } |
| - cleared, unmarked, cur, err := m.d.Book.RetryHostAt(k.acct, k.app, build, host, epoch) |
| + cleared, unmarked, cur, err := m.d.Book.RetryHostAt(k.acct, k.app, build, host, book, epoch) |
| requeue := cleared && want == build |
| if requeue { |
| s.q.wantBuild(host, activator.CauseRetry, build, m.d.Now()) |
| @@ -847,6 +849,8 @@ func (m *Manager) RetryHost(host, build string, epoch uint64) bool { |
| } |
| if !cleared && !unmarked && cur != epoch { |
| m.d.Logf("rollout: retry %s on %s: the grant is for epoch %d, the restriction's is %d, so nothing was cleared", build, host, epoch, cur) |
| + } else if !cleared && !unmarked && book != m.d.Book.ID() { |
| + m.d.Logf("rollout: retry %s on %s: the grant is for book %q, not this box's, so nothing was cleared", build, host, book) |
| } |
| if requeue { |
| m.retryBuild(k, build, host) |
diff --git a/internal/rollout/retrygrant_test.go b/internal/rollout/retrygrant_test.go| index b2f833ef8..10f16dbef 100644 |
| --- a/internal/rollout/retrygrant_test.go |
| +++ b/internal/rollout/retrygrant_test.go |
| @@ -13,7 +13,7 @@ import ( |
| // retry is the console's signed host retry as the box applies it: a grant |
| // naming build, carrying the epoch the box's status publishes for it. |
| func (h *harness) retry(host, build string) bool { |
| - return h.m.RetryHost(host, build, h.book.RestrictionEpoch("bab", "blog", build, host)) |
| + return h.m.RetryHost(host, build, h.book.ID(), h.book.RestrictionEpoch("bab", "blog", build, host)) |
| } |
| // Plan review finding 13: a restriction's epoch is a persisted book |
| @@ -103,7 +103,7 @@ func TestARetryGrantForAnotherBuildClearsNothing(t *testing.T) { |
| h.install("v2", "a", "b") |
| h.pass() |
| epoch := h.book.RestrictionEpoch("bab", "blog", "v2", "b") |
| - if h.m.RetryHost("b", "v9", epoch) { |
| + if h.m.RetryHost("b", "v9", h.book.ID(), epoch) { |
| t.Fatal("a grant for v9 cleared something") |
| } |
| if !h.book.Blocks("bab", "blog", "v2", "b", false) { |
| @@ -112,7 +112,7 @@ func TestARetryGrantForAnotherBuildClearsNothing(t *testing.T) { |
| if _, ok := h.book.HoldRanOn("b"); !ok { |
| t.Fatal("a grant for v9 cleared b's v2 marker") |
| } |
| - if h.m.RetryHost("b", "v2", 0) { |
| + if h.m.RetryHost("b", "v2", h.book.ID(), 0) { |
| t.Fatal("a zero epoch cleared something") |
| } |
| if !h.retry("b", "v2") { |
| @@ -162,12 +162,12 @@ func TestARetryGrantPredatingASecondFailureClearsNothing(t *testing.T) { |
| h.pass() |
| waitFor(t, "a failure record", func() bool { _, ok := h.book.Lookup("bab", "blog", "v2"); return ok }) |
| first := h.book.RestrictionEpoch("bab", "blog", "v2", "b") |
| - if !h.m.RetryHost("b", "v2", first) { |
| + if !h.m.RetryHost("b", "v2", h.book.ID(), first) { |
| t.Fatal("the first grant cleared nothing") |
| } |
| // b's canary fails: the same build fails again on the same host. |
| waitFor(t, "b's canary to fail", func() bool { return !h.m.attemptRunning("b") && h.book.Blocks("bab", "blog", "v2", "b", false) }) |
| - if h.m.RetryHost("b", "v2", first) { |
| + if h.m.RetryHost("b", "v2", h.book.ID(), first) { |
| t.Fatal("a grant from before b's second failure cleared it") |
| } |
| if !h.book.Blocks("bab", "blog", "v2", "b", false) { |
| @@ -187,7 +187,7 @@ func TestARetryGrantPredatingAHoldDownClearsNothing(t *testing.T) { |
| b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h"}) |
| signed := b.RestrictionEpoch("bab", "blog", "v2", "h") |
| b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", HeldDown: []string{"h"}}) |
| - if rec, mark, _, _ := b.RetryHostAt("bab", "blog", "v2", "h", signed); rec || mark { |
| + if rec, mark, _, _ := b.RetryHostAt("bab", "blog", "v2", "h", b.ID(), signed); rec || mark { |
| t.Fatalf("a grant predating the hold-down cleared record %v, marker %v", rec, mark) |
| } |
| if !b.HeldDown("bab", "blog", "v2", "h") { |
| @@ -206,7 +206,7 @@ func TestARetryGrantReplayedAfterAReopenClearsNothing(t *testing.T) { |
| b.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Mode: string(activator.ModeHold), |
| HeldDown: []string{"h"}, MarkHoldRan: true, HoldRanExec: "a1", HoldRanAt: t0}) |
| grant := b.RestrictionEpoch("bab", "blog", "v2", "h") |
| - if rec, mark, _, err := b.RetryHostAt("bab", "blog", "v2", "h", grant); !rec || !mark || err != nil { |
| + if rec, mark, _, err := b.RetryHostAt("bab", "blog", "v2", "h", b.ID(), grant); !rec || !mark || err != nil { |
| t.Fatalf("the grant cleared record %v, marker %v (%v); want both", rec, mark, err) |
| } |
| b2, err := OpenFailureBook(dd) |
| @@ -216,7 +216,7 @@ func TestARetryGrantReplayedAfterAReopenClearsNothing(t *testing.T) { |
| // The retried canary fails again, after the reopen. |
| b2.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "h", Mode: string(activator.ModeHold), |
| HeldDown: []string{"h"}, MarkHoldRan: true, HoldRanExec: "a2", HoldRanAt: t0.Add(time.Minute), Seq: b2.Snapshot()}) |
| - if rec, mark, _, _ := b2.RetryHostAt("bab", "blog", "v2", "h", grant); rec || mark { |
| + if rec, mark, _, _ := b2.RetryHostAt("bab", "blog", "v2", "h", b.ID(), grant); rec || mark { |
| t.Fatalf("the replayed grant cleared record %v, marker %v", rec, mark) |
| } |
| if !b2.HeldDown("bab", "blog", "v2", "h") { |
| @@ -246,7 +246,10 @@ func TestStatusPublishesTheHeldEpoch(t *testing.T) { |
| if got := r.Hosts["b"]; got.HeldBy != "v4" || got.HeldEpoch == 0 || got.HeldEpoch != h.book.RestrictionEpoch("bab", "blog", "v4", "b") { |
| t.Fatalf("b = %+v: want held by the marker's v4 at its epoch", got) |
| } |
| - if !h.m.RetryHost("b", r.Hosts["b"].HeldBy, r.Hosts["b"].HeldEpoch) { |
| + if got := r.Hosts["b"]; got.HeldBook == "" || got.HeldBook != h.book.ID() || r.Hosts["a"].HeldBook != h.book.ID() { |
| + t.Fatalf("held books %q and %q: want the book's identity %q on both arms", r.Hosts["a"].HeldBook, got.HeldBook, h.book.ID()) |
| + } |
| + if !h.m.RetryHost("b", r.Hosts["b"].HeldBy, r.Hosts["b"].HeldBook, r.Hosts["b"].HeldEpoch) { |
| t.Fatal("a grant signed from the status report cleared nothing") |
| } |
| } |
diff --git a/internal/rollout/sim_test.go b/internal/rollout/sim_test.go| index 8c50c1490..06f29fdbb 100644 |
| --- a/internal/rollout/sim_test.go |
| +++ b/internal/rollout/sim_test.go |
| @@ -1510,7 +1510,7 @@ func (s *sim) hostRetry() { |
| clears := recorded && !slices.Contains(rec.Retried, h.name) |
| // The console's grant names the build and the epoch the box reports |
| // for it (Task 8); the sim reads the epoch just as the console would. |
| - ok := s.m.RetryHost(h.name, b, s.book.RestrictionEpoch("bab", "blog", b, h.name)) |
| + ok := s.m.RetryHost(h.name, b, s.book.ID(), s.book.RestrictionEpoch("bab", "blog", b, h.name)) |
| // The retry exempts h from the record that stands now, and from no |
| // record written after that one goes (pass): with no record there is |
| // nothing to exempt it from. |
diff --git a/internal/rollout/status.go b/internal/rollout/status.go| index 6fd9fbbcb..5cedf5a4c 100644 |
| --- a/internal/rollout/status.go |
| +++ b/internal/rollout/status.go |
| @@ -87,6 +87,7 @@ func (m *Manager) Status() status.RolloutReport { |
| // The epoch the console signs into its retry grant |
| // (plan review finding 13). |
| hr.HeldEpoch = m.d.Book.RestrictionEpoch(s.key.acct, s.key.app, rec.Build, host) |
| + hr.HeldBook = m.d.Book.ID() // final review round 1, finding 3 |
| if m.d.Book.HeldDown(s.key.acct, s.key.app, h.wantBuild, host) { |
| hr.Attempt = status.AttemptDown |
| } |
| @@ -107,6 +108,7 @@ func (m *Manager) Status() status.RolloutReport { |
| case hr.Attempt == "": |
| hr.HeldBy = mark.Build |
| hr.HeldEpoch = m.d.Book.RestrictionEpoch(s.key.acct, s.key.app, mark.Build, host) |
| + hr.HeldBook = m.d.Book.ID() |
| if rec, ok := m.d.Book.Lookup(s.key.acct, s.key.app, mark.Build); ok { |
| hr.Reason = reasonWord(rec.Reason) |
| } |
diff --git a/internal/status/snapshot.go b/internal/status/snapshot.go| index c0dc42b44..2ce054b4c 100644 |
| --- a/internal/status/snapshot.go |
| +++ b/internal/status/snapshot.go |
| @@ -297,6 +297,12 @@ type HostRollout struct { |
| // `carlos instances retry` grant, which the box honours only while the |
| // restriction is unchanged. Absent from an older box, which is 0. |
| HeldEpoch uint64 `json:"held_epoch,omitempty"` |
| + // HeldBook is the identity of the failure book HeldEpoch counts in |
| + // (rollout.FailureBook.ID): epochs are a counter every book starts |
| + // alike, so the console signs both into its grant and the box clears |
| + // only for its own book (final review round 1, finding 3). Absent from |
| + // an older box: a grant signed without it clears nothing. |
| + HeldBook string `json:"held_book,omitempty"` |
| // Transient is set when the activator owns the host's process but has no |
| // current generation for it: the process is starting or stopping, so the |
| // instance is neither asleep nor awake and no serving build is reported. |