diff --git a/awx/main/models/__init__.py b/awx/main/models/__init__.py index 8ea35858c4..315b7480c2 100644 --- a/awx/main/models/__init__.py +++ b/awx/main/models/__init__.py @@ -269,6 +269,16 @@ class Group(CommonModelNameNotUnique): def get_absolute_url(self): return reverse('main:group_detail', args=(self.pk,)) + def mark_inactive(self, save=True): + ''' + When marking groups inactive, remove all associations to related + groups/hosts. + ''' + super(Group, self).mark_inactive(save=save) + self.parents.clear() + self.children.clear() + self.hosts.clear() + def update_has_active_failures(self): failed_hosts = self.all_hosts.filter(active=True, last_job_host_summary__job__active=True, diff --git a/awx/main/signals.py b/awx/main/signals.py index a54c44ff91..3180b4e4fd 100644 --- a/awx/main/signals.py +++ b/awx/main/signals.py @@ -8,7 +8,7 @@ import threading # Django from django.contrib.auth.models import User from django.db import DatabaseError -from django.db.models.signals import post_save, pre_delete, post_delete, m2m_changed +from django.db.models.signals import pre_save, post_save, pre_delete, post_delete, m2m_changed from django.dispatch import receiver # Django-REST-Framework @@ -45,6 +45,7 @@ def update_inventory_has_active_failures(sender, **kwargs): prevent unnecessary recursive calls. ''' if not getattr(_inventory_updating, 'is_updating', False): + instance = kwargs['instance'] if sender == Group.hosts.through: sender_name = 'group.hosts' elif sender == Group.parents.through: @@ -53,15 +54,19 @@ def update_inventory_has_active_failures(sender, **kwargs): sender_name = unicode(sender._meta.verbose_name) if kwargs['signal'] == post_save: sender_action = 'saved' + if instance.active: # No need to update for active instances. + return elif kwargs['signal'] == post_delete: sender_action = 'deleted' - else: + elif kwargs['signal'] == m2m_changed and kwargs['action'] in ('post_add', 'post_remove', 'post_clear'): sender_action = 'changed' + else: + return 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 + inventory = instance.inventory update_hosts = issubclass(sender, Job) inventory.update_has_active_failures(update_hosts=update_hosts) finally: @@ -102,18 +107,30 @@ def migrate_children_from_deleted_group_to_parent_groups(sender, **kwargs): child_group, parent_group) parent_group.children.add(child_group) +@receiver(pre_save, sender=Group) +def save_related_pks_before_group_marked_inactive(sender, **kwargs): + instance = kwargs['instance'] + if not instance.pk: + return + instance._saved_parents_pks = set(instance.parents.values_list('pk', flat=True)) + instance._saved_hosts_pks = set(instance.hosts.values_list('pk', flat=True)) + instance._saved_children_pks = set(instance.children.values_list('pk', flat=True)) + @receiver(post_save, sender=Group) def migrate_children_from_inactive_group_to_parent_groups(sender, **kwargs): instance = kwargs['instance'] if instance.active: return - for parent_group in instance.parents.all(): - for child_host in instance.hosts.all(): - logger.debug('moving host %s to parent %s after making group %s inactive', + parents_pks = getattr(instance, '_saved_parents_pks', []) + hosts_pks = getattr(instance, '_saved_hosts_pks', []) + children_pks = getattr(instance, '_saved_children_pks', []) + for parent_group in Group.objects.filter(pk__in=parents_pks): + for child_host in Host.objects.filter(pk__in=hosts_pks): + logger.debug('moving host %s to parent %s after marking group %s inactive', child_host, parent_group, instance) parent_group.hosts.add(child_host) - for child_group in instance.children.all(): - logger.debug('moving group %s to parent %s after making group %s inactive', + for child_group in Group.objects.filter(pk__in=children_pks): + logger.debug('moving group %s to parent %s after marking group %s inactive', child_group, parent_group, instance) parent_group.children.add(child_group) parent_group.children.remove(instance) diff --git a/awx/main/tests/inventory.py b/awx/main/tests/inventory.py index 3151c73bb1..f2d1473b27 100644 --- a/awx/main/tests/inventory.py +++ b/awx/main/tests/inventory.py @@ -333,7 +333,7 @@ class InventoryTest(BaseTest): self.post(url5, data=remove_me, expect=204, auth=self.get_other_credentials()) got = self.get(url5, expect=200, auth=self.get_other_credentials()) self.assertEquals(got['count'], 3) - + ################################################### # VARIABLES @@ -566,6 +566,17 @@ class InventoryTest(BaseTest): # try to double disassociate to see what happens (should no-op) self.post(subgroups_url3, data=result, expect=204, auth=self.get_other_credentials()) + # removed group should be automatically marked inactive once it no longer has any parents. + removed_group = Group.objects.get(pk=result['id']) + self.assertTrue(removed_group.parents.count()) + self.assertTrue(removed_group.active) + for parent in removed_group.parents.all(): + parent_children_url = reverse('main:group_children_list', args=(parent.pk,)) + data = {'id': removed_group.pk, 'disassociate': 1} + self.post(parent_children_url, data, expect=204, auth=self.get_super_credentials()) + removed_group = Group.objects.get(pk=result['id']) + self.assertFalse(removed_group.active) + ######################################################### # FIXME: TAGS @@ -628,11 +639,15 @@ class InventoryTest(BaseTest): self.assertFalse(h_d in g_c.hosts.all()) # Mark group C inactive. Its child groups and hosts should now also be - # attached to group A. Group D hosts should be unchanged. + # attached to group A. Group D hosts should be unchanged. Group C + # should also no longer have any group or host relationships. g_c.mark_inactive() self.assertTrue(g_d in g_a.children.all()) self.assertTrue(h_c in g_a.hosts.all()) self.assertFalse(h_d in g_a.hosts.all()) + self.assertFalse(g_c.parents.all()) + self.assertFalse(g_c.children.all()) + self.assertFalse(g_c.hosts.all()) def test_group_parents_and_children(self): # Test for various levels of group parent/child relations, with hosts, diff --git a/awx/main/tests/tasks.py b/awx/main/tests/tasks.py index c1d6b66f93..0974ef21fe 100644 --- a/awx/main/tests/tasks.py +++ b/awx/main/tests/tasks.py @@ -377,11 +377,13 @@ class RunJobTest(BaseCeleryTest): 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) + # Un-mark host as inactive (need to force update of flag on group and + # inventory) host = self.host host.name = '_'.join(host.name.split('_')[3:]) or 'undeleted host' host.active = True host.save() + host.update_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) @@ -404,10 +406,11 @@ class RunJobTest(BaseCeleryTest): 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) + # Un-mark job as inactive (need to force update of flag) job.name = '_'.join(job.name.split('_')[3:]) or 'undeleted job' job.active = True job.save() + job.inventory.update_has_active_failures() 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) diff --git a/awx/main/views.py b/awx/main/views.py index e00050889c..1191acd1e3 100644 --- a/awx/main/views.py +++ b/awx/main/views.py @@ -457,6 +457,19 @@ class GroupChildrenList(SubListCreateAPIView): parent_model = Group relationship = 'children' + def unattach(self, request, *args, **kwargs): + ''' + Special case for disassociating a child group from the parent. If the + child group has no more parents, then automatically mark it inactive. + ''' + response = super(GroupChildrenList, self).unattach(request, *args, **kwargs) + if response.status_code != status.HTTP_204_NO_CONTENT: + return response + sub = self.model.objects.get(pk=request.DATA.get('id', None)) + if sub.parents.filter(active=True).count() == 0: + sub.mark_inactive() + return response + class GroupHostsList(SubListCreateAPIView): ''' the list of hosts directly below a group '''