From f18a46c072b964b6c9ceffc9832e4221f64dc9c4 Mon Sep 17 00:00:00 2001 From: Eli Qiao Date: Thu, 17 Dec 2015 15:50:11 +0800 Subject: [PATCH] API: Improve os-migrateLive input parameters This is os-migrateLive API changes: * 2.25 - Make block_migration to support 'auto' value, remove disk_over_commit. Partially implements: blueprint making-live-migration-api-friendly APIImpact DocImpact Change-Id: Ibb0d50f0f7444028ef9d0c294aea41edf0024b31 --- .../v2.25/live-migrate-server.json | 6 + .../versions/v21-version-get-resp.json | 2 +- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 4 +- nova/api/openstack/compute/migrate_server.py | 28 ++-- .../compute/schemas/migrate_server.py | 12 ++ .../openstack/rest_api_version_history.rst | 6 + nova/conductor/manager.py | 29 ++-- .../v2.25/live-migrate-server.json.tpl | 6 + .../api_sample_tests/test_migrate_server.py | 10 ++ .../openstack/compute/test_migrate_server.py | 127 ++++++++++++------ nova/tests/unit/conductor/test_conductor.py | 1 + ...gration-api-friendly-3b547f4e0958ee05.yaml | 9 ++ 13 files changed, 177 insertions(+), 65 deletions(-) create mode 100644 doc/api_samples/os-migrate-server/v2.25/live-migrate-server.json create mode 100644 nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.25/live-migrate-server.json.tpl create mode 100644 releasenotes/notes/bp-making-live-migration-api-friendly-3b547f4e0958ee05.yaml diff --git a/doc/api_samples/os-migrate-server/v2.25/live-migrate-server.json b/doc/api_samples/os-migrate-server/v2.25/live-migrate-server.json new file mode 100644 index 000000000000..0777861df537 --- /dev/null +++ b/doc/api_samples/os-migrate-server/v2.25/live-migrate-server.json @@ -0,0 +1,6 @@ +{ + "os-migrateLive": { + "host": "01c0cadef72d47e28a672a76060d492c", + "block_migration": "auto" + } +} diff --git a/doc/api_samples/versions/v21-version-get-resp.json b/doc/api_samples/versions/v21-version-get-resp.json index afb4e3dcc294..41151e581139 100644 --- a/doc/api_samples/versions/v21-version-get-resp.json +++ b/doc/api_samples/versions/v21-version-get-resp.json @@ -19,7 +19,7 @@ } ], "status": "CURRENT", - "version": "2.24", + "version": "2.25", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index 1e34a553e7ec..f5bbb1931878 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.24", + "version": "2.25", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index 33f364d99900..431acc12b636 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -69,6 +69,8 @@ REST_API_VERSION_HISTORY = """REST API Version History: Also add migration_type for /os-migrations and add ref link for it when the migration is an in progress live migration. * 2.24 - Add API to cancel a running live migration + * 2.25 - Make block_migration support 'auto' and remove + disk_over_commit for os-migrateLive. """ @@ -78,7 +80,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.24" +_MAX_API_VERSION = "2.25" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index 18437ae7ddf8..b1f4a5e70299 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -16,6 +16,7 @@ from oslo_utils import strutils from webob import exc +from nova.api.openstack import api_version_request from nova.api.openstack import common from nova.api.openstack.compute.schemas import migrate_server from nova.api.openstack import extensions @@ -61,20 +62,30 @@ class MigrateServerController(wsgi.Controller): @wsgi.response(202) @extensions.expected_errors((400, 404, 409)) @wsgi.action('os-migrateLive') - @validation.schema(migrate_server.migrate_live) + @validation.schema(migrate_server.migrate_live, "2.1", "2.24") + @validation.schema(migrate_server.migrate_live_v2_25, "2.25") def _migrate_live(self, req, id, body): """Permit admins to (live) migrate a server to a new host.""" context = req.environ["nova.context"] authorize(context, action='migrate_live') - block_migration = body["os-migrateLive"]["block_migration"] - disk_over_commit = body["os-migrateLive"]["disk_over_commit"] host = body["os-migrateLive"]["host"] + block_migration = body["os-migrateLive"]["block_migration"] - block_migration = strutils.bool_from_string(block_migration, - strict=True) - disk_over_commit = strutils.bool_from_string(disk_over_commit, - strict=True) + if api_version_request.is_supported(req, min_version='2.25'): + if block_migration == 'auto': + block_migration = None + else: + block_migration = strutils.bool_from_string(block_migration, + strict=True) + disk_over_commit = None + else: + disk_over_commit = body["os-migrateLive"]["disk_over_commit"] + + block_migration = strutils.bool_from_string(block_migration, + strict=True) + disk_over_commit = strutils.bool_from_string(disk_over_commit, + strict=True) try: instance = common.get_instance(self.compute_api, context, id) @@ -92,7 +103,8 @@ class MigrateServerController(wsgi.Controller): exception.InvalidSharedStorage, exception.HypervisorUnavailable, exception.MigrationPreCheckError, - exception.LiveMigrationWithOldNovaNotSafe) as ex: + exception.LiveMigrationWithOldNovaNotSafe, + exception.LiveMigrationWithOldNovaNotSupported) as ex: raise exc.HTTPBadRequest(explanation=ex.format_message()) except exception.InstanceIsLocked as e: raise exc.HTTPConflict(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/schemas/migrate_server.py b/nova/api/openstack/compute/schemas/migrate_server.py index 47d9429c2f9a..0121fa9db439 100644 --- a/nova/api/openstack/compute/schemas/migrate_server.py +++ b/nova/api/openstack/compute/schemas/migrate_server.py @@ -37,3 +37,15 @@ migrate_live = { 'required': ['os-migrateLive'], 'additionalProperties': False, } + +block_migration = copy.deepcopy(parameter_types.boolean) +block_migration['enum'].append('auto') + +migrate_live_v2_25 = copy.deepcopy(migrate_live) + +del migrate_live_v2_25['properties']['os-migrateLive']['properties'][ + 'disk_over_commit'] +migrate_live_v2_25['properties']['os-migrateLive']['properties'][ + 'block_migration'] = block_migration +migrate_live_v2_25['properties']['os-migrateLive']['required'] = ( + ['block_migration', 'host']) diff --git a/nova/api/openstack/rest_api_version_history.rst b/nova/api/openstack/rest_api_version_history.rst index c363136c52dc..b49e12a5ef9d 100644 --- a/nova/api/openstack/rest_api_version_history.rst +++ b/nova/api/openstack/rest_api_version_history.rst @@ -212,3 +212,9 @@ user documentation. A new API call to cancel a running live migration:: DELETE /servers//migrations/ + +2.25 +---- + + Modify input parameter for ``os-migrateLive``. The block_migration will + support 'auto' value, and disk_over_commit flag will be removed. diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 68a65c0b0cc1..199d87078a6e 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -160,19 +160,21 @@ class ComputeTaskManager(base.Base): compute_rpcapi.LAST_VERSION = None self.compute_rpcapi = compute_rpcapi.ComputeAPI() - @messaging.expected_exceptions(exception.NoValidHost, - exception.ComputeServiceUnavailable, - exception.InvalidHypervisorType, - exception.InvalidCPUInfo, - exception.UnableToMigrateToSelf, - exception.DestinationHypervisorTooOld, - exception.InvalidLocalStorage, - exception.InvalidSharedStorage, - exception.HypervisorUnavailable, - exception.InstanceInvalidState, - exception.MigrationPreCheckError, - exception.LiveMigrationWithOldNovaNotSafe, - exception.UnsupportedPolicyException) + @messaging.expected_exceptions( + exception.NoValidHost, + exception.ComputeServiceUnavailable, + exception.InvalidHypervisorType, + exception.InvalidCPUInfo, + exception.UnableToMigrateToSelf, + exception.DestinationHypervisorTooOld, + exception.InvalidLocalStorage, + exception.InvalidSharedStorage, + exception.HypervisorUnavailable, + exception.InstanceInvalidState, + exception.MigrationPreCheckError, + exception.LiveMigrationWithOldNovaNotSafe, + exception.LiveMigrationWithOldNovaNotSupported, + exception.UnsupportedPolicyException) def migrate_server(self, context, instance, scheduler_hint, live, rebuild, flavor, block_migration, disk_over_commit, reservations=None, clean_shutdown=True, request_spec=None): @@ -319,6 +321,7 @@ class ComputeTaskManager(base.Base): exception.InstanceInvalidState, exception.MigrationPreCheckError, exception.LiveMigrationWithOldNovaNotSafe, + exception.LiveMigrationWithOldNovaNotSupported, exception.MigrationSchedulerRPCError) as ex: with excutils.save_and_reraise_exception(): # TODO(johngarbutt) - eventually need instance actions here diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.25/live-migrate-server.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.25/live-migrate-server.json.tpl new file mode 100644 index 000000000000..6a1635c279f4 --- /dev/null +++ b/nova/tests/functional/api_sample_tests/api_samples/os-migrate-server/v2.25/live-migrate-server.json.tpl @@ -0,0 +1,6 @@ +{ + "os-migrateLive": { + "host": "%(hostname)s", + "block_migration": "auto" + } +} diff --git a/nova/tests/functional/api_sample_tests/test_migrate_server.py b/nova/tests/functional/api_sample_tests/test_migrate_server.py index 3cb32894d8e9..49b388cf14b3 100644 --- a/nova/tests/functional/api_sample_tests/test_migrate_server.py +++ b/nova/tests/functional/api_sample_tests/test_migrate_server.py @@ -79,3 +79,13 @@ class MigrateServerSamplesJsonTest(test_servers.ServersSampleBase): 'live-migrate-server', {'hostname': self.compute.host}) self.assertEqual(202, response.status_code) + + +class MigrateServerSamplesJsonTestV225(MigrateServerSamplesJsonTest): + extension_name = "os-migrate-server" + microversion = '2.25' + scenarios = [('v2_25', {'api_major_version': 'v2.1'})] + + def test_post_migrate(self): + # no changes for migrate-server + pass diff --git a/nova/tests/unit/api/openstack/compute/test_migrate_server.py b/nova/tests/unit/api/openstack/compute/test_migrate_server.py index f56dfd0f2af1..5c3663329787 100644 --- a/nova/tests/unit/api/openstack/compute/test_migrate_server.py +++ b/nova/tests/unit/api/openstack/compute/test_migrate_server.py @@ -17,6 +17,7 @@ from oslo_utils import uuidutils import six import webob +from nova.api.openstack import api_version_request from nova.api.openstack.compute.legacy_v2.contrib import admin_actions as \ migrate_server_v2 from nova.api.openstack.compute import migrate_server as \ @@ -32,6 +33,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): controller_name = 'MigrateServerController' validation_error = exception.ValidationError _api_version = '2.1' + disk_over_commit = False def setUp(self): super(MigrateServerTestsV21, self).setUp() @@ -45,13 +47,20 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): _fake_controller) self.mox.StubOutWithMock(self.compute_api, 'get') + def _get_migration_body(self, **kwargs): + return {'os-migrateLive': self._get_params(**kwargs)} + + def _get_params(self, **kwargs): + return {'host': kwargs.get('host'), + 'block_migration': kwargs.get('block_migration') or False, + 'disk_over_commit': self.disk_over_commit} + def test_migrate(self): method_translations = {'_migrate': 'resize', '_migrate_live': 'live_migrate'} - body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}}} - args_map = {'_migrate_live': ((False, False, 'hostname'), {})} + body_map = {'_migrate_live': self._get_migration_body(host='hostname')} + args_map = {'_migrate_live': ((False, self.disk_over_commit, + 'hostname'), {})} self._test_actions(['_migrate', '_migrate_live'], body_map=body_map, method_translations=method_translations, args_map=args_map) @@ -59,28 +68,24 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): def test_migrate_none_hostname(self): method_translations = {'_migrate': 'resize', '_migrate_live': 'live_migrate'} - body_map = {'_migrate_live': {'os-migrateLive': {'host': None, - 'block_migration': False, - 'disk_over_commit': False}}} - args_map = {'_migrate_live': ((False, False, None), {})} + body_map = {'_migrate_live': self._get_migration_body(host=None)} + args_map = {'_migrate_live': ((False, self.disk_over_commit, 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 = {'os-migrateLive': {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}} + body_map = 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 = {'os-migrateLive': {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}} - args_map = {'_migrate_live': ((False, False, 'hostname'), {})} + body_map = self._get_migration_body(host='hostname') + args_map = {'_migrate_live': ((False, self.disk_over_commit, + 'hostname'), {})} exception_arg = {'_migrate': 'migrate', '_migrate_live': 'os-migrateLive'} self._test_actions_raise_conflict_on_invalid_state( @@ -91,10 +96,11 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): def test_actions_with_locked_instance(self): method_translations = {'_migrate': 'resize', '_migrate_live': 'live_migrate'} - body_map = {'_migrate_live': {'os-migrateLive': {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}}} - args_map = {'_migrate_live': ((False, False, 'hostname'), {})} + + body_map = {'_migrate_live': + self._get_migration_body(host='hostname')} + args_map = {'_migrate_live': ((False, self.disk_over_commit, + 'hostname'), {})} self._test_actions_with_locked_instance( ['_migrate', '_migrate_live'], body_map=body_map, args_map=args_map, method_translations=method_translations) @@ -118,7 +124,7 @@ 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, - False, 'hostname') + self.disk_over_commit, 'hostname') self.mox.ReplayAll() @@ -133,9 +139,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self.assertEqual(202, status_int) def test_migrate_live_enabled(self): - param = {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False} + param = self._get_params(host='hostname') self._test_migrate_live_succeeded(param) def test_migrate_live_enabled_with_string_param(self): @@ -145,17 +149,15 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self._test_migrate_live_succeeded(param) def test_migrate_live_without_host(self): - body = {'os-migrateLive': - {'block_migration': False, - 'disk_over_commit': False}} + body = self._get_migration_body() + del body['os-migrateLive']['host'] self.assertRaises(self.validation_error, self.controller._migrate_live, self.req, fakes.FAKE_UUID, body=body) def test_migrate_live_without_block_migration(self): - body = {'os-migrateLive': - {'host': 'hostname', - 'disk_over_commit': False}} + body = self._get_migration_body() + del body['os-migrateLive']['block_migration'] self.assertRaises(self.validation_error, self.controller._migrate_live, self.req, fakes.FAKE_UUID, body=body) @@ -169,10 +171,7 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self.req, fakes.FAKE_UUID, body=body) def test_migrate_live_with_invalid_block_migration(self): - body = {'os-migrateLive': - {'host': 'hostname', - 'block_migration': "foo", - 'disk_over_commit': False}} + body = self._get_migration_body(block_migration='foo') self.assertRaises(self.validation_error, self.controller._migrate_live, self.req, fakes.FAKE_UUID, body=body) @@ -187,9 +186,9 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): self.req, fakes.FAKE_UUID, body=body) def test_migrate_live_missing_dict_param(self): - body = {'os-migrateLive': {'dummy': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}} + body = self._get_migration_body(host='hostname') + del body['os-migrateLive']['host'] + body['os-migrateLive']['dummy'] = 'hostname' self.assertRaises(self.validation_error, self.controller._migrate_live, self.req, fakes.FAKE_UUID, body=body) @@ -203,14 +202,12 @@ class MigrateServerTestsV21(admin_only_action_common.CommonTests): instance = self._stub_instance_get(uuid=uuid) self.compute_api.live_migrate(self.context, instance, False, - False, 'hostname').AndRaise(fake_exc) + self.disk_over_commit, + 'hostname').AndRaise(fake_exc) self.mox.ReplayAll() - body = {'os-migrateLive': - {'host': 'hostname', - 'block_migration': False, - 'disk_over_commit': False}} + body = self._get_migration_body(host='hostname') ex = self.assertRaises(expected_exc, self.controller._migrate_live, self.req, instance.uuid, body=body) @@ -285,6 +282,54 @@ class MigrateServerTestsV2(MigrateServerTestsV21): _api_version = '2' +class MigrateServerTestsV225(MigrateServerTestsV21): + + # We don't have disk_over_commit in v2.25 + disk_over_commit = None + + def setUp(self): + super(MigrateServerTestsV225, self).setUp() + self.req.api_version_request = api_version_request.APIVersionRequest( + '2.25') + + def _get_params(self, **kwargs): + return {'host': kwargs.get('host'), + 'block_migration': kwargs.get('block_migration') or False} + + def test_migrate_live_enabled_with_string_param(self): + param = {'host': 'hostname', + 'block_migration': "False"} + self._test_migrate_live_succeeded(param) + + def test_migrate_live_without_disk_over_commit(self): + pass + + def test_migrate_live_with_invalid_disk_over_commit(self): + pass + + def test_live_migrate_block_migration_auto(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'), {})} + self._test_actions(['_migrate_live'], body_map=body_map, + method_translations=method_translations, + args_map=args_map) + + def test_migrate_live_with_disk_over_commit_raise(self): + body = {'os-migrateLive': + {'host': 'hostname', + 'block_migration': 'auto', + 'disk_over_commit': False}} + self.assertRaises(self.validation_error, + self.controller._migrate_live, + self.req, fakes.FAKE_UUID, body=body) + + def test_migrate_live_migration_with_old_nova_not_supported(self): + self._test_migrate_live_failed_with_exception( + exception.LiveMigrationWithOldNovaNotSupported()) + + class MigrateServerPolicyEnforcementV21(test.NoDBTestCase): def setUp(self): diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 632cacd116a5..d23c2bc08c75 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1153,6 +1153,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): exc.DestinationHypervisorTooOld(), exc.HypervisorUnavailable(host='dummy'), exc.LiveMigrationWithOldNovaNotSafe(server='dummy'), + exc.LiveMigrationWithOldNovaNotSupported(), exc.MigrationPreCheckError(reason='dummy'), exc.InvalidSharedStorage(path='dummy', reason='dummy'), exc.NoValidHost(reason='dummy'), diff --git a/releasenotes/notes/bp-making-live-migration-api-friendly-3b547f4e0958ee05.yaml b/releasenotes/notes/bp-making-live-migration-api-friendly-3b547f4e0958ee05.yaml new file mode 100644 index 000000000000..42c20ad49c6c --- /dev/null +++ b/releasenotes/notes/bp-making-live-migration-api-friendly-3b547f4e0958ee05.yaml @@ -0,0 +1,9 @@ +--- +feature: + - Make block_migration to support 'auto' value, which means nova will decide + the value of block_migration during live-migration, and remove + disk_over_commit flag for os-migrateLive action in microversion 2.23. +upgrade: + - We can not use microversion 2.25 to do live-migration during upgrade, + nova-api will raise bad request if there is still old compute node in the + cluster.