From 21eba1e88f5e9b7cd0542f06a3f4be685032f5f5 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Tue, 20 Oct 2026 18:09:07 +0200 Subject: [PATCH] fix(backup): carry hard links, ACLs and xattrs on every backup and recover hop baudolo 7.1.x writes volume generations with `rsync -aHAX`, so the producing host keeps `user.*` extended attributes (OpenCloud keeps its storage metadata in `user.oc.*`). Every later copy dropped them again, because none of the other rsync calls passed `-X` (nor `-A`/`-H`): - NFS export snapshots (svc-bkp-nfs-2-local `script.sh`), - the backup host's pull (svc-bkp-remote-2-local `pull_specific_host.py`), - the device sync (svc-bkp-local-2-device `script.py`), - every recover into a live target (`utils/recovery/base.py`, used by the volume, NFS and device recover scripts), - the `recover full` and `recover remote` tree pulls. A disaster recovery through the documented chain therefore restored OpenCloud without `user.oc.*`, silently: the copies succeeded and the dirval stamp does not cover attributes. All of these now run `rsync -aHAX` (`-aHAXP` for the device sync). A target filesystem without xattr support makes rsync exit non-zero as soon as a source file carries attributes, so the hop fails loudly instead of dropping them; there is no per-device opt-out. The single-image pull in `recover remote` keeps `-a`. The provider's ssh-wrapper needs no change: its glob accepts any server flags and its character guard allows them. Two tests write a `user.oc.id` attribute and assert it survives the NFS snapshot and the recover; with the old `-a` both fail with `OSError: [Errno 61] No data available`. Closes #1021 Co-Authored-By: Claude Opus 5.5 (1M context) --- cli/administration/recover/README.md | 2 +- cli/administration/recover/full.py | 2 +- cli/administration/recover/remote.py | 2 +- cli/administration/recover/schema.yml | 2 +- roles/svc-bkp-local-2-device/README.md | 2 +- roles/svc-bkp-local-2-device/files/python/recover.py | 2 +- roles/svc-bkp-local-2-device/files/python/script.py | 2 +- roles/svc-bkp-nfs-2-local/README.md | 4 ++-- roles/svc-bkp-nfs-2-local/files/python/recover.py | 2 +- roles/svc-bkp-nfs-2-local/files/shell/script.sh | 2 +- .../files/python/pull_specific_host.py | 2 +- roles/svc-bkp-volume-2-local/README.md | 4 ++-- roles/svc-bkp-volume-2-local/files/python/recover.py | 2 +- .../python/cli/administration/recover/test_full.py | 4 ++-- .../roles/svc-bkp-nfs-2-local/files/test_script.py | 8 ++++++++ tests/unit/python/utils/recovery/test_base.py | 10 ++++++++++ utils/recovery/base.py | 4 ++-- 17 files changed, 37 insertions(+), 19 deletions(-) diff --git a/cli/administration/recover/README.md b/cli/administration/recover/README.md index 64aa753e32..a11c05d2d9 100644 --- a/cli/administration/recover/README.md +++ b/cli/administration/recover/README.md @@ -78,7 +78,7 @@ expanding `volume` per docker volume. ## Safety -Each recover mirrors a snapshot with `rsync -a --delete` and first runs the +Each recover mirrors a snapshot with `rsync -aHAX --delete` and first runs the role's backup unit as a rollback point. `--no-safety-backup` skips that (only when the target holds nothing worth saving: fresh host / empty / disposable). `--preview` prints the exact commands without running them. diff --git a/cli/administration/recover/full.py b/cli/administration/recover/full.py index 63e90ea3f9..b73bbe888a 100644 --- a/cli/administration/recover/full.py +++ b/cli/administration/recover/full.py @@ -72,7 +72,7 @@ def plan(root: Path) -> tuple[list[tuple[str, str]], list[str]]: def _pull_cmd(source: str) -> list[str]: remote = source if ":" in source else f"{source}:{paths.BACKUP_ROOT}" - return ["rsync", "-a", f"{remote}/", f"{PULL_STAGE}/"] + return ["rsync", "-aHAX", f"{remote}/", f"{PULL_STAGE}/"] def _is_device(source: str) -> bool: diff --git a/cli/administration/recover/remote.py b/cli/administration/recover/remote.py index 44c7b72994..2626ea42f5 100644 --- a/cli/administration/recover/remote.py +++ b/cli/administration/recover/remote.py @@ -47,7 +47,7 @@ def commands( return ( [ ["ssh", host, shlex.join(remote)], - ["rsync", "-a", f"{host}:{REMOTE_ROOT}/", f"{LOCAL_ROOT}/"], + ["rsync", "-aHAX", f"{host}:{REMOTE_ROOT}/", f"{LOCAL_ROOT}/"], ], LOCAL_ROOT, ) diff --git a/cli/administration/recover/schema.yml b/cli/administration/recover/schema.yml index 8a14dda950..a854298a23 100644 --- a/cli/administration/recover/schema.yml +++ b/cli/administration/recover/schema.yml @@ -58,4 +58,4 @@ flags: skip the pre-recover safety backup of the current target (only when it holds nothing worth saving: fresh host / empty / disposable) -mechanism: "rsync -a --delete, preceded by a pre-recover safety backup by default" +mechanism: "rsync -aHAX --delete, preceded by a pre-recover safety backup by default" diff --git a/roles/svc-bkp-local-2-device/README.md b/roles/svc-bkp-local-2-device/README.md index aca5be1eb3..962a9a2ce9 100644 --- a/roles/svc-bkp-local-2-device/README.md +++ b/roles/svc-bkp-local-2-device/README.md @@ -97,7 +97,7 @@ Run `files/python/recover.py` on the host with the backup device attached: recover.py /dev/sdX1 /mnt/usb-recover /var/lib/infinito/backup [--snapshot ] ``` -The script opens the LUKS device (interactive passphrase prompt; `--passphrase-stdin` for automation), mounts it, picks the newest device snapshot across every machine hash (a fresh host after total loss has a new machine id), mirrors it into the target (`rsync -a --delete`) and unmounts/closes the device again. No pre-recover service backup runs: the device itself is the backup this role maintains. Afterwards restore individual applications from the backup root with the matching role's `recover.py` (`svc-bkp-volume-2-local` for docker volumes, `svc-bkp-nfs-2-local` for NFS exports). +The script opens the LUKS device (interactive passphrase prompt; `--passphrase-stdin` for automation), mounts it, picks the newest device snapshot across every machine hash (a fresh host after total loss has a new machine id), mirrors it into the target (`rsync -aHAX --delete`) and unmounts/closes the device again. No pre-recover service backup runs: the device itself is the backup this role maintains. Afterwards restore individual applications from the backup root with the matching role's `recover.py` (`svc-bkp-volume-2-local` for docker volumes, `svc-bkp-nfs-2-local` for NFS exports). ## Credits diff --git a/roles/svc-bkp-local-2-device/files/python/recover.py b/roles/svc-bkp-local-2-device/files/python/recover.py index beaff5b0b6..f7c21954df 100755 --- a/roles/svc-bkp-local-2-device/files/python/recover.py +++ b/roles/svc-bkp-local-2-device/files/python/recover.py @@ -8,7 +8,7 @@ Opens the LUKS device (interactive passphrase prompt, or stdin with ``--passphrase-stdin``), mounts it, picks the newest device snapshot (any machine hash: a fresh host after total loss has a new /etc/machine-id) and mirrors it into the target -(``rsync -a --delete``), then unmounts and closes the device again. +(``rsync -aHAX --delete``), then unmounts and closes the device again. No pre-recover service backup runs: the device itself is the backup this role maintains. diff --git a/roles/svc-bkp-local-2-device/files/python/script.py b/roles/svc-bkp-local-2-device/files/python/script.py index 222193d962..9e6a162635 100755 --- a/roles/svc-bkp-local-2-device/files/python/script.py +++ b/roles/svc-bkp-local-2-device/files/python/script.py @@ -53,7 +53,7 @@ def main(): try: rsync_command = [ "rsync", - "-aP", + "-aHAXP", "--numeric-ids", "--delete", "--delete-excluded", diff --git a/roles/svc-bkp-nfs-2-local/README.md b/roles/svc-bkp-nfs-2-local/README.md index 8b2168073c..0acd7775f5 100644 --- a/roles/svc-bkp-nfs-2-local/README.md +++ b/roles/svc-bkp-nfs-2-local/README.md @@ -44,7 +44,7 @@ flowchart TD UNIT["svc-bkp-nfs-2-local.<version>.<domain>.service"] --> LOCK["ExecStartPre: sys-lock against the manipulation group"] LOCK --> SCRIPT["ExecStart: script.sh <export_base> <backups_dir>
backup-nfs-to-local <state>/backup"] SCRIPT -->|export_base missing| ALARM["ERROR exit 1
OnFailure alarm"] - SCRIPT --> RSYNC["rsync -a --delete
--exclude /<state>/backup (no backups-in-backups)
--link-dest previous generation (hard links)"] + SCRIPT --> RSYNC["rsync -aHAX --delete
--exclude /<state>/backup (no backups-in-backups)
--link-dest previous generation (hard links)"] RSYNC -->|exit 24: live export churn| WARN["WARN, snapshot kept"] RSYNC -->|any other failure| CLEAN["generation removed, exit != 0"] RSYNC --> TREE["<backups_dir>/<sha256(machine-id)>/
backup-nfs-to-local/<YYYYmmddHHMMSS>/files/..."] @@ -105,7 +105,7 @@ recover.py //backup-nfs-to-local//files/`. -2. Run the script; it first starts the role's deployed backup unit (a fresh differential `backup-nfs-to-local` generation of the live export), then mirrors the snapshot into the target (`rsync -a --delete`, with the shared `/backup` root protected from deletion and never copied in). `--no-safety-backup` skips the unit run when the target holds nothing worth saving. +2. Run the script; it first starts the role's deployed backup unit (a fresh differential `backup-nfs-to-local` generation of the live export), then mirrors the snapshot into the target (`rsync -aHAX --delete`, with the shared `/backup` root protected from deletion and never copied in). `--no-safety-backup` skips the unit run when the target holds nothing worth saving. 3. Redeploy the stack; the NFS clients re-mount and pick up the restored state. The target subtree must already exist; the script refuses to create export subtrees implicitly. diff --git a/roles/svc-bkp-nfs-2-local/files/python/recover.py b/roles/svc-bkp-nfs-2-local/files/python/recover.py index ef159bed13..a2b0f7ebfe 100755 --- a/roles/svc-bkp-nfs-2-local/files/python/recover.py +++ b/roles/svc-bkp-nfs-2-local/files/python/recover.py @@ -6,7 +6,7 @@ Runs the role's deployed backup unit first (the usual differential ``backup-nfs-to-local`` generation of the live export), then mirrors the -snapshot into the target (``rsync -a --delete``). Run on the host +snapshot into the target (``rsync -aHAX --delete``). Run on the host serving the export with every consuming stack stopped; NFS clients re-mount on redeploy. diff --git a/roles/svc-bkp-nfs-2-local/files/shell/script.sh b/roles/svc-bkp-nfs-2-local/files/shell/script.sh index b353e816a3..415a7e893e 100644 --- a/roles/svc-bkp-nfs-2-local/files/shell/script.sh +++ b/roles/svc-bkp-nfs-2-local/files/shell/script.sh @@ -45,7 +45,7 @@ trap cleanup_failed_generation ERR mkdir -p "${DEST_DIR}" -rsync_args=(-a --numeric-ids --delete) +rsync_args=(-aHAX --numeric-ids --delete) if [[ -n "${EXCLUDE_REL}" ]]; then rsync_args+=(--exclude "/${EXCLUDE_REL#/}") fi diff --git a/roles/svc-bkp-remote-2-local/files/python/pull_specific_host.py b/roles/svc-bkp-remote-2-local/files/python/pull_specific_host.py index 3c57de188f..63553b9e81 100755 --- a/roles/svc-bkp-remote-2-local/files/python/pull_specific_host.py +++ b/roles/svc-bkp-remote-2-local/files/python/pull_specific_host.py @@ -120,7 +120,7 @@ def pull_backups(hostname: str, backups_dir: str) -> None: Path(local_backup_destination_path).mkdir(parents=True, exist_ok=True) rsync_command = ( - f"rsync -a --partial --info=stats2 " + f"rsync -aHAX --partial --info=stats2 " f"--numeric-ids --delete --delete-excluded --timeout=300 " f'-e "ssh {SSH_OPTS}" ' f'--rsync-path="sudo rsync" ' diff --git a/roles/svc-bkp-volume-2-local/README.md b/roles/svc-bkp-volume-2-local/README.md index 1a09e9415b..196385d452 100644 --- a/roles/svc-bkp-volume-2-local/README.md +++ b/roles/svc-bkp-volume-2-local/README.md @@ -78,7 +78,7 @@ flowchart TD TREE --> RECOVER["files/python/recover.py volume
resolves Mountpoint + Options"] RECOVER -->|"Options set"| PROBE{"mountpoint -q
(ssh for --docker-host)"} PROBE -->|unmounted| REFUSE["SystemExit: a restore would land
on the node disk and be shadowed
on the next mount"] - PROBE -->|mounted| RSYNC["rsync -a --delete through the
mounted export onto the server"] + PROBE -->|mounted| RSYNC["rsync -aHAX --delete through the
mounted export onto the server"] DRILL["DR drill [9/9]
mounts the export at the volume
mountpoint with the volume's own opts"] --> RECOVER RSYNC --> VERIFY["marker asserted on the
NFS server's disk"] ``` @@ -304,7 +304,7 @@ recover.py //backup-docker-to-local// ``` 1. Stop the consuming project (`docker compose down` / `docker stack rm `). -2. Run the script; it first starts the role's deployed backup unit (a fresh differential baudolo generation of every volume and database), resolves the volume's mountpoint and mirrors the snapshot into it (`rsync -a --delete`). `--no-safety-backup` skips the unit run when the target holds nothing worth saving. +2. Run the script; it first starts the role's deployed backup unit (a fresh differential baudolo generation of every volume and database), resolves the volume's mountpoint and mirrors the snapshot into it (`rsync -aHAX --delete`). `--no-safety-backup` skips the unit run when the target holds nothing worth saving. 3. Restore the databases with the `--databases` mode above **while the project is still down**, then start it again; on swarm, NFS-backed volumes are restored via `svc-bkp-nfs-2-local`'s `recover.py` instead (see below). ## Credits diff --git a/roles/svc-bkp-volume-2-local/files/python/recover.py b/roles/svc-bkp-volume-2-local/files/python/recover.py index b1ecdc5873..5d4d0829bf 100755 --- a/roles/svc-bkp-volume-2-local/files/python/recover.py +++ b/roles/svc-bkp-volume-2-local/files/python/recover.py @@ -9,7 +9,7 @@ Both modes run the role's deployed backup unit first, unless ``--no-safety-backup`` says the target holds nothing worth saving. Volume mode mirrors the snapshot into the volume's mountpoint -(``rsync -a --delete``); stop the consuming project first. With +(``rsync -aHAX --delete``); stop the consuming project first. With ``--docker-host ssh://user@host`` it works on a remote host. Database mode (``--databases``) replays every dump with diff --git a/tests/unit/python/cli/administration/recover/test_full.py b/tests/unit/python/cli/administration/recover/test_full.py index 6d980f1b5f..7395eef314 100644 --- a/tests/unit/python/cli/administration/recover/test_full.py +++ b/tests/unit/python/cli/administration/recover/test_full.py @@ -74,7 +74,7 @@ class PullCmdTest(unittest.TestCase): full._pull_cmd("user@srchost"), [ "rsync", - "-a", + "-aHAX", f"user@srchost:{paths.BACKUP_ROOT}/", f"{full.PULL_STAGE}/", ], @@ -83,7 +83,7 @@ class PullCmdTest(unittest.TestCase): def test_host_with_explicit_path(self): self.assertEqual( full._pull_cmd("host:/custom/root"), - ["rsync", "-a", "host:/custom/root/", f"{full.PULL_STAGE}/"], + ["rsync", "-aHAX", "host:/custom/root/", f"{full.PULL_STAGE}/"], ) diff --git a/tests/unit/python/roles/svc-bkp-nfs-2-local/files/test_script.py b/tests/unit/python/roles/svc-bkp-nfs-2-local/files/test_script.py index 452821bcec..acf56c75f5 100644 --- a/tests/unit/python/roles/svc-bkp-nfs-2-local/files/test_script.py +++ b/tests/unit/python/roles/svc-bkp-nfs-2-local/files/test_script.py @@ -74,6 +74,14 @@ class NfsBackupScriptTests(unittest.TestCase): ).read_text() # nocheck: cache-read tempdir fixture self.assertEqual(content, "hello") + def test_snapshot_keeps_user_xattrs(self): + (self.source / "a.txt").write_text("hello") + os.setxattr(self.source / "a.txt", "user.oc.id", b"space-1") + result = self._run() + self.assertEqual(result.returncode, 0, result.stderr) + snapshot = self._generations()[0] / "files" / "a.txt" + self.assertEqual(os.getxattr(snapshot, "user.oc.id"), b"space-1") + def test_second_run_hardlinks_unchanged_files(self): (self.source / "a.txt").write_text("hello") self.assertEqual(self._run(generation="20240101000000").returncode, 0) diff --git a/tests/unit/python/utils/recovery/test_base.py b/tests/unit/python/utils/recovery/test_base.py index b084fba0f6..e4672cc0e9 100644 --- a/tests/unit/python/utils/recovery/test_base.py +++ b/tests/unit/python/utils/recovery/test_base.py @@ -1,5 +1,6 @@ from __future__ import annotations +import os import tempfile import unittest import unittest.mock as mock @@ -55,6 +56,15 @@ class TestDirectoryRecovery(unittest.TestCase): self.assertEqual(text, "from-snapshot") self.assertFalse((target / "live.txt").exists()) + def test_restore_keeps_user_xattrs(self) -> None: + with tempfile.TemporaryDirectory() as td: + source, target = self._mk(td) + os.setxattr(source / "restored.txt", "user.oc.id", b"space-1") + _Recovery(str(source), str(target)).restore() + self.assertEqual( + os.getxattr(target / "restored.txt", "user.oc.id"), b"space-1" + ) + def test_missing_unit_refuses(self) -> None: with tempfile.TemporaryDirectory() as td: source, target = self._mk(td) diff --git a/utils/recovery/base.py b/utils/recovery/base.py index 3d7087c31e..6ca5a51ad1 100644 --- a/utils/recovery/base.py +++ b/utils/recovery/base.py @@ -1,7 +1,7 @@ """Abstract recover flow shared by every ``svc-bkp-*`` role. A recover mirrors a snapshot into a live target directory -(``rsync -a --delete``), so the target's current state would be lost +(``rsync -aHAX --delete``), so the target's current state would be lost without a net. The flow therefore first runs the role's deployed backup service (``systemctl start`` on the role's unit), which applies the usual differential backup logic to the live data before anything is @@ -87,7 +87,7 @@ class DirectoryRecovery(ABC): subprocess.run( [ "rsync", - "-a", + "-aHAX", "--numeric-ids", "--delete", *self.rsync_extra_args,