From c11d4fbf2e7aeb247660eae14fb8511bf5c5b51a Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 5 Oct 2026 21:21:46 +0200 Subject: [PATCH] R-179: the uninstall removes the NAS network-storage units MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Units carrying the agent's network-storage marker (mnt-*.automount first, then mnt-*.mount) are disabled --now, reset-failed and removed before the drive umount loop; a share that will not stop is not forced — its unit is kept and named in KEPT with the commands. Enrolled-drive and foreign units are never touched. scripts/test_hostinstall.py: test_net_units_* (5). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS --- scripts/felhom-host-install.sh | 45 ++++++++++++++++ scripts/test_hostinstall.py | 98 +++++++++++++++++++++++++++++++++- 2 files changed, 141 insertions(+), 2 deletions(-) diff --git a/scripts/felhom-host-install.sh b/scripts/felhom-host-install.sh index 5761ee3c..f401ff35 100644 --- a/scripts/felhom-host-install.sh +++ b/scripts/felhom-host-install.sh @@ -843,6 +843,9 @@ _uninstall_statement() { echo " - break-glass watchdog + OOB artifacts (where present); guest-hook snippet; dnsmasq snippets; the mkfs, pbs-apply," echo " backup-target-apply, os-apply and priv-apply wrappers; the crash guard; the config-bundle record" echo " - pveum: the Felhom roles/user/token/scoped ACL$( $pool_removed && printf '; the emptied %s pool' "$PVE_POOL")" + if (( ${#_NET_UNITS_REMOVED[@]} > 0 )); then + echo " - the network-share (NAS) mount units: ${_NET_UNITS_REMOVED[*]} (the data stays on the NAS)" + fi echo " - the install state file" if $REMOVE_GOLDEN; then echo " - the golden vzdump (--remove-golden)"; fi else @@ -850,6 +853,10 @@ _uninstall_statement() { fi echo " KEPT (lives on deliberately — remove/rotate these out-of-band if the customer is leaving):" if [[ "$scope" == "full" ]]; then + if (( ${#_NET_UNITS_BUSY[@]} > 0 )); then + echo " - network-share unit(s) that would not stop (busy, not forced): ${_NET_UNITS_BUSY[*]} — once free:" + echo " systemctl disable --now -- ; rm $NET_UNIT_DIR/; systemctl daemon-reload" + fi echo " - the enrolled drives + ALL data under /mnt/felhom-drives — unmounted only, NEVER wiped;" if [[ ${#_busy_mounts[@]} -gt 0 ]]; then echo " physically removable now, EXCEPT still mounted (busy — stop the apps and retry): ${_busy_mounts[*]}" @@ -1058,6 +1065,42 @@ _teardown_wg_tunnel() { return 0 } +# _remove_network_storage_units — the NAS network-storage .automount/.mount pairs the agent writes (R-179). +# +# A box that ever had a network share kept `mnt-felhom\x2ddrives-.{mount,automount}` after a full +# uninstall, the automount in `failed` state and the parent bind still mounted (demo-hp 2026-08-03). Only +# units carrying the agent's network-storage marker are touched (felhom-agent internal/storage/netmount.go +# netUnitMarker) — never an enrolled drive's unit, never anyone else's. Automounts first (else they +# re-trigger the mount), then mounts. A share that will not stop is NOT forced: it is named and its unit +# kept. Sets _NET_UNITS_REMOVED / _NET_UNITS_BUSY. Pinned by scripts/test_hostinstall.py (test_net_units_*). +NET_UNIT_DIR="/etc/systemd/system" +NET_UNIT_MARKER="Managed by felhom-agent (network storage)" +_NET_UNITS_REMOVED=() +_NET_UNITS_BUSY=() +_remove_network_storage_units() { + local kind f unit + for kind in automount mount; do + for f in "$NET_UNIT_DIR"/mnt-*."$kind"; do + [[ -f "$f" ]] || continue + grep -qF "$NET_UNIT_MARKER" "$f" 2>/dev/null || continue + unit=$(basename "$f") + run systemctl disable --now -- "$unit" 2>/dev/null || true + if ! $DRY_RUN && systemctl is-active --quiet -- "$unit" 2>/dev/null; then + log_warn " network share unit $unit will not stop (busy) — NOT forcing; its unit file is kept (named in KEPT below)." + _NET_UNITS_BUSY+=("$unit") + continue + fi + run systemctl reset-failed -- "$unit" 2>/dev/null || true + run rm -f "$f" + _NET_UNITS_REMOVED+=("$unit") + done + done + if (( ${#_NET_UNITS_REMOVED[@]} > 0 )); then + log_success " removed ${#_NET_UNITS_REMOVED[@]} network-storage unit(s): ${_NET_UNITS_REMOVED[*]}" + fi + return 0 +} + run_uninstall() { log_step "UNINSTALL — local host teardown" @@ -1260,6 +1303,8 @@ run_uninstall() { fi if [[ -f /etc/systemd/system/felhom-shared-parent.service ]]; then run rm -f /etc/systemd/system/felhom-shared-parent.service; else log_skip " felhom-shared-parent.service already absent"; fi if [[ -f /usr/local/sbin/felhom-shared-parent.sh ]]; then run rm -f /usr/local/sbin/felhom-shared-parent.sh; fi + # R-179: the NAS shares' units go BEFORE the umount loop below, so a stopped share is not "busy". + _remove_network_storage_units run systemctl daemon-reload # GL-4: unmount every enrolled/network drive mounted UNDER /mnt/felhom-drives (deepest first) # BEFORE the root self-bind. Plain umount ONLY — NEVER -l/-f: a lazy/forced unmount on a busy diff --git a/scripts/test_hostinstall.py b/scripts/test_hostinstall.py index 0384f742..c947f830 100644 --- a/scripts/test_hostinstall.py +++ b/scripts/test_hostinstall.py @@ -11,7 +11,7 @@ functions act on are variables the test points into the temp directory. Nothing Where behaviour cannot be isolated, a STATIC test checks the wiring (the function is CALLED, from the right place) — a helper defined and never called is the seam-built-never-wired shape. -Rows: R-275, R-276, R-881 (uninstall residue); R-306 (--preflight-only writes no state); R-130 (lvm minimum wording); R-180 (archive storage outside the ACL set). +Rows: R-275, R-276, R-881 (uninstall residue); R-306 (--preflight-only writes no state); R-130 (lvm minimum wording); R-180 (archive storage outside the ACL set); R-179 (NAS units). Run: python3 scripts/test_hostinstall.py """ import os @@ -97,7 +97,7 @@ class Sandbox: def stub(self, name, body="exit 0"): p = os.path.join(self.bin, name) with open(p, "w") as f: - f.write('#!/bin/sh\necho "%s $*" >> "%s"\n%s\n' % (name, self.calls, body)) + f.write('#!/bin/sh\nprintf "%%s\\n" "%s $*" >> "%s"\n%s\n' % (name, self.calls, body)) os.chmod(p, 0o755) def path(self, *parts): @@ -421,6 +421,100 @@ def test_archive_storage_check_is_wired_into_preflight(): assert pre and tok and max(pre) < tok.start(), "preflight no longer runs before the token step" +# ── R-179: the NAS network-storage units ───────────────────────────────────────────────────────── +AGENT_NETMOUNT = os.path.join(os.path.dirname(os.path.dirname(HERE)), "felhom-agent", "internal", "storage", "netmount.go") +NET_MARK = "# Managed by felhom-agent (network storage) — do not edit by hand.\n" +SHARE = r"mnt-felhom\x2ddrives-Felhom\x2dShare" +NET_SYSTEMCTL = """ +for u in "$@"; do last="$u"; done +case "$1" in + is-active) [ -e "$SB/active/$last" ]; exit $? ;; + disable) [ -e "$SB/sticky/$last" ] || rm -f "$SB/active/$last"; exit 0 ;; +esac +exit 0""" + + +def _net_sandbox(sticky=()): + sb = Sandbox() + os.mkdir(sb.path("units")); os.mkdir(sb.path("active")); os.mkdir(sb.path("sticky")) + sb.stub("systemctl", NET_SYSTEMCTL) + units = {SHARE + ".automount": NET_MARK + "[Automount]\n", + SHARE + ".mount": NET_MARK + "[Mount]\n", + r"mnt-felhom\x2ddrives-disk1.mount": "# Managed by felhom-agent — do not edit by hand.\n[Mount]\n", + "mnt-other.mount": "[Mount]\nWhere=/mnt/other\n"} + for n, c in units.items(): + open(sb.path("units", n), "w").write(c) + open(sb.path("active", SHARE + ".mount"), "w").close() + for n in sticky: + open(sb.path("sticky", n), "w").close() + return sb + + +NET_RUN = ('NET_UNIT_DIR="$SB/units"; NET_UNIT_MARKER="Managed by felhom-agent (network storage)"\n' + '_NET_UNITS_REMOVED=(); _NET_UNITS_BUSY=()\n_remove_network_storage_units\n' + 'echo "REMOVED=${#_NET_UNITS_REMOVED[@]} BUSY=${_NET_UNITS_BUSY[*]}"\n') + + +def test_net_units_removed_and_only_ours(): + sb = _net_sandbox() + try: + rc, out = sb.run(prelude() + func("_remove_network_storage_units") + NET_RUN) + assert rc == 0, out + left = sorted(os.listdir(sb.path("units"))) + assert left == sorted([r"mnt-felhom\x2ddrives-disk1.mount", "mnt-other.mount"]), \ + "wrong units left after the uninstall: %s" % left + log = sb.logged() + a = log.find("disable --now -- %s.automount" % SHARE) + m = log.find("disable --now -- %s.mount" % SHARE) + assert a >= 0 and m >= 0 and a < m, "automount must be stopped before its mount:\n%s" % log + assert "disk1" not in log and "mnt-other" not in log, "touched a unit that is not a network share:\n%s" % log + assert "REMOVED=2 BUSY=" in out, out + finally: + sb.close() + + +def test_net_units_busy_share_is_not_forced(): + sb = _net_sandbox(sticky=(SHARE + ".mount",)) + try: + rc, out = sb.run(prelude() + func("_remove_network_storage_units") + NET_RUN) + assert rc == 0, out + assert os.path.exists(sb.path("units", SHARE + ".mount")), "a busy share's unit was removed" + assert not os.path.exists(sb.path("units", SHARE + ".automount")) + assert "BUSY=%s.mount" % SHARE in out and "NOT forcing" in out, out + assert "umount" not in sb.logged(), sb.logged() + finally: + sb.close() + + +def test_net_units_dry_run_changes_nothing(): + sb = _net_sandbox() + try: + rc, out = sb.run(prelude(dry=True) + func("_remove_network_storage_units") + NET_RUN) + assert rc == 0, out + assert len(os.listdir(sb.path("units"))) == 4, os.listdir(sb.path("units")) + assert "disable" not in sb.logged(), sb.logged() + finally: + sb.close() + + +def test_net_units_wired_before_the_umount_loop(): + body = func("run_uninstall") + call = body.find("_remove_network_storage_units") + loop = body.find("findmnt -rn -o TARGET") + assert call > 0, "run_uninstall does not call _remove_network_storage_units (R-179)" + assert call < loop, "the share units must be stopped before the drive umount loop" + + +def test_net_unit_marker_matches_the_agent(): + m = re.search(r'^NET_UNIT_MARKER="([^"]+)"', SRC, re.M) + assert m, "NET_UNIT_MARKER not found" + if not os.path.exists(AGENT_NETMOUNT): + print(" note: %s absent (CI) — marker currency not checked" % AGENT_NETMOUNT) + return + a = re.search(r'netUnitMarker\s*=\s*"([^"]+)"', open(AGENT_NETMOUNT, encoding="utf-8").read()) + assert a and a.group(1) == m.group(1), "installer marker %r != agent netUnitMarker %r" % (m.group(1), a and a.group(1)) + + def main(): tests = [(n, f) for n, f in sorted(globals().items()) if n.startswith("test_") and callable(f)] fails = 0