Simplify interface for creating snapshot of volume-backed instance

Initialize new image metadata inside the creation method instead of
outside it. This reduce code duplication.

Also this fixes snapshot creation in EC2, since it had bugs in image
metadata initialization.

Closes-Bug: #1439820
Change-Id: If3efa2a274517a9ad51d5dfff0134ae9e8e702a4

nova/api/ec2/cloud.py

@@ -1799,27 +1799,12 @@ class CloudController(object):
                               'task_state': instance.task_state})
                     raise exception.InternalError(message=err)
 
-        glance_uuid = instance.image_ref
-        ec2_image_id = ec2utils.glance_id_to_ec2_id(context, glance_uuid)
-        src_image = self._get_image(context, ec2_image_id)
-        image_meta = dict(src_image)
-
-        def _unmap_id_property(properties, name):
-            if properties[name]:
-                properties[name] = ec2utils.id_to_glance_id(context,
-                                                            properties[name])
-
-        # ensure the ID properties are unmapped back to the glance UUID
-        _unmap_id_property(image_meta['properties'], 'kernel_id')
-        _unmap_id_property(image_meta['properties'], 'ramdisk_id')
-
         # meaningful image name
         name_map = dict(instance=instance_uuid, now=timeutils.isotime())
         name = name or _('image of %(instance)s at %(now)s') % name_map
 
         new_image = self.compute_api.snapshot_volume_backed(context,
                                                             instance,
-                                                            image_meta,
                                                             name)
 
         ec2_id = ec2utils.glance_id_to_ec2_id(context, new_image['id'])

nova/api/openstack/compute/legacy_v2/servers.py

@@ -1086,19 +1086,9 @@ class Controller(wsgi.Controller):
                         'compute:snapshot_volume_backed',
                         {'project_id': context.project_id,
                         'user_id': context.user_id})
-                img = instance.image_ref
-                if not img:
-                    properties = bdms.root_metadata(
-                            context, self.compute_api.image_api,
-                            self.compute_api.volume_api)
-                    image_meta = {'properties': properties}
-                else:
-                    image_meta = self.compute_api.image_api.get(context, img)
-
                 image = self.compute_api.snapshot_volume_backed(
                                                        context,
                                                        instance,
-                                                       image_meta,
                                                        image_name,
                                                        extra_properties=props)
             else:

nova/api/openstack/compute/servers.py

@@ -1035,19 +1035,9 @@ class ServersController(wsgi.Controller):
             if self.compute_api.is_volume_backed_instance(context, instance,
                                                           bdms):
                 authorize(context, action="create_image:allow_volume_backed")
-                img = instance.image_ref
-                if not img:
-                    properties = bdms.root_metadata(
-                            context, self.compute_api.image_api,
-                            self.compute_api.volume_api)
-                    image_meta = {'properties': properties}
-                else:
-                    image_meta = self.compute_api.image_api.get(context, img)
-
                 image = self.compute_api.snapshot_volume_backed(
                                                        context,
                                                        instance,
-                                                       image_meta,
                                                        image_name,
                                                        extra_properties=
                                                        metadata)

nova/compute/api.py

@@ -2282,17 +2282,27 @@ class API(base.Base):
     # NOTE(melwitt): We don't check instance lock for snapshot because lock is
     #                intended to prevent accidental change/delete of instances
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
-    def snapshot_volume_backed(self, context, instance, image_meta, name,
+    def snapshot_volume_backed(self, context, instance, name,
                                extra_properties=None):
         """Snapshot the given volume-backed instance.
 
         :param instance: nova.objects.instance.Instance object
-        :param image_meta: metadata for the new image
         :param name: name of the backup or snapshot
         :param extra_properties: dict of extra image properties to include
 
         :returns: the new image metadata
         """
+        # TODO(ft): Get metadata from the instance to avoid waste DB request
+        img = instance.image_ref
+        if not img:
+            bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
+                    context, instance.uuid)
+            properties = bdms.root_metadata(
+                    context, self.image_api,
+                    self.volume_api)
+            image_meta = {'properties': properties}
+        else:
+            image_meta = self.image_api.get(context, img)
         image_meta['name'] = name
         image_meta['is_public'] = False
         properties = image_meta['properties']

nova/tests/unit/api/ec2/test_cinder_cloud.py

@@ -1108,10 +1108,12 @@ class CinderCloudTestCase(test.TestCase):
         (volumes, snapshots) = self._setUpImageSet(
             create_volumes_and_snapshots=True)
 
-        kwargs = {'image_id': 'ami-1',
+        kwargs = {'image_id': 'ami-2',
                   'instance_type': CONF.default_flavor,
                   'max_count': 1}
         ec2_instance_id = self._run_instance(**kwargs)
 
+        self.cloud.create_image(self.context, ec2_instance_id)
+
         self.cloud.terminate_instances(self.context, [ec2_instance_id])
         self._restart_compute_service()

nova/tests/unit/compute/test_compute_api.py

@@ -1980,6 +1980,9 @@ class _ComputeAPIUnitTestMixIn(object):
         def fake_get_all_by_instance(context, instance, use_slave=False):
             return copy.deepcopy(instance_bdms)
 
+        def fake_image_get(context, image_id):
+            return copy.deepcopy(image_meta)
+
         def fake_image_create(context, image_meta, data=None):
             self.assertThat(image_meta, matchers.DictMatches(expect_meta))
 
@@ -2000,6 +2003,8 @@ class _ComputeAPIUnitTestMixIn(object):
 
         self.stubs.Set(db, 'block_device_mapping_get_all_by_instance',
                        fake_get_all_by_instance)
+        self.stubs.Set(self.compute_api.image_api, 'get',
+                       fake_image_get)
         self.stubs.Set(self.compute_api.image_api, 'create',
                        fake_image_create)
         self.stubs.Set(self.compute_api.volume_api, 'get',
@@ -2013,7 +2018,7 @@ class _ComputeAPIUnitTestMixIn(object):
 
         # No block devices defined
         self.compute_api.snapshot_volume_backed(
-            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+            self.context, instance, 'test-snapshot')
 
         bdm = fake_block_device.FakeDbBlockDeviceDict(
                 {'no_device': False, 'volume_id': '1', 'boot_index': 0,
@@ -2033,7 +2038,7 @@ class _ComputeAPIUnitTestMixIn(object):
 
         # All the db_only fields and the volume ones are removed
         self.compute_api.snapshot_volume_backed(
-            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+            self.context, instance, 'test-snapshot')
 
         self.assertEqual(quiesce_expected, quiesced[0])
         self.assertEqual(quiesce_expected, quiesced[1])
@@ -2052,7 +2057,7 @@ class _ComputeAPIUnitTestMixIn(object):
 
         # Check that the mappgins from the image properties are included
         self.compute_api.snapshot_volume_backed(
-            self.context, instance, copy.deepcopy(image_meta), 'test-snapshot')
+            self.context, instance, 'test-snapshot')
 
         self.assertEqual(quiesce_expected, quiesced[0])
         self.assertEqual(quiesce_expected, quiesced[1])