From 72fd48b9cb9593dc864de05c4f8ff48829024689 Mon Sep 17 00:00:00 2001
From: Bob Fournier <bfournie@redhat.com>
Date: Wed, 31 Mar 2021 14:58:26 -0400
Subject: [PATCH] Restrict syncing of boot mode to Supermicro

The fix for https://storyboard.openstack.org/#!/story/2008252 synced
the boot mode after changing the boot device, because Supermicro nodes
reset the boot mode if not included in the boot device set. However this
can cause a problem on Dell nodes when changing the mode uefi->bios or
bios->uefi. Restrict the syncing of the boot mode to Supermicro.

Story: 2008712
Task: 42046
Change-Id: I9f305cb3f33766c1c93cf4347368b1ce025fc635
(cherry picked from commit 8bd25a9882e64ae378974e974c39b874d4b9b0e8)
---
 ironic/drivers/modules/redfish/management.py  |  7 +++++-
 .../modules/redfish/test_management.py        | 23 ++++++++++---------
 ...device-to-supermicro-218e8cb57735c685.yaml | 11 +++++++++
 3 files changed, 29 insertions(+), 12 deletions(-)
 create mode 100644 releasenotes/notes/restrict-sync-mode-after-device-to-supermicro-218e8cb57735c685.yaml

diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py
index 5539c7948b..085d2fe84a 100644
--- a/ironic/drivers/modules/redfish/management.py
+++ b/ironic/drivers/modules/redfish/management.py
@@ -231,7 +231,12 @@ class RedfishManagement(base.ManagementInterface):
 
         # Ensure that boot mode is synced with what is set.
         # Some BMCs reset it to default (BIOS) when changing the boot device.
-        boot_mode_utils.sync_boot_mode(task)
+        # It should only be synced on these vendors as other vendor
+        # implementations will result in an error
+        # (see https://storyboard.openstack.org/#!/story/2008712)
+        vendor = task.node.properties.get('vendor', None)
+        if vendor and vendor.lower() == 'supermicro':
+            boot_mode_utils.sync_boot_mode(task)
 
     def get_boot_device(self, task):
         """Get the current boot device for a node.
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py
index 495d2320a9..9f69a7c7ee 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_management.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py
@@ -98,8 +98,7 @@ class RedfishManagementTestCase(db_base.DbTestCase):
                 # Asserts
                 fake_system.set_system_boot_options.assert_has_calls(
                     [mock.call(expected,
-                               enabled=sushy.BOOT_SOURCE_ENABLED_ONCE),
-                     mock.call(mode=sushy.BOOT_SOURCE_MODE_BIOS)])
+                               enabled=sushy.BOOT_SOURCE_ENABLED_ONCE)])
                 mock_get_system.assert_called_with(task.node)
                 self.assertNotIn('redfish_boot_device',
                                  task.node.driver_internal_info)
@@ -125,8 +124,7 @@ class RedfishManagementTestCase(db_base.DbTestCase):
 
                 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)])
+                               enabled=expected)])
                 mock_get_system.assert_called_with(task.node)
                 self.assertNotIn('redfish_boot_device',
                                  task.node.driver_internal_info)
@@ -153,8 +151,7 @@ class RedfishManagementTestCase(db_base.DbTestCase):
                     task, boot_devices.PXE, persistent=target)
 
                 fake_system.set_system_boot_options.assert_has_calls(
-                    [mock.call(sushy.BOOT_SOURCE_TARGET_PXE, enabled=None),
-                     mock.call(mode=sushy.BOOT_SOURCE_MODE_BIOS)])
+                    [mock.call(sushy.BOOT_SOURCE_TARGET_PXE, enabled=None)])
                 mock_get_system.assert_called_with(task.node)
 
                 # Reset mocks
@@ -260,11 +257,15 @@ class RedfishManagementTestCase(db_base.DbTestCase):
                                       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)])
+                if vendor == 'SuperMicro':
+                    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)])
+                else:
+                    fake_system.set_system_boot_options.assert_has_calls(
+                        [mock.call(sushy.BOOT_SOURCE_TARGET_PXE,
+                                   enabled=expected)])
 
                 # Reset mocks
                 fake_system.set_system_boot_options.reset_mock()
diff --git a/releasenotes/notes/restrict-sync-mode-after-device-to-supermicro-218e8cb57735c685.yaml b/releasenotes/notes/restrict-sync-mode-after-device-to-supermicro-218e8cb57735c685.yaml
new file mode 100644
index 0000000000..0436b81e6b
--- /dev/null
+++ b/releasenotes/notes/restrict-sync-mode-after-device-to-supermicro-218e8cb57735c685.yaml
@@ -0,0 +1,11 @@
+---
+fixes:
+  - |
+    The fix for story
+    `2008252 <https://storyboard.openstack.org/#!/story/2008252>`_ synced
+    the boot mode after changing the boot device because Supermicro nodes
+    reset the boot mode if not included in the boot device set. However this
+    can cause a problem on Dell nodes when changing the mode uefi->bios or
+    bios->uefi, see `story 2008712
+    <https://storyboard.openstack.org/#!/story/2008712>`_ for details.
+    Restrict the syncing of the boot mode to Supermicro.