From b5b9775d2b59d1f83e270cef0e8bc13589ca4201 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Tue, 15 Sep 2026 13:43:03 +0000 Subject: [PATCH] fix(patch): gate phases on the window deadline, queue undelivered results, retry startup inventory The window deadline no longer kills a running package manager: it only gates the start of each phase, and a started upgrade runs under a 2 hour backstop that sends SIGTERM on Linux. A PatchResult whose send fails is queued and flushed on the next command stream, retaking the reboot decision. deb822 folded Suites continuation lines are filtered with the field. The startup static inventory report is retried until it succeeds. --- CLAUDE.md | 17 ++++++ internal/sync/patch.go | 45 ++++++++++++++- internal/sync/queue.go | 56 ++++++++++++++++++ internal/sync/queue_test.go | 88 +++++++++++++++++++++++++++++ internal/sync/sync.go | 23 +++++++- internal/updates/aptsec.go | 34 ++++++++--- internal/updates/aptsec_test.go | 35 ++++++++++++ internal/updates/phase.go | 23 ++++++++ internal/updates/phase_test.go | 36 ++++++++++++ internal/updates/updates.go | 16 +++--- internal/updates/updates_linux.go | 69 +++++++++++++--------- internal/updates/updates_windows.go | 8 ++- 12 files changed, 404 insertions(+), 46 deletions(-) create mode 100644 internal/sync/queue.go create mode 100644 internal/sync/queue_test.go create mode 100644 internal/updates/phase.go create mode 100644 internal/updates/phase_test.go diff --git a/CLAUDE.md b/CLAUDE.md index e04a21e..cdc40cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -140,6 +140,23 @@ deletes every other list file). apk and pacman have no security metadata and report `unsupported`; security-only never falls back to installing everything. One run at a time: a second command answers `busy`. +The deadline (`deadline_unix`, the window end) only gates the **start** of each +phase: the apt index refresh, the upgrade command, the Windows install script. +A phase that has not started by then is refused with "the maintenance window +ended before could start" (`canStart` in `internal/updates/phase.go`). +A started upgrade is never killed by the window: it runs under a 2 hour +backstop from its own start (`defaultApplyCap`), which on Linux sends SIGTERM +and waits 5 minutes before a kill. Interrupting a package manager mid-transaction +is worse than letting it finish late. + +A `PatchResult` whose send fails (the command stream reconnected while the +patch ran) is kept in a bounded queue (32, oldest dropped) and flushed on the +next stream right after `AgentReady`. A queued result that announced a reboot +has the reboot decision taken again at flush time, and the host still reboots +only once the result is delivered. The startup static inventory report is +retried every 30 seconds, up to 10 attempts: it carries the boot time the +control plane uses to prove a patch reboot. + ## Two constants that mirror the control plane Neither can be shared - this is a separate module and the control plane's are diff --git a/internal/sync/patch.go b/internal/sync/patch.go index 1d1f812..3c5cfce 100644 --- a/internal/sync/patch.go +++ b/internal/sync/patch.go @@ -57,10 +57,15 @@ func handleApplyUpdates(send func(*pb.AgentMessage) error, cfg *config.Config, c pr.RebootRequired = updates.RebootRequired() pr.Rebooting = pr.Status == pb.PatchStatusOK && shouldReboot(c.RebootIfRequired, pr.RebootRequired, time.Now(), deadline) - if err := send(&pb.AgentMessage{ServerId: cfg.ServerID, AgentToken: cfg.AgentToken, PatchResult: pr}); err != nil { - log.Printf("send patch result (cmd=%s): %v", cmd.CommandId, err) + msg := &pb.AgentMessage{ServerId: cfg.ServerID, AgentToken: cfg.AgentToken, PatchResult: pr} + if err := send(msg); err != nil { + log.Printf("send patch result (cmd=%s): %v; keeping it for the next command stream", cmd.CommandId, err) // A reboot nobody was told about looks like a crash. Without a - // delivered result, do not reboot. + // delivered result, do not reboot: the queued result retakes the + // decision when it is finally sent. + if pendingResults.push(pendingResult{msg: msg, requested: c.RebootIfRequired, deadline: deadline}) { + log.Printf("patch result queue full: dropped the oldest undelivered result") + } return } log.Printf("patch result sent (cmd=%s status=%s pending_after=%d rebooting=%v)", cmd.CommandId, pr.Status, pr.PendingAfter, pr.Rebooting) @@ -71,6 +76,40 @@ func handleApplyUpdates(send func(*pb.AgentMessage) error, cfg *config.Config, c } } +// pendingResults holds PatchResults whose send failed, typically because the +// command stream reconnected while a patch ran. The server guards result +// writes on status and command id, so a late result is safe to deliver. +var pendingResults = &resultQueue{max: maxPendingResults} + +// flushPendingResults delivers queued results on a newly established stream. +// A queued result that announced a reboot has that decision taken again now: +// time has passed, so the window may be too close to its end, or the reboot +// may no longer be owed. Rebooting is cleared before sending when it no +// longer holds, and the host reboots only once the result is delivered. +func flushPendingResults(send func(*pb.AgentMessage) error) { + pendingResults.flush(func(p pendingResult) error { + pr := p.msg.PatchResult + if pr.Rebooting { + owed := updates.RebootRequired() + pr.RebootRequired = owed + if !shouldReboot(p.requested, owed, time.Now(), p.deadline) { + pr.Rebooting = false + } + } + if err := send(p.msg); err != nil { + log.Printf("send queued patch result (cmd=%s): %v", pr.CommandId, err) + return err + } + log.Printf("queued patch result sent (cmd=%s status=%s rebooting=%v)", pr.CommandId, pr.Status, pr.Rebooting) + if pr.Rebooting { + if err := updates.ScheduleReboot(); err != nil { + log.Printf("schedule reboot (cmd=%s): %v", pr.CommandId, err) + } + } + return nil + }) +} + // reportPendingUpdates re-checks pending updates and reports them, returning // the count, or -1 if either step failed. func reportPendingUpdates(cfg *config.Config) int { diff --git a/internal/sync/queue.go b/internal/sync/queue.go new file mode 100644 index 0000000..e85e0c6 --- /dev/null +++ b/internal/sync/queue.go @@ -0,0 +1,56 @@ +package agentsync + +import ( + "sync" + "time" + + "gitea.hostxtra.co.uk/vantage/vantage-shared/grpc/pb" +) + +// maxPendingResults bounds the results kept for a later stream. A host that +// cannot reach the control plane for a very long time drops its oldest +// results rather than growing without limit. +const maxPendingResults = 32 + +// pendingResult is a PatchResult that could not be sent, with what the reboot +// decision needs to be taken again when it finally can be. +type pendingResult struct { + msg *pb.AgentMessage + requested bool // the command's RebootIfRequired + deadline time.Time // the command's deadline, zero for a manual run +} + +// resultQueue holds undelivered PatchResults until the next command stream. +type resultQueue struct { + mu sync.Mutex + items []pendingResult + max int +} + +// push appends p, dropping the oldest entry when the queue is full. It +// reports whether an entry was dropped. +func (q *resultQueue) push(p pendingResult) bool { + q.mu.Lock() + defer q.mu.Unlock() + q.items = append(q.items, p) + if len(q.items) > q.max { + q.items = q.items[len(q.items)-q.max:] + return true + } + return false +} + +// flush sends the queued results oldest first, removing each once sent. It +// stops at the first failure and keeps that entry and everything after it +// for the next stream. The lock is held throughout so two streams never +// deliver the same result. +func (q *resultQueue) flush(send func(pendingResult) error) { + q.mu.Lock() + defer q.mu.Unlock() + for len(q.items) > 0 { + if err := send(q.items[0]); err != nil { + return + } + q.items = q.items[1:] + } +} diff --git a/internal/sync/queue_test.go b/internal/sync/queue_test.go new file mode 100644 index 0000000..5a2e932 --- /dev/null +++ b/internal/sync/queue_test.go @@ -0,0 +1,88 @@ +package agentsync + +import ( + "errors" + "testing" + + "gitea.hostxtra.co.uk/vantage/vantage-shared/grpc/pb" +) + +func pending(id string) pendingResult { + return pendingResult{msg: &pb.AgentMessage{PatchResult: &pb.PatchResult{CommandId: id}}} +} + +func ids(q *resultQueue) []string { + q.mu.Lock() + defer q.mu.Unlock() + var out []string + for _, p := range q.items { + out = append(out, p.msg.PatchResult.CommandId) + } + return out +} + +func TestResultQueueEnqueue(t *testing.T) { + q := &resultQueue{max: 3} + q.push(pending("a")) + q.push(pending("b")) + if got := ids(q); len(got) != 2 || got[0] != "a" || got[1] != "b" { + t.Fatalf("got %v", got) + } +} + +func TestResultQueueBoundDropsOldest(t *testing.T) { + q := &resultQueue{max: 2} + q.push(pending("a")) + q.push(pending("b")) + if dropped := q.push(pending("c")); !dropped { + t.Fatal("push over the bound must report a drop") + } + if got := ids(q); len(got) != 2 || got[0] != "b" || got[1] != "c" { + t.Fatalf("got %v, want [b c]", got) + } +} + +func TestResultQueueDefaultBound(t *testing.T) { + if maxPendingResults != 32 { + t.Fatalf("maxPendingResults = %d, want 32", maxPendingResults) + } +} + +func TestResultQueueFlushOrderAndRemoval(t *testing.T) { + q := &resultQueue{max: 8} + for _, id := range []string{"a", "b", "c"} { + q.push(pending(id)) + } + var sent []string + q.flush(func(p pendingResult) error { + sent = append(sent, p.msg.PatchResult.CommandId) + return nil + }) + if len(sent) != 3 || sent[0] != "a" || sent[1] != "b" || sent[2] != "c" { + t.Fatalf("flush order %v, want [a b c]", sent) + } + if got := ids(q); len(got) != 0 { + t.Fatalf("sent entries must be removed, left %v", got) + } +} + +func TestResultQueueFlushRetainsOnFailure(t *testing.T) { + q := &resultQueue{max: 8} + for _, id := range []string{"a", "b", "c"} { + q.push(pending(id)) + } + var sent []string + q.flush(func(p pendingResult) error { + if p.msg.PatchResult.CommandId == "b" { + return errors.New("stream gone") + } + sent = append(sent, p.msg.PatchResult.CommandId) + return nil + }) + if len(sent) != 1 || sent[0] != "a" { + t.Fatalf("sent %v, want [a]", sent) + } + if got := ids(q); len(got) != 2 || got[0] != "b" || got[1] != "c" { + t.Fatalf("retained %v, want [b c]", got) + } +} diff --git a/internal/sync/sync.go b/internal/sync/sync.go index a7fea44..ea85a56 100644 --- a/internal/sync/sync.go +++ b/internal/sync/sync.go @@ -243,6 +243,11 @@ func connectAndHandleStream(ctx context.Context, cfg *config.Config) error { return stream.Send(msg) } + // Results that could not be sent on an earlier stream go out first. In + // its own goroutine: a queued reboot re-check can take a while on + // Windows, and the receive loop below must start promptly. + go flushPendingResults(send) + // Stream liveness, tracked here rather than left to gRPC keepalive. // // Keepalive operates on the transport, and behind an L7 proxy the transport @@ -446,7 +451,7 @@ func runInventory(ctx context.Context, cfg *config.Config) { } defer client.Close() - report := func(static bool) { + report := func(static bool) error { r := inventory.Collect(static) r.ServerId = cfg.ServerID r.AgentToken = cfg.AgentToken @@ -462,10 +467,18 @@ func runInventory(ctx context.Context, cfg *config.Config) { } if err := client.ReportInventory(r); err != nil { log.Printf("report inventory: %v", err) + return err } + return nil } - report(true) + // The startup static report carries the boot time the server uses to + // prove a patch reboot happened, so a failure is retried on the next + // ticks (every 30 seconds, up to startupStaticAttempts in total) instead + // of waiting a quarter of an hour for the next static snapshot. + const startupStaticAttempts = 10 + attempts := 1 + startupPending := report(true) != nil ticker := time.NewTicker(30 * time.Second) defer ticker.Stop() tick := 0 @@ -475,6 +488,12 @@ func runInventory(ctx context.Context, cfg *config.Config) { return case <-ticker.C: tick++ + if startupPending && attempts < startupStaticAttempts { + attempts++ + startupPending = report(true) != nil + continue + } + startupPending = false report(tick%30 == 0) } } diff --git a/internal/updates/aptsec.go b/internal/updates/aptsec.go index 40e6fd9..797b35a 100644 --- a/internal/updates/aptsec.go +++ b/internal/updates/aptsec.go @@ -68,14 +68,28 @@ func filterOneLine(content string) string { return b.String() } +// deb822Fields groups a paragraph's lines into fields. A line that starts +// with a space or tab continues the field above it (a folded or multi-line +// value, such as a long Suites list or an inline Signed-By key block). +func deb822Fields(para string) [][]string { + var fields [][]string + for _, l := range strings.Split(strings.Trim(para, "\n"), "\n") { + if (strings.HasPrefix(l, " ") || strings.HasPrefix(l, "\t")) && len(fields) > 0 { + fields[len(fields)-1] = append(fields[len(fields)-1], l) + continue + } + fields = append(fields, []string{l}) + } + return fields +} + func filterDeb822(content string) string { var b strings.Builder for _, para := range strings.Split(strings.ReplaceAll(content, "\r\n", "\n"), "\n\n") { - lines := strings.Split(strings.Trim(para, "\n"), "\n") var out []string isDeb, enabled, kept := false, true, false - for _, l := range lines { - key, val, found := strings.Cut(l, ":") + for _, field := range deb822Fields(para) { + key, val, found := strings.Cut(field[0], ":") k := strings.ToLower(strings.TrimSpace(key)) v := strings.TrimSpace(val) switch { @@ -88,19 +102,25 @@ func filterDeb822(content string) string { case found && k == "enabled": enabled = strings.ToLower(v) != "no" case found && k == "suites": + // The value runs across every continuation line. The filtered + // result is written back as one line and the continuation + // lines are dropped with the rest of the original field. + all := strings.Fields(strings.Join(append([]string{v}, field[1:]...), " ")) var sec []string - for _, s := range strings.Fields(v) { + for _, s := range all { if isSecuritySuite(s) { sec = append(sec, s) } } if len(sec) == 0 { - continue // drop the line; the paragraph is dropped below + continue // drop the field; the paragraph is dropped below } kept = true - l = "Suites: " + strings.Join(sec, " ") + out = append(out, "Suites: "+strings.Join(sec, " ")) + continue } - out = append(out, l) + // Every other field, continuation lines included, stays verbatim. + out = append(out, field...) } if isDeb && enabled && kept { b.WriteString(strings.Join(out, "\n")) diff --git a/internal/updates/aptsec_test.go b/internal/updates/aptsec_test.go index 7d099ba..2d71a52 100644 --- a/internal/updates/aptsec_test.go +++ b/internal/updates/aptsec_test.go @@ -78,3 +78,38 @@ func TestSecuritySourcesNone(t *testing.T) { t.Fatal("ok = true with no security suite, want false") } } + +// A folded Suites field continues on lines that start with whitespace. Those +// lines belong to Suites and must be filtered with it, not copied verbatim. +func TestSecuritySourcesDeb822FoldedSuites(t *testing.T) { + files := map[string]string{"/x.sources": "Types: deb\nURIs: http://a/\nSuites: noble\n noble-security\nComponents: main\n"} + _, d822, ok := securitySources(files) + if !ok || !strings.Contains(d822, "Suites: noble-security\n") { + t.Fatalf("got ok=%v\n%s", ok, d822) + } + if strings.Contains(d822, "\n noble") { + t.Fatalf("the folded continuation line must be dropped:\n%s", d822) + } +} + +// A folded Suites field with no security suite on any line drops the paragraph. +func TestSecuritySourcesDeb822FoldedSuitesNoSecurity(t *testing.T) { + files := map[string]string{"/x.sources": "Types: deb\nURIs: http://a/\nSuites: noble\n\tnoble-updates\nComponents: main\n"} + if _, _, ok := securitySources(files); ok { + t.Fatal("no security suite across the folded lines, want ok=false") + } +} + +// An inline Signed-By key block is a multi-line field of its own. Its +// continuation lines stay verbatim, including the "." blank-line marker. +func TestSecuritySourcesDeb822InlineSignedBy(t *testing.T) { + key := "Signed-By: -----BEGIN PGP PUBLIC KEY BLOCK-----\n .\n mQINBGRkZXYBEAC\n -----END PGP PUBLIC KEY BLOCK-----\n" + files := map[string]string{"/x.sources": "Types: deb\nURIs: http://a/\nSuites: noble noble-security\nComponents: main\n" + key} + _, d822, ok := securitySources(files) + if !ok || !strings.Contains(d822, "Suites: noble-security\n") { + t.Fatalf("got ok=%v\n%s", ok, d822) + } + if !strings.Contains(d822, key) { + t.Fatalf("inline key block must be kept verbatim:\n%s", d822) + } +} diff --git a/internal/updates/phase.go b/internal/updates/phase.go new file mode 100644 index 0000000..1e50086 --- /dev/null +++ b/internal/updates/phase.go @@ -0,0 +1,23 @@ +package updates + +import ( + "fmt" + "time" +) + +// canStart is the whole "may this phase begin" decision. The maintenance +// window deadline only gates the start of a phase: a package manager that is +// already running is never interrupted by it, because killing apt, dnf or +// Windows Update partway through a transaction is worse than letting it +// finish late. A zero deadline is a manual run, which always may start. +func canStart(now, deadline time.Time) bool { + return deadline.IsZero() || now.Before(deadline) +} + +// startGate returns the error reported when a phase is refused. +func startGate(deadline time.Time, phase string) error { + if canStart(time.Now(), deadline) { + return nil + } + return fmt.Errorf("the maintenance window ended before %s could start", phase) +} diff --git a/internal/updates/phase_test.go b/internal/updates/phase_test.go new file mode 100644 index 0000000..2ddcdd1 --- /dev/null +++ b/internal/updates/phase_test.go @@ -0,0 +1,36 @@ +package updates + +import ( + "strings" + "testing" + "time" +) + +func TestCanStart(t *testing.T) { + now := time.Date(2026, 9, 20, 2, 30, 0, 0, time.UTC) + cases := []struct { + name string + deadline time.Time + want bool + }{ + {"no deadline (manual run)", time.Time{}, true}, + {"deadline ahead", now.Add(time.Second), true}, + {"deadline exactly now", now, false}, + {"deadline passed", now.Add(-time.Minute), false}, + } + for _, c := range cases { + if got := canStart(now, c.deadline); got != c.want { + t.Errorf("%s: got %v, want %v", c.name, got, c.want) + } + } +} + +func TestStartGate(t *testing.T) { + if err := startGate(time.Time{}, "the upgrade"); err != nil { + t.Fatalf("zero deadline: %v", err) + } + err := startGate(time.Now().Add(-time.Minute), "the upgrade") + if err == nil || !strings.Contains(err.Error(), "the maintenance window ended before the upgrade could start") { + t.Fatalf("passed deadline: %v", err) + } +} diff --git a/internal/updates/updates.go b/internal/updates/updates.go index 99d3378..4b54cb2 100644 --- a/internal/updates/updates.go +++ b/internal/updates/updates.go @@ -26,8 +26,11 @@ type ApplyOptions struct { // metadata reports Unsupported and installs nothing: it never falls back // to installing everything. SecurityOnly bool - // Deadline is when the upgrade must be finished, normally the end of the - // maintenance window. Zero means defaultApplyCap from now. + // Deadline is the end of the maintenance window. It only gates the start + // of each phase (index refresh, upgrade, Windows install): a phase that + // has not started by then is not started, and one already running is + // allowed to finish, bounded by defaultApplyCap from its own start. Zero + // means a manual run with no window. Deadline time.Time } @@ -42,6 +45,9 @@ type Result struct { // ErrBusy means another Apply is already running on this host. var ErrBusy = errors.New("an update run is already in progress on this host") +// defaultApplyCap is the backstop for one started upgrade command, counted +// from that command's own start. It exists for a package manager that hangs, +// not to enforce the window. const defaultApplyCap = 2 * time.Hour var applyMu sync.Mutex @@ -53,11 +59,7 @@ func Apply(opts ApplyOptions) (Result, error) { return Result{}, ErrBusy } defer applyMu.Unlock() - deadline := opts.Deadline - if deadline.IsZero() { - deadline = time.Now().Add(defaultApplyCap) - } - return apply(opts.SecurityOnly, deadline) + return apply(opts.SecurityOnly, opts.Deadline) } // ScheduleReboot restarts the host after a short grace period, so a result diff --git a/internal/updates/updates_linux.go b/internal/updates/updates_linux.go index 2491a37..e7f1f95 100644 --- a/internal/updates/updates_linux.go +++ b/internal/updates/updates_linux.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "strings" + "syscall" "time" ) @@ -26,8 +27,6 @@ func detectPM() string { return "" } - - func checkAvailable() ([]PackageUpdate, error) { switch detectPM() { case "apt": @@ -47,30 +46,53 @@ func checkAvailable() ([]PackageUpdate, error) { } } - // aptRefreshTimeout bounds the index refresh alone. The upgrade itself runs -// until the caller's deadline: one shared five-minute limit used to kill large +// under defaultApplyCap: one shared five-minute limit used to kill large // upgrades partway through. const aptRefreshTimeout = 5 * time.Minute -func run(ctx context.Context, out io.Writer, env []string, name string, args ...string) error { - fmt.Fprintf(out, "$ %s %s\n", name, strings.Join(args, " ")) +// termGrace is how long a command has to exit after SIGTERM before it is +// killed. Package managers finish or roll back their current step on TERM. +const termGrace = 5 * time.Minute + +// phase is one package manager command, started only if the window deadline +// has not passed and then bounded by its own limit from its own start. +type phase struct { + deadline time.Time + out io.Writer + env []string +} + +func (p phase) run(name string, limit time.Duration, args ...string) error { + return p.runNamed("the upgrade", name, limit, args...) +} + +func (p phase) runNamed(label, name string, limit time.Duration, args ...string) error { + if err := startGate(p.deadline, label); err != nil { + return err + } + ctx, cancel := context.WithTimeout(context.Background(), limit) + defer cancel() + fmt.Fprintf(p.out, "$ %s %s\n", name, strings.Join(args, " ")) cmd := exec.CommandContext(ctx, name, args...) - cmd.Stdout, cmd.Stderr = out, out - cmd.Env = append(os.Environ(), env...) + cmd.Stdout, cmd.Stderr = p.out, p.out + cmd.Env = append(os.Environ(), p.env...) + // On the backstop, ask the package manager to stop rather than killing it + // outright, and give it time to leave its database consistent. + cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } + cmd.WaitDelay = termGrace return cmd.Run() } func apply(securityOnly bool, deadline time.Time) (Result, error) { out := newTailBuffer(outputTailMax) - ctx, cancel := context.WithDeadline(context.Background(), deadline) - defer cancel() + ph := phase{deadline: deadline, out: out} var err error switch pm := detectPM(); pm { case "apt": var res Result - res, err = applyApt(ctx, out, securityOnly) + res, err = applyApt(deadline, out, securityOnly) if res.Unsupported { res.Output = out.String() return res, nil @@ -80,12 +102,12 @@ func apply(securityOnly bool, deadline time.Time) (Result, error) { if securityOnly { args = append(args, "--security") } - err = run(ctx, out, nil, pm, args...) + err = ph.run(pm, defaultApplyCap, args...) case "zypper": if securityOnly { - err = run(ctx, out, nil, "zypper", "--non-interactive", "patch", "--category", "security") + err = ph.run("zypper", defaultApplyCap, "--non-interactive", "patch", "--category", "security") } else { - err = run(ctx, out, nil, "zypper", "--non-interactive", "update") + err = ph.run("zypper", defaultApplyCap, "--non-interactive", "update") } // 102 and 103 mean "installed, and a reboot or restart is now needed". // That is success; RebootRequired reports the rest. @@ -97,25 +119,22 @@ func apply(securityOnly bool, deadline time.Time) (Result, error) { if securityOnly { return Result{Unsupported: true, Reason: "pacman publishes no security metadata"}, nil } - err = run(ctx, out, nil, "pacman", "-Syu", "--noconfirm") + err = ph.run("pacman", defaultApplyCap, "-Syu", "--noconfirm") case "apk": if securityOnly { return Result{Unsupported: true, Reason: "apk publishes no security metadata"}, nil } - if err = run(ctx, out, nil, "apk", "update"); err == nil { - err = run(ctx, out, nil, "apk", "upgrade") + if err = ph.runNamed("the apk index refresh", "apk", aptRefreshTimeout, "update"); err == nil { + err = ph.run("apk", defaultApplyCap, "upgrade") } default: return Result{Unsupported: true, Reason: "no supported package manager found"}, nil } - if ctx.Err() == context.DeadlineExceeded { - err = fmt.Errorf("stopped at the end of the maintenance window: %w", err) - } return Result{Output: out.String()}, err } -func applyApt(ctx context.Context, out io.Writer, securityOnly bool) (Result, error) { - env := []string{"DEBIAN_FRONTEND=noninteractive"} +func applyApt(deadline time.Time, out io.Writer, securityOnly bool) (Result, error) { + ph := phase{deadline: deadline, out: out, env: []string{"DEBIAN_FRONTEND=noninteractive"}} var srcOpts []string if securityOnly { dir, res, err := writeSecuritySourceParts() @@ -131,15 +150,13 @@ func applyApt(ctx context.Context, out io.Writer, securityOnly bool) (Result, er "-o", "APT::Get::List-Cleanup=0", } } - rctx, rcancel := context.WithTimeout(ctx, aptRefreshTimeout) - defer rcancel() - if err := run(rctx, out, env, "apt-get", append([]string{"update", "-q"}, srcOpts...)...); err != nil { + if err := ph.runNamed("the apt index refresh", "apt-get", aptRefreshTimeout, append([]string{"update", "-q"}, srcOpts...)...); err != nil { return Result{}, fmt.Errorf("apt-get update: %w", err) } args := []string{"upgrade", "-y", "-q", "-o", "Dpkg::Options::=--force-confdef", "-o", "Dpkg::Options::=--force-confold"} - return Result{}, run(ctx, out, env, "apt-get", append(args, srcOpts...)...) + return Result{}, ph.runNamed("the upgrade", "apt-get", defaultApplyCap, append(args, srcOpts...)...) } // writeSecuritySourceParts writes the security-only sources to a temporary diff --git a/internal/updates/updates_windows.go b/internal/updates/updates_windows.go index 9a9da4a..26c6d6b 100644 --- a/internal/updates/updates_windows.go +++ b/internal/updates/updates_windows.go @@ -62,7 +62,13 @@ func checkAvailable() ([]PackageUpdate, error) { } func apply(securityOnly bool, deadline time.Time) (Result, error) { - ctx, cancel := context.WithDeadline(context.Background(), deadline) + if err := startGate(deadline, "the Windows Update install"); err != nil { + return Result{}, err + } + // The window deadline only gates the start. Once running, the install is + // bounded by defaultApplyCap from its own start, with the default kill: + // Windows has no SIGTERM to offer PowerShell. + ctx, cancel := context.WithTimeout(context.Background(), defaultApplyCap) defer cancel() out, err := winexec.Run(ctx, applyScriptFor(securityOnly))