Merge "Allow "server migrate" (not live) to take "--host" option"

2 files changed, 49 insertions, 15 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index 3e1deed5..1d022f03 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -1470,13 +1470,13 @@ class MigrateServer(command.Command):
                    'and ``--live-migration`` are used, ``--live-migration`` '
                    'takes priority.'),
         )
-        # TODO(mriedem): Add support for --os-compute-api-version >= 2.56 where
-        # you can cold migrate to a specified target host.
         host_group.add_argument(
             '--host',
             metavar='<hostname>',
-            help=_('Live migrate the server to the specified host. Requires '
-                   '``--os-compute-api-version`` 2.30 or greater.'),
+            help=_('Migrate the server to the specified host. Requires '
+                   '``--os-compute-api-version`` 2.30 or greater when used '
+                   'with the ``--live-migration`` option, otherwise requires '
+                   '``--os-compute-api-version`` 2.56 or greater.'),
         )
         migration_group = parser.add_mutually_exclusive_group()
         migration_group.add_argument(
@@ -1566,16 +1566,22 @@ class MigrateServer(command.Command):
                 kwargs['disk_over_commit'] = parsed_args.disk_overcommit
             server.live_migrate(**kwargs)
         else:
-            if (parsed_args.block_migration or parsed_args.disk_overcommit or
-                    parsed_args.host):
-                # TODO(mriedem): Allow --host for cold migration if
-                # --os-compute-api-version >= 2.56.
+            if parsed_args.block_migration or parsed_args.disk_overcommit:
                 raise exceptions.CommandError(
                     "--live-migration must be specified if "
-                    "--block-migration, --disk-overcommit or --host is "
+                    "--block-migration or --disk-overcommit is "
                     "specified")
+            if parsed_args.host:
+                if (compute_client.api_version <
+                        api_versions.APIVersion('2.56')):
+                    msg = _(
+                        '--os-compute-api-version 2.56 or greater is '
+                        'required to use --host without --live-migration.'
+                    )
+                    raise exceptions.CommandError(msg)
 
-            server.migrate()
+            kwargs = {'host': parsed_args.host} if parsed_args.host else {}
+            server.migrate(**kwargs)
 
         if parsed_args.wait:
             if utils.wait_for_status(
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
index 4a37a812..473de802 100644
--- a/openstackclient/tests/unit/compute/v2/test_server.py
+++ b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -2528,6 +2528,32 @@ class TestServerMigrate(TestServer):
         self.assertNotCalled(self.servers_mock.live_migrate)
         self.assertIsNone(result)
 
+    def test_server_migrate_with_host_2_56(self):
+        # Tests that --host is allowed for a cold migration
+        # for microversion 2.56 and greater.
+        arglist = [
+            '--host', 'fakehost', self.server.id,
+        ]
+        verifylist = [
+            ('live', None),
+            ('live_migration', False),
+            ('host', 'fakehost'),
+            ('block_migration', False),
+            ('disk_overcommit', False),
+            ('wait', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.56')
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.server.migrate.assert_called_with(host='fakehost')
+        self.assertNotCalled(self.servers_mock.live_migrate)
+        self.assertIsNone(result)
+
     def test_server_migrate_with_block_migration(self):
         arglist = [
             '--block-migration', self.server.id,
@@ -2566,8 +2592,9 @@ class TestServerMigrate(TestServer):
         self.assertNotCalled(self.servers_mock.live_migrate)
         self.assertNotCalled(self.servers_mock.migrate)
 
-    def test_server_migrate_with_host(self):
-        # Tests that --host is not allowed for a cold migration.
+    def test_server_migrate_with_host_pre_2_56(self):
+        # Tests that --host is not allowed for a cold migration
+        # before microversion 2.56 (the test defaults to 2.1).
         arglist = [
             '--host', 'fakehost', self.server.id,
         ]
@@ -2585,9 +2612,10 @@ class TestServerMigrate(TestServer):
                                parsed_args)
 
         # Make sure it's the error we expect.
-        self.assertIn("--live-migration must be specified if "
-                      "--block-migration, --disk-overcommit or --host is "
-                      "specified", six.text_type(ex))
+        self.assertIn('--os-compute-api-version 2.56 or greater is required '
+                      'to use --host without --live-migration.',
+                      six.text_type(ex))
+
         self.servers_mock.get.assert_called_with(self.server.id)
         self.assertNotCalled(self.servers_mock.live_migrate)
         self.assertNotCalled(self.servers_mock.migrate)