From 07de847d7bdae22dd2a4a9e2c02113d4803d0883 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 11 Apr 2019 18:15:09 -0400 Subject: [PATCH] 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. Conflicts: nova/cmd/manage.py nova/tests/functional/test_nova_manage.py NOTE(mriedem): The manage.py conflicts are due to not having change I0e9a804ae7717252175f7fe409223f5eb8f50013 or change Iba230201803ef3d33bccaaf83eb10453eea43f20 in Rocky. The test_nova_manage conflict is due to not having change If6aa37d9b6b48791e070799ab026c816fda4441c in Rocky. Change-Id: Ide31957306602c1f306ebfa48d6e95f48b1e8ead (cherry picked from commit ded3e4d9007b3806665bc1b7bec2705bcbe2977e) (cherry picked from commit 4acec3e715b72f2f191ea544f135e40dc8fdaa81) --- doc/source/cli/nova-manage.rst | 5 +- nova/cmd/manage.py | 72 ++++++++++++------- nova/tests/functional/test_nova_manage.py | 21 ++++++ ...-allocations-dry-run-1761fab00f7967d1.yaml | 6 ++ 4 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 releasenotes/notes/heal-allocations-dry-run-1761fab00f7967d1.yaml diff --git a/doc/source/cli/nova-manage.rst b/doc/source/cli/nova-manage.rst index ac61c97a5ec5..abd9b27af29d 100644 --- a/doc/source/cli/nova-manage.rst +++ b/doc/source/cli/nova-manage.rst @@ -299,7 +299,7 @@ Nova Cells v2 Placement ~~~~~~~~~ -``nova-manage placement heal_allocations [--max-count ] [--verbose]`` +``nova-manage placement heal_allocations [--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 @@ -318,6 +318,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. + This command requires that the ``[api_database]/connection`` and ``[placement]`` configuration options are set. Placement API >= 1.28 is required. diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index fd402a175ef1..806aeccdadd1 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -1809,7 +1809,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 @@ -1819,6 +1819,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 @@ -1866,17 +1868,22 @@ class PlacementCommands(object): # provider allocations. allocations['project_id'] = instance.project_id allocations['user_id'] = instance.user_id - # We use 1.28 for PUT /allocations/{consumer_id} to mirror - # the body structure from get_allocations_for_consumer. - resp = placement.put('/allocations/%s' % instance.uuid, - allocations, version='1.28') - if resp: - output(_('Successfully updated allocations for ' - 'instance %s.') % instance.uuid) - return True + if dry_run: + output(_('[dry-run] Update allocations for instance ' + '%(instance)s: %(allocations)s') % + {'instance': instance.uuid, 'allocations': allocations}) else: - raise exception.AllocationUpdateFailed( - instance=instance.uuid, error=resp.text) + # We use 1.28 for PUT /allocations/{consumer_id} to mirror + # the body structure from get_allocations_for_consumer. + resp = placement.put('/allocations/%s' % instance.uuid, + allocations, version='1.28') + if resp: + output(_('Successfully updated allocations for ' + 'instance %s.') % instance.uuid) + return True + else: + raise exception.AllocationUpdateFailed( + instance=instance.uuid, error=resp.text) # This instance doesn't have allocations so we need to find # its compute node resource provider. @@ -1887,20 +1894,26 @@ class PlacementCommands(object): # on its embedded flavor. resources = scheduler_utils.resources_from_flavor( instance, instance.flavor) - if placement.put_allocations( - ctxt, node_uuid, instance.uuid, resources, - instance.project_id, instance.user_id): - output(_('Successfully created allocations for ' - 'instance %(instance)s against resource ' - 'provider %(provider)s.') % - {'instance': instance.uuid, 'provider': node_uuid}) - return True + if dry_run: + output(_('[dry-run] Create allocations for instance %(instance)s ' + 'on provider %(node_uuid)s: %(resources)s') % + {'instance': instance.uuid, 'node_uuid': node_uuid, + 'resources': resources}) else: - raise exception.AllocationCreateFailed( - instance=instance.uuid, provider=node_uuid) + if placement.put_allocations( + ctxt, node_uuid, instance.uuid, resources, + instance.project_id, instance.user_id): + 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 @@ -1910,6 +1923,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 @@ -1945,7 +1960,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 @@ -1982,7 +1998,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: @@ -1996,8 +2015,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 @@ -2059,7 +2076,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 diff --git a/nova/tests/functional/test_nova_manage.py b/nova/tests/functional/test_nova_manage.py index 4bc391a72c57..3b7a6c531f6c 100644 --- a/nova/tests/functional/test_nova_manage.py +++ b/nova/tests/functional/test_nova_manage.py @@ -641,6 +641,14 @@ class TestNovaManagePlacementHealAllocations( self.assertIn(rp_uuid, allocations) self.assertFlavorMatchesAllocation( self.flavor, allocations[rp_uuid]['resources']) + # 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()) @@ -653,6 +661,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): diff --git a/releasenotes/notes/heal-allocations-dry-run-1761fab00f7967d1.yaml b/releasenotes/notes/heal-allocations-dry-run-1761fab00f7967d1.yaml new file mode 100644 index 000000000000..37a904f7863a --- /dev/null +++ b/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.