Merge "IPA does not boot up after cleaning reboot for 'redfish' bios interface"

ironic/drivers/modules/redfish/bios.py

@@ -22,6 +22,7 @@ from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
 from ironic.drivers import base
+from ironic.drivers.modules import deploy_utils
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic import objects
 
@@ -97,19 +98,39 @@ class RedfishBIOS(base.BIOSInterface):
         :raises: RedfishError on an error from the Sushy library
         """
         system = redfish_utils.get_system(task.node)
-        LOG.debug('Factory reset BIOS settings for node %(node_uuid)s',
-                  {'node_uuid': task.node.uuid})
         try:
-            system.bios.reset_bios()
+            bios = system.bios
+        except sushy.exceptions.MissingAttributeError:
+            error_msg = (_('Redfish BIOS factory reset failed for node '
+                           '%s, because BIOS settings are not supported.') %
+                         task.node.uuid)
+            LOG.error(error_msg)
+            raise exception.RedfishError(error=error_msg)
+
+        node = task.node
+        info = node.driver_internal_info
+        reboot_requested = info.get('post_factory_reset_reboot_requested')
+        if not reboot_requested:
+            LOG.debug('Factory reset BIOS configuration for node %(node)s',
+                      {'node': node.uuid})
+            try:
+                bios.reset_bios()
             except sushy.exceptions.SushyError as e:
                 error_msg = (_('Redfish BIOS factory reset failed for node '
                                '%(node)s. Error: %(error)s') %
-                         {'node': task.node.uuid, 'error': e})
+                             {'node': node.uuid, 'error': e})
                 LOG.error(error_msg)
                 raise exception.RedfishError(error=error_msg)
 
             self.post_reset(task)
             self._set_cleaning_reboot(task)
+            return states.CLEANWAIT
+        else:
+            current_attrs = bios.attributes
+            LOG.debug('Post factory reset, BIOS configuration for node '
+                      '%(node_uuid)s: %(attrs)r',
+                      {'node_uuid': node.uuid, 'attrs': current_attrs})
+            self._clear_reboot_requested(task)
 
     @base.clean_step(priority=0, argsinfo={
         'settings': {
@@ -182,6 +203,8 @@ class RedfishBIOS(base.BIOSInterface):
 
         :param task: a TaskManager instance containing the node to act on.
         """
+        deploy_opts = deploy_utils.build_agent_options(task.node)
+        task.driver.boot.prepare_ramdisk(task, deploy_opts)
         self._reboot(task)
 
     def post_configuration(self, task, settings):
@@ -195,6 +218,8 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         :param settings: a list of BIOS settings to be updated.
         """
+        deploy_opts = deploy_utils.build_agent_options(task.node)
+        task.driver.boot.prepare_ramdisk(task, deploy_opts)
         self._reboot(task)
 
     def get_properties(self):
@@ -252,7 +277,9 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         """
         info = task.node.driver_internal_info
+        info['post_factory_reset_reboot_requested'] = True
         info['cleaning_reboot'] = True
+        info['skip_current_clean_step'] = False
         task.node.driver_internal_info = info
         task.node.save()
 
@@ -276,10 +303,9 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         """
         info = task.node.driver_internal_info
-        if 'post_config_reboot_requested' in info:
+        info.pop('post_config_reboot_requested', None)
-            del info['post_config_reboot_requested']
+        info.pop('post_factory_reset_reboot_requested', None)
-        if 'requested_bios_attrs' in info:
+        info.pop('requested_bios_attrs', None)
-            del info['requested_bios_attrs']
         task.node.driver_internal_info = info
         task.node.save()
 

ironic/tests/unit/drivers/modules/redfish/test_bios.py

@@ -19,6 +19,8 @@ from ironic.common import exception
 from ironic.common import states
 from ironic.conductor import task_manager
 from ironic.conductor import utils as manager_utils
+from ironic.drivers.modules import deploy_utils
+from ironic.drivers.modules import pxe as pxe_boot
 from ironic.drivers.modules.redfish import bios as redfish_bios
 from ironic.drivers.modules.redfish import utils as redfish_utils
 from ironic import objects
@@ -158,9 +160,14 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_setting_list.delete.assert_called_once_with(
                 task.context, task.node.id, delete_names)
 
+    @mock.patch.object(pxe_boot.PXEBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
-    def test_factory_reset(self, mock_power_action, mock_get_system):
+    def test_factory_reset_step1(self, mock_power_action, mock_get_system,
+                                 mock_build_agent_options, mock_prepare):
+        mock_build_agent_options.return_value = {'a': 'b'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.bios.factory_reset(task)
@@ -168,6 +175,26 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_power_action.assert_called_once_with(task, states.REBOOT)
             bios = mock_get_system(task.node).bios
             bios.reset_bios.assert_called_once()
+            mock_build_agent_options.assert_called_once_with(task.node)
+            mock_prepare.assert_called_once_with(mock.ANY, task, {'a': 'b'})
+            info = task.node.driver_internal_info
+            self.assertTrue(
+                all(x in info for x in (
+                    'post_factory_reset_reboot_requested', 'cleaning_reboot',
+                    'skip_current_clean_step')))
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_factory_reset_step2(self, mock_get_system):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            driver_internal_info = task.node.driver_internal_info
+            driver_internal_info['post_factory_reset_reboot_requested'] = True
+            task.node.driver_internal_info = driver_internal_info
+            task.node.save()
+            task.driver.bios.factory_reset(task)
+            mock_get_system.assert_called_with(task.node)
+            info = task.node.driver_internal_info
+            self.assertNotIn('post_factory_reset_reboot_requested', info)
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_factory_reset_fail(self, mock_get_system):
@@ -188,13 +215,19 @@ class RedfishBiosTestCase(db_base.DbTestCase):
                 exception.RedfishError, 'BIOS factory reset failed',
                 task.driver.bios.factory_reset, task)
 
+    @mock.patch.object(pxe_boot.PXEBoot, 'prepare_ramdisk',
+                       spec_set=True, autospec=True)
+    @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True)
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(manager_utils, 'node_power_action', autospec=True)
     def test_apply_configuration_step1(self, mock_power_action,
-                                       mock_get_system):
+                                       mock_get_system,
+                                       mock_build_agent_options,
+                                       mock_prepare):
         settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'},
                     {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
         attributes = {s['name']: s['value'] for s in settings}
+        mock_build_agent_options.return_value = {'a': 'b'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             task.driver.bios.apply_configuration(task, settings)
@@ -202,6 +235,13 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             mock_power_action.assert_called_once_with(task, states.REBOOT)
             bios = mock_get_system(task.node).bios
             bios.set_attributes.assert_called_once_with(attributes)
+            mock_build_agent_options.assert_called_once_with(task.node)
+            mock_prepare.assert_called_once_with(mock.ANY, task, {'a': 'b'})
+            info = task.node.driver_internal_info
+            self.assertTrue(
+                all(x in info for x in (
+                    'post_config_reboot_requested', 'cleaning_reboot',
+                    'skip_current_clean_step')))
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_apply_configuration_step2(self, mock_get_system):
@@ -210,15 +250,16 @@ class RedfishBiosTestCase(db_base.DbTestCase):
         requested_attrs = {'ProcTurboMode': 'Enabled'}
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
-            task.node.driver_internal_info[
+            driver_internal_info = task.node.driver_internal_info
-                'post_config_reboot_requested'] = True
+            driver_internal_info['post_config_reboot_requested'] = True
-            task.node.driver_internal_info[
+            driver_internal_info['requested_bios_attrs'] = requested_attrs
-                'requested_bios_attrs'] = requested_attrs
+            task.node.driver_internal_info = driver_internal_info
-            task.driver.bios._clear_reboot_requested = mock.MagicMock()
+            task.node.save()
             task.driver.bios.apply_configuration(task, settings)
             mock_get_system.assert_called_with(task.node)
-            task.driver.bios._clear_reboot_requested\
+            info = task.node.driver_internal_info
-                .assert_called_once_with(task)
+            self.assertNotIn('post_config_reboot_requested', info)
+            self.assertNotIn('requested_bios_attrs', info)
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_apply_configuration_not_supported(self, mock_get_system):

releasenotes/notes/story-2006217-redfish-bios-cleaning-fails-fee32f04dd97cbd2.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes an issue where clean steps of ``redfish`` BIOS interface do not
+    boot up the IPA ramdisk after cleaning reboot. See `story 2006217
+    <https://storyboard.openstack.org/#!/story/2006217>`__ for details.