Support ZFS snapshots on datasets with child datasets

TrueCloud Backup's "Take Snapshot" option is rejected on any path containing
child datasets:

  This option is only available for datasets that have no further nesting

That excludes every pool running Apps, where each app is its own dataset and
often has config/pgdata children. Without the option the backup reads live
files, so databases are captured mid-write and an app that continuously
rewrites its files can stall a run as restic chases a moving target.

The stock guard is correct and must not simply be removed. create_snapshot()
already takes a recursive ZFS snapshot, but points the backup tool at the
parent dataset's .zfs/snapshot/, and ZFS does not expose child datasets there:

  /mnt/Tap/.zfs/snapshot/<snap>/apps/                -> 0 entries
  /mnt/Tap/apps/lidarr/config/.zfs/snapshot/<snap>/  -> the real data

Deleting the check would make restic walk a near-empty tree, report success,
and upload almost nothing.

Implement the missing traversal instead. After the recursive snapshot is taken,
each descendant dataset's own .zfs/snapshot/<snap> is bind-mounted into a
staging tree mirroring the original layout, and the backup tool is pointed at
the staging root. The guard is relaxed only after that machinery is in place.

Safety properties:
- staging failure aborts the backup; a partial tree is never handed to restic
- a post-mount pass asserts every target is a mountpoint and the root is
  non-empty, so this cannot regress into the empty backup it exists to prevent
- apply.sh patches crud.py last, so a partial failure leaves the guard intact
  rather than exposing "guard removed, traversal missing"
- every injected block no-ops when _truecloud_nested is absent
- unmountable/locked datasets are skipped and reported, never dropped silently
- scoped to cloud_backup; cloudsync has no teardown wired in, so its guard stays

The staging root is stable per task, so restic can find its parent snapshot
between runs; stock's timestamped .zfs path changes every run and forces a
full re-scan.

Add CI (shellcheck, bash -n, ruff, pytest on 3.11-3.13), including tests that
compile the *_BLOCK strings, which are Python source appended to live
middlewared modules and were previously unchecked.

Also: sync stale version strings, untrack a committed .pyc, gitignore
__pycache__.
This commit is contained in:
flan
2026-07-12 21:20:22 +00:00
parent 4ded8cff3d
commit a572eb2164
14 changed files with 1113 additions and 15 deletions
+97
View File
@@ -0,0 +1,97 @@
"""The *_BLOCK strings in apply.sh are Python source injected into middleware.
A syntax error in one of them would be appended to a live middlewared module and
break the box at boot. They are string literals, so nothing type-checks them --
these tests do.
"""
import ast
import os
import re
import pytest
APPLY_SH = os.path.join(os.path.dirname(__file__), "..", "patch", "apply.sh")
EXPECTED_BLOCKS = {
"B2_BLOCK",
"RESTIC_BLOCK",
"SNAPSHOT_BLOCK",
"CRUD_BLOCK",
"SYNC_BLOCK",
}
def heredoc_source():
with open(APPLY_SH, encoding="utf-8") as fh:
src = fh.read()
m = re.search(r"<< 'PYEOF'\n(.*?)\nPYEOF", src, re.S)
assert m, "could not find the PYEOF heredoc in apply.sh"
return m.group(1)
def extract_blocks():
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
return blocks
def test_heredoc_itself_compiles():
compile(heredoc_source(), "apply.sh:PYEOF", "exec")
def test_all_expected_blocks_present():
assert set(extract_blocks()) == EXPECTED_BLOCKS
@pytest.mark.parametrize("name", sorted(EXPECTED_BLOCKS))
def test_injected_block_is_valid_python(name):
block = extract_blocks()[name]
compile(block, f"apply.sh:{name}", "exec")
@pytest.mark.parametrize("name", sorted(EXPECTED_BLOCKS))
def test_injected_block_carries_the_idempotency_marker(name):
# patch_file() truncates each target file at "\n# TRUECLOUD_PATCH" before
# re-appending, so every block must start with that marker or repeated runs
# would stack duplicate copies into the middleware module.
assert extract_blocks()[name].lstrip("\n").startswith("# TRUECLOUD_PATCH")
@pytest.mark.parametrize("name", ["SNAPSHOT_BLOCK", "CRUD_BLOCK", "SYNC_BLOCK"])
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
# traversal in place would mean silently-empty backups.
block = extract_blocks()[name]
assert "_tc_nested = None" in block
assert "if _tc_nested is not None:" 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"]
def test_guard_is_relaxed_only_after_traversal_is_installed():
# Ordering in apply.sh is a safety property: copy module -> patch snapshot.py
# -> patch sync.py -> patch crud.py. crud.py (which unlocks the feature) must
# come last, so a partial failure never leaves "guard removed, traversal gone".
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)"),
]
assert order == sorted(order), "crud.py must be patched last"