Skip to content

fix(vm): reset var on a version bump so extensions match the rootfs - #196

Open
lee-reinhardt wants to merge 1 commit into
mainfrom
fix-vm-update-reset-var
Open

fix(vm): reset var on a version bump so extensions match the rootfs#196
lee-reinhardt wants to merge 1 commit into
mainfrom
fix-vm-update-reset-var

Conversation

@lee-reinhardt

@lee-reinhardt lee-reinhardt commented Aug 11, 2026

Copy link
Copy Markdown
Member

Problem

avocado vm update replaces kernel/initramfs/rootfs and leaves var alone (seed_only). But var holds more than user state: the VM's own system extensions live in /var/lib/avocado/images and are merged onto /usr and /etc at boot. So an update boots a new kernel and rootfs against the old userspace.

Nothing catches this. Each extension's extension-release is synthesized at merge time with ID=_any (systemd-sysext's "matches any OS" wildcard), so stale extensions merge silently and the VM looks healthy while running old code.

Change

  • On a version bump whose var image sha differs from the installed one, fetch the new seed and delete the live var.btrfs after committing artifacts. lifecycle::start already re-seeds a missing disk and re-applies the configured size, so the boot path stays the single owner of both, and a mid-update crash leaves no half-written disk.
  • The reset fires only when the var sha actually changed (a boot-artifact-only release costs neither the ~450 MB download nor the user's state), an installed manifest exists (a first install can't delete a disk seeded from a dev --vm-source tree), and the live disk exists (a never-started VM has nothing to reset, and host applications don't see a var_reset for a reset that didn't happen). A seed file that merely went missing from the install dir is re-fetched at the same sha without touching the live disk.
  • The disk's size is recorded as runtime.var_size in config.yaml before the delete, and vm start resolves size as flag > config > default. Growing the disk used to be sticky only via the file itself, which the reset would otherwise quietly shrink back to the 50G default.
  • Authorization is explicit. Interactively, the confirmation spells out what is reset and requires typing update (matching vm reset). With --yes, a distinct --reset-var flag is required — scripts that pass -y to skip a y/N never wipe state they didn't opt into. --output json never prompts (it would corrupt the NDJSON stream) and emits a {"event":"var_reset"} line so host applications can drop cached install state and tell the user to re-run avocado install / avocado build.
  • Surface the min_cli_version refusal instead of swallowing it: previously an incompatible CLI reported "no update available" forever. Now vm update fails with the actionable "run avocado upgrade first" message, and under --output json a structured {"update_available": true, "cli_too_old": true, ...} line goes out before the non-zero exit so stdout parsers can tell "blocked on CLI version" from "check broke". Newness is checked before compatibility so an already-installed release stays quiet.
  • Fetch the remote manifest once instead of twice, so the installed manifest always describes the artifacts that were actually committed.

Testing

  • 17 new unit tests over plan_downloads, should_reseed_var, record_var_size_floor, config round-trip, and the update-check decision.
  • End-to-end against a fake channel: too-old CLI fails with the actionable message; changed var sha re-seeds and emits var_reset; unchanged var sha leaves the live disk byte-for-byte; aborting the prompt leaves everything intact.
  • Validated against a real running VM: update stopped the VM, reset var, restarted; the new extensions were merged after boot, SSH reconnected across the host-key regeneration, and the var disk was re-grown to its configured size.

Copilot AI lite review requested due to automatic review settings August 11, 2026 20:35
@lee-reinhardt
lee-reinhardt force-pushed the fix-vm-update-reset-var branch from 91c3163 to a555056 Compare August 11, 2026 20:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the VM update flow so that when a VM release ships a new var seed image (sha change), avocado vm update re-seeds /var by deleting the live var.btrfs after committing the new artifacts—preventing stale system extensions in /var/lib/avocado/images from silently merging onto a newer rootfs. It also changes VM update checking to explicitly surface “CLI too old” refusals (instead of reporting “no update available”), and ensures the remote manifest is fetched once and then reused for both planning and persistence.

Changes:

  • Introduces VmUpdateStatus to distinguish “update available”, “CLI too old”, and “no update / check unavailable”, and propagates min_cli_version refusals to vm update.
  • Updates vm update to plan downloads from a single fetched manifest, and conditionally reset /var when the seed-only var artifact sha changes.
  • Adds unit tests for both update-check decision logic and download planning behavior around var reseeding.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/utils/vm_update_check.rs Returns a richer update-check outcome (including CLI incompatibility) and adds targeted tests for decision behavior.
src/commands/vm/update.rs Fetches manifest once, plans seed-only var updates by sha, prompts accordingly, and deletes the live var disk to force reseeding on next boot.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/commands/vm/update.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/commands/vm/update.rs:308

  • manifest.json is treated as the “install complete” marker, but it’s written with std::fs::write, which can truncate/partially write the file if the process crashes or the disk fills mid-write. That can leave the install in a broken state (corrupt or empty manifest) even though artifacts were already committed. Writing to a temp file and renameing into place makes this step atomic on POSIX filesystems.
        let manifest_path = install_dir.join("manifest.json");
        let manifest_bytes = serde_json::to_vec_pretty(
            &serde_json::from_str::<serde_json::Value>(&manifest_raw)
                .context("re-parsing remote manifest for the install dir")?,
        )?;

@nicksinas nicksinas left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against a local checkout; cargo test is green. The direction is right and the docs carry their reasoning well. Six things I'd want resolved, inline. #1 is the one I'd call blocking; #2 contradicts a claim in the PR description.

Comment thread src/commands/vm/update.rs
Comment thread src/commands/vm/update.rs
Comment thread src/commands/vm/update.rs Outdated
Comment thread src/commands/vm/update.rs
Comment thread src/commands/vm/update.rs Outdated
Comment thread src/commands/vm/update.rs
@lee-reinhardt
lee-reinhardt force-pushed the fix-vm-update-reset-var branch from d758077 to 4184fa7 Compare August 12, 2026 16:31
`vm update` replaced kernel/initramfs/rootfs and left `var` alone, on the
`seed_only` premise that var is user state and the in-VM agent would
migrate it at boot. That migration doesn't exist, and var isn't only user
state: the VM's own system extensions live in /var/lib/avocado/images and
are merged onto /usr and /etc at boot. So an update booted a new kernel
and rootfs against the old userspace.

Nothing caught it. Each extension's extension-release is synthesized at
merge time carrying ID=_any — systemd-sysext's "matches any OS" wildcard —
so mismatched extensions merged silently and the VM looked healthy.

Until the guest can migrate in place, a version bump now fetches the new
seed and deletes the live disk; lifecycle::start re-seeds and re-applies
the configured size on the next boot, so the boot path stays the sole
owner of both. Deleting rather than copying also means an update that dies
mid-way leaves no half-written disk to mistake for state.

The reset fires only when the var artifact's sha actually changed, an
installed manifest exists, and the live disk exists — so a boot-artifact
release, a first install, and a never-started VM all keep their state (and
host applications don't see a var_reset for a reset that didn't happen).
A seed file that merely went missing from the install dir is re-fetched
at the same sha without touching the live disk.

The disk's size is recorded as runtime.var_size in config.yaml before the
delete, and vm start now resolves size as flag > config > default —
growing the disk used to be sticky only via the file itself, which the
reset would otherwise quietly shrink back to 50G.

Authorization is explicit: interactively the prompt spells out what is
reset and requires typing 'update', matching `vm reset`; with --yes a
distinct --reset-var flag is required, so scripts that pass -y to skip a
y/N never wipe state they didn't opt into. --output json never prompts
(it would corrupt the NDJSON stream) and emits a `var_reset` event so a
host application can drop cached install state and tell the user to
re-run install/build.

Also here, both load-bearing for the above:

- Surface a min_cli_version refusal instead of swallowing it. That check
  is what stops an old CLI half-applying a release, but check_for_vm_update
  collapsed the error into "no update available" — so the protection
  presented as a CLI that silently never updates its VM again. Now a
  distinct VmUpdateStatus::CliTooOld that `vm update` fails on, which
  callers already surface via the non-zero exit; under --output json a
  structured line with cli_too_old goes out first. Newness is checked
  before compatibility so an incompatible release we already have stays
  quiet.

- Fetch the remote manifest once instead of twice. The second GET wrote
  the installed manifest; if the release were re-published mid-run it
  would describe artifacts we didn't commit.
@lee-reinhardt
lee-reinhardt force-pushed the fix-vm-update-reset-var branch from 4184fa7 to e40ce47 Compare August 13, 2026 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants