Reducing the number of compute calls to Glance

Fixes bug 886224

Change-Id: Ibd270d24eb68cc2503fee933a2154125995d352d

nova/compute/manager.py

@@ -127,6 +127,11 @@ def decorated_function(self, context, instance_id, *args, **kwargs):
     return decorated_function
 
 
+def _get_image_meta(context, image_ref):
+    image_service, image_id = nova.image.get_image_service(context, image_ref)
+    return image_service.show(context, image_id)
+
+
 class ComputeManager(manager.SchedulerDependentManager):
     """Manages the running instances from creation to destruction."""
 
@@ -310,7 +315,7 @@ def _setup_block_device_mapping(self, context, instance_id):
 
     def _run_instance(self, context, instance_id, **kwargs):
         """Launch a new instance with specified options."""
-        def _check_image_size():
+        def _check_image_size(image_meta):
             """Ensure image is smaller than the maximum size allowed by the
             instance_type.
 
@@ -325,13 +330,6 @@ def _check_image_size():
                    image, but is accurate because it reflects the image's
                    actual size.
             """
-            # NOTE(jk0): image_ref is defined in the DB model, image_href is
-            # used by the image service. This should be refactored to be
-            # consistent.
-            image_href = instance['image_ref']
-            image_service, image_id = nova.image.get_image_service(context,
-                                                                   image_href)
-            image_meta = image_service.show(context, image_id)
 
             try:
                 size_bytes = image_meta['size']
@@ -355,6 +353,7 @@ def _check_image_size():
 
             allowed_size_bytes = allowed_size_gb * 1024 * 1024 * 1024
 
+            image_id = image_meta['id']
             LOG.debug(_("image_id=%(image_id)s, image_size_bytes="
                         "%(size_bytes)d, allowed_size_bytes="
                         "%(allowed_size_bytes)d") % locals())
@@ -427,7 +426,9 @@ def _logging_error(instance_id, message):
         if instance['name'] in self.driver.list_instances():
             raise exception.Error(_("Instance has already been created"))
 
-        _check_image_size()
+        image_meta = _get_image_meta(context, instance['image_ref'])
+
+        _check_image_size(image_meta)
 
         LOG.audit(_("instance %s: starting..."), instance_id,
                   context=context)
@@ -460,7 +461,7 @@ def _logging_error(instance_id, message):
 
             # TODO(vish) check to make sure the availability zone matches
             with _logging_error(instance_id, "failed to spawn"):
-                self.driver.spawn(context, instance,
+                self.driver.spawn(context, instance, image_meta,
                                   network_info, block_device_info)
 
             current_power_state = self._get_power_state(context, instance)
@@ -664,7 +665,10 @@ def rebuild_instance(self, context, instance_id, **kwargs):
         instance_ref.admin_pass = kwargs.get('new_pass',
                 utils.generate_password(FLAGS.password_length))
 
-        self.driver.spawn(context, instance_ref, network_info, bd_mapping)
+        image_meta = _get_image_meta(context, instance_ref['image_ref'])
+
+        self.driver.spawn(context, instance_ref, image_meta,
+                          network_info, bd_mapping)
 
         current_power_state = self._get_power_state(context, instance_ref)
         self._instance_update(context,
@@ -1146,8 +1150,13 @@ def finish_resize(self, context, instance_id, migration_id, disk_info):
                                             instance_ref.uuid)
 
         network_info = self._get_instance_nw_info(context, instance_ref)
+
+        # Have to look up image here since we depend on disk_format later
+        image_meta = _get_image_meta(context, instance_ref['image_ref'])
+
         self.driver.finish_migration(context, migration_ref, instance_ref,
-                                     disk_info, network_info, resize_instance)
+                                     disk_info, network_info, image_meta,
+                                     resize_instance)
 
         self._instance_update(context,
                               instance_id,

nova/tests/glance/stubs.py

@@ -14,6 +14,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import copy
 import StringIO
 
 from nova import exception
@@ -74,11 +75,15 @@ def set_auth_token(self, auth_tok):
         pass
 
     def get_image_meta(self, image_id):
-        return self.IMAGE_FIXTURES[int(image_id)]['image_meta']
+        meta = copy.deepcopy(self.IMAGE_FIXTURES[int(image_id)]['image_meta'])
+        meta['id'] = image_id
+        return meta
 
     def get_image(self, image_id):
         image = self.IMAGE_FIXTURES[int(image_id)]
-        return image['image_meta'], image['image_data']
+        meta = copy.deepcopy(image['image_meta'])
+        meta['id'] = image_id
+        return meta, image['image_data']
 
 
 NOW_GLANCE_FORMAT = "2010-10-11T10:30:22"

nova/tests/test_vmwareapi.py

@@ -74,6 +74,12 @@ def setUp(self):
                            'mac': 'DE:AD:BE:EF:00:00',
                            'rxtx_cap': 3})]
 
+        self.image = {
+            'id': 'c1c8ce3d-c2e0-4247-890c-ccf5cc1c004c',
+            'disk_format': 'vhd',
+            'size': 512,
+        }
+
     def tearDown(self):
         super(VMWareAPIVMTestCase, self).tearDown()
         vmwareapi_fake.cleanup()
@@ -95,7 +101,8 @@ def _create_vm(self):
         """Create and spawn the VM."""
         self._create_instance_in_the_db()
         self.type_data = db.instance_type_get_by_name(None, 'm1.large')
-        self.conn.spawn(self.context, self.instance, self.network_info)
+        self.conn.spawn(self.context, self.instance, self.image,
+                        self.network_info)
         self._check_vm_record()
 
     def _check_vm_record(self):

nova/tests/test_xenapi.py

@@ -423,7 +423,9 @@ def _test_spawn(self, image_ref, kernel_id, ramdisk_id,
         if empty_dns:
             network_info[0][1]['dns'] = []
 
-        self.conn.spawn(self.context, instance, network_info)
+        image_meta = {'id': glance_stubs.FakeGlance.IMAGE_VHD,
+                      'disk_format': 'vhd'}
+        self.conn.spawn(self.context, instance, image_meta, network_info)
         self.create_vm_record(self.conn, os_type, instance_id)
         self.check_vm_record(self.conn, check_injection)
         self.assertTrue(instance.os_type)
@@ -694,8 +696,10 @@ def _create_instance(self, instance_id=1, spawn=True):
                            'label': 'fake',
                            'mac': 'DE:AD:BE:EF:00:00',
                            'rxtx_cap': 3})]
+        image_meta = {'id': glance_stubs.FakeGlance.IMAGE_VHD,
+                      'disk_format': 'vhd'}
         if spawn:
-            self.conn.spawn(self.context, instance, network_info)
+            self.conn.spawn(self.context, instance, image_meta, network_info)
         return instance
 
 
@@ -842,9 +846,10 @@ def fake_finish_revert_migration(*args, **kwargs):
                            'label': 'fake',
                            'mac': 'DE:AD:BE:EF:00:00',
                            'rxtx_cap': 3})]
+        image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'}
         conn.finish_migration(self.context, self.migration, instance,
                               dict(base_copy='hurr', cow='durr'),
-                              network_info, resize_instance=True)
+                              network_info, image_meta, resize_instance=True)
         self.assertEqual(self.called, True)
         self.assertEqual(self.fake_vm_start_called, True)
 
@@ -883,9 +888,10 @@ def fake_vdi_resize(*args, **kwargs):
                            'label': 'fake',
                            'mac': 'DE:AD:BE:EF:00:00',
                            'rxtx_cap': 3})]
+        image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'}
         conn.finish_migration(self.context, self.migration, instance,
                               dict(base_copy='hurr', cow='durr'),
-                              network_info, resize_instance=True)
+                              network_info, image_meta, resize_instance=True)
         self.assertEqual(self.called, True)
         self.assertEqual(self.fake_vm_start_called, True)
 
@@ -918,9 +924,10 @@ def fake_vdi_resize(*args, **kwargs):
                            'label': 'fake',
                            'mac': 'DE:AD:BE:EF:00:00',
                            'rxtx_cap': 3})]
+        image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'}
         conn.finish_migration(self.context, self.migration, instance,
                               dict(base_copy='hurr', cow='durr'),
-                              network_info, resize_instance=True)
+                              network_info, image_meta, resize_instance=True)
 
     def test_finish_migrate_no_resize_vdi(self):
         instance = db.instance_create(self.context, self.instance_values)
@@ -949,9 +956,10 @@ def fake_vdi_resize(*args, **kwargs):
                            'rxtx_cap': 3})]
 
         # Resize instance would be determined by the compute call
+        image_meta = {'id': instance.image_ref, 'disk_format': 'vhd'}
         conn.finish_migration(self.context, self.migration, instance,
                               dict(base_copy='hurr', cow='durr'),
-                              network_info, resize_instance=False)
+                              network_info, image_meta, resize_instance=False)
 
 
 class XenAPIImageTypeTestCase(test.TestCase):
@@ -986,8 +994,9 @@ class FakeInstance(object):
 
     def assert_disk_type(self, disk_type):
         ctx = context.RequestContext('fake', 'fake')
-        dt = vm_utils.VMHelper.determine_disk_image_type(
-            self.fake_instance, ctx)
+        fake_glance = glance_stubs.FakeGlance('')
+        image_meta = fake_glance.get_image_meta(self.fake_instance.image_ref)
+        dt = vm_utils.VMHelper.determine_disk_image_type(image_meta)
         self.assertEqual(disk_type, dt)
 
     def test_instance_disk(self):

nova/virt/driver.py

@@ -127,7 +127,7 @@ def list_instances_detail(self):
         # TODO(Vek): Need to pass context in for access to auth_token
         raise NotImplementedError()
 
-    def spawn(self, context, instance,
+    def spawn(self, context, instance, image_meta,
               network_info=None, block_device_info=None):
         """
         Create a new instance/VM/domain on the virtualization platform.
@@ -143,6 +143,8 @@ def spawn(self, context, instance,
         :param instance: Instance object as returned by DB layer.
                          This function should use the data there to guide
                          the creation of the new instance.
+        :param image_meta: image object returned by nova.image.glance that
+                           defines the image from which to boot this instance
         :param network_info:
            :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info`
         :param block_device_info: Information about block devices to be
@@ -253,11 +255,14 @@ def snapshot(self, context, instance, image_id):
         raise NotImplementedError()
 
     def finish_migration(self, context, migration, instance, disk_info,
-                         network_info, resize_instance):
+                         network_info, image_meta, resize_instance):
         """Completes a resize, turning on the migrated instance
 
         :param network_info:
            :py:meth:`~nova.network.manager.NetworkManager.get_instance_nw_info`
+        :param image_meta: image object returned by nova.image.glance that
+                           defines the image from which this instance
+                           was created
         """
         raise NotImplementedError()
 

nova/virt/fake.py

@@ -97,7 +97,7 @@ def plug_vifs(self, instance, network_info):
         """Plugin VIFs into networks."""
         pass
 
-    def spawn(self, context, instance,
+    def spawn(self, context, instance, image_meta,
               network_info=None, block_device_info=None):
         name = instance.name
         state = power_state.RUNNING

nova/virt/hyperv.py

@@ -138,7 +138,7 @@ def list_instances_detail(self):
 
         return instance_infos
 
-    def spawn(self, context, instance,
+    def spawn(self, context, instance, image_meta,
               network_info=None, block_device_info=None):
         """ Create a new VM and start it."""
         vm = self._lookup(instance.name)

nova/virt/libvirt/connection.py

@@ -641,7 +641,7 @@ def poll_unconfirmed_resizes(self, resize_confirm_window):
     # NOTE(ilyaalekseyev): Implementation like in multinics
     # for xenapi(tr3buchet)
     @exception.wrap_exception()
-    def spawn(self, context, instance, network_info,
+    def spawn(self, context, instance, image_meta, network_info,
               block_device_info=None):
         xml = self.to_xml(instance, network_info, False,
                           block_device_info=block_device_info)

nova/virt/vmwareapi/vmops.py

@@ -79,7 +79,7 @@ def list_instances(self):
         LOG.debug(_("Got total of %s instances") % str(len(lst_vm_names)))
         return lst_vm_names
 
-    def spawn(self, context, instance, network_info):
+    def spawn(self, context, instance, image_meta, network_info):
         """
         Creates a VM instance.
 

nova/virt/vmwareapi_conn.py

@@ -124,10 +124,10 @@ def list_instances(self):
         """List VM instances."""
         return self._vmops.list_instances()
 
-    def spawn(self, context, instance, network_info,
+    def spawn(self, context, instance, image_meta, network_info,
               block_device_mapping=None):
         """Create VM instance."""
-        self._vmops.spawn(context, instance, network_info)
+        self._vmops.spawn(context, instance, image_meta, network_info)
 
     def snapshot(self, context, instance, name):
         """Create snapshot from a running VM instance."""

nova/virt/xenapi/vm_utils.py

@@ -717,7 +717,7 @@ def _fetch_image_glance_disk(cls, context, session, instance, image,
             raise e
 
     @classmethod
-    def determine_disk_image_type(cls, instance, context):
+    def determine_disk_image_type(cls, image_meta):
         """Disk Image Types are used to determine where the kernel will reside
         within an image. To figure out which type we're dealing with, we use
         the following rules:
@@ -736,36 +736,25 @@ def log_disk_format(image_type):
                              ImageType.DISK_VHD: 'DISK_VHD',
                              ImageType.DISK_ISO: 'DISK_ISO'}
             disk_format = pretty_format[image_type]
-            image_ref = instance.image_ref
-            instance_id = instance.id
+            image_ref = image_meta['id']
             LOG.debug(_("Detected %(disk_format)s format for image "
-                        "%(image_ref)s, instance %(instance_id)s") % locals())
+                        "%(image_ref)s") % locals())
 
-        def determine_from_glance():
+        def determine_from_image_meta():
             glance_disk_format2nova_type = {
                 'ami': ImageType.DISK,
                 'aki': ImageType.KERNEL,
                 'ari': ImageType.RAMDISK,
                 'raw': ImageType.DISK_RAW,
                 'vhd': ImageType.DISK_VHD,
                 'iso': ImageType.DISK_ISO}
-            image_ref = instance.image_ref
-            glance_client, image_id = glance.get_glance_client(context,
-                                                               image_ref)
-            meta = glance_client.get_image_meta(image_id)
-            disk_format = meta['disk_format']
+            disk_format = image_meta['disk_format']
             try:
                 return glance_disk_format2nova_type[disk_format]
             except KeyError:
                 raise exception.InvalidDiskFormat(disk_format=disk_format)
 
-        def determine_from_instance():
-            if instance.kernel_id:
-                return ImageType.DISK
-            else:
-                return ImageType.DISK_RAW
-
-        image_type = determine_from_glance()
+        image_type = determine_from_image_meta()
 
         log_disk_format(image_type)
         return image_type