Keep the last-good service assets until an upgrade is judged by plombardi89 · Pull Request #387 · Azure/AKSFlexNode

Philip Lombardi (plombardi89) · 2026-10-20T20:17:04Z

Problem

At every start, the daemon rewrites the agent's service assets from its own binary, and reloads systemd:

the recovery script, /usr/local/lib/aks-flex-node/aks-flex-node-recovery.sh;
the recovery unit;
the agent's own unit.

An upgraded candidate does this too, before its upgrade is judged.

Recovery runs the last-good binary's recover-agent-upgrade, but through the recovery unit and script the candidate just wrote. And after a failed upgrade, systemd restarts the last-good binary through the agent's unit, which the candidate also wrote. So a candidate whose assets are broken breaks its own rollback, and can leave the node on the failed version.

This came up in review of the ACL branch (#370), which mirrors this flow for its container units.

Fix

The daemon now keeps the last-good version's assets until the upgrade is judged:

At startup, while an AgentUpgrade's signal exists (ensureDaemonServiceAssets), the daemon keeps the assets on disk and writes only missing ones. In that state the daemon may be the upgrade's candidate, not yet judged, or recovery is under way. Either way, the assets on disk are the last-good version's.
Once the result is published and the signal is cleared, it writes its own and reloads systemd, before it opens the startup gate, so before any MachineOperation can start another upgrade.
A failed upgrade keeps its signal, so the last-good version's assets stay.
If writing them after a successful upgrade fails, that's only logged: the last-good version's still work, and the next start writes them again.
Unchanged:
- without a signal, the daemon converges the assets at startup as before;
- InstallService still writes them all;
- direct activation (agent-upgrade), which creates no signal, still writes them all.

For testing, the startup gate's loop in daemon.Run moved into openGateAfterAgentUpgradeJudgment. It's unchanged apart from the new step.

Testing

Unit tests for startup convergence:
- with a signal, the last-good version's units and script stay byte for byte;
- with a signal, a missing one is still written;
- without a signal, all three are rewritten as before.
Unit tests for the gate loop:
- once the upgrade is judged, the held assets are written, then the gate opens;
- while recovery keeps the signal, neither happens;
- a failed publish is retried;
- the gate opens even when writing the assets fails.
Mutations: never holding the assets back fails the first group, and never writing them after judgment fails the second.
Go tests, golangci-lint, and the docs checks pass. main's e2e agent-upgrade, including the broken candidate's rollback, runs on this PR.

Each says what follows once it merges: a rebase, and the same change in the container's executor or convergence.

Copilot

🟢 Approval recommended

The focused changes preserve rollback assets and startup ordering, with regression coverage and no unresolved findings.

No open issues 🎉

What changed in this PR

Protects AgentUpgrade rollback by preserving last-good service assets until the upgrade is judged.

Changes:

Keeps existing units and recovery scripts while an upgrade signal exists, writing only missing assets.
Converges held assets after judgment, before opening the startup gate.
Adds regression tests and documents the behavior.

File	Description
pkg/daemon/lifecycle.go	Preserves existing assets during pending upgrades.
pkg/daemon/lifecycle_test.go	Tests preservation, missing assets, and normal convergence.
pkg/daemon/host_agent_activation.go	Retains unconditional writes for direct activation.
pkg/daemon/daemon.go	Converges held assets before opening the startup gate.
pkg/daemon/daemon_test.go	Tests gate ordering, recovery, retries, and convergence failures.
docs/usage/operations.md	Documents last-good asset preservation.

_{Review effort: Balanced}

At every start, the daemon rewrote the agent's systemd units and its recovery script from its own binary, an upgraded candidate included, before its upgrade was judged. Recovery runs the last-good binary's recover-agent-upgrade, but through that recovery unit and script, and systemd restarts the last-good binary through the agent's unit. So a candidate whose assets were broken broke its own rollback, and could leave the node on the failed version. While an AgentUpgrade's signal exists, the daemon may be the upgrade's candidate, not yet judged, or recovery is under way, and the assets on disk are the last-good version's. At startup, the daemon now keeps them then, writing only missing ones. Once it has published the upgrade's result and the signal is cleared, it writes its own, before it opens the startup gate. A failed upgrade keeps its signal, so the last-good version's assets stay. Failing to write them after a successful upgrade is only logged: the last-good version's still work, and the next start writes them again. Without a signal, the daemon converges them at startup as before, and install and direct activation still write them all. The startup gate's loop moved into openGateAfterAgentUpgradeJudgment, unchanged but for that step, so it can be tested.

Copilot

🟢 Approval recommended

The focused changes preserve rollback assets and gate ordering, with regression coverage and no unresolved findings.

No open issues 🎉

_{Review effort: Balanced}

Each says what follows once it merges: a rebase, and the same change in the container's executor or convergence.

#387 has a host's daemon keep the last-good version's service assets while an AgentUpgrade's signal exists, writing its own once the upgrade is judged, as recovery, and the restart into the last-good version, run through them. The container's convergence now does the same with its units: while the signal exists, it writes only missing ones, and reports that it held them back. It still copies the agent's binary into the machine, as judging the upgrade needs it there, and still removes old images, which keeps the selected and last-good ones. Run chooses, once for its mode, what the startup gate runs once the upgrade is judged: on a host, #387's rewrite of the assets ensureDaemonServiceAssets held back, which ensureHostInstall now returns; in the container, the convergence's writeUnits, if it held the units back. openGateAfterAgentUpgradeJudgment is #387's, unchanged. The signal check #387 inlined is agentUpgradePending, which both use.

DESIGN.md's upgrade steps now say the agent resets its unit's start-limit count before an upgrade's restart, and that the candidate keeps the last-good version's units until the upgrade is judged, as #386 and #387 have a host's daemon do. Where the container runs as kernel_t and may not reset the count, a sixth start in five minutes still fails. TODO.md: both items are done, with #386 and #387 merged into main and ported to the container.

Brings in #386, which resets the agent unit's start limit before an upgrade's restart, and #387, which keeps the last-good service assets while an AgentUpgrade is pending and writes them once it's judged. - ensureDaemonServiceAssets, #387's, and ensureAgentUpgradeServiceAssets both go through ensureServiceAssetsUnderHostRoot, which resolves the host root once, as this branch does, rather than main's fixed /usr/local paths and recoveryScriptPath, which this branch removed. - The daemon installs its host binary, then converges the assets, holding them back while an upgrade is pending, as before removing the legacy seed. That removal is safe while they're held: units that name the seed exist only on a host not yet moved off /usr/local, where hostroot.LegacyReleased has it remove nothing. - #387's openGateAfterAgentUpgradeJudgment keeps the startup gate's loop, and its last step, now proceed, is this branch's: once the upgrade is judged and the held assets are written, reconcile the host root, then open the gate, or hold it until the daemon is replaced.

Each says what follows once it merges: a rebase, and the same change in the container's executor or convergence.

while an AgentUpgrade's signal exists, writing its own once the upgrade is judged, as recovery, and the restart into the last-good version, run through them. The container's convergence now does the same with its units: while the signal exists, it writes only missing ones, and reports that it held them back. It still copies the agent's binary into the machine, as judging the upgrade needs it there, and still removes old images, which keeps the selected and last-good ones. Run chooses, once for its mode, what the startup gate runs once the upgrade is judged: on a host, #387's rewrite of the assets ensureDaemonServiceAssets held back, which ensureHostInstall now returns; in the container, the convergence's writeUnits, if it held the units back. openGateAfterAgentUpgradeJudgment is #387's, unchanged. The signal check #387 inlined is agentUpgradePending, which both use.