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

Author: Balazs Gibizer

nova/cmd/manage.py

@@ -1658,59 +1658,29 @@ def _get_compute_node_uuid(ctxt, instance, node_cache):
         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)
 
         # Now get the resource allocations for the instance based
         # 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 @@ def _heal_allocations_for_instance(self, ctxt, instance, node_cache,
             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 @@ def _heal_allocations_for_instance(self, ctxt, instance, node_cache,
             # 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)
-
-        output(_('Instance %s already has allocations with '
-                 'matching consumer project/user.') % instance.uuid)
-        return
+            need_healing = 'Update'
+            allocations = self._heal_missing_project_and_user_id(
+                allocations, instance)
+
+        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):

nova/tests/functional/test_nova_manage.py

@@ -652,13 +652,14 @@ def test_heal_allocations_update_sentinel_consumer(self):
         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 @@ def test_heal_allocations_dry_run(self):
         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

nova/tests/unit/test_nova_manage.py

@@ -2478,7 +2478,7 @@ def test_heal_allocations_put_allocations_fails(
             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(