From 97cee86a2fb35dd3e12f152ee6c214dca895dbde Mon Sep 17 00:00:00 2001 From: Ilya Etingof Date: Mon, 2 Mar 2020 13:09:21 +0100 Subject: [PATCH] Add `set_system_boot_options` method Some BMC implementations do not support changing all attributes of the Redfish `Boot` object. To address that, this patch adds a new `set_system_boot_options` method to the `System` object superseding the set_system_boot_source` method. The new method has all boot parameters optional to allow for more atomicity when PATCH'ing Redfish `Boot` object. When sending HTTP PATCH request, this new method will only include those items into the JSON document, that are explicitly passed by the user. This change might improve interoperability with BMCs that do not handle certain attributes of the `Boot` object. Change-Id: I4fa9f718de99def4da162acd4636a17d53ee9a51 Story: 2007355 Task: 38907 --- ...decouple-boot-params-c75e80f5951abb12.yaml | 10 +++ sushy/resources/system/system.py | 83 +++++++++++++------ .../unit/resources/system/test_system.py | 58 +++++++++++++ 3 files changed, 124 insertions(+), 27 deletions(-) create mode 100644 releasenotes/notes/decouple-boot-params-c75e80f5951abb12.yaml diff --git a/releasenotes/notes/decouple-boot-params-c75e80f5951abb12.yaml b/releasenotes/notes/decouple-boot-params-c75e80f5951abb12.yaml new file mode 100644 index 00000000..7a5a5026 --- /dev/null +++ b/releasenotes/notes/decouple-boot-params-c75e80f5951abb12.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + Adds a new ``set_system_boot_options`` method to the ``System`` object + superseding the ``set_system_boot_source`` method. The new method has + all boot parameters optional to allow for more atomicity when PATCH'ing + Redfish ``Boot`` object. The new method will only include those items in + the PATCH document, that are explicitly passed by the user. This change + might improve interoperability with BMCs that do not handle certain + attributes of the ``Boot`` object. diff --git a/sushy/resources/system/system.py b/sushy/resources/system/system.py index 9423fdb0..350cf7bb 100644 --- a/sushy/resources/system/system.py +++ b/sushy/resources/system/system.py @@ -16,6 +16,7 @@ # This is referred from Redfish standard schema. # https://redfish.dmtf.org/schemas/ComputerSystem.v1_5_0.json +import collections import logging from sushy import exceptions @@ -207,41 +208,44 @@ class System(base.ResourceBase): set(sys_maps.BOOT_SOURCE_TARGET_MAP). intersection(self.boot.allowed_values)]) - def set_system_boot_source(self, target, - enabled=sys_cons.BOOT_SOURCE_ENABLED_ONCE, - mode=None): - """Set the boot source. + def set_system_boot_options(self, target=None, enabled=None, mode=None): + """Set boot source and/or boot frequency and/or boot mode. - Set the boot source to use on next reboot of the System. + Set the boot source and/or boot frequency and/or boot mode to use + on next reboot of the System. - :param target: The target boot source. + :param target: The target boot source, optional. :param enabled: The frequency, whether to set it for the next reboot only (BOOT_SOURCE_ENABLED_ONCE) or persistent to all future reboots (BOOT_SOURCE_ENABLED_CONTINUOUS) or disabled - (BOOT_SOURCE_ENABLED_DISABLED). - :param mode: The boot mode, UEFI (BOOT_SOURCE_MODE_UEFI) or - BIOS (BOOT_SOURCE_MODE_BIOS). + (BOOT_SOURCE_ENABLED_DISABLED), optional. + :param mode: The boot mode (UEFI: BOOT_SOURCE_MODE_UEFI or + BIOS: BOOT_SOURCE_MODE_BIOS), optional. :raises: InvalidParameterValueError, if any information passed is invalid. """ - valid_targets = self.get_allowed_system_boot_source_values() - if target not in valid_targets: - raise exceptions.InvalidParameterValueError( - parameter='target', value=target, valid_values=valid_targets) + data = collections.defaultdict(dict) - if enabled not in sys_maps.BOOT_SOURCE_ENABLED_MAP_REV: - raise exceptions.InvalidParameterValueError( - parameter='enabled', value=enabled, - valid_values=list(sys_maps.BOOT_SOURCE_ENABLED_MAP_REV)) + if target is not None: + valid_targets = self.get_allowed_system_boot_source_values() + if target not in valid_targets: + raise exceptions.InvalidParameterValueError( + parameter='target', value=target, + valid_values=valid_targets) - data = { - 'Boot': { - 'BootSourceOverrideTarget': - sys_maps.BOOT_SOURCE_TARGET_MAP_REV[target], - 'BootSourceOverrideEnabled': - sys_maps.BOOT_SOURCE_ENABLED_MAP_REV[enabled] - } - } + fishy_target = sys_maps.BOOT_SOURCE_TARGET_MAP_REV[target] + + data['Boot']['BootSourceOverrideTarget'] = fishy_target + + if enabled is not None: + if enabled not in sys_maps.BOOT_SOURCE_ENABLED_MAP_REV: + raise exceptions.InvalidParameterValueError( + parameter='enabled', value=enabled, + valid_values=list(sys_maps.BOOT_SOURCE_ENABLED_MAP_REV)) + + fishy_freq = sys_maps.BOOT_SOURCE_ENABLED_MAP_REV[enabled] + + data['Boot']['BootSourceOverrideEnabled'] = fishy_freq if mode is not None: if mode not in sys_maps.BOOT_SOURCE_MODE_MAP_REV: @@ -249,13 +253,38 @@ class System(base.ResourceBase): parameter='mode', value=mode, valid_values=list(sys_maps.BOOT_SOURCE_MODE_MAP_REV)) - data['Boot']['BootSourceOverrideMode'] = ( - sys_maps.BOOT_SOURCE_MODE_MAP_REV[mode]) + fishy_mode = sys_maps.BOOT_SOURCE_MODE_MAP_REV[mode] + + data['Boot']['BootSourceOverrideMode'] = fishy_mode # TODO(lucasagomes): Check the return code and response body ? # Probably we should call refresh() as well. self._conn.patch(self.path, data=data) + # TODO(etingof): we should remove this method, eventually + def set_system_boot_source( + self, target, enabled=sys_cons.BOOT_SOURCE_ENABLED_ONCE, + mode=None): + """Set boot source and/or boot frequency and/or boot mode. + + Set the boot source and/or boot frequency and/or boot mode to use + on next reboot of the System. + + This method is obsoleted by `set_system_boot_options`. + + :param target: The target boot source. + :param enabled: The frequency, whether to set it for the next + reboot only (BOOT_SOURCE_ENABLED_ONCE) or persistent to all + future reboots (BOOT_SOURCE_ENABLED_CONTINUOUS) or disabled + (BOOT_SOURCE_ENABLED_DISABLED). + Default is `BOOT_SOURCE_ENABLED_ONCE`. + :param mode: The boot mode (UEFI: BOOT_SOURCE_MODE_UEFI or + BIOS: BOOT_SOURCE_MODE_BIOS), optional. + :raises: InvalidParameterValueError, if any information passed is + invalid. + """ + self.set_system_boot_options(target, enabled, mode) + def set_indicator_led(self, state): """Set IndicatorLED to the given state. diff --git a/sushy/tests/unit/resources/system/test_system.py b/sushy/tests/unit/resources/system/test_system.py index 6d8dffc9..dcfa4871 100644 --- a/sushy/tests/unit/resources/system/test_system.py +++ b/sushy/tests/unit/resources/system/test_system.py @@ -226,6 +226,64 @@ class SystemTestCase(base.TestCase): self.assertIsInstance(values, set) self.assertEqual(1, mock_log.call_count) + def test_set_system_boot_options(self): + self.sys_inst.set_system_boot_options( + sushy.BOOT_SOURCE_TARGET_PXE, + enabled=sushy.BOOT_SOURCE_ENABLED_CONTINUOUS, + mode=sushy.BOOT_SOURCE_MODE_UEFI) + self.sys_inst._conn.patch.assert_called_once_with( + '/redfish/v1/Systems/437XR1138R2', + data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', + 'BootSourceOverrideTarget': 'Pxe', + 'BootSourceOverrideMode': 'UEFI'}}) + + def test_set_system_boot_options_no_mode_specified(self): + self.sys_inst.set_system_boot_options( + sushy.BOOT_SOURCE_TARGET_HDD, + enabled=sushy.BOOT_SOURCE_ENABLED_ONCE) + self.sys_inst._conn.patch.assert_called_once_with( + '/redfish/v1/Systems/437XR1138R2', + data={'Boot': {'BootSourceOverrideEnabled': 'Once', + 'BootSourceOverrideTarget': 'Hdd'}}) + + def test_set_system_boot_options_no_target_specified(self): + self.sys_inst.set_system_boot_options( + enabled=sushy.BOOT_SOURCE_ENABLED_CONTINUOUS, + mode=sushy.BOOT_SOURCE_MODE_UEFI) + self.sys_inst._conn.patch.assert_called_once_with( + '/redfish/v1/Systems/437XR1138R2', + data={'Boot': {'BootSourceOverrideEnabled': 'Continuous', + 'BootSourceOverrideMode': 'UEFI'}}) + + def test_set_system_boot_options_no_freq_specified(self): + self.sys_inst.set_system_boot_options( + target=sushy.BOOT_SOURCE_TARGET_PXE, + mode=sushy.BOOT_SOURCE_MODE_UEFI) + self.sys_inst._conn.patch.assert_called_once_with( + '/redfish/v1/Systems/437XR1138R2', + data={'Boot': {'BootSourceOverrideTarget': 'Pxe', + 'BootSourceOverrideMode': 'UEFI'}}) + + def test_set_system_boot_options_nothing_specified(self): + self.sys_inst.set_system_boot_options() + self.sys_inst._conn.patch.assert_called_once_with( + '/redfish/v1/Systems/437XR1138R2', data={}) + + def test_set_system_boot_options_invalid_target(self): + self.assertRaises(exceptions.InvalidParameterValueError, + self.sys_inst.set_system_boot_source, + 'invalid-target') + + def test_set_system_boot_options_invalid_enabled(self): + with self.assertRaisesRegex( + exceptions.InvalidParameterValueError, + '"enabled" value.*{0}'.format( + list(sys_map.BOOT_SOURCE_ENABLED_MAP_REV))): + + self.sys_inst.set_system_boot_options( + sushy.BOOT_SOURCE_TARGET_HDD, + enabled='invalid-enabled') + def test_set_system_boot_source(self): self.sys_inst.set_system_boot_source( sushy.BOOT_SOURCE_TARGET_PXE,