Compare commits
7
Commits
v0.6.0-rc1
...
v0.6.0-rc6
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2b8ef107f7 | ||
|
|
b50567e9a7 | ||
|
|
1b2407f6e2 | ||
|
|
ab0b66c47d | ||
|
|
469a2e4651 | ||
|
|
518a22d87e | ||
|
|
8c1b4c45f5 |
@@ -170,14 +170,15 @@ jobs:
|
||||
if: ${{ steps.report.outputs.broken == '1' && contains(github.server_url, 'github.com') }}
|
||||
env:
|
||||
GH_TOKEN: ${{ github.token }}
|
||||
TITLE: "Incompatible with upcoming TrueNAS: ${{ steps.report.outputs.refs }}"
|
||||
TITLE: "TrueNAS compatibility: the patch's assumptions no longer hold"
|
||||
run: |
|
||||
# One issue per set of broken refs, reopened/updated rather than duplicated
|
||||
# daily -- a bot that files the same issue every morning gets muted, and
|
||||
# then it is not a warning system any more.
|
||||
# Lowest-numbered match, for the same reason as the Gitea step below.
|
||||
existing="$(gh issue list --state all --search "$TITLE" \
|
||||
--json number,title \
|
||||
--jq '.[] | select(.title == env.TITLE) | .number' | head -1)"
|
||||
--jq '[.[] | select(.title == env.TITLE) | .number] | min // empty')"
|
||||
|
||||
if [ -n "$existing" ]; then
|
||||
gh issue comment "$existing" --body-file /tmp/issue.md
|
||||
@@ -191,26 +192,46 @@ jobs:
|
||||
env:
|
||||
TOKEN: ${{ secrets.GITEA_TOKEN || github.token }}
|
||||
API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
|
||||
TITLE: "Incompatible with upcoming TrueNAS: ${{ steps.report.outputs.refs }}"
|
||||
TITLE: "TrueNAS compatibility: the patch's assumptions no longer hold"
|
||||
run: |
|
||||
body="$(jq -Rs . < /tmp/issue.md)"
|
||||
title="$(printf '%s' "$TITLE" | jq -Rs .)"
|
||||
# python3, not jq: jq is not guaranteed on a self-hosted runner, and a bug
|
||||
# report that dies on a missing tool is a warning system that does not warn.
|
||||
python3 - <<'PY'
|
||||
import json, os, urllib.error, urllib.request
|
||||
|
||||
# Same title => same issue. Comment on it instead of filing a new one.
|
||||
number="$(curl -sf -H "Authorization: token $TOKEN" \
|
||||
"$API/issues?state=all&type=issues" \
|
||||
| jq -r --arg t "$TITLE" '.[] | select(.title == $t) | .number' | head -1)"
|
||||
api, token, title = os.environ["API"], os.environ["TOKEN"], os.environ["TITLE"]
|
||||
with open("/tmp/issue.md", encoding="utf-8") as fh:
|
||||
body = fh.read()
|
||||
headers = {"Authorization": f"token {token}",
|
||||
"Content-Type": "application/json"}
|
||||
|
||||
if [ -n "$number" ]; then
|
||||
curl -sS -X POST "$API/issues/$number/comments" \
|
||||
-H "Authorization: token $TOKEN" -H "Content-Type: application/json" \
|
||||
-d "$(printf '{"body":%s}' "$body")" -o /dev/null -w 'comment -> %{http_code}\n'
|
||||
curl -sS -X PATCH "$API/issues/$number" \
|
||||
-H "Authorization: token $TOKEN" -H "Content-Type: application/json" \
|
||||
-d '{"state":"open"}' -o /dev/null -w 'reopen -> %{http_code}\n'
|
||||
else
|
||||
curl -sS -X POST "$API/issues" \
|
||||
-H "Authorization: token $TOKEN" -H "Content-Type: application/json" \
|
||||
-d "$(printf '{"title":%s,"body":%s}' "$title" "$body")" \
|
||||
-o /dev/null -w 'create -> %{http_code}\n'
|
||||
fi
|
||||
def call(url, method, data=None):
|
||||
req = urllib.request.Request(
|
||||
url, method=method, headers=headers,
|
||||
data=json.dumps(data).encode() if data else None)
|
||||
with urllib.request.urlopen(req) as r: # noqa: S310
|
||||
return json.load(r) if r.length != 0 else {}
|
||||
|
||||
# Same title => same issue. Comment on it rather than filing a new one every
|
||||
# morning: a bot that duplicates itself daily gets muted, and then it is not
|
||||
# a warning system any more.
|
||||
# LOWEST-numbered match, not "whichever the API returns first". Two issues
|
||||
# with the same title already existed once (the old title embedded the ref
|
||||
# list, so the identity changed when that set changed), and an
|
||||
# order-dependent pick would have alternated between them, reopening one and
|
||||
# commenting on the other. Lowest number is stable no matter what the API
|
||||
# sorts by.
|
||||
issues = call(f"{api}/issues?state=all&type=issues", "GET")
|
||||
matches = sorted((i for i in issues if i["title"] == title),
|
||||
key=lambda i: i["number"])
|
||||
match = matches[0] if matches else None
|
||||
|
||||
if match:
|
||||
n = match["number"]
|
||||
call(f"{api}/issues/{n}/comments", "POST", {"body": body})
|
||||
call(f"{api}/issues/{n}", "PATCH", {"state": "open"})
|
||||
print(f"commented on and reopened issue #{n}")
|
||||
else:
|
||||
made = call(f"{api}/issues", "POST", {"title": title, "body": body})
|
||||
print(f"filed issue #{made['number']}")
|
||||
PY
|
||||
|
||||
@@ -164,27 +164,45 @@ jobs:
|
||||
*-rc*|*-beta*|*-alpha*) prerelease=true ;;
|
||||
esac
|
||||
|
||||
# jq -Rs so the notes are JSON-encoded properly: the changelog is full of
|
||||
# quotes, backticks and newlines, and hand-built JSON would mangle them.
|
||||
body="$(jq -Rs . < /tmp/notes.md)"
|
||||
payload="$(printf '{"tag_name":%s,"name":%s,"body":%s,"prerelease":%s}' \
|
||||
"$(printf '%s' "$TAG" | jq -Rs .)" \
|
||||
"$(printf '%s' "$TAG" | jq -Rs .)" \
|
||||
"$body" "$prerelease")"
|
||||
# python3, not jq. The changelog is full of quotes, backticks and newlines,
|
||||
# so the body must be properly JSON-encoded -- but `jq` is not guaranteed on
|
||||
# a self-hosted Gitea runner, and a publish step that dies on a missing tool
|
||||
# leaves a tag with no release behind it. python3 is guaranteed: setup-python
|
||||
# ran above.
|
||||
python3 - "$TAG" "$API" "$TOKEN" "$prerelease" <<'PY'
|
||||
import json, sys, urllib.error, urllib.request
|
||||
|
||||
existing="$(curl -sf -H "Authorization: token $TOKEN" \
|
||||
"$API/releases/tags/$TAG" 2>/dev/null || true)"
|
||||
tag, api, token, prerelease = sys.argv[1:5]
|
||||
with open("/tmp/notes.md", encoding="utf-8") as fh:
|
||||
body = fh.read()
|
||||
|
||||
if [ -n "$existing" ]; then
|
||||
id="$(printf '%s' "$existing" | jq -r .id)"
|
||||
echo "Release $TAG exists (id=$id) — updating notes."
|
||||
curl -sS -X PATCH "$API/releases/$id" \
|
||||
-H "Authorization: token $TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "$payload" -o /dev/null -w 'PATCH -> %{http_code}\n'
|
||||
else
|
||||
curl -sS -X POST "$API/releases" \
|
||||
-H "Authorization: token $TOKEN" \
|
||||
-H "Content-Type: application/json" \
|
||||
-d "$payload" -o /dev/null -w 'POST -> %{http_code}\n'
|
||||
fi
|
||||
payload = {
|
||||
"tag_name": tag, "name": tag, "body": body,
|
||||
"prerelease": prerelease == "true",
|
||||
}
|
||||
headers = {
|
||||
"Authorization": f"token {token}",
|
||||
"Content-Type": "application/json",
|
||||
}
|
||||
|
||||
def call(url, method, data=None):
|
||||
req = urllib.request.Request(
|
||||
url, method=method, headers=headers,
|
||||
data=json.dumps(data).encode() if data else None)
|
||||
with urllib.request.urlopen(req) as r: # noqa: S310
|
||||
return r.status, json.load(r) if r.length != 0 else {}
|
||||
|
||||
try:
|
||||
_, existing = call(f"{api}/releases/tags/{tag}", "GET")
|
||||
except urllib.error.HTTPError as e:
|
||||
if e.code != 404:
|
||||
raise
|
||||
existing = None
|
||||
|
||||
if existing:
|
||||
status, _ = call(f"{api}/releases/{existing['id']}", "PATCH", payload)
|
||||
print(f"updated release {tag} -> {status}")
|
||||
else:
|
||||
status, _ = call(f"{api}/releases", "POST", payload)
|
||||
print(f"created release {tag} (prerelease={payload['prerelease']}) -> {status}")
|
||||
PY
|
||||
|
||||
@@ -66,6 +66,50 @@ worse than no alert, because one day it carries a security fix.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **A few snapshots leaked on every nested run, forever.** Found on real hardware, in
|
||||
the one place it could be: a 256-snapshot backup of `/mnt/Tap` swept 253 cleanly and
|
||||
left **3 behind** with `dataset is busy`.
|
||||
|
||||
The cause is ZFS's own automount. Reading anything under
|
||||
`<dataset>/.zfs/snapshot/<snap>/` makes ZFS **automount that snapshot**, and it stays
|
||||
mounted for `zfs_expire_snapshot` seconds (**300** by default) after the last access.
|
||||
`teardown()` unmounts *our* bind mounts — but not the automount underneath — so
|
||||
`zfs destroy` refuses for exactly the datasets restic read most recently. Then
|
||||
`cleanup_task()` removed the sidecar anyway, destroying the only record that those
|
||||
snapshots existed. Nothing would ever have reclaimed them.
|
||||
|
||||
Three changes, and the third is the one that makes it safe rather than merely
|
||||
unlikely:
|
||||
- `release_snapdirs()` unmounts ZFS's own `.zfs/snapshot` automounts (deepest first)
|
||||
before deleting, so the snapshots are not busy in the first place.
|
||||
- `delete_snapshot_tree()` **retries** the transient busy, and **returns the
|
||||
snapshots it could not delete** instead of swallowing them.
|
||||
- **The sidecar is now removed only on a confirmed-clean sweep** — including on the
|
||||
staging-failure path, which used to remove it *before* the caller swept. The
|
||||
asymmetry is deliberate: a sidecar left behind when the tree is already gone costs
|
||||
one no-op delete on the next run, while a sidecar removed while the tree still
|
||||
exists is unrecoverable. Survivors are reclaimed by the next run.
|
||||
|
||||
**Expect the occasional straggler, and expect it to clean itself up.** On a
|
||||
256-snapshot tree this reliably sweeps ~255 immediately and may leave **one**: it is
|
||||
whatever restic read last, so its 300-second window has barely opened. That one is
|
||||
logged, its sidecar is kept, and the next run reclaims it before doing anything else.
|
||||
The leak is bounded at a single cycle rather than growing without limit — which is
|
||||
the property that actually matters. Blocking a backup job for five minutes to chase
|
||||
the last snapshot would be a worse trade, so it is not made.
|
||||
|
||||
- **Installing the patch permanently blocked updating it.** `install.sh` does
|
||||
`chmod +x update.sh`, and git recorded `update.sh` as `100644` — so the chmod was a
|
||||
*tracked modification*, and `update.sh` refuses to run over a dirty tree. Install
|
||||
once and you could never update again; the error even told you to run
|
||||
`git checkout -- .`, which just undoes the exec bit so the next install can re-dirty
|
||||
it. A real box sat on an old version for exactly this reason.
|
||||
|
||||
Fixed on both sides: the scripts `install.sh` chmods are now executable in git (so
|
||||
the chmod is a no-op), and `update.sh`'s dirty check now looks at **content**, not
|
||||
file mode — `git diff --numstat` reports `0 0` for a mode-only change. A test
|
||||
asserts every script in `install.sh`'s chmod loop is already `100755` in git.
|
||||
|
||||
- **Nested snapshots were broken on TrueNAS 24.10 and 25.04, and had been all
|
||||
along.** `SYNC_BLOCK`'s wrapper spelled out the stock signature and forwarded five
|
||||
arguments — but those releases declare `restic_backup(middleware, job,
|
||||
|
||||
@@ -22,7 +22,7 @@ Clone it onto a **pool** (not the boot device — that is wiped on TrueNAS upgra
|
||||
then run `install.sh` as root:
|
||||
|
||||
```bash
|
||||
git clone https://git.onetick.ninja/flan/truenas-truecloud-patch.git \
|
||||
git clone https://github.com/sudolulo/truenas-truecloud-patch.git \
|
||||
/mnt/tank/truenas-truecloud-patch # replace `tank` with your pool
|
||||
cd /mnt/tank/truenas-truecloud-patch
|
||||
sudo bash install.sh
|
||||
|
||||
@@ -158,7 +158,7 @@ python3 /mnt/tank/truenas-truecloud-patch/patch/create_task.py verify
|
||||
| What you see | What it means |
|
||||
|---|---|
|
||||
| `[OK] providers`, `[OK]`/`[SKIP] nested_snapshots` | Fine. Nothing to do. |
|
||||
| `WARNING: … pattern not found` (UI) | The Angular bundle changed. The UI dropdown reverts to Storj-only, but **backups keep working** — create tasks with `create_task.py` meanwhile, and [open an issue](https://git.onetick.ninja/flan/truenas-truecloud-patch/issues) with your TrueNAS version. |
|
||||
| `WARNING: … pattern not found` (UI) | The Angular bundle changed. The UI dropdown reverts to Storj-only, but **backups keep working** — create tasks with `create_task.py` meanwhile, and [open an issue](https://github.com/sudolulo/truenas-truecloud-patch/issues) with your TrueNAS version. |
|
||||
| `WARNING: truecloud-patch is NOT COMPATIBLE with this TrueNAS version` | This TrueNAS changed middleware underneath the patch, and the named module was **deliberately not applied** — see `incompatible.json` for exactly which assumption broke. TrueNAS is left stock, so nothing is half-patched. Check [TrueNAS compatibility](../README.md#truenas-compatibility), then `bash update.sh` once a release supports your version; it re-applies itself on the next boot. This is **not** the kill switch and needs no manual reset. |
|
||||
| `[FAIL] providers` | **Your B2/S3 backups will not run.** middlewared is fine, but the credential/URL handling is gone. Open an issue with your version. |
|
||||
| `[FAIL] nested_snapshots` | The stock guard is back, so tasks with `snapshot = true` on a nested dataset will fail validation. Turn the option off on those tasks until it's fixed. |
|
||||
|
||||
@@ -95,6 +95,25 @@ re-scan each time.
|
||||
|
||||
### Snapshot lifecycle
|
||||
|
||||
> **A snapshot may survive a run, and that is expected.** ZFS **automounts**
|
||||
> `<dataset>/.zfs/snapshot/<snap>` the moment it is read, and holds it for
|
||||
> `zfs_expire_snapshot` seconds (**300** by default) after the last access. So
|
||||
> whatever restic read *last* is still pinned when we try to destroy it, and
|
||||
> `zfs destroy` refuses with `dataset is busy`.
|
||||
>
|
||||
> The patch unmounts those automounts itself and retries, which clears ~255 of 256 on
|
||||
> a real pool. The one that remains is **logged, its sidecar is kept, and the next run
|
||||
> reclaims it before doing anything else** — so the leak is bounded at a single cycle
|
||||
> instead of growing forever. Seeing one `could not delete snapshot … it will be
|
||||
> reclaimed on the next run` in the log is normal. Seeing the count *grow* run over run
|
||||
> is not, and would be a bug.
|
||||
>
|
||||
> This is why the sidecar is removed **only on a confirmed-clean sweep**: it is the
|
||||
> only record those snapshots exist, and a run that dropped it while they were still
|
||||
> around would orphan them permanently. That is precisely what happened before this was
|
||||
> fixed.
|
||||
|
||||
|
||||
`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.
|
||||
|
||||
+1
-1
@@ -109,7 +109,7 @@ If a module shows `[FAIL]`:
|
||||
risk.
|
||||
4. **If the detail says the module doesn't exist**, a TrueNAS update renamed
|
||||
or restructured the internal API.
|
||||
[Open an issue](https://git.onetick.ninja/flan/truenas-truecloud-patch/issues)
|
||||
[Open an issue](https://github.com/sudolulo/truenas-truecloud-patch/issues)
|
||||
with your TrueNAS version number and the full verify output.
|
||||
|
||||
---
|
||||
|
||||
+2
-2
@@ -4,7 +4,7 @@
|
||||
# Prerequisites: run as root on TrueNAS SCALE with middlewared running.
|
||||
# Clone this repository to a persistent ZFS pool first:
|
||||
#
|
||||
# git clone https://git.onetick.ninja/flan/truenas-truecloud-patch \
|
||||
# git clone https://github.com/sudolulo/truenas-truecloud-patch \
|
||||
# /mnt/<pool>/truenas-truecloud-patch
|
||||
# cd /mnt/<pool>/truenas-truecloud-patch && bash install.sh
|
||||
#
|
||||
@@ -72,7 +72,7 @@ done
|
||||
if [ ! -f "$PATCH_DIR/patch/apply.sh" ]; then
|
||||
echo "ERROR: patch files not found at $PATCH_DIR/patch/" >&2
|
||||
echo "Run install.sh from a clone of the repository on a persistent pool:" >&2
|
||||
echo " git clone https://git.onetick.ninja/flan/truenas-truecloud-patch \\" >&2
|
||||
echo " git clone https://github.com/sudolulo/truenas-truecloud-patch \\" >&2
|
||||
echo " /mnt/<pool>/truenas-truecloud-patch" >&2
|
||||
echo " cd /mnt/<pool>/truenas-truecloud-patch && bash install.sh" >&2
|
||||
exit 1
|
||||
|
||||
+3
-3
@@ -111,7 +111,7 @@ def main():
|
||||
print(
|
||||
"[truecloud-patch] WARNING: filterByProviders pattern not found in any JS bundle.\n"
|
||||
"[truecloud-patch] The TrueNAS webui may have been restructured in this version.\n"
|
||||
"[truecloud-patch] File an issue at https://git.onetick.ninja/flan/truenas-truecloud-patch\n"
|
||||
"[truecloud-patch] File an issue at https://github.com/sudolulo/truenas-truecloud-patch\n"
|
||||
f"[truecloud-patch] TrueNAS version info: {_tnversion()}"
|
||||
)
|
||||
return
|
||||
@@ -136,7 +136,7 @@ def main():
|
||||
f"[truecloud-patch] WARNING: {count} replacement(s) in {path}; "
|
||||
f"expected exactly 1 — skipping write to avoid corrupting the bundle.\n"
|
||||
f"[truecloud-patch] File an issue at "
|
||||
f"https://git.onetick.ninja/flan/truenas-truecloud-patch"
|
||||
f"https://github.com/sudolulo/truenas-truecloud-patch"
|
||||
)
|
||||
return
|
||||
|
||||
@@ -154,7 +154,7 @@ def main():
|
||||
"[truecloud-patch] The UI is UNCHANGED and still works. This means the "
|
||||
"pattern no longer fits this TrueNAS build.\n"
|
||||
"[truecloud-patch] File an issue at "
|
||||
"https://git.onetick.ninja/flan/truenas-truecloud-patch"
|
||||
"https://github.com/sudolulo/truenas-truecloud-patch"
|
||||
)
|
||||
return
|
||||
|
||||
|
||||
+202
-32
@@ -58,6 +58,7 @@ import contextlib
|
||||
import os
|
||||
import stat
|
||||
import subprocess
|
||||
import time
|
||||
|
||||
__all__ = [
|
||||
"STAGING_BASE",
|
||||
@@ -113,21 +114,39 @@ def sidecar_for(staging_root: str) -> str:
|
||||
return staging_root + ".snapshot"
|
||||
|
||||
|
||||
def _write_sidecar(staging_root: str, snapshot: str) -> None:
|
||||
"""Record the pinned snapshot on disk. Blocking; call via run_in_thread."""
|
||||
def _write_sidecar(staging_root: str, snapshots) -> None:
|
||||
"""Record every snapshot tree this task still owns. One per line.
|
||||
|
||||
A LIST, not a single name -- and that is not over-engineering, it is a bug fix.
|
||||
|
||||
The sidecar used to hold one snapshot, so a run that reclaimed an older tree,
|
||||
FAILED to finish reclaiming it, and then recorded its own snapshot would
|
||||
**overwrite the only record of the survivor** -- orphaning it permanently, which is
|
||||
exactly the outcome the sidecar exists to prevent. Observed live: a snapshot
|
||||
survived one run, the next run's reclaim also failed (ZFS's 300s automount window
|
||||
had not elapsed, because the runs were minutes apart), and the record was
|
||||
destroyed anyway.
|
||||
|
||||
Now every still-pending tree is carried forward until it is actually gone.
|
||||
"""
|
||||
if isinstance(snapshots, str):
|
||||
snapshots = [snapshots]
|
||||
with contextlib.suppress(OSError):
|
||||
os.makedirs(os.path.dirname(staging_root), exist_ok=True)
|
||||
with open(sidecar_for(staging_root), "w", encoding="utf-8") as fh:
|
||||
fh.write(snapshot)
|
||||
fh.write("\n".join(dict.fromkeys(snapshots))) # de-duped, order kept
|
||||
|
||||
|
||||
def _read_sidecar(staging_root: str) -> str | None:
|
||||
"""The snapshot a previous run recorded here, if any."""
|
||||
def _read_sidecar(staging_root: str):
|
||||
"""Every snapshot tree a previous run recorded here. [] if none.
|
||||
|
||||
Tolerates the old single-line format, which is just a one-element list.
|
||||
"""
|
||||
try:
|
||||
with open(sidecar_for(staging_root), encoding="utf-8") as fh:
|
||||
return fh.read().strip() or None
|
||||
return [ln.strip() for ln in fh if ln.strip()]
|
||||
except OSError:
|
||||
return None
|
||||
return []
|
||||
|
||||
|
||||
def _remove_sidecar(staging_root: str) -> None:
|
||||
@@ -362,6 +381,48 @@ def teardown(staging_root, runner=_run, mounts_file="/proc/self/mounts"):
|
||||
return errors
|
||||
|
||||
|
||||
def snapdir_automounts(snapshot_name, mounts_file="/proc/self/mounts"):
|
||||
"""Every ``<dataset>/.zfs/snapshot/<snap>`` ZFS automount for this snapshot."""
|
||||
suffix = "/.zfs/snapshot/" + snapshot_name
|
||||
found = []
|
||||
try:
|
||||
with open(mounts_file, encoding="utf-8") as fh:
|
||||
for line in fh:
|
||||
parts = line.split()
|
||||
if len(parts) > 1:
|
||||
mp = parts[1].replace("\\040", " ")
|
||||
if mp.endswith(suffix):
|
||||
found.append(mp)
|
||||
except OSError:
|
||||
return []
|
||||
return sorted(found, key=_depth, reverse=True) # deepest first
|
||||
|
||||
|
||||
def release_snapdirs(snapshot_name, runner=_run, mounts_file="/proc/self/mounts"):
|
||||
"""Unmount ZFS's OWN snapshot automounts, so the snapshots can be destroyed.
|
||||
|
||||
Reading anything under ``<dataset>/.zfs/snapshot/<snap>/`` makes ZFS **automount**
|
||||
that snapshot, and it stays mounted for ``zfs_expire_snapshot`` seconds (300 by
|
||||
default) after the last access. teardown() unmounts OUR bind mounts -- but the
|
||||
automount underneath them survives, and while it exists ``zfs destroy`` refuses
|
||||
with *"dataset is busy"*.
|
||||
|
||||
Proven on a real pool: a 256-snapshot recursive tree swept cleanly except for the
|
||||
three datasets restic had read most recently. Those failed with EBUSY, and because
|
||||
cleanup_task removed the sidecar anyway, they were orphaned **permanently** -- a
|
||||
small leak, but a growing one, and exactly the failure this module exists to
|
||||
prevent.
|
||||
|
||||
Deepest first, so a child's automount is released before its parent's.
|
||||
"""
|
||||
errors = []
|
||||
for mp in snapdir_automounts(snapshot_name, mounts_file=mounts_file):
|
||||
res = runner(["umount", mp])
|
||||
if res.returncode != 0:
|
||||
errors.append(f"{mp}: {(res.stderr or '').strip()}")
|
||||
return errors
|
||||
|
||||
|
||||
# ── orchestration (middleware is duck-typed; no middlewared import) ───────────
|
||||
#
|
||||
# These are SYNCHRONOUS and talk to middlewared via `middleware.call_sync`, which
|
||||
@@ -424,15 +485,31 @@ def get_dataset_recursive(datasets, directory):
|
||||
)
|
||||
|
||||
|
||||
def delete_snapshot_tree(middleware, snapshot, logger=None):
|
||||
def delete_snapshot_tree(middleware, snapshot, logger=None, attempts=4,
|
||||
sleep=time.sleep):
|
||||
"""Delete the parent snapshot AND every child created by ``zfs snapshot -r``.
|
||||
|
||||
Returns the snapshots it could NOT delete -- callers must not throw that away.
|
||||
|
||||
``zfs.snapshot.delete`` is non-recursive by default and stock calls it with
|
||||
no options, so relying on stock would orphan one snapshot per descendant
|
||||
dataset on every run. Idempotent: tolerates the parent already being gone
|
||||
(stock's ``finally`` may have won the race once our mounts were released).
|
||||
|
||||
"dataset is busy" is EXPECTED here and is TRANSIENT. ZFS automounts
|
||||
``<dataset>/.zfs/snapshot/<snap>`` when it is read and keeps it mounted for
|
||||
``zfs_expire_snapshot`` seconds (300 by default) afterwards. So the datasets restic
|
||||
touched last are still pinned when we try to destroy them. We release the
|
||||
automounts explicitly and then retry -- on a real 256-snapshot tree, exactly three
|
||||
snapshots hit this, and before the fix they were orphaned permanently.
|
||||
"""
|
||||
dataset = snapshot.partition("@")[0]
|
||||
dataset, _, snapname = snapshot.partition("@")
|
||||
|
||||
# Release ZFS's own automounts first, or `zfs destroy` refuses with EBUSY on
|
||||
# everything restic read in the last few minutes.
|
||||
for err in release_snapdirs(snapname):
|
||||
if logger:
|
||||
logger.debug("truecloud-patch: could not release snapdir %s", err)
|
||||
|
||||
# Fast path: ONE recursive delete removes the parent and every child that
|
||||
# `zfs snapshot -r` created (252 on a real pool). Deleting them individually
|
||||
@@ -441,7 +518,7 @@ def delete_snapshot_tree(middleware, snapshot, logger=None):
|
||||
# exists to prevent.
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", snapshot, {"recursive": True})
|
||||
return
|
||||
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
|
||||
# mounts were released), which the sweep below handles. Log it rather than
|
||||
@@ -472,14 +549,53 @@ def delete_snapshot_tree(middleware, snapshot, logger=None):
|
||||
)
|
||||
names = [snapshot]
|
||||
|
||||
for name in names:
|
||||
def confirm_gone(failed):
|
||||
"""Drop any name ZFS no longer has, even though its delete raised.
|
||||
|
||||
A delete that raised "does not exist" SUCCEEDED as far as we care, and must
|
||||
not be retried or reported. The query is only a refinement: if it cannot be
|
||||
answered we keep the delete's own verdict, rather than inventing survivors --
|
||||
a false survivor keeps the sidecar forever and is reported as a leak that
|
||||
isn't there.
|
||||
"""
|
||||
if not failed:
|
||||
return []
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", name)
|
||||
except Exception as e: # noqa: BLE001 - already gone is fine
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: could not delete snapshot %s: %r", name, e
|
||||
)
|
||||
live = middleware.call_sync(
|
||||
"zfs.snapshot.query", [["name", "^", dataset]], {"select": ["name"]}
|
||||
)
|
||||
except Exception: # noqa: BLE001 - cannot refine; trust the delete's verdict
|
||||
return list(failed)
|
||||
live = {s["name"] for s in live}
|
||||
return [n for n in failed if n in live]
|
||||
|
||||
remaining = list(names)
|
||||
for attempt in range(attempts):
|
||||
failed = []
|
||||
for name in remaining:
|
||||
try:
|
||||
middleware.call_sync("zfs.snapshot.delete", name)
|
||||
except Exception: # noqa: BLE001 - busy, or already gone; sorted out below
|
||||
failed.append(name)
|
||||
|
||||
remaining = confirm_gone(failed)
|
||||
if not remaining:
|
||||
return []
|
||||
|
||||
if attempt < attempts - 1:
|
||||
# EBUSY is the automount expiring. Release again (anything that walks
|
||||
# .zfs can re-automount a snapshot) and give it a moment.
|
||||
release_snapdirs(snapname)
|
||||
sleep(5)
|
||||
|
||||
for name in remaining:
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: could not delete snapshot %s after %d attempts "
|
||||
"(still busy?) -- it will be reclaimed on the next run",
|
||||
name, attempts,
|
||||
)
|
||||
return remaining
|
||||
|
||||
|
||||
def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
|
||||
@@ -508,24 +624,38 @@ def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
|
||||
# A previous run may have crashed mid-flight; never build on top of that.
|
||||
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 = _read_sidecar(staging_root)
|
||||
if stale and stale != snapshot:
|
||||
# ...and if it left snapshot trees behind, they are still on disk and nothing else
|
||||
# will ever reclaim them. Sweep them before recording our own, or a single crashed
|
||||
# run orphans 160+ snapshots permanently.
|
||||
#
|
||||
# Anything a reclaim FAILS to delete is carried forward, not dropped. Overwriting
|
||||
# the sidecar with only our own snapshot is what destroyed the record of a survivor
|
||||
# once already: the reclaim ran, hit ZFS's 300-second automount window (the runs
|
||||
# were minutes apart), left one snapshot behind, and then the record of it was
|
||||
# overwritten -- a permanent orphan, created by the very code meant to prevent one.
|
||||
pending = []
|
||||
for stale in _read_sidecar(staging_root):
|
||||
if stale == snapshot:
|
||||
continue
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: reclaiming snapshot tree from an earlier "
|
||||
"interrupted run: %s", stale,
|
||||
"run: %s", stale,
|
||||
)
|
||||
delete_snapshot_tree(middleware, stale, logger=logger)
|
||||
pending.extend(delete_snapshot_tree(middleware, stale, logger=logger))
|
||||
|
||||
if pending and logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: %d snapshot(s) from an earlier run are still busy; "
|
||||
"carrying them forward to the next run", len(pending),
|
||||
)
|
||||
|
||||
# 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.
|
||||
_write_sidecar(staging_root, snapshot)
|
||||
_write_sidecar(staging_root, [*pending, snapshot])
|
||||
|
||||
try:
|
||||
mounts, skipped = plan_staging(
|
||||
@@ -541,8 +671,18 @@ def stage_nested(middleware, path, snapshot, base_dataset, base_mountpoint,
|
||||
apply_plan(mounts)
|
||||
verify_staged(mounts)
|
||||
except Exception:
|
||||
# Tear down the mounts, but KEEP the sidecar.
|
||||
#
|
||||
# The caller (SNAPSHOT_BLOCK) sweeps the snapshot tree on the way out, and if
|
||||
# any of it is still busy it will survive -- and the sidecar is the only record
|
||||
# that it exists. Removing it here would orphan those snapshots permanently.
|
||||
#
|
||||
# The asymmetry is deliberate: a sidecar left behind when the tree is already
|
||||
# gone is harmless (the next run tries to delete a tree that is not there,
|
||||
# finds nothing, and moves on), while a sidecar removed while the tree still
|
||||
# exists is unrecoverable. Only a confirmed-clean sweep removes it -- see
|
||||
# cleanup_task().
|
||||
teardown(staging_root)
|
||||
_remove_sidecar(staging_root)
|
||||
raise
|
||||
|
||||
if logger:
|
||||
@@ -559,9 +699,9 @@ def cleanup_task(middleware, task_name, logger=None):
|
||||
Safe to call unconditionally: a no-op when the task was never staged.
|
||||
"""
|
||||
staging_root = staging_root_for(task_name)
|
||||
snapshot = _read_sidecar(staging_root)
|
||||
pinned = _read_sidecar(staging_root)
|
||||
|
||||
if snapshot is None and not os.path.isdir(staging_root):
|
||||
if not pinned and not os.path.isdir(staging_root):
|
||||
return # never staged; nothing to do
|
||||
|
||||
errors = teardown(staging_root)
|
||||
@@ -569,8 +709,39 @@ def cleanup_task(middleware, task_name, logger=None):
|
||||
for err in errors:
|
||||
logger.warning("truecloud-patch: staging teardown: %s", err)
|
||||
|
||||
if snapshot is not None:
|
||||
delete_snapshot_tree(middleware, snapshot, logger=logger)
|
||||
if not pinned:
|
||||
_remove_sidecar(staging_root)
|
||||
return
|
||||
|
||||
# Every tree this task still owns -- ours, plus anything an earlier run could not
|
||||
# finish reclaiming.
|
||||
survivors = []
|
||||
for snapshot in pinned:
|
||||
survivors.extend(delete_snapshot_tree(middleware, snapshot, logger=logger))
|
||||
|
||||
# KEEP the sidecar if anything survived. It is the only record that those
|
||||
# snapshots exist, and removing it orphans them permanently.
|
||||
#
|
||||
# That is not theoretical: on a real 256-snapshot tree, three snapshots were still
|
||||
# pinned by ZFS's own .zfs/snapshot automount (which lingers for 300s after the
|
||||
# last read), failed to delete with "dataset is busy", and the sidecar was removed
|
||||
# anyway -- so nothing would ever have reclaimed them. A small leak, but one that
|
||||
# grows by a few snapshots on every single run, forever.
|
||||
#
|
||||
# Left in place, the next run's stage_nested() sees a stale sidecar naming a
|
||||
# different snapshot and sweeps that tree first -- by which time the automounts are
|
||||
# long gone and the delete succeeds.
|
||||
if survivors:
|
||||
if logger:
|
||||
logger.warning(
|
||||
"truecloud-patch: %d snapshot(s) could not be deleted (still busy); "
|
||||
"recording them so the next run reclaims them: %s",
|
||||
len(survivors), ", ".join(survivors),
|
||||
)
|
||||
# The SURVIVORS, not the trees we asked to delete. Writing the original list
|
||||
# back would keep re-sweeping trees that are already gone.
|
||||
_write_sidecar(staging_root, survivors)
|
||||
return
|
||||
|
||||
_remove_sidecar(staging_root)
|
||||
|
||||
@@ -599,8 +770,7 @@ def cleanup_all(base=None, runner=_run, mounts_file="/proc/self/mounts",
|
||||
# a sidecar is the only record that an interrupted run's snapshot tree (one
|
||||
# snapshot per descendant dataset) is still on disk.
|
||||
for sc in sorted(glob_fn(os.path.join(base, "*.snapshot"))):
|
||||
snap = read_sidecar(sc[: -len(".snapshot")])
|
||||
if snap:
|
||||
for snap in read_sidecar(sc[: -len(".snapshot")]):
|
||||
lines.append(f" NOTE: an interrupted backup left snapshot '{snap}' behind.")
|
||||
lines.append(f" Remove it and its children: zfs destroy -r '{snap}'")
|
||||
|
||||
|
||||
Regular → Executable
@@ -94,3 +94,40 @@ class TestTheReadmeStaysAReadme:
|
||||
assert "24.10" in text[:text.index("## Install")], (
|
||||
"the minimum TrueNAS version must be visible above the install steps"
|
||||
)
|
||||
|
||||
|
||||
class TestInstallDoesNotDirtyTheCheckout:
|
||||
"""install.sh chmod +x's scripts. If git records them as 100644, that chmod is a
|
||||
TRACKED MODIFICATION -- and update.sh refuses to run over a dirty tree.
|
||||
|
||||
So installing once permanently blocked updating, for every user, with a message
|
||||
telling them to `git checkout -- .` (which would just undo the exec bit and let
|
||||
the next install re-dirty it). Found on a real box that had been stuck on an old
|
||||
version for exactly this reason.
|
||||
|
||||
Every script install.sh makes executable must already be executable in git.
|
||||
"""
|
||||
|
||||
def test_every_chmodded_script_is_already_executable_in_git(self):
|
||||
import re
|
||||
import subprocess
|
||||
|
||||
with open(os.path.join(ROOT, "install.sh"), encoding="utf-8") as fh:
|
||||
m = re.search(r"^for _exe in (.+?); do", fh.read(), re.M)
|
||||
assert m, "could not find install.sh's chmod loop"
|
||||
scripts = m.group(1).split()
|
||||
|
||||
out = subprocess.run(
|
||||
["git", "ls-files", "-s", *scripts],
|
||||
cwd=ROOT, capture_output=True, text=True, check=True,
|
||||
).stdout
|
||||
|
||||
not_exec = [
|
||||
line.split("\t")[-1] for line in out.strip().splitlines()
|
||||
if not line.startswith("100755")
|
||||
]
|
||||
assert not not_exec, (
|
||||
"install.sh chmod +x's these, but git records them as non-executable — "
|
||||
"so installing dirties the checkout and update.sh then refuses to run:\n "
|
||||
+ "\n ".join(not_exec)
|
||||
)
|
||||
|
||||
@@ -205,3 +205,31 @@ class TestSignificance:
|
||||
assert version_tuple("0.4.2") > version_tuple("0.4.1")
|
||||
# Pre-release suffixes are dropped, not ranked above the release.
|
||||
assert version_tuple("v0.5.0-rc1") == version_tuple("v0.5.0")
|
||||
|
||||
|
||||
class TestCandidateNotesResolveToTheBaseVersion:
|
||||
"""`notes v0.6.0-rc1` must return v0.6.0's section.
|
||||
|
||||
A candidate ships the same code as the release it is a candidate for, and the
|
||||
CHANGELOG only ever has the one section. Without this, the release workflow cut
|
||||
v0.6.0-rc1, passed every gate, and then died extracting the body -- so the tag
|
||||
existed but nothing was ever published. Caught in an rc, which is the entire
|
||||
point of having them.
|
||||
"""
|
||||
|
||||
CHANGELOG = "# C\n\n## v0.6.0 — 2026-07-13\n\n### Added\n- the thing\n\n## v0.5.1 — 2026-07-13\n\n- older\n"
|
||||
|
||||
def test_an_rc_resolves_to_its_base_version(self):
|
||||
body = extract_notes(self.CHANGELOG, "v0.6.0-rc1")
|
||||
assert "the thing" in body
|
||||
assert "older" not in body
|
||||
|
||||
def test_rc10_too(self):
|
||||
assert "the thing" in extract_notes(self.CHANGELOG, "v0.6.0-rc10")
|
||||
|
||||
def test_the_plain_version_still_works(self):
|
||||
assert "the thing" in extract_notes(self.CHANGELOG, "v0.6.0")
|
||||
|
||||
def test_a_genuinely_missing_section_still_raises(self):
|
||||
with pytest.raises(KeyError):
|
||||
extract_notes(self.CHANGELOG, "v9.9.9-rc1")
|
||||
|
||||
@@ -348,7 +348,14 @@ class TestStageNestedOrdering:
|
||||
|
||||
assert mw.snapshots == [], "the crashed run's snapshot tree must be reclaimed"
|
||||
|
||||
def test_sidecar_is_removed_when_staging_fails(self, tmp_path, monkeypatch):
|
||||
def test_sidecar_is_KEPT_when_staging_fails(self, tmp_path, monkeypatch):
|
||||
# The caller sweeps the snapshot tree on the way out, and anything still busy
|
||||
# SURVIVES that sweep -- with the sidecar as its only record. Removing the
|
||||
# sidecar here would orphan those snapshots permanently.
|
||||
#
|
||||
# The asymmetry is the point: a sidecar left behind when the tree is already
|
||||
# gone costs one no-op delete on the next run; a sidecar removed while the tree
|
||||
# still exists is unrecoverable.
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
@@ -361,7 +368,12 @@ class TestStageNestedOrdering:
|
||||
"cloud_backup-5", DATASETS,
|
||||
)
|
||||
|
||||
assert not os.path.exists(sidecar_for(root))
|
||||
assert os.path.exists(sidecar_for(root)), (
|
||||
"sidecar removed on staging failure — any snapshot the caller's sweep "
|
||||
"cannot delete is now orphaned forever"
|
||||
)
|
||||
with open(sidecar_for(root), encoding="utf-8") as fh:
|
||||
assert fh.read().strip() == "Tap@snap"
|
||||
|
||||
|
||||
class TestCleanupTask:
|
||||
@@ -609,3 +621,275 @@ class TestStagingRootFor:
|
||||
# os.path.join(BASE, "..") normalises to /run — teardown would rmdir it.
|
||||
root = staging_root_for(name)
|
||||
assert os.path.normpath(root).startswith("/run/truecloud-nested/")
|
||||
|
||||
|
||||
class TestZfsAutomountKeepsSnapshotsBusy:
|
||||
""""dataset is busy" is EXPECTED, TRANSIENT, and used to orphan snapshots forever.
|
||||
|
||||
Reading `<dataset>/.zfs/snapshot/<snap>/` makes ZFS **automount** that snapshot,
|
||||
and it stays mounted for zfs_expire_snapshot seconds (300 by default) after the
|
||||
last access. teardown() unmounts OUR bind mounts, but not the automount underneath
|
||||
-- so `zfs destroy` refuses with EBUSY for everything restic read recently.
|
||||
|
||||
Observed on a real 256-snapshot tree: 253 swept cleanly, and the 3 datasets restic
|
||||
had touched last failed with "dataset is busy". cleanup_task then removed the
|
||||
sidecar anyway, so nothing would ever reclaim them. A few snapshots leaked per run,
|
||||
forever.
|
||||
"""
|
||||
|
||||
MOUNTS = (
|
||||
"tmpfs /run tmpfs rw 0 0\n"
|
||||
"Tap/apps/prometheus /mnt/Tap/apps/prometheus/.zfs/snapshot/snap1 zfs ro 0 0\n"
|
||||
"Tap/apps/standing/data /mnt/Tap/apps/standing/data/.zfs/snapshot/snap1 zfs ro 0 0\n"
|
||||
"Tap /mnt/Tap/.zfs/snapshot/snap1 zfs ro 0 0\n"
|
||||
"Tap/other /mnt/Tap/other/.zfs/snapshot/OTHER zfs ro 0 0\n"
|
||||
)
|
||||
|
||||
def _mounts_file(self, tmp_path):
|
||||
p = tmp_path / "mounts"
|
||||
p.write_text(self.MOUNTS)
|
||||
return str(p)
|
||||
|
||||
def test_it_finds_the_automounts_for_this_snapshot_only(self, tmp_path):
|
||||
import truecloud_nested as tn
|
||||
found = tn.snapdir_automounts("snap1", mounts_file=self._mounts_file(tmp_path))
|
||||
assert "/mnt/Tap/other/.zfs/snapshot/OTHER" not in found
|
||||
assert len(found) == 3
|
||||
|
||||
def test_deepest_first(self, tmp_path):
|
||||
# A child's automount must be released before its parent's.
|
||||
import truecloud_nested as tn
|
||||
found = tn.snapdir_automounts("snap1", mounts_file=self._mounts_file(tmp_path))
|
||||
assert found[-1] == "/mnt/Tap/.zfs/snapshot/snap1"
|
||||
|
||||
def test_release_snapdirs_unmounts_them(self, tmp_path):
|
||||
import truecloud_nested as tn
|
||||
called = []
|
||||
|
||||
class R:
|
||||
returncode = 0
|
||||
stderr = ""
|
||||
|
||||
def runner(cmd):
|
||||
called.append(cmd)
|
||||
return R()
|
||||
|
||||
errs = tn.release_snapdirs("snap1", runner=runner,
|
||||
mounts_file=self._mounts_file(tmp_path))
|
||||
assert errs == []
|
||||
assert all(c[0] == "umount" for c in called)
|
||||
assert len(called) == 3
|
||||
|
||||
|
||||
class BusyMiddleware(FakeMiddleware):
|
||||
"""Deletes fail with EBUSY until `busy_until_attempt` passes -- like a ZFS
|
||||
automount expiring."""
|
||||
|
||||
def __init__(self, snapshots, busy, busy_for=2):
|
||||
super().__init__(snapshots)
|
||||
self.busy = set(busy)
|
||||
self.busy_for = busy_for
|
||||
self.attempts = 0
|
||||
|
||||
def call_sync(self, method, *args):
|
||||
if method == "zfs.snapshot.delete":
|
||||
name = args[0]
|
||||
opts = args[1] if len(args) > 1 else {}
|
||||
if opts.get("recursive"):
|
||||
raise RuntimeError("cannot destroy snapshot: dataset is busy")
|
||||
if name in self.busy:
|
||||
self.attempts += 1
|
||||
if self.attempts <= self.busy_for * len(self.busy):
|
||||
raise RuntimeError(f"cannot destroy '{name}': dataset is busy")
|
||||
return super().call_sync(method, *args)
|
||||
|
||||
|
||||
class TestDeleteRetriesAndReportsSurvivors:
|
||||
def test_a_transient_busy_is_retried_and_wins(self, monkeypatch):
|
||||
import truecloud_nested as tn
|
||||
monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
|
||||
|
||||
mw = BusyMiddleware(
|
||||
["Tap@snap", "Tap/apps@snap", "Tap/apps/prometheus@snap"],
|
||||
busy=["Tap/apps/prometheus@snap"], busy_for=1,
|
||||
)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
assert survivors == []
|
||||
assert mw.snapshots == []
|
||||
|
||||
def test_a_permanently_busy_snapshot_is_REPORTED_not_swallowed(self, monkeypatch):
|
||||
import truecloud_nested as tn
|
||||
monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
|
||||
|
||||
mw = BusyMiddleware(
|
||||
["Tap@snap", "Tap/apps/prometheus@snap"],
|
||||
busy=["Tap/apps/prometheus@snap"], busy_for=99,
|
||||
)
|
||||
survivors = tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
assert survivors == ["Tap/apps/prometheus@snap"]
|
||||
assert mw.snapshots == ["Tap/apps/prometheus@snap"]
|
||||
|
||||
def test_the_automounts_are_released_before_deleting(self, monkeypatch):
|
||||
import truecloud_nested as tn
|
||||
order = []
|
||||
monkeypatch.setattr(tn, "release_snapdirs",
|
||||
lambda name, **k: order.append(("release", name)) or [])
|
||||
mw = FakeMiddleware(["Tap@snap"])
|
||||
real = mw.call_sync
|
||||
|
||||
def spy(method, *args):
|
||||
order.append((method, args[0] if args else None))
|
||||
return real(method, *args)
|
||||
|
||||
mw.call_sync = spy
|
||||
tn.delete_snapshot_tree(mw, "Tap@snap", sleep=lambda _s: None)
|
||||
assert order[0] == ("release", "snap"), order
|
||||
|
||||
|
||||
class TestSidecarSurvivesAnIncompleteSweep:
|
||||
def test_the_sidecar_is_KEPT_when_snapshots_could_not_be_deleted(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
# It is the ONLY record those snapshots exist. Removing it orphans them
|
||||
# permanently -- which is exactly what happened on the real box.
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
monkeypatch.setattr(tn, "release_snapdirs", lambda *a, **k: [])
|
||||
root = tn.staging_root_for("cloud_backup-5")
|
||||
os.makedirs(root, exist_ok=True)
|
||||
with open(sidecar_for(root), "w", encoding="utf-8") as fh:
|
||||
fh.write("Tap@snap")
|
||||
|
||||
mw = BusyMiddleware(["Tap@snap", "Tap/apps/prometheus@snap"],
|
||||
busy=["Tap/apps/prometheus@snap"], busy_for=99)
|
||||
monkeypatch.setattr(tn, "delete_snapshot_tree",
|
||||
lambda m, s, logger=None: ["Tap/apps/prometheus@snap"])
|
||||
|
||||
tn.cleanup_task(mw, "cloud_backup-5")
|
||||
assert os.path.exists(sidecar_for(root)), (
|
||||
"sidecar removed despite survivors — they are now orphaned forever"
|
||||
)
|
||||
|
||||
def test_the_sidecar_is_removed_on_a_clean_sweep(self, tmp_path, monkeypatch):
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
root = tn.staging_root_for("cloud_backup-5")
|
||||
os.makedirs(root, exist_ok=True)
|
||||
with open(sidecar_for(root), "w", encoding="utf-8") as fh:
|
||||
fh.write("Tap@snap")
|
||||
|
||||
monkeypatch.setattr(tn, "delete_snapshot_tree", lambda m, s, logger=None: [])
|
||||
tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
|
||||
assert not os.path.exists(sidecar_for(root))
|
||||
|
||||
|
||||
class TestTheSidecarCarriesEveryPendingTree:
|
||||
"""The sidecar holds a LIST, and that is a bug fix, not a generalisation.
|
||||
|
||||
It used to hold ONE snapshot. So a run that reclaimed an older tree, FAILED to
|
||||
finish reclaiming it, and then recorded its own snapshot would **overwrite the only
|
||||
record of the survivor** — orphaning it permanently, via the exact code written to
|
||||
prevent orphans.
|
||||
|
||||
Observed live: a snapshot survived one run; the next run's reclaim also failed
|
||||
(ZFS's 300s automount window had not elapsed, because the two runs were minutes
|
||||
apart); the record was overwritten; the snapshot was orphaned for good.
|
||||
"""
|
||||
|
||||
def test_round_trips_a_list(self, tmp_path):
|
||||
import truecloud_nested as tn
|
||||
root = str(tmp_path / "cloud_backup-5")
|
||||
tn._write_sidecar(root, ["Tap@a", "Tap@b"])
|
||||
assert tn._read_sidecar(root) == ["Tap@a", "Tap@b"]
|
||||
|
||||
def test_reads_the_old_single_line_format(self, tmp_path):
|
||||
# Boxes upgrading from an older version have a one-line sidecar on disk.
|
||||
import truecloud_nested as tn
|
||||
root = str(tmp_path / "cloud_backup-5")
|
||||
os.makedirs(os.path.dirname(sidecar_for(root)), exist_ok=True)
|
||||
with open(sidecar_for(root), "w", encoding="utf-8") as fh:
|
||||
fh.write("Tap@legacy")
|
||||
assert tn._read_sidecar(root) == ["Tap@legacy"]
|
||||
|
||||
def test_a_failed_reclaim_is_carried_forward_not_overwritten(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
# THE bug. stage_nested reclaims an old tree, cannot finish, then records its
|
||||
# own snapshot -- the survivor must still be in the sidecar afterwards.
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
root = tn.staging_root_for("cloud_backup-5")
|
||||
os.makedirs(os.path.dirname(root), exist_ok=True)
|
||||
tn._write_sidecar(root, ["Tap@old"])
|
||||
|
||||
# The reclaim of Tap@old leaves one snapshot behind (still busy).
|
||||
monkeypatch.setattr(
|
||||
tn, "delete_snapshot_tree",
|
||||
lambda m, s, logger=None: ["Tap/apps/x@old"] if s == "Tap@old" else [],
|
||||
)
|
||||
stub_core(monkeypatch, tn, plan=([("/src", root)], []))
|
||||
|
||||
tn.stage_nested(FakeMiddleware(), "/mnt/Tap", "Tap@new", "Tap", "/mnt/Tap",
|
||||
"cloud_backup-5", DATASETS)
|
||||
|
||||
recorded = tn._read_sidecar(root)
|
||||
assert "Tap/apps/x@old" in recorded, (
|
||||
"the failed reclaim's survivor was dropped — orphaned forever"
|
||||
)
|
||||
assert "Tap@new" in recorded, "our own snapshot must also be recorded"
|
||||
|
||||
def test_cleanup_sweeps_every_pending_tree_and_records_only_survivors(
|
||||
self, tmp_path, monkeypatch
|
||||
):
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
root = tn.staging_root_for("cloud_backup-5")
|
||||
os.makedirs(root, exist_ok=True)
|
||||
tn._write_sidecar(root, ["Tap@old", "Tap@new"])
|
||||
|
||||
swept = []
|
||||
|
||||
def fake_delete(m, s, logger=None):
|
||||
swept.append(s)
|
||||
return ["Tap/apps/x@new"] if s == "Tap@new" else []
|
||||
|
||||
monkeypatch.setattr(tn, "delete_snapshot_tree", fake_delete)
|
||||
tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
|
||||
|
||||
assert swept == ["Tap@old", "Tap@new"], "both pending trees must be swept"
|
||||
# Only the SURVIVOR is written back -- re-recording Tap@old would make every
|
||||
# future run re-sweep a tree that is already gone.
|
||||
assert tn._read_sidecar(root) == ["Tap/apps/x@new"]
|
||||
|
||||
def test_a_fully_clean_sweep_removes_the_sidecar(self, tmp_path, monkeypatch):
|
||||
import truecloud_nested as tn
|
||||
|
||||
monkeypatch.setattr(tn, "STAGING_BASE", str(tmp_path))
|
||||
root = tn.staging_root_for("cloud_backup-5")
|
||||
os.makedirs(root, exist_ok=True)
|
||||
tn._write_sidecar(root, ["Tap@a", "Tap@b"])
|
||||
monkeypatch.setattr(tn, "delete_snapshot_tree", lambda m, s, logger=None: [])
|
||||
|
||||
tn.cleanup_task(FakeMiddleware(), "cloud_backup-5")
|
||||
assert not os.path.exists(sidecar_for(root))
|
||||
|
||||
def test_cleanup_all_reports_each_pending_snapshot_on_its_own_line(self, tmp_path):
|
||||
# It formats them for a human during uninstall. A list rendered into an
|
||||
# f-string would print "['Tap@a', 'Tap@b']" at them.
|
||||
import truecloud_nested as tn
|
||||
root = str(tmp_path / "cloud_backup-5")
|
||||
tn._write_sidecar(root, ["Tap@a", "Tap@b"])
|
||||
|
||||
lines, _errors = tn.cleanup_all(
|
||||
base=str(tmp_path),
|
||||
glob_fn=lambda _p: [sidecar_for(root)],
|
||||
mounts_file=os.devnull,
|
||||
)
|
||||
notes = [ln for ln in lines if "left snapshot" in ln]
|
||||
assert len(notes) == 2
|
||||
assert "'Tap@a'" in notes[0] and "'Tap@b'" in notes[1]
|
||||
assert "[" not in "".join(notes)
|
||||
|
||||
@@ -107,10 +107,15 @@ def changelog_versions(text: str) -> list[str]:
|
||||
def extract_notes(text: str, version: str) -> str:
|
||||
"""The body of one version's section, without its heading.
|
||||
|
||||
A release candidate resolves to its BASE version: v0.6.0-rc2 ships the same code
|
||||
as v0.6.0 and therefore the same notes, and the CHANGELOG only ever has the one
|
||||
section. Without this, the release workflow cut the tag, passed every gate, and
|
||||
then died extracting the body -- so the candidate existed but was never published.
|
||||
|
||||
Raises KeyError if the version has no section -- a release with an empty or
|
||||
wrong body is worse than a failed release.
|
||||
"""
|
||||
want = normalise(version)
|
||||
want = base_version(version)
|
||||
lines = text.splitlines()
|
||||
|
||||
start = None
|
||||
|
||||
@@ -123,7 +123,7 @@ cd "$PATCH_DIR"
|
||||
|
||||
if ! git rev-parse --git-dir >/dev/null 2>&1; then
|
||||
echo "ERROR: $PATCH_DIR is not a git clone — nothing to update." >&2
|
||||
echo " Re-clone from https://git.onetick.ninja/flan/truenas-truecloud-patch" >&2
|
||||
echo " Re-clone from https://github.com/sudolulo/truenas-truecloud-patch" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -136,9 +136,21 @@ fi
|
||||
|
||||
# A dirty tree means someone edited or scp'd files in place; merging over that
|
||||
# silently loses their changes, or conflicts halfway through.
|
||||
if [ -n "$(git status --porcelain --untracked-files=no)" ]; then
|
||||
#
|
||||
# CONTENT changes only. A mode-only change (100644 -> 100755) is not somebody's work
|
||||
# and must not block an update -- and it is not hypothetical: install.sh chmod +x's
|
||||
# these very scripts, so on any version where git recorded one as 100644, INSTALLING
|
||||
# dirtied the checkout and update.sh then refused to run. Install once, and updating
|
||||
# was blocked forever, with an error telling the user to `git checkout -- .` (which
|
||||
# merely undoes the exec bit so the next install can re-dirty it). A real box sat on
|
||||
# an old version for exactly this reason.
|
||||
#
|
||||
# `git diff --numstat` reports "0 0 file" for a mode-only change, so anything with a
|
||||
# nonzero insert or delete count is a genuine edit.
|
||||
_dirty=$(git diff --numstat HEAD -- . | awk '$1 != 0 || $2 != 0 { print $3 }')
|
||||
if [ -n "$_dirty" ]; then
|
||||
echo "ERROR: the working tree has uncommitted changes:" >&2
|
||||
git status --short --untracked-files=no >&2
|
||||
printf ' M %s\n' $_dirty >&2
|
||||
echo "" >&2
|
||||
echo " Refusing to update over them. Commit, stash, or discard them first:" >&2
|
||||
echo " git -C $PATCH_DIR checkout -- ." >&2
|
||||
|
||||
Reference in New Issue
Block a user