From 9fadbbb5b091f9560e97b15ebc45f30036b043f3 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 13 Aug 2026 23:43:02 +0200 Subject: [PATCH] ci: add per-database concurrency groups to database-migration migrate jobs The three `migrate-*` jobs had no concurrency group, so two dispatches for the same cloud and environment could run `migrate` concurrently against one schema_migrations table. Each job now takes a `db-migrate--` group. These jobs are not Terraform state writers: they run `terraform init` plus `terraform output -raw` to read a database endpoint and take no state lock, so reusing the `aws-tfstate-*` / `gcp-tfstate-*` / `azure-tfstate-*` groups from #1806 and #1801 would serialize migrations against unrelated deploys while still not identifying the database. The group keys on the resource that actually needs protecting instead, leaving different clouds and different environments free to run concurrently, including under `cloud=all`. `cancel-in-progress: false` throughout: cancelling mid-`migrate` leaves schema_migrations dirty and needs the CUDLY_FORCE_MIGRATION_VERSION recovery path. `inputs.environment` is a required `choice` on `workflow_dispatch` but a plain `type: string` on `workflow_call`, where GitHub does not enforce a non-empty value, so the group expression falls back to an `unset` sentinel rather than collapsing every environment into one group on an empty interpolation. `validate` and `summary` touch no database and are deliberately left without a group, as is the workflow top level, which would otherwise serialize the three clouds against each other. Refs #1593 --- .github/workflows/database-migration.yml | 49 ++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/.github/workflows/database-migration.yml b/.github/workflows/database-migration.yml index b2eb6cf14..429e2648e 100644 --- a/.github/workflows/database-migration.yml +++ b/.github/workflows/database-migration.yml @@ -225,6 +225,39 @@ jobs: if: | needs.validate.outputs.is_safe == 'true' && (inputs.cloud == 'aws' || inputs.cloud == 'all') + # Serializes migrations against one database: two dispatches for the same + # cloud and environment must not run `migrate` concurrently against the same + # schema_migrations table (#1593). Deliberately NOT the `aws-tfstate-*` group + # deploy-aws-lambda.yml and rollback.yml use: this job only runs `terraform + # init` + `terraform output -raw` to read the DB endpoint, takes no state + # lock, and the resource needing protection is the database, not the state + # file. Different clouds, and the same cloud in different environments, are + # different databases and stay concurrent. + # + # `inputs.environment` is a required `choice` on `workflow_dispatch`, so that + # path is constrained to dev|staging|prod server-side. On the `workflow_call` + # path it is a plain `type: string`; `required: true` makes the caller pass + # the key but GitHub does not enforce that the value is non-empty, and an + # empty suffix would collapse every environment into one group. The fallback + # below keeps such a call in its own group instead. `validate` rejects an + # environment outside the allowlist before this job runs, so the sentinel + # should be unreachable in practice; it is here so the group can never be + # silently ambiguous if that ordering ever changes. + # + # `cancel-in-progress: false` because cancelling mid-`migrate` leaves + # schema_migrations dirty, which then needs the CUDLY_FORCE_MIGRATION_VERSION + # recovery path in internal/database/postgres/migrations/migrate.go. Two + # consequences, both accepted as better than concurrent writers to one + # database: + # - GitHub keeps one pending entry per group, so a queued migration can be + # evicted by a later dispatch. Unlike rollback.yml, the `summary` job + # below does not exit 1 on a non-success result, so an evicted migration + # leaves the run green; read the per-cloud results it prints. + # - if the `environment:` binding below carries required reviewers, it is + # undocumented whether a job parked awaiting approval holds its group. + concurrency: + group: db-migrate-aws-${{ inputs.environment || 'unset' }} + cancel-in-progress: false permissions: id-token: write contents: read @@ -319,6 +352,14 @@ jobs: if: | needs.validate.outputs.is_safe == 'true' && (inputs.cloud == 'gcp' || inputs.cloud == 'all') + # Serializes migrations against the GCP database for one environment + # (#1593). A different database from `migrate-aws`'s, so a different group: + # `cloud=all` still migrates the three clouds concurrently. The group-key, + # empty-suffix and `cancel-in-progress: false` reasoning documented on + # `migrate-aws` applies here identically. + concurrency: + group: db-migrate-gcp-${{ inputs.environment || 'unset' }} + cancel-in-progress: false permissions: id-token: write contents: read @@ -398,6 +439,14 @@ jobs: if: | needs.validate.outputs.is_safe == 'true' && (inputs.cloud == 'azure' || inputs.cloud == 'all') + # Serializes migrations against the Azure database for one environment + # (#1593). A different database from `migrate-aws`'s and `migrate-gcp`'s, so + # a different group: `cloud=all` still migrates the three clouds + # concurrently. The group-key, empty-suffix and `cancel-in-progress: false` + # reasoning documented on `migrate-aws` applies here identically. + concurrency: + group: db-migrate-azure-${{ inputs.environment || 'unset' }} + cancel-in-progress: false permissions: id-token: write contents: read