From 0915d26cdf3938dde2bc89892c756b3fa84987e5 Mon Sep 17 00:00:00 2001 From: Richard Pioso Date: Thu, 28 May 2020 21:54:42 -0400 Subject: [PATCH] Fix Redfish handle no continuous override boot src Some hardware Redfish services have dropped support for continuously overriding the boot source when the boot mode is Unified Extensible Firmware Interface (UEFI). See this discussion: http://lists.openstack.org/pipermail/openstack-discuss/2020-April/014543.html A workaround falls back to one-time override, restoring it on every reboot or power on. [1] However, it does not work, because, per the Distributed Management Task Force (DMTF) Redfish standard, one-time override is disabled on the next boot cycle. [2] This fixes that by explicitly setting the boot source override to one-time every time it is restored. [1] https://storyboard.openstack.org/#!/story/2007527 [2] https://redfish.dmtf.org/schemas/v1/ComputerSystem.v1_11_0.json Story: 2007733 Task: 39897 Change-Id: I6f5a11a77e7b16cdd0d837fdec51c3e9aeea9a31 (cherry picked from commit c56777929adf9c380f20777f5b3bfc37370b859e) --- ironic/drivers/modules/redfish/management.py | 26 +++++++++++-------- .../modules/redfish/test_management.py | 6 +++-- ...h-sadness-workaround-ed02cb310ff369f4.yaml | 11 ++++++++ 3 files changed, 30 insertions(+), 13 deletions(-) create mode 100644 releasenotes/notes/fix-redfish-sadness-workaround-ed02cb310ff369f4.yaml diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index dff790ce56..f415b6d20f 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -69,19 +69,28 @@ if sushy: v: k for k, v in INDICATOR_MAP.items()} -def _set_boot_device(task, system, device, enabled=None): +def _set_boot_device(task, system, device, persistent=False): """An internal routine to set the boot device. :param task: a task from TaskManager. :param system: a Redfish System object. :param device: the Redfish boot device. - :param enabled: Redfish boot device persistence value or None. + :param persistent: Boolean value. True if the boot device will + persist to all future boots, False if not. + Default: False. :raises: SushyError on an error from the Sushy library """ + desired_enabled = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent] + current_enabled = system.boot.get('enabled') + + # NOTE(etingof): this can be racy, esp if BMC is not RESTful + enabled = (desired_enabled + if desired_enabled != current_enabled else None) + try: system.set_system_boot_options(device, enabled=enabled) except sushy.exceptions.SushyError as e: - if enabled == sushy.BOOT_SOURCE_ENABLED_CONTINUOUS: + if desired_enabled == sushy.BOOT_SOURCE_ENABLED_CONTINUOUS: # NOTE(dtantsur): continuous boot device settings have been # removed from Redfish, and some vendors stopped supporting # it before an alternative was provided. As a work around, @@ -194,15 +203,10 @@ class RedfishManagement(base.ManagementInterface): system = redfish_utils.get_system(task.node) - desired_persistence = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent] - current_persistence = system.boot.get('enabled') - - # NOTE(etingof): this can be racy, esp if BMC is not RESTful - enabled = (desired_persistence - if desired_persistence != current_persistence else None) try: - _set_boot_device(task, system, BOOT_DEVICE_MAP_REV[device], - enabled=enabled) + _set_boot_device( + task, system, BOOT_DEVICE_MAP_REV[device], + persistent=persistent) except sushy.exceptions.SushyError as e: error_msg = (_('Redfish set boot device failed for node ' '%(node)s. Error: %(error)s') % diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 694b445e53..f7070a34c9 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -214,7 +214,8 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.driver.management.restore_boot_device(task, fake_system) fake_system.set_system_boot_options.assert_called_once_with( - sushy.BOOT_SOURCE_TARGET_HDD, enabled=None) + sushy.BOOT_SOURCE_TARGET_HDD, + enabled=sushy.BOOT_SOURCE_ENABLED_ONCE) # The stored boot device is kept intact self.assertEqual( sushy.BOOT_SOURCE_TARGET_HDD, @@ -243,7 +244,8 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.driver.management.restore_boot_device(task, fake_system) fake_system.set_system_boot_options.assert_called_once_with( - sushy.BOOT_SOURCE_TARGET_HDD, enabled=None) + sushy.BOOT_SOURCE_TARGET_HDD, + enabled=sushy.BOOT_SOURCE_ENABLED_ONCE) self.assertTrue(mock_log.called) # The stored boot device is kept intact self.assertEqual( diff --git a/releasenotes/notes/fix-redfish-sadness-workaround-ed02cb310ff369f4.yaml b/releasenotes/notes/fix-redfish-sadness-workaround-ed02cb310ff369f4.yaml new file mode 100644 index 0000000000..989a7a646f --- /dev/null +++ b/releasenotes/notes/fix-redfish-sadness-workaround-ed02cb310ff369f4.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixes a workaround for hardware that does not support persistent + boot device setting with the ``redfish`` or ``idrac-redfish`` + management interface implementation. When such situation is + detected, ironic falls back to one-time boot device setting, + restoring it on every reboot or power on. + + For more information, see `story 2007733 + `_.