fix(install): render the systemd units from their templates, not from heredocs

The installers carried their own inline copies of units that also exist as
templates under systemd/, and the copies drifted.

install_service.sh renders ledmatrix.service from the template correctly, then
wrote ledmatrix-web.service from a heredoc that predated it -- missing
Wants=network-online.target, RestartSec=10, SyslogIdentifier, CacheDirectory,
CacheDirectoryMode and Environment=USE_THREADING=1. install_web_service.sh had
a third copy, and install_wifi_monitor.sh a fourth, that one already differing
from its template (syslog where the template says journal).

startup_validator.py compares the installed unit against the template, so a
rig installed this way warned on every boot -- and the remedy the warning
names, "re-run scripts/install/install_service.sh", reinstalled the same stale
copy. The warning could never clear. Reproduced on a live rig running exactly
that unit.

All three installers now render systemd/*.service through the same placeholder
substitution. The template gains a __USER__ placeholder rather than hardcoding
User=root, because the web interface runs as whoever installed it.

That last point was a second, independent cause of a permanent warning: the
validator substituted a fixed "root", so any non-root install reported drift
forever. It now reads User= from the installed unit -- an install-time
decision, not something the template dictates -- and compares everything else
strictly. first_time_install.sh already reads the installed User= the same way.

Tests cover a non-root web unit not warning, a genuinely changed directive in
that unit still warning, the User= fallback, and a grep-based guard that no
installer under scripts/install/ contains an inline unit body. That guard is
what found the install_wifi_monitor.sh copy.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
This commit is contained in:
ChuckBuilds
2026-09-09 18:17:35 -04:00
co-authored by Claude Opus 5
parent a29c84208e
commit 94f70eab31
6 changed files with 176 additions and 95 deletions
+89
View File
@@ -150,3 +150,92 @@ def test_a_missing_installed_unit_is_silent(validator, tmp_path):
validator._validate_systemd_units()
assert not validator.warnings
assert not validator.errors
# --- the web unit: one template, three copies, and a warning that never cleared ---
#
# install_service.sh and install_web_service.sh each carried their own inline
# heredoc of ledmatrix-web.service. install_service.sh's had drifted -- no
# Wants=network-online.target, RestartSec, SyslogIdentifier or CacheDirectory --
# and that is what was installed on real rigs. The validator correctly reported
# the drift and told the user to re-run install_service.sh, which reinstalled the
# same stale copy, so the warning could never clear. Separately, the template
# hardcoded User=root while the installers write whoever ran them, so even the
# *correct* installer produced a permanent warning on any non-root install.
def _render(template_text, project_root, user):
"""Exactly what the installers' sed does."""
return (template_text
.replace("__PROJECT_ROOT_DIR__", str(project_root))
.replace("__USER__", user))
def test_the_web_unit_installed_as_a_non_root_user_is_not_drift(validator, tmp_path):
"""The web interface runs as whoever installed it, not as root.
This is the case that warned forever: nothing the user could do would make
an installed `User=pi` match a template that said `User=root`.
"""
project_root = Path("src/startup_validator.py").resolve().parent.parent
template_rel = "systemd/ledmatrix-web.service"
template = project_root / template_rel
if not template.is_file():
pytest.skip("repo unit template not present")
installed = tmp_path / "ledmatrix-web.service"
installed.write_text(
_render(template.read_text(encoding="utf-8"), project_root, "hdpi"),
encoding="utf-8")
validator._UNITS = ((template_rel, str(installed)),)
validator._validate_systemd_units()
assert not validator.warnings, (
f"a correctly installed non-root web unit warned: {validator.warnings}")
def test_the_web_unit_still_reports_a_real_changed_directive(validator, tmp_path):
"""Ignoring User= must not make the check blind to everything else."""
project_root = Path("src/startup_validator.py").resolve().parent.parent
template_rel = "systemd/ledmatrix-web.service"
template = project_root / template_rel
if not template.is_file():
pytest.skip("repo unit template not present")
rendered = _render(template.read_text(encoding="utf-8"), project_root, "hdpi")
# Drop RestartSec -- one of the directives the stale heredoc was missing.
stale = "\n".join(l for l in rendered.splitlines() if not l.startswith("RestartSec="))
installed = tmp_path / "ledmatrix-web.service"
installed.write_text(stale + "\n", encoding="utf-8")
validator._UNITS = ((template_rel, str(installed)),)
validator._validate_systemd_units()
assert validator.warnings, "a web unit missing RestartSec= produced no warning"
assert not validator.errors
def test_installed_user_falls_back_to_root():
"""systemd defaults a system unit with no User= to root, so we must too."""
assert StartupValidator._installed_user("[Service]\nExecStart=/x\n") == "root"
assert StartupValidator._installed_user("[Service]\nUser=pi\n") == "pi"
assert StartupValidator._installed_user("[Service]\n User=hdpi \n") == "hdpi"
def test_no_installer_carries_its_own_copy_of_a_unit():
"""The regression guard.
Both installers used to inline the unit as a heredoc, and the two copies
drifted from the template and from each other. A unit body in a shell script
is the bug, so assert there isn't one rather than asserting the current
contents match -- matching contents is exactly what silently stops being
true.
"""
project_root = Path("src/startup_validator.py").resolve().parent.parent
offenders = []
for script in sorted((project_root / "scripts" / "install").glob("*.sh")):
text = script.read_text(encoding="utf-8", errors="replace")
if "[Unit]" in text and "Description=" in text:
offenders.append(script.name)
assert not offenders, (
f"{offenders} contain an inline systemd unit; render "
f"systemd/*.service instead so there is one source of truth")