feat: answer ApplyUpdatesCmd with a PatchResult and reboot when the command asks

2026-09-15 08:09:40 +00:00
parent db75d7dcbd
commit ecd703d502
4 changed files with 143 additions and 19 deletions
@@ -93,8 +93,13 @@ too.

 ## What the agent will not do

- **It never reboots a host.** `ApplyUpdatesCmd` installs and stops there;
-  `inventory.reboot_required` reports that one is owed.
+- **It reboots a host only when told to and only when owed.** An
+  `ApplyUpdatesCmd` with `reboot_if_required` set, on a host whose OS reports a
+  reboot is owed, with at least 5 minutes left before `deadline_unix`, reboots
+  after a one-minute grace period (`shutdown -r +1`, `shutdown /r /t 60`), and
+  only once the `PatchResult` announcing it has been sent. Anything else
+  installs and stops there, and `inventory.reboot_required` reports what is
+  owed.
 - **It decides what it will not touch.** The protected workload set is computed
  and enforced agent-side - `vantage-agent.service`, `VantageAgent` on Windows,
  and its own container ID from `/proc/self/cgroup`. The control plane may name
@@ -124,6 +129,17 @@ need a PowerShell Gallery install on every host and fails on an air-gapped
 fleet. `CurrentVersion` is empty on Windows and `NewVersion` carries the KB
 article ID: a Windows update is not a version bump of a named package.

+## Patching
+
+`updates.Apply` takes a scope and a deadline and answers with the tail of the
+package manager's output. Security-only uses `--security` on dnf/yum,
+`zypper patch --category security`, the Security and Critical classifications
+on Windows, and for apt a temporary `SourceParts` directory holding only the
+`-security` suites (with `APT::Get::List-Cleanup=0`, or the reduced update
+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`.
+
 ## Two constants that mirror the control plane

 Neither can be shared - this is a separate module and the control plane's are
@@ -0,0 +1,96 @@
+package agentsync
+
+import (
+	"errors"
+	"log"
+	"time"
+
+	"gitea.hostxtra.co.uk/vantage/vantage-agent/internal/config"
+	grpcclient "gitea.hostxtra.co.uk/vantage/vantage-agent/internal/grpc"
+	"gitea.hostxtra.co.uk/vantage/vantage-agent/internal/updates"
+	"gitea.hostxtra.co.uk/vantage/vantage-shared/grpc/pb"
+)
+
+// minRebootLeeway is the least time that must remain in the window for the
+// agent to start a reboot. A reboot that lands after the window closes is the
+// outage the window existed to prevent.
+const minRebootLeeway = 5 * time.Minute
+
+// shouldReboot is the whole reboot decision. The agent reboots a host only
+// when the command asked, the OS reports a reboot is owed, and the window
+// still has room for it. A command with no deadline is a manual run, which
+// never reboots.
+func shouldReboot(requested, owed bool, now, deadline time.Time) bool {
+	if !requested || !owed || deadline.IsZero() {
+		return false
+	}
+	return deadline.Sub(now) >= minRebootLeeway
+}
+
+func handleApplyUpdates(send func(*pb.AgentMessage) error, cfg *config.Config, cmd *pb.ServerCommand) {
+	c := cmd.ApplyUpdates
+	var deadline time.Time
+	if c.DeadlineUnix > 0 {
+		deadline = time.Unix(c.DeadlineUnix, 0)
+	}
+	log.Printf("applying OS updates (cmd=%s scope=%q reboot=%v)", cmd.CommandId, c.Scope, c.RebootIfRequired)
+
+	res, err := updates.Apply(updates.ApplyOptions{SecurityOnly: c.Scope == pb.PatchScopeSecurity, Deadline: deadline})
+	pr := &pb.PatchResult{CommandId: cmd.CommandId, OutputTail: res.Output, PendingAfter: -1}
+	switch {
+	case errors.Is(err, updates.ErrBusy):
+		pr.Status, pr.Message = pb.PatchStatusBusy, err.Error()
+	case err != nil:
+		pr.Status, pr.Message = pb.PatchStatusFailed, err.Error()
+	case res.Unsupported:
+		pr.Status, pr.Message = pb.PatchStatusUnsupported, res.Reason
+	default:
+		pr.Status = pb.PatchStatusOK
+	}
+
+	// Refresh the pending list whether the run succeeded or not, so the counts
+	// the operator sees are this host's real state. A busy refusal changed
+	// nothing, and the run in progress will report for itself.
+	if pr.Status != pb.PatchStatusBusy {
+		pr.PendingAfter = int32(reportPendingUpdates(cfg))
+	}
+	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)
+		// A reboot nobody was told about looks like a crash. Without a
+		// delivered result, do not reboot.
+		return
+	}
+	log.Printf("patch result sent (cmd=%s status=%s pending_after=%d rebooting=%v)", cmd.CommandId, pr.Status, pr.PendingAfter, pr.Rebooting)
+	if pr.Rebooting {
+		if err := updates.ScheduleReboot(); err != nil {
+			log.Printf("schedule reboot (cmd=%s): %v", cmd.CommandId, err)
+		}
+	}
+}
+
+// reportPendingUpdates re-checks pending updates and reports them, returning
+// the count, or -1 if either step failed.
+func reportPendingUpdates(cfg *config.Config) int {
+	pkgs, err := updates.CheckAvailable()
+	if err != nil {
+		log.Printf("post-apply update check: %v", err)
+		return -1
+	}
+	list := make([]pb.PackageUpdate, len(pkgs))
+	for i, p := range pkgs {
+		list[i] = pb.PackageUpdate{Name: p.Name, CurrentVersion: p.CurrentVersion, NewVersion: p.NewVersion}
+	}
+	client, err := grpcclient.New(cfg.ServerURL, cfg.TLS)
+	if err != nil {
+		log.Printf("post-apply report dial: %v", err)
+		return len(pkgs)
+	}
+	defer client.Close()
+	if err := client.ReportUpdates(cfg.ServerID, cfg.AgentToken, list); err != nil {
+		log.Printf("post-apply ReportUpdates: %v", err)
+	}
+	return len(pkgs)
+}
@@ -0,0 +1,28 @@
+package agentsync
+
+import (
+	"testing"
+	"time"
+)
+
+func TestShouldReboot(t *testing.T) {
+	now := time.Date(2026, 9, 20, 2, 30, 0, 0, time.UTC)
+	cases := []struct {
+		name            string
+		requested, owed bool
+		deadline        time.Time
+		want            bool
+	}{
+		{"asked, owed, plenty of time", true, true, now.Add(time.Hour), true},
+		{"not asked", false, true, now.Add(time.Hour), false},
+		{"nothing owed", true, false, now.Add(time.Hour), false},
+		{"exactly five minutes left", true, true, now.Add(5 * time.Minute), true},
+		{"under five minutes left", true, true, now.Add(4*time.Minute + 59*time.Second), false},
+		{"no deadline (manual run)", true, true, time.Time{}, false},
+	}
+	for _, c := range cases {
+		if got := shouldReboot(c.requested, c.owed, now, c.deadline); got != c.want {
+			t.Errorf("%s: got %v, want %v", c.name, got, c.want)
+		}
+	}
+}
@@ -341,7 +341,7 @@ func connectAndHandleStream(ctx context.Context, cfg *config.Config) error {
 			go handleUpdateAgent(cmd)
 		}
 		if cmd.ApplyUpdates != nil {
-			go handleApplyUpdates(cfg, cmd)
+			go handleApplyUpdates(send, cfg, cmd)
 		}
 		if cmd.CleanupWorkspace != nil {
 			go handleCleanupWorkspace(cmd)
@@ -480,22 +480,6 @@ func runInventory(ctx context.Context, cfg *config.Config) {
 	}
 }

-func handleApplyUpdates(cfg *config.Config, cmd *pb.ServerCommand) {
-	log.Printf("applying OS updates (cmd=%s)…", cmd.CommandId)
-	if _, err := updates.Apply(updates.ApplyOptions{}); err != nil {
-		log.Printf("OS upgrade failed (cmd=%s): %v", cmd.CommandId, err)
-		return
-	}
-	log.Printf("OS updates applied successfully (cmd=%s)", cmd.CommandId)
-
-	client, err := grpcclient.New(cfg.ServerURL, cfg.TLS)
-	if err != nil {
-		return
-	}
-	defer client.Close()
-	_ = client.ReportUpdates(cfg.ServerID, cfg.AgentToken, nil)
-}
-
 func handleCleanupWorkspace(cmd *pb.ServerCommand) {
 	id := cmd.CleanupWorkspace.WorkspaceId
 	dir := agentexec.WorkspacePath(id)