amadan

carlos / platform Public

zdd-5 task 6: a retried canary overrides only the halt, never the settle

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude a3a8e46c995e4a28078202842e966470d37d6ec3 parent 8eb83e6
2 files changed, +115 −16
  • internal/rollout/admit_test.go +81 −0
  • internal/rollout/queue.go +34 −16
diff --git a/internal/rollout/admit_test.go b/internal/rollout/admit_test.go
index 55ea11355..94818e9f7 100644
--- a/internal/rollout/admit_test.go
+++ b/internal/rollout/admit_test.go
@@ -235,3 +235,84 @@ func TestADeferredHostDoesNotHoldTheCanarySlot(t *testing.T) {
t.Fatalf("admitted %+v, want b as the canary", ds)
}
}
+
+// A forced canary overrides the halt and nothing else: spec §6.2 says no other
+// swap in a cohort starts until its first one settles, and swap_concurrency
+// counts the canary too. So `deploy --retry` while the retried host's swap
+// still runs must not let the next host become a second canary beside it.
+func TestARetryThenAnUnhaltLeavesOneCanaryUntilItSettles(t *testing.T) {
+ q := awakeQueue(noBlocks, "a", "b", "c")
+ q.concurrency = 2
+ releaseAll(q, "v2", "a", "b", "c")
+ q.halt("v2", "health check failed")
+ q.retryHost("b")
+ forced := q.admit(currentView(q), t0)
+ if len(forced) != 1 || forced[0].ID.Host != "b" || !forced[0].Canary {
+ t.Fatalf("admitted %+v, want b alone, as a canary", forced)
+ }
+ q.unhalt("v2")
+ if more := q.admit(currentView(q), t0); len(more) != 0 {
+ t.Fatalf("admitted %+v beside the retried canary, which has not settled", more)
+ }
+ q.finish(forced[0], activator.SwapDone)
+ rest := q.admit(currentView(q), t0)
+ if len(rest) != 2 || rest[0].Canary || rest[1].Canary {
+ t.Fatalf("after the settle admitted %+v, want a and c and no canary", rest)
+ }
+}
+
+// Retrying a host in a cohort that is not halted gives it no way round an
+// unsettled canary.
+func TestARetriedInstanceWaitsForTheCohortsUnsettledCanary(t *testing.T) {
+ q := awakeQueue(noBlocks, "a", "b")
+ q.concurrency = 2
+ releaseAll(q, "v2", "a", "b")
+ first := q.admit(currentView(q), t0)
+ if len(first) != 1 || first[0].ID.Host != "a" || !first[0].Canary {
+ t.Fatalf("admitted %+v, want a as the canary", first)
+ }
+ q.retryHost("b")
+ if more := q.admit(currentView(q), t0); len(more) != 0 {
+ t.Fatalf("admitted %+v beside a's unsettled canary", more)
+ }
+ q.finish(first[0], activator.SwapDone)
+ if ds := q.admit(currentView(q), t0); len(ds) != 1 || ds[0].ID.Host != "b" {
+ t.Fatalf("after the settle admitted %+v, want b", ds)
+ }
+}
+
+// A halted cohort whose canary is still running (halt supersedes it, but it
+// has not been abandoned yet) holds a retried host back too.
+func TestARetriedInstanceWaitsForARunningCanaryInAHaltedCohort(t *testing.T) {
+ q := awakeQueue(noBlocks, "a", "b")
+ q.concurrency = 2
+ releaseAll(q, "v2", "a", "b")
+ first := q.admit(currentView(q), t0)
+ q.halt("v2", "another host failed")
+ q.retryHost("b")
+ if more := q.admit(currentView(q), t0); len(more) != 0 {
+ t.Fatalf("admitted %+v beside a running canary", more)
+ }
+ q.finish(first[0], activator.SwapAbandoned)
+ if ds := q.admit(currentView(q), t0); len(ds) != 1 || ds[0].ID.Host != "b" || !ds[0].Canary {
+ t.Fatalf("after the abandon admitted %+v, want b as a canary", ds)
+ }
+}
+
+// A retry on a host with nothing pending must not leave a flag that lets an
+// unrelated later attempt skip the settle, or a halt.
+func TestAStaleRetryDoesNotBypassTheSettleOrAHalt(t *testing.T) {
+ q := awakeQueue(noBlocks, "a", "b", "c")
+ q.concurrency = 3
+ q.retryHost("c") // c has no desired build: nothing to retry
+ releaseAll(q, "v2", "a", "b", "c")
+ first := q.admit(currentView(q), t0)
+ if len(first) != 1 || first[0].ID.Host != "a" {
+ t.Fatalf("admitted %+v, want only a's canary: c's old retry bought nothing", first)
+ }
+ q.halt("v2", "health check failed")
+ q.finish(first[0], activator.SwapFailed)
+ if ds := q.admit(currentView(q), t0); len(ds) != 0 {
+ t.Fatalf("admitted %+v from a halted cohort on a stale retry", ds)
+ }
+}
diff --git a/internal/rollout/queue.go b/internal/rollout/queue.go
index 4db495063..e4cc04531 100644
--- a/internal/rollout/queue.go
+++ b/internal/rollout/queue.go
@@ -60,9 +60,9 @@ type hostSlot struct {
configAfter time.Time // a failed config attempt's backoff (spec §8.1)
notBefore time.Time // a staging refusal's wait (spec §4.2)
- running *dispatch
- buildRev uint64 // newest(host): moved only by a change of desired build
- forceCanary bool // `carlos instances retry`: the next attempt is a canary
+ running *dispatch
+ buildRev uint64 // newest(host): moved only by a change of desired build
+ forceBuild string // `carlos instances retry`: lets the adoption onto this build through a halt
}
// cohort is one app's attempts on this box onto one build, of one class
@@ -295,8 +295,10 @@ func (q *appQueue) admit(view map[string]string, now time.Time) []dispatch {
// read the two waits, so they are enforced here: a staging refusal holds
// every attempt for the host until notBefore, and a failed config attempt
// waits out its backoff. A cohort's first attempt is its canary and runs
-// alone until it settles; a halted adoption cohort admits nothing but a
-// retried host's own canary.
+// alone until it settles, and that holds for a retried host too: a retry
+// overrides the halt of an adoption cohort and nothing else, so it still
+// waits for any canary of the cohort that is running and unsettled.
+// A halted adoption cohort admits nothing but such a retried host.
func (q *appQueue) gate(h *hostSlot, now time.Time) (dispatch, bool) {
d, ok := q.pending(h)
if !ok || now.Before(h.notBefore) {
@@ -307,10 +309,16 @@ func (q *appQueue) gate(h *hostSlot, now time.Time) (dispatch, bool) {
}
c := q.cohortFor(d.Cohort)
switch {
- case h.forceCanary:
- d.Canary = true
case c.halted && d.Class == classAdopt:
- return dispatch{}, false
+ // The flag is tied to the build it was set for, so a retry on a host
+ // with nothing pending cannot lift a later cohort's halt.
+ if h.forceBuild == "" || h.forceBuild != d.ID.Build {
+ return dispatch{}, false
+ }
+ if c.canary != "" && !c.settled {
+ return dispatch{}, false
+ }
+ d.Canary = true
case c.canary == "":
d.Canary = true
case !c.settled:
@@ -321,12 +329,14 @@ func (q *appQueue) gate(h *hostSlot, now time.Time) (dispatch, bool) {
// start marks d running. A cohort's canary is claimed here, at the moment it
// actually starts, so a host gated out by the concurrency limit never holds
-// the canary slot.
+// the canary slot. A retried host's canary claims it like any other, and a
+// settled flag left over from before the halt starts again from false.
func (q *appQueue) start(h *hostSlot, d dispatch) {
- if d.Canary && !h.forceCanary {
- q.cohortFor(d.Cohort).canary = h.host
+ if d.Canary {
+ c := q.cohortFor(d.Cohort)
+ c.canary, c.settled = h.host, false
}
- h.forceCanary = false
+ h.forceBuild = ""
h.running = &d
q.running++
}
@@ -392,17 +402,25 @@ func (q *appQueue) halt(build, reason string) (superseded []string) {
}
// unhalt reopens build's adoption cohort after `carlos deploy --retry`: its
-// next attempt is a fresh canary.
+// next attempt is a fresh canary. A canary still running (a retried host's
+// swap) stays the cohort's canary, so nothing else starts until it settles.
func (q *appQueue) unhalt(build string) {
if c := q.cohorts[cohortKey(build, classAdopt)]; c != nil {
- *c = cohort{}
+ running := ""
+ if !c.settled {
+ running = c.canary
+ }
+ *c = cohort{canary: running}
}
}
-// retryHost is `carlos instances retry <host>`: its next attempt is a canary.
+// retryHost is `carlos instances retry <host>`: if the host is waiting on a
+// build, that adoption runs as a canary even through a halt. The flag is
+// bound to the build the host wants now, so a retry with nothing pending, or
+// a later change of desire, leaves nothing behind that gating could honour.
func (q *appQueue) retryHost(host string) {
if h := q.hosts[host]; h != nil {
- h.forceCanary = true
+ h.forceBuild = h.wantBuild
}
}