diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index 88d472bcb..103247ac8 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -343,3 +343,4 @@ Version History * **1.2** endpoints for manipulating introspection rules. * **1.3** endpoint for canceling running introspection * **1.4** endpoint for reapplying the introspection over stored data. +* **1.5** support for Ironic node names. diff --git a/ironic_inspector/common/ironic.py b/ironic_inspector/common/ironic.py index 131be0010..d220a3624 100644 --- a/ironic_inspector/common/ironic.py +++ b/ironic_inspector/common/ironic.py @@ -14,6 +14,7 @@ import socket from ironicclient import client +from ironicclient import exceptions as ironic_exc from oslo_config import cfg from ironic_inspector.common.i18n import _ @@ -116,6 +117,14 @@ LEGACY_MAP = { } +class NotFound(utils.Error): + """Node not found in Ironic.""" + + def __init__(self, node_ident, code=404, *args, **kwargs): + msg = _('Node %s was not found in Ironic') % node_ident + super(NotFound, self).__init__(msg, code, *args, **kwargs) + + def reset_ironic_session(): """Reset the global session variable. @@ -200,5 +209,24 @@ def dict_to_capabilities(caps_dict): if value is not None]) +def get_node(node_id, ironic=None, **kwargs): + """Get a node from Ironic. + + :param node_id: node UUID or name. + :param ironic: ironic client instance. + :param kwargs: arguments to pass to Ironic client. + :raises: Error on failure + """ + ironic = ironic if ironic is not None else get_client() + + try: + return ironic.node.get(node_id, **kwargs) + except ironic_exc.NotFound: + raise NotFound(node_id) + except ironic_exc.HttpError as exc: + raise utils.Error(_("Cannot get node %(node)s: %(exc)s") % + {'node': node_id, 'exc': exc}) + + def list_opts(): return keystone.add_auth_options(IRONIC_OPTS, IRONIC_GROUP) diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index 95632c99e..f819c8f3c 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -18,7 +18,6 @@ import string import time from eventlet import semaphore -from ironicclient import exceptions from oslo_config import cfg from ironic_inspector.common.i18n import _, _LI, _LW @@ -64,23 +63,16 @@ def _validate_ipmi_credentials(node, new_ipmi_credentials): return new_username, new_password -def introspect(uuid, new_ipmi_credentials=None, token=None): +def introspect(node_id, new_ipmi_credentials=None, token=None): """Initiate hardware properties introspection for a given node. - :param uuid: node uuid + :param node_id: node UUID or name :param new_ipmi_credentials: tuple (new username, new password) or None :param token: authentication token :raises: Error """ ironic = ir_utils.get_client(token) - - try: - node = ironic.node.get(uuid) - except exceptions.NotFound: - raise utils.Error(_("Cannot find node %s") % uuid, code=404) - except exceptions.HttpError as exc: - raise utils.Error(_("Cannot get node %(node)s: %(exc)s") % - {'node': uuid, 'exc': exc}) + node = ir_utils.get_node(node_id, ironic=ironic) ir_utils.check_provision_state(node, with_credentials=new_ipmi_credentials) @@ -179,16 +171,16 @@ def _background_introspect_locked(ironic, node_info): node_info=node_info) -def abort(uuid, token=None): +def abort(node_id, token=None): """Abort running introspection. - :param uuid: node uuid + :param node_id: node UUID or name :param token: authentication token :raises: Error """ - LOG.debug('Aborting introspection for node %s', uuid) + LOG.debug('Aborting introspection for node %s', node_id) ironic = ir_utils.get_client(token) - node_info = node_cache.get_node(uuid, ironic=ironic, locked=False) + node_info = node_cache.get_node(node_id, ironic=ironic, locked=False) # check pending operations locked = node_info.acquire_lock(blocking=False) diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index fb64f6b24..de913fff2 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -47,7 +47,7 @@ app = flask.Flask(__name__) LOG = utils.getProcessingLogger(__name__) MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 4) +CURRENT_API_VERSION = (1, 5) _LOGGING_EXCLUDED_KEYS = ('logs',) @@ -178,14 +178,11 @@ def api_continue(): # TODO(sambetts) Add API discovery for this endpoint -@app.route('/v1/introspection/', methods=['GET', 'POST']) +@app.route('/v1/introspection/', methods=['GET', 'POST']) @convert_exceptions -def api_introspection(uuid): +def api_introspection(node_id): utils.check_auth(flask.request) - if not uuidutils.is_uuid_like(uuid): - raise utils.Error(_('Invalid UUID value'), code=400) - if flask.request.method == 'POST': new_ipmi_password = flask.request.args.get('new_ipmi_password', type=str, @@ -198,34 +195,34 @@ def api_introspection(uuid): else: new_ipmi_credentials = None - introspect.introspect(uuid, + introspect.introspect(node_id, new_ipmi_credentials=new_ipmi_credentials, token=flask.request.headers.get('X-Auth-Token')) return '', 202 else: - node_info = node_cache.get_node(uuid) + node_info = node_cache.get_node(node_id) return flask.json.jsonify(finished=bool(node_info.finished_at), error=node_info.error or None) -@app.route('/v1/introspection//abort', methods=['POST']) +@app.route('/v1/introspection//abort', methods=['POST']) @convert_exceptions -def api_introspection_abort(uuid): +def api_introspection_abort(node_id): utils.check_auth(flask.request) - - if not uuidutils.is_uuid_like(uuid): - raise utils.Error(_('Invalid UUID value'), code=400) - - introspect.abort(uuid, token=flask.request.headers.get('X-Auth-Token')) + introspect.abort(node_id, token=flask.request.headers.get('X-Auth-Token')) return '', 202 -@app.route('/v1/introspection//data', methods=['GET']) +@app.route('/v1/introspection//data', methods=['GET']) @convert_exceptions -def api_introspection_data(uuid): +def api_introspection_data(node_id): utils.check_auth(flask.request) + if CONF.processing.store_data == 'swift': - res = swift.get_introspection_data(uuid) + if not uuidutils.is_uuid_like(node_id): + node = ir_utils.get_node(node_id, fields=['uuid']) + node_id = node.uuid + res = swift.get_introspection_data(node_id) return res, 200, {'Content-Type': 'application/json'} else: return error_response(_('Inspector is not configured to store data. ' @@ -234,9 +231,9 @@ def api_introspection_data(uuid): code=404) -@app.route('/v1/introspection//data/unprocessed', methods=['POST']) +@app.route('/v1/introspection//data/unprocessed', methods=['POST']) @convert_exceptions -def api_introspection_reapply(uuid): +def api_introspection_reapply(node_id): utils.check_auth(flask.request) if flask.request.content_length: @@ -244,7 +241,7 @@ def api_introspection_reapply(uuid): 'supported yet'), code=400) if CONF.processing.store_data == 'swift': - process.reapply(uuid) + process.reapply(node_id) return '', 202 else: return error_response(_('Inspector is not configured to store' diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 67d02dc7f..e06a08402 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -22,6 +22,7 @@ from oslo_concurrency import lockutils from oslo_config import cfg from oslo_db import exception as db_exc from oslo_utils import excutils +from oslo_utils import uuidutils from sqlalchemy import text from ironic_inspector import db @@ -201,11 +202,11 @@ class NodeInfo(object): self._attributes = None @classmethod - def from_row(cls, row, ironic=None, lock=None): + def from_row(cls, row, ironic=None, lock=None, node=None): """Construct NodeInfo from a database row.""" fields = {key: row[key] for key in ('uuid', 'started_at', 'finished_at', 'error')} - return cls(ironic=ironic, lock=lock, **fields) + return cls(ironic=ironic, lock=lock, node=node, **fields) def invalidate_cache(self): """Clear all cached info, so that it's reloaded next time.""" @@ -218,7 +219,7 @@ class NodeInfo(object): def node(self): """Get Ironic node object associated with the cached node record.""" if self._node is None: - self._node = self.ironic.node.get(self.uuid) + self._node = ir_utils.get_node(self.uuid, ironic=self.ironic) return self._node def create_ports(self, macs): @@ -438,14 +439,21 @@ def _list_node_uuids(): return {x.uuid for x in db.model_query(db.Node.uuid)} -def get_node(uuid, ironic=None, locked=False): - """Get node from cache by it's UUID. +def get_node(node_id, ironic=None, locked=False): + """Get node from cache. - :param uuid: node UUID. + :param node_id: node UUID or name. :param ironic: optional ironic client instance :param locked: if True, get a lock on node before fetching its data :returns: structure NodeInfo. """ + if uuidutils.is_uuid_like(node_id): + node = None + uuid = node_id + else: + node = ir_utils.get_node(node_id, ironic=ironic) + uuid = node.uuid + if locked: lock = _get_lock(uuid) lock.acquire() @@ -457,7 +465,7 @@ def get_node(uuid, ironic=None, locked=False): if row is None: raise utils.Error(_('Could not find node %s in cache') % uuid, code=404) - return NodeInfo.from_row(row, ironic=ironic, lock=lock) + return NodeInfo.from_row(row, ironic=ironic, lock=lock, node=node) except Exception: with excutils.save_and_reraise_exception(): if lock is not None: diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index e4ae86a9b..0cf267b9f 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -21,7 +21,6 @@ import os import eventlet import json -from ironicclient import exceptions from oslo_config import cfg from oslo_utils import excutils @@ -217,12 +216,10 @@ def process(introspection_data): try: node = node_info.node() - except exceptions.NotFound: - msg = _('Node was found in cache, but is not found in Ironic') - node_info.finished(error=msg) - _store_logs(introspection_data, node_info) - raise utils.Error(msg, code=404, node_info=node_info, - data=introspection_data) + except ir_utils.NotFound as exc: + with excutils.save_and_reraise_exception(): + node_info.finished(error=str(exc)) + _store_logs(introspection_data, node_info) try: result = _process_node(node, introspection_data, node_info) @@ -343,20 +340,20 @@ def _finish(ironic, node_info, introspection_data, power_off=True): node_info=node_info, data=introspection_data) -def reapply(uuid): +def reapply(node_ident): """Re-apply introspection steps. Re-apply preprocessing, postprocessing and introspection rules on stored data. - :param uuid: node uuid to use + :param node_ident: node UUID or name :raises: utils.Error """ LOG.debug('Processing re-apply introspection request for node ' - 'UUID: %s', uuid) - node_info = node_cache.get_node(uuid, locked=False) + 'UUID: %s', node_ident) + node_info = node_cache.get_node(node_ident, locked=False) if not node_info.acquire_lock(blocking=False): # Note (mkovacik): it should be sufficient to check data # presence & locking. If either introspection didn't start diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py index f7a0c25b6..3a093fa73 100644 --- a/ironic_inspector/test/unit/test_introspect.py +++ b/ironic_inspector/test/unit/test_introspect.py @@ -189,12 +189,12 @@ class TestIntrospect(BaseTest): cli = client_mock.return_value cli.node.get.side_effect = exceptions.NotFound() self.assertRaisesRegexp(utils.Error, - 'Cannot find node', + 'Node %s was not found' % self.uuid, introspect.introspect, self.uuid) cli.node.get.side_effect = exceptions.BadRequest() self.assertRaisesRegexp(utils.Error, - 'Cannot get node', + '%s: Bad Request' % self.uuid, introspect.introspect, self.uuid) self.assertEqual(0, self.node_info.ports.call_count) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index ec3b825fe..d7b727d87 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -104,12 +104,6 @@ class TestApiIntrospect(BaseAPITest): self.assertEqual(403, res.status_code) self.assertFalse(introspect_mock.called) - @mock.patch.object(introspect, 'introspect', autospec=True) - def test_introspect_invalid_uuid(self, introspect_mock): - uuid_dummy = 'invalid-uuid' - res = self.app.post('/v1/introspection/%s' % uuid_dummy) - self.assertEqual(400, res.status_code) - @mock.patch.object(process, 'process', autospec=True) class TestApiContinue(BaseAPITest): @@ -233,6 +227,30 @@ class TestApiGetData(BaseAPITest): self.assertFalse(swift_conn.get_object.called) self.assertEqual(404, res.status_code) + @mock.patch.object(ir_utils, 'get_node', autospec=True) + @mock.patch.object(main.swift, 'SwiftAPI', autospec=True) + def test_with_name(self, swift_mock, get_mock): + get_mock.return_value = mock.Mock(uuid=self.uuid) + CONF.set_override('store_data', 'swift', 'processing') + data = { + 'ipmi_address': '1.2.3.4', + 'cpus': 2, + 'cpu_arch': 'x86_64', + 'memory_mb': 1024, + 'local_gb': 20, + 'interfaces': { + 'em1': {'mac': '11:22:33:44:55:66', 'ip': '1.2.0.1'}, + } + } + swift_conn = swift_mock.return_value + swift_conn.get_object.return_value = json.dumps(data) + res = self.app.get('/v1/introspection/name1/data') + name = 'inspector_data-%s' % self.uuid + swift_conn.get_object.assert_called_once_with(name) + self.assertEqual(200, res.status_code) + self.assertEqual(data, json.loads(res.data.decode('utf-8'))) + get_mock.assert_called_once_with('name1', fields=['uuid']) + @mock.patch.object(process, 'reapply', autospec=True) class TestApiReapply(BaseAPITest): diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index 0c101340b..bf481834b 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -336,7 +336,25 @@ class TestNodeCacheGetNode(test_base.NodeTest): self.assertTrue(info._locked) def test_not_found(self): - self.assertRaises(utils.Error, node_cache.get_node, 'foo') + self.assertRaises(utils.Error, node_cache.get_node, + uuidutils.generate_uuid()) + + def test_with_name(self): + started_at = time.time() - 42 + session = db.get_session() + with session.begin(): + db.Node(uuid=self.uuid, started_at=started_at).save(session) + ironic = mock.Mock() + ironic.node.get.return_value = self.node + + info = node_cache.get_node('name', ironic=ironic) + + self.assertEqual(self.uuid, info.uuid) + self.assertEqual(started_at, info.started_at) + self.assertIsNone(info.finished_at) + self.assertIsNone(info.error) + self.assertFalse(info._locked) + ironic.node.get.assert_called_once_with('name') @mock.patch.object(time, 'time', lambda: 42.0) diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index 79ef8c6d1..0e948aeb7 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -130,7 +130,7 @@ class TestProcess(BaseProcessTest): self.cli.node.get.side_effect = exceptions.NotFound() self.assertRaisesRegexp(utils.Error, - 'not found', + 'Node %s was not found' % self.uuid, process.process, self.data) self.cli.node.get.assert_called_once_with(self.uuid) self.assertFalse(self.process_mock.called) diff --git a/releasenotes/notes/names-82d9f84153a228ec.yaml b/releasenotes/notes/names-82d9f84153a228ec.yaml new file mode 100644 index 000000000..ffcf468e1 --- /dev/null +++ b/releasenotes/notes/names-82d9f84153a228ec.yaml @@ -0,0 +1,5 @@ +--- +features: + - Add support for using Ironic node names in API instead of UUIDs. + Note that using node names in the introspection status API will require + a call to Ironic to be made by the service.