volume: Migrate 'volume migrate' to SDK

stephenfin · stephenfin · commit 5e5f12ba4019 · 2025-07-18T11:20:42.000+01:00
Change-Id: I99af5fce0c2c184e300dfbf5624b91eeed38c94b
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
diff --git a/openstackclient/tests/unit/volume/v2/test_volume.py b/openstackclient/tests/unit/volume/v2/test_volume.py
@@ -1293,69 +1293,80 @@ def test_volume_list_backward_compatibility(self):
             self.assertIn(self.mock_volume.name, each_volume)
 
 
-class TestVolumeMigrate(TestVolume):
-    _volume = volume_fakes.create_one_volume()
-
+class TestVolumeMigrate(volume_fakes.TestVolume):
     def setUp(self):
         super().setUp()
 
-        self.volumes_mock.get.return_value = self._volume
-        self.volumes_mock.migrate_volume.return_value = None
-        # Get the command object to test
+        self.volume = sdk_fakes.generate_fake_resource(_volume.Volume)
+        self.volume_sdk_client.find_volume.return_value = self.volume
+        self.volume_sdk_client.migrate_volume.return_value = None
+
         self.cmd = volume.MigrateVolume(self.app, None)
 
     def test_volume_migrate(self):
         arglist = [
             "--host",
             "host@backend-name#pool",
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", False),
             ("lock_volume", False),
             ("host", "host@backend-name#pool"),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.volumes_mock.get.assert_called_once_with(self._volume.id)
-        self.volumes_mock.migrate_volume.assert_called_once_with(
-            self._volume.id, "host@backend-name#pool", False, False
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_volume.assert_called_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.volume_sdk_client.migrate_volume.assert_called_once_with(
+            self.volume.id,
+            host="host@backend-name#pool",
+            force_host_copy=False,
+            lock_volume=False,
+        )
+
     def test_volume_migrate_with_option(self):
         arglist = [
             "--force-host-copy",
             "--lock-volume",
             "--host",
             "host@backend-name#pool",
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", True),
             ("lock_volume", True),
             ("host", "host@backend-name#pool"),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.volumes_mock.get.assert_called_once_with(self._volume.id)
-        self.volumes_mock.migrate_volume.assert_called_once_with(
-            self._volume.id, "host@backend-name#pool", True, True
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_volume.assert_called_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.volume_sdk_client.migrate_volume.assert_called_once_with(
+            self.volume.id,
+            host="host@backend-name#pool",
+            force_host_copy=True,
+            lock_volume=True,
+        )
+
     def test_volume_migrate_without_host(self):
         arglist = [
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", False),
             ("lock_volume", False),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
 
         self.assertRaises(
@@ -1366,6 +1377,9 @@ def test_volume_migrate_without_host(self):
             verifylist,
         )
 
+        self.volume_sdk_client.find_volume.assert_not_called()
+        self.volume_sdk_client.migrate_volume.assert_not_called()
+
 
 class TestVolumeSet(TestVolume):
     volume_type = volume_fakes.create_one_volume_type()
diff --git a/openstackclient/tests/unit/volume/v3/test_volume.py b/openstackclient/tests/unit/volume/v3/test_volume.py
@@ -1665,71 +1665,79 @@ def test_volume_list_backward_compatibility(self):
 
 
 class TestVolumeMigrate(volume_fakes.TestVolume):
-    _volume = volume_fakes.create_one_volume()
-
     def setUp(self):
         super().setUp()
 
-        self.volumes_mock = self.volume_client.volumes
-        self.volumes_mock.reset_mock()
+        self.volume = sdk_fakes.generate_fake_resource(_volume.Volume)
+        self.volume_sdk_client.find_volume.return_value = self.volume
+        self.volume_sdk_client.migrate_volume.return_value = None
 
-        self.volumes_mock.get.return_value = self._volume
-        self.volumes_mock.migrate_volume.return_value = None
-        # Get the command object to test
         self.cmd = volume.MigrateVolume(self.app, None)
 
     def test_volume_migrate(self):
         arglist = [
             "--host",
             "host@backend-name#pool",
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", False),
             ("lock_volume", False),
             ("host", "host@backend-name#pool"),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.volumes_mock.get.assert_called_once_with(self._volume.id)
-        self.volumes_mock.migrate_volume.assert_called_once_with(
-            self._volume.id, "host@backend-name#pool", False, False
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_volume.assert_called_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.volume_sdk_client.migrate_volume.assert_called_once_with(
+            self.volume.id,
+            host="host@backend-name#pool",
+            force_host_copy=False,
+            lock_volume=False,
+        )
+
     def test_volume_migrate_with_option(self):
         arglist = [
             "--force-host-copy",
             "--lock-volume",
             "--host",
             "host@backend-name#pool",
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", True),
             ("lock_volume", True),
             ("host", "host@backend-name#pool"),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         result = self.cmd.take_action(parsed_args)
-        self.volumes_mock.get.assert_called_once_with(self._volume.id)
-        self.volumes_mock.migrate_volume.assert_called_once_with(
-            self._volume.id, "host@backend-name#pool", True, True
-        )
         self.assertIsNone(result)
 
+        self.volume_sdk_client.find_volume.assert_called_with(
+            self.volume.id, ignore_missing=False
+        )
+        self.volume_sdk_client.migrate_volume.assert_called_once_with(
+            self.volume.id,
+            host="host@backend-name#pool",
+            force_host_copy=True,
+            lock_volume=True,
+        )
+
     def test_volume_migrate_without_host(self):
         arglist = [
-            self._volume.id,
+            self.volume.id,
         ]
         verifylist = [
             ("force_host_copy", False),
             ("lock_volume", False),
-            ("volume", self._volume.id),
+            ("volume", self.volume.id),
         ]
 
         self.assertRaises(
@@ -1740,6 +1748,9 @@ def test_volume_migrate_without_host(self):
             verifylist,
         )
 
+        self.volume_sdk_client.find_volume.assert_not_called()
+        self.volume_sdk_client.migrate_volume.assert_not_called()
+
 
 class TestVolumeSet(volume_fakes.TestVolume):
     volume_type = volume_fakes.create_one_volume_type()
diff --git a/openstackclient/volume/v2/volume.py b/openstackclient/volume/v2/volume.py
@@ -613,13 +613,15 @@ def get_parser(self, prog_name):
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
-        volume = utils.find_resource(volume_client.volumes, parsed_args.volume)
-        volume_client.volumes.migrate_volume(
+        volume_client = self.app.client_manager.sdk_connection.volume
+        volume = volume_client.find_volume(
+            parsed_args.volume, ignore_missing=False
+        )
+        volume_client.migrate_volume(
             volume.id,
-            parsed_args.host,
-            parsed_args.force_host_copy,
-            parsed_args.lock_volume,
+            host=parsed_args.host,
+            force_host_copy=parsed_args.force_host_copy,
+            lock_volume=parsed_args.lock_volume,
         )
 
 
diff --git a/openstackclient/volume/v3/volume.py b/openstackclient/volume/v3/volume.py
@@ -761,16 +761,19 @@ def get_parser(self, prog_name):
                 "(possibly by another operation)"
             ),
         )
+        # TODO(stephenfin): Add --cluster argument
         return parser
 
     def take_action(self, parsed_args):
-        volume_client = self.app.client_manager.volume
-        volume = utils.find_resource(volume_client.volumes, parsed_args.volume)
-        volume_client.volumes.migrate_volume(
+        volume_client = self.app.client_manager.sdk_connection.volume
+        volume = volume_client.find_volume(
+            parsed_args.volume, ignore_missing=False
+        )
+        volume_client.migrate_volume(
             volume.id,
-            parsed_args.host,
-            parsed_args.force_host_copy,
-            parsed_args.lock_volume,
+            host=parsed_args.host,
+            force_host_copy=parsed_args.force_host_copy,
+            lock_volume=parsed_args.lock_volume,
         )
 
 
