From f15d5b9a37260b3876f9dadeb030412e6e1053b2 Mon Sep 17 00:00:00 2001 From: Naohiro Tamura Date: Fri, 31 Jul 2015 00:34:08 +0900 Subject: [PATCH] Generic power interface for soft reboot and soft power off This patch updates the generic power interface to support SOFT_REBOOT and SOFT_POWER_OFF. And also it introduces "timeout" optional parameter for all power operations. Partial-Bug: #1526226 Change-Id: I1c9bbd1f11f6a8565607c874b3c99aa10eeb62a5 --- doc/source/dev/webapi-version-history.rst | 6 + etc/ironic/ironic.conf.sample | 6 + ironic/api/controllers/v1/node.py | 30 +++- ironic/api/controllers/v1/utils.py | 9 ++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/states.py | 6 + ironic/conductor/manager.py | 34 ++++- ironic/conductor/rpcapi.py | 12 +- ironic/conductor/utils.py | 134 +++++++++++------- ironic/conf/conductor.py | 5 + ironic/drivers/base.py | 18 ++- ironic/drivers/fake.py | 8 ++ ironic/drivers/modules/fake.py | 21 ++- ironic/tests/unit/api/v1/test_nodes.py | 112 ++++++++++++++- ironic/tests/unit/conductor/test_manager.py | 62 +++++++- ironic/tests/unit/conductor/test_rpcapi.py | 2 +- ironic/tests/unit/conductor/test_utils.py | 90 ++++++++++++ ...soft-reboot-poweroff-9fdb0a4306dd668d.yaml | 7 + setup.cfg | 1 + 19 files changed, 492 insertions(+), 75 deletions(-) create mode 100644 releasenotes/notes/soft-reboot-poweroff-9fdb0a4306dd668d.yaml diff --git a/doc/source/dev/webapi-version-history.rst b/doc/source/dev/webapi-version-history.rst index d5702debae..2f8e333226 100644 --- a/doc/source/dev/webapi-version-history.rst +++ b/doc/source/dev/webapi-version-history.rst @@ -2,6 +2,12 @@ REST API Version History ======================== +**1.27** (Ocata) + + Add ``soft rebooting`` and ``soft power off`` as possible values + for the ``target`` field of the power state change payload, and + also add ``timeout`` field to it. + **1.26** (Ocata) Add portgroup ``mode`` and ``properties`` fields. diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index ac5c5cf7dc..28b18349cd 100644 --- a/etc/ironic/ironic.conf.sample +++ b/etc/ironic/ironic.conf.sample @@ -1013,6 +1013,12 @@ # disable timeout. (integer value) #clean_callback_timeout = 1800 +# Timeout (in seconds) of soft reboot and soft power off +# operation. This value always has to be positive. (integer +# value) +# Minimum value: 1 +#soft_power_off_timeout = 600 + [console] diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index f7a134cbbf..9885f6f244 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -102,6 +102,12 @@ PROVISION_ACTION_STATES = (ir_states.VERBS['manage'], _NODES_CONTROLLER_RESERVED_WORDS = None +ALLOWED_TARGET_POWER_STATES = (ir_states.POWER_ON, + ir_states.POWER_OFF, + ir_states.REBOOT, + ir_states.SOFT_REBOOT, + ir_states.SOFT_POWER_OFF) + def get_nodes_controller_reserved_names(): global _NODES_CONTROLLER_RESERVED_WORDS @@ -434,16 +440,22 @@ class NodeStatesController(rest.RestController): @METRICS.timer('NodeStatesController.power') @expose.expose(None, types.uuid_or_name, wtypes.text, + wtypes.IntegerType(minimum=1), status_code=http_client.ACCEPTED) - def power(self, node_ident, target): + def power(self, node_ident, target, timeout=None): """Set the power state of the node. :param node_ident: the UUID or logical name of a node. :param target: The desired power state of the node. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :raises: ClientSideError (HTTP 409) if a power operation is already in progress. :raises: InvalidStateRequested (HTTP 400) if the requested target state is not valid or if the node is in CLEANING state. + :raises: NotAcceptable (HTTP 406) for soft reboot, soft power off or + timeout parameter, if requested version of the API is less than 1.27. + :raises: Invalid (HTTP 400) if timeout value is less than 1. """ cdict = pecan.request.context.to_policy_values() @@ -454,9 +466,16 @@ class NodeStatesController(rest.RestController): rpc_node = api_utils.get_rpc_node(node_ident) topic = pecan.request.rpcapi.get_topic_for(rpc_node) - if target not in [ir_states.POWER_ON, - ir_states.POWER_OFF, - ir_states.REBOOT]: + if ((target in [ir_states.SOFT_REBOOT, ir_states.SOFT_POWER_OFF] or + timeout) and not api_utils.allow_soft_power_off()): + raise exception.NotAcceptable() + # FIXME(naohirot): This check is workaround because + # wtypes.IntegerType(minimum=1) is not effective + if timeout is not None and timeout < 1: + raise exception.Invalid( + _("timeout has to be positive integer")) + + if target not in ALLOWED_TARGET_POWER_STATES: raise exception.InvalidStateRequested( action=target, node=node_ident, state=rpc_node.power_state) @@ -470,7 +489,8 @@ class NodeStatesController(rest.RestController): pecan.request.rpcapi.change_node_power_state(pecan.request.context, rpc_node.uuid, target, - topic) + timeout=timeout, + topic=topic) # Set the HTTP Location Header url_args = '/'.join([node_ident, 'states']) pecan.response.location = link.build_url('nodes', url_args) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 714a0ee410..b031efac61 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -369,6 +369,15 @@ def allow_raid_config(): return pecan.request.version.minor >= versions.MINOR_12_RAID_CONFIG +def allow_soft_power_off(): + """Check if Soft Power Off is allowed for the node. + + Version 1.27 of the API allows Soft Power Off, including Soft Reboot, for + the node. + """ + return pecan.request.version.minor >= versions.MINOR_27_SOFT_POWER_OFF + + def allow_links_node_states_and_driver_properties(): """Check if links are displayable. diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 13a138f6c7..65997e3d8c 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -57,6 +57,7 @@ BASE_VERSION = 1 # Add port.portgroup_uuid field. # v1.25: Add possibility to unset chassis_uuid from node. # v1.26: Add portgroup.mode and portgroup.properties. +# v1.27: Add soft reboot, soft power off and timeout. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -85,11 +86,12 @@ MINOR_23_PORTGROUPS = 23 MINOR_24_PORTGROUPS_SUBCONTROLLERS = 24 MINOR_25_UNSET_CHASSIS_UUID = 25 MINOR_26_PORTGROUP_MODE_PROPERTIES = 26 +MINOR_27_SOFT_POWER_OFF = 27 # When adding another version, update MINOR_MAX_VERSION and also update # doc/source/dev/webapi-version-history.rst with a detailed explanation of # what the version has changed. -MINOR_MAX_VERSION = MINOR_26_PORTGROUP_MODE_PROPERTIES +MINOR_MAX_VERSION = MINOR_27_SOFT_POWER_OFF # String representations of the minor and maximum versions MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/states.py b/ironic/common/states.py index 150966497a..e270e78313 100644 --- a/ironic/common/states.py +++ b/ironic/common/states.py @@ -212,6 +212,12 @@ POWER_OFF = 'power off' REBOOT = 'rebooting' """ Node is rebooting. """ +SOFT_REBOOT = 'soft rebooting' +""" Node is rebooting gracefully. """ + +SOFT_POWER_OFF = 'soft power off' +""" Node is in the process of soft power off. """ + ##################### # State machine model diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 062aff4b4f..9161ef718e 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -83,7 +83,7 @@ class ConductorManager(base_manager.BaseConductorManager): """Ironic Conductor manager main class.""" # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. - RPC_API_VERSION = '1.38' + RPC_API_VERSION = '1.39' target = messaging.Target(version=RPC_API_VERSION) @@ -179,7 +179,8 @@ class ConductorManager(base_manager.BaseConductorManager): @messaging.expected_exceptions(exception.InvalidParameterValue, exception.NoFreeConductorWorker, exception.NodeLocked) - def change_node_power_state(self, context, node_id, new_state): + def change_node_power_state(self, context, node_id, new_state, + timeout=None): """RPC method to encapsulate changes to a node's state. Perform actions such as power on, power off. The validation is @@ -191,8 +192,12 @@ class ConductorManager(base_manager.BaseConductorManager): :param context: an admin context. :param node_id: the id or uuid of a node. :param new_state: the desired power state of the node. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :raises: NoFreeConductorWorker when there is no free worker to start async task. + :raises: InvalidParameterValue + :raises: MissingParameterValue """ LOG.debug("RPC change_node_power_state called for node %(node)s. " @@ -202,19 +207,38 @@ class ConductorManager(base_manager.BaseConductorManager): with task_manager.acquire(context, node_id, shared=False, purpose='changing node power state') as task: task.driver.power.validate(task) + + if (new_state not in + task.driver.power.get_supported_power_states(task)): + # FIXME(naohirot): + # After driver composition, we should print power interface + # name here instead of driver. + raise exception.InvalidParameterValue( + _('The driver %(driver)s does not support the power state,' + ' %(state)s') % + {'driver': task.node.driver, 'state': new_state}) + + if new_state in (states.SOFT_REBOOT, states.SOFT_POWER_OFF): + power_timeout = (timeout or + CONF.conductor.soft_power_off_timeout) + else: + power_timeout = timeout + # Set the target_power_state and clear any last_error, since we're # starting a new operation. This will expose to other processes # and clients that work is in progress. - if new_state == states.REBOOT: + if new_state in (states.POWER_ON, states.REBOOT, + states.SOFT_REBOOT): task.node.target_power_state = states.POWER_ON else: - task.node.target_power_state = new_state + task.node.target_power_state = states.POWER_OFF + task.node.last_error = None task.node.save() task.set_spawn_error_hook(utils.power_state_error_handler, task.node, task.node.power_state) task.spawn_after(self._spawn_worker, utils.node_power_action, - task, new_state) + task, new_state, timeout=power_timeout) @METRICS.timer('ConductorManager.vendor_passthru') @messaging.expected_exceptions(exception.NoFreeConductorWorker, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 369806f1c3..d6d433edfc 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -85,11 +85,12 @@ class ConductorAPI(object): | 1.36 - Added create_node | 1.37 - Added destroy_volume_target and update_volume_target | 1.38 - Added vif_attach, vif_detach, vif_list + | 1.39 - Added timeout optional parameter to change_node_power_state """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. - RPC_API_VERSION = '1.38' + RPC_API_VERSION = '1.39' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -179,7 +180,8 @@ class ConductorAPI(object): cctxt = self.client.prepare(topic=topic or self.topic, version='1.1') return cctxt.call(context, 'update_node', node_obj=node_obj) - def change_node_power_state(self, context, node_id, new_state, topic=None): + def change_node_power_state(self, context, node_id, new_state, + topic=None, timeout=None): """Change a node's power state. Synchronously, acquire lock and start the conductor background task @@ -188,14 +190,16 @@ class ConductorAPI(object): :param context: request context. :param node_id: node id or uuid. :param new_state: one of ironic.common.states power state values + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :param topic: RPC topic. Defaults to self.topic. :raises: NoFreeConductorWorker when there is no free worker to start async task. """ - cctxt = self.client.prepare(topic=topic or self.topic, version='1.6') + cctxt = self.client.prepare(topic=topic or self.topic, version='1.39') return cctxt.call(context, 'change_node_power_state', node_id=node_id, - new_state=new_state) + new_state=new_state, timeout=timeout) def vendor_passthru(self, context, node_id, driver_method, http_method, info, topic=None): diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 60ee9016c3..ec58e01482 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -15,6 +15,7 @@ from oslo_config import cfg from oslo_log import log from oslo_utils import excutils +from oslo_utils import reflection from ironic.common import exception from ironic.common.i18n import _, _LE, _LI, _LW @@ -67,15 +68,15 @@ def node_set_boot_device(task, device, persistent=False): @task_manager.require_exclusive_lock -def node_power_action(task, new_state): +def node_power_action(task, new_state, timeout=None): """Change power state or reset for a node. Perform the requested power action if the transition is required. :param task: a TaskManager instance containing the node to act on. - :param new_state: Any power state from ironic.common.states. If the - state is 'REBOOT' then a reboot will be attempted, otherwise - the node power state is directly set to 'state'. + :param new_state: Any power state from ironic.common.states. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :raises: InvalidParameterValue when the wrong state is specified or the wrong driver info is specified. :raises: other exceptions by the node's power driver if something @@ -86,50 +87,63 @@ def node_power_action(task, new_state): task, fields.NotificationLevel.INFO, fields.NotificationStatus.START, new_state) node = task.node - target_state = states.POWER_ON if new_state == states.REBOOT else new_state - if new_state != states.REBOOT: - try: - curr_state = task.driver.power.get_power_state(task) - except Exception as e: - with excutils.save_and_reraise_exception(): - node['last_error'] = _( - "Failed to change power state to '%(target)s'. " - "Error: %(error)s") % {'target': new_state, 'error': e} - node['target_power_state'] = states.NOSTATE - node.save() - notify_utils.emit_power_set_notification( - task, fields.NotificationLevel.ERROR, - fields.NotificationStatus.ERROR, new_state) + if new_state in (states.POWER_ON, states.REBOOT, states.SOFT_REBOOT): + target_state = states.POWER_ON + elif new_state in (states.POWER_OFF, states.SOFT_POWER_OFF): + target_state = states.POWER_OFF + else: + target_state = None - if curr_state == new_state: - # Neither the ironic service nor the hardware has erred. The - # node is, for some reason, already in the requested state, - # though we don't know why. eg, perhaps the user previously - # requested the node POWER_ON, the network delayed those IPMI - # packets, and they are trying again -- but the node finally - # responds to the first request, and so the second request - # gets to this check and stops. - # This isn't an error, so we'll clear last_error field - # (from previous operation), log a warning, and return. - node['last_error'] = None - # NOTE(dtantsur): under rare conditions we can get out of sync here - node['power_state'] = new_state + def _not_going_to_change(): + # Neither the ironic service nor the hardware has erred. The + # node is, for some reason, already in the requested state, + # though we don't know why. eg, perhaps the user previously + # requested the node POWER_ON, the network delayed those IPMI + # packets, and they are trying again -- but the node finally + # responds to the first request, and so the second request + # gets to this check and stops. + # This isn't an error, so we'll clear last_error field + # (from previous operation), log a warning, and return. + node['last_error'] = None + # NOTE(dtantsur): under rare conditions we can get out of sync here + node['power_state'] = curr_state + node['target_power_state'] = states.NOSTATE + node.save() + notify_utils.emit_power_set_notification( + task, fields.NotificationLevel.INFO, + fields.NotificationStatus.END, new_state) + LOG.warning(_LW("Not going to change node %(node)s power " + "state because current state = requested state " + "= '%(state)s'."), + {'node': node.uuid, 'state': curr_state}) + + try: + curr_state = task.driver.power.get_power_state(task) + except Exception as e: + with excutils.save_and_reraise_exception(): + node['last_error'] = _( + "Failed to change power state to '%(target)s'. " + "Error: %(error)s") % {'target': new_state, 'error': e} node['target_power_state'] = states.NOSTATE node.save() notify_utils.emit_power_set_notification( - task, fields.NotificationLevel.INFO, - fields.NotificationStatus.END, new_state) - LOG.warning(_LW("Not going to change node %(node)s power " - "state because current state = requested state " - "= '%(state)s'."), - {'node': node.uuid, 'state': curr_state}) - return + task, fields.NotificationLevel.ERROR, + fields.NotificationStatus.ERROR, new_state) - if curr_state == states.ERROR: - # be optimistic and continue action - LOG.warning(_LW("Driver returns ERROR power state for node %s."), - node.uuid) + if curr_state == states.POWER_ON: + if new_state == states.POWER_ON: + _not_going_to_change() + return + elif curr_state == states.POWER_OFF: + if new_state in (states.POWER_OFF, states.SOFT_POWER_OFF): + _not_going_to_change() + return + else: + # if curr_state == states.ERROR: + # be optimistic and continue action + LOG.warning(_LW("Driver returns ERROR power state for node %s."), + node.uuid) # Set the target_power_state and clear any last_error, if we're # starting a new operation. This will expose to other processes @@ -142,15 +156,37 @@ def node_power_action(task, new_state): # take power action try: if new_state != states.REBOOT: - task.driver.power.set_power_state(task, new_state) + 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( + _LW("The set_power_state method of %s(driver_name)s " + "doesn't support 'timeout' parameter."), + {'driver_name': node.driver}) + task.driver.power.set_power_state(task, new_state) else: - task.driver.power.reboot(task) + if ('timeout' in reflection.get_signature( + task.driver.power.reboot).parameters): + task.driver.power.reboot(task, timeout=timeout) + else: + LOG.warning(_LW("The reboot method of %s(driver_name)s " + "doesn't support 'timeout' parameter."), + {'driver_name': node.driver}) + task.driver.power.reboot(task) except Exception as e: with excutils.save_and_reraise_exception(): node['target_power_state'] = states.NOSTATE node['last_error'] = _( - "Failed to change power state to '%(target)s'. " - "Error: %(error)s") % {'target': target_state, 'error': e} + "Failed to change power state to '%(target_state)s' " + "by '%(new_state)s'. Error: %(error)s") % { + 'target_state': target_state, + 'new_state': new_state, + 'error': e} node.save() notify_utils.emit_power_set_notification( task, fields.NotificationLevel.ERROR, @@ -164,8 +200,10 @@ def node_power_action(task, new_state): task, fields.NotificationLevel.INFO, fields.NotificationStatus.END, new_state) LOG.info(_LI('Successfully set node %(node)s power state to ' - '%(state)s.'), - {'node': node.uuid, 'state': target_state}) + '%(target_state)s by %(new_state)s.'), + {'node': node.uuid, + 'target_state': target_state, + 'new_state': new_state}) @task_manager.require_exclusive_lock diff --git a/ironic/conf/conductor.py b/ironic/conf/conductor.py index 5eeee0f1e9..932b82c48e 100644 --- a/ironic/conf/conductor.py +++ b/ironic/conf/conductor.py @@ -131,6 +131,11 @@ opts = [ 'ramdisk doing the cleaning. If the timeout is reached ' 'the node will be put in the "clean failed" provision ' 'state. Set to 0 to disable timeout.')), + cfg.IntOpt('soft_power_off_timeout', + default=600, + min=1, + help=_('Timeout (in seconds) of soft reboot and soft power ' + 'off operation. This value always has to be positive.')), ] diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index ec74dbd2db..de4095221d 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -31,6 +31,7 @@ import six from ironic.common import exception from ironic.common.i18n import _, _LE, _LW from ironic.common import raid +from ironic.common import states from ironic.drivers.modules.network import common as net_common LOG = logging.getLogger(__name__) @@ -493,25 +494,38 @@ class PowerInterface(BaseInterface): """ @abc.abstractmethod - def set_power_state(self, task, power_state): + def set_power_state(self, task, power_state, timeout=None): """Set the power state of the task's node. :param task: a TaskManager instance containing the node to act on. :param power_state: Any power state from :mod:`ironic.common.states`. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :raises: MissingParameterValue if a required parameter is missing. """ @abc.abstractmethod - def reboot(self, task): + def reboot(self, task, timeout=None): """Perform a hard reboot of the task's node. Drivers are expected to properly handle case when node is powered off by powering it on. :param task: a TaskManager instance containing the node to act on. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :raises: MissingParameterValue if a required parameter is missing. """ + def get_supported_power_states(self, task): + """Get a list of the supported power states. + + :param task: A TaskManager instance containing the node to act on. + :returns: A list with the supported power states defined + in :mod:`ironic.common.states`. + """ + return [states.POWER_ON, states.POWER_OFF, states.REBOOT] + class ConsoleInterface(BaseInterface): """Interface for console-related actions.""" diff --git a/ironic/drivers/fake.py b/ironic/drivers/fake.py index 35d0e92add..be838e9751 100644 --- a/ironic/drivers/fake.py +++ b/ironic/drivers/fake.py @@ -78,6 +78,14 @@ class FakeDriver(base.BaseDriver): self.raid = fake.FakeRAID() +class FakeSoftPowerDriver(FakeDriver): + """Example implementation of a Driver.""" + + def __init__(self): + super(FakeSoftPowerDriver, self).__init__() + self.power = fake.FakeSoftPower() + + class FakeIPMIToolDriver(base.BaseDriver): """Example implementation of a Driver.""" diff --git a/ironic/drivers/modules/fake.py b/ironic/drivers/modules/fake.py index 42641e4ca6..c905bd0967 100644 --- a/ironic/drivers/modules/fake.py +++ b/ironic/drivers/modules/fake.py @@ -46,7 +46,7 @@ class FakePower(base.PowerInterface): def set_power_state(self, task, power_state): if power_state not in [states.POWER_ON, states.POWER_OFF]: raise exception.InvalidParameterValue( - _("set_power_state called with an invalid power" + _("set_power_state called with an invalid power " "state: %s.") % power_state) task.node.power_state = power_state @@ -54,6 +54,25 @@ class FakePower(base.PowerInterface): pass +class FakeSoftPower(FakePower): + """Example implementation of a simple soft power operations.""" + + def set_power_state(self, task, power_state, timeout=None): + if power_state not in [states.POWER_ON, states.POWER_OFF, + states.SOFT_REBOOT, states.SOFT_POWER_OFF]: + raise exception.InvalidParameterValue( + _("set_power_state called with an invalid power " + "state: %s.") % power_state) + task.node.power_state = power_state + + def reboot(self, task, timeout=None): + pass + + def get_supported_power_states(self, task): + return [states.POWER_ON, states.POWER_OFF, states.REBOOT, + states.SOFT_REBOOT, states.SOFT_POWER_OFF] + + class FakeBoot(base.BootInterface): """Example implementation of a simple boot interface.""" diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index 50aa3ab6a1..b7c543ea55 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -2345,21 +2345,120 @@ class TestPut(test_api_base.BaseApiTest): self.mock_dnih = p.start() self.addCleanup(p.stop) - def test_power_state(self): - response = self.put_json('/nodes/%s/states/power' % self.node.uuid, - {'target': states.POWER_ON}) + def _test_power_state_success(self, target_state, timeout, api_version): + if timeout is None: + body = {'target': target_state} + else: + body = {'target': target_state, 'timeout': timeout} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/power' % self.node.uuid, body) + else: + response = self.put_json( + '/nodes/%s/states/power' % self.node.uuid, body, + headers={api_base.Version.string: api_version}) + self.assertEqual(http_client.ACCEPTED, response.status_code) self.assertEqual(b'', response.body) self.mock_cnps.assert_called_once_with(mock.ANY, self.node.uuid, - states.POWER_ON, - 'test-topic') + target_state, + timeout=timeout, + topic='test-topic') # Check location header self.assertIsNotNone(response.location) expected_location = '/v1/nodes/%s/states' % self.node.uuid self.assertEqual(urlparse.urlparse(response.location).path, expected_location) + def _test_power_state_failure(self, target_state, http_status_code, + timeout, api_version): + if timeout is None: + body = {'target': target_state} + else: + body = {'target': target_state, 'timeout': timeout} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/power' % self.node.uuid, body, + expect_errors=True) + else: + response = self.put_json( + '/nodes/%s/states/power' % self.node.uuid, body, + headers={api_base.Version.string: api_version}, + expect_errors=True) + + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_status_code, response.status_code) + self.assertTrue(response.json['error_message']) + + def test_power_state_power_on_no_timeout_no_ver(self): + self._test_power_state_success(states.POWER_ON, None, None) + + def test_power_state_power_on_no_timeout_valid_soft_ver(self): + self._test_power_state_success(states.POWER_ON, None, "1.27") + + def test_power_state_power_on_no_timeout_invalid_soft_ver(self): + self._test_power_state_success(states.POWER_ON, None, "1.26") + + def test_power_state_power_on_valid_timeout_no_ver(self): + self._test_power_state_failure( + states.POWER_ON, http_client.NOT_ACCEPTABLE, 2, None) + + def test_power_state_power_on_valid_timeout_valid_soft_ver(self): + self._test_power_state_success(states.POWER_ON, 2, "1.27") + + def test_power_state_power_on_valid_timeout_invalid_soft_ver(self): + self._test_power_state_failure( + states.POWER_ON, http_client.NOT_ACCEPTABLE, 2, "1.26") + + def test_power_state_power_on_invalid_timeout_no_ver(self): + self._test_power_state_failure( + states.POWER_ON, http_client.BAD_REQUEST, 0, None) + + def test_power_state_power_on_invalid_timeout_valid_soft_ver(self): + self._test_power_state_failure( + states.POWER_ON, http_client.BAD_REQUEST, 0, "1.27") + + def test_power_state_power_on_invalid_timeout_invalid_soft_ver(self): + self._test_power_state_failure( + states.POWER_ON, http_client.BAD_REQUEST, 0, "1.26") + + def test_power_state_soft_power_off_no_timeout_no_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, None, None) + + def test_power_state_soft_power_off_no_timeout_valid_soft_ver(self): + self._test_power_state_success(states.SOFT_POWER_OFF, None, "1.27") + + def test_power_state_soft_power_off_no_timeout_invalid_soft_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, None, "1.26") + + def test_power_state_soft_power_off_valid_timeout_no_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, 2, None) + + def test_power_state_soft_power_off_valid_timeout_valid_soft_ver(self): + self._test_power_state_success(states.SOFT_POWER_OFF, 2, "1.27") + + def test_power_state_soft_power_off_valid_timeout_invalid_soft_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, 2, "1.26") + + def test_power_state_soft_power_off_invalid_timeout_no_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, 0, None) + + def test_power_state_soft_power_off_invalid_timeout_valid_soft_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.BAD_REQUEST, 0, "1.27") + + def test_power_state_soft_power_off_invalid_timeout_invalid_soft_ver(self): + self._test_power_state_failure( + states.SOFT_POWER_OFF, http_client.NOT_ACCEPTABLE, 0, "1.26") + def test_power_state_by_name_unsupported(self): response = self.put_json('/nodes/%s/states/power' % self.node.name, {'target': states.POWER_ON}, @@ -2375,7 +2474,8 @@ class TestPut(test_api_base.BaseApiTest): self.mock_cnps.assert_called_once_with(mock.ANY, self.node.uuid, states.POWER_ON, - 'test-topic') + timeout=None, + topic='test-topic') # Check location header self.assertIsNotNone(response.location) expected_location = '/v1/nodes/%s/states' % self.node.name diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 20269423ea..7a023711c0 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -85,6 +85,36 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, # background task's link callback. self.assertIsNone(node.reservation) + def test_change_node_power_state_soft_power_off_timeout(self): + # Test change_node_power_state with timeout optional parameter + # including integration with conductor.utils.node_power_action and + # lower. + mgr_utils.mock_the_extension_manager(driver="fake_soft_power") + self.driver = driver_factory.get_driver("fake_soft_power") + node = obj_utils.create_test_node(self.context, + driver='fake_soft_power', + power_state=states.POWER_ON) + self._start_service() + + with mock.patch.object(self.driver.power, + 'get_power_state') as get_power_mock: + get_power_mock.return_value = states.POWER_ON + + self.service.change_node_power_state(self.context, + node.uuid, + states.SOFT_POWER_OFF, + timeout=2) + self._stop_service() + + get_power_mock.assert_called_once_with(mock.ANY) + node.refresh() + self.assertEqual(states.POWER_OFF, node.power_state) + self.assertIsNone(node.target_power_state) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + @mock.patch.object(conductor_utils, 'node_power_action') def test_change_node_power_state_node_already_locked(self, pwr_act_mock): @@ -135,7 +165,7 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.NoFreeConductorWorker, exc.exc_info[0]) spawn_mock.assert_called_once_with(mock.ANY, mock.ANY, - mock.ANY) + mock.ANY, timeout=mock.ANY) node.refresh() self.assertEqual(initial_state, node.power_state) self.assertIsNone(node.target_power_state) @@ -343,7 +373,8 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, states.POWER_ON) spawn_mock.assert_called_once_with( - conductor_utils.node_power_action, mock.ANY, states.POWER_ON) + conductor_utils.node_power_action, mock.ANY, states.POWER_ON, + timeout=None) self.assertFalse(mock_notif.called) @mock.patch('ironic.objects.node.NodeSetPowerStateNotification') @@ -381,6 +412,33 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, 'baremetal.node.power_set.end', obj_fields.NotificationLevel.INFO) + def test_change_node_power_state_unsupported_state(self): + # Test change_node_power_state where unsupported power state raises + # an exception + initial_state = states.POWER_ON + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=initial_state) + self._start_service() + + with mock.patch.object(self.driver.power, + 'get_supported_power_states') as supported_mock: + supported_mock.return_value = [ + states.POWER_ON, states.POWER_OFF, states.REBOOT] + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_power_state, + self.context, + node.uuid, + states.SOFT_POWER_OFF) + + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + + node.refresh() + supported_mock.assert_called_once_with(mock.ANY) + self.assertEqual(states.POWER_ON, node.power_state) + self.assertIsNone(node.target_power_state) + self.assertIsNone(node.last_error) + @mgr_utils.mock_record_keepalive class CreateNodeTestCase(mgr_utils.ServiceSetUpMixin, diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index fddfb8cc38..df7481ee08 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -188,7 +188,7 @@ class RPCAPITestCase(base.DbTestCase): def test_change_node_power_state(self): self._test_rpcapi('change_node_power_state', 'call', - version='1.6', + version='1.39', node_id=self.fake_node['uuid'], new_state=states.POWER_ON) diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index 421983cbb0..f1c393d068 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -495,6 +495,96 @@ class NodePowerActionTestCase(base.DbTestCase): obj_fields.NotificationLevel.ERROR) +class NodeSoftPowerActionTestCase(base.DbTestCase): + + def setUp(self): + super(NodeSoftPowerActionTestCase, self).setUp() + mgr_utils.mock_the_extension_manager(driver="fake_soft_power") + self.driver = driver_factory.get_driver("fake_soft_power") + + def test_node_power_action_power_soft_reboot(self): + """Test for soft reboot a node.""" + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + driver='fake_soft_power', + power_state=states.POWER_ON) + task = task_manager.TaskManager(self.context, node.uuid) + + with mock.patch.object(self.driver.power, + 'get_power_state') as get_power_mock: + get_power_mock.return_value = states.POWER_ON + + conductor_utils.node_power_action(task, states.SOFT_REBOOT) + + node.refresh() + get_power_mock.assert_called_once_with(mock.ANY) + self.assertEqual(states.POWER_ON, node['power_state']) + self.assertIsNone(node['target_power_state']) + self.assertIsNone(node['last_error']) + + def test_node_power_action_power_soft_reboot_timeout(self): + """Test for soft reboot a node.""" + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + driver='fake_soft_power', + power_state=states.POWER_ON) + task = task_manager.TaskManager(self.context, node.uuid) + + with mock.patch.object(self.driver.power, + 'get_power_state') as get_power_mock: + get_power_mock.return_value = states.POWER_ON + + conductor_utils.node_power_action(task, states.SOFT_REBOOT, + timeout=2) + + node.refresh() + get_power_mock.assert_called_once_with(mock.ANY) + self.assertEqual(states.POWER_ON, node['power_state']) + self.assertIsNone(node['target_power_state']) + self.assertIsNone(node['last_error']) + + def test_node_power_action_soft_power_off(self): + """Test node_power_action to turn node soft power off.""" + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + driver='fake_soft_power', + power_state=states.POWER_ON) + task = task_manager.TaskManager(self.context, node.uuid) + + with mock.patch.object(self.driver.power, + 'get_power_state') as get_power_mock: + get_power_mock.return_value = states.POWER_ON + + conductor_utils.node_power_action(task, states.SOFT_POWER_OFF) + + node.refresh() + get_power_mock.assert_called_once_with(mock.ANY) + self.assertEqual(states.POWER_OFF, node['power_state']) + self.assertIsNone(node['target_power_state']) + self.assertIsNone(node['last_error']) + + def test_node_power_action_soft_power_off_timeout(self): + """Test node_power_action to turn node soft power off.""" + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + driver='fake_soft_power', + power_state=states.POWER_ON) + task = task_manager.TaskManager(self.context, node.uuid) + + with mock.patch.object(self.driver.power, + 'get_power_state') as get_power_mock: + get_power_mock.return_value = states.POWER_ON + + conductor_utils.node_power_action(task, states.SOFT_POWER_OFF, + timeout=2) + + node.refresh() + get_power_mock.assert_called_once_with(mock.ANY) + self.assertEqual(states.POWER_OFF, node['power_state']) + self.assertIsNone(node['target_power_state']) + self.assertIsNone(node['last_error']) + + class CleanupAfterTimeoutTestCase(tests_base.TestCase): def setUp(self): super(CleanupAfterTimeoutTestCase, self).setUp() diff --git a/releasenotes/notes/soft-reboot-poweroff-9fdb0a4306dd668d.yaml b/releasenotes/notes/soft-reboot-poweroff-9fdb0a4306dd668d.yaml new file mode 100644 index 0000000000..8cb35ea672 --- /dev/null +++ b/releasenotes/notes/soft-reboot-poweroff-9fdb0a4306dd668d.yaml @@ -0,0 +1,7 @@ +--- +features: + - Support ``soft rebooting`` and ``soft power off`` requests to + change node's power state with API version 1.27, and also + introduce ``timeout`` optional parameter and + ``[conductor]/soft_power_off_timeout`` configuration option. + Custom power drivers may be enhanced to support this feature. diff --git a/setup.cfg b/setup.cfg index 5a0e1cd953..3b09425709 100644 --- a/setup.cfg +++ b/setup.cfg @@ -52,6 +52,7 @@ ironic.drivers = agent_vbox = ironic.drivers.agent:AgentAndVirtualBoxDriver agent_ucs = ironic.drivers.agent:AgentAndUcsDriver fake = ironic.drivers.fake:FakeDriver + fake_soft_power = ironic.drivers.fake:FakeSoftPowerDriver fake_agent = ironic.drivers.fake:FakeAgentDriver fake_inspector = ironic.drivers.fake:FakeIPMIToolInspectorDriver fake_ipmitool = ironic.drivers.fake:FakeIPMIToolDriver