Skip to content

fix(migrate): fail the job when migrations remain pending (ENG-565) - #497

Merged
islandbitcoin merged 2 commits into
mainfrom
fix/migrate-job-fail-loudly
Aug 26, 2026
Merged

fix(migrate): fail the job when migrations remain pending (ENG-565)#497
islandbitcoin merged 2 commits into
mainfrom
fix/migrate-job-fail-loudly

Conversation

@bobodread876

Copy link
Copy Markdown
Collaborator

The other half of ENG-565. Config value: lnflash/deployments#195.

The defect

Every Mongo-touching workload gates on this Job through the wait-for-mongodb-migrate initContainer. That gate is worthless if the Job cannot fail — and it couldn't:

#!/bin/sh
migrate-mongo status
migrate-mongo up      # aborts the whole run on the first throwing migration;
                      # exit code discarded
migrate-mongo status  # always succeeds → script exits 0

No set -e. So prod applied no migration between 2026-03-28 and 2026-08-26 while every deploy reported success. 20260330161656 threw on an empty NOTIFICATION_TOPICS, migrate-mongo stopped, and the two migrations behind it never ran.

It surfaced five months later, in an unrelated PR, as an api pod crash-looping while building the npub unique index whose dedupe migration had never executed.

The fix

  • set -eu — a throwing migration now fails the Job directly.
  • After up, re-read status; if anything is still PENDING, exit 1. migrate-mongo reporting success while leaving work undone is exactly the case the first change doesn't cover.

A green Job that applied nothing was the real defect. The crash-loop was just the first symptom loud enough to notice.

Notes

  • grep is present in the node:24-alpine base (/bin/grep) — verified, not assumed.
  • Script passes sh -n.
  • Expect this to turn currently-"passing" deploys red where a cluster is behind. That is the point — but it means the first deploy after this merges may fail loudly on any env carrying pending migrations. deployments#195 clears the known ones.

@linear

linear Bot commented Aug 26, 2026

Copy link
Copy Markdown

ENG-565

@bobodread876
bobodread876 force-pushed the fix/migrate-job-fail-loudly branch from 70e7a38 to c3b719a Compare August 26, 2026 17:45
Every Mongo-touching workload waits on this Job via the
wait-for-mongodb-migrate initContainer, so the gate is only worth
anything if the Job can fail. It could not.

The script was three unchecked commands with no set -e:

  migrate-mongo status
  migrate-mongo up      # aborts the whole run on the first throwing
                        # migration; exit code discarded
  migrate-mongo status  # always succeeds -> script exits 0

So prod applied NO migration between 2026-03-28 and 2026-08-26 while
every deploy reported success. 20260330161656 threw on an empty
NOTIFICATION_TOPICS, migrate-mongo stopped, and the two migrations
behind it never ran. It surfaced only when an api pod crash-looped
building the npub unique index whose dedupe migration had never
executed -- five months later, in a different PR.

Now: set -eu, so a throwing migration fails the Job directly; and after
the up step, if status still reports PENDING the script exits 1 rather
than letting pods start against a database that is not at the schema
this release expects.

The same hole was hiding a failure in CI. `make test-migrate` runs the
migrations against a throwaway mongo with no NOTIFICATION_TOPICS, so
20260317125624 has been failing there too -- and the swallowed exit code
made the job report green. docker-compose.yml now sets the variable, so
the clean-migration test actually exercises the full chain. The first
run of the stricter script is what turned that long-standing false green
red.

The prod/test config value is fixed in deployments#195.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
@bobodread876
bobodread876 force-pushed the fix/migrate-job-fail-loudly branch from 3ea0b3d to fc61cd6 Compare August 26, 2026 17:54
Surfaced by the stricter migrate script in this PR: `make test-migrate`
runs the whole chain against a throwaway mongo, and this migration died
there with "ns does not exist: galoy.bridgevirtualaccounts".

`col.indexes()` throws when the namespace has never been created, unlike
`createIndex` further down, which creates it implicitly. So the
migration only worked on a database where the app had already written a
virtual account. Both clusters have the collection (prod 16 docs, test
0), so this changes nothing for them -- it is fresh databases that
could not replay the chain, which defeats the purpose of the clean
migration test.

Treating "no namespace" as "no indexes" is the correct reading, not
merely a tolerant one: there cannot be a stale plain index on a
collection that does not exist.

Verified: `make test-migrate` now runs every migration from empty to
"All migrations applied." and exits 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NEoz7nBtdtsHyuYG5wNPQV
@islandbitcoin
islandbitcoin merged commit ff8afb2 into main Aug 26, 2026
15 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.

2 participants