openstack
/
cinder
Code Issues Proposed changes

Allow attribute lazy loading in VolumeType OVO 

VolumeType OVO class does not allow lazy loading of attributes, which is
usually not a problem if you are loading the volume type using
get_by_id, but is a problem if you load for example a list of volumes
and then go to their volume_type.extra_specs, as this will raise an
error because extra_specs cannot be retrieved.

This patch changes VolumeType OVO class to allow lazy loading all
optional fields.

Change-Id: Ief143a6c981cec4bdb21888776d610aa9d5dc9d8
This commit is contained in:
Gorka Eguileor 2016-09-07 16:57:13 +02:00
parent d004edf4e0
commit e4921b4990
7 changed files with 96 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cinder/objects/base.py
View File

@ -116,6 +116,7 @@ OBJ_VERSIONS.add('1.10', {'Group': '1.0', 'GroupList': '1.0', 'Volume': '1.5',
'RequestSpec': '1.1', 'VolumeProperties': '1.1'})
OBJ_VERSIONS.add('1.11', {'GroupSnapshot': '1.0', 'GroupSnapshotList': '1.0',
'Group': '1.1'})
OBJ_VERSIONS.add('1.12', {'VolumeType': '1.3'})
class CinderObjectRegistry(base.VersionedObjectRegistry):

33
cinder/objects/volume_type.py
View File

@ -30,7 +30,8 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
# Version 1.0: Initial version
# Version 1.1: Changed extra_specs to DictOfNullableStringsField
# Version 1.2: Added qos_specs
VERSION = '1.2'
# Version 1.3: Add qos_specs_id
VERSION = '1.3'
OPTIONAL_FIELDS = ('extra_specs', 'projects', 'qos_specs')
@ -41,6 +42,7 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
'is_public': fields.BooleanField(default=True, nullable=True),
'projects': fields.ListOfStringsField(nullable=True),
'extra_specs': fields.DictOfNullableStringsField(nullable=True),
'qos_specs_id': fields.UUIDField(nullable=True),
'qos_specs': fields.ObjectField('QualityOfServiceSpecs',
nullable=True),
}
@ -57,6 +59,8 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
for k, v in primitive['extra_specs'].items():
if v is None:
primitive['extra_specs'][k] = ''
if target_version < (1, 3):
primitive.pop('qos_specs_id', None)
@classmethod
def _get_expected_attrs(cls, context, *args, **kwargs):
@ -123,6 +127,33 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
self.update(updated_values)
self.obj_reset_changes(updated_values.keys())
def obj_load_attr(self, attrname):
if attrname not in self.OPTIONAL_FIELDS:
raise exception.ObjectActionError(
action='obj_load_attr',
reason=_('attribute %s not lazy-loadable') % attrname)
if not self._context:
raise exception.OrphanedObjectError(method='obj_load_attr',
objtype=self.obj_name())
if attrname == 'extra_specs':
self.extra_specs = db.volume_type_extra_specs_get(self._context,
self.id)
elif attrname == 'qos_specs':
if self.qos_specs_id:
self.qos_specs = objects.QualityOfServiceSpecs.get_by_id(
self._context, self.qos_specs_id)
else:
self.qos_specs = None
elif attrname == 'projects':
volume_type_projects = db.volume_type_access_get_all(self._context,
self.id)
self.projects = [x.project_id for x in volume_type_projects]
self.obj_reset_changes(fields=[attrname])
@base.CinderObjectRegistry.register
class VolumeTypeList(base.ObjectListBase, base.CinderObject):

1
cinder/tests/unit/api/v1/stubs.py
View File

@ -204,4 +204,5 @@ def stub_volume_type_get(context, id, *args, **kwargs):
'created_at': None,
'deleted_at': None,
'updated_at': None,
'qos_specs_id': fake.QOS_SPEC_ID,
'deleted': False}

1
cinder/tests/unit/api/v2/stubs.py
View File

@ -251,6 +251,7 @@ def stub_volume_type_get(context, id, *args, **kwargs):
'created_at': None,
'deleted_at': None,
'updated_at': None,
'qos_specs_id': fake.QOS_SPEC_ID,
'deleted': False}

2
cinder/tests/unit/objects/test_objects.py
View File

@ -44,7 +44,7 @@ object_data = {
'VolumeAttachment': '1.0-b30dacf62b2030dd83d8a1603f1064ff',
'VolumeAttachmentList': '1.0-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'VolumeProperties': '1.1-cadac86b2bdc11eb79d1dcea988ff9e8',
'VolumeType': '1.2-02ecb0baac87528d041f4ddd95b95579',
'VolumeType': '1.3-a5d8c3473db9bc3bbcdbab9313acf4d1',
'VolumeTypeList': '1.1-15ecf022a68ddbb8c2a6739cfc9f8f5e',
'GroupType': '1.0-d4a7b272199d0b0d6fc3ceed58539d30',
'GroupTypeList': '1.0-1b54e51ad0fc1f3a8878f5010e7e16dc',

62
cinder/tests/unit/objects/test_volume_type.py
View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import ddt
import mock
from oslo_utils import timeutils
import pytz
@ -24,6 +25,7 @@ from cinder.tests.unit import fake_volume
from cinder.tests.unit import objects as test_objects
@ddt.ddt
class TestVolumeType(test_objects.BaseObjectsTestCase):
@mock.patch('cinder.db.sqlalchemy.api._volume_type_get_full')
@ -63,13 +65,17 @@ class TestVolumeType(test_objects.BaseObjectsTestCase):
fake.VOLUME_TYPE_ID)
self._compare(self, db_volume_type, volume_type)
def test_obj_make_compatible(self):
@ddt.data('1.0', '1.1')
def test_obj_make_compatible(self, version):
volume_type = objects.VolumeType(context=self.context)
volume_type.extra_specs = {'foo': None, 'bar': 'baz'}
primitive = volume_type.obj_to_primitive('1.0')
volume_type.qos_specs_id = fake.QOS_SPEC_ID
primitive = volume_type.obj_to_primitive(version)
volume_type = objects.VolumeType.obj_from_primitive(primitive)
self.assertEqual('', volume_type.extra_specs['foo'])
foo = '' if version == '1.0' else None
self.assertEqual(foo, volume_type.extra_specs['foo'])
self.assertEqual('baz', volume_type.extra_specs['bar'])
self.assertFalse(volume_type.obj_attr_is_set('qos_specs_id'))
@mock.patch('cinder.volume.volume_types.create')
def test_create(self, volume_type_create):
@ -146,6 +152,56 @@ class TestVolumeType(test_objects.BaseObjectsTestCase):
mock.call(self.context,
fake.VOLUME_TYPE_ID)])
@mock.patch('cinder.objects.QualityOfServiceSpecs.get_by_id')
@mock.patch('cinder.db.sqlalchemy.api._volume_type_get')
def test_lazy_loading_qos(self, get_mock, qos_get_mock):
qos_get_mock.return_value = objects.QualityOfServiceSpecs(
id=fake.QOS_SPEC_ID)
vol_type = fake_volume.fake_db_volume_type(
qos_specs_id=fake.QOS_SPEC_ID)
get_mock.return_value = vol_type
volume_type = objects.VolumeType.get_by_id(self.context,
vol_type['id'])
self._compare(self, qos_get_mock.return_value, volume_type.qos_specs)
qos_get_mock.assert_called_once_with(self.context, fake.QOS_SPEC_ID)
@mock.patch('cinder.db.volume_type_access_get_all')
@mock.patch('cinder.db.sqlalchemy.api._volume_type_get')
def test_lazy_loading_projects(self, get_mock, get_projects_mock):
vol_type = fake_volume.fake_db_volume_type(
qos_specs_id=fake.QOS_SPEC_ID)
get_mock.return_value = vol_type
projects = [models.VolumeTypeProjects(project_id=fake.PROJECT_ID),
models.VolumeTypeProjects(project_id=fake.PROJECT2_ID)]
get_projects_mock.return_value = projects
volume_type = objects.VolumeType.get_by_id(self.context,
vol_type['id'])
# Simulate this type has been loaded by a volume get_all method
del volume_type.projects
self.assertEqual([p.project_id for p in projects],
volume_type.projects)
get_projects_mock.assert_called_once_with(self.context, vol_type['id'])
@mock.patch('cinder.db.volume_type_extra_specs_get')
@mock.patch('cinder.db.sqlalchemy.api._volume_type_get')
def test_lazy_loading_extra_specs(self, get_mock, get_specs_mock):
get_specs_mock.return_value = {'key': 'value', 'key2': 'value2'}
vol_type = fake_volume.fake_db_volume_type(
qos_specs_id=fake.QOS_SPEC_ID)
get_mock.return_value = vol_type
volume_type = objects.VolumeType.get_by_id(self.context,
vol_type['id'])
# Simulate this type has been loaded by a volume get_all method
del volume_type.extra_specs
self.assertEqual(get_specs_mock.return_value, volume_type.extra_specs)
get_specs_mock.assert_called_once_with(self.context, vol_type['id'])
class TestVolumeTypeList(test_objects.BaseObjectsTestCase):
@mock.patch('cinder.volume.volume_types.get_all_types')

1
cinder/tests/unit/test_qos_specs.py
View File

@ -52,6 +52,7 @@ def fake_db_get_vol_type(vol_type_number=1):
'deleted': False,
'is_public': True,
'projects': [],
'qos_specs_id': fake.QOS_SPEC_ID,
'extra_specs': None}