Skip to content

Rehearse the upgrade, rollback, and recovery paths in CI - #177

Merged
icebergai-review-bot[bot] merged 3 commits into
mainfrom
rehearsal-v3
Aug 16, 2026
Merged

Rehearse the upgrade, rollback, and recovery paths in CI#177
icebergai-review-bot[bot] merged 3 commits into
mainfrom
rehearsal-v3

Conversation

@richardmhope

Copy link
Copy Markdown
Contributor

Part 2 of #147, and the last.

Closes #147

docs/releases.md claims an upgrade path is supported and that a backup can be
restored. This makes both claims something a reader can check rather than something
the project asserts about itself.

make rehearse runs against real Postgres — CI on every pull request, an operator
locally — and does four things, each for a failure the others do not cover:

Every revision applied one at a time a migration that only works when the whole schema is created at once passes the head jump and fails the upgrade an operator actually runs
Reversed one at a time, then re-applied a downgrade that leaves a table, index or type behind fails the second upgrade on a name that already exists
The previous release upgraded onto this tree the supported path itself, through a git worktree at the tag so the migrations come from the release while uv run still resolves this environment
A backup destroyed and restored with a row written before the dump and asserted after it, then the restored schema compared against the models

The third leg skips loudly until a release tag exists. A rehearsal that reports
success without having run is worse than one that says it could not.

The comparison earned its place on the first run

It found three drifts between the migrations and the models that SQLite cannot
express — and that apps/api/tests/test_migrations.py has therefore been green
through this whole time:

  • finding.validation_reason declared length=64 in 0009 while enum_type()
    produces 32 for every enum in the codebase.
  • validation_policy.rationale created as TEXT while the model says
    max_length=2000.
  • owner_group.name, routing_rule.name and response_target.severity created by
    0014 as unique indexes, where Field(unique=True) produces unique
    constraints — which is how every other unique column in this schema was made.

Migration 0016 reconciles them. No behaviour changes. They matter because
autogenerate compares against the models: left alone, the next --autogenerate
would fold these into an unrelated migration, and its reviewer would have to work
out which half was intended.

Where it runs

In the existing Postgres job rather than a new one — it needs exactly that service
and that environment, and a second Postgres to say the same thing twice is a minute
of CI nobody gets back.

That job's checkout gains two things:

  • fetch-depth: 0, because a shallow clone has no tags, so the previous-release
    leg would skip for the rest of the project's life while reporting success.
  • persist-credentials: false, because it now checks out a historical tag with
    git worktree and needs no credential to do it. The other four checkouts in
    ci.yml have the same pre-existing finding and are deliberately left alone here.

Verification

Run end to end against Postgres 18 locally: all four rehearsals pass and the schema
comparison is clean with 0016 applied. Before 0016 it reported exactly the three
drifts above — which is how they were found.

#147 acceptance criteria

make check green: 1819 passed.

Part 2 of #147, and the last. Closes #147.

`docs/releases.md` claims an upgrade path is supported and that a backup can be
restored. This makes both claims something a reader can check rather than
something the project asserts about itself. `make rehearse` runs against a real
Postgres — CI on every pull request, an operator locally — and does four things,
each for a failure the others do not cover:

**Every revision applied one at a time**, rather than in one `head` jump. A
migration that only works when the whole schema is created at once passes the
jump and fails the upgrade an operator actually runs.

**Reversed one at a time, then re-applied.** Reversibility is what makes rollback
mechanically possible, and the re-apply is the assertion: a downgrade that leaves
a table, an index or a type behind fails the second upgrade on a name that
already exists.

**The previous release upgraded onto this tree.** The supported path itself,
through a `git worktree` at the tag so the migrations come from the release while
`uv run` still resolves this environment. It skips *loudly* until a release tag
exists — a rehearsal that reports success without having run is worse than one
that says it could not.

**A backup destroyed and restored**, with a row written before the dump and
asserted after it, then the restored schema compared against the models. A backup
nobody has restored is a plan, not a backup.

That last comparison earned its place on the first run: it found three drifts
between the migrations and the models that SQLite cannot express, and that
`test_migrations.py` has therefore been green through. Migration 0016 reconciles
them — two VARCHAR lengths declared wider than `enum_type()`/`max_length` produce,
and three unique columns that 0014 created as unique *indexes* where
`Field(unique=True)` produces unique *constraints*, which is how every other
unique column in this schema was created. No behaviour changes. They matter
because autogenerate compares against the models: left alone, the next
`--autogenerate` would fold these into an unrelated migration and its reviewer
would have to work out which half was intended.

The rehearsal lives in the existing Postgres job rather than a new one — it needs
exactly that service and that environment, and a second Postgres to say the same
thing twice is a minute of CI nobody gets back. That job's checkout gains
`fetch-depth: 0` (a shallow clone has no tags, so the previous-release leg would
skip for the rest of the project's life while reporting success) and
`persist-credentials: false` (it checks out a historical tag with
`git worktree` and needs no credential to do it).

Verified end to end against Postgres 18 locally: all four rehearsals pass, and
the schema comparison is clean with 0016 applied.

make check green: 1819 passed.
CI failed on the first real run, four rehearsals in:

    pg_dump: error: aborting because of server version mismatch
    pg_dump: detail: server version: 18.6; pg_dump version: 16.14

The runner ships an older libpq than the service container, and `pg_dump`
refuses to dump a server newer than itself. Nothing to do with this project —
which is exactly why it is worth naming rather than working around.

Two changes. CI installs `postgresql-client-18` before the rehearsal, from the
PGDG repository the image already has configured. And the script checks the two
versions up front and says what to install, because the mismatch otherwise
arrives after several minutes of rehearsals that all succeeded, in a message that
reads like a bug in the script.

Verified both ways locally against Postgres 18: with a matching client all four
rehearsals pass, and with a deliberately old one the pre-check refuses before any
work is done.
`postgresql-client-18` is not in the runner image's package lists, so the
previous fix traded one failure for another. Adding the PGDG apt repository would
work, but it puts a third-party package source into a security project's CI to
fetch a binary the job already has locally — the service container is
`postgres:18-alpine`, and its client tools are the server's own version by
construction.

So the step points `psql` and `pg_dump` at that container through two shims on
PATH. No download, no version to keep in step with the service, and the rehearsal
script stays oblivious: it calls both from PATH, which is what an operator does
too. The version pre-check added alongside it stays, because a local run can
still have a mismatched client and deserves to be told which package to install.
@richardmhope

Copy link
Copy Markdown
Contributor Author

CI green. The first two runs failed on the rehearsal, both for environment
reasons worth recording:

  1. pg_dump: aborting because of server version mismatch — the runner ships
    libpq 16 and the service container is Postgres 18.6, and pg_dump refuses to
    dump a server newer than itself.
  2. E: Unable to locate package postgresql-client-18 — the obvious fix, which
    traded one failure for another.

Rather than add the PGDG apt repository (a third-party package source in a
security project's CI, to fetch a binary the job already has locally), the step
points psql and pg_dump at the service container's own tools through two
shims on PATH. Version match by construction, nothing downloaded, and the script
stays oblivious — it calls both from PATH, which is what an operator does.

The version pre-check added alongside stays, because a local run can still have
a mismatched client and deserves to be told which package to install rather than
being handed pg_dump's message four rehearsals in. Verified both ways against
Postgres 18 locally: matching client passes all four legs, deliberately old
client refuses before any work is done.

The run log confirms it ran rather than skipped:

pg_dump (PostgreSQL) 18.6
▸ Applying every revision one at a time
▸ Reversing one revision at a time, then re-applying
▸ SKIPPED: no release tag exists yet, so there is no supported upgrade to rehearse
▸ Backing up, destroying, and restoring
▸ Asserting the restored schema still matches the models
✓ upgrade, rollback, and recovery rehearsed

The third leg is the loud skip by design — there is no release tag yet, and a
rehearsal that reported success without having run would be worse than one that
says it could not.

@icebergai-review-bot icebergai-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

APPROVE

Completed bounded review across 1 immutable scope(s). No actionable findings in the supplied change.

Scope health

Convergence: healthy. Review mode: initial.
Recommended action: CONTINUE_INCREMENTAL.

  • No escalation signals.

Prior findings

Finding Status
No prior finding state

New findings

No new findings.

Fix-induced regressions

  • None evidenced.

Uncertainty

  • No material uncertainty recorded.

Validation

  • Reviewed the supplied immutable diff; exact-head CI is reported as passed.

Residual risks

  • None identified.

@icebergai-review-bot
icebergai-review-bot Bot merged commit f9eb3eb into main Aug 16, 2026
6 checks passed
@icebergai-review-bot
icebergai-review-bot Bot deleted the rehearsal-v3 branch August 16, 2026 07:00
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.

Establish signed releases and supported upgrade paths

1 participant