From 8df85facec2008c6b579e0b198415f7065cc0d83 Mon Sep 17 00:00:00 2001 From: Naohiro Tamura Date: Mon, 17 Oct 2016 15:25:13 +0900 Subject: Add a missing error check in ipmitool driver's reboot This patch adds a missing error check into ipmitool power driver's reboot so that the reboot can fail properly if power off failed. Change-Id: Icd061fe51555be3200b154c5e43e0f082864c93f Closes-bug: #1633992 (cherry picked from commit c073fb714df0477a625e2e4eee89edfee4a03576) --- ironic/drivers/modules/ipmitool.py | 6 ++++-- ironic/tests/unit/drivers/modules/test_ipmitool.py | 22 +++++++++++++++++++++- ...ror-check-ipmitool-reboot-ca7823202c5ab71d.yaml | 4 ++++ 3 files changed, 29 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/add-error-check-ipmitool-reboot-ca7823202c5ab71d.yaml diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 9cf145e30..13d8e57b5 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -828,11 +828,13 @@ class IPMIPower(base.PowerInterface): :raises: MissingParameterValue if required ipmi parameters are missing. :raises: InvalidParameterValue if an invalid power state was specified. :raises: PowerStateFailure if the final state of the node is not - POWER_ON. + POWER_ON or the intermediate state of the node is not POWER_OFF. """ driver_info = _parse_driver_info(task.node) - _power_off(driver_info) + intermediate_state = _power_off(driver_info) + if intermediate_state != states.POWER_OFF: + raise exception.PowerStateFailure(pstate=states.POWER_OFF) driver_utils.ensure_next_boot_device(task, driver_info) state = _power_on(driver_info) diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index dd60990fd..5b0b1d0bf 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -1456,6 +1456,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): def test_reboot_ok(self, mock_on, mock_off, mock_next_boot): manager = mock.MagicMock() # NOTE(rloo): if autospec is True, then manager.mock_calls is empty + mock_off.return_value = states.POWER_OFF mock_on.return_value = states.POWER_ON manager.attach_mock(mock_off, 'power_off') manager.attach_mock(mock_on, 'power_on') @@ -1471,9 +1472,28 @@ class IPMIToolDriverTestCase(db_base.DbTestCase): @mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType) @mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType) - def test_reboot_fail(self, mock_on, mock_off): + def test_reboot_fail_power_off(self, mock_on, mock_off): manager = mock.MagicMock() # NOTE(rloo): if autospec is True, then manager.mock_calls is empty + mock_off.return_value = states.ERROR + manager.attach_mock(mock_off, 'power_off') + manager.attach_mock(mock_on, 'power_on') + expected = [mock.call.power_off(self.info)] + + with task_manager.acquire(self.context, + self.node.uuid) as task: + self.assertRaises(exception.PowerStateFailure, + self.driver.power.reboot, + task) + + self.assertEqual(manager.mock_calls, expected) + + @mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType) + @mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType) + def test_reboot_fail_power_on(self, mock_on, mock_off): + manager = mock.MagicMock() + # NOTE(rloo): if autospec is True, then manager.mock_calls is empty + mock_off.return_value = states.POWER_OFF mock_on.return_value = states.ERROR manager.attach_mock(mock_off, 'power_off') manager.attach_mock(mock_on, 'power_on') diff --git a/releasenotes/notes/add-error-check-ipmitool-reboot-ca7823202c5ab71d.yaml b/releasenotes/notes/add-error-check-ipmitool-reboot-ca7823202c5ab71d.yaml new file mode 100644 index 000000000..d5ce622f9 --- /dev/null +++ b/releasenotes/notes/add-error-check-ipmitool-reboot-ca7823202c5ab71d.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - adds a missing error check into ipmitool power driver's reboot so + that the reboot can fail properly if power off failed. -- cgit v1.2.1