diff options
Diffstat (limited to 'packages/meshbay-node')
| -rw-r--r-- | packages/meshbay-node/src/meshbay_node/daemon.py | 113 | ||||
| -rw-r--r-- | packages/meshbay-node/tests/test_cli_dispatch.py | 60 |
2 files changed, 101 insertions, 72 deletions
diff --git a/packages/meshbay-node/src/meshbay_node/daemon.py b/packages/meshbay-node/src/meshbay_node/daemon.py index 5fc70fe..b34e710 100644 --- a/packages/meshbay-node/src/meshbay_node/daemon.py +++ b/packages/meshbay-node/src/meshbay_node/daemon.py @@ -888,6 +888,32 @@ def _resolve_group(cfg: Config, group: str | None) -> str: sys.exit(1) +def _systemctl_user(verb: str, unit: str, *, not_running_hint: str, + success: str, watch: str | None) -> None: + """ + Run `systemctl --user <verb> <unit>` and report the result. + + The lifecycle authority is the unit, not this process: systemd already + knows which PID it started, restarts it on failure (`Restart=on-failure` + in the unit) and reloads it correctly (`ExecReload=`). Anything this CLI + did instead — finding a process by pattern-matching its command line, + signalling it, respawning it — is a second, worse implementation of what + systemd is already doing, and pattern-matching a process list has already + hit a real developer's real running node by accident. + """ + import subprocess + + result = subprocess.run(["systemctl", "--user", verb, unit], + capture_output=True, text=True) + if result.returncode != 0: + detail = (result.stderr or result.stdout).strip() + print(detail or not_running_hint) + sys.exit(1) + print(success) + if watch: + print(watch) + + # ── Entry point ─────────────────────────────────────────────────────────────── def main() -> None: @@ -904,7 +930,8 @@ def main() -> None: "browser with this node | member list|invite|revoke|unpin " "| group list|add|remove | gek init|rotate | file list|rm " "| denylist show|clear | reload: re-read node.toml " - "| restart-daemon: full stop + start " + "(systemctl --user reload) | restart-daemon: restart " + "the systemd unit (systemctl --user restart) " "| calibrate-argon2: benchmark") parser.add_argument("subcommand", nargs="?", help="'pair' for operator; list|invite|revoke|unpin for " @@ -1159,69 +1186,33 @@ def main() -> None: if args.command == "reload": # Milestone 14.8. The daemon re-reads node.toml; groups that appeared or # whose roots changed are picked up without dropping live connections. - cfg = load_config(args.config or DEFAULT_CONFIG_PATH) - import os as _os - import signal as _signal - import subprocess as _subprocess - # `--` is pgrep's own end-of-options marker and must be its own argument; - # folded into the pattern it searches for a process literally called - # "-- -m …". Anchored to the end of the command line so it matches the - # daemon and never a shell that merely mentions it — the same trap - # deploy-node.sh documents, where an unanchored pattern kills the script. - pid_out = _subprocess.run( - ["pgrep", "-f", "--", r"-m meshbay_node\.daemon$"], - capture_output=True, text=True) - pids = [int(x) for x in pid_out.stdout.split()] - if not pids: - print("Node is not running — start it with: meshbay-node") - sys.exit(1) - for pid in pids: - _os.kill(pid, _signal.SIGHUP) - print(f"sent SIGHUP to {len(pids)} daemon process(es)") - print("watch the result: tail -f /tmp/meshbay-node.log") + # + # Delegated to systemd rather than hunting a PID with pgrep and signalling + # it directly: an unanchored (or merely unlucky) pattern match there has + # already SIGHUPed a developer's own running node by accident — see the + # comment this replaced, and test_cli_dispatch.py's stub_daemon fixture, + # which had to stub os.kill for exactly that reason. The unit already + # declares `ExecReload=/bin/kill -HUP $MAINPID`, so systemd sends the + # signal to the one process it actually started. + _systemctl_user( + "reload", "meshbay-node", + not_running_hint="Node is not running as a systemd unit — start it " + "with: systemctl --user start meshbay-node", + success="sent reload to meshbay-node", + watch="watch the result: journalctl --user -u meshbay-node -f") return if args.command == "restart-daemon": - import os as _os - import signal as _signal - import subprocess as _subprocess - cfg = load_config(args.config or DEFAULT_CONFIG_PATH) - pid_out = _subprocess.run( - ["pgrep", "-f", "--", r"-m meshbay_node\.daemon$"], - capture_output=True, text=True) - pids = [int(x) for x in pid_out.stdout.split()] - if pids: - for pid in pids: - _os.kill(pid, _signal.SIGTERM) - print(f"stopped {len(pids)} daemon process(es)") - for pid in pids: - try: - _os.waitpid(pid, 0) - except ChildProcessError: - import time as _time - _time.sleep(2) - else: - print("no running daemon found — starting fresh") - log_path = "/tmp/meshbay-node.log" - config_flag = ["--config", str(args.config)] if args.config else [] - _subprocess.Popen( - [sys.executable, "-m", "meshbay_node.daemon"] + config_flag, - stdout=open(log_path, "a"), - stderr=_subprocess.STDOUT, - start_new_session=True, - ) - import time as _time - _time.sleep(3) - pid_out2 = _subprocess.run( - ["pgrep", "-f", "--", r"-m meshbay_node\.daemon$"], - capture_output=True, text=True) - new_pids = [int(x) for x in pid_out2.stdout.split()] - if new_pids: - print(f"daemon started (PID {new_pids[0]})") - print(f"log: tail -f {log_path}") - else: - print(f"daemon may have failed to start — check {log_path}") - sys.exit(1) + # Same reasoning as reload: no PID hunting, no manual respawn — systemd + # already knows how to stop and start this unit, and does not need this + # process to guess where its log file is. + _systemctl_user( + "restart", "meshbay-node", + not_running_hint="meshbay-node is not installed as a systemd unit — " + "see packaging/systemd/", + success="meshbay-node restarted via systemd", + watch="check status: systemctl --user status meshbay-node\n" + "watch logs: journalctl --user -u meshbay-node -f") return if args.command == "denylist": diff --git a/packages/meshbay-node/tests/test_cli_dispatch.py b/packages/meshbay-node/tests/test_cli_dispatch.py index fdcd82e..d9edb17 100644 --- a/packages/meshbay-node/tests/test_cli_dispatch.py +++ b/packages/meshbay-node/tests/test_cli_dispatch.py @@ -77,19 +77,21 @@ def stub_daemon(monkeypatch, tmp_path): import getpass monkeypatch.setattr(getpass, "getpass", lambda *a, **kw: "test-password") - # `reload` looks for a real daemon and signals it. Without this the test - # SIGHUPs whatever node happens to be running on the machine — which it did, - # once, before this was added. A test must not reach outside itself. - import os + # `reload` and `restart-daemon` shell out to `systemctl --user`. Without + # this the test would run that against whatever session bus is actually + # available — a test must not reach outside itself, which is exactly what + # the pgrep/os.kill version of this fixture existed to prevent before + # those commands were rewritten to delegate to systemd. import subprocess - signalled: list[int] = [] - monkeypatch.setattr( - subprocess, "run", - lambda *a, **k: subprocess.CompletedProcess(a[0], 0, stdout="4242\n", - stderr="")) - monkeypatch.setattr(os, "kill", lambda pid, sig: signalled.append(pid)) - calls.append(("_signalled", signalled)) + systemctl_calls: list[list[str]] = [] + + def fake_run(argv, **kw): + systemctl_calls.append(argv) + return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") + + monkeypatch.setattr(subprocess, "run", fake_run) + calls.append(("_systemctl_calls", systemctl_calls)) return calls @@ -130,3 +132,39 @@ def test_the_verb_list_here_matches_the_parser(): assert not untested, ( f"CLI verbs with no dispatch test: {sorted(untested)} — add them to " f"VERBS above") + + +@pytest.mark.parametrize("argv,verb", [ + (["reload"], "reload"), + (["restart-daemon"], "restart"), +]) +def test_lifecycle_commands_delegate_to_systemctl_user( + argv, verb, stub_daemon, monkeypatch, capsys): + """ + `reload` and `restart-daemon` must ask systemd to do it, not hunt a PID + with pgrep and signal it directly — that pattern-matched a developer's own + running node by accident once, which is why it was replaced. + """ + monkeypatch.setattr(sys, "argv", ["meshbay-node", *argv]) + daemon_mod.main() + + systemctl_calls = dict(stub_daemon)["_systemctl_calls"] + assert systemctl_calls == [["systemctl", "--user", verb, "meshbay-node"]] + assert not capsys.readouterr().err + + +@pytest.mark.parametrize("argv", [["reload"], ["restart-daemon"]]) +def test_lifecycle_commands_report_systemctl_failure( + argv, stub_daemon, monkeypatch, capsys): + """A unit that refuses (not installed, not running) must exit non-zero.""" + import subprocess + + monkeypatch.setattr( + subprocess, "run", + lambda a, **k: subprocess.CompletedProcess( + a, 1, stdout="", stderr="Unit meshbay-node.service not loaded.\n")) + monkeypatch.setattr(sys, "argv", ["meshbay-node", *argv]) + with pytest.raises(SystemExit) as exc: + daemon_mod.main() + assert exc.value.code == 1 + assert "not loaded" in capsys.readouterr().out |