diff --git a/docker-compose.yml b/docker-compose.yml index 4eb915848..e33ba70a8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -131,6 +131,13 @@ services: dockerfile: Dockerfile-migrate environment: - MONGODB_ADDRESS=mongodb + # Required by the notification-topic migrations; they throw without it. + # Absent here, `make test-migrate` never got past 20260317125624 — and + # because the entrypoint swallowed the failure, CI reported the clean + # migration run as green anyway. Same defect that let prod go five + # months unmigrated (ENG-565). Values are placeholders: this is a + # throwaway database, so only the shape matters. + - NOTIFICATION_TOPICS=EMERGENCY,ATTENTION mongodb: image: mongo:${MONGODB_VERSION:-6.0.5} ports: [] diff --git a/scripts/mongodb-migrate.sh b/scripts/mongodb-migrate.sh index 31938604e..3f6cbbdc4 100755 --- a/scripts/mongodb-migrate.sh +++ b/scripts/mongodb-migrate.sh @@ -1,5 +1,41 @@ #!/bin/sh +# +# Runs on every helm release, and EVERY Mongo-touching workload waits on it via +# the `wait-for-mongodb-migrate` initContainer. That gate is only worth +# anything if this script can fail — so it does. +# +# It did not, until 2026-08-26. `migrate-mongo up` aborts the whole run on the +# first migration that throws, and with no `set -e` and a trailing `status` +# (which always succeeds) the Job exited 0 regardless. Prod applied no +# migration for five months while every deploy reported success, and the +# breakage only surfaced when an api pod crash-looped building a unique index +# whose dedupe migration had never run. See ENG-565. +set -eu -node_modules/.bin/migrate-mongo status -f src/migrations/migrate-mongo-config.js -node_modules/.bin/migrate-mongo up -f src/migrations/migrate-mongo-config.js -node_modules/.bin/migrate-mongo status -f src/migrations/migrate-mongo-config.js +CONFIG=src/migrations/migrate-mongo-config.js +MIGRATE=node_modules/.bin/migrate-mongo + +echo "=== migrations before ===" +"$MIGRATE" status -f "$CONFIG" + +echo "=== applying ===" +# `set -e` already aborts here on a non-zero exit, which is the common failure +# (a migration throwing). Belt and braces below for the case where migrate-mongo +# reports success while leaving work undone. +"$MIGRATE" up -f "$CONFIG" + +echo "=== migrations after ===" +STATUS_AFTER=$("$MIGRATE" status -f "$CONFIG") +echo "$STATUS_AFTER" + +# A green run that applied nothing is indistinguishable from a real one unless +# we look. Anything still PENDING here means the DB is not at the schema this +# release expects, and admitting pods against it is how the index-build +# crash-loop happened. +if echo "$STATUS_AFTER" | grep -q "PENDING"; then + echo "ERROR: migrations still PENDING after 'up' — refusing to report success." >&2 + echo "Pods gate on this Job; letting it pass would start them against an unmigrated database." >&2 + exit 1 +fi + +echo "All migrations applied." diff --git a/src/migrations/20260423000000-bridge-virtual-account-unique-accountid.ts b/src/migrations/20260423000000-bridge-virtual-account-unique-accountid.ts index 3b6d676b4..cb67027dc 100644 --- a/src/migrations/20260423000000-bridge-virtual-account-unique-accountid.ts +++ b/src/migrations/20260423000000-bridge-virtual-account-unique-accountid.ts @@ -70,7 +70,12 @@ module.exports = { } // ── Step 2: drop stale plain index if it exists ────────────────────────── - const existingIndexes = await col.indexes() + // `.indexes()` throws "ns does not exist" on a collection the app has not + // created yet — which is every fresh database, including the clean run in + // `make test-migrate`. `createIndex` below creates the collection + // implicitly, so treating "no namespace" as "no indexes" is correct rather + // than merely tolerant. + const existingIndexes = await col.indexes().catch(() => []) const hasPlainIndex = existingIndexes.some( (idx) => idx.name === INDEX_NAME && !idx.unique, ) @@ -88,7 +93,7 @@ module.exports = { const col = db.collection(COLLECTION) // Drop the unique index - const existingIndexes = await col.indexes() + const existingIndexes = await col.indexes().catch(() => []) const hasUniqueIndex = existingIndexes.some( (idx) => idx.name === INDEX_NAME && idx.unique, )