openstack/ironic
Code Issues Proposed changes

Improve redfish set-boot-mode implementation

Browse Source 
Refrain from touching unrelated boot params when setting boot mode
through Redfish BMC.

This should hopefully improve interoperability with Redfish BMCs.

Change-Id: I461e54012045463b5e97daf26a93eb9dce6f6964
Story: 2007355
Task: 38910
This commit is contained in:
Ilya Etingof 2020-03-02 18:48:37 +01:00 committed by Julia Kreger
parent 305822c37c
commit 5be4c4f5c5
5 changed files with 26 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
driver-requirements.txt
View File

@ -11,7 +11,7 @@ python-dracclient>=3.1.0,<4.0.0
python-xclarityclient>=0.1.6
# The Redfish hardware type uses the Sushy library
sushy>=3.1.0
sushy>=3.2.0
# Ansible-deploy interface
ansible>=2.7

23
ironic/drivers/modules/redfish/management.py
View File

@ -127,7 +127,7 @@ class RedfishManagement(base.ManagementInterface):
system = redfish_utils.get_system(task.node)
try:
system.set_system_boot_source(
system.set_system_boot_options(
BOOT_DEVICE_MAP_REV[device],
enabled=BOOT_DEVICE_PERSISTENT_MAP_REV[persistent])
except sushy.exceptions.SushyError as e:
@ -189,27 +189,8 @@ class RedfishManagement(base.ManagementInterface):
"""
system = redfish_utils.get_system(task.node)
boot_device = system.boot.get('target')
if not boot_device:
error_msg = (_('Cannot change boot mode on node %(node)s '
'because its boot device is not set.') %
{'node': task.node.uuid})
LOG.error(error_msg)
raise exception.RedfishError(error_msg)
boot_override = system.boot.get('enabled')
if not boot_override:
error_msg = (_('Cannot change boot mode on node %(node)s '
'because its boot source override is not set.') %
{'node': task.node.uuid})
LOG.error(error_msg)
raise exception.RedfishError(error_msg)
try:
system.set_system_boot_source(
boot_device,
enabled=boot_override,
mode=BOOT_MODE_MAP_REV[mode])
system.set_system_boot_options(mode=BOOT_MODE_MAP_REV[mode])
except sushy.exceptions.SushyError as e:
error_msg = (_('Setting boot mode to %(mode)s '

24
ironic/tests/unit/drivers/modules/redfish/test_management.py
View File

@ -96,12 +96,12 @@ class RedfishManagementTestCase(db_base.DbTestCase):
task.driver.management.set_boot_device(task, target)
# Asserts
fake_system.set_system_boot_source.assert_called_once_with(
fake_system.set_system_boot_options.assert_called_once_with(
expected, enabled=sushy.BOOT_SOURCE_ENABLED_ONCE)
mock_get_system.assert_called_once_with(task.node)
# Reset mocks
fake_system.set_system_boot_source.reset_mock()
fake_system.set_system_boot_options.reset_mock()
mock_get_system.reset_mock()
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
@ -119,19 +119,19 @@ class RedfishManagementTestCase(db_base.DbTestCase):
task.driver.management.set_boot_device(
task, boot_devices.PXE, persistent=target)
fake_system.set_system_boot_source.assert_called_once_with(
fake_system.set_system_boot_options.assert_called_once_with(
sushy.BOOT_SOURCE_TARGET_PXE, enabled=expected)
mock_get_system.assert_called_once_with(task.node)
# Reset mocks
fake_system.set_system_boot_source.reset_mock()
fake_system.set_system_boot_options.reset_mock()
mock_get_system.reset_mock()
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
def test_set_boot_device_fail(self, mock_get_system, mock_sushy):
fake_system = mock.Mock()
fake_system.set_system_boot_source.side_effect = (
fake_system.set_system_boot_options.side_effect = (
sushy.exceptions.SushyError()
)
mock_get_system.return_value = fake_system
@ -140,7 +140,7 @@ class RedfishManagementTestCase(db_base.DbTestCase):
self.assertRaisesRegex(
exception.RedfishError, 'Redfish set boot device',
task.driver.management.set_boot_device, task, boot_devices.PXE)
fake_system.set_system_boot_source.assert_called_once_with(
fake_system.set_system_boot_options.assert_called_once_with(
sushy.BOOT_SOURCE_TARGET_PXE,
enabled=sushy.BOOT_SOURCE_ENABLED_ONCE)
mock_get_system.assert_called_once_with(task.node)
@ -183,19 +183,19 @@ class RedfishManagementTestCase(db_base.DbTestCase):
task.driver.management.set_boot_mode(task, mode=mode)
# Asserts
fake_system.set_system_boot_source.assert_called_once_with(
mock.ANY, enabled=mock.ANY, mode=mode)
fake_system.set_system_boot_options.assert_called_once_with(
mode=mode)
mock_get_system.assert_called_once_with(task.node)
# Reset mocks
fake_system.set_system_boot_source.reset_mock()
fake_system.set_system_boot_options.reset_mock()
mock_get_system.reset_mock()
@mock.patch.object(sushy, 'Sushy', autospec=True)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
def test_set_boot_mode_fail(self, mock_get_system, mock_sushy):
fake_system = mock.Mock()
fake_system.set_system_boot_source.side_effect = (
fake_system.set_system_boot_options.side_effect = (
sushy.exceptions.SushyError)
mock_get_system.return_value = fake_system
with task_manager.acquire(self.context, self.node.uuid,
@ -203,8 +203,8 @@ class RedfishManagementTestCase(db_base.DbTestCase):
self.assertRaisesRegex(
exception.RedfishError, 'Setting boot mode',
task.driver.management.set_boot_mode, task, boot_modes.UEFI)
fake_system.set_system_boot_source.assert_called_once_with(
mock.ANY, enabled=mock.ANY, mode=boot_modes.UEFI)
fake_system.set_system_boot_options.assert_called_once_with(
mode=boot_modes.UEFI)
mock_get_system.assert_called_once_with(task.node)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)

10
releasenotes/notes/decouple-boot-params-2b05806435ad21e5.yaml Normal file
View File

@ -0,0 +1,10 @@
---
fixes:
- |
Improves interoperability with Redfish BMCs by untying node boot
mode change from other boot parameters change (such as boot device,
boot frequency).
upgrade:
- |
The required minimum version of the ``sushy`` python Redfish API client
library is now version ``3.2.0``.

2
zuul.d/ironic-jobs.yaml
View File

@ -43,8 +43,8 @@
IRONIC_TEMPEST_WHOLE_DISK_IMAGE: False
IRONIC_VM_COUNT: 1
IRONIC_VM_EPHEMERAL_DISK: 1
IRONIC_VM_LOG_DIR: '{{ devstack_base_dir }}/ironic-bm-logs'
IRONIC_VM_SPECS_RAM: 384
IRONIC_VM_LOG_DIR: '{{ devstack_base_dir }}/ironic-bm-logs'
# NOTE(dtantsur): in some jobs we end up with 12 disks total, so reduce
# each of them. For don't need all 10 GiB for CirrOS anyway.
IRONIC_VM_SPECS_DISK: 4