diff --git a/nova/db/sqlalchemy/api_models.py b/nova/db/sqlalchemy/api_models.py index b063a0bc62..4c9fd4d88b 100644 --- a/nova/db/sqlalchemy/api_models.py +++ b/nova/db/sqlalchemy/api_models.py @@ -127,6 +127,10 @@ class FlavorExtraSpecs(API_BASE): key = Column(String(255), nullable=False) value = Column(String(255)) flavor_id = Column(Integer, ForeignKey('flavors.id'), nullable=False) + flavor = orm.relationship(Flavors, backref='extra_specs', + foreign_keys=flavor_id, + primaryjoin=( + 'FlavorExtraSpecs.flavor_id == Flavors.id')) class FlavorProjects(API_BASE): @@ -138,6 +142,10 @@ class FlavorProjects(API_BASE): id = Column(Integer, primary_key=True) flavor_id = Column(Integer, ForeignKey('flavors.id'), nullable=False) project_id = Column(String(255), nullable=False) + flavor = orm.relationship(Flavors, backref='projects', + foreign_keys=flavor_id, + primaryjoin=( + 'FlavorProjects.flavor_id == Flavors.id')) class BuildRequest(API_BASE): diff --git a/nova/objects/flavor.py b/nova/objects/flavor.py index a03c38bfaa..164407d505 100644 --- a/nova/objects/flavor.py +++ b/nova/objects/flavor.py @@ -12,7 +12,15 @@ # License for the specific language governing permissions and limitations # under the License. +from sqlalchemy import or_ +from sqlalchemy.orm import joinedload +from sqlalchemy.sql.expression import asc +from sqlalchemy.sql import true + from nova import db +from nova.db.sqlalchemy import api as db_api +from nova.db.sqlalchemy.api import require_context +from nova.db.sqlalchemy import api_models from nova import exception from nova import objects from nova.objects import base @@ -20,6 +28,7 @@ from nova.objects import fields OPTIONAL_FIELDS = ['extra_specs', 'projects'] +DEPRECATED_FIELDS = ['deleted', 'deleted_at'] # TODO(berrange): Remove NovaObjectDictCompat @@ -61,11 +70,21 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, for name, field in flavor.fields.items(): if name in OPTIONAL_FIELDS: continue + if name in DEPRECATED_FIELDS and name not in db_flavor: + continue value = db_flavor[name] if isinstance(field, fields.IntegerField): value = value if value is not None else 0 flavor[name] = value + # NOTE(danms): This is to support processing the API flavor + # model, which does not have these deprecated fields. When we + # remove compatibility with the old InstanceType model, we can + # remove this as well. + if any(f not in db_flavor for f in DEPRECATED_FIELDS): + flavor.deleted_at = None + flavor.deleted = False + if 'extra_specs' in expected_attrs: flavor.extra_specs = db_flavor['extra_specs'] @@ -75,6 +94,53 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, flavor.obj_reset_changes() return flavor + @staticmethod + @db_api.api_context_manager.reader + def _flavor_get_query_from_db(context): + query = context.session.query(api_models.Flavors).\ + options(joinedload('extra_specs')) + if not context.is_admin: + the_filter = [api_models.Flavors.is_public == true()] + the_filter.extend([ + api_models.Flavors.projects.any(project_id=context.project_id) + ]) + query = query.filter(or_(*the_filter)) + return query + + @staticmethod + @require_context + def _flavor_get_from_db(context, id): + """Returns a dict describing specific flavor.""" + result = Flavor._flavor_get_query_from_db(context).\ + filter_by(id=id).\ + first() + if not result: + raise exception.FlavorNotFound(flavor_id=id) + return db_api._dict_with_extra_specs(result) + + @staticmethod + @require_context + def _flavor_get_by_name_from_db(context, name): + """Returns a dict describing specific flavor.""" + result = Flavor._flavor_get_query_from_db(context).\ + filter_by(name=name).\ + first() + if not result: + raise exception.FlavorNotFoundByName(flavor_name=name) + return db_api._dict_with_extra_specs(result) + + @staticmethod + @require_context + def _flavor_get_by_flavor_id_from_db(context, flavor_id): + """Returns a dict describing specific flavor_id.""" + result = Flavor._flavor_get_query_from_db(context).\ + filter_by(flavorid=flavor_id).\ + order_by(asc("id")).\ + first() + if not result: + raise exception.FlavorNotFound(flavor_id=flavor_id) + return db_api._dict_with_extra_specs(result) + @base.remotable def _load_projects(self): self.projects = [x['project_id'] for x in @@ -130,20 +196,30 @@ class Flavor(base.NovaPersistentObject, base.NovaObject, @base.remotable_classmethod def get_by_id(cls, context, id): - db_flavor = db.flavor_get(context, id) + try: + db_flavor = cls._flavor_get_from_db(context, id) + except exception.FlavorNotFound: + db_flavor = db.flavor_get(context, id) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) @base.remotable_classmethod def get_by_name(cls, context, name): - db_flavor = db.flavor_get_by_name(context, name) + try: + db_flavor = cls._flavor_get_by_name_from_db(context, name) + except exception.FlavorNotFoundByName: + db_flavor = db.flavor_get_by_name(context, name) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) @base.remotable_classmethod def get_by_flavor_id(cls, context, flavor_id, read_deleted=None): - db_flavor = db.flavor_get_by_flavor_id(context, flavor_id, - read_deleted) + try: + db_flavor = cls._flavor_get_by_flavor_id_from_db(context, + flavor_id) + except exception.FlavorNotFound: + db_flavor = db.flavor_get_by_flavor_id(context, flavor_id, + read_deleted) return cls._from_db_object(context, cls(context), db_flavor, expected_attrs=['extra_specs']) diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_access.py b/nova/tests/unit/api/openstack/compute/test_flavor_access.py index e906fe18fc..d6b0277235 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_access.py @@ -155,13 +155,17 @@ class FlavorAccessTestV21(test.NoDBTestCase): for d1, d2 in zip(result, expected): self.assertEqual(d1['id'], d2['id']) - def test_list_flavor_access_public(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_list_flavor_access_public(self, mock_api_get): # query os-flavor-access on public flavor should return 404 self.assertRaises(exc.HTTPNotFound, self.flavor_access_controller.index, self.req, '1') - def test_list_flavor_access_private(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_list_flavor_access_private(self, mock_api_get): expected = {'flavor_access': [ {'flavor_id': '2', 'tenant_id': 'proj2'}, {'flavor_id': '2', 'tenant_id': 'proj3'}]} @@ -283,7 +287,9 @@ class FlavorAccessTestV21(test.NoDBTestCase): else: return self.flavor_action_controller._removeTenantAccess - def test_add_tenant_access(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_add_tenant_access(self, mock_api_get): def stub_add_flavor_access(context, flavorid, projectid): self.assertEqual('3', flavorid, "flavorid") self.assertEqual("proj2", projectid, "projectid") @@ -320,7 +326,9 @@ class FlavorAccessTestV21(test.NoDBTestCase): self.assertRaises(self.validation_ex, add_access, req, '2', body=body) - def test_add_tenant_access_with_already_added_access(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_add_tenant_access_with_already_added_access(self, mock_api_get): def stub_add_flavor_access(context, flavorid, projectid): raise exception.FlavorAccessExists(flavor_id=flavorid, project_id=projectid) @@ -331,7 +339,9 @@ class FlavorAccessTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPConflict, add_access, self.req, '3', body=body) - def test_remove_tenant_access_with_bad_access(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_remove_tenant_access_with_bad_access(self, mock_api_get): def stub_remove_flavor_access(context, flavorid, projectid): raise exception.FlavorAccessNotFound(flavor_id=flavorid, project_id=projectid) @@ -342,7 +352,9 @@ class FlavorAccessTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPNotFound, remove_access, self.req, '3', body=body) - def test_add_tenant_access_is_public(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_add_tenant_access_is_public(self, mock_api_get): body = {'addTenantAccess': {'tenant': 'proj2'}} req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', use_admin_context=True) @@ -351,7 +363,9 @@ class FlavorAccessTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPConflict, add_access, req, '1', body=body) - def test_delete_tenant_access_with_no_tenant(self): + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db', + side_effect=exception.FlavorNotFound(flavor_id='foo')) + def test_delete_tenant_access_with_no_tenant(self, mock_api_get): req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', use_admin_context=True) remove_access = self._get_remove_access() diff --git a/nova/tests/unit/objects/test_flavor.py b/nova/tests/unit/objects/test_flavor.py index 3a792db383..7c6dfb02d1 100644 --- a/nova/tests/unit/objects/test_flavor.py +++ b/nova/tests/unit/objects/test_flavor.py @@ -12,10 +12,15 @@ # License for the specific language governing permissions and limitations # under the License. +import datetime + import mock from nova import db +from nova.db.sqlalchemy import api as db_api +from nova.db.sqlalchemy import api_models from nova import exception +from nova import objects from nova.objects import flavor as flavor_obj from nova.tests.unit.objects import test_objects @@ -41,10 +46,36 @@ fake_flavor = { } +fake_api_flavor = { + 'created_at': None, + 'updated_at': None, + 'id': 1, + 'name': 'm1.foo', + 'memory_mb': 1024, + 'vcpus': 4, + 'root_gb': 20, + 'ephemeral_gb': 0, + 'flavorid': 'm1.foo', + 'swap': 0, + 'rxtx_factor': 1.0, + 'vcpu_weight': 1, + 'disabled': False, + 'is_public': True, + 'extra_specs': {'foo': 'bar'}, + } + + class _TestFlavor(object): @staticmethod def _compare(test, db, obj): for field, value in db.items(): + # NOTE(danms): The datetimes on SQLA models are tz-unaware, + # but the object has tz-aware datetimes. If we're comparing + # a model to an object (as opposed to a fake dict), just + # ignore the datetimes in the comparison. + if (isinstance(db, api_models.API_BASE) and + isinstance(value, datetime.datetime)): + continue test.assertEqual(db[field], obj[field]) def test_get_by_id(self): @@ -66,6 +97,60 @@ class _TestFlavor(object): 'm1.foo') self._compare(self, fake_flavor, flavor) + @mock.patch('nova.objects.Flavor._flavor_get_from_db') + def test_api_get_by_id_from_api(self, mock_get): + mock_get.return_value = fake_api_flavor + flavor = flavor_obj.Flavor.get_by_id(self.context, 1) + self._compare(self, fake_api_flavor, flavor) + mock_get.assert_called_once_with(self.context, 1) + + @mock.patch('nova.objects.Flavor._flavor_get_by_name_from_db') + def test_get_by_name_from_api(self, mock_get): + mock_get.return_value = fake_api_flavor + flavor = flavor_obj.Flavor.get_by_name(self.context, 'm1.foo') + self._compare(self, fake_api_flavor, flavor) + mock_get.assert_called_once_with(self.context, 'm1.foo') + + @mock.patch('nova.objects.Flavor._flavor_get_by_flavor_id_from_db') + def test_get_by_flavor_id_from_api(self, mock_get): + mock_get.return_value = fake_api_flavor + flavor = flavor_obj.Flavor.get_by_flavor_id(self.context, 'm1.foo') + self._compare(self, fake_api_flavor, flavor) + mock_get.assert_called_once_with(self.context, 'm1.foo') + + @staticmethod + @db_api.api_context_manager.writer + def _create_api_flavor(context): + fake_db_flavor = dict(fake_api_flavor) + del fake_db_flavor['extra_specs'] + flavor = api_models.Flavors() + flavor.update(fake_db_flavor) + flavor.save(context.session) + + fake_db_extra_spec = {'flavor_id': flavor['id'], + 'key': 'foo', 'value': 'bar'} + flavor_es = api_models.FlavorExtraSpecs() + flavor_es.update(fake_db_extra_spec) + flavor_es.save(context.session) + + return flavor + + def test_get_by_id_from_db(self): + db_flavor = self._create_api_flavor(self.context) + flavor = objects.Flavor.get_by_id(self.context, db_flavor['id']) + self._compare(self, db_flavor, flavor) + + def test_get_by_name_from_db(self): + db_flavor = self._create_api_flavor(self.context) + flavor = objects.Flavor.get_by_name(self.context, db_flavor['name']) + self._compare(self, db_flavor, flavor) + + def test_get_by_flavor_id_from_db(self): + db_flavor = self._create_api_flavor(self.context) + flavor = objects.Flavor.get_by_flavor_id(self.context, + db_flavor['flavorid']) + self._compare(self, db_flavor, flavor) + def test_add_access(self): elevated = self.context.elevated() flavor = flavor_obj.Flavor(context=elevated, flavorid='123')