mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-20 18:09:05 +00:00
Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
cf521bdfd8 |
@@ -49,20 +49,6 @@ class ResourceMetrics:
|
||||
self.total_execution_time = self.total_execution_time / self.call_count
|
||||
|
||||
|
||||
#: How often a plugin's metrics are written to the cache, in seconds.
|
||||
#:
|
||||
#: Persisting on every call meant a small file rewritten roughly nine times a
|
||||
#: minute per plugin. On a rig with fourteen active plugins that was ~126
|
||||
#: writes a minute for metrics alone, and since each ~350-byte file costs a
|
||||
#: 4KB block plus an ext4 journal entry, it dominated the device's write
|
||||
#: volume -- on an SD card, which wears out.
|
||||
#:
|
||||
#: The in-memory copy stays authoritative and exact; only the cross-process
|
||||
#: snapshot the web UI reads is delayed, and telemetry up to half a minute old
|
||||
#: is still a fair description of a long-running plugin.
|
||||
_METRICS_PERSIST_INTERVAL = 30.0
|
||||
|
||||
|
||||
class PluginResourceMonitor:
|
||||
"""
|
||||
Monitors resource usage for plugins.
|
||||
@@ -89,10 +75,6 @@ class PluginResourceMonitor:
|
||||
# Resource metrics per plugin
|
||||
self._metrics: Dict[str, ResourceMetrics] = {}
|
||||
self._limits: Dict[str, ResourceLimits] = {}
|
||||
# When each plugin's metrics last reached the cache. Metrics change on
|
||||
# every call, so they cannot be de-duplicated the way health state can;
|
||||
# they are rate-limited instead. See _METRICS_PERSIST_INTERVAL.
|
||||
self._metrics_persisted_at: Dict[str, float] = {}
|
||||
|
||||
# Thread-local storage for execution tracking
|
||||
self._local = threading.local()
|
||||
@@ -250,8 +232,18 @@ class PluginResourceMonitor:
|
||||
# CPU is harder to measure per-call, so we track it separately
|
||||
metrics.cpu_percent = self._get_process_cpu_percent()
|
||||
|
||||
# Persist metrics, at most once per interval per plugin.
|
||||
self._persist_metrics(plugin_id, metrics)
|
||||
# Persist metrics
|
||||
cache_key = self._get_metrics_key(plugin_id)
|
||||
self.cache_manager.set(cache_key, {
|
||||
'memory_mb': metrics.memory_mb,
|
||||
'cpu_percent': metrics.cpu_percent,
|
||||
'execution_time': metrics.execution_time,
|
||||
'call_count': metrics.call_count,
|
||||
'total_execution_time': metrics.total_execution_time,
|
||||
'max_execution_time': metrics.max_execution_time,
|
||||
'min_execution_time': metrics.min_execution_time if metrics.min_execution_time != float('inf') else 0.0,
|
||||
'last_update_time': metrics.last_update_time
|
||||
})
|
||||
|
||||
# Check limits
|
||||
if limits:
|
||||
@@ -371,37 +363,6 @@ class PluginResourceMonitor:
|
||||
summaries[plugin_id] = self.get_metrics_summary(plugin_id)
|
||||
return summaries
|
||||
|
||||
def _persist_metrics(self, plugin_id: str, metrics: ResourceMetrics,
|
||||
force: bool = False) -> None:
|
||||
"""Write a plugin's metrics to the cache, at most once per interval.
|
||||
|
||||
Caller must hold ``self._lock``.
|
||||
"""
|
||||
# Monotonic, not wall clock: these devices have no RTC, so the clock
|
||||
# jumps by however far off boot-time was the moment NTP first syncs.
|
||||
# A forward jump would allow an early write, a backward one would
|
||||
# stall the snapshot well past the interval.
|
||||
now = time.monotonic()
|
||||
if not force and now - self._metrics_persisted_at.get(plugin_id, 0.0) \
|
||||
< _METRICS_PERSIST_INTERVAL:
|
||||
return
|
||||
cache_key = self._get_metrics_key(plugin_id)
|
||||
self.cache_manager.set(cache_key, {
|
||||
'memory_mb': metrics.memory_mb,
|
||||
'cpu_percent': metrics.cpu_percent,
|
||||
'execution_time': metrics.execution_time,
|
||||
'call_count': metrics.call_count,
|
||||
'total_execution_time': metrics.total_execution_time,
|
||||
'max_execution_time': metrics.max_execution_time,
|
||||
'min_execution_time': (metrics.min_execution_time
|
||||
if metrics.min_execution_time != float('inf')
|
||||
else 0.0),
|
||||
'last_update_time': metrics.last_update_time,
|
||||
})
|
||||
# Only after the write lands. Marking it first would mean a failed
|
||||
# set() bought the next interval's silence without leaving a snapshot.
|
||||
self._metrics_persisted_at[plugin_id] = now
|
||||
|
||||
def reset_metrics(self, plugin_id: str) -> None:
|
||||
"""Reset metrics for a plugin."""
|
||||
with self._lock:
|
||||
@@ -409,7 +370,4 @@ class PluginResourceMonitor:
|
||||
self._metrics[plugin_id] = ResourceMetrics()
|
||||
cache_key = self._get_metrics_key(plugin_id)
|
||||
self.cache_manager.delete(cache_key)
|
||||
# Let the next call persist immediately rather than leaving the
|
||||
# deleted key absent for the rest of the interval.
|
||||
self._metrics_persisted_at.pop(plugin_id, None)
|
||||
|
||||
|
||||
@@ -62,6 +62,9 @@ class StartupValidator:
|
||||
# Validate plugins if plugin manager is available
|
||||
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
|
||||
|
||||
@@ -74,6 +77,71 @@ 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:
|
||||
|
||||
@@ -127,67 +127,3 @@ class TestForceReload:
|
||||
fresh = mon.get_metrics_summary("p", force_reload=True)
|
||||
assert fresh["call_count"] == 7
|
||||
assert any(c.kwargs.get("memory_ttl") == 0 for c in cache.get.call_args_list)
|
||||
|
||||
|
||||
class TestMetricsPersistenceChurn:
|
||||
"""Metrics are telemetry; writing them on every call wore the SD card.
|
||||
|
||||
Each write is a ~350-byte file, which on ext4 costs a 4KB block plus a
|
||||
journal entry. At roughly nine calls a minute per plugin across fourteen
|
||||
plugins it dominated the device's write volume.
|
||||
"""
|
||||
|
||||
def test_repeated_calls_persist_once_per_interval(self):
|
||||
cache = _cache()
|
||||
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
||||
for _ in range(50):
|
||||
mon.monitor_call("p", lambda: None)
|
||||
writes = [c for c in cache.set.call_args_list
|
||||
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
||||
assert len(writes) == 1, (
|
||||
f"50 calls produced {len(writes)} metric writes; expected 1")
|
||||
|
||||
def test_the_interval_elapsing_allows_the_next_write(self, monkeypatch):
|
||||
import src.plugin_system.resource_monitor as rm
|
||||
cache = _cache()
|
||||
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
||||
mon.monitor_call("p", lambda: None)
|
||||
# pretend the interval has passed
|
||||
mon._metrics_persisted_at["p"] -= rm._METRICS_PERSIST_INTERVAL + 1
|
||||
mon.monitor_call("p", lambda: None)
|
||||
writes = [c for c in cache.set.call_args_list
|
||||
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
||||
assert len(writes) == 2
|
||||
|
||||
def test_in_memory_metrics_stay_exact_while_writes_are_skipped(self):
|
||||
mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
|
||||
for _ in range(20):
|
||||
mon.monitor_call("p", lambda: None)
|
||||
assert mon.get_metrics("p").call_count == 20
|
||||
|
||||
def test_reset_lets_the_next_call_persist_immediately(self):
|
||||
cache = _cache()
|
||||
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
||||
mon.monitor_call("p", lambda: None)
|
||||
mon.reset_metrics("p")
|
||||
mon.monitor_call("p", lambda: None)
|
||||
writes = [c for c in cache.set.call_args_list
|
||||
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
||||
assert len(writes) == 2, "reset should clear the throttle timestamp"
|
||||
|
||||
def test_a_failed_write_does_not_buy_the_next_interval_of_silence(self):
|
||||
"""A set() that raises must not count as having persisted.
|
||||
|
||||
Marking the timestamp before the write would leave no snapshot in the
|
||||
cache and still suppress the next 30 seconds of attempts.
|
||||
"""
|
||||
cache = _cache()
|
||||
cache.set.side_effect = [OSError("disk full"), None]
|
||||
mon = PluginResourceMonitor(cache, enable_monitoring=False)
|
||||
with pytest.raises(OSError):
|
||||
mon.monitor_call("p", lambda: None)
|
||||
# the very next call must try again rather than skip the interval
|
||||
mon.monitor_call("p", lambda: None)
|
||||
writes = [c for c in cache.set.call_args_list
|
||||
if c.args and str(c.args[0]).startswith("plugin_metrics:")]
|
||||
assert len(writes) == 2, "a failed write should be retried, not skipped"
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user