From 50eb7cf2f34b69a44386d5c5dc8509083b84c46d Mon Sep 17 00:00:00 2001 From: Peter van Arkel Date: Thu, 21 May 2026 01:10:45 +0200 Subject: [PATCH] (ui) Make retention planning warnings explicit Show keep/delete reasons in the retention plan, surface scheduled prune limit warnings, and explain base snapshot protection before retention is applied. Also surface incomplete snapshots from the retention views without deleting them automatically, so interrupted backups are visible on the dashboard, host detail, and retention plan. --- src/pobsync_backend/retention.py | 22 +++- .../templates/pobsync_backend/base.html | 11 ++ .../templates/pobsync_backend/dashboard.html | 14 +++ .../pobsync_backend/host_detail.html | 24 ++++ .../pobsync_backend/retention_plan.html | 82 +++++++++++++- .../tests/test_sql_retention.py | 3 + src/pobsync_backend/tests/test_views.py | 103 ++++++++++++++++++ src/pobsync_backend/views.py | 47 +++++++- 8 files changed, 297 insertions(+), 9 deletions(-) diff --git a/src/pobsync_backend/retention.py b/src/pobsync_backend/retention.py index 5c52908..e986d8d 100644 --- a/src/pobsync_backend/retention.py +++ b/src/pobsync_backend/retention.py @@ -23,6 +23,7 @@ def run_sql_retention_plan(*, host: str, kind: str, protect_bases: bool) -> dict host_config = _enabled_host_config(host) retention = _retention_for_host(host_config) snapshots = _snapshots_for_retention(host_config=host_config, kind=kind) + incomplete_snapshots = _incomplete_snapshots_for_host(host_config) plan = build_retention_plan( snapshots=snapshots, @@ -36,6 +37,7 @@ def run_sql_retention_plan(*, host: str, kind: str, protect_bases: bool) -> dict keep, reasons = apply_base_protection(snapshots=snapshots, keep=keep, reasons=reasons) delete = [snapshot for snapshot in snapshots if snapshot.dirname not in keep] + keep_items = [snapshot for snapshot in snapshots if snapshot.dirname in keep] return { "ok": True, @@ -45,7 +47,12 @@ def run_sql_retention_plan(*, host: str, kind: str, protect_bases: bool) -> dict "retention": retention, "source": "sql", "keep": sorted(keep), - "delete": [_snapshot_to_delete_item(snapshot) for snapshot in delete], + "keep_items": [_snapshot_to_item(snapshot, reasons=reasons.get(snapshot.dirname, [])) for snapshot in keep_items], + "delete": [_snapshot_to_item(snapshot, reasons=["outside retention policy"]) for snapshot in delete], + "incomplete": [ + _snapshot_to_item(snapshot, reasons=["incomplete snapshot; excluded from retention cleanup"]) + for snapshot in incomplete_snapshots + ], "reasons": reasons, } @@ -146,6 +153,15 @@ def _snapshots_for_retention(*, host_config: HostConfig, kind: str) -> list[Snap return [_snapshot_from_record(record) for record in records] +def _incomplete_snapshots_for_host(host_config: HostConfig) -> list[Snapshot]: + records = ( + SnapshotRecord.objects.filter(host=host_config, kind=SnapshotRecord.Kind.INCOMPLETE) + .select_related("base") + .order_by("-started_at", "dirname") + ) + return [_snapshot_from_record(record) for record in records] + + def _snapshot_from_record(record: SnapshotRecord) -> Snapshot: return Snapshot( kind=record.kind, @@ -173,13 +189,15 @@ def _base_meta_from_record(record: SnapshotRecord) -> dict[str, str] | None: return None -def _snapshot_to_delete_item(snapshot: Snapshot) -> dict[str, Any]: +def _snapshot_to_item(snapshot: Snapshot, *, reasons: list[str]) -> dict[str, Any]: return { "dirname": snapshot.dirname, "kind": snapshot.kind, "path": snapshot.path, "dt": snapshot.dt.isoformat(), "status": snapshot.status, + "reasons": reasons, + "reason": ", ".join(reasons), } diff --git a/src/pobsync_backend/templates/pobsync_backend/base.html b/src/pobsync_backend/templates/pobsync_backend/base.html index 5156766..7cf0dc8 100644 --- a/src/pobsync_backend/templates/pobsync_backend/base.html +++ b/src/pobsync_backend/templates/pobsync_backend/base.html @@ -231,6 +231,17 @@ .host-card-stat.wide { grid-column: 1 / -1; } + .host-card-warning { + background: #fffaf0; + border: 1px solid #e7cf8a; + border-radius: 6px; + color: var(--running); + display: flex; + flex-wrap: wrap; + gap: 8px; + margin-top: 14px; + padding: 10px; + } .messages { display: grid; gap: 8px; margin-bottom: 18px; } .message { background: var(--panel); diff --git a/src/pobsync_backend/templates/pobsync_backend/dashboard.html b/src/pobsync_backend/templates/pobsync_backend/dashboard.html index 37fd93d..9b2b6ba 100644 --- a/src/pobsync_backend/templates/pobsync_backend/dashboard.html +++ b/src/pobsync_backend/templates/pobsync_backend/dashboard.html @@ -118,6 +118,20 @@ + {% if host.retention_warning.has_warning %} +
+ retention + {% if host.retention_warning.prune_exceeded %} + Scheduled prune would delete {{ host.retention_warning.delete_count }} snapshot(s), above max {{ host.retention_warning.max_delete }}. + {% endif %} + {% if host.retention_warning.incomplete_count %} + {{ host.retention_warning.incomplete_count }} incomplete snapshot(s) need review. + {% endif %} + {% if host.retention_warning.error %} + {{ host.retention_warning.error }} + {% endif %} +
+ {% endif %} {% empty %}

No hosts configured yet.

diff --git a/src/pobsync_backend/templates/pobsync_backend/host_detail.html b/src/pobsync_backend/templates/pobsync_backend/host_detail.html index 9180799..e10f681 100644 --- a/src/pobsync_backend/templates/pobsync_backend/host_detail.html +++ b/src/pobsync_backend/templates/pobsync_backend/host_detail.html @@ -68,6 +68,30 @@ + {% if retention_warning.has_warning %} +
+

Retention Warnings

+
+ {% if retention_warning.prune_exceeded %} +
+ Scheduled pruning would delete {{ retention_warning.delete_count }} snapshot(s), above max delete + {{ retention_warning.max_delete }}. Scheduled pruning will refuse this plan until the limit or retention + selection is adjusted. +
+ {% endif %} + {% if retention_warning.incomplete_count %} +
+ {{ retention_warning.incomplete_count }} incomplete snapshot(s) exist. Retention does not delete incomplete + snapshots automatically; inspect them before cleanup. +
+ {% endif %} + {% if retention_warning.error %} +
{{ retention_warning.error }}
+ {% endif %} +
+
+ {% endif %} + {% if effective_config %}

Effective Config

diff --git a/src/pobsync_backend/templates/pobsync_backend/retention_plan.html b/src/pobsync_backend/templates/pobsync_backend/retention_plan.html index 1b1f9ec..0d94a11 100644 --- a/src/pobsync_backend/templates/pobsync_backend/retention_plan.html +++ b/src/pobsync_backend/templates/pobsync_backend/retention_plan.html @@ -18,8 +18,31 @@
Kind
{{ plan.kind }}
Keep
{{ plan.keep|length }}
Would Delete
{{ plan.delete|length }}
+
Scheduled Limit
{{ scheduled_prune_limit|default:"none" }}
+
Incomplete
{{ plan.incomplete|length }}
+ {% if scheduled_prune_exceeded %} +
+

Scheduled Prune Limit

+

+ This plan would delete {{ plan.delete|length }} snapshot(s), which exceeds the scheduled prune limit of + {{ scheduled_prune_limit }}. Scheduled pruning will refuse to apply this plan until the limit or retention + selection is adjusted. +

+
+ {% endif %} + + {% if plan.incomplete %} +
+

Incomplete Snapshots

+

+ {{ plan.incomplete|length }} incomplete snapshot(s) exist for this host. Retention does not delete incomplete + snapshots automatically because they can indicate an interrupted backup that should be inspected first. +

+
+ {% endif %} +

Policy

@@ -28,6 +51,17 @@
Monthly: {{ plan.retention.monthly }}
Yearly: {{ plan.retention.yearly }}
Protect bases: {{ protect_bases|yesno:"yes,no" }}
+
+ {% if protect_bases %} + Base snapshots referenced by kept snapshots are also kept and marked with a base-of reason. + {% else %} + Base snapshots are only kept when they match the regular retention policy. + {% endif %} +
+ {% if schedule %} +
Schedule pruning: {{ schedule.prune|yesno:"enabled,disabled" }}
+
Schedule max delete: {{ schedule.prune_max_delete }}
+ {% endif %}
@@ -40,6 +74,7 @@ Dirname Started Status + Reason Path @@ -50,10 +85,11 @@ {{ snapshot.dirname }} {{ snapshot.dt }} {{ snapshot.status|default:"" }} + {{ snapshot.reason }} {{ snapshot.path }} {% empty %} - Retention would not delete snapshots for this selection. + Retention would not delete snapshots for this selection. {% endfor %} @@ -71,7 +107,7 @@ {{ apply_form.max_delete.errors }} {{ apply_form.max_delete }} -
Must be at least the number of snapshots shown in Would Delete.
+
Must be at least {{ plan.delete|length }} for the snapshots shown in Would Delete.
@@ -99,20 +135,54 @@ + + + - {% for dirname, reasons in plan.reasons.items %} + {% for snapshot in plan.keep_items %} - - + + + + + {% empty %} - + {% endfor %}
Kind DirnameStartedStatus Reasons
{{ dirname }}{{ reasons|join:", " }}{{ snapshot.kind }}{{ snapshot.dirname }}{{ snapshot.dt }}{{ snapshot.status|default:"" }}{{ snapshot.reason }}
No snapshots matched this retention selection.
No snapshots matched this retention selection.
+ + {% if plan.incomplete %} +
+

Incomplete Snapshots

+ + + + + + + + + + + + {% for snapshot in plan.incomplete %} + + + + + + + + {% endfor %} + +
DirnameStartedStatusReasonPath
{{ snapshot.dirname }}{{ snapshot.dt }}{{ snapshot.status|default:"" }}{{ snapshot.reason }}{{ snapshot.path }}
+
+ {% endif %} {% endblock %} diff --git a/src/pobsync_backend/tests/test_sql_retention.py b/src/pobsync_backend/tests/test_sql_retention.py index 32302ab..33e1c86 100644 --- a/src/pobsync_backend/tests/test_sql_retention.py +++ b/src/pobsync_backend/tests/test_sql_retention.py @@ -32,7 +32,10 @@ class SqlRetentionTests(TestCase): self.assertEqual(plan["source"], "sql") self.assertEqual(plan["keep"], [new.dirname]) + self.assertEqual([item["dirname"] for item in plan["keep_items"]], [new.dirname]) self.assertEqual([item["dirname"] for item in plan["delete"]], [old.dirname]) + self.assertEqual(plan["delete"][0]["reason"], "outside retention policy") + self.assertEqual(plan["incomplete"], []) def test_plan_can_protect_base_snapshot_from_sql_relation(self) -> None: host = HostConfig.objects.create( diff --git a/src/pobsync_backend/tests/test_views.py b/src/pobsync_backend/tests/test_views.py index 9c66eee..69f9dae 100644 --- a/src/pobsync_backend/tests/test_views.py +++ b/src/pobsync_backend/tests/test_views.py @@ -99,6 +99,35 @@ class ViewTests(TestCase): self.assertContains(response, "manual") self.assertContains(response, "1000") + def test_dashboard_surfaces_retention_warnings(self) -> None: + self.client.force_login(self.staff_user) + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + ScheduleConfig.objects.create(host=host, cron_expr="15 2 * * *", enabled=True, prune=True, prune_max_delete=1) + self._snapshot(host, "20260517-021500Z__OLDSNP1") + self._snapshot(host, "20260518-021500Z__OLDSNP2") + self._snapshot(host, "20260519-021500Z__NEWSNAP") + SnapshotRecord.objects.create( + host=host, + kind=SnapshotRecord.Kind.INCOMPLETE, + dirname="20260519-031500Z__BROKEN01", + path=f"/backups/{host.host}/.incomplete/20260519-031500Z__BROKEN01", + status="failed", + started_at=datetime(2026, 5, 19, 3, 15, tzinfo=timezone.utc), + ) + + response = self.client.get(reverse("dashboard")) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Scheduled prune would delete 2 snapshot(s), above max 1.") + self.assertContains(response, "1 incomplete snapshot(s) need review.") + def test_dashboard_links_latest_snapshot_for_each_host(self) -> None: self.client.force_login(self.staff_user) host = HostConfig.objects.create(host="web-01", address="web-01.example.test") @@ -1351,6 +1380,30 @@ class ViewTests(TestCase): self.assertContains(response, new_snapshot.dirname) self.assertContains(response, "newest") self.assertContains(response, "Would Delete") + self.assertContains(response, "outside retention policy") + + def test_retention_plan_warns_when_scheduled_prune_limit_is_exceeded(self) -> None: + self.client.force_login(self.staff_user) + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + ScheduleConfig.objects.create(host=host, cron_expr="15 2 * * *", enabled=True, prune=True, prune_max_delete=1) + self._snapshot(host, "20260517-021500Z__OLDSNP1") + self._snapshot(host, "20260518-021500Z__OLDSNP2") + self._snapshot(host, "20260519-021500Z__NEWSNAP") + + response = self.client.get(reverse("host_retention_plan", args=[host.host])) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Scheduled Prune Limit") + self.assertContains(response, "would delete 2 snapshot(s)") + self.assertContains(response, "scheduled prune limit of") + self.assertContains(response, "Schedule max delete: 1") def test_retention_plan_can_enable_base_protection(self) -> None: self.client.force_login(self.staff_user) @@ -1371,8 +1424,58 @@ class ViewTests(TestCase): self.assertEqual(response.status_code, 200) self.assertContains(response, "Protect bases: yes") + self.assertContains(response, "Base snapshots referenced by kept snapshots") self.assertContains(response, f"base-of:{child.dirname}") + def test_retention_plan_surfaces_incomplete_snapshots_without_deleting_them(self) -> None: + self.client.force_login(self.staff_user) + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + self._snapshot(host, "20260518-021500Z__OLDSNAP") + self._snapshot(host, "20260519-021500Z__NEWSNAP") + SnapshotRecord.objects.create( + host=host, + kind=SnapshotRecord.Kind.INCOMPLETE, + dirname="20260519-031500Z__BROKEN01", + path=f"/backups/{host.host}/.incomplete/20260519-031500Z__BROKEN01", + status="failed", + started_at=datetime(2026, 5, 19, 3, 15, tzinfo=timezone.utc), + ) + + response = self.client.get(reverse("host_retention_plan", args=[host.host])) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Incomplete Snapshots") + self.assertContains(response, "20260519-031500Z__BROKEN01") + self.assertContains(response, "excluded from retention cleanup") + + def test_host_detail_surfaces_retention_warnings(self) -> None: + self.client.force_login(self.staff_user) + host = HostConfig.objects.create( + host="web-01", + address="web-01.example.test", + retention_daily=0, + retention_weekly=0, + retention_monthly=0, + retention_yearly=0, + ) + ScheduleConfig.objects.create(host=host, cron_expr="15 2 * * *", enabled=True, prune=True, prune_max_delete=1) + self._snapshot(host, "20260517-021500Z__OLDSNP1") + self._snapshot(host, "20260518-021500Z__OLDSNP2") + self._snapshot(host, "20260519-021500Z__NEWSNAP") + + response = self.client.get(reverse("host_detail", args=[host.host])) + + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Retention Warnings") + self.assertContains(response, "Scheduled pruning would delete 2 snapshot(s), above max delete") + def test_retention_plan_rejects_invalid_kind(self) -> None: self.client.force_login(self.staff_user) host = HostConfig.objects.create(host="web-01", address="web-01.example.test") diff --git a/src/pobsync_backend/views.py b/src/pobsync_backend/views.py index 524ea23..4ad50ad 100644 --- a/src/pobsync_backend/views.py +++ b/src/pobsync_backend/views.py @@ -55,6 +55,7 @@ def dashboard(request): .first() ) host_config.next_run_at = _next_run_for_host(host_config) + host_config.retention_warning = _retention_warning_for_host(host_config, _schedule_for_host(host_config)) stats_summary = collect_dashboard_stats(hosts=hosts, global_config=global_config) context = { "hosts": hosts, @@ -274,6 +275,7 @@ def host_detail(request, host: str): context = { "host": host_config, "schedule": schedule, + "retention_warning": _retention_warning_for_host(host_config, schedule), "next_run_at": _next_run_for_schedule(schedule, host_config), "scheduler_timezone": timezone.get_current_timezone_name(), "discovery": inspect_snapshot_discovery(host=host_config), @@ -526,17 +528,23 @@ def host_retention_plan(request, host: str): except PobsyncError as exc: messages.error(request, str(exc)) return redirect("host_detail", host=host_config.host) + schedule = _schedule_for_host(host_config) + scheduled_prune_limit = schedule.prune_max_delete if schedule and schedule.prune else None + delete_count = len(plan["delete"]) context = { "host": host_config, "kind": kind, "protect_bases": protect_bases, "plan": plan, + "schedule": schedule, + "scheduled_prune_limit": scheduled_prune_limit, + "scheduled_prune_exceeded": scheduled_prune_limit is not None and delete_count > scheduled_prune_limit, "apply_form": RetentionApplyForm( host_name=host_config.host, initial={ "kind": kind, "protect_bases": protect_bases, - "max_delete": len(plan["delete"]), + "max_delete": delete_count, }, ), } @@ -652,6 +660,43 @@ def _next_run_for_schedule(schedule: ScheduleConfig | None, host_config: HostCon return None +def _retention_warning_for_host(host_config: HostConfig, schedule: ScheduleConfig | None) -> dict[str, object]: + incomplete_count = host_config.snapshots.filter(kind=SnapshotRecord.Kind.INCOMPLETE).count() + warning: dict[str, object] = { + "has_warning": incomplete_count > 0, + "incomplete_count": incomplete_count, + } + if schedule is None or not schedule.prune or not host_config.enabled: + return warning + try: + plan = run_sql_retention_plan( + host=host_config.host, + kind="scheduled", + protect_bases=bool(schedule.prune_protect_bases), + ) + except PobsyncError as exc: + warning.update( + { + "has_warning": True, + "error": str(exc), + } + ) + return warning + + delete_count = len(plan.get("delete") or []) + warning.update( + { + "delete_count": delete_count, + "max_delete": schedule.prune_max_delete, + "protect_bases": bool(schedule.prune_protect_bases), + "prune_exceeded": delete_count > schedule.prune_max_delete, + } + ) + if warning["prune_exceeded"]: + warning["has_warning"] = True + return warning + + def _default_schedule_initial() -> dict[str, object]: return { "cron_expr": "15 2 * * *",