Merge "Fixes power-on failure for 'ilo' hardware type" into stable/stein

3 files changed, 59 insertions, 6 deletions

diff --git a/ironic/drivers/modules/ilo/power.py b/ironic/drivers/modules/ilo/power.py
index 94861f7b6..a35bf2d78 100644
--- a/ironic/drivers/modules/ilo/power.py
+++ b/ironic/drivers/modules/ilo/power.py
@@ -94,12 +94,13 @@ def _get_power_state(node):
         return states.ERROR
 
 
-def _wait_for_state_change(node, target_state,
+def _wait_for_state_change(node, target_state, requested_state,
                            is_final_state=True, timeout=None):
     """Wait for the power state change to get reflected.
 
     :param node: The node.
-    :param target_state: target power state of the node.
+    :param target_state: calculated target power state of the node.
+    :param requested_state: actual requested power state of the node.
     :param is_final_state: True, if the given target state is the final
         expected power state of the node. Default is True.
     :param timeout: timeout (in seconds) positive integer (> 0) for any
@@ -140,7 +141,14 @@ def _wait_for_state_change(node, target_state,
             target_state == states.SOFT_REBOOT and not is_final_state):
             state_to_check = ilo_common.POST_POWEROFF_STATE
         else:
-            state_to_check = ilo_common.POST_FINISHEDPOST_STATE
+            # It may not be able to finish POST if no bootable device is
+            # found. Track (POST_FINISHEDPOST_STATE) only for soft reboot.
+            # For other power-on cases track for beginning of POST operation
+            # (POST_INPOST_STATE) to return.
+            state_to_check = (
+                ilo_common.POST_FINISHEDPOST_STATE if
+                requested_state == states.SOFT_REBOOT else
+                ilo_common.POST_INPOST_STATE)
 
     def _wait(state):
         if use_post_state:
@@ -197,6 +205,7 @@ def _set_power_state(task, target_state, timeout=None):
     # Check if its soft power operation
     soft_power_op = target_state in [states.SOFT_POWER_OFF, states.SOFT_REBOOT]
 
+    requested_state = target_state
     if target_state == states.SOFT_REBOOT:
         if _get_power_state(node) == states.POWER_OFF:
             target_state = states.POWER_ON
@@ -239,7 +248,8 @@ def _set_power_state(task, target_state, timeout=None):
         is_final_state = target_state in (states.SOFT_POWER_OFF,
                                           states.POWER_ON)
         time_consumed = _wait_for_state_change(
-            node, target_state, is_final_state=is_final_state, timeout=timeout)
+            node, target_state, requested_state,
+            is_final_state=is_final_state, timeout=timeout)
         if target_state == states.SOFT_REBOOT:
             _attach_boot_iso_if_needed(task)
             try:
@@ -252,11 +262,12 @@ def _set_power_state(task, target_state, timeout=None):
             # Re-calculate timeout available for power-on operation
             rem_timeout = timeout - time_consumed
             time_consumed += _wait_for_state_change(
-                node, states.SOFT_REBOOT, is_final_state=True,
+                node, states.SOFT_REBOOT, requested_state, is_final_state=True,
                 timeout=rem_timeout)
     else:
         time_consumed = _wait_for_state_change(
-            node, target_state, is_final_state=True, timeout=timeout)
+            node, target_state, requested_state, is_final_state=True,
+            timeout=timeout)
     LOG.info("The node %(node_id)s operation of '%(state)s' "
              "is completed in %(time_consumed)s seconds.",
              {'node_id': node.uuid, 'state': target_state,
diff --git a/ironic/tests/unit/drivers/modules/ilo/test_power.py b/ironic/tests/unit/drivers/modules/ilo/test_power.py
index 4c2905cc7..8a84c9137 100644
--- a/ironic/tests/unit/drivers/modules/ilo/test_power.py
+++ b/ironic/tests/unit/drivers/modules/ilo/test_power.py
@@ -103,6 +103,24 @@ class IloPowerInternalMethodsTestCase(test_common.BaseIloTest):
 
         ilo_mock_object.reset_server.assert_called_once_with()
 
+    @mock.patch.object(ilo_common, 'get_server_post_state', spec_set=True,
+                       autospec=True)
+    def test__set_power_state_reboot_ok_post_supported(
+            self, get_post_mock, get_ilo_object_mock):
+        CONF.set_override('power_retry', 0, 'ilo')
+        CONF.set_override('power_wait', 1, 'ilo')
+        CONF.set_override('soft_power_off_timeout', 200, 'conductor')
+        ilo_mock_object = get_ilo_object_mock.return_value
+        get_post_mock.side_effect = (['FinishedPost', 'FinishedPost',
+                                      'PowerOff', 'InPost'])
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            ilo_power._set_power_state(task, states.REBOOT)
+            get_post_mock.assert_called_with(task.node)
+
+        ilo_mock_object.reset_server.assert_called_once_with()
+
     def test__set_power_state_off_fail(self, get_ilo_object_mock):
         ilo_mock_object = get_ilo_object_mock.return_value
         ilo_mock_object.get_host_power_status.return_value = 'ON'
@@ -128,6 +146,23 @@ class IloPowerInternalMethodsTestCase(test_common.BaseIloTest):
         ilo_mock_object.get_host_power_status.assert_called_with()
         ilo_mock_object.set_host_power.assert_called_once_with('ON')
 
+    @mock.patch.object(ilo_common, 'get_server_post_state', spec_set=True,
+                       autospec=True)
+    def test__set_power_state_on_ok_post_supported(
+            self, get_post_mock, get_ilo_object_mock):
+        CONF.set_override('power_retry', 0, 'ilo')
+        CONF.set_override('power_wait', 1, 'ilo')
+        CONF.set_override('soft_power_off_timeout', 200, 'conductor')
+        ilo_mock_object = get_ilo_object_mock.return_value
+        get_post_mock.side_effect = ['PowerOff', 'PowerOff', 'InPost']
+
+        target_state = states.POWER_ON
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            ilo_power._set_power_state(task, target_state)
+            get_post_mock.assert_called_with(task.node)
+        ilo_mock_object.set_host_power.assert_called_once_with('ON')
+
     @mock.patch.object(ilo_power.LOG, 'info')
     @mock.patch.object(ilo_power, '_attach_boot_iso_if_needed',
                        spec_set=True, autospec=True)
diff --git a/releasenotes/notes/story-2006288-ilo-power-on-fails-with-no-boot-device-b698fef59b04e515.yaml b/releasenotes/notes/story-2006288-ilo-power-on-fails-with-no-boot-device-b698fef59b04e515.yaml
new file mode 100644
index 000000000..2f6712bfa
--- /dev/null
+++ b/releasenotes/notes/story-2006288-ilo-power-on-fails-with-no-boot-device-b698fef59b04e515.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes an issue in powering-on of server in ``ilo`` hardware type. Server
+    was failing to return success for power-on operation if no bootable
+    device was found. See `story 2006288
+    <https://storyboard.openstack.org/#!/story/2006288>`__ for details.