Merge "Make Flavor.get operations prefer the API database"
This commit is contained in:
commit
2e2c225e83
@ -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):
|
||||
|
@ -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'])
|
||||
|
||||
|
@ -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()
|
||||
|
@ -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')
|
||||
|
Loading…
x
Reference in New Issue
Block a user