1bfaa32601ceac50fd3b31055455ddf51a705fc7
10
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 flan
|
e426045255 |
An unchanged fingerprint froze the report's body, not just its comments
Two questions were sharing one answer. "Have the findings changed?" gates COMMENTS. They notify, and a daily "still broken, same as yesterday" is what teaches everyone to ignore the one that finally matters. "Is the body still true?" gates the BODY. Editing an issue body notifies nobody on either forge, so keeping it honest is free. Conflated, an unchanged fingerprint froze the body -- and the fingerprint ignores, by design, everything that moves on its own: healthy rows, the hardware-verified column, point releases, and how a row is LABELLED. So the master -> 27-dev relabel would have shipped to the README and never to the issue anybody actually opens. The report would have gone on saying "master (unreleased) BROKEN" -- the precise false alarm the relabel exists to kill -- until iX happened to break something else. The body is now rewritten whenever it is stale; comments stay strictly a changelog of real changes. Bodies are compared after normalising line endings, because a forge that round-trips \r\n would otherwise trigger a silent rewrite every run and leave the issue looking freshly touched every morning. |
||
|
flan
|
862bdd3399 |
master is 27-dev, not the next release; and check the next maintenance release
Two ways the matrix misled the person it exists for -- somebody deciding whether to trust this with their backups. master is not the next release. iX branches each major onto its own release/ line and master rolls straight on to the one after: every recent commit on master targets 27.0.0-BETA.1 while 26 is still in beta. So a BROKEN master row, rendered "master (unreleased)", read as "the version you are about to install is broken" when the breakage is a major release away on a line nobody can download. It is now labelled from the newest major in the matrix plus one, so it becomes 28-dev by itself once 27 branches. The break, for the record, is NAS-141498 (2026-06-24), "Convert cloud_backup plugin to the typesafe pattern": it re-signatures restic_backup and get_restic_config, splitting entry/credentials out of the cloud_backup dict. Not being chased while the 27 line churns. The next maintenance release was never checked -- and it is the one that reaches users. Shipped came from TS-* tags, unreleased from release/* branches carrying -BETA/-RC. A branched-but-untagged MAINTENANCE release is neither: release/25.10.5 has no tag, and its line has already shipped, so the "a prerelease of a shipped line is history" filter threw it out. It was invisible, and it is exactly what a 25.10.4 box gets on its next update; a break in it reaches real users before the daily check ever looks, on the only line anybody runs. A plain release/X.Y.Z branch now counts when its line HAS shipped and it sorts NEWER than that line's newest tag. Both exclusions fall out of the same rule: release/24.10-RC.2 sorts older than TS-24.10.2.4 (history), and the typo branch release/25.20.2.2 is on a line with no tag at all (not a release line). This surfaced two refs never checked before -- release/25.10.5 and release/24.10.2.5 -- both of which pass. is_unreleased() keys off where a ref came from (a branch is by construction not shipped) rather than hunting -BETA/-RC in the name. Otherwise release/25.10.5 counts as shipped and a break in it fails the build as a live outage, on a version nobody is running yet. |
||
|
flan
|
52b11eada2 |
The bug-report bot re-filed itself every run on Gitea; nine copies
`find_issue()` skipped pull requests by testing for the PRESENCE of the `pull_request` key. GitHub omits that key on a plain issue. Gitea sends it as `null`. So on Gitea every issue was thrown away as if it were a PR, the lookup came back empty on every run, and the bot took the "nothing filed yet" branch and opened a brand-new report instead of editing the one already open. Nine of them piled up on the canonical forge. Four were filed AFTER the commit that was supposed to stop exactly this -- the same failure it was written to prevent, moved from comments to issues. It survived because `find_issue` was the one function here with no test; the mirror deduped correctly, and the mirror is what anybody would have looked at. Test the value, not the key: it is the only form that is true on both forges. Pinned by tests that carry each forge's payload shape by hand. Also send both paging parameters. GitHub reads `per_page`, Gitea reads `limit`, and each ignores the other's; Gitea's default page is 30, so the lookup would have begun missing the report once the pile it was creating outgrew one page. |
||
|
flan
|
df412eeff7 |
test: make three vacuous tests actually test something; drop one duplicate
The test suite is not bloated -- 3,701 lines of test code against 3,760 lines of product code, one duplicate pair in 356 tests, 8% single-assertion tests. The waste was not volume, it was four tests that looked like coverage and provided none: - test_parents_are_mounted_before_children: the fixture was already in depth order, so the sort it exists for was never exercised. Deleting `mounts.sort(key=_depth)` passed the whole suite. It now uses datasets whose NAME order differs from their MOUNTPOINT depth, which is the only case the sort is for. - test_it_NEVER_touches_the_current_run: the "current" snapshot was a minute old, so the age floor excluded it regardless and the same-snapname guard never ran. That guard only matters for a run that OUTLIVES the floor -- which a first full upload easily does, and where collecting it would yank the snapshot out from under a backup that is still reading from it. Now tested with a 12-hour-old current run. - the fingerprint/unknown test put the unreadable file in `providers`, which is not broken -- so fingerprint() skipped the whole module via is_broken() and the filter under test never executed. The blip has to land in the module that IS broken. - test_the_real_truenas_versions was a byte-identical copy of test_async_middleware_is_detected under a name promising more. Replaced with the fact actually worth pinning: the flavour probe must read STOCK source, because apply.sh re-runs on an already-patched overlay and our own SNAPSHOT_SYNC block is a plain `def create_snapshot` -- reading it would report a 25.10 box as synchronous and inject the wrong wrapper. All four now fail when the code they name is broken. |
||
|
flan
|
ce6998a935 |
fix: second audit — the delete check did nothing on a real box, and five guards were untestable
The most important finding is that the FIRST audit's fix was wrong. _can_delete() asked `callable(getattr(service, "delete"))`. But CRUDService defines `delete` on the BASE class and dispatches to self.do_delete at call time, so a bound `delete` exists on every CRUDService subclass whether or not it still implements one. The check was therefore answering "is this a CRUDService?" — precisely the weaker "is the namespace registered?" question its own docstring said must never be asked. It would still have picked a gutted pool.snapshot and failed every delete. It now walks the MRO and ignores middlewared.service.* plumbing, so only a PLUGIN class defining delete/do_delete counts. The test double was equally wrong: it modelled a gutted service as object(), a shape middlewared cannot produce, so the test passed against a fake it could never have caught in the field. It is now CRUDService-shaped. Also: - The recursive delete's fast path returned [] without confirming anything was destroyed. A delete that returns cleanly is not proof — iX has already gutted pool.snapshot.do_update on master into a no-op that returns None. cleanup_task read "no survivors" as a clean sweep, dropped the sidecar (the only record), and would have orphaned ~250 snapshots per run, silently. It confirms against ZFS now, and the by-name sweep trusts ZFS rather than the API's return value. - When ZFS cannot be read, the sweep no longer claims success. The two mistakes are not symmetric: a false survivor self-heals (sidecar kept, next run reclaims, record clears), a lost record does not. - _write_sidecar swallowed OSError. The sidecar is the only record the snapshots exist; failing to write it must never be invisible. - stage_nested now refuses UP FRONT when middleware has no usable snapshot delete, rather than discovering it after restic has already run. Tests. The autouse fixture added in the last commit did not work: `runner=_run`, `mounts_file="/proc/self/mounts"` and `sleep=time.sleep` are frozen into __defaults__ at def time, so monkeypatching the module attribute never reached them. 19 tests were still reading the real mount table — one matching name from running a real `umount` on the NAS — and the retry loop really slept. All three are late-bound now; the suite reads nothing outside tmp_path and runs in 1.1s. Every mutation the audit reported as SURVIVING now fails the suite: the naive delete check, the unconfirmed fast path, the malformed-row guard, a disconnected GC, eager service resolution, compat's method check, compat's unknown handling, a single-quoted filtered query in apply.sh, and the get-service assumption. Also: fingerprint() folded `unknown` problems into a broken module, so one transient 429 rewrote the bug report and the next clean run rewrote it back. Problems are state-tagged; only definite breakage is digested. Verified on TrueNAS 26.0.0-BETA.1: zvol-orphan case 0 orphans, 292-dataset backup 0 orphans / 0 leaked mounts / 0 stale sidecars, byte-identical restore of a 4-deep child dataset. |
||
|
flan
|
413cd60ed4 |
fix: own the snapshot sweep unconditionally; align the runtime and the manifest
Four audits of the TrueNAS 26 branch. The findings, in severity order. 1. TrueNAS 26 orphaned a snapshot on every run, with no backstop. Stock decides `recursive` by its own rule, and on 26 that rule is no longer ours. <= 25.10 its create_snapshot called get_dataset_recursive() — the same function this module vendors — so "stock went recursive" and "we have something to stage" were the same question. 26 uses filesystem.statfs: recursive = (path == the dataset's mountpoint). A dataset whose only descendants are ZVOLs or legacy/none-mountpoint datasets now gets a RECURSIVE snapshot while the patch sees nothing to stage. The patch then handed the snapshot back to stock, which destroys the parent only. No staging tree meant no sidecar, and the GC only ever ran from stage_nested — so nothing on the box would ever have found the children. Reproduced on the VM: one orphan per zvol, every run, forever, backup green. Ownership of the sweep is no longer conditional on staging (own_snapshot()). 2. The runtime resolved a NAMESPACE; compat.py verified a METHOD. get_service() only proves a namespace is registered. compat checks the namespace AND that it defines delete/do_delete. So if iX guts the method but keeps the service — which they have already done to pool.snapshot.do_update on master — compat falls through to zfs.snapshot and reports the box healthy, while the runtime picks pool.snapshot and fails every delete. Both sides now ask "can this namespace delete?", and a test binds the two lists together. 3. query_filesystems() silently dropped malformed rows — the one remaining silent-omission path, and a direct contradiction of the cardinal rule. It raises now. A missing `zfs` binary raised FileNotFoundError rather than ZfsError; also fixed. 4. The retry loop discarded the delete error and reported every survivor as "(still busy?)" — naming the one cause that is benign and hiding the ones that are permanent. It keeps and reports the real error. Also: the staging-failure handler could lose the original exception if its own sweep raised; get_service is now a checked assumption; normalise_dataset and two dead MiddlewareCall properties removed; stale comments corrected. Tests: five of them were shelling out to the REAL pool (`zfs list -r Tap`, 2148 snapshots) and passed here only because this box has no zfs binary — they would have gone red on the NAS, which is the one machine the release process requires them green on. An autouse fixture now makes that impossible. Mutation-tested: reverting any of the five fixes above now fails the suite; before, all 293 passed. Verified on TrueNAS 26.0.0-BETA.1 (zvol leak reproduced, then closed; 292-dataset backup, 0 orphans, byte-identical restore of a 4-deep hidden dataset) and on 25.10.4 (pool.snapshot.delete honours recursive=True). |
||
|
flan
|
605231b39f |
feat: TrueNAS 26 support; enumerate datasets and snapshots from ZFS, not middleware
TrueNAS 26 deletes plugins/zfs_/ outright, taking the private zfs.dataset.query, zfs.snapshot.query and zfs.snapshot.delete with it. All three were on the nested module's critical path, so nested snapshots were BROKEN on 26. Snapshot deletion now resolves its namespace at runtime: pool.snapshot on 25.10 and 26, zfs.snapshot on 24.10 and 25.04. No single namespace spans every supported release. tools/compat.py checks the same list the runtime uses, so what CI verifies and what runs cannot drift apart. Enumeration does NOT move to pool.dataset.query / pool.snapshot.query, and that is the point of this commit. Those methods exist, are documented, and are covered by iX's deprecation policy — and they are not like-for-like replacements. They apply a visibility policy that hides ix-apps/*, .system/* and .ix-virt/*: 84 of 270 datasets on a real pool, including live application data. Staging from that view omits them silently, and plan_staging never sees them, so they do not even reach the skipped list. The snapshot query hides the same datasets' snapshots, so the sweep orphans one per hidden dataset on every run. So: read the truth from ZFS, make changes through middleware. zfs list cannot be filtered by policy and behaves identically on every release. A failing zfs list raises rather than returning an empty list — "no datasets" and "the command broke" must never look the same. No shipped release is affected: v0.6.1 and earlier use the private zfs.dataset.query, which returns all 270 datasets. The bug existed only in this port. Verified on a real TrueNAS 26.0.0-BETA.1 install: 274-snapshot recursive backup of a 292-dataset pool, zero orphaned snapshots, zero leaked mounts, and a byte-identical restore of a four-level-deep child dataset that pool.dataset.query hides. |
||
|
flan
|
ecb64878ff |
compat: check the middlewared METHODS we call, not just the symbols we wrap
This gap was hiding a catastrophe. TrueNAS 26 deletes plugins/zfs_/dataset.py and plugins/zfs_/snapshot.py outright, taking zfs.dataset.query, zfs.snapshot.query and zfs.snapshot.delete with them (26 uses filesystem.statfs and zfs.resource.*). Nothing about the five cloud_backup files reveals that, so every other check went green -- including the one I had just added. The patch would have applied cleanly and then failed on the first backup, or worse: snapshotted fine and failed to DELETE, orphaning one snapshot per descendant dataset (250 on a real pool) on every run, forever. So 26 is BROKEN and the nested module will not apply there. The async/sync wrapper work and the vendored get_dataset_recursive stay -- they are correct and necessary -- but 26 is not supported until the ZFS calls are ported, and that needs a real 26 box to verify. Shipping a port nobody has run is the failure this project exists to avoid. Also: do_delete is recognised as delete (24.10/25.04 use the CRUDService convention), which was reporting both as BROKEN -- a false verdict that would have disabled nested snapshots on boxes where they work. |
||
|
flan
|
498b2690e1 |
TrueNAS 26 support: one sync implementation, two wrappers
26 rewrites cloud_backup from async to synchronous AND deletes get_dataset_recursive(), which SNAPSHOT_BLOCK called out of the host module's namespace. Either is a broken backup found at restore time. The nested module is now one synchronous implementation talking to middlewared via call_sync, behind two thin wrappers. apply.sh reads which flavour the installed middleware declares and injects the matching one: <= 25.10 reaches it through 'await middleware.run_in_thread(...)', 26 is already in a worker thread and calls it directly. The snapshot/bind-mount/failure logic exists once -- an async twin would mean every future fix had to land twice. A middleware whose three wrapped functions disagree about asyncness is refused, not guessed at. get_dataset_recursive is vendored, removing the dependency on both versions rather than asserting it. master stays BROKEN on purpose: iX are still renaming middleware->context, cloud_backup->entry and adding a required credentials param there. Chasing a branch that moves daily is how you ship a patch nobody tested. |
||
|
flan
|
ea090c7f72 |
Audit fixes: a compat verdict must never be able to brick a working box
The audit found the new machinery could do more harm than the bugs it prevents. - apply.sh reused the 'nothing left to do' exit -- which touches the PERMANENT kill switch, cleared only by install.sh, never by update.sh -- for the incompatible case. On TrueNAS 26 (providers ok, nested opt-out) both modules go quiet, so the switch would fire and the release that fixed 26 could never re-enable itself. Retirement and incompatibility now take different exits. - A network blip, a re-export, or a conditional def all read as BROKEN. Each is now 'unknown', which changes nothing, rather than evidence strong enough to disable a module. - 'native' outranked BROKEN everywhere but apply.sh, so a TrueNAS that reworded the guard AND reshaped the functions rendered as good news. - compat.py --tree read B2_BLOCK's own 'restic = True' as native support, so the documented way to check a live box lied on every patched machine. - The signature check was a name-subset test. It passed reorders, kw-only conversions, and added required params -- and it had already passed a real bug: restic_backup takes 4 args on 24.10/25.04, and the wrapper forwarded 5. Nested backups have been raising TypeError on those releases the whole time. The wrapper now forwards *args/**kwargs. - release.sh --promote was unreachable: it died if the tag existed, the gate died if it did not. The tests hid it by always tagging first. |