Merge "Add a missing error check in ipmitool driver's reboot" into stable/newton

3 files changed, 29 insertions, 3 deletions

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.