Commit Graph
4 Commits
Author SHA1 Message Date
jpmschweitzerandClaude 4e68767771 refactor(health-report): put summary where the other writer puts it
check_history has two producers. sysmon-go writes `summary` at the top level
beside `status`; this module wrote it under `metrics`. So a reader had to know
which producer wrote a row before it could find out what the row said, and a
query written the obvious way found one and silently missed the other.

That is the T-36 failure repeating. There, per-domain queries returned rows from
August and looked like a system that had stopped reporting, because the data was
nested under a composite row nobody had mentioned. Nothing was missing; the
query was asking the wrong shape. verify.sh had already grown a coalesce over
both spellings, which is the tell: a compatibility shim that hides a schema
disagreement rather than resolving it.

D-33 made this table a contract between producers, and a contract needs one
spelling.

Summary is now a required parameter with no default. sysmon-go enforces the same
thing through Domain.Run's signature, and the reason is identical: a row whose
substance is missing looks exactly like a row whose check found nothing to say.
Both call sites pass it; the failure path passes the exception rather than
leaving the field to the metrics blob.

Old rows keep the nested spelling and verify.sh keeps reading both, because
rewriting history to match a new convention is a worse trade than a fallback
with a reason attached.

Also drops "Three consequences" from the module docstring, which by then listed
five. A hardcoded count beside the thing it counts is the same defect as
install.sh printing "wrote 8 keys" while writing ten — this morning's bug, in
prose instead of code.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-11 14:51:32 +02:00
jpmschweitzerandClaude b62024b21d fix(executors): run backups off the event loop
The scheduler serves its own REST API from the same loop that runs executors,
and the config backup spends ~21 minutes inside tarfile and zlib. Called inline
that starves the loop for the whole window: the service was unreachable
03:05-03:25 every night, and again at 07:39 today when the job was triggered by
hand to prove the T-69 report path. asyncio.to_thread is the fix -- zlib
releases the GIL while compressing, so the loop is scheduled normally.

The outage was invisible for as long as it existed. The hourly health check
fires at :35 and the outage runs 03:05-03:25, so no sample ever landed inside
it. A fixed-phase hourly probe cannot see a 20-minute event; that is aliasing,
not bad luck, and it would have stayed hidden indefinitely.

health_report.report_async joins it: psycopg2 is a blocking driver, so
reporting from the loop held it for the connect and insert -- up to
connect_timeout seconds precisely when the database is unreachable, which is
when a report matters most. Both backup executors use it now.

Caveat worth knowing: the engine wraps executors in asyncio.wait_for and a
thread cannot be cancelled, so on timeout the task is recorded failed while the
tar runs to completion. Still strictly better than blocking everything, and the
configured 3600s is well clear of the observed 1263s.

Seven tests in this file had been red since the initial commit -- they came
over with the portainer-core extraction, patched the Path class wholesale,
asserted "backed up" against a function returning "Backup completed: ...", and
one wrapped its call in except Exception: pass with its only assertion
commented out. There is no CI test gate here, so nothing reported it. Replaced
with tests that build real archives in tmp_path and assert on their contents.

The new loop test is the one that matters and it is mutation-checked: with
to_thread reverted it counts 0 heartbeat ticks, with it ~40.

Their structural demands (_create_tar_filter, a sync _cleanup_old_backups) were
adopted because threading wanted that shape anyway. Their exclude semantics
were not. The tests assert fnmatch behaviour and the deployed config is written
against substring matching -- it excludes logs as ".log", and paths as
unanchored fragments like "ollama/models/*" against members named
"docker-data/ollama/...". Under fnmatch neither matches, and the nightly
archive would silently gain many GB of model blobs instead of shrinking. That
is now pinned by tests naming the consequence, so the "improvement" fails loudly.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-11 11:35:29 +02:00
jpmschweitzer d9cbaee1fc feat: backup executors report their own outcome to check_history
The health record used to learn about backups by polling the mtime of the
newest archive, hourly, against a 48-hour threshold — for a job that runs once
a day. Forty-seven of every forty-eight runs could not produce a new answer.

Worse, file age cannot distinguish a failed backup from one that has not run
yet: when tonight's job dies, yesterday's archive is 24 hours old and still
reads healthy, and keeps reading healthy until hour 48. A failure stayed
invisible for two days to the check whose only job was noticing it. These
executors know at 03:05.

Reporting wraps the work rather than living inside it, so the failure path
cannot be forgotten — an exception is reported as critical and re-raised,
leaving the task's own status untouched. Reporting only success would reproduce
exactly the blind spot this replaces.

A reporting failure never fails the backup. Everything in health_report is
caught and logged, which means the absence of rows is the only symptom a broken
reporter produces — so monitor for rows, not for errors.

Not yet functional in production: scheduler_user holds DELETE and SELECT on
check_history (it prunes the table nightly) but not INSERT. Until that grant is
made the report logs a refusal and skips. The ticket predicted this as the step
that would fail silently, which is why it is named in the changelog and handled
as its own exception rather than folded into a generic catch.

Verified against baseline: tests/test_config_backup_executor.py and
tests/test_portainer_backup_executor.py report 7 failed / 17 passed both with
and without this change, so the pre-existing failures are untouched.

Workspace D-33, T-69.
2026-08-11 00:11:52 +02:00
jpmschweitzerandClaude 933196c2a2 feat(executors): back up Portainer's own state
Portainer keeps every stack definition, endpoint, user and access-control
rule in a BoltDB inside the portainer_data Docker volume. That volume
sits under /var/lib/docker/volumes/, and the daily config backup covers
~/docker-data and code-server-config only — so the thing that defines all
24 stacks was the one thing not backed up.

Calls Portainer's /api/backup rather than tarring the volume. BoltDB is a
single memory-mapped file, so copying it while Portainer writes can
capture a torn page; the API serialises a consistent snapshot.

A 200 whose body is not a readable archive is treated as failure. An
archive that will not open is worse than a missing one, because it looks
like a backup until the day it is needed. Writing that check found a real
gap in it: a truncated tar.gz raises EOFError, which is neither TarError
nor OSError, so the first version of the guard let it through.

Archives contain TLS certificates and private keys and are written 0600.
Retention only ever deletes files matching the exact name this executor
writes, so an unrelated archive left in the same directory survives.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-08 19:56:44 +02:00