From 5b6f44efff5ad721779fd9a7e80d6a5330fad7d4 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Mon, 6 Jan 2020 19:23:39 +0000 Subject: [PATCH] compute: Report COMPUTE_RESCUE_BFV and check during rescue This change introduces and checks the COMPUTE_RESCUE_BFV trait that was introduced in os-traits 2.2.0 in the compute layer during an instance rescue when the instance boots from a volume. An additional kwarg ``allow_bfv_rescue`` flag is also added to the signature of the rescue method within the compute API. This defaults to False and will be used in a following change to indicate when the request is using a high enough microversion to invoke this new capability. The ``supports_bfv_rescue`` capability tracked within the virt drivers that this trait maps to is only added to the powervm driver for now due to the way in which these capabilities are checked by the ``TestPowerVMDriver.test_driver_capabilities`` test. Implements: blueprint virt-bfv-instance-rescue Change-Id: Ic2ad1468d31b7707b7f8f2b845a9cf47d9d076d5 --- nova/compute/api.py | 18 ++- nova/tests/unit/compute/test_compute_api.py | 157 ++++++++++++++++++++ nova/virt/driver.py | 4 +- nova/virt/powervm/driver.py | 1 + 4 files changed, 177 insertions(+), 3 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5c543887d11d..1064a77b69e8 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4247,7 +4247,8 @@ class API(base.Base): @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED, vm_states.ERROR]) def rescue(self, context, instance, rescue_password=None, - rescue_image_ref=None, clean_shutdown=True): + rescue_image_ref=None, clean_shutdown=True, + allow_bfv_rescue=False): """Rescue the given instance.""" bdms = objects.BlockDeviceMappingList.get_by_instance_uuid( @@ -4256,7 +4257,20 @@ class API(base.Base): if bdm.volume_id: vol = self.volume_api.get(context, bdm.volume_id) self.volume_api.check_attached(context, vol) - if compute_utils.is_volume_backed_instance(context, instance, bdms): + + volume_backed = compute_utils.is_volume_backed_instance( + context, instance, bdms) + + if volume_backed and allow_bfv_rescue: + cn = objects.ComputeNode.get_by_host_and_nodename( + context, instance.host, instance.node) + traits = self.placementclient.get_provider_traits( + context, cn.uuid).traits + if os_traits.COMPUTE_RESCUE_BFV not in traits: + reason = _("Host unable to rescue a volume-backed instance") + raise exception.InstanceNotRescuable(instance_id=instance.uuid, + reason=reason) + elif volume_backed: reason = _("Cannot rescue a volume-backed instance") raise exception.InstanceNotRescuable(instance_id=instance.uuid, reason=reason) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 021295feb57e..c0e99d4d3577 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -20,6 +20,7 @@ import ddt import fixtures import iso8601 import mock +import os_traits as ot from oslo_messaging import exceptions as oslo_exceptions from oslo_serialization import jsonutils from oslo_utils import fixture as utils_fixture @@ -5237,6 +5238,162 @@ class _ComputeAPIUnitTestMixIn(object): rpcapi_unrescue_instance.assert_called_once_with( self.context, instance=instance) + @mock.patch('nova.objects.compute_node.ComputeNode' + '.get_by_host_and_nodename') + @mock.patch('nova.compute.utils.is_volume_backed_instance', + return_value=True) + @mock.patch('nova.objects.block_device.BlockDeviceMappingList' + '.get_by_instance_uuid') + def test_rescue_bfv_with_required_trait(self, mock_get_bdms, + mock_is_volume_backed, + mock_get_cn): + instance = self._create_instance_obj() + bdms = objects.BlockDeviceMappingList(objects=[ + objects.BlockDeviceMapping( + boot_index=0, image_id=uuids.image_id, source_type='image', + destination_type='volume', volume_type=None, + snapshot_id=None, volume_id=uuids.volume_id, + volume_size=None)]) + with test.nested( + mock.patch.object(self.compute_api.placementclient, + 'get_provider_traits'), + mock.patch.object(self.compute_api.volume_api, 'get'), + mock.patch.object(self.compute_api.volume_api, 'check_attached'), + mock.patch.object(instance, 'save'), + mock.patch.object(self.compute_api, '_record_action_start'), + mock.patch.object(self.compute_api.compute_rpcapi, + 'rescue_instance') + ) as ( + mock_get_traits, mock_get_volume, mock_check_attached, + mock_instance_save, mock_record_start, mock_rpcapi_rescue + ): + # Mock out the returned compute node, bdms and volume + mock_get_cn.return_value = mock.Mock(uuid=uuids.cn) + mock_get_bdms.return_value = bdms + mock_get_volume.return_value = mock.sentinel.volume + + # Ensure the required trait is returned, allowing BFV rescue + mock_trait_info = mock.Mock(traits=[ot.COMPUTE_RESCUE_BFV]) + mock_get_traits.return_value = mock_trait_info + + # Try to rescue the instance + self.compute_api.rescue(self.context, instance, + rescue_image_ref=uuids.rescue_image_id, + allow_bfv_rescue=True) + + # Assert all of the calls made in the compute API + mock_get_bdms.assert_called_once_with(self.context, instance.uuid) + mock_get_volume.assert_called_once_with( + self.context, uuids.volume_id) + mock_check_attached.assert_called_once_with( + self.context, mock.sentinel.volume) + mock_is_volume_backed.assert_called_once_with( + self.context, instance, bdms) + mock_get_cn.assert_called_once_with( + self.context, instance.host, instance.node) + mock_get_traits.assert_called_once_with(self.context, uuids.cn) + mock_instance_save.assert_called_once_with( + expected_task_state=[None]) + mock_record_start.assert_called_once_with( + self.context, instance, instance_actions.RESCUE) + mock_rpcapi_rescue.assert_called_once_with( + self.context, instance=instance, rescue_password=None, + rescue_image_ref=uuids.rescue_image_id, clean_shutdown=True) + + # Assert that the instance task state as set in the compute API + self.assertEqual(task_states.RESCUING, instance.task_state) + + @mock.patch('nova.objects.compute_node.ComputeNode' + '.get_by_host_and_nodename') + @mock.patch('nova.compute.utils.is_volume_backed_instance', + return_value=True) + @mock.patch('nova.objects.block_device.BlockDeviceMappingList' + '.get_by_instance_uuid') + def test_rescue_bfv_without_required_trait(self, mock_get_bdms, + mock_is_volume_backed, + mock_get_cn): + instance = self._create_instance_obj() + bdms = objects.BlockDeviceMappingList(objects=[ + objects.BlockDeviceMapping( + boot_index=0, image_id=uuids.image_id, source_type='image', + destination_type='volume', volume_type=None, + snapshot_id=None, volume_id=uuids.volume_id, + volume_size=None)]) + with test.nested( + mock.patch.object(self.compute_api.placementclient, + 'get_provider_traits'), + mock.patch.object(self.compute_api.volume_api, 'get'), + mock.patch.object(self.compute_api.volume_api, 'check_attached'), + ) as ( + mock_get_traits, mock_get_volume, mock_check_attached + ): + # Mock out the returned compute node, bdms and volume + mock_get_bdms.return_value = bdms + mock_get_volume.return_value = mock.sentinel.volume + mock_get_cn.return_value = mock.Mock(uuid=uuids.cn) + + # Ensure the required trait is not returned, denying BFV rescue + mock_trait_info = mock.Mock(traits=[]) + mock_get_traits.return_value = mock_trait_info + + # Assert that any attempt to rescue a bfv instance on a compute + # node that does not report the COMPUTE_RESCUE_BFV trait fails and + # raises InstanceNotRescuable + self.assertRaises(exception.InstanceNotRescuable, + self.compute_api.rescue, self.context, instance, + rescue_image_ref=None, allow_bfv_rescue=True) + + # Assert the calls made in the compute API prior to the failure + mock_get_bdms.assert_called_once_with(self.context, instance.uuid) + mock_get_volume.assert_called_once_with( + self.context, uuids.volume_id) + mock_check_attached.assert_called_once_with( + self.context, mock.sentinel.volume) + mock_is_volume_backed.assert_called_once_with( + self.context, instance, bdms) + mock_get_cn.assert_called_once_with( + self.context, instance.host, instance.node) + mock_get_traits.assert_called_once_with( + self.context, uuids.cn) + + @mock.patch('nova.compute.utils.is_volume_backed_instance', + return_value=True) + @mock.patch('nova.objects.block_device.BlockDeviceMappingList' + '.get_by_instance_uuid') + def test_rescue_bfv_without_allow_flag(self, mock_get_bdms, + mock_is_volume_backed): + instance = self._create_instance_obj() + bdms = objects.BlockDeviceMappingList(objects=[ + objects.BlockDeviceMapping( + boot_index=0, image_id=uuids.image_id, source_type='image', + destination_type='volume', volume_type=None, + snapshot_id=None, volume_id=uuids.volume_id, + volume_size=None)]) + with test.nested( + mock.patch.object(self.compute_api.volume_api, 'get'), + mock.patch.object(self.compute_api.volume_api, 'check_attached'), + ) as ( + mock_get_volume, mock_check_attached + ): + # Mock out the returned bdms and volume + mock_get_bdms.return_value = bdms + mock_get_volume.return_value = mock.sentinel.volume + + # Assert that any attempt to rescue a bfv instance with + # allow_bfv_rescue=False fails and raises InstanceNotRescuable + self.assertRaises(exception.InstanceNotRescuable, + self.compute_api.rescue, self.context, instance, + rescue_image_ref=None, allow_bfv_rescue=False) + + # Assert the calls made in the compute API prior to the failure + mock_get_bdms.assert_called_once_with(self.context, instance.uuid) + mock_get_volume.assert_called_once_with( + self.context, uuids.volume_id) + mock_check_attached.assert_called_once_with( + self.context, mock.sentinel.volume) + mock_is_volume_backed.assert_called_once_with( + self.context, instance, bdms) + def test_set_admin_password_invalid_state(self): # Tests that InstanceInvalidState is raised when not ACTIVE. instance = self._create_instance_obj({'vm_state': vm_states.STOPPED}) diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 484e42da1d5f..5940de21d8e1 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -125,9 +125,10 @@ CAPABILITY_TRAITS_MAP = { "supports_image_type_vmdk": os_traits.COMPUTE_IMAGE_TYPE_VMDK, # Added in os-traits 2.0.0 "supports_image_type_ploop": os_traits.COMPUTE_IMAGE_TYPE_PLOOP, - # Added in os-traits 2.1.0. "supports_migrate_to_same_host": os_traits.COMPUTE_SAME_HOST_COLD_MIGRATE, + # Added in os-traits 2.2.0. + "supports_bfv_rescue": os_traits.COMPUTE_RESCUE_BFV, } @@ -178,6 +179,7 @@ class ComputeDriver(object): "supports_trusted_certs": False, "supports_pcpus": False, "supports_accelerators": False, + "supports_bfv_rescue": False, # Image type support flags "supports_image_type_aki": False, diff --git a/nova/virt/powervm/driver.py b/nova/virt/powervm/driver.py index 4c565ba96061..251d8bfe4e7b 100644 --- a/nova/virt/powervm/driver.py +++ b/nova/virt/powervm/driver.py @@ -68,6 +68,7 @@ class PowerVMDriver(driver.ComputeDriver): # capabilities on the instance rather than on the class. self.capabilities = { 'has_imagecache': False, + 'supports_bfv_rescue': False, 'supports_evacuate': False, 'supports_migrate_to_same_host': False, 'supports_attach_interface': True,