Make ipmi_force_boot_device more user friendly

Look for boolean and string like booleans in driver_info['ipmi_force_boot_device']
to make setting the option more user friendly / less error prone.

Change-Id: I2917761055db5286183ce265089c19dea98947ad
Story: 2004444

ironic/drivers/modules/ipmitool.py

@@ -986,7 +986,8 @@ class IPMIManagement(base.ManagementInterface):
                      'to False, so not sending ipmi boot-timeout-disable',
                      {'node_uuid', task.node.uuid})
 
-        if task.node.driver_info.get('ipmi_force_boot_device', False):
+        ifbd = task.node.driver_info.get('ipmi_force_boot_device', False)
+        if strutils.bool_from_string(ifbd):
             driver_utils.force_persistent_boot(task,
                                                device,
                                                persistent)
@@ -1052,8 +1053,9 @@ class IPMIManagement(base.ManagementInterface):
         """
         driver_info = task.node.driver_info
         driver_internal_info = task.node.driver_internal_info
+        ifbd = driver_info.get('ipmi_force_boot_device', False)
 
-        if (driver_info.get('ipmi_force_boot_device', False)
+        if (strutils.bool_from_string(ifbd)
                 and driver_internal_info.get('persistent_boot_device')
                 and driver_internal_info.get('is_next_boot_persistent', True)):
             return {

ironic/drivers/utils.py

@@ -18,6 +18,7 @@ import tempfile
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import base64
+from oslo_utils import strutils
 from oslo_utils import timeutils
 import six
 
@@ -192,8 +193,8 @@ def ensure_next_boot_device(task, driver_info):
     :param task: Node object.
     :param driver_info: Node driver_info.
     """
-
-    if driver_info.get('force_boot_device', False):
+    ifbd = driver_info.get('force_boot_device', False)
+    if strutils.bool_from_string(ifbd):
         driver_internal_info = task.node.driver_internal_info
         if driver_internal_info.get('is_next_boot_persistent') is False:
             driver_internal_info.pop('is_next_boot_persistent', None)

ironic/tests/unit/drivers/modules/test_ipmitool.py

@@ -1922,15 +1922,34 @@ class IPMIToolDriverTestCase(Base):
                       mock.call(self.info, "chassis bootdev pxe")]
         mock_exec.assert_has_calls(mock_calls)
 
+    @mock.patch.object(driver_utils, 'force_persistent_boot', autospec=True)
+    @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
+    def test_management_interface_no_force_set_boot_device(self,
+                                                           mock_exec,
+                                                           mock_force_boot):
+        mock_exec.return_value = [None, None]
+
+        with task_manager.acquire(self.context, self.node.uuid) as task:
+            driver_info = task.node.driver_info
+            driver_info['ipmi_force_boot_device'] = 'False'
+            task.node.driver_info = driver_info
+            self.info['force_boot_device'] = 'False'
+            self.management.set_boot_device(task, boot_devices.PXE)
+
+        mock_calls = [mock.call(self.info, "raw 0x00 0x08 0x03 0x08"),
+                      mock.call(self.info, "chassis bootdev pxe")]
+        mock_exec.assert_has_calls(mock_calls)
+        self.assertFalse(mock_force_boot.called)
+
     @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
     def test_management_interface_force_set_boot_device_ok(self, mock_exec):
         mock_exec.return_value = [None, None]
 
         with task_manager.acquire(self.context, self.node.uuid) as task:
             driver_info = task.node.driver_info
-            driver_info['ipmi_force_boot_device'] = True
+            driver_info['ipmi_force_boot_device'] = 'True'
             task.node.driver_info = driver_info
-            self.info['force_boot_device'] = True
+            self.info['force_boot_device'] = 'True'
             self.management.set_boot_device(task, boot_devices.PXE)
             task.node.refresh()
             self.assertIs(
@@ -1948,9 +1967,9 @@ class IPMIToolDriverTestCase(Base):
 
         with task_manager.acquire(self.context, self.node.uuid) as task:
             driver_info = task.node.driver_info
-            driver_info['ipmi_force_boot_device'] = True
+            driver_info['ipmi_force_boot_device'] = 'True'
             task.node.driver_info = driver_info
-            self.info['force_boot_device'] = True
+            self.info['force_boot_device'] = 'True'
             self.management.set_boot_device(task, boot_devices.PXE, True)
             self.assertEqual(
                 boot_devices.PXE,
@@ -2117,7 +2136,7 @@ class IPMIToolDriverTestCase(Base):
 
     def test_get_force_boot_device_persistent(self):
         with task_manager.acquire(self.context, self.node.uuid) as task:
-            task.node.driver_info['ipmi_force_boot_device'] = True
+            task.node.driver_info['ipmi_force_boot_device'] = 'True'
             task.node.driver_internal_info['persistent_boot_device'] = 'pxe'
             bootdev = self.management.get_boot_device(task)
             self.assertEqual('pxe', bootdev['boot_device'])

releasenotes/notes/story-2004444-f540d9bbc3532ad0.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    For the IPMI hardware type setting the driver option
+    ``node['driver_info']['ipmi_force_boot_device']`` has been made more
+    flexible to avoid situations where
+    ``openstack baremetal node set --driver-info ipmi_force_boot_device=False node-0``
+    leaves the option enabled.