Merge pull request #6796 from wwitzel3/issue-4382

Tweak background Inventory deletion.

awx/api/views.py

@@ -59,7 +59,7 @@ import ansiconv
 from social.backends.utils import load_backends
 
 # AWX
-from awx.main.tasks import send_notifications, update_host_smart_inventory_memberships, delete_inventory
+from awx.main.tasks import send_notifications, update_host_smart_inventory_memberships
 from awx.main.access import get_user_queryset
 from awx.main.ha import is_ha_environment
 from awx.api.authentication import TaskAuthentication, TokenGetAuthentication
@@ -1839,15 +1839,13 @@ class InventoryDetail(ControlledByScmMixin, RetrieveUpdateDestroyAPIView):
 
     def destroy(self, request, *args, **kwargs):
         obj = self.get_object()
-        if obj.pending_deletion is True:
-            return Response(dict(error=_("Inventory is already being deleted.")), status=status.HTTP_400_BAD_REQUEST)
         if not request.user.can_access(self.model, 'delete', obj):
             raise PermissionDenied()
-        obj.websocket_emit_status('pending_deletion')
+        try:
-        delete_inventory.delay(obj.id)
+            obj.schedule_deletion()
-        obj.pending_deletion = True
+            return Response(status=status.HTTP_202_ACCEPTED)
-        obj.save(update_fields=['pending_deletion'])
+        except RuntimeError, e:
-        return Response(status=status.HTTP_202_ACCEPTED)
+            return Response(dict(error=_("{0}".format(e))), status=status.HTTP_400_BAD_REQUEST)
 
 
 class InventoryActivityStreamList(ActivityStreamEnforcementMixin, SubListAPIView):

awx/main/migrations/0038_v320_release.py

@@ -76,6 +76,11 @@ class Migration(migrations.Migration):
             name='pending_deletion',
             field=models.BooleanField(default=False, help_text='Flag indicating the inventory is being deleted.', editable=False),
         ),
+        migrations.AlterField(
+            model_name='inventory',
+            name='organization',
+            field=models.ForeignKey(related_name='inventories', on_delete=models.deletion.SET_NULL, to='main.Organization', help_text='Organization containing this inventory.', null=True),
+        ),
 
         # Facts
         migrations.AlterField(

awx/main/models/inventory.py

@@ -63,7 +63,8 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin):
         'Organization',
         related_name='inventories',
         help_text=_('Organization containing this inventory.'),
-        on_delete=models.CASCADE,
+        on_delete=models.SET_NULL,
+        null=True,
     )
     variables = models.TextField(
         blank=True,
@@ -373,6 +374,16 @@ class Inventory(CommonModelNameNotUnique, ResourceMixin):
             raise ValidationError(_("Credential kind must be 'insights'."))
         return self.insights_credential
 
+    @transaction.atomic
+    def schedule_deletion(self):
+        from awx.main.tasks import delete_inventory
+        if self.pending_deletion is True:
+            raise RuntimeError("Inventory is already pending deletion.")
+        self.websocket_emit_status('pending_deletion')
+        delete_inventory.delay(self.pk)
+        self.pending_deletion = True
+        self.save(update_fields=['pending_deletion'])
+
 
 class SmartInventoryMembership(BaseModel):
     '''

awx/main/signals.py

@@ -510,3 +510,13 @@ def get_current_user_from_drf_request(sender, **kwargs):
     request = get_current_request()
     drf_request = getattr(request, 'drf_request', None)
     return (getattr(drf_request, 'user', False), 0)
+
+
+@receiver(pre_delete, sender=Organization)
+def delete_inventory_for_org(sender, instance, **kwargs):
+    inventories = Inventory.objects.filter(organization__pk=instance.pk)
+    for inventory in inventories:
+        try:
+            inventory.schedule_deletion()
+        except RuntimeError, e:
+            logger.debug(e)

awx/main/tasks.py

@@ -360,18 +360,17 @@ def update_host_smart_inventory_memberships():
 def delete_inventory(inventory_id):
     with ignore_inventory_computed_fields(), \
             ignore_inventory_group_removal():
-        with transaction.atomic():
+        try:
-            try:
+            i = Inventory.objects.get(id=inventory_id)
-                i = Inventory.objects.get(id=inventory_id)
-            except Inventory.DoesNotExist:
-                logger.error("Delete Inventory failed due to missing inventory: " + str(inventory_id))
-                return
             i.delete()
-    emit_channel_notification(
+            emit_channel_notification(
-        'inventories-status_changed',
+                'inventories-status_changed',
-        {'group_name': 'inventories', 'inventory_id': inventory_id, 'status': 'deleted'}
+                {'group_name': 'inventories', 'inventory_id': inventory_id, 'status': 'deleted'}
-    )
+            )
-    logger.debug('Deleted inventory: %s' % inventory_id)
+            logger.debug('Deleted inventory: %s' % inventory_id)
+        except Inventory.DoesNotExist:
+            logger.error("Delete Inventory failed due to missing inventory: " + str(inventory_id))
+            return
 
 
 class BaseTask(Task):

awx/main/tests/functional/api/test_inventory.py

@@ -59,7 +59,7 @@ def test_async_inventory_duplicate_deletion_prevention(delete, get, inventory, a
 
     resp = delete(reverse('api:inventory_detail', kwargs={'pk': inventory.id}), alice)
     assert resp.status_code == 400
-    assert resp.data['error'] == 'Inventory is already being deleted.'
+    assert resp.data['error'] == 'Inventory is already pending deletion.'
 
 
 @pytest.mark.parametrize('order_by', ('script', '-script', 'script,pk', '-script,pk'))
@@ -314,12 +314,12 @@ class TestControlledBySCM:
 @pytest.mark.django_db
 class TestInsightsCredential:
     def test_insights_credential(self, patch, insights_inventory, admin_user, insights_credential):
-        patch(insights_inventory.get_absolute_url(), 
+        patch(insights_inventory.get_absolute_url(),
               {'insights_credential': insights_credential.id}, admin_user,
               expect=200)
 
     def test_non_insights_credential(self, patch, insights_inventory, admin_user, scm_credential):
-        patch(insights_inventory.get_absolute_url(), 
+        patch(insights_inventory.get_absolute_url(),
               {'insights_credential': scm_credential.id}, admin_user,
               expect=400)