fix(patching): final review fixes
Chart Release / chart (push) Successful in 19s
Server Deploy / deploy (push) Successful in 6m12s

- no dispatch in the last 15 minutes of a window; no-result timeout from dispatch time
- per-server output moves to patch_run_outputs (16MB document limit)
- reboot proven by a changed boot time; RebootTimeout 45m, ResultGrace 20m
- window update and delete are server-scoped against the policies using them
- scheduler puts the claim back on an error after it, so the next tick retries
- cancelled runs with failures alert; MCP apply_updates audits per server
- apply-updates 503 body documented; openapi regenerated
- web: cleared numeric fields no longer save as 0; Run now asks for confirmation
This commit is contained in:
2026-09-15 13:49:28 +00:00
parent 3ecea7c39f
commit 3f2d20868e
22 changed files with 436 additions and 71 deletions
+36 -11
View File
@@ -20,9 +20,19 @@ import (
const MinAgentVersion = "1.4.0"
const (
ResultGrace = 10 * time.Minute // after the window deadline, before "no result"
ManualTimeout = 2 * time.Hour // the agent's own cap when no deadline is sent
RebootTimeout = 20 * time.Minute // for a post-boot inventory report
// ResultGrace covers what the agent does after the upgrade and before it
// answers: a pending-update re-check of up to 10 minutes (Windows Update
// search) plus a 2 minute reboot check, with room to spare.
ResultGrace = 20 * time.Minute
// ManualTimeout is the agent's backstop for one started upgrade, counted
// from its own start. The window end never stops a running upgrade.
ManualTimeout = 2 * time.Hour
// RebootTimeout is how long a rebooting server has to send a post-boot
// inventory report. Windows cumulative updates routinely take over 20.
RebootTimeout = 45 * time.Minute
// LatestStartBeforeEnd is the tail of a window in which no server starts
// patching: a late start would run long past the window end.
LatestStartBeforeEnd = 15 * time.Minute
)
// AgentSupportsPatchResults compares major.minor.patch. Empty, "dev" and
@@ -88,6 +98,9 @@ func Advance(run models.PatchRun, now time.Time, connected map[string]bool) []Tr
return nil
}
windowOpen := run.WindowEnd == nil || now.Before(*run.WindowEnd)
// In the last LatestStartBeforeEnd of a window nothing new starts: queued
// and waiting servers simply wait, and close at WindowEnd as usual.
mayStart := run.WindowEnd == nil || now.Before(run.WindowEnd.Add(-LatestStartBeforeEnd))
inFlight := 0
for _, s := range run.Servers {
@@ -109,6 +122,8 @@ func Advance(run models.PatchRun, now time.Time, connected map[string]bool) []Tr
to = models.PatchSrvMissedOffline
}
out = append(out, Transition{ServerID: s.ServerID, From: s.Status, To: to})
case !mayStart:
// The window tail: no dispatch, no transition.
case run.MaxConcurrent > 0 && inFlight >= run.MaxConcurrent:
// No slot this tick.
case !connected[s.ServerID]:
@@ -125,17 +140,17 @@ func Advance(run models.PatchRun, now time.Time, connected map[string]bool) []Tr
}
case models.PatchSrvRebooting:
if s.RebootedAt != nil && now.After(s.RebootedAt.Add(RebootTimeout)) {
out = append(out, Transition{ServerID: s.ServerID, From: s.Status, To: models.PatchSrvFailed, Error: "did not come back within 20 minutes"})
out = append(out, Transition{ServerID: s.ServerID, From: s.Status, To: models.PatchSrvFailed, Error: fmt.Sprintf("did not come back within %d minutes", int(RebootTimeout.Minutes()))})
}
}
}
return out
}
// resultDeadline is the same for windowed and manual runs: the agent lets a
// started upgrade finish past the window end, so the window end says nothing
// about when a result is due. The base is the server's dispatch time.
func resultDeadline(run models.PatchRun, s models.PatchServerRun) time.Time {
if run.WindowEnd != nil {
return run.WindowEnd.Add(ResultGrace)
}
start := run.StartedAt
if s.StartedAt != nil {
start = *s.StartedAt
@@ -176,11 +191,21 @@ func ApplyResult(s models.PatchServerRun, r *pb.PatchResult, now time.Time) (mod
return s, true
}
// VerifyReboot settles a rebooting server from a static inventory report. Only
// a boot time later than the reboot command proves the host restarted: a
// snapshot sent during the one-minute grace period must not count.
// VerifyReboot settles a rebooting server from a static inventory report.
// When the boot time reported before the reboot is known, a later boot time
// is the proof: both come from the host clock, so skew against the server
// clock does not matter. Otherwise only a boot time later than the reboot
// command counts. Either way a snapshot sent during the one-minute grace
// period, before the host went down, does not.
func VerifyReboot(s models.PatchServerRun, bootTime time.Time, rebootRequired bool, now time.Time) (models.PatchServerRun, bool) {
if s.Status != models.PatchSrvRebooting || s.RebootedAt == nil || !bootTime.After(*s.RebootedAt) {
if s.Status != models.PatchSrvRebooting || s.RebootedAt == nil {
return s, false
}
proven := bootTime.After(*s.RebootedAt)
if s.BootTimeBefore != nil {
proven = bootTime.After(*s.BootTimeBefore)
}
if !proven {
return s, false
}
if rebootRequired {
+86 -10
View File
@@ -93,36 +93,90 @@ func TestAdvanceWindowCloses(t *testing.T) {
}
}
func TestAdvanceNoResultTimeout(t *testing.T) {
// A windowed run times out from the server's own dispatch time, not from the
// window end: a server dispatched late in the window may finish past it.
func TestAdvanceNoResultTimeoutFromDispatch(t *testing.T) {
run := windowRun(0, srv("a", models.PatchSrvPatching))
if ts := Advance(run, t0.Add(2*time.Hour+9*time.Minute), nil); len(ts) != 0 {
t.Fatalf("inside grace: %+v", ts)
run.Servers[0].StartedAt = tp(t0.Add(90 * time.Minute))
deadline := t0.Add(90*time.Minute + ManualTimeout + ResultGrace)
if ts := Advance(run, run.WindowEnd.Add(ResultGrace+time.Minute), nil); len(ts) != 0 {
t.Fatalf("past WindowEnd+grace but inside the dispatch timeout: %+v", ts)
}
ts := Advance(run, t0.Add(2*time.Hour+11*time.Minute), nil)
if ts := Advance(run, deadline.Add(-time.Minute), nil); len(ts) != 0 {
t.Fatalf("inside the dispatch timeout: %+v", ts)
}
ts := Advance(run, deadline.Add(time.Minute), nil)
if len(ts) != 1 || ts[0].To != models.PatchSrvFailed || ts[0].Error == "" {
t.Fatalf("past grace: %+v", ts)
t.Fatalf("past the dispatch timeout: %+v", ts)
}
}
// Without a server StartedAt the run's own start is the base.
func TestAdvanceNoResultTimeoutFallsBackToRunStart(t *testing.T) {
run := windowRun(0, srv("a", models.PatchSrvPatching))
if ts := Advance(run, t0.Add(2*time.Hour+19*time.Minute), nil); len(ts) != 0 {
t.Fatalf("inside timeout: %+v", ts)
}
if ts := Advance(run, t0.Add(2*time.Hour+21*time.Minute), nil); len(ts) != 1 || ts[0].To != models.PatchSrvFailed {
t.Fatalf("past timeout: %+v", ts)
}
}
func TestAdvanceManualRunTimeout(t *testing.T) {
run := models.PatchRun{Status: models.PatchRunRunning, StartedAt: t0, Servers: []models.PatchServerRun{srv("a", models.PatchSrvPatching)}}
run.Servers[0].StartedAt = tp(t0)
if ts := Advance(run, t0.Add(2*time.Hour+9*time.Minute), nil); len(ts) != 0 {
if ts := Advance(run, t0.Add(2*time.Hour+19*time.Minute), nil); len(ts) != 0 {
t.Fatalf("manual inside timeout: %+v", ts)
}
if ts := Advance(run, t0.Add(2*time.Hour+11*time.Minute), nil); len(ts) != 1 || ts[0].To != models.PatchSrvFailed {
if ts := Advance(run, t0.Add(2*time.Hour+21*time.Minute), nil); len(ts) != 1 || ts[0].To != models.PatchSrvFailed {
t.Fatalf("manual past timeout: %+v", ts)
}
}
func TestTimingConstants(t *testing.T) {
if ResultGrace != 20*time.Minute || RebootTimeout != 45*time.Minute || LatestStartBeforeEnd != 15*time.Minute {
t.Fatalf("ResultGrace=%v RebootTimeout=%v LatestStartBeforeEnd=%v", ResultGrace, RebootTimeout, LatestStartBeforeEnd)
}
}
// No server starts patching in the last 15 minutes of a window: it would
// either be cut short or run long past the window end.
func TestAdvanceNoDispatchInWindowTail(t *testing.T) {
run := windowRun(0, srv("q", models.PatchSrvQueued), srv("w", models.PatchSrvWaitingOffline), srv("o", models.PatchSrvQueued))
cutoff := run.WindowEnd.Add(-LatestStartBeforeEnd)
online := map[string]bool{"q": true, "w": true}
for _, at := range []time.Time{cutoff, cutoff.Add(time.Minute), run.WindowEnd.Add(-time.Second)} {
if ts := Advance(run, at, online); len(ts) != 0 {
t.Fatalf("at %v: nothing may change in the window tail, got %+v", at.Sub(t0), ts)
}
}
}
func TestAdvanceDispatchJustBeforeWindowTail(t *testing.T) {
run := windowRun(0, srv("q", models.PatchSrvQueued))
at := run.WindowEnd.Add(-LatestStartBeforeEnd - time.Second)
ts := Advance(run, at, map[string]bool{"q": true})
if len(ts) != 1 || !ts[0].Dispatch {
t.Fatalf("dispatch must be allowed just before the tail: %+v", ts)
}
}
// A manual run has no window and no tail.
func TestAdvanceManualRunHasNoTail(t *testing.T) {
run := models.PatchRun{Status: models.PatchRunRunning, StartedAt: t0, Servers: []models.PatchServerRun{srv("a", models.PatchSrvQueued)}}
if ts := Advance(run, t0.Add(10*time.Hour), map[string]bool{"a": true}); len(ts) != 1 || !ts[0].Dispatch {
t.Fatalf("manual run must dispatch: %+v", ts)
}
}
func TestAdvanceRebootTimeout(t *testing.T) {
run := windowRun(0, srv("a", models.PatchSrvRebooting))
run.Servers[0].RebootedAt = tp(t0)
if ts := Advance(run, t0.Add(19*time.Minute), nil); len(ts) != 0 {
if ts := Advance(run, t0.Add(44*time.Minute), nil); len(ts) != 0 {
t.Fatalf("inside reboot timeout: %+v", ts)
}
ts := Advance(run, t0.Add(21*time.Minute), nil)
if len(ts) != 1 || ts[0].To != models.PatchSrvFailed {
ts := Advance(run, t0.Add(46*time.Minute), nil)
if len(ts) != 1 || ts[0].To != models.PatchSrvFailed || ts[0].Error != "did not come back within 45 minutes" {
t.Fatalf("past reboot timeout: %+v", ts)
}
}
@@ -202,6 +256,28 @@ func TestVerifyReboot(t *testing.T) {
}
}
// With the boot time recorded before the reboot, a changed boot time is the
// proof, whatever the skew between the host clock and the server clock.
func TestVerifyRebootChangedBoot(t *testing.T) {
s := srv("a", models.PatchSrvRebooting)
s.RebootedAt = tp(t0)
s.BootTimeBefore = tp(t0.Add(-10 * 24 * time.Hour))
now := t0.Add(5 * time.Minute)
// The host clock runs 10 minutes slow: its new boot time reads earlier
// than the server's RebootedAt, yet the boot did change.
got, ok := VerifyReboot(s, t0.Add(-8*time.Minute), false, now)
if !ok || got.Status != models.PatchSrvSucceeded {
t.Fatalf("changed boot behind a slow clock must be proven: %+v ok=%v", got, ok)
}
// A fast host clock with an unchanged boot is not proof.
if _, ok := VerifyReboot(s, *s.BootTimeBefore, false, now); ok {
t.Error("an unchanged boot time is not proof")
}
if _, ok := VerifyReboot(s, s.BootTimeBefore.Add(-time.Minute), false, now); ok {
t.Error("an earlier boot time is not proof")
}
}
func TestFinalize(t *testing.T) {
mk := func(statuses ...string) models.PatchRun {
r := windowRun(0)