host: opt-in stop for orphaned microvm@<name>.service on removal from microvm.vms - #509
Open
brandonros wants to merge 4 commits into
Open
host: opt-in stop for orphaned microvm@<name>.service on removal from microvm.vms#509brandonros wants to merge 4 commits into
brandonros wants to merge 4 commits into
Conversation
… microvm.vms When a VM is removed from microvm.vms.<name> and nixos-rebuild switch is run, the corresponding microvm@<name>.service instance keeps running. switch-to-configuration detects service removal by looking for the base unit file's disappearance, but the base file for microvm@<name>.service is the template microvm@.service, which the host module unconditionally defines — so the removal branch never fires. The .wants symlink under microvms.target.wants is not inspected either. Add system.activationScripts.microvm-stop-orphans, gated behind a new microvm.host.stopOrphans option (default false — opt-in, no behavior change for existing users). When enabled, it iterates install-microvm-*.service files in the previous generation (/run/current-system/etc/systemd/system) and stops any microvm@<name>.service whose <name> is no longer declared. Using the install-microvm-<name>.service marker scopes the reconciliation to previously-declarative VMs only; imperative VMs started via the microvm command are never touched. State under /var/lib/microvms/<name>/ is preserved, per the position stated in microvm-nix#302 — VMs may be only temporarily deactivated. Refs: microvm-nix#508, microvm-nix#302 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
brandonros
force-pushed
the
fix/stop-orphaned-declarative-vms
branch
from
April 22, 2026 13:48
e34c7f3 to
fa52032
Compare
- Use lib.concatMapAttrsStringSep instead of concatStringsSep + attrNames.
- Use lib.getExe' config.systemd.package "systemctl" instead of
hardcoding ${pkgs.systemd}/bin/systemctl, so the configured
systemd package is used. Note: getExe' (not getExe), since
systemd's meta.mainProgram is unset and getExe would resolve to
the `systemd` binary, not `systemctl`.
- Reformat new doc sections to one sentence per line per project
style, plus small wording tightens.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Thanks for the review @SuperSandro2000! All ten suggestions applied in 2f61b71 with one minor correction:
|
astro
reviewed
Apr 23, 2026
Per review from @astro: switch the orphan-detection source from filesystem introspection of the previous generation to systemd's in-memory state via `systemctl list-unit-files`. During activation scripts, /etc already reflects the new generation (etc activation runs first), but systemd has not yet been told to daemon-reload — so `systemctl list-unit-files 'install-microvm-*.service'` returns the previous generation's unit files. This removes the need to reach into /run/current-system/etc and simplifies the script. Also: drop the `lib.getExe' config.systemd.package "systemctl"` wrapper in favor of bare `systemctl`, which is in the activation script `$PATH` as @astro notes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Author
|
Thanks @astro — both addressed in a2de3b2. The activation script now uses |
karaolidis
reviewed
May 3, 2026
|
|
||
| system.activationScripts.microvm-stop-orphans = lib.optionalString config.microvm.host.stopOrphans '' | ||
| declared=" ${lib.concatMapAttrsStringSep " " (name: _: name) config.microvm.vms} " | ||
| for unit in $(systemctl list-unit-files --no-legend 'install-microvm-*.service' 2>/dev/null | awk '{print $1}'); do |
There was a problem hiding this comment.
The problem with LLM-generated code and PRs, such as this one, is that they don't work:
...
/nix/store/a0kzb6wmdkp6h9f6z6cs367iw7f44f6f-nixos-system-jupiter-26.05.19700101.dirty/activate: line 130: awk: command not found
...
awk is not on the activation script's PATH, causing "awk: command not found" during system activation. Parse the unit name with bash builtins via `read` from a process substitution instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the reconciliation gap where
nixos-rebuild switchdoes not stop amicrovm@<name>.serviceafter<name>is removed frommicrovm.vms. Addresses the stop-on-removal goal from #302 (@astro: ""I'd accept PRs that stop declarative VMs on removal""), scoped to stopping only — state under/var/lib/microvms/<name>/is preserved per the same comment.Tracked as #508.
Opt-in by default. New behavior is gated behind
microvm.host.stopOrphanswhich defaults tofalse— existing users see no change. Users who want NixOS-style reconciliation for their declarative VMs setmicrovm.host.stopOrphans = true;.Why
restartIfChanged/ the prior #302 discussion doesn't cover thisThe fix briefly discussed in #302 (flipping
restartIfChangedto matchoci-containers) never shipped, and would not have fixed this bug.X-RestartIfChangedcontrols restart on config change; stop-on-removal is a separate path inswitch-to-configuration-ng(collect_unit_changes,src/main.rs:1078-1089) that checks whether the service's base unit file disappeared.For
microvm@<name>.service, the base unit file is the templatemicrovm@.service, which the host module unconditionally defines atnixos-modules/host/default.nix:240. Sonew_base_unit_filealways exists and the stop-on-removal branch never fires — regardless ofrestartIfChanged. The.wantssymlinks undermicrovms.target.wants/are not examined either (no references in the Rust source).microvm.vms.<name>.restartIfChangedalready exists as a per-VM option (#104, #110); that half of the #302 agreement effectively landed. The stop-on-removal half is the remaining gap.What this PR does
Adds
system.activationScripts.microvm-stop-orphanstonixos-modules/host/default.nix. When enabled, on each activation it:install-microvm-*.servicefiles in/run/current-system/etc/systemd/system/— these are the VMs that were declarative in the previous generation.<name>is still inattrNames config.microvm.vmsof the new generation.microvm@<name>.serviceis currently active, callssystemctl stop microvm@<name>.service.Using
install-microvm-<name>.serviceas the declarative-VM marker is important: imperative VMs managed via themicrovmcommand have noinstall-microvm-<name>.serviceunit, so they are never touched by this reconciliation.What this PR explicitly does NOT do
microvm.host.stopOrphans = true;./var/lib/microvms/<name>/or any VM state, matching the concern in #302. State cleanup remains manual, as documented indoc/src/microvm-command.md.install-microvm-<name>.servicemarker.Restart=always,restartIfChanged, or any other lifecycle semantics.Files changed
nixos-modules/host/default.nix— new activation script (16 lines, inert when the option isfalse).nixos-modules/host/options.nix— newmicrovm.host.stopOrphansoption (defaultfalse).doc/src/declarative.md— new ""Reconciliation on host rebuild"" section, framed as opt-in.doc/src/microvm-command.md— split ""Removing MicroVMs"" into Imperative / Declarative / State cleanup subsections; declarative path mentions the opt-in.Test plan
I don't have a NixOS test host to iterate on, and local evaluation of the full flake checks exhausts my machine's memory, so this relies on CI. Happy to add a nixosTest in a follow-up — the natural shape is to declare two VMs with
microvm.host.stopOrphans = true;, switch to a specialisation that drops one, and assertmicrovm@<dropped>.serviceis inactive while/var/lib/microvms/<dropped>/remains.microvm.host.stopOrphans = true;set🤖 Generated with Claude Code