From 63475ca723aae93d4419a0d1632bc6dba6ca8de0 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Fri, 22 Apr 2016 15:08:25 +0100 Subject: [PATCH] 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 | 2 ++ ironic/drivers/modules/ipmitool.py | 2 ++ .../unit/drivers/modules/test_ipminative.py | 9 ++++--- .../unit/drivers/modules/test_ipmitool.py | 26 ++++++++++--------- .../bmc_reset-warm-9396ac444cafd734.yaml | 4 +++ 5 files changed, 27 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/bmc_reset-warm-9396ac444cafd734.yaml diff --git a/ironic/drivers/modules/ipminative.py b/ironic/drivers/modules/ipminative.py index d461d3ed71..920d973693 100644 --- a/ironic/drivers/modules/ipminative.py +++ b/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 diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 2fb9f84adb..cef19d25b9 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/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: diff --git a/ironic/tests/unit/drivers/modules/test_ipminative.py b/ironic/tests/unit/drivers/modules/test_ipminative.py index b89e528a5b..7c815b2d1b 100644 --- a/ironic/tests/unit/drivers/modules/test_ipminative.py +++ b/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') diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 68f822378d..d9af799817 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/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'] diff --git a/releasenotes/notes/bmc_reset-warm-9396ac444cafd734.yaml b/releasenotes/notes/bmc_reset-warm-9396ac444cafd734.yaml new file mode 100644 index 0000000000..03449d6235 --- /dev/null +++ b/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".