mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-20 18:09:05 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
500cfbc9f4 | ||
|
|
a372b43cd1 |
@@ -63,9 +63,6 @@ class StartupValidator:
|
||||
if self.plugin_manager:
|
||||
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
|
||||
|
||||
if is_valid:
|
||||
@@ -77,71 +74,6 @@ class StartupValidator:
|
||||
|
||||
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:
|
||||
"""Validate configuration files."""
|
||||
try:
|
||||
|
||||
@@ -8,18 +8,6 @@ Type=simple
|
||||
User=root
|
||||
WorkingDirectory=__PROJECT_ROOT_DIR__
|
||||
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
|
||||
# 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
|
||||
|
||||
@@ -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")
|
||||
@@ -1,133 +0,0 @@
|
||||
"""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
|
||||
Reference in New Issue
Block a user