Microversion 2.68: Remove 'forced' live migrations, evacuations

Update the commands and Python API bindings to reflect the new
microversion. The various evacuate microversion functions are DRY'd up
along the way.

Change-Id: Ibfc905292258ffde05800387e5d6bbad4823085c
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Depends-On: https://review.openstack.org/#/c/634600/
Implements: blueprint remove-force-flag-from-live-migrate-and-evacuate

7 files changed, 214 insertions, 63 deletions

diff --git a/doc/source/cli/nova.rst b/doc/source/cli/nova.rst
index b8a212d4..f3bc8eab 100644
--- a/doc/source/cli/nova.rst
+++ b/doc/source/cli/nova.rst
@@ -1249,7 +1249,7 @@ Evacuate server from failed host.
 
 ``--force``
   Force an evacuation by not verifying the provided destination host by the
-  scheduler. (Supported by API versions '2.29' -'2.latest')
+  scheduler. (Supported by API versions '2.29' - '2.67')
 
   .. warning:: This could result in failures to actually evacuate the
     server to the specified host. It is recommended to either not specify
@@ -1650,7 +1650,7 @@ Evacuate all instances from failed host.
 
 ``--force``
   Force an evacuation by not verifying the provided destination host by the
-  scheduler. (Supported by API versions '2.29' -'2.latest')
+  scheduler. (Supported by API versions '2.29' - '2.67')
 
   .. warning:: This could result in failures to actually evacuate the
     server to the specified host. It is recommended to either not specify
@@ -1701,7 +1701,7 @@ Live migrate all instances off the specified host to other available hosts.
 
 ``--force``
   Force a live-migration by not verifying the provided destination host by
-  the scheduler. (Supported by API versions '2.30' -'2.latest')
+  the scheduler. (Supported by API versions '2.30' - '2.67')
 
   .. warning:: This could result in failures to actually live migrate the
     servers to the specified host. It is recommended to either not specify
@@ -2369,7 +2369,7 @@ Migrate running server to a new machine.
 
 ``--force``
   Force a live-migration by not verifying the provided destination host by
-  the scheduler. (Supported by API versions '2.30' -'2.latest')
+  the scheduler. (Supported by API versions '2.30' - '2.67')
 
   .. warning:: This could result in failures to actually live migrate the
     server to the specified host. It is recommended to either not specify
diff --git a/novaclient/__init__.py b/novaclient/__init__.py
index 43a8d1a9..a4eef58c 100644
--- a/novaclient/__init__.py
+++ b/novaclient/__init__.py
@@ -25,4 +25,4 @@ API_MIN_VERSION = api_versions.APIVersion("2.1")
 # when client supported the max version, and bumped sequentially, otherwise
 # the client may break due to server side new version may include some
 # backward incompatible change.
-API_MAX_VERSION = api_versions.APIVersion("2.67")
+API_MAX_VERSION = api_versions.APIVersion("2.68")
diff --git a/novaclient/tests/unit/v2/test_servers.py b/novaclient/tests/unit/v2/test_servers.py
index 34ce21f9..e153fd57 100644
--- a/novaclient/tests/unit/v2/test_servers.py
+++ b/novaclient/tests/unit/v2/test_servers.py
@@ -1655,3 +1655,43 @@ class ServersV267Test(ServersV263Test):
                                nics='none', block_device_mapping_v2=bdm)
         self.assertIn("Block device volume_type is not supported before "
                       "microversion 2.67", six.text_type(ex))
+
+
+class ServersV268Test(ServersV267Test):
+
+    api_version = "2.68"
+
+    def test_evacuate(self):
+        s = self.cs.servers.get(1234)
+        ret = s.evacuate('fake_target_host')
+        self.assert_request_id(ret, fakes.FAKE_REQUEST_ID_LIST)
+        self.assert_called('POST', '/servers/1234/action',
+                           {'evacuate': {'host': 'fake_target_host'}})
+
+        ret = self.cs.servers.evacuate(s, 'fake_target_host')
+        self.assert_request_id(ret, fakes.FAKE_REQUEST_ID_LIST)
+        self.assert_called('POST', '/servers/1234/action',
+                           {'evacuate': {'host': 'fake_target_host'}})
+
+        ex = self.assertRaises(TypeError, self.cs.servers.evacuate,
+                               'fake_target_host', force=True)
+        self.assertIn('force', six.text_type(ex))
+
+    def test_live_migrate_server(self):
+        s = self.cs.servers.get(1234)
+        ret = s.live_migrate(host='hostname', block_migration='auto')
+        self.assert_request_id(ret, fakes.FAKE_REQUEST_ID_LIST)
+        self.assert_called('POST', '/servers/1234/action',
+                           {'os-migrateLive': {'host': 'hostname',
+                                               'block_migration': 'auto'}})
+
+        ret = self.cs.servers.live_migrate(s, host='hostname',
+                                           block_migration='auto')
+        self.assert_request_id(ret, fakes.FAKE_REQUEST_ID_LIST)
+        self.assert_called('POST', '/servers/1234/action',
+                           {'os-migrateLive': {'host': 'hostname',
+                                               'block_migration': 'auto'}})
+
+        ex = self.assertRaises(TypeError, self.cs.servers.live_migrate,
+                               host='hostname', force=True)
+        self.assertIn('force', six.text_type(ex))
diff --git a/novaclient/tests/unit/v2/test_shell.py b/novaclient/tests/unit/v2/test_shell.py
index ae11d89a..fbf93910 100644
--- a/novaclient/tests/unit/v2/test_shell.py
+++ b/novaclient/tests/unit/v2/test_shell.py
@@ -2690,6 +2690,18 @@ class ShellTest(utils.TestCase):
                                                'block_migration': 'auto',
                                                'force': True}})
 
+    def test_live_migration_v2_68(self):
+        self.run_command('live-migration sample-server hostname',
+                         api_version='2.68')
+        self.assert_called('POST', '/servers/1234/action',
+                           {'os-migrateLive': {'host': 'hostname',
+                                               'block_migration': 'auto'}})
+
+        self.assertRaises(
+            SystemExit, self.run_command,
+            'live-migration --force sample-server hostname',
+            api_version='2.68')
+
     def test_live_migration_force_complete(self):
         self.run_command('live-migration-force-complete sample-server 1',
                          api_version='2.22')
@@ -3437,6 +3449,17 @@ class ShellTest(utils.TestCase):
                            {'evacuate': {'host': 'new_host',
                                          'force': True}})
 
+    def test_evacuate_v2_68(self):
+        self.run_command('evacuate sample-server new_host',
+                         api_version='2.68')
+        self.assert_called('POST', '/servers/1234/action',
+                           {'evacuate': {'host': 'new_host'}})
+
+        self.assertRaises(
+            SystemExit, self.run_command,
+            'evacuate --force sample-server new_host',
+            api_version='2.68')
+
     def test_evacuate_with_no_target_host(self):
         self.run_command('evacuate sample-server')
         self.assert_called('POST', '/servers/1234/action',
diff --git a/novaclient/v2/servers.py b/novaclient/v2/servers.py
index 0843e4e3..fa296bf6 100644
--- a/novaclient/v2/servers.py
+++ b/novaclient/v2/servers.py
@@ -444,7 +444,7 @@ class Server(base.Resource):
             block_migration = "auto"
         return self.manager.live_migrate(self, host, block_migration)
 
-    @api_versions.wraps("2.30")
+    @api_versions.wraps("2.30", "2.67")
     def live_migrate(self, host=None, block_migration=None, force=None):
         """
         Migrates a running instance to a new machine.
@@ -459,6 +459,20 @@ class Server(base.Resource):
             block_migration = "auto"
         return self.manager.live_migrate(self, host, block_migration, force)
 
+    @api_versions.wraps("2.68")
+    def live_migrate(self, host=None, block_migration=None):
+        """
+        Migrates a running instance to a new machine.
+
+        :param host: destination host name.
+        :param block_migration: if True, do block_migration, the default
+                                value is None which is mapped to 'auto'.
+        :returns: An instance of novaclient.base.TupleWithMeta
+        """
+        if block_migration is None:
+            block_migration = "auto"
+        return self.manager.live_migrate(self, host, block_migration)
+
     def reset_state(self, state='error'):
         """
         Reset the state of an instance to active or error.
@@ -524,7 +538,7 @@ class Server(base.Resource):
         """
         return self.manager.evacuate(self, host, password)
 
-    @api_versions.wraps("2.29")
+    @api_versions.wraps("2.29", "2.67")
     def evacuate(self, host=None, password=None, force=None):
         """
         Evacuate an instance from failed host to specified host.
@@ -537,6 +551,18 @@ class Server(base.Resource):
         """
         return self.manager.evacuate(self, host, password, force)
 
+    @api_versions.wraps("2.68")
+    def evacuate(self, host=None, password=None):
+        """
+        Evacuate an instance from failed host to specified host.
+
+        :param host: Name of the target host
+        :param password: string to set as admin password on the evacuated
+                         server.
+        :returns: An instance of novaclient.base.TupleWithMeta
+        """
+        return self.manager.evacuate(self, host, password)
+
     def interface_list(self):
         """
         List interfaces attached to an instance.
@@ -1670,6 +1696,24 @@ class ServerManager(base.BootingManagerWithFind):
 
         return result
 
+    def _live_migrate(self, server, host, block_migration, disk_over_commit,
+                      force):
+        """Inner function to abstract changes in live migration API."""
+        body = {
+            'host': host,
+            'block_migration': block_migration,
+        }
+
+        if disk_over_commit is not None:
+            body['disk_over_commit'] = disk_over_commit
+
+        # NOTE(stephenfin): For some silly reason, we don't set this if it's
+        # False, hence why we're not explicitly checking against None
+        if force:
+            body['force'] = force
+
+        return self._action('os-migrateLive', server, body)
+
     @api_versions.wraps('2.0', '2.24')
     def live_migrate(self, server, host, block_migration, disk_over_commit):
         """
@@ -1681,10 +1725,10 @@ class ServerManager(base.BootingManagerWithFind):
         :param disk_over_commit: if True, allow disk overcommit.
         :returns: An instance of novaclient.base.TupleWithMeta
         """
-        return self._action('os-migrateLive', server,
-                            {'host': host,
-                             'block_migration': block_migration,
-                             'disk_over_commit': disk_over_commit})
+        return self._live_migrate(server, host,
+                                  block_migration=block_migration,
+                                  disk_over_commit=disk_over_commit,
+                                  force=None)
 
     @api_versions.wraps('2.25', '2.29')
     def live_migrate(self, server, host, block_migration):
@@ -1697,11 +1741,12 @@ class ServerManager(base.BootingManagerWithFind):
                                 'auto'
         :returns: An instance of novaclient.base.TupleWithMeta
         """
-        return self._action('os-migrateLive', server,
-                            {'host': host,
-                             'block_migration': block_migration})
+        return self._live_migrate(server, host,
+                                  block_migration=block_migration,
+                                  disk_over_commit=None,
+                                  force=None)
 
-    @api_versions.wraps('2.30')
+    @api_versions.wraps('2.30', '2.67')
     def live_migrate(self, server, host, block_migration, force=None):
         """
         Migrates a running instance to a new machine.
@@ -1713,10 +1758,26 @@ class ServerManager(base.BootingManagerWithFind):
         :param force: forces to bypass the scheduler if host is provided.
         :returns: An instance of novaclient.base.TupleWithMeta
         """
-        body = {'host': host, 'block_migration': block_migration}
-        if force:
-            body['force'] = force
-        return self._action('os-migrateLive', server, body)
+        return self._live_migrate(server, host,
+                                  block_migration=block_migration,
+                                  disk_over_commit=None,
+                                  force=force)
+
+    @api_versions.wraps('2.68')
+    def live_migrate(self, server, host, block_migration):
+        """
+        Migrates a running instance to a new machine.
+
+        :param server: instance id which comes from nova list.
+        :param host: destination host name.
+        :param block_migration: if True, do block_migration, can be set as
+                                'auto'
+        :returns: An instance of novaclient.base.TupleWithMeta
+        """
+        return self._live_migrate(server, host,
+                                  block_migration=block_migration,
+                                  disk_over_commit=None,
+                                  force=None)
 
     def reset_state(self, server, state='error'):
         """
@@ -1771,78 +1832,89 @@ class ServerManager(base.BootingManagerWithFind):
                           base.getid(server), 'security_groups',
                           SecurityGroup)
 
+    def _evacuate(self, server, host, on_shared_storage, password, force):
+        """Inner function to abstract changes in evacuate API."""
+        body = {}
+
+        if on_shared_storage is not None:
+            body['onSharedStorage'] = on_shared_storage
+
+        if host is not None:
+            body['host'] = host
+
+        if password is not None:
+            body['adminPass'] = password
+
+        if force:
+            body['force'] = force
+
+        resp, body = self._action_return_resp_and_body('evacuate', server,
+                                                       body)
+        return base.TupleWithMeta((resp, body), resp)
+
     @api_versions.wraps("2.0", "2.13")
     def evacuate(self, server, host=None, on_shared_storage=True,
                  password=None):
         """
         Evacuate a server instance.
 
-        :param server: The :class:`Server` (or its ID) to share onto.
+        :param server: The :class:`Server` (or its ID) to evacuate to.
         :param host: Name of the target host.
         :param on_shared_storage: Specifies whether instance files located
                         on shared storage
         :param password: string to set as password on the evacuated server.
         :returns: An instance of novaclient.base.TupleWithMeta
         """
-
-        body = {'onSharedStorage': on_shared_storage}
-        if host is not None:
-            body['host'] = host
-
-        if password is not None:
-            body['adminPass'] = password
-
-        resp, body = self._action_return_resp_and_body('evacuate', server,
-                                                       body)
-        return base.TupleWithMeta((resp, body), resp)
+        return self._evacuate(server, host,
+                              on_shared_storage=on_shared_storage,
+                              password=password,
+                              force=None)
 
     @api_versions.wraps("2.14", "2.28")
     def evacuate(self, server, host=None, password=None):
         """
         Evacuate a server instance.
 
-        :param server: The :class:`Server` (or its ID) to share onto.
+        :param server: The :class:`Server` (or its ID) to evacuate to.
         :param host: Name of the target host.
         :param password: string to set as password on the evacuated server.
         :returns: An instance of novaclient.base.TupleWithMeta
         """
+        return self._evacuate(server, host,
+                              on_shared_storage=None,
+                              password=password,
+                              force=None)
 
-        body = {}
-        if host is not None:
-            body['host'] = host
-
-        if password is not None:
-            body['adminPass'] = password
-
-        resp, body = self._action_return_resp_and_body('evacuate', server,
-                                                       body)
-        return base.TupleWithMeta((resp, body), resp)
-
-    @api_versions.wraps("2.29")
+    @api_versions.wraps("2.29", "2.67")
     def evacuate(self, server, host=None, password=None, force=None):
         """
         Evacuate a server instance.
 
-        :param server: The :class:`Server` (or its ID) to share onto.
+        :param server: The :class:`Server` (or its ID) to evacuate to.
         :param host: Name of the target host.
         :param password: string to set as password on the evacuated server.
         :param force: forces to bypass the scheduler if host is provided.
         :returns: An instance of novaclient.base.TupleWithMeta
         """
+        return self._evacuate(server, host,
+                              on_shared_storage=None,
+                              password=password,
+                              force=force)
 
-        body = {}
-        if host is not None:
-            body['host'] = host
-
-        if password is not None:
-            body['adminPass'] = password
-
-        if force:
-            body['force'] = force
+    @api_versions.wraps("2.68")
+    def evacuate(self, server, host=None, password=None):
+        """
+        Evacuate a server instance.
 
-        resp, body = self._action_return_resp_and_body('evacuate', server,
-                                                       body)
-        return base.TupleWithMeta((resp, body), resp)
+        :param server: The :class:`Server` (or its ID) to evacuate to.
+        :param host: Name of the target host.
+        :param password: string to set as password on the evacuated server.
+        :returns: An instance of novaclient.base.TupleWithMeta
+        """
+        return self._evacuate(server, host,
+                              on_shared_storage=None,
+                              password=password,
+                              force=None)
 
     def interface_list(self, server):
         """
diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py
index c98724c9..a86c0d8e 100644
--- a/novaclient/v2/shell.py
+++ b/novaclient/v2/shell.py
@@ -3464,7 +3464,8 @@ def _print_aggregate_details(cs, aggregate):
            'actually live migrate the server to the specified host. It is '
            'recommended to either not specify a host so that the scheduler '
            'will pick one, or specify a host without --force.'),
-    start_version='2.30')
+    start_version='2.30',
+    end_version='2.67')
 def do_live_migration(cs, args):
     """Migrate running server to a new machine."""
 
@@ -4453,10 +4454,12 @@ def do_quota_class_update(cs, args):
            'actually evacuate the server to the specified host. It is '
            'recommended to either not specify a host so that the scheduler '
            'will pick one, or specify a host without --force.'),
-    start_version='2.29')
+    start_version='2.29',
+    end_version='2.67')
 def do_evacuate(cs, args):
     """Evacuate server from failed host."""
 
+    # TODO(stephenfin): Simply call '_server_evacuate' instead?
     server = _find_server(cs, args.server)
     on_shared_storage = getattr(args, 'on_shared_storage', None)
     force = getattr(args, 'force', None)
@@ -4843,8 +4846,11 @@ def _server_evacuate(cs, server, args):
     success = True
     error_message = ""
     try:
-        if api_versions.APIVersion("2.29") <= cs.api_version:
-            # if microversion >= 2.29
+        if api_versions.APIVersion('2.68') <= cs.api_version:
+            # if microversion >= 2.68
+            cs.servers.evacuate(server=server['uuid'], host=args.target_host)
+        elif api_versions.APIVersion('2.29') <= cs.api_version:
+            # if microversion 2.29 - 2.67
             force = getattr(args, 'force', None)
             cs.servers.evacuate(server=server['uuid'], host=args.target_host,
                                 force=force)
@@ -4910,7 +4916,8 @@ def _hyper_servers(cs, host, strict):
            'actually evacuate the server to the specified host. It is '
            'recommended to either not specify a host so that the scheduler '
            'will pick one, or specify a host without --force.'),
-    start_version='2.29')
+    start_version='2.29',
+    end_version='2.67')
 @utils.arg(
     '--strict',
     dest='strict',
@@ -5000,7 +5007,8 @@ def _server_live_migrate(cs, server, args):
            'actually live migrate the servers to the specified host. It is '
            'recommended to either not specify a host so that the scheduler '
            'will pick one, or specify a host without --force.'),
-    start_version='2.30')
+    start_version='2.30',
+    end_version='2.67')
 @utils.arg(
     '--strict',
     dest='strict',
diff --git a/releasenotes/notes/deprecate-force-option-7116d792bba17f09.yaml b/releasenotes/notes/deprecate-force-option-7116d792bba17f09.yaml
new file mode 100644
index 00000000..89e5ffed
--- /dev/null
+++ b/releasenotes/notes/deprecate-force-option-7116d792bba17f09.yaml
@@ -0,0 +1,8 @@
+---
+upgrade:
+  - |
+    Added support for `microversion 2.68`_, which removes the ``--force`` option
+    from the ``nova evacuate``, ``nova live-migration``, ``nova host-evacuate``
+    and ``nova host-evacuate-live`` commands.
+
+    .. _microversion 2.68: https://docs.openstack.org/nova/latest/api_microversion_history.html#id61