diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index 219d655d7c6..31b9cba31a7 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -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) diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index 0f91bf18e3e..86ae02c74ef 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -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) diff --git a/cinder/api/v3/volumes.py b/cinder/api/v3/volumes.py index dec83abf15b..75c4b54849f 100644 --- a/cinder/api/v3/volumes.py +++ b/cinder/api/v3/volumes.py @@ -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) diff --git a/cinder/objects/volume_type.py b/cinder/objects/volume_type.py index 206d2958f1a..10be51084b4 100644 --- a/cinder/objects/volume_type.py +++ b/cinder/objects/volume_type.py @@ -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): diff --git a/cinder/tests/unit/consistencygroup/test_cg.py b/cinder/tests/unit/consistencygroup/test_cg.py index cfa48e55a80..f4f84e6a1e3 100644 --- a/cinder/tests/unit/consistencygroup/test_cg.py +++ b/cinder/tests/unit/consistencygroup/test_cg.py @@ -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 diff --git a/cinder/tests/unit/group/test_groups_manager.py b/cinder/tests/unit/group/test_groups_manager.py index f93a899c546..39ac194cde4 100644 --- a/cinder/tests/unit/group/test_groups_manager.py +++ b/cinder/tests/unit/group/test_groups_manager.py @@ -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. diff --git a/cinder/tests/unit/objects/test_volume_type.py b/cinder/tests/unit/objects/test_volume_type.py index 2a2b24349ae..c5ec6c8efec 100644 --- a/cinder/tests/unit/objects/test_volume_type.py +++ b/cinder/tests/unit/objects/test_volume_type.py @@ -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) diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index f3dc76a861e..90fb954d170 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -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, diff --git a/cinder/volume/api.py b/cinder/volume/api.py index e488caaf10a..2c7b9db1599 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -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