openstack
/
ironic
Code Issues Proposed changes

Fix handling of 'timeout' parameter to power methods 

The PowerInterface methods set_power_state() and reboot() were enhanced
to take a 'timeout' parameter [1]. To handle Interfaces that didn't
support timeout, conductor.utils.node_power_action() used
reflection.get_signature() to determine whether or not the node's
PowerInterface's methods could handle a timeout parameter.

It turns out that there was a bug with the
ironic_lib.metrics.timer decorator [2], such that
reflection.get_signature() did not return the method parameters. This
means that for PowerInterfaces that had this decorator, the conductor
would incorrectly think that 'timeout' was not supported, even if it
were supported.

Instead of trying to decide whether a PowerInterface supports 'timeout',
the conductor now assumes that it does. This patch changes all in-tree
PowerInterfaces so that they accept a 'timeout' parameter for reboot()
and set_power_state().

For any out-of-tree implementations that don't accept a 'timeout'
parameter, a TypeError exception will be raised.

[1] f15d5b9a37
[2] https://bugs.launchpad.net/ironic/+bug/1746730

Closes-Bug: #1746849
Change-Id: Iae28e94c34d4d69593644d0e5f4542558db9bb79
This commit is contained in:
Ruby Loo 2018-02-01 16:48:15 -05:00
parent d4e35611c4
commit 9e87cebc12
16 changed files with 404 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
ironic/conductor/utils.py
View File

@ -16,7 +16,6 @@ from oslo_config import cfg
from oslo_log import log
from oslo_service import loopingcall
from oslo_utils import excutils
from oslo_utils import reflection
from ironic.common import exception
from ironic.common.i18n import _
@ -226,32 +225,12 @@ def node_power_action(task, new_state, timeout=None):
task.driver.storage.attach_volumes(task)
if new_state != states.REBOOT:
if ('timeout' in reflection.get_signature(
task.driver.power.set_power_state).parameters):
task.driver.power.set_power_state(task, new_state,
timeout=timeout)
else:
# FIXME(naohirot):
# After driver composition, we should print power interface
# name here instead of driver.
LOG.warning(
"The set_power_state method of %(driver_name)s "
"doesn't support 'timeout' parameter.",
{'driver_name': node.driver})
task.driver.power.set_power_state(task, new_state)
task.driver.power.set_power_state(task, new_state, timeout=timeout)
else:
# TODO(TheJulia): We likely ought to consider toggling
# volume attachments, although we have no mechanism to
# really verify what cinder has connector wise.
if ('timeout' in reflection.get_signature(
task.driver.power.reboot).parameters):
task.driver.power.reboot(task, timeout=timeout)
else:
LOG.warning("The reboot method of %(driver_name)s "
"doesn't support 'timeout' parameter.",
{'driver_name': node.driver})
task.driver.power.reboot(task)
task.driver.power.reboot(task, timeout=timeout)
except Exception as e:
with excutils.save_and_reraise_exception():
node['target_power_state'] = states.NOSTATE

23
ironic/drivers/modules/cimc/power.py
View File

@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from oslo_log import log as logging
from oslo_service import loopingcall
from oslo_utils import importutils
@ -25,6 +26,8 @@ from ironic.drivers.modules.cimc import common
imcsdk = importutils.try_import('ImcSdk')
LOG = logging.getLogger(__name__)
if imcsdk:
CIMC_TO_IRONIC_POWER_STATE = {
@ -116,15 +119,24 @@ class Power(base.PowerInterface):
states.ERROR)
@task_manager.require_exclusive_lock
def set_power_state(self, task, pstate):
def set_power_state(self, task, pstate, timeout=None):
"""Set the power state of the task's node.
:param task: a TaskManager instance containing the node to act on.
:param pstate: Any power state from :mod:`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: MissingParameterValue if a required parameter is missing.
:raises: InvalidParameterValue if an invalid power state is passed
:raises: CIMCException if there is an error communicating with CIMC
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning(
"The 'cimc' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
if pstate not in IRONIC_TO_CIMC_POWER_STATE:
msg = _("set_power_state called for %(node)s with "
"invalid state %(state)s")
@ -150,16 +162,23 @@ class Power(base.PowerInterface):
raise exception.PowerStateFailure(pstate=pstate)
@task_manager.require_exclusive_lock
def reboot(self, task):
def reboot(self, task, timeout=None):
"""Perform a hard reboot of the task's node.
If the node is already powered on then it shall reboot the node, if
its off then the node will just be turned on.
:param task: a TaskManager instance containing the node to act on.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: MissingParameterValue if a required parameter is missing.
:raises: CIMCException if there is an error communicating with CIMC
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'cimc' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
current_power_state = self.get_power_state(task)
if current_power_state == states.POWER_ON:

20
ironic/drivers/modules/drac/power.py
View File

@ -149,27 +149,43 @@ class DracPower(base.PowerInterface):
@METRICS.timer('DracPower.set_power_state')
@task_manager.require_exclusive_lock
def set_power_state(self, task, power_state):
def set_power_state(self, task, power_state, timeout=None):
"""Set the power state of the node.
:param task: a TaskManager instance containing the node to act on.
:param power_state: a power state from :mod:`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: InvalidParameterValue if required DRAC credentials are
missing.
:raises: DracOperationError on an error from python-dracclient.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning(
"The 'idrac' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
_set_power_state(task.node, power_state)
@METRICS.timer('DracPower.reboot')
@task_manager.require_exclusive_lock
def reboot(self, task):
def reboot(self, task, timeout=None):
"""Perform a reboot of the task's node.
:param task: a TaskManager instance containing the node to act on.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: InvalidParameterValue if required DRAC credentials are
missing.
:raises: DracOperationError on an error from python-dracclient.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'idrac' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
current_power_state = _get_power_state(task.node)
if current_power_state == states.POWER_ON:

22
ironic/drivers/modules/ilo/power.py
View File

@ -200,30 +200,44 @@ class IloPower(base.PowerInterface):
@METRICS.timer('IloPower.set_power_state')
@task_manager.require_exclusive_lock
def set_power_state(self, task, power_state):
def set_power_state(self, task, power_state, timeout=None):
"""Turn the current power state on or off.
:param task: a TaskManager instance.
:param node: The Node.
:param power_state: The desired power state POWER_ON,POWER_OFF or
REBOOT from :mod:`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: InvalidParameterValue if an invalid power state was specified.
:raises: IloOperationError on an error from IloClient library.
:raises: PowerStateFailure if the power couldn't be set to power_state.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'ilo' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
_set_power_state(task, power_state)
@METRICS.timer('IloPower.reboot')
@task_manager.require_exclusive_lock
def reboot(self, task):
def reboot(self, task, timeout=None):
"""Reboot the node
:param task: a TaskManager instance.
:param node: The Node.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: PowerStateFailure if the final state of the node is not
POWER_ON.
:raises: IloOperationError on an error from IloClient library.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'ilo' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
node = task.node
current_pstate = _get_power_state(node)
if current_pstate == states.POWER_ON:

19
ironic/drivers/modules/snmp.py
View File

@ -700,7 +700,7 @@ class SNMPPower(base.PowerInterface):
return power_state
@task_manager.require_exclusive_lock
def set_power_state(self, task, pstate):
def set_power_state(self, task, pstate, timeout=None):
"""Turn the power on or off.
Set the power state of a node.
@ -708,6 +708,7 @@ class SNMPPower(base.PowerInterface):
:param task: A instance of `ironic.manager.task_manager.TaskManager`.
:param pstate: Either POWER_ON or POWER_OFF from :class:
`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: MissingParameterValue if required SNMP parameters are missing.
:raises: InvalidParameterValue if SNMP parameters are invalid or
`pstate` is invalid.
@ -715,6 +716,13 @@ class SNMPPower(base.PowerInterface):
as requested after the timeout.
:raises: SNMPFailure if an SNMP request fails.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning(
"The 'snmp' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
driver = _get_driver(task.node)
if pstate == states.POWER_ON:
@ -729,16 +737,23 @@ class SNMPPower(base.PowerInterface):
raise exception.PowerStateFailure(pstate=pstate)
@task_manager.require_exclusive_lock
def reboot(self, task):
def reboot(self, task, timeout=None):
"""Cycles the power to a node.
:param task: A instance of `ironic.manager.task_manager.TaskManager`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: MissingParameterValue if required SNMP parameters are missing.
:raises: InvalidParameterValue if SNMP parameters are invalid.
:raises: PowerStateFailure if the final power state of the node is not
POWER_ON after the timeout.
:raises: SNMPFailure if an SNMP request fails.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'snmp' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
driver = _get_driver(task.node)
state = driver.power_reset()

20
ironic/drivers/modules/ucs/power.py
View File

@ -120,7 +120,7 @@ class Power(base.PowerInterface):
@task_manager.require_exclusive_lock
@ucs_helper.requires_ucs_client
def set_power_state(self, task, pstate, helper=None):
def set_power_state(self, task, pstate, timeout=None, helper=None):
"""Turn the power on or off.
Set the power state of a node.
@ -128,6 +128,7 @@ class Power(base.PowerInterface):
:param task: instance of `ironic.manager.task_manager.TaskManager`.
:param pstate: Either POWER_ON or POWER_OFF from :class:
`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:param helper: ucs helper instance
:raises: InvalidParameterValue if an invalid power state was specified.
:raises: MissingParameterValue if required CiscoDriver parameters
@ -135,6 +136,13 @@ class Power(base.PowerInterface):
:raises: UcsOperationError on error from UCS Client.
:raises: PowerStateFailure if the desired power state couldn't be set.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning(
"The 'ucsm' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
if pstate not in (states.POWER_ON, states.POWER_OFF):
msg = _("set_power_state called with invalid power state "
@ -170,15 +178,23 @@ class Power(base.PowerInterface):
@task_manager.require_exclusive_lock
@ucs_helper.requires_ucs_client
def reboot(self, task, helper=None):
def reboot(self, task, timeout=None, helper=None):
"""Cycles the power to a node.
:param task: a TaskManager instance.
:param timeout: timeout (in seconds). Unsupported by this interface.
:param helper: ucs helper instance.
:raises: UcsOperationError on error from UCS Client.
:raises: PowerStateFailure if the final state of the node is not
POWER_ON.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'ucsm' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
try:
ucs_power_handle = ucs_power.UcsPower(helper)
ucs_power_handle.reboot()

19
ironic/drivers/modules/xclarity/power.py
View File

@ -71,15 +71,23 @@ class XClarityPower(base.PowerInterface):
@METRICS.timer('XClarityPower.set_power_state')
@task_manager.require_exclusive_lock
def set_power_state(self, task, power_state):
def set_power_state(self, task, power_state, timeout=None):
"""Turn the current power state on or off.
:param task: a TaskManager instance.
:param power_state: The desired power state POWER_ON, POWER_OFF or
REBOOT from :mod:`ironic.common.states`.
:param timeout: timeout (in seconds). Unsupported by this interface.
:raises: InvalidParameterValue if an invalid power state was specified.
:raises: XClarityError if XClarity fails setting the power state.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning(
"The 'xclarity' Power Interface's 'set_power_state' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
if power_state == states.REBOOT:
target_power_state = self.get_power_state(task)
@ -103,10 +111,17 @@ class XClarityPower(base.PowerInterface):
@METRICS.timer('XClarityPower.reboot')
@task_manager.require_exclusive_lock
def reboot(self, task):
def reboot(self, task, timeout=None):
"""Reboot the node
:param task: a TaskManager instance.
:param timeout: timeout (in seconds). Unsupported by this interface.
"""
# TODO(rloo): Support timeouts!
if timeout is not None:
LOG.warning("The 'xclarity' Power Interface's 'reboot' method "
"doesn't support the 'timeout' parameter. Ignoring "
"timeout=%(timeout)s",
{'timeout': timeout})
self.set_power_state(task, states.REBOOT)

6
ironic/tests/unit/conductor/test_manager.py
View File

@ -201,7 +201,8 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin,
self._stop_service()
get_power_mock.assert_called_once_with(mock.ANY)
set_power_mock.assert_called_once_with(mock.ANY, new_state)
set_power_mock.assert_called_once_with(
mock.ANY, new_state, timeout=None)
node.refresh()
self.assertEqual(initial_state, node.power_state)
self.assertIsNone(node.target_power_state)
@ -334,7 +335,8 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin,
# Give async worker a chance to finish
self._stop_service()
set_power_mock.assert_called_once_with(mock.ANY, states.POWER_ON)
set_power_mock.assert_called_once_with(
mock.ANY, states.POWER_ON, timeout=None)
# 2 notifications should be sent: 1 .start and 1 .error
self.assertEqual(2, mock_notif.call_count)

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

@ -20,6 +20,7 @@ from ironic.common import network
from ironic.common import states
from ironic.conductor import task_manager
from ironic.conductor import utils as conductor_utils
from ironic.drivers import base as drivers_base
from ironic import objects
from ironic.objects import fields as obj_fields
from ironic.tests import base as tests_base
@ -31,6 +32,25 @@ from ironic.tests.unit.objects import utils as obj_utils
CONF = cfg.CONF
class TestPowerNoTimeout(drivers_base.PowerInterface):
"""Missing 'timeout' parameter for get_power_state & reboot"""
def get_properties(self):
return {}
def validate(self, task):
pass
def get_power_state(self, task):
return task.node.power_state
def set_power_state(self, task, power_state, timeout=None):
task.node.power_state = power_state
def reboot(self, task):
pass
class NodeSetBootDeviceTestCase(db_base.DbTestCase):
def test_node_set_boot_device_non_existent_device(self):
@ -185,7 +205,7 @@ class NodePowerActionTestCase(db_base.DbTestCase):
self.assertFalse(get_power_mock.called)
node.refresh()
reboot_mock.assert_called_once_with(mock.ANY)
reboot_mock.assert_called_once_with(mock.ANY, timeout=None)
self.assertEqual(states.POWER_ON, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertIsNone(node['last_error'])
@ -442,8 +462,8 @@ class NodePowerActionTestCase(db_base.DbTestCase):
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
set_power_mock.assert_called_once_with(mock.ANY,
states.POWER_ON)
set_power_mock.assert_called_once_with(
mock.ANY, states.POWER_ON, timeout=None)
self.assertEqual(states.POWER_OFF, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertIsNotNone(node['last_error'])
@ -476,8 +496,8 @@ class NodePowerActionTestCase(db_base.DbTestCase):
node.refresh()
get_power_mock.assert_called_once_with(mock.ANY)
set_power_mock.assert_called_once_with(mock.ANY,
states.POWER_ON)
set_power_mock.assert_called_once_with(
mock.ANY, states.POWER_ON, timeout=None)
self.assertEqual(states.POWER_OFF, node.power_state)
self.assertIsNone(node.target_power_state)
self.assertIsNotNone(node.last_error)
@ -700,6 +720,33 @@ class NodePowerActionTestCase(db_base.DbTestCase):
'baremetal.node.power_set.error',
obj_fields.NotificationLevel.ERROR)
def test_node_power_action_reboot_no_timeout(self):
"""Test node reboot using Power Interface with no timeout arg."""
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
driver='fake-hardware',
console_interface='no-console',
inspect_interface='no-inspect',
raid_interface='no-raid',
rescue_interface='no-rescue',
vendor_interface='no-vendor',
power_state=states.POWER_ON)
self.config(enabled_boot_interfaces=['fake'])
self.config(enabled_deploy_interfaces=['fake'])
self.config(enabled_management_interfaces=['fake'])
self.config(enabled_power_interfaces=['fake'])
task = task_manager.TaskManager(self.context, node.uuid)
task.driver.power = TestPowerNoTimeout()
self.assertRaisesRegex(TypeError,
'unexpected keyword argument',
conductor_utils.node_power_action,
task, states.REBOOT)
node.refresh()
self.assertEqual(states.POWER_ON, node['power_state'])
self.assertIsNone(node['target_power_state'])
self.assertTrue('unexpected keyword argument' in node['last_error'])
class NodeSoftPowerActionTestCase(db_base.DbTestCase):

48
ironic/tests/unit/drivers/modules/cimc/test_power.py
View File

@ -190,7 +190,8 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
handle.get_imc_managedobject.assert_called_with(
None, None, params={"Dn": "sys/rack-unit-1"})
def test_set_power_state_on_ok(self, mock_handle):
@mock.patch.object(power.LOG, 'warning')
def test_set_power_state_on_ok(self, mock_log, mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
with mock_handle(task) as handle:
@ -213,6 +214,7 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
handle.get_imc_managedobject.assert_called_with(
None, None, params={"Dn": "sys/rack-unit-1"})
self.assertFalse(mock_log.called)
def test_set_power_state_on_fail(self, mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
@ -236,6 +238,33 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
handle.get_imc_managedobject.assert_called_with(
None, None, params={"Dn": "sys/rack-unit-1"})
@mock.patch.object(power.LOG, 'warning')
def test_set_power_state_on_timeout(self, mock_log, mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
with mock_handle(task) as handle:
mock_rack_unit = mock.MagicMock()
mock_rack_unit.get_attr.side_effect = [
imcsdk.ComputeRackUnit.CONST_OPER_POWER_OFF,
imcsdk.ComputeRackUnit.CONST_OPER_POWER_ON
]
handle.get_imc_managedobject.return_value = [mock_rack_unit]
task.driver.power.set_power_state(task, states.POWER_ON,
timeout=10)
handle.set_imc_managedobject.assert_called_once_with(
None, class_id="ComputeRackUnit",
params={
imcsdk.ComputeRackUnit.ADMIN_POWER:
imcsdk.ComputeRackUnit.CONST_ADMIN_POWER_UP,
imcsdk.ComputeRackUnit.DN: "sys/rack-unit-1"
})
handle.get_imc_managedobject.assert_called_with(
None, None, params={"Dn": "sys/rack-unit-1"})
self.assertTrue(mock_log.called)
def test_set_power_state_off_ok(self, mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
@ -282,14 +311,17 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
handle.get_imc_managedobject.assert_called_with(
None, None, params={"Dn": "sys/rack-unit-1"})
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.Power, "set_power_state", autospec=True)
@mock.patch.object(power.Power, "get_power_state", autospec=True)
def test_reboot_on(self, mock_get_state, mock_set_state, mock_handle):
def test_reboot_on(self, mock_get_state, mock_set_state, mock_log,
mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
mock_get_state.return_value = states.POWER_ON
task.driver.power.reboot(task)
mock_set_state.assert_called_with(mock.ANY, task, states.REBOOT)
self.assertFalse(mock_log.called)
@mock.patch.object(power.Power, "set_power_state", autospec=True)
@mock.patch.object(power.Power, "get_power_state", autospec=True)
@ -299,3 +331,15 @@ class PowerTestCase(test_common.CIMCBaseTestCase):
mock_get_state.return_value = states.POWER_OFF
task.driver.power.reboot(task)
mock_set_state.assert_called_with(mock.ANY, task, states.POWER_ON)
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.Power, "set_power_state", autospec=True)
@mock.patch.object(power.Power, "get_power_state", autospec=True)
def test_reboot_on_timeout(self, mock_get_state, mock_set_state, mock_log,
mock_handle):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
mock_get_state.return_value = states.POWER_ON
task.driver.power.reboot(task, timeout=30)
mock_set_state.assert_called_with(mock.ANY, task, states.REBOOT)
self.assertTrue(mock_log.called)

35
ironic/tests/unit/drivers/modules/drac/test_power.py
View File

@ -71,7 +71,8 @@ class DracPowerTestCase(db_base.DbTestCase):
mock_client.get_power_state.assert_called_once_with()
def test_set_power_state(self, mock_get_drac_client):
@mock.patch.object(drac_power.LOG, 'warning')
def test_set_power_state(self, mock_log, mock_get_drac_client):
mock_client = mock_get_drac_client.return_value
with task_manager.acquire(self.context, self.node.uuid,
@ -80,6 +81,7 @@ class DracPowerTestCase(db_base.DbTestCase):
drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
mock_client.set_power_state.assert_called_once_with(drac_power_state)
self.assertFalse(mock_log.called)
def test_set_power_state_fail(self, mock_get_drac_client):
mock_client = mock_get_drac_client.return_value
@ -95,7 +97,21 @@ class DracPowerTestCase(db_base.DbTestCase):
drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
mock_client.set_power_state.assert_called_once_with(drac_power_state)
def test_reboot_while_powered_on(self, mock_get_drac_client):
@mock.patch.object(drac_power.LOG, 'warning')
def test_set_power_state_timeout(self, mock_log, mock_get_drac_client):
mock_client = mock_get_drac_client.return_value
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.power.set_power_state(task, states.POWER_OFF,
timeout=11)
drac_power_state = drac_power.REVERSE_POWER_STATES[states.POWER_OFF]
mock_client.set_power_state.assert_called_once_with(drac_power_state)
self.assertTrue(mock_log.called)
@mock.patch.object(drac_power.LOG, 'warning')
def test_reboot_while_powered_on(self, mock_log, mock_get_drac_client):
mock_client = mock_get_drac_client.return_value
mock_client.get_power_state.return_value = drac_constants.POWER_ON
@ -105,6 +121,21 @@ class DracPowerTestCase(db_base.DbTestCase):
drac_power_state = drac_power.REVERSE_POWER_STATES[states.REBOOT]
mock_client.set_power_state.assert_called_once_with(drac_power_state)
self.assertFalse(mock_log.called)
@mock.patch.object(drac_power.LOG, 'warning')
def test_reboot_while_powered_on_timeout(self, mock_log,
mock_get_drac_client):
mock_client = mock_get_drac_client.return_value
mock_client.get_power_state.return_value = drac_constants.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.power.reboot(task, timeout=42)
drac_power_state = drac_power.REVERSE_POWER_STATES[states.REBOOT]
mock_client.set_power_state.assert_called_once_with(drac_power_state)
self.assertTrue(mock_log.called)
def test_reboot_while_powered_off(self, mock_get_drac_client):
mock_client = mock_get_drac_client.return_value

35
ironic/tests/unit/drivers/modules/ilo/test_power.py
View File

@ -209,20 +209,35 @@ class IloPowerTestCase(db_base.DbTestCase):
task.driver.power.get_power_state(task))
mock_get_power.assert_called_once_with(task.node)
@mock.patch.object(ilo_power.LOG, 'warning')
@mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
autospec=True)
def test_set_power_state(self, mock_set_power):
def test_set_power_state(self, mock_set_power, mock_log):
mock_set_power.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.power.set_power_state(task, states.POWER_ON)
mock_set_power.assert_called_once_with(task, states.POWER_ON)
self.assertFalse(mock_log.called)
@mock.patch.object(ilo_power.LOG, 'warning')
@mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
autospec=True)
def test_set_power_state_timeout(self, mock_set_power, mock_log):
mock_set_power.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
task.driver.power.set_power_state(task, states.POWER_ON,
timeout=13)
mock_set_power.assert_called_once_with(task, states.POWER_ON)
self.assertTrue(mock_log.called)
@mock.patch.object(ilo_power.LOG, 'warning')
@mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
autospec=True)
@mock.patch.object(ilo_power, '_get_power_state', spec_set=True,
autospec=True)
def test_reboot(self, mock_get_power, mock_set_power):
def test_reboot(self, mock_get_power, mock_set_power, mock_log):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
mock_get_power.return_value = states.POWER_ON
@ -230,3 +245,19 @@ class IloPowerTestCase(db_base.DbTestCase):
task.driver.power.reboot(task)
mock_get_power.assert_called_once_with(task.node)
mock_set_power.assert_called_once_with(task, states.REBOOT)
self.assertFalse(mock_log.called)
@mock.patch.object(ilo_power.LOG, 'warning')
@mock.patch.object(ilo_power, '_set_power_state', spec_set=True,
autospec=True)
@mock.patch.object(ilo_power, '_get_power_state', spec_set=True,
autospec=True)
def test_reboot_timeout(self, mock_get_power, mock_set_power, mock_log):
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
mock_get_power.return_value = states.POWER_ON
mock_set_power.return_value = states.POWER_ON
task.driver.power.reboot(task, timeout=123)
mock_get_power.assert_called_once_with(task.node)
mock_set_power.assert_called_once_with(task, states.REBOOT)
self.assertTrue(mock_log.called)

33
ironic/tests/unit/drivers/modules/test_snmp.py
View File

@ -1267,12 +1267,24 @@ class SNMPDriverTestCase(db_base.DbTestCase):
task.driver.power.get_power_state, task)
mock_driver.power_state.assert_called_once_with()
def test_set_power_state_on(self, mock_get_driver):
@mock.patch.object(snmp.LOG, 'warning')
def test_set_power_state_on(self, mock_log, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_on.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.set_power_state(task, states.POWER_ON)
mock_driver.power_on.assert_called_once_with()
self.assertFalse(mock_log.called)
@mock.patch.object(snmp.LOG, 'warning')
def test_set_power_state_on_timeout(self, mock_log, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_on.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.set_power_state(task, states.POWER_ON,
timeout=222)
mock_driver.power_on.assert_called_once_with()
self.assertTrue(mock_log.called)
def test_set_power_state_off(self, mock_get_driver):
mock_driver = mock_get_driver.return_value
@ -1305,7 +1317,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
task, states.POWER_OFF)
mock_driver.power_off.assert_called_once_with()
def test_set_power_state_on_timeout(self, mock_get_driver):
def test_set_power_state_on_error(self, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_on.return_value = states.ERROR
with task_manager.acquire(self.context, self.node.uuid) as task:
@ -1314,7 +1326,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
task, states.POWER_ON)
mock_driver.power_on.assert_called_once_with()
def test_set_power_state_off_timeout(self, mock_get_driver):
def test_set_power_state_off_error(self, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_off.return_value = states.ERROR
with task_manager.acquire(self.context, self.node.uuid) as task:
@ -1323,12 +1335,23 @@ class SNMPDriverTestCase(db_base.DbTestCase):
task, states.POWER_OFF)
mock_driver.power_off.assert_called_once_with()
def test_reboot(self, mock_get_driver):
@mock.patch.object(snmp.LOG, 'warning')
def test_reboot(self, mock_log, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_reset.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.reboot(task)
mock_driver.power_reset.assert_called_once_with()
self.assertFalse(mock_log.called)
@mock.patch.object(snmp.LOG, 'warning')
def test_reboot_timeout(self, mock_log, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_reset.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.reboot(task, timeout=1)
mock_driver.power_reset.assert_called_once_with()
self.assertTrue(mock_log.called)
def test_reboot_snmp_failure(self, mock_get_driver):
mock_driver = mock_get_driver.return_value
@ -1338,7 +1361,7 @@ class SNMPDriverTestCase(db_base.DbTestCase):
task.driver.power.reboot, task)
mock_driver.power_reset.assert_called_once_with()
def test_reboot_timeout(self, mock_get_driver):
def test_reboot_error(self, mock_get_driver):
mock_driver = mock_get_driver.return_value
mock_driver.power_reset.return_value = states.ERROR
with task_manager.acquire(self.context, self.node.uuid) as task:

53
ironic/tests/unit/drivers/modules/ucs/test_power.py
View File

@ -137,13 +137,15 @@ class UcsPowerTestCase(db_base.DbTestCase):
task)
power.get_power_state.assert_called_with()
@mock.patch.object(ucs_power.LOG, 'warning')
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
spec_set=True, autospec=True)
def test_set_power_state(self, mock_power_helper, mock__wait, mock_helper):
def test_set_power_state(self, mock_power_helper, mock__wait, mock_helper,
mock_log):
target_state = states.POWER_ON
mock_power = mock_power_helper.return_value
mock_power.get_power_state.side_effect = ['down', 'up']
@ -157,6 +159,32 @@ class UcsPowerTestCase(db_base.DbTestCase):
mock_power.set_power_state.assert_called_once_with('up')
mock_power.get_power_state.assert_called_once_with()
mock__wait.assert_called_once_with(target_state, mock_power)
self.assertFalse(mock_log.called)
@mock.patch.object(ucs_power.LOG, 'warning')
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
spec_set=True, autospec=True)
def test_set_power_state_timeout(self, mock_power_helper, mock__wait,
mock_helper, mock_log):
target_state = states.POWER_ON
mock_power = mock_power_helper.return_value
mock_power.get_power_state.side_effect = ['down', 'up']
mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
mock__wait.return_value = target_state
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
self.assertIsNone(self.interface.set_power_state(task,
target_state,
timeout=23))
mock_power.set_power_state.assert_called_once_with('up')
mock_power.get_power_state.assert_called_once_with()
mock__wait.assert_called_once_with(target_state, mock_power)
self.assertTrue(mock_log.called)
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)
@ -249,13 +277,15 @@ class UcsPowerTestCase(db_base.DbTestCase):
mock_power.get_power_state.assert_called_once_with()
mock__wait.assert_called_once_with(target_state, mock_power)
@mock.patch.object(ucs_power.LOG, 'warning')
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
spec_set=True, autospec=True)
def test_reboot(self, mock_power_helper, mock__wait, mock_helper):
def test_reboot(self, mock_power_helper, mock__wait, mock_helper,
mock_log):
mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
mock_power = mock_power_helper.return_value
mock__wait.return_value = states.POWER_ON
@ -263,6 +293,25 @@ class UcsPowerTestCase(db_base.DbTestCase):
shared=False) as task:
self.assertIsNone(self.interface.reboot(task))
mock_power.reboot.assert_called_once_with()
self.assertFalse(mock_log.called)
@mock.patch.object(ucs_power.LOG, 'warning')
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power._wait_for_state_change',
spec_set=True, autospec=True)
@mock.patch('ironic.drivers.modules.ucs.power.ucs_power.UcsPower',
spec_set=True, autospec=True)
def test_reboot_timeout(self, mock_power_helper, mock__wait, mock_helper,
mock_log):
mock_helper.generate_ucsm_handle.return_value = (True, mock.Mock())
mock_power = mock_power_helper.return_value
mock__wait.return_value = states.POWER_ON
with task_manager.acquire(self.context, self.node.uuid,
shared=False) as task:
self.assertIsNone(self.interface.reboot(task, timeout=88))
mock_power.reboot.assert_called_once_with()
self.assertTrue(mock_log.called)
@mock.patch('ironic.drivers.modules.ucs.helper.ucs_helper',
spec_set=True, autospec=True)

34
ironic/tests/unit/drivers/modules/xclarity/test_power.py
View File

@ -90,13 +90,28 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase):
task.driver.power.get_power_state,
task)
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.XClarityPower, 'get_power_state',
return_value=states.POWER_ON)
def test_set_power(self, mock_set_power_state, mock_get_xc_client):
def test_set_power(self, mock_set_power_state, mock_log,
mock_get_xc_client):
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.set_power_state(task, states.POWER_ON)
expected = task.driver.power.get_power_state(task)
self.assertEqual(expected, states.POWER_ON)
self.assertFalse(mock_log.called)
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.XClarityPower, 'get_power_state',
return_value=states.POWER_ON)
def test_set_power_timeout(self, mock_set_power_state, mock_log,
mock_get_xc_client):
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.set_power_state(task, states.POWER_ON,
timeout=21)
expected = task.driver.power.get_power_state(task)
self.assertEqual(expected, states.POWER_ON)
self.assertTrue(mock_log.called)
def test_set_power_fail(self, mock_xc_client):
with task_manager.acquire(self.context, self.node.uuid) as task:
@ -111,3 +126,20 @@ class XClarityPowerDriverTestCase(db_base.DbTestCase):
self.assertRaises(common.XClarityError,
task.driver.power.set_power_state,
task, states.POWER_OFF)
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.XClarityPower, 'set_power_state')
def test_reboot(self, mock_set_power_state, mock_log, mock_get_xc_client):
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.reboot(task)
mock_set_power_state.assert_called_with(task, states.REBOOT)
self.assertFalse(mock_log.called)
@mock.patch.object(power.LOG, 'warning')
@mock.patch.object(power.XClarityPower, 'set_power_state')
def test_reboot_timeout(self, mock_set_power_state, mock_log,
mock_get_xc_client):
with task_manager.acquire(self.context, self.node.uuid) as task:
task.driver.power.reboot(task, timeout=55)
mock_set_power_state.assert_called_with(task, states.REBOOT)
self.assertTrue(mock_log.called)

13
releasenotes/notes/add-timeout-parameter-to-power-methods-5f632c936497685e.yaml Normal file
View File

@ -0,0 +1,13 @@
---
fixes:
- |
Fixes an issue where the ``timeout`` that was specified for a node's
power state transition was not used for the *ipmitool*, *irmc* and
*oneview* Power Interfaces.
See `bug 1746849 <https://bugs.launchpad.net/ironic/+bug/1746849>`_
for details.
other:
- |
The ironic-conductor expects that all PowerInterface's set_power_state()
and reboot() methods accept a ``timeout`` parameter. Any out-of-tree
implementations that don't, will cause TypeError exceptions to be raised.