@@ -55,9 +128,24 @@
.dh-count { font-variant-numeric: tabular-nums; font-weight: 600; }
.dh-count-ok { color: #2e7d32; }
.dh-count-warn { color: #b54708; }
+ /* Backup status panel (#1443) */
+ .dh-section-heading { font-size: 15px; margin: 24px 0 8px; }
+ .dh-backup { border-collapse: collapse; margin-bottom: 8px; max-width: 60em; }
+ .dh-backup caption {
+ caption-side: bottom;
+ text-align: left;
+ font-size: 12px;
+ color: #777;
+ padding-top: 6px;
+ max-width: 60em;
+ }
+ .dh-backup th, .dh-backup td { padding: 6px 12px 6px 0; text-align: left; vertical-align: top; }
+ .dh-backup th { white-space: nowrap; font-weight: 600; }
+ .dh-backup-error { white-space: pre-wrap; word-break: break-word; }
@media (prefers-color-scheme: dark) {
.dh-intro { color: #bbb; }
.dh-desc { color: #999; }
+ .dh-backup caption { color: #999; }
.dh-count-ok { color: #7bc47f; }
.dh-count-warn { color: #e8a14b; }
}
diff --git a/website/templates/admin/index.html b/website/templates/admin/index.html
index 9341d98e..d8ed7836 100644
--- a/website/templates/admin/index.html
+++ b/website/templates/admin/index.html
@@ -28,6 +28,29 @@
{% endif %}
+{% comment %}
+ Unhealthy-backup warning (#1443). Shown ONLY when something is actually
+ wrong — a permanently-green banner turns into invisible chrome, and this one
+ needs to be read. BACKUP_STATUS comes from MakeabilityLabAdminSite.each_context
+ and is only populated for superusers, so this is inert for everyone else.
+ 'should_warn' rather than 'not healthy' so that a local dev machine with no
+ db-backup service running doesn't nag; the servers still warn. The scriptable
+ equivalent is /version.json's 'backup_ok'.
+{% endcomment %}
+{% if BACKUP_STATUS.should_warn %}
+
+
⚠️
+
Warning: database backups are not healthy.
+
{{ BACKUP_STATUS.problem }}
+ {% if BACKUP_STATUS.last_backup_file %}
+ Most recent dump: {{ BACKUP_STATUS.last_backup_file }}
+ ({{ BACKUP_STATUS.size_display }}).
+ {% endif %}
+ See the Data Health dashboard
+ for detail, and docs/BACKUPS.md for how to investigate.
+
+{% endif %}
+
{% if user.is_superuser %}
🩺
diff --git a/website/tests/test_backup_status.py b/website/tests/test_backup_status.py
new file mode 100644
index 00000000..36ab8024
--- /dev/null
+++ b/website/tests/test_backup_status.py
@@ -0,0 +1,213 @@
+"""
+Tests for database-backup health reporting (#1443).
+
+The end-to-end proof that a dump actually *restores* lives in
+``scripts/test_backup_restore.sh`` and ``scripts/test_backup_restore_django.sh``
+(they need Docker, so they can't run in this suite). These tests cover the
+Django half: reading the sidecar's status file and reporting it correctly.
+
+The most important behavior pinned here is that a broken or missing status file
+degrades to "unknown" instead of raising. This code runs inside
+``each_context``, so an exception would take the entire admin down — the exact
+opposite of what a backup-health feature should do.
+"""
+
+import json
+import os
+import tempfile
+from datetime import datetime, timedelta, timezone
+
+from django.test import SimpleTestCase, override_settings
+
+from website.utils.backup_status import format_bytes, get_backup_status
+
+NOW = datetime(2026, 8, 7, 12, 0, 0, tzinfo=timezone.utc)
+
+
+def _status_payload(**overrides):
+ """A well-formed status.json body, matching what pg_backup.sh writes."""
+ payload = {
+ 'schema_version': 1,
+ 'database': 'makeability',
+ 'last_attempt_at': '2026-08-07T03:00:00Z',
+ 'last_attempt_ok': True,
+ 'error': None,
+ 'last_backup_at': '2026-08-07T03:00:00Z',
+ 'last_backup_file': 'makeability-2026-08-07.sql.gz',
+ 'last_backup_bytes': 12684,
+ 'oldest_backup_at': '2026-07-25T03:00:00Z',
+ 'backup_count': 14,
+ 'retention_days': 14,
+ }
+ payload.update(overrides)
+ return payload
+
+
+class BackupStatusFileTests(SimpleTestCase):
+ """Reading and interpreting the sidecar's status file."""
+
+ def setUp(self):
+ self.tmp = tempfile.TemporaryDirectory()
+ self.addCleanup(self.tmp.cleanup)
+ self.path = os.path.join(self.tmp.name, 'status.json')
+
+ def _write(self, payload):
+ with open(self.path, 'w', encoding='utf-8') as handle:
+ if isinstance(payload, str):
+ handle.write(payload)
+ else:
+ json.dump(payload, handle)
+ return self.path
+
+ def test_healthy_backup_is_reported_healthy(self):
+ self._write(_status_payload())
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertTrue(status['available'])
+ self.assertTrue(status['ok'])
+ self.assertTrue(status['healthy'])
+ self.assertFalse(status['stale'])
+ self.assertFalse(status['should_warn'])
+ self.assertIsNone(status['problem'])
+ self.assertEqual(status['age_hours'], 9.0)
+ self.assertEqual(status['backup_count'], 14)
+ self.assertEqual(status['last_backup_file'], 'makeability-2026-08-07.sql.gz')
+
+ @override_settings(BACKUP_STALE_AFTER_HOURS=36)
+ def test_backup_older_than_threshold_is_stale(self):
+ old = (NOW - timedelta(hours=50)).strftime('%Y-%m-%dT%H:%M:%SZ')
+ self._write(_status_payload(last_backup_at=old))
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertTrue(status['stale'])
+ self.assertFalse(status['healthy'])
+ self.assertTrue(status['should_warn'])
+ self.assertIn('50 hours old', status['problem'])
+
+ @override_settings(BACKUP_STALE_AFTER_HOURS=36)
+ def test_backup_inside_threshold_is_not_stale(self):
+ # 30h is over a day old but under the threshold: one missed nightly run
+ # (or clock skew) must not raise an alarm, or the warning gets ignored.
+ recent = (NOW - timedelta(hours=30)).strftime('%Y-%m-%dT%H:%M:%SZ')
+ self._write(_status_payload(last_backup_at=recent))
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertFalse(status['stale'])
+ self.assertTrue(status['healthy'])
+
+ def test_failed_attempt_is_reported_with_its_error(self):
+ # The case that would otherwise look identical to "hasn't run yet".
+ self._write(_status_payload(
+ last_attempt_ok=False,
+ error='pg_dump failed (exit 1): connection refused',
+ ))
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertFalse(status['ok'])
+ self.assertFalse(status['healthy'])
+ self.assertTrue(status['should_warn'])
+ self.assertIn('connection refused', status['problem'])
+
+ def test_status_with_no_successful_backup_is_not_healthy(self):
+ # Sidecar ran, reported success, but no dump file exists yet. That is
+ # not a healthy state even though last_attempt_ok is true.
+ self._write(_status_payload(last_backup_at=None, last_backup_file=None,
+ backup_count=0))
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertFalse(status['healthy'])
+ self.assertTrue(status['stale'])
+ self.assertIn('No successful backup', status['problem'])
+
+ def test_missing_file_does_not_raise(self):
+ status = get_backup_status(status_file=os.path.join(self.tmp.name, 'nope.json'),
+ now=NOW)
+ self.assertFalse(status['available'])
+ self.assertFalse(status['healthy'])
+ self.assertIsNone(status['age_hours'])
+ self.assertIn('No backup status file', status['problem'])
+
+ def test_malformed_json_does_not_raise(self):
+ # A status file caught mid-write, or truncated. This runs inside
+ # each_context; raising here would break every admin page.
+ self._write('{"last_attempt_ok": true, "last_backu')
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertFalse(status['available'])
+ self.assertFalse(status['healthy'])
+ self.assertTrue(status['should_warn'])
+ self.assertIn('unreadable', status['problem'])
+
+ def test_json_that_is_not_an_object_does_not_raise(self):
+ self._write('["not", "a", "dict"]')
+ status = get_backup_status(status_file=self.path, now=NOW)
+ self.assertFalse(status['available'])
+ self.assertIn('unreadable', status['problem'])
+
+ def test_unparseable_timestamp_does_not_raise(self):
+ self._write(_status_payload(last_backup_at='not-a-timestamp'))
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertTrue(status['available'])
+ self.assertIsNone(status['age_hours'])
+ self.assertFalse(status['healthy'])
+
+ def test_missing_keys_do_not_raise(self):
+ self._write({'last_attempt_ok': True})
+ status = get_backup_status(status_file=self.path, now=NOW)
+
+ self.assertTrue(status['available'])
+ self.assertIsNone(status['last_backup_file'])
+ self.assertFalse(status['healthy'])
+
+ def test_every_documented_key_is_always_present(self):
+ # Templates render these unconditionally, so the shape must not depend
+ # on which failure path produced it.
+ expected = {
+ 'available', 'healthy', 'ok', 'stale', 'age_hours', 'last_backup_at',
+ 'last_backup_file', 'last_backup_bytes', 'last_attempt_at',
+ 'oldest_backup_at', 'backup_count', 'retention_days', 'database',
+ 'error', 'problem', 'should_warn', 'size_display', 'status_file',
+ }
+ self._write(_status_payload())
+ self.assertTrue(expected.issubset(get_backup_status(self.path, NOW).keys()))
+ os.remove(self.path)
+ self.assertTrue(expected.issubset(get_backup_status(self.path, NOW).keys()))
+
+
+class BackupWarningSuppressionTests(SimpleTestCase):
+ """
+ A missing status file means different things in different environments.
+
+ Keyed off DJANGO_ENV, not DEBUG: the test server runs DEBUG=True and still
+ needs to be told its backups aren't running.
+ """
+
+ def setUp(self):
+ self.tmp = tempfile.TemporaryDirectory()
+ self.addCleanup(self.tmp.cleanup)
+ self.missing = os.path.join(self.tmp.name, 'absent.json')
+
+ @override_settings(DJANGO_ENV='PROD')
+ def test_missing_file_warns_on_prod(self):
+ self.assertTrue(get_backup_status(self.missing, NOW)['should_warn'])
+
+ @override_settings(DJANGO_ENV='TEST')
+ def test_missing_file_warns_on_test_server_despite_debug(self):
+ self.assertTrue(get_backup_status(self.missing, NOW)['should_warn'])
+
+ @override_settings(DJANGO_ENV=None)
+ def test_missing_file_is_quiet_in_local_dev(self):
+ # A developer not running the db-backup service shouldn't be nagged.
+ self.assertFalse(get_backup_status(self.missing, NOW)['should_warn'])
+
+
+class FormatBytesTests(SimpleTestCase):
+ def test_formats_common_sizes(self):
+ self.assertEqual(format_bytes(512), '512 B')
+ self.assertEqual(format_bytes(12684), '12.4 KB')
+ self.assertEqual(format_bytes(5 * 1024 * 1024), '5.0 MB')
+
+ def test_handles_missing_and_garbage(self):
+ self.assertEqual(format_bytes(None), '—')
+ self.assertEqual(format_bytes('nonsense'), '—')
diff --git a/website/tests/test_version_endpoint.py b/website/tests/test_version_endpoint.py
index 86682451..255c81cc 100644
--- a/website/tests/test_version_endpoint.py
+++ b/website/tests/test_version_endpoint.py
@@ -15,6 +15,8 @@
import json
import os
+import tempfile
+from datetime import datetime, timedelta, timezone
from django.test import SimpleTestCase, override_settings
from django.urls import resolve, reverse
@@ -68,6 +70,60 @@ def test_payload_from_settings_and_no_store_header(self):
# Which rotation handler is live (#1439) -- the only remote way to see
# that the multiprocess-safe handler degraded.
self.assertIn("log_rotation", data)
+ # Backup health (#1443) -- the dumps live in a Docker volume on a host
+ # with no shell, so these fields are the only way to check backups
+ # without logging into /admin.
+ self.assertIn("backup_ok", data)
+ self.assertIn("last_backup_at", data)
+ self.assertIn("backup_age_hours", data)
+ self.assertIn("backup_count", data)
+
+ def test_reports_backup_health_from_status_file(self):
+ """A healthy status file surfaces as ``backup_ok: true`` plus its age."""
+ with tempfile.TemporaryDirectory() as tmp:
+ path = os.path.join(tmp, "status.json")
+ now = datetime.now(timezone.utc)
+ with open(path, "w", encoding="utf-8") as handle:
+ json.dump({
+ "last_attempt_ok": True,
+ "error": None,
+ "last_backup_at": now.strftime("%Y-%m-%dT%H:%M:%SZ"),
+ "last_backup_file": "makeability-2026-08-07.sql.gz",
+ "last_backup_bytes": 12684,
+ "backup_count": 14,
+ "retention_days": 14,
+ }, handle)
+ with override_settings(BACKUP_STATUS_FILE=path):
+ data = json.loads(self.client.get("/version/").content)
+
+ self.assertTrue(data["backup_ok"])
+ self.assertEqual(data["backup_count"], 14)
+ self.assertIsNotNone(data["last_backup_at"])
+ self.assertLess(data["backup_age_hours"], 1)
+
+ def test_reports_unhealthy_backup_without_leaking_internals(self):
+ """A stale backup reports ``backup_ok: false``.
+
+ The endpoint is public, so it deliberately carries no error text and no
+ filesystem paths -- just enough for an external check to go red.
+ """
+ with tempfile.TemporaryDirectory() as tmp:
+ path = os.path.join(tmp, "status.json")
+ stale = datetime.now(timezone.utc) - timedelta(hours=100)
+ with open(path, "w", encoding="utf-8") as handle:
+ json.dump({
+ "last_attempt_ok": False,
+ "error": "pg_dump failed (exit 1): connection refused",
+ "last_backup_at": stale.strftime("%Y-%m-%dT%H:%M:%SZ"),
+ "backup_count": 3,
+ }, handle)
+ with override_settings(BACKUP_STATUS_FILE=path):
+ data = json.loads(self.client.get("/version/").content)
+
+ self.assertFalse(data["backup_ok"])
+ self.assertNotIn("backup_problem", data)
+ self.assertNotIn("connection refused", json.dumps(data))
+ self.assertNotIn(path, json.dumps(data))
def test_server_reflects_wsgi_server_software(self):
# The view reports request.META["SERVER_SOFTWARE"] verbatim; on the real
diff --git a/website/utils/backup_status.py b/website/utils/backup_status.py
new file mode 100644
index 00000000..91d322ee
--- /dev/null
+++ b/website/utils/backup_status.py
@@ -0,0 +1,179 @@
+"""
+Read the database-backup status file written by the ``db-backup`` sidecar (#1443).
+
+The sidecar (``scripts/pg_backup.sh``, wired up in ``docker-compose.yml``) writes
+a small JSON file after every pass — successful or not — to a volume this
+container mounts read-only. This module turns that file into a dict the admin
+dashboard, the Data Health page, and ``/version.json`` all render.
+
+Why this exists at all: the dumps land in a Docker named volume on a host with
+no shell access, inside a ``PGDATA`` this container cannot even traverse. The
+status file is the only channel through which backup health is observable. That
+makes *this* module's failure modes important — it must degrade to "unknown"
+rather than raise, or a missing backup would take the admin down with it.
+
+Usage::
+
+ from website.utils.backup_status import get_backup_status
+ status = get_backup_status()
+ if not status['healthy']:
+ print(status['problem']) # e.g. "Last backup is 51 hours old"
+"""
+
+import json
+import logging
+import os
+from datetime import datetime, timezone
+
+from django.conf import settings
+
+logger = logging.getLogger(__name__)
+
+#: Shape returned when the status file can't be read or understood at all.
+#: Deliberately not an exception: a broken status file must not break /admin.
+_UNKNOWN = {
+ 'available': False,
+ 'healthy': False,
+ 'ok': None,
+ 'stale': False,
+ 'age_hours': None,
+ 'last_backup_at': None,
+ 'last_backup_file': None,
+ 'last_backup_bytes': None,
+ 'last_attempt_at': None,
+ 'oldest_backup_at': None,
+ 'backup_count': None,
+ 'retention_days': None,
+ 'database': None,
+ 'error': None,
+ # Kept in the unknown shape too so templates can render every key
+ # unconditionally instead of guarding each one.
+ 'problem': None,
+ 'should_warn': False,
+ 'size_display': '—',
+ 'status_file': None,
+}
+
+
+def _parse_iso(value):
+ """Parse an ISO-8601 UTC timestamp from the status file, or return None.
+
+ The sidecar writes ``...Z``; ``fromisoformat`` only learned to accept the
+ ``Z`` suffix in Python 3.11, so normalize it rather than relying on that.
+ """
+ if not value or not isinstance(value, str):
+ return None
+ try:
+ return datetime.fromisoformat(value.replace('Z', '+00:00'))
+ except (ValueError, TypeError):
+ return None
+
+
+def format_bytes(num):
+ """Render a byte count as a short human string ('12.4 MB'), or '—'."""
+ if num is None:
+ return '—'
+ try:
+ num = float(num)
+ except (TypeError, ValueError):
+ return '—'
+ for unit in ('B', 'KB', 'MB', 'GB'):
+ if abs(num) < 1024.0 or unit == 'GB':
+ return f"{num:.0f} {unit}" if unit == 'B' else f"{num:.1f} {unit}"
+ num /= 1024.0
+ return f"{num:.1f} GB"
+
+
+def get_backup_status(status_file=None, now=None):
+ """
+ Return a dict describing database-backup health. Never raises.
+
+ Args:
+ status_file: path to status.json. Defaults to ``settings.BACKUP_STATUS_FILE``.
+ now: aware ``datetime`` used as "now" (for tests). Defaults to real UTC now.
+
+ Returns:
+ dict with at least ``available`` (was the file readable), ``healthy``
+ (readable AND last attempt succeeded AND not stale), ``problem`` (a
+ one-line human explanation when not healthy, else None), plus the
+ parsed fields and a few display-formatted extras.
+
+ A note on the three unhealthy states, which mean different things:
+ * ``available=False`` — no status file. Either backups have never run on
+ this host, or the volume isn't mounted. Expected in local dev.
+ * ``ok=False`` — the sidecar ran and *failed*; ``error`` says why.
+ * ``stale=True`` — the last pass may have succeeded, but the newest dump
+ is older than ``BACKUP_STALE_AFTER_HOURS``, so backups have stopped.
+ """
+ if status_file is None:
+ status_file = getattr(settings, 'BACKUP_STATUS_FILE', None)
+ if now is None:
+ now = datetime.now(timezone.utc)
+
+ status = dict(_UNKNOWN)
+ status['status_file'] = status_file
+
+ if not status_file or not os.path.exists(status_file):
+ status['problem'] = 'No backup status file found.'
+ # On a server this is a real problem — the sidecar never ran, or the
+ # status volume isn't mounted. In local dev, where a developer may
+ # simply not be running the db-backup service, it is expected and must
+ # not nag. Keyed off DJANGO_ENV rather than DEBUG on purpose: the test
+ # server runs DEBUG=True and still needs to be warned.
+ status['should_warn'] = getattr(settings, 'DJANGO_ENV', None) in ('PROD', 'TEST')
+ return status
+
+ try:
+ with open(status_file, 'r', encoding='utf-8') as handle:
+ data = json.load(handle)
+ if not isinstance(data, dict):
+ raise ValueError('status file is not a JSON object')
+ except (OSError, ValueError) as exc:
+ # Truncated file, bad JSON, permissions — all reported the same way.
+ # A file that exists but can't be parsed is always worth warning about.
+ logger.warning("Could not read backup status file %s: %s", status_file, exc)
+ status['problem'] = f'Backup status file is unreadable ({exc}).'
+ status['should_warn'] = True
+ return status
+
+ status['available'] = True
+ status['ok'] = bool(data.get('last_attempt_ok'))
+ status['error'] = data.get('error') or None
+ status['database'] = data.get('database')
+ status['last_backup_file'] = data.get('last_backup_file')
+ status['last_backup_bytes'] = data.get('last_backup_bytes')
+ status['backup_count'] = data.get('backup_count')
+ status['retention_days'] = data.get('retention_days')
+ status['last_backup_at'] = _parse_iso(data.get('last_backup_at'))
+ status['last_attempt_at'] = _parse_iso(data.get('last_attempt_at'))
+ status['oldest_backup_at'] = _parse_iso(data.get('oldest_backup_at'))
+ status['size_display'] = format_bytes(status['last_backup_bytes'])
+
+ stale_after = getattr(settings, 'BACKUP_STALE_AFTER_HOURS', 36)
+ if status['last_backup_at'] is not None:
+ age = (now - status['last_backup_at']).total_seconds() / 3600.0
+ status['age_hours'] = round(age, 1)
+ status['stale'] = age > stale_after
+ else:
+ # A readable status file that names no dump means nothing has ever been
+ # backed up successfully here — treat that as stale, not as healthy.
+ status['stale'] = True
+
+ status['healthy'] = bool(status['ok']) and not status['stale']
+
+ # A status file exists, so whatever it says is worth showing everywhere,
+ # local dev included: a failing sidecar is a bug wherever it happens.
+ status['should_warn'] = not status['healthy']
+
+ if status['healthy']:
+ status['problem'] = None
+ elif not status['ok']:
+ status['problem'] = f"Last backup attempt failed: {status['error'] or 'unknown error'}"
+ elif status['age_hours'] is None:
+ status['problem'] = 'No successful backup has been recorded yet.'
+ else:
+ status['problem'] = (
+ f"Last backup is {status['age_hours']:.0f} hours old "
+ f"(expected at most {stale_after})."
+ )
+ return status
diff --git a/website/views/version.py b/website/views/version.py
index e790e7b0..562ee0f5 100644
--- a/website/views/version.py
+++ b/website/views/version.py
@@ -63,6 +63,8 @@
from django.conf import settings
from django.http import JsonResponse
+from website.utils.backup_status import get_backup_status
+
# Module logger (configured in settings.LOGGING).
_logger = logging.getLogger(__name__)
@@ -104,6 +106,7 @@ def version(request, format=None):
wrapper applied in ``website/urls.py`` doesn't choke on the suffixed route.
"""
build_info = _read_build_info()
+ backup = get_backup_status()
payload = {
"version": settings.ML_WEBSITE_VERSION,
"description": settings.ML_WEBSITE_VERSION_DESCRIPTION,
@@ -122,6 +125,18 @@ def version(request, format=None):
# "ConcurrentRotatingFileHandler" means the multiprocess-safe handler
# degraded and Gunicorn's workers can race on rollover again.
"log_rotation": settings.LOG_ROTATION,
+ # Database backup health (#1443). The nightly dump lands in a Docker
+ # volume on a host with no shell, so this is the only way to check it
+ # without logging into /admin. backup_ok false means either the last
+ # pass failed or the newest dump has gone stale -- backup_problem says
+ # which. No paths or error internals beyond that: this endpoint is
+ # public.
+ "backup_ok": backup["healthy"],
+ "last_backup_at": (
+ backup["last_backup_at"].isoformat() if backup["last_backup_at"] else None
+ ),
+ "backup_age_hours": backup["age_hours"],
+ "backup_count": backup["backup_count"],
}
response = JsonResponse(payload)
response["Cache-Control"] = "no-store"
From faa8cb6f9047bc6e68ab88a00162a657b5d3681f Mon Sep 17 00:00:00 2001
From: Jon Froehlich
Date: Fri, 7 Aug 2026 10:39:34 -0700
Subject: [PATCH 2/3] Test retention pruning and bump to 2.33.0 (#1443)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The prune path had no coverage, including the guard that keeps the newest
dump regardless of age — the one that stops a long-failing backup from
ending with pruning deleting the last good restore point. Both are now
exercised against a scratch volume with artificially aged files.
---
makeabilitylab/settings.py | 4 ++--
scripts/test_backup_restore.sh | 44 +++++++++++++++++++++++++++++++++-
2 files changed, 45 insertions(+), 3 deletions(-)
diff --git a/makeabilitylab/settings.py b/makeabilitylab/settings.py
index 98f2bed8..bf5c2d82 100644
--- a/makeabilitylab/settings.py
+++ b/makeabilitylab/settings.py
@@ -87,8 +87,8 @@
SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https')
# Makeability Lab Global Variables, including Makeability Lab version
-ML_WEBSITE_VERSION = "2.32.1" # Keep this updated with each release and also change the short description below
-ML_WEBSITE_VERSION_DESCRIPTION = "debug.log rotation is now multiprocess-safe (concurrent-log-handler): Gunicorn's three workers previously raced on rollover and silently lost log records (#1439)."
+ML_WEBSITE_VERSION = "2.33.0" # Keep this updated with each release and also change the short description below
+ML_WEBSITE_VERSION_DESCRIPTION = "The database now writes a nightly pg_dump into its own volume, so the infrastructure team's snapshots always contain a consistent restore point. Backup health shows on this dashboard and /version.json (#1443)."
DATE_MAKEABILITYLAB_FORMED = datetime.date(2012, 1, 1) # Date Makeability Lab was formed
MAX_BANNERS = 7 # Maximum number of banners on a page
diff --git a/scripts/test_backup_restore.sh b/scripts/test_backup_restore.sh
index 7ed2be0e..35b3f762 100755
--- a/scripts/test_backup_restore.sh
+++ b/scripts/test_backup_restore.sh
@@ -64,7 +64,7 @@ assert_true() {
# and at that point WORK_DIR is still needed.
cleanup_docker() {
docker rm -f "$DB_CONTAINER" >/dev/null 2>&1
- docker volume rm -f "$DATA_VOL" "$STATUS_VOL" "$DATA_VOL-dirty" >/dev/null 2>&1
+ docker volume rm -f "$DATA_VOL" "$STATUS_VOL" "$DATA_VOL-dirty" "$PROJECT-scratch" >/dev/null 2>&1
docker network rm "$NET" >/dev/null 2>&1
}
cleanup() { cleanup_docker; rm -rf "$WORK_DIR"; }
@@ -238,6 +238,48 @@ assert_eq "existing dump survived the failed pass" "1" "$(status_field backup_co
# Restore a good status for the rest of the run.
run_backup_pass >/dev/null 2>&1
+# ---------------------------------------------------------------------------
+step "Retention pruning"
+# ---------------------------------------------------------------------------
+# Run against a scratch directory via BACKUP_DIR so this can age files freely
+# without disturbing the real dump the restore test below depends on. No
+# database is needed: when today's dump already exists the pass is prune-only.
+prune_pass() {
+ # prune_pass
+ docker run --rm -v "$PROJECT-scratch:/scratch" "$IMAGE" bash -c "
+ rm -rf /scratch/pg_backups /scratch/status; mkdir -p /scratch/pg_backups; $2"
+ docker run --rm \
+ -e BACKUP_DIR=/scratch/pg_backups -e STATUS_DIR=/scratch/status \
+ -e BACKUP_RETENTION_DAYS="$1" -e PGDATABASE="$DB_NAME" \
+ -v "$PROJECT-scratch:/scratch" \
+ -v "$SCRIPT_DIR:/backup-scripts:ro" \
+ "$IMAGE" bash /backup-scripts/pg_backup.sh >/dev/null 2>&1
+ docker run --rm -v "$PROJECT-scratch:/scratch" "$IMAGE" \
+ sh -c 'ls -1 /scratch/pg_backups 2>/dev/null | sort | tr "\n" " "'
+}
+
+TODAY_FILE="$DB_NAME-$TODAY.sql.gz"
+REMAINING="$(prune_pass 14 "
+ touch -d '40 days ago' /scratch/pg_backups/$DB_NAME-old-40.sql.gz
+ touch -d '20 days ago' /scratch/pg_backups/$DB_NAME-old-20.sql.gz
+ touch -d '5 days ago' /scratch/pg_backups/$DB_NAME-recent-5.sql.gz
+ touch /scratch/pg_backups/$TODAY_FILE")"
+# Listing is `ls | sort`, so the date-stamped name sorts before "recent-".
+assert_eq "prunes past retention, keeps what's inside it" \
+ "$TODAY_FILE $DB_NAME-recent-5.sql.gz " "$REMAINING"
+
+# The guard that matters: if every dump on disk is older than the retention
+# window, pruning must still leave the newest one. Without it, a backup that had
+# been failing for longer than the window would end with pruning deleting the
+# last good dump — turning one broken backup into total data loss.
+REMAINING="$(prune_pass 1 "
+ touch -d '40 days ago' /scratch/pg_backups/$DB_NAME-old-40.sql.gz
+ touch -d '30 days ago' /scratch/pg_backups/$DB_NAME-old-30.sql.gz
+ touch -d '20 days ago' /scratch/pg_backups/$TODAY_FILE")"
+assert_eq "never prunes the last remaining dump, however old" "$TODAY_FILE " "$REMAINING"
+
+docker volume rm -f "$PROJECT-scratch" >/dev/null 2>&1
+
# ---------------------------------------------------------------------------
step "DISASTER: copy the dump out, then destroy the database and its volume"
# ---------------------------------------------------------------------------
From e035237dbf580b7ed5915a4e2e68d34832e32be3 Mon Sep 17 00:00:00 2001
From: Jon Froehlich
Date: Fri, 7 Aug 2026 12:19:35 -0700
Subject: [PATCH 3/3] Address code-review findings on the #1443 backup sidecar
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- Add AdminBackupWarningTests (website/tests/test_backup_status.py):
4 integration tests hitting /admin/ and /admin/data-health/ through
a real superuser/editor request, mirroring AdminLoggingWarningTests
for the sibling LOG_TO_FILE feature. The existing tests only proved
get_backup_status() returns the right dict in isolation, never that
MakeabilityLabAdminSite.each_context actually wires BACKUP_STATUS
into the rendered templates or that the superuser gate holds at the
HTTP layer.
- Fix scripts/test_backup_restore_django.sh's "m2m authors restored in
order" assertion: it sorted both sides before comparing, so it would
have passed even if the restore lost SortedManyToManyField's
sort_value ordering (the two test emails happen to also be
alphabetical). Seed the through-table out of alphabetical order and
compare unsorted, so the assertion can actually fail.
- Note the python3 host dependency in scripts/test_backup_restore.sh's
header comment (used by its status.json-parsing helpers, unlike the
rest of the script which runs entirely inside containers).
- Make the db-backup sidecar's entrypoint loop respond to
`docker compose stop`/`down` immediately instead of always eating
the full 10s stop grace period: bash only processes a trapped signal
between foreground commands, so a SIGTERM arriving during the
hourly/retry `sleep` sat unhandled until the sleep finished. Add
`trap 'exit 0' TERM INT` and background+wait the sleep so the trap
fires right away. Verified live: docker stop went from ~10s to 0.12s.
Verified end to end in the 1443 worktree:
- `manage.py test website.tests.test_backup_status
website.tests.test_version_endpoint --settings=makeabilitylab.settings_test`
— 24/24 new/touched tests pass (2 unrelated pre-existing failures in
test_logging_config, same root cause noted in the PR: local image
predates #1439's concurrent-log-handler dependency).
- `scripts/test_backup_restore.sh` — 35/35 pass.
- `scripts/test_backup_restore_django.sh` — 15/15 pass, including the
reworked ordering assertion.
- Brought up the real db-backup sidecar via
docker-compose-local-dev.yml and confirmed live: correct status.json
written, and `docker stop` returns in 0.12s post-fix.
🤖 Generated with [Claude Code](https://claude.com/claude-code) — Sonnet 5, claude-sonnet-5
---
docker-compose-local-dev.yml | 8 ++-
docker-compose.yml | 10 +++-
scripts/test_backup_restore.sh | 4 +-
scripts/test_backup_restore_django.sh | 13 +++-
website/tests/test_backup_status.py | 86 +++++++++++++++++++++++++++
5 files changed, 115 insertions(+), 6 deletions(-)
diff --git a/docker-compose-local-dev.yml b/docker-compose-local-dev.yml
index 411b1243..548d5d04 100644
--- a/docker-compose-local-dev.yml
+++ b/docker-compose-local-dev.yml
@@ -120,7 +120,13 @@ services:
# The backup script, mounted as a directory so edits survive git checkouts.
- ./scripts:/backup-scripts:ro
- entrypoint: ["/bin/bash", "-c", "while true; do if bash /backup-scripts/pg_backup.sh; then sleep \"$${BACKUP_POLL_SECONDS}\"; else sleep \"$${BACKUP_RETRY_SECONDS}\"; fi; done"]
+ # `sleep ... & wait $!` plus the leading `trap`, instead of a plain
+ # foreground `sleep`, so `docker compose stop`/`down` return promptly:
+ # bash only acts on a trapped signal between foreground commands, so a
+ # SIGTERM arriving during a plain `sleep 3600` would sit unhandled for up
+ # to an hour and this container would always eat the full stop grace
+ # period before Docker escalates to SIGKILL.
+ entrypoint: ["/bin/bash", "-c", "trap 'exit 0' TERM INT; while true; do if bash /backup-scripts/pg_backup.sh; then sleep \"$${BACKUP_POLL_SECONDS}\" & wait $!; else sleep \"$${BACKUP_RETRY_SECONDS}\" & wait $!; fi; done"]
depends_on:
db:
diff --git a/docker-compose.yml b/docker-compose.yml
index 923150b9..e3d0200a 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -23,6 +23,14 @@ services:
# `docker compose up -d` only recreates a container whose config changed, so a
# loop inside the bind-mounted script would keep running the old code forever
# after a deploy. Re-invoking the script each pass makes logic changes deploy.
+ #
+ # The `sleep ... & wait $!` (rather than a plain foreground `sleep`) and the
+ # leading `trap` are so `docker compose stop`/`down` return promptly: bash
+ # only acts on a trapped signal between foreground commands, so with a plain
+ # `sleep 3600` a SIGTERM sent while idle would sit unhandled for up to an
+ # hour and this container would always eat the full stop grace period before
+ # Docker escalates to SIGKILL. Backgrounding the sleep and `wait`-ing on it
+ # makes the trap run immediately instead.
db-backup:
image: "${POSTGRES_IMAGE:-postgres}"
restart: always
@@ -38,7 +46,7 @@ services:
- db-data:/var/lib/postgresql/data
- backup-status:/var/backup-status
- ./scripts:/backup-scripts:ro
- entrypoint: ["/bin/bash", "-c", "while true; do if bash /backup-scripts/pg_backup.sh; then sleep \"$${BACKUP_POLL_SECONDS}\"; else sleep \"$${BACKUP_RETRY_SECONDS}\"; fi; done"]
+ entrypoint: ["/bin/bash", "-c", "trap 'exit 0' TERM INT; while true; do if bash /backup-scripts/pg_backup.sh; then sleep \"$${BACKUP_POLL_SECONDS}\" & wait $!; else sleep \"$${BACKUP_RETRY_SECONDS}\" & wait $!; fi; done"]
depends_on:
- db
website:
diff --git a/scripts/test_backup_restore.sh b/scripts/test_backup_restore.sh
index 35b3f762..d09fb4d5 100755
--- a/scripts/test_backup_restore.sh
+++ b/scripts/test_backup_restore.sh
@@ -20,7 +20,9 @@
# Usage:
# bash scripts/test_backup_restore.sh
#
-# Requires: docker, and the postgres image used by the stack.
+# Requires: docker, the postgres image used by the stack, and python3 on the
+# host (used to parse status.json in the assertions below — everything
+# postgres-specific runs inside a container, but that parsing does not).
set -uo pipefail
diff --git a/scripts/test_backup_restore_django.sh b/scripts/test_backup_restore_django.sh
index 7d89ce7e..97942484 100755
--- a/scripts/test_backup_restore_django.sh
+++ b/scripts/test_backup_restore_django.sh
@@ -121,7 +121,11 @@ proj = Project.objects.create(name='Sidewalk "Quoted" Project', short_name='back
pub = Publication.objects.create(title='Notes on the Analytical Engine',
date=datetime.date(2024, 5, 1))
-pub.authors.add(p1, p2) # SortedManyToManyField through-table
+# Added out of alphabetical order on purpose: SortedManyToManyField orders by
+# insertion (sort_value), not by name, so this is the order a restore must
+# reproduce. Adding them already-alphabetical would let a restore that lost
+# the sort_value column pass by coincidence.
+pub.authors.add(p2, p1) # zhang@example.edu, then jose@example.edu
pub.projects.add(proj)
News.objects.create(title='Lab news with HTML & entities',
@@ -196,9 +200,11 @@ assert_eq "manage.py check passes" "0" "$?"
cat > "$WORK_DIR/verify.py" <<'PY'
from website.models import Person, Publication, News
pub = Publication.objects.get(title='Notes on the Analytical Engine')
+# Not sorted: this must reflect SortedManyToManyField's own ordering
+# (sort_value), which is what proves the restore preserved it.
authors = list(pub.authors.all().values_list('email', flat=True))
news = News.objects.get(title__startswith='Lab news')
-print('AUTHORS=' + ','.join(sorted(a or '' for a in authors)))
+print('AUTHORS=' + ','.join(a or '' for a in authors))
print('UNICODE=' + Person.objects.get(email='zhang@example.edu').first_name)
print('APOSTROPHE=' + Person.objects.get(email='pat@example.edu').last_name)
print('PROJECTS=' + str(pub.projects.count()))
@@ -209,7 +215,8 @@ VERIFY_OUT="$(docker run --rm --network "$NET" --user root -v "$REPO_DIR:/code"
python manage.py shell -c "exec(open('/verify.py').read())" 2>&1)"
get() { echo "$VERIFY_OUT" | grep "^$1=" | head -1 | cut -d= -f2-; }
-assert_eq "m2m authors restored in order" "jose@example.edu,zhang@example.edu" "$(get AUTHORS)"
+assert_eq "m2m authors restored in insertion order (not alphabetical)" \
+ "zhang@example.edu,jose@example.edu" "$(get AUTHORS)"
assert_eq "unicode field via ORM" "张" "$(get UNICODE)"
assert_eq "apostrophe field via ORM" "O'Brien" "$(get APOSTROPHE)"
assert_eq "publication↔project m2m" "1" "$(get PROJECTS)"
diff --git a/website/tests/test_backup_status.py b/website/tests/test_backup_status.py
index 36ab8024..c6651133 100644
--- a/website/tests/test_backup_status.py
+++ b/website/tests/test_backup_status.py
@@ -10,6 +10,13 @@
degrades to "unknown" instead of raising. This code runs inside
``each_context``, so an exception would take the entire admin down — the exact
opposite of what a backup-health feature should do.
+
+``BackupStatusFileTests``/``BackupWarningSuppressionTests``/``FormatBytesTests``
+exercise ``get_backup_status()`` directly. ``AdminBackupWarningTests`` below
+goes one layer further and proves the wiring itself -- that
+``MakeabilityLabAdminSite.each_context`` actually reaches the rendered
+``/admin/`` and Data Health pages -- mirroring ``AdminLoggingWarningTests`` in
+``test_logging_config.py`` for the sibling ``LOG_TO_FILE`` feature.
"""
import json
@@ -17,8 +24,11 @@
import tempfile
from datetime import datetime, timedelta, timezone
+from django.contrib.auth import get_user_model
from django.test import SimpleTestCase, override_settings
+from django.urls import reverse
+from website.tests.base import DatabaseTestCase
from website.utils.backup_status import format_bytes, get_backup_status
NOW = datetime(2026, 8, 7, 12, 0, 0, tzinfo=timezone.utc)
@@ -202,6 +212,82 @@ def test_missing_file_is_quiet_in_local_dev(self):
self.assertFalse(get_backup_status(self.missing, NOW)['should_warn'])
+class AdminBackupWarningTests(DatabaseTestCase):
+ """
+ The admin dashboard callout and Data Health panel that surface backup
+ health (#1443), rendered through a real request rather than calling
+ ``get_backup_status()`` in isolation.
+
+ ``get_backup_status()`` being correct doesn't prove
+ ``MakeabilityLabAdminSite.each_context`` actually wires ``BACKUP_STATUS``
+ into the templates, or that the superuser gate in ``each_context`` is
+ doing its job at the HTTP layer -- that's what these tests are for.
+ """
+
+ def setUp(self):
+ super().setUp()
+ User = get_user_model()
+ self.superuser = User.objects.create_superuser(
+ username="backupadmin", email="backupadmin@example.com", password="pw-for-test"
+ )
+ self.editor = User.objects.create_user(
+ username="backupeditor",
+ email="backupeditor@example.com",
+ password="pw-for-test",
+ is_staff=True,
+ )
+ self.tmp = tempfile.TemporaryDirectory()
+ self.addCleanup(self.tmp.cleanup)
+ self.status_file = os.path.join(self.tmp.name, 'status.json')
+
+ def _write_status(self, **overrides):
+ with open(self.status_file, 'w', encoding='utf-8') as handle:
+ json.dump(_status_payload(**overrides), handle)
+ return self.status_file
+
+ def test_warning_shown_to_superuser_when_backup_unhealthy(self):
+ self._write_status(last_attempt_ok=False,
+ error='pg_dump failed (exit 1): connection refused')
+ with override_settings(BACKUP_STATUS_FILE=self.status_file):
+ self.client.force_login(self.superuser)
+ response = self.client.get("/admin/")
+ self.assertEqual(response.status_code, 200)
+ self.assertContains(response, "database backups are not healthy")
+ self.assertContains(response, "connection refused")
+
+ def test_no_warning_when_backup_healthy(self):
+ self._write_status()
+ with override_settings(BACKUP_STATUS_FILE=self.status_file):
+ self.client.force_login(self.superuser)
+ response = self.client.get("/admin/")
+ self.assertEqual(response.status_code, 200)
+ self.assertNotContains(response, "database backups are not healthy")
+
+ def test_warning_hidden_from_non_superusers(self):
+ """Only the maintainer can act on a backup failure, so don't alarm editors."""
+ self._write_status(last_attempt_ok=False, error='connection refused')
+ with override_settings(BACKUP_STATUS_FILE=self.status_file):
+ self.client.force_login(self.editor)
+ response = self.client.get("/admin/")
+ self.assertEqual(response.status_code, 200)
+ self.assertNotContains(response, "database backups are not healthy")
+
+ def test_data_health_panel_always_shown_even_when_healthy(self):
+ """
+ Unlike the ``/admin/`` callout, the Data Health panel is rendered
+ unconditionally -- "when did it last succeed?" is worth showing even
+ when nothing is wrong.
+ """
+ self._write_status()
+ with override_settings(BACKUP_STATUS_FILE=self.status_file):
+ self.client.force_login(self.superuser)
+ response = self.client.get(reverse("admin:data_health_dashboard"))
+ self.assertEqual(response.status_code, 200)
+ self.assertContains(response, "Database backups")
+ self.assertContains(response, "Healthy")
+ self.assertContains(response, "makeability-2026-08-07.sql.gz")
+
+
class FormatBytesTests(SimpleTestCase):
def test_formats_common_sizes(self):
self.assertEqual(format_bytes(512), '512 B')