diff --git a/.github/workflows/linting-and-tests.yml b/.github/workflows/linting-and-tests.yml index 504363baa..d94506638 100644 --- a/.github/workflows/linting-and-tests.yml +++ b/.github/workflows/linting-and-tests.yml @@ -85,6 +85,59 @@ jobs: python manage.py makemigrations --check python manage.py lintmigrations + # the following CI check is to prevent developers from dropping columns in a way that could cause downtime + # (the proper way to drop columns is documented in dev/README.md) + # + # we've been bitten by this before (see https://raintank-corp.slack.com/archives/C081TNWM73N as an example) + ensure-database-migrations-drop-columns-the-correct-way: + name: "Ensure database migrations drop columns the correct way" + runs-on: ubuntu-latest + steps: + - name: Checkout PR code + uses: actions/checkout@v3 + with: + # Fetch all history so we can compare with the base branch + fetch-depth: 0 + # Checkout the head commit of the PR + ref: ${{ github.event.pull_request.head.sha }} + + - name: Fetch base branch + run: git fetch origin ${{ github.event.pull_request.base.ref }}:${{ github.event.pull_request.base.ref }} + + - name: Check for RemoveField in Migrations + # yamllint disable rule:line-length + run: | + # Get the list of files changed in the PR + git diff --name-only ${{ github.event.pull_request.base.ref }}...${{ github.event.pull_request.head.sha }} > changed_files.txt + + # Filter for migration files + grep -E '^.*/migrations/.*\.py$' changed_files.txt > migration_files.txt || true + + # Initialize a flag + FAILED=0 + + # Check each migration file for 'migrations.RemoveField' + if [ -s migration_files.txt ]; then + while IFS= read -r file; do + echo "Checking $file for migrations.RemoveField..." + if grep -q 'migrations.RemoveField' "$file"; then + echo "❌ Error: Found migrations.RemoveField in $file" + FAILED=1 + else + echo "✅ No RemoveField found in $file" + fi + done < migration_files.txt + else + echo "No migration files changed." + fi + + # Fail the job if RemoveField was found + if [ "$FAILED" -eq 1 ]; then + echo "❌ Error: Found migrations.RemoveField in one or more migration files. Please check out our documentation at https://github.com/grafana/oncall/tree/dev/dev#removing-a-nullable-field-from-a-model on how to properly drop columns." + exit 1 + fi + # yamllint enable rule:line-length + unit-test-helm-chart: name: "Helm Chart Unit Tests" runs-on: ubuntu-latest-16-cores diff --git a/engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py b/engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py index 03c5f5343..7c23e789e 100644 --- a/engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py +++ b/engine/apps/alerts/migrations/0066_remove_channelfilter__slack_channel_id_and_more.py @@ -12,14 +12,6 @@ class Migration(migrations.Migration): operations = [ linter.IgnoreMigration(), - migrations.RemoveField( - model_name='channelfilter', - name='_slack_channel_id', - ), - migrations.RemoveField( - model_name='resolutionnoteslackmessage', - name='_slack_channel_id', - ), migrations.DeleteModel( name='AlertGroupPostmortem', ), diff --git a/engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py b/engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py new file mode 100644 index 000000000..b819201d0 --- /dev/null +++ b/engine/apps/alerts/migrations/0067_remove_channelfilter__slack_channel_id_state.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.16 on 2024-11-20 20:21 + +import common.migrations.remove_field +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0066_remove_channelfilter__slack_channel_id_and_more'), + ] + + operations = [ + common.migrations.remove_field.RemoveFieldState( + model_name='channelfilter', + name='_slack_channel_id', + ), + ] diff --git a/engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py b/engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py new file mode 100644 index 000000000..a270331c7 --- /dev/null +++ b/engine/apps/alerts/migrations/0068_remove_resolutionnoteslackmessage__slack_channel_id_state.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.16 on 2024-11-20 20:23 + +import common.migrations.remove_field +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('alerts', '0067_remove_channelfilter__slack_channel_id_state'), + ] + + operations = [ + common.migrations.remove_field.RemoveFieldState( + model_name='resolutionnoteslackmessage', + name='_slack_channel_id', + ), + ] diff --git a/engine/apps/schedules/migrations/0020_remove_oncallschedule_channel.py b/engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py similarity index 62% rename from engine/apps/schedules/migrations/0020_remove_oncallschedule_channel.py rename to engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py index e4d191382..23e3f96f6 100644 --- a/engine/apps/schedules/migrations/0020_remove_oncallschedule_channel.py +++ b/engine/apps/schedules/migrations/0020_remove_oncallschedule_channel_state.py @@ -1,7 +1,7 @@ -# Generated by Django 4.2.16 on 2024-11-06 21:13 +# Generated by Django 4.2.16 on 2024-11-20 20:12 +import common.migrations.remove_field from django.db import migrations -import django_migration_linter as linter class Migration(migrations.Migration): @@ -11,8 +11,7 @@ class Migration(migrations.Migration): ] operations = [ - linter.IgnoreMigration(), - migrations.RemoveField( + common.migrations.remove_field.RemoveFieldState( model_name='oncallschedule', name='channel', ), diff --git a/engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id.py b/engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py similarity index 63% rename from engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id.py rename to engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py index 6d415bdb4..4e5d61ebe 100644 --- a/engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id.py +++ b/engine/apps/user_management/migrations/0028_remove_organization_general_log_channel_id_state.py @@ -1,7 +1,7 @@ -# Generated by Django 4.2.16 on 2024-11-06 21:11 +# Generated by Django 4.2.16 on 2024-11-20 17:58 +import common.migrations.remove_field from django.db import migrations -import django_migration_linter as linter class Migration(migrations.Migration): @@ -11,8 +11,7 @@ class Migration(migrations.Migration): ] operations = [ - linter.IgnoreMigration(), - migrations.RemoveField( + common.migrations.remove_field.RemoveFieldState( model_name='organization', name='general_log_channel_id', ),