fix(patch): gate phases on the window deadline, queue undelivered results, retry startup inventory
Agent Release / build (push) Successful in 3m40s
Agent Release / msi (push) Successful in 4m54s

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.
This commit is contained in:
2026-09-15 13:43:03 +00:00
parent ecd703d502
commit b5b9775d2b
12 changed files with 404 additions and 46 deletions
+42 -3
View File
@@ -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 {
+56
View File
@@ -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:]
}
}
+88
View File
@@ -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)
}
}
+21 -2
View File
@@ -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)
}
}
+27 -7
View File
@@ -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"))
+35
View File
@@ -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)
}
}
+23
View File
@@ -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)
}
+36
View File
@@ -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)
}
}
+9 -7
View File
@@ -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
+43 -26
View File
@@ -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
+7 -1
View File
@@ -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))