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

3 files changed, 124 insertions, 58 deletions

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 062aff4b4..9161ef718 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 369806f1c..d6d433edf 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 60ee9016c..ec58e0148 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 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
+    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
+
+    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