openstack/cinder
Code Issues Proposed changes

Use ovo instead of dict

Browse Source 
Currently when Cinder calls volume.api.create(), it transfers
dict of volume type to the interface.

This patch is to get volume type object, and transfer it to
volume.api.create() interface. So that it uses object fields
instead of dict.

Change-Id: I981b5060abaff280aa34a9570c65300f467045d6
This commit is contained in:
lisali 2016-12-05 08:01:16 +00:00
parent a491f3e60f
commit 1794131293
9 changed files with 206 additions and 192 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
cinder/api/v1/volumes.py
View File

@ -25,11 +25,11 @@ from webob import exc
from cinder.api import common
from cinder.api.openstack import wsgi
from cinder.i18n import _, _LI
from cinder import objects
from cinder.objects import fields
from cinder import utils
from cinder import volume as cinder_volume
from cinder.volume import utils as volume_utils
from cinder.volume import volume_types
LOG = logging.getLogger(__name__)
@ -229,7 +229,7 @@ class VolumeController(wsgi.Controller):
if req_volume_type:
# Not found exception will be handled at the wsgi level
kwargs['volume_type'] = (
volume_types.get_by_name_or_id(context, req_volume_type))
objects.VolumeType.get_by_name_or_id(context, req_volume_type))
kwargs['metadata'] = volume.get('metadata', None)

4
cinder/api/v2/volumes.py
View File

@ -30,10 +30,10 @@ from cinder import exception
from cinder import group as group_api
from cinder.i18n import _, _LI
from cinder.image import glance
from cinder import objects
from cinder import utils
from cinder import volume as cinder_volume
from cinder.volume import utils as volume_utils
from cinder.volume import volume_types
CONF = cfg.CONF
@ -199,7 +199,7 @@ class VolumeController(wsgi.Controller):
if req_volume_type:
# Not found exception will be handled at the wsgi level
kwargs['volume_type'] = (
volume_types.get_by_name_or_id(context, req_volume_type))
objects.VolumeType.get_by_name_or_id(context, req_volume_type))
kwargs['metadata'] = volume.get('metadata', None)

4
cinder/api/v3/volumes.py
View File

@ -24,10 +24,10 @@ from cinder.api.v2 import volumes as volumes_v2
from cinder.api.v3.views import volumes as volume_views_v3
from cinder import exception
from cinder import group as group_api
from cinder import objects
from cinder.i18n import _, _LI
import cinder.policy
from cinder import utils
from cinder.volume import volume_types
LOG = logging.getLogger(__name__)
@ -197,7 +197,7 @@ class VolumeController(volumes_v2.VolumeController):
if req_volume_type:
# Not found exception will be handled at the wsgi level
kwargs['volume_type'] = (
volume_types.get_by_name_or_id(context, req_volume_type))
objects.VolumeType.get_by_name_or_id(context, req_volume_type))
kwargs['metadata'] = volume.get('metadata', None)

7
cinder/objects/volume_type.py
View File

@ -154,6 +154,13 @@ class VolumeType(base.CinderPersistentObject, base.CinderObject,
self.obj_reset_changes(fields=[attrname])
@classmethod
def get_by_name_or_id(cls, context, identity):
orm_obj = volume_types.get_by_name_or_id(context, identity)
expected_attrs = cls._get_expected_attrs(context)
return cls._from_db_object(context, cls(context),
orm_obj, expected_attrs=expected_attrs)
@base.CinderObjectRegistry.register
class VolumeTypeList(base.ObjectListBase, base.CinderObject):

22
cinder/tests/unit/consistencygroup/test_cg.py
View File

@ -24,6 +24,7 @@ from cinder import quota
from cinder.tests.unit import conf_fixture
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_snapshot
from cinder.tests.unit import fake_volume
from cinder.tests.unit import utils as tests_utils
from cinder.tests.unit import volume as base
import cinder.volume
@ -716,17 +717,16 @@ class ConsistencyGroupTestCase(base.BaseVolumeTestCase):
context.get_admin_context(),
dict(name=conf_fixture.def_vol_type, extra_specs={})
)
db_vol_type = db.volume_type_get(context.get_admin_context(),
vol_type.id)
cg = {
'id': '1',
'name': 'cg1',
'volume_type_id': db_vol_type['id'],
}
fake_type = {
'id': '9999',
'name': 'fake',
}
vol_type = objects.VolumeType.get_by_id(self.context,
vol_type.id)
cg = objects.ConsistencyGroup(self.context,
id=fake.CONSISTENCY_GROUP_ID,
name='cg1',
volume_type_id=vol_type.id)
fake_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='fake')
vol_api = cinder.volume.api.API()
# Volume type must be provided when creating a volume in a

9
cinder/tests/unit/group/test_groups_manager.py
View File

@ -28,6 +28,7 @@ from cinder import test
from cinder.tests.unit import conf_fixture
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import fake_snapshot
from cinder.tests.unit import fake_volume
from cinder.tests.unit import utils as tests_utils
from cinder.volume import api as volume_api
from cinder.volume import configuration as conf
@ -723,10 +724,10 @@ class GroupManagerTestCase(test.TestCase):
group_type_id=fake.GROUP_TYPE_ID,
host=CONF.host)
fake_type = {
'id': '9999',
'name': 'fake',
}
fake_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='fake')
# Volume type must be provided when creating a volume in a
# group.

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

@ -65,6 +65,14 @@ class TestVolumeType(test_objects.BaseObjectsTestCase):
fake.VOLUME_TYPE_ID)
self._compare(self, db_volume_type, volume_type)
@mock.patch('cinder.volume.volume_types.get_by_name_or_id')
def test_get_by_name_or_id(self, volume_type_get):
db_volume_type = fake_volume.fake_db_volume_type()
volume_type_get.return_value = db_volume_type
volume_type = objects.VolumeType.get_by_name_or_id(
self.context, fake.VOLUME_TYPE_ID)
self._compare(self, db_volume_type, volume_type)
@ddt.data('1.0', '1.1')
def test_obj_make_compatible(self, version):
volume_type = objects.VolumeType(context=self.context)

280
cinder/tests/unit/volume/test_volume.py
View File

@ -847,26 +847,31 @@ class VolumeTestCase(base.BaseVolumeTestCase):
"""Test volume create from snapshot with types including mistmatch."""
volume_api = cinder.volume.api.API()
db.volume_type_create(
context.get_admin_context(),
{'name': 'foo',
'extra_specs': {'volume_backend_name': 'dev_1'}})
db.volume_type_create(
context.get_admin_context(),
{'name': 'biz', 'extra_specs': {'volume_backend_name': 'dev_2'}})
foo_type = db.volume_type_get_by_name(context.get_admin_context(),
'foo')
biz_type = db.volume_type_get_by_name(context.get_admin_context(),
'biz')
foo_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='foo',
extra_specs={'volume_backend_name': 'dev_1'})
biz_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE2_ID,
name='foo',
extra_specs={'volume_backend_name': 'dev_2'})
source_vol = fake_volume.fake_volume_obj(
self.context,
id=fake.VOLUME_ID,
status='available',
volume_size=10,
volume_type_id=biz_type.id)
source_vol.volume_type = biz_type
snapshot = {'id': fake.SNAPSHOT_ID,
'status': fields.SnapshotStatus.AVAILABLE,
'volume_size': 10,
'volume_type_id': biz_type['id']}
'volume_type_id': biz_type.id}
snapshot_obj = fake_snapshot.fake_snapshot_obj(self.context,
**snapshot)
snapshot_obj.volume = source_vol
# Make sure the case of specifying a type that
# doesn't match the snapshots type fails
self.assertRaises(exception.InvalidInput,
@ -881,7 +886,9 @@ class VolumeTestCase(base.BaseVolumeTestCase):
# Make sure that trying to specify a type
# when the snapshots type is None fails
snapshot_obj.volume_type_id = None
self.assertRaises(exception.InvalidVolumeType,
snapshot_obj.volume.volume_type_id = None
snapshot_obj.volume.volume_type = None
self.assertRaises(exception.InvalidInput,
volume_api.create,
self.context,
size=1,
@ -890,43 +897,36 @@ class VolumeTestCase(base.BaseVolumeTestCase):
volume_type=foo_type,
snapshot=snapshot_obj)
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type') as mock_get_type:
mock_get_type.return_value = biz_type
snapshot_obj.volume_type_id = foo_type['id']
volume_api.create(self.context, size=1, name='fake_name',
description='fake_desc', volume_type=foo_type,
snapshot=snapshot_obj)
db.volume_type_destroy(context.get_admin_context(),
foo_type['id'])
db.volume_type_destroy(context.get_admin_context(),
biz_type['id'])
snapshot_obj.volume_type_id = foo_type.id
snapshot_obj.volume.volume_type_id = foo_type.id
snapshot_obj.volume.volume_type = foo_type
volume_api.create(self.context, size=1, name='fake_name',
description='fake_desc', volume_type=foo_type,
snapshot=snapshot_obj)
@mock.patch('cinder.volume.flows.api.create_volume.get_flow')
def test_create_volume_from_source_with_types(self, _get_flow):
"""Test volume create from source with types including mistmatch."""
volume_api = cinder.volume.api.API()
foo_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='foo',
extra_specs={'volume_backend_name': 'dev_1'})
db.volume_type_create(
context.get_admin_context(),
{'name': 'foo',
'extra_specs': {'volume_backend_name': 'dev_1'}})
biz_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE2_ID,
name='biz',
extra_specs={'volume_backend_name': 'dev_2'})
db.volume_type_create(
context.get_admin_context(),
{'name': 'biz', 'extra_specs': {'volume_backend_name': 'dev_2'}})
foo_type = db.volume_type_get_by_name(context.get_admin_context(),
'foo')
biz_type = db.volume_type_get_by_name(context.get_admin_context(),
'biz')
source_vol = {'id': fake.VOLUME_ID,
'status': 'available',
'volume_size': 10,
'volume_type': biz_type,
'volume_type_id': biz_type['id']}
source_vol = fake_volume.fake_volume_obj(
self.context,
id=fake.VOLUME_ID,
status='available',
volume_size=0,
volume_type_id=biz_type.id)
source_vol.volume_type = biz_type
self.assertRaises(exception.InvalidInput,
volume_api.create,
@ -939,9 +939,9 @@ class VolumeTestCase(base.BaseVolumeTestCase):
# Make sure that trying to specify a type
# when the source type is None fails
source_vol['volume_type_id'] = None
source_vol['volume_type'] = None
self.assertRaises(exception.InvalidVolumeType,
source_vol.volume_type_id = None
source_vol.volume_type = None
self.assertRaises(exception.InvalidInput,
volume_api.create,
self.context,
size=1,
@ -950,99 +950,95 @@ class VolumeTestCase(base.BaseVolumeTestCase):
volume_type=foo_type,
source_volume=source_vol)
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type') as mock_get_type:
mock_get_type.return_value = biz_type
source_vol['volume_type_id'] = biz_type['id']
source_vol['volume_type'] = biz_type
volume_api.create(self.context, size=1, name='fake_name',
description='fake_desc', volume_type=biz_type,
source_volume=source_vol)
db.volume_type_destroy(context.get_admin_context(),
foo_type['id'])
db.volume_type_destroy(context.get_admin_context(),
biz_type['id'])
source_vol.volume_type_id = biz_type.id
source_vol.volume_type = biz_type
volume_api.create(self.context, size=1, name='fake_name',
description='fake_desc', volume_type=biz_type,
source_volume=source_vol)
@mock.patch('cinder.volume.flows.api.create_volume.get_flow')
def test_create_volume_from_source_with_same_backend(self, _get_flow):
"""Test volume create from source with type mismatch same backend."""
volume_api = cinder.volume.api.API()
foo_type = {
'name': 'foo',
'qos_specs_id': None,
'deleted': False,
'created_at': datetime.datetime(2015, 5, 8, 0, 40, 5, 408232),
'updated_at': None,
'extra_specs': {'volume_backend_name': 'dev_1'},
'is_public': True,
'deleted_at': None,
'id': '29e43b50-2cd7-4d0c-8ddd-2119daab3a38',
'description': None}
foo_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='foo',
qos_specs_id=None,
deleted=False,
created_at=datetime.datetime(2015, 5, 8, 0, 40, 5, 408232),
updated_at=None,
extra_specs={'volume_backend_name': 'dev_1'},
is_public=True,
deleted_at=None,
description=None)
biz_type = {
'name': 'biz',
'qos_specs_id': None,
'deleted': False,
'created_at': datetime.datetime(2015, 5, 8, 0, 20, 5, 408232),
'updated_at': None,
'extra_specs': {'volume_backend_name': 'dev_1'},
'is_public': True,
'deleted_at': None,
'id': '34e54c31-3bc8-5c1d-9fff-2225bcce4b59',
'description': None}
biz_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE2_ID,
name='biz',
qos_specs_id=None,
deleted=False,
created_at=datetime.datetime(2015, 5, 8, 0, 20, 5, 408232),
updated_at=None,
extra_specs={'volume_backend_name': 'dev_1'},
is_public=True,
deleted_at=None,
description=None)
source_vol = {'id': fake.VOLUME_ID,
'status': 'available',
'volume_size': 10,
'volume_type': biz_type,
'volume_type_id': biz_type['id']}
with mock.patch.object(cinder.volume.volume_types,
'get_volume_type') as mock_get_type:
mock_get_type.return_value = biz_type
volume_api.create(self.context,
size=1,
name='fake_name',
description='fake_desc',
volume_type=foo_type,
source_volume=source_vol)
source_vol = fake_volume.fake_volume_obj(
self.context,
id=fake.VOLUME_ID,
status='available',
volume_size=10,
volume_type_id=biz_type.id)
source_vol.volume_type = biz_type
volume_api.create(self.context,
size=1,
name='fake_name',
description='fake_desc',
volume_type=foo_type,
source_volume=source_vol)
@mock.patch('cinder.volume.flows.api.create_volume.get_flow')
def test_create_from_source_and_snap_only_one_backend(self, _get_flow):
"""Test create from source and snap with type mismatch one backend."""
volume_api = cinder.volume.api.API()
foo_type = {
'name': 'foo',
'qos_specs_id': None,
'deleted': False,
'created_at': datetime.datetime(2015, 5, 8, 0, 40, 5, 408232),
'updated_at': None,
'extra_specs': {'some_key': 3},
'is_public': True,
'deleted_at': None,
'id': '29e43b50-2cd7-4d0c-8ddd-2119daab3a38',
'description': None}
foo_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='foo',
qos_specs_id=None,
deleted=False,
created_at=datetime.datetime(2015, 5, 8, 0, 40, 5, 408232),
updated_at=None,
extra_specs={'some_key': 3},
is_public=True,
deleted_at=None,
description=None)
biz_type = {
'name': 'biz',
'qos_specs_id': None,
'deleted': False,
'created_at': datetime.datetime(2015, 5, 8, 0, 20, 5, 408232),
'updated_at': None,
'extra_specs': {'some_other_key': 4},
'is_public': True,
'deleted_at': None,
'id': '34e54c31-3bc8-5c1d-9fff-2225bcce4b59',
'description': None}
biz_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE2_ID,
name='biz',
qos_specs_id=None,
deleted=False,
created_at=datetime.datetime(2015, 5, 8, 0, 20, 5, 408232),
updated_at=None,
extra_specs={'some_other_key': 4},
is_public=True,
deleted_at=None,
description=None)
source_vol = {'id': fake.VOLUME_ID,
'status': 'available',
'volume_size': 10,
'volume_type': biz_type,
'volume_type_id': biz_type['id']}
source_vol = fake_volume.fake_volume_obj(
self.context,
id=fake.VOLUME_ID,
status='available',
volume_size=10,
volume_type_id=biz_type.id)
source_vol.volume_type = biz_type
snapshot = {'id': fake.SNAPSHOT_ID,
'status': fields.SnapshotStatus.AVAILABLE,
@ -1050,6 +1046,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
'volume_type_id': biz_type['id']}
snapshot_obj = fake_snapshot.fake_snapshot_obj(self.context,
**snapshot)
snapshot_obj.volume = source_vol
with mock.patch('cinder.db.service_get_all') as mock_get_service, \
mock.patch.object(volume_api,
@ -1073,22 +1070,24 @@ class VolumeTestCase(base.BaseVolumeTestCase):
def _test_create_from_source_snapshot_encryptions(
self, is_snapshot=False):
volume_api = cinder.volume.api.API()
foo_type = {
'name': 'foo',
'extra_specs': {'volume_backend_name': 'dev_1'},
'id': fake.VOLUME_TYPE_ID,
'description': None}
foo_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE_ID,
name='foo',
extra_specs={'volume_backend_name': 'dev_1'})
biz_type = fake_volume.fake_volume_type_obj(
self.context,
id=fake.VOLUME_TYPE2_ID,
name='biz',
extra_specs={'volume_backend_name': 'dev_1'})
biz_type = {
'name': 'foo',
'extra_specs': {'volume_backend_name': 'dev_1'},
'id': fake.VOLUME_TYPE2_ID,
'description': None}
source_vol = {'id': fake.VOLUME_ID,
'status': 'available',
'volume_size': 1,
'volume_type': biz_type,
'volume_type_id': biz_type['id']}
source_vol = fake_volume.fake_volume_obj(
self.context,
id=fake.VOLUME_ID,
status='available',
volume_size=1,
volume_type_id=biz_type.id)
source_vol.volume_type = biz_type
snapshot = {'id': fake.SNAPSHOT_ID,
'status': fields.SnapshotStatus.AVAILABLE,
@ -1096,6 +1095,7 @@ class VolumeTestCase(base.BaseVolumeTestCase):
'volume_type_id': biz_type['id']}
snapshot_obj = fake_snapshot.fake_snapshot_obj(self.context,
**snapshot)
snapshot_obj.volume = source_vol
with mock.patch.object(
cinder.volume.volume_types,

60
cinder/volume/api.py
View File

@ -174,31 +174,34 @@ class API(base.Base):
return tuple(azs)
def _retype_is_possible(self, context,
source_type_id, target_type_id):
safe = False
source_type, target_type):
elevated = context.elevated()
# If encryptions are different, it is not allowed
# to create volume from source volume or snapshot.
if volume_types.volume_types_encryption_changed(
elevated, source_type_id, target_type_id):
elevated,
source_type.id if source_type else None,
target_type.id if target_type else None):
return False
services = objects.ServiceList.get_all_by_topic(
elevated,
constants.VOLUME_TOPIC,
disabled=True)
if len(services.objects) == 1:
safe = True
else:
source_type = volume_types.get_volume_type(
elevated,
source_type_id)
target_type = volume_types.get_volume_type(
elevated,
target_type_id)
if (volume_utils.matching_backend_name(
source_type['extra_specs'], target_type['extra_specs'])):
safe = True
return safe
return True
source_extra_specs = {}
if source_type:
with source_type.obj_as_admin():
source_extra_specs = source_type.extra_specs
target_extra_specs = {}
if target_type:
with target_type.obj_as_admin():
target_extra_specs = target_type.extra_specs
if (volume_utils.matching_backend_name(
source_extra_specs, target_extra_specs)):
return True
return False
def _is_volume_migrating(self, volume):
# The migration status 'none' means no migration has ever been done
@ -242,8 +245,8 @@ class API(base.Base):
msg = _("volume_type must be provided when creating "
"a volume in a consistency group.")
raise exception.InvalidInput(reason=msg)
cg_voltypeids = consistencygroup.get('volume_type_id')
if volume_type.get('id') not in cg_voltypeids:
cg_voltypeids = consistencygroup.volume_type_id
if volume_type.id not in cg_voltypeids:
msg = _("Invalid volume_type provided: %s (requested "
"type must be supported by this consistency "
"group).") % volume_type
@ -255,26 +258,21 @@ class API(base.Base):
"a volume in a group.")
raise exception.InvalidInput(reason=msg)
vol_type_ids = [v_type.id for v_type in group.volume_types]
if volume_type.get('id') not in vol_type_ids:
if volume_type.id not in vol_type_ids:
msg = _("Invalid volume_type provided: %s (requested "
"type must be supported by this "
"group).") % volume_type
raise exception.InvalidInput(reason=msg)
if volume_type and 'extra_specs' not in volume_type:
extra_specs = volume_types.get_volume_type_extra_specs(
volume_type['id'])
volume_type['extra_specs'] = extra_specs
if source_volume and volume_type:
if volume_type['id'] != source_volume['volume_type_id']:
if volume_type.id != source_volume.volume_type_id:
if not self._retype_is_possible(
context,
source_volume['volume_type_id'],
volume_type['id']):
source_volume.volume_type,
volume_type):
msg = _("Invalid volume_type provided: %s (requested type "
"is not compatible; either match source volume, "
"or omit type argument).") % volume_type['id']
"or omit type argument).") % volume_type.id
raise exception.InvalidInput(reason=msg)
# When cloning replica (for testing), volume type must be omitted
@ -284,13 +282,13 @@ class API(base.Base):
raise exception.InvalidInput(reason=msg)
if snapshot and volume_type:
if volume_type['id'] != snapshot.volume_type_id:
if volume_type.id != snapshot.volume_type_id:
if not self._retype_is_possible(context,
snapshot.volume_type_id,
volume_type['id']):
snapshot.volume.volume_type,
volume_type):
msg = _("Invalid volume_type provided: %s (requested "
"type is not compatible; recommend omitting "
"the type argument).") % volume_type['id']
"the type argument).") % volume_type.id
raise exception.InvalidInput(reason=msg)
# Determine the valid availability zones that the volume could be