compute: Allow retrieval of migration by UUID

stephenfin · stephenfin · commit cffec4517f8b · 2022-03-09T17:31:44.000Z
The nova API doesn't allow you to retrieve migration records by UUID,
only ID. This is confusing. Work around it by listing records and
filtering this list.

Change-Id: I932c9c70420e85056509513e005bb78168e70611
Signed-off-by: Stephen Finucane &lt;sfinucan@redhat.com&gt;
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
@@ -21,6 +21,7 @@
 import json
 import logging
 import os
+import uuid
 
 from cliff import columns as cliff_columns
 import iso8601
@@ -3037,6 +3038,18 @@ def take_action(self, parsed_args):
         return self.print_migrations(parsed_args, compute_client, migrations)
 
 
+def _get_migration_by_uuid(compute_client, server_id, migration_uuid):
+    for migration in compute_client.server_migrations.list(server_id):
+        if migration.uuid == migration_uuid:
+            return migration
+            break
+    else:
+        msg = _(
+            'In-progress live migration %s is not found for server %s.'
+        )
+        raise exceptions.CommandError(msg % (migration_uuid, server_id))
+
+
 class ShowMigration(command.ShowOne):
     """Show an in-progress live migration for a given server.
 
@@ -3069,13 +3082,38 @@ def take_action(self, parsed_args):
             )
             raise exceptions.CommandError(msg)
 
+        if not parsed_args.migration.isdigit():
+            try:
+                uuid.UUID(parsed_args.migration)
+            except ValueError:
+                msg = _(
+                    'The <migration> argument must be an ID or UUID'
+                )
+                raise exceptions.CommandError(msg)
+
+            if compute_client.api_version < api_versions.APIVersion('2.59'):
+                msg = _(
+                    '--os-compute-api-version 2.59 or greater is required to '
+                    'retrieve server migrations by UUID'
+                )
+                raise exceptions.CommandError(msg)
+
         server = utils.find_resource(
             compute_client.servers,
             parsed_args.server,
         )
-        server_migration = compute_client.server_migrations.get(
-            server.id, parsed_args.migration,
-        )
+
+        # the nova API doesn't currently allow retrieval by UUID but it's a
+        # reasonably common operation so emulate this behavior by listing
+        # migrations - the responses are identical
+        if not parsed_args.migration.isdigit():
+            server_migration = _get_migration_by_uuid(
+                compute_client, server.id, parsed_args.migration,
+            )
+        else:
+            server_migration = compute_client.server_migrations.get(
+                server.id, parsed_args.migration,
+            )
 
         columns = (
             'ID',
@@ -3136,12 +3174,39 @@ def take_action(self, parsed_args):
             )
             raise exceptions.CommandError(msg)
 
+        if not parsed_args.migration.isdigit():
+            try:
+                uuid.UUID(parsed_args.migration)
+            except ValueError:
+                msg = _(
+                    'The <migration> argument must be an ID or UUID'
+                )
+                raise exceptions.CommandError(msg)
+
+            if compute_client.api_version < api_versions.APIVersion('2.59'):
+                msg = _(
+                    '--os-compute-api-version 2.59 or greater is required to '
+                    'abort server migrations by UUID'
+                )
+                raise exceptions.CommandError(msg)
+
         server = utils.find_resource(
             compute_client.servers,
             parsed_args.server,
         )
+
+        # the nova API doesn't currently allow retrieval by UUID but it's a
+        # reasonably common operation so emulate this behavior by listing
+        # migrations - the responses are identical
+        migration_id = parsed_args.migration
+        if not parsed_args.migration.isdigit():
+            migration_id = _get_migration_by_uuid(
+                compute_client, server.id, parsed_args.migration,
+            ).id
+
         compute_client.server_migrations.live_migration_abort(
-            server.id, parsed_args.migration)
+            server.id, migration_id,
+        )
 
 
 class ForceCompleteMigration(command.Command):
@@ -3174,12 +3239,39 @@ def take_action(self, parsed_args):
             )
             raise exceptions.CommandError(msg)
 
+        if not parsed_args.migration.isdigit():
+            try:
+                uuid.UUID(parsed_args.migration)
+            except ValueError:
+                msg = _(
+                    'The <migration> argument must be an ID or UUID'
+                )
+                raise exceptions.CommandError(msg)
+
+            if compute_client.api_version < api_versions.APIVersion('2.59'):
+                msg = _(
+                    '--os-compute-api-version 2.59 or greater is required to '
+                    'abort server migrations by UUID'
+                )
+                raise exceptions.CommandError(msg)
+
         server = utils.find_resource(
             compute_client.servers,
             parsed_args.server,
         )
+
+        # the nova API doesn't currently allow retrieval by UUID but it's a
+        # reasonably common operation so emulate this behavior by listing
+        # migrations - the responses are identical
+        migration_id = parsed_args.migration
+        if not parsed_args.migration.isdigit():
+            migration_id = _get_migration_by_uuid(
+                compute_client, server.id, parsed_args.migration,
+            ).id
+
         compute_client.server_migrations.live_migrate_force_complete(
-            server.id, parsed_args.migration)
+            server.id, migration_id,
+        )
 
 
 class PauseServer(command.Command):
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -6232,6 +6232,88 @@ def test_server_migration_show_pre_v224(self):
             '--os-compute-api-version 2.24 or greater is required',
             str(ex))
 
+    def test_server_migration_show_by_uuid(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+        self.server_migrations_mock.list.return_value = [self.server_migration]
+
+        self.columns += ('UUID',)
+        self.data += (self.server_migration.uuid,)
+
+        arglist = [
+            self.server.id,
+            self.server_migration.uuid,  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        columns, data = self.cmd.take_action(parsed_args)
+
+        self.assertEqual(self.columns, columns)
+        self.assertEqual(self.data, data)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server_migrations_mock.list.assert_called_with(self.server.id)
+        self.server_migrations_mock.get.assert_not_called()
+
+    def test_server_migration_show_by_uuid_no_matches(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+        self.server_migrations_mock.list.return_value = []
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            'In-progress live migration 69f95745-bfe3-4302-90f7-5b0022cba1ce',
+            str(ex))
+
+    def test_server_migration_show_by_uuid_pre_v259(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.58')
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.59 or greater is required',
+            str(ex))
+
+    def test_server_migration_show_invalid_id(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.24')
+
+        arglist = [
+            self.server.id,
+            'foo',  # invalid migration ID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            'The <migration> argument must be an ID or UUID',
+            str(ex))
+
 
 class TestServerMigrationAbort(TestServer):
 
@@ -6283,6 +6365,69 @@ def test_migration_abort_pre_v224(self):
             '--os-compute-api-version 2.24 or greater is required',
             str(ex))
 
+    def test_server_migration_abort_by_uuid(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+
+        self.server_migration = compute_fakes.FakeServerMigration\
+            .create_one_server_migration()
+        self.server_migrations_mock.list.return_value = [self.server_migration]
+
+        arglist = [
+            self.server.id,
+            self.server_migration.uuid,  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server_migrations_mock.list.assert_called_with(self.server.id)
+        self.server_migrations_mock.live_migration_abort.assert_called_with(
+            self.server.id, self.server_migration.id)
+        self.assertIsNone(result)
+
+    def test_server_migration_abort_by_uuid_no_matches(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+
+        self.server_migrations_mock.list.return_value = []
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            'In-progress live migration 69f95745-bfe3-4302-90f7-5b0022cba1ce',
+            str(ex))
+
+    def test_server_migration_abort_by_uuid_pre_v259(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.58')
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.59 or greater is required',
+            str(ex))
+
 
 class TestServerMigrationForceComplete(TestServer):
 
@@ -6334,6 +6479,69 @@ def test_migration_force_complete_pre_v222(self):
             '--os-compute-api-version 2.22 or greater is required',
             str(ex))
 
+    def test_server_migration_force_complete_by_uuid(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+
+        self.server_migration = compute_fakes.FakeServerMigration\
+            .create_one_server_migration()
+        self.server_migrations_mock.list.return_value = [self.server_migration]
+
+        arglist = [
+            self.server.id,
+            self.server_migration.uuid,  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server_migrations_mock.list.assert_called_with(self.server.id)
+        self.server_migrations_mock.live_migrate_force_complete\
+            .assert_called_with(self.server.id, self.server_migration.id)
+        self.assertIsNone(result)
+
+    def test_server_migration_force_complete_by_uuid_no_matches(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.59')
+
+        self.server_migrations_mock.list.return_value = []
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            'In-progress live migration 69f95745-bfe3-4302-90f7-5b0022cba1ce',
+            str(ex))
+
+    def test_server_migration_force_complete_by_uuid_pre_v259(self):
+        self.app.client_manager.compute.api_version = api_versions.APIVersion(
+            '2.58')
+
+        arglist = [
+            self.server.id,
+            '69f95745-bfe3-4302-90f7-5b0022cba1ce',  # arbitrary migration UUID
+        ]
+        verifylist = []
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        ex = self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+        self.assertIn(
+            '--os-compute-api-version 2.59 or greater is required',
+            str(ex))
+
 
 class TestServerPause(TestServer):
 
diff --git a/releasenotes/notes/server-migration-by-uuid-59f8272f63abee5d.yaml b/releasenotes/notes/server-migration-by-uuid-59f8272f63abee5d.yaml
@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    The ``server migration abort``, ``server migration force complete`` and
+    ``server migration show`` commands will now accept a server migration UUID
+    in addition to an ID.
