Browse Source

migration(crons): Clean up orphaned monitors (#46899)

Cleans up orphaned monitors that don't have a valid `project` or
`organization`
Richard Ortenberg 1 year ago
parent
commit
4fdd396736

+ 1 - 1
migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0405_rulesnooze_user_null
+sentry: 0406_monitor_cleanup
 social_auth: 0001_initial

+ 67 - 0
src/sentry/migrations/0406_monitor_cleanup.py

@@ -0,0 +1,67 @@
+# Generated by Django 2.2.28 on 2023-04-04 16:36
+
+from django.db import migrations
+
+from sentry.monitors.models import MonitorStatus
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapperWithProgressBar
+
+
+def clean_up_monitors(apps, schema_editor):
+    Monitor = apps.get_model("sentry", "Monitor")
+    Organization = apps.get_model("sentry", "Organization")
+    Project = apps.get_model("sentry", "Project")
+
+    queryset = RangeQuerySetWrapperWithProgressBar(
+        Monitor.objects.filter(monitorenvironment__isnull=True)
+        .exclude(status__in=[MonitorStatus.PENDING_DELETION, MonitorStatus.DELETION_IN_PROGRESS])
+        .values_list(
+            "id",
+            "organization_id",
+            "project_id",
+        ),
+        result_value_getter=lambda item: item[0],
+    )
+
+    monitors_to_clean_up = []
+
+    for monitor_id, organization_id, project_id in queryset:
+        try:
+            Organization.objects.get(id=organization_id)
+            Project.objects.get(id=project_id)
+        except (Organization.DoesNotExist, Project.DoesNotExist):
+            monitors_to_clean_up.append(monitor_id)
+
+    Monitor.objects.filter(id__in=monitors_to_clean_up).delete()
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0405_rulesnooze_user_null"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            clean_up_monitors,
+            migrations.RunPython.noop,
+            hints={
+                "tables": [
+                    "sentry_monitor",
+                    "sentry_organization",
+                    "sentry_project",
+                ]
+            },
+        ),
+    ]

+ 67 - 0
tests/sentry/migrations/test_0406_monitor_cleanup.py

@@ -0,0 +1,67 @@
+from datetime import timedelta
+
+import pytest
+from django.utils import timezone
+
+from sentry.monitors.models import Monitor, MonitorType, ScheduleType
+from sentry.testutils.cases import TestMigrations
+
+DEFAULT_ENVIRONMENT_NAME = "production"
+
+
+class MigrateMonitorEnvironmentBackfillInitialTest(TestMigrations):
+    migrate_from = "0405_rulesnooze_user_null"
+    migrate_to = "0406_monitor_cleanup"
+
+    def setup_before_migration(self, apps):
+        self.monitor = Monitor.objects.create(
+            name="exists",
+            organization_id=self.organization.id,
+            project_id=self.project.id,
+            next_checkin=timezone.now() + timedelta(minutes=1),
+            type=MonitorType.CRON_JOB,
+            config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
+        )
+
+        deleted_project = self.create_project(
+            organization=self.organization,
+            name="deleted_project",
+            slug="delete",
+            teams=[self.team],
+            fire_project_created=True,
+        )
+
+        self.project_monitor = Monitor.objects.create(
+            organization_id=self.organization.id,
+            project_id=deleted_project.id,
+            next_checkin=timezone.now() + timedelta(minutes=2),
+            type=MonitorType.CRON_JOB,
+            config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
+        )
+
+        deleted_project.delete()
+
+        deleted_organization = self.create_organization(name="deleted_org", owner=self.user)
+        deleted_org_project = self.create_project(organization=deleted_organization)
+
+        self.org_monitor = Monitor.objects.create(
+            organization_id=deleted_organization.id,
+            project_id=deleted_org_project.id,
+            next_checkin=timezone.now() + timedelta(minutes=3),
+            type=MonitorType.CRON_JOB,
+            config={"schedule": "* * * * *", "schedule_type": ScheduleType.CRONTAB},
+        )
+
+        deleted_organization.delete()
+
+    def test(self):
+        monitor = Monitor.objects.get(id=self.monitor.id)
+
+        assert monitor is not None
+        assert monitor.name == "exists"
+
+        with pytest.raises(Monitor.DoesNotExist):
+            Monitor.objects.get(id=self.project_monitor.id)
+
+        with pytest.raises(Monitor.DoesNotExist):
+            Monitor.objects.get(id=self.org_monitor.id)