Add --dry-run option to heal_allocations CLI

This resolves one of the TODOs in the heal_allocations CLI
by adding a --dry-run option which will still print the output
as we process instances but not commit any allocation changes
to placement, just print out that they would happen.

Change-Id: Ide31957306602c1f306ebfa48d6e95f48b1e8ead

doc/source/cli/nova-manage.rst

@@ -330,7 +330,7 @@ Nova Cells v2
 Placement
 ~~~~~~~~~
 
-``nova-manage placement heal_allocations [--max-count <max_count>] [--verbose]``
+``nova-manage placement heal_allocations [--max-count <max_count>] [--verbose] [--dry-run]``
     Iterates over non-cell0 cells looking for instances which do not have
     allocations in the Placement service and which are not undergoing a task
     state transition. For each instance found, allocations are created against
@@ -349,6 +349,9 @@ Placement
 
     Specify ``--verbose`` to get detailed progress output during execution.
 
+    Specify ``--dry-run`` to print output but not commit any changes. The
+    return code should be 4. *(Since 20.0.0 Train)*
+
     This command requires that the ``[api_database]/connection`` and
     ``[placement]`` configuration options are set. Placement API >= 1.28 is
     required.

nova/cmd/manage.py

@@ -1818,7 +1818,7 @@ class PlacementCommands(object):
         return node_uuid
 
     def _heal_allocations_for_instance(self, ctxt, instance, node_cache,
-                                       output, placement):
+                                       output, placement, dry_run):
         """Checks the given instance to see if it needs allocation healing
 
         :param ctxt: cell-targeted nova.context.RequestContext
@@ -1828,6 +1828,8 @@ class PlacementCommands(object):
         :param outout: function that takes a single message for verbose output
         :param placement: nova.scheduler.client.report.SchedulerReportClient
             to communicate with the Placement service API.
+        :param dry_run: Process instances and print output but do not commit
+            any changes.
         :return: True if allocations were created or updated for the instance,
             None if nothing needed to be done
         :raises: nova.exception.ComputeHostNotFound if a compute node for a
@@ -1887,16 +1889,21 @@ class PlacementCommands(object):
             # We use CONSUMER_GENERATION_VERSION for PUT
             # /allocations/{consumer_id} to mirror the body structure from
             # get_allocs_for_consumer.
-            resp = placement.put(
+            if dry_run:
-                '/allocations/%s' % instance.uuid,
+                output(_('[dry-run] Update allocations for instance '
-                allocations, version=report.CONSUMER_GENERATION_VERSION)
+                         '%(instance)s: %(allocations)s') %
-            if resp:
+                       {'instance': instance.uuid, 'allocations': allocations})
-                output(_('Successfully updated allocations for '
-                         'instance %s.') % instance.uuid)
-                return True
             else:
-                raise exception.AllocationUpdateFailed(
+                resp = placement.put(
-                    consumer_uuid=instance.uuid, error=resp.text)
+                    '/allocations/%s' % instance.uuid,
+                    allocations, version=report.CONSUMER_GENERATION_VERSION)
+                if resp:
+                    output(_('Successfully updated allocations for '
+                             'instance %s.') % instance.uuid)
+                    return True
+                else:
+                    raise exception.AllocationUpdateFailed(
+                        consumer_uuid=instance.uuid, error=resp.text)
 
         # This instance doesn't have allocations so we need to find
         # its compute node resource provider.
@@ -1907,21 +1914,27 @@ class PlacementCommands(object):
         # on its embedded flavor.
         resources = scheduler_utils.resources_from_flavor(
             instance, instance.flavor)
-        if placement.put_allocations(
+        if dry_run:
-                ctxt, node_uuid, instance.uuid, resources,
+            output(_('[dry-run] Create allocations for instance %(instance)s '
-                instance.project_id, instance.user_id,
+                     'on provider %(node_uuid)s: %(resources)s') %
-                consumer_generation=None):
+                   {'instance': instance.uuid, 'node_uuid': node_uuid,
-            output(_('Successfully created allocations for '
+                    'resources': resources})
-                     'instance %(instance)s against resource '
-                     'provider %(provider)s.') %
-                   {'instance': instance.uuid, 'provider': node_uuid})
-            return True
         else:
-            raise exception.AllocationCreateFailed(
+            if placement.put_allocations(
-                instance=instance.uuid, provider=node_uuid)
+                    ctxt, node_uuid, instance.uuid, resources,
+                    instance.project_id, instance.user_id,
+                    consumer_generation=None):
+                output(_('Successfully created allocations for '
+                         'instance %(instance)s against resource '
+                         'provider %(provider)s.') %
+                       {'instance': instance.uuid, 'provider': node_uuid})
+                return True
+            else:
+                raise exception.AllocationCreateFailed(
+                    instance=instance.uuid, provider=node_uuid)
 
     def _heal_instances_in_cell(self, ctxt, max_count, unlimited, output,
-                                placement):
+                                placement, dry_run):
         """Checks for instances to heal in a given cell.
 
         :param ctxt: cell-targeted nova.context.RequestContext
@@ -1931,6 +1944,8 @@ class PlacementCommands(object):
         :param outout: function that takes a single message for verbose output
         :param placement: nova.scheduler.client.report.SchedulerReportClient
             to communicate with the Placement service API.
+        :param dry_run: Process instances and print output but do not commit
+            any changes.
         :return: Number of instances that had allocations created.
         :raises: nova.exception.ComputeHostNotFound if a compute node for a
             given instance cannot be found
@@ -1966,7 +1981,8 @@ class PlacementCommands(object):
             # continue.
             for instance in instances:
                 if self._heal_allocations_for_instance(
-                        ctxt, instance, node_cache, output, placement):
+                        ctxt, instance, node_cache, output, placement,
+                        dry_run):
                     num_processed += 1
 
             # Make sure we don't go over the max count. Note that we
@@ -2004,7 +2020,10 @@ class PlacementCommands(object):
                '0 or 4.')
     @args('--verbose', action='store_true', dest='verbose', default=False,
           help='Provide verbose output during execution.')
-    def heal_allocations(self, max_count=None, verbose=False):
+    @args('--dry-run', action='store_true', dest='dry_run', default=False,
+          help='Runs the command and prints output but does not commit any '
+               'changes. The return code should be 4.')
+    def heal_allocations(self, max_count=None, verbose=False, dry_run=False):
         """Heals instance allocations in the Placement service
 
         Return codes:
@@ -2018,8 +2037,6 @@ class PlacementCommands(object):
         * 127: Invalid input.
         """
         # NOTE(mriedem): Thoughts on ways to expand this:
-        # - add a --dry-run option to just print which instances would have
-        #   allocations created for them
         # - allow passing a specific cell to heal
         # - allow filtering on enabled/disabled cells
         # - allow passing a specific instance to heal
@@ -2081,7 +2098,8 @@ class PlacementCommands(object):
             with context.target_cell(ctxt, cell) as cctxt:
                 try:
                     num_processed += self._heal_instances_in_cell(
-                        cctxt, limit_per_cell, unlimited, output, placement)
+                        cctxt, limit_per_cell, unlimited, output, placement,
+                        dry_run)
                 except exception.ComputeHostNotFound as e:
                     print(e.format_message())
                     return 2

nova/tests/functional/test_nova_manage.py

@@ -633,6 +633,14 @@ class TestNovaManagePlacementHealAllocations(
         allocations = allocations['allocations']
         self.assertIn(rp_uuid, allocations)
         self.assertFlavorMatchesAllocation(self.flavor, server['id'], rp_uuid)
+        # First do a dry run.
+        result = self.cli.heal_allocations(verbose=True, dry_run=True)
+        # Nothing changed so the return code should be 4.
+        self.assertEqual(4, result, self.output.getvalue())
+        output = self.output.getvalue()
+        self.assertIn('Processed 0 instances.', output)
+        self.assertIn('[dry-run] Update allocations for instance %s' %
+                      server['id'], output)
         # Now run heal_allocations which should update the consumer info.
         result = self.cli.heal_allocations(verbose=True)
         self.assertEqual(0, result, self.output.getvalue())
@@ -645,6 +653,19 @@ class TestNovaManagePlacementHealAllocations(
         self.assertEqual(server['tenant_id'], allocations['project_id'])
         self.assertEqual(server['user_id'], allocations['user_id'])
 
+    def test_heal_allocations_dry_run(self):
+        """Tests to make sure the --dry-run option does not commit changes."""
+        # Create a server with no allocations.
+        server, rp_uuid = self._boot_and_assert_no_allocations(
+            self.flavor, 'cell1')
+        result = self.cli.heal_allocations(verbose=True, dry_run=True)
+        # Nothing changed so the return code should be 4.
+        self.assertEqual(4, result, self.output.getvalue())
+        output = self.output.getvalue()
+        self.assertIn('Processed 0 instances.', output)
+        self.assertIn('[dry-run] Create allocations for instance %s on '
+                      'provider %s' % (server['id'], rp_uuid), output)
+
 
 class TestNovaManagePlacementSyncAggregates(
         integrated_helpers.ProviderUsageBaseTestCase):

releasenotes/notes/heal-allocations-dry-run-1761fab00f7967d1.yaml

@@ -0,0 +1,6 @@
+---
+other:
+  - |
+    A ``--dry-run`` option has been added to the
+    ``nova-manage placement heal_allocations`` CLI which allows running the
+    command to get output without committing any changes to placement.