For Supermicro BMCs set enable when changing boot device

When setting the boot settings on Supermicro BMCs, the
BootSourceOverrideEnabled must be set to the desired
value whenever the BootSourceOverrideTarget is set or
it will revert to the default value (Once).  This is
different than what is currently implemented for other
BMCs in which the BootSourceOverrideEnabled is not set
if it matches the current setting.

This change uses the vendor setting to determine if it's
a Supermicro BMC.

Story: 2008547
Task: 41652

Change-Id: I1b1a6baafd4cc4daa2fbdb82f69ded6253b1fcbf
(cherry picked from commit 4a7d50ce56)
(cherry picked from commit 7d74ea0eee)
(cherry picked from commit 2e4e875568)

ironic/drivers/modules/redfish/management.py

@@ -69,12 +69,26 @@ def _set_boot_device(task, system, device, persistent=False):
                        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
+    # The BMC handling of the persistent setting is vendor specific.
-    enabled = (desired_enabled
+    # Some vendors require that it not be set if currently equal to
-               if desired_enabled != current_enabled else None)
+    # desired state (see https://storyboard.openstack.org/#!/story/2007355).
+    # Supermicro BMCs handle it in the opposite manner - the
+    # persistent setting must be set when setting the boot device
+    # (see https://storyboard.openstack.org/#!/story/2008547).
+    vendor = task.node.properties.get('vendor', None)
+    if vendor and vendor.lower() == 'supermicro':
+        enabled = BOOT_DEVICE_PERSISTENT_MAP_REV[persistent]
+        LOG.debug('Setting BootSourceOverrideEnable to %(enable)s '
+                  'on Supermicro BMC, node %(node)s',
+                  {'enable': enabled, 'node': task.node.uuid})
+    else:
+        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)
 
     use_new_set_system_boot = True
 

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

@@ -262,6 +262,36 @@ class RedfishManagementTestCase(db_base.DbTestCase):
                 sushy.BOOT_SOURCE_TARGET_PXE,
                 task.node.driver_internal_info['redfish_boot_device'])
 
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_set_boot_device_persistency_vendor(self, mock_get_system):
+        fake_system = mock_get_system.return_value
+        fake_system.boot.get.return_value = \
+            sushy.BOOT_SOURCE_ENABLED_CONTINUOUS
+
+        values = [
+            ('SuperMicro', sushy.BOOT_SOURCE_ENABLED_CONTINUOUS),
+            ('SomeVendor', None)
+        ]
+
+        for vendor, expected in values:
+            properties = self.node.properties
+            properties['vendor'] = vendor
+            self.node.properties = properties
+            self.node.save()
+            with task_manager.acquire(self.context, self.node.uuid,
+                                      shared=False) as task:
+                task.driver.management.set_boot_device(
+                    task, boot_devices.PXE, persistent=True)
+
+                fake_system.set_system_boot_options.assert_has_calls(
+                    [mock.call(sushy.BOOT_SOURCE_TARGET_PXE,
+                               enabled=expected),
+                     mock.call(mode=sushy.BOOT_SOURCE_MODE_BIOS)])
+
+                # Reset mocks
+                fake_system.set_system_boot_options.reset_mock()
+                mock_get_system.reset_mock()
+
     def test_restore_boot_device(self):
         fake_system = mock.Mock()
         with task_manager.acquire(self.context, self.node.uuid,

releasenotes/notes/supermicro-redfish-override-enabled-aa51686ed33d3061.yaml

@@ -0,0 +1,15 @@
+---
+fixes:
+  - |
+    When Ironic configures the BootSourceOverrideTarget setting via Redfish,
+    on Supermicro BMCs it must always configure BootSourceOverrideEnabled or
+    that will revert to default (Once) on the BMC, see `story 2008547
+    <https://storyboard.openstack.org/#!/story/2008547>`_ for details.
+    This is different than what is currently implemented for other BMCs in
+    which the BootSourceOverrideEnabled is not configured if it matches the
+    current setting (see `story 2007355
+    <https://storyboard.openstack.org/#!/story/2007355>`_).
+
+    This requires that node.properties['vendor'] be 'supermicro' which will
+    be set by Ironic from the Redfish system response or can be set
+    manually.