Compare commits

..
Author SHA1 Message Date
ChuckBuilds cf521bdfd8 fix(startup): warn when an installed systemd unit has drifted from the repo's
Nothing re-applies systemd units after the first install. `git pull` -- which
is what the web UI's update button runs -- brings a new template into the
checkout, but no code in web_interface/ or src/ copies it to
/etc/systemd/system, and nothing anywhere runs `systemctl daemon-reload`. The
unit that actually runs is whatever first_time_install.sh wrote on day one.

So every hardening added to a unit is inert on existing installs, silently.
Measured on a live rig:

    installed  /etc/systemd/system/ledmatrix.service   2026-08-06
    template   systemd/ledmatrix.service               2026-08-19
    contents                                           differ

with the practical result that the MemoryMax=85% the repo's template specifies
was not being enforced at all -- `systemctl show` reported
MemoryMax=infinity. Anyone reading the template would reasonably believe the
service was capped.

Startup now compares each installed unit against its substituted template and
warns when they differ, naming install_service.sh as the remedy.

A warning, not an error, and deliberately not a silent rewrite: editing files
under /etc and restarting services is the installer's job, not something a
display process should do to a machine while it is booting. Making it fatal
would also brick every development checkout whose unit is legitimately absent
or hand-edited.

Comparison ignores comments, blank lines and ordering. The template carries
explanatory comments the installed copy will not have, and systemd does not
care about order within a section, so a literal comparison would warn on every
boot and be ignored within a week.

Mutation-checked three ways: never reporting drift fails, making it fatal
fails, and -- after the first attempt missed it -- comparing raw text now fails
too. That last gap is worth noting: the comment-insensitivity tests originally
exercised the helper directly, so a comparison that stopped calling the helper
passed them all. The test that catches it goes through _validate_systemd_units.

29 startup-validator tests pass.
2026-08-20 00:48:06 -04:00
4 changed files with 201 additions and 107 deletions
+68
View File
@@ -63,6 +63,9 @@ class StartupValidator:
if self.plugin_manager: if self.plugin_manager:
self._validate_plugins() self._validate_plugins()
# Warn when the running systemd unit no longer matches the repo's
self._validate_systemd_units()
is_valid = len(self.errors) == 0 is_valid = len(self.errors) == 0
if is_valid: if is_valid:
@@ -74,6 +77,71 @@ class StartupValidator:
return (is_valid, self.errors.copy(), self.warnings.copy()) return (is_valid, self.errors.copy(), self.warnings.copy())
#: Units this project installs, and where each is installed to.
_UNITS = (
("systemd/ledmatrix.service", "/etc/systemd/system/ledmatrix.service"),
("systemd/ledmatrix-web.service", "/etc/systemd/system/ledmatrix-web.service"),
)
def _validate_systemd_units(self) -> None:
"""Warn when an installed unit has drifted from the repo's template.
Nothing re-applies these after the first install. `git pull` -- which is
what the web UI's update button runs -- brings a new template into the
checkout, but nothing copies it to /etc/systemd/system and nothing runs
`systemctl daemon-reload`, so the unit that actually runs is whatever
first_time_install.sh wrote on day one.
That makes every hardening added to a unit inert on existing installs.
Measured on one rig: the installed unit was thirteen days older than the
repo's and differed in content, so a MemoryMax the repo had specified
was not being enforced at all -- `systemctl show` reported
MemoryMax=infinity.
A warning rather than an error, and certainly not a silent rewrite:
editing files under /etc and restarting services is the installer's job,
not something a display process should do to a machine while it boots.
The remedy is to re-run scripts/install/install_service.sh.
"""
try:
project_root = Path(__file__).resolve().parent.parent
for template_rel, installed_path in self._UNITS:
template = project_root / template_rel
installed = Path(installed_path)
if not template.is_file() or not installed.is_file():
continue
# The template carries placeholders the installer substitutes,
# so compare the substituted form rather than the raw file.
expected = template.read_text(encoding="utf-8")
expected = expected.replace("__PROJECT_ROOT_DIR__", str(project_root))
expected = expected.replace("__USER__", "root")
try:
actual = installed.read_text(encoding="utf-8")
except PermissionError:
continue
if self._unit_body(expected) != self._unit_body(actual):
self.warnings.append(
f"{installed.name} differs from {template_rel}; the "
"installed unit is not refreshed by an update, so "
"settings added to the template are not in effect. "
"Re-run scripts/install/install_service.sh to apply them."
)
except OSError as e:
self.logger.debug("Could not compare systemd units: %s", e)
@staticmethod
def _unit_body(text: str) -> str:
"""A unit's meaningful lines: no comments, no blanks, no ordering noise."""
lines = []
for line in text.splitlines():
line = line.strip()
if line and not line.startswith("#"):
lines.append(line)
return "\n".join(sorted(lines))
def _validate_config(self) -> None: def _validate_config(self) -> None:
"""Validate configuration files.""" """Validate configuration files."""
try: try:
-12
View File
@@ -8,18 +8,6 @@ Type=simple
User=root User=root
WorkingDirectory=__PROJECT_ROOT_DIR__ WorkingDirectory=__PROJECT_ROOT_DIR__
Environment=PYTHONDONTWRITEBYTECODE=1 Environment=PYTHONDONTWRITEBYTECODE=1
# glibc gives each allocating thread its own malloc arena, up to 8 x CPU count,
# and an arena that has grown is never handed back to the OS. This process runs
# 9 threads on a 3-core Pi, so the ceiling is 24 arenas -- and a rig measured at
# 1030 MB resident held 23 large anonymous mappings on 64 MB-aligned addresses,
# 920 MB of them, while the live data it was actually holding (widest scroll
# strip seen: 35,746 x 64) accounts for roughly 15 MB. That gap is arena bloat,
# not leaked objects: RSS was flat across repeated sampling, not climbing.
#
# Capping the arenas trades a little allocator concurrency for a large amount of
# resident memory on a device that has neither to spare. 2 is the usual value;
# raise it if frame times regress.
Environment=MALLOC_ARENA_MAX=2
ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py ExecStart=/usr/bin/python3 __PROJECT_ROOT_DIR__/run.py
# Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken # Restart=always, not on-failure: run.py exiting 0 (a clean shutdown path taken
# for a reason that no longer applies, e.g. a config reload) would otherwise leave # for a reason that no longer applies, e.g. a config reload) would otherwise leave
-95
View File
@@ -1,95 +0,0 @@
"""The display unit must cap glibc's malloc arenas.
glibc hands each allocating thread its own malloc arena, up to 8 x CPU count,
and an arena that has grown is never returned to the OS. This process runs
threads for the render loop, the update workers and the background fetchers, so
on a 3-core Pi the ceiling is 24 arenas.
Measured on a live rig, 2.5 hours in:
RSS 1030 MB
Private_Dirty 988 MB
anonymous mappings > 10 MB 23 (ceiling is 8 x 3 = 24)
largest few 104, 79, 66, 63, 63 MB, on 64 MB-aligned addresses
against live data that accounts for perhaps 15 MB -- the widest scroll strip
observed was 35,746 x 64, about 7 MB as RGB and the same again for its numpy
mirror. Repeated sampling showed RSS flat between 990 and 1030 MB rather than
climbing, so this is arena bloat rather than a leak: memory Python has freed
but glibc is holding per-arena.
The device had 59 MB free at the time.
Capping the arena count trades a little allocator concurrency for that resident
memory. The render loop is latency-sensitive, so if p99 frame time regresses the
right response is to raise this rather than remove it.
"""
import re
from pathlib import Path
import pytest
UNIT = (Path(__file__).resolve().parent.parent / "systemd" / "ledmatrix.service")
#: The value the unit is expected to carry. 2 is the usual choice for a
#: threaded Python process; 1-4 all keep some of the saving, but only one of
#: them is what this project ships.
EXPECTED_ARENA_MAX = 2
def _environment(unit_text):
return dict(
line.split("=", 2)[1:3] if line.count("=") >= 2 else (line.split("=", 1)[1], "")
for line in unit_text.splitlines()
if line.startswith("Environment=")
)
def test_the_unit_exists():
assert UNIT.is_file(), f"{UNIT} is missing"
def test_malloc_arena_max_is_capped():
env = _environment(UNIT.read_text(encoding="utf-8"))
assert "MALLOC_ARENA_MAX" in env, (
"the display unit does not cap glibc arenas; on a 3-core Pi the default "
"ceiling is 24 and a measured rig held 23 of them, 920 MB"
)
value = int(env["MALLOC_ARENA_MAX"])
# Pinned, not a range. A range let a change to 4 -- which hands most of the
# saving back -- pass unnoticed, which was the point of the finding that
# prompted this. Raising it is a legitimate response to a frame-time
# regression, but it should be a visible edit here rather than a silent
# drift, so the number lives in one place and changing it shows up in
# review.
assert value == EXPECTED_ARENA_MAX, (
f"MALLOC_ARENA_MAX={value}, expected {EXPECTED_ARENA_MAX}. If this was "
"raised deliberately because frame times regressed, update "
"EXPECTED_ARENA_MAX here and say so in the commit."
)
def test_the_reason_is_recorded_next_to_it():
"""A bare tuning knob invites removal by whoever meets it next."""
text = UNIT.read_text(encoding="utf-8")
index = text.index("Environment=MALLOC_ARENA_MAX")
preamble = text[:index].splitlines()[-12:]
comment = "\n".join(line for line in preamble if line.startswith("#"))
assert "arena" in comment.lower(), "no explanation precedes the setting"
assert re.search(r"\d", comment), (
"the explanation cites no measurement, so a reader cannot tell whether "
"it still applies to their hardware"
)
@pytest.mark.parametrize("unit", ["ledmatrix.service"])
def test_the_unit_still_parses_as_ini(unit):
"""systemd will refuse a malformed unit, and the panel stays dark."""
import configparser
path = UNIT.parent / unit
parser = configparser.ConfigParser(strict=False)
# systemd allows repeated keys; ConfigParser needs them merged, not rejected.
parser.read_string(path.read_text(encoding="utf-8"))
assert parser.has_section("Service")
assert parser.has_option("Service", "ExecStart")
+133
View File
@@ -0,0 +1,133 @@
"""An installed unit that no longer matches the repo's must be reported.
Nothing re-applies systemd units after the first install. `git pull` -- what
the web UI's update button runs -- brings a new template into the checkout, but
no code in web_interface/ or src/ copies it to /etc/systemd/system or runs
`systemctl daemon-reload`. The unit that actually runs is whatever
first_time_install.sh wrote on day one.
So every hardening added to a unit is inert on existing installs. Measured on a
live rig: the installed unit was dated 2026-08-06 and the repo's 2026-08-19,
and they differed -- with the result that a MemoryMax=85% present in the repo's
template was not being enforced at all. `systemctl show` reported
MemoryMax=infinity.
This is a warning, not an error, and deliberately not a silent rewrite:
editing files under /etc and restarting services is the installer's job, not
something a display process should do to a machine while it boots.
"""
import logging
from pathlib import Path
from unittest.mock import MagicMock
import pytest
from src.startup_validator import StartupValidator
@pytest.fixture
def validator():
v = StartupValidator(config_manager=MagicMock())
v.logger = logging.getLogger("test")
v.warnings = []
v.errors = []
return v
def test_a_matching_unit_produces_no_warning(validator, tmp_path):
"""The installed unit, substituted exactly as the installer would."""
project_root = Path("src/startup_validator.py").resolve().parent.parent
template_rel = "systemd/ledmatrix.service"
template = project_root / template_rel
if not template.is_file():
pytest.skip("repo unit template not present")
installed = tmp_path / "ledmatrix.service"
installed.write_text(
template.read_text(encoding="utf-8")
.replace("__PROJECT_ROOT_DIR__", str(project_root))
.replace("__USER__", "root"),
encoding="utf-8")
validator._UNITS = ((template_rel, str(installed)),)
validator._validate_systemd_units()
assert not validator.warnings, f"a matching unit warned: {validator.warnings}"
assert not validator.errors
def test_comments_and_blank_lines_are_not_drift():
"""Otherwise every comment the repo adds would look like a changed unit."""
a = "[Service]\n# explains a setting\nExecStart=/x\nRestart=always\n"
b = "[Service]\nExecStart=/x\n\nRestart=always\n"
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
def test_a_changed_directive_is_drift():
a = "[Service]\nExecStart=/x\nMemoryMax=85%\n"
b = "[Service]\nExecStart=/x\n"
assert StartupValidator._unit_body(a) != StartupValidator._unit_body(b)
def test_reordered_directives_are_not_drift():
"""systemd does not care about order within a section, so neither should this."""
a = "[Service]\nExecStart=/x\nRestart=always\n"
b = "[Service]\nRestart=always\nExecStart=/x\n"
assert StartupValidator._unit_body(a) == StartupValidator._unit_body(b)
def test_cosmetic_differences_do_not_warn(validator, tmp_path):
"""Through the real comparison, not the helper.
The repo's template carries explanatory comments the installed copy may not
have, and the installer does not preserve ordering or blank lines. If those
counted as drift, every boot would warn and the warning would be ignored.
Asserting this on _unit_body alone would not catch a comparison that stopped
calling it -- which is exactly what a careless edit does.
"""
project_root = Path("src/startup_validator.py").resolve().parent.parent
template_rel = "systemd/ledmatrix.service"
template = project_root / template_rel
if not template.is_file():
pytest.skip("repo unit template not present")
substituted = (template.read_text(encoding="utf-8")
.replace("__PROJECT_ROOT_DIR__", str(project_root))
.replace("__USER__", "root"))
# Same directives, stripped of comments and blank lines and reordered.
directives = sorted(line.strip() for line in substituted.splitlines()
if line.strip() and not line.strip().startswith("#"))
installed = tmp_path / "ledmatrix.service"
installed.write_text("\n".join(reversed(directives)) + "\n", encoding="utf-8")
validator._UNITS = ((template_rel, str(installed)),)
validator._validate_systemd_units()
assert not validator.warnings, (
f"cosmetic-only difference reported as drift: {validator.warnings}")
def test_drift_is_reported_as_a_warning(validator, tmp_path):
"""The whole point: a real difference must surface, and only as a warning."""
installed = tmp_path / "ledmatrix.service"
installed.write_text("[Service]\nExecStart=/usr/bin/python3 /x/run.py\n")
project_root = Path("src/startup_validator.py").resolve().parent.parent
template_rel = "systemd/ledmatrix.service"
template = project_root / template_rel
if not template.is_file():
pytest.skip("repo unit template not present")
validator._UNITS = ((template_rel, str(installed)),)
validator._validate_systemd_units()
assert validator.warnings, "a differing unit produced no warning"
assert "install_service.sh" in validator.warnings[0], (
"the warning does not tell the user how to fix it")
assert not validator.errors, "drift must not be fatal at startup"
def test_a_missing_installed_unit_is_silent(validator, tmp_path):
"""Development checkouts have no /etc/systemd unit; that is not drift."""
validator._UNITS = (("systemd/ledmatrix.service", str(tmp_path / "absent.service")),)
validator._validate_systemd_units()
assert not validator.warnings
assert not validator.errors