Merge "Revert "Driver assisted migration on retype when it's safe"" into stable/2023.1

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': ('<is> 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)

cinder/volume/manager.py

@@ -2622,35 +2622,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,
+                       new_type_id=None) -> None:
-                       diff=None) -> None:
         """Migrate the volume to the specified host (called on source host)."""
         try:
             volume_utils.require_driver_initialized(self.driver)
@@ -2668,7 +2645,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,
@@ -2687,8 +2664,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:
@@ -3146,7 +3121,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,

releasenotes/notes/retype-assisted-migration-6cdc7f9b21beb859.yaml

@@ -1,7 +0,0 @@
----
-fixes:
-  - |
-    `Bug #1886543 <https://bugs.launchpad.net/cinder/+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.