From e2866609bbb4e70e2b781a80ecc1ad0bccf93813 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 24 Apr 2019 13:02:01 +0200 Subject: [PATCH] pull out put_allocation call from _heal_* Both allocation healing steps calls the placement API. This patch pulls out the placement updating code to a single place. To do that it change the healing steps to only generate / update the allocation individually and then at the end of the healing there will be a single placement update with this allocation. This will help us to include the port related allocation into the instance allocation by modifying a single place in the code. Related-Bug: #1819923 Change-Id: I0e9f9a488141da599c10af8cabb4f6a5d111104f --- nova/cmd/manage.py | 90 +++++++++++------------ nova/tests/functional/test_nova_manage.py | 12 +-- nova/tests/unit/test_nova_manage.py | 2 +- 3 files changed, 49 insertions(+), 55 deletions(-) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index d615b56d9b0e..d5ad33af59bf 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -1658,9 +1658,7 @@ class PlacementCommands(object): node_cache[instance.node] = node_uuid return node_uuid - def _heal_missing_alloc( - self, ctxt, instance, node_cache, dry_run, output, placement): - + def _heal_missing_alloc(self, ctxt, instance, node_cache): node_uuid = self._get_compute_node_uuid( ctxt, instance, node_cache) @@ -1668,49 +1666,21 @@ class PlacementCommands(object): # on its embedded flavor. resources = scheduler_utils.resources_from_flavor( instance, instance.flavor) - 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: - payload = { - 'allocations': { - node_uuid: {'resources': resources}, - }, - 'project_id': instance.project_id, - 'user_id': instance.user_id, - 'consumer_generation': None - } - resp = placement.put_allocations(ctxt, instance.uuid, payload) - if resp: - 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_missing_project_and_user_id( - self, ctxt, allocations, instance, dry_run, output, placement): + payload = { + 'allocations': { + node_uuid: {'resources': resources}, + }, + 'project_id': instance.project_id, + 'user_id': instance.user_id, + 'consumer_generation': None + } + return payload + def _heal_missing_project_and_user_id(self, allocations, instance): allocations['project_id'] = instance.project_id allocations['user_id'] = instance.user_id - if dry_run: - output(_('[dry-run] Update allocations for instance ' - '%(instance)s: %(allocations)s') % - {'instance': instance.uuid, 'allocations': allocations}) - else: - resp = placement.put_allocations(ctxt, instance.uuid, allocations) - if resp: - output(_('Successfully updated allocations for ' - 'instance %s.') % instance.uuid) - return True - else: - raise exception.AllocationUpdateFailed( - consumer_uuid=instance.uuid, error=resp.text) + return allocations def _heal_allocations_for_instance(self, ctxt, instance, node_cache, output, placement, dry_run): @@ -1756,14 +1726,15 @@ class PlacementCommands(object): output(_("Allocation retrieval failed: %s") % e) allocations = None + need_healing = False # get_allocations_for_consumer uses safe_connect which will # return None if we can't communicate with Placement, and the # response can have an empty {'allocations': {}} response if # there are no allocations for the instance so handle both if not allocations or not allocations.get('allocations'): # This instance doesn't have allocations - return self._heal_missing_alloc( - ctxt, instance, node_cache, dry_run, output, placement) + need_healing = 'Create' + allocations = self._heal_missing_alloc(ctxt, instance, node_cache) if (allocations.get('project_id') != instance.project_id or allocations.get('user_id') != instance.user_id): @@ -1772,12 +1743,33 @@ class PlacementCommands(object): # and re-put them. We don't use put_allocations here # because we don't want to mess up shared or nested # provider allocations. - return self._heal_missing_project_and_user_id( - ctxt, allocations, instance, dry_run, output, placement) + need_healing = 'Update' + allocations = self._heal_missing_project_and_user_id( + allocations, instance) - output(_('Instance %s already has allocations with ' - 'matching consumer project/user.') % instance.uuid) - return + if need_healing: + if dry_run: + output(_('[dry-run] %(operation)s allocations for instance ' + '%(instance)s: %(allocations)s') % + {'operation': need_healing, + 'instance': instance.uuid, + 'allocations': allocations}) + else: + resp = placement.put_allocations(ctxt, instance.uuid, + allocations) + if resp: + output(_('Successfully %(operation)sd allocations for ' + 'instance %(instance)s.') % + {'operation': need_healing.lower(), + 'instance': instance.uuid}) + return True + else: + raise exception.AllocationUpdateFailed( + consumer_uuid=instance.uuid, error='') + else: + output(_('Instance %s already has allocations with ' + 'matching consumer project/user.') % instance.uuid) + return def _heal_instances_in_cell(self, ctxt, max_count, unlimited, output, placement, dry_run, instance_uuid): diff --git a/nova/tests/functional/test_nova_manage.py b/nova/tests/functional/test_nova_manage.py index 28fa78baab21..dcfd3a92dde1 100644 --- a/nova/tests/functional/test_nova_manage.py +++ b/nova/tests/functional/test_nova_manage.py @@ -652,13 +652,14 @@ class TestNovaManagePlacementHealAllocations( 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) + 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()) output = self.output.getvalue() - self.assertIn('Successfully updated allocations for instance', output) + self.assertIn( + 'Successfully updated allocations for', output) self.assertIn('Processed 1 instances.', output) # Now assert that the consumer was actually updated. allocations = self.placement_api.get( @@ -676,8 +677,9 @@ class TestNovaManagePlacementHealAllocations( 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) + self.assertIn('[dry-run] Create allocations for instance ' + '%s' % server['id'], output) + self.assertIn(rp_uuid, output) def test_heal_allocations_specific_instance(self): """Tests the case that a specific instance is processed and only that diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index fe5524fcf155..f5a0c431acc2 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -2478,7 +2478,7 @@ class TestNovaManagePlacement(test.NoDBTestCase): mock_get_compute_node, mock_get_allocs, mock_get_instances, mock_get_all_cells): self.assertEqual(3, self.cli.heal_allocations()) - self.assertIn('Failed to create allocations for instance', + self.assertIn('Failed to update allocations for consumer', self.output.getvalue()) instance = mock_get_instances.return_value[0] mock_res_from_flavor.assert_called_once_with(