Skip to content

feat(backups): delete, retention, PostgreSQL coverage and a findable destination (#13307) - #13487

Merged
mrveiss merged 2 commits into
Dev_new_guifrom
issue-13307
Aug 4, 2026
Merged

mrveiss merged 2 commits into
Dev_new_guifrom
issue-13307

Conversation

@mrveiss

@mrveiss mrveiss commented Aug 4, 2026

Copy link
Copy Markdown
Owner

Thinking Path

Backend half of #13307, against the three decisions taken on the issue: Postgres in scope, /var/lib/slm/backups, delete + retention.

The ordering follows the issue's own principle — what protects data soonest, not what was asked first. Adding PostgreSQL matters most: the page is called "Backups", it creates something called a backup, and PostgreSQL — users, LLC work items, chat/session data — was simply absent. A restore would not bring any of it back, and you would find that out during recovery.

Two things surfaced while building it that were more dangerous than the missing features.

Deleting a backup could have deleted the live database. _complete_backup records the remote path when the copy to SLM storage fails — for Redis that is /var/lib/redis/dump.rdb on the target node, the running database. A naive delete_backup that unlinks backup.backup_path would destroy the data the backup exists to protect. _unlink_backup_file is therefore scoped to BACKUP_STORAGE_DIR and removes the record while leaving anything outside it alone.

Moving the destination broke module import. backup_service = BackupService() runs at import and used to mkdir unconditionally. That was safe while the destination was a home directory; against root-owned /var/lib/slm/backups it raises PermissionError and import services.backup fails outright — taking the API down on any host where Ansible has not run yet, and in CI. Found by running it, not by reading it. The mkdir is now best-effort with a warning naming the role that owns the directory, and the copy step surfaces a real error at the point it matters.

What Changed

Destination — config.py defaults to /var/lib/slm/backups; services/backup.py drops its dead hasattr(settings, "backup_dir") fallback. settings has always defined backup_dir, so the /var/lib/slm/backups written there could never apply — two files naming different defaults, with the unreachable one being correct. One source now.

Delete — DELETE /backups/{backup_id} → 204. @router.delete count in api/stateful.py was 0, so nothing could reclaim space and retention was unimplementable. Refuses an in-progress backup (409); succeeds when the file is already gone, because a row pointing at a missing file is what a half-finished cleanup leaves behind and refusing would make that state unrecoverable through the API.

Retention — apply_retention(node_id, service_type, keep_count, max_age_days), defaults 10 / 30 days, either dimension disabled with 0. Runs after each successful backup rather than on a timer: a new good backup is the only moment it is safe to drop an old one, and it needs no scheduler this service does not have. Only completed backups count toward keep_count — counting failures would let three bad nights evict the last good backup, precisely when it is the only thing between you and data loss.

PostgreSQL — execute_postgres_backup, dispatched for postgres/postgresql. The dump command mirrors roles/postgresql/templates/autobot-pg-backup.sh.j2 exactly (su - postgres -c "pg_dumpall --clean --if-exists" | gzip), so there is one way this cluster is dumped and no password is embedded. Dumps to .partial and renames, so an interrupted run leaves nothing the copy step could pick up. The staged dump is removed from the node in a finally — it is a full copy of the cluster sitting in /tmp.

Three helpers were Redis-specific and are now shared rather than duplicated: _get_rdb_file_info → _get_remote_file_info, _copy_backup_to_storage gains a suffix (the .rdb extension was hardcoded and would have labelled every Postgres dump as a Redis one), and _complete_backup takes remote_path instead of hardcoding /var/lib/redis/dump.rdb — which would have pointed a failed Postgres backup at Redis's live dump.

Provisioning — the slm_manager role creates /var/lib/slm/backups at 0750, not the 0755 its directory loop uses: a backup now contains a full Redis and PostgreSQL dump, so it carries every credential, user record and chat message the platform holds.

Verification

$ python3 -m pytest autobot-slm-backend/tests/test_backup_delete_retention_13307.py -q
13 passed

$ python3 -m pytest autobot-slm-backend/tests/ -q --ignore=autobot-slm-backend/tests/api/test_scim.py
1086 passed, 1 skipped, 1 xfailed

$ cd autobot-slm-backend/ansible && ansible-playbook --syntax-check playbooks/deploy-slm-manager.yml -i localhost,
playbook: playbooks/deploy-slm-manager.yml       # exit 0

The two destructive guards were invert-tested — each fails on exactly its own regression:

$ # _unlink_backup_file unscoped from BACKUP_STORAGE_DIR
1 failed, 12 passed
$ # failed backups counted toward the keep budget
1 failed, 12 passed
$ # restored
13 passed

tests/api/test_scim.py fails collection with ModuleNotFoundError: services.system_secrets_vault. Pre-existing — reproduced on an untouched checkout, unrelated to this change.

Tests load services/backup.py from its own path rather than importing it: the harness replaces config and much of services with MagicMocks, and asserting against those is vacuous — every attribute is truthy, so a test passes whether or not the code does anything. Where even that is not enough (the harness's config still leaks in), the property is pinned at source level with the reason recorded inline rather than asserted against a mock.

Scope not delivered

The frontend half — rendering backup_path in BackupsView.vue (currently zero references, the direct fix for "no idea where they are created"), plus delete/retention controls and i18n across all 11 locales — is not in this PR and #13307 stays open. Export/import and remote destinations remain follow-ups per the decision on the issue.

Refs #13307

Model Used

Opus 5 (1M context)

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant