From 86e4587fb36aec74102d58c50d614fc6c006ebd3 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Tue, 5 Mar 2013 13:57:10 -0500 Subject: Remove uses of instance['instance_type'] from nova/compute Note that some tests were verifying things that can no longer fail, namely rebuilding instances who use flavors that have since been disabled. These tests are removed here. Also, this changes the order of a piece of logic in the resize/migrate path where before we would have ended up checking the instance type that we fetched from sys_meta to see if it was disabled (which makes no sense now). Reversing the check (to see if we're doing a resize or a migrate before checking the new flavor) eliminates the problem of the stashed type not having the 'disabled' attribute. This is one change in a series aimed at removing the use of instance-linked instance_type objects, in favor of the decoupled type data in system_metadata. See bug 1140119 for more details. Change-Id: I214a693e3bb16c0a365eb9b3afe73601beba4a22 --- nova/compute/api.py | 11 ++++---- nova/tests/api/openstack/fakes.py | 5 +++- nova/tests/compute/test_compute.py | 53 ++------------------------------------ nova/tests/test_utils.py | 10 +++++++ nova/utils.py | 7 +++++ 5 files changed, 29 insertions(+), 57 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 415162339..a1ead5163 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1234,7 +1234,7 @@ class API(base.Base): def restore(self, context, instance): """Restore a previously deleted (but not reclaimed) instance.""" # Reserve quotas - instance_type = instance['instance_type'] + instance_type = instance_types.extract_instance_type(instance) num_instances, quota_reservations = self._check_num_instances_quota( context, instance_type, 1, 1) @@ -1650,7 +1650,8 @@ class API(base.Base): #disk format of vhd is non-shrinkable if orig_image.get('disk_format') == 'vhd': - min_disk = instance['instance_type']['root_gb'] + instance_type = instance_types.extract_instance_type(instance) + min_disk = instance_type['root_gb'] else: #set new image values to the original image values min_disk = orig_image.get('min_disk') @@ -1741,7 +1742,7 @@ class API(base.Base): metadata = kwargs.get('metadata', {}) self._check_metadata_properties_quota(context, metadata) - instance_type = instance['instance_type'] + instance_type = instance_types.extract_instance_type(instance) if instance_type['memory_mb'] < int(image.get('min_ram') or 0): raise exception.InstanceTypeMemoryTooSmall() if instance_type['root_gb'] < int(image.get('min_disk') or 0): @@ -1952,7 +1953,7 @@ class API(base.Base): the original flavor_id. If flavor_id is not None, the instance should be migrated to a new host and resized to the new flavor_id. """ - current_instance_type = instance['instance_type'] + current_instance_type = instance_types.extract_instance_type(instance) # If flavor_id is not provided, only migrate the instance. if not flavor_id: @@ -1978,7 +1979,7 @@ class API(base.Base): # NOTE(sirp): We don't want to force a customer to change their flavor # when Ops is migrating off of a failed host. - if new_instance_type['disabled'] and not same_instance_type: + if not same_instance_type and new_instance_type['disabled']: raise exception.FlavorNotFound(flavor_id=flavor_id) # NOTE(markwash): look up the image early to avoid auth problems later diff --git a/nova/tests/api/openstack/fakes.py b/nova/tests/api/openstack/fakes.py index 1bc7b313e..97321212c 100644 --- a/nova/tests/api/openstack/fakes.py +++ b/nova/tests/api/openstack/fakes.py @@ -45,6 +45,7 @@ from nova.openstack.common import timeutils from nova import quota from nova.tests import fake_network from nova.tests.glance import stubs as glance_stubs +from nova import utils from nova import wsgi @@ -434,6 +435,7 @@ def stub_instance(id, user_id=None, project_id=None, host=None, metadata = [] inst_type = instance_types.get_instance_type_by_flavor_id(int(flavor_id)) + sys_meta = instance_types.save_instance_type_info({}, inst_type) if host is not None: host = str(host) @@ -497,7 +499,8 @@ def stub_instance(id, user_id=None, project_id=None, host=None, "shutdown_terminate": True, "disable_terminate": False, "security_groups": security_groups, - "root_device_name": root_device_name} + "root_device_name": root_device_name, + "system_metadata": utils.dict_to_metadata(sys_meta)} instance.update(info_cache) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index c86992f3d..9c01906b2 100644 --- a/nova/tests/compute/test_compute.py +++ b/nova/tests/compute/test_compute.py @@ -4791,7 +4791,7 @@ class ComputeAPITestCase(BaseTestCase): {'extra_param': 'value1'}) self.assertEqual(image['name'], 'snap1') - instance_type = instance['instance_type'] + instance_type = instance_types.extract_instance_type(instance) self.assertEqual(image['min_ram'], self.fake_image['min_ram']) self.assertEqual(image['min_disk'], instance_type['root_gb']) properties = image['properties'] @@ -5187,7 +5187,7 @@ 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) - orig_instance_type = instance['instance_type'] + orig_instance_type = instance_types.extract_instance_type(instance) self.compute.run_instance(self.context, instance=instance) # We need to set the host to something 'known'. Unfortunately, # the compute manager is using a cached copy of CONF.host, @@ -7022,33 +7022,6 @@ class DisabledInstanceTypesTestCase(BaseTestCase): self.assertRaises(exception.InstanceTypeNotFound, self.compute_api.create, self.context, self.inst_type, None) - def test_can_rebuild_instance_from_visible_instance_type(self): - instance = self._create_fake_instance() - image_href = 'fake-image-id' - admin_password = 'blah' - - instance['instance_type']['disabled'] = True - - # Assert no errors were raised - self.compute_api.rebuild(self.context, instance, image_href, - admin_password) - - def test_can_rebuild_instance_from_disabled_instance_type(self): - """ - A rebuild or a restore should only change the 'image', - not the 'instance_type'. Therefore, should be allowed even - when the slice is on disabled type already. - """ - instance = self._create_fake_instance() - image_href = 'fake-image-id' - admin_password = 'blah' - - instance['instance_type']['disabled'] = True - - # Assert no errors were raised - self.compute_api.rebuild(self.context, instance, image_href, - admin_password) - def test_can_resize_to_visible_instance_type(self): instance = self._create_fake_instance() orig_get_instance_type_by_flavor_id =\ @@ -7092,28 +7065,6 @@ class DisabledInstanceTypesTestCase(BaseTestCase): self.assertRaises(exception.FlavorNotFound, self.compute_api.resize, self.context, instance, '4') - def test_can_migrate_to_visible_instance_type(self): - instance = self._create_fake_instance() - instance['instance_type']['disabled'] = False - - # FIXME(sirp): for legacy this raises FlavorNotFound instead of - # InstanceTypeNotFound; we should eventually make it raise - # InstanceTypeNotFound for consistency. - self.compute_api.resize(self.context, instance, None) - - def test_can_migrate_to_disabled_instance_type(self): - """ - We don't want to require a customers instance-type to change when ops - is migrating a failed server. - """ - instance = self._create_fake_instance() - instance['instance_type']['disabled'] = True - - # FIXME(sirp): for legacy this raises FlavorNotFound instead of - # InstanceTypeNotFound; we should eventually make it raise - # InstanceTypeNotFound for consistency. - self.compute_api.resize(self.context, instance, None) - class ComputeReschedulingTestCase(BaseTestCase): """Tests re-scheduling logic for new build requests.""" diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py index d22fb12ea..c601bb0af 100644 --- a/nova/tests/test_utils.py +++ b/nova/tests/test_utils.py @@ -916,6 +916,16 @@ class MetadataToDictTestCase(test.TestCase): def test_metadata_to_dict_empty(self): self.assertEqual(utils.metadata_to_dict([]), {}) + def test_dict_to_metadata(self): + expected = [{'key': 'foo1', 'value': 'bar1'}, + {'key': 'foo2', 'value': 'bar2'}] + self.assertEqual(utils.dict_to_metadata(dict(foo1='bar1', + foo2='bar2')), + expected) + + def test_dict_to_metadata_empty(self): + self.assertEqual(utils.dict_to_metadata({}), []) + class WrappedCodeTestCase(test.TestCase): """Test the get_wrapped_function utility method.""" diff --git a/nova/utils.py b/nova/utils.py index eb6c44106..fe6c75df3 100644 --- a/nova/utils.py +++ b/nova/utils.py @@ -1362,6 +1362,13 @@ def metadata_to_dict(metadata): return result +def dict_to_metadata(metadata): + result = [] + for key, value in metadata.iteritems(): + result.append(dict(key=key, value=value)) + return result + + def get_wrapped_function(function): """Get the method at the bottom of a stack of decorators.""" if not hasattr(function, 'func_closure') or not function.func_closure: -- cgit