Deprecate openstack server migrate --host option

Per the discussion at the Train Forum [1] this deprecates
the problematic --live option on the server migrate command
which, depending on the compute API version used, forcefully
bypasses the scheduler and also does not allow you to live
migrate a server and let the scheduler pick a host.

The --live option is replaced here with two new options:

* --live-migration: this simply tells the command you want to
  perform a live rather than cold migration; if specified with
  --live the --live-migration option takes priority.

* --host: when specified, this will request a target host for
  the live migration and will be validated by the scheduler;
  if not specified, the scheduler will pick a host. This option
  is mutually exclusive with --live.

We can build on the --host option by supporting cold migrations
with a specified host when using compute API version 2.56 or
greater but that will come in a separate change.

If the --live option is ever used we log a warning.

Note there are several related changes for this issue:

- https://review.openstack.org/#/c/628334/
- https://review.openstack.org/#/c/626949/
- https://review.openstack.org/#/c/627801/
- https://review.openstack.org/#/c/589012/
- https://review.openstack.org/#/c/460059/

This change allows us to deprecate the --live option and provide
a replacement which is backward compatible without having to use
something potentially error-prone like nargs='?'.

Closes-Bug: #1411190

[1] https://etherpad.openstack.org/p/DEN-osc-compute-api-gaps

Change-Id: I95d3d588e4abeb6848bdccf6915f7b5da40b5d4f

1 files changed, 71 insertions, 7 deletions

diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
index cb9f8d43..fefd4b37 100644
--- a/openstackclient/compute/v2/server.py
+++ b/openstackclient/compute/v2/server.py
@@ -1407,9 +1407,38 @@ class MigrateServer(command.Command):
             help=_('Server (name or ID)'),
         )
         parser.add_argument(
+            '--live-migration',
+            dest='live_migration',
+            action='store_true',
+            help=_('Live migrate the server. Use the ``--host`` option to '
+                   'specify a target host for the migration which will be '
+                   'validated by the scheduler.'),
+        )
+        # The --live and --host options are mutually exclusive ways of asking
+        # for a target host during a live migration.
+        host_group = parser.add_mutually_exclusive_group()
+        # TODO(mriedem): Remove --live in the next major version bump after
+        #  the Train release.
+        host_group.add_argument(
             '--live',
             metavar='<hostname>',
-            help=_('Target hostname'),
+            help=_('**Deprecated** This option is problematic in that it '
+                   'requires a host and prior to compute API version 2.30, '
+                   'specifying a host during live migration will bypass '
+                   'validation by the scheduler which could result in '
+                   'failures to actually migrate the server to the specified '
+                   'host or over-subscribe the host. Use the '
+                   '``--live-migration`` option instead. If both this option '
+                   '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.'),
         )
         migration_group = parser.add_mutually_exclusive_group()
         migration_group.add_argument(
@@ -1447,6 +1476,15 @@ class MigrateServer(command.Command):
         )
         return parser
 
+    def _log_warning_for_live(self, parsed_args):
+        if parsed_args.live:
+            # NOTE(mriedem): The --live option requires a host and if
+            # --os-compute-api-version is less than 2.30 it will forcefully
+            # bypass the scheduler which is dangerous.
+            self.log.warning(_(
+                'The --live option has been deprecated. Please use the '
+                '--live-migration option instead.'))
+
     def take_action(self, parsed_args):
 
         def _show_progress(progress):
@@ -1460,19 +1498,45 @@ class MigrateServer(command.Command):
             compute_client.servers,
             parsed_args.server,
         )
-        if parsed_args.live:
+        # Check for live migration.
+        if parsed_args.live or parsed_args.live_migration:
+            # Always log a warning if --live is used.
+            self._log_warning_for_live(parsed_args)
             kwargs = {
-                'host': parsed_args.live,
                 'block_migration': parsed_args.block_migration
             }
+            # Prefer --live-migration over --live if both are specified.
+            if parsed_args.live_migration:
+                # Technically we could pass a non-None host with
+                # --os-compute-api-version < 2.30 but that is the same thing
+                # as the --live option bypassing the scheduler which we don't
+                # want to support, so if the user is using --live-migration
+                # and --host, we want to enforce that they are using version
+                # 2.30 or greater.
+                if (parsed_args.host and
+                        compute_client.api_version <
+                        api_versions.APIVersion('2.30')):
+                    raise exceptions.CommandError(
+                        '--os-compute-api-version 2.30 or greater is required '
+                        'when using --host')
+                # The host parameter is required in the API even if None.
+                kwargs['host'] = parsed_args.host
+            else:
+                kwargs['host'] = parsed_args.live
+
             if compute_client.api_version < api_versions.APIVersion('2.25'):
                 kwargs['disk_over_commit'] = parsed_args.disk_overcommit
             server.live_migrate(**kwargs)
         else:
-            if parsed_args.block_migration or parsed_args.disk_overcommit:
-                raise exceptions.CommandError("--live must be specified if "
-                                              "--block-migration or "
-                                              "--disk-overcommit is specified")
+            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.
+                raise exceptions.CommandError(
+                    "--live-migration must be specified if "
+                    "--block-migration, --disk-overcommit or --host is "
+                    "specified")
+
             server.migrate()
 
         if parsed_args.wait: