From d7802bddb135c984022b7959c19dfa701e7bc73c Mon Sep 17 00:00:00 2001 From: Sam Rolfe Date: Mon, 12 Oct 2026 16:29:10 +1100 Subject: [PATCH] fix(ollama): toggle the NixOS system service on .27, not the deleted user unit The ollama card was wired to 'systemctl --user', but the .27 user unit was removed when Ollama moved to services.ollama (NixOS system service). The panel therefore reported Stopped while the service was Running and its buttons failed. - new controller kind 'systemd-system' -> sudo -n ... - resolve systemctl to its store path first: NixOS puts a symlink on PATH but the NOPASSWD sudoers rule names the store path, so bare 'sudo systemctl' does not match (verified: full path exit=0, bare name 'a password is required') - _remote() now shlex.join()s the remote command; ssh joins trailing argv with spaces, which was word-splitting the 'sh -c' wrapper and failing with a sudo usage error - services.toml: ollama -> kind=systemd-system, target=ollama (matches the sudoers rule verbatim), default_state=running - docs/portal-architecture.md: document the three controller kinds + gotchas Verified end-to-end from .13: start->running, stop->stopped, start->running, real systemctl agrees, and /v1/models lists both qwen2.5 models over the LAN. --- .../service-controller/service_controller.py | 64 ++++++++++++++----- deploy/service-controller/services.toml | 19 ++++-- docs/portal-architecture.md | 22 +++++++ 3 files changed, 85 insertions(+), 20 deletions(-) diff --git a/deploy/service-controller/service_controller.py b/deploy/service-controller/service_controller.py index 716906c..d1fa35b 100644 --- a/deploy/service-controller/service_controller.py +++ b/deploy/service-controller/service_controller.py @@ -19,6 +19,7 @@ from __future__ import annotations import json import os +import shlex import subprocess import threading import time @@ -84,29 +85,62 @@ def _container_state(target: str) -> str: return out or ("unknown" if r.returncode != 0 else "unknown") -def _systemd_user_state(target: str, remote: str | None = None) -> str: - cmd = ["systemctl", "--user", "is-active", target] - if remote: - cmd = ["ssh", "-o", "StrictHostKeyChecking=no", "-o", "BatchMode=yes", remote] + cmd - r = subprocess.run(cmd, capture_output=True, text=True) +SSH_OPTS = ["-o", "StrictHostKeyChecking=no", "-o", "BatchMode=yes"] + + +def _remote(cmd: list[str], remote: str | None) -> list[str]: + """Wrap a command in ssh when the target service lives on another host. + + ssh joins its trailing arguments with spaces before the remote shell parses + them, so the command is quoted as one string first — otherwise remote + commands containing quotes or shell syntax (e.g. the `sh -c` used by + systemd-system) get word-split and fail. + """ + return ["ssh", *SSH_OPTS, remote, shlex.join(cmd)] if remote else cmd + + +def _systemd_state(target: str, remote: str | None = None, system: bool = False) -> str: + """`is-active` needs no privilege, so this never uses sudo.""" + cmd = ["systemctl", "is-active", target] if system else ["systemctl", "--user", "is-active", target] + r = subprocess.run(_remote(cmd, remote), capture_output=True, text=True) out = (r.stdout.strip() or "inactive").lower() - if out == "active": - return "running" - return out + return "running" if out == "active" else out def _do(kind: str, action: str, target: str, remote: str | None = None) -> str: - """Run start/stop. Returns the state after a short settle.""" + """Run start/stop. Returns the state after a short settle. + + Kind semantics: + container -> docker (sam is in the docker group) + systemd-user -> systemctl --user (local user units on .13) + systemd-system -> sudo -n + (system units; `remote` sends it over ssh) + + `systemd-system` requires a NOPASSWD sudoers rule for that exact + command+argument on the target host. On .27 this is granted for + `systemctl start|stop|restart ollama` (see hosts/sam-4screen-desktop), + where `target` must match the rule verbatim: "ollama", not "ollama.service". + + Note: sudo matches the *literal* PATH-resolved path, and NixOS puts a + symlink (`/run/current-system/sw/bin/systemctl`) on PATH, so a bare + `sudo systemctl ...` does not match a store-path rule. We therefore resolve + `systemctl` to its canonical store path first. This also keeps working after + a nixpkgs bump changes the systemd store hash. + """ if kind == "container": subprocess.run(["docker", action, target], capture_output=True, text=True) time.sleep(1.5) return _container_state(target) - cmd = ["systemctl", "--user", action, target] - if remote: - cmd = ["ssh", "-o", "StrictHostKeyChecking=no", "-o", "BatchMode=yes", remote] + cmd - subprocess.run(cmd, capture_output=True, text=True) + if kind == "systemd-system": + shell = f'sudo -n "$(readlink -f "$(command -v systemctl)")" {action} {target}' + cmd = ["sh", "-c", shell] + else: + cmd = ["systemctl", "--user", action, target] + res = subprocess.run(_remote(cmd, remote), capture_output=True, text=True) + if res.returncode != 0 and kind == "systemd-system": + audit(f"WARN {kind} {action} {target} exit={res.returncode} {res.stderr.strip()[:160]}") time.sleep(1.5) - return _systemd_user_state(target, remote=remote) + return _systemd_state(target, remote=remote, system=(kind == "systemd-system")) def _targets(s: dict[str, Any]) -> list[str]: @@ -128,7 +162,7 @@ def _probe(s: dict[str, Any]) -> tuple[str, list[str]]: if kind == "container": states.append(_container_state(tgt)) else: - states.append(_systemd_user_state(tgt, remote=remote)) + states.append(_systemd_state(tgt, remote=remote, system=(kind == "systemd-system"))) n_ok = sum(1 for st in states if st in ("running", "active", "started")) n_none = sum(1 for st in states if st in ("unknown",)) or 0 if n_ok == len(states): diff --git a/deploy/service-controller/services.toml b/deploy/service-controller/services.toml index 34c4655..b180a5b 100644 --- a/deploy/service-controller/services.toml +++ b/deploy/service-controller/services.toml @@ -5,8 +5,12 @@ # voice/whisper/HA path, or n8n/prefect-server (shared infra, always on). # # kind: -# container -> docker start/stop (sam is in the docker group) -# systemd-user-> systemctl --user start/stop +# container -> docker start/stop (sam is in the docker group) +# systemd-user -> systemctl --user start/stop (local .13 user units) +# systemd-system -> sudo -n systemctl start/stop (system units; `remote` +# sends it over ssh). Needs a matching NOPASSWD sudoers rule on +# the target host, and `target` must match it verbatim +# ("ollama", not "ollama.service"). # # group "media": family-facing editors (shared by everyone). # group "pipeline": photo ingestion (prefect worker + dashboard) — admin only. @@ -95,13 +99,18 @@ ram_mb = 120 default_state = "running" who = ["sam"] +# Ollama on .27 — now a NixOS *system* service (services.ollama in +# hosts/sam-4screen-desktop/configuration.nix). The old user unit was deleted, so +# this must be toggled with `sudo systemctl`, not `systemctl --user`. Bound to +# 0.0.0.0:11434 for OmniRoute/dsh on .13. Models live in /var/lib/ollama/models. +# RAM is the loaded model; OLLAMA_KEEP_ALIVE=5m unloads it when idle. [[services]] id = "ollama" -kind = "systemd-user" -target = "ollama.service" +kind = "systemd-system" +target = "ollama" remote = "sam@192.168.20.27" group = "ai" label = "Local LLM Server (Ollama .27)" ram_mb = 9000 -default_state = "stopped" +default_state = "running" who = ["sam", "jo", "harry", "finn"] diff --git a/docs/portal-architecture.md b/docs/portal-architecture.md index eff586e..68b3c0e 100644 --- a/docs/portal-architecture.md +++ b/docs/portal-architecture.md @@ -112,6 +112,28 @@ To allow family members to free up RAM on `.13` without exposing `/var/run/docke | `prefect-worker` | `prefect-worker` (systemd user) | ~129 MB | `pipeline` | `sam` | | `photo-dashboard` | `photo-dashboard` (systemd user) | ~112 MB | `pipeline` | `sam` | | `worldmonitor` | 4-container group (Container) | ~120 MB | `admin` | `sam` | +| `ollama` | `ollama` on **`.27`** (`systemd-system`, via ssh) | ~9000 MB (loaded model) | `ai` | `sam`, `jo`, `harry`, `finn` | + +### Controller kinds (`kind` in `services.toml`) + +| `kind` | How it acts | Notes | +| :--- | :--- | :--- | +| `container` | `docker start/stop ` | Same host (`.13`); `sam` is in the `docker` group. | +| `systemd-user` | `systemctl --user ` | Local `.13` user units (paperclip, prefect-worker, photo-dashboard). | +| `systemd-system` | `sudo -n ` | System units, local or on a `remote` host over ssh. | + +**`systemd-system` gotchas** (both were hit while wiring `ollama`): + +1. It needs a matching **NOPASSWD** sudoers rule on the target host, and the + `target` must match the rule verbatim — `ollama`, never `ollama.service`. +2. On NixOS, `systemctl` on `PATH` is a symlink (`/run/current-system/sw/bin/systemctl`) + while the sudoers rule names the store path, and sudo matches the *literal* + resolved path. A bare `sudo systemctl …` therefore does **not** match. The + controller resolves `systemctl` to its canonical store path first, which also + survives nixpkgs bumps that change the systemd store hash. +3. `_remote()` quotes the remote command with `shlex.join` before handing it to + ssh, because ssh joins its trailing argv with spaces before the remote shell + parses it. ---