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.
This commit is contained in:
@@ -6,6 +6,54 @@ is deliberate: see [Releasing](docs/releasing.md). Twelve releases were cut on
|
||||
live, every one of those interrupts every user. An alert people learn to ignore is
|
||||
worse than no alert, because one day it carries a security fix.
|
||||
|
||||
## Unreleased
|
||||
### Added
|
||||
|
||||
- **TrueNAS 26 support, verified on a real TrueNAS 26 install.** 26 deletes
|
||||
`plugins/zfs_/` outright, taking the private `zfs.dataset.query`,
|
||||
`zfs.snapshot.query` and `zfs.snapshot.delete` with it. Every one of those was on
|
||||
the nested module's critical path, so nested snapshots were **BROKEN** on 26 and
|
||||
`apply.sh` correctly refused to apply the module there.
|
||||
|
||||
Snapshot **deletion** now resolves its namespace at runtime — `pool.snapshot` on
|
||||
25.10 and 26, `zfs.snapshot` on 24.10 and 25.04, because 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.
|
||||
|
||||
Hardware-verified on TrueNAS 26.0.0-BETA.1: a 274-snapshot recursive backup of a
|
||||
292-dataset pool, then a **byte-identical restore of a four-level-deep child
|
||||
dataset**.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Enumeration no longer trusts middleware's dataset and snapshot queries — they
|
||||
are filtered.** This is the important one, and it is the bug that a test VM caught
|
||||
and no amount of source analysis ever could have.
|
||||
|
||||
The obvious port of the deleted private `zfs.dataset.query` was the public
|
||||
`pool.dataset.query`. It exists, it is documented, it is covered by iX's
|
||||
deprecation policy — and it is **not a like-for-like replacement**. It applies a
|
||||
*visibility policy*: it hides the datasets TrueNAS considers its own — `ix-apps/*`,
|
||||
`.system/*`, `.ix-virt/*`. On a real pool that is **84 of 270 datasets**, and
|
||||
`ix-apps` holds **live application data**.
|
||||
|
||||
Staging from that view would have silently omitted every one of them. Worse,
|
||||
`plan_staging()` would never have seen them, so they would not have appeared in its
|
||||
`skipped` list either — no warning, no failure, just a green backup quietly missing
|
||||
data. That is precisely the failure this module exists to prevent. The snapshot
|
||||
query lies the same way (205 of 274), so the sweep would have orphaned one snapshot
|
||||
per hidden dataset, on every run, forever.
|
||||
|
||||
The module now **reads the truth from ZFS and makes changes through middleware**:
|
||||
enumeration is `zfs list`, which no policy can filter and which behaves identically
|
||||
on every release; mutation stays a middleware call, so TrueNAS's own bookkeeping
|
||||
stays consistent. 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 call the *private*
|
||||
`zfs.dataset.query`, which returns all 270 datasets. The bug existed only in the
|
||||
unreleased TrueNAS 26 port.
|
||||
|
||||
## v0.6.1 — 2026-07-13
|
||||
### Fixed
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ If something is wrong, the reason is in `apply.log` — start at
|
||||
| 24.10.2.4 | ok | ok | — |
|
||||
| 25.04.2.6 | ok | ok | — |
|
||||
| 25.10.4 | ok | ok | nested + providers; 252-snapshot recursive backup of /mnt/Tap, 18m |
|
||||
| 26.0.0-BETA.3 _(unreleased)_ | ok | **BROKEN** | — |
|
||||
| 26.0.0-BETA.3 _(unreleased)_ | ok | ok | — |
|
||||
| master _(unreleased)_ | **BROKEN** | **BROKEN** | — |
|
||||
|
||||
| verdict | meaning |
|
||||
@@ -77,6 +77,12 @@ source. It does not mean a human ran a backup on it — that is the
|
||||
The table is **regenerated daily by CI** against iXsystems' actual middleware source
|
||||
— it is not a claim somebody typed once and forgot.
|
||||
|
||||
**On TrueNAS 26:** the patch was run on a real TrueNAS **26.0.0-BETA.1** install — a
|
||||
274-snapshot recursive backup of a 292-dataset pool, followed by a byte-identical
|
||||
restore of a four-level-deep child dataset. The *Hardware-verified* column tracks the
|
||||
newest beta iX has tagged (currently BETA.3), so it does not carry that mark: a build
|
||||
nobody has actually run a backup on does not get credit for one.
|
||||
|
||||
**TrueNAS 26: nested snapshots are not supported yet, and upgrading will not break
|
||||
you.** 26 rewrites `cloud_backup` and deletes the ZFS methods this module calls. On
|
||||
26 `apply.sh` finds that the assumptions no longer hold and **does not apply the
|
||||
|
||||
+7
-4
@@ -535,7 +535,7 @@ if _tc_nested is not None:
|
||||
# snapshot=true, not just ours. Two consequences, and the second is worse:
|
||||
#
|
||||
# * everything below is a NEW failure mode for tasks that worked before we
|
||||
# were installed. A `zfs.dataset.query` that errors would break a
|
||||
# were installed. A `pool.dataset.query` that errors would break a
|
||||
# CloudSync job we have no business touching.
|
||||
# * if a CloudSync task ever were staged, nothing would ever tear it down:
|
||||
# the teardown is wired into cloud_backup's restic_backup finally, and
|
||||
@@ -556,9 +556,12 @@ if _tc_nested is not None:
|
||||
# our staging plan would not -- silently omitting it from the backup.
|
||||
# Read afterwards, an unsnapshotted dataset instead trips the isdir()
|
||||
# check in plan_staging and fails the run loudly. Loud beats silent.
|
||||
datasets = middleware.call_sync(
|
||||
"zfs.dataset.query", [["type", "=", "FILESYSTEM"]]
|
||||
)
|
||||
# query_filesystems() reads ZFS directly. It deliberately does NOT use
|
||||
# pool.dataset.query: that applies a visibility policy and hides
|
||||
# TrueNAS-internal datasets (ix-apps/*, .system/*, .ix-virt/*) -- 84 of
|
||||
# 270 on a real pool, including live app data. Staging from the filtered
|
||||
# view omits them silently, which is the one thing this must never do.
|
||||
datasets = _tc_nested.query_filesystems(middleware)
|
||||
# OUR copy of get_dataset_recursive, not the host module's: TrueNAS 26
|
||||
# deleted that helper (create_snapshot uses filesystem.statfs now), so
|
||||
# calling it out of the module namespace is a NameError there.
|
||||
|
||||
+225
-18
@@ -34,7 +34,7 @@ feature exists to prevent, and it would be worse than not having the feature.
|
||||
|
||||
Snapshot lifecycle -- read this before changing anything
|
||||
--------------------------------------------------------
|
||||
``zfs.snapshot.delete`` defaults to ``recursive=False``, and stock
|
||||
The snapshot delete call defaults to ``recursive=False``, and stock
|
||||
``restic_backup()`` calls it with no options. Stock gets away with that because
|
||||
its validation means ``recursive`` is never actually True in the field. Enabling
|
||||
nested datasets makes recursive snapshots real, so the parent
|
||||
@@ -62,17 +62,24 @@ import subprocess
|
||||
import time
|
||||
|
||||
__all__ = [
|
||||
"SNAPSHOT_SERVICES",
|
||||
"STAGING_BASE",
|
||||
"StagingError",
|
||||
"ZfsError",
|
||||
"apply_plan",
|
||||
"cleanup_all",
|
||||
"cleanup_task",
|
||||
"current_mounts_under",
|
||||
"delete_snapshot_tree",
|
||||
"gc_stale_snapshots",
|
||||
"list_snapshot_names",
|
||||
"mounted_snapshots",
|
||||
"normalise_dataset",
|
||||
"pick_snapshot_service",
|
||||
"plan_staging",
|
||||
"query_filesystems",
|
||||
"sidecar_for",
|
||||
"snapshot_service",
|
||||
"snapshot_tree_names",
|
||||
"stage_nested",
|
||||
"stale_snapshot_names",
|
||||
@@ -81,6 +88,204 @@ __all__ = [
|
||||
"verify_staged",
|
||||
]
|
||||
|
||||
|
||||
# ── how this module talks to the system ──────────────────────────────────────
|
||||
#
|
||||
# READ the truth from ZFS. MAKE CHANGES through middleware.
|
||||
#
|
||||
# That split is not stylistic. It was forced by finding, on a real TrueNAS 26 box,
|
||||
# that middleware's query APIs apply a VISIBILITY POLICY:
|
||||
#
|
||||
# zfs list 274 datasets 205 from pool.dataset.query
|
||||
# zfs list -t snapshot 274 snapshots 205 from pool.snapshot.query
|
||||
#
|
||||
# The missing 69 are the datasets TrueNAS considers its own -- `ix-apps/*`,
|
||||
# `.system/*`, `.ix-virt/*` -- and on the real pool that is 84 of 270, including
|
||||
# `ix-apps`, which holds live application data. Enumerating from that view would
|
||||
# have silently omitted every one of them from the staging plan and from the
|
||||
# snapshot sweep: a green backup missing data, and one orphaned snapshot per
|
||||
# hidden dataset on every run. Both are exactly what this module exists to
|
||||
# prevent.
|
||||
#
|
||||
# This went unnoticed because the patch used to call the PRIVATE `zfs.dataset.query`
|
||||
# and `zfs.snapshot.query`, which return everything. TrueNAS 26 deleted them, and
|
||||
# the public replacements are NOT like-for-like -- they are filtered. So
|
||||
# enumeration now reads ZFS directly, which no policy can filter and which behaves
|
||||
# identically on every release.
|
||||
#
|
||||
# MUTATION still goes through middleware, so TrueNAS's own bookkeeping stays
|
||||
# consistent -- and an exact-name delete works fine even on a dataset the query
|
||||
# hides. The one wrinkle is that no single snapshot namespace spans every
|
||||
# supported release, so it is resolved at runtime rather than pinned:
|
||||
#
|
||||
# 24.10, 25.04 `zfs.snapshot` (public back then; `pool.snapshot` does not exist)
|
||||
# 25.10 both -- `pool.snapshot` public, `zfs.snapshot` demoted to private
|
||||
# 26 `pool.snapshot` only -- `plugins/zfs_/` is gone
|
||||
#
|
||||
#: Snapshot CRUD namespaces, best first. `tools/compat.py` checks this exact list
|
||||
#: (MiddlewareCall.also), so what CI verifies and what runs cannot drift apart.
|
||||
SNAPSHOT_SERVICES = ("pool.snapshot", "zfs.snapshot")
|
||||
|
||||
|
||||
def pick_snapshot_service(has_service):
|
||||
"""First namespace in SNAPSHOT_SERVICES that this middleware exposes.
|
||||
|
||||
Pure: `has_service(name) -> bool`. Returns None if middleware has none of
|
||||
them, which is a middleware we have never seen and must not guess about.
|
||||
"""
|
||||
for name in SNAPSHOT_SERVICES:
|
||||
if has_service(name):
|
||||
return name
|
||||
return None
|
||||
|
||||
|
||||
def _has_service(middleware, name):
|
||||
try:
|
||||
middleware.get_service(name)
|
||||
except Exception:
|
||||
# get_service raises KeyError for an unregistered namespace. Anything
|
||||
# else here is equally a "cannot use it", and guessing YES on a service
|
||||
# that is not really there would fail later, mid-backup, holding a
|
||||
# snapshot -- the worst possible moment.
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def snapshot_service(middleware):
|
||||
"""The snapshot CRUD namespace this middleware actually has."""
|
||||
name = pick_snapshot_service(lambda n: _has_service(middleware, n))
|
||||
if name is None:
|
||||
raise StagingError(
|
||||
"middleware exposes neither " + " nor ".join(SNAPSHOT_SERVICES)
|
||||
+ ". Refusing to stage a nested backup, because the snapshot it "
|
||||
"creates could not then be swept."
|
||||
)
|
||||
return name
|
||||
|
||||
|
||||
def normalise_dataset(row):
|
||||
"""A `pool.dataset.query` row -> the shape this module's planner speaks.
|
||||
|
||||
The planner does not consume this any more -- :func:`query_filesystems` reads
|
||||
ZFS directly, because the middleware query is filtered (see the note above).
|
||||
It is kept because it is the safe way to consume a middleware dataset row if
|
||||
anything ever needs to, and because the two traps below are not obvious and
|
||||
cost real debugging to find:
|
||||
|
||||
* `mountpoint` is a plain string there, not ``{"value": ...}``.
|
||||
* `mounted` is still a property dict, but its ``value`` is ``"YES"``/``"NO"``
|
||||
-- UPPERCASE, where the old API said ``"yes"``. The planner tests
|
||||
``== "no"``, so an unmounted dataset would read as mounted and the planner
|
||||
would try to stage a snapdir that is not there. Read ``parsed``, which is a
|
||||
real bool, and only fall back to the string.
|
||||
"""
|
||||
mounted = row.get("mounted")
|
||||
if isinstance(mounted, dict):
|
||||
parsed = mounted.get("parsed")
|
||||
if isinstance(parsed, bool):
|
||||
is_mounted = parsed
|
||||
else:
|
||||
is_mounted = str(mounted.get("value", "yes")).lower() != "no"
|
||||
elif isinstance(mounted, bool):
|
||||
is_mounted = mounted
|
||||
else:
|
||||
# Absent means the caller did not ask for the property. Assume mounted:
|
||||
# the planner's own snapdir probe is the real check, and assuming
|
||||
# UNmounted would silently drop datasets that hold data.
|
||||
is_mounted = True
|
||||
|
||||
mountpoint = row.get("mountpoint")
|
||||
if isinstance(mountpoint, dict): # tolerate the old shape too
|
||||
mountpoint = mountpoint.get("value", "")
|
||||
|
||||
return {
|
||||
"name": row["name"],
|
||||
"properties": {
|
||||
"mountpoint": {"value": mountpoint or ""},
|
||||
"mounted": {"value": "yes" if is_mounted else "no"},
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
class ZfsError(Exception):
|
||||
"""`zfs list` failed. Enumeration is unreliable, so the caller must not guess."""
|
||||
|
||||
|
||||
def _zfs_lines(args, runner=None):
|
||||
"""`zfs <args>` as a list of tab-split rows. Raises ZfsError if it fails.
|
||||
|
||||
Never returns a partial or empty list on failure: a caller that cannot tell
|
||||
"no datasets" from "the command broke" will happily stage nothing, or sweep
|
||||
nothing, and report success.
|
||||
"""
|
||||
runner = runner or _run
|
||||
r = runner(["zfs", *args])
|
||||
if r.returncode != 0:
|
||||
raise ZfsError((r.stderr or "").strip() or f"zfs {' '.join(args)} failed")
|
||||
return [ln.split("\t") for ln in r.stdout.splitlines() if ln.strip()]
|
||||
|
||||
|
||||
def query_filesystems(middleware=None, runner=None):
|
||||
"""Every FILESYSTEM dataset, in the shape the planner speaks -- read from ZFS.
|
||||
|
||||
NOT from `pool.dataset.query`, and this is the single most important decision
|
||||
in this file.
|
||||
|
||||
middleware's dataset query applies a VISIBILITY POLICY: it hides the datasets
|
||||
TrueNAS considers its own -- `ix-apps/*`, `.system/*`, `.ix-virt/*`. That is
|
||||
**84 of 270 datasets** on the real pool, and `ix-apps` holds live application
|
||||
data. Building the staging plan from that view would silently omit every one of
|
||||
them. Worse, `plan_staging()` would never even SEE them, so they would not turn
|
||||
up in its `skipped` list either -- no warning, no failure, just a green backup
|
||||
quietly missing data. That is exactly the failure this whole module exists to
|
||||
prevent, and it is the failure the cardinal rule at the top of this file is
|
||||
about.
|
||||
|
||||
It worked before only because the patch called the PRIVATE `zfs.dataset.query`,
|
||||
which returned everything. TrueNAS 26 deleted it. The public replacement is not
|
||||
a like-for-like: it is a filtered view.
|
||||
|
||||
So: **read the truth from ZFS, make changes through middleware.** ZFS cannot
|
||||
apply a policy to what it reports, and `zfs list` behaves identically on every
|
||||
release -- which also means one code path instead of a version conditional.
|
||||
|
||||
`middleware` is accepted and ignored, so callers need not care where the data
|
||||
comes from.
|
||||
"""
|
||||
rows = _zfs_lines(
|
||||
["list", "-H", "-p", "-o", "name,mountpoint,mounted", "-t", "filesystem"],
|
||||
runner=runner,
|
||||
)
|
||||
return [
|
||||
{
|
||||
"name": name,
|
||||
"properties": {
|
||||
"mountpoint": {"value": mountpoint},
|
||||
# `zfs list` prints yes/no; the planner already speaks that.
|
||||
"mounted": {"value": mounted},
|
||||
},
|
||||
}
|
||||
for name, mountpoint, mounted in (r for r in rows if len(r) == 3)
|
||||
]
|
||||
|
||||
|
||||
def list_snapshot_names(dataset, runner=None):
|
||||
"""Every snapshot at or under `dataset` -- read from ZFS, for the same reason.
|
||||
|
||||
`pool.snapshot.query` filters exactly like the dataset query does: on this box
|
||||
it returned 205 of 274 snapshots, hiding the internal datasets' snapshots. A
|
||||
sweep built on that view leaves one orphan per hidden dataset, on every run,
|
||||
forever -- which is the bug this module was written to fix in the first place.
|
||||
|
||||
(An EXACT-name delete still works on a hidden dataset, so mutations may keep
|
||||
going through middleware. It is only enumeration that lies.)
|
||||
"""
|
||||
rows = _zfs_lines(
|
||||
["list", "-H", "-o", "name", "-t", "snapshot", "-r", dataset],
|
||||
runner=runner,
|
||||
)
|
||||
return [r[0] for r in rows]
|
||||
|
||||
#: Where staging trees are assembled. tmpfs; bind mounts consume no space.
|
||||
STAGING_BASE = "/run/truecloud-nested"
|
||||
|
||||
@@ -560,7 +765,7 @@ def get_dataset_recursive(datasets, directory):
|
||||
|
||||
|
||||
def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
sleep=time.sleep):
|
||||
sleep=time.sleep, list_snapshots=None):
|
||||
"""Delete the parent snapshot AND every child created by ``zfs snapshot -r``.
|
||||
|
||||
Returns the snapshots it could NOT delete -- callers must not throw that away.
|
||||
@@ -578,6 +783,8 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
snapshots hit this, and before the fix they were orphaned permanently.
|
||||
"""
|
||||
dataset, _, snapname = snapshot.partition("@")
|
||||
svc = snapshot_service(middleware)
|
||||
list_snapshots = list_snapshots or list_snapshot_names
|
||||
|
||||
# Release ZFS's own automounts first, or `zfs destroy` refuses with EBUSY on
|
||||
# everything restic read in the last few minutes.
|
||||
@@ -591,7 +798,7 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
# through 252 sequential deletes leaves exactly the orphans this function
|
||||
# exists to prevent.
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", snapshot, {"recursive": True})
|
||||
middleware.call_sync(f"{svc}.delete", snapshot, {"recursive": True})
|
||||
return []
|
||||
except Exception as e: # noqa: BLE001 - fall through to the explicit sweep
|
||||
# Usually just "parent already gone" (stock's finally won the race once our
|
||||
@@ -608,13 +815,14 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
# our mounts are released -- which fails the recursive delete while the
|
||||
# children survive. Sweep them by name.
|
||||
try:
|
||||
snaps = middleware.call_sync(
|
||||
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
|
||||
)
|
||||
# From ZFS, not middleware: the snapshot query hides internal datasets'
|
||||
# snapshots (205 of 274 on the test box), and a sweep that cannot see them
|
||||
# orphans one per hidden dataset on every run. See list_snapshot_names().
|
||||
#
|
||||
# An empty result means the tree is already gone -- delete nothing, and
|
||||
# do not fall back to the parent, which would only log a spurious
|
||||
# "does not exist" warning on every clean run.
|
||||
names = snapshot_tree_names(snapshot, [s["name"] for s in snaps])
|
||||
names = snapshot_tree_names(snapshot, list_snapshots(dataset))
|
||||
except Exception as e: # noqa: BLE001 - fall back to at least the parent
|
||||
if logger:
|
||||
logger.warning(
|
||||
@@ -635,12 +843,9 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
if not failed:
|
||||
return []
|
||||
try:
|
||||
live = middleware.call_sync(
|
||||
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
|
||||
)
|
||||
live = set(list_snapshots(dataset))
|
||||
except Exception: # noqa: BLE001 - cannot refine; trust the delete's verdict
|
||||
return list(failed)
|
||||
live = {s["name"] for s in live}
|
||||
return [n for n in failed if n in live]
|
||||
|
||||
remaining = list(names)
|
||||
@@ -648,7 +853,7 @@ def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
failed = []
|
||||
for name in remaining:
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", name)
|
||||
middleware.call_sync(f"{svc}.delete", name)
|
||||
except Exception: # noqa: BLE001 - busy, or already gone; sorted out below
|
||||
failed.append(name)
|
||||
|
||||
@@ -694,7 +899,7 @@ def mounted_snapshots(mounts_file="/proc/self/mounts"):
|
||||
|
||||
|
||||
def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
|
||||
now=None, mounts_file="/proc/self/mounts"):
|
||||
now=None, mounts_file="/proc/self/mounts", list_snapshots=None):
|
||||
"""Delete snapshots this task left behind in an earlier run. Returns what remains.
|
||||
|
||||
The backstop for when the RECORD is gone, not just the snapshots: the sidecar lives
|
||||
@@ -707,11 +912,13 @@ def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
|
||||
"""
|
||||
dataset = current_snapshot.partition("@")[0]
|
||||
now = now or datetime.datetime.now(datetime.UTC)
|
||||
svc = snapshot_service(middleware)
|
||||
list_snapshots = list_snapshots or list_snapshot_names
|
||||
|
||||
try:
|
||||
snaps = middleware.call_sync(
|
||||
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
|
||||
)
|
||||
# From ZFS: middleware's snapshot query hides internal datasets, and an
|
||||
# orphan it cannot see is an orphan nothing will ever collect.
|
||||
all_names = list_snapshots(dataset)
|
||||
except Exception as e: # noqa: BLE001 - cannot enumerate; collect nothing
|
||||
if logger:
|
||||
logger.warning(
|
||||
@@ -720,7 +927,7 @@ def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
|
||||
return []
|
||||
|
||||
stale = stale_snapshot_names(
|
||||
task_name, current_snapshot, [s["name"] for s in snaps], now,
|
||||
task_name, current_snapshot, all_names, now,
|
||||
in_use=mounted_snapshots(mounts_file),
|
||||
)
|
||||
if not stale:
|
||||
@@ -736,7 +943,7 @@ def gc_stale_snapshots(middleware, task_name, current_snapshot, logger=None,
|
||||
remaining = []
|
||||
for name in stale:
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", name)
|
||||
middleware.call_sync(f"{svc}.delete", name)
|
||||
except Exception as e: # noqa: BLE001 - busy, or gone; either way, next run
|
||||
remaining.append(name)
|
||||
if logger:
|
||||
|
||||
@@ -438,8 +438,22 @@ class TestOnlyOurOwnTasksAreTouched:
|
||||
# The point is to add NO new failure mode to a CloudSync task. If any
|
||||
# middleware call happened before the bail-out, we would already have broken
|
||||
# the thing we are trying not to touch.
|
||||
#
|
||||
# Checked against whichever interactions the block ACTUALLY contains, not a
|
||||
# fixed list: the dataset query moved behind `_tc_nested.query_filesystems()`
|
||||
# when it switched to the public pool.* API, and a hardcoded
|
||||
# `middleware.call_sync(` simply stopped being found -- a test that silently
|
||||
# stops testing is worse than no test.
|
||||
block = extract_blocks()[name]
|
||||
gate = block.index('if not name.startswith("cloud_backup"):')
|
||||
for call in ("middleware.call_sync(", "_tc_nested.stage_nested(",
|
||||
"_tc_nested.delete_snapshot_tree("):
|
||||
|
||||
interactions = [
|
||||
"middleware.call_sync(",
|
||||
"_tc_nested.query_filesystems(",
|
||||
"_tc_nested.stage_nested(",
|
||||
"_tc_nested.delete_snapshot_tree(",
|
||||
]
|
||||
present = [c for c in interactions if c in block]
|
||||
assert present, "found no middleware interaction at all -- the test is vacuous"
|
||||
for call in present:
|
||||
assert gate < block.index(call), f"{call} runs before the cloud_backup gate"
|
||||
|
||||
+88
-38
@@ -51,24 +51,37 @@ GOOD = {
|
||||
"async def restic_backup(middleware, job, cloud_backup, dry_run=False, "
|
||||
"rate_limit=None):\n pass\n"
|
||||
),
|
||||
# The middlewared METHODS the injected code calls. TrueNAS 26 deleted both of
|
||||
# these files, taking zfs.dataset.query / zfs.snapshot.query / zfs.snapshot.delete
|
||||
# with them -- see TestMiddlewareMethodsWeCall.
|
||||
"plugins/zfs_/dataset.py": (
|
||||
"class ZFSDataset(CRUDService):\n"
|
||||
# The middlewared METHODS the injected code calls. These now go through the
|
||||
# PUBLIC pool.* API: TrueNAS 26 deleted plugins/zfs_/ outright, taking the whole
|
||||
# private zfs.* service with it -- see TestMiddlewareMethodsWeCall.
|
||||
#
|
||||
# This default tree is a MODERN box (25.10/26): it has pool.snapshot and no
|
||||
# zfs.snapshot. The older shape is built explicitly where it is tested.
|
||||
"plugins/pool_/dataset.py": (
|
||||
"class PoolDatasetService(CRUDService):\n"
|
||||
" class Config:\n"
|
||||
" namespace = 'zfs.dataset'\n"
|
||||
" namespace = 'pool.dataset'\n"
|
||||
" def query(self, filters, options):\n pass\n"
|
||||
),
|
||||
"plugins/zfs_/snapshot.py": (
|
||||
"class ZFSSnapshot(CRUDService):\n"
|
||||
"plugins/pool_/snapshot.py": (
|
||||
"class PoolSnapshotService(CRUDService):\n"
|
||||
" class Config:\n"
|
||||
" namespace = 'zfs.snapshot'\n"
|
||||
" namespace = 'pool.snapshot'\n"
|
||||
" def query(self, filters, options):\n pass\n"
|
||||
" def delete(self, id_, options={}):\n pass\n"
|
||||
),
|
||||
}
|
||||
|
||||
#: A 24.10/25.04 box: `pool.snapshot` does not exist yet and the snapshot CRUD
|
||||
#: service still answers to the (then-public) `zfs.snapshot`.
|
||||
ZFS_ERA_SNAPSHOT = (
|
||||
"class ZFSSnapshot(CRUDService):\n"
|
||||
" class Config:\n"
|
||||
" namespace = 'zfs.snapshot'\n"
|
||||
" def query(self, filters, options):\n pass\n"
|
||||
" def do_delete(self, id_, options={}):\n pass\n"
|
||||
)
|
||||
|
||||
|
||||
def loader(files):
|
||||
def load(path):
|
||||
@@ -303,52 +316,86 @@ class TestMiddlewareMethodsWeCall:
|
||||
per descendant dataset (250 on a real pool) on every run, forever.
|
||||
"""
|
||||
|
||||
ZFS_SNAPSHOT = (
|
||||
"class ZFSSnapshot(CRUDService):\n"
|
||||
" class Config:\n"
|
||||
" namespace = 'zfs.snapshot'\n"
|
||||
" def query(self, filters, options):\n pass\n"
|
||||
" def delete(self, id_, options={}):\n pass\n"
|
||||
)
|
||||
ZFS_DATASET = (
|
||||
"class ZFSDataset(CRUDService):\n"
|
||||
" class Config:\n"
|
||||
" namespace = 'zfs.dataset'\n"
|
||||
" def query(self, filters, options):\n pass\n"
|
||||
)
|
||||
POOL_SNAPSHOT = GOOD["plugins/pool_/snapshot.py"]
|
||||
|
||||
def _tree(self, **over):
|
||||
files = dict(GOOD)
|
||||
files["plugins/zfs_/snapshot.py"] = self.ZFS_SNAPSHOT
|
||||
files["plugins/zfs_/dataset.py"] = self.ZFS_DATASET
|
||||
files.update(over)
|
||||
return files
|
||||
|
||||
#: A 26 box: pool.snapshot only.
|
||||
def _modern(self, **over):
|
||||
return self._tree(**over)
|
||||
|
||||
#: A 24.10/25.04 box: zfs.snapshot only -- plugins/pool_/snapshot.py does not
|
||||
#: exist yet.
|
||||
def _zfs_era(self, **over):
|
||||
return self._tree(**{
|
||||
"plugins/pool_/snapshot.py": None,
|
||||
"plugins/zfs_/snapshot.py": ZFS_ERA_SNAPSHOT,
|
||||
**over,
|
||||
})
|
||||
|
||||
def test_present_methods_are_ok(self):
|
||||
r = check_files(self._tree())
|
||||
r = check_files(self._modern())
|
||||
assert r[NESTED]["ok"], r[NESTED]["problems"]
|
||||
|
||||
def test_a_deleted_plugin_file_is_broken(self):
|
||||
# Literally TrueNAS 26: plugins/zfs_/snapshot.py does not exist.
|
||||
r = check_files(self._tree(**{"plugins/zfs_/snapshot.py": None}))
|
||||
def test_the_OLD_zfs_era_snapshot_service_also_satisfies_the_call(self):
|
||||
# 24.10 and 25.04 have no `pool.snapshot` at all -- the CRUD service is the
|
||||
# then-public `zfs.snapshot`. Pinning only the modern spelling marked both of
|
||||
# those releases BROKEN and would have switched nested snapshots OFF on boxes
|
||||
# where they work perfectly. The runtime picks the same way; see
|
||||
# pick_snapshot_service().
|
||||
r = check_files(self._zfs_era())
|
||||
assert r[NESTED]["ok"], r[NESTED]["problems"]
|
||||
|
||||
def test_it_is_broken_only_when_NEITHER_namespace_exists(self):
|
||||
# The real failure: middleware drops the last spelling we know how to call.
|
||||
r = check_files(self._tree(**{
|
||||
"plugins/pool_/snapshot.py": None,
|
||||
"plugins/zfs_/snapshot.py": None,
|
||||
}))
|
||||
assert is_broken(r[NESTED])
|
||||
details = " ".join(p["detail"] for p in r[NESTED]["problems"])
|
||||
assert "zfs.snapshot.delete" in details
|
||||
assert "pool.snapshot.delete" in details
|
||||
assert "zfs.snapshot.delete" in details, (
|
||||
"the report must say BOTH spellings were tried, or whoever reads it will "
|
||||
"think we simply never looked for the one their box has"
|
||||
)
|
||||
|
||||
def test_we_do_NOT_depend_on_a_middleware_dataset_query_at_all(self):
|
||||
# iX could delete plugins/pool_/dataset.py tomorrow and the patch would not
|
||||
# care, because the staging plan is enumerated from ZFS, not from middleware.
|
||||
#
|
||||
# That is deliberate, and it was expensive to learn. `pool.dataset.query`
|
||||
# exists and is correctly shaped -- and it LIES: it applies a visibility
|
||||
# policy that hides ix-apps/*, .system/* and .ix-virt/* (84 of 270 datasets
|
||||
# on the real pool, including live app data). No source check could ever
|
||||
# have caught that; only running it could. So there is no assumption here
|
||||
# left to break.
|
||||
r = check_files(self._modern(**{"plugins/pool_/dataset.py": None}))
|
||||
assert r[NESTED]["ok"], r[NESTED]["problems"]
|
||||
|
||||
ids = {c.id for c in compat.MIDDLEWARE_CALLS}
|
||||
assert not any("dataset" in i or "query" in i for i in ids), (
|
||||
"a dataset/snapshot QUERY assumption crept back into the manifest -- "
|
||||
"middleware's queries are filtered; enumerate from ZFS"
|
||||
)
|
||||
|
||||
def test_a_renamed_namespace_is_broken(self):
|
||||
r = check_files(self._tree(**{
|
||||
"plugins/zfs_/snapshot.py": self.ZFS_SNAPSHOT.replace(
|
||||
"'zfs.snapshot'", "'zfs.resource.snapshot'"),
|
||||
"plugins/pool_/snapshot.py": self.POOL_SNAPSHOT.replace(
|
||||
"'pool.snapshot'", "'zfs.resource.snapshot'"),
|
||||
"plugins/zfs_/snapshot.py": None,
|
||||
}))
|
||||
assert is_broken(r[NESTED])
|
||||
|
||||
def test_the_CRUDService_do_prefix_is_accepted(self):
|
||||
# 24.10 and 25.04 declare `do_delete`; 25.10 renamed it to `delete`. BOTH
|
||||
# answer to zfs.snapshot.delete. Accepting only the literal name reported the
|
||||
# two older releases as broken -- a false BROKEN that would have switched off
|
||||
# nested snapshots on boxes where they work perfectly.
|
||||
r = check_files(self._tree(**{
|
||||
"plugins/zfs_/snapshot.py": self.ZFS_SNAPSHOT.replace(
|
||||
# A CRUDService exposes `delete` from a method NAMED `do_delete`. Both
|
||||
# spellings are live across the matrix. Accepting only the literal name
|
||||
# reported working releases as broken.
|
||||
r = check_files(self._modern(**{
|
||||
"plugins/pool_/snapshot.py": self.POOL_SNAPSHOT.replace(
|
||||
"def delete(", "def do_delete("),
|
||||
}))
|
||||
assert r[NESTED]["ok"], r[NESTED]["problems"]
|
||||
@@ -356,6 +403,9 @@ class TestMiddlewareMethodsWeCall:
|
||||
def test_the_snapshot_delete_reason_names_the_orphan_risk(self):
|
||||
# If this ever regresses, whoever reads the bug report must understand that
|
||||
# it is not a cosmetic failure.
|
||||
r = check_files(self._tree(**{"plugins/zfs_/snapshot.py": None}))
|
||||
r = check_files(self._tree(**{
|
||||
"plugins/pool_/snapshot.py": None,
|
||||
"plugins/zfs_/snapshot.py": None,
|
||||
}))
|
||||
whys = " ".join(p["why"] for p in r[NESTED]["problems"])
|
||||
assert "orphan" in whys
|
||||
|
||||
+146
-17
@@ -204,18 +204,52 @@ class FakeMiddleware:
|
||||
truecloud_nested.py. TrueNAS <= 25.10 reaches it through
|
||||
`await middleware.run_in_thread(...)` and TrueNAS 26 calls it directly, but the
|
||||
logic below the boundary is the same code either way, so it is tested once.
|
||||
|
||||
`snapshot_ns` picks which middleware GENERATION this is, because they do not
|
||||
agree on what the snapshot service is called:
|
||||
|
||||
pool.snapshot 25.10 and 26 (26 has ONLY this -- plugins/zfs_/ is gone)
|
||||
zfs.snapshot 24.10 and 25.04 (pool.snapshot does not exist yet)
|
||||
|
||||
A method in the namespace this box does NOT have raises, exactly as middleware
|
||||
does ("Method does not exist"). That is what makes the runtime picker testable:
|
||||
a module that guessed wrong would blow up here instead of silently orphaning
|
||||
snapshots on somebody's NAS.
|
||||
"""
|
||||
|
||||
def __init__(self, snapshots=None):
|
||||
def __init__(self, snapshots=None, snapshot_ns="pool.snapshot"):
|
||||
self.snapshots = list(snapshots or [])
|
||||
self.calls = []
|
||||
self.logger = None
|
||||
self.snapshot_ns = snapshot_ns
|
||||
|
||||
def get_service(self, name):
|
||||
if name != self.snapshot_ns:
|
||||
raise KeyError(name) # middleware raises KeyError for an unknown ns
|
||||
return object()
|
||||
|
||||
def list_snapshots(self, dataset):
|
||||
"""Stands in for `zfs list -t snapshot -r <dataset>`.
|
||||
|
||||
Enumeration comes from ZFS now, NOT from middleware -- middleware's query
|
||||
hides internal datasets, and a sweep that cannot see a snapshot can never
|
||||
collect it. Passed in as `list_snapshots=` so the seam is explicit.
|
||||
"""
|
||||
return [
|
||||
n for n in self.snapshots
|
||||
if n.split("@")[0] == dataset or n.startswith(dataset + "/")
|
||||
]
|
||||
|
||||
def call_sync(self, method, *args):
|
||||
self.calls.append((method, args))
|
||||
if method == "zfs.snapshot.query":
|
||||
namespace, _, op = method.rpartition(".")
|
||||
|
||||
if namespace != self.snapshot_ns:
|
||||
raise RuntimeError("Method does not exist")
|
||||
|
||||
if op == "query":
|
||||
return [{"name": n} for n in self.snapshots]
|
||||
if method == "zfs.snapshot.delete":
|
||||
if op == "delete":
|
||||
name = args[0]
|
||||
opts = args[1] if len(args) > 1 else {}
|
||||
if name not in self.snapshots:
|
||||
@@ -265,44 +299,44 @@ class TestDeleteSnapshotTree:
|
||||
mw = FakeMiddleware([
|
||||
"Tap@snap", "Tap/apps@snap", "Tap/apps/lidarr@snap", "Tap@keepme",
|
||||
])
|
||||
delete_snapshot_tree(mw, "Tap@snap")
|
||||
delete_snapshot_tree(mw, "Tap@snap", list_snapshots=mw.list_snapshots)
|
||||
assert mw.snapshots == ["Tap@keepme"]
|
||||
|
||||
def test_is_idempotent_when_stock_already_removed_the_parent(self):
|
||||
# Stock's finally can win the race once our mounts are released.
|
||||
mw = FakeMiddleware(["Tap/apps@snap", "Tap/apps/lidarr@snap"])
|
||||
delete_snapshot_tree(mw, "Tap@snap")
|
||||
delete_snapshot_tree(mw, "Tap@snap", list_snapshots=mw.list_snapshots)
|
||||
assert mw.snapshots == []
|
||||
|
||||
def test_uses_a_single_recursive_delete_not_252_individual_ones(self):
|
||||
# 252 sequential deletes are slow AND not atomic: a run killed part-way
|
||||
# through leaves exactly the orphans this function exists to prevent.
|
||||
mw = FakeMiddleware(["Tap@snap", "Tap/apps@snap", "Tap/apps/lidarr@snap"])
|
||||
delete_snapshot_tree(mw, "Tap@snap")
|
||||
delete_snapshot_tree(mw, "Tap@snap", list_snapshots=mw.list_snapshots)
|
||||
assert mw.snapshots == []
|
||||
deletes = [a for m, a in mw.calls if m == "zfs.snapshot.delete"]
|
||||
deletes = [a for m, a in mw.calls if m.endswith(".delete")]
|
||||
assert len(deletes) == 1, "should be ONE recursive call, not one per snapshot"
|
||||
assert deletes[0][1] == {"recursive": True}
|
||||
assert not [m for m, _a in mw.calls if m == "zfs.snapshot.query"], (
|
||||
assert not [m for m, _a in mw.calls if m.endswith(".query")], (
|
||||
"no enumeration needed on the fast path"
|
||||
)
|
||||
|
||||
def test_survives_recursive_and_query_failure_by_deleting_the_parent(self):
|
||||
class Broken(FakeMiddleware):
|
||||
def call_sync(self, method, *args):
|
||||
if method == "zfs.snapshot.query":
|
||||
if method.endswith(".query"):
|
||||
raise RuntimeError("boom")
|
||||
if method == "zfs.snapshot.delete" and len(args) > 1:
|
||||
if method.endswith(".delete") and len(args) > 1:
|
||||
raise RuntimeError("recursive delete unavailable")
|
||||
return super().call_sync(method, *args)
|
||||
|
||||
mw = Broken(["Tap@snap"])
|
||||
delete_snapshot_tree(mw, "Tap@snap")
|
||||
delete_snapshot_tree(mw, "Tap@snap", list_snapshots=mw.list_snapshots)
|
||||
assert mw.snapshots == []
|
||||
|
||||
def test_leaves_unrelated_snapshots_alone_when_the_tree_is_gone(self):
|
||||
mw = FakeMiddleware(["Tap@unrelated"])
|
||||
delete_snapshot_tree(mw, "Tap@snap")
|
||||
delete_snapshot_tree(mw, "Tap@snap", list_snapshots=mw.list_snapshots)
|
||||
assert mw.snapshots == ["Tap@unrelated"]
|
||||
|
||||
|
||||
@@ -692,7 +726,7 @@ class BusyMiddleware(FakeMiddleware):
|
||||
self.attempts = 0
|
||||
|
||||
def call_sync(self, method, *args):
|
||||
if method == "zfs.snapshot.delete":
|
||||
if method.endswith(".delete"):
|
||||
name = args[0]
|
||||
opts = args[1] if len(args) > 1 else {}
|
||||
if opts.get("recursive"):
|
||||
@@ -713,7 +747,7 @@ class TestDeleteRetriesAndReportsSurvivors:
|
||||
["Tap@snap", "Tap/apps@snap", "Tap/apps/prometheus@snap"],
|
||||
busy=["Tap/apps/prometheus@snap"], busy_for=1,
|
||||
)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None, list_snapshots=mw.list_snapshots)
|
||||
assert survivors == []
|
||||
assert mw.snapshots == []
|
||||
|
||||
@@ -725,7 +759,7 @@ class TestDeleteRetriesAndReportsSurvivors:
|
||||
["Tap@snap", "Tap/apps/prometheus@snap"],
|
||||
busy=["Tap/apps/prometheus@snap"], busy_for=99,
|
||||
)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None, list_snapshots=mw.list_snapshots)
|
||||
assert survivors == ["Tap/apps/prometheus@snap"]
|
||||
assert mw.snapshots == ["Tap/apps/prometheus@snap"]
|
||||
|
||||
@@ -742,7 +776,7 @@ class TestDeleteRetriesAndReportsSurvivors:
|
||||
return real(method, *args)
|
||||
|
||||
mw.call_sync = spy
|
||||
tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None, list_snapshots=mw.list_snapshots)
|
||||
assert order[0] == ("release", "snap"), order
|
||||
|
||||
|
||||
@@ -1002,6 +1036,7 @@ class TestGarbageCollectorExecution:
|
||||
remaining = tn.gc_stale_snapshots(
|
||||
mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
|
||||
now=now, mounts_file=str(mounts),
|
||||
list_snapshots=mw.list_snapshots,
|
||||
)
|
||||
assert remaining == []
|
||||
assert mw.snapshots == [
|
||||
@@ -1026,6 +1061,7 @@ class TestGarbageCollectorExecution:
|
||||
remaining = tn.gc_stale_snapshots(
|
||||
mw, "cloud_backup-5", "Tap@cloud_backup-5-20260714115900",
|
||||
now=now, mounts_file=str(mounts),
|
||||
list_snapshots=mw.list_snapshots,
|
||||
)
|
||||
assert remaining == [orphan]
|
||||
|
||||
@@ -1039,7 +1075,7 @@ class TestGarbageCollectorExecution:
|
||||
|
||||
class Broken(FakeMiddleware):
|
||||
def call_sync(self, method, *args):
|
||||
if method == "zfs.snapshot.query":
|
||||
if method.endswith(".query"):
|
||||
raise RuntimeError("middleware is having a day")
|
||||
return super().call_sync(method, *args)
|
||||
|
||||
@@ -1049,3 +1085,96 @@ class TestGarbageCollectorExecution:
|
||||
now=dt.datetime(2026, 7, 14, 12, 0, 0, tzinfo=dt.UTC),
|
||||
mounts_file=str(mounts),
|
||||
) == []
|
||||
|
||||
|
||||
class TestEnumerationComesFromZfsNotMiddleware:
|
||||
"""The bug that a source check can never catch, found only by running it.
|
||||
|
||||
Porting the deleted private `zfs.dataset.query` to the public
|
||||
`pool.dataset.query` looked obviously right: the method exists, it is
|
||||
documented, iX will not delete it. Every test passed and `compat.py` went green
|
||||
on TrueNAS 26.
|
||||
|
||||
It was wrong. The public query applies a VISIBILITY POLICY -- on a real box it
|
||||
returns 205 of 274 datasets, hiding `ix-apps/*`, `.system/*` and `.ix-virt/*`.
|
||||
On the production pool that is 84 of 270, and `ix-apps` holds LIVE APPLICATION
|
||||
DATA. The staging plan would have omitted every one of them, and `plan_staging`
|
||||
would never have seen them, so they would not even appear in `skipped`. A green
|
||||
backup, silently missing data -- the precise failure this module exists to
|
||||
prevent.
|
||||
|
||||
The snapshot query lies the same way, so the sweep would orphan one snapshot per
|
||||
hidden dataset, forever.
|
||||
|
||||
Hence: READ from ZFS, MUTATE through middleware. These tests hold that line.
|
||||
"""
|
||||
|
||||
def test_query_filesystems_shells_out_to_zfs(self):
|
||||
import truecloud_nested as tn
|
||||
|
||||
seen = []
|
||||
|
||||
class R:
|
||||
returncode = 0
|
||||
stdout = (
|
||||
"scratch\t/mnt/scratch\tyes\n"
|
||||
"scratch/ix-apps\t/mnt/scratch/ix-apps\tyes\n" # middleware HIDES this one
|
||||
"scratch/.system\tlegacy\tno\n"
|
||||
)
|
||||
stderr = ""
|
||||
|
||||
def runner(cmd):
|
||||
seen.append(cmd)
|
||||
return R()
|
||||
|
||||
rows = tn.query_filesystems(runner=runner)
|
||||
|
||||
assert seen and seen[0][0] == "zfs", "must read ZFS, not call middleware"
|
||||
names = [r["name"] for r in rows]
|
||||
assert "scratch/ix-apps" in names, (
|
||||
"ix-apps is exactly what pool.dataset.query hides, and exactly what "
|
||||
"holds live app data. If it is not here the backup omits it silently."
|
||||
)
|
||||
# ...and it still speaks the shape the planner expects.
|
||||
row = next(r for r in rows if r["name"] == "scratch/.system")
|
||||
assert row["properties"]["mountpoint"]["value"] == "legacy"
|
||||
assert row["properties"]["mounted"]["value"] == "no"
|
||||
|
||||
def test_a_failing_zfs_raises_rather_than_returning_an_empty_list(self):
|
||||
# The whole failure class in one assertion. "No datasets" and "the command
|
||||
# broke" must never look the same: a caller that cannot tell them apart
|
||||
# stages nothing, sweeps nothing, and reports success.
|
||||
import truecloud_nested as tn
|
||||
|
||||
class R:
|
||||
returncode = 1
|
||||
stdout = ""
|
||||
stderr = "cannot open 'scratch': no such pool"
|
||||
|
||||
with pytest.raises(tn.ZfsError, match="no such pool"):
|
||||
tn.query_filesystems(runner=lambda cmd: R())
|
||||
|
||||
with pytest.raises(tn.ZfsError):
|
||||
tn.list_snapshot_names("scratch", runner=lambda cmd: R())
|
||||
|
||||
def test_list_snapshot_names_reads_the_whole_tree_from_zfs(self):
|
||||
import truecloud_nested as tn
|
||||
|
||||
seen = []
|
||||
|
||||
class R:
|
||||
returncode = 0
|
||||
stdout = "scratch@s\nscratch/ix-apps@s\n"
|
||||
stderr = ""
|
||||
|
||||
def runner(cmd):
|
||||
seen.append(cmd)
|
||||
return R()
|
||||
|
||||
names = tn.list_snapshot_names("scratch", runner=runner)
|
||||
assert names == ["scratch@s", "scratch/ix-apps@s"]
|
||||
assert "-t" in seen[0] and "snapshot" in seen[0] and "-r" in seen[0]
|
||||
assert "scratch/ix-apps@s" in names, (
|
||||
"pool.snapshot.query hides this; a sweep that cannot see it orphans it "
|
||||
"on every single run, forever"
|
||||
)
|
||||
|
||||
+127
-40
@@ -162,54 +162,119 @@ class MiddlewareCall:
|
||||
the whole design: declining is always the cheaper mistake.
|
||||
"""
|
||||
|
||||
def __init__(self, ident, module, method, path, why=""):
|
||||
def __init__(self, ident, module, method, path, why="", also=()):
|
||||
self.id = ident
|
||||
self.module = module
|
||||
self.method = method # "zfs.snapshot.delete"
|
||||
self.method = method # "pool.snapshot.delete"
|
||||
self.path = path # plugin file that declares it
|
||||
self.why = why
|
||||
#: Equally acceptable spellings of the SAME call, as (method, path) pairs.
|
||||
#:
|
||||
#: No single snapshot namespace spans every supported release. 24.10 and
|
||||
#: 25.04 expose the CRUD service as the public `zfs.snapshot`; 25.10
|
||||
#: promoted it to `pool.snapshot` and demoted `zfs.snapshot` to private;
|
||||
#: 26 deleted `plugins/zfs_/` entirely. Pinning either one alone marks
|
||||
#: half the matrix BROKEN and declines to apply on versions that work
|
||||
#: perfectly well.
|
||||
#:
|
||||
#: The call is satisfied if ANY option is present. The runtime picks the
|
||||
#: same way -- see `pick_snapshot_service()` in the nested module -- so
|
||||
#: what this checks and what the patch does cannot drift apart.
|
||||
self.also = tuple(also)
|
||||
|
||||
@property
|
||||
def options(self):
|
||||
"""Every (method, path) that would satisfy this call, best first."""
|
||||
return ((self.method, self.path), *self.also)
|
||||
|
||||
@staticmethod
|
||||
def namespace_of(method):
|
||||
return method.rsplit(".", 1)[0]
|
||||
|
||||
@staticmethod
|
||||
def name_of(method):
|
||||
return method.rsplit(".", 1)[1]
|
||||
|
||||
@property
|
||||
def namespace(self):
|
||||
return self.method.rsplit(".", 1)[0]
|
||||
return self.namespace_of(self.method)
|
||||
|
||||
@property
|
||||
def name(self):
|
||||
return self.method.rsplit(".", 1)[1]
|
||||
return self.name_of(self.method)
|
||||
|
||||
|
||||
#: Every middlewared method the nested module calls at runtime.
|
||||
#: The middleware methods the nested module CALLS.
|
||||
#:
|
||||
#: These used to be the PRIVATE `zfs.*` service (`zfs.dataset.query`,
|
||||
#: `zfs.snapshot.delete`, `zfs.snapshot.query`). TrueNAS 26 deleted
|
||||
#: `plugins/zfs_/` outright and every one of them vanished -- silently, because a
|
||||
#: private service carries no stability contract and nothing warned us. The patch
|
||||
#: would have applied cleanly and then failed on the first backup.
|
||||
#:
|
||||
#: The replacements are the PUBLIC `pool.*` API, and switching to it is not merely
|
||||
#: a TrueNAS 26 fix -- it is the correct call on every version:
|
||||
#:
|
||||
#: * It is public, documented, and covered by iX's deprecation policy, so it
|
||||
#: cannot be deleted from under us the way `zfs.*` just was.
|
||||
#: * The same methods, in the same files, exist on 24.10 through 26. One code
|
||||
#: path, no version conditionals.
|
||||
#: * `pool.snapshot.delete` takes `recursive`, which the private call did not.
|
||||
#: The old sweep had to enumerate ~250 snapshots and delete them one at a
|
||||
#: time, and any it missed leaked forever.
|
||||
MIDDLEWARE_CALLS = [
|
||||
MiddlewareCall(
|
||||
"call-zfs-dataset-query", NESTED, "zfs.dataset.query",
|
||||
"plugins/zfs_/dataset.py",
|
||||
why="SNAPSHOT_BLOCK enumerates FILESYSTEM datasets to build the staging plan",
|
||||
),
|
||||
MiddlewareCall(
|
||||
"call-zfs-snapshot-delete", NESTED, "zfs.snapshot.delete",
|
||||
"plugins/zfs_/snapshot.py",
|
||||
"call-snapshot-delete", NESTED, "pool.snapshot.delete",
|
||||
"plugins/pool_/snapshot.py",
|
||||
also=[("zfs.snapshot.delete", "plugins/zfs_/snapshot.py")],
|
||||
why="delete_snapshot_tree() sweeps the recursive snapshot. Without it every "
|
||||
"run orphans one snapshot per descendant dataset (250 on a real pool)",
|
||||
),
|
||||
MiddlewareCall(
|
||||
"call-zfs-snapshot-query", NESTED, "zfs.snapshot.query",
|
||||
"plugins/zfs_/snapshot.py",
|
||||
why="delete_snapshot_tree()'s fallback sweep enumerates the tree by name",
|
||||
),
|
||||
]
|
||||
|
||||
# There is deliberately NO entry here for a dataset or snapshot QUERY.
|
||||
#
|
||||
# The patch used to call `zfs.dataset.query` / `zfs.snapshot.query` (private, and
|
||||
# deleted in TrueNAS 26). The obvious port was to the public `pool.dataset.query` /
|
||||
# `pool.snapshot.query` -- and that port was WRONG in a way no source check could
|
||||
# ever have caught, because the methods are all present and correctly shaped.
|
||||
#
|
||||
# They are simply filtered. On a real box they return 205 of 274 datasets and 205
|
||||
# of 274 snapshots, hiding `ix-apps/*`, `.system/*` and `.ix-virt/*` -- 84 of 270
|
||||
# on the production pool, including live application data. Staging from that view
|
||||
# silently omits them; sweeping from it orphans one snapshot per hidden dataset,
|
||||
# forever.
|
||||
#
|
||||
# So the module enumerates from ZFS itself and there is no middleware assumption
|
||||
# left to check. That is the point: the fewer things we assume about middleware,
|
||||
# the less there is for iX to break. Only the MUTATION is still a middleware call,
|
||||
# and that is the one entry above.
|
||||
|
||||
|
||||
def check_call(c: MiddlewareCall, src: str | None,
|
||||
method: str | None = None, path: str | None = None,
|
||||
) -> tuple[str, str | None]:
|
||||
"""Is `method` still registered by middlewared?
|
||||
|
||||
`method`/`path` name WHICH spelling of the call is being tried -- a call may
|
||||
have several equally acceptable ones (see MiddlewareCall.also). They default
|
||||
to the preferred spelling.
|
||||
"""
|
||||
method = method or c.method
|
||||
path = path or c.path
|
||||
namespace = MiddlewareCall.namespace_of(method)
|
||||
name = MiddlewareCall.name_of(method)
|
||||
|
||||
def check_call(c: MiddlewareCall, src: str | None) -> tuple[str, str | None]:
|
||||
"""Is `c.method` still registered by middlewared?"""
|
||||
if src is None:
|
||||
return "broken", (
|
||||
f"{c.path} no longer exists, so `{c.method}` is gone"
|
||||
f"{path} no longer exists, so `{method}` is gone"
|
||||
)
|
||||
|
||||
try:
|
||||
tree = ast.parse(_stock(src))
|
||||
except SyntaxError as e:
|
||||
return "unknown", f"{c.path} does not parse: {e}"
|
||||
return "unknown", f"{path} does not parse: {e}"
|
||||
|
||||
# namespace = 'zfs.snapshot' on some Service class in this file...
|
||||
namespaces = {
|
||||
@@ -220,10 +285,10 @@ def check_call(c: MiddlewareCall, src: str | None) -> tuple[str, str | None]:
|
||||
and isinstance(n.value.value, str)
|
||||
and any(isinstance(t, ast.Name) and t.id == "namespace" for t in n.targets)
|
||||
}
|
||||
if c.namespace not in namespaces:
|
||||
if namespace not in namespaces:
|
||||
return "broken", (
|
||||
f"{c.path} no longer declares namespace {c.namespace!r} "
|
||||
f"(found: {sorted(namespaces) or 'none'}), so `{c.method}` is gone"
|
||||
f"{path} no longer declares namespace {namespace!r} "
|
||||
f"(found: {sorted(namespaces) or 'none'}), so `{method}` is gone"
|
||||
)
|
||||
|
||||
# ...and it defines the method.
|
||||
@@ -238,8 +303,8 @@ def check_call(c: MiddlewareCall, src: str | None) -> tuple[str, str | None]:
|
||||
n.name for n in ast.walk(tree)
|
||||
if isinstance(n, ast.FunctionDef | ast.AsyncFunctionDef)
|
||||
}
|
||||
if c.name not in defined and f"do_{c.name}" not in defined:
|
||||
return "broken", f"{c.path} no longer defines `{c.method}`"
|
||||
if name not in defined and f"do_{name}" not in defined:
|
||||
return "broken", f"{path} no longer defines `{method}`"
|
||||
|
||||
return "ok", None
|
||||
|
||||
@@ -555,28 +620,46 @@ def check(loader, modules=None) -> dict:
|
||||
})
|
||||
|
||||
# The methods the injected code CALLS, not just the symbols it wraps.
|
||||
#
|
||||
# A call may have several equally acceptable spellings, because no single
|
||||
# snapshot namespace spans every supported release (24.10 has `zfs.snapshot`,
|
||||
# 26 has only `pool.snapshot`). It is satisfied if ANY of them is present --
|
||||
# exactly as the runtime resolves it -- and BROKEN only when they all vanish.
|
||||
for c in MIDDLEWARE_CALLS:
|
||||
if c.module not in out:
|
||||
continue
|
||||
try:
|
||||
text = src(c.path)
|
||||
except Unreadable as e:
|
||||
out[c.module]["unknown"] = True
|
||||
out[c.module]["problems"].append({
|
||||
"id": c.id, "detail": f"could not read {c.path}: {e}", "why": c.why,
|
||||
})
|
||||
|
||||
satisfied, unknown, details = False, False, []
|
||||
for method, path in c.options:
|
||||
try:
|
||||
text = src(path)
|
||||
except Unreadable as e:
|
||||
unknown = True
|
||||
details.append(f"could not read {path}: {e}")
|
||||
continue
|
||||
|
||||
status, detail = check_call(c, text, method, path)
|
||||
if status == "ok":
|
||||
satisfied = True
|
||||
break
|
||||
if status == "unknown":
|
||||
unknown = True
|
||||
details.append(detail)
|
||||
|
||||
if satisfied:
|
||||
continue
|
||||
|
||||
status, detail = check_call(c, text)
|
||||
if status == "broken":
|
||||
out[c.module]["ok"] = False
|
||||
out[c.module]["problems"].append({
|
||||
"id": c.id, "detail": detail, "why": c.why,
|
||||
})
|
||||
elif status == "unknown":
|
||||
# Every spelling failed. If we could not READ one of them we do not know
|
||||
# that it is broken -- a rate-limited fetch is not a regression.
|
||||
if unknown:
|
||||
out[c.module]["unknown"] = True
|
||||
out[c.module]["problems"].append({
|
||||
"id": c.id, "detail": detail, "why": c.why,
|
||||
"id": c.id, "detail": "; ".join(details), "why": c.why,
|
||||
})
|
||||
else:
|
||||
out[c.module]["ok"] = False
|
||||
out[c.module]["problems"].append({
|
||||
"id": c.id, "detail": "; ".join(details), "why": c.why,
|
||||
})
|
||||
|
||||
for module, (path, phrase, native_when_present) in NATIVE_PROBES.items():
|
||||
@@ -804,6 +887,10 @@ def is_broken(r: dict) -> bool:
|
||||
#: a strictly weaker claim than "a restore worked". Add a row only after doing it.
|
||||
HARDWARE_VERIFIED = {
|
||||
"25.10.4": "nested + providers; 252-snapshot recursive backup of /mnt/Tap, 18m",
|
||||
"26.0.0-BETA.1": (
|
||||
"nested + providers; 274-snapshot recursive backup of a 292-dataset pool, "
|
||||
"restored a 4-deep child dataset byte-identical"
|
||||
),
|
||||
}
|
||||
|
||||
_LEGEND = """
|
||||
|
||||
Reference in New Issue
Block a user