From cb338cb7692e12cc94515f1f09008d0e328c1505 Mon Sep 17 00:00:00 2001 From: melanie witt Date: Thu, 28 Aug 2014 17:39:10 +0000 Subject: [PATCH] object-ify flavors api and compute/api side of RPC This change converts flavors to use objects instead of direct database access. The rpcapi primitivizes objects before they go over the wire. The flavors.add_flavor_access() and flavors.remove_flavor_access() functions were also removed as flavor object add_access() and remove_access() should be used instead. Related to blueprint kilo-objects Change-Id: I86432720b600e479d8d8b7f7531a2c85ab364319 --- nova/cells/rpcapi.py | 4 + nova/compute/flavors.py | 74 +++----- nova/compute/resource_tracker.py | 5 +- nova/compute/rpcapi.py | 4 + nova/conductor/rpcapi.py | 4 + nova/scheduler/rpcapi.py | 4 + nova/scheduler/utils.py | 9 +- .../compute/contrib/test_flavor_manage.py | 2 - nova/tests/unit/compute/test_compute_api.py | 56 +++--- nova/tests/unit/conductor/test_conductor.py | 14 +- .../unit/scheduler/test_scheduler_utils.py | 7 +- nova/tests/unit/test_flavors.py | 174 +++++++----------- 12 files changed, 156 insertions(+), 201 deletions(-) diff --git a/nova/cells/rpcapi.py b/nova/cells/rpcapi.py index 6d1d69a110bc..09c51c16f650 100644 --- a/nova/cells/rpcapi.py +++ b/nova/cells/rpcapi.py @@ -150,6 +150,10 @@ class CellsAPI(object): build_inst_kwargs['instances'] = instances_p build_inst_kwargs['image'] = jsonutils.to_primitive( build_inst_kwargs['image']) + if 'filter_properties' in build_inst_kwargs: + flavor = build_inst_kwargs['filter_properties']['instance_type'] + flavor_p = objects_base.obj_to_primitive(flavor) + build_inst_kwargs['filter_properties']['instance_type'] = flavor_p cctxt = self.client.prepare(version='1.8') cctxt.cast(ctxt, 'build_instances', build_inst_kwargs=build_inst_kwargs) diff --git a/nova/compute/flavors.py b/nova/compute/flavors.py index 10ae4484c87a..4257053bf93f 100644 --- a/nova/compute/flavors.py +++ b/nova/compute/flavors.py @@ -22,7 +22,6 @@ import re import uuid from oslo.config import cfg -from oslo.db import exception as db_exc from oslo.utils import strutils import six @@ -31,6 +30,7 @@ from nova import db from nova import exception from nova.i18n import _ from nova.i18n import _LE +from nova import objects from nova.openstack.common import log as logging from nova import utils @@ -170,11 +170,9 @@ def create(name, memory, vcpus, root_gb, ephemeral_gb=0, flavorid=None, except ValueError: raise exception.InvalidInput(reason=_("is_public must be a boolean")) - try: - return db.flavor_create(context.get_admin_context(), kwargs) - except db_exc.DBError as e: - LOG.exception(_LE('DB error: %s'), e) - raise exception.FlavorCreateFailed() + flavor = objects.Flavor(context=context.get_admin_context(), **kwargs) + flavor.create() + return flavor def destroy(name): @@ -182,7 +180,9 @@ def destroy(name): try: if not name: raise ValueError() - db.flavor_destroy(context.get_admin_context(), name) + flavor = objects.Flavor(name=name) + ctxt = context.get_admin_context() + flavor.destroy(ctxt) except (ValueError, exception.NotFound): LOG.exception(_LE('Instance type %s not found for deletion'), name) raise exception.FlavorNotFoundByName(flavor_name=name) @@ -191,32 +191,30 @@ def destroy(name): def get_all_flavors(ctxt=None, inactive=False, filters=None): """Get all non-deleted flavors as a dict. - Pass true as argument if you want deleted flavors returned also. + Pass inactive=True if you want deleted flavors returned also. """ if ctxt is None: ctxt = context.get_admin_context() - inst_types = db.flavor_get_all( - ctxt, inactive=inactive, filters=filters) + inst_types = objects.FlavorList.get_all(ctxt, inactive=inactive, + filters=filters) inst_type_dict = {} for inst_type in inst_types: - inst_type_dict[inst_type['id']] = inst_type + inst_type_dict[inst_type.id] = inst_type return inst_type_dict -def get_all_flavors_sorted_list(ctxt=None, inactive=False, filters=None, - sort_key='flavorid', sort_dir='asc', - limit=None, marker=None): +def get_all_flavors_sorted_list(ctxt=None, filters=None, sort_key='flavorid', + sort_dir='asc', limit=None, marker=None): """Get all non-deleted flavors as a sorted list. - - Pass true as argument if you want deleted flavors returned also. """ if ctxt is None: ctxt = context.get_admin_context() - return db.flavor_get_all(ctxt, filters=filters, sort_key=sort_key, - sort_dir=sort_dir, limit=limit, marker=marker) + return objects.FlavorList.get_all(ctxt, filters=filters, sort_key=sort_key, + sort_dir=sort_dir, limit=limit, + marker=marker) def get_default_flavor(): @@ -236,7 +234,7 @@ def get_flavor(instance_type_id, ctxt=None, inactive=False): if inactive: ctxt = ctxt.elevated(read_deleted="yes") - return db.flavor_get(ctxt, instance_type_id) + return objects.Flavor.get_by_id(ctxt, instance_type_id) def get_flavor_by_name(name, ctxt=None): @@ -247,7 +245,7 @@ def get_flavor_by_name(name, ctxt=None): if ctxt is None: ctxt = context.get_admin_context() - return db.flavor_get_by_name(ctxt, name) + return objects.Flavor.get_by_name(ctxt, name) # TODO(termie): flavor-specific code should probably be in the API that uses @@ -260,8 +258,7 @@ def get_flavor_by_flavor_id(flavorid, ctxt=None, read_deleted="yes"): if ctxt is None: ctxt = context.get_admin_context(read_deleted=read_deleted) - # NOTE(melwitt): return a copy temporarily until conversion to object - return dict(db.flavor_get_by_flavor_id(ctxt, flavorid, read_deleted)) + return objects.Flavor.get_by_flavor_id(ctxt, flavorid, read_deleted) def get_flavor_access_by_flavor_id(flavorid, ctxt=None): @@ -269,35 +266,20 @@ def get_flavor_access_by_flavor_id(flavorid, ctxt=None): if ctxt is None: ctxt = context.get_admin_context() - return db.flavor_access_get_by_flavor_id(ctxt, flavorid) - - -def add_flavor_access(flavorid, projectid, ctxt=None): - """Add flavor access for project.""" - if ctxt is None: - ctxt = context.get_admin_context() - - return db.flavor_access_add(ctxt, flavorid, projectid) - - -def remove_flavor_access(flavorid, projectid, ctxt=None): - """Remove flavor access for project.""" - if ctxt is None: - ctxt = context.get_admin_context() - - return db.flavor_access_remove(ctxt, flavorid, projectid) + flavor = objects.Flavor.get_by_flavor_id(ctxt, flavorid) + return flavor.projects def extract_flavor(instance, prefix=''): - """Create an InstanceType-like object from instance's system_metadata + """Create a Flavor object from instance's system_metadata information. """ - instance_type = {} + flavor = objects.Flavor() sys_meta = utils.instance_sys_meta(instance) - for key, type_fn in system_metadata_flavor_props.items(): + for key in system_metadata_flavor_props.keys(): type_key = '%sinstance_type_%s' % (prefix, key) - instance_type[key] = type_fn(sys_meta[type_key]) + setattr(flavor, key, sys_meta[type_key]) # NOTE(danms): We do NOT save all of extra_specs, but only the # NUMA-related ones that we need to avoid an uglier alternative. This @@ -306,12 +288,12 @@ def extract_flavor(instance, prefix=''): extra_specs = [(k, v) for k, v in sys_meta.items() if k.startswith('%sinstance_type_extra_' % prefix)] if extra_specs: - instance_type['extra_specs'] = {} + flavor.extra_specs = {} for key, value in extra_specs: extra_key = key[len('%sinstance_type_extra_' % prefix):] - instance_type['extra_specs'][extra_key] = value + flavor.extra_specs[extra_key] = value - return instance_type + return flavor def save_flavor_info(metadata, instance_type, prefix=''): diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 93246af6c7d4..423143d3d416 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -782,10 +782,7 @@ class ResourceTracker(object): if not instance_type_id: instance_type_id = instance['instance_type_id'] return objects.Flavor.get_by_id(context, instance_type_id) - # NOTE (ndipanov): Make sure we don't try to lazy-load extra_specs - # from the object, if there were none stashed in system_metadata - extracted_flavor.setdefault('extra_specs', {}) - return objects.Flavor(context, **extracted_flavor) + return extracted_flavor def _get_usage_dict(self, object_or_dict, **updates): """Make a usage dict _update methods expect. diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index a6c49f861ea3..a2c5b62b2602 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -887,6 +887,10 @@ class ComputeAPI(object): requested_networks = [(network_id, address, port_id) for (network_id, address, port_id, _) in requested_networks.as_tuples()] + if 'instance_type' in filter_properties: + flavor = filter_properties['instance_type'] + flavor_p = objects_base.obj_to_primitive(flavor) + filter_properties = dict(filter_properties, instance_type=flavor_p) cctxt = self.client.prepare(server=host, version=version) cctxt.cast(ctxt, 'build_and_run_instance', instance=instance, diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index 08de767a4ee0..1be17e4edec7 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -417,6 +417,10 @@ class ComputeTaskAPI(object): admin_password, injected_files, requested_networks, security_groups, block_device_mapping, legacy_bdm=True): image_p = jsonutils.to_primitive(image) + if 'instance_type' in filter_properties: + flavor = filter_properties['instance_type'] + flavor_p = objects_base.obj_to_primitive(flavor) + filter_properties = dict(filter_properties, instance_type=flavor_p) kw = {'instances': instances, 'image': image_p, 'filter_properties': filter_properties, 'admin_password': admin_password, diff --git a/nova/scheduler/rpcapi.py b/nova/scheduler/rpcapi.py index cab09631fddc..131bbe76e544 100644 --- a/nova/scheduler/rpcapi.py +++ b/nova/scheduler/rpcapi.py @@ -103,6 +103,10 @@ class SchedulerAPI(object): serializer=serializer) def select_destinations(self, ctxt, request_spec, filter_properties): + if 'instance_type' in filter_properties: + flavor = filter_properties['instance_type'] + flavor_p = objects_base.obj_to_primitive(flavor) + filter_properties = dict(filter_properties, instance_type=flavor_p) cctxt = self.client.prepare() return cctxt.call(ctxt, 'select_destinations', request_spec=request_spec, filter_properties=filter_properties) diff --git a/nova/scheduler/utils.py b/nova/scheduler/utils.py index 1a5f3cfa39a1..5ff7ada8ebed 100644 --- a/nova/scheduler/utils.py +++ b/nova/scheduler/utils.py @@ -21,7 +21,6 @@ from oslo.serialization import jsonutils from nova.compute import flavors from nova.compute import utils as compute_utils -from nova import db from nova import exception from nova.i18n import _, _LE, _LW from nova import notifications @@ -57,10 +56,10 @@ def build_request_spec(ctxt, image, instances, instance_type=None): if instance_type is None: instance_type = flavors.extract_flavor(instance) - # NOTE(comstud): This is a bit ugly, but will get cleaned up when - # we're passing an InstanceType internal object. - extra_specs = db.flavor_extra_specs_get(ctxt, instance_type['flavorid']) - instance_type['extra_specs'] = extra_specs + + if isinstance(instance_type, objects.Flavor): + instance_type = obj_base.obj_to_primitive(instance_type) + request_spec = { 'image': image or {}, 'instance_properties': instance, diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_flavor_manage.py b/nova/tests/unit/api/openstack/compute/contrib/test_flavor_manage.py index 3d44e4970b10..20eb575d55dc 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_flavor_manage.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_flavor_manage.py @@ -425,8 +425,6 @@ class PrivateFlavorManageTestV21(test.TestCase): def test_create_public_flavor_should_not_create_flavor_access(self): self.expected["flavor"]["os-flavor-access:is_public"] = True - self.mox.StubOutWithMock(flavors, "add_flavor_access") - self.mox.ReplayAll() body = self._get_response() for key in self.expected["flavor"]: self.assertEqual(body["flavor"][key], self.expected["flavor"][key]) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 10ac29d3dd09..46390dd92b72 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -77,7 +77,7 @@ class _ComputeAPIUnitTestMixIn(object): exclude_states = set() return vm_state - exclude_states - def _create_flavor(self, params=None): + def _create_flavor(self, **updates): flavor = {'id': 1, 'flavorid': 1, 'name': 'm1.tiny', @@ -91,10 +91,15 @@ class _ComputeAPIUnitTestMixIn(object): 'deleted': 0, 'disabled': False, 'is_public': True, + 'deleted_at': None, + 'created_at': datetime.datetime(2012, 1, 19, 18, + 49, 30, 877329), + 'updated_at': None, } - if params: - flavor.update(params) - return flavor + if updates: + flavor.update(updates) + return objects.Flavor._from_db_object(self.context, objects.Flavor(), + flavor) def _create_instance_obj(self, params=None, flavor=None): """Create a test instance.""" @@ -129,7 +134,7 @@ class _ComputeAPIUnitTestMixIn(object): instance.project_id = self.project_id instance.host = 'fake_host' instance.node = NODENAME - instance.instance_type_id = flavor['id'] + instance.instance_type_id = flavor.id instance.ami_launch_index = 0 instance.memory_mb = 0 instance.vcpus = 0 @@ -516,10 +521,9 @@ class _ComputeAPIUnitTestMixIn(object): self.context, objects.Migration(), fake_db_migration) inst.instance_type_id = migration.new_instance_type_id - old_flavor = {'vcpus': 1, - 'memory_mb': 512} - deltas['cores'] = -old_flavor['vcpus'] - deltas['ram'] = -old_flavor['memory_mb'] + old_flavor = self._create_flavor(vcpus=1, memory_mb=512) + deltas['cores'] = -old_flavor.vcpus + deltas['ram'] = -old_flavor.memory_mb self.mox.StubOutWithMock(objects.Migration, 'get_by_instance_and_status') @@ -1220,11 +1224,11 @@ class _ComputeAPIUnitTestMixIn(object): current_flavor = flavors.extract_flavor(fake_inst) if flavor_id_passed: - new_flavor = dict(id=200, flavorid='new-flavor-id', - name='new_flavor', disabled=False) + new_flavor = self._create_flavor(id=200, flavorid='new-flavor-id', + name='new_flavor', disabled=False) if same_flavor: cur_flavor = flavors.extract_flavor(fake_inst) - new_flavor['id'] = cur_flavor['id'] + new_flavor.id = cur_flavor.id flavors.get_flavor_by_flavor_id( 'new-flavor-id', read_deleted='no').AndReturn(new_flavor) @@ -1240,8 +1244,8 @@ class _ComputeAPIUnitTestMixIn(object): resvs) self.compute_api._upsize_quota_delta( - self.context, new_flavor, - current_flavor).AndReturn('deltas') + self.context, mox.IsA(objects.Flavor), + mox.IsA(objects.Flavor)).AndReturn('deltas') self.compute_api._reserve_quota_delta(self.context, 'deltas', fake_inst).AndReturn(fake_quotas) @@ -1274,9 +1278,9 @@ class _ComputeAPIUnitTestMixIn(object): def _check_mig(ctxt): self.assertEqual(fake_inst.uuid, mig.instance_uuid) - self.assertEqual(current_flavor['id'], + self.assertEqual(current_flavor.id, mig.old_instance_type_id) - self.assertEqual(new_flavor['id'], + self.assertEqual(new_flavor.id, mig.new_instance_type_id) self.assertEqual('finished', mig.status) @@ -1299,7 +1303,8 @@ class _ComputeAPIUnitTestMixIn(object): self.compute_api.compute_task_api.resize_instance( self.context, fake_inst, extra_kwargs, scheduler_hint=scheduler_hint, - flavor=new_flavor, reservations=expected_reservations) + flavor=mox.IsA(objects.Flavor), + reservations=expected_reservations) self.mox.ReplayAll() @@ -1376,8 +1381,8 @@ class _ComputeAPIUnitTestMixIn(object): 'resize_instance') fake_inst = obj_base.obj_to_primitive(self._create_instance_obj()) - fake_flavor = dict(id=200, flavorid='flavor-id', name='foo', - disabled=True) + fake_flavor = self._create_flavor(id=200, flavorid='flavor-id', + name='foo', disabled=True) flavors.get_flavor_by_flavor_id( 'flavor-id', read_deleted='no').AndReturn(fake_flavor) @@ -1391,8 +1396,8 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(flavors, 'get_flavor_by_flavor_id') def test_resize_to_zero_disk_flavor_fails(self, get_flavor_by_flavor_id): fake_inst = self._create_instance_obj() - fake_flavor = dict(id=200, flavorid='flavor-id', name='foo', - root_gb=0) + fake_flavor = self._create_flavor(id=200, flavorid='flavor-id', + name='foo', root_gb=0) get_flavor_by_flavor_id.return_value = fake_flavor @@ -1412,15 +1417,14 @@ class _ComputeAPIUnitTestMixIn(object): 'resize_instance') fake_inst = obj_base.obj_to_primitive(self._create_instance_obj()) - current_flavor = flavors.extract_flavor(fake_inst) - fake_flavor = dict(id=200, flavorid='flavor-id', name='foo', - disabled=False) + fake_flavor = self._create_flavor(id=200, flavorid='flavor-id', + name='foo', disabled=False) flavors.get_flavor_by_flavor_id( 'flavor-id', read_deleted='no').AndReturn(fake_flavor) deltas = dict(resource=0) self.compute_api._upsize_quota_delta( - self.context, fake_flavor, - current_flavor).AndReturn(deltas) + self.context, mox.IsA(objects.Flavor), + mox.IsA(objects.Flavor)).AndReturn(deltas) usage = dict(in_use=0, reserved=0) quotas = {'resource': 0} usages = {'resource': usage} diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 0570ada2174a..7e7e6d002dcb 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1192,7 +1192,7 @@ class _BaseTaskTestCase(object): inst = fake_instance.fake_db_instance(image_ref='image_ref') inst_obj = objects.Instance._from_db_object( self.context, objects.Instance(), inst, []) - flavor = flavors.get_default_flavor() + flavor = obj_base.obj_to_primitive(flavors.get_default_flavor()) flavor['extra_specs'] = 'extra_specs' request_spec = {'instance_type': flavor, 'instance_properties': {}} @@ -1242,10 +1242,9 @@ class _BaseTaskTestCase(object): system_metadata=system_metadata, expected_attrs=['system_metadata']) for i in xrange(2)] instance_type = flavors.extract_flavor(instances[0]) - instance_type['extra_specs'] = 'fake-specs' + instance_type_p = jsonutils.to_primitive(instance_type) instance_properties = jsonutils.to_primitive(instances[0]) - self.mox.StubOutWithMock(db, 'flavor_extra_specs_get') self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group') self.mox.StubOutWithMock(self.conductor_manager.scheduler_client, 'select_destinations') @@ -1255,15 +1254,12 @@ class _BaseTaskTestCase(object): self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi, 'build_and_run_instance') - db.flavor_extra_specs_get( - self.context, - instance_type['flavorid']).AndReturn('fake-specs') scheduler_utils.setup_instance_group(self.context, None, None) self.conductor_manager.scheduler_client.select_destinations( self.context, {'image': {'fake_data': 'should_pass_silently'}, 'instance_properties': jsonutils.to_primitive( instances[0]), - 'instance_type': instance_type, + 'instance_type': instance_type_p, 'instance_uuids': [inst.uuid for inst in instances], 'num_instances': 2}, {'retry': {'num_attempts': 1, 'hosts': []}}).AndReturn( @@ -1283,7 +1279,7 @@ class _BaseTaskTestCase(object): request_spec={ 'image': {'fake_data': 'should_pass_silently'}, 'instance_properties': instance_properties, - 'instance_type': instance_type, + 'instance_type': instance_type_p, 'instance_uuids': [inst.uuid for inst in instances], 'num_instances': 2}, filter_properties={'retry': {'num_attempts': 1, @@ -1309,7 +1305,7 @@ class _BaseTaskTestCase(object): request_spec={ 'image': {'fake_data': 'should_pass_silently'}, 'instance_properties': instance_properties, - 'instance_type': instance_type, + 'instance_type': instance_type_p, 'instance_uuids': [inst.uuid for inst in instances], 'num_instances': 2}, filter_properties={'limits': [], diff --git a/nova/tests/unit/scheduler/test_scheduler_utils.py b/nova/tests/unit/scheduler/test_scheduler_utils.py index 0dfade7deb8f..f690aac3753d 100644 --- a/nova/tests/unit/scheduler/test_scheduler_utils.py +++ b/nova/tests/unit/scheduler/test_scheduler_utils.py @@ -48,9 +48,7 @@ class SchedulerUtilsTestCase(test.NoDBTestCase): instance_type = {'flavorid': 'fake-id'} self.mox.StubOutWithMock(flavors, 'extract_flavor') - self.mox.StubOutWithMock(db, 'flavor_extra_specs_get') flavors.extract_flavor(mox.IgnoreArg()).AndReturn(instance_type) - db.flavor_extra_specs_get(self.context, mox.IgnoreArg()).AndReturn([]) self.mox.ReplayAll() request_spec = scheduler_utils.build_request_spec(self.context, image, @@ -58,14 +56,11 @@ class SchedulerUtilsTestCase(test.NoDBTestCase): self.assertEqual({}, request_spec['image']) @mock.patch.object(flavors, 'extract_flavor') - @mock.patch.object(db, 'flavor_extra_specs_get') - def test_build_request_spec_with_object(self, flavor_extra_specs_get, - extract_flavor): + def test_build_request_spec_with_object(self, extract_flavor): instance_type = {'flavorid': 'fake-id'} instance = fake_instance.fake_instance_obj(self.context) extract_flavor.return_value = instance_type - flavor_extra_specs_get.return_value = [] request_spec = scheduler_utils.build_request_spec(self.context, None, [instance]) diff --git a/nova/tests/unit/test_flavors.py b/nova/tests/unit/test_flavors.py index 46fd81d6dbc0..2b47e1eca1ed 100644 --- a/nova/tests/unit/test_flavors.py +++ b/nova/tests/unit/test_flavors.py @@ -21,6 +21,8 @@ from nova import db from nova.db.sqlalchemy import api as sql_session from nova.db.sqlalchemy import models from nova import exception +from nova import objects +from nova.objects import base as obj_base from nova import test @@ -67,7 +69,7 @@ class InstanceTypeTestCase(test.TestCase): def _generate_flavorid(self): """return a flavorid not in the DB.""" nonexistent_flavor = 2700 - flavor_ids = [value["id"] for key, value in + flavor_ids = [value.id for key, value in flavors.get_all_flavors().iteritems()] while nonexistent_flavor in flavor_ids: nonexistent_flavor += 1 @@ -78,62 +80,6 @@ class InstanceTypeTestCase(test.TestCase): """return first flavor name.""" return flavors.get_all_flavors().keys()[0] - def test_add_instance_type_access(self): - user_id = 'fake' - project_id = 'fake' - ctxt = context.RequestContext(user_id, project_id, is_admin=True) - flavor_id = 'flavor1' - type_ref = flavors.create('some flavor', 256, 1, 120, 100, - flavorid=flavor_id) - access_ref = flavors.add_flavor_access(flavor_id, - project_id, - ctxt=ctxt) - self.assertEqual(access_ref["project_id"], project_id) - self.assertEqual(access_ref["instance_type_id"], type_ref["id"]) - - def test_add_flavor_access_already_exists(self): - user_id = 'fake' - project_id = 'fake' - ctxt = context.RequestContext(user_id, project_id, is_admin=True) - flavor_id = 'flavor1' - flavors.create('some flavor', 256, 1, 120, 100, flavorid=flavor_id) - flavors.add_flavor_access(flavor_id, project_id, ctxt=ctxt) - self.assertRaises(exception.FlavorAccessExists, - flavors.add_flavor_access, - flavor_id, project_id, ctxt) - - def test_add_flavor_access_invalid_flavor(self): - user_id = 'fake' - project_id = 'fake' - ctxt = context.RequestContext(user_id, project_id, is_admin=True) - flavor_id = 'no_such_flavor' - self.assertRaises(exception.FlavorNotFound, - flavors.add_flavor_access, - flavor_id, project_id, ctxt) - - def test_remove_flavor_access(self): - user_id = 'fake' - project_id = 'fake' - ctxt = context.RequestContext(user_id, project_id, is_admin=True) - flavor_id = 'flavor1' - flavors.create('some flavor', 256, 1, 120, 100, flavorid=flavor_id) - flavors.add_flavor_access(flavor_id, project_id, ctxt) - flavors.remove_flavor_access(flavor_id, project_id, ctxt) - - projects = flavors.get_flavor_access_by_flavor_id(flavor_id, - ctxt) - self.assertEqual([], projects) - - def test_remove_flavor_access_does_not_exist(self): - user_id = 'fake' - project_id = 'fake' - ctxt = context.RequestContext(user_id, project_id, is_admin=True) - flavor_id = 'flavor1' - flavors.create('some flavor', 256, 1, 120, 100, flavorid=flavor_id) - self.assertRaises(exception.FlavorAccessNotFound, - flavors.remove_flavor_access, - flavor_id, project_id, ctxt=ctxt) - def test_get_all_instance_types(self): # Ensures that all flavors can be retrieved. session = sql_session.get_session() @@ -160,9 +106,10 @@ class InstanceTypeTestCase(test.TestCase): def test_will_get_flavor_by_id(self): default_instance_type = flavors.get_default_flavor() - instance_type_id = default_instance_type['id'] + instance_type_id = default_instance_type.id fetched = flavors.get_flavor(instance_type_id) - self.assertEqual(default_instance_type, fetched) + self.assertIsInstance(fetched, objects.Flavor) + self.assertEqual(default_instance_type.flavorid, fetched.flavorid) def test_will_not_get_flavor_by_unknown_id(self): # Ensure get by name returns default flavor with no name. @@ -178,7 +125,9 @@ class InstanceTypeTestCase(test.TestCase): # Ensure get by name returns default flavor with no name. default = flavors.get_default_flavor() actual = flavors.get_flavor_by_name(None) - self.assertEqual(default, actual) + self.assertIsInstance(default, objects.Flavor) + self.assertIsInstance(actual, objects.Flavor) + self.assertEqual(default.flavorid, actual.flavorid) def test_will_not_get_flavor_with_bad_name(self): # Ensure get by name returns default flavor with bad name. @@ -193,9 +142,10 @@ class InstanceTypeTestCase(test.TestCase): def test_will_get_instance_by_flavor_id(self): default_instance_type = flavors.get_default_flavor() - flavorid = default_instance_type['flavorid'] + flavorid = default_instance_type.flavorid fetched = flavors.get_flavor_by_flavor_id(flavorid) - self.assertEqual(default_instance_type, fetched) + self.assertIsInstance(fetched, objects.Flavor) + self.assertEqual(default_instance_type.flavorid, fetched.flavorid) def test_can_read_deleted_types_using_flavor_id(self): # Ensure deleted flavors can be read when querying flavor_id. @@ -204,15 +154,15 @@ class InstanceTypeTestCase(test.TestCase): inst_type = flavors.create(inst_type_name, 256, 1, 120, 100, inst_type_flavor_id) - self.assertEqual(inst_type_name, inst_type["name"]) + self.assertEqual(inst_type_name, inst_type.name) # NOTE(jk0): The deleted flavor will show up here because the context # in get_flavor_by_flavor_id() is set to use read_deleted by # default. - flavors.destroy(inst_type["name"]) + flavors.destroy(inst_type.name) deleted_inst_type = flavors.get_flavor_by_flavor_id( inst_type_flavor_id) - self.assertEqual(inst_type_name, deleted_inst_type["name"]) + self.assertEqual(inst_type_name, deleted_inst_type.name) def test_read_deleted_false_converting_flavorid(self): """Ensure deleted flavors are not returned when not needed (for @@ -225,17 +175,25 @@ class InstanceTypeTestCase(test.TestCase): instance_type = flavors.get_flavor_by_flavor_id( "test1", read_deleted="no") - self.assertEqual("instance_type1_redo", instance_type["name"]) + self.assertEqual("instance_type1_redo", instance_type.name) def test_get_all_flavors_sorted_list_sort(self): # Test default sort all_flavors = flavors.get_all_flavors_sorted_list() - self.assertEqual(DEFAULT_FLAVORS, all_flavors) + self.assertEqual(len(DEFAULT_FLAVORS), len(all_flavors)) + for i in range(len(all_flavors)): + f = all_flavors[i] + self.assertIsInstance(f, objects.Flavor) + self.assertEqual(DEFAULT_FLAVORS[i]['flavorid'], f.flavorid) # Test sorted by name all_flavors = flavors.get_all_flavors_sorted_list(sort_key='name') expected = sorted(DEFAULT_FLAVORS, key=lambda item: item['name']) - self.assertEqual(expected, all_flavors) + self.assertEqual(len(expected), len(all_flavors)) + for i in range(len(all_flavors)): + f = all_flavors[i] + self.assertIsInstance(f, objects.Flavor) + self.assertEqual(expected[i]['flavorid'], f.flavorid) def test_get_all_flavors_sorted_list_limit(self): limited_flavors = flavors.get_all_flavors_sorted_list(limit=2) @@ -245,12 +203,17 @@ class InstanceTypeTestCase(test.TestCase): all_flavors = flavors.get_all_flavors_sorted_list() # Set the 3rd result as the marker - marker_flavorid = all_flavors[2]['flavorid'] + marker_flavorid = all_flavors[2].flavorid marked_flavors = flavors.get_all_flavors_sorted_list( marker=marker_flavorid) # We expect everything /after/ the 3rd result expected_results = all_flavors[3:] - self.assertEqual(expected_results, marked_flavors) + self.assertEqual(len(expected_results), len(marked_flavors)) + for i in range(len(marked_flavors)): + f = marked_flavors[i] + self.assertIsInstance(f, objects.Flavor) + self.assertEqual(expected_results[i].flavorid, + f.flavorid) def test_get_inactive_flavors(self): flav1 = flavors.create('flavor1', 256, 1, 120) @@ -258,12 +221,12 @@ class InstanceTypeTestCase(test.TestCase): flavors.destroy('flavor1') returned_flavors_ids = flavors.get_all_flavors().keys() - self.assertNotIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertNotIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) returned_flavors_ids = flavors.get_all_flavors(inactive=True).keys() - self.assertIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) def test_get_inactive_flavors_with_same_name(self): flav1 = flavors.create('flavor', 256, 1, 120) @@ -271,12 +234,12 @@ class InstanceTypeTestCase(test.TestCase): flav2 = flavors.create('flavor', 512, 4, 250) returned_flavors_ids = flavors.get_all_flavors().keys() - self.assertNotIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertNotIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) returned_flavors_ids = flavors.get_all_flavors(inactive=True).keys() - self.assertIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) def test_get_inactive_flavors_with_same_flavorid(self): flav1 = flavors.create('flavor', 256, 1, 120, 100, "flavid") @@ -284,12 +247,12 @@ class InstanceTypeTestCase(test.TestCase): flav2 = flavors.create('flavor', 512, 4, 250, 100, "flavid") returned_flavors_ids = flavors.get_all_flavors().keys() - self.assertNotIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertNotIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) returned_flavors_ids = flavors.get_all_flavors(inactive=True).keys() - self.assertIn(flav1['id'], returned_flavors_ids) - self.assertIn(flav2['id'], returned_flavors_ids) + self.assertIn(flav1.id, returned_flavors_ids) + self.assertIn(flav2.id, returned_flavors_ids) class InstanceTypeToolsTest(test.TestCase): @@ -298,19 +261,20 @@ class InstanceTypeToolsTest(test.TestCase): def _test_extract_flavor(self, prefix): instance_type = flavors.get_default_flavor() + instance_type_p = obj_base.obj_to_primitive(instance_type) metadata = {} - flavors.save_flavor_info(metadata, instance_type, - prefix) + flavors.save_flavor_info(metadata, instance_type, prefix) instance = {'system_metadata': self._dict_to_metadata(metadata)} _instance_type = flavors.extract_flavor(instance, prefix) + _instance_type_p = obj_base.obj_to_primitive(_instance_type) props = flavors.system_metadata_flavor_props.keys() - for key in instance_type.keys(): + for key in instance_type_p.keys(): if key not in props: - del instance_type[key] + del instance_type_p[key] - self.assertEqual(instance_type, _instance_type) + self.assertEqual(instance_type_p, _instance_type_p) def test_extract_flavor(self): self._test_extract_flavor('') @@ -494,10 +458,10 @@ class CreateInstanceTypeTest(test.TestCase): self.assertInvalidInput('flavor1', 64, 1, 120, rxtx_factor=0.0) flavor = flavors.create('flavor1', 64, 1, 120, rxtx_factor=1.0) - self.assertEqual(1.0, flavor['rxtx_factor']) + self.assertEqual(1.0, flavor.rxtx_factor) flavor = flavors.create('flavor2', 64, 1, 120, rxtx_factor=1.1) - self.assertEqual(1.1, flavor['rxtx_factor']) + self.assertEqual(1.1, flavor.rxtx_factor) def test_rxtx_factor_must_be_within_sql_float_range(self): _context = context.get_admin_context() @@ -511,7 +475,7 @@ class CreateInstanceTypeTest(test.TestCase): flavor = flavors.create('flavor2', 64, 1, 120, rxtx_factor=flavors.SQL_SP_FLOAT_MAX) - self.assertEqual(flavors.SQL_SP_FLOAT_MAX, flavor['rxtx_factor']) + self.assertEqual(flavors.SQL_SP_FLOAT_MAX, flavor.rxtx_factor) def test_is_public_must_be_valid_bool_string(self): self.assertInvalidInput('flavor1', 64, 1, 120, is_public='foo') @@ -528,21 +492,21 @@ class CreateInstanceTypeTest(test.TestCase): def test_flavorid_populated(self): flavor1 = flavors.create('flavor1', 64, 1, 120) - self.assertIsNot(None, flavor1['flavorid']) + self.assertIsNot(None, flavor1.flavorid) flavor2 = flavors.create('flavor2', 64, 1, 120, flavorid='') - self.assertIsNot(None, flavor2['flavorid']) + self.assertIsNot(None, flavor2.flavorid) flavor3 = flavors.create('flavor3', 64, 1, 120, flavorid='foo') - self.assertEqual('foo', flavor3['flavorid']) + self.assertEqual('foo', flavor3.flavorid) def test_default_values(self): flavor1 = flavors.create('flavor1', 64, 1, 120) - self.assertIsNot(None, flavor1['flavorid']) - self.assertEqual(flavor1['ephemeral_gb'], 0) - self.assertEqual(flavor1['swap'], 0) - self.assertEqual(flavor1['rxtx_factor'], 1.0) + self.assertIsNot(None, flavor1.flavorid) + self.assertEqual(flavor1.ephemeral_gb, 0) + self.assertEqual(flavor1.swap, 0) + self.assertEqual(flavor1.rxtx_factor, 1.0) def test_basic_create(self): # Ensure instance types can be created. @@ -550,10 +514,10 @@ class CreateInstanceTypeTest(test.TestCase): # Create new type and make sure values stick flavor = flavors.create('flavor', 64, 1, 120) - self.assertEqual(flavor['name'], 'flavor') - self.assertEqual(flavor['memory_mb'], 64) - self.assertEqual(flavor['vcpus'], 1) - self.assertEqual(flavor['root_gb'], 120) + self.assertEqual(flavor.name, 'flavor') + self.assertEqual(flavor.memory_mb, 64) + self.assertEqual(flavor.vcpus, 1) + self.assertEqual(flavor.root_gb, 120) # Ensure new type shows up in list new_list = flavors.get_all_flavors() @@ -572,11 +536,15 @@ class CreateInstanceTypeTest(test.TestCase): flavors.destroy('flavor') self.assertRaises(exception.FlavorNotFound, - flavors.get_flavor, flavor['id']) + flavors.get_flavor, flavor.id) # Deleted instance should not be in list anymore new_list = flavors.get_all_flavors() - self.assertEqual(original_list, new_list) + self.assertEqual(len(original_list), len(new_list)) + for k in original_list.keys(): + f = original_list[k] + self.assertIsInstance(f, objects.Flavor) + self.assertEqual(f.flavorid, new_list[k].flavorid) def test_duplicate_names_fail(self): # Ensures that name duplicates raise FlavorCreateFailed.