Agent: add host-driven binary upgrade - #592
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a host-driven agent binary activation path (a hidden unbounded-agent agent-upgrade [--preflight] command) and a reusable activation library that coordinates with the existing Kubernetes AgentUpgrade MachineOperation via a shared lock and signal gating. It also expands unit coverage and introduces an E2E Kind/QEMU validation for upgrading from a legacy single-binary deployment into the managed blue/green layout.
Changes:
- Introduce
pkg/agent/agentbinaryhost activation APIs (preflight + apply) with shared lock coordination. - Add a hidden
agent-upgradeCLI command and systemd adapter for host-driven upgrades, plus integrate lock contention handling into MachineOperation upgrades. - Extend tests and CI E2E coverage with a host-driven upgrade scenario.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/agent/goalstates/constants.go | Adds a shared host activation lock path constant. |
| pkg/agent/agentbinary/activation.go | New host-driven activation implementation (planning, install/switch, lock). |
| pkg/agent/agentbinary/activation_test.go | Unit tests for host activation planning, switching, rollback, and locking. |
| hack/agent/e2e-kind/e2e.py | Adds a host-driven upgrade E2E scenario validating legacy single-binary adoption. |
| designs/agent-upgrade.md | Documents host-driven agent-upgrade command and shared locking semantics. |
| cmd/agent/internal/daemon/lifecycle.go | Renders daemon assets using resolved upgrade paths (env overrides). |
| cmd/agent/internal/daemon/hostupgrade.go | Implements systemd adapter (preflight/prepare/restart/health) for host upgrades. |
| cmd/agent/internal/daemon/hostupgrade_test.go | Unit test ensuring host preflight rejects active MachineOperation signal. |
| cmd/agent/internal/daemon/controller.go | Wires shared activation lock path into the MachineOperation controller target. |
| cmd/agent/internal/daemon/controller_test.go | Adds lock contention test coverage for AgentUpgrade reconciliation. |
| cmd/agent/internal/daemon/controller_machineoperation.go | Acquires shared lock before staging/restarting; requeues on contention. |
| cmd/agent/internal/cmd/cmd.go | Registers the new hidden agent-upgrade command. |
| cmd/agent/internal/cmd/agentupgrade.go | Implements host-driven agent-upgrade --preflight and apply flow. |
| cmd/agent/internal/cmd/agentupgrade_test.go | Adds unit coverage for host-driven preflight output and non-mutation. |
| .github/workflows/agent-e2e-kind.yaml | Runs the new host-driven upgrade E2E step in CI. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Azure validation completed with an isolated one-node AKS v1.35.6 cluster and an Ubuntu 24.04 Azure VM using the branch-built agent. Validated on the VM:
The external kubelet registered with AKS but remained NotReady because this isolated VM was not integrated with AKS Azure CNI networking. The host binary upgrade and daemon activation path itself passed. The existing QEMU/Kind CI matrix separately validates Node Ready after activation on all three host/nspawn combinations. The temporary Azure resource group, AKS cluster, VM, public IP, and related resources were deleted after the run, and deletion was verified complete. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
pkg/agent/agentbinary/activation.go:250
- Rollback reuses the caller's context. If
RestartorWaitHealthyfailed because that context was canceled or its deadline expired, this restart immediately fails too, so only the symlink is reverted and the failed candidate may keep running. Perform rollback service operations with a bounded cleanup context derived fromcontext.WithoutCancel(ctx)so cancellation cannot prevent restoring the previous daemon.
if err := service.Restart(ctx); err != nil {
cmd/agent/internal/cmd/assets/agent-upgrade-plan.txt.tmpl:7
- This always claims last-good will point to
RollbackPath, but an identical candidate with an already-valid last-good link deliberately preserves that existing link (TestActivateHostDaemonIdenticalCandidatePreservesLastGood). The preflight output therefore reports a mutation that activation will not perform; render the target only when initialization, a candidate change, or repair requires it.
Last-good link: {{ .LastGoodLinkPath }} -> {{ .RollbackPath }}
pkg/agent/agentbinary/activation_preflight.go:29
- Preflight only validates path syntax/collisions, so it succeeds when a destination is already a directory (for example
Layout.BinaryPath, as exercised by the link-switch failure test, or the selected inactive slot). Those entries cannot be atomically replaced by a file/symlink, making the subsequent activation deterministically fail after it may have installed binaries or prepared service assets. Inspect destination entry types here and reject non-replaceable entries so--preflightactually identifies this blocking condition.
if err := validateActivationOptions(opts); err != nil {
return ActivationPlan{}, err
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
pkg/agent/agentbinary/activation_preflight.go:80
- Reject a selected target that resolves to the active binary. For example, with
BluePath -> GreenPathandCurrentPath -> BluePath,currentIsBlueselectsGreenPath, but that is alsocurrentPath; activation then overwrites the only rollback copy, so a failed candidate cannot be recovered. Please also add a regression test for this aliased-slot layout.
plan.TargetPath = opts.Layout.BluePath
if currentIsBlue {
plan.TargetPath = opts.Layout.GreenPath
}
pkg/agent/agentbinary/activation.go:76
RolledBackis set beforerollbackHostActivationreturns in every failure branch. If restoring the current symlink itself fails, the returned result still reportsRolledBack: trueeven though no rollback occurred. Set it only after a successful rollback, or expose separate attempted/succeeded state for API consumers.
RolledBack bool
|
Additional hardening in
Focused race tests cover candidate snapshot pinning, aliased slots, destination rejection, canceled-context rollback, unsuccessful rollback reporting, and conditional preflight rendering. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
pkg/agent/agentbinary/activation_preflight.go:34
--preflightnever runs the candidate'sversioncommand, so any executable file can produce a successful plan even though the real activation will fail atVerify. This contradicts the preflight contract that blocking conditions are validated before reporting success; verify the resolved candidate here as part of preflight.
candidatePath, err := executablePath(opts.CandidatePath)
if err != nil {
return ActivationPlan{}, fmt.Errorf("resolve candidate agent binary: %w", err)
}
pkg/agent/agentbinary/activation.go:282
- The snapshot is later executed by
Verify, but placing it in the default temporary directory makes valid upgrades fail on hosts where/tmpis mountednoexec; it can also acquire a temporary-file SELinux label. This repository already avoids/tmpfor SELinux-sensitive atomic files (cmd/agent/internal/daemon/sysutil.go:13-21). Create the executable snapshot on a trusted executable filesystem or verify the pinned source without executing a/tmpcopy.
snapshotDir, err = os.MkdirTemp("", "host-agent-activation-*")
if err != nil {
return "", nil, fmt.Errorf("create private host agent activation directory: %w", err)
}
snapshotPath = filepath.Join(snapshotDir, "candidate")
if err := utilio.InstallFile(snapshotPath, source, mode); err != nil {
cmd/agent/internal/cmd/assets/agent-upgrade-plan.txt.tmpl:9
- This output always describes a current-link switch, although
switchActivationLinksskips that update when an initialized layout already has an identical candidate. It also omitsServicePlan.Description, so a successful plan does not report that service configuration is already current. Render the link only when initialization/change requires it and include the service assessment so preflight accurately describes the plan promised in the design.
Current link: {{ .CurrentLinkPath }} -> {{ .TargetPath }}
{{if .UpdateLastGood}}Last-good link: {{ .LastGoodLinkPath }} -> {{ .RollbackPath }}
{{end}}Initialize managed layout: {{ .InitializeLayout }}
Planned actions:
Summary
unbounded-agent agent-upgrade [--preflight]host-driven activation commandValidation
go test ./pkg/agent/agentbinary ./pkg/agent/goalstates ./cmd/agent/internal/daemon ./cmd/agent/internal/cmdgolangci-lintfor changed Go packagesgo build -o bin/unbounded-agent ./cmd/agent