openstack/nova
Code Issues Proposed changes

API: Remove evacuate/live-migrate 'force' parameter

Browse Source 
Add a new microversion that removes support for the aforementioned
argument, which cannot be adequately guaranteed in the new placement
world.

Change-Id: I2a395aa6eccad75a97fa49e993b0300bdcfc7258
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Implements: blueprint remove-force-flag-from-live-migrate-and-evacuate
APIImpact
This commit is contained in:
Stephen Finucane 2019-02-03 10:41:03 +00:00 committed by Matt Riedemann
parent c1fb445b8d
commit 36a91936a8
22 changed files with 297 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
api-ref/source/parameters.yaml
View File

@ -3143,6 +3143,7 @@ force_evacuate:
required: false
type: boolean
min_version: 2.29
max_version: 2.67
force_live_migrate:
description: |
Force a live-migration by not verifying the provided destination host by
@ -3156,6 +3157,7 @@ force_live_migrate:
required: false
type: boolean
min_version: 2.30
max_version: 2.67
force_migration_complete:
description: |
The action to force an in-progress live migration to complete.

4
api-ref/source/servers-action-evacuate.inc
View File

@ -11,6 +11,10 @@ Evacuates a server from a failed host to a new host.
- In the request body, if ``onSharedStorage`` is set, then do not set ``adminPass``.
- The target host should not be the same as the instance host.
Starting from API version 2.68, the ``force`` parameter is no longer accepted
as this could not be meaningfully supported by servers with complex resource
allocations.
Normal response codes: 200
Error response codes: badRequest(400), unauthorized(401), forbidden(403),

4
api-ref/source/servers-admin-action.inc
View File

@ -137,6 +137,10 @@ Nova responds immediately, and no pre-live-migration checks are returned.
The instance will not immediately change state to ``ERROR``, if a failure of
the live-migration checks occurs.
Starting from API version 2.68, the ``force`` parameter is no longer accepted
as this could not be meaningfully supported by servers with complex resource
allocations.
Normal response codes: 202
Error response codes: badRequest(400), unauthorized(401), forbidden(403)

6
doc/api_samples/os-migrate-server/v2.68/live-migrate-server.json Normal file
View File

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

2
doc/api_samples/versions/v21-version-get-resp.json
View File

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

2
doc/api_samples/versions/versions-get-resp.json
View File

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

4
nova/api/openstack/api_version_request.py
View File

@ -164,6 +164,8 @@ REST_API_VERSION_HISTORY = """REST API Version History:
os-migrations API.
* 2.67 - Adds the optional ``volume_type`` field to the
``block_device_mapping_v2`` parameter when creating a server.
* 2.68 - Remove support for forced live migration and evacuate server
actions.
"""
# The minimum and maximum versions of the API supported
@ -172,7 +174,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.67"
_MAX_API_VERSION = "2.68"
DEFAULT_API_VERSION = _MIN_API_VERSION
# Almost all proxy APIs which are related to network, images and baremetal

3
nova/api/openstack/compute/evacuate.py
View File

@ -75,7 +75,8 @@ class EvacuateController(wsgi.Controller):
@wsgi.action('evacuate')
@validation.schema(evacuate.evacuate, "2.0", "2.13")
@validation.schema(evacuate.evacuate_v214, "2.14", "2.28")
@validation.schema(evacuate.evacuate_v2_29, "2.29")
@validation.schema(evacuate.evacuate_v2_29, "2.29", "2.67")
@validation.schema(evacuate.evacuate_v2_68, "2.68")
def _evacuate(self, req, id, body):
"""Permit admins to evacuate a server from a failed host
to a new one.

3
nova/api/openstack/compute/migrate_server.py
View File

@ -84,7 +84,8 @@ class MigrateServerController(wsgi.Controller):
@wsgi.action('os-migrateLive')
@validation.schema(migrate_server.migrate_live, "2.0", "2.24")
@validation.schema(migrate_server.migrate_live_v2_25, "2.25", "2.29")
@validation.schema(migrate_server.migrate_live_v2_30, "2.30")
@validation.schema(migrate_server.migrate_live_v2_30, "2.30", "2.67")
@validation.schema(migrate_server.migrate_live_v2_68, "2.68")
def _migrate_live(self, req, id, body):
"""Permit admins to (live) migrate a server to a new host."""
context = req.environ["nova.context"]

5
nova/api/openstack/compute/rest_api_version_history.rst
View File

@ -860,3 +860,8 @@ following APIs to filter by changes before or equal to the resource
Adds the ``volume_type`` parameter to ``block_device_mapping_v2``, which can
be used to specify cinder ``volume_type`` when creating a server.
2.68
----
Remove support for forced live migration and evacuate server actions.

4
nova/api/openstack/compute/schemas/evacuate.py
View File

@ -42,3 +42,7 @@ del evacuate_v214['properties']['evacuate']['required']
evacuate_v2_29 = copy.deepcopy(evacuate_v214)
evacuate_v2_29['properties']['evacuate']['properties'][
'force'] = parameter_types.boolean
# v2.68 removes the 'force' parameter added in v2.29, meaning it is identical
# to v2.14
evacuate_v2_68 = copy.deepcopy(evacuate_v214)

4
nova/api/openstack/compute/schemas/migrate_server.py
View File

@ -68,3 +68,7 @@ migrate_live_v2_25['properties']['os-migrateLive']['required'] = (
migrate_live_v2_30 = copy.deepcopy(migrate_live_v2_25)
migrate_live_v2_30['properties']['os-migrateLive']['properties'][
'force'] = parameter_types.boolean
# v2.68 removes the 'force' parameter added in v2.30, meaning it is identical
# to v2.25
migrate_live_v2_68 = copy.deepcopy(migrate_live_v2_25)

2
nova/compute/api.py
View File

@ -4659,6 +4659,8 @@ class API(base.Base):
source=fields_obj.NotificationSource.API)
# NOTE(sbauza): Force is a boolean by the new related API version
# TODO(stephenfin): Any reason we can't use 'not force' here to handle
# the pre-v2.29 API microversion, which wouldn't set force
if force is False and host:
nodes = objects.ComputeNodeList.get_all_by_host(context, host)
# NOTE(sbauza): Unset the host to make sure we call the scheduler

5
nova/tests/functional/api_sample_tests/api_samples/os-evacuate/v2.68/server-evacuate-find-host-req.json.tpl Normal file
View File

@ -0,0 +1,5 @@
{
"evacuate": {
"adminPass": "%(adminPass)s"
}
}

6
nova/tests/functional/api_sample_tests/api_samples/os-evacuate/v2.68/server-evacuate-req.json.tpl Normal file
View File

@ -0,0 +1,6 @@
{
"evacuate": {
"host": "%(host)s",
"adminPass": "%(adminPass)s"
}
}

6
nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.68/live-migrate-server.json.tpl Normal file
View File

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

30
nova/tests/functional/api_sample_tests/test_evacuate.py
View File

@ -180,3 +180,33 @@ class EvacuateJsonTestV229(EvacuateJsonTestV214):
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
on_shared_storage=None, preserve_ephemeral=mock.ANY,
host='testHost', request_spec=mock.ANY)
class EvacuateJsonTestV268(EvacuateJsonTestV229):
microversion = '2.68'
scenarios = [('v2_68', {'api_major_version': 'v2.1'})]
@mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance')
@mock.patch('nova.objects.ComputeNodeList.get_all_by_host')
def test_server_evacuate(self, compute_node_get_all_by_host, rebuild_mock):
# Note (wingwj): The host can't be the same one
req_subs = {
'host': 'testHost',
"adminPass": "MySecretPass",
}
fake_computes = objects.ComputeNodeList(
objects=[objects.ComputeNode(host='testHost',
hypervisor_hostname='host')])
compute_node_get_all_by_host.return_value = fake_computes
self._test_evacuate(req_subs, 'server-evacuate-req',
server_resp=None, expected_resp_code=200)
rebuild_mock.assert_called_once_with(mock.ANY, instance=mock.ANY,
orig_image_ref=mock.ANY, image_ref=mock.ANY,
injected_files=mock.ANY, new_pass="MySecretPass",
orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY,
on_shared_storage=None, preserve_ephemeral=mock.ANY,
host=None, request_spec=mock.ANY)
def test_server_evacuate_with_force(self):
# doesn't apply to v2.68+, which removed the ability to force migrate
pass

67
nova/tests/functional/api_sample_tests/test_migrate_server.py
View File

@ -21,6 +21,21 @@ from nova import objects
from nova.tests.functional.api_sample_tests import test_servers
def fake_get_compute(context, host):
# TODO(stephenfin): It's gross that we even need this in a functional test
# where we can control the running compute services. Stop doing it.
service = dict(host=host,
binary='nova-compute',
topic='compute',
report_count=1,
updated_at='foo',
hypervisor_type='bar',
hypervisor_version=(
versionutils.convert_version_to_int('1.0')),
disabled=False)
return {'compute_node': [service]}
class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase):
sample_dir = "os-migrate-server"
USE_NEUTRON = True
@ -47,26 +62,15 @@ class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase):
def fake_live_migrate(_self, context, instance, scheduler_hint,
block_migration, disk_over_commit, request_spec):
self.assertEqual(self.uuid, instance["uuid"])
self.assertEqual(self.uuid, instance.uuid)
host = scheduler_hint["host"]
self.assertEqual(self.host_attended, host)
self.stub_out(
'nova.conductor.manager.ComputeTaskManager._live_migrate',
fake_live_migrate)
def fake_get_compute(context, host):
service = dict(host=host,
binary='nova-compute',
topic='compute',
report_count=1,
updated_at='foo',
hypervisor_type='bar',
hypervisor_version=(
versionutils.convert_version_to_int('1.0')),
disabled=False)
return {'compute_node': [service]}
self.stub_out("nova.db.api.service_get_by_compute_host",
self.stub_out(
'nova.db.api.service_get_by_compute_host',
fake_get_compute)
response = self._do_post('servers/%s/action' % self.uuid,
@ -183,3 +187,38 @@ class MigrateServerSamplesJsonTestV256(test_servers.ServersSampleBase):
response = self._do_post('servers/%s/action' % self.uuid,
'migrate-server-null', {})
self.assertEqual(202, response.status_code)
class MigrateServerSamplesJsonTestV268(test_servers.ServersSampleBase):
sample_dir = "os-migrate-server"
microversion = '2.68'
scenarios = [('v2_68', {'api_major_version': 'v2.1'})]
USE_NEUTRON = True
def setUp(self):
"""setUp Method for MigrateServer api samples extension
This method creates the server that will be used in each tests
"""
super(MigrateServerSamplesJsonTestV268, self).setUp()
self.uuid = self._post_server()
def test_post_live_migrate_server(self):
# Get api samples to server live migrate request.
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)
self.stub_out(
'nova.conductor.manager.ComputeTaskManager._live_migrate',
fake_live_migrate)
self.stub_out(
'nova.db.api.service_get_by_compute_host',
fake_get_compute)
response = self._do_post('servers/%s/action' % self.uuid,
'live-migrate-server',
req_subs)
self.assertEqual(202, response.status_code)

88
nova/tests/functional/test_servers.py
View File

@ -2450,6 +2450,9 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
# the ability to force evacuate a server is removed entirely in 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
@ -2513,6 +2516,29 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
self._delete_and_check_allocations(server)
def test_evacuate_forced_host_v268(self):
"""Evacuating a server with a forced host was removed in API
microversion 2.68. This test ensures that the request is rejected.
"""
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
# evacuate the server and force the destination host which bypasses
# the scheduler
post = {
'evacuate': {
'host': dest_hostname,
'force': True
}
}
ex = self.assertRaises(client.OpenStackApiException,
self.api.post_server_action,
server['id'], post)
self.assertIn("'force' was unexpected", six.text_type(ex))
# NOTE(gibi): there is a similar test in SchedulerOnlyChecksTargetTest but
# we want this test here as well because ServerMovingTest is a parent class
# of multiple test classes that run this test case with different compute
@ -2542,7 +2568,6 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
post = {
'evacuate': {
'host': dest_hostname,
'force': False
}
}
self.api.post_server_action(server['id'], post)
@ -2863,8 +2888,15 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
source_rp_uuid = self._get_provider_uuid_by_host(source_hostname)
dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname)
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
# live migrate the server and force the destination host which bypasses
# the scheduler
post = {
'os-migrateLive': {
'host': dest_hostname,
@ -2894,6 +2926,31 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
self._delete_and_check_allocations(server)
def test_live_migrate_forced_v268(self):
"""Live migrating a server with a forced host was removed in API
microversion 2.68. This test ensures that the request is rejected.
"""
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
# live migrate the server and force the destination host which bypasses
# the scheduler
post = {
'os-migrateLive': {
'host': dest_hostname,
'block_migration': True,
'force': True,
}
}
ex = self.assertRaises(client.OpenStackApiException,
self.api.post_server_action,
server['id'], post)
self.assertIn("'force' was unexpected", six.text_type(ex))
def test_live_migrate(self):
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
@ -3011,6 +3068,10 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase):
source_rp_uuid = self._get_provider_uuid_by_host(source_hostname)
dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname)
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
@ -4769,6 +4830,10 @@ class ConsumerGenerationConflictTest(
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor, source_hostname)
@ -4810,6 +4875,10 @@ class ConsumerGenerationConflictTest(
source_rp_uuid = self._get_provider_uuid_by_host(source_hostname)
dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname)
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor, source_hostname)
@ -4894,6 +4963,9 @@ class ConsumerGenerationConflictTest(
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
# the ability to force evacuate a server is removed entirely in 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor, source_hostname)
@ -5072,6 +5144,10 @@ class ServerMovingTestsWithNestedResourceRequests(
source_rp_uuid = self._get_provider_uuid_by_host(source_hostname)
dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname)
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
post = {
@ -5116,6 +5192,9 @@ class ServerMovingTestsWithNestedResourceRequests(
source_hostname = self.compute1.host
dest_hostname = self.compute2.host
# the ability to force evacuate a server is removed entirely in 2.68
self.api.microversion = '2.67'
server = self._boot_and_check_allocations(
self.flavor1, source_hostname)
@ -5213,6 +5292,10 @@ class ServerMovingTestsFromFlatToNested(
# CUSTOM_MAGIC inventory to the root compute RP
orig_update_provider_tree = fake.MediumFakeDriver.update_provider_tree
# the ability to force live migrate a server is removed entirely in
# 2.68
self.api.microversion = '2.67'
def stub_update_provider_tree(self, provider_tree, nodename,
allocations=None):
# do the regular inventory update
@ -5293,6 +5376,9 @@ class ServerMovingTestsFromFlatToNested(
# CUSTOM_MAGIC inventory to the root compute RP
orig_update_provider_tree = fake.MediumFakeDriver.update_provider_tree
# the ability to force evacuate a server is removed entirely in 2.68
self.api.microversion = '2.67'
def stub_update_provider_tree(self, provider_tree, nodename,
allocations=None):
# do the regular inventory update

32
nova/tests/unit/api/openstack/compute/test_evacuate.py
View File

@ -95,8 +95,7 @@ class EvacuateTestV21(test.NoDBTestCase):
controller=None):
controller = controller or self.controller
body = {'evacuate': body}
self.assertRaises(exception,
controller._evacuate,
return self.assertRaises(exception, controller._evacuate,
self.admin_req, uuid or self.UUID, body=body)
def test_evacuate_with_valid_instance(self):
@ -367,8 +366,7 @@ class EvacuateTestV214(EvacuateTestV21):
controller = controller or self.controller
body.pop('onSharedStorage', None)
body = {'evacuate': body}
self.assertRaises(exception,
controller._evacuate,
return self.assertRaises(exception, controller._evacuate,
self.admin_req, uuid or self.UUID, body=body)
@mock.patch.object(compute_api.API, 'evacuate')
@ -479,3 +477,29 @@ class EvacuateTestV229(EvacuateTestV214):
def test_forced_evacuate_with_no_host_provided(self):
self._check_evacuate_failure(webob.exc.HTTPBadRequest,
{'force': 'true'})
class EvacuateTestV268(EvacuateTestV229):
def setUp(self):
super(EvacuateTestV268, self).setUp()
self.admin_req = fakes.HTTPRequest.blank('', use_admin_context=True,
version='2.68')
self.req = fakes.HTTPRequest.blank('', version='2.68')
def test_evacuate_with_valid_instance(self):
admin_pass = 'MyNewPass'
res = self._get_evacuate_response({'host': 'my-host',
'adminPass': admin_pass})
self.assertIsNone(res)
@mock.patch.object(compute_api.API, 'evacuate')
def test_evacuate_instance_with_forced_host(self, mock_evacuate):
ex = self._check_evacuate_failure(self.validation_error,
{'host': 'my-host',
'force': 'true'})
self.assertIn('force', six.text_type(ex))
def test_forced_evacuate_with_no_host_provided(self):
# not applicable for v2.68, which removed the 'force' parameter
pass

34
nova/tests/unit/api/openstack/compute/test_migrate_server.py
View File

@ -372,13 +372,13 @@ class MigrateServerTestsV230(MigrateServerTestsV225):
def _test_live_migrate(self, force=False):
if force is True:
litteral_force = 'true'
literal_force = 'true'
else:
litteral_force = 'false'
literal_force = 'false'
method_translations = {'_migrate_live': 'live_migrate'}
body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto',
'force': litteral_force}}}
'force': literal_force}}}
args_map = {'_migrate_live': ((None, None, 'hostname', force,
self.async_), {})}
self._test_actions(['_migrate_live'], body_map=body_map,
@ -602,6 +602,34 @@ class MigrateServerTestsV256(MigrateServerTestsV234):
self._test_migrate_exception(exc_info, webob.exc.HTTPBadRequest)
class MigrateServerTestsV268(MigrateServerTestsV256):
force = False
def setUp(self):
super(MigrateServerTestsV268, self).setUp()
self.req.api_version_request = api_version_request.APIVersionRequest(
'2.68')
def test_live_migrate(self):
method_translations = {'_migrate_live': 'live_migrate'}
body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto'}}}
args_map = {'_migrate_live': ((None, None, 'hostname', False,
self.async_), {})}
self._test_actions(['_migrate_live'], body_map=body_map,
method_translations=method_translations,
args_map=args_map)
def test_live_migrate_with_forced_host(self):
body = {'os-migrateLive': {'host': 'hostname',
'block_migration': 'auto',
'force': 'true'}}
ex = self.assertRaises(self.validation_error,
self.controller._migrate_live,
self.req, fakes.FAKE_UUID, body=body)
self.assertIn('force', six.text_type(ex))
class MigrateServerPolicyEnforcementV21(test.NoDBTestCase):
def setUp(self):

8
releasenotes/notes/remove-live-migrate-evacuate-force-flag-cb50608d5930585c.yaml Normal file
View File

@ -0,0 +1,8 @@
---
upgrade:
- |
It is no longer possible to force server live migrations or evacuations
to a specific destination host starting with API microversion 2.68.
This is because it is not possible to support these requests for servers
with complex resource allocations. It is still possible to request a
destination host but it will be validated by the scheduler.