FUP for Add a placement audit command

Since I537ed74503d208957f0a97af3ab754a6750dac20 had some clean-up comments,
we can just provide a follow-up change.

Change-Id: Ie8b5147322e13ad7df966b5c3c41ef0418e4f64c
Related-Bug: #1793569

doc/source/cli/nova-manage.rst

@@ -685,7 +685,6 @@ Placement
        * - 255
          - An unexpected error occurred.
 
-
 ``nova-manage placement audit [--verbose] [--delete] [--resource_provider <uuid>]``
     Iterates over all the Resource Providers (or just one if you provide the
     UUID) and then verifies if the compute allocations are either related to

nova/cmd/manage.py

@@ -2454,8 +2454,8 @@ class PlacementCommands(object):
         allocations = placement.get_allocs_for_consumer(
             ctxt, consumer_uuid)
         if len(allocations['allocations']) > 1:
-            # This consumer has resources spreaded amongst
+            # This consumer has resources spread among multiple RPs (think
-            # multiple RPs (think nested or shared for example)
+            # nested or shared for example)
             # We then need to just update the usage to remove
             # the orphaned resources on the specific RP
             del allocations['allocations'][provider['uuid']]
@@ -2669,7 +2669,7 @@ class PlacementCommands(object):
             resource_providers = self._get_resource_providers(ctxt, placement)
 
         for provider in resource_providers:
-            (nb_p, faults) = self._check_orphaned_allocations_for_provider(
+            nb_p, faults = self._check_orphaned_allocations_for_provider(
                 ctxt, placement, output, provider, delete)
             num_processed += nb_p
             if faults > 0:

nova/tests/functional/test_nova_manage.py

@@ -1417,16 +1417,6 @@ class TestNovaManagePlacementAudit(
 
         self.flavor = self.api.get_flavors()[0]
 
-    def _delete_instance_but_keep_its_allocations(self, server):
-        """Mocks out the call to Placement for deleting the allocations but
-           still performs the instance deletion.
-        """
-
-        with mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                        'delete_allocation_for_instance'):
-            self.api.delete_server(server['id'])
-            self._wait_until_deleted(server)
-
     def test_audit_orphaned_allocation_from_instance_delete(self):
         """Creates a server and deletes it by retaining its allocations so the
            audit command can find it.
@@ -1458,15 +1448,17 @@ class TestNovaManagePlacementAudit(
              'rp_uuid': rp_uuid},
             output)
         self.assertIn('Processed 1 allocation.', output)
+        # Here we don't want to delete the found allocations
+        self.assertNotIn(
+            'Deleted allocations for consumer UUID %s' % server['id'], output)
         self.assertEqual(3, ret)
 
         # Now ask the audit command to delete the rogue allocations.
         ret = self.cli.audit(delete=True, verbose=True)
 
         # The allocations are now deleted
-        self.assertRequestMatchesUsage({'VCPU': 0,
+        self.assertRequestMatchesUsage(
-                                        'MEMORY_MB': 0,
+            {'VCPU': 0, 'MEMORY_MB': 0, 'DISK_GB': 0}, rp_uuid)
-                                        'DISK_GB': 0}, rp_uuid)
 
         output = self.output.getvalue()
         self.assertIn(
@@ -1525,8 +1517,7 @@ class TestNovaManagePlacementAudit(
         self.assertIn(
             'Allocations for consumer UUID %(consumer_uuid)s on '
             'Resource Provider %(rp_uuid)s can be deleted' %
-            {'consumer_uuid': migration_uuid,
+            {'consumer_uuid': migration_uuid, 'rp_uuid': source_rp_uuid},
-             'rp_uuid': source_rp_uuid},
             output)
         self.assertIn('Processed 1 allocation.', output)
         self.assertEqual(3, ret)
@@ -1550,7 +1541,7 @@ class TestNovaManagePlacementAudit(
         self.assertIn('Processed 1 allocation.', output)
         self.assertEqual(4, ret)
 
-    # TODO(sbauza): Mock this test once bug #1829479 is fixed
+    # TODO(sbauza): Remove this test once bug #1829479 is fixed
     def test_audit_orphaned_allocations_from_deleted_compute_evacuate(self):
         """Evacuate a server and the delete the source node so that it will
            leave a source allocation that the audit command will find.

releasenotes/notes/placement-audit-59a00dcfb188c6ac.yaml

@@ -1,12 +1,11 @@
 ---
 other:
   - |
-    A new ``nova-manage placement audit`` CLI has been added to help identify
+    A new ``nova-manage`` command, ``placement audit``, has been added.
-    orphaned compute allocations in the Placement API that are no longer
+    This can be used to identify and optionally remove compute allocations in
-    related to either instances or migrations.
+    placement that are no longer referenced by existing instances or
-    Some race conditions in Nova could not remove allocations for some
+    migrations. These orphaned allocations typically occur due to race
-    instances or migrations when they're done and then it would create some
+    conditions during instance migration or removal and will result in capacity
-    capacity issues. Thanks to the command, you could know the orphaned
+    issues if not addressed.
-    allocations and ask to remove them.
     For more details on CLI usage, see the man page entry:
     https://docs.openstack.org/nova/latest/cli/nova-manage.html#placement