Merge "Create BDMs/tags in cell with instance when over-quota"

nova/conductor/manager.py

@@ -1255,6 +1255,8 @@ class ComputeTaskManager(base.Base):
                     instance_uuids, return_alternates=True)
         except Exception as exc:
             LOG.exception('Failed to schedule instances')
+            # FIXME(mriedem): If the tags are not persisted with the instance
+            # in cell0 then the API will not show them.
             self._bury_in_cell0(context, request_specs[0], exc,
                                 build_requests=build_requests,
                                 block_device_mapping=block_device_mapping)
@@ -1281,6 +1283,8 @@ class ComputeTaskManager(base.Base):
                     LOG.error('No host-to-cell mapping found for selected '
                               'host %(host)s. Setup is incomplete.',
                               {'host': host.service_host})
+                    # FIXME(mriedem): If the tags are not persisted with the
+                    # instance in cell0 then the API will not show them.
                     self._bury_in_cell0(
                         context, request_spec, exc,
                         build_requests=[build_request], instances=[instance],
@@ -1334,6 +1338,7 @@ class ComputeTaskManager(base.Base):
                     self._cleanup_build_artifacts(context, exc, instances,
                                                   build_requests,
                                                   request_specs,
+                                                  block_device_mapping, tags,
                                                   cell_mapping_cache)
 
         zipped = six.moves.zip(build_requests, request_specs, host_lists,
@@ -1410,7 +1415,8 @@ class ComputeTaskManager(base.Base):
                     limits=host.limits, host_list=host_list)
 
     def _cleanup_build_artifacts(self, context, exc, instances, build_requests,
-                                 request_specs, cell_mapping_cache):
+                                 request_specs, block_device_mappings, tags,
+                                 cell_mapping_cache):
         for (instance, build_request, request_spec) in six.moves.zip(
                 instances, build_requests, request_specs):
             # Skip placeholders that were buried in cell0 or had their
@@ -1431,6 +1437,21 @@ class ComputeTaskManager(base.Base):
             inst_mapping.cell_mapping = cell
             inst_mapping.save()
 
+            # In order to properly clean-up volumes when deleting a server in
+            # ERROR status with no host, we need to store BDMs in the same
+            # cell.
+            if block_device_mappings:
+                self._create_block_device_mapping(
+                    cell, instance.flavor, instance.uuid,
+                    block_device_mappings)
+
+            # Like BDMs, the server tags provided by the user when creating the
+            # server should be persisted in the same cell so they can be shown
+            # from the API.
+            if tags:
+                with nova_context.target_cell(context, cell) as cctxt:
+                    self._create_tags(cctxt, instance.uuid, tags)
+
             # Be paranoid about artifacts being deleted underneath us.
             try:
                 build_request.destroy()

nova/tests/functional/regressions/test_bug_1806064.py

@@ -117,14 +117,16 @@ class BootFromVolumeOverQuotaRaceDeleteTest(
             # This would raise InstanceNotFound if the instance isn't in cell1.
             instance = objects.Instance.get_by_uuid(cctxt, server['id'])
             self.assertIsNone(instance.host, 'instance.host should not be set')
+            # Make sure the BDMs and tags also exist in cell1.
+            bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
+                cctxt, instance.uuid)
+            self.assertEqual(1, len(bdms), 'BDMs were not created in cell1')
+            tags = objects.TagList.get_by_resource_id(cctxt, instance.uuid)
+            self.assertEqual(1, len(tags), 'Tags were not created in cell1')
 
         # Make sure we can still view the tags on the server before it is
         # deleted.
-        # FIXME(mriedem): This is bug 1806064 where the tags are not created
-        # in cell1 along with the instance when the quota check race failure
-        # occurs. Uncomment once fixed.
-        # self.assertEqual(['bfv'], server['tags'])
-        self.assertEqual([], server['tags'])
+        self.assertEqual(['bfv'], server['tags'])
 
         # Now delete the server which, since it does not have a host, will be
         # deleted "locally" from the API.
@@ -132,12 +134,7 @@ class BootFromVolumeOverQuotaRaceDeleteTest(
         self._wait_until_deleted(server)
 
         # The volume should have been detached by the API.
-        # FIXME(mriedem): This is bug 1806064 where the volume is not detached
-        # because the related BDM record was not created in cell1 along with
-        # the instance so the API could not "see" it. Uncomment once fixed.
-        # self.assertIsNone(
-        #     self.cinder_fixture.volume_ids_for_instance(server['id']))
-        self.assertEqual(volume_id,
-                         # volume_ids_for_instance is a generator so listify
-                         list(self.cinder_fixture.volume_ids_for_instance(
-                             server['id']))[0])
+        attached_volumes = self.cinder_fixture.volume_ids_for_instance(
+            server['id'])
+        # volume_ids_for_instance is a generator so listify
+        self.assertEqual(0, len(list(attached_volumes)))