Commit Graph
6 Commits
Author SHA1 Message Date
flan c2e1976659 Merge nested-dataset snapshot support (v0.3.0)
Adds a second, opt-in module to the patch: TrueCloud Backup's "Take Snapshot"
option now works on datasets that have child datasets — i.e. every pool running
Apps, where each app is its own dataset and stock refuses the config with
"This option is only available for datasets that have no further nesting".

Stock's guard is correct and is not merely deleted. create_snapshot() already
takes a recursive ZFS snapshot but points restic at the PARENT dataset's
.zfs/snapshot/, where ZFS does not expose child datasets — so removing the check
would make restic walk a near-empty tree, report SUCCESS, and upload almost
nothing. The missing traversal is implemented instead: each descendant's own
.zfs/snapshot is bind-mounted into a staging tree and restic is pointed at that.
The guard is relaxed only after the machinery is in place.

The patch is now two modules (providers, nested) which retire independently as
TrueNAS ships each capability natively; the kill switch fires only when both are
done.

Also adds CI (shellcheck, bash -n, ruff, pytest on 3.11-3.13; 67 tests),
including a pass that compile()s the Python source this patch appends into live
middlewared modules — previously unchecked, and a syntax error there breaks the
box at boot.

Nested snapshots are OFF by default (install.sh --enable-nested-snapshots).
The mount --bind staging step has not yet been exercised by a live backup run.
2026-07-12 22:17:48 +00:00
flan eca6eb3f3b Retire the two modules independently; calm the README down
The native-support check looked only for native B2 restic support and, on
finding it, set the kill switch and disabled the whole patch. That was fine when
providers were the only thing here. With nested-dataset snapshots in the patch it
is wrong: TrueNAS is likely to ship one capability long before the other, and a
single all-or-nothing kill switch would silently take a still-needed module down
with the superseded one.

apply.sh now treats the patch as two independent modules:

  providers  b2.py + restic.py + the UI credential dropdown
             native when B2RcloneRemote carries a real get_restic_config()

  nested     plugins/cloud/{snapshot,crud}.py + cloud_backup/sync.py (opt-in)
             native when the "no further nesting" validation is gone from
             plugins/cloud/crud.py

Each is detected and skipped on its own. The kill switch fires only once BOTH are
done (native, or nested was never enabled). The UI patch belongs to providers and
is skipped with it. The deferred middlewared restart now fires when ANY
still-needed module landed -- keying it off providers alone would have left a
freshly-patched nested module on disk and never loaded on a native-B2 box.
hook_status.json reports each module with an active flag and a reason.

README: dropped the warning boxes and the disclaimer's fear-bulleting in favour
of plain statements, documented the two-module design and the per-module
auto-disable, and added a Development section disclosing AI assistance. The one
caveat kept, as a plain sentence rather than a banner: the mount --bind staging
step has not yet been exercised by a live backup run.

67 tests, ruff and shellcheck clean.
2026-07-12 22:17:37 +00:00
flan a80de88078 Distinguish a missing snapshot from an unreadable one
os.path.isdir() returns False both when a snapshot directory does not exist and
when it cannot be stat'd. Staging aborted either way -- correct -- but reported
every case as "has no snapshot", which sends you hunting for a snapshot that is
sitting right there.

Found while dry-running the planner against a real recursive snapshot of Tap:
running as a non-root user, /mnt/Tap/apps/paperless/data is mode 0700 and the
probe reported "has no snapshot" when `zfs list` showed the snapshot present.
Middleware runs as root so this would not fire in production, but a backup
system must not misreport why it failed.

plan_staging now takes a probe() that classifies the path as ok / missing /
unreadable, and the error names which.

Dry-run results against Tap (250 datasets, real `zfs snapshot -r`):
- 170 mounted filesystems under /mnt/Tap/, and the plan produces exactly 170
  descendant mounts -- no omissions
- 18 legacy-mountpoint datasets reported as skipped, never dropped silently
- Tap/ix-apps mounts at /mnt/.ix-apps, correctly outside the backup path
- parent snapshot exposes 0 entries under /apps; the staged sources expose 71,
  and lidarr/config resolves with lidarr.db present
- snapshot_tree_names() identifies all 250; deleting only the parent (what stock
  does) leaves 249 orphans, and the sweep clears them
2026-07-12 22:04:56 +00:00
flan bb26edf351 Make nested snapshots opt-in; fix snapshot leaks found in audit
Opt-in
------
Nested-dataset snapshot support changes how backups read their source data, so
it is now off by default and gated behind a marker file:

  install.sh --enable-nested-snapshots
  install.sh --disable-nested-snapshots

With neither flag install.sh preserves the current setting, so a routine
`git pull && bash install.sh` can never silently flip it. When disabled,
apply.sh skips the patch entirely and the stock guard remains. uninstall.sh
tears down staging mounts and removes the marker.

Snapshot lifecycle
------------------
zfs.snapshot.delete defaults to recursive=False and stock restic_backup() calls
it with no options. Stock is safe only because its validation means recursive is
never True in the field. Enabling nested datasets makes recursive snapshots real:
the parent then has one child snapshot per descendant dataset (160+ on an Apps
pool), so stock's delete would orphan every child on EVERY successful run.

The patch now owns the lifecycle end to end:

- delete_snapshot_tree() sweeps the parent and all children, and is idempotent
  against stock's finally winning the race once our mounts are released
- on a staging failure the tree is deleted here, because sync.py never completes
  `snapshot, local_path = await create_snapshot(...)` and so its finally deletes
  nothing at all
- the snapshot is recorded in a sidecar file before anything is mounted, so a
  middlewared restart mid-backup cannot orphan it
- a crashed run's snapshot tree is reclaimed on the next run instead of being
  overwritten and leaked

Silent-omission fix
-------------------
The dataset list is now enumerated AFTER the snapshot. Read beforehand it could
miss a dataset created in the gap, which the recursive snapshot would capture but
the staging plan would not -- silently omitting its data. Read afterwards, an
unsnapshotted dataset trips the staging check and fails the run loudly.

Also from the audit
-------------------
- plan_staging scopes by dataset name, so skipped-dataset warnings no longer
  include every mountpoint-less dataset on the box, which buried the ones that
  matter
- staging_root_for rejects "." / ".." components that would escape the staging
  base, and resolves STAGING_BASE at call time rather than freezing it into a
  default argument
- uninstall.sh no longer `rm -rf`s a tree that may still contain live bind
  mounts, and unmounts by path depth rather than string length
- apply_plan takes an injectable isdir; verify_staged drops an unused parameter
- pin the shellcheck action instead of tracking @master

61 tests, ruff and shellcheck clean.
2026-07-12 21:52:09 +00:00
flan a572eb2164 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__.
2026-07-12 21:20:22 +00:00
flan da1be97377 create_task.py: migrate REST /api/v2.0 -> midclt (removed in TrueNAS 26.04)
The tool created cloud_backup tasks via POST /api/v2.0/cloud_backup, which is deprecated
and removed in TrueNAS 26.04. It now calls the middleware directly with midclt
(cloudsync.credentials.query / cloud_backup.query / cloud_backup.create), so it runs on
the TrueNAS host with no host address or API key. --host/--api-key/--insecure are kept
accepted-but-ignored for compatibility. Dropped the ssl/urllib HTTP client. v0.2.0.
2026-07-08 15:09:38 -04:00