amadan
zdd-6 final fix wave 1: W1 a final state survives missing observations, not a newer build
A swapped, asleep or held host whose box later reports another build installed goes back to pending, named by what is installed; a URL-live host that a later report follows is reclassified by it (Astra plan 6 R1 #3).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 files changed,
+73
−18
deploywatch.go+30 −11deploywatch_test.go+43 −7
diff --git a/deploywatch.go b/deploywatch.go| index a27702c23..87cc84c4c 100644 |
| --- a/deploywatch.go |
| +++ b/deploywatch.go |
| @@ -179,9 +179,10 @@ type watchedHost struct { |
| // host the watch follows (R1). It never sends the host to a URL poll. |
| dark bool |
| state rolloutState |
| - // fin is the report the host went final on. A final state is final for |
| - // the life of the watch (R2): what it printed and what it counts come |
| - // from fin, never from a later report. |
| + // fin is the report the host went final on. A final state survives |
| + // missing observations, and later reports that agree the build is |
| + // installed: what it printed and what it counts come from fin. A report |
| + // that the box has another build installed clears it (apply). |
| fin *apiRollout |
| urlLive bool |
| urlDiag string |
| @@ -356,13 +357,21 @@ func reportsAny(recs []apiInstanceRecord, targets []watchTarget) bool { |
| // apply folds one answered list into the hosts. |
| // |
| +// - A host with a report the watch follows is watched through it from now |
| +// on (R4), also when it was polled by URL, even live: the report is the |
| +// box's word about which build runs there, and a URL answer is not |
| +// (Astra plan 6 R1 #3). Its report is refreshed every list even once it |
| +// is final, because its box is done only on the box's latest scheduler |
| +// word. |
| +// - A final state persists only across MISSING observations: a host |
| +// absent from a truncated list, a dark box. An explicit report that the |
| +// host's box has another build installed takes a swapped, asleep or held |
| +// host back to pending, named by what is installed (pendingDetail): the |
| +// watched build no longer runs there, and Review Focus 4 forbids success |
| +// for a replaced build. A failure stays failed: the build did fail. |
| // - A host missing from a truncated list keeps its mode and state (R3): |
| // the cap cut it off, nothing about it changed, and switching it to URL |
| // polling would wake it if it is asleep. |
| -// - A host with a report the watch follows is watched through it from now |
| -// on (R4: also when it was being polled by URL). Its report is refreshed |
| -// every list even once it is final, because its box is done only on the |
| -// box's latest scheduler word; its state is not (R2). |
| // - A host the watch follows whose report is gone is dark (R1): pending if |
| // it was not final, never polled. |
| // - Any other host is watched by URL, exactly as watchVersion would. |
| @@ -374,18 +383,28 @@ func (rw rolloutWatch) apply(hosts []*watchedHost, recs []apiInstanceRecord, tru |
| for _, h := range hosts { |
| ro, ok := listed[h.t.Host] |
| switch { |
| - case !ok && truncated: |
| - case h.mode == modeURL && h.urlLive: |
| - // Live by URL is final too. |
| case follows(ro): |
| + if h.mode != modeRollout { |
| + // What a URL answer printed ("live") stands only if the |
| + // report agrees; otherwise the report's own word prints |
| + // once it is final. |
| + h.state, h.fin = rolloutPending, nil |
| + } |
| h.mode, h.ro, h.dark = modeRollout, ro, false |
| - if !h.state.final() { |
| + switch { |
| + case !h.state.final(): |
| h.state = classifyRollout(ro, rw.ver) |
| if h.state.final() { |
| fin := *ro |
| h.fin = &fin |
| } |
| + h.printed = h.printed && h.state == rolloutSwapped |
| + case h.state != rolloutFailed && ro.Installed != rw.ver: |
| + h.state, h.fin, h.printed = rolloutPending, nil, false |
| } |
| + case !ok && truncated: |
| + case h.mode == modeURL && h.urlLive: |
| + // Live by URL is final too, until a report says otherwise. |
| case h.mode == modeRollout: |
| h.dark = true |
| default: |
diff --git a/deploywatch_test.go b/deploywatch_test.go| index 8d4c86c38..3d8a31282 100644 |
| --- a/deploywatch_test.go |
| +++ b/deploywatch_test.go |
| @@ -312,10 +312,13 @@ func TestWatchRolloutDarkBoxNeverPollsOverHTTP(t *testing.T) { |
| } |
| } |
| -// R2: a final state is final for the life of the watch. a.test swaps and |
| -// its box goes dark; d.test swaps and its box then reports a newer build. |
| -// Neither returns to pending, so the watch ends once x2's canary settles. |
| -func TestWatchRolloutFinalIsStickyThroughADarkBoxAndANewerBuild(t *testing.T) { |
| +// A final state survives a missing observation, never an explicit later |
| +// report (Astra plan 6 R1 #3; Review Focus 4). a.test swaps and its box goes |
| +// dark: it stays swapped. d.test swaps and its box then reports b3 installed |
| +// and serving: B no longer runs there, so d.test is pending again, named by |
| +// what is installed, and the watch must not succeed for B once x2's canary |
| +// settles. |
| +func TestWatchRolloutFinalSurvivesADarkBoxButNotANewerBuild(t *testing.T) { |
| var out bytes.Buffer |
| var calls atomic.Int64 |
| list := sweeps(&calls, |
| @@ -335,16 +338,49 @@ func TestWatchRolloutFinalIsStickyThroughADarkBoxAndANewerBuild(t *testing.T) { |
| rec("b.test", &apiRollout{Box: "x2", Installed: B, Serving: B, Awake: true, SchedulerIdle: true}), |
| }, |
| ) |
| - if err := testWatch(&out, list, noURL(t)).run(targetsFor("a.test", "b.test", "d.test")); err != nil { |
| - t.Fatalf("watch: %v\n%s", err, out.String()) |
| + err := testWatch(&out, list, noURL(t)).run(targetsFor("a.test", "b.test", "d.test")) |
| + if err == nil { |
| + t.Fatalf("the watch succeeded for %s though d.test's box reports b3:\n%s", B, out.String()) |
| + } |
| + if !strings.Contains(err.Error(), "\n d.test: "+fmt.Sprintf(watchPendingOther, "b3")) { |
| + t.Errorf("timeout does not name d.test by what is installed: %v", err) |
| } |
| - for _, h := range []string{"a.test", "b.test", "d.test"} { |
| + for _, h := range []string{"a.test", "b.test"} { |
| + if strings.Contains(err.Error(), "\n "+h+": ") { |
| + t.Errorf("timeout names %s, which is final: %v", h, err) |
| + } |
| if got := strings.Count(out.String(), watchWordSwapped+" https://"+h+"\n"); got != 1 { |
| t.Errorf("%s printed swapped %d times:\n%s", h, got, out.String()) |
| } |
| } |
| } |
| +// A host watched by URL that a later report follows is reclassified by the |
| +// report (Astra plan 6 R1 #3). c.test answers live by URL while b.test's |
| +// canary settles; the next list reports c.test's box with b3 installed, so |
| +// the watch must not count c.test done for B. |
| +func TestWatchRolloutLaterReportReclassifiesAURLLiveHost(t *testing.T) { |
| + var out bytes.Buffer |
| + var calls atomic.Int64 |
| + var pl pollLog |
| + settling := rec("b.test", &apiRollout{Box: "x1", Installed: B, Serving: "b1", Awake: true, Attempt: status.AttemptSettling, AttemptBuild: B}) |
| + list := sweeps(&calls, |
| + []apiInstanceRecord{settling, rec("c.test", nil)}, |
| + []apiInstanceRecord{settling, rec("c.test", &apiRollout{Box: "x2", Installed: "b3", Serving: "b3", Awake: true, SchedulerIdle: true})}, |
| + []apiInstanceRecord{ |
| + rec("b.test", &apiRollout{Box: "x1", Installed: B, Serving: B, Awake: true, SchedulerIdle: true}), |
| + rec("c.test", &apiRollout{Box: "x2", Installed: "b3", Serving: "b3", Awake: true, SchedulerIdle: true}), |
| + }, |
| + ) |
| + err := testWatch(&out, list, pl.check(map[string]bool{"c.test": true})).run(targetsFor("b.test", "c.test")) |
| + if err == nil || !strings.Contains(err.Error(), "\n c.test: "+fmt.Sprintf(watchPendingOther, "b3")) { |
| + t.Fatalf("err = %v\n%s", err, out.String()) |
| + } |
| + if got := pl.hosts()["c.test"]; got != 1 { |
| + t.Errorf("c.test polled %d times, want once (before its report)", got) |
| + } |
| +} |
| + |
| // R3: absence from a truncated list keeps a host where it was. b.test is |
| // missing from the first (truncated) sweep and the list then fails once; |
| // it is never polled over HTTP, and its report, when it comes, decides it. |