Reducing the number of compute calls to Glance

Fixes bug 886224

Change-Id: Ibd270d24eb68cc2503fee933a2154125995d352d

13 files changed, 86 insertions, 61 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 3d937b73d..660824d2e 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -127,6 +127,11 @@ def checks_instance_lock(function):
     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 @@ class ComputeManager(manager.SchedulerDependentManager):
 
     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 @@ class ComputeManager(manager.SchedulerDependentManager):
                    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 @@ class ComputeManager(manager.SchedulerDependentManager):
 
             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 @@ class ComputeManager(manager.SchedulerDependentManager):
         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 @@ class ComputeManager(manager.SchedulerDependentManager):
 
             # 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 @@ class ComputeManager(manager.SchedulerDependentManager):
         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 @@ class ComputeManager(manager.SchedulerDependentManager):
                                             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,
diff --git a/nova/tests/glance/stubs.py b/nova/tests/glance/stubs.py
index 1567393e3..5c0f4f05e 100644
--- a/nova/tests/glance/stubs.py
+++ b/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 @@ class FakeGlance(object):
         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"
diff --git a/nova/tests/test_vmwareapi.py b/nova/tests/test_vmwareapi.py
index 704e362fd..9dacda4b3 100644
--- a/nova/tests/test_vmwareapi.py
+++ b/nova/tests/test_vmwareapi.py
@@ -74,6 +74,12 @@ class VMWareAPIVMTestCase(test.TestCase):
                            '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 @@ class VMWareAPIVMTestCase(test.TestCase):
         """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):
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index 6587bedd1..ee8d5ae20 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -423,7 +423,9 @@ class XenAPIVMTestCase(test.TestCase):
         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 @@ class XenAPIVMTestCase(test.TestCase):
                            '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 @@ class XenAPIMigrateInstance(test.TestCase):
                            '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 @@ class XenAPIMigrateInstance(test.TestCase):
                            '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 @@ class XenAPIMigrateInstance(test.TestCase):
                            '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 @@ class XenAPIMigrateInstance(test.TestCase):
                            '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 XenAPIDetermineDiskImageTestCase(test.TestCase):
 
     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):
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 6bd83fb4d..bcdd20f2c 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -127,7 +127,7 @@ class ComputeDriver(object):
         # 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 @@ class ComputeDriver(object):
         :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 @@ class ComputeDriver(object):
         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()
 
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index ad40297a3..d4a554c81 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -97,7 +97,7 @@ class FakeConnection(driver.ComputeDriver):
         """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
diff --git a/nova/virt/hyperv.py b/nova/virt/hyperv.py
index 16fd94e7f..34c784e7a 100644
--- a/nova/virt/hyperv.py
+++ b/nova/virt/hyperv.py
@@ -138,7 +138,7 @@ class HyperVConnection(driver.ComputeDriver):
 
         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)
diff --git a/nova/virt/libvirt/connection.py b/nova/virt/libvirt/connection.py
index a2022ec18..c2a13f6a7 100644
--- a/nova/virt/libvirt/connection.py
+++ b/nova/virt/libvirt/connection.py
@@ -641,7 +641,7 @@ class LibvirtConnection(driver.ComputeDriver):
     # 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)
diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py
index b4cb7f037..3206bab71 100644
--- a/nova/virt/vmwareapi/vmops.py
+++ b/nova/virt/vmwareapi/vmops.py
@@ -79,7 +79,7 @@ class VMWareVMOps(object):
         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.

 

diff --git a/nova/virt/vmwareapi_conn.py b/nova/virt/vmwareapi_conn.py
index bef8b00f8..4d6e9d5fb 100644
--- a/nova/virt/vmwareapi_conn.py
+++ b/nova/virt/vmwareapi_conn.py
@@ -124,10 +124,10 @@ class VMWareESXConnection(driver.ComputeDriver):
         """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."""

diff --git a/nova/virt/xenapi/vm_utils.py b/nova/virt/xenapi/vm_utils.py
index 006cdbbc6..2c394ce64 100644
--- a/nova/virt/xenapi/vm_utils.py
+++ b/nova/virt/xenapi/vm_utils.py
@@ -717,7 +717,7 @@ w
             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,12 +736,11 @@ w
                              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,
@@ -749,23 +748,13 @@ w
                 '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
diff --git a/nova/virt/xenapi/vmops.py b/nova/virt/xenapi/vmops.py
index a797fa3ea..4f53c8ab9 100644
--- a/nova/virt/xenapi/vmops.py
+++ b/nova/virt/xenapi/vmops.py
@@ -139,13 +139,13 @@ class VMOps(object):
         self._start(instance, vm_ref)
 
     def finish_migration(self, context, migration, instance, disk_info,
-                         network_info, resize_instance):
+                         network_info, image_meta, resize_instance):
         vdi_uuid = self.link_disks(instance, disk_info['base_copy'],
                 disk_info['cow'])
 
         vm_ref = self._create_vm(context, instance,
                                  [dict(vdi_type='os', vdi_uuid=vdi_uuid)],
-                                 network_info)
+                                 network_info, image_meta)
         if resize_instance:
             self.resize_instance(instance, vdi_uuid)
 
@@ -165,8 +165,8 @@ class VMOps(object):
         LOG.debug(_("Starting instance %s"), instance.name)
         self._session.call_xenapi('VM.start', vm_ref, False, False)
 
-    def _create_disks(self, context, instance):
-        disk_image_type = VMHelper.determine_disk_image_type(instance, context)
+    def _create_disks(self, context, instance, image_meta):
+        disk_image_type = VMHelper.determine_disk_image_type(image_meta)
         vdis = VMHelper.fetch_image(context, self._session,
                 instance, instance.image_ref,
                 instance.user_id, instance.project_id,
@@ -178,7 +178,7 @@ class VMOps(object):
 
         return vdis
 
-    def spawn(self, context, instance, network_info):
+    def spawn(self, context, instance, image_meta, network_info):
         vdis = None
         try:
             # 1. Vanity Step
@@ -193,13 +193,14 @@ class VMOps(object):
                                            total_steps=BUILD_TOTAL_STEPS)
 
             # 2. Fetch the Image over the Network
-            vdis = self._create_disks(context, instance)
+            vdis = self._create_disks(context, instance, image_meta)
             self._update_instance_progress(context, instance,
                                            step=2,
                                            total_steps=BUILD_TOTAL_STEPS)
 
             # 3. Create the VM records
-            vm_ref = self._create_vm(context, instance, vdis, network_info)
+            vm_ref = self._create_vm(context, instance, vdis, network_info,
+                                     image_meta)
             self._update_instance_progress(context, instance,
                                            step=3,
                                            total_steps=BUILD_TOTAL_STEPS)
@@ -222,7 +223,7 @@ class VMOps(object):
         """Spawn a rescue instance."""
         self.spawn(context, instance, network_info)
 
-    def _create_vm(self, context, instance, vdis, network_info):
+    def _create_vm(self, context, instance, vdis, network_info, image_meta):
         """Create VM instance."""
         instance_name = instance.name
         vm_ref = VMHelper.lookup(self._session, instance_name)
@@ -233,7 +234,7 @@ class VMOps(object):
         if not VMHelper.ensure_free_mem(self._session, instance):
             raise exception.InsufficientFreeMemory(uuid=instance.uuid)
 
-        disk_image_type = VMHelper.determine_disk_image_type(instance, context)
+        disk_image_type = VMHelper.determine_disk_image_type(image_meta)
         kernel = None
         ramdisk = None
         try:
diff --git a/nova/virt/xenapi_conn.py b/nova/virt/xenapi_conn.py
index 47554e757..ab9c3e368 100644
--- a/nova/virt/xenapi_conn.py
+++ b/nova/virt/xenapi_conn.py
@@ -193,10 +193,10 @@ class XenAPIConnection(driver.ComputeDriver):
     def list_instances_detail(self):
         return self._vmops.list_instances_detail()
 
-    def spawn(self, context, instance,
+    def spawn(self, context, instance, image_meta,
               network_info=None, block_device_info=None):
         """Create VM instance"""
-        self._vmops.spawn(context, instance, network_info)
+        self._vmops.spawn(context, instance, image_meta, network_info)
 
     def confirm_migration(self, migration, instance, network_info):
         """Confirms a resize, destroying the source VM"""
@@ -208,10 +208,10 @@ class XenAPIConnection(driver.ComputeDriver):
         self._vmops.finish_revert_migration(instance)
 
     def finish_migration(self, context, migration, instance, disk_info,
-                         network_info, resize_instance=False):
+                         network_info, image_meta, resize_instance=False):
         """Completes a resize, turning on the migrated instance"""
         self._vmops.finish_migration(context, migration, instance, disk_info,
-                                     network_info, resize_instance)
+                                     network_info, image_meta, resize_instance)
 
     def snapshot(self, context, instance, image_id):
         """ Create snapshot from a running VM instance """