Implemented AC-166, added signal handlers to update has_active_failures flag whenever host/group/job are changed/deleted and whenever group-host and group-group relationships change.

app_setup/templates/local_settings.py.j2

@@ -106,6 +106,7 @@ LOGGING['handlers']['syslog'] = {
 
 # Enable the following lines to turn on lots of permissions-related logging.
 #LOGGING['loggers']['awx.main.access']['propagate'] = True
+#LOGGING['loggers']['awx.main.signals']['propagate'] = True
 #LOGGING['loggers']['awx.main.permissions']['propagate'] = True
 
 # Define additional environment variables to be passed to subprocess started by

awx/main/management/commands/inventory_import.py

@@ -437,16 +437,18 @@ class Command(NoArgsCommand):
         LOGGER.info("MODIFYING INVENTORY: %s" % inventory.name)
 
         # if overwrite is set, for each host in the database but NOT in the local
-        # list, delete it
+        # list, delete it. Delete individually so signal handlers will run.
         if overwrite:
             LOGGER.info("deleting any hosts not in the remote source: %s" % host_names.keys())
-            Host.objects.exclude(name__in = host_names.keys()).filter(inventory=inventory).delete()
+            for host in Host.objects.exclude(name__in = host_names.keys()).filter(inventory=inventory):
+                host.delete()
 
         # if overwrite is set, for each group in the database but NOT in the local
-        # list, delete it
+        # list, delete it. Delete individually so signal handlers will run.
         if overwrite:
             LOGGER.info("deleting any groups not in the remote source")
-            Group.objects.exclude(name__in = group_names.keys()).filter(inventory=inventory).delete()
+            for group in Group.objects.exclude(name__in = group_names.keys()).filter(inventory=inventory):
+                group.delete()
 
         # if overwrite is set, throw away all invalid child relationships for groups
         if overwrite:

awx/main/models/__init__.py

@@ -4,6 +4,7 @@
 # Python
 import hmac
 import json
+import logging
 import os
 import shlex
 
@@ -12,10 +13,8 @@ import yaml
 
 # Django
 from django.conf import settings
-from django.db import models, DatabaseError
+from django.db import models
 from django.db.models import CASCADE, SET_NULL, PROTECT
-from django.db.models.signals import post_save
-from django.dispatch import receiver
 from django.utils.translation import ugettext_lazy as _
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
@@ -30,15 +29,14 @@ from taggit.managers import TaggableManager
 # Django-Celery
 from djcelery.models import TaskMeta
 
-# Django-REST-Framework
-from rest_framework.authtoken.models import Token
-
 __all__ = ['PrimordialModel', 'Organization', 'Team', 'Project', 'Credential',
            'Inventory', 'Host', 'Group', 'Permission', 'JobTemplate', 'Job',
            'JobHostSummary', 'JobEvent', 'PERM_INVENTORY_ADMIN',
            'PERM_INVENTORY_READ', 'PERM_INVENTORY_WRITE',
            'PERM_INVENTORY_DEPLOY', 'PERM_INVENTORY_CHECK']
 
+logger = logging.getLogger('awx.main.models')
+
 # TODO: reporting model TBD
 
 PERM_INVENTORY_ADMIN  = 'admin'
@@ -168,10 +166,19 @@ class Inventory(CommonModel):
         except ValueError:
             return yaml.safe_load(self.variables)
 
-    def update_has_active_failures(self):
+    def update_has_active_failures(self, update_groups=True, update_hosts=True):
+        if update_hosts:
+            for host in self.hosts.filter(active=True):
+                host.update_has_active_failures(update_inventory=False,
+                                                update_groups=False)
+        if update_groups:
+            for group in self.groups.filter(active=True):
+                group.update_has_active_failures()
         failed_hosts = self.hosts.filter(active=True, has_active_failures=True)
-        self.has_active_failures = bool(failed_hosts.count())
+        has_active_failures = bool(failed_hosts.count())
-        self.save()
+        if self.has_active_failures != has_active_failures:
+            self.has_active_failures = has_active_failures
+            self.save()
 
 class Host(CommonModelNameNotUnique):
     '''
@@ -199,15 +206,20 @@ class Host(CommonModelNameNotUnique):
     def get_absolute_url(self):
         return reverse('main:host_detail', args=(self.pk,))
 
-    def update_has_active_failures(self, update_groups=True, update_inventory=True):
+    def update_has_active_failures(self, update_inventory=True,
-        self.has_active_failures = bool(self.last_job_host_summary and
+                                   update_groups=True):
-                                        self.last_job_host_summary.failed)
+        has_active_failures = bool(self.last_job_host_summary and
-        self.save()
+                                   self.last_job_host_summary.job.active and
+                                   self.last_job_host_summary.failed)
+        if self.has_active_failures != has_active_failures:
+            self.has_active_failures = has_active_failures
+            self.save()
+        if update_inventory:
+            self.inventory.update_has_active_failures(update_groups=False,
+                                                      update_hosts=False)
         if update_groups:
             for group in self.all_groups.filter(active=True):
                 group.update_has_active_failures()
-        if update_inventory:
-            self.inventory.update_has_active_failures()
 
     @property
     def variables_dict(self):
@@ -259,9 +271,12 @@ class Group(CommonModelNameNotUnique):
 
     def update_has_active_failures(self):
         failed_hosts = self.all_hosts.filter(active=True,
+                                             last_job_host_summary__job__active=True,
                                              last_job_host_summary__failed=True)
-        self.has_active_failures = bool(failed_hosts.count())
+        has_active_failures = bool(failed_hosts.count())
-        self.save()
+        if self.has_active_failures != has_active_failures:
+            self.has_active_failures = has_active_failures
+            self.save()
 
     @property
     def variables_dict(self):
@@ -1224,16 +1239,5 @@ from awx.main.access import *
 User.add_to_class('get_queryset', get_user_queryset)
 User.add_to_class('can_access', check_user_access)
 
-@receiver(post_save, sender=User)
+# Import signal handlers only after models have been defined.
-def create_auth_token_for_user(sender, **kwargs):
+import awx.main.signals
-    instance = kwargs.get('instance', None)
-    if instance:
-        try:
-            Token.objects.get_or_create(user=instance)
-        except DatabaseError:
-            pass    
-    # Only fails when creating a new superuser from syncdb on a
-    # new database (before migrate has been called).
-
-# FIXME: Update Group.has_active_failures when a Host/Group is deleted or
-# marked inactive, or when a Host-Group or Group-Group relationship is updated.

awx/main/signals.py

@@ -0,0 +1,77 @@
+# Copyright (c) 2013 AnsibleWorks, Inc.
+# All Rights Reserved.
+
+# Python
+import logging
+import threading
+
+# Django
+from django.contrib.auth.models import User
+from django.db import DatabaseError
+from django.db.models.signals import post_save, post_delete, m2m_changed
+from django.dispatch import receiver
+
+# Django-REST-Framework
+from rest_framework.authtoken.models import Token
+
+# AWX
+from awx.main.models import *
+
+__all__ = []
+
+logger = logging.getLogger('awx.main.signals')
+
+@receiver(post_save, sender=User)
+def create_auth_token_for_user(sender, **kwargs):
+    instance = kwargs.get('instance', None)
+    if instance:
+        try:
+            Token.objects.get_or_create(user=instance)
+        except DatabaseError:
+            pass    
+    # Only fails when creating a new superuser from syncdb on a
+    # new database (before migrate has been called).
+
+
+# Update has_active_failures for inventory/groups when a Host/Group is deleted
+# or marked inactive, when a Host-Group or Group-Group relationship is updated,
+# or when a Job is deleted or marked inactive.
+
+_inventory_updating = threading.local()
+
+def update_inventory_has_active_failures(sender, **kwargs):
+    '''
+    Signal handler and wrapper around inventory.update_has_active_failures to
+    prevent unnecessary recursive calls.
+    '''
+    if not getattr(_inventory_updating, 'is_updating', False):
+        if sender == Group.hosts.through:
+            sender_name = 'group.hosts'
+        elif sender == Group.parents.through:
+            sender_name = 'group.parents'
+        else:
+            sender_name = unicode(sender._meta.verbose_name)
+        if kwargs['signal'] == post_save:
+            sender_action = 'saved'
+        elif kwargs['signal'] == post_delete:
+            sender_action = 'deleted'
+        else:
+            sender_action = 'changed'
+        logger.debug('%s %s, updating inventory has_active_failures: %r %r',
+                     sender_name, sender_action, sender, kwargs)
+        try:
+            _inventory_updating.is_updating = True
+            inventory = kwargs['instance'].inventory
+            update_hosts = issubclass(sender, Job)
+            inventory.update_has_active_failures(update_hosts=update_hosts)
+        finally:
+            _inventory_updating.is_updating = False
+
+post_save.connect(update_inventory_has_active_failures, sender=Host)
+post_delete.connect(update_inventory_has_active_failures, sender=Host)
+post_save.connect(update_inventory_has_active_failures, sender=Group)
+post_delete.connect(update_inventory_has_active_failures, sender=Group)
+m2m_changed.connect(update_inventory_has_active_failures, sender=Group.hosts.through)
+m2m_changed.connect(update_inventory_has_active_failures, sender=Group.parents.through)
+post_save.connect(update_inventory_has_active_failures, sender=Job)
+post_delete.connect(update_inventory_has_active_failures, sender=Job)

awx/main/tests/tasks.py

@@ -349,6 +349,79 @@ class RunJobTest(BaseCeleryTest):
         self.assertEqual(job.unreachable_hosts.count(), 0)
         self.assertEqual(job.skipped_hosts.count(), 0)
         self.assertEqual(job.processed_hosts.count(), 1)
+        return job
+
+    def test_update_has_active_failures_when_inventory_changes(self):
+        job = self.test_run_job_that_fails()
+        # Add host to new group (should set has_active_failures)
+        new_group = self.inventory.groups.create(name='new group')
+        self.assertFalse(new_group.has_active_failures)
+        new_group.hosts.add(self.host)
+        new_group = Group.objects.get(pk=new_group.pk)
+        self.assertTrue(new_group.has_active_failures)
+        # Remove host from new group (should clear has_active_failures)
+        new_group.hosts.remove(self.host)
+        new_group = Group.objects.get(pk=new_group.pk)
+        self.assertFalse(new_group.has_active_failures)
+        # Add existing group to new group (should set flag)
+        new_group.children.add(self.group)
+        new_group = Group.objects.get(pk=new_group.pk)
+        self.assertTrue(new_group.has_active_failures)
+        # Remove existing group from new group (should clear flag)
+        new_group.children.remove(self.group)
+        new_group = Group.objects.get(pk=new_group.pk)
+        self.assertFalse(new_group.has_active_failures)
+        # Mark host inactive (should clear flag on parent group and inventory)
+        self.host.mark_inactive()
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertFalse(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertFalse(self.inventory.has_active_failures)
+        # Un-mark host as inactive (should set flag on group and inventory)
+        host = self.host
+        host.name = '_'.join(host.name.split('_')[3:]) or 'undeleted host'
+        host.active = True
+        host.save()
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertTrue(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertTrue(self.inventory.has_active_failures)
+        # Delete host. (should clear flag)
+        self.host.delete()
+        self.host = None
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertFalse(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertFalse(self.inventory.has_active_failures)
+
+    def test_update_has_active_failures_when_job_removed(self):
+        job = self.test_run_job_that_fails()
+        # Mark job as inactive (should clear flags).
+        job.mark_inactive()
+        self.host = Host.objects.get(pk=self.host.pk)
+        self.assertFalse(self.host.has_active_failures)
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertFalse(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertFalse(self.inventory.has_active_failures)
+        # Un-mark job as inactive (should set flag on host, group and inventory)
+        job.name = '_'.join(job.name.split('_')[3:]) or 'undeleted job'
+        job.active = True
+        job.save()
+        self.host = Host.objects.get(pk=self.host.pk)
+        self.assertTrue(self.host.has_active_failures)
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertTrue(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertTrue(self.inventory.has_active_failures)
+        # Delete job entirely.
+        job.delete()
+        self.host = Host.objects.get(pk=self.host.pk)
+        self.assertFalse(self.host.has_active_failures)
+        self.group = Group.objects.get(pk=self.group.pk)
+        self.assertFalse(self.group.has_active_failures)
+        self.inventory = Inventory.objects.get(pk=self.inventory.pk)
+        self.assertFalse(self.inventory.has_active_failures)
 
     def test_check_job_where_task_would_fail(self):
         self.create_test_project(TEST_PLAYBOOK2)

awx/settings/defaults.py

@@ -329,11 +329,15 @@ LOGGING = {
             'handlers': ['console', 'file', 'syslog'],
             'level': 'DEBUG',
         },
+        'awx.main.access': {
+            'handlers': ['null'],
+            'propagate': False,
+        },
         'awx.main.permissions': {
             'handlers': ['null'],
             'propagate': False,
         },
-        'awx.main.access': {
+        'awx.main.signals': {
             'handlers': ['null'],
             'propagate': False,
         },

awx/settings/local_settings.py.example

@@ -108,6 +108,7 @@ LOGGING['handlers']['syslog'] = {
 
 # Enable the following lines to turn on lots of permissions-related logging.
 #LOGGING['loggers']['awx.main.access']['propagate'] = True
+#LOGGING['loggers']['awx.main.signals']['propagate'] = True
 #LOGGING['loggers']['awx.main.permissions']['propagate'] = True
 
 # Define additional environment variables to be passed to subprocess started by