Finish a restore without a browser, from the set that was asked for
Some checks failed
CI / naming (push) Successful in 4s
CI / frontend (push) Successful in 14s
CI / migrations-mysql (push) Failing after 14s
CI / backend (push) Failing after 4m57s

Closes the three defects in the imported restore path, at Ben's request. Each
was proven against a real stack before and after: two backup sets with different
data and different instance files, then a restore of the OLDER one, which is the
case that was broken.

A restore no longer depends on a tab staying open. The backup sidecar has no
Flask, so it loads the dump and leaves a sentinel for the api container to run
the migrations. The only thing that ever acted on that sentinel was the poll
loop of the modal that queued the restore, so closing the tab - or letting a
token expire - during the minutes a load takes left the data at the old revision
under new code, every query on a newer column failing, and nothing retrying. A
watcher in the api container now claims and runs it. The endpoint no longer runs
the upgrade at all; it only makes sure something is pending, which is also how a
failed upgrade is retried. That takes Alembic out of the request path, where it
could outrun the gunicorn worker timeout and be killed mid-chain, leaving a
half-applied revision the next attempt trips over.

The claim is a rename, because rename(2) is atomic: several workers and a
browser poll can race the same sentinel and exactly one wins. A failed upgrade
parks itself as .upgrade-failed rather than re-arming, since putting it back
would rerun the same broken chain on every poll forever.

Instance files come from the set being restored. The restic call asked for
'latest' regardless of which set was named, so restoring anything but the newest
paired that set's DATABASE with the NEWEST snapshot's uploads and plugins.json -
a mismatch nothing downstream could detect. Now scoped by the set's own tag,
which backup.sh has always written. The tree is replaced rather than restored
over, so files created after the snapshot are gone afterwards: a restore has to
put the site back to the snapshot, not merge into it. Done as a staged swap
rather than 'restic restore --delete --target /', because that aims a delete
pass at the container root and the cost of getting its filter wrong is the whole
filesystem.

Nothing destructive happens before a copy is taken. The dump carries DROP TABLE
per table with no transaction around it, and the instance tree is replaced
wholesale, so a wrong set or a failure part-way through left nothing to go back
to. restore.sh now writes a pre-restore-<stamp>/ set - dump, instance tarball
and a README saying what it is and how to load it - and refuses to touch
anything if that copy cannot be written.

Two more found while proving the above, both of which stopped the overlay
working at all:

* Every script under scripts/backup/ was committed non-executable, while every
  other .sh in the repo is 755. The compose entrypoints are
  /opt/backup/watch.sh and backup.sh from a read-only bind mount, so the backup
  and watcher containers could not start from a clean checkout on any machine,
  and nothing could fix it at runtime.

* Dockerfile.backup could not build without the corporate CA staged. A plain
  COPY of the gitignored cert fails outright; the trailing-glob form does too,
  because BuildKit fails a COPY whose pattern matches nothing - which is exactly
  the case being handled. Copying the directory and testing for the file inside
  it is the form that works both ways. Verified by building the image with no
  cert present.

Suite 2600 passed / 9 skipped. The finalizer has unit tests covering the atomic
claim, the parked failure and the endpoint no longer running the upgrade.

Scope: product
This commit is contained in:
cproudlock
2026-09-06 17:38:10 -04:00
parent 83190422c9
commit fa6b99bf91
13 changed files with 392 additions and 53 deletions

View File

@@ -0,0 +1,142 @@
"""The post-restore schema upgrade: who runs it, and what happens when it fails.
A restore loads the dump in the Flask-less backup sidecar and leaves a sentinel,
because only the api container can run the migrations. These tests pin the two
properties that failure taught us to care about: the upgrade must not depend on
a browser staying open, and it must never run twice at once.
"""
import os
import pytest
from shopdb.core.api import shopdbbackups
@pytest.fixture
def backups_root(tmp_path, monkeypatch):
"""Point the backup routes at a real, empty directory."""
root = tmp_path / 'backups'
root.mkdir()
monkeypatch.setenv(shopdbbackups.BACKUPS_ROOT_ENV, str(root))
return root
def _arm(root, setname='wj-2026-09-06T120000Z'):
(root / shopdbbackups.UPGRADE_SENTINEL).write_text(setname, encoding='utf-8')
return setname
def test_only_one_claimant_wins_a_pending_upgrade(app, backups_root):
"""Two workers racing the same sentinel must not both run Alembic."""
setname = _arm(backups_root)
with app.app_context():
first = shopdbbackups._claim_upgrade()
second = shopdbbackups._claim_upgrade()
assert first == setname
assert second is None, 'a second worker claimed an upgrade already in flight'
assert not (backups_root / shopdbbackups.UPGRADE_SENTINEL).exists()
assert (backups_root / shopdbbackups.UPGRADE_CLAIM).exists()
def test_claiming_nothing_is_not_an_error(app, backups_root):
with app.app_context():
assert shopdbbackups._claim_upgrade() is None
def test_a_successful_finalize_clears_the_claim_and_says_done(app, backups_root,
monkeypatch):
setname = _arm(backups_root)
monkeypatch.setattr(shopdbbackups, '_run_schema_upgrade', lambda: None)
with app.app_context():
claimed = shopdbbackups._claim_upgrade()
assert shopdbbackups._finalize_claimed(claimed) is True
assert not (backups_root / shopdbbackups.UPGRADE_CLAIM).exists()
with app.app_context():
status = shopdbbackups._read_status('restore')
assert status and status['phase'] == 'done'
assert status['setname'] == setname
def test_a_failed_upgrade_parks_itself_instead_of_retrying_forever(app,
backups_root,
monkeypatch):
"""A failing migration must not be re-armed automatically.
Putting the sentinel back would run the same broken chain on every poll for
as long as the container lives.
"""
_arm(backups_root)
def _boom():
raise RuntimeError('Can\'t locate revision identified by abc123')
monkeypatch.setattr(shopdbbackups, '_run_schema_upgrade', _boom)
with app.app_context():
claimed = shopdbbackups._claim_upgrade()
assert shopdbbackups._finalize_claimed(claimed) is False
assert (backups_root / shopdbbackups.UPGRADE_FAILED).exists()
assert not (backups_root / shopdbbackups.UPGRADE_SENTINEL).exists(), \
'a failed upgrade re-armed itself and will retry forever'
assert not (backups_root / shopdbbackups.UPGRADE_CLAIM).exists()
with app.app_context():
status = shopdbbackups._read_status('restore')
assert status and status['phase'] == 'failed'
def test_the_endpoint_does_not_run_the_upgrade_itself(app, client, auth_headers,
backups_root, monkeypatch):
"""Alembic in a request can outrun the worker timeout and be killed mid-chain."""
_arm(backups_root)
ran = []
monkeypatch.setattr(shopdbbackups, '_run_schema_upgrade',
lambda: ran.append(True))
response = client.post('/api/shopdbbackups/finalize-restore',
headers=auth_headers)
assert response.status_code == 200
assert ran == [], 'the request ran the schema upgrade inline'
assert response.get_json()['data']['queued'] is True
assert (backups_root / shopdbbackups.UPGRADE_SENTINEL).exists(), \
'the endpoint consumed the sentinel the watcher needs to claim'
def test_the_endpoint_re_arms_a_failed_upgrade(app, client, auth_headers,
backups_root):
"""Retrying a failed upgrade is what this endpoint is for now."""
(backups_root / shopdbbackups.UPGRADE_FAILED).write_text('wj-set',
encoding='utf-8')
response = client.post('/api/shopdbbackups/finalize-restore',
headers=auth_headers)
assert response.status_code == 200
assert response.get_json()['data']['retried'] is True
assert (backups_root / shopdbbackups.UPGRADE_SENTINEL).read_text() == 'wj-set'
assert not (backups_root / shopdbbackups.UPGRADE_FAILED).exists()
def test_nothing_pending_reports_nothing_pending(app, client, auth_headers,
backups_root):
response = client.post('/api/shopdbbackups/finalize-restore',
headers=auth_headers)
assert response.status_code == 200
assert response.get_json()['data']['queued'] is False
def test_an_unconfigured_deployment_refuses(app, client, auth_headers,
tmp_path, monkeypatch):
"""No store behind the feature means say so, not 500 on a missing path."""
monkeypatch.setenv(shopdbbackups.BACKUPS_ROOT_ENV,
str(tmp_path / 'nothing-here'))
response = client.post('/api/shopdbbackups/finalize-restore',
headers=auth_headers)
assert response.status_code == 404