TrueNAS 26 support: one sync implementation, two wrappers
CI / python 3.13 (push) Successful in 15s
CI / shell (shellcheck + syntax) (push) Successful in 8s
CI / python 3.11 (push) Successful in 14s
CI / python 3.12 (push) Successful in 16s
TrueNAS compatibility / compat (push) Successful in 9s

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.
This commit is contained in:
2026-07-13 18:18:28 +00:00
parent cf2c6a8a02
commit 498b2690e1
8 changed files with 475 additions and 179 deletions
+24 -5
View File
@@ -57,11 +57,30 @@ worse than no alert, because one day it carries a security fix.
appeared somewhere in the signature, so it happily passed a call that could never
work.
- **TrueNAS 26 rewrites the entire `cloud_backup` path from async to synchronous.**
Every block the nested module injects is an `async def` wrapping an `await`ed
original, so on 26 it would hand `sync.py` a coroutine where it unpacks a tuple —
a broken backup, discovered at restore time. On TrueNAS 26 the nested module now
stays off rather than applying and breaking.
### Added
- **TrueNAS 26 support.** 26 rewrites the entire `cloud_backup` path from async to
**synchronous**, and separately **deletes `get_dataset_recursive()`** — which one
of the injected blocks called out of the host module's namespace. Either one is a
broken backup found at restore time: an `async def` wrapper hands `sync.py` a
coroutine where it unpacks a tuple, and the vanished helper is a straight
`NameError`.
The nested module is now **one synchronous implementation** (talking to middlewared
through `call_sync`) behind **two thin wrappers**. `apply.sh` reads which flavour
the installed middleware declares and injects the matching one: TrueNAS ≤ 25.10
reaches it via `await middleware.run_in_thread(...)`, and TrueNAS 26 — already in a
worker thread — calls it directly. The logic that owns the snapshots, the bind
mounts and the failure modes exists **once**; an async twin would mean every future
fix had to land twice, and the one that got missed would be the one that eats a
backup.
A middleware whose three wrapped functions **disagree** about async-ness is refused
outright rather than guessed at. And `get_dataset_recursive` is now carried as our
own copy — removing the dependency on both versions instead of asserting it.
Both breaks were found by the daily compatibility check **while 26 was still in
beta**, which is the entire point of it.
- **An incompatible TrueNAS no longer sets the permanent kill switch.** `apply.sh`
reused a "nothing left to do" exit that touches `disabled`, which suppresses
+25 -11
View File
@@ -6,8 +6,8 @@
> 24.10**. There is nothing here to install on an older release, and `install.sh`
> will refuse.
>
> Verified on **24.10**, **25.04** and **25.10**. Not yet compatible with the
> unreleased **26.0** (see [TrueNAS compatibility](#truenas-compatibility)).
> Verified on **24.10**, **25.04**, **25.10**, and the unreleased **26.0 beta**
> (see [TrueNAS compatibility](#truenas-compatibility)).
Extends TrueNAS SCALE's **TrueCloud Backup** feature to:
@@ -44,7 +44,7 @@ costs a fraction of the new Storj price.
| 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 |
@@ -62,17 +62,31 @@ The table above is **regenerated daily by CI** — it is not a claim somebody ty
once and forgot. **TrueCloud Backup does not exist before 24.10**, so earlier
versions are absent rather than "unsupported".
### ⚠️ TrueNAS 26 breaks nested snapshots (not yet released)
### TrueNAS 26 — supported, and this is how we knew in advance
TrueNAS 26 rewrites the whole `cloud_backup` path **from async to synchronous**.
Every block the nested module injects is an `async def` wrapping an `await`ed
original, so on 26 it would hand `sync.py` a coroutine where it unpacks a tuple.
Every block the nested module injected was an `async def` wrapping an `await`ed
original, so on 26 it would have handed `sync.py` a coroutine where it unpacks a
tuple, and 26 also **deleted `get_dataset_recursive()`**, which one of those blocks
called. Both are backup-breaking, and neither would have surfaced until a restore
failed.
You do not need to do anything. `apply.sh` checks these assumptions against the
middleware **actually installed on your box** at every boot, and will not apply a
module that no longer fits. On TrueNAS 26 the nested module simply stays off:
backups keep running, without nested-dataset coverage. A broken backup is worse
than a missing feature.
The daily compatibility check found both **while 26 was still in beta**, and filed
the bug report itself. The patch now reads which flavour of `cloud_backup` your box
has and injects the wrapper that matches — one implementation of the actual logic,
two thin wrappers — and carries its own copy of the deleted helper.
**If a future TrueNAS breaks it anyway, nothing bad happens quietly.** `apply.sh`
re-checks these assumptions against the middleware *actually installed on your box*
at every boot and will not apply a module that no longer fits: TrueNAS is left
stock, backups keep running without that module's feature, and the reason is named
in `apply.log`. A broken backup is worse than a missing feature.
`master` (the development branch after 26) currently reports **BROKEN**: iXsystems
are still reshaping these functions there — renaming `middleware` to `context`,
`cloud_backup` to `entry`, adding a required `credentials` parameter. That is a
moving target and is deliberately not chased; the check will keep reporting it until
it settles into a beta, which is exactly when it becomes worth fixing.
### How this is kept honest
+132 -46
View File
@@ -491,20 +491,41 @@ else:
# the guard removed but the traversal missing -- that would be a silently empty
# backup, the worst possible outcome.
SNAPSHOT_BLOCK = """
# ── nested blocks: one core, two wrappers ─────────────────────────────────────
#
# TrueNAS <= 25.10 has an ASYNC cloud_backup path; TrueNAS 26 rewrote it SYNCHRONOUS
# (`middleware.call_sync` throughout, no awaits). An `async def` wrapper on 26 hands
# sync.py a coroutine where it unpacks a tuple, and a `def` wrapper on 25.10 blocks
# the event loop. So each block is assembled from:
#
# * a CORE, written once, synchronous, using middleware.call_sync -- which is safe
# from a worker thread and deadlocks on the event loop; and
# * a WRAPPER matching the stock function's own flavour, chosen at apply time by
# reading whether the installed middlewared declares it `async def`.
#
# On <= 25.10 the async wrapper hops to a thread via `await middleware.run_in_thread`
# -- exactly the thread call_sync needs. On 26 the stock function is already running
# in middlewared's thread pool (its own code calls call_sync), so the sync wrapper
# calls the core directly.
#
# The logic that matters -- snapshots, bind mounts, failure modes -- exists once.
# An async twin would mean every future fix had to land twice, and the one that got
# missed would be the one that eats a backup.
_NESTED_IMPORT = """
# TRUECLOUD_PATCH — added by truenas-truecloud-patch/patch/apply.sh
try:
from middlewared.plugins.cloud import _truecloud_nested as _tc_nested
except ImportError:
_tc_nested = None
"""
SNAPSHOT_CORE = _NESTED_IMPORT + """
if _tc_nested is not None:
_tc_orig_create_snapshot = create_snapshot
async def create_snapshot(middleware, path, name="cloud_task-onetime"):
# Stock takes the (already recursive) snapshot; we only replace the PATH.
snapshot, snap_path = await _tc_orig_create_snapshot(middleware, path, name)
def _tc_stage(middleware, path, name, snapshot, snap_path):
# Synchronous, and always called from a worker thread (see above).
_logger = getattr(middleware, "logger", None)
try:
# Enumerate datasets AFTER the snapshot, never before. The snapshot is
@@ -513,10 +534,13 @@ 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 = await middleware.call(
datasets = middleware.call_sync(
"zfs.dataset.query", [["type", "=", "FILESYSTEM"]]
)
dataset, nested = get_dataset_recursive(datasets, path)
# 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.
dataset, nested = _tc_nested.get_dataset_recursive(datasets, path)
if not nested:
# No children: stock behaviour, untouched. Stock's `finally` owns
@@ -524,38 +548,42 @@ if _tc_nested is not None:
# because a non-nested snapshot has no children).
return snapshot, snap_path
staging_root = await _tc_nested.stage_nested(
staging_root = _tc_nested.stage_nested(
middleware, path, snapshot,
dataset["name"], dataset["properties"]["mountpoint"]["value"],
name, datasets, logger=_logger,
)
except Exception:
# The snapshot exists, but this exception means sync.py never completes
# `snapshot, local_path = await create_snapshot(...)`, so its local
# `snapshot` stays None and its `finally` deletes NOTHING. Sweep the
# tree ourselves or leak the parent plus one snapshot per descendant
# dataset (160+ here) on every failed run.
await _tc_nested.delete_snapshot_tree(middleware, snapshot, logger=_logger)
# `snapshot, local_path = create_snapshot(...)`, so its local `snapshot`
# stays None and its `finally` deletes NOTHING. Sweep the tree ourselves
# or leak the parent plus one snapshot per descendant dataset (160+ here)
# on every failed run.
_tc_nested.delete_snapshot_tree(middleware, snapshot, logger=_logger)
raise
return snapshot, staging_root
"""
SNAPSHOT_ASYNC = SNAPSHOT_CORE + """
async def create_snapshot(middleware, path, name="cloud_task-onetime"):
snapshot, snap_path = await _tc_orig_create_snapshot(middleware, path, name)
return await middleware.run_in_thread(
_tc_stage, middleware, path, name, snapshot, snap_path
)
create_snapshot._truecloud_patched = True
"""
CRUD_BLOCK = """
# TRUECLOUD_PATCH — added by truenas-truecloud-patch/patch/apply.sh
try:
from middlewared.plugins.cloud import _truecloud_nested as _tc_nested
except ImportError:
_tc_nested = None
SNAPSHOT_SYNC = SNAPSHOT_CORE + """
def create_snapshot(middleware, path, name="cloud_task-onetime"):
snapshot, snap_path = _tc_orig_create_snapshot(middleware, path, name)
return _tc_stage(middleware, path, name, snapshot, snap_path)
if _tc_nested is not None:
_tc_orig_validate = CloudTaskServiceMixin._validate
async def _tc_validate(self, app, verrors, name, data):
await _tc_orig_validate(self, app, verrors, name, data)
create_snapshot._truecloud_patched = True
"""
_CRUD_FILTER = """
# Only cloud_backup: staging teardown is wired into cloud_backup.sync's
# finally. cloudsync would leak bind mounts, so leave its guard intact.
if getattr(getattr(self, "_config", None), "namespace", "") != "cloud_backup":
@@ -576,34 +604,63 @@ if _tc_nested is not None:
CloudTaskServiceMixin._validate._truecloud_patched = True
"""
SYNC_BLOCK = """
# TRUECLOUD_PATCH — added by truenas-truecloud-patch/patch/apply.sh
try:
from middlewared.plugins.cloud import _truecloud_nested as _tc_nested
except ImportError:
_tc_nested = None
CRUD_ASYNC = _NESTED_IMPORT + """
if _tc_nested is not None:
_tc_orig_validate = CloudTaskServiceMixin._validate
async def _tc_validate(self, app, verrors, name, data):
await _tc_orig_validate(self, app, verrors, name, data)
""" + _CRUD_FILTER
CRUD_SYNC = _NESTED_IMPORT + """
if _tc_nested is not None:
_tc_orig_validate = CloudTaskServiceMixin._validate
def _tc_validate(self, app, verrors, name, data):
_tc_orig_validate(self, app, verrors, name, data)
""" + _CRUD_FILTER
# *args/**kwargs, not the stock signature spelled out.
#
# 24.10 and 25.04 have `restic_backup(middleware, job, cloud_backup, dry_run)`;
# 25.10 added `rate_limit`. Naming them and forwarding all five raised
# `TypeError: takes 4 positional arguments but 5 were given` on every nested backup
# on the two older releases. Forwarding whatever we were handed makes this wrapper
# indifferent to iX adding or dropping a trailing parameter -- which they have now
# done twice.
#
# Our bind mounts pin the ZFS snapshot, so stock's `finally` cannot destroy it
# (EBUSY) and logs one benign warning. We unmount here and then delete it for real.
SYNC_ASYNC = _NESTED_IMPORT + """
if _tc_nested is not None:
_tc_orig_restic_backup = restic_backup
async def restic_backup(middleware, job, cloud_backup, *args, **kwargs):
# *args/**kwargs, not the stock signature spelled out.
#
# 24.10 and 25.04 have `restic_backup(middleware, job, cloud_backup, dry_run)`;
# 25.10 added `rate_limit`. Naming them here and forwarding all five raised
# `TypeError: takes 4 positional arguments but 5 were given` on every nested
# backup on the two older releases. Forwarding whatever we were handed makes
# this wrapper indifferent to iX adding or dropping a trailing parameter --
# which they have now done twice.
#
# Our bind mounts pin the ZFS snapshot, so stock's `finally` cannot
# destroy it (EBUSY) and logs one benign warning. We unmount here and
# then delete the snapshot for real.
try:
return await _tc_orig_restic_backup(middleware, job, cloud_backup, *args, **kwargs)
finally:
try:
await _tc_nested.cleanup_task(
await middleware.run_in_thread(
_tc_nested.cleanup_task,
middleware,
f"cloud_backup-{cloud_backup.get('id', 'onetime')}",
)
except Exception as e:
middleware.logger.warning("truecloud-patch: staging cleanup failed: %r", e)
restic_backup._truecloud_patched = True
"""
SYNC_SYNC = _NESTED_IMPORT + """
if _tc_nested is not None:
_tc_orig_restic_backup = restic_backup
def restic_backup(middleware, job, cloud_backup, *args, **kwargs):
try:
return _tc_orig_restic_backup(middleware, job, cloud_backup, *args, **kwargs)
finally:
try:
_tc_nested.cleanup_task(
middleware,
f"cloud_backup-{cloud_backup.get('id', 'onetime')}",
logger=getattr(middleware, "logger", None),
@@ -706,10 +763,39 @@ else:
if missing:
raise FileNotFoundError('missing: ' + ', '.join(missing))
# Which flavour of cloud_backup is installed? <= 25.10 is async; TrueNAS 26
# rewrote it synchronous. Inject the wrapper that matches: an `async def` on
# 26 hands sync.py a coroutine where it unpacks a tuple, and a plain `def` on
# 25.10 blocks the event loop.
#
# None means the three stock functions disagree, or one could not be read.
# Refuse rather than guess -- a half-converted middleware is one this patch
# has never seen, and guessing wrong there costs a backup, not a feature.
# nested_src is <repo>/patch/truecloud_nested.py, so tools/ is its sibling.
# APPEND, never insert(0) -- shadowing the stdlib for this interpreter is a
# far worse failure than not finding compat.
sys.path.append(
os.path.join(os.path.dirname(os.path.dirname(nested_src)), 'tools')
)
import compat
_flavour = compat.async_flavour_tree(mw_dir)
if _flavour is None:
raise RuntimeError(
'cannot tell whether this TrueNAS cloud_backup path is async or '
'sync (the wrapped functions disagree, or could not be read)'
)
_snapshot_block = SNAPSHOT_ASYNC if _flavour else SNAPSHOT_SYNC
_sync_block = SYNC_ASYNC if _flavour else SYNC_SYNC
_crud_block = CRUD_ASYNC if _flavour else CRUD_SYNC
shutil.copyfile(nested_src, nested_dst) # 1. traversal implementation
patch_file(snapshot_py, SNAPSHOT_BLOCK) # 2. build the staging tree
patch_file(sync_path, SYNC_BLOCK) # 3. tear it down afterwards
patch_file(crud_py, CRUD_BLOCK) # 4. ONLY NOW allow nested tasks
patch_file(snapshot_py, _snapshot_block) # 2. build the staging tree
patch_file(sync_path, _sync_block) # 3. tear it down afterwards
patch_file(crud_py, _crud_block) # 4. ONLY NOW allow nested tasks
print('OK: cloud_backup is %s; injected the matching wrappers.'
% ('async (TrueNAS <= 25.10)' if _flavour else 'synchronous (TrueNAS 26+)'))
nested_ok = True
nested_detail = 'nested-dataset snapshots enabled (staging tree)'
+79 -20
View File
@@ -362,10 +362,69 @@ def teardown(staging_root, runner=_run, mounts_file="/proc/self/mounts"):
return errors
# ── async orchestration (middleware is duck-typed; no middlewared import) ─────
# ── orchestration (middleware is duck-typed; no middlewared import) ───────────
#
# These are SYNCHRONOUS and talk to middlewared via `middleware.call_sync`, which
# is safe from a worker thread and deadlocks on the event loop. That is the whole
# reason this file has one implementation instead of two:
#
# TrueNAS <= 25.10 cloud_backup is async. The injected wrapper is `async def` and
# hands these to `await middleware.run_in_thread(...)`, which is
# exactly the thread `call_sync` needs.
# TrueNAS >= 26 cloud_backup is synchronous and already runs in middlewared's
# thread pool (its own code calls `call_sync`). The injected
# wrapper calls these directly.
#
# So the async/sync difference lives entirely in the three injected blocks, and the
# logic below -- the part with the snapshots, the bind mounts and the failure modes
# -- is written once. Duplicating it as an async twin would mean every future fix
# had to be made twice, and the one that got missed would be the one that eats a
# backup.
async def delete_snapshot_tree(middleware, snapshot, logger=None):
def get_dataset_recursive(datasets, directory):
"""The dataset containing `directory`, and whether anything is nested under it.
Vendored from middlewared's own plugins/cloud/snapshot.py (TrueNAS <= 25.10),
because TrueNAS 26 DELETED it -- create_snapshot there uses filesystem.statfs
instead. The injected block used to call it out of the host module's namespace,
which on 26 is a straight NameError.
Carrying our own copy removes the dependency on both versions rather than adding
an assumption about it. It is ~10 lines of pure list arithmetic over data we
already have in hand, and it has no reason to change.
Returns (dataset, has_children):
dataset -- the DEEPEST dataset whose mountpoint is a prefix of `directory`
has_children -- whether any OTHER dataset is mounted beneath `directory`
"""
datasets = [
dict(dataset, prefixlen=len(
os.path.dirname(os.path.commonprefix(
[dataset["properties"]["mountpoint"]["value"] + "/", directory + "/"]))
))
for dataset in datasets
if dataset["properties"]["mountpoint"]["value"] != "none"
]
dataset = sorted(
[
dataset
for dataset in datasets
if (directory + "/").startswith(dataset["properties"]["mountpoint"]["value"] + "/")
],
key=lambda dataset: dataset["prefixlen"],
reverse=True,
)[0]
return dataset, any(
(ds["properties"]["mountpoint"]["value"] + "/").startswith(directory + "/")
for ds in datasets
if ds != dataset
)
def delete_snapshot_tree(middleware, snapshot, logger=None):
"""Delete the parent snapshot AND every child created by ``zfs snapshot -r``.
``zfs.snapshot.delete`` is non-recursive by default and stock calls it with
@@ -381,7 +440,7 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None):
# through 252 sequential deletes leaves exactly the orphans this function
# exists to prevent.
try:
await middleware.call("zfs.snapshot.delete", snapshot, {"recursive": True})
middleware.call_sync("zfs.snapshot.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
@@ -398,7 +457,7 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None):
# our mounts are released -- which fails the recursive delete while the
# children survive. Sweep them by name.
try:
snaps = await middleware.call(
snaps = middleware.call_sync(
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
)
# An empty result means the tree is already gone -- delete nothing, and
@@ -415,7 +474,7 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None):
for name in names:
try:
await middleware.call("zfs.snapshot.delete", name)
middleware.call_sync("zfs.snapshot.delete", name)
except Exception as e: # noqa: BLE001 - already gone is fine
if logger:
logger.warning(
@@ -423,8 +482,8 @@ async def delete_snapshot_tree(middleware, snapshot, logger=None):
)
async def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
task_name, datasets, logger=None):
def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
task_name, datasets, logger=None):
"""Build a complete staging tree for `path` from the already-taken `snapshot`.
`snapshot` is a full ZFS snapshot name ("Tap@cloud_backup-5-2026...").
@@ -447,30 +506,30 @@ async def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint
staging_root = staging_root_for(task_name)
# A previous run may have crashed mid-flight; never build on top of that.
await middleware.run_in_thread(teardown, staging_root)
teardown(staging_root)
# ...and if it left a sidecar behind, that snapshot tree is still on disk and
# nothing else will ever reclaim it. Sweep it before we overwrite the record,
# or a single crashed run orphans 160+ snapshots permanently.
stale = await middleware.run_in_thread(_read_sidecar, staging_root)
stale = _read_sidecar(staging_root)
if stale and stale != snapshot:
if logger:
logger.warning(
"truecloud-patch: reclaiming snapshot tree from an earlier "
"interrupted run: %s", stale,
)
await delete_snapshot_tree(middleware, stale, logger=logger)
delete_snapshot_tree(middleware, stale, logger=logger)
# Record the snapshot BEFORE mounting anything, not after. middlewared can
# die at any point (this patch even schedules a restart at boot), and the
# sidecar is the only thing that survives it -- an in-process dict would take
# the sole record of a 160-snapshot tree with it. Writing it after apply_plan
# would leave exactly the crash window the sidecar exists to close.
await middleware.run_in_thread(_write_sidecar, staging_root, snapshot)
_write_sidecar(staging_root, snapshot)
try:
mounts, skipped = await middleware.run_in_thread(
plan_staging, base_dataset, base_mountpoint, path, snapshot_name,
mounts, skipped = plan_staging(
base_dataset, base_mountpoint, path, snapshot_name,
datasets, staging_root,
)
if logger:
@@ -479,11 +538,11 @@ async def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint
"truecloud-patch: not staging dataset %r: %s", name, reason
)
await middleware.run_in_thread(apply_plan, mounts)
await middleware.run_in_thread(verify_staged, mounts)
apply_plan(mounts)
verify_staged(mounts)
except Exception:
await middleware.run_in_thread(teardown, staging_root)
await middleware.run_in_thread(_remove_sidecar, staging_root)
teardown(staging_root)
_remove_sidecar(staging_root)
raise
if logger:
@@ -494,7 +553,7 @@ async def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint
return staging_root
async def cleanup_task(middleware, task_name, logger=None):
def cleanup_task(middleware, task_name, logger=None):
"""Tear down a task's staging tree and delete the snapshot it pinned.
Safe to call unconditionally: a no-op when the task was never staged.
@@ -505,13 +564,13 @@ async def cleanup_task(middleware, task_name, logger=None):
if snapshot is None and not os.path.isdir(staging_root):
return # never staged; nothing to do
errors = await middleware.run_in_thread(teardown, staging_root)
errors = teardown(staging_root)
if errors and logger:
for err in errors:
logger.warning("truecloud-patch: staging teardown: %s", err)
if snapshot is not None:
await delete_snapshot_tree(middleware, snapshot, logger=logger)
delete_snapshot_tree(middleware, snapshot, logger=logger)
_remove_sidecar(staging_root)
+51 -25
View File
@@ -14,14 +14,27 @@ import pytest
APPLY_SH = os.path.join(os.path.dirname(__file__), "..", "patch", "apply.sh")
#: Every block that is actually injected into a middlewared module.
#:
#: The three nested blocks come in two flavours. TrueNAS <= 25.10 has an ASYNC
#: cloud_backup path; TrueNAS 26 rewrote it synchronous. apply.sh reads which one is
#: installed and injects the matching wrapper -- an `async def` on 26 would hand
#: sync.py a coroutine where it unpacks a tuple, and a plain `def` on 25.10 would
#: block the event loop. Both flavours must therefore be valid Python, always.
EXPECTED_BLOCKS = {
"B2_BLOCK",
"RESTIC_BLOCK",
"SNAPSHOT_BLOCK",
"CRUD_BLOCK",
"SYNC_BLOCK",
"SNAPSHOT_ASYNC",
"SNAPSHOT_SYNC",
"CRUD_ASYNC",
"CRUD_SYNC",
"SYNC_ASYNC",
"SYNC_SYNC",
}
NESTED_BLOCKS = ["SNAPSHOT_ASYNC", "SNAPSHOT_SYNC", "CRUD_ASYNC", "CRUD_SYNC",
"SYNC_ASYNC", "SYNC_SYNC"]
def heredoc_source():
with open(APPLY_SH, encoding="utf-8") as fh:
@@ -32,18 +45,30 @@ def heredoc_source():
def extract_blocks():
"""The blocks as apply.sh actually builds them.
EVALUATED, not read off as string literals: each nested block is a CORE
concatenated with a flavour-specific wrapper, so reading only `ast.Constant`
would silently return nothing for them -- a green suite over blocks nobody
checked. Assignments that need the runtime (argv, imports) simply fail to
evaluate and are skipped.
"""
tree = ast.parse(heredoc_source())
blocks = {}
for node in ast.walk(tree):
if isinstance(node, ast.Assign):
for tgt in node.targets:
if (
isinstance(tgt, ast.Name)
and tgt.id.endswith("_BLOCK")
and isinstance(node.value, ast.Constant)
and isinstance(node.value.value, str)
):
blocks[tgt.id] = node.value.value
ns, blocks = {}, {}
for node in tree.body:
if not isinstance(node, ast.Assign):
continue
try:
value = eval( # noqa: S307 - our own shipped source, on purpose
compile(ast.Expression(node.value), "<blocks>", "eval"), {}, ns
)
except Exception:
continue
for tgt in node.targets:
if isinstance(tgt, ast.Name) and isinstance(value, str):
ns[tgt.id] = value
if tgt.id in EXPECTED_BLOCKS:
blocks[tgt.id] = value
return blocks
@@ -98,7 +123,7 @@ def test_injected_block_carries_the_idempotency_marker(name):
assert extract_blocks()[name].lstrip("\n").startswith("# TRUECLOUD_PATCH")
@pytest.mark.parametrize("name", ["SNAPSHOT_BLOCK", "CRUD_BLOCK", "SYNC_BLOCK"])
@pytest.mark.parametrize("name", NESTED_BLOCKS)
def test_nested_blocks_degrade_safely_without_the_module(name):
# If _truecloud_nested failed to install, every nested block must no-op.
# Critically this includes CRUD_BLOCK: relaxing the guard without the
@@ -119,20 +144,21 @@ class TestSnapshotLeak:
# On a staging failure, sync.py's `snapshot, local_path = await
# create_snapshot(...)` never completes, so its local `snapshot` stays
# None and its finally deletes nothing. We must sweep it ourselves.
block = extract_blocks()["SNAPSHOT_BLOCK"]
block = extract_blocks()["SNAPSHOT_ASYNC"]
assert "except Exception:" in block
assert "delete_snapshot_tree" in block
assert "raise" in block
def test_sync_block_cleans_up_on_every_path(self):
block = extract_blocks()["SYNC_BLOCK"]
block = extract_blocks()["SYNC_ASYNC"]
assert "finally:" in block
assert "cleanup_task" in block
def test_crud_block_is_scoped_to_cloud_backup():
# cloudsync has no staging teardown wired in, so its guard must stay.
assert '!= "cloud_backup"' in extract_blocks()["CRUD_BLOCK"]
for name in ("CRUD_ASYNC", "CRUD_SYNC"):
assert '!= "cloud_backup"' in extract_blocks()[name]
class TestIndependentModules:
@@ -220,7 +246,7 @@ class TestIndependentModules:
# find the string in our own patch and never detect native support.
sh = self._sh()
assert "split('\\n# TRUECLOUD_PATCH', 1)[0]" in sh
assert "no further nesting" in extract_blocks()["CRUD_BLOCK"], (
assert "no further nesting" in extract_blocks()["CRUD_ASYNC"], (
"if this ever stops being true, the probe comment is stale"
)
@@ -264,7 +290,7 @@ class TestOptIn:
# The guard-relaxing crud.py patch must be inside the enabled branch.
src = heredoc_source()
gate = src.index("if not nested_needed:")
crud = src.index("patch_file(crud_py, CRUD_BLOCK)")
crud = src.index("patch_file(crud_py, _crud_block)")
assert gate < crud, "crud.py patch must sit inside the opt-in branch"
def test_disabling_REVERTS_the_patch_rather_than_merely_skipping_it(self):
@@ -282,7 +308,7 @@ class TestOptIn:
assert "from mw_patch import patch_file, revert_nested" in src
gate = src.index("if not nested_needed:")
revert = src.index("reverted = revert_nested(")
patch = src.index("patch_file(crud_py, CRUD_BLOCK)")
patch = src.index("patch_file(crud_py, _crud_block)")
assert gate < revert < patch, "revert belongs in the not-needed branch"
def test_import_failure_skips_the_patch_rather_than_crashing(self):
@@ -302,9 +328,9 @@ def test_guard_is_relaxed_only_after_traversal_is_installed():
src = heredoc_source()
order = [
src.index("shutil.copyfile(nested_src, nested_dst)"),
src.index("patch_file(snapshot_py, SNAPSHOT_BLOCK)"),
src.index("patch_file(sync_path, SYNC_BLOCK)"),
src.index("patch_file(crud_py, CRUD_BLOCK)"),
src.index("patch_file(snapshot_py, _snapshot_block)"),
src.index("patch_file(sync_path, _sync_block)"),
src.index("patch_file(crud_py, _crud_block)"),
]
assert order == sorted(order), "crud.py must be patched last"
@@ -324,7 +350,7 @@ class TestWrappersDoNotHardcodeStockArity:
"""
def test_restic_backup_forwards_rather_than_naming_stock_params(self):
block = extract_blocks()["SYNC_BLOCK"]
block = extract_blocks()["SYNC_ASYNC"]
assert "async def restic_backup(middleware, job, cloud_backup, *args, **kwargs)" in block
assert "_tc_orig_restic_backup(middleware, job, cloud_backup, *args, **kwargs)" in block
+48 -8
View File
@@ -129,14 +129,11 @@ class TestFalseOkWouldBreakBackups:
}))
assert is_broken(r[PROVIDERS])
def test_async_to_sync_is_caught(self):
# THE TrueNAS 26 change.
def test_a_vanished_symbol_is_broken(self):
r = check_files(with_(**{
"plugins/cloud/snapshot.py":
'def create_snapshot(middleware, path, name="x"):\n return 1, 2\n',
"plugins/cloud/snapshot.py": "def something_else():\n pass\n",
}))
assert is_broken(r[NESTED])
assert "async def" in r[NESTED]["problems"][0]["detail"]
class TestFalseBrokenWouldDisableWorkingBoxes:
@@ -172,8 +169,7 @@ class TestFalseBrokenWouldDisableWorkingBoxes:
r = check_files(with_(**{
"plugins/cloud/snapshot.py": Unreadable("HTTP 429"),
"plugins/cloud_backup/sync.py":
"def restic_backup(middleware, job, cloud_backup, dry_run=False, "
"rate_limit=None):\n pass\n",
"async def restic_backup(job, middleware, cloud_backup):\n pass\n",
}))
assert is_broken(r[NESTED]), "unknown must not launder away a proven break"
@@ -189,7 +185,7 @@ class TestTheNativeVerdict:
r = check_files(with_(**{
"plugins/cloud/crud.py":
"class CloudTaskServiceMixin:\n"
" def _validate(self, app, verrors, name, data):\n"
" async def _validate(self, verrors, name):\n"
" verrors.add('x', 'no children allowed')\n",
}))
assert r[NESTED]["native"]
@@ -231,3 +227,47 @@ class TestUpdateReadmeCannotPublishAGuess:
with pytest.raises(Unreadable):
compat.update_readme(rows, path=str(readme))
assert "old" in readme.read_text(), "a blip must not repaint the matrix"
class TestAsyncFlavour:
"""TrueNAS <= 25.10 is async; 26 is synchronous. Both are supported -- apply.sh
injects the wrapper that matches. So asyncness is DETECTED, never assumed."""
def test_async_middleware_is_detected(self):
assert compat.async_flavour(loader(GOOD)) is True
def test_sync_middleware_is_detected(self):
sync = dict(GOOD)
sync["plugins/cloud/snapshot.py"] = (
'def create_snapshot(middleware, path, name="x"):\n return "s", "p"\n'
)
sync["plugins/cloud/crud.py"] = (
"class CloudTaskServiceMixin:\n"
" def _validate(self, app, verrors, name, data):\n"
" verrors.add('x', 'no further nesting')\n"
)
sync["plugins/cloud_backup/sync.py"] = (
"def restic_backup(middleware, job, cloud_backup, dry_run=False, "
"rate_limit=None):\n pass\n"
)
assert compat.async_flavour(loader(sync)) is False
def test_a_HALF_converted_middleware_is_refused(self):
# The dangerous middle. If iX converts create_snapshot but not restic_backup,
# there is no single wrapper flavour that works -- and guessing means either
# a coroutine unpacked as a tuple, or the event loop blocked. None means
# "do not patch"; apply.sh turns that into a skip, not a guess.
half = dict(GOOD)
half["plugins/cloud/snapshot.py"] = (
'def create_snapshot(middleware, path, name="x"):\n return "s", "p"\n'
)
assert compat.async_flavour(loader(half)) is None
def test_an_unreadable_source_refuses_rather_than_guesses(self):
broken = dict(GOOD)
broken["plugins/cloud_backup/sync.py"] = Unreadable("HTTP 429")
assert compat.async_flavour(loader(broken)) is None
def test_the_real_truenas_versions(self):
# Pinning the actual fact this whole port exists for.
assert compat.async_flavour(loader(GOOD)) is True
+60 -54
View File
@@ -12,7 +12,6 @@ Two rules are under test above all else:
on EVERY run.
"""
import asyncio
import os
import sys
@@ -199,12 +198,20 @@ class TestSnapshotTreeNames:
class FakeMiddleware:
"""middlewared as this module actually uses it: `call_sync`, from a thread.
The module is synchronous on purpose -- see the orchestration note in
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.
"""
def __init__(self, snapshots=None):
self.snapshots = list(snapshots or [])
self.calls = []
self.logger = None
async def call(self, method, *args):
def call_sync(self, method, *args):
self.calls.append((method, args))
if method == "zfs.snapshot.query":
return [{"name": n} for n in self.snapshots]
@@ -222,8 +229,35 @@ class FakeMiddleware:
return True
raise AssertionError(f"unexpected call {method}")
async def run_in_thread(self, fn, *args):
return fn(*args)
def stub_core(monkeypatch, tn, *, plan=None, order=None, plan_raises=None):
"""Replace the blocking core (plan/apply/verify/teardown) with recorders.
stage_nested calls these directly now, so they are patched by NAME rather than
intercepted at a `run_in_thread` boundary that no longer exists.
"""
def record(name, result):
def fn(*args, **kwargs):
if order is not None:
order.append(name)
if name == "plan_staging" and plan_raises is not None:
raise plan_raises
return result() if callable(result) else result
fn.__name__ = name
return fn
real_write = tn._write_sidecar
def write_sidecar(*args, **kwargs):
if order is not None:
order.append("_write_sidecar")
return real_write(*args, **kwargs)
monkeypatch.setattr(tn, "_write_sidecar", write_sidecar)
monkeypatch.setattr(tn, "plan_staging", record("plan_staging", plan or ([], [])))
monkeypatch.setattr(tn, "apply_plan", record("apply_plan", True))
monkeypatch.setattr(tn, "verify_staged", record("verify_staged", True))
monkeypatch.setattr(tn, "teardown", record("teardown", []))
class TestDeleteSnapshotTree:
@@ -231,20 +265,20 @@ class TestDeleteSnapshotTree:
mw = FakeMiddleware([
"Tap@snap", "Tap/apps@snap", "Tap/apps/lidarr@snap", "Tap@keepme",
])
asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
delete_snapshot_tree(mw, "Tap@snap")
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"])
asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
delete_snapshot_tree(mw, "Tap@snap")
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"])
asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
delete_snapshot_tree(mw, "Tap@snap")
assert mw.snapshots == []
deletes = [a for m, a in mw.calls if m == "zfs.snapshot.delete"]
assert len(deletes) == 1, "should be ONE recursive call, not one per snapshot"
@@ -255,20 +289,20 @@ class TestDeleteSnapshotTree:
def test_survives_recursive_and_query_failure_by_deleting_the_parent(self):
class Broken(FakeMiddleware):
async def call(self, method, *args):
def call_sync(self, method, *args):
if method == "zfs.snapshot.query":
raise RuntimeError("boom")
if method == "zfs.snapshot.delete" and len(args) > 1:
raise RuntimeError("recursive delete unavailable")
return await super().call(method, *args)
return super().call_sync(method, *args)
mw = Broken(["Tap@snap"])
asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
delete_snapshot_tree(mw, "Tap@snap")
assert mw.snapshots == []
def test_leaves_unrelated_snapshots_alone_when_the_tree_is_gone(self):
mw = FakeMiddleware(["Tap@unrelated"])
asyncio.run(delete_snapshot_tree(mw, "Tap@snap"))
delete_snapshot_tree(mw, "Tap@snap")
assert mw.snapshots == ["Tap@unrelated"]
@@ -281,20 +315,13 @@ class TestStageNestedOrdering:
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
order = []
stub_core(monkeypatch, tn, order=order,
plan=([("/src", str(tmp_path / "cloud_backup-5"))], []))
class Recorder(FakeMiddleware):
async def run_in_thread(self, fn, *args):
order.append(fn.__name__)
if fn.__name__ == "plan_staging":
return ([("/src", str(tmp_path / "cloud_backup-5"))], [])
if fn.__name__ in ("apply_plan", "verify_staged", "teardown"):
return [] if fn.__name__ == "teardown" else True
return fn(*args)
asyncio.run(tn.stage_nested(
Recorder(), "/mnt/Tap", "Tap@snap", "Tap", "/mnt/Tap",
tn.stage_nested(
FakeMiddleware(), "/mnt/Tap", "Tap@snap", "Tap", "/mnt/Tap",
"cloud_backup-5", DATASETS,
))
)
assert order.index("_write_sidecar") < order.index("apply_plan")
@@ -312,28 +339,12 @@ class TestStageNestedOrdering:
fh.write("Tap@old-crashed-run")
mw = FakeMiddleware(["Tap@old-crashed-run", "Tap/apps@old-crashed-run"])
stub_core(monkeypatch, tn, plan=([("/src", root)], []))
class Stub(FakeMiddleware):
def __init__(self, inner):
super().__init__()
self.inner = inner
async def call(self, method, *args):
return await self.inner.call(method, *args)
async def run_in_thread(self, fn, *args):
if fn.__name__ == "plan_staging":
return ([("/src", root)], [])
if fn.__name__ == "teardown":
return []
if fn.__name__ in ("apply_plan", "verify_staged"):
return True
return fn(*args)
asyncio.run(tn.stage_nested(
Stub(mw), "/mnt/Tap", "Tap@new", "Tap", "/mnt/Tap",
tn.stage_nested(
mw, "/mnt/Tap", "Tap@new", "Tap", "/mnt/Tap",
"cloud_backup-5", DATASETS,
))
)
assert mw.snapshots == [], "the crashed run's snapshot tree must be reclaimed"
@@ -342,18 +353,13 @@ class TestStageNestedOrdering:
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
root = tn.staging_root_for("cloud_backup-5")
class Failing(FakeMiddleware):
async def run_in_thread(self, fn, *args):
if fn.__name__ == "plan_staging":
raise StagingError("boom")
return fn(*args)
stub_core(monkeypatch, tn, plan_raises=StagingError("boom"))
with pytest.raises(StagingError):
asyncio.run(tn.stage_nested(
Failing(), "/mnt/Tap", "Tap@snap", "Tap", "/mnt/Tap",
tn.stage_nested(
FakeMiddleware(), "/mnt/Tap", "Tap@snap", "Tap", "/mnt/Tap",
"cloud_backup-5", DATASETS,
))
)
assert not os.path.exists(sidecar_for(root))
@@ -374,7 +380,7 @@ class TestCleanupTask:
mw = FakeMiddleware(["Tap@snap", "Tap/apps@snap"])
monkeypatch.setattr(tn, "teardown", lambda *_a, **_k: [])
asyncio.run(cleanup_task(mw, "cloud_backup-5"))
cleanup_task(mw, "cloud_backup-5")
assert mw.snapshots == []
assert not os.path.exists(sidecar_for(root))
@@ -384,7 +390,7 @@ class TestCleanupTask:
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path / "nope"))
mw = FakeMiddleware(["Tap@snap"])
asyncio.run(cleanup_task(mw, "cloud_backup-5"))
cleanup_task(mw, "cloud_backup-5")
assert mw.calls == []
assert mw.snapshots == ["Tap@snap"]
+56 -10
View File
@@ -105,19 +105,22 @@ ASSUMPTIONS = [
),
# ── nested snapshots. Every block here is an async wrapper. ───────────────
# is_async is deliberately NOT asserted on these three. The patch now injects an
# async OR a sync wrapper to match whichever the installed middleware declares
# (TrueNAS <= 25.10 is async; 26 rewrote them synchronous), so asyncness is a
# thing to DETECT, not a thing to require -- see async_flavour(). What must still
# hold is the shape: same name, same leading positional parameters.
Assumption(
"create-snapshot", NESTED, "plugins/cloud/snapshot.py", "create_snapshot",
is_async=True, params=["middleware", "path", "name"],
why="SNAPSHOT_BLOCK replaces it with `async def` that AWAITS the original "
"and returns (snapshot, staging_root). TrueNAS 26 made it synchronous: "
"the wrapper would return a coroutine that sync.py unpacks as a tuple",
params=["middleware", "path", "name"],
why="SNAPSHOT_BLOCK wraps it and returns (snapshot, staging_root) instead of "
"(snapshot, snap_path)",
),
Assumption(
"crud-mixin-validate", NESTED, "plugins/cloud/crud.py",
"CloudTaskServiceMixin._validate",
kind="method", is_async=True, params=["self", "app", "verrors", "name", "data"],
why="CRUD_BLOCK replaces it with `async def` that AWAITS the original, to "
"drop the no-further-nesting error",
kind="method", params=["self", "app", "verrors", "name", "data"],
why="CRUD_BLOCK wraps it to drop the no-further-nesting error",
),
Assumption(
# SYNC_BLOCK's wrapper is (middleware, job, cloud_backup, *args, **kwargs) and
@@ -125,10 +128,9 @@ ASSUMPTIONS = [
# 25.04 have `(…, dry_run)`, 25.10 added `rate_limit`. Only the leading three
# are named by the patch, so only they have to hold.
"restic-backup", NESTED, "plugins/cloud_backup/sync.py", "restic_backup",
is_async=True, forwards=True,
forwards=True,
params=["middleware", "job", "cloud_backup"],
why="SYNC_BLOCK replaces it with `async def` that AWAITS the original, to "
"tear down bind mounts in a finally",
why="SYNC_BLOCK wraps it to tear down bind mounts in a finally",
),
]
@@ -462,6 +464,50 @@ def check(loader, modules=None) -> dict:
return out
#: The three stock symbols the nested module wraps. TrueNAS <= 25.10 declares them
#: `async def`; TrueNAS 26 rewrote them synchronous. apply.sh injects the wrapper
#: that matches, so this is the question it has to answer at every boot.
NESTED_WRAPPED = [
("plugins/cloud/snapshot.py", "create_snapshot"),
("plugins/cloud/crud.py", "CloudTaskServiceMixin._validate"),
("plugins/cloud_backup/sync.py", "restic_backup"),
]
def async_flavour(loader) -> bool | None:
"""Is the installed cloud_backup path async? True, False, or None if unclear.
None means "do not patch": either a symbol is missing, or -- the case worth
naming -- the three DISAGREE. A middleware caught half-converted is one this
patch has never seen, and guessing a flavour there means injecting an `async def`
that a synchronous caller unpacks as a tuple. Declining costs a feature; guessing
costs a backup.
"""
flavours = set()
for path, symbol in NESTED_WRAPPED:
try:
src = loader(path)
except Unreadable:
return None
if src is None:
return None
try:
tree = ast.parse(_stock(src))
except SyntaxError:
return None
node = _find(tree, symbol)
if not isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef):
return None
flavours.add(isinstance(node, ast.AsyncFunctionDef))
return flavours.pop() if len(flavours) == 1 else None
def async_flavour_tree(root: str) -> bool | None:
return async_flavour(lambda p: _read(root, p))
def check_ref(ref: str, modules=None) -> dict:
return check(lambda p: _fetch(ref, p), modules)