From 5be6c403a4a4cc75f7953de83d2c2f2eccf57769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Mon, 10 Dec 2018 17:51:42 +0200 Subject: [PATCH] Flip model, for tracking status changes of the Check objects. --- CHANGELOG.md | 1 + hc/api/admin.py | 7 +- hc/api/management/commands/sendalerts.py | 106 ++++++++++++++--------- hc/api/migrations/0045_flip.py | 25 ++++++ hc/api/models.py | 28 +++++- hc/api/tests/test_sendalerts.py | 73 ++++++++++------ 6 files changed, 166 insertions(+), 74 deletions(-) create mode 100644 hc/api/migrations/0045_flip.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 801be26f..66315bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. - Additional python usage examples - Allow simultaneous access to checks from different teams - Add CORS support to API endpoints +- Flip model, for tracking status changes of the Check objects ### Bug Fixes - Fix after-login redirects (the "?next=" query parameter) diff --git a/hc/api/admin.py b/hc/api/admin.py index 196ec69e..1e6069ca 100644 --- a/hc/api/admin.py +++ b/hc/api/admin.py @@ -2,7 +2,7 @@ from django.contrib import admin from django.core.paginator import Paginator from django.db import connection from django.utils.safestring import mark_safe -from hc.api.models import Channel, Check, Notification, Ping +from hc.api.models import Channel, Check, Flip, Notification, Ping from hc.lib.date import format_duration @@ -197,3 +197,8 @@ class NotificationsAdmin(admin.ModelAdmin): def channel_value(self, obj): return obj.channel.value + + +@admin.register(Flip) +class FlipsAdmin(admin.ModelAdmin): + list_display = ("id", "created", "owner", "old_status", "new_status") diff --git a/hc/api/management/commands/sendalerts.py b/hc/api/management/commands/sendalerts.py index 09fc8920..1adf87f1 100644 --- a/hc/api/management/commands/sendalerts.py +++ b/hc/api/management/commands/sendalerts.py @@ -3,32 +3,39 @@ from threading import Thread from django.core.management.base import BaseCommand from django.utils import timezone -from hc.api.models import Check +from hc.api.models import Check, Flip -def notify(check_id, stdout): - check = Check.objects.get(id=check_id) +def notify(flip_id, stdout): + flip = Flip.objects.get(id=flip_id) + + check = flip.owner + # Set the historic status here but *don't save it*. + # It would be nicer to pass the status explicitly, as a separate parameter. + check.status = flip.new_status + # And just to make sure it doesn't get saved by a future coding accident: + setattr(check, "save", None) + tmpl = "Sending alert, status=%s, code=%s\n" - stdout.write(tmpl % (check.status, check.code)) + stdout.write(tmpl % (flip.new_status, check.code)) # Set dates for followup nags - if check.status == "down" and check.user.profile: + if flip.new_status == "down" and check.user.profile: check.user.profile.set_next_nag_date() # Send notifications - errors = check.send_alert() + errors = check.send_alert(flip) for ch, error in errors: stdout.write("ERROR: %s %s %s\n" % (ch.kind, ch.value, error)) -def notify_on_thread(check_id, stdout): - t = Thread(target=notify, args=(check_id, stdout)) +def notify_on_thread(flip_id, stdout): + t = Thread(target=notify, args=(flip_id, stdout)) t.start() class Command(BaseCommand): help = 'Sends UP/DOWN email alerts' - owned = Check.objects.filter(user__isnull=False).order_by("alert_after") def add_arguments(self, parser): parser.add_argument( @@ -47,56 +54,73 @@ class Command(BaseCommand): help='Send alerts synchronously, without using threads', ) - def handle_one(self, use_threads=True): - """ Process a single check. """ + def process_one_flip(self, use_threads=True): + """ Find unprocessed flip, send notifications. """ - now = timezone.now() + # Order by processed, otherwise Django will automatically order by id + # and make the query less efficient + q = Flip.objects.filter(processed=None).order_by("processed") + flip = q.first() + if flip is None: + return False - # Look for checks that are going down - q = self.owned.filter(alert_after__lt=now, status="up") - check = q.first() + q = Flip.objects.filter(id=flip.id, processed=None) + num_updated = q.update(processed=timezone.now()) + if num_updated != 1: + # Nothing got updated: another worker process got there first. + return True - # If none found, look for checks that are going up - if not check: - q = self.owned.filter(alert_after__gt=now, status="down") - check = q.first() + if use_threads: + notify_on_thread(flip.id, self.stdout) + else: + notify(flip.id, self.stdout) + + return True + def handle_going_down(self): + """ Process a single check going down. """ + + now = timezone.now() + + check = Check.objects.filter(alert_after__lt=now, status="up").first() if check is None: return False - q = Check.objects.filter(id=check.id, status=check.status) - current_status = check.get_status() - - # During the grace period sendalerts considers the check as "up": - if current_status == "grace": - current_status = "up" + q = Check.objects.filter(id=check.id, status="up") - if check.status == current_status: - # Stored status is already up-to-date. Update alert_after - # as needed but don't send notifications + current_status = check.get_status() + if current_status != "down": + # It is not down yet. Update alert_after q.update(alert_after=check.get_alert_after()) return True - else: - # Atomically update status to the opposite - num_updated = q.update(status=current_status) - if num_updated == 1: - # Send notifications only if status update succeeded - # (no other sendalerts process got there first) - if use_threads: - notify_on_thread(check.id, self.stdout) - else: - notify(check.id, self.stdout) - return True + # Atomically update status + num_updated = q.update(status="down") + if num_updated != 1: + # Nothing got updated: another worker process got there first. + return True + + flip = Flip(owner=check) + flip.created = check.get_alert_after() + flip.old_status = "up" + flip.new_status = "down" + flip.save() - return False + check.status = "down" + check.save() + return True def handle(self, use_threads=True, loop=True, *args, **options): self.stdout.write("sendalerts is now running\n") i, sent = 0, 0 while True: - while self.handle_one(use_threads): + # Create flips for any checks going down + while self.handle_going_down(): + pass + + # Process the unprocessed flips + while self.process_one_flip(use_threads): sent += 1 if not loop: diff --git a/hc/api/migrations/0045_flip.py b/hc/api/migrations/0045_flip.py new file mode 100644 index 00000000..d9348db5 --- /dev/null +++ b/hc/api/migrations/0045_flip.py @@ -0,0 +1,25 @@ +# Generated by Django 2.1.4 on 2018-12-10 15:15 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0044_auto_20181120_2004'), + ] + + operations = [ + migrations.CreateModel( + name='Flip', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', models.DateTimeField()), + ('processed', models.DateTimeField(blank=True, db_index=True, null=True)), + ('old_status', models.CharField(choices=[('up', 'Up'), ('down', 'Down'), ('new', 'New'), ('paused', 'Paused')], max_length=8)), + ('new_status', models.CharField(choices=[('up', 'Up'), ('down', 'Down'), ('new', 'New'), ('paused', 'Paused')], max_length=8)), + ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='api.Check')), + ], + ), + ] diff --git a/hc/api/models.py b/hc/api/models.py index 82963536..3eccd9a4 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -92,8 +92,12 @@ class Check(models.Model): def email(self): return "%s@%s" % (self.code, settings.PING_EMAIL_DOMAIN) - def send_alert(self): - if self.status not in ("up", "down"): + def send_alert(self, flip): + if flip.new_status == "up" and flip.old_status in ("new", "paused"): + # Don't send alerts on new->up and paused->up transitions + return [] + + if flip.new_status not in ("up", "down"): raise NotImplementedError("Unexpected status: %s" % self.status) errors = [] @@ -199,8 +203,16 @@ class Check(models.Model): self.last_ping_was_fail = is_fail self.has_confirmation_link = "confirm" in str(body).lower() self.alert_after = self.get_alert_after() - if self.status in ("new", "paused"): - self.status = "up" + + new_status = "down" if is_fail else "up" + if self.status != new_status: + flip = Flip(owner=self) + flip.created = self.last_ping + flip.old_status = self.status + flip.new_status = new_status + flip.save() + + self.status = new_status self.save() self.refresh_from_db() @@ -539,3 +551,11 @@ class Notification(models.Model): def bounce_url(self): return settings.SITE_ROOT + reverse("hc-api-bounce", args=[self.code]) + + +class Flip(models.Model): + owner = models.ForeignKey(Check, models.CASCADE) + created = models.DateTimeField() + processed = models.DateTimeField(null=True, blank=True, db_index=True) + old_status = models.CharField(max_length=8, choices=STATUSES) + new_status = models.CharField(max_length=8, choices=STATUSES) diff --git a/hc/api/tests/test_sendalerts.py b/hc/api/tests/test_sendalerts.py index 6ddb0712..dc568e63 100644 --- a/hc/api/tests/test_sendalerts.py +++ b/hc/api/tests/test_sendalerts.py @@ -5,7 +5,7 @@ from mock import Mock, patch from django.core.management import call_command from django.utils.timezone import now from hc.api.management.commands.sendalerts import Command, notify -from hc.api.models import Check +from hc.api.models import Flip, Check from hc.test import BaseTestCase @@ -18,50 +18,54 @@ class SendAlertsTestCase(BaseTestCase): check.alert_after = check.get_alert_after() check.save() - # Expect no exceptions-- - Command().handle_one() + Command().handle_going_down() - @patch("hc.api.management.commands.sendalerts.notify_on_thread") - def test_it_notifies_when_check_goes_down(self, mock_notify): + self.assertEqual(Flip.objects.count(), 0) + + def test_it_creates_a_flip_when_check_goes_down(self): check = Check(user=self.alice, status="up") check.last_ping = now() - timedelta(days=2) check.alert_after = check.get_alert_after() check.save() - result = Command().handle_one() + result = Command().handle_going_down() # If it finds work, it should return True self.assertTrue(result) + # It should create a flip object + flip = Flip.objects.get() + self.assertEqual(flip.owner_id, check.id) + self.assertEqual(flip.new_status, "down") + # It should change stored status to "down" check.refresh_from_db() self.assertEqual(check.status, "down") - # It should call `notify_on_thread` - self.assertTrue(mock_notify.called) - @patch("hc.api.management.commands.sendalerts.notify_on_thread") - def test_it_notifies_when_check_goes_up(self, mock_notify): - check = Check(user=self.alice, status="down") + def test_it_processes_flip(self, mock_notify): + check = Check(user=self.alice, status="up") check.last_ping = now() check.alert_after = check.get_alert_after() check.save() - result = Command().handle_one() + flip = Flip(owner=check, created=check.last_ping) + flip.old_status = "down" + flip.new_status = "up" + flip.save() + + result = Command().process_one_flip() # If it finds work, it should return True self.assertTrue(result) - # It should change stored status to "up" - check.refresh_from_db() - self.assertEqual(check.status, "up") + # It should set the processed date + flip.refresh_from_db() + self.assertTrue(flip.processed) # It should call `notify_on_thread` self.assertTrue(mock_notify.called) - # alert_after now should be set - self.assertTrue(check.alert_after) - @patch("hc.api.management.commands.sendalerts.notify_on_thread") def test_it_updates_alert_after(self, mock_notify): check = Check(user=self.alice, status="up") @@ -69,19 +73,17 @@ class SendAlertsTestCase(BaseTestCase): check.alert_after = check.last_ping check.save() - result = Command().handle_one() + result = Command().handle_going_down() # If it finds work, it should return True self.assertTrue(result) - # It should change stored status to "down" - check.refresh_from_db() - # alert_after should have been increased + check.refresh_from_db() self.assertTrue(check.alert_after > check.last_ping) - # notify_on_thread should *not* have been called - self.assertFalse(mock_notify.called) + # a flip should have not been created + self.assertEqual(Flip.objects.count(), 0) @patch("hc.api.management.commands.sendalerts.notify") def test_it_works_synchronously(self, mock_notify): @@ -105,7 +107,12 @@ class SendAlertsTestCase(BaseTestCase): check.alert_after = check.get_alert_after() check.save() - notify(check.id, Mock()) + flip = Flip(owner=check, created=check.last_ping) + flip.old_status = "up" + flip.new_status = "down" + flip.save() + + notify(flip.id, Mock()) self.profile.refresh_from_db() self.assertIsNotNone(self.profile.next_nag_date) @@ -119,7 +126,12 @@ class SendAlertsTestCase(BaseTestCase): check.alert_after = check.get_alert_after() check.save() - notify(check.id, Mock()) + flip = Flip(owner=check, created=check.last_ping) + flip.old_status = "up" + flip.new_status = "down" + flip.save() + + notify(flip.id, Mock()) self.bobs_profile.refresh_from_db() self.assertIsNotNone(self.bobs_profile.next_nag_date) @@ -135,7 +147,12 @@ class SendAlertsTestCase(BaseTestCase): check.alert_after = check.get_alert_after() check.save() - notify(check.id, Mock()) + flip = Flip(owner=check, created=check.last_ping) + flip.old_status = "up" + flip.new_status = "down" + flip.save() + + notify(flip.id, Mock()) self.profile.refresh_from_db() - self.assertEqual(self.profile.next_nag_date, original_nag_date) \ No newline at end of file + self.assertEqual(self.profile.next_nag_date, original_nag_date)