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

3 files changed, 64 insertions, 5 deletions

diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py
index 9d49a21eb..276239433 100644
--- a/ironic/drivers/modules/redfish/management.py
+++ b/ironic/drivers/modules/redfish/management.py
@@ -89,12 +89,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
-    enabled = (desired_enabled
-               if desired_enabled != current_enabled else None)
+    # The BMC handling of the persistent setting is vendor specific.
+    # Some vendors require that it not be set if currently equal to
+    # 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)
 
     try:
         system.set_system_boot_options(device, enabled=enabled)
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py
index dc67e17d6..98f3b75f7 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_management.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py
@@ -245,6 +245,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,
diff --git a/releasenotes/notes/supermicro-redfish-override-enabled-aa51686ed33d3061.yaml b/releasenotes/notes/supermicro-redfish-override-enabled-aa51686ed33d3061.yaml
new file mode 100644
index 000000000..f3af2a169
--- /dev/null
+++ b/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.