Skip to content

Fail the deploy when the DB migration crash-loops#5409

Merged
habdelra merged 4 commits into
mainfrom
worktree-cs-11787-migration-deploy-gate
Jul 6, 2026
Merged

Fail the deploy when the DB migration crash-loops#5409
habdelra merged 4 commits into
mainfrom
worktree-cs-11787-migration-deploy-gate

Conversation

@habdelra

@habdelra habdelra commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Problem

The Deploy boxel to staging / Wait for db-migration step did sleep 180 and moved on. It never checked whether the migration actually applied — it keyed success on the pg-migration ECS service rollout, not on the migration committing. When the migration container exits non-zero it crash-loops (ECS restarts it), yet the step still reported success and the rest of the stack deployed against a DB missing the migration (relation "prerendered_html" does not exist).

Rollout state can't stand in for migration success here: the pg-migration service has no health check, so ECS marks it healthy the moment the container reaches RUNNING — which is while node-pg-migrate is still executing. So a slow migration (a backfill / index build) can be reported complete before its schema has actually landed.

Fix

Run the migration as a one-shot ECS task and gate the deploy on its exit code — the migration container exits with node-pg-migrate's status, which is an unambiguous "committed / failed" signal.

migrate-db now:

  • registers a pg-migration task-definition revision pinned to the freshly built image;
  • runs it as a one-shot Fargate task in the service's own subnets + security group (the SG that's allowed to reach the DB), overriding the command to run the migrations without the service's trailing sleep infinity so the task exits when the migration finishes;
  • waits for the task to stop and fails the deploy unless the migration container exited 0 (dumping the stop reason and a CloudWatch console link on failure). A hang is bounded by a generous timeout.

Every downstream service (deploy-ai-bot, deploy-bot-runner, deploy-worker, deploy-realm-server) now gates on this job, so nothing rolls out ahead of the schema. The post-migrate-db sleep job is gone.

The migrate command moves into packages/postgres/scripts/run-migrations.sh, shared by the container CMD (which keeps && sleep infinity for the service) and the one-shot command override, so the two can't drift.

Requires

  • cardstack/infra#719 — grants the shared github deploy role ecs:RunTask (it had StartTask, which is EC2-only). Must merge/apply before this workflow can launch the task.

Follow-ups (not in this PR)

  • The long-lived pg-migration ECS service is now vestigial — it no longer runs the migration, just idles on its last image. It can be retired (desired count → 0, or removed) in a separate infra change.
  • The merge-race that dropped a superseded deploy's migration wait isn't addressed here; with this gate the superseding deploy runs and verifies its own migration, so the bad outcome is prevented regardless.

Testing

The pipeline can't be exercised outside a real deploy. The gate script was validated against a stubbed ECS CLI across success (exit 0), migration failure (exit ≠ 0), task-placement failure, and timeout — and the task-definition re-render was verified to strip the read-only fields register-task-definition rejects, swap the image, and preserve the DB-credential secrets. End-to-end confirmation lands on the next staging deploy (after infra#719).

The `Wait for db-migration` step used to `sleep 180` and proceed, keying
success on the pg-migration ECS service rollout rather than on the migration
actually applying. A migration that exits non-zero crash-loops the service
while the step still reports success, so the rest of the stack deploys against
a DB missing the migration.

Replace the blind sleep with a gate that watches the service's new deployment:
succeed once it reaches rolloutState COMPLETED (the migrated task is up and
stable, which the container's `... && sleep infinity` command only permits on
a zero exit), and fail the moment a task on the new task-definition revision
stops (the crash loop). The service has no ECS deployment circuit breaker, so
rolloutState never flips to FAILED on its own — the stopped task's exit is read
directly and fails the deploy.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 68ab241055

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/manual-deploy.yml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the manual deploy workflow to gate the rest of the deploy on the DB migration service actually succeeding, instead of waiting a fixed amount of time and proceeding regardless of whether the migration crash-looped.

Changes:

  • Replaces the sleep 180 wait with an ECS API–backed polling gate that watches the PRIMARY pg-migration deployment.
  • Fails fast when a task on the newly deployed task definition stops (indicating a migration failure/crash loop), and times out for hangs.
  • Adds OIDC-based AWS credentials configuration and prints a CloudWatch logs console link on failure/timeout.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/manual-deploy.yml Outdated
Gate the deploy on the migration container's actual exit code instead of on
ECS service-rollout state. The pg-migration service has no health check, so a
rollout reaches COMPLETED as soon as the container is RUNNING — which happens
while node-pg-migrate is still executing — so rollout completion can't tell a
finished migration from an in-progress one, and a slow migration would unblock
the deploy before its schema landed.

`migrate-db` now registers a task-definition revision pinned to the freshly
built image and runs it as a one-shot Fargate task (in the pg-migration
service's own subnets/security group, the one allowed to reach the DB), with
the command overridden to run the migrations without the service's trailing
`sleep infinity`. It waits for the task to stop and fails the deploy unless the
migration container exited 0 — so a failed or crash-looping migration blocks
every downstream service.

The migrate command moves into packages/postgres/scripts/run-migrations.sh so
the container CMD (service) and the one-shot command override share one source.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread packages/postgres/scripts/run-migrations.sh Outdated
The dev `migrate` script in package.json diverges (ensure-db-exists, PG* env,
--no-check-order, --verbose=false), so "mirrors" was inaccurate. The comment
now just explains why --ignore-pattern is needed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Realm Server Test Results

    1 files  ±0      1 suites  ±0   9m 27s ⏱️ +4s
1 678 tests ±0  1 678 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 757 runs  ±0  1 757 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit b040e5d. ± Comparison against earlier commit d0aea49.

@habdelra

habdelra commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

[Claude Code 🤖] Tested in a forced staging deploy (with the prerequisite ecs:RunTask grant, cardstack/infra#719, applied).

The migrate-db job registered a pg-migration task-definition revision pinned to the built image, ran it as a one-shot Fargate task, and gated on the result:

Started migration task: <task>
migration task status: PROVISIONING → PENDING → RUNNING → DEPROVISIONING → STOPPED
migration container exit code: 0
DB migration succeeded.

The migrations applied (container exited 0), the gate passed on that exit code, and the schema-dependent services (worker, realm-server) then deployed after it — confirming the deploy is gated on the migration's real outcome. A separate check verified the gate fails closed: when the migration can't run, migrate-db fails and realm-server/worker are skipped rather than deploying ahead of the schema.

@habdelra
habdelra requested a review from a team July 6, 2026 18:05
@habdelra
habdelra merged commit 8dc8ebe into main Jul 6, 2026
111 of 134 checks passed
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.

3 participants