amadan

carlos / platform Public

zdd-6 task 10: fix round 1

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude cc57273deaadef5e8d2f84ec24695838042d5aec parent bd27d31
4 files changed, +145 −8
  • deploywatch.go +24 −8
  • deploywatch_test.go +91 −0
  • internal/rolloutwords/words.go +13 −0
  • internal/rolloutwords/words_test.go +17 −0
diff --git a/deploywatch.go b/deploywatch.go
index db3f3c3ca..a27702c23 100644
--- a/deploywatch.go
+++ b/deploywatch.go
@@ -80,7 +80,10 @@ const (
// minute, shared with the deploy's own calls), and each read costs the
// console a records list and a box join, so it keeps its own cadence; a host
// watched by URL is still polled every deployWatchPollEvery, as before.
+// firstListTries is how many times a failing first list is asked before the
+// watch falls back to watchVersion.
const (
+ firstListTries = 3
deployWatchListEvery = 5 * time.Second
deployWatchPollEvery = 2 * time.Second
)
@@ -148,12 +151,14 @@ func classifyRollout(r *apiRollout, ver string) rolloutState {
}
// follows reports whether the watch follows an instance through r: a report
-// with per-host state, or one from a box that refuses every managed start
-// (its instances wait, and must not be woken by a poll). A box that declined
-// overlap runs today's stop-then-start paths, so its report is no report:
-// the instance is watched by URL, like a flag-off box's (R6).
+// with per-host state, or one from a box that cannot arm its scheduler for
+// any reason but a known decline: a refusing box's instances wait, and a
+// code this binary does not know is a newer box's problem, so neither may be
+// woken by a poll. A box that declined overlap runs today's stop-then-start
+// paths, so its report is no report: the instance is watched by URL, like a
+// flag-off box's (R6; fix round 1).
func follows(r *apiRollout) bool {
- return r != nil && (r.Unavailable == "" || rolloutwords.UnavailableRefuses(r.Unavailable))
+ return r != nil && !rolloutwords.UnavailableDeclines(r.Unavailable)
}
type watchMode int
@@ -212,11 +217,22 @@ type rolloutWatch struct {
func (rw rolloutWatch) run(targets []watchTarget) error {
deadline := time.Now().Add(rw.timeout)
+ // A failed first list (a rate limit, a console restart) says nothing
+ // about the boxes, so it is retried on the list's cadence before the
+ // watch gives up on reports (fix round 1).
recs, truncated, err := rw.list()
+ for tries := 1; err != nil && tries < firstListTries; tries++ {
+ time.Sleep(max(rw.listEvery, rw.interval))
+ recs, truncated, err = rw.list()
+ }
lastList := time.Now()
- if err != nil || !reportsAny(recs, targets) {
- // No box reports a rollout for any target: today's watch, byte for
- // byte (R10).
+ // Today's watch, byte for byte (R10), only when the console has shown
+ // the whole list and no box reports a rollout for any target; or when
+ // it never answered, so a broken list endpoint still leaves the deploy
+ // watched as it always was. A truncated list never falls back: a host
+ // it cut off may sit on a reporting box, and a URL poll would wake it
+ // (R3); its listed, unreported hosts are polled by URL anyway (R4).
+ if err != nil || (!truncated && !reportsAny(recs, targets)) {
if rw.onURL != nil {
rw.onURL()
}
diff --git a/deploywatch_test.go b/deploywatch_test.go
index e76d47fef..8d4c86c38 100644
--- a/deploywatch_test.go
+++ b/deploywatch_test.go
@@ -553,3 +553,94 @@ func TestDeployHeldLine(t *testing.T) {
}
}
}
+
+// hitServer is an instance's origin that counts the HTTP requests reaching
+// it, so a test sees a poll from either path: the rollout watch's check or a
+// fallback to watchVersion, which dials the origin itself.
+func hitServer(t *testing.T) (string, *atomic.Int64) {
+ var hits atomic.Int64
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ hits.Add(1)
+ w.Header().Set("X-Carlos-Version", B)
+ }))
+ t.Cleanup(srv.Close)
+ return srv.URL, &hits
+}
+
+// Fix round 1: a failed first list is retried, not a reason to URL-poll a
+// fleet whose boxes report (that wakes the sleeper here).
+func TestWatchRolloutFirstListErrorIsRetried(t *testing.T) {
+ origin, hits := hitServer(t)
+ var calls atomic.Int64
+ list := scripted(&calls,
+ sweep{err: errors.New("instances list: 429")},
+ sweep{recs: []apiInstanceRecord{rec("b.test", &apiRollout{Box: "x1", Installed: B, Good: "b1", SchedulerIdle: true})}},
+ )
+ rw := testWatch(io.Discard, list, noURL(t))
+ onURL := 0
+ rw.onURL = func() { onURL++ }
+ if err := rw.run([]watchTarget{{Host: "b.test", Origin: origin}}); err != nil {
+ t.Fatal(err)
+ }
+ if hits.Load() != 0 || onURL != 0 || calls.Load() != 2 {
+ t.Fatalf("%d HTTP polls, onURL %d, %d lists: want 0, 0, 2", hits.Load(), onURL, calls.Load())
+ }
+}
+
+// Fix round 1: three failed first lists fall back to today's watch, so a
+// broken list endpoint never leaves a fleet unwatched.
+func TestWatchRolloutThreeFailedFirstListsFallBack(t *testing.T) {
+ origin, hits := hitServer(t)
+ var calls atomic.Int64
+ var out bytes.Buffer
+ failed := sweep{err: errors.New("instances list: 503")}
+ list := scripted(&calls, failed, failed, failed,
+ sweep{recs: []apiInstanceRecord{rec("x.test", &apiRollout{Box: "x1", Installed: B, Good: "b1", SchedulerIdle: true})}},
+ )
+ rw := testWatch(&out, list, nil)
+ onURL := 0
+ rw.onURL = func() { onURL++ }
+ if err := rw.run([]watchTarget{{Host: "x.test", Origin: origin}}); err != nil {
+ t.Fatal(err)
+ }
+ if calls.Load() != 3 || onURL != 1 || hits.Load() == 0 || !strings.Contains(out.String(), "live https://x.test") {
+ t.Fatalf("%d lists, onURL %d, %d HTTP polls, output:\n%s", calls.Load(), onURL, hits.Load(), out.String())
+ }
+}
+
+// Fix round 1 (R3 on the first sweep): a truncated first list with no
+// report among the listed targets is no evidence that no box reports.
+// c.test (listed, unreported) is watched by URL; b.test (cut off, then
+// reported asleep) is never requested.
+func TestWatchRolloutTruncatedFirstListDoesNotFallBack(t *testing.T) {
+ bOrigin, bHits := hitServer(t)
+ var calls atomic.Int64
+ var pl pollLog
+ list := scripted(&calls,
+ sweep{recs: []apiInstanceRecord{rec("c.test", nil)}, truncated: true},
+ sweep{recs: []apiInstanceRecord{
+ rec("b.test", &apiRollout{Box: "x1", Installed: B, Good: "b1", SchedulerIdle: true}),
+ rec("c.test", nil),
+ }},
+ )
+ rw := testWatch(io.Discard, list, pl.check(map[string]bool{"c.test": true}))
+ targets := []watchTarget{{Host: "b.test", Origin: bOrigin}, {Host: "c.test", Origin: "http://127.0.0.1:1"}}
+ if err := rw.run(targets); err != nil {
+ t.Fatal(err)
+ }
+ if got := pl.hosts(); bHits.Load() != 0 || got["b.test"] != 0 || got["c.test"] != 1 {
+ t.Fatalf("b.test origin hit %d times, check polled %v", bHits.Load(), got)
+ }
+}
+
+// Fix round 1 (R6): a code this binary has no phrase for is a newer box's
+// problem; its instances are followed and never requested.
+func TestWatchRolloutUnknownUnavailableCodeIsNeverPolled(t *testing.T) {
+ origin, hits := hitServer(t)
+ var calls atomic.Int64
+ list := sweeps(&calls, []apiInstanceRecord{rec("a.test", &apiRollout{Box: "x1", Installed: B, Unavailable: "future-code"})})
+ err := testWatch(io.Discard, list, noURL(t)).run([]watchTarget{{Host: "a.test", Origin: origin}})
+ if hits.Load() != 0 || err == nil || !strings.Contains(err.Error(), "a.test: "+rolloutwords.Unavailable("future-code")) {
+ t.Fatalf("%d HTTP polls, err = %v", hits.Load(), err)
+ }
+}
diff --git a/internal/rolloutwords/words.go b/internal/rolloutwords/words.go
index 665d7d5f0..7641e1907 100644
--- a/internal/rolloutwords/words.go
+++ b/internal/rolloutwords/words.go
@@ -115,6 +115,19 @@ func Unavailable(code string) string {
return fmt.Sprintf(unavailableUnknown, code)
}
+// UnavailableDeclines reports whether code is one under which the box
+// declined CARLOS_SWAP=overlap and runs today's stop-then-start paths. An
+// unknown code (a newer box) is not: a reader must not assume its instances
+// are safe to wake.
+func UnavailableDeclines(code string) bool {
+ switch code {
+ case "no-cgroup-delegation", "no-generation-control", "generations-off":
+ return true
+ default:
+ return false
+ }
+}
+
// UnavailableRefuses reports whether code is one under which the box
// refuses every managed start (as opposed to running today's paths).
func UnavailableRefuses(code string) bool {
diff --git a/internal/rolloutwords/words_test.go b/internal/rolloutwords/words_test.go
index dc5f69b71..0c5a22f29 100644
--- a/internal/rolloutwords/words_test.go
+++ b/internal/rolloutwords/words_test.go
@@ -116,3 +116,20 @@ func TestEveryUnavailableCodeHasAPhrase(t *testing.T) {
t.Errorf("UnavailableRefuses(future) = true, want false")
}
}
+
+// UnavailableDeclines is true for exactly the three codes under which a box
+// runs today's stop-then-start paths. A refusing code is not one, and nor is
+// a code this binary does not know: a reader cannot assume a newer box's
+// problem leaves its instances safe to wake.
+func TestUnavailableDeclinesOnlyTheKnownDeclinedCodes(t *testing.T) {
+ for _, code := range []string{"no-cgroup-delegation", "no-generation-control", "generations-off"} {
+ if !UnavailableDeclines(code) {
+ t.Errorf("UnavailableDeclines(%q) = false, want true", code)
+ }
+ }
+ for _, code := range []string{"no-deploy-store", "no-release-key", "failure-book-unreadable", "future", ""} {
+ if UnavailableDeclines(code) {
+ t.Errorf("UnavailableDeclines(%q) = true, want false", code)
+ }
+ }
+}