openstack/ironic
Code Issues Proposed changes

Merge "ipmitool: reboot: Don't power off node if already off"

Browse Source
This commit is contained in:
Jenkins 2017-10-08 13:19:25 +00:00 committed by Gerrit Code Review
commit b8a38d9731
5 changed files with 64 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ironic/conductor/utils.py
View File

@ -124,6 +124,12 @@ def _can_skip_state_change(task, new_state):
:returns: True if should ignore the requested power state change. False
otherwise
"""
# We only ignore certain state changes. So if the desired new_state is not
# one of them, then we can return early and not do an un-needed
# get_power_state() call
if new_state not in (states.POWER_ON, states.POWER_OFF,
states.SOFT_POWER_OFF):
return False
node = task.node

6
ironic/drivers/modules/ipmitool.py
View File

@ -833,7 +833,11 @@ class IPMIPower(base.PowerInterface):
"""
driver_info = _parse_driver_info(task.node)
_power_off(task, driver_info, timeout=timeout)
# NOTE(jlvillal): Some BMCs will error if setting power state to off if
# the node is already turned off.
current_status = _power_status(driver_info)
if current_status != states.POWER_OFF:
_power_off(task, driver_info, timeout=timeout)
driver_utils.ensure_next_boot_device(task, driver_info)
_power_on(task, driver_info, timeout=timeout)

13
ironic/tests/unit/conductor/test_utils.py
View File

@ -179,7 +179,10 @@ class NodePowerActionTestCase(db_base.DbTestCase):
task = task_manager.TaskManager(self.context, node.uuid)
with mock.patch.object(self.driver.power, 'reboot') as reboot_mock:
conductor_utils.node_power_action(task, states.REBOOT)
with mock.patch.object(self.driver.power,
'get_power_state') as get_power_mock:
conductor_utils.node_power_action(task, states.REBOOT)
self.assertFalse(get_power_mock.called)
node.refresh()
reboot_mock.assert_called_once_with(mock.ANY)
@ -205,7 +208,7 @@ class NodePowerActionTestCase(db_base.DbTestCase):
"INVALID_POWER_STATE")
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
self.assertFalse(get_power_mock.called)
self.assertEqual(states.POWER_ON, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertIsNotNone(node['last_error'])
@ -240,7 +243,7 @@ class NodePowerActionTestCase(db_base.DbTestCase):
"INVALID_POWER_STATE")
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
self.assertFalse(get_power_mock.called)
self.assertEqual(states.POWER_ON, node.power_state)
self.assertIsNone(node.target_power_state)
self.assertIsNotNone(node.last_error)
@ -720,7 +723,7 @@ class NodeSoftPowerActionTestCase(db_base.DbTestCase):
conductor_utils.node_power_action(task, states.SOFT_REBOOT)
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
self.assertFalse(get_power_mock.called)
self.assertEqual(states.POWER_ON, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertIsNone(node['last_error'])
@ -741,7 +744,7 @@ class NodeSoftPowerActionTestCase(db_base.DbTestCase):
timeout=2)
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
self.assertFalse(get_power_mock.called)
self.assertEqual(states.POWER_ON, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertIsNone(node['last_error'])

41
ironic/tests/unit/drivers/modules/test_ipmitool.py
View File

@ -1680,6 +1680,8 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
@mock.patch.object(driver_utils, 'ensure_next_boot_device', autospec=True)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_ON)
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
@ -1695,11 +1697,34 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
self.driver.power.reboot(task)
mock_next_boot.assert_called_once_with(task, self.info)
self.assertEqual(manager.mock_calls, expected)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(driver_utils, 'ensure_next_boot_device', autospec=True)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_OFF)
def test_reboot_already_off(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')
with task_manager.acquire(self.context,
self.node.uuid) as task:
expected = [mock.call.power_on(task, self.info, timeout=None)]
self.driver.power.reboot(task)
mock_next_boot.assert_called_once_with(task, self.info)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(driver_utils, 'ensure_next_boot_device', autospec=True)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_ON)
def test_reboot_timeout_ok(self, mock_on, mock_off, mock_next_boot):
manager = mock.MagicMock()
# NOTE(rloo): if autospec is True, then manager.mock_calls is empty
@ -1714,10 +1739,12 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
self.driver.power.reboot(task, timeout=2)
mock_next_boot.assert_called_once_with(task, self.info)
self.assertEqual(manager.mock_calls, expected)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_ON)
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
@ -1733,10 +1760,12 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
self.driver.power.reboot,
task)
self.assertEqual(manager.mock_calls, expected)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_ON)
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
@ -1754,10 +1783,12 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
self.driver.power.reboot,
task)
self.assertEqual(manager.mock_calls, expected)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(ipmi, '_power_off', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_on', spec_set=types.FunctionType)
@mock.patch.object(ipmi, '_power_status',
lambda driver_info: states.POWER_ON)
def test_reboot_timeout_fail(self, mock_on, mock_off):
manager = mock.MagicMock()
# NOTE(rloo): if autospec is True, then manager.mock_calls is empty
@ -1774,7 +1805,7 @@ class IPMIToolDriverTestCase(db_base.DbTestCase):
self.driver.power.reboot,
task, timeout=2)
self.assertEqual(manager.mock_calls, expected)
self.assertEqual(expected, manager.mock_calls)
@mock.patch.object(ipmi, '_parse_driver_info', autospec=True)
def test_vendor_passthru_validate__parse_driver_info_fail(self, info_mock):

9
releasenotes/notes/reboot-do-not-power-off-if-already-1452256167d40009.yaml Normal file
View File

@ -0,0 +1,9 @@
---
fixes:
- |
Fixes a problem when using ipmitool and rebooting a node which would cause
a deploy to fail. Now when rebooting a node we check if the node is already
powered off, if it is we don't attempt to power off the node. This is
because some BMCs will error if the node is already powered off and an
ipmitool request is made to power it off. See
https://bugs.launchpad.net/ironic/+bug/1718794 for details.