Repository navigation
✨ hub+spoke: expose scheduled-upgrade progress, drain countdown and upgrade history - #14953
Conversation
kubestellar-prow
Bot
added
the
dco-signoff: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kubestellar-prow
Bot
added
needs-rebase
hivecommons-hive
Bot
added
author/human
Open PR overlap checkBase branch: Conflicts are checked with every PR merged onto the current tip of
|
…rade history A hub-instructed upgrade is now a visible state machine on both surfaces (scheduled → draining → pulling → rolling → verifying → done | missed): the hub derives it per hive on /api/saas/my-hives (upgradeState, upgradeHistory) from its latch, the spoke's drain report and the schedule; the spoke derives the same vocabulary on /api/status.upgradeProgress from its drain latch, self-upgrade marker, landed outcome and the hub's policy. The drain report gains per-agent turn elapsed so the card can say "2/5 agents in turn: reviewer 38m, scanner 12m — proceeds in 40m". The heartbeat policy gains next_window_at (the hive's own window regardless of channel — the stable channel's nextUpdateAt is the promotion ETA, which is what made 'overdue since' unreadable) and upgrade_history. The hub persists the last 14 attempts and held windows per hive under /data/saas/upgrade-history/ (HIVE_HUB_UPGRADE_HISTORY_MAX), recorded from the manual and auto arm paths, the heartbeat transition detector, the orphan sweep and the schedule gate's hold recorder, so a missed window survives a restart and is never silent. Refs #14950 (PR A of two). Signed-off-by: Andrew Anderson <andy@clubanderson.com>
d7951ae to
e0723a1
Compare
kubestellar-prow
Bot
removed
the
needs-rebase
github-actions
Bot
added
the
conflicts-with-open-pr
|
@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this hive's repositories have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply. |
hivecommons-hive
Bot
added
the
role/trusted
|
@clubanderson this PR is held at Why:
Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path. |
hivecommons-hive
Bot
added
stage/050-review-pending
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed (intent-alignment) — the diff matches #14950 PR A: shared state vocabulary (pkg/hub/spoke/upgrade_progress.go), hub derivation + /api/saas/my-hives wiring (pkg/hub/upgrade_progress.go, saas_hives_api.go:1019), spoke /api/status.upgradeProgress (pkg/dashboard/upgrade_progress.go), per-agent turns[] (cmd/hive/self_upgrade_drain.go:148-154), next_window_at regardless of channel (heartbeat_upgrade_policy.go:53-60), persisted history with env cap (upgrade_history.go). UI / interrupt action correctly deferred to PR B per the issue. No scope creep; api-reference.md churn is the citation-fix script.
One advisory, non-blocking:
windowMissed has no stale-open cutoff — upgrade_history.go windowMissed returns unconditionally when the last row is Open(), whereas start() closes an open row older than upgradeHistoryRecentFor as "superseded". The PR body itself names paths that leave a row open without reporting (channel switch, state reset). If such a row lingers and the gate keeps holding (so start() is never reached), every subsequent held window is silently dropped — the exact outcome #14950 §2 says must not recur ("one row per held calendar-day window"). Suggested fix, mirroring start():
if n := len(list); n > 0 {
last := &list[n-1]
if last.Open() {
- return
+ if at.Sub(last.StartedAt) < upgradeHistoryRecentFor {
+ return
+ }
+ last.Outcome = spoke.UpgradeOutcomeMissed
+ last.Reason = "superseded by a held window"
+ last.EndedAt = at.UTC()
}
if last.Trigger == spoke.UpgradeTriggerWindow && sameUpgradeWindow(last.StartedAt, at) {— hive: agent=adjudicator-3 backend=copilot model=claude-fable-5.1 copilot=1.0.88
|
merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at e0723a1. |
|
Thank you for your contribution! Your PR has been merged. We'd love to hear how your experience was: share feedback |
There was a problem hiding this comment.
Security review (read at d7951ae) — no blocking findings.
One low advisory, trust boundary: the new spoke-supplied UpgradeDrainReport.Turns[].Agent (src/pkg/hub/heartbeat.go:185) is copied raw into the registry at src/pkg/hub/server.go:2283 (entry.UpgradeDrain = payload.UpgradeDrain) and then concatenated into human-readable Detail at src/pkg/hub/upgrade_progress.go:104 via spoke.FormatTurns, which is served on /api/saas/my-hives and will be rendered by PR B. The sibling spoke prose field UpgradeError is run through sanitizeProseField at server.go:2307; Turns (and the pre-existing InFlight) are not. Suggest sanitizing at ingestion so PR B's pill inherits a clean string:
--- a/src/pkg/hub/server.go
+++ b/src/pkg/hub/server.go
@@ -2283 @@
- entry.UpgradeDrain = payload.UpgradeDrain
+ entry.UpgradeDrain = payload.UpgradeDrain
+ if d := entry.UpgradeDrain; d != nil {
+ for i := range d.InFlight {
+ d.InFlight[i] = sanitizeProseField(d.InFlight[i])
+ }
+ for i := range d.Turns {
+ d.Turns[i].Agent = sanitizeProseField(d.Turns[i].Agent)
+ }
+ }Checked and fine: upgradeHistoryPath rejects traversal via isValidName + ../separator checks (upgrade_history.go:56-61, tested); history files are only keyed by hub-owned hive ids; payload.HiveID is used only as a map key for recent(); spoke /api/status.upgradeProgress exposes only data already reachable via /api/version marker/outcome.
— hive: agent=adjudicator-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
docs-currency — read at head d7951ae.
The new operator-reference.md section, release-channels.md cross-link and changelog fragment all check out against the code: every documented field (nextWindowAt, drainDeadline, drainRemainingS, agentsInTurn[].{agent,elapsedS}, agentsTotal, detail, reason, endedAt) matches the JSON tags in src/pkg/hub/spoke/upgrade_progress.go:69-114; upgrade_policy.next_window_at / upgrade_history match src/pkg/hub/spoke/heartbeat.go:2911-2915; upgrade_drain.turns matches heartbeat.go:578; 14 / HIVE_HUB_UPGRADE_HISTORY_MAX / /data/saas/upgrade-history / 24 h match src/pkg/hub/upgrade_history.go:23-39; the "45 min + 20-minute grace" matches config/hub_settings.go:17 and upgradeDrainOverrun = orphanedUpgradeClearAfter = 2×staleUpgradeTimeout in src/pkg/hub/stale_upgrade.go:23-27,252.
One in-scope gap (low): src/docs/api-reference.md was only re-cited, not updated. The PR ran check-api-reference-citations.sh --fix (42 line-number changes) but the two endpoint rows that gain fields still don't mention them, while both rows otherwise enumerate payload fields with issue refs (api-reference.md:20 for /api/status, :603 for /api/saas/my-hives). Issue #14950 item 5 asks for docs covering the payload fields; the endpoint table is where an API consumer looks first. Suggested additions (append to the Description cell of each row):
| `GET` | `/api/status` | ... `countsStaleSince` mark totals served from last-good figures — the previous enumeration (#13382) or the ForgeState cache's last-good counts (#13376) | `pkg/dashboard/server.go:1535` |
+ ... (#13376); `upgradeProgress` (#14950) is the spoke's scheduled-upgrade state `{state, target?, from?, startedAt?, nextWindowAt?, drainDeadline?, drainRemainingS?, agentsInTurn[]{agent,elapsedS}?, agentsTotal?, reason?, detail, endedAt?}` with `state` ∈ `scheduled|draining|pulling|rolling|verifying|done|missed`, derived at serve time from the drain latch, self-upgrade marker, last landed outcome and heartbeat policy — see [operator-reference](operator-reference.md#scheduled-upgrade-progress-and-history) | ... | `GET` | `/api/saas/my-hives` | Hub auth | My Hives. Each row carries `quadrant` ... See [Fleet quadrant](fleet-health.md#fleet-quadrant) | ...
+ ... See [Fleet quadrant](fleet-health.md#fleet-quadrant). Each row also carries `upgradeState` (same shape as the spoke's `/api/status.upgradeProgress`, derived from the registry latch, schedule and persisted history) and `upgradeHistory[]` `{target, from, trigger, startedAt, endedAt, outcome, reason}` — the last `HIVE_HUB_UPGRADE_HISTORY_MAX` (14) attempts and held windows (#14950) | ...— hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88
hivecommons-hive
Bot
removed
role/trusted
There was a problem hiding this comment.
Style / maintainability review (read at head e0723a1, which superseded the dispatched d7951ae).
1. The "shared vocabulary" is only half shared — the history→state derivation is copy-pasted between hub and spoke. pkg/hub/upgrade_progress.go and pkg/dashboard/upgrade_progress.go carry byte-identical copies of latestClosedAttempt (hub/upgrade_progress.go:187-199 ↔ dashboard/upgrade_progress.go:563-575), the upgradeHistoryRecentFor = 24h const (hub/upgrade_history.go:33 ↔ dashboard/upgrade_progress.go:561, whose own comment says it "mirrors the hub's window"), and the ~20-line history-row → UpgradeProgress block with its "upgraded to … ago" / "missed window … ago: …" strings (hub/upgrade_progress.go:142-166 ↔ dashboard/upgrade_progress.go:519-543, differing only in orDash vs orDashDashboard). #14950's stated goal is that "both dashboards speak the same words"; two copies of the words is exactly how they drift the first time one Detail string is reworded. Both files already import pkg/hub/spoke, which this PR introduces as the shared package — the derivation belongs there:
--- a/src/pkg/hub/spoke/upgrade_progress.go
+++ b/src/pkg/hub/spoke/upgrade_progress.go
@@
+// HistoryRecentFor bounds how long a closed attempt still speaks for the
+// hive's current state; after it both dashboards fall back to "scheduled".
+const HistoryRecentFor = 24 * time.Hour
+
+// LatestClosedAttempt returns the most recently ended row of a history.
+func LatestClosedAttempt(history []UpgradeAttempt) (UpgradeAttempt, bool) { … }
+
+// ProgressFromAttempt renders a closed history row as the current state.
+func ProgressFromAttempt(last UpgradeAttempt, nextWindow string, now time.Time) *UpgradeProgress {
+ p := &UpgradeProgress{State: last.Outcome, Target: last.Target, From: last.From,
+ StartedAt: rfc3339(last.StartedAt), EndedAt: rfc3339(last.EndedAt),
+ NextWindowAt: nextWindow, Reason: last.Reason}
+ ago := FormatShortDuration(now.Sub(last.EndedAt)) + " ago"
+ switch {
+ case last.Outcome == UpgradeOutcomeDone:
+ p.Detail = "upgraded to " + orDash(last.Target) + " " + ago
+ case last.Trigger == UpgradeTriggerWindow:
+ p.Detail = "missed window " + ago + ": " + last.Reason
+ default:
+ p.Detail = "missed " + ago + ": " + last.Reason
+ }
+ return p
+}then both callers collapse to if last, ok := spoke.LatestClosedAttempt(h); ok && now.Sub(last.EndedAt) <= spoke.HistoryRecentFor { return spoke.ProgressFromAttempt(last, nextWindow, now) }. That also retires orDashDashboard and itoaDashboard (dashboard/upgrade_progress.go:604-611) — the latter is a one-line rename of strconv.Itoa with no callers outside this file.
**2. UpgradeAttempt.EndedAt time.Time \json:"endedAt,omitempty"` (spoke/upgrade_progress.go:76) — omitemptyis a no-op on a struct, so every open row is serialized with"endedAt":"0001-01-01T00:00:00Z"in/api/saas/my-hives.upgradeHistory, the heartbeat upgrade_history, and the on-disk file, while the sibling UpgradeProgress uses RFC3339 strings that really are omitted (:112). PR B's tooltip will have to special-case the zero time. Either make the field a string like UpgradeProgress`, or at least stop promising omission:
// EndedAt is the zero time while the attempt is still open.
EndedAt time.Time `json:"endedAt"`
— hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88
There was a problem hiding this comment.
Read the tree at e0723a1 (hub history store, both derivations, the heartbeat/sweep/arm call sites and the cmd/hive drain). The lifecycle wiring is sound: recordHeartbeatTransitions runs after s.mu.Unlock() (server.go:2722), the sweep and beginUpgrade paths mutate the registry before the next beat so recordUpgradeHistoryTransition cannot double-open or double-close, and start()'s same-row dedupe makes the observed path harmless even where it overlaps a hub-armed row. setKickTimes is correctly ordered after decide and clears on release.
One verified gap, in the state machine's precedence:
upgradeProgressFor reports missed for the whole duration of a retry after a failed upgrade — pkg/hub/upgrade_progress.go:61 checks e.UpgradeFailed before e.Upgrading (:76). beginUpgrade (saas_upgrade.go:60-75, called from the manual arm at :582) sets Upgrading=true but does not clear UpgradeFailed, and the heartbeat carries a prior failure forward until the spoke reports a new GitHash (server.go:2542-2545) — which is exactly when Upgrading drops too. So on "upgrade failed → operator clicks Upgrade", the card and /api/status say missed: <old error> through draining, pulling and rolling, and never show the live states this PR exists to surface. The orphan sweep's converged path does clear the flag (stale_upgrade.go:451), but the manual and auto-fire arms do not. Severity medium (display only, but it hides the countdown precisely on the retry an operator is watching). Cheapest fix is to gate the first branch on !e.Upgrading || e.UpgradeFailedAt.After(e.UpgradeStartedAt), or clear UpgradeFailed in beginUpgrade. upgrade_progress_test.go:32 covers UpgradeFailed alone, not UpgradeFailed && Upgrading.
Everything else I checked matched the body. Not a duplicate of #14968 — that one is stacked on this and carries only the UI commit on top.
— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88
Fixes #14950 (PR A of two — data model, history, hub/spoke payloads, tests. PR B will add the hub-card / spoke-dashboard pills, the history tooltip and the "Upgrade now (interrupt turns)" action.)
Motivated by #14856 / #14925: three hives silently missed two daily windows and neither dashboard said so.
What this adds
State machine as data (
pkg/hub/spoke/upgrade_progress.go— shared vocabulary):scheduled→draining→pulling→rolling→verifying→done|missed./api/saas/my-hivesrows gainupgradeState(*spoke.UpgradeProgress) andupgradeHistory, derived on read byupgradeProgressForfrom the registry latch (Upgrading,UpgradeDrain,SpokeReportedUpgrading,UpgradeFailed,LastHeartbeatvsUpgradeStartedAt), the SaaSHive schedule/hold reason and the persisted history. Draining carriesdrainDeadline/drainRemainingS(anchored on the beat that carried the report) andagentsInTurnwith per-agent elapsed:2/5 agents in turn: reviewer 38m, scanner 12m — proceeds in 40m.next_window_at— the hive's own daily/weekly slot regardless of channel (onstable,next_update_atis the promotion ETA, which is what made "overdue since" read as a mystery time) — andupgrade_history./api/status.upgradeProgress, derived at serve time (pkg/dashboard/upgrade_progress.go) from the live drain latch (newServer.SetUpgradeDrainSource, installed fromcmd/hive), the self-upgrade marker, the last landed outcome and the policy. The drain report now includesturns[](agent,elapsedS) from each held agent'sLastKick.Missed-window history (
pkg/hub/upgrade_history.go): per-hive JSON under/data/saas/upgrade-history/<id>.json, last 14 rows (HIVE_HUB_UPGRADE_HISTORY_MAX), leaf mutex, atomic write, loaded at startup, dropped on hive delete. Rows:target,from,trigger(scheduled/instant/manual/observed/window),startedAt,endedAt,outcome,reason. Recorded from the existing lifecycle points only — no parallel mechanism:handleUpgradeHive) and auto fire (triggerAutoUpgrades) open a row;recordHeartbeatTransitionsopens an observed row for a rollout the hub did not arm and closes rows on landed / spoke-reported failure / latch cleared;never completed after …: <reason>,abandoned after N attempts);noteScheduledUpgradeHold(reason)records onewindowrow per held calendar day (ET), updated in place if the reason changes; a fired window records the attempt instead.supersededwhen the next one opens.Tests
upgrade_history_test.go(start/end/retarget, observed rollout, dropped orphan miss, stale-open supersession, one-row-per-day window dedupe, rotation + env cap, persist/load/forget, path traversal),upgrade_progress_test.go(13-case state table, InFlight, policynext_window_aton a stable daily hive, row wiring, transition recorder), spokeFormatTurns/FormatShortDuration,cmd/hivedrainTurns, dashboardbuildUpgradeProgresstable +/api/statusround-trip (drain outranks marker,?fields=filter). Notime.Sleep. Rancheck-api-reference-citations.sh --fix.Docs
operator-reference.mdnew section "Scheduled-upgrade progress and history" (state table, drain window, history),release-channels.mdstep 4 cross-link,changelog.d/added-upgrade-progress-visibility.md.