Run post-backup pruning through SQL retention
Stop passing prune options into the legacy scheduled backup engine from the Django backup command. Record the completed snapshot first, then apply retention through the SQL-backed retention service so pruning sees the same SnapshotRecord state as the admin and retention command. Also record prune failures on BackupRun.result instead of leaving the run in an ambiguous state.
This commit is contained in:
@@ -141,6 +141,7 @@ The public command surface is Django-first. The old YAML/cron CLI has been retir
|
|||||||
Discovered snapshots are stored in `SnapshotRecord`, including the base snapshot metadata and a nullable SQL link to the
|
Discovered snapshots are stored in `SnapshotRecord`, including the base snapshot metadata and a nullable SQL link to the
|
||||||
base record when it is known.
|
base record when it is known.
|
||||||
The Django retention command plans from `SnapshotRecord` instead of rediscovering snapshots from the filesystem.
|
The Django retention command plans from `SnapshotRecord` instead of rediscovering snapshots from the filesystem.
|
||||||
|
Post-backup pruning from Django also uses the SQL retention service after the completed snapshot is recorded.
|
||||||
|
|
||||||
The remaining internal engine code still contains reusable backup primitives:
|
The remaining internal engine code still contains reusable backup primitives:
|
||||||
|
|
||||||
@@ -152,7 +153,6 @@ The remaining internal engine code still contains reusable backup primitives:
|
|||||||
Next refactor targets:
|
Next refactor targets:
|
||||||
|
|
||||||
- Surface `SnapshotRecord` data through API/admin views instead of filesystem inspection.
|
- Surface `SnapshotRecord` data through API/admin views instead of filesystem inspection.
|
||||||
- Move post-backup pruning onto the SQL retention service.
|
|
||||||
- Move more snapshot lifecycle details into typed domain objects.
|
- Move more snapshot lifecycle details into typed domain objects.
|
||||||
- Replace remaining dictionary-shaped config at engine boundaries.
|
- Replace remaining dictionary-shaped config at engine boundaries.
|
||||||
- Remove legacy YAML import/export once production migration no longer needs it.
|
- Remove legacy YAML import/export once production migration no longer needs it.
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ from pobsync.commands.run_scheduled import run_scheduled
|
|||||||
from pobsync.paths import PobsyncPaths
|
from pobsync.paths import PobsyncPaths
|
||||||
from pobsync_backend.config_source import DjangoConfigSource
|
from pobsync_backend.config_source import DjangoConfigSource
|
||||||
from pobsync_backend.models import BackupRun, HostConfig
|
from pobsync_backend.models import BackupRun, HostConfig
|
||||||
|
from pobsync_backend.retention import run_sql_retention_apply
|
||||||
from pobsync_backend.snapshot_discovery import infer_snapshot_kind, upsert_snapshot_record
|
from pobsync_backend.snapshot_discovery import infer_snapshot_kind, upsert_snapshot_record
|
||||||
|
|
||||||
|
|
||||||
@@ -45,9 +46,7 @@ class Command(BaseCommand):
|
|||||||
prefix=paths.home,
|
prefix=paths.home,
|
||||||
host=host.host,
|
host=host.host,
|
||||||
dry_run=bool(options["dry_run"]),
|
dry_run=bool(options["dry_run"]),
|
||||||
prune=bool(options["prune"]),
|
prune=False,
|
||||||
prune_max_delete=int(options["prune_max_delete"]),
|
|
||||||
prune_protect_bases=bool(options["prune_protect_bases"]),
|
|
||||||
config_source=DjangoConfigSource(),
|
config_source=DjangoConfigSource(),
|
||||||
)
|
)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
@@ -72,7 +71,38 @@ class Command(BaseCommand):
|
|||||||
snapshot_record, _created = upsert_snapshot_record(host=host, kind=kind, snapshot_dir=snapshot_path)
|
snapshot_record, _created = upsert_snapshot_record(host=host, kind=kind, snapshot_dir=snapshot_path)
|
||||||
except ValueError:
|
except ValueError:
|
||||||
snapshot_record = None
|
snapshot_record = None
|
||||||
|
|
||||||
|
if result.get("ok") and not result.get("dry_run") and options["prune"]:
|
||||||
|
try:
|
||||||
|
result["prune"] = run_sql_retention_apply(
|
||||||
|
prefix=paths.home,
|
||||||
|
host=host.host,
|
||||||
|
kind="scheduled",
|
||||||
|
protect_bases=bool(options["prune_protect_bases"]),
|
||||||
|
yes=True,
|
||||||
|
max_delete=int(options["prune_max_delete"]),
|
||||||
|
acquire_lock=False,
|
||||||
|
)
|
||||||
|
except Exception as exc:
|
||||||
|
result["prune"] = {"ok": False, "error": str(exc), "type": type(exc).__name__}
|
||||||
|
run.status = BackupRun.Status.FAILED
|
||||||
|
run.result = result
|
||||||
|
run.snapshot = snapshot_record
|
||||||
|
run.save(
|
||||||
|
update_fields=[
|
||||||
|
"status",
|
||||||
|
"ended_at",
|
||||||
|
"snapshot_path",
|
||||||
|
"snapshot",
|
||||||
|
"base_path",
|
||||||
|
"rsync_exit_code",
|
||||||
|
"result",
|
||||||
|
],
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
|
||||||
run.snapshot = snapshot_record
|
run.snapshot = snapshot_record
|
||||||
|
run.result = result
|
||||||
run.save(
|
run.save(
|
||||||
update_fields=[
|
update_fields=[
|
||||||
"status",
|
"status",
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ from django.core.management import call_command
|
|||||||
from django.core.management.base import CommandError
|
from django.core.management.base import CommandError
|
||||||
from django.test import TestCase
|
from django.test import TestCase
|
||||||
|
|
||||||
|
from pobsync.errors import ConfigError
|
||||||
from pobsync.util import write_yaml_atomic
|
from pobsync.util import write_yaml_atomic
|
||||||
from pobsync_backend.models import BackupRun, GlobalConfig, HostConfig, SnapshotRecord
|
from pobsync_backend.models import BackupRun, GlobalConfig, HostConfig, SnapshotRecord
|
||||||
|
|
||||||
@@ -51,6 +52,99 @@ class RunBackupRecordsSnapshotTests(TestCase):
|
|||||||
self.assertEqual(record.kind, "scheduled")
|
self.assertEqual(record.kind, "scheduled")
|
||||||
self.assertEqual(record.status, "success")
|
self.assertEqual(record.status, "success")
|
||||||
|
|
||||||
|
def test_prune_uses_sql_retention_after_snapshot_record_is_created(self) -> None:
|
||||||
|
with TemporaryDirectory() as tmp:
|
||||||
|
backup_root = Path(tmp) / "backups"
|
||||||
|
GlobalConfig.objects.create(name="default", backup_root=str(backup_root))
|
||||||
|
host = HostConfig.objects.create(host="web-01", address="web-01.example.test")
|
||||||
|
snapshot_dir = backup_root / host.host / "scheduled" / "20260519-021500Z__ABCDEFGH"
|
||||||
|
meta_dir = snapshot_dir / "meta"
|
||||||
|
meta_dir.mkdir(parents=True)
|
||||||
|
write_yaml_atomic(meta_dir / "meta.yaml", {"status": "success", "started_at": "2026-05-19T02:15:00Z"})
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("pobsync_backend.management.commands.run_pobsync_backup.run_scheduled") as run_scheduled,
|
||||||
|
patch(
|
||||||
|
"pobsync_backend.management.commands.run_pobsync_backup.run_sql_retention_apply"
|
||||||
|
) as retention_apply,
|
||||||
|
):
|
||||||
|
run_scheduled.return_value = {
|
||||||
|
"ok": True,
|
||||||
|
"dry_run": False,
|
||||||
|
"host": host.host,
|
||||||
|
"snapshot": str(snapshot_dir),
|
||||||
|
"base": None,
|
||||||
|
"rsync": {"exit_code": 0},
|
||||||
|
}
|
||||||
|
retention_apply.return_value = {"ok": True, "source": "sql", "deleted": []}
|
||||||
|
call_command(
|
||||||
|
"run_pobsync_backup",
|
||||||
|
host.host,
|
||||||
|
prefix=str(Path(tmp) / "home"),
|
||||||
|
prune=True,
|
||||||
|
prune_max_delete=3,
|
||||||
|
prune_protect_bases=True,
|
||||||
|
stdout=StringIO(),
|
||||||
|
)
|
||||||
|
|
||||||
|
run_scheduled.assert_called_once()
|
||||||
|
self.assertFalse(run_scheduled.call_args.kwargs["prune"])
|
||||||
|
retention_apply.assert_called_once_with(
|
||||||
|
prefix=Path(tmp) / "home",
|
||||||
|
host=host.host,
|
||||||
|
kind="scheduled",
|
||||||
|
protect_bases=True,
|
||||||
|
yes=True,
|
||||||
|
max_delete=3,
|
||||||
|
acquire_lock=False,
|
||||||
|
)
|
||||||
|
run = BackupRun.objects.get()
|
||||||
|
self.assertEqual(run.status, BackupRun.Status.SUCCESS)
|
||||||
|
self.assertEqual(run.result["prune"], {"ok": True, "source": "sql", "deleted": []})
|
||||||
|
|
||||||
|
def test_prune_failure_is_recorded_on_backup_run(self) -> None:
|
||||||
|
with TemporaryDirectory() as tmp:
|
||||||
|
backup_root = Path(tmp) / "backups"
|
||||||
|
GlobalConfig.objects.create(name="default", backup_root=str(backup_root))
|
||||||
|
host = HostConfig.objects.create(host="web-01", address="web-01.example.test")
|
||||||
|
snapshot_dir = backup_root / host.host / "scheduled" / "20260519-021500Z__ABCDEFGH"
|
||||||
|
meta_dir = snapshot_dir / "meta"
|
||||||
|
meta_dir.mkdir(parents=True)
|
||||||
|
write_yaml_atomic(meta_dir / "meta.yaml", {"status": "success", "started_at": "2026-05-19T02:15:00Z"})
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("pobsync_backend.management.commands.run_pobsync_backup.run_scheduled") as run_scheduled,
|
||||||
|
patch(
|
||||||
|
"pobsync_backend.management.commands.run_pobsync_backup.run_sql_retention_apply"
|
||||||
|
) as retention_apply,
|
||||||
|
):
|
||||||
|
run_scheduled.return_value = {
|
||||||
|
"ok": True,
|
||||||
|
"dry_run": False,
|
||||||
|
"host": host.host,
|
||||||
|
"snapshot": str(snapshot_dir),
|
||||||
|
"base": None,
|
||||||
|
"rsync": {"exit_code": 0},
|
||||||
|
}
|
||||||
|
retention_apply.side_effect = ConfigError("Deletion blocked by --max-delete=0")
|
||||||
|
|
||||||
|
with self.assertRaises(ConfigError):
|
||||||
|
call_command(
|
||||||
|
"run_pobsync_backup",
|
||||||
|
host.host,
|
||||||
|
prefix=str(Path(tmp) / "home"),
|
||||||
|
prune=True,
|
||||||
|
prune_max_delete=0,
|
||||||
|
stdout=StringIO(),
|
||||||
|
)
|
||||||
|
|
||||||
|
run = BackupRun.objects.get()
|
||||||
|
self.assertEqual(run.status, BackupRun.Status.FAILED)
|
||||||
|
self.assertIsNotNone(run.snapshot)
|
||||||
|
self.assertEqual(run.result["prune"]["ok"], False)
|
||||||
|
self.assertEqual(run.result["prune"]["type"], "ConfigError")
|
||||||
|
self.assertEqual(run.result["prune"]["error"], "Deletion blocked by --max-delete=0")
|
||||||
|
|
||||||
def test_failed_backup_upserts_incomplete_snapshot_record(self) -> None:
|
def test_failed_backup_upserts_incomplete_snapshot_record(self) -> None:
|
||||||
with TemporaryDirectory() as tmp:
|
with TemporaryDirectory() as tmp:
|
||||||
backup_root = Path(tmp) / "backups"
|
backup_root = Path(tmp) / "backups"
|
||||||
|
|||||||
Reference in New Issue
Block a user