delete consumers which no longer have allocations

We made the decision [1] to delete consumer records when those consumers
no longer had any allocations referring to them (as opposed to keeping
those consumer records around and incrementing the consumer generation
for them).

This patch adds a small check within the larger
AllocationList.create_all() and AllocationList.delete_all() DB
transactions that deletes consumer records when no allocation records
remain that reference that consumer. This patch does not, however,
attempt to clean up any "orphaned" consumer records that may have been
created in previous calls to PUT|POST /allocations that removed the last
remaining allocations for a consumer.

[1] https://goo.gl/DpAGbW

Change-Id: Ic2b82146d28be64b363b0b8e2e8d180b515bc0a0
Closes-bug: #1780799

nova/api/openstack/placement/objects/consumer.py

@@ -62,6 +62,29 @@ def create_incomplete_consumers(ctx, batch_size):
     return res.rowcount, res.rowcount
 
 
+@db_api.placement_context_manager.writer
+def delete_consumers_if_no_allocations(ctx, consumer_uuids):
+    """Looks to see if any of the supplied consumers has any allocations and if
+    not, deletes the consumer record entirely.
+
+    :param ctx: `nova.api.openstack.placement.context.RequestContext` that
+                contains an oslo_db Session
+    :param consumer_uuids: UUIDs of the consumers to check and maybe delete
+    """
+    # Delete consumers that are not referenced in the allocations table
+    cons_to_allocs_join = sa.outerjoin(
+        CONSUMER_TBL, _ALLOC_TBL,
+        CONSUMER_TBL.c.uuid == _ALLOC_TBL.c.consumer_id)
+    subq = sa.select([CONSUMER_TBL.c.uuid]).select_from(cons_to_allocs_join)
+    subq = subq.where(sa.and_(
+        _ALLOC_TBL.c.consumer_id.is_(None),
+        CONSUMER_TBL.c.uuid.in_(consumer_uuids)))
+    no_alloc_consumers = [r[0] for r in ctx.session.execute(subq).fetchall()]
+    del_stmt = CONSUMER_TBL.delete()
+    del_stmt = del_stmt.where(CONSUMER_TBL.c.uuid.in_(no_alloc_consumers))
+    ctx.session.execute(del_stmt)
+
+
 @db_api.placement_context_manager.reader
 def _get_consumer_by_uuid(ctx, uuid):
     # The SQL for this looks like the following:

nova/api/openstack/placement/objects/resource_provider.py

@@ -1981,6 +1981,15 @@ class AllocationList(base.ObjectListBase, base.VersionedObject):
             rp.generation = _increment_provider_generation(context, rp)
         for consumer in visited_consumers.values():
             consumer.increment_generation()
+        # If any consumers involved in this transaction ended up having no
+        # allocations, delete the consumer records. Exclude consumers that had
+        # *some resource* in the allocation list with a total > 0 since clearly
+        # those consumers have allocations...
+        cons_with_allocs = set(a.consumer.uuid for a in allocs if a.used > 0)
+        all_cons = set(c.uuid for c in visited_consumers.values())
+        consumers_to_check = all_cons - cons_with_allocs
+        consumer_obj.delete_consumers_if_no_allocations(
+            context, consumers_to_check)
 
     @classmethod
     def get_all_by_resource_provider(cls, context, rp):
@@ -2076,6 +2085,8 @@ class AllocationList(base.ObjectListBase, base.VersionedObject):
         # that fact and do an efficient batch delete
         consumer_uuid = self.objects[0].consumer.uuid
         _delete_allocations_for_consumer(self._context, consumer_uuid)
+        consumer_obj.delete_consumers_if_no_allocations(
+            self._context, [consumer_uuid])
 
     def __repr__(self):
         strings = [repr(x) for x in self.objects]

nova/tests/functional/api/openstack/placement/db/test_consumer.py

@@ -20,6 +20,7 @@ from nova.api.openstack.placement.objects import project as project_obj
 from nova.api.openstack.placement.objects import resource_provider as rp_obj
 from nova.api.openstack.placement.objects import user as user_obj
 from nova import context
+from nova import rc_fields as fields
 from nova import test
 from nova.tests import fixtures
 from nova.tests.functional.api.openstack.placement.db import test_base as tb
@@ -190,3 +191,91 @@ class CreateIncompleteConsumersTestCase(test.NoDBTestCase):
         self._check_incomplete_consumers(self.ctx)
         res = consumer_obj.create_incomplete_consumers(self.ctx, 10)
         self.assertEqual((0, 0), res)
+
+
+class DeleteConsumerIfNoAllocsTestCase(tb.PlacementDbBaseTestCase):
+    def test_delete_consumer_if_no_allocs(self):
+        """AllocationList.create_all() should attempt to delete consumers that
+        no longer have any allocations. Due to the REST API not having any way
+        to query for consumers directly (only via the GET
+        /allocations/{consumer_uuid} endpoint which returns an empty dict even
+        when no consumer record exists for the {consumer_uuid}) we need to do
+        this functional test using only the object layer.
+        """
+        # We will use two consumers in this test, only one of which will get
+        # all of its allocations deleted in a transaction (and we expect that
+        # consumer record to be deleted)
+        c1 = consumer_obj.Consumer(
+            self.ctx, uuid=uuids.consumer1, user=self.user_obj,
+            project=self.project_obj)
+        c1.create()
+        c2 = consumer_obj.Consumer(
+            self.ctx, uuid=uuids.consumer2, user=self.user_obj,
+            project=self.project_obj)
+        c2.create()
+
+        # Create some inventory that we will allocate
+        cn1 = self._create_provider('cn1')
+        tb.add_inventory(cn1, fields.ResourceClass.VCPU, 8)
+        tb.add_inventory(cn1, fields.ResourceClass.MEMORY_MB, 2048)
+        tb.add_inventory(cn1, fields.ResourceClass.DISK_GB, 2000)
+
+        # Now allocate some of that inventory to two different consumers
+        allocs = [
+            rp_obj.Allocation(
+                self.ctx, consumer=c1, resource_provider=cn1,
+                resource_class=fields.ResourceClass.VCPU, used=1),
+            rp_obj.Allocation(
+                self.ctx, consumer=c1, resource_provider=cn1,
+                resource_class=fields.ResourceClass.MEMORY_MB, used=512),
+            rp_obj.Allocation(
+                self.ctx, consumer=c2, resource_provider=cn1,
+                resource_class=fields.ResourceClass.VCPU, used=1),
+            rp_obj.Allocation(
+                self.ctx, consumer=c2, resource_provider=cn1,
+                resource_class=fields.ResourceClass.MEMORY_MB, used=512),
+        ]
+        alloc_list = rp_obj.AllocationList(self.ctx, objects=allocs)
+        alloc_list.create_all()
+
+        # Validate that we have consumer records for both consumers
+        for c_uuid in (uuids.consumer1, uuids.consumer2):
+            c_obj = consumer_obj.Consumer.get_by_uuid(self.ctx, c_uuid)
+            self.assertIsNotNone(c_obj)
+
+        # OK, now "remove" the allocation for consumer2 by setting the used
+        # value for both allocated resources to 0 and re-running the
+        # AllocationList.create_all(). This should end up deleting the consumer
+        # record for consumer2
+        allocs = [
+            rp_obj.Allocation(
+                self.ctx, consumer=c2, resource_provider=cn1,
+                resource_class=fields.ResourceClass.VCPU, used=0),
+            rp_obj.Allocation(
+                self.ctx, consumer=c2, resource_provider=cn1,
+                resource_class=fields.ResourceClass.MEMORY_MB, used=0),
+        ]
+        alloc_list = rp_obj.AllocationList(self.ctx, objects=allocs)
+        alloc_list.create_all()
+
+        # consumer1 should still exist...
+        c_obj = consumer_obj.Consumer.get_by_uuid(self.ctx, uuids.consumer1)
+        self.assertIsNotNone(c_obj)
+
+        # but not consumer2...
+        self.assertRaises(
+            exception.NotFound, consumer_obj.Consumer.get_by_uuid,
+            self.ctx, uuids.consumer2)
+
+        # DELETE /allocations/{consumer_uuid} is the other place where we
+        # delete all allocations for a consumer. Let's delete all for consumer1
+        # and check that the consumer record is deleted
+        alloc_list = rp_obj.AllocationList.get_all_by_consumer_id(
+            self.ctx, uuids.consumer1)
+        alloc_list.delete_all()
+
+        # consumer1 should no longer exist in the DB since we just deleted all
+        # of its allocations
+        self.assertRaises(
+            exception.NotFound, consumer_obj.Consumer.get_by_uuid,
+            self.ctx, uuids.consumer1)