Merge pull request #166 from stackhpc/bugfix/2024.1/ironic-rebuild

Fix ironic instance rebuild with API >=2.93

Author: jovial

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

@@ -2221,7 +2221,8 @@ def test_detach_interface(self, mock_uv):
                        '_add_instance_info_to_node')
     @mock.patch.object(objects.Instance, 'save')
     def _test_rebuild(self, mock_save, mock_add_instance_info,
-                      mock_looping, mock_wait_active, preserve=False):
+                      mock_looping, mock_wait_active, 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
@@ -2239,7 +2240,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])
@@ -2276,11 +2279,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, '_generate_configdrive')
     @mock.patch.object(configdrive, 'required_by')
     @mock.patch.object(ironic_driver.IronicDriver,

nova/virt/ironic/driver.py

@@ -405,13 +405,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,
                                    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,
@@ -1730,12 +1733,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)