Merge "Add instance_type_get() to virt api"

12 files changed, 95 insertions, 90 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 9ca00bd9b..49f62bc2f 100755
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -314,6 +314,10 @@ class ComputeVirtAPI(virtapi.VirtAPI):
         return self._compute.conductor_api.agent_build_get_by_triple(
             context, hypervisor, os, architecture)
 
+    def instance_type_get(self, context, instance_type_id):
+        return self._compute.conductor_api.instance_type_get(context,
+                                                             instance_type_id)
+
 
 class ComputeManager(manager.SchedulerDependentManager):
     """Manages the running instances from creation to destruction."""
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index e1e4a00e7..fe3279b2a 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1813,18 +1813,6 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False):
     with session.begin():
         instance_ref = _instance_get_by_uuid(context, instance_uuid,
                                              session=session)
-        # TODO(deva): remove extra_specs from here after it is included
-        #             in system_metadata. Until then, the baremetal driver
-        #             needs extra_specs added to instance[]
-        inst_type_ref = _instance_type_get_query(context, session=session).\
-                            filter_by(id=instance_ref['instance_type_id']).\
-                            first()
-        if inst_type_ref:
-            instance_ref['extra_specs'] = \
-                _dict_with_extra_specs(inst_type_ref).get('extra_specs', {})
-        else:
-            instance_ref['extra_specs'] = {}
-
         if "expected_task_state" in values:
             # it is not a db column so always pop out
             expected = values.pop("expected_task_state")
diff --git a/nova/tests/baremetal/test_pxe.py b/nova/tests/baremetal/test_pxe.py
index 4f4c9f7db..fd82625ef 100644
--- a/nova/tests/baremetal/test_pxe.py
+++ b/nova/tests/baremetal/test_pxe.py
@@ -37,6 +37,7 @@ from nova.virt.baremetal import db
 from nova.virt.baremetal import pxe
 from nova.virt.baremetal import utils as bm_utils
 from nova.virt.disk import api as disk_api
+from nova.virt import fake as fake_virt
 
 CONF = cfg.CONF
 
@@ -61,7 +62,7 @@ class BareMetalPXETestCase(bm_db_base.BMDBTestCase):
         super(BareMetalPXETestCase, self).setUp()
         self.flags(**COMMON_FLAGS)
         self.flags(**BAREMETAL_FLAGS)
-        self.driver = pxe.PXE()
+        self.driver = pxe.PXE(fake_virt.FakeVirtAPI())
 
         fake_image.stub_out_image_service(self.stubs)
         self.addCleanup(fake_image.FakeImageService_reset)
@@ -239,20 +240,21 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase):
         self.assertEqual(sizes[1], 1)
 
     def test_get_tftp_image_info(self):
+        instance_type = utils.get_test_instance_type()
         # Raises an exception when options are neither specified
         # on the instance nor in configuration file
         CONF.baremetal.deploy_kernel = None
         CONF.baremetal.deploy_ramdisk = None
         self.assertRaises(exception.NovaException,
                 pxe.get_tftp_image_info,
-                self.instance)
+                self.instance, instance_type)
 
         # Test that other non-true values also raise an exception
         CONF.baremetal.deploy_kernel = ""
         CONF.baremetal.deploy_ramdisk = ""
         self.assertRaises(exception.NovaException,
                 pxe.get_tftp_image_info,
-                self.instance)
+                self.instance, instance_type)
 
         # Even if the instance includes kernel_id and ramdisk_id,
         # we still need deploy_kernel_id and deploy_ramdisk_id.
@@ -262,7 +264,7 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase):
         self.instance['ramdisk_id'] = 'bbbb'
         self.assertRaises(exception.NovaException,
                 pxe.get_tftp_image_info,
-                self.instance)
+                self.instance, instance_type)
 
         # If an instance doesn't specify deploy_kernel_id or deploy_ramdisk_id,
         # but defaults are set in the config file, we should use those.
@@ -272,7 +274,7 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase):
         CONF.baremetal.deploy_kernel = 'cccc'
         CONF.baremetal.deploy_ramdisk = 'dddd'
         base = os.path.join(CONF.baremetal.tftp_root, self.instance['uuid'])
-        res = pxe.get_tftp_image_info(self.instance)
+        res = pxe.get_tftp_image_info(self.instance, instance_type)
         expected = {
                 'kernel': ['aaaa', os.path.join(base, 'kernel')],
                 'ramdisk': ['bbbb', os.path.join(base, 'ramdisk')],
@@ -290,8 +292,8 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase):
                 'deploy_kernel_id': 'eeee',
                 'deploy_ramdisk_id': 'ffff',
             }
-        self.instance['extra_specs'] = extra_specs
-        res = pxe.get_tftp_image_info(self.instance)
+        instance_type['extra_specs'] = extra_specs
+        res = pxe.get_tftp_image_info(self.instance, instance_type)
         self.assertEqual(res['deploy_kernel'][0], 'eeee')
         self.assertEqual(res['deploy_ramdisk'][0], 'ffff')
 
@@ -301,10 +303,10 @@ class PXEClassMethodsTestCase(BareMetalPXETestCase):
                 'deploy_kernel_id': '',
                 'deploy_ramdisk_id': '',
             }
-        self.instance['extra_specs'] = extra_specs
+        instance_type['extra_specs'] = extra_specs
         self.assertRaises(exception.NovaException,
                 pxe.get_tftp_image_info,
-                self.instance)
+                self.instance, instance_type)
 
 
 class PXEPrivateMethodsTestCase(BareMetalPXETestCase):
@@ -320,12 +322,13 @@ class PXEPrivateMethodsTestCase(BareMetalPXETestCase):
     def test_cache_tftp_images(self):
         self.instance['kernel_id'] = 'aaaa'
         self.instance['ramdisk_id'] = 'bbbb'
+        instance_type = utils.get_test_instance_type()
         extra_specs = {
                 'deploy_kernel_id': 'cccc',
                 'deploy_ramdisk_id': 'dddd',
             }
-        self.instance['extra_specs'] = extra_specs
-        image_info = pxe.get_tftp_image_info(self.instance)
+        instance_type['extra_specs'] = extra_specs
+        image_info = pxe.get_tftp_image_info(self.instance, instance_type)
 
         self.mox.StubOutWithMock(os, 'makedirs')
         self.mox.StubOutWithMock(os.path, 'exists')
@@ -390,12 +393,15 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
 
     def test_cache_images(self):
         self._create_node()
+        self.mox.StubOutWithMock(self.driver.virtapi, 'instance_type_get')
         self.mox.StubOutWithMock(pxe, "get_tftp_image_info")
         self.mox.StubOutWithMock(self.driver, "_cache_tftp_images")
         self.mox.StubOutWithMock(self.driver, "_cache_image")
         self.mox.StubOutWithMock(self.driver, "_inject_into_image")
 
-        pxe.get_tftp_image_info(self.instance).AndReturn([])
+        self.driver.virtapi.instance_type_get(
+            self.context, self.instance['instance_type_id']).AndReturn({})
+        pxe.get_tftp_image_info(self.instance, {}).AndReturn([])
         self.driver._cache_tftp_images(self.context, self.instance, [])
         self.driver._cache_image(self.context, self.instance, [])
         self.driver._inject_into_image(self.context, self.node, self.instance,
@@ -440,6 +446,7 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         pxe_path = pxe.get_pxe_config_file_path(self.instance)
         image_path = pxe.get_image_file_path(self.instance)
 
+        self.mox.StubOutWithMock(self.driver.virtapi, 'instance_type_get')
         self.mox.StubOutWithMock(pxe, 'get_tftp_image_info')
         self.mox.StubOutWithMock(pxe, 'get_partition_sizes')
         self.mox.StubOutWithMock(bm_utils, 'random_alnum')
@@ -447,7 +454,9 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         self.mox.StubOutWithMock(bm_utils, 'write_to_file')
         self.mox.StubOutWithMock(bm_utils, 'create_link_without_raise')
 
-        pxe.get_tftp_image_info(self.instance).AndReturn(image_info)
+        self.driver.virtapi.instance_type_get(
+            self.context, self.instance['instance_type_id']).AndReturn({})
+        pxe.get_tftp_image_info(self.instance, {}).AndReturn(image_info)
         pxe.get_partition_sizes(self.instance).AndReturn((0, 0))
         bm_utils.random_alnum(32).AndReturn('alnum')
         pxe.build_pxe_config(
@@ -466,18 +475,24 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
 
     def test_activate_and_deactivate_bootloader(self):
         self._create_node()
-        extra_specs = {
+        instance_type = {
+            'extra_specs': {
                 'deploy_kernel_id': 'eeee',
                 'deploy_ramdisk_id': 'ffff',
+                }
             }
-        self.instance['extra_specs'] = extra_specs
         self.instance['uuid'] = 'fake-uuid'
 
+        self.mox.StubOutWithMock(self.driver.virtapi, 'instance_type_get')
         self.mox.StubOutWithMock(bm_utils, 'write_to_file')
         self.mox.StubOutWithMock(bm_utils, 'create_link_without_raise')
         self.mox.StubOutWithMock(bm_utils, 'unlink_without_raise')
         self.mox.StubOutWithMock(bm_utils, 'rmtree_without_raise')
 
+        self.driver.virtapi.instance_type_get(
+            self.context, self.instance['instance_type_id']).AndReturn(
+                instance_type)
+
         # create the config file
         bm_utils.write_to_file(mox.StrContains('fake-uuid'),
                                mox.StrContains(CONF.baremetal.tftp_root))
@@ -525,7 +540,9 @@ class PXEPublicMethodsTestCase(BareMetalPXETestCase):
         self.mox.StubOutWithMock(pxe, 'get_tftp_image_info')
         self.mox.StubOutWithMock(self.driver, '_collect_mac_addresses')
 
-        pxe.get_tftp_image_info(self.instance).\
+        extra_specs = dict(extra_specs=dict(deploy_ramdisk_id='ignore',
+                                            deploy_kernel_id='ignore'))
+        pxe.get_tftp_image_info(self.instance, extra_specs).\
                 AndRaise(exception.NovaException)
         bm_utils.unlink_without_raise(pxe_path)
         self.driver._collect_mac_addresses(self.context, self.node).\
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index d375c036e..78eee9ae9 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -5073,7 +5073,6 @@ class ComputeAPITestCase(BaseTestCase):
         instance = self._create_fake_instance(dict(host='host2'))
         instance = db.instance_get_by_uuid(self.context, instance['uuid'])
         instance = jsonutils.to_primitive(instance)
-        instance['instance_type']['extra_specs'] = []
         orig_instance_type = instance['instance_type']
         self.compute.run_instance(self.context, instance=instance)
         # We need to set the host to something 'known'.  Unfortunately,
diff --git a/nova/tests/compute/test_virtapi.py b/nova/tests/compute/test_virtapi.py
index 568bf456d..e3887d033 100644
--- a/nova/tests/compute/test_virtapi.py
+++ b/nova/tests/compute/test_virtapi.py
@@ -77,6 +77,10 @@ class VirtAPIBaseTest(test.TestCase, test.APICoverage):
         self.assertExpected('agent_build_get_by_triple',
                             'fake-hv', 'gnu/hurd', 'fake-arch')
 
+    def test_instance_type_get(self):
+        self.assertExpected('instance_type_get',
+                            'fake-instance-type')
+
 
 class FakeVirtAPITest(VirtAPIBaseTest):
 
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 346e0b2b7..c34c94bc7 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -451,41 +451,6 @@ class DbApiTestCase(test.TestCase):
         self.assertEquals("building", old_ref["vm_state"])
         self.assertEquals("needscoffee", new_ref["vm_state"])
 
-    def test_instance_update_with_extra_specs(self):
-        # Ensure _extra_specs are returned from _instance_update.
-        ctxt = context.get_admin_context()
-
-        # create a flavor
-        inst_type_dict = dict(
-                    name="test_flavor",
-                    memory_mb=1,
-                    vcpus=1,
-                    root_gb=1,
-                    ephemeral_gb=1,
-                    flavorid=105)
-        inst_type_ref = db.instance_type_create(ctxt, inst_type_dict)
-
-        # add some extra spec to our flavor
-        spec = {'test_spec': 'foo'}
-        db.instance_type_extra_specs_update_or_create(
-                    ctxt,
-                    inst_type_ref['flavorid'],
-                    spec)
-
-        # create instance, just populates db, doesn't pull extra_spec
-        instance = db.instance_create(
-                    ctxt,
-                    {'instance_type_id': inst_type_ref['id']})
-        self.assertNotIn('extra_specs', instance)
-
-        # update instance, used when starting instance to set state, etc
-        (old_ref, new_ref) = db.instance_update_and_get_original(
-                    ctxt,
-                    instance['uuid'],
-                    {})
-        self.assertEquals(spec, old_ref['extra_specs'])
-        self.assertEquals(spec, new_ref['extra_specs'])
-
     def _test_instance_update_updates_metadata(self, metadata_type):
         ctxt = context.get_admin_context()
 
diff --git a/nova/virt/baremetal/base.py b/nova/virt/baremetal/base.py
index 8cd9e9b3c..2029400ba 100644
--- a/nova/virt/baremetal/base.py
+++ b/nova/virt/baremetal/base.py
@@ -21,8 +21,8 @@ from nova.virt.baremetal import baremetal_states
 
 class NodeDriver(object):
 
-    def __init__(self):
-        pass
+    def __init__(self, virtapi):
+        self.virtapi = virtapi
 
     def cache_images(self, context, node, instance, **kwargs):
         raise NotImplementedError()
diff --git a/nova/virt/baremetal/driver.py b/nova/virt/baremetal/driver.py
index 8dff0a785..3c4dd579f 100755
--- a/nova/virt/baremetal/driver.py
+++ b/nova/virt/baremetal/driver.py
@@ -122,7 +122,7 @@ class BareMetalDriver(driver.ComputeDriver):
         super(BareMetalDriver, self).__init__(virtapi)
 
         self.driver = importutils.import_object(
-                CONF.baremetal.driver)
+                CONF.baremetal.driver, virtapi)
         self.vif_driver = importutils.import_object(
                 CONF.baremetal.vif_driver)
         self.firewall_driver = firewall.load_driver(
diff --git a/nova/virt/baremetal/pxe.py b/nova/virt/baremetal/pxe.py
index 1e98126e2..fe92399ca 100644
--- a/nova/virt/baremetal/pxe.py
+++ b/nova/virt/baremetal/pxe.py
@@ -148,13 +148,13 @@ def build_network_config(network_info):
     return network_config
 
 
-def get_deploy_aki_id(instance):
-    return instance.get('extra_specs', {}).\
+def get_deploy_aki_id(instance_type):
+    return instance_type.get('extra_specs', {}).\
                 get('deploy_kernel_id', CONF.baremetal.deploy_kernel)
 
 
-def get_deploy_ari_id(instance):
-    return instance.get('extra_specs', {}).\
+def get_deploy_ari_id(instance_type):
+    return instance_type.get('extra_specs', {}).\
                 get('deploy_ramdisk_id', CONF.baremetal.deploy_ramdisk)
 
 
@@ -196,13 +196,13 @@ def get_pxe_mac_path(mac):
         )
 
 
-def get_tftp_image_info(instance):
+def get_tftp_image_info(instance, instance_type):
     """Generate the paths for tftp files for this instance
 
     Raises NovaException if
     - instance does not contain kernel_id or ramdisk_id
     - deploy_kernel_id or deploy_ramdisk_id can not be read from
-      instance['extra_specs'] and defaults are not set
+      instance_type['extra_specs'] and defaults are not set
 
     """
     image_info = {
@@ -214,8 +214,8 @@ def get_tftp_image_info(instance):
     try:
         image_info['kernel'][0] = str(instance['kernel_id'])
         image_info['ramdisk'][0] = str(instance['ramdisk_id'])
-        image_info['deploy_kernel'][0] = get_deploy_aki_id(instance)
-        image_info['deploy_ramdisk'][0] = get_deploy_ari_id(instance)
+        image_info['deploy_kernel'][0] = get_deploy_aki_id(instance_type)
+        image_info['deploy_ramdisk'][0] = get_deploy_ari_id(instance_type)
     except KeyError as e:
         pass
 
@@ -237,8 +237,8 @@ def get_tftp_image_info(instance):
 class PXE(base.NodeDriver):
     """PXE bare metal driver."""
 
-    def __init__(self):
-        super(PXE, self).__init__()
+    def __init__(self, virtapi):
+        super(PXE, self).__init__(virtapi)
 
     def _collect_mac_addresses(self, context, node):
         macs = set()
@@ -341,7 +341,9 @@ class PXE(base.NodeDriver):
     def cache_images(self, context, node, instance,
             admin_password, image_meta, injected_files, network_info):
         """Prepare all the images for this instance."""
-        tftp_image_info = get_tftp_image_info(instance)
+        instance_type = self.virtapi.instance_type_get(
+            context, instance['instance_type_id'])
+        tftp_image_info = get_tftp_image_info(instance, instance_type)
         self._cache_tftp_images(context, instance, tftp_image_info)
 
         self._cache_image(context, instance, image_meta)
@@ -374,7 +376,9 @@ class PXE(base.NodeDriver):
             ./pxelinux.cfg/
                  {mac} -> ../{uuid}/config
         """
-        image_info = get_tftp_image_info(instance)
+        instance_type = self.virtapi.instance_type_get(
+            context, instance['instance_type_id'])
+        image_info = get_tftp_image_info(instance, instance_type)
         (root_mb, swap_mb) = get_partition_sizes(instance)
         pxe_config_file_path = get_pxe_config_file_path(instance)
         image_file_path = get_image_file_path(instance)
@@ -416,8 +420,13 @@ class PXE(base.NodeDriver):
         except exception.NodeNotFound:
             pass
 
+        # NOTE(danms): the instance_type extra_specs do not need to be
+        # present/correct at deactivate time, so pass something empty
+        # to avoid an extra lookup
+        instance_type = dict(extra_specs=dict(deploy_ramdisk_id='ignore',
+                                              deploy_kernel_id='ignore'))
         try:
-            image_info = get_tftp_image_info(instance)
+            image_info = get_tftp_image_info(instance, instance_type)
         except exception.NovaException:
             pass
         else:
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 5545dcf96..8afff569c 100755
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -470,3 +470,6 @@ class FakeVirtAPI(virtapi.VirtAPI):
     def agent_build_get_by_triple(self, context, hypervisor, os, architecture):
         return db.agent_build_get_by_triple(context,
                                             hypervisor, os, architecture)
+
+    def instance_type_get(self, context, instance_type_id):
+        return db.instance_type_get(context, instance_type_id)
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 5ba97a8a4..c5612e411 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1986,7 +1986,7 @@ class LibvirtDriver(driver.ComputeDriver):
 
         return cpu
 
-    def get_guest_disk_config(self, instance, name, disk_mapping,
+    def get_guest_disk_config(self, instance, name, disk_mapping, inst_type,
                               image_type=None):
         image = self.image_backend.image(instance,
                                          name,
@@ -1996,7 +1996,7 @@ class LibvirtDriver(driver.ComputeDriver):
                                   disk_info['dev'],
                                   disk_info['type'],
                                   self.disk_cachemode,
-                                  instance['extra_specs'])
+                                  inst_type['extra_specs'])
 
     def get_guest_storage_config(self, instance, image_meta,
                                  disk_info,
@@ -2019,24 +2019,28 @@ class LibvirtDriver(driver.ComputeDriver):
             if rescue:
                 diskrescue = self.get_guest_disk_config(instance,
                                                         'disk.rescue',
-                                                        disk_mapping)
+                                                        disk_mapping,
+                                                        inst_type)
                 devices.append(diskrescue)
 
                 diskos = self.get_guest_disk_config(instance,
                                                     'disk',
-                                                    disk_mapping)
+                                                    disk_mapping,
+                                                    inst_type)
                 devices.append(diskos)
             else:
                 if 'disk' in disk_mapping:
                     diskos = self.get_guest_disk_config(instance,
                                                         'disk',
-                                                        disk_mapping)
+                                                        disk_mapping,
+                                                        inst_type)
                     devices.append(diskos)
 
                 if 'disk.local' in disk_mapping:
                     disklocal = self.get_guest_disk_config(instance,
                                                            'disk.local',
-                                                           disk_mapping)
+                                                           disk_mapping,
+                                                           inst_type)
                     devices.append(disklocal)
                     self.virtapi.instance_update(
                         nova_context.get_admin_context(), instance['uuid'],
@@ -2048,13 +2052,14 @@ class LibvirtDriver(driver.ComputeDriver):
                     diskeph = self.get_guest_disk_config(
                         instance,
                         blockinfo.get_eph_disk(eph),
-                        disk_mapping)
+                        disk_mapping, inst_type)
                     devices.append(diskeph)
 
                 if 'disk.swap' in disk_mapping:
                     diskswap = self.get_guest_disk_config(instance,
                                                           'disk.swap',
-                                                          disk_mapping)
+                                                          disk_mapping,
+                                                          inst_type)
                     devices.append(diskswap)
                     self.virtapi.instance_update(
                         nova_context.get_admin_context(), instance['uuid'],
@@ -2072,6 +2077,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 diskconfig = self.get_guest_disk_config(instance,
                                                         'disk.config',
                                                         disk_mapping,
+                                                        inst_type,
                                                         'raw')
                 devices.append(diskconfig)
 
@@ -2100,7 +2106,10 @@ class LibvirtDriver(driver.ComputeDriver):
             'ramdisk_id' if a ramdisk is needed for the rescue image and
             'kernel_id' if a kernel is needed for the rescue image.
         """
-        inst_type = instance['instance_type']
+
+        inst_type = self.virtapi.instance_type_get(
+            nova_context.get_admin_context(read_deleted='yes'),
+            instance['instance_type_id'])
         inst_path = libvirt_utils.get_instance_path(instance)
         disk_mapping = disk_info['mapping']
 
@@ -2112,7 +2121,7 @@ class LibvirtDriver(driver.ComputeDriver):
         guest.vcpus = inst_type['vcpus']
 
         quota_items = ['cpu_shares', 'cpu_period', 'cpu_quota']
-        for key, value in instance['extra_specs'].iteritems():
+        for key, value in inst_type['extra_specs'].iteritems():
             if key in quota_items:
                 setattr(guest, key, value)
 
diff --git a/nova/virt/virtapi.py b/nova/virt/virtapi.py
index 53cbabc30..963b3c788 100644
--- a/nova/virt/virtapi.py
+++ b/nova/virt/virtapi.py
@@ -97,3 +97,10 @@ class VirtAPI(object):
         :param architecture: agent architecture
         """
         raise NotImplementedError()
+
+    def instance_type_get(self, context, instance_type_id):
+        """Get information about an instance type
+        :param context: security context
+        :param instance_type_id: the id of the instance type in question
+        """
+        raise NotImplementedError()