From 8c2e76598995f0d417653c60a63ea342baf4e880 Mon Sep 17 00:00:00 2001 From: Sahid Orentino Ferdjaoui Date: Mon, 19 Sep 2022 11:35:45 +0200 Subject: [PATCH] compute: enhance compute evacuate instance to support target state Related to the bp/allowing-target-state-for-evacuate. This change is extending compute API to accept a new argument targetState. The targetState argument when set will force state of an evacuated instance to the destination host. Signed-off-by: Sahid Orentino Ferdjaoui Change-Id: I9660d42937ad62d647afc6be965f166cc5631392 --- nova/compute/api.py | 8 ++-- nova/compute/manager.py | 25 +++++++--- nova/compute/rpcapi.py | 18 +++++-- nova/compute/vm_states.py | 3 ++ nova/conductor/api.py | 6 ++- nova/conductor/manager.py | 8 ++-- nova/conductor/rpcapi.py | 15 ++++-- nova/exception.py | 10 ++++ nova/objects/service.py | 5 +- .../api_sample_tests/test_evacuate.py | 14 +++--- nova/tests/unit/compute/test_api.py | 24 ++++++---- nova/tests/unit/compute/test_compute.py | 32 ++++++++----- nova/tests/unit/compute/test_compute_mgr.py | 17 ++++--- nova/tests/unit/compute/test_rpcapi.py | 48 +++++++++++++++++-- nova/tests/unit/conductor/test_conductor.py | 33 +++++++++++-- 15 files changed, 201 insertions(+), 65 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 1669ca7baad5..6b2023c19f32 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3797,7 +3797,8 @@ class API: orig_sys_metadata=orig_sys_metadata, bdms=bdms, preserve_ephemeral=preserve_ephemeral, host=host, request_spec=request_spec, - reimage_boot_volume=reimage_boot_volume) + reimage_boot_volume=reimage_boot_volume, + target_state=None) def _check_volume_status(self, context, bdms): """Check whether the status of the volume is "in-use". @@ -5617,7 +5618,7 @@ class API: @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, vm_states.ERROR], task_state=None) def evacuate(self, context, instance, host, on_shared_storage, - admin_password=None, force=None): + admin_password=None, force=None, target_state=None): """Running evacuate to target host. Checking vm compute host state, if the host not in expected_state, @@ -5628,6 +5629,7 @@ class API: :param on_shared_storage: True if instance files on shared storage :param admin_password: password to set on rebuilt instance :param force: Force the evacuation to the specific host target + :param target_state: Set a target state for the evacuated instance """ LOG.debug('vm evacuation scheduled', instance=instance) @@ -5691,7 +5693,7 @@ class API: on_shared_storage=on_shared_storage, host=host, request_spec=request_spec, - ) + target_state=target_state) def get_migrations(self, context, filters): """Get all migrations for the given filters.""" diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9a76e51b7abf..8fc4e75059f3 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -618,7 +618,7 @@ class ComputeVirtAPI(virtapi.VirtAPI): class ComputeManager(manager.Manager): """Manages the running instances from creation to destruction.""" - target = messaging.Target(version='6.1') + target = messaging.Target(version='6.2') def __init__(self, compute_driver=None, *args, **kwargs): """Load configuration options and connect to the hypervisor.""" @@ -3674,7 +3674,7 @@ class ComputeManager(manager.Manager): bdms, recreate, on_shared_storage, preserve_ephemeral, migration, scheduled_node, limits, request_spec, accel_uuids, - reimage_boot_volume): + reimage_boot_volume, target_state): """Destroy and re-make this instance. A 'rebuild' effectively purges all existing data from the system and @@ -3709,6 +3709,7 @@ class ComputeManager(manager.Manager): :param reimage_boot_volume: Boolean to specify whether the user has explicitly requested to rebuild a boot volume + :param target_state: Set a target state for the evacuated instance. """ # recreate=True means the instance is being evacuated from a failed @@ -3773,7 +3774,8 @@ class ComputeManager(manager.Manager): image_meta, injected_files, new_pass, orig_sys_metadata, bdms, evacuate, on_shared_storage, preserve_ephemeral, migration, request_spec, allocs, rebuild_claim, - scheduled_node, limits, accel_uuids, reimage_boot_volume) + scheduled_node, limits, accel_uuids, reimage_boot_volume, + target_state) except (exception.ComputeResourcesUnavailable, exception.RescheduledException) as e: if isinstance(e, exception.ComputeResourcesUnavailable): @@ -3833,7 +3835,7 @@ class ComputeManager(manager.Manager): injected_files, new_pass, orig_sys_metadata, bdms, evacuate, on_shared_storage, preserve_ephemeral, migration, request_spec, allocations, rebuild_claim, scheduled_node, limits, accel_uuids, - reimage_boot_volume): + reimage_boot_volume, target_state): """Helper to avoid deep nesting in the top-level method.""" provider_mapping = None @@ -3857,7 +3859,8 @@ class ComputeManager(manager.Manager): context, instance, orig_image_ref, image_meta, injected_files, new_pass, orig_sys_metadata, bdms, evacuate, on_shared_storage, preserve_ephemeral, migration, request_spec, allocations, - provider_mapping, accel_uuids, reimage_boot_volume) + provider_mapping, accel_uuids, reimage_boot_volume, + target_state) @staticmethod def _get_image_name(image_meta): @@ -3871,10 +3874,18 @@ class ComputeManager(manager.Manager): injected_files, new_pass, orig_sys_metadata, bdms, evacuate, on_shared_storage, preserve_ephemeral, migration, request_spec, allocations, request_group_resource_providers_mapping, - accel_uuids, reimage_boot_volume): + accel_uuids, reimage_boot_volume, target_state): orig_vm_state = instance.vm_state if evacuate: + if target_state and orig_vm_state != vm_states.ERROR: + # This will ensure that at destination the instance will have + # the desired state. + if target_state not in vm_states.ALLOW_TARGET_STATES: + raise exception.InstanceEvacuateNotSupportedTargetState( + target_state=target_state) + orig_vm_state = target_state + if request_spec: # NOTE(gibi): Do a late check of server group policy as # parallel scheduling could violate such policy. This will @@ -11347,7 +11358,7 @@ class _ComputeV5Proxy(object): bdms, recreate, on_shared_storage, preserve_ephemeral, migration, scheduled_node, limits, request_spec, - accel_uuids, False) + accel_uuids, False, None) # 5.13 support for optional accel_uuids argument def shelve_instance(self, context, instance, image_id, diff --git a/nova/compute/rpcapi.py b/nova/compute/rpcapi.py index 3ac4c6dcfae8..4a97a908075f 100644 --- a/nova/compute/rpcapi.py +++ b/nova/compute/rpcapi.py @@ -403,6 +403,7 @@ class ComputeAPI(object): * ... - Rename the instance_type argument of resize_instance() to flavor * 6.1 - Add reimage_boot_volume parameter to rebuild_instance() + * 6.2 - Add target_state parameter to rebuild_instance() ''' VERSION_ALIASES = { @@ -424,6 +425,7 @@ class ComputeAPI(object): 'xena': '6.0', 'yoga': '6.0', 'zed': '6.1', + 'antilope': '6.2', } @property @@ -1083,7 +1085,7 @@ class ComputeAPI(object): image_ref, orig_image_ref, orig_sys_metadata, bdms, recreate, on_shared_storage, host, node, preserve_ephemeral, migration, limits, request_spec, accel_uuids, - reimage_boot_volume): + reimage_boot_volume, target_state): # NOTE(edleafe): compute nodes can only use the dict form of limits. if isinstance(limits, objects.SchedulerLimits): @@ -1096,11 +1098,19 @@ class ComputeAPI(object): 'limits': limits, 'request_spec': request_spec, 'accel_uuids': accel_uuids, - 'reimage_boot_volume': reimage_boot_volume + 'reimage_boot_volume': reimage_boot_volume, + 'target_state': target_state, } - - version = '6.1' + version = '6.2' client = self.router.client(ctxt) + if not client.can_send_version(version): + if msg_args['target_state']: + raise exception.UnsupportedRPCVersion( + api="rebuild_instance", + required="6.2") + else: + del msg_args['target_state'] + version = '6.1' if not client.can_send_version(version): if msg_args['reimage_boot_volume']: raise exception.NovaException( diff --git a/nova/compute/vm_states.py b/nova/compute/vm_states.py index 633894c1ea43..1a916ea59a6d 100644 --- a/nova/compute/vm_states.py +++ b/nova/compute/vm_states.py @@ -76,3 +76,6 @@ ALLOW_TRIGGER_CRASH_DUMP = [ACTIVE, PAUSED, RESCUED, RESIZED, ERROR] # states we allow resources to be freed in ALLOW_RESOURCE_REMOVAL = [DELETED, SHELVED_OFFLOADED] + +# states we allow for evacuate instance +ALLOW_TARGET_STATES = [STOPPED] diff --git a/nova/conductor/api.py b/nova/conductor/api.py index 778fdd6c731a..843c8ce3a306 100644 --- a/nova/conductor/api.py +++ b/nova/conductor/api.py @@ -144,7 +144,8 @@ class ComputeTaskAPI(object): injected_files, new_pass, orig_sys_metadata, bdms, recreate=False, on_shared_storage=False, preserve_ephemeral=False, host=None, - request_spec=None, reimage_boot_volume=False): + request_spec=None, reimage_boot_volume=False, + target_state=None): self.conductor_compute_rpcapi.rebuild_instance(context, instance=instance, new_pass=new_pass, @@ -158,7 +159,8 @@ class ComputeTaskAPI(object): preserve_ephemeral=preserve_ephemeral, host=host, request_spec=request_spec, - reimage_boot_volume=reimage_boot_volume) + reimage_boot_volume=reimage_boot_volume, + target_state=target_state) def cache_images(self, context, aggregate, image_ids): """Request images be pre-cached on hosts within an aggregate. diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index d37de5c9bb23..817751933127 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -235,7 +235,7 @@ class ComputeTaskManager: may involve coordinating activities on multiple compute nodes. """ - target = messaging.Target(namespace='compute_task', version='1.24') + target = messaging.Target(namespace='compute_task', version='1.25') def __init__(self): self.compute_rpcapi = compute_rpcapi.ComputeAPI() @@ -1152,7 +1152,8 @@ class ComputeTaskManager: injected_files, new_pass, orig_sys_metadata, bdms, recreate, on_shared_storage, preserve_ephemeral=False, host=None, - request_spec=None, reimage_boot_volume=False): + request_spec=None, reimage_boot_volume=False, + target_state=None): # recreate=True means the instance is being evacuated from a failed # host to a new destination host. The 'recreate' variable name is # confusing, so rename it to evacuate here at the top, which is simpler @@ -1356,7 +1357,8 @@ class ComputeTaskManager: limits=limits, request_spec=request_spec, accel_uuids=accel_uuids, - reimage_boot_volume=reimage_boot_volume) + reimage_boot_volume=reimage_boot_volume, + target_state=target_state) def _validate_image_traits_for_rebuild(self, context, instance, image_ref): """Validates that the traits specified in the image can be satisfied diff --git a/nova/conductor/rpcapi.py b/nova/conductor/rpcapi.py index ffaecd2c9528..a5f0cf0094cd 100644 --- a/nova/conductor/rpcapi.py +++ b/nova/conductor/rpcapi.py @@ -287,6 +287,7 @@ class ComputeTaskAPI(object): 1.22 - Added confirm_snapshot_based_resize() 1.23 - Added revert_snapshot_based_resize() 1.24 - Add reimage_boot_volume parameter to rebuild_instance() + 1.25 - Add target_state parameter to rebuild_instance() """ def __init__(self): @@ -428,8 +429,8 @@ class ComputeTaskAPI(object): image_ref, orig_image_ref, orig_sys_metadata, bdms, recreate=False, on_shared_storage=False, host=None, preserve_ephemeral=False, request_spec=None, - reimage_boot_volume=False): - version = '1.24' + reimage_boot_volume=False, target_state=None): + version = '1.25' kw = {'instance': instance, 'new_pass': new_pass, 'injected_files': injected_files, @@ -442,8 +443,16 @@ class ComputeTaskAPI(object): 'preserve_ephemeral': preserve_ephemeral, 'host': host, 'request_spec': request_spec, - 'reimage_boot_volume': reimage_boot_volume + 'reimage_boot_volume': reimage_boot_volume, + 'target_state': target_state, } + if not self.client.can_send_version(version): + if kw['target_state']: + raise exception.UnsupportedRPCVersion( + api="rebuild_instance", required="1.25") + else: + del kw['target_state'] + version = '1.24' if not self.client.can_send_version(version): if kw['reimage_boot_volume']: raise exception.NovaException( diff --git a/nova/exception.py b/nova/exception.py index f89dc107fac2..2c4bd77d3467 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -1451,6 +1451,11 @@ class InstanceEvacuateNotSupported(Invalid): msg_fmt = _('Instance evacuate is not supported.') +class InstanceEvacuateNotSupportedTargetState(Invalid): + msg_fmt = _("Target state '%(target_state)s' for instance evacuate " + "is not supported.") + + class DBNotAllowed(NovaException): msg_fmt = _('%(binary)s attempted direct database access which is ' 'not allowed by policy') @@ -1479,6 +1484,11 @@ class UnsupportedRescueImage(Invalid): msg_fmt = _("Requested rescue image '%(image)s' is not supported") +class UnsupportedRPCVersion(Invalid): + msg_fmt = _("Unsupported RPC version for %(api)s. " + "Required >= %(required)s") + + class Base64Exception(NovaException): msg_fmt = _("Invalid Base 64 data for file %(path)s") diff --git a/nova/objects/service.py b/nova/objects/service.py index 6ae3f6a7686c..0ed443ef1703 100644 --- a/nova/objects/service.py +++ b/nova/objects/service.py @@ -31,7 +31,7 @@ LOG = logging.getLogger(__name__) # NOTE(danms): This is the global service version counter -SERVICE_VERSION = 65 +SERVICE_VERSION = 66 # NOTE(danms): This is our SERVICE_VERSION history. The idea is that any @@ -228,6 +228,9 @@ SERVICE_VERSION_HISTORY = ( # Version 65: Compute RPC v6.1: # Added stable local node identity {'compute_rpc': '6.1'}, + # Version 66: Compute RPC v6.2: + # Add target_state parameter to rebuild_instance() + {'compute_rpc': '6.2'}, ) # This is the version after which we can rely on having a persistent diff --git a/nova/tests/functional/api_sample_tests/test_evacuate.py b/nova/tests/functional/api_sample_tests/test_evacuate.py index ab3aa95ad838..a92d942715f2 100644 --- a/nova/tests/functional/api_sample_tests/test_evacuate.py +++ b/nova/tests/functional/api_sample_tests/test_evacuate.py @@ -80,7 +80,7 @@ class EvacuateJsonTest(test_servers.ServersSampleBase): orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY, on_shared_storage=False, preserve_ephemeral=mock.ANY, host='testHost', request_spec=mock.ANY, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) @mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance') def test_server_evacuate_find_host(self, rebuild_mock): @@ -97,7 +97,7 @@ class EvacuateJsonTest(test_servers.ServersSampleBase): orig_sys_metadata=mock.ANY, bdms=mock.ANY, recreate=mock.ANY, on_shared_storage=False, preserve_ephemeral=mock.ANY, host=None, request_spec=mock.ANY, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) class EvacuateJsonTestV214(EvacuateJsonTest): @@ -119,7 +119,7 @@ class EvacuateJsonTestV214(EvacuateJsonTest): 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, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) @mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance') def test_server_evacuate_find_host(self, rebuild_mock): @@ -135,7 +135,7 @@ class EvacuateJsonTestV214(EvacuateJsonTest): 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, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) class EvacuateJsonTestV229(EvacuateJsonTestV214): @@ -163,7 +163,7 @@ class EvacuateJsonTestV229(EvacuateJsonTestV214): 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, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) @mock.patch('nova.conductor.manager.ComputeTaskManager.rebuild_instance') @mock.patch('nova.objects.ComputeNodeList.get_all_by_host') @@ -184,7 +184,7 @@ 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, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) class EvacuateJsonTestV268(EvacuateJsonTestV229): @@ -211,7 +211,7 @@ class EvacuateJsonTestV268(EvacuateJsonTestV229): 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, - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) def test_server_evacuate_with_force(self): # doesn't apply to v2.68+, which removed the ability to force migrate diff --git a/nova/tests/unit/compute/test_api.py b/nova/tests/unit/compute/test_api.py index 41d8bcd05eb4..9d6e9ba4bd0e 100644 --- a/nova/tests/unit/compute/test_api.py +++ b/nova/tests/unit/compute/test_api.py @@ -4081,7 +4081,8 @@ class _ComputeAPIUnitTestMixIn(object): injected_files=[], bdms=bdms, preserve_ephemeral=False, host=None, request_spec=fake_spec, - reimage_boot_volume=True) + reimage_boot_volume=True, + target_state=None) _check_auto_disk_config.assert_called_once_with( image=image, auto_disk_config=None) _checks_for_create_and_rebuild.assert_called_once_with( @@ -4096,7 +4097,8 @@ class _ComputeAPIUnitTestMixIn(object): instance, uuids.image_ref, admin_pass, - reimage_boot_volume=False) + reimage_boot_volume=False, + target_state=None) @mock.patch.object(objects.RequestSpec, 'save') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @@ -4155,7 +4157,8 @@ class _ComputeAPIUnitTestMixIn(object): instance, uuids.image_ref, admin_pass, - reimage_boot_volume=False) + reimage_boot_volume=False, + target_state=None) @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') @mock.patch.object(objects.Instance, 'save') @@ -4205,7 +4208,8 @@ class _ComputeAPIUnitTestMixIn(object): orig_image_ref=uuids.image_ref, orig_sys_metadata=orig_system_metadata, bdms=bdms, preserve_ephemeral=False, host=instance.host, - request_spec=fake_spec, reimage_boot_volume=False) + request_spec=fake_spec, reimage_boot_volume=False, + target_state=None) _check_auto_disk_config.assert_called_once_with( image=image, auto_disk_config=None) @@ -4278,7 +4282,8 @@ class _ComputeAPIUnitTestMixIn(object): orig_image_ref=uuids.image_ref, orig_sys_metadata=orig_system_metadata, bdms=bdms, preserve_ephemeral=False, host=None, - request_spec=fake_spec, reimage_boot_volume=False) + request_spec=fake_spec, reimage_boot_volume=False, + target_state=None) # assert the request spec was modified so the scheduler picks # the existing instance host/node req_spec_save.assert_called_once_with() @@ -4346,7 +4351,8 @@ class _ComputeAPIUnitTestMixIn(object): orig_image_ref=uuids.image_ref, orig_sys_metadata=orig_system_metadata, bdms=bdms, preserve_ephemeral=False, host=instance.host, - request_spec=fake_spec, reimage_boot_volume=False) + request_spec=fake_spec, reimage_boot_volume=False, + target_state=None) _check_auto_disk_config.assert_called_once_with( image=image, auto_disk_config=None) @@ -4405,7 +4411,8 @@ class _ComputeAPIUnitTestMixIn(object): orig_image_ref=uuids.image_ref, orig_sys_metadata=orig_system_metadata, bdms=bdms, preserve_ephemeral=False, host=instance.host, - request_spec=fake_spec, reimage_boot_volume=False) + request_spec=fake_spec, reimage_boot_volume=False, + target_state=None) _check_auto_disk_config.assert_called_once_with( image=image, auto_disk_config=None) @@ -4469,7 +4476,8 @@ class _ComputeAPIUnitTestMixIn(object): orig_image_ref=uuids.image_ref, orig_sys_metadata=orig_system_metadata, bdms=bdms, preserve_ephemeral=False, host=instance.host, - request_spec=fake_spec, reimage_boot_volume=False) + request_spec=fake_spec, reimage_boot_volume=False, + target_state=None) _check_auto_disk_config.assert_called_once_with( image=image, auto_disk_config=None) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 8eeb5f8f5828..49cf15ec17df 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -2731,7 +2731,7 @@ class ComputeTestCase(BaseTestCase, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, request_spec=None, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) self.compute.terminate_instance(self.context, instance, []) def test_rebuild_driver(self): @@ -2762,7 +2762,7 @@ class ComputeTestCase(BaseTestCase, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, request_spec=None, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) self.assertTrue(called['rebuild']) self.compute.terminate_instance(self.context, instance, []) @@ -2815,7 +2815,7 @@ class ComputeTestCase(BaseTestCase, bdms=bdms, recreate=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, on_shared_storage=False, request_spec=None, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) self.assertTrue(called['rebuild']) self.compute.terminate_instance(self.context, instance, []) @@ -2834,7 +2834,8 @@ class ComputeTestCase(BaseTestCase, new_pass="new_password", orig_sys_metadata=sys_metadata, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits=None, - request_spec=None, accel_uuids=[], reimage_boot_volume=False) + request_spec=None, accel_uuids=[], reimage_boot_volume=False, + target_state=None) self.compute.terminate_instance(self.context, instance, []) def test_rebuild_launched_at_time(self): @@ -2855,7 +2856,7 @@ class ComputeTestCase(BaseTestCase, new_pass="new_password", orig_sys_metadata={}, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, request_spec=None, - accel_uuids=[], reimage_boot_volume=False) + accel_uuids=[], reimage_boot_volume=False, target_state=None) instance.refresh() self.assertEqual(cur_time, instance['launched_at'].replace(tzinfo=None)) @@ -2889,7 +2890,7 @@ class ComputeTestCase(BaseTestCase, orig_sys_metadata=sys_metadata, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, request_spec=None, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) self.compute.terminate_instance(self.context, instance, []) @mock.patch.object(objects.BlockDeviceMappingList, 'get_by_instance_uuid') @@ -4617,7 +4618,8 @@ class ComputeTestCase(BaseTestCase, 'request_spec': None, 'on_shared_storage': False, 'accel_uuids': (), - 'reimage_boot_volume': False}), + 'reimage_boot_volume': False, + 'target_state': None}), ("set_admin_password", task_states.UPDATING_PASSWORD, {'new_pass': None}), ("rescue_instance", task_states.RESCUING, @@ -5136,7 +5138,7 @@ class ComputeTestCase(BaseTestCase, orig_sys_metadata=orig_sys_metadata, bdms=[], recreate=False, on_shared_storage=False, preserve_ephemeral=False, migration=None, scheduled_node=None, limits={}, request_spec=None, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) inst_ref.refresh() @@ -11961,7 +11963,7 @@ class ComputeAPITestCase(BaseTestCase): force=False) @mock.patch('nova.compute.utils.notify_about_instance_action') - def _test_evacuate(self, mock_notify, force=None): + def _test_evacuate(self, mock_notify, force=None, target_state=None): instance = self._create_fake_instance_obj(services=True) self.assertIsNone(instance.task_state) @@ -11998,7 +12000,8 @@ class ComputeAPITestCase(BaseTestCase): host='fake_dest_host', on_shared_storage=True, admin_password=None, - force=force) + force=force, + target_state=target_state) if force is False: host = None else: @@ -12015,7 +12018,8 @@ class ComputeAPITestCase(BaseTestCase): recreate=True, on_shared_storage=True, request_spec=fake_spec, - host=host) + host=host, + target_state=target_state) do_test() instance.refresh() @@ -12047,6 +12051,9 @@ class ComputeAPITestCase(BaseTestCase): def test_evacuate_with_forced_host(self): self._test_evacuate(force=True) + def test_evacuate_with_target_state(self): + self._test_evacuate(target_state="stopped") + @mock.patch('nova.servicegroup.api.API.service_is_up', return_value=False) def test_fail_evacuate_with_non_existing_destination(self, _service_is_up): @@ -13540,7 +13547,8 @@ class EvacuateHostTestCase(BaseTestCase): image_ref, injected_files, 'newpass', {}, bdms, recreate=True, on_shared_storage=on_shared_storage, migration=migration, preserve_ephemeral=False, scheduled_node=node, limits=limits, - request_spec=None, accel_uuids=[], reimage_boot_volume=False) + request_spec=None, accel_uuids=[], reimage_boot_volume=False, + target_state=None) if vm_states_is_stopped: mock_notify_rebuild.assert_has_calls([ mock.call(ctxt, self.inst, self.inst.host, phase='start', diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index fec13de7f06b..2eb7d227efa2 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -5342,7 +5342,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.rebuild_instance( self.context, instance, None, None, None, None, None, None, - recreate, False, False, None, scheduled_node, {}, None, [], False) + recreate, False, False, None, scheduled_node, {}, None, [], False, + None) mock_set.assert_called_once_with(None, 'failed') mock_notify_about_instance_usage.assert_called_once_with( mock.ANY, instance, 'rebuild.error', fault=mock_rebuild.side_effect @@ -5454,7 +5455,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, preserve_ephemeral=False, migration=None, scheduled_node='fake-node', limits={}, request_spec=request_spec, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, + target_state=None) mock_validate_policy.assert_called_once_with( elevated_context, instance, {'group': [uuids.group]}) @@ -5494,7 +5496,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, recreate=True, on_shared_storage=None, preserve_ephemeral=False, migration=None, scheduled_node='fake-node', limits={}, request_spec=request_spec, accel_uuids=[], - reimage_boot_volume=False) + reimage_boot_volume=False, target_state=None) mock_validate_policy.assert_called_once_with( elevated_context, instance, {'group': [uuids.group]}) @@ -5520,7 +5522,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.rebuild_instance( self.context, instance, None, None, None, None, None, None, False, - False, False, migration, None, {}, None, [], False) + False, False, migration, None, {}, None, [], False, + None) self.assertFalse(mock_get.called) self.assertEqual(node, instance.node) self.assertEqual('done', migration.status) @@ -5542,7 +5545,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, self.compute.rebuild_instance( self.context, instance, None, None, None, None, None, None, True, False, False, mock.sentinel.migration, None, {}, - None, [], False) + None, [], False, None) mock_get.assert_called_once_with(mock.ANY, self.compute.host) mock_rt.finish_evacuation.assert_called_once_with( instance, 'new-node', mock.sentinel.migration) @@ -5625,7 +5628,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, preserve_ephemeral, {}, {}, self.allocations, mock.sentinel.mapping, [], - False) + False, None) mock_notify_usage.assert_has_calls( [mock.call(self.context, instance, "rebuild.start", @@ -5884,7 +5887,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, request_spec=objects.RequestSpec(), allocations=self.allocations, request_group_resource_providers_mapping=mock.sentinel.mapping, - accel_uuids=[], reimage_boot_volume=False) + accel_uuids=[], reimage_boot_volume=False, target_state=None) self.assertIn('Trusted image certificates provided on host', str(ex)) def test_reverts_task_state_instance_not_found(self): diff --git a/nova/tests/unit/compute/test_rpcapi.py b/nova/tests/unit/compute/test_rpcapi.py index 55d0fc53e855..6f78678a9203 100644 --- a/nova/tests/unit/compute/test_rpcapi.py +++ b/nova/tests/unit/compute/test_rpcapi.py @@ -836,7 +836,8 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): orig_sys_metadata=None, recreate=True, on_shared_storage=True, preserve_ephemeral=True, migration=None, node=None, limits=None, request_spec=None, accel_uuids=[], - reimage_boot_volume=False, version='6.1') + reimage_boot_volume=False, target_state=None, + version='6.2') def test_rebuild_instance_old_rpcapi(self): # With rpcapi < 5.12, accel_uuids must be dropped in the client call. @@ -868,9 +869,11 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): ctxt, instance=self.fake_instance_obj, accel_uuids=['938af7f9-f136-4e5a-bdbe-3b6feab54311'], node=None, host=None, reimage_boot_volume=False, - **rebuild_args) + target_state=None, **rebuild_args) - mock_client.can_send_version.assert_has_calls([mock.call('6.0'), + mock_client.can_send_version.assert_has_calls([mock.call('6.2'), + mock.call('6.1'), + mock.call('6.0'), mock.call('5.12')]) mock_client.prepare.assert_called_with( server=self.fake_instance_obj.host, version='5.0') @@ -890,7 +893,7 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): compute_api.router.client.return_value = mock_client # Force can_send_version to [False, True, True], so that 6.0 # version is used. - mock_client.can_send_version.side_effect = [False, True, True] + mock_client.can_send_version.side_effect = [False, False, True, True] mock_cctx = mock.MagicMock() mock_client.prepare.return_value = mock_cctx rebuild_args = { @@ -908,12 +911,47 @@ class ComputeRpcAPITestCase(test.NoDBTestCase): 'limits': None, 'accel_uuids': [], 'reimage_boot_volume': True, + 'target_state': None, } self.assertRaises( exception.NovaException, compute_api.rebuild_instance, ctxt, instance=self.fake_instance_obj, node=None, host=None, **rebuild_args) - mock_client.can_send_version.assert_has_calls([mock.call('6.1')]) + mock_client.can_send_version.assert_has_calls([mock.call('6.2')]) + + def test_rebuild_instance_evacuate_old_rpcapi(self): + # With rpcapi < 6.2, if evacuate we should raise error. + ctxt = context.RequestContext('fake_user', 'fake_project') + compute_api = compute_rpcapi.ComputeAPI() + compute_api.router.client = mock.Mock() + mock_client = mock.MagicMock() + compute_api.router.client.return_value = mock_client + # Force can_send_version to return False. + mock_client.can_send_version.return_value = False + mock_cctx = mock.MagicMock() + mock_client.prepare.return_value = mock_cctx + rebuild_args = { + 'new_pass': 'admin_password', + 'injected_files': 'files_to_inject', + 'image_ref': uuids.image_ref, + 'orig_image_ref': uuids.orig_image_ref, + 'orig_sys_metadata': 'orig_sys_meta', + 'bdms': {}, + 'recreate': False, + 'on_shared_storage': False, + 'preserve_ephemeral': False, + 'request_spec': None, + 'migration': None, + 'limits': None, + 'accel_uuids': [], + 'reimage_boot_volume': True, + 'target_state': 'stopped', + } + self.assertRaises( + exception.UnsupportedRPCVersion, + compute_api.rebuild_instance, + ctxt, instance=self.fake_instance_obj, + node=None, host=None, **rebuild_args) def test_reserve_block_device_name(self): self.flags(long_rpc_timeout=1234) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index e942217a6cd7..971570dfb583 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -389,7 +389,8 @@ class _BaseTaskTestCase(object): 'preserve_ephemeral': False, 'host': 'compute-host', 'request_spec': None, - 'reimage_boot_volume': False} + 'reimage_boot_volume': False, + 'target_state': None} if update_args: rebuild_args.update(update_args) compute_rebuild_args = copy.deepcopy(rebuild_args) @@ -4751,9 +4752,34 @@ class ConductorTaskRPCAPITestCase(_BaseTaskTestCase, mock.sentinel.migration) can_send_version.assert_called_once_with('1.23') + def test_evacuate_old_rpc_with_target_state(self): + inst_obj = self._create_fake_instance_obj() + rebuild_args, compute_args = self._prepare_rebuild_args( + {'host': inst_obj.host, + 'target_state': 'stopped'}) + with mock.patch.object( + self.conductor.client, 'can_send_version', return_value=False): + self.assertRaises(exc.UnsupportedRPCVersion, + self.conductor.rebuild_instance, + self.context, inst_obj, **rebuild_args) + + def test_evacuate_old_rpc_without_target_state(self): + inst_obj = self._create_fake_instance_obj() + rebuild_args, compute_args = self._prepare_rebuild_args( + {'host': inst_obj.host, + 'target_state': None}) + with mock.patch.object( + self.conductor.client, 'can_send_version', + return_value=False) as can_send_version: + self.conductor.rebuild_instance( + self.context, inst_obj, **rebuild_args) + can_send_version.assert_has_calls([ + mock.call('1.25'), mock.call('1.24'), + mock.call('1.12')]) + def test_rebuild_instance_volume_backed(self): inst_obj = self._create_fake_instance_obj() - version = '1.24' + version = '1.25' cctxt_mock = mock.MagicMock() rebuild_args, compute_args = self._prepare_rebuild_args( {'host': inst_obj.host}) @@ -4785,7 +4811,8 @@ class ConductorTaskRPCAPITestCase(_BaseTaskTestCase, self.conductor.rebuild_instance, self.context, inst_obj, **rebuild_args) - can_send_version.assert_called_once_with('1.24') + can_send_version.assert_has_calls([mock.call('1.25'), + mock.call('1.24')]) class ConductorTaskAPITestCase(_BaseTaskTestCase, test_compute.BaseTestCase):