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

@@ -4247,7 +4247,8 @@ def resume(self, context, instance):
     @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 @@ def rescue(self, context, instance, rescue_password=None,
             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)

nova/tests/unit/compute/test_compute_api.py

@@ -20,6 +20,7 @@
 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 @@ def test_unrescue(self):
             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})

nova/virt/driver.py

@@ -125,9 +125,10 @@ def block_device_info_get_mapping(block_device_info):
     "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,

nova/virt/powervm/driver.py

@@ -68,6 +68,7 @@ def __init__(self, virtapi):
         # 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,