From 127ae5a57504b77a4efa438287fe5745183def2f Mon Sep 17 00:00:00 2001 From: John Henderson Date: Mon, 9 Mar 2026 09:58:42 +0000 Subject: [PATCH 1/3] Add data_migrate gem to run data + schema migrations in deploy workflow We sometimes forget to enable the feature flag required for data migrations. Adding the data_migrate gem lets us run data migrations automatically as part of the GitHub deployment workflow, giving better visibility into migration/deployment progress. Also avoids needing to request console access to run data migrations manually. Jira-Issue: MAV-4101 --- .github/workflows/deploy.yml | 154 +++++++++++++++++++++++++++++++---- Gemfile | 1 + Gemfile.lock | 4 + db/schema.rb | 3 + 4 files changed, 148 insertions(+), 14 deletions(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 540f1ab062..c2fcc19e09 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -181,11 +181,10 @@ jobs: steps: - run: echo "Proceeding with deployment to $environment" - run-migrations: - name: Run migrations + pre-deploy: + name: Pre deploy runs-on: ubuntu-latest needs: approve-deployments - if: ${{ inputs.server_types == 'all' || inputs.server_types == 'ops' }} permissions: id-token: write steps: @@ -201,10 +200,10 @@ jobs: with: path: ${{ runner.temp }} name: ${{ inputs.environment }}-ops-task-definition - - name: Register migration task definition - id: register-migration-task-definition + - name: Register pre deploy task definition + id: register-pre-deploy-task-definition run: | - family_name="mavis-migration-task-definition-$environment" + family_name="mavis-pre-deploy-task-definition-$environment" file_path="${{ runner.temp }}/migration-task-definition.json" jq --arg f "$family_name" '.family = $f' \ "${{ runner.temp }}/ops-task-definition.json" > "$file_path" @@ -220,7 +219,7 @@ jobs: SLACK_MAVIS_RELEASES_WEBHOOK_URL: ${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }} # yamllint disable rule:line-length run: | - TASK_DEFINITION_ARN=${{ steps.register-migration-task-definition.outputs.task_definition_arn }} + PRE_DEPLOY_TASK_DEFINITION_ARN=${{ steps.register-pre-deploy-task-definition.outputs.task_definition_arn }} SUBNET_ID=$(aws ec2 describe-subnets --filters "Name=tag:Name,Values=private-subnet-$environment-a" --query 'Subnets[0].SubnetId' --output text) SECURITY_GROUP_ID=$(aws ec2 describe-security-groups --filters "Name=group-name,Values=ops-service-$environment" --query 'SecurityGroups[0].GroupId' --output text) @@ -230,7 +229,7 @@ jobs: while [ $ATTEMPT -le $MAX_ATTEMPTS ]; do TASK_ARN=$(aws ecs run-task \ --cluster "$cluster_name" \ - --task-definition "$TASK_DEFINITION_ARN" \ + --task-definition "$PRE_DEPLOY_TASK_DEFINITION_ARN" \ --launch-type FARGATE \ --network-configuration "awsvpcConfiguration={subnets=[$SUBNET_ID],securityGroups=[$SECURITY_GROUP_ID]}" \ --overrides '{ @@ -314,8 +313,8 @@ jobs: deploy-service: name: Deploy service runs-on: ubuntu-latest - needs: [prepare-deployment, approve-deployments, run-migrations] - if: ${{ !cancelled() && needs.run-migrations.result != 'failure' }} + needs: [prepare-deployment, approve-deployments, pre-deploy] + if: ${{ !cancelled() }} permissions: id-token: write strategy: @@ -361,15 +360,142 @@ jobs: echo "Task definition arns don't match, likely due to a rollback to the previous version. Deployment failed." exit 1 fi + # yamllint enable rule:line-length + post-deploy: + name: Post deploy + runs-on: ubuntu-latest + needs: [approve-deployments, deploy-service] + if: ${{ !cancelled() && needs.deploy-service.result != 'failure' }} + permissions: + id-token: write + steps: + - name: Checkout code + uses: actions/checkout@v6 + - name: Configure AWS Credentials + uses: aws-actions/configure-aws-credentials@v6 + with: + role-to-assume: arn:aws:iam::${{ env.aws_account_id }}:role/GithubDeployECSService + aws-region: eu-west-2 + - name: Register post deploy task definition + id: register-post-deploy-task-definition + run: | + family_name="mavis-post-deploy-task-definition-$environment" + file_path="${{ runner.temp }}/migration-task-definition.json" + jq --arg f "$family_name" '.family = $f' \ + "${{ runner.temp }}/ops-task-definition.json" > "$file_path" + task_definition_arn=$(aws ecs register-task-definition \ + --cli-input-json file://$file_path \ + --query 'taskDefinition.taskDefinitionArn' \ + --output text + ) + echo "task_definition_arn=$task_definition_arn" >> "$GITHUB_OUTPUT" + - name: Run data migrations + id: run-data-migrations + env: + SLACK_MAVIS_RELEASES_WEBHOOK_URL: ${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }} + # yamllint disable rule:line-length + run: | + POST_DEPLOY_TASK_DEFINITION_ARN=${{ steps.register-post-deploy-task-definition.outputs.task_definition_arn }} + SUBNET_ID=$(aws ec2 describe-subnets --filters "Name=tag:Name,Values=private-subnet-$environment-a" --query 'Subnets[0].SubnetId' --output text) + SECURITY_GROUP_ID=$(aws ec2 describe-security-groups --filters "Name=group-name,Values=ops-service-$environment" --query 'SecurityGroups[0].GroupId' --output text) + + MAX_ATTEMPTS=3 + ATTEMPT=1 + + while [ $ATTEMPT -le $MAX_ATTEMPTS ]; do + TASK_ARN=$(aws ecs run-task \ + --cluster "$cluster_name" \ + --task-definition "$POST_DEPLOY_TASK_DEFINITION_ARN" \ + --launch-type FARGATE \ + --network-configuration "awsvpcConfiguration={subnets=[$SUBNET_ID],securityGroups=[$SECURITY_GROUP_ID]}" \ + --overrides '{ + "containerOverrides": [{ + "name": "application", + "command": ["bin/rails","data:migrate"] + }] + }' \ + --query 'tasks[0].taskArn' \ + --output text) + + echo "Waiting for task to complete: $TASK_ARN" + + # shellcheck disable=SC2001 + TASK_ID=$(sed 's:^.*/::' <<< "$TASK_ARN") + AWS_CONSOLE_URL="https://eu-west-2.console.aws.amazon.com/ecs/v2/clusters/$cluster_name/tasks/$TASK_ID/logs" + + echo "View logs in AWS Console: $AWS_CONSOLE_URL" + if [ "$environment" = 'production' ]; then + ./.github/send_slack_notification.sh "${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }}" "$AWS_CONSOLE_URL" "Running data migrations attempt $ATTEMPT/$MAX_ATTEMPTS" + fi + + MAX_WAIT_TIME=3600 + POLL_INTERVAL=10 # Poll every 10 seconds + ELAPSED=0 + + while [ "$ELAPSED" -lt "$MAX_WAIT_TIME" ]; do + TASK_STATUS=$(aws ecs describe-tasks \ + --cluster "$cluster_name" \ + --tasks "$TASK_ID" \ + --query 'tasks[0].lastStatus' \ + --output text) + + if [ "$TASK_STATUS" = "STOPPED" ]; then + echo "Task has stopped" + break + fi + + sleep "$POLL_INTERVAL" + ELAPSED=$((ELAPSED + POLL_INTERVAL)) + done + + if [ "$ELAPSED" -ge "$MAX_WAIT_TIME" ]; then + echo "ERROR: Data migration task did not complete within $MAX_WAIT_TIME seconds." + exit 1 + fi + + EXIT_CODE=$(aws ecs describe-tasks \ + --cluster "$cluster_name" \ + --tasks "$TASK_ARN" \ + --query 'tasks[0].containers[0].exitCode' \ + --output text) + + echo "Container exit code: $EXIT_CODE" + + if [ "$EXIT_CODE" = "0" ]; then + echo "Data migrations completed" + break + else + echo "ECS task failed with exit code: $EXIT_CODE" + if [ "$ATTEMPT" = "$MAX_ATTEMPTS" ]; then + exit 1 + fi + ATTEMPT=$((ATTEMPT+1)) + fi + done # yamllint enable rule:line-length + - name: Notify data migrations completed + if: ${{ env.environment == 'production' }} + uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a + with: + webhook: ${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }} + webhook-type: incoming-webhook + payload: | + text: "Data migrations finished successfully :white_check_mark:" + blocks: + - type: "section" + text: + type: "mrkdwn" + text: "Data migrations finished successfully :white_check_mark:" notify-deployment-complete: name: Notify deployment status runs-on: ubuntu-latest - needs: [deploy-service, run-migrations] + needs: [pre-deploy, deploy-service, post-deploy] if: ${{ !cancelled() && inputs.environment == 'production' }} steps: - name: Notify deployment success - if: ${{ needs.deploy-service.result == 'success' }} + if: >- + ${{ needs.deploy-service.result == 'success' || + needs.post-deploy.result == 'success' }} uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a with: webhook: ${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }} @@ -384,7 +510,7 @@ jobs: - name: Notify deployment failure if: >- ${{ needs.deploy-service.result == 'failure' || - needs.run-migrations.result == 'failure' }} + needs.pre-deploy.result == 'failure' }} uses: slackapi/slack-github-action@91efab103c0de0a537f72a35f6b8cda0ee76bf0a with: webhook: ${{ secrets.SLACK_MAVIS_RELEASES_WEBHOOK_URL }} @@ -400,7 +526,7 @@ jobs: - type: "section" fields: - type: "mrkdwn" - text: "*Failed job:*\n${{ needs.deploy-service.result == 'failure' && 'deploy-service' || 'run-migrations' }}" + text: "*Failed job:*\n${{ needs.deploy-service.result == 'failure' && 'deploy-service' || 'pre-deploy' || 'post-deploy' }}" - type: "mrkdwn" text: "<${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}|View workflow run>" # yamllint enable rule:line-length diff --git a/Gemfile b/Gemfile index 9ff3039929..6de67662cc 100644 --- a/Gemfile +++ b/Gemfile @@ -31,6 +31,7 @@ gem "caxlsx" gem "charlock_holmes" gem "config" gem "csv" +gem "data_migrate" gem "devise" gem "discard" gem "dry-cli" diff --git a/Gemfile.lock b/Gemfile.lock index a15d62083c..dfc1936a5a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -275,6 +275,9 @@ GEM cuprite (0.17) capybara (~> 3.0) ferrum (~> 0.17.0) + data_migrate (11.3.1) + activerecord (>= 6.1) + railties (>= 6.1) database_cleaner-active_record (2.2.2) activerecord (>= 5.a) database_cleaner-core (~> 2.0) @@ -960,6 +963,7 @@ DEPENDENCIES cssbundling-rails csv cuprite + data_migrate database_cleaner-active_record debug devise diff --git a/db/schema.rb b/db/schema.rb index 824d4826f4..19be9cd3e7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -311,6 +311,9 @@ t.index ["team_id"], name: "index_consents_on_team_id" end + create_table "data_migrations", primary_key: "version", id: :string, force: :cascade do |t| + end + create_table "flipper_features", force: :cascade do |t| t.datetime "created_at", null: false t.string "key", null: false From 71a52d58f40d41cbcceab060e3b7b7ad4b502668 Mon Sep 17 00:00:00 2001 From: John Henderson Date: Mon, 9 Mar 2026 09:59:11 +0000 Subject: [PATCH 2/3] Data migration: backfill `NotifyLogEntry` purpose column using data migrate gem Taking the opportunity to backfill a column using this gem instead of having to do it manually. Jira-Issue: MAV-4101 --- .github/workflows/deploy.yml | 2 +- ...backfill_purpose_for_notify_log_entries.rb | 65 +++++++++++++++++++ db/data_schema.rb | 3 + 3 files changed, 69 insertions(+), 1 deletion(-) create mode 100644 db/data/20260305165603_backfill_purpose_for_notify_log_entries.rb create mode 100644 db/data_schema.rb diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index c2fcc19e09..874880b113 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -360,7 +360,7 @@ jobs: echo "Task definition arns don't match, likely due to a rollback to the previous version. Deployment failed." exit 1 fi - # yamllint enable rule:line-length + # yamllint enable rule:line-length post-deploy: name: Post deploy runs-on: ubuntu-latest diff --git a/db/data/20260305165603_backfill_purpose_for_notify_log_entries.rb b/db/data/20260305165603_backfill_purpose_for_notify_log_entries.rb new file mode 100644 index 0000000000..cd9691e3c8 --- /dev/null +++ b/db/data/20260305165603_backfill_purpose_for_notify_log_entries.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +class BackfillPurposeForNotifyLogEntries < ActiveRecord::Migration[8.1] + def up + migration = self.class.name + started_at = Time.zone.now + + batch_size = 1000 + total_records = NotifyLogEntry.count + total_batches = (total_records / batch_size.to_f).ceil + + batch_processed = 0 + records_updated = 0 + + # Helps us monitor progress in CloudWatch + Rails.logger.info(event: "data_migration_start", migration:, total_records:, total_batches:) + + NotifyLogEntry.find_in_batches(batch_size:) do |notify_log_entries| + notify_log_entries.filter_map do |notify_log_entry| + template_name = NotifyTemplate.find_by_id( + notify_log_entry.template_id, + channel: notify_log_entry.type.to_sym + )&.name + + next unless template_name + + purpose = NotifyLogEntry.purpose_for_template_name(template_name) + + next unless purpose + + notify_log_entry.update_column( + :purpose, + NotifyLogEntry.purposes.fetch(purpose) + ) + + records_updated += 1 + end + + batch_processed += 1 + + Rails.logger.info( + event: "data_migration_batch", + migration:, + records_updated:, + batch_size:, + batch_processed:, + total_batches:, + ) + end + + duration_minutes = ((Time.zone.now - started_at) / 60.0).round + + Rails.logger.info( + event: "data_migration_finish", + migration:, + duration_minutes:, + total_records:, + records_updated:, + ) + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/data_schema.rb b/db/data_schema.rb new file mode 100644 index 0000000000..e5eaff18b6 --- /dev/null +++ b/db/data_schema.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +DataMigrate::Data.define(version: 20_260_304_173_751) From 7274b76319aac20110fdc3e7e0c5906df5013f71 Mon Sep 17 00:00:00 2001 From: John Henderson Date: Mon, 9 Mar 2026 09:59:37 +0000 Subject: [PATCH 3/3] Remove manual data migration for `NotifyLogEntry` Now that this backfill is moved into a data migration, we can remove the manual version. Jira-Issue: MAV-4101 --- .../backfill_notify_log_entries.rb | 49 ------------------- lib/tasks/data_migration.rake | 8 --- .../backfill_notify_log_entries_spec.rb | 29 ----------- 3 files changed, 86 deletions(-) delete mode 100644 app/lib/data_migration/backfill_notify_log_entries.rb delete mode 100644 lib/tasks/data_migration.rake delete mode 100644 spec/lib/data_migration/backfill_notify_log_entries_spec.rb diff --git a/app/lib/data_migration/backfill_notify_log_entries.rb b/app/lib/data_migration/backfill_notify_log_entries.rb deleted file mode 100644 index 8e910fe887..0000000000 --- a/app/lib/data_migration/backfill_notify_log_entries.rb +++ /dev/null @@ -1,49 +0,0 @@ -# frozen_string_literal: true - -class DataMigration::BackfillNotifyLogEntries - def call - progress_bar = - # rubocop:disable Rails/SaveBang - ProgressBar.create( - total: NotifyLogEntry.count, - format: "%a %b\u{15E7}%i %p%% %t", - progress_mark: " ", - remainder_mark: "\u{FF65}" - ) - # rubocop:enable Rails/SaveBang - - NotifyLogEntry.find_in_batches do |notify_log_entries| - notify_log_entries.filter_map do |notify_log_entry| - template_name = resolved_template_name(notify_log_entry) - - next unless template_name - - purpose = NotifyLogEntry.purpose_for_template_name(template_name) - - next unless purpose - - notify_log_entry.update_column( - :purpose, - NotifyLogEntry.purposes.fetch(purpose) - ) - end - - progress_bar.progress += 1 - end - - progress_bar.finish - end - - def self.call(...) = new(...).call - - private_class_method :new - - private - - def resolved_template_name(notify_log_entry) - NotifyTemplate.find_by_id( - notify_log_entry.template_id, - channel: notify_log_entry.type.to_sym - )&.name - end -end diff --git a/lib/tasks/data_migration.rake b/lib/tasks/data_migration.rake deleted file mode 100644 index 4ccf184544..0000000000 --- a/lib/tasks/data_migration.rake +++ /dev/null @@ -1,8 +0,0 @@ -# frozen_string_literal: true - -namespace :data_migration do - desc "Backfill purpose column for NotifyLogEntry records" - task backfill_notify_log_entries: :environment do - DataMigration::BackfillNotifyLogEntries.call - end -end diff --git a/spec/lib/data_migration/backfill_notify_log_entries_spec.rb b/spec/lib/data_migration/backfill_notify_log_entries_spec.rb deleted file mode 100644 index 1983230296..0000000000 --- a/spec/lib/data_migration/backfill_notify_log_entries_spec.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -describe DataMigration::BackfillNotifyLogEntries do - describe ".call" do - let!(:mapped_entry) do - create( - :notify_log_entry, - :email, - template_id: "14e88a09-4281-4257-9574-6afeaeb42715", - purpose: nil - ) - end - let!(:unknown_entry) do - create( - :notify_log_entry, - :email, - template_id: "99999999-9999-9999-9999-999999999999", - purpose: nil - ) - end - - it "backfills purpose from historical template ID mappings" do - described_class.call - - expect(mapped_entry.reload.purpose).to eq("consent_request") - expect(unknown_entry.reload.purpose).to be_nil - end - end -end