FIX: IPMI bmc_reset() always executed as "warm"

When the invoked via the CLI/API the bmc_reset() "warm" parameter will
receive a string, prior to this patch the string would be seem as True
(since the parameter was expecting a boolen) causing the driver to
always reset the BMC in "warm" mode.

This patch fixes the problem by converting the string passed to boolean
inside the method.

Change-Id: I052ef1ee911703c9b7316362aced17b6303b991d
Closes-Bug: #1573614

ironic/drivers/modules/ipminative.py

@@ -26,6 +26,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_utils import excutils
 from oslo_utils import importutils
+from oslo_utils import strutils
 
 from ironic.common import boot_devices
 from ironic.common import exception
@@ -689,6 +690,7 @@ class VendorPassthru(base.VendorInterface):
 
         """
         driver_info = _parse_driver_info(task.node)
+        warm = strutils.bool_from_string(warm)
         # NOTE(yuriyz): pyghmi 0.8.0 does not have a method for BMC reset
         command = '0x03' if warm else '0x02'
         raw_command = '0x06 ' + command

ironic/drivers/modules/ipmitool.py

@@ -42,6 +42,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_service import loopingcall
 from oslo_utils import excutils
+from oslo_utils import strutils
 import six
 
 from ironic.common import boot_devices
@@ -1038,6 +1039,7 @@ class VendorPassthru(base.VendorInterface):
         """
         node_uuid = task.node.uuid
 
+        warm = strutils.bool_from_string(warm)
         if warm:
             warm_param = 'warm'
         else:

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

@@ -595,8 +595,7 @@ class IPMINativeDriverTestCase(db_base.DbTestCase):
         send_raw_mock.assert_called_once_with(self.info, bytes)
 
     @mock.patch.object(ipminative, '_send_raw', autospec=True)
-    def _test_bmc_reset(self, warm, send_raw_mock):
-        expected_bytes = '0x06 0x03' if warm else '0x06 0x02'
+    def _test_bmc_reset(self, warm, expected_bytes, send_raw_mock):
         with task_manager.acquire(self.context,
                                   self.node.uuid) as task:
             self.driver.vendor.bmc_reset(task, http_method='POST', warm=warm)
@@ -604,7 +603,9 @@ class IPMINativeDriverTestCase(db_base.DbTestCase):
         send_raw_mock.assert_called_once_with(self.info, expected_bytes)
 
     def test_bmc_reset_cold(self):
-        self._test_bmc_reset(False)
+        for param in (False, 'false', 'off', 'n', 'no'):
+            self._test_bmc_reset(param, '0x06 0x02')
 
     def test_bmc_reset_warm(self):
-        self._test_bmc_reset(True)
+        for param in (True, 'true', 'on', 'y', 'yes'):
+            self._test_bmc_reset(param, '0x06 0x03')

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

@@ -1499,21 +1499,23 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
                                         method='bmc_reset',
                                         warm=False)
 
-    @mock.patch.object(ipmi.VendorPassthru, 'bmc_reset', autospec=True)
-    def test_vendor_passthru_call_bmc_reset_warm(self, bmc_mock):
+    @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
+    def _vendor_passthru_call_bmc_reset(self, warm, expected,
+                                        mock_exec):
+        mock_exec.return_value = [None, None]
         with task_manager.acquire(self.context, self.node['uuid'],
                                   shared=False) as task:
-            self.driver.vendor.bmc_reset(task, 'POST', warm=True)
-            bmc_mock.assert_called_once_with(
-                self.driver.vendor, task, 'POST', warm=True)
+            self.driver.vendor.bmc_reset(task, 'POST', warm=warm)
+            mock_exec.assert_called_once_with(
+                mock.ANY, 'bmc reset %s' % expected)
 
-    @mock.patch.object(ipmi.VendorPassthru, 'bmc_reset', autospec=True)
-    def test_vendor_passthru_call_bmc_reset_cold(self, bmc_mock):
-        with task_manager.acquire(self.context, self.node['uuid'],
-                                  shared=False) as task:
-            self.driver.vendor.bmc_reset(task, 'POST', warm=False)
-            bmc_mock.assert_called_once_with(
-                self.driver.vendor, task, 'POST', warm=False)
+    def test_vendor_passthru_call_bmc_reset_warm(self):
+        for param in (True, 'true', 'on', 'y', 'yes'):
+            self._vendor_passthru_call_bmc_reset(param, 'warm')
+
+    def test_vendor_passthru_call_bmc_reset_cold(self):
+        for param in (False, 'false', 'off', 'n', 'no'):
+            self._vendor_passthru_call_bmc_reset(param, 'cold')
 
     def test_vendor_passthru_vendor_routes(self):
         expected = ['send_raw', 'bmc_reset']

releasenotes/notes/bmc_reset-warm-9396ac444cafd734.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - Fix a problem that caused the bmc_reset() vendor passthru method
+    from the IPMI drivers to be always executed as "warm".