amadan
zdd-5 task 10: a held build no longer refuses a wake for an instance with no good build
Spec §8.5: an instance with no good build has no fallback, which is today's behaviour. decideWake and planCheck now refuse such an instance only for a hold-down; a record alone plans the installed build as a first wake with no fallback.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+52
−8
internal/rollout/wakeplan.go+16 −6internal/rollout/wakeplan_test.go+36 −2
diff --git a/internal/rollout/wakeplan.go b/internal/rollout/wakeplan.go| index df6e0248d..c7f66911c 100644 |
| --- a/internal/rollout/wakeplan.go |
| +++ b/internal/rollout/wakeplan.go |
| @@ -33,7 +33,8 @@ type wakeInputs struct { |
| // this instance has never served is a first wake; in overlap mode it carries |
| // the good build as its fallback. A build that failed in hold mode on this |
| // instance refuses the wake: nothing falls back automatically once a hold |
| -// build has run against the database (spec §6.4, §8.3). An error is a |
| +// build has run against the database (spec §6.4, §8.3). An instance with no |
| +// good build is never refused by a record alone (spec §8.5). An error is a |
| // staging refusal for the caller to report. |
| func decideWake(in wakeInputs) (activator.WakePlan, error) { |
| rt := in.Route |
| @@ -54,10 +55,15 @@ func decideWake(in wakeInputs) (activator.WakePlan, error) { |
| if hasGood && installed == rt.GoodVersion { |
| return recovery, nil |
| } |
| - if in.Book.Blocks(acct, app, installed, host, false) { |
| - if !hasGood { |
| - return activator.WakePlan{Refuse: "build " + installed + " failed on this box and this instance has no good build"}, nil |
| - } |
| + // A record holds the installed build for this instance: boot the good |
| + // build instead. With no good build (every instance, until it first |
| + // serves after the upgrade) there is nothing to fall back to, which is |
| + // today's behaviour and not a regression (spec §8.5, §8.2): the wake |
| + // plans the installed build as a first wake with no fallback, as it |
| + // would had no record been written. Refusing it would turn one |
| + // sibling's failure into an outage for every sleeping sibling. A |
| + // hold-down was refused above and stays refused (spec §6.4). |
| + if hasGood && in.Book.Blocks(acct, app, installed, host, false) { |
| return recovery, nil |
| } |
| path := release.VersionPath(in.OptDir, acct, installed, app) |
| @@ -149,15 +155,19 @@ func (m *Manager) Plan(ctx context.Context, rt registry.Route) activator.WakePla |
| // §7.4, §8.1–8.4): a first wake's build that a failure record now holds for |
| // this instance (a sibling's failure since the plan, or a hold-down) may not |
| // boot. The good build always may: it is what a held build falls back to. |
| +// An instance with no good build is refused only by a hold-down: a record |
| +// alone would not move it onto anything, and it has no fallback by design |
| +// until it first serves (spec §8.5), so refusing it would be an outage. |
| // The book is in memory; nothing here blocks. |
| func (m *Manager) planCheck(rt registry.Route, plan activator.WakePlan) func(build string) error { |
| acct, app, host := release.RouteAccount(rt), rt.App, rt.Host |
| boot := plan.Boot.Version |
| + hasGood := rt.GoodVersion != "" && rt.GoodDigest != "" |
| return func(build string) error { |
| if !plan.FirstWake || build != boot { |
| return nil |
| } |
| - if m.d.Book.HeldDown(acct, app, build, host) || m.d.Book.Blocks(acct, app, build, host, false) { |
| + if m.d.Book.HeldDown(acct, app, build, host) || (hasGood && m.d.Book.Blocks(acct, app, build, host, false)) { |
| return fmt.Errorf("build %s is held by a failure recorded since the wake was planned", build) |
| } |
| return nil |
diff --git a/internal/rollout/wakeplan_test.go b/internal/rollout/wakeplan_test.go| index ab2f50b92..0b3adca5c 100644 |
| --- a/internal/rollout/wakeplan_test.go |
| +++ b/internal/rollout/wakeplan_test.go |
| @@ -54,8 +54,14 @@ func TestDecideWake(t *testing.T) { |
| if p, _ := decideWake(inputs(wakeRoute("v2", "v1"), held, overlap)); p.Boot.Version != "v1" || p.FirstWake { |
| t.Fatalf("installed build held by a record: boot the good build, got %+v", p) |
| } |
| - if p, _ := decideWake(inputs(wakeRoute("v2", ""), held, overlap)); p.Refuse == "" { |
| - t.Fatal("held, with no good build: nothing to boot") |
| + // Spec §8.5: an instance with no good build has no fallback, which is |
| + // today's behaviour. A sibling's record must not refuse it outright. |
| + if p, _ := decideWake(inputs(wakeRoute("v2", ""), held, overlap)); p.Refuse != "" || p.Boot.Version != "v2" || !p.FirstWake || |
| + p.Fallback != nil || p.Mode != activator.ModeOverlap { |
| + t.Fatalf("held, no good build, not held down: boot the installed build as a first wake with no fallback, got %+v", p) |
| + } |
| + if p, _ := decideWake(inputs(wakeRoute("v2", ""), down, overlap)); p.Refuse == "" { |
| + t.Fatal("held down with no good build: it stays down (spec §6.4)") |
| } |
| if p, _ := decideWake(inputs(wakeRoute("v2", "v1"), down, overlap)); p.Refuse == "" { |
| t.Fatal("held down after a hold failure: no automatic fallback (spec §6.4)") |
| @@ -450,6 +456,34 @@ func TestAPlansCheckRefusesABuildHeldSinceThePlan(t *testing.T) { |
| } |
| } |
| +// Spec §8.5: a record written after the plan does not refuse an instance with |
| +// no good build either. It is not moved onto anything else, and refusing it |
| +// would turn a sibling's failure into an outage. A hold-down still refuses. |
| +func TestAPlansCheckPassesAHostWithNoGoodBuildWhenARecordLandsAfterThePlan(t *testing.T) { |
| + h := newHarness(t, "a") |
| + h.stage("v2") |
| + h.install("v2", "a") |
| + r := h.rows.get("a") |
| + r.GoodVersion, r.GoodDigest = "", "" |
| + h.rows.put(r) |
| + p := h.m.Plan(context.Background(), h.rows.get("a")) |
| + if !p.FirstWake || p.Boot.Version != "v2" || p.Fallback != nil || p.Check == nil { |
| + t.Fatalf("plan %+v: want a first wake onto v2, no fallback, with its check", p) |
| + } |
| + if _, err := h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "b", At: t0}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := p.Check("v2"); err != nil { |
| + t.Fatalf("no good build, a sibling's record since the plan: the check must pass, got %v", err) |
| + } |
| + if _, err := h.book.Record(FailureRecord{Account: "bab", App: "blog", Build: "v2", Host: "a", HeldDown: []string{"a"}, At: t0}); err != nil { |
| + t.Fatal(err) |
| + } |
| + if err := p.Check("v2"); err == nil { |
| + t.Fatal("held down on this instance since the plan: the check must still refuse") |
| + } |
| +} |
| + |
| // Spec §2.9 (rev 17), §6.4, §8.3: an overlap attempt on a foreign-layout row |
| // runs as a hold, and the activator says so (SwapResult.Mode). Its failure |
| // is a hold build's failure: held down, so no wake boots the good build in |