(bugfix) Guard dry-run logs and recursive rsync defaults
Clear the reused dry-run rsync log before each dry-run so run details only show output from the current execution. Populate new Django global configs with the existing safe rsync and exclude defaults, including archive mode and standard pseudo-filesystem exclusions. Add a host check that fails when effective rsync args do not include archive or recursive transfer, preventing real backups that only report "skipping directory .".
This commit is contained in:
@@ -151,6 +151,7 @@ def run_scheduled(
|
|||||||
if dry_run:
|
if dry_run:
|
||||||
dest = f"/tmp/pobsync-dryrun/{host}/"
|
dest = f"/tmp/pobsync-dryrun/{host}/"
|
||||||
dryrun_log = Path(f"/tmp/pobsync-dryrun/{host}/rsync.log")
|
dryrun_log = Path(f"/tmp/pobsync-dryrun/{host}/rsync.log")
|
||||||
|
dryrun_log.unlink(missing_ok=True)
|
||||||
|
|
||||||
cmd = build_rsync_command(
|
cmd = build_rsync_command(
|
||||||
rsync_binary=str(rsync_binary),
|
rsync_binary=str(rsync_binary),
|
||||||
|
|||||||
@@ -78,9 +78,31 @@ def collect_host_checks(host: HostConfig, global_config: GlobalConfig | None = N
|
|||||||
checks.append(_host_path_check("Host backup root", host_root, must_exist=True, must_be_writable=True))
|
checks.append(_host_path_check("Host backup root", host_root, must_exist=True, must_be_writable=True))
|
||||||
for subdir in HOST_BACKUP_SUBDIRS:
|
for subdir in HOST_BACKUP_SUBDIRS:
|
||||||
checks.append(_host_path_check(f"Host directory: {subdir}", host_root / subdir, must_exist=True, must_be_writable=True))
|
checks.append(_host_path_check(f"Host directory: {subdir}", host_root / subdir, must_exist=True, must_be_writable=True))
|
||||||
|
checks.append(_rsync_recursion_check(host, global_config))
|
||||||
return checks
|
return checks
|
||||||
|
|
||||||
|
|
||||||
|
def _rsync_recursion_check(host: HostConfig, global_config: GlobalConfig) -> SelfCheck:
|
||||||
|
args = [*list(global_config.rsync_args or []), *list(global_config.rsync_extra_args or []), *list(host.rsync_extra_args or [])]
|
||||||
|
if _has_recursive_rsync_arg(args):
|
||||||
|
return SelfCheck("Host rsync recursion", "ok", "Rsync args include archive or recursive transfer.", " ".join(args))
|
||||||
|
return SelfCheck(
|
||||||
|
"Host rsync recursion",
|
||||||
|
"failed",
|
||||||
|
"Rsync args do not include archive or recursive transfer.",
|
||||||
|
"Add --archive or --recursive before running a real backup.",
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _has_recursive_rsync_arg(args: list[str]) -> bool:
|
||||||
|
for arg in args:
|
||||||
|
if arg in {"--archive", "--recursive"}:
|
||||||
|
return True
|
||||||
|
if arg.startswith("-") and not arg.startswith("--") and any(flag in arg for flag in ("a", "r")):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
def _host_path_check(
|
def _host_path_check(
|
||||||
name: str,
|
name: str,
|
||||||
path: Path,
|
path: Path,
|
||||||
|
|||||||
@@ -67,6 +67,28 @@ class RunScheduledConfigSourceTests(SimpleTestCase):
|
|||||||
self.assertEqual(result["rsync"]["exit_code"], 12)
|
self.assertEqual(result["rsync"]["exit_code"], 12)
|
||||||
self.assertEqual(result["rsync"]["log_tail"], ["Permission denied (publickey).", "rsync error"])
|
self.assertEqual(result["rsync"]["log_tail"], ["Permission denied (publickey).", "rsync error"])
|
||||||
|
|
||||||
|
def test_dry_run_clears_previous_log_before_running(self) -> None:
|
||||||
|
def fake_run_rsync(command, log_path, timeout_seconds):
|
||||||
|
self.assertFalse(log_path.exists())
|
||||||
|
log_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
log_path.write_text("current run only\n", encoding="utf-8")
|
||||||
|
return RsyncResult(exit_code=0, command=command)
|
||||||
|
|
||||||
|
old_log = Path("/tmp/pobsync-dryrun/web-01/rsync.log")
|
||||||
|
old_log.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
old_log.write_text("old failure\n", encoding="utf-8")
|
||||||
|
|
||||||
|
with patch("pobsync.commands.run_scheduled.run_rsync", side_effect=fake_run_rsync):
|
||||||
|
result = run_scheduled(
|
||||||
|
prefix=Path("/missing-prefix"),
|
||||||
|
host="web-01",
|
||||||
|
dry_run=True,
|
||||||
|
config_source=FakeConfigSource(),
|
||||||
|
)
|
||||||
|
|
||||||
|
self.assertTrue(result["ok"])
|
||||||
|
self.assertEqual(result["rsync"]["log_tail"], ["current run only"])
|
||||||
|
|
||||||
def test_successful_real_run_applies_prune_when_requested(self) -> None:
|
def test_successful_real_run_applies_prune_when_requested(self) -> None:
|
||||||
with TemporaryDirectory() as tmp:
|
with TemporaryDirectory() as tmp:
|
||||||
prefix = Path(tmp) / "home"
|
prefix = Path(tmp) / "home"
|
||||||
|
|||||||
@@ -331,6 +331,8 @@ class ViewTests(TestCase):
|
|||||||
|
|
||||||
self.assertEqual(response.status_code, 200)
|
self.assertEqual(response.status_code, 200)
|
||||||
self.assertContains(response, f'value="{credential.id}" selected')
|
self.assertContains(response, f'value="{credential.id}" selected')
|
||||||
|
self.assertContains(response, "--archive")
|
||||||
|
self.assertContains(response, "/proc/***")
|
||||||
|
|
||||||
def test_global_config_form_renders_static_container_backup_root_on_edit(self) -> None:
|
def test_global_config_form_renders_static_container_backup_root_on_edit(self) -> None:
|
||||||
self.client.force_login(self.staff_user)
|
self.client.force_login(self.staff_user)
|
||||||
@@ -522,6 +524,17 @@ class ViewTests(TestCase):
|
|||||||
self.assertTrue((backup_root / host.host / "manual").is_dir())
|
self.assertTrue((backup_root / host.host / "manual").is_dir())
|
||||||
self.assertTrue((backup_root / host.host / ".incomplete").is_dir())
|
self.assertTrue((backup_root / host.host / ".incomplete").is_dir())
|
||||||
|
|
||||||
|
def test_host_detail_warns_when_rsync_is_not_recursive(self) -> None:
|
||||||
|
self.client.force_login(self.staff_user)
|
||||||
|
GlobalConfig.objects.create(name="default", backup_root="/backups", rsync_args=[])
|
||||||
|
host = HostConfig.objects.create(host="web-01", address="web-01.example.test")
|
||||||
|
|
||||||
|
response = self.client.get(reverse("host_detail", args=[host.host]))
|
||||||
|
|
||||||
|
self.assertEqual(response.status_code, 200)
|
||||||
|
self.assertContains(response, "Host rsync recursion")
|
||||||
|
self.assertContains(response, "Rsync args do not include archive or recursive transfer.")
|
||||||
|
|
||||||
def test_scan_host_known_key_action_updates_selected_credential(self) -> None:
|
def test_scan_host_known_key_action_updates_selected_credential(self) -> None:
|
||||||
self.client.force_login(self.staff_user)
|
self.client.force_login(self.staff_user)
|
||||||
credential = SshCredential.objects.create(name="default-key", key_path="/var/lib/pobsync/state/ssh-credentials/1/identity")
|
credential = SshCredential.objects.create(name="default-key", key_path="/var/lib/pobsync/state/ssh-credentials/1/identity")
|
||||||
|
|||||||
@@ -13,6 +13,7 @@ from django.shortcuts import get_object_or_404, redirect, render
|
|||||||
from django.views.decorators.http import require_POST
|
from django.views.decorators.http import require_POST
|
||||||
|
|
||||||
from pobsync.errors import PobsyncError
|
from pobsync.errors import PobsyncError
|
||||||
|
from pobsync.config.defaults import DEFAULT_EXCLUDES, DEFAULT_RSYNC_ARGS
|
||||||
|
|
||||||
from .backup_runner import queue_backup_run
|
from .backup_runner import queue_backup_run
|
||||||
from .forms import (
|
from .forms import (
|
||||||
@@ -519,7 +520,9 @@ def _default_global_initial() -> dict[str, object]:
|
|||||||
"ssh_user": "root",
|
"ssh_user": "root",
|
||||||
"ssh_port": 22,
|
"ssh_port": 22,
|
||||||
"rsync_binary": "rsync",
|
"rsync_binary": "rsync",
|
||||||
|
"rsync_args": DEFAULT_RSYNC_ARGS,
|
||||||
"default_source_root": "/",
|
"default_source_root": "/",
|
||||||
|
"excludes_default": DEFAULT_EXCLUDES,
|
||||||
"retention_daily": 14,
|
"retention_daily": 14,
|
||||||
"retention_weekly": 8,
|
"retention_weekly": 8,
|
||||||
"retention_monthly": 12,
|
"retention_monthly": 12,
|
||||||
|
|||||||
Reference in New Issue
Block a user