From edc37cbe1db539372a5e96296d7fbbebe9c50b2f Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Tue, 1 Dec 2015 17:22:48 +0000 Subject: [PATCH] API to manually clean nodes This adds an API to allow manual cleaning of nodes, via PUT /v1/nodes//states/provision. The argument 'target' is 'clean', and the argument 'clean_steps' (the list of clean steps to be performed on the node) must be specified. The API version is bumped to 1.15. Change-Id: I0e34407133684e34c4ab9446b3521a24f3038f92 Partial-Bug: #1526290 --- doc/source/webapi/v1.rst | 6 + ironic/api/controllers/v1/node.py | 96 ++++++++++++++- ironic/api/controllers/v1/versions.py | 4 +- ironic/tests/unit/api/v1/test_nodes.py | 116 ++++++++++++++++++ .../notes/manual-clean-4cc2437be1aea69a.yaml | 5 + 5 files changed, 224 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/manual-clean-4cc2437be1aea69a.yaml diff --git a/doc/source/webapi/v1.rst b/doc/source/webapi/v1.rst index 2fbff0e005..6be4810145 100644 --- a/doc/source/webapi/v1.rst +++ b/doc/source/webapi/v1.rst @@ -32,6 +32,12 @@ always requests the newest supported API version. API Versions History -------------------- +**1.15** + + Add ability to do manual cleaning when a node is in the manageable + provision state via PUT v1/nodes//states/provision, + target:clean, clean_steps:[...]. + **1.14** Make the following endpoints discoverable via Ironic API: diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 474dd826cb..90c4f0d651 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -37,6 +37,7 @@ from ironic.api import expose from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states as ir_states +from ironic.conductor import utils as conductor_utils from ironic import objects @@ -68,6 +69,7 @@ MIN_VERB_VERSIONS = { ir_states.VERBS['inspect']: versions.MINOR_6_INSPECT_STATE, ir_states.VERBS['abort']: versions.MINOR_13_ABORT_VERB, + ir_states.VERBS['clean']: versions.MINOR_15_MANUAL_CLEAN, } # States where calling do_provisioning_action makes sense @@ -399,8 +401,10 @@ class NodeStatesController(rest.RestController): pecan.response.location = link.build_url('nodes', url_args) @expose.expose(None, types.uuid_or_name, wtypes.text, - wtypes.text, status_code=http_client.ACCEPTED) - def provision(self, node_ident, target, configdrive=None): + wtypes.text, types.jsontype, + status_code=http_client.ACCEPTED) + def provision(self, node_ident, target, configdrive=None, + clean_steps=None): """Asynchronous trigger the provisioning of the node. This will set the target provision state of the node, and a @@ -415,11 +419,34 @@ class NodeStatesController(rest.RestController): :param configdrive: Optional. A gzipped and base64 encoded configdrive. Only valid when setting provision state to "active". + :param clean_steps: An ordered list of cleaning steps that will be + performed on the node. A cleaning step is a dictionary with + required keys 'interface' and 'step', and optional key 'args'. If + specified, the value for 'args' is a keyword variable argument + dictionary that is passed to the cleaning step method.:: + + { 'interface': , + 'step': , + 'args': {: , ..., : } } + + For example (this isn't a real example, this cleaning step + doesn't exist):: + + { 'interface': 'deploy', + 'step': 'upgrade_firmware', + 'args': {'force': True} } + + This is required (and only valid) when target is "clean". :raises: NodeLocked (HTTP 409) if the node is currently locked. :raises: ClientSideError (HTTP 409) if the node is already being provisioned. + :raises: InvalidParameterValue (HTTP 400), if validation of + clean_steps or power driver interface fails. :raises: InvalidStateRequested (HTTP 400) if the requested transition is not possible from the current state. + :raises: NodeInMaintenance (HTTP 400), if operation cannot be + performed because the node is in maintenance mode. + :raises: NoFreeConductorWorker (HTTP 503) if no workers are available. :raises: NotAcceptable (HTTP 406) if the API version specified does not allow the requested state transition. """ @@ -456,6 +483,12 @@ class NodeStatesController(rest.RestController): raise wsme.exc.ClientSideError( msg, status_code=http_client.BAD_REQUEST) + if clean_steps and target != ir_states.VERBS['clean']: + msg = (_('"clean_steps" is only valid when setting target ' + 'provision state to %s') % ir_states.VERBS['clean']) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + # Note that there is a race condition. The node state(s) could change # by the time the RPC call is made and the TaskManager manager gets a # lock. @@ -473,6 +506,15 @@ class NodeStatesController(rest.RestController): elif target == ir_states.VERBS['inspect']: pecan.request.rpcapi.inspect_hardware( pecan.request.context, rpc_node.uuid, topic=topic) + elif target == ir_states.VERBS['clean']: + if not clean_steps: + msg = (_('"clean_steps" is required when setting target ' + 'provision state to %s') % ir_states.VERBS['clean']) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + _check_clean_steps(clean_steps) + pecan.request.rpcapi.do_node_clean( + pecan.request.context, rpc_node.uuid, clean_steps, topic) elif target in PROVISION_ACTION_STATES: pecan.request.rpcapi.do_provisioning_action( pecan.request.context, rpc_node.uuid, target, topic) @@ -486,6 +528,56 @@ class NodeStatesController(rest.RestController): pecan.response.location = link.build_url('nodes', url_args) +def _check_clean_steps(clean_steps): + """Ensure all necessary keys are present and correct in clean steps. + + Check that the user-specified clean steps are in the expected format and + include the required information. + + :param clean_steps: a list of clean steps. For more details, see the + clean_steps parameter of :func:`NodeStatesController.provision`. + :raises: InvalidParameterValue if validation of clean steps fails. + """ + if not isinstance(clean_steps, list): + raise exception.InvalidParameterValue(_('clean_steps must be a ' + 'list of dictionaries.')) + all_errors = [] + interfaces = conductor_utils.CLEANING_INTERFACE_PRIORITY.keys() + for step in clean_steps: + if not isinstance(step, dict): + all_errors.append(_('Clean step must be a dictionary; invalid ' + 'step: %(step)s.') % {'step': step}) + continue + + errors = [] + unknown = set(step) - set(['interface', 'step', 'args']) + if unknown: + errors.append(_('Unrecognized keys %(keys)s') + % {'keys': ', '.join(unknown)}) + + for key in ['interface', 'step']: + if key not in step or not step[key]: + errors.append(_('Missing value for key "%(key)s"') + % {'key': key}) + elif key == 'interface': + if step[key] not in interfaces: + errors.append(_('"interface" value must be one of ' + '%(valid)s') + % {'valid': ', '.join(interfaces)}) + + args = step.get('args') + if args is not None and not isinstance(args, dict): + errors.append(_('"args" must be a dictionary')) + + if errors: + errors.append(_('invalid step: %(step)s.') % {'step': step}) + all_errors.append('; '.join(errors)) + + if all_errors: + raise exception.InvalidParameterValue( + _('Invalid clean_steps. %s') % ' '.join(all_errors)) + + class Node(base.APIBase): """API representation of a bare metal node. diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index a9367d3fff..82545d042f 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -44,6 +44,7 @@ BASE_VERSION = 1 # v1.14: Make the following endpoints discoverable via API: # 1. '/v1/nodes//states' # 2. '/v1/drivers//properties' +# v1.15: Add ability to do manual cleaning of nodes MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -60,11 +61,12 @@ MINOR_11_ENROLL_STATE = 11 MINOR_12_RAID_CONFIG = 12 MINOR_13_ABORT_VERB = 13 MINOR_14_LINKS_NODESTATES_DRIVERPROPERTIES = 14 +MINOR_15_MANUAL_CLEAN = 15 # When adding another version, update MINOR_MAX_VERSION and also update # doc/source/webapi/v1.rst with a detailed explanation of what the version has # changed. -MINOR_MAX_VERSION = MINOR_14_LINKS_NODESTATES_DRIVERPROPERTIES +MINOR_MAX_VERSION = MINOR_15_MANUAL_CLEAN # String representations of the minor and maximum versions MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/tests/unit/api/v1/test_nodes.py b/ironic/tests/unit/api/v1/test_nodes.py index 643a9a1548..46ea9c1dbe 100644 --- a/ironic/tests/unit/api/v1/test_nodes.py +++ b/ironic/tests/unit/api/v1/test_nodes.py @@ -2013,6 +2013,66 @@ class TestPut(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + def test_provision_with_cleansteps_not_clean(self): + self.node.provision_state = states.MANAGEABLE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['provide'], + 'clean_steps': 'foo'}, + headers={api_base.Version.string: "1.4"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + + def test_clean_unsupported(self): + self.node.provision_state = states.MANAGEABLE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['clean']}, + headers={api_base.Version.string: "1.14"}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, ret.status_code) + + def test_clean_no_cleansteps(self): + self.node.provision_state = states.MANAGEABLE + self.node.save() + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['clean']}, + headers={api_base.Version.string: "1.15"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + + @mock.patch.object(rpcapi.ConductorAPI, 'do_node_clean') + @mock.patch.object(api_node, '_check_clean_steps') + def test_clean_check_steps_fail(self, mock_check, mock_rpcapi): + self.node.provision_state = states.MANAGEABLE + self.node.save() + mock_check.side_effect = exception.InvalidParameterValue('bad') + clean_steps = [{"step": "upgrade_firmware", "interface": "deploy"}] + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['clean'], + 'clean_steps': clean_steps}, + headers={api_base.Version.string: "1.15"}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + mock_check.assert_called_once_with(clean_steps) + self.assertFalse(mock_rpcapi.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'do_node_clean') + @mock.patch.object(api_node, '_check_clean_steps') + def test_clean(self, mock_check, mock_rpcapi): + self.node.provision_state = states.MANAGEABLE + self.node.save() + clean_steps = [{"step": "upgrade_firmware", "interface": "deploy"}] + ret = self.put_json('/nodes/%s/states/provision' % self.node.uuid, + {'target': states.VERBS['clean'], + 'clean_steps': clean_steps}, + headers={api_base.Version.string: "1.15"}) + self.assertEqual(http_client.ACCEPTED, ret.status_code) + self.assertEqual(b'', ret.body) + mock_check.assert_called_once_with(clean_steps) + mock_rpcapi.assert_called_once_with(mock.ANY, self.node.uuid, + clean_steps, 'test-topic') + def test_set_console_mode_enabled(self): with mock.patch.object(rpcapi.ConductorAPI, 'set_console_mode') as mock_scm: @@ -2258,3 +2318,59 @@ class TestPut(test_api_base.BaseApiTest): mock_get): self._test_set_node_maintenance_mode(mock_update, mock_get, None, self.node.name, is_by_name=True) + + +class TestCheckCleanSteps(base.TestCase): + def test__check_clean_steps_not_list(self): + clean_steps = {"step": "upgrade_firmware", "interface": "deploy"} + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'list', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_not_dict(self): + clean_steps = ['clean step'] + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'dictionary', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_key_invalid(self): + clean_steps = [{"unknown": "upgrade_firmware", "interface": "deploy"}] + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'Unrecognized', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_missing_interface(self): + clean_steps = [{"step": "upgrade_firmware"}] + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'interface', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_missing_step_value(self): + clean_steps = [{"step": None, "interface": "deploy"}] + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'step', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_interface_value_invalid(self): + clean_steps = [{"step": "upgrade_firmware", "interface": "not"}] + self.assertRaisesRegexp(exception.InvalidParameterValue, + '"interface" value must be one of', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_step_args_value_invalid(self): + clean_steps = [{"step": "upgrade_firmware", "interface": "deploy", + "args": "invalid args"}] + self.assertRaisesRegexp(exception.InvalidParameterValue, + 'args', + api_node._check_clean_steps, clean_steps) + + def test__check_clean_steps_valid(self): + clean_steps = [{"step": "upgrade_firmware", "interface": "deploy"}] + api_node._check_clean_steps(clean_steps) + + step1 = {"step": "upgrade_firmware", "interface": "deploy", + "args": {"arg1": "value1", "arg2": "value2"}} + api_node._check_clean_steps([step1]) + + step2 = {"step": "configure raid", "interface": "raid"} + api_node._check_clean_steps([step1, step2]) diff --git a/releasenotes/notes/manual-clean-4cc2437be1aea69a.yaml b/releasenotes/notes/manual-clean-4cc2437be1aea69a.yaml new file mode 100644 index 0000000000..370c62ecc1 --- /dev/null +++ b/releasenotes/notes/manual-clean-4cc2437be1aea69a.yaml @@ -0,0 +1,5 @@ +--- +features: + - Adds support for manual cleaning. This is available with API + version 1.15. For more information, see + http://specs.openstack.org/openstack/ironic-specs/specs/approved/manual-cleaning.html