Merge "Use the SDK in server migration list"

5 files changed, 97 insertions, 84 deletions

diff --git a/openstackclient/compute/v2/server_migration.py b/openstackclient/compute/v2/server_migration.py
index 919b67bd..016d15d7 100644
--- a/openstackclient/compute/v2/server_migration.py
+++ b/openstackclient/compute/v2/server_migration.py
@@ -15,6 +15,7 @@
 import uuid
 
 from novaclient import api_versions
+from openstack import utils as sdk_utils
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -130,22 +131,22 @@ class ListMigration(command.Lister):
         # the same as the column header names.
         columns = [
             'source_node', 'dest_node', 'source_compute', 'dest_compute',
-            'dest_host', 'status', 'instance_uuid', 'old_instance_type_id',
-            'new_instance_type_id', 'created_at', 'updated_at',
+            'dest_host', 'status', 'server_id', 'old_flavor_id',
+            'new_flavor_id', 'created_at', 'updated_at',
         ]
 
         # Insert migrations UUID after ID
-        if compute_client.api_version >= api_versions.APIVersion("2.59"):
+        if sdk_utils.supports_microversion(compute_client, "2.59"):
             column_headers.insert(0, "UUID")
             columns.insert(0, "uuid")
 
-        if compute_client.api_version >= api_versions.APIVersion("2.23"):
+        if sdk_utils.supports_microversion(compute_client, "2.23"):
             column_headers.insert(0, "Id")
             columns.insert(0, "id")
             column_headers.insert(len(column_headers) - 2, "Type")
             columns.insert(len(columns) - 2, "migration_type")
 
-        if compute_client.api_version >= api_versions.APIVersion("2.80"):
+        if sdk_utils.supports_microversion(compute_client, "2.80"):
             if parsed_args.project:
                 column_headers.insert(len(column_headers) - 2, "Project")
                 columns.insert(len(columns) - 2, "project_id")
@@ -159,19 +160,23 @@ class ListMigration(command.Lister):
         )
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
         identity_client = self.app.client_manager.identity
 
-        search_opts = {
-            'host': parsed_args.host,
-            'status': parsed_args.status,
-        }
+        search_opts = {}
+
+        if parsed_args.host is not None:
+            search_opts['host'] = parsed_args.host
+
+        if parsed_args.status is not None:
+            search_opts['status'] = parsed_args.status
 
         if parsed_args.server:
-            search_opts['instance_uuid'] = utils.find_resource(
-                compute_client.servers,
-                parsed_args.server,
-            ).id
+            server = compute_client.find_server(parsed_args.server)
+            if server is None:
+                msg = _('Unable to find server: %s') % parsed_args.server
+                raise exceptions.CommandError(msg)
+            search_opts['instance_uuid'] = server.id
 
         if parsed_args.type:
             migration_type = parsed_args.type
@@ -181,7 +186,7 @@ class ListMigration(command.Lister):
             search_opts['migration_type'] = migration_type
 
         if parsed_args.marker:
-            if compute_client.api_version < api_versions.APIVersion('2.59'):
+            if not sdk_utils.supports_microversion(compute_client, "2.59"):
                 msg = _(
                     '--os-compute-api-version 2.59 or greater is required to '
                     'support the --marker option'
@@ -190,16 +195,17 @@ class ListMigration(command.Lister):
             search_opts['marker'] = parsed_args.marker
 
         if parsed_args.limit:
-            if compute_client.api_version < api_versions.APIVersion('2.59'):
+            if not sdk_utils.supports_microversion(compute_client, "2.59"):
                 msg = _(
                     '--os-compute-api-version 2.59 or greater is required to '
                     'support the --limit option'
                 )
                 raise exceptions.CommandError(msg)
             search_opts['limit'] = parsed_args.limit
+            search_opts['paginated'] = False
 
         if parsed_args.changes_since:
-            if compute_client.api_version < api_versions.APIVersion('2.59'):
+            if not sdk_utils.supports_microversion(compute_client, "2.59"):
                 msg = _(
                     '--os-compute-api-version 2.59 or greater is required to '
                     'support the --changes-since option'
@@ -208,7 +214,7 @@ class ListMigration(command.Lister):
             search_opts['changes_since'] = parsed_args.changes_since
 
         if parsed_args.changes_before:
-            if compute_client.api_version < api_versions.APIVersion('2.66'):
+            if not sdk_utils.supports_microversion(compute_client, "2.66"):
                 msg = _(
                     '--os-compute-api-version 2.66 or greater is required to '
                     'support the --changes-before option'
@@ -217,7 +223,7 @@ class ListMigration(command.Lister):
             search_opts['changes_before'] = parsed_args.changes_before
 
         if parsed_args.project:
-            if compute_client.api_version < api_versions.APIVersion('2.80'):
+            if not sdk_utils.supports_microversion(compute_client, "2.80"):
                 msg = _(
                     '--os-compute-api-version 2.80 or greater is required to '
                     'support the --project option'
@@ -231,7 +237,7 @@ class ListMigration(command.Lister):
             ).id
 
         if parsed_args.user:
-            if compute_client.api_version < api_versions.APIVersion('2.80'):
+            if not sdk_utils.supports_microversion(compute_client, "2.80"):
                 msg = _(
                     '--os-compute-api-version 2.80 or greater is required to '
                     'support the --user option'
@@ -244,7 +250,7 @@ class ListMigration(command.Lister):
                 parsed_args.user_domain,
             ).id
 
-        migrations = compute_client.migrations.list(**search_opts)
+        migrations = list(compute_client.migrations(**search_opts))
 
         return self.print_migrations(parsed_args, compute_client, migrations)
 
diff --git a/openstackclient/tests/functional/compute/v2/test_server.py b/openstackclient/tests/functional/compute/v2/test_server.py
index b1afe875..37183a79 100644
--- a/openstackclient/tests/functional/compute/v2/test_server.py
+++ b/openstackclient/tests/functional/compute/v2/test_server.py
@@ -1415,3 +1415,9 @@ class ServerTests(common.ComputeTestCase):
 
         raw_output = self.openstack('server volume list ' + server_name)
         self.assertEqual('\n', raw_output)
+
+    def test_server_migration_list(self):
+        # Verify that the command does not raise an exception when we list
+        # migrations, including when we specify a query.
+        self.openstack('server migration list')
+        self.openstack('server migration list --limit 1')
diff --git a/openstackclient/tests/unit/compute/v2/test_server_migration.py b/openstackclient/tests/unit/compute/v2/test_server_migration.py
index c4cbac47..93c1865a 100644
--- a/openstackclient/tests/unit/compute/v2/test_server_migration.py
+++ b/openstackclient/tests/unit/compute/v2/test_server_migration.py
@@ -13,6 +13,7 @@
 from unittest import mock
 
 from novaclient import api_versions
+from openstack import utils as sdk_utils
 from osc_lib import exceptions
 from osc_lib import utils as common_utils
 
@@ -35,10 +36,6 @@ class TestServerMigration(compute_fakes.TestComputev2):
             self.app.client_manager.compute.server_migrations
         self.server_migrations_mock.reset_mock()
 
-        # Get a shortcut to the compute client MigrationManager mock
-        self.migrations_mock = self.app.client_manager.compute.migrations
-        self.migrations_mock.reset_mock()
-
         self.app.client_manager.sdk_connection = mock.Mock()
         self.app.client_manager.sdk_connection.compute = mock.Mock()
         self.sdk_client = self.app.client_manager.sdk_connection.compute
@@ -53,22 +50,21 @@ class TestListMigration(TestServerMigration):
         'Old Flavor', 'New Flavor', 'Created At', 'Updated At'
     ]
 
-    # These are the fields that come back in the response from the REST API.
     MIGRATION_FIELDS = [
         'source_node', 'dest_node', 'source_compute', 'dest_compute',
-        'dest_host', 'status', 'instance_uuid', 'old_instance_type_id',
-        'new_instance_type_id', 'created_at', 'updated_at'
+        'dest_host', 'status', 'server_id', 'old_flavor_id',
+        'new_flavor_id', 'created_at', 'updated_at'
     ]
 
     def setUp(self):
         super().setUp()
 
         self.server = compute_fakes.FakeServer.create_one_server()
-        self.servers_mock.get.return_value = self.server
+        self.sdk_client.find_server.return_value = self.server
 
         self.migrations = compute_fakes.FakeMigration.create_migrations(
             count=3)
-        self.migrations_mock.list.return_value = self.migrations
+        self.sdk_client.migrations.return_value = self.migrations
 
         self.data = (common_utils.get_item_properties(
             s, self.MIGRATION_FIELDS) for s in self.migrations)
@@ -76,6 +72,20 @@ class TestListMigration(TestServerMigration):
         # Get the command object to test
         self.cmd = server_migration.ListMigration(self.app, None)
 
+        patcher = mock.patch.object(
+            sdk_utils, 'supports_microversion', return_value=True)
+        self.addCleanup(patcher.stop)
+        self.supports_microversion_mock = patcher.start()
+        self._set_mock_microversion(
+            self.app.client_manager.compute.api_version.get_string())
+
+    def _set_mock_microversion(self, mock_v):
+        """Set a specific microversion for the mock supports_microversion()."""
+        self.supports_microversion_mock.reset_mock(return_value=True)
+        self.supports_microversion_mock.side_effect = (
+            lambda _, v:
+            api_versions.APIVersion(v) <= api_versions.APIVersion(mock_v))
+
     def test_server_migration_list_no_options(self):
         arglist = []
         verifylist = []
@@ -84,12 +94,9 @@ class TestListMigration(TestServerMigration):
         columns, data = self.cmd.take_action(parsed_args)
 
         # Set expected values
-        kwargs = {
-            'status': None,
-            'host': None,
-        }
+        kwargs = {}
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.assertEqual(self.MIGRATION_COLUMNS, columns)
         self.assertEqual(tuple(self.data), tuple(data))
@@ -117,8 +124,8 @@ class TestListMigration(TestServerMigration):
             'migration_type': 'migration',
         }
 
-        self.servers_mock.get.assert_called_with('server1')
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.find_server.assert_called_with('server1')
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.assertEqual(self.MIGRATION_COLUMNS, columns)
         self.assertEqual(tuple(self.data), tuple(data))
@@ -133,18 +140,17 @@ class TestListMigrationV223(TestListMigration):
         'Type', 'Created At', 'Updated At'
     ]
 
-    # These are the fields that come back in the response from the REST API.
+    # These are the Migration object fields.
     MIGRATION_FIELDS = [
         'id', 'source_node', 'dest_node', 'source_compute', 'dest_compute',
-        'dest_host', 'status', 'instance_uuid', 'old_instance_type_id',
-        'new_instance_type_id', 'migration_type', 'created_at', 'updated_at'
+        'dest_host', 'status', 'server_id', 'old_flavor_id',
+        'new_flavor_id', 'migration_type', 'created_at', 'updated_at'
     ]
 
     def setUp(self):
         super().setUp()
 
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.23')
+        self._set_mock_microversion('2.23')
 
     def test_server_migration_list(self):
         arglist = [
@@ -159,10 +165,9 @@ class TestListMigrationV223(TestListMigration):
         # Set expected values
         kwargs = {
             'status': 'migrating',
-            'host': None,
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.assertEqual(self.MIGRATION_COLUMNS, columns)
         self.assertEqual(tuple(self.data), tuple(data))
@@ -177,19 +182,18 @@ class TestListMigrationV259(TestListMigration):
         'Old Flavor', 'New Flavor', 'Type', 'Created At', 'Updated At'
     ]
 
-    # These are the fields that come back in the response from the REST API.
+    # These are the Migration object fields.
     MIGRATION_FIELDS = [
         'id', 'uuid', 'source_node', 'dest_node', 'source_compute',
-        'dest_compute', 'dest_host', 'status', 'instance_uuid',
-        'old_instance_type_id', 'new_instance_type_id', 'migration_type',
+        'dest_compute', 'dest_host', 'status', 'server_id',
+        'old_flavor_id', 'new_flavor_id', 'migration_type',
         'created_at', 'updated_at'
     ]
 
     def setUp(self):
         super().setUp()
 
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.59')
+        self._set_mock_microversion('2.59')
 
     def test_server_migration_list(self):
         arglist = [
@@ -211,19 +215,18 @@ class TestListMigrationV259(TestListMigration):
         kwargs = {
             'status': 'migrating',
             'limit': 1,
+            'paginated': False,
             'marker': 'test_kp',
-            'host': None,
             'changes_since': '2019-08-09T08:03:25Z',
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.assertEqual(self.MIGRATION_COLUMNS, columns)
         self.assertEqual(tuple(self.data), tuple(data))
 
     def test_server_migration_list_with_limit_pre_v259(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.58')
+        self._set_mock_microversion('2.58')
         arglist = [
             '--status', 'migrating',
             '--limit', '1'
@@ -242,8 +245,7 @@ class TestListMigrationV259(TestListMigration):
             str(ex))
 
     def test_server_migration_list_with_marker_pre_v259(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.58')
+        self._set_mock_microversion('2.58')
         arglist = [
             '--status', 'migrating',
             '--marker', 'test_kp'
@@ -262,8 +264,7 @@ class TestListMigrationV259(TestListMigration):
             str(ex))
 
     def test_server_migration_list_with_changes_since_pre_v259(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.58')
+        self._set_mock_microversion('2.58')
         arglist = [
             '--status', 'migrating',
             '--changes-since', '2019-08-09T08:03:25Z'
@@ -291,19 +292,18 @@ class TestListMigrationV266(TestListMigration):
         'Old Flavor', 'New Flavor', 'Type', 'Created At', 'Updated At'
     ]
 
-    # These are the fields that come back in the response from the REST API.
+    # These are the Migration object fields.
     MIGRATION_FIELDS = [
         'id', 'uuid', 'source_node', 'dest_node', 'source_compute',
-        'dest_compute', 'dest_host', 'status', 'instance_uuid',
-        'old_instance_type_id', 'new_instance_type_id', 'migration_type',
+        'dest_compute', 'dest_host', 'status', 'server_id',
+        'old_flavor_id', 'new_flavor_id', 'migration_type',
         'created_at', 'updated_at'
     ]
 
     def setUp(self):
         super().setUp()
 
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.66')
+        self._set_mock_microversion('2.66')
 
     def test_server_migration_list_with_changes_before(self):
         arglist = [
@@ -327,20 +327,19 @@ class TestListMigrationV266(TestListMigration):
         kwargs = {
             'status': 'migrating',
             'limit': 1,
+            'paginated': False,
             'marker': 'test_kp',
-            'host': None,
             'changes_since': '2019-08-07T08:03:25Z',
             'changes_before': '2019-08-09T08:03:25Z',
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.assertEqual(self.MIGRATION_COLUMNS, columns)
         self.assertEqual(tuple(self.data), tuple(data))
 
     def test_server_migration_list_with_changes_before_pre_v266(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.65')
+        self._set_mock_microversion('2.65')
         arglist = [
             '--status', 'migrating',
             '--changes-before', '2019-08-09T08:03:25Z'
@@ -368,11 +367,11 @@ class TestListMigrationV280(TestListMigration):
         'Old Flavor', 'New Flavor', 'Type', 'Created At', 'Updated At'
     ]
 
-    # These are the fields that come back in the response from the REST API.
+    # These are the Migration object fields.
     MIGRATION_FIELDS = [
         'id', 'uuid', 'source_node', 'dest_node', 'source_compute',
-        'dest_compute', 'dest_host', 'status', 'instance_uuid',
-        'old_instance_type_id', 'new_instance_type_id', 'migration_type',
+        'dest_compute', 'dest_host', 'status', 'server_id',
+        'old_flavor_id', 'new_flavor_id', 'migration_type',
         'created_at', 'updated_at'
     ]
 
@@ -391,8 +390,7 @@ class TestListMigrationV280(TestListMigration):
         self.projects_mock.get.return_value = self.project
         self.users_mock.get.return_value = self.user
 
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.80')
+        self._set_mock_microversion('2.80')
 
     def test_server_migration_list_with_project(self):
         arglist = [
@@ -418,14 +416,14 @@ class TestListMigrationV280(TestListMigration):
         kwargs = {
             'status': 'migrating',
             'limit': 1,
+            'paginated': False,
             'marker': 'test_kp',
-            'host': None,
             'project_id': self.project.id,
             'changes_since': '2019-08-07T08:03:25Z',
             'changes_before': "2019-08-09T08:03:25Z",
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.MIGRATION_COLUMNS.insert(
             len(self.MIGRATION_COLUMNS) - 2, "Project")
@@ -439,8 +437,7 @@ class TestListMigrationV280(TestListMigration):
         self.MIGRATION_FIELDS.remove('project_id')
 
     def test_get_migrations_with_project_pre_v280(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.79')
+        self._set_mock_microversion('2.79')
         arglist = [
             '--status', 'migrating',
             '--changes-before', '2019-08-09T08:03:25Z',
@@ -478,20 +475,21 @@ class TestListMigrationV280(TestListMigration):
             ('user', self.user.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
         columns, data = self.cmd.take_action(parsed_args)
 
         # Set expected values
         kwargs = {
             'status': 'migrating',
             'limit': 1,
+            'paginated': False,
             'marker': 'test_kp',
-            'host': None,
             'user_id': self.user.id,
             'changes_since': '2019-08-07T08:03:25Z',
             'changes_before': "2019-08-09T08:03:25Z",
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.MIGRATION_COLUMNS.insert(
             len(self.MIGRATION_COLUMNS) - 2, "User")
@@ -505,8 +503,7 @@ class TestListMigrationV280(TestListMigration):
         self.MIGRATION_FIELDS.remove('user_id')
 
     def test_get_migrations_with_user_pre_v280(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.79')
+        self._set_mock_microversion('2.79')
         arglist = [
             '--status', 'migrating',
             '--changes-before', '2019-08-09T08:03:25Z',
@@ -550,14 +547,14 @@ class TestListMigrationV280(TestListMigration):
         kwargs = {
             'status': 'migrating',
             'limit': 1,
-            'host': None,
+            'paginated': False,
             'project_id': self.project.id,
             'user_id': self.user.id,
             'changes_since': '2019-08-07T08:03:25Z',
             'changes_before': "2019-08-09T08:03:25Z",
         }
 
-        self.migrations_mock.list.assert_called_with(**kwargs)
+        self.sdk_client.migrations.assert_called_with(**kwargs)
 
         self.MIGRATION_COLUMNS.insert(
             len(self.MIGRATION_COLUMNS) - 2, "Project")
@@ -576,8 +573,7 @@ class TestListMigrationV280(TestListMigration):
         self.MIGRATION_FIELDS.remove('user_id')
 
     def test_get_migrations_with_project_and_user_pre_v280(self):
-        self.app.client_manager.compute.api_version = api_versions.APIVersion(
-            '2.79')
+        self._set_mock_microversion('2.79')
         arglist = [
             '--status', 'migrating',
             '--changes-before', '2019-08-09T08:03:25Z',
diff --git a/releasenotes/notes/switch-server-migration-to-sdk-4e4530f787f90fd2.yaml b/releasenotes/notes/switch-server-migration-to-sdk-4e4530f787f90fd2.yaml
new file mode 100644
index 00000000..318ac097
--- /dev/null
+++ b/releasenotes/notes/switch-server-migration-to-sdk-4e4530f787f90fd2.yaml
@@ -0,0 +1,5 @@
+---
+features:
+  - |
+    The ``server migration *`` commands now use the OpenStackSDK instead of
+    novaclient.
diff --git a/requirements.txt b/requirements.txt
index c787ef76..1ae8cec4 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -6,7 +6,7 @@ pbr!=2.1.0,>=2.0.0 # Apache-2.0
 
 cliff>=3.5.0 # Apache-2.0
 iso8601>=0.1.11 # MIT
-openstacksdk>=0.102.0 # Apache-2.0
+openstacksdk>=0.103.0 # Apache-2.0
 osc-lib>=2.3.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.utils>=3.33.0 # Apache-2.0