diff --git a/cinder/tests/unit/volume/test_volume_migration.py b/cinder/tests/unit/volume/test_volume_migration.py index 965ee09792a..73a8fbc72b8 100644 --- a/cinder/tests/unit/volume/test_volume_migration.py +++ b/cinder/tests/unit/volume/test_volume_migration.py @@ -105,52 +105,6 @@ class VolumeMigrationTestCase(base.BaseVolumeTestCase): self.assertEqual('newhost', volume.host) self.assertEqual('success', volume.migration_status) - @mock.patch('cinder.volume.manager.VolumeManager.' - '_can_use_driver_migration') - def test_migrate_volume_driver_for_retype(self, mock_can_use): - """Test volume migration done by driver on a retype.""" - # Mock driver and rpc functions - mock_driver = self.mock_object(self.volume.driver, 'migrate_volume', - return_value=(True, {})) - - volume = tests_utils.create_volume(self.context, size=0, - host=CONF.host, - migration_status='migrating') - host_obj = {'host': 'newhost', 'capabilities': {}} - self.volume.migrate_volume(self.context, volume, host_obj, False, - fake.VOLUME_TYPE2_ID, mock.sentinel.diff) - - mock_can_use.assert_called_once_with(mock.sentinel.diff) - mock_driver.assert_called_once_with(self.context, volume, host_obj) - # check volume properties - volume = objects.Volume.get_by_id(context.get_admin_context(), - volume.id) - self.assertEqual('newhost', volume.host) - self.assertEqual('success', volume.migration_status) - self.assertEqual(fake.VOLUME_TYPE2_ID, volume.volume_type_id) - - @mock.patch('cinder.volume.manager.VolumeManager._migrate_volume_generic') - @mock.patch('cinder.volume.manager.VolumeManager.' - '_can_use_driver_migration') - def test_migrate_volume_driver_for_retype_generic(self, mock_can_use, - mock_generic): - """Test generic volume migration on a retype after driver can't.""" - # Mock driver and rpc functions - mock_driver = self.mock_object(self.volume.driver, 'migrate_volume', - return_value=(False, None)) - - volume = tests_utils.create_volume(self.context, size=0, - host=CONF.host, - migration_status='migrating') - host_obj = {'host': 'newhost', 'capabilities': {}} - self.volume.migrate_volume(self.context, volume, host_obj, False, - fake.VOLUME_TYPE2_ID, mock.sentinel.diff) - - mock_can_use.assert_called_once_with(mock.sentinel.diff) - mock_driver.assert_called_once_with(self.context, volume, host_obj) - mock_generic.assert_called_once_with(self.context, volume, host_obj, - fake.VOLUME_TYPE2_ID) - def test_migrate_volume_driver_cross_az(self): """Test volume migration done by driver.""" # Mock driver and rpc functions @@ -1068,21 +1022,3 @@ class VolumeMigrationTestCase(base.BaseVolumeTestCase): self.volume.retype(self.context, volume, new_vol_type.id, host_obj, migration_policy='on-demand') vt_get.assert_not_called() - - @ddt.data( - (None, True), - ({'encryption': {'cipher': ('v1', 'v2')}}, False), - ({'qos_specs': {'key1': ('v1', 'v2')}}, False), - ({'encryption': {}, 'qos_specs': {}, 'extra_specs': {}}, True), - ({'encryption': {}, 'qos_specs': {}, - 'extra_specs': {'volume_backend_name': ('ceph1', 'ceph2'), - 'RESKEY:availability_zones': ('nova', 'nova2')}}, - True), - ({'encryption': {}, 'qos_specs': {}, - 'extra_specs': {'thin_provisioning_support': (' True', None)}}, - False), - ) - @ddt.unpack - def test__can_use_driver_migration(self, diff, expected): - res = self.volume._can_use_driver_migration(diff) - self.assertEqual(expected, res) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 3aa32c66ee0..0774390a29f 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -2620,35 +2620,12 @@ class VolumeManager(manager.CleanableManager, resource=volume) return volume.id - def _can_use_driver_migration(self, diff): - """Return when we can use driver assisted migration on a retype.""" - # We can if there's no retype or there are no difference in the types - if not diff: - return True - - extra_specs = diff.get('extra_specs') - qos = diff.get('qos_specs') - enc = diff.get('encryption') - - # We cant' if QoS or Encryption changes and we can if there are no - # extra specs changes. - if qos or enc or not extra_specs: - return not (qos or enc) - - # We can use driver assisted migration if we only change the backend - # name, and the AZ. - extra_specs = extra_specs.copy() - extra_specs.pop('volume_backend_name', None) - extra_specs.pop('RESKEY:availability_zones', None) - return not extra_specs - def migrate_volume(self, ctxt: context.RequestContext, volume, host, force_host_copy: bool = False, - new_type_id=None, - diff=None) -> None: + new_type_id=None) -> None: """Migrate the volume to the specified host (called on source host).""" try: volume_utils.require_driver_initialized(self.driver) @@ -2666,7 +2643,7 @@ class VolumeManager(manager.CleanableManager, volume.migration_status = 'migrating' volume.save() - if not force_host_copy and self._can_use_driver_migration(diff): + if not force_host_copy and new_type_id is None: try: LOG.debug("Issue driver.migrate_volume.", resource=volume) moved, model_update = self.driver.migrate_volume(ctxt, @@ -2685,8 +2662,6 @@ class VolumeManager(manager.CleanableManager, updates.update(status_update) if model_update: updates.update(model_update) - if new_type_id: - updates['volume_type_id'] = new_type_id volume.update(updates) volume.save() except Exception: @@ -3138,7 +3113,7 @@ class VolumeManager(manager.CleanableManager, try: self.migrate_volume(context, volume, host, - new_type_id=new_type_id, diff=diff) + new_type_id=new_type_id) except Exception: with excutils.save_and_reraise_exception(): _retype_error(context, volume, old_reservations, diff --git a/releasenotes/notes/retype-assisted-migration-6cdc7f9b21beb859.yaml b/releasenotes/notes/retype-assisted-migration-6cdc7f9b21beb859.yaml deleted file mode 100644 index 54e513d8a17..00000000000 --- a/releasenotes/notes/retype-assisted-migration-6cdc7f9b21beb859.yaml +++ /dev/null @@ -1,7 +0,0 @@ ---- -fixes: - - | - `Bug #1886543 `_: - On retypes requiring a migration, try to use the driver assisted mechanism - when moving from one backend to another when we know it's safe from the - volume type perspective.