From 0bf2a4e8849d26068a60cf7af1ac978269fbc7b4 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 29 Jun 2017 16:47:08 +0200 Subject: [PATCH] Make redfish power interface wait for the power state change Currently it's not consistent with other power interfaces. Change-Id: I82dd74278121857f5bd810878961aa6fb50e53ff Related-Bug: #1526477 --- ironic/drivers/modules/redfish/power.py | 18 +++- .../drivers/modules/redfish/test_power.py | 83 +++++++++++++++++-- .../redfish-power-wait-c5d95432a3a714c4.yaml | 6 ++ 3 files changed, 96 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/redfish-power-wait-c5d95432a3a714c4.yaml diff --git a/ironic/drivers/modules/redfish/power.py b/ironic/drivers/modules/redfish/power.py index b01eb3aced..8f568bfb9e 100644 --- a/ironic/drivers/modules/redfish/power.py +++ b/ironic/drivers/modules/redfish/power.py @@ -20,6 +20,7 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states from ironic.conductor import task_manager +from ironic.conductor import utils as cond_utils from ironic.drivers import base from ironic.drivers.modules.redfish import utils as redfish_utils @@ -43,6 +44,12 @@ if sushy: states.SOFT_POWER_OFF: sushy.RESET_GRACEFUL_SHUTDOWN } +TARGET_STATE_MAP = { + states.REBOOT: states.POWER_ON, + states.SOFT_REBOOT: states.POWER_ON, + states.SOFT_POWER_OFF: states.POWER_OFF, +} + class RedfishPower(base.PowerInterface): @@ -93,7 +100,7 @@ class RedfishPower(base.PowerInterface): :param task: a TaskManager instance containing the node to act on. :param power_state: Any power state from :mod:`ironic.common.states`. - :param timeout: Not used by this driver. + :param timeout: Time to wait for the node to reach the requested state. :raises: MissingParameterValue if a required parameter is missing. :raises: RedfishConnectionError when it fails to connect to Redfish :raises: RedfishError on an error from the Sushy library @@ -108,12 +115,16 @@ class RedfishPower(base.PowerInterface): LOG.error(error_msg) raise exception.RedfishError(error=error_msg) + target_state = TARGET_STATE_MAP.get(power_state, power_state) + cond_utils.node_wait_for_power_state(task, target_state, + timeout=timeout) + @task_manager.require_exclusive_lock def reboot(self, task, timeout=None): """Perform a hard reboot of the task's node. :param task: a TaskManager instance containing the node to act on. - :param timeout: Not used by this driver. + :param timeout: Time to wait for the node to become powered on. :raises: MissingParameterValue if a required parameter is missing. :raises: RedfishConnectionError when it fails to connect to Redfish :raises: RedfishError on an error from the Sushy library @@ -133,6 +144,9 @@ class RedfishPower(base.PowerInterface): LOG.error(error_msg) raise exception.RedfishError(error=error_msg) + cond_utils.node_wait_for_power_state(task, states.POWER_ON, + timeout=timeout) + def get_supported_power_states(self, task): """Get a list of the supported power states. diff --git a/ironic/tests/unit/drivers/modules/redfish/test_power.py b/ironic/tests/unit/drivers/modules/redfish/test_power.py index d8e9a5ec1b..70e28ffdcc 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_power.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_power.py @@ -35,6 +35,7 @@ class MockedSushyError(Exception): pass +@mock.patch('eventlet.greenthread.sleep', lambda _t: None) class RedfishPowerTestCase(db_base.DbTestCase): def setUp(self): @@ -97,16 +98,58 @@ class RedfishPowerTestCase(db_base.DbTestCase): (states.SOFT_POWER_OFF, sushy.RESET_GRACEFUL_SHUTDOWN) ] - fake_system = mock_get_system.return_value for target, expected in expected_values: + if target in (states.POWER_OFF, states.SOFT_POWER_OFF): + final = sushy.SYSTEM_POWER_STATE_OFF + transient = sushy.SYSTEM_POWER_STATE_ON + else: + final = sushy.SYSTEM_POWER_STATE_ON + transient = sushy.SYSTEM_POWER_STATE_OFF + + system_result = [ + mock.Mock(power_state=transient) + ] * 3 + [mock.Mock(power_state=final)] + mock_get_system.side_effect = system_result + task.driver.power.set_power_state(task, target) # Asserts - fake_system.reset_system.assert_called_once_with(expected) - mock_get_system.assert_called_once_with(task.node) + system_result[0].reset_system.assert_called_once_with(expected) + mock_get_system.assert_called_with(task.node) + self.assertEqual(4, mock_get_system.call_count) + + # Reset mocks + mock_get_system.reset_mock() + + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_not_reached(self, mock_get_system): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.config(power_state_change_timeout=2, group='conductor') + expected_values = [ + (states.POWER_ON, sushy.RESET_ON), + (states.POWER_OFF, sushy.RESET_FORCE_OFF), + (states.REBOOT, sushy.RESET_FORCE_RESTART), + (states.SOFT_REBOOT, sushy.RESET_GRACEFUL_RESTART), + (states.SOFT_POWER_OFF, sushy.RESET_GRACEFUL_SHUTDOWN) + ] + + for target, expected in expected_values: + fake_system = mock_get_system.return_value + if target in (states.POWER_OFF, states.SOFT_POWER_OFF): + fake_system.power_state = sushy.SYSTEM_POWER_STATE_ON + else: + fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF + + self.assertRaises(exception.PowerStateFailure, + task.driver.power.set_power_state, + task, target) + + # Asserts + fake_system.reset_system.assert_called_once_with(expected) + mock_get_system.assert_called_with(task.node) # Reset mocks - fake_system.reset_system.reset_mock() mock_get_system.reset_mock() @mock.patch('ironic.drivers.modules.redfish.power.sushy') @@ -127,7 +170,6 @@ class RedfishPowerTestCase(db_base.DbTestCase): @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_reboot(self, mock_get_system): - fake_system = mock_get_system.return_value with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: expected_values = [ @@ -136,17 +178,40 @@ class RedfishPowerTestCase(db_base.DbTestCase): ] for current, expected in expected_values: - fake_system.power_state = current + system_result = [ + # Initial state + mock.Mock(power_state=current), + # Transient state - powering off + mock.Mock(power_state=sushy.SYSTEM_POWER_STATE_OFF), + # Final state - down powering off + mock.Mock(power_state=sushy.SYSTEM_POWER_STATE_ON) + ] + mock_get_system.side_effect = system_result + task.driver.power.reboot(task) # Asserts - fake_system.reset_system.assert_called_once_with(expected) - mock_get_system.assert_called_once_with(task.node) + system_result[0].reset_system.assert_called_once_with(expected) + mock_get_system.assert_called_with(task.node) + self.assertEqual(3, mock_get_system.call_count) # Reset mocks - fake_system.reset_system.reset_mock() mock_get_system.reset_mock() + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_reboot_not_reached(self, mock_get_system): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + fake_system = mock_get_system.return_value + fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF + + self.assertRaises(exception.PowerStateFailure, + task.driver.power.reboot, task) + + # Asserts + fake_system.reset_system.assert_called_once_with(sushy.RESET_ON) + mock_get_system.assert_called_with(task.node) + @mock.patch('ironic.drivers.modules.redfish.power.sushy') @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_reboot_fail(self, mock_get_system, mock_sushy): diff --git a/releasenotes/notes/redfish-power-wait-c5d95432a3a714c4.yaml b/releasenotes/notes/redfish-power-wait-c5d95432a3a714c4.yaml new file mode 100644 index 0000000000..65b79514a6 --- /dev/null +++ b/releasenotes/notes/redfish-power-wait-c5d95432a3a714c4.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + The ``redfish`` power interface now waits for a power action to apply. + Previously it used to return immediately, which was not consistent with + how other power interfaces work.