diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index e373ae70943..6f39b71bcf4 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -174,6 +174,9 @@ class CreateVolumeFlowTestCase(test.TestCase): 'ExtractVolumeRequestTask.' '_get_encryption_key_id', mock.Mock()) @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs') + @mock.patch( + 'cinder.objects.volume_type.VolumeType.get_by_name_or_id', + mock.Mock(return_value={})) def test_extract_volume_request_replication_status(self, replication_status, extra_specs, @@ -211,6 +214,9 @@ class CreateVolumeFlowTestCase(test.TestCase): 'ExtractVolumeRequestTask.' '_get_encryption_key_id') @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs') + @mock.patch( + 'cinder.objects.volume_type.VolumeType.get_by_name_or_id', + mock.Mock(return_value={})) def test_extract_volume_request_from_image_encrypted( self, fake_get_qos, @@ -256,8 +262,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_volume_request_from_image( self, + fake_get_type, fake_get_type_id, fake_get_qos, fake_is_encrypted): @@ -271,6 +279,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_image_service.create(self.ctxt, image_meta) fake_key_manager = mock_key_manager.MockKeyManager() volume_type = {'name': 'type1'} + fake_get_type.return_value = volume_type task = create_volume.ExtractVolumeRequestTask( fake_image_service, @@ -306,7 +315,8 @@ class CreateVolumeFlowTestCase(test.TestCase): 'group_id': None, 'refresh_az': False, 'replication_status': 'disabled', - 'backup_id': None} + 'backup_id': None, + 'multiattach': False} self.assertEqual(expected_result, result) @mock.patch('cinder.volume.volume_types.is_encrypted') @@ -468,8 +478,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_availability_zones_with_fallback( self, + fake_get_type, fake_get_type_id, fake_get_qos, fake_is_encrypted): @@ -485,6 +497,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_image_service.create(self.ctxt, image_meta) fake_key_manager = mock_key_manager.MockKeyManager() volume_type = {'name': 'type1'} + fake_get_type.return_value = volume_type task = create_volume.ExtractVolumeRequestTask( fake_image_service, @@ -519,6 +532,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'cgsnapshot_id': None, 'group_id': None, 'refresh_az': True, + 'multiattach': False, 'replication_status': 'disabled', 'backup_id': None} self.assertEqual(expected_result, result) @@ -582,8 +596,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_volume_request_task_with_large_volume_size( self, + fake_get_type, fake_get_type_id, fake_get_qos, fake_is_encrypted): @@ -596,6 +612,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_image_service.create(self.ctxt, image_meta) fake_key_manager = mock_key_manager.MockKeyManager() volume_type = {'name': 'type1'} + fake_get_type.return_value = volume_type task = create_volume.ExtractVolumeRequestTask( fake_image_service, @@ -631,6 +648,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'cgsnapshot_id': None, 'refresh_az': False, 'group_id': None, + 'multiattach': False, 'backup_id': None} self.assertEqual(expected_result, result) @@ -639,8 +657,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_volume_request_from_image_with_qos_specs( self, + fake_get_type, fake_get_type_id, fake_get_qos, fake_is_encrypted): @@ -653,7 +673,8 @@ class CreateVolumeFlowTestCase(test.TestCase): image_meta['size'] = 1 fake_image_service.create(self.ctxt, image_meta) fake_key_manager = mock_key_manager.MockKeyManager() - volume_type = {'name': 'type1'} + volume_type = {'name': 'type1', 'id': 1} + fake_get_type.return_value = volume_type task = create_volume.ExtractVolumeRequestTask( fake_image_service, @@ -689,6 +710,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'cgsnapshot_id': None, 'group_id': None, 'refresh_az': False, + 'multiattach': False, 'replication_status': 'disabled', 'backup_id': None} self.assertEqual(expected_result, result) @@ -700,8 +722,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_image_volume_type_from_image( self, + fake_get_type, fake_get_type_id, fake_get_vol_type, fake_get_def_vol_type, @@ -709,6 +733,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_is_encrypted): image_volume_type = {'name': 'type_from_image'} + fake_get_type.return_value = image_volume_type fake_image_service = fake_image.FakeImageService() image_id = 6 image_meta = {} @@ -755,6 +780,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'cgsnapshot_id': None, 'group_id': None, 'refresh_az': False, + 'multiattach': False, 'replication_status': 'disabled', 'backup_id': None} self.assertEqual(expected_result, result) @@ -766,8 +792,10 @@ class CreateVolumeFlowTestCase(test.TestCase): @mock.patch('cinder.volume.flows.api.create_volume.' 'ExtractVolumeRequestTask.' '_get_volume_type_id') + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_image_volume_type_from_image_invalid_type( self, + fake_get_type, fake_get_type_id, fake_get_def_vol_type, fake_get_qos, @@ -793,6 +821,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_is_encrypted.return_value = False fake_get_type_id.return_value = 1 fake_get_def_vol_type.return_value = {'name': 'fake_vol_type'} + fake_get_type.return_value = {'name': 'fake_vol_type'} fake_db_get_vol_type.side_effect = ( exception.VolumeTypeNotFoundByName(volume_type_name='invalid')) fake_get_qos.return_value = {'qos_specs': None} @@ -821,6 +850,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'consistencygroup_id': None, 'cgsnapshot_id': None, 'group_id': None, + 'multiattach': False, 'refresh_az': False, 'replication_status': 'disabled', 'backup_id': None} @@ -835,10 +865,12 @@ class CreateVolumeFlowTestCase(test.TestCase): '_get_volume_type_id') @ddt.data((8, None), (9, {'cinder_img_volume_type': None})) @ddt.unpack + @mock.patch('cinder.objects.volume_type.VolumeType.get_by_name_or_id') def test_extract_image_volume_type_from_image_properties_error( self, image_id, fake_img_properties, + fake_get_type, fake_get_type_id, fake_get_def_vol_type, fake_get_qos, @@ -861,6 +893,7 @@ class CreateVolumeFlowTestCase(test.TestCase): fake_is_encrypted.return_value = False fake_get_type_id.return_value = 1 fake_get_def_vol_type.return_value = {'name': 'fake_vol_type'} + fake_get_type.return_value = {'name': 'fake_vol_type'} fake_get_qos.return_value = {'qos_specs': None} result = task.execute(self.ctxt, size=1, @@ -888,6 +921,7 @@ class CreateVolumeFlowTestCase(test.TestCase): 'cgsnapshot_id': None, 'group_id': None, 'refresh_az': False, + 'multiattach': False, 'replication_status': 'disabled', 'backup_id': None} self.assertEqual(expected_result, result) diff --git a/cinder/tests/unit/volume/test_volume_retype.py b/cinder/tests/unit/volume/test_volume_retype.py index caee4634792..3a0e6ee1f3a 100644 --- a/cinder/tests/unit/volume/test_volume_retype.py +++ b/cinder/tests/unit/volume/test_volume_retype.py @@ -116,7 +116,6 @@ class VolumeRetypeTestCase(base.BaseVolumeTestCase): mock.call(volume_policies.MULTIATTACH_POLICY, target_obj=mock.ANY), mock.call(volume_policies.CREATE_POLICY), - mock.call(volume_policies.MULTIATTACH_POLICY), mock.call(volume_policies.CREATE_POLICY), mock.call(vol_action_policies.RETYPE_POLICY, target_obj=mock.ANY), mock.call(vol_action_policies.RETYPE_POLICY, target_obj=mock.ANY), diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 9c59bf41f5d..faea17216f6 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -289,9 +289,6 @@ class API(base.Base): utils.check_metadata_properties(metadata) - if (volume_type and self._is_multiattach(volume_type)) or multiattach: - context.authorize(vol_policy.MULTIATTACH_POLICY) - create_what = { 'context': context, 'raw_size': size, @@ -308,7 +305,7 @@ class API(base.Base): 'optional_args': {'is_quota_committed': False}, 'consistencygroup': consistencygroup, 'cgsnapshot': cgsnapshot, - 'multiattach': multiattach, + 'raw_multiattach': multiattach, 'group': group, 'group_snapshot': group_snapshot, 'source_group': source_group, @@ -349,8 +346,6 @@ class API(base.Base): # Refresh the object here, otherwise things ain't right vref = objects.Volume.get_by_id( context, vref['id']) - vref.multiattach = (self._is_multiattach(volume_type) or - multiattach) vref.save() LOG.info("Create volume request issued successfully.", resource=vref) diff --git a/cinder/volume/flows/api/create_volume.py b/cinder/volume/flows/api/create_volume.py index 9607d7169e9..1b2a804574d 100644 --- a/cinder/volume/flows/api/create_volume.py +++ b/cinder/volume/flows/api/create_volume.py @@ -69,7 +69,8 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): 'source_volid', 'volume_type', 'volume_type_id', 'encryption_key_id', 'consistencygroup_id', 'cgsnapshot_id', 'qos_specs', 'group_id', - 'refresh_az', 'backup_id', 'availability_zones']) + 'refresh_az', 'backup_id', 'availability_zones', + 'multiattach']) def __init__(self, image_service, availability_zones, **kwargs): super(ExtractVolumeRequestTask, self).__init__(addons=[ACTION], @@ -430,7 +431,8 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): def execute(self, context, size, snapshot, image_id, source_volume, availability_zone, volume_type, metadata, key_manager, - consistencygroup, cgsnapshot, group, group_snapshot, backup): + consistencygroup, cgsnapshot, group, group_snapshot, backup, + multiattach=False): utils.check_exclusive_options(snapshot=snapshot, imageRef=image_id, @@ -478,12 +480,22 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): source_volume, image_meta) - if encryption_key_id is not None and volume_type is not None: + if volume_type_id: + volume_type = objects.VolumeType.get_by_name_or_id( + context, volume_type_id) extra_specs = volume_type.get('extra_specs', {}) - if extra_specs.get('multiattach', '') == ' True': + # NOTE(tommylikehu): Although the parameter `multiattach` from + # create volume API is deprecated now, we still need to consider + # it when multiattach is not enabled in volume type. + multiattach = (extra_specs.get( + 'multiattach', '') == ' True' or multiattach) + if multiattach and encryption_key_id: msg = _('Multiattach cannot be used with encrypted volumes.') raise exception.InvalidVolume(reason=msg) + if multiattach: + context.authorize(policy.MULTIATTACH_POLICY) + specs = {} if volume_type_id: qos_specs = volume_types.get_volume_type_qos_specs(volume_type_id) @@ -517,6 +529,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): 'replication_status': replication_status, 'refresh_az': refresh_az, 'backup_id': backup_id, + 'multiattach': multiattach, 'availability_zones': availability_zones } @@ -858,7 +871,8 @@ def get_flow(db_api, image_service_api, availability_zones, create_what, availability_zones, rebind={'size': 'raw_size', 'availability_zone': 'raw_availability_zone', - 'volume_type': 'raw_volume_type'})) + 'volume_type': 'raw_volume_type', + 'multiattach': 'raw_multiattach'})) api_flow.add(QuotaReserveTask(), EntryCreateTask(), QuotaCommitTask())