Enable cold migration with target host(2/2)

This function enables users to specify a target host
when cold migrating a VM instance.

This patch modifies the migration API.

APIImpact
    Add an optional parameter 'host' in cold migration action.

Change-Id: Iee356c4dd097c846b6ca8617ead6a061300c83f8
Implements: blueprint cold-migration-with-target-queens

api-ref/source/parameters.yaml

@@ -2896,6 +2896,14 @@ host_migration:
   in: body
   required: true
   type: string
+host_migration_2_56:
+  description: |
+    The host to which to migrate the server. If you specify ``null`` or
+    don't specify this parameter, the scheduler chooses a host.
+  in: body
+  required: false
+  type: string
+  min_version: 2.56
 host_name_body:
   description: |
     The name of the host.
@@ -3897,10 +3905,12 @@ metadata_object:
   type: object
 migrate:
   description: |
-    The action.
+    The action to cold migrate a server.
+    This parameter can be ``null``.
+    Up to microversion 2.55, this parameter should be ``null``.
   in: body
   required: true
-  type: string
+  type: object
 migrate_dest_compute:
   description: |
     The target compute for a migration.

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

@@ -56,10 +56,15 @@ Migrate Server (migrate Action)
 
 .. rest_method:: POST /servers/{server_id}/action
 
-Migrates a server to a host. The scheduler chooses the host.
+Migrates a server to a host.
 
 Specify the ``migrate`` action in the request body.
 
+Up to microversion 2.55, the scheduler chooses the host.
+Starting from microversion 2.56, the ``host`` parameter is available
+to specify the destination host. If you specify ``null`` or don't specify
+this parameter, the scheduler chooses a host.
+
 Policy defaults enable only users with the administrative role to
 perform this operation. Cloud providers can change these permissions
 through the ``policy.json`` file.
@@ -76,12 +81,18 @@ Request
 
   - server_id: server_id_path
   - migrate: migrate
+  - host: host_migration_2_56
 
-**Example Migrate Server (migrate Action)**
+**Example Migrate Server (migrate Action) (v2.1)**
 
 .. literalinclude:: ../../doc/api_samples/os-migrate-server/migrate-server.json
    :language: javascript
 
+**Example Migrate Server (migrate Action) (v2.56)**
+
+.. literalinclude:: ../../doc/api_samples/os-migrate-server/v2.56/migrate-server.json
+   :language: javascript
+
 Response
 --------
 

doc/api_samples/os-migrate-server/v2.56/migrate-server-null.json

@@ -0,0 +1,3 @@
+{
+    "migrate": null
+}

doc/api_samples/os-migrate-server/v2.56/migrate-server.json

@@ -0,0 +1,5 @@
+{
+    "migrate": {
+        "host": "host1"
+    }
+}

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

@@ -19,7 +19,7 @@
             }
         ],
         "status": "CURRENT",
-        "version": "2.55",
+        "version": "2.56",
         "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.55",
+            "version": "2.56",
             "min_version": "2.1",
             "updated": "2013-07-23T11:33:21Z"
         }

nova/api/openstack/api_version_request.py

@@ -130,6 +130,9 @@ REST_API_VERSION_HISTORY = """REST API Version History:
              and responses are also changed.
     * 2.54 - Enable reset key pair while rebuilding instance.
     * 2.55 - Added flavor.description to GET/POST/PUT flavors APIs.
+    * 2.56 - Add a host parameter in migrate request body in order to
+             enable users to specify a target host in cold migration.
+             The target host is checked by the scheduler.
 """
 
 # The minimum and maximum versions of the API supported
@@ -138,7 +141,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.55"
+_MAX_API_VERSION = "2.56"
 DEFAULT_API_VERSION = _MIN_API_VERSION
 
 # Almost all proxy APIs which are related to network, images and baremetal

nova/api/openstack/compute/migrate_server.py

@@ -40,24 +40,33 @@ class MigrateServerController(wsgi.Controller):
     @wsgi.response(202)
     @extensions.expected_errors((400, 403, 404, 409))
     @wsgi.action('migrate')
+    @validation.schema(migrate_server.migrate_v2_56, "2.56")
     def _migrate(self, req, id, body):
         """Permit admins to migrate a server to a new host."""
         context = req.environ['nova.context']
         context.can(ms_policies.POLICY_ROOT % 'migrate')
 
+        host_name = None
+        if (api_version_request.is_supported(req, min_version='2.56') and
+            body['migrate'] is not None):
+            host_name = body['migrate'].get('host')
+
         instance = common.get_instance(self.compute_api, context, id)
         try:
-            self.compute_api.resize(req.environ['nova.context'], instance)
+            self.compute_api.resize(req.environ['nova.context'], instance,
+                                    host_name=host_name)
         except (exception.TooManyInstances, exception.QuotaError) as e:
             raise exc.HTTPForbidden(explanation=e.format_message())
-        except exception.InstanceIsLocked as e:
+        except (exception.InstanceIsLocked,
+                exception.CannotMigrateWithTargetHost) as e:
             raise exc.HTTPConflict(explanation=e.format_message())
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'migrate', id)
         except exception.InstanceNotFound as e:
             raise exc.HTTPNotFound(explanation=e.format_message())
-        except exception.NoValidHost as e:
+        except (exception.NoValidHost, exception.ComputeHostNotFound,
+                exception.CannotMigrateToSameHost) as e:
             raise exc.HTTPBadRequest(explanation=e.format_message())
 
     @wsgi.response(202)

nova/api/openstack/compute/rest_api_version_history.rst

@@ -701,3 +701,11 @@ Adds a ``description`` field to the flavor resource in the following APIs:
 * ``PUT /flavors/{flavor_id}``
 
 The embedded flavor description will not be included in server representations.
+
+2.56
+----
+
+  Updates the POST request body for the ``migrate`` action to include the
+  the optional ``host`` string field defaulted to ``null``. If ``host`` is
+  set the migrate action verifies the provided host with the nova scheduler
+  and uses it as the destination for the migration.

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

@@ -20,6 +20,21 @@ from nova.api.validation import parameter_types
 host = copy.deepcopy(parameter_types.hostname)
 host['type'] = ['string', 'null']
 
+migrate_v2_56 = {
+    'type': 'object',
+    'properties': {
+        'migrate': {
+            'type': ['object', 'null'],
+            'properties': {
+                'host': host,
+            },
+            'additionalProperties': False,
+        },
+    },
+    'required': ['migrate'],
+    'additionalProperties': False,
+}
+
 migrate_live = {
     'type': 'object',
     'properties': {

nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.56/migrate-server-null.json.tpl

@@ -0,0 +1,3 @@
+{
+    "migrate": null
+}

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

@@ -0,0 +1,5 @@
+{
+    "migrate": {
+        "host": %(hostname)s
+    }
+}

nova/tests/functional/api_sample_tests/test_migrate_server.py

@@ -141,3 +141,42 @@ class MigrateServerSamplesJsonTestV230(MigrateServerSamplesJsonTest):
                                  {'hostname': hostname,
                                   'force': 'False'})
         self.assertEqual(400, response.status_code)
+
+
+class MigrateServerSamplesJsonTestV256(test_servers.ServersSampleBase):
+    sample_dir = "os-migrate-server"
+    microversion = '2.56'
+    scenarios = [('v2_56', {'api_major_version': 'v2.1'})]
+
+    def setUp(self):
+        """setUp Method for MigrateServer api samples extension
+
+        This method creates the server that will be used in each tests
+        """
+        super(MigrateServerSamplesJsonTestV256, self).setUp()
+        self.uuid = self._post_server()
+
+    @mock.patch('nova.conductor.manager.ComputeTaskManager._cold_migrate')
+    def test_post_migrate(self, mock_cold_migrate):
+        response = self._do_post('servers/%s/action' % self.uuid,
+                                 'migrate-server',
+                                 {'hostname': 'null'})
+        self.assertEqual(202, response.status_code)
+
+    @mock.patch('nova.objects.ComputeNodeList.get_all_by_host',
+                return_value=[objects.ComputeNode(
+                    host='target-host', hypervisor_hostname='target-node')])
+    @mock.patch('nova.conductor.manager.ComputeTaskManager._cold_migrate')
+    def test_post_migrate_with_host(self, mock_cold_migrate,
+                                    mock_get_all_by_host):
+        response = self._do_post('servers/%s/action' % self.uuid,
+                                 'migrate-server',
+                                 {'hostname': '"target-host"'})
+        self.assertEqual(202, response.status_code)
+
+    @mock.patch('nova.conductor.manager.ComputeTaskManager._cold_migrate')
+    def test_post_migrate_null(self, mock_cold_migrate):
+        # Check backward compatibility.
+        response = self._do_post('servers/%s/action' % self.uuid,
+                                 'migrate-server-null', {})
+        self.assertEqual(202, response.status_code)

nova/tests/functional/integrated_helpers.py

@@ -137,11 +137,14 @@ class _IntegratedTestBase(test.TestCase):
     def get_invalid_image(self):
         return uuids.fake
 
-    def _build_minimal_create_server_request(self):
+    def _build_minimal_create_server_request(self, image_uuid=None):
         server = {}
 
-        # We now have a valid imageId
+        # NOTE(takashin): In API version 2.36, image APIs were deprecated.
-        server[self._image_ref_parameter] = self.api.get_images()[0]['id']
+        # In API version 2.36 or greater, self.api.get_images() returns
+        # a 404 error. In that case, 'image_uuid' should be specified.
+        server[self._image_ref_parameter] = (image_uuid or
+                                             self.api.get_images()[0]['id'])
 
         # Set a valid flavorId
         flavor = self.api.get_flavors()[0]

nova/tests/functional/test_servers.py

@@ -24,6 +24,7 @@ from oslo_utils import timeutils
 
 from nova.compute import api as compute_api
 from nova.compute import instance_actions
+from nova.compute import manager as compute_manager
 from nova.compute import rpcapi
 from nova import context
 from nova import db
@@ -2995,3 +2996,83 @@ class ServerSoftDeleteTests(ProviderUsageBaseTestCase):
         # Now we want a real delete
         self.flags(reclaim_instance_interval=0)
         self._delete_and_check_allocations(server)
+
+
+class ServerTestV256Common(ServersTestBase):
+    api_major_version = 'v2.1'
+    microversion = '2.56'
+    ADMIN_API = True
+
+    def _create_server(self):
+        server = self._build_minimal_create_server_request(
+            image_uuid='a2459075-d96c-40d5-893e-577ff92e721c')
+        server.update({'networks': 'auto'})
+        post = {'server': server}
+        response = self.api.api_post('/servers', post).body
+        return response['server']
+
+
+class ServerTestV256SingleCellMultiHostTestCase(ServerTestV256Common):
+    """Happy path test where we create a server on one host, migrate it to
+    another host of our choosing and ensure it lands there.
+    """
+    def _setup_compute_service(self):
+        # Set up 3 compute services in the same cell
+        for host in ('host1', 'host2', 'host3'):
+            fake.set_nodes([host])
+            self.addCleanup(fake.restore_nodes)
+            self.start_service('compute', host=host)
+
+    @staticmethod
+    def _get_target_and_other_hosts(host):
+        target_other_hosts = {'host1': ['host2', 'host3'],
+                              'host2': ['host3', 'host1'],
+                              'host3': ['host1', 'host2']}
+        return target_other_hosts[host]
+
+    def test_migrate_server_to_host_in_same_cell(self):
+        server = self._create_server()
+        server = self._wait_for_state_change(server, 'BUILD')
+        source_host = server['OS-EXT-SRV-ATTR:host']
+        target_host = self._get_target_and_other_hosts(source_host)[0]
+        self.api.post_server_action(server['id'],
+                                    {'migrate': {'host': target_host}})
+        # Assert the server is now on the target host.
+        server = self.api.get_server(server['id'])
+        self.assertEqual(target_host, server['OS-EXT-SRV-ATTR:host'])
+
+
+class ServerTestV256RescheduleTestCase(ServerTestV256Common):
+
+    def _setup_compute_service(self):
+        # Set up 3 compute services in the same cell
+        for host in ('host1', 'host2', 'host3'):
+            fake.set_nodes([host])
+            self.addCleanup(fake.restore_nodes)
+            self.start_service('compute', host=host)
+
+    @staticmethod
+    def _get_target_and_other_hosts(host):
+        target_other_hosts = {'host1': ['host2', 'host3'],
+                              'host2': ['host3', 'host1'],
+                              'host3': ['host1', 'host2']}
+        return target_other_hosts[host]
+
+    @mock.patch.object(compute_manager.ComputeManager, '_prep_resize',
+                       side_effect=exception.MigrationError(
+                           reason='Test Exception'))
+    def test_migrate_server_not_reschedule(self, mock_prep_resize):
+        server = self._create_server()
+        found_server = self._wait_for_state_change(server, 'BUILD')
+
+        target_host, other_host = self._get_target_and_other_hosts(
+            found_server['OS-EXT-SRV-ATTR:host'])
+
+        self.assertRaises(client.OpenStackApiException,
+                          self.api.post_server_action,
+                          server['id'],
+                          {'migrate': {'host': target_host}})
+        self.assertEqual(1, mock_prep_resize.call_count)
+        found_server = self.api.get_server(server['id'])
+        # Check that rescheduling is not occurred.
+        self.assertNotEqual(other_host, found_server['OS-EXT-SRV-ATTR:host'])

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

@@ -230,7 +230,7 @@ class CommonTests(CommonMixin, test.NoDBTestCase):
         body_map = body_map or {}
         for action in actions:
             self._test_non_existing_instance(action,
-                                             body_map=body_map)
+                                             body_map=body_map.get(action))
             # Re-mock this.
             self.mox.StubOutWithMock(self.compute_api, 'get')
 
@@ -247,7 +247,7 @@ class CommonTests(CommonMixin, test.NoDBTestCase):
             self.mox.StubOutWithMock(self.compute_api,
                                      method or action.replace('_', ''))
             self._test_invalid_state(action, method=method,
-                                     body_map=body_map,
+                                     body_map=body_map.get(action),
                                      compute_api_args_map=args_map,
                                      exception_arg=exception_arg)
             # Re-mock this.

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

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 from oslo_utils import uuidutils
 import six
 import webob
@@ -21,9 +22,11 @@ from nova.api.openstack import api_version_request
 from nova.api.openstack.compute import migrate_server as \
     migrate_server_v21
 from nova import exception
+from nova import objects
 from nova import test
 from nova.tests.unit.api.openstack.compute import admin_only_action_common
 from nova.tests.unit.api.openstack import fakes
+from nova.tests import uuidsentinel as uuids
 
 
 class MigrateServerTestsV21(admin_only_action_common.CommonTests):
@@ -34,6 +37,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
     disk_over_commit = False
     force = None
     async = False
+    host_name = None
 
     def setUp(self):
         super(MigrateServerTestsV21, self).setUp()
@@ -61,7 +65,8 @@ 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', self.force, self.async),
-                                      {})}
+                                      {}),
+                    '_migrate': ((), {'host_name': self.host_name})}
         self._test_actions(['_migrate', '_migrate_live'], body_map=body_map,
                            method_translations=method_translations,
                            args_map=args_map)
@@ -72,23 +77,27 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
         body_map = {'_migrate_live': self._get_migration_body(host=None)}
         args_map = {'_migrate_live': ((False, self.disk_over_commit, None,
                                        self.force, self.async),
-                                      {})}
+                                      {}),
+                    '_migrate': ((), {'host_name': None})}
         self._test_actions(['_migrate', '_migrate_live'], body_map=body_map,
                            method_translations=method_translations,
                            args_map=args_map)
 
     def test_migrate_with_non_existed_instance(self):
-        body_map = self._get_migration_body(host='hostname')
+        body_map = {'_migrate_live':
+                    self._get_migration_body(host='hostname')}
         self._test_actions_with_non_existed_instance(
             ['_migrate', '_migrate_live'], body_map=body_map)
 
     def test_migrate_raise_conflict_on_invalid_state(self):
         method_translations = {'_migrate': 'resize',
                                '_migrate_live': 'live_migrate'}
-        body_map = self._get_migration_body(host='hostname')
+        body_map = {'_migrate_live':
+                    self._get_migration_body(host='hostname')}
         args_map = {'_migrate_live': ((False, self.disk_over_commit,
                                        'hostname', self.force, self.async),
-                                      {})}
+                                      {}),
+                    '_migrate': ((), {'host_name': self.host_name})}
         exception_arg = {'_migrate': 'migrate',
                          '_migrate_live': 'os-migrateLive'}
         self._test_actions_raise_conflict_on_invalid_state(
@@ -104,7 +113,8 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
                     self._get_migration_body(host='hostname')}
         args_map = {'_migrate_live': ((False, self.disk_over_commit,
                                        'hostname', self.force, self.async),
-                                      {})}
+                                      {}),
+                    '_migrate': ((), {'host_name': self.host_name})}
         self._test_actions_with_locked_instance(
             ['_migrate', '_migrate_live'], body_map=body_map,
             args_map=args_map, method_translations=method_translations)
@@ -112,12 +122,14 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests):
     def _test_migrate_exception(self, exc_info, expected_result):
         self.mox.StubOutWithMock(self.compute_api, 'resize')
         instance = self._stub_instance_get()
-        self.compute_api.resize(self.context, instance).AndRaise(exc_info)
+        self.compute_api.resize(
+            self.context, instance,
+            host_name=self.host_name).AndRaise(exc_info)
 
         self.mox.ReplayAll()
         self.assertRaises(expected_result,
                           self.controller._migrate,
-                          self.req, instance['uuid'], {'migrate': None})
+                          self.req, instance['uuid'], body={'migrate': None})
 
     def test_migrate_too_many_instances(self):
         exc_info = exception.TooManyInstances(overs='', req='', used=0,
@@ -438,6 +450,100 @@ class MigrateServerTestsV234(MigrateServerTestsV230):
                           self.req, instance.uuid, body=body)
 
 
+class MigrateServerTestsV256(MigrateServerTestsV234):
+    host_name = 'fake-host'
+    method_translations = {'_migrate': 'resize'}
+    body_map = {'_migrate': {'migrate': {'host': host_name}}}
+    args_map = {'_migrate': ((), {'host_name': host_name})}
+
+    def setUp(self):
+        super(MigrateServerTestsV256, self).setUp()
+        self.req.api_version_request = api_version_request.APIVersionRequest(
+            '2.56')
+
+    def _test_migrate_validation_error(self, body):
+        self.assertRaises(self.validation_error,
+                          self.controller._migrate,
+                          self.req, fakes.FAKE_UUID, body=body)
+
+    def _test_migrate_exception(self, exc_info, expected_result):
+        @mock.patch.object(self.compute_api, 'get')
+        @mock.patch.object(self.compute_api, 'resize', side_effect=exc_info)
+        def _test(mock_resize, mock_get):
+            instance = objects.Instance(uuid=uuids.instance)
+            self.assertRaises(expected_result,
+                              self.controller._migrate,
+                              self.req, instance['uuid'],
+                              body={'migrate': {'host': self.host_name}})
+        _test()
+
+    def test_migrate(self):
+        self._test_actions(['_migrate'], body_map=self.body_map,
+                           method_translations=self.method_translations,
+                           args_map=self.args_map)
+
+    def test_migrate_without_host(self):
+        # The request body is: '{"migrate": null}'
+        body_map = {'_migrate': {'migrate': None}}
+        args_map = {'_migrate': ((), {'host_name': None})}
+        self._test_actions(['_migrate'], body_map=body_map,
+                           method_translations=self.method_translations,
+                           args_map=args_map)
+
+    def test_migrate_none_hostname(self):
+        # The request body is: '{"migrate": {"host": null}}'
+        body_map = {'_migrate': {'migrate': {'host': None}}}
+        args_map = {'_migrate': ((), {'host_name': None})}
+        self._test_actions(['_migrate'], body_map=body_map,
+                           method_translations=self.method_translations,
+                           args_map=args_map)
+
+    def test_migrate_with_non_existed_instance(self):
+        self._test_actions_with_non_existed_instance(
+            ['_migrate'], body_map=self.body_map)
+
+    def test_migrate_raise_conflict_on_invalid_state(self):
+        exception_arg = {'_migrate': 'migrate'}
+        self._test_actions_raise_conflict_on_invalid_state(
+            ['_migrate'], body_map=self.body_map,
+            args_map=self.args_map,
+            method_translations=self.method_translations,
+            exception_args=exception_arg)
+
+    def test_actions_with_locked_instance(self):
+        self._test_actions_with_locked_instance(
+            ['_migrate'], body_map=self.body_map,
+            args_map=self.args_map,
+            method_translations=self.method_translations)
+
+    def test_migrate_without_migrate_object(self):
+        self._test_migrate_validation_error({})
+
+    def test_migrate_invalid_migrate_object(self):
+        self._test_migrate_validation_error({'migrate': 'fake-host'})
+
+    def test_migrate_with_additional_property(self):
+        self._test_migrate_validation_error(
+            {'migrate': {'host': self.host_name,
+                         'additional': 'foo'}})
+
+    def test_migrate_with_host_length_more_than_255(self):
+        self._test_migrate_validation_error(
+            {'migrate': {'host': 'a' * 256}})
+
+    def test_migrate_nonexistent_host(self):
+        exc_info = exception.ComputeHostNotFound(host='nonexistent_host')
+        self._test_migrate_exception(exc_info, webob.exc.HTTPBadRequest)
+
+    def test_migrate_no_request_spec(self):
+        exc_info = exception.CannotMigrateWithTargetHost()
+        self._test_migrate_exception(exc_info, webob.exc.HTTPConflict)
+
+    def test_migrate_to_same_host(self):
+        exc_info = exception.CannotMigrateToSameHost()
+        self._test_migrate_exception(exc_info, webob.exc.HTTPBadRequest)
+
+
 class MigrateServerPolicyEnforcementV21(test.NoDBTestCase):
 
     def setUp(self):

releasenotes/notes/cold-migration-with-target-queens-2dcd09c3a3414302.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - When cold migrating a server, the ``host`` parameter is available
+    as of microversion 2.56. The target host is checked by the scheduler.