carlos / platform Public

zdd-5 final review: a declined box keeps the bootstrap rule

A declined box held an instance for a blocking record even with no good
build. The armed planner boots the installed build then (spec 8.5), so the
shared heldBuild predicate now holds on a record only when the instance has
a good build. A hold-down still holds regardless.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago e0f231c4d67734b114a798a610c4246a9df13874 parent 4003155

2 files changed, +50 −16

internal/host/rolloutdeclined.go +12 −5
internal/host/rolloutdeclined_test.go +38 −11

diff --git a/internal/host/rolloutdeclined.go b/internal/host/rolloutdeclined.go

index 520dd3736..d63a6659c 100644

--- a/internal/host/rolloutdeclined.go

+++ b/internal/host/rolloutdeclined.go

@@ -88,9 +88,16 @@ func honourDeclinedFailures(svc *activator.Service, reg *registry.Registry, dd,

// it. Only a managed route can be held: the scheduler records nothing else.

// Both the installed build (root's stamp) and the build live/<host> names

// are asked, because a wake execs the live link and root repoints it before

-// it stamps the row; an unreadable link is left to the stamp. A build the

-// instance has served (its good build) is never held by a record alone, as

-// rollout.Blocks says, but a hold-down holds it whatever it served.

+// it stamps the row; an unreadable link is left to the stamp.

+//

+// The rule is the armed planner's (rollout.decideWake, spec §8.5), with the

+// one difference that a declined box cannot boot the good build instead, so

+// it refuses where the armed box would recover. A hold-down holds whatever

+// the instance served. A plain record holds a build only when the instance

+// has a good build to fall back to (a build it has served is never held by a

+// record alone); an instance with none boots its installed build as today,

+// because refusing it would turn one sibling's failure into an outage for

+// every sleeping sibling.

func (h *failureHolds) heldBuild(rt registry.Route) (build string, heldDown bool) {

if !rollout.Managed(rt) {

return "", false

@@ -100,6 +107,7 @@ func (h *failureHolds) heldBuild(rt registry.Route) (build string, heldDown bool

builds = append(builds, live)

}

acct := release.RouteAccount(rt)

+ hasGood := rt.GoodVersion != ""

for _, b := range builds {

if b == "" {

continue

@@ -107,8 +115,7 @@ func (h *failureHolds) heldBuild(rt registry.Route) (build string, heldDown bool

if h.book.HeldDown(acct, rt.App, b, rt.Host) {

return b, true

}

- served := rt.GoodVersion != "" && b == rt.GoodVersion

- if h.book.Blocks(acct, rt.App, b, rt.Host, served) {

+ if hasGood && h.book.Blocks(acct, rt.App, b, rt.Host, b == rt.GoodVersion) {

return b, false

}

diff --git a/internal/host/rolloutdeclined_test.go b/internal/host/rolloutdeclined_test.go

index 242bb1d0b..b1bcf4d1e 100644

--- a/internal/host/rolloutdeclined_test.go

+++ b/internal/host/rolloutdeclined_test.go

@@ -163,6 +163,14 @@ func TestDeclinedBoxRefusesAWakeOntoAHeldBuild(t *testing.T) {

good := managedRoute("good.example.com", "v2")

good.GoodVersion, good.GoodDigest = "v2", "sha256:good"

+ // A held instance has a good build (v1) to have fallen back to: spec

+ // §8.5's bootstrap rule refuses on a record only then.

+ blocked := managedRoute("a.example.com", "v2")

+ blocked.GoodVersion, blocked.GoodDigest = "v1", "sha256:v1"

+ // No good build, installed build blocked, not held down: boots as today.

+ noGood := managedRoute("nogood.example.com", "v2")

+ // Held down with no good build: still refused.

+ downNoGood := managedRoute("down.example.com", "v2")

downGood := managedRoute("downgood.example.com", "v2")

downGood.GoodVersion, downGood.GoodDigest = "v2", "sha256:good"

unit := managedRoute("unit.example.com", "v2")

@@ -170,18 +178,25 @@ func TestDeclinedBoxRefusesAWakeOntoAHeldBuild(t *testing.T) {

// Installed v1, but root has already repointed live/<host> at v2 and

// not yet stamped the row: the wake would exec v2.

repointed := managedRoute("repointed.example.com", "v1")

+ repointed.GoodVersion, repointed.GoodDigest = "v1", "sha256:v1"

+ // The same repoint on an instance with no good build boots as today.

+ repointedNoGood := managedRoute("repointednogood.example.com", "v1")

if err := release.StageArtifact(optDir, acct, "v2", "blog", []byte("v2")); err != nil {

t.Fatal(err)

}

- if err := release.PointLive(optDir, repointed.Host, acct, "v2", "blog"); err != nil {

- t.Fatal(err)

+ for _, h := range []string{repointed.Host, repointedNoGood.Host} {

+ if err := release.PointLive(optDir, h, acct, "v2", "blog"); err != nil {

+ t.Fatal(err)

+ }

}

for _, tc := range []struct {

rt registry.Route

refuse string

}{

- {managedRoute("a.example.com", "v2"), refuseNoSwaps},

- {managedRoute("down.example.com", "v2"), "build v2 failed in hold mode on this instance; recovery is explicit"},

+ {blocked, refuseNoSwaps},

+ {noGood, ""},

+ {repointedNoGood, ""},

+ {downNoGood, "build v2 failed in hold mode on this instance; recovery is explicit"},

{downGood, "build v2 failed in hold mode on this instance; recovery is explicit"},

{repointed, refuseNoSwaps},

{good, ""},

@@ -217,7 +232,12 @@ func TestDeclinedLegsLeaveAHeldInstanceRunning(t *testing.T) {

writeFailureBook(t, dd, []rollout.FailureRecord{{Account: acct, App: "blog", Build: "v2", Host: "sib.example.com"}}, nil)

before := snapshotBook(t, dd)

held, sleeper, free := managedRoute("held.example.com", "v2"), managedRoute("sleeper.example.com", "v2"), managedRoute("free.example.com", "v1")

- for _, rt := range []registry.Route{held, sleeper, free} {

+ // Held and asleep only because they have a good build to have kept; an

+ // instance with none boots its blocked build as today, so it is free.

+ held.GoodVersion, held.GoodDigest = "v1", "sha256:v1"

+ sleeper.GoodVersion, sleeper.GoodDigest = "v1", "sha256:v1"

+ noGood := managedRoute("nogood.example.com", "v2")

+ for _, rt := range []registry.Route{held, sleeper, free, noGood} {

if err := reg.Upsert(rt); err != nil {

t.Fatal(err)

}

@@ -229,15 +249,16 @@ func TestDeclinedLegsLeaveAHeldInstanceRunning(t *testing.T) {

var bounced []string

bounce := func(h string) bool { bounced = append(bounced, h); return true }

mem := newRestartMemory()

- routes := []registry.Route{held, free}

+ routes := []registry.Route{held, free, noGood}

askRestart(t, ctx, s, "bab", "blog")

restartRequestLeg(ctx, routes, s, t.TempDir(), bounce, nil, mem)

askRestart(t, ctx, s, "bab", "blog")

if !restartRequestLeg(ctx, routes, s, t.TempDir(), bounce, nil, mem) {

t.Fatal("the restart leg did not ask for the root pass")

}

- if !slices.Equal(bounced, []string{"free.example.com"}) {

- t.Fatalf("restart bounced %v, want only the instance no record holds", bounced)

+ slices.Sort(bounced)

+ if !slices.Equal(bounced, []string{"free.example.com", "nogood.example.com"}) {

+ t.Fatalf("restart bounced %v, want only the instances no record holds", bounced)

}

// Config sync, end to end over the real pass.

@@ -266,8 +287,9 @@ func TestDeclinedLegsLeaveAHeldInstanceRunning(t *testing.T) {

if err := appconfig.SyncExecWithCallbacks(ctx, reg, store, priv, t.TempDir(), "", cb); err != nil {

t.Fatal(err)

}

- if !slices.Equal(restarted, []string{free.Host}) {

- t.Fatalf("config sync restarted %v, want only the instance no record holds", restarted)

+ slices.Sort(restarted)

+ if !slices.Equal(restarted, []string{free.Host, noGood.Host}) {

+ t.Fatalf("config sync restarted %v, want only the instances no record holds", restarted)

}

if a := acks[held.Host]; a.Restarted || a.Hibernating || a.RestartError != rolloutRestartRefused {

t.Fatalf("held awake ack = %+v, want RestartError %q", a, rolloutRestartRefused)

@@ -275,6 +297,9 @@ func TestDeclinedLegsLeaveAHeldInstanceRunning(t *testing.T) {

if a := acks[sleeper.Host]; a.Restarted || !a.Hibernating || a.RestartError != "" {

t.Fatalf("held sleeper ack = %+v, want delivered on wake", a)

}

+ if a := acks[noGood.Host]; !a.Restarted || a.Hibernating || a.RestartError != "" {

+ t.Fatalf("no-good ack = %+v, want today's restarted", a)

+ }

if a := acks[free.Host]; !a.Restarted || a.Hibernating || a.RestartError != "" {

t.Fatalf("free ack = %+v, want today's restarted", a)

}

@@ -294,8 +319,10 @@ func TestDeclinedBoxKeepsAHeldBuildOffARealActivator(t *testing.T) {

env := newFastFixture(t)

t.Setenv("CARLOS_OPT_DIR", env.optDir)

host := "warm.test"

+ // GoodVersion v1: a record holds a build only for an instance that has a

+ // good build (the bootstrap rule), and this one has served v1.

rt := registry.Route{Host: host, Kind: "instance", App: "fastapp", Account: "bab", Channel: "edge",

- Status: "ready", Hibernate: true, Version: "v1"}

+ Status: "ready", Hibernate: true, Version: "v1", GoodVersion: "v1", GoodDigest: "sha256:v1"}

if err := env.reg.Upsert(rt); err != nil {

t.Fatal(err)

}