Fix ironic instance rebuild with API >=2.93

When you attempt to rebuild an Ironic instance, using the CLI,
it will pick the latest micversion, which triggers sending
to the ironic driver:
reimage_boot_volume = true

The ironic driver then raises an exception, saying it is unable
to rebuild a volume backed instance. This happens even when
the instance is not volume backed, and rebuild would be fine.

This patch ensures we only raise the "can't rebuild"
exception when the instance is volume backed.

Closes-Bug: #2127017

Change-Id: Ibb8e83a1d36506df34a324ca0da9e2f129a38cfa
Signed-off-by: John Garbutt <john.garbutt@stackhpc.com>
Assisted-By: Github Copilot Free
(cherry picked from commit 2c8f0e0b3a9c42260e86b894c4e5344a16b187bc)

Author: JohnGarbutt

nova/tests/unit/virt/ironic/test_driver.py

@@ -2276,7 +2276,8 @@ def test_detach_interface(self, mock_uv):
     @mock.patch.object(objects.Instance, 'save')
     def _test_rebuild(self, mock_save, mock_add_instance_info,
                       mock_looping, mock_wait_active, mock_metadata,
-                      preserve=False):
+                      preserve=False, reimage_boot_volume=False,
+                      block_device_info=None):
         node_uuid = uuidutils.generate_uuid()
         node = _get_cached_node(id=node_uuid, instance_id=self.instance_id)
         self.mock_conn.get_node.return_value = node
@@ -2300,7 +2301,9 @@ def _test_rebuild(self, mock_save, mock_add_instance_info,
             context=self.ctx, instance=instance, image_meta=image_meta,
             injected_files=None, admin_password=None, allocations={},
             bdms=None, detach_block_devices=None, attach_block_devices=None,
-            preserve_ephemeral=preserve)
+            preserve_ephemeral=preserve,
+            reimage_boot_volume=reimage_boot_volume,
+            block_device_info=block_device_info)
 
         mock_save.assert_called_once_with(
             expected_task_state=[task_states.REBUILDING])
@@ -2339,11 +2342,25 @@ def test_rebuild_no_preserve_ephemeral(self, mock_required_by,
     def test_rebuild_with_configdrive(self, mock_required_by,
                                       mock_configdrive):
         mock_required_by.return_value = True
-        self._test_rebuild()
+        self._test_rebuild(reimage_boot_volume=True)
         # assert configdrive was generated
         mock_configdrive.assert_called_once_with(
             self.ctx, mock.ANY, mock.ANY, mock.ANY, extra_md={}, files=None)
 
+    @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive')
+    @mock.patch.object(configdrive, 'required_by')
+    def test_rebuild_bfv_fails(self, mock_required_by,
+                                      mock_configdrive):
+        mock_required_by.return_value = False
+        block_device_info = self._create_fake_block_device_info()
+        e = self.assertRaises(exception.NovaException,
+           self._test_rebuild,
+           reimage_boot_volume=True,
+           block_device_info=block_device_info)
+        self.assertEqual(
+            "Ironic doesn't support rebuilding volume backed instances.",
+            str(e))
+
     @mock.patch.object(ironic_driver.IronicDriver,
                        'get_instance_driver_metadata')
     @mock.patch.object(ironic_driver.IronicDriver, '_generate_configdrive')

nova/virt/ironic/driver.py

@@ -408,13 +408,16 @@ def failed_spawn_cleanup(self, instance):
             return
         self._cleanup_deploy(node, instance)
 
+    def _is_boot_from_volume(self, block_device_info):
+        root_bdm = block_device.get_root_bdm(
+            virt_driver.block_device_info_get_mapping(block_device_info))
+        return root_bdm is not None
+
     def _add_instance_info_to_node(self, node, instance, image_meta, flavor,
                                    metadata, preserve_ephemeral=None,
                                    block_device_info=None):
 
-        root_bdm = block_device.get_root_bdm(
-            virt_driver.block_device_info_get_mapping(block_device_info))
-        boot_from_volume = root_bdm is not None
+        boot_from_volume = self._is_boot_from_volume(block_device_info)
         patch = patcher.create(node).get_deploy_patch(instance,
                                                       image_meta,
                                                       flavor,
@@ -1739,12 +1742,15 @@ def rebuild(self, context, instance, image_meta, injected_files,
         :param preserve_ephemeral: Boolean value; if True the ephemeral
             must be preserved on rebuild.
         :param accel_uuids: Accelerator UUIDs. Ignored by this driver.
-        :param reimage_boot_volume: Re-image the volume backed instance.
+        :param reimage_boot_volume: Ironic driver raises when rebuild
+            of a boot from volume instance, as its not yet supported.
         """
         if reimage_boot_volume:
-            raise exception.NovaException(
-                _("Ironic doesn't support rebuilding volume backed "
-                  "instances."))
+            boot_from_volume = self._is_boot_from_volume(block_device_info)
+            if boot_from_volume:
+                raise exception.NovaException(
+                    _("Ironic doesn't support rebuilding volume backed "
+                      "instances."))
 
         LOG.debug('Rebuild called for instance', instance=instance)