API change for verifying the scheduler when live migrating

After modifying the evacuate action, we now add a new microversion
change for modifying the live-migrate call so that the scheduler is
called when the admin user provides an hostname unless the force
field is provided.

APIImpact

Implements: blueprint check-destination-on-migrations-newton

Change-Id: I212cbb44f46d7cb36b5d8c74a79065d38fc526d8

api-ref/source/parameters.yaml

@@ -1413,6 +1413,14 @@ force_evacuate:
   required: false
   type: boolean
   min_version: 2.29
+force_live_migrate:
+  description: |
+    Force a live-migration by not verifying the provided destination host by
+    the scheduler.
+  in: body
+  required: false
+  type: boolean
+  min_version: 2.30
 forced_down:
   in: body
   required: true

api-ref/source/servers-admin-action.inc

@@ -170,6 +170,7 @@ Request
   - host: host_migration
   - block_migration: block_migration
   - disk_over_commit: disk_over_commit
+  - force: force_live_migrate
 
 **Example Live-Migrate Server (os-migrateLive Action): JSON request**
 

doc/api_samples/os-migrate-server/v2.30/live-migrate-server.json

@@ -0,0 +1,7 @@
+{
+    "os-migrateLive": {
+        "host": "01c0cadef72d47e28a672a76060d492c",
+        "block_migration": "auto",
+        "force": false
+    }
+}

doc/api_samples/versions/v21-version-get-resp.json

@@ -19,7 +19,7 @@
             }
         ],
         "status": "CURRENT",
-        "version": "2.29",
+        "version": "2.30",
         "min_version": "2.1",
         "updated": "2013-07-23T11:33:21Z"
     }

doc/api_samples/versions/versions-get-resp.json

@@ -22,7 +22,7 @@
                 }
             ],
             "status": "CURRENT",
-            "version": "2.29",
+            "version": "2.30",
             "min_version": "2.1",
             "updated": "2013-07-23T11:33:21Z"
         }

nova/api/openstack/api_version_request.py

@@ -77,6 +77,8 @@ REST_API_VERSION_HISTORY = """REST API Version History:
     * 2.28 - Changes compute_node.cpu_info from string to object
     * 2.29 - Add a force flag in evacuate request body and change the
              behaviour for the host flag by calling the scheduler.
+    * 2.30 - Add a force flag in live-migrate request body and change the
+             behaviour for the host flag by calling the scheduler.
 """
 
 # The minimum and maximum versions of the API supported
@@ -85,7 +87,7 @@ REST_API_VERSION_HISTORY = """REST API Version History:
 # Note(cyeoh): This only applies for the v2.1 API once microversions
 # support is fully merged. It does not affect the V2 API.
 _MIN_API_VERSION = "2.1"
-_MAX_API_VERSION = "2.29"
+_MAX_API_VERSION = "2.30"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 

nova/api/openstack/compute/migrate_server.py

@@ -24,6 +24,7 @@ from nova.api.openstack import wsgi
 from nova.api import validation
 from nova import compute
 from nova import exception
+from nova.i18n import _
 
 ALIAS = "os-migrate-server"
 
@@ -63,7 +64,8 @@ class MigrateServerController(wsgi.Controller):
     @extensions.expected_errors((400, 404, 409))
     @wsgi.action('os-migrateLive')
     @validation.schema(migrate_server.migrate_live, "2.1", "2.24")
-    @validation.schema(migrate_server.migrate_live_v2_25, "2.25")
+    @validation.schema(migrate_server.migrate_live_v2_25, "2.25", "2.29")
+    @validation.schema(migrate_server.migrate_live_v2_30, "2.30")
     def _migrate_live(self, req, id, body):
         """Permit admins to (live) migrate a server to a new host."""
         context = req.environ["nova.context"]
@@ -71,7 +73,14 @@ class MigrateServerController(wsgi.Controller):
 
         host = body["os-migrateLive"]["host"]
         block_migration = body["os-migrateLive"]["block_migration"]
+        force = None
 
+        if api_version_request.is_supported(req, min_version='2.30'):
+            force = body["os-migrateLive"].get("force", False)
+            force = strutils.bool_from_string(force, strict=True)
+            if force is True and not host:
+                message = _("Can't force to a non-provided destination")
+                raise exc.HTTPBadRequest(explanation=message)
         if api_version_request.is_supported(req, min_version='2.25'):
             if block_migration == 'auto':
                 block_migration = None
@@ -90,7 +99,7 @@ class MigrateServerController(wsgi.Controller):
         try:
             instance = common.get_instance(self.compute_api, context, id)
             self.compute_api.live_migrate(context, instance, block_migration,
-                                          disk_over_commit, host)
+                                          disk_over_commit, host, force)
         except exception.InstanceUnknownCell as e:
             raise exc.HTTPNotFound(explanation=e.format_message())
         except (exception.NoValidHost,

nova/api/openstack/compute/schemas/migrate_server.py

@@ -49,3 +49,7 @@ migrate_live_v2_25['properties']['os-migrateLive']['properties'][
     'block_migration'] = block_migration
 migrate_live_v2_25['properties']['os-migrateLive']['required'] = (
     ['block_migration', 'host'])
+
+migrate_live_v2_30 = copy.deepcopy(migrate_live_v2_25)
+migrate_live_v2_30['properties']['os-migrateLive']['properties'][
+    'force'] = parameter_types.boolean

nova/api/openstack/rest_api_version_history.rst

@@ -309,3 +309,12 @@ user documentation.
   Also changes the evacuate action behaviour when providing a ``host`` string
   field by calling the nova scheduler to verify the provided host unless the
   ``force`` attribute is set.
+
+2.30
+----
+
+Updates the POST request body for the ``live-migrate`` action to include the
+optional ``force`` boolean field defaulted to False.
+Also changes the live-migrate action behaviour when providing a ``host``
+string field by calling the nova scheduler to verify the provided host unless
+the ``force`` attribute is set.

nova/compute/api.py

@@ -3352,7 +3352,7 @@ class API(base.Base):
     @check_instance_cell
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED])
     def live_migrate(self, context, instance, block_migration,
-                     disk_over_commit, host_name):
+                     disk_over_commit, host_name, force=None):
         """Migrate a server lively to a new host."""
         LOG.debug("Going to try to live migrate instance to %s",
                   host_name or "another host", instance=instance)
@@ -3369,6 +3369,30 @@ class API(base.Base):
             # Some old instances can still have no RequestSpec object attached
             # to them, we need to support the old way
             request_spec = None
+
+        # NOTE(sbauza): Force is a boolean by the new related API version
+        if force is False and host_name:
+            nodes = objects.ComputeNodeList.get_all_by_host(context, host_name)
+            if not nodes:
+                raise exception.ComputeHostNotFound(host=host_name)
+            # NOTE(sbauza): Unset the host to make sure we call the scheduler
+            host_name = None
+            # FIXME(sbauza): Since only Ironic driver uses more than one
+            # compute per service but doesn't support evacuations,
+            # let's provide the first one.
+            target = nodes[0]
+            if request_spec:
+                # TODO(sbauza): Hydrate a fake spec for old instances not yet
+                # having a request spec attached to them (particularly true for
+                # cells v1). For the moment, let's keep the same behaviour for
+                # all the instances but provide the destination only if a spec
+                # is found.
+                destination = objects.Destination(
+                    host=target.host,
+                    node=target.hypervisor_hostname
+                )
+                request_spec.requested_destination = destination
+
         try:
             self.compute_task_api.live_migrate_instance(context, instance,
                 host_name, block_migration=block_migration,

nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.30/live-migrate-server.json.tpl

@@ -0,0 +1,7 @@
+{
+    "os-migrateLive": {
+        "host": "%(hostname)s",
+        "block_migration": "auto",
+        "force": "%(force)s"
+    }
+}

nova/tests/functional/api_sample_tests/test_migrate_server.py

@@ -17,6 +17,7 @@ import mock
 from oslo_utils import versionutils
 
 import nova.conf
+from nova import objects
 from nova.tests.functional.api_sample_tests import test_servers
 
 CONF = nova.conf.CONF
@@ -40,6 +41,7 @@ class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase):
         """
         super(MigrateServerSamplesJsonTest, self).setUp()
         self.uuid = self._post_server()
+        self.host_attended = self.compute.host
 
     @mock.patch('nova.conductor.manager.ComputeTaskManager._cold_migrate')
     def test_post_migrate(self, mock_cold_migrate):
@@ -48,13 +50,15 @@ class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase):
                                  'migrate-server', {})
         self.assertEqual(202, response.status_code)
 
-    def test_post_live_migrate_server(self):
-        # Get api samples to server live migrate request.
+    def _check_post_live_migrate_server(self, req_subs=None):
+        if not req_subs:
+            req_subs = {'hostname': self.compute.host}
+
         def fake_live_migrate(_self, context, instance, scheduler_hint,
                               block_migration, disk_over_commit, request_spec):
             self.assertEqual(self.uuid, instance["uuid"])
             host = scheduler_hint["host"]
-            self.assertEqual(self.compute.host, host)
+            self.assertEqual(self.host_attended, host)
 
         self.stub_out(
             'nova.conductor.manager.ComputeTaskManager._live_migrate',
@@ -75,9 +79,13 @@ class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase):
 
         response = self._do_post('servers/%s/action' % self.uuid,
                                  'live-migrate-server',
-                                 {'hostname': self.compute.host})
+                                 req_subs)
         self.assertEqual(202, response.status_code)
 
+    def test_post_live_migrate_server(self):
+        # Get api samples to server live migrate request.
+        self._check_post_live_migrate_server()
+
 
 class MigrateServerSamplesJsonTestV225(MigrateServerSamplesJsonTest):
     extension_name = "os-migrate-server"
@@ -87,3 +95,32 @@ class MigrateServerSamplesJsonTestV225(MigrateServerSamplesJsonTest):
     def test_post_migrate(self):
         # no changes for migrate-server
         pass
+
+
+class MigrateServerSamplesJsonTestV230(MigrateServerSamplesJsonTest):
+    extension_name = "os-migrate-server"
+    microversion = '2.30'
+    scenarios = [('v2_30', {'api_major_version': 'v2.1'})]
+
+    def test_post_migrate(self):
+        # no changes for migrate-server
+        pass
+
+    @mock.patch('nova.objects.ComputeNodeList.get_all_by_host')
+    def test_post_live_migrate_server(self, compute_node_get_all_by_host):
+        # Get api samples to server live migrate request.
+
+        fake_computes = objects.ComputeNodeList(
+            objects=[objects.ComputeNode(host='testHost',
+                                         hypervisor_hostname='host')])
+        compute_node_get_all_by_host.return_value = fake_computes
+        self.host_attended = None
+        self._check_post_live_migrate_server(
+            req_subs={'hostname': self.compute.host,
+                      'force': 'False'})
+
+    def test_post_live_migrate_server_with_force(self):
+        self.host_attended = self.compute.host
+        self._check_post_live_migrate_server(
+            req_subs={'hostname': self.compute.host,
+                      'force': 'True'})

nova/tests/unit/api/openstack/compute/test_migrate_server.py

@@ -32,6 +32,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
     validation_error = exception.ValidationError
     _api_version = '2.1'
     disk_over_commit = False
+    force = None
 
     def setUp(self):
         super(MigrateServerTestsV21, self).setUp()
@@ -58,7 +59,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
                                '_migrate_live': 'live_migrate'}
         body_map = {'_migrate_live': self._get_migration_body(host='hostname')}
         args_map = {'_migrate_live': ((False, self.disk_over_commit,
-                                       'hostname'), {})}
+                                       'hostname', self.force), {})}
         self._test_actions(['_migrate', '_migrate_live'], body_map=body_map,
                            method_translations=method_translations,
                            args_map=args_map)
@@ -67,7 +68,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
         method_translations = {'_migrate': 'resize',
                                '_migrate_live': 'live_migrate'}
         body_map = {'_migrate_live': self._get_migration_body(host=None)}
-        args_map = {'_migrate_live': ((False, self.disk_over_commit, None),
+        args_map = {'_migrate_live': ((False, self.disk_over_commit, None,
+                                       self.force),
                                       {})}
         self._test_actions(['_migrate', '_migrate_live'], body_map=body_map,
                            method_translations=method_translations,
@@ -83,7 +85,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
                                '_migrate_live': 'live_migrate'}
         body_map = self._get_migration_body(host='hostname')
         args_map = {'_migrate_live': ((False, self.disk_over_commit,
-                                       'hostname'), {})}
+                                       'hostname', self.force), {})}
         exception_arg = {'_migrate': 'migrate',
                          '_migrate_live': 'os-migrateLive'}
         self._test_actions_raise_conflict_on_invalid_state(
@@ -98,7 +100,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
         body_map = {'_migrate_live':
                     self._get_migration_body(host='hostname')}
         args_map = {'_migrate_live': ((False, self.disk_over_commit,
-                                       'hostname'), {})}
+                                       'hostname', self.force), {})}
         self._test_actions_with_locked_instance(
             ['_migrate', '_migrate_live'], body_map=body_map,
             args_map=args_map, method_translations=method_translations)
@@ -122,7 +124,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
         self.mox.StubOutWithMock(self.compute_api, 'live_migrate')
         instance = self._stub_instance_get()
         self.compute_api.live_migrate(self.context, instance, False,
-                                      self.disk_over_commit, 'hostname')
+                                      self.disk_over_commit, 'hostname',
+                                      self.force)
 
         self.mox.ReplayAll()
 
@@ -201,7 +204,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
         instance = self._stub_instance_get(uuid=uuid)
         self.compute_api.live_migrate(self.context, instance, False,
                                       self.disk_over_commit,
-                                      'hostname').AndRaise(fake_exc)
+                                      'hostname', self.force
+                                      ).AndRaise(fake_exc)
 
         self.mox.ReplayAll()
 
@@ -304,7 +308,8 @@ class MigrateServerTestsV225(MigrateServerTestsV21):
         method_translations = {'_migrate_live': 'live_migrate'}
         body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname',
                                       'block_migration': 'auto'}}}
-        args_map = {'_migrate_live': ((None, None, 'hostname'), {})}
+        args_map = {'_migrate_live': ((None, None, 'hostname', self.force),
+                                      {})}
         self._test_actions(['_migrate_live'], body_map=body_map,
                            method_translations=method_translations,
                            args_map=args_map)
@@ -323,6 +328,44 @@ class MigrateServerTestsV225(MigrateServerTestsV21):
             exception.LiveMigrationWithOldNovaNotSupported())
 
 
+class MigrateServerTestsV230(MigrateServerTestsV225):
+
+    force = False
+
+    def setUp(self):
+        super(MigrateServerTestsV230, self).setUp()
+        self.req.api_version_request = api_version_request.APIVersionRequest(
+            '2.30')
+
+    def _test_live_migrate(self, force=False):
+        if force is True:
+            litteral_force = 'true'
+        else:
+            litteral_force = 'false'
+        method_translations = {'_migrate_live': 'live_migrate'}
+        body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname',
+                                      'block_migration': 'auto',
+                                      'force': litteral_force}}}
+        args_map = {'_migrate_live': ((None, None, 'hostname', force),
+                                      {})}
+        self._test_actions(['_migrate_live'], body_map=body_map,
+                           method_translations=method_translations,
+                           args_map=args_map)
+
+    def test_live_migrate(self):
+        self._test_live_migrate()
+
+    def test_live_migrate_with_forced_host(self):
+        self._test_live_migrate(force=True)
+
+    def test_forced_live_migrate_with_no_provided_host(self):
+        body = {'os-migrateLive':
+                {'force': 'true'}}
+        self.assertRaises(self.validation_error,
+                          self.controller._migrate_live,
+                          self.req, fakes.FAKE_UUID, body=body)
+
+
 class MigrateServerPolicyEnforcementV21(test.NoDBTestCase):
 
     def setUp(self):

nova/tests/unit/compute/test_compute.py

@@ -10086,28 +10086,38 @@ class ComputeAPITestCase(BaseTestCase):
         mock_refresh.assert_called_once_with(mock.sentinel.ctxt,
                                              mock.sentinel.instances)
 
-    def test_live_migrate(self):
+    def _test_live_migrate(self, force=None):
         instance, instance_uuid = self._run_instance()
 
         rpcapi = self.compute_api.compute_task_api
         fake_spec = objects.RequestSpec()
 
         @mock.patch.object(rpcapi, 'live_migrate_instance')
+        @mock.patch.object(objects.ComputeNodeList, 'get_all_by_host')
         @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid')
         @mock.patch.object(self.compute_api, '_record_action_start')
         def do_test(record_action_start, get_by_instance_uuid,
-                    live_migrate_instance):
+                    get_all_by_host, live_migrate_instance):
             get_by_instance_uuid.return_value = fake_spec
+            get_all_by_host.return_value = objects.ComputeNodeList(
+                objects=[objects.ComputeNode(
+                    host='fake_dest_host',
+                    hypervisor_hostname='fake_dest_node')])
 
             self.compute_api.live_migrate(self.context, instance,
                                           block_migration=True,
                                           disk_over_commit=True,
-                                          host_name='fake_dest_host')
+                                          host_name='fake_dest_host',
+                                          force=force)
 
             record_action_start.assert_called_once_with(self.context, instance,
                                                         'live-migration')
+            if force is False:
+                host = None
+            else:
+                host = 'fake_dest_host'
             live_migrate_instance.assert_called_once_with(
-                self.context, instance, 'fake_dest_host',
+                self.context, instance, host,
                 block_migration=True,
                 disk_over_commit=True,
                 request_spec=fake_spec)
@@ -10115,6 +10125,33 @@ class ComputeAPITestCase(BaseTestCase):
         do_test()
         instance.refresh()
         self.assertEqual(instance['task_state'], task_states.MIGRATING)
+        if force is False:
+            req_dest = fake_spec.requested_destination
+            self.assertIsNotNone(req_dest)
+            self.assertIsInstance(req_dest, objects.Destination)
+            self.assertEqual('fake_dest_host', req_dest.host)
+            self.assertEqual('fake_dest_node', req_dest.node)
+
+    def test_live_migrate(self):
+        self._test_live_migrate()
+
+    def test_live_migrate_with_not_forced_host(self):
+        self._test_live_migrate(force=False)
+
+    def test_live_migrate_with_forced_host(self):
+        self._test_live_migrate(force=True)
+
+    def test_fail_live_migrate_with_non_existing_destination(self):
+        instance = self._create_fake_instance_obj(services=True)
+        self.assertIsNone(instance.task_state)
+
+        self.assertRaises(
+            exception.ComputeHostNotFound,
+            self.compute_api.live_migrate, self.context.elevated(),
+            instance, block_migration=True,
+            disk_over_commit=True,
+            host_name='fake_dest_host',
+            force=False)
 
     def _test_evacuate(self, force=None):
         instance = self._create_fake_instance_obj(services=True)

releasenotes/notes/check_destination_when_livemig-e69d32e02d7a18c9.yaml

@@ -0,0 +1,11 @@
+---
+features:
+  - On live-migrate actions, the default behaviour when providing a host in
+    the request body changed. Now, instead of bypassing the scheduler when
+    asking for a destination, it will instead call it with the requested
+    destination to make sure the proposed host is accepted by all the filters
+    and the original request.
+    In case the administrator doesn't want to call the scheduler when providing
+    a destination, a new request body field called ``force`` (defaulted to
+    False) will modify that new behaviour by forcing the live-migrate operation
+    to the destination without verifying the scheduler.