fix(hub): derive Restart required from boot default vs running system
NixOS already knows whether a reboot is pending: /nix/var/nix/profiles/ system vs /run/current-system. Marker files only the Hub's own updater wrote desynced for terminal-updated machines (and markers from older updaters could never clear), pinning the badge on forever. Reconcile REBOOT_REQUIRED against live state on every read; the stale marker self-heals to IDLE. The .generation marker write is now informational.
This commit is contained in:
@@ -83,6 +83,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
- Successful staged updates now record the exact NixOS generation. The Hub
|
||||
keeps showing "Restart required" until that generation is active, then
|
||||
clears the marker after reboot (with log-based recovery for older updates).
|
||||
- Pending-reboot state is now derived from NixOS itself — the boot default
|
||||
(`/nix/var/nix/profiles/system`) versus the running `/run/current-system`
|
||||
— instead of from Hub-written marker files. Updates performed from a
|
||||
terminal or support session (which never touch the Hub's status files)
|
||||
previously left a stale `REBOOT_REQUIRED` marker the Hub could never
|
||||
clear, pinning the "Restart required" badge forever even after many
|
||||
reboots. The marker self-heals to `IDLE` on the next status read whenever
|
||||
boot default and running system agree, and recording the informational
|
||||
`.generation` marker can no longer fail an otherwise successful update.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@@ -55,7 +55,7 @@ from .security_helpers import (
|
||||
load_session_store,
|
||||
save_session_store,
|
||||
)
|
||||
from .update_state import staged_generation_is_active
|
||||
from .update_state import effective_update_status
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -1648,11 +1648,14 @@ def _write_update_status(status: str):
|
||||
def _read_update_status() -> str:
|
||||
"""Read and reconcile the persistent update status.
|
||||
|
||||
``REBOOT_REQUIRED`` intentionally survives Hub/browser restarts before the
|
||||
reboot. Once the staged generation is the running ``/run/current-system``,
|
||||
clear that marker so a completed reboot cannot leave the Hub asking for
|
||||
another reboot forever. The generation helper can recover older updates
|
||||
from the final nixos-rebuild log line when no explicit marker exists.
|
||||
``REBOOT_REQUIRED`` survives Hub/browser restarts before the reboot, but
|
||||
it is a CLAIM about live NixOS state, not the source of truth: the boot
|
||||
default (``/nix/var/nix/profiles/system``) versus the running
|
||||
``/run/current-system``. Re-validating on every read keeps the Hub
|
||||
correct when the system was updated from a terminal or support session
|
||||
(which never writes Hub markers), and lets an old marker that predates
|
||||
the reconciliation feature self-heal instead of demanding reboots
|
||||
forever. The stale marker file is removed once cleared.
|
||||
"""
|
||||
try:
|
||||
with open(UPDATE_STATUS, "r") as f:
|
||||
@@ -1660,15 +1663,14 @@ def _read_update_status() -> str:
|
||||
except FileNotFoundError:
|
||||
return "IDLE"
|
||||
|
||||
if status == "REBOOT_REQUIRED" and staged_generation_is_active(
|
||||
UPDATE_GENERATION, UPDATE_LOG
|
||||
):
|
||||
_write_update_status("IDLE")
|
||||
effective = effective_update_status(status)
|
||||
if effective != status:
|
||||
_write_update_status(effective)
|
||||
try:
|
||||
os.remove(UPDATE_GENERATION)
|
||||
except OSError:
|
||||
pass
|
||||
return "IDLE"
|
||||
return effective
|
||||
|
||||
return status
|
||||
|
||||
|
||||
@@ -1,81 +1,88 @@
|
||||
"""Persistent update-state helpers for the Sovran Hub.
|
||||
"""Update-state helpers for the Sovran Hub.
|
||||
|
||||
The full-system updater stages a NixOS generation with ``nixos-rebuild boot``.
|
||||
That generation is not active until the machine reboots. These helpers let the
|
||||
Hub distinguish a genuinely pending reboot from an old REBOOT_REQUIRED marker
|
||||
that survived the reboot.
|
||||
That generation is not active until the machine reboots — and the same is true
|
||||
for updates started from a terminal or an SSH support session, which never go
|
||||
near the Hub's status files.
|
||||
|
||||
This module deliberately has no FastAPI or systemd dependencies so its state
|
||||
reconciliation can be tested without importing the Hub server.
|
||||
The ONLY reliable indicator that a reboot is pending is NixOS itself: the
|
||||
system profile (``/nix/var/nix/profiles/system``), which ``nixos-rebuild``
|
||||
points at the newest generation on every ``boot`` AND every ``switch``, versus
|
||||
``/run/current-system``, the generation actually running since the last boot.
|
||||
When the two differ, a staged generation has not been booted yet.
|
||||
|
||||
Earlier revisions reconstructed this from a marker file and log tails written
|
||||
by the Hub's own updater. Any system updated by other means — or whose
|
||||
``REBOOT_REQUIRED`` status was written by an updater older than the marker
|
||||
feature — left the Hub showing "Restart required" forever: the recorded
|
||||
generation could never equal the (since advanced) running one, so the marker
|
||||
could never be cleared.
|
||||
|
||||
This module has no FastAPI or systemd dependencies so the policy can be tested
|
||||
without importing the Hub server.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import re
|
||||
|
||||
# The NixOS system profile. ``nixos-rebuild boot`` and ``nixos-rebuild
|
||||
# switch`` both add a generation here; ``boot`` additionally makes it the
|
||||
# bootloader default. The path is a symlink chain (``system`` ->
|
||||
# ``system-N-link`` -> ``/nix/store/...-nixos-system-...``).
|
||||
BOOT_PROFILE_PATH = "/nix/var/nix/profiles/system"
|
||||
CURRENT_SYSTEM_PATH = "/run/current-system"
|
||||
|
||||
|
||||
# Nix store hashes use the lower-case Nix base32 alphabet. Keep the output
|
||||
# name deliberately conservative: a system generation has no path separators.
|
||||
_SYSTEM_GENERATION_RE = re.compile(
|
||||
r"^/nix/store/[0-9a-z]{32}-nixos-system-[A-Za-z0-9._+\-]+$"
|
||||
)
|
||||
_LOG_GENERATION_RE = re.compile(
|
||||
r"The new configuration is "
|
||||
r"(/nix/store/[0-9a-z]{32}-nixos-system-[A-Za-z0-9._+\-]+)"
|
||||
)
|
||||
def reboot_is_pending(
|
||||
boot_profile_path: str = BOOT_PROFILE_PATH,
|
||||
current_system_path: str = CURRENT_SYSTEM_PATH,
|
||||
) -> bool:
|
||||
"""Return whether a staged NixOS generation has not been booted yet.
|
||||
|
||||
This is deliberately independent of how the update was started — Hub
|
||||
"Update System", terminal ``nixos-rebuild boot``, or a support session all
|
||||
move the system profile the same way:
|
||||
|
||||
def _valid_generation(value: str) -> str | None:
|
||||
"""Return a normalized NixOS generation path, or ``None`` if invalid."""
|
||||
candidate = value.strip()
|
||||
if _SYSTEM_GENERATION_RE.fullmatch(candidate):
|
||||
return candidate
|
||||
return None
|
||||
* after ``nixos-rebuild boot``: profile -> new, current -> old → pending
|
||||
* after rebooting: both -> new → cleared
|
||||
* after ``nixos-rebuild switch``: both move together → no reboot
|
||||
ever needed (switch activates immediately)
|
||||
* after a rollback: both point at the rollback target → cleared
|
||||
|
||||
|
||||
def read_staged_generation(marker_path: str, log_path: str) -> str | None:
|
||||
"""Read the generation staged by the last successful Hub update.
|
||||
|
||||
New updater versions write ``marker_path`` explicitly. For an update that
|
||||
started with an older updater, recover the same value from the final
|
||||
``nixos-rebuild`` log line. Only the tail is needed and bounding the read
|
||||
avoids loading a potentially large build log during every status poll.
|
||||
Unreadable or missing paths are treated as "not pending": the Hub must
|
||||
never demand a reboot it cannot substantiate.
|
||||
"""
|
||||
try:
|
||||
with open(marker_path, "r", encoding="utf-8") as marker:
|
||||
generation = _valid_generation(marker.read())
|
||||
if generation:
|
||||
return generation
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
try:
|
||||
with open(log_path, "rb") as log:
|
||||
log.seek(0, os.SEEK_END)
|
||||
size = log.tell()
|
||||
log.seek(max(0, size - 131_072), os.SEEK_SET)
|
||||
tail = log.read().decode("utf-8", errors="replace")
|
||||
except OSError:
|
||||
return None
|
||||
|
||||
matches = list(_LOG_GENERATION_RE.finditer(tail))
|
||||
if not matches:
|
||||
return None
|
||||
return _valid_generation(matches[-1].group(1))
|
||||
|
||||
|
||||
def staged_generation_is_active(
|
||||
marker_path: str,
|
||||
log_path: str,
|
||||
current_system_path: str = "/run/current-system",
|
||||
) -> bool:
|
||||
"""Return whether the staged update generation is now the running system."""
|
||||
staged = read_staged_generation(marker_path, log_path)
|
||||
if not staged:
|
||||
return False
|
||||
try:
|
||||
boot_default = os.path.realpath(boot_profile_path)
|
||||
current = os.path.realpath(current_system_path)
|
||||
except OSError:
|
||||
return False
|
||||
return current == staged
|
||||
if not os.path.exists(boot_default) or not os.path.exists(current):
|
||||
return False
|
||||
return boot_default != current
|
||||
|
||||
|
||||
def effective_update_status(
|
||||
status: str,
|
||||
boot_profile_path: str = BOOT_PROFILE_PATH,
|
||||
current_system_path: str = CURRENT_SYSTEM_PATH,
|
||||
) -> str:
|
||||
"""Map a persisted Hub status to the one that reflects live NixOS state.
|
||||
|
||||
Only ``REBOOT_REQUIRED`` is re-validated: it means "the update staged a
|
||||
generation the machine has not booted into", a claim that must stay true
|
||||
no matter which tool performed the last update. When the boot default IS
|
||||
the running system the claim is stale — the staged generation booted, was
|
||||
superseded by a newer update, or the marker was written by an updater that
|
||||
could never clear it — so the effective status is ``IDLE``.
|
||||
|
||||
All other statuses (``RUNNING``, ``FAILED``, ``SUCCESS``, ``IDLE``) pass
|
||||
through unchanged; RUNNING staleness is handled separately against the
|
||||
systemd unit itself.
|
||||
"""
|
||||
if status == "REBOOT_REQUIRED" and not reboot_is_pending(
|
||||
boot_profile_path, current_system_path
|
||||
):
|
||||
return "IDLE"
|
||||
return status
|
||||
|
||||
@@ -199,9 +199,11 @@ let
|
||||
echo "[ERROR] nixos-rebuild boot failed"
|
||||
RC=1
|
||||
elif ! readlink -f /nix/var/nix/profiles/system > "$GENERATION"; then
|
||||
echo "[ERROR] update was built but its staged generation could not be recorded"
|
||||
# The marker is informational only. The Hub derives pending-reboot
|
||||
# state from the NixOS system profile itself, so failing to record
|
||||
# the marker must not fail an otherwise successful update.
|
||||
echo "[WARNING] update succeeded but its staged generation could not be recorded"
|
||||
rm -f "$GENERATION"
|
||||
RC=1
|
||||
fi
|
||||
echo ""
|
||||
fi
|
||||
|
||||
+117
-53
@@ -17,80 +17,144 @@ if str(_APP_PARENT) not in sys.path:
|
||||
sys.path.insert(0, str(_APP_PARENT))
|
||||
|
||||
from sovran_systemsos_web.update_state import ( # noqa: E402
|
||||
read_staged_generation,
|
||||
staged_generation_is_active,
|
||||
effective_update_status,
|
||||
reboot_is_pending,
|
||||
)
|
||||
|
||||
|
||||
GENERATION = (
|
||||
"/nix/store/rmi0g35cd8w60k0ig7pm6kb8kzws8b7x-"
|
||||
"nixos-system-nixos-26.11.20260817.ec2d622"
|
||||
# Real store paths observed on the incident machine that prompted this rework.
|
||||
RUNNING_GENERATION = (
|
||||
"84rsiqi66nc68jbikd26ms50ap831xf8-nixos-system-nixos-26.11.20260817.ec2d622"
|
||||
)
|
||||
OTHER_GENERATION = (
|
||||
"/nix/store/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-"
|
||||
"nixos-system-nixos-26.11.20260816.old"
|
||||
PREVIOUS_GENERATION = (
|
||||
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-nixos-system-nixos-26.11.20260816.old"
|
||||
)
|
||||
# What the stale Hub log still claimed was staged: a two-week-old update cycle.
|
||||
HUB_LOG_GENERATION = (
|
||||
"yis0saq6p8fhqcaii0h2yzqf0blhdwns-nixos-system-nixos-26.11.20260804.e72e4f2"
|
||||
)
|
||||
|
||||
|
||||
class TestStagedGenerationState(unittest.TestCase):
|
||||
class TestRebootPendingState(unittest.TestCase):
|
||||
"""Reboot-pending state is derived from NixOS, not from Hub marker files.
|
||||
|
||||
The system profile (what boots next) is compared against
|
||||
/run/current-system (what is running). This stays correct no matter
|
||||
which tool performed the update: Hub "Update System", a terminal
|
||||
``nixos-rebuild``, or an SSH support session.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
self.tmp = tempfile.TemporaryDirectory()
|
||||
self.addCleanup(self.tmp.cleanup)
|
||||
root = Path(self.tmp.name)
|
||||
self.marker = root / "update.generation"
|
||||
self.log = root / "update.log"
|
||||
self.store = root / "store"
|
||||
self.store.mkdir()
|
||||
# /nix/var/nix/profiles/system is a two-hop chain: the diagnostics on
|
||||
# the incident box showed ``system`` -> ``system-148-link`` -> store.
|
||||
self.profile = root / "system"
|
||||
self.profile_entry = root / "system-148-link"
|
||||
# /run/current-system links straight to the running store path.
|
||||
self.current = root / "current-system"
|
||||
|
||||
def test_explicit_generation_marker_is_read(self):
|
||||
self.marker.write_text(GENERATION + "\n", encoding="utf-8")
|
||||
self.assertEqual(
|
||||
read_staged_generation(str(self.marker), str(self.log)), GENERATION
|
||||
def _store_dir(self, name: str) -> str:
|
||||
path = self.store / name
|
||||
path.mkdir(exist_ok=True)
|
||||
return str(path)
|
||||
|
||||
def _stage(self, booted: str, running: str) -> None:
|
||||
"""Stage ``booted`` as the boot default with ``running`` live."""
|
||||
os.symlink(self._store_dir(booted), self.profile_entry)
|
||||
os.symlink(self.profile_entry, self.profile)
|
||||
os.symlink(self._store_dir(running), self.current)
|
||||
|
||||
def test_hub_update_staged_and_not_yet_rebooted_is_pending(self):
|
||||
self._stage(booted=RUNNING_GENERATION, running=PREVIOUS_GENERATION)
|
||||
self.assertTrue(
|
||||
reboot_is_pending(str(self.profile), str(self.current))
|
||||
)
|
||||
|
||||
def test_legacy_updater_generation_is_recovered_from_log_tail(self):
|
||||
self.log.write_text(
|
||||
"building the system configuration...\n"
|
||||
f"Done. The new configuration is {GENERATION}\n"
|
||||
def test_staged_generation_booted_is_no_longer_pending(self):
|
||||
self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION)
|
||||
self.assertFalse(
|
||||
reboot_is_pending(str(self.profile), str(self.current))
|
||||
)
|
||||
|
||||
def test_terminal_switch_needs_no_reboot(self):
|
||||
# nixos-rebuild switch moves the profile AND current-system together.
|
||||
self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION)
|
||||
self.assertFalse(
|
||||
reboot_is_pending(str(self.profile), str(self.current))
|
||||
)
|
||||
|
||||
def test_rollback_leaves_nothing_pending(self):
|
||||
# nixos-rebuild switch --rollback points both at the rollback target.
|
||||
self._stage(booted=PREVIOUS_GENERATION, running=PREVIOUS_GENERATION)
|
||||
self.assertFalse(
|
||||
reboot_is_pending(str(self.profile), str(self.current))
|
||||
)
|
||||
|
||||
def test_unverifiable_state_is_never_a_reboot_demand(self):
|
||||
# No profile and no running system readable: the Hub must not nag
|
||||
# about a reboot it cannot substantiate.
|
||||
self.assertFalse(
|
||||
reboot_is_pending(str(self.profile), str(self.current))
|
||||
)
|
||||
|
||||
def test_stale_hub_marker_clears_after_terminal_updates(self):
|
||||
"""The exact incident: terminal-updated machine, frozen Hub marker.
|
||||
|
||||
The user's last Hub update (old updater, weeks prior) left
|
||||
REBOOT_REQUIRED behind. Every update since ran in a terminal and
|
||||
never touched the Hub's files, so the log still records a staged
|
||||
generation from 2026-08-04 while the machine runs a 2026-08-17
|
||||
build. With the profile and the running system in agreement, the
|
||||
stale claim must reconcile to IDLE regardless of anything the old
|
||||
marker/log files say.
|
||||
"""
|
||||
root = Path(self.tmp.name)
|
||||
log = root / "sovran-hub-update.log"
|
||||
log.write_text(
|
||||
"Done. The new configuration is "
|
||||
f"/nix/store/{HUB_LOG_GENERATION}\n"
|
||||
"✓ Update staged successfully\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
self.assertEqual(
|
||||
read_staged_generation(str(self.marker), str(self.log)), GENERATION
|
||||
)
|
||||
|
||||
def test_latest_generation_line_wins(self):
|
||||
self.log.write_text(
|
||||
f"Done. The new configuration is {OTHER_GENERATION}\n"
|
||||
f"Done. The new configuration is {GENERATION}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
self.assertEqual(
|
||||
read_staged_generation(str(self.marker), str(self.log)), GENERATION
|
||||
)
|
||||
|
||||
def test_invalid_marker_is_ignored(self):
|
||||
self.marker.write_text("/tmp/not-a-generation\n", encoding="utf-8")
|
||||
self.log.write_text("no completed generation\n", encoding="utf-8")
|
||||
self.assertIsNone(read_staged_generation(str(self.marker), str(self.log)))
|
||||
|
||||
def test_staged_generation_is_active_after_reboot(self):
|
||||
self.marker.write_text(GENERATION + "\n", encoding="utf-8")
|
||||
os.symlink(GENERATION, self.current)
|
||||
self.assertTrue(
|
||||
staged_generation_is_active(
|
||||
str(self.marker), str(self.log), str(self.current)
|
||||
)
|
||||
)
|
||||
|
||||
def test_staged_generation_remains_pending_before_reboot(self):
|
||||
self.marker.write_text(GENERATION + "\n", encoding="utf-8")
|
||||
os.symlink(OTHER_GENERATION, self.current)
|
||||
# Deliberately no sovran-hub-update.generation marker: the updater
|
||||
# that produced this state predates the marker feature.
|
||||
self.assertFalse(
|
||||
staged_generation_is_active(
|
||||
str(self.marker), str(self.log), str(self.current)
|
||||
)
|
||||
(root / "sovran-hub-update.generation").exists()
|
||||
)
|
||||
|
||||
self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION)
|
||||
self.assertEqual(
|
||||
effective_update_status(
|
||||
"REBOOT_REQUIRED", str(self.profile), str(self.current)
|
||||
),
|
||||
"IDLE",
|
||||
)
|
||||
|
||||
def test_genuine_pending_reboot_claim_survives(self):
|
||||
# A staged generation that has NOT been booted yet: the claim is
|
||||
# true and must keep surfacing until the reboot really happens.
|
||||
self._stage(booted=RUNNING_GENERATION, running=PREVIOUS_GENERATION)
|
||||
self.assertEqual(
|
||||
effective_update_status(
|
||||
"REBOOT_REQUIRED", str(self.profile), str(self.current)
|
||||
),
|
||||
"REBOOT_REQUIRED",
|
||||
)
|
||||
|
||||
def test_other_statuses_pass_through_unchanged(self):
|
||||
self._stage(booted=RUNNING_GENERATION, running=RUNNING_GENERATION)
|
||||
for status in ("RUNNING", "FAILED", "SUCCESS", "IDLE"):
|
||||
self.assertEqual(
|
||||
effective_update_status(
|
||||
status, str(self.profile), str(self.current)
|
||||
),
|
||||
status,
|
||||
)
|
||||
|
||||
|
||||
class TestUpdatePollingWiring(unittest.TestCase):
|
||||
"""Guard the browser failure modes that caused a permanent spinner."""
|
||||
|
||||
Reference in New Issue
Block a user