Merge "For Supermicro BMCs set enable when changing boot device" into stable/train

3 files changed, 64 insertions, 5 deletions

diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py
index ad53a5ac0..0d2d19956 100644
--- a/ironic/drivers/modules/redfish/management.py
+++ b/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
-    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)
 
     use_new_set_system_boot = True
 
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py
index d2734bd67..c167353d4 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_management.py
+++ b/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,
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.