Bugfix #780784. KeyError when creating custom image.

Authors

@@ -59,6 +59,7 @@ Mark Washenberger <mark.washenberger@rackspace.com>
 Masanori Itoh <itoumsn@nttdata.co.jp>
 Matt Dietz <matt.dietz@rackspace.com>
 Michael Gundlach <michael.gundlach@rackspace.com>
+Mike Scherbakov <mihgen@gmail.com>
 Monsyne Dragon <mdragon@rackspace.com>
 Monty Taylor <mordred@inaugust.com>
 MORITA Kazutaka <morita.kazutaka@gmail.com>

nova/compute/api.py

@@ -515,9 +515,10 @@ class API(base.Base):
         :returns: A dict containing image metadata
         """
         properties = {'instance_id': str(instance_id),
-                      'user_id': str(context.user_id)}
+                      'user_id': str(context.user_id),
+                      'image_state': 'creating'}
         sent_meta = {'name': name, 'is_public': False,
-                     'properties': properties}
+                     'status': 'creating', 'properties': properties}
         recv_meta = self.image_service.create(context, sent_meta)
         params = {'image_id': recv_meta['id']}
         self._cast_compute_message('snapshot_instance', context, instance_id,

nova/image/fake.py

@@ -19,6 +19,7 @@
 
 import copy
 import datetime
+import random
 
 from nova import exception
 from nova import flags
@@ -32,7 +33,7 @@ LOG = logging.getLogger('nova.image.fake')
 FLAGS = flags.FLAGS
 
 
-class FakeImageService(service.BaseImageService):
+class _FakeImageService(service.BaseImageService):
     """Mock (fake) image service for unit testing."""
 
     def __init__(self):
@@ -48,9 +49,10 @@ class FakeImageService(service.BaseImageService):
                  'container_format': 'ami',
                  'disk_format': 'raw',
                  'properties': {'kernel_id': FLAGS.null_kernel,
-                                'ramdisk_id': FLAGS.null_kernel}}
+                                'ramdisk_id': FLAGS.null_kernel,
+                                'architecture': 'x86_64'}}
         self.create(None, image)
-        super(FakeImageService, self).__init__()
+        super(_FakeImageService, self).__init__()
 
     def index(self, context, filters=None):
         """Returns list of images."""
@@ -74,19 +76,25 @@ class FakeImageService(service.BaseImageService):
                  image_id, self.images)
         raise exception.ImageNotFound(image_id=image_id)
 
-    def create(self, context, data):
+    def create(self, context, metadata, data=None):
         """Store the image data and return the new image id.
 
         :raises: Duplicate if the image already exist.
 
         """
-        image_id = int(data['id'])
+        try:
+            image_id = int(metadata['id'])
+        except KeyError:
+            image_id = random.randint(0, 2 ** 31 - 1)
+
         if self.images.get(image_id):
             raise exception.Duplicate()
 
-        self.images[image_id] = copy.deepcopy(data)
+        metadata['id'] = image_id
+        self.images[image_id] = copy.deepcopy(metadata)
+        return self.images[image_id]
 
-    def update(self, context, image_id, data):
+    def update(self, context, image_id, metadata, data=None):
         """Replace the contents of the given image with the new data.
 
         :raises: ImageNotFound if the image does not exist.
@@ -95,7 +103,7 @@ class FakeImageService(service.BaseImageService):
         image_id = int(image_id)
         if not self.images.get(image_id):
             raise exception.ImageNotFound(image_id=image_id)
-        self.images[image_id] = copy.deepcopy(data)
+        self.images[image_id] = copy.deepcopy(metadata)
 
     def delete(self, context, image_id):
         """Delete the given image.
@@ -111,3 +119,9 @@ class FakeImageService(service.BaseImageService):
     def delete_all(self):
         """Clears out all images."""
         self.images.clear()
+
+_fakeImageService = _FakeImageService()
+
+
+def FakeImageService():
+    return _fakeImageService

nova/tests/test_libvirt.py

@@ -161,6 +161,7 @@ class LibvirtConnTestCase(test.TestCase):
                      'vcpus':         2,
                      'project_id':    'fake',
                      'bridge':        'br101',
+                     'image_id':      '123456',
                      'instance_type_id': '5'}  # m1.small
 
     def lazy_load_library_exists(self):
@@ -281,6 +282,68 @@ class LibvirtConnTestCase(test.TestCase):
         instance_data = dict(self.test_instance)
         self._check_xml_and_container(instance_data)
 
+    def test_snapshot(self):
+        FLAGS.image_service = 'nova.image.fake.FakeImageService'
+
+        # Only file-based instance storages are supported at the moment
+        test_xml = """
+            <domain type='kvm'>
+                <devices>
+                    <disk type='file'>
+                        <source file='filename'/>
+                    </disk>
+                </devices>
+            </domain>
+            """
+
+        class FakeVirtDomain(object):
+
+            def __init__(self):
+                pass
+
+            def snapshotCreateXML(self, *args):
+                return None
+
+            def XMLDesc(self, *args):
+                return test_xml
+
+        def fake_lookup(instance_name):
+            if instance_name == instance_ref.name:
+                return FakeVirtDomain()
+
+        def fake_execute(*args):
+            # Touch filename to pass 'with open(out_path)'
+            open(args[-1], "a").close()
+
+        # Start test
+        image_service = utils.import_object(FLAGS.image_service)
+
+        # Assuming that base image already exists in image_service
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        properties = {'instance_id': instance_ref['id'],
+                      'user_id': str(self.context.user_id)}
+        snapshot_name = 'test-snap'
+        sent_meta = {'name': snapshot_name, 'is_public': False,
+                     'status': 'creating', 'properties': properties}
+        # Create new image. It will be updated in snapshot method
+        # To work with it from snapshot, the single image_service is needed
+        recv_meta = image_service.create(context, sent_meta)
+
+        self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
+        connection.LibvirtConnection._conn.lookupByName = fake_lookup
+        self.mox.StubOutWithMock(connection.utils, 'execute')
+        connection.utils.execute = fake_execute
+
+        self.mox.ReplayAll()
+
+        conn = connection.LibvirtConnection(False)
+        conn.snapshot(instance_ref, recv_meta['id'])
+
+        snapshot = image_service.show(context, recv_meta['id'])
+        self.assertEquals(snapshot['properties']['image_state'], 'available')
+        self.assertEquals(snapshot['status'], 'active')
+        self.assertEquals(snapshot['name'], snapshot_name)
+
     def test_multi_nic(self):
         instance_data = dict(self.test_instance)
         network_info = _create_network_info(2)

nova/virt/libvirt/connection.py

@@ -391,12 +391,15 @@ class LibvirtConnection(driver.ComputeDriver):
         elevated = context.get_admin_context()
 
         base = image_service.show(elevated, instance['image_id'])
+        snapshot = image_service.show(elevated, image_id)
 
         metadata = {'disk_format': base['disk_format'],
                     'container_format': base['container_format'],
                     'is_public': False,
-                    'name': '%s.%s' % (base['name'], image_id),
+                    'status': 'active',
-                    'properties': {'architecture': base['architecture'],
+                    'name': snapshot['name'],
+                    'properties': {'architecture':
+                                        base['properties']['architecture'],
                                    'kernel_id': instance['kernel_id'],
                                    'image_location': 'snapshot',
                                    'image_state': 'available',