diff --git a/CHANGELOG.md b/CHANGELOG.md index cda9e3e..859a99c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,28 @@ ### Changed +- **The patch is now two independent modules, and each retires on its own.** + Previously the native-support check looked only for native B2 restic support + and, on finding it, set the kill switch and disabled *everything*. With a + second capability in the patch that would silently take a still-needed module + down with the superseded one — TrueNAS is likely to ship one of these long + before the other. + + `apply.sh` now detects each separately (`providers`: does `B2RcloneRemote` + carry a real `get_restic_config()`; `nested`: is the *"no further nesting"* + validation still in `plugins/cloud/crud.py`), skips just the superseded one, + and only sets the kill switch once **both** are done. 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 rewritten to be less alarmist: dropped the warning boxes and the + disclaimer's fear-bulleting in favour of plain statements, and documented the + two-module design. The one caveat kept as a plain sentence: the `mount --bind` + staging step has not yet been exercised by a live backup run. + - Version strings in `install.sh`, `uninstall.sh`, and `recover.sh` were stale at `0.0.4`; all scripts now report the same version. - `patch_ui.py`: replaced a `try`/`except`/`pass` with `contextlib.suppress` diff --git a/README.md b/README.md index 5c69057..56f11b2 100644 --- a/README.md +++ b/README.md @@ -27,31 +27,41 @@ costs a fraction of the new Storj price. --- -## ⚠ Disclaimer — please read before installing +## Before you install -**This project is unofficial, unsupported, and not affiliated with iXsystems -or the TrueNAS project in any way.** +This project is unofficial and not affiliated with iXsystems. A few things worth +knowing: -By installing this patch you accept the following: +- It targets **internal middleware APIs** with no stability contract, so a + TrueNAS update can break it. Every patch is fail-safe: if it can't apply, + middlewared starts normally and the reason is logged to `apply.log`. Check the + log after an update. +- If you file a TrueNAS bug report, **remove the patch first** and reproduce on a + stock system. +- **Test your restores.** That is true of any backup, but it matters more here — + see [Verifying it works](#verifying-it-works). +- Provided as-is, no warranty. See LICENSE. -- **Unsupported configuration.** TrueNAS support staff are not obligated to - help with any issue on a system running this patch. If you file a bug report, - remove the patch first and reproduce the issue on an unmodified system. +The patch is two independent modules — **providers** (B2/S3) and **nested** +(snapshots on nested datasets) — and each retires on its own once TrueNAS ships +that capability natively. See [Native support](#if-truenas-adds-native-support). -- **May break on TrueNAS updates.** The patch targets internal middleware APIs - that are not part of any public contract. They can change at any time. When - they do, the patch silently degrades to Storj-only behaviour rather than - breaking TrueNAS — but you should check the log after each update. +## Development -- **Your backups are your responsibility.** Verify that your backup jobs - complete successfully and that restores work before relying on them for - disaster recovery. +Parts of this project were written with AI assistance (Claude). All of it is +reviewed and tested before release; the test suite and CI exist in large part to +make that review meaningful. Bugs are mine. -- **No warranty.** This software is provided as-is. See the LICENSE file. +```bash +pip install pytest ruff +ruff check patch tests +pytest tests +``` -If TrueNAS adds native B2 or S3 support to TrueCloud Backup, the patch -detects it at boot, disables itself, and tells you to run `uninstall.sh` — -see [Native support](#if-truenas-adds-native-support) below. +CI runs shellcheck, `bash -n`, ruff, and pytest on Python 3.11–3.13. The tests +include a pass that `compile()`s the `*_BLOCK` strings in `patch/apply.sh` — +those are Python source appended into live `middlewared` modules, so a syntax +error there would break the box at boot. --- @@ -77,29 +87,35 @@ restarting. Expect one middlewared restart shortly after every boot; the UI and API are briefly unavailable while it happens, and running services are not affected. -| Layer | What changes | Technique | +| Module | What changes | Technique | |---|---|---| -| **Backend** | `B2RcloneRemote` gains `get_restic_config()` — skipped automatically if TrueNAS already provides one on the class. `restic.py` URL builder is fixed: strips the stray leading slash and converts the slash separator to a colon (`b2:bucket:path`), which is the format restic 0.16.x expects. URL wrapper is a no-op if the URL is already correctly formed. | File patch applied inside the overlayfs upper layer | -| **Nested snapshots** | `_truecloud_nested.py` is installed into `plugins/cloud/`, and `plugins/cloud/{snapshot,crud}.py` + `plugins/cloud_backup/sync.py` are patched so `snapshot = true` works on a dataset that has child datasets. See [below](#nested-dataset-snapshots). | New module + file patches inside the overlayfs upper layer | -| **UI** | The Angular bundle's `filterByProviders` binding is widened from `["STORJ_IX"]` to `["STORJ_IX","S3","B2"]` | In-place text replacement in the compiled JS chunk; original is backed up before patching | +| **providers** | `B2RcloneRemote` gains `get_restic_config()` — skipped automatically if TrueNAS already provides one on the class. `restic.py` URL builder is fixed: strips the stray leading slash and converts the slash separator to a colon (`b2:bucket:path`), which is the format restic 0.16.x expects. URL wrapper is a no-op if the URL is already correctly formed. | File patch applied inside the overlayfs upper layer | +| **providers** (UI) | The Angular bundle's `filterByProviders` binding is widened from `["STORJ_IX"]` to `["STORJ_IX","S3","B2"]` | In-place text replacement in the compiled JS chunk; original is backed up before patching | +| **nested** (opt-in) | `_truecloud_nested.py` is installed into `plugins/cloud/`, and `plugins/cloud/{snapshot,crud}.py` + `plugins/cloud_backup/sync.py` are patched so `snapshot = true` works on a dataset that has child datasets. See [below](#nested-dataset-snapshots). | New module + file patches inside the overlayfs upper layer | All changes are **fail-safe**: if a patch cannot be applied (e.g. TrueNAS -restructured the relevant code), middlewared starts normally with Storj-only -support and the reason is logged to `apply.log` in your repo root. +restructured the relevant code), middlewared starts normally, the affected module +is simply inactive, and the reason is logged to `apply.log` in your repo root. The +two modules are independent — one failing or going native does not disable the +other. ## Nested-dataset snapshots -> **Opt-in, and off by default.** This feature changes how backups read their -> source data, so it is never enabled implicitly: -> -> ```bash -> bash install.sh --enable-nested-snapshots # turn it on -> bash install.sh --disable-nested-snapshots # turn it back off -> ``` -> -> With neither flag, `install.sh` leaves the current setting alone — so a -> `git pull && bash install.sh` can never silently flip it. The B2/S3 provider -> patch is unaffected either way. +**Opt-in, off by default.** It changes how backups read their source data, so it +is never enabled implicitly: + +```bash +bash install.sh --enable-nested-snapshots +bash install.sh --disable-nested-snapshots +``` + +With neither flag `install.sh` leaves the setting alone, so `git pull && bash +install.sh` won't flip it. The providers module is unaffected either way. + +The planner and the snapshot lifecycle have been validated against a real +250-dataset pool. The `mount --bind` staging step has not yet been exercised by a +live backup run, so confirm your first backup actually contains child-dataset +data before relying on it — see [Verifying it works](#verifying-it-works). TrueCloud Backup's **Take Snapshot** option makes restic read from a frozen ZFS snapshot instead of live files. Without it the backup reads data *while apps are @@ -118,7 +134,7 @@ That rules out **any pool running Apps** — every app is its own dataset, usual with `config`/`pgdata` children of its own. On a typical box that is 100+ nested datasets, so the feature is effectively unusable exactly where it matters most. -### Why stock refuses — and why you must not simply delete the check +### Why stock refuses The guard is **correct**. `plugins/cloud/snapshot.py` already takes a *recursive* ZFS snapshot — but it then points restic at the **parent** dataset's @@ -130,14 +146,13 @@ through a parent's snapshot directory: /mnt/Tap/apps/lidarr/config/.zfs/snapshot// -> the real data ``` -So if you just remove the validation, restic walks a **near-empty tree, reports -SUCCESS, and uploads almost nothing.** You get a green backup job protecting no -data. iX gate the config rather than ship a backup that lies about succeeding. +So if you just remove the validation, restic walks a near-empty tree, reports +SUCCESS, and uploads almost nothing — a green backup job protecting no data. iX +gate the config rather than ship a backup that lies about succeeding. -> **⚠ If you are tempted to patch this yourself: deleting those four lines in -> `plugins/cloud/crud.py` is the obvious move and it is catastrophic.** The -> guard is load-bearing. It must be *replaced* with a working traversal, never -> merely removed. +That is worth spelling out, because deleting those four lines in +`plugins/cloud/crud.py` is the obvious "fix" and it is the wrong one. The guard +is load-bearing: it has to be *replaced* with a working traversal, not removed. ### What this patch does instead @@ -171,10 +186,69 @@ its parent snapshot between runs. Stock's `.zfs/snapshot/-/` path changes every run, defeating restic's parent detection and forcing a full re-scan each time. -**Expected log noise:** stock `restic_backup()` deletes the ZFS snapshot in its -own `finally`, which fails with `EBUSY` while the staging mounts pin it. You will -see one benign `Error deleting snapshot ...` warning per run; the patch then -unmounts and deletes the snapshot for real. +### 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 a *recursive* snapshot never actually happens in the field. +Enabling nested datasets makes them real: on a 250-dataset pool, +`zfs snapshot -r` creates **250 snapshots**, and stock's delete removes only the +parent — orphaning **249 on every successful run** (measured, not theorised). + +So the patch owns the whole lifecycle: + +- **Sweeps the parent and every child**, and is idempotent against stock's + `finally` winning the race once the mounts are released. +- **Records the snapshot in a sidecar file before mounting anything**, so a + middlewared restart mid-backup cannot orphan the tree (this patch *schedules* + a restart at boot, so that is not hypothetical). +- **Reclaims the tree left by a crashed run** instead of overwriting the record. +- **Deletes the tree when staging fails** — sync.py's own `finally` deletes + *nothing* in that case, because its `snapshot` local never gets assigned. +- **Enumerates datasets *after* the snapshot, never before.** A list read + beforehand can miss a dataset created in the gap, which the recursive snapshot + *would* capture but the staging plan would not — a silent omission. + +**Expected log noise:** stock's delete fails with `EBUSY` while the staging +mounts pin the snapshot. You will see one benign `Error deleting snapshot ...` +warning per run; the patch then unmounts and deletes the tree for real. + +### Verifying it works + +This feature exists because a backup can report SUCCESS while containing +nothing, so check the contents rather than the exit status: + +```bash +# 1. Does the restic snapshot actually contain child-dataset data? +# Pick a path that lives in a CHILD dataset (e.g. an app's config). +midclt call cloud_backup.list_snapshots | head + +# 2. List a child-dataset path inside the newest restic snapshot. +# If this is empty, the staging tree did not work and you are backing up NOTHING. +midclt call cloud_backup.list_snapshot_directory "" "/apps/lidarr/config" +``` + +You should see the app's real files (`lidarr.db`, `config.xml`, …). An empty +listing means the child datasets were not staged; disable the feature and open an +issue. + +```bash +# 3. No snapshots may be left behind after a run. +zfs list -t snapshot -r | grep -c cloud_backup- # expect 0 between runs + +# 4. No staging mounts may be left behind. +mount | grep truecloud-nested # expect no output +``` + +### Troubleshooting + +| Symptom | Cause | +|---|---| +| `This option is only available for datasets that have no further nesting` | Feature not enabled. Run `install.sh --enable-nested-snapshots`, then restart middlewared. | +| Backup fails: `dataset '…' has no snapshot '…'; refusing to back up an incomplete tree` | Working as designed — a descendant dataset was not covered by the snapshot. The backup is refused rather than silently omitting that data. | +| Backup fails: `snapshot '…' cannot be read (Permission denied)` | The snapshot exists but is unreadable. Middleware runs as root, so this indicates a real permissions problem, not a missing snapshot. | +| `cloud_backup-*` snapshots accumulating | The sweep is not running. Check `apply.log` for the nested patch applying, and confirm `sync.py` carries the `TRUECLOUD_PATCH` block. | +| Stale mounts under `/run/truecloud-nested` | A crashed run. The next run tears them down; `uninstall.sh` also cleans them. | ## Supported providers after patching @@ -332,25 +406,35 @@ and restores the original UI bundle from backup. ## If TrueNAS adds native support -`apply.sh` checks at every boot whether TrueNAS has shipped native B2 restic -support (by inspecting `B2RcloneRemote.__dict__`). If it has: +The patch is **two independent modules**, and each retires on its own — TrueNAS +is likely to ship one of these natively long before the other, and a module +going native must not take the other one down with it. -1. The kill switch (`disabled` file) is set — no patching on any future boot. -2. Any active overlays are unmounted immediately. -3. The following message is written to `apply.log`: +| Module | What it does | Detected as native when | +|---|---|---| +| **providers** | B2/S3 credentials for TrueCloud Backup (`b2.py`, `restic.py`, UI dropdown) | `B2RcloneRemote` carries a real `get_restic_config()` | +| **nested** | Snapshots on datasets with child datasets (`plugins/cloud/*`) | the *"no further nesting"* validation is gone from `plugins/cloud/crud.py` | -``` -NOTICE: TrueNAS now provides native B2 restic support — truecloud-patch is no longer needed. -NOTICE: Setting kill switch; patching will be skipped on all future boots. -NOTICE: Run the following to fully remove the patch: -NOTICE: bash /mnt/tank/truenas-truecloud-patch/uninstall.sh -``` +At every boot `apply.sh` checks both: + +- **One module goes native** → that module is skipped and logged; the other keeps + working, and the patch stays installed. +- **Both are done** (native, or nested was never enabled) → the kill switch + (`disabled` file) is set, overlays are unmounted, and `apply.log` tells you to + run `uninstall.sh`. + +So on a box using only the provider patch, native B2 support retires the whole +thing as before. On a box that also uses nested snapshots, native B2 support +retires *just* that half. Check the log after any TrueNAS update: ```bash -cat /mnt/tank/truenas-truecloud-patch/apply.log | tail -20 +tail -20 /mnt/tank/truenas-truecloud-patch/apply.log ``` +`hook_status.json` reports each module separately (`module.providers`, +`module.nested_snapshots`) with an `active` flag and a reason. + **Scenarios where the auto-detect may not fire** (manual check needed): | Scenario | What happens | Action | diff --git a/patch/apply.sh b/patch/apply.sh index 96b9907..59a408c 100755 --- a/patch/apply.sh +++ b/patch/apply.sh @@ -109,10 +109,20 @@ echo "Using Python: $PYTHON" # Combines what were previously four separate Python invocations into one to # avoid repeated interpreter startup overhead under the PREINIT timeout budget. +# The patch has two independent modules, and each retires on its own: +# +# providers — B2/S3 credentials for TrueCloud Backup (b2.py, restic.py, UI) +# nested — snapshots on datasets that have child datasets (cloud/*.py) +# +# TrueNAS may well ship one natively long before the other, so a single +# all-or-nothing kill switch would silently take a still-needed module down with +# the superseded one. Each module is detected separately and skipped on its own; +# the global kill switch fires only once BOTH are native. + _tc_info=$("$PYTHON" -c " import inspect, os, sys -result = {'native': 'no', 'site_pkg': '', 'mw_dir': ''} +result = {'native_b2': 'no', 'native_nested': 'no', 'site_pkg': '', 'mw_dir': ''} try: import middlewared @@ -126,6 +136,7 @@ except ImportError: except Exception: pass +# providers: does B2RcloneRemote already carry a real get_restic_config()? try: import middlewared.rclone.remote.b2 as _b2_mod from middlewared.rclone.remote.b2 import B2RcloneRemote @@ -137,21 +148,59 @@ try: except (OSError, TypeError): method_src = '' if 'NotImplementedError' not in method_src: - result['native'] = 'yes' + result['native_b2'] = 'yes' except Exception: pass -print(result['native']) +# nested: stock gates nested datasets with a validation in plugins/cloud/crud.py. +# We only ever append to that file, so the stock text survives our patch -- if the +# guard is gone, iX removed it, which means they implemented the traversal. +# If the file cannot be read we assume 'no' and keep patching: worst case the +# patch declines to apply and the option simply stays unavailable. +try: + crud = os.path.join(result['mw_dir'], 'plugins', 'cloud', 'crud.py') + with open(crud, encoding='utf-8', errors='replace') as fh: + if 'no further nesting' not in fh.read(): + result['native_nested'] = 'yes' +except Exception: + pass + +print(result['native_b2']) +print(result['native_nested']) print(result['site_pkg']) print(result['mw_dir']) -" 2>/dev/null || printf 'no\n\n\n') +" 2>/dev/null || printf 'no\nno\n\n\n') -_tc_native=$(printf '%s' "$_tc_info" | sed -n '1p') -SITE_PKG=$(printf '%s' "$_tc_info" | sed -n '2p') -_MW_DIR=$(printf '%s' "$_tc_info" | sed -n '3p') +_tc_native_b2=$(printf '%s' "$_tc_info" | sed -n '1p') +_tc_native_nested=$(printf '%s' "$_tc_info" | sed -n '2p') +SITE_PKG=$(printf '%s' "$_tc_info" | sed -n '3p') +_MW_DIR=$(printf '%s' "$_tc_info" | sed -n '4p') -if [ "$_tc_native" = "yes" ]; then - echo "NOTICE: TrueNAS now provides native B2 restic support — truecloud-patch is no longer needed." +# Nested support is opt-in; if it was never enabled, it cannot be the reason to +# keep the patch alive. +if [ -f "$PATCH_DIR/nested_snapshots_enabled" ]; then + _NESTED_ENABLED=1 +else + _NESTED_ENABLED=0 +fi + +# Is either module still doing something useful? +_providers_needed=1 +[ "$_tc_native_b2" = "yes" ] && _providers_needed=0 + +_nested_needed=0 +if [ "$_NESTED_ENABLED" = "1" ] && [ "$_tc_native_nested" != "yes" ]; then + _nested_needed=1 +fi + +if [ "$_providers_needed" = "0" ] && [ "$_nested_needed" = "0" ]; then + echo "NOTICE: Nothing left for truecloud-patch to do:" + [ "$_tc_native_b2" = "yes" ] && echo "NOTICE: - TrueNAS now provides native B2 restic support." + if [ "$_tc_native_nested" = "yes" ]; then + echo "NOTICE: - TrueNAS now handles snapshots on nested datasets natively." + elif [ "$_NESTED_ENABLED" = "0" ]; then + echo "NOTICE: - Nested-dataset snapshots are not enabled (opt-in)." + fi echo "NOTICE: Setting kill switch; patching will be skipped on all future boots." echo "NOTICE: Run the following to fully remove the patch:" echo "NOTICE: bash $PATCH_DIR/uninstall.sh" @@ -166,12 +215,21 @@ if [ "$_tc_native" = "yes" ]; then exit 0 fi +if [ "$_providers_needed" = "0" ]; then + echo "NOTICE: TrueNAS now provides native B2 restic support — the providers module" + echo "NOTICE: is superseded and will be skipped. The nested-snapshot module is still" + echo "NOTICE: active, so the patch stays installed." +fi +if [ "$_NESTED_ENABLED" = "1" ] && [ "$_tc_native_nested" = "yes" ]; then + echo "NOTICE: TrueNAS now handles nested-dataset snapshots natively — that module is" + echo "NOTICE: superseded and will be skipped. You can drop --enable-nested-snapshots." +fi + # ── Step 1: backend patch ───────────────────────────────────────────────────── echo "--- backend patch ---" -_b2_ok=0 -_restic_ok=0 +_backend_ok=0 if [ -z "$SITE_PKG" ]; then echo "WARNING: Cannot determine site-packages directory; skipping backend patch." @@ -187,25 +245,23 @@ else _SYNC_PY="$_MW_DIR/plugins/cloud_backup/sync.py" _NESTED_SRC="$PATCH_DIR/patch/truecloud_nested.py" - # Nested-dataset snapshot support is OPT-IN. It changes how backups read - # their source data, so it is never enabled implicitly by a `git pull`. - # Enable: bash install.sh --enable-nested-snapshots - # Disable: bash install.sh --disable-nested-snapshots - if [ -f "$PATCH_DIR/nested_snapshots_enabled" ]; then - _NESTED_ENABLED=1 - else - _NESTED_ENABLED=0 - fi - + # Each module is applied only if it is still needed. _providers_needed and + # _nested_needed were computed above (native-support detection + the opt-in + # marker), so one module going native never disables the other. # ── patch b2.py + restic.py + nested-snapshot + hook_status.json ──────── # (single subprocess: PREINIT has a tight timeout budget) if "$PYTHON" - "$_B2_PY" "$_RESTIC_PY" "$PATCH_DIR/hook_status.json" \ - "$_CLOUD_DIR" "$_SYNC_PY" "$_NESTED_SRC" "$_NESTED_ENABLED" << 'PYEOF' + "$_CLOUD_DIR" "$_SYNC_PY" "$_NESTED_SRC" \ + "$_providers_needed" "$_nested_needed" "$_NESTED_ENABLED" \ + "$_tc_native_nested" << 'PYEOF' import json, os, shutil, sys, time b2_path, restic_path, status_path = sys.argv[1], sys.argv[2], sys.argv[3] cloud_dir, sync_path, nested_src = sys.argv[4], sys.argv[5], sys.argv[6] -nested_enabled = sys.argv[7] == "1" +providers_needed = sys.argv[7] == "1" +nested_needed = sys.argv[8] == "1" +nested_enabled = sys.argv[9] == "1" +nested_native = sys.argv[10] == "yes" B2_BLOCK = """ # TRUECLOUD_PATCH — added by truenas-truecloud-patch/patch/apply.sh @@ -404,27 +460,39 @@ def patch_file(path, block): b2_ok = restic_ok = False nested_ok = False -if os.path.exists(b2_path): - try: - patch_file(b2_path, B2_BLOCK) - b2_ok = True - print(f"OK: Patched b2.py → {b2_path}") - except Exception as e: - print(f"WARNING: Failed to patch b2.py: {e}") +# ── module: providers (B2/S3) ───────────────────────────────────────────────── +# Skipped entirely once TrueNAS ships native B2 restic support. That must not +# take the nested module down with it, so the two are gated independently. +providers_detail = '' +if not providers_needed: + providers_detail = 'superseded: TrueNAS provides native B2 restic support' + print('INFO: Providers module skipped — TrueNAS now supports B2 natively.') else: - print(f"WARNING: b2.py not found at {b2_path}") + if os.path.exists(b2_path): + try: + patch_file(b2_path, B2_BLOCK) + b2_ok = True + print(f"OK: Patched b2.py → {b2_path}") + except Exception as e: + print(f"WARNING: Failed to patch b2.py: {e}") + else: + print(f"WARNING: b2.py not found at {b2_path}") -if os.path.exists(restic_path): - try: - patch_file(restic_path, RESTIC_BLOCK) - restic_ok = True - print(f"OK: Patched restic.py → {restic_path}") - except Exception as e: - print(f"WARNING: Failed to patch restic.py: {e}") -else: - print(f"WARNING: restic.py not found at {restic_path}") + if os.path.exists(restic_path): + try: + patch_file(restic_path, RESTIC_BLOCK) + restic_ok = True + print(f"OK: Patched restic.py → {restic_path}") + except Exception as e: + print(f"WARNING: Failed to patch restic.py: {e}") + else: + print(f"WARNING: restic.py not found at {restic_path}") + providers_detail = ( + 'patched on disk in overlay at boot' if (b2_ok and restic_ok) + else 'b2.py/restic.py not found or write failed' + ) -# ── nested-dataset snapshot support ─────────────────────────────────────────── +# ── module: nested-dataset snapshots ────────────────────────────────────────── # Order matters: install the traversal machinery FIRST, relax the validation # guard LAST. If anything fails partway, the guard is still in place and the # option stays unavailable -- we never expose "guard removed, traversal missing". @@ -433,6 +501,12 @@ if not nested_enabled: nested_detail = 'disabled (opt-in; enable with: install.sh --enable-nested-snapshots)' print('INFO: Nested-dataset snapshot support is disabled (opt-in feature).') print('INFO: Enable with: bash install.sh --enable-nested-snapshots') +elif nested_native: + nested_detail = 'superseded: TrueNAS handles nested-dataset snapshots natively' + print('INFO: Nested module skipped — TrueNAS now handles nesting natively.') +elif not nested_needed: + nested_detail = 'not needed' + print('INFO: Nested module skipped.') else: try: snapshot_py = os.path.join(cloud_dir, 'snapshot.py') @@ -459,13 +533,23 @@ else: print('WARNING: for nested datasets. Existing backups are unaffected.') patches = { + 'module.providers': { + 'ok': bool(b2_ok and restic_ok) or not providers_needed, + 'active': providers_needed, + 'detail': providers_detail, + }, + 'module.nested_snapshots': { + 'ok': nested_ok or not nested_needed, + 'active': nested_needed, + 'detail': nested_detail, + }, 'middlewared.rclone.remote.b2': { 'ok': b2_ok, - 'detail': 'patched on disk in overlay at boot' if b2_ok else 'b2.py not found or write failed', + 'detail': 'patched on disk in overlay at boot' if b2_ok else providers_detail, }, 'middlewared.plugins.cloud_backup.restic': { 'ok': restic_ok, - 'detail': 'patched on disk in overlay at boot' if restic_ok else 'restic.py not found or write failed', + 'detail': 'patched on disk in overlay at boot' if restic_ok else providers_detail, }, 'middlewared.plugins.cloud.nested_snapshot': { 'ok': nested_ok, @@ -482,35 +566,45 @@ try: except OSError as e: print(f'WARNING: Could not write hook_status.json: {e}') -sys.exit(0 if (b2_ok and restic_ok) else 1) +# Exit 0 only if every module that is still NEEDED applied cleanly. A module that +# was skipped (superseded or opt-out) is not a failure. +_providers_done = (not providers_needed) or (b2_ok and restic_ok) +_nested_done = (not nested_needed) or nested_ok +sys.exit(0 if (_providers_done and _nested_done) else 1) PYEOF then - _b2_ok=1 - _restic_ok=1 + _backend_ok=1 else - # Individual results already printed above; exit code 1 means at least one failed. - true + # Individual results already printed above; exit code 1 means at least + # one module that was still needed failed to apply. + _backend_ok=0 fi fi # ── Step 2: Angular bundle ──────────────────────────────────────────────────── +# Belongs to the providers module (it widens the credential dropdown), so it is +# skipped along with it once TrueNAS supports B2 natively. echo "--- UI patch ---" -# Ensure the webui directory is writable before patch_ui.py tries to create a -# backup and write the patched bundle. On immutable OS we mount an overlay. -_webui_dir="" -for _d in /usr/share/truenas/webui /usr/share/truenas-ui /var/www/truenas; do - if [ -d "$_d" ]; then - _webui_dir="$_d" - break +if [ "$_providers_needed" = "0" ]; then + echo "Skipped — providers module superseded by native B2 support." +else + # Ensure the webui directory is writable before patch_ui.py tries to create a + # backup and write the patched bundle. On immutable OS we mount an overlay. + _webui_dir="" + for _d in /usr/share/truenas/webui /usr/share/truenas-ui /var/www/truenas; do + if [ -d "$_d" ]; then + _webui_dir="$_d" + break + fi + done + if [ -n "$_webui_dir" ]; then + _ensure_writable "$_webui_dir" "ui" || true # non-fatal; patch_ui.py reports the error fi -done -if [ -n "$_webui_dir" ]; then - _ensure_writable "$_webui_dir" "ui" || true # non-fatal; patch_ui.py reports the error -fi -"$PYTHON" "$PATCH_DIR/patch/patch_ui.py" || echo "WARNING: patch_ui.py exited non-zero; UI dropdown may still show Storj only." + "$PYTHON" "$PATCH_DIR/patch/patch_ui.py" || echo "WARNING: patch_ui.py exited non-zero; UI dropdown may still show Storj only." +fi # ── Step 3: deferred middlewared restart (boot runs only) ───────────────────── # At boot this script is spawned by middlewared, which already imported the @@ -531,9 +625,14 @@ fi echo "--- deferred restart ---" +# Restart when ANY still-needed backend module landed. Keying this off the +# providers module alone would skip the restart on a box where B2 has gone native +# but the nested module was freshly patched — leaving it on disk and never loaded. if ! grep -aq middlewared "/proc/$PPID/cmdline" 2>/dev/null; then echo "Manual run (parent is not middlewared) — no restart scheduled." -elif [ "$_b2_ok" != "1" ] || [ "$_restic_ok" != "1" ]; then +elif [ "$_providers_needed" = "0" ] && [ "$_nested_needed" = "0" ]; then + echo "No backend module active — no restart scheduled (nothing new to load)." +elif [ "${_backend_ok:-0}" != "1" ]; then echo "Backend patch incomplete — no restart scheduled (nothing new to load)." else # A failed unit from an earlier attempt this boot would block systemd-run. diff --git a/tests/test_apply_blocks.py b/tests/test_apply_blocks.py index caaa17a..c59e652 100644 --- a/tests/test_apply_blocks.py +++ b/tests/test_apply_blocks.py @@ -105,12 +105,61 @@ def test_crud_block_is_scoped_to_cloud_backup(): assert '!= "cloud_backup"' in extract_blocks()["CRUD_BLOCK"] +class TestIndependentModules: + """The two modules must retire independently. + + TrueNAS may ship native B2 support long before (or after) it handles nested + datasets. A single all-or-nothing kill switch would silently take a + still-needed module down with the superseded one. + """ + + def _sh(self): + with open(APPLY_SH, encoding="utf-8") as fh: + return fh.read() + + def test_native_support_is_detected_per_module(self): + sh = self._sh() + assert "native_b2" in sh + assert "native_nested" in sh + assert "no further nesting" in sh, "nested native-support probe" + + def test_kill_switch_only_when_both_modules_are_done(self): + sh = self._sh() + assert '[ "$_providers_needed" = "0" ] && [ "$_nested_needed" = "0" ]' in sh + # ...and that is the only place the kill switch is actually set. (Ignore + # comment lines, which mention the same path.) + code = [ln for ln in sh.splitlines() if not ln.lstrip().startswith("#")] + sets = [ln for ln in code if 'touch "$PATCH_DIR/disabled"' in ln] + assert len(sets) == 1, f"kill switch set in {len(sets)} places" + + def test_each_module_is_gated_separately(self): + src = heredoc_source() + assert "if not providers_needed:" in src + assert "elif nested_native:" in src + + def test_ui_patch_is_tied_to_the_providers_module(self): + # The UI change widens the credential dropdown; it is meaningless once B2 + # is native, but must NOT be skipped merely because nested is off. + sh = self._sh() + i = sh.index("--- UI patch ---") + assert '[ "$_providers_needed" = "0" ]' in sh[i:i + 400] + + def test_restart_fires_when_any_needed_module_landed(self): + # Keying the restart off providers alone would leave a freshly-patched + # nested module on disk and never loaded on a native-B2 box. + sh = self._sh() + i = sh.index("--- deferred restart ---") + tail = sh[i:] + assert '_backend_ok' in tail + assert '"$_b2_ok"' not in tail + + class TestOptIn: """Nested-snapshot support must be opt-in and must never self-enable.""" def test_heredoc_gates_on_the_opt_in_flag(self): src = heredoc_source() - assert "nested_enabled = sys.argv[7]" in src + assert re.search(r"nested_enabled = sys\.argv\[\d+\] == \"1\"", src) assert "if not nested_enabled:" in src def test_apply_sh_reads_the_marker_file(self):