Merge "Add MANAGEABLE state and associated transitions"

ironic/api/controllers/v1/__init__.py

@@ -58,7 +58,8 @@ MIN_VER = base.Version({base.Version.string: "1.1"})
 
 # v1.2: Renamed NOSTATE ("None") to AVAILABLE ("available")
 # v1.3: Add node.driver_internal_info
-MAX_VER = base.Version({base.Version.string: "1.3"})
+# v1.4: Add MANAGEABLE state
+MAX_VER = base.Version({base.Version.string: "1.4"})
 
 
 class MediaType(base.APIBase):

ironic/api/controllers/v1/node.py

@@ -68,6 +68,14 @@ def hide_driver_internal_info(obj):
         obj.driver_internal_info = wsme.Unset
 
 
+def check_allow_management_verbs(verb):
+    # v1.4 added the MANAGEABLE state and two verbs to move nodes into
+    # and out of that state. Reject requests to do this in older versions
+    if (pecan.request.version.minor < 4 and
+            verb in [ir_states.VERBS['manage'], ir_states.VERBS['provide']]):
+        raise exception.NotAcceptable()
+
+
 class NodePatchType(types.JsonPatchType):
 
     @staticmethod
@@ -313,7 +321,8 @@ class NodeStatesController(rest.RestController):
         if target not in [ir_states.POWER_ON,
                           ir_states.POWER_OFF,
                           ir_states.REBOOT]:
-            raise exception.InvalidStateRequested(state=target, node=node_uuid)
+            raise exception.InvalidStateRequested(
+                    action=target, node=node_uuid, state=rpc_node.power_state)
 
         pecan.request.rpcapi.change_node_power_state(pecan.request.context,
                                                      node_uuid, target, topic)
@@ -324,7 +333,7 @@ class NodeStatesController(rest.RestController):
     @wsme_pecan.wsexpose(None, types.uuid, wtypes.text, wtypes.text,
                          status_code=202)
     def provision(self, node_uuid, target, configdrive=None):
-        """Asynchronous trigger the provisioning of the node.
+        """Asynchronously trigger the provisioning of the node.
 
         This will set the target provision state of the node, and a
         background task will begin which actually applies the state
@@ -338,34 +347,34 @@ class NodeStatesController(rest.RestController):
         :param configdrive: Optional. A gzipped and base64 encoded
             configdrive. Only valid when setting provision state
             to "active".
+        :raises: NodeLocked (HTTP 409) if the node is currently locked.
         :raises: ClientSideError (HTTP 409) if the node is already being
                  provisioned.
-        :raises: ClientSideError (HTTP 400) if the node is already in
+        :raises: InvalidStateRequested (HTTP 400) if the requested transition
-                 the requested state.
+                 is not possible from the current state.
-        :raises: InvalidStateRequested (HTTP 400) if the requested target
+        :raises: NotAcceptable (HTTP 406) if the API version specified does
-                 state is not valid.
+                 not allow the requested state transition.
         """
+        check_allow_management_verbs(target)
         rpc_node = objects.Node.get_by_uuid(pecan.request.context, node_uuid)
         topic = pecan.request.rpcapi.get_topic_for(rpc_node)
 
-        if target == rpc_node.provision_state:
+        # Normally, we let the task manager recognize and deal with
-            msg = (_("Node %(node)s is already in the '%(state)s' state.") %
+        # NodeLocked exceptions. However, that isn't done until the RPC calls
-                   {'node': rpc_node['uuid'], 'state': target})
+        # below. In order to main backward compatibility with our API HTTP
-            raise wsme.exc.ClientSideError(msg, status_code=400)
+        # response codes, we have this check here to deal with cases where
+        # a node is already being operated on (DEPLOYING or such) and we
+        # want to continue returning 409. Without it, we'd return 400.
+        if rpc_node.reservation:
+            raise exception.NodeLocked(node=rpc_node.uuid,
+                                       host=rpc_node.reservation)
 
-        if target not in (ir_states.ACTIVE, ir_states.DELETED,
+        m = ir_states.machine.copy()
-                          ir_states.REBUILD):
+        m.initialize(rpc_node.provision_state)
-            raise exception.InvalidStateRequested(state=target, node=node_uuid)
+        if not m.is_valid_event(ir_states.VERBS.get(target, target)):
-
+            raise exception.InvalidStateRequested(
-        valid_states_if_processing = [ir_states.DEPLOYFAIL]
+                    action=target, node=node_uuid,
-        if target == ir_states.DELETED:
+                    state=rpc_node.provision_state)
-            valid_states_if_processing.append(ir_states.DEPLOYWAIT)
-
-        if (rpc_node.target_provision_state is not None and
-                rpc_node.provision_state not in valid_states_if_processing):
-            msg = (_('Node %s is already being provisioned or decommissioned.')
-                   % rpc_node.uuid)
-            raise wsme.exc.ClientSideError(msg, status_code=409)  # Conflict
 
         if configdrive and target != ir_states.ACTIVE:
             msg = (_('Adding a config drive is only supported when setting '
@@ -386,6 +395,15 @@ class NodeStatesController(rest.RestController):
         elif target == ir_states.DELETED:
             pecan.request.rpcapi.do_node_tear_down(
                     pecan.request.context, node_uuid, topic)
+        elif target in (
+                ir_states.VERBS['manage'], ir_states.VERBS['provide']):
+            pecan.request.rpcapi.do_provisioning_action(
+                    pecan.request.context, node_uuid, target, topic)
+        else:
+            msg = (_('The requested action "%(action)s" could not be '
+                     'understood.') % {'action': target})
+            raise exception.InvalidStateRequested(message=msg)
+
         # Set the HTTP Location Header
         url_args = '/'.join([node_uuid, 'states'])
         pecan.response.location = link.build_url('nodes', url_args)

ironic/common/exception.py

@@ -163,7 +163,8 @@ class InvalidMAC(Invalid):
 
 
 class InvalidStateRequested(Invalid):
-    message = _("Invalid state '%(state)s' requested for node %(node)s.")
+    message = _('The requested action "%(action)s" can not be performed '
+                'on node "%(node)s" while it is in state "%(state)s".')
 
 
 class PatchError(Invalid):

ironic/common/states.py

@@ -37,6 +37,25 @@ LOG = logging.getLogger(__name__)
 # Provisioning states
 #####################
 
+# TODO(deva): add add'l state mappings here
+VERBS = {
+        'active': 'deploy',
+        'deleted': 'delete',
+        'manage': 'manage',
+        'provide': 'provide',
+    }
+""" Mapping of state-changing events that are PUT to the REST API
+
+This is a mapping of target states which are PUT to the API, eg,
+    PUT /v1/node/states/provision {'target': 'active'}
+
+The dict format is:
+    {target string used by the API: internal verb}
+
+This provides a reference set of supported actions, and in the future
+may be used to support renaming these actions.
+"""
+
 NOSTATE = None
 """ No state information.
 
@@ -44,6 +63,14 @@ This state is used with power_state to represent a lack of knowledge of
 power state, and in target_*_state fields when there is no target.
 """
 
+MANAGEABLE = 'manageable'
+""" Node is in a manageable state.
+
+This state indicates that Ironic has verified, at least once, that it had
+sufficient information to manage the hardware. While in this state, the node
+is not available for provisioning (it must be in the AVAILABLE state for that).
+"""
+
 AVAILABLE = 'available'
 """ Node is available for use and scheduling.
 
@@ -91,6 +118,8 @@ In Kilo, this will be a transitory value of provision_state, and never
 represented in target_provision_state.
 """
 
+# TODO(deva): add CLEAN* states
+
 ERROR = 'error'
 """ An error occurred during node processing.
 
@@ -140,10 +169,18 @@ watchers['on_enter'] = on_enter
 machine = fsm.FSM()
 
 # Add stable states
+machine.add_state(MANAGEABLE, **watchers)
 machine.add_state(AVAILABLE, **watchers)
 machine.add_state(ACTIVE, **watchers)
 machine.add_state(ERROR, **watchers)
 
+# From MANAGEABLE, a node may be made available
+# TODO(deva): add CLEAN* states to this path
+machine.add_transition(MANAGEABLE, AVAILABLE, 'provide')
+
+# From AVAILABLE, a node may be made unavailable by managing it
+machine.add_transition(AVAILABLE, MANAGEABLE, 'manage')
+
 # Add deploy* states
 # NOTE(deva): Juno shows a target_provision_state of DEPLOYDONE
 #             this is changed in Kilo to ACTIVE
@@ -165,6 +202,8 @@ machine.add_transition(DEPLOYING, DEPLOYFAIL, 'fail')
 # A failed deployment may be retried
 # ironic/conductor/manager.py:do_node_deploy()
 machine.add_transition(DEPLOYFAIL, DEPLOYING, 'rebuild')
+# NOTE(deva): Juno allows a client to send "active" to initiate a rebuild
+machine.add_transition(DEPLOYFAIL, DEPLOYING, 'deploy')
 
 # A deployment may also wait on external callbacks
 machine.add_transition(DEPLOYING, DEPLOYWAIT, 'wait')

ironic/conductor/manager.py

@@ -172,7 +172,7 @@ class ConductorManager(periodic_task.PeriodicTasks):
     """Ironic Conductor manager main class."""
 
     # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's.
-    RPC_API_VERSION = '1.22'
+    RPC_API_VERSION = '1.23'
 
     target = messaging.Target(version=RPC_API_VERSION)
 
@@ -616,8 +616,7 @@ class ConductorManager(periodic_task.PeriodicTasks):
                                    exception.NodeLocked,
                                    exception.NodeInMaintenance,
                                    exception.InstanceDeployFailure,
-                                   exception.InvalidParameterValue,
+                                   exception.InvalidStateRequested)
-                                   exception.MissingParameterValue)
     def do_node_deploy(self, context, node_id, rebuild=False,
                        configdrive=None):
         """RPC method to initiate deployment to a node.
@@ -637,6 +636,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
         :raises: NodeInMaintenance if the node is in maintenance mode.
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task.
+        :raises: InvalidStateRequested when the requested state is not a valid
+                 target from the current state.
 
         """
         LOG.debug("RPC do_node_deploy called for node %s." % node_id)
@@ -682,18 +683,14 @@ class ConductorManager(periodic_task.PeriodicTasks):
                                               configdrive),
                                    err_handler=provisioning_error_handler)
             except exception.InvalidState:
-                raise exception.InstanceDeployFailure(_(
+                raise exception.InvalidStateRequested(
-                    "Request received to %(what)s %(node)s, but "
+                        action=event, node=task.node.uuid,
-                    "this is not possible in the current state of "
+                        state=task.node.provision_state)
-                    "'%(state)s'. ") % {'what': event,
-                                        'node': node.uuid,
-                                        'state': node.provision_state})
 
     @messaging.expected_exceptions(exception.NoFreeConductorWorker,
                                    exception.NodeLocked,
                                    exception.InstanceDeployFailure,
-                                   exception.InvalidParameterValue,
+                                   exception.InvalidStateRequested)
-                                   exception.MissingParameterValue)
     def do_node_tear_down(self, context, node_id):
         """RPC method to tear down an existing node deployment.
 
@@ -705,12 +702,13 @@ class ConductorManager(periodic_task.PeriodicTasks):
         :raises: InstanceDeployFailure
         :raises: NoFreeConductorWorker when there is no free worker to start
                  async task
+        :raises: InvalidStateRequested when the requested state is not a valid
+                 target from the current state.
 
         """
         LOG.debug("RPC do_node_tear_down called for node %s." % node_id)
 
         with task_manager.acquire(context, node_id, shared=False) as task:
-            node = task.node
             try:
                 # NOTE(ghe): Valid power driver values are needed to perform
                 # a tear-down. Deploy info is useful to purge the cache but not
@@ -719,8 +717,8 @@ class ConductorManager(periodic_task.PeriodicTasks):
             except (exception.InvalidParameterValue,
                     exception.MissingParameterValue) as e:
                 raise exception.InstanceDeployFailure(_(
-                    "RPC do_node_tear_down failed to validate power info. "
+                    "Failed to validate power driver interface. "
-                    "Error: %(msg)s") % {'msg': e})
+                    "Can not delete instance. Error: %(msg)s") % {'msg': e})
 
             try:
                 task.process_event('delete',
@@ -728,10 +726,36 @@ class ConductorManager(periodic_task.PeriodicTasks):
                                    call_args=(do_node_tear_down, task),
                                    err_handler=provisioning_error_handler)
             except exception.InvalidState:
-                raise exception.InstanceDeployFailure(_(
+                raise exception.InvalidStateRequested(
-                    "RPC do_node_tear_down "
+                        action='delete', node=task.node.uuid,
-                    "not allowed for node %(node)s in state %(state)s")
+                        state=task.node.provision_state)
-                    % {'node': node_id, 'state': node.provision_state})
+
+    @messaging.expected_exceptions(exception.NoFreeConductorWorker,
+                                   exception.NodeLocked,
+                                   exception.InvalidParameterValue,
+                                   exception.MissingParameterValue,
+                                   exception.InvalidStateRequested)
+    def do_provisioning_action(self, context, node_id, action):
+        """RPC method to initiate certain provisioning state transitions.
+
+        Initiate a provisioning state change through the state machine,
+        rather than through an RPC call to do_node_deploy / do_node_tear_down
+
+        :param context: an admin context.
+        :param node_id: the id or uuid of a node.
+        :param action: an action. One of ironic.common.states.VERBS
+        :raises: InvalidParameterValue
+        :raises: InvalidStateRequested
+        :raises: NoFreeConductorWorker
+
+        """
+        with task_manager.acquire(context, node_id, shared=False) as task:
+            try:
+                task.process_event(action)
+            except exception.InvalidState:
+                raise exception.InvalidStateRequested(
+                        action=action, node=task.node.uuid,
+                        state=task.node.provision_state)
 
     @periodic_task.periodic_task(
             spacing=CONF.conductor.sync_power_state_interval)

ironic/conductor/rpcapi.py

@@ -65,11 +65,12 @@ class ConductorAPI(object):
     |    1.21 - Added get_node_vendor_passthru_methods and
     |           get_driver_vendor_passthru_methods
     |    1.22 - Added configdrive parameter to do_node_deploy.
+    |    1.23 - Added do_provisioning_action
 
     """
 
     # NOTE(rloo): This must be in sync with manager.ConductorManager's.
-    RPC_API_VERSION = '1.22'
+    RPC_API_VERSION = '1.23'
 
     def __init__(self, topic=None):
         super(ConductorAPI, self).__init__()
@@ -303,6 +304,25 @@ class ConductorAPI(object):
         cctxt = self.client.prepare(topic=topic or self.topic, version='1.6')
         return cctxt.call(context, 'do_node_tear_down', node_id=node_id)
 
+    def do_provisioning_action(self, context, node_id, action, topic=None):
+        """Signal to conductor service to perform the given action on a node.
+
+        :param context: request context.
+        :param node_id: node id or uuid.
+        :param action: an action. One of ironic.common.states.VERBS
+        :param topic: RPC topic. Defaults to self.topic.
+        :raises: InvalidParameterValue
+        :raises: NoFreeConductorWorker when there is no free worker to start
+                async task.
+        :raises: InvalidStateRequested if the requested action can not
+                 be performed.
+
+        This encapsulates some provisioning actions in a single call.
+        """
+        cctxt = self.client.prepare(topic=topic or self.topic, version='1.23')
+        return cctxt.call(context, 'do_provisioning_action',
+                          node_id=node_id, action=action)
+
     def validate_driver_interfaces(self, context, node_id, topic=None):
         """Validate the `core` and `standardized` interfaces for drivers.
 

ironic/tests/api/v1/test_nodes.py

@@ -1257,6 +1257,53 @@ class TestPut(test_api_base.FunctionalTest):
                             expect_errors=True)
         self.assertEqual(400, ret.status_code)
 
+    def test_manage_raises_error_before_1_2(self):
+        ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                            {'target': states.VERBS['manage']},
+                            headers={},
+                            expect_errors=True)
+        self.assertEqual(406, ret.status_code)
+
+    @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
+    def test_provide_from_manage(self, mock_dpa):
+        self.node.provision_state = states.MANAGEABLE
+        self.node.save()
+
+        ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                            {'target': states.VERBS['provide']},
+                            headers={api_base.Version.string: "1.4"})
+        self.assertEqual(202, ret.status_code)
+        self.assertEqual('', ret.body)
+        mock_dpa.assert_called_once_with(mock.ANY, self.node.uuid,
+                                         states.VERBS['provide'],
+                                         'test-topic')
+
+    @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
+    def test_manage_from_available(self, mock_dpa):
+        self.node.provision_state = states.AVAILABLE
+        self.node.save()
+
+        ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                            {'target': states.VERBS['manage']},
+                            headers={api_base.Version.string: "1.4"})
+        self.assertEqual(202, ret.status_code)
+        self.assertEqual('', ret.body)
+        mock_dpa.assert_called_once_with(mock.ANY, self.node.uuid,
+                                         states.VERBS['manage'],
+                                         'test-topic')
+
+    @mock.patch.object(rpcapi.ConductorAPI, 'do_provisioning_action')
+    def test_bad_requests_in_managed_state(self, mock_dpa):
+        self.node.provision_state = states.MANAGEABLE
+        self.node.save()
+
+        for state in [states.ACTIVE, states.REBUILD, states.DELETED]:
+            ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid,
+                                {'target': states.ACTIVE},
+                                expect_errors=True)
+            self.assertEqual(400, ret.status_code)
+        self.assertEqual(0, mock_dpa.call_count)
+
     def test_set_console_mode_enabled(self):
         with mock.patch.object(rpcapi.ConductorAPI,
                                'set_console_mode') as mock_scm:

ironic/tests/conductor/test_manager.py

@@ -925,7 +925,7 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
                                 self.service.do_node_deploy,
                                 self.context, node['uuid'])
         # Compare true exception hidden by @messaging.expected_exceptions
-        self.assertEqual(exception.InstanceDeployFailure, exc.exc_info[0])
+        self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0])
         # This is a sync operation last_error should be None.
         self.assertIsNone(node.last_error)
         # Verify reservation has been cleared.
@@ -1246,7 +1246,7 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
                                 self.service.do_node_deploy,
                                 self.context, node['uuid'], rebuild=True)
         # Compare true exception hidden by @messaging.expected_exceptions
-        self.assertEqual(exception.InstanceDeployFailure, exc.exc_info[0])
+        self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0])
         # Last_error should be None.
         self.assertIsNone(node.last_error)
         # Verify reservation has been cleared.
@@ -1304,7 +1304,7 @@ class DoNodeDeployTearDownTestCase(_ServiceSetUpMixin,
                                 self.service.do_node_tear_down,
                                 self.context, node['uuid'])
         # Compare true exception hidden by @messaging.expected_exceptions
-        self.assertEqual(exception.InstanceDeployFailure, exc.exc_info[0])
+        self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0])
 
     @mock.patch('ironic.drivers.modules.fake.FakePower.validate')
     def test_do_node_tear_down_validate_fail(self, mock_validate):