Support Ironic node names in our API

This change drops check on UUID validness from our API.
It also has a subtle effect of doing Ironic node fetching in
the introspection status and data fetch calls, which might make them
slightly longer (but only when name is used).

A new helper common.ironic.get_node is created to unify how we fetch nodes
from Ironic. It also provides nicer exceptions.

Change-Id: I20cf65e57910568b70a62c3f9269a962e78a07e2
Closes-Bug: #1523902
This commit is contained in:
Dmitry Tantsur 2016-02-04 17:21:53 +01:00 committed by dparalen
parent 86c4c15ed9
commit af6fbf0717
11 changed files with 128 additions and 64 deletions

View File

@ -343,3 +343,4 @@ Version History
* **1.2** endpoints for manipulating introspection rules. * **1.2** endpoints for manipulating introspection rules.
* **1.3** endpoint for canceling running introspection * **1.3** endpoint for canceling running introspection
* **1.4** endpoint for reapplying the introspection over stored data. * **1.4** endpoint for reapplying the introspection over stored data.
* **1.5** support for Ironic node names.

View File

@ -14,6 +14,7 @@
import socket import socket
from ironicclient import client from ironicclient import client
from ironicclient import exceptions as ironic_exc
from oslo_config import cfg from oslo_config import cfg
from ironic_inspector.common.i18n import _ 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(): def reset_ironic_session():
"""Reset the global session variable. """Reset the global session variable.
@ -200,5 +209,24 @@ def dict_to_capabilities(caps_dict):
if value is not None]) 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(): def list_opts():
return keystone.add_auth_options(IRONIC_OPTS, IRONIC_GROUP) return keystone.add_auth_options(IRONIC_OPTS, IRONIC_GROUP)

View File

@ -18,7 +18,6 @@ import string
import time import time
from eventlet import semaphore from eventlet import semaphore
from ironicclient import exceptions
from oslo_config import cfg from oslo_config import cfg
from ironic_inspector.common.i18n import _, _LI, _LW 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 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. """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 new_ipmi_credentials: tuple (new username, new password) or None
:param token: authentication token :param token: authentication token
:raises: Error :raises: Error
""" """
ironic = ir_utils.get_client(token) ironic = ir_utils.get_client(token)
node = ir_utils.get_node(node_id, ironic=ironic)
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})
ir_utils.check_provision_state(node, with_credentials=new_ipmi_credentials) 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) node_info=node_info)
def abort(uuid, token=None): def abort(node_id, token=None):
"""Abort running introspection. """Abort running introspection.
:param uuid: node uuid :param node_id: node UUID or name
:param token: authentication token :param token: authentication token
:raises: Error :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) 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 # check pending operations
locked = node_info.acquire_lock(blocking=False) locked = node_info.acquire_lock(blocking=False)

View File

@ -47,7 +47,7 @@ app = flask.Flask(__name__)
LOG = utils.getProcessingLogger(__name__) LOG = utils.getProcessingLogger(__name__)
MINIMUM_API_VERSION = (1, 0) MINIMUM_API_VERSION = (1, 0)
CURRENT_API_VERSION = (1, 4) CURRENT_API_VERSION = (1, 5)
_LOGGING_EXCLUDED_KEYS = ('logs',) _LOGGING_EXCLUDED_KEYS = ('logs',)
@ -178,14 +178,11 @@ def api_continue():
# TODO(sambetts) Add API discovery for this endpoint # TODO(sambetts) Add API discovery for this endpoint
@app.route('/v1/introspection/<uuid>', methods=['GET', 'POST']) @app.route('/v1/introspection/<node_id>', methods=['GET', 'POST'])
@convert_exceptions @convert_exceptions
def api_introspection(uuid): def api_introspection(node_id):
utils.check_auth(flask.request) 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': if flask.request.method == 'POST':
new_ipmi_password = flask.request.args.get('new_ipmi_password', new_ipmi_password = flask.request.args.get('new_ipmi_password',
type=str, type=str,
@ -198,34 +195,34 @@ def api_introspection(uuid):
else: else:
new_ipmi_credentials = None new_ipmi_credentials = None
introspect.introspect(uuid, introspect.introspect(node_id,
new_ipmi_credentials=new_ipmi_credentials, new_ipmi_credentials=new_ipmi_credentials,
token=flask.request.headers.get('X-Auth-Token')) token=flask.request.headers.get('X-Auth-Token'))
return '', 202 return '', 202
else: 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), return flask.json.jsonify(finished=bool(node_info.finished_at),
error=node_info.error or None) error=node_info.error or None)
@app.route('/v1/introspection/<uuid>/abort', methods=['POST']) @app.route('/v1/introspection/<node_id>/abort', methods=['POST'])
@convert_exceptions @convert_exceptions
def api_introspection_abort(uuid): def api_introspection_abort(node_id):
utils.check_auth(flask.request) utils.check_auth(flask.request)
introspect.abort(node_id, token=flask.request.headers.get('X-Auth-Token'))
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'))
return '', 202 return '', 202
@app.route('/v1/introspection/<uuid>/data', methods=['GET']) @app.route('/v1/introspection/<node_id>/data', methods=['GET'])
@convert_exceptions @convert_exceptions
def api_introspection_data(uuid): def api_introspection_data(node_id):
utils.check_auth(flask.request) utils.check_auth(flask.request)
if CONF.processing.store_data == 'swift': 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'} return res, 200, {'Content-Type': 'application/json'}
else: else:
return error_response(_('Inspector is not configured to store data. ' return error_response(_('Inspector is not configured to store data. '
@ -234,9 +231,9 @@ def api_introspection_data(uuid):
code=404) code=404)
@app.route('/v1/introspection/<uuid>/data/unprocessed', methods=['POST']) @app.route('/v1/introspection/<node_id>/data/unprocessed', methods=['POST'])
@convert_exceptions @convert_exceptions
def api_introspection_reapply(uuid): def api_introspection_reapply(node_id):
utils.check_auth(flask.request) utils.check_auth(flask.request)
if flask.request.content_length: if flask.request.content_length:
@ -244,7 +241,7 @@ def api_introspection_reapply(uuid):
'supported yet'), code=400) 'supported yet'), code=400)
if CONF.processing.store_data == 'swift': if CONF.processing.store_data == 'swift':
process.reapply(uuid) process.reapply(node_id)
return '', 202 return '', 202
else: else:
return error_response(_('Inspector is not configured to store' return error_response(_('Inspector is not configured to store'

View File

@ -22,6 +22,7 @@ from oslo_concurrency import lockutils
from oslo_config import cfg from oslo_config import cfg
from oslo_db import exception as db_exc from oslo_db import exception as db_exc
from oslo_utils import excutils from oslo_utils import excutils
from oslo_utils import uuidutils
from sqlalchemy import text from sqlalchemy import text
from ironic_inspector import db from ironic_inspector import db
@ -201,11 +202,11 @@ class NodeInfo(object):
self._attributes = None self._attributes = None
@classmethod @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.""" """Construct NodeInfo from a database row."""
fields = {key: row[key] fields = {key: row[key]
for key in ('uuid', 'started_at', 'finished_at', 'error')} 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): def invalidate_cache(self):
"""Clear all cached info, so that it's reloaded next time.""" """Clear all cached info, so that it's reloaded next time."""
@ -218,7 +219,7 @@ class NodeInfo(object):
def node(self): def node(self):
"""Get Ironic node object associated with the cached node record.""" """Get Ironic node object associated with the cached node record."""
if self._node is None: 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 return self._node
def create_ports(self, macs): 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)} return {x.uuid for x in db.model_query(db.Node.uuid)}
def get_node(uuid, ironic=None, locked=False): def get_node(node_id, ironic=None, locked=False):
"""Get node from cache by it's UUID. """Get node from cache.
:param uuid: node UUID. :param node_id: node UUID or name.
:param ironic: optional ironic client instance :param ironic: optional ironic client instance
:param locked: if True, get a lock on node before fetching its data :param locked: if True, get a lock on node before fetching its data
:returns: structure NodeInfo. :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: if locked:
lock = _get_lock(uuid) lock = _get_lock(uuid)
lock.acquire() lock.acquire()
@ -457,7 +465,7 @@ def get_node(uuid, ironic=None, locked=False):
if row is None: if row is None:
raise utils.Error(_('Could not find node %s in cache') % uuid, raise utils.Error(_('Could not find node %s in cache') % uuid,
code=404) code=404)
return NodeInfo.from_row(row, ironic=ironic, lock=lock) return NodeInfo.from_row(row, ironic=ironic, lock=lock, node=node)
except Exception: except Exception:
with excutils.save_and_reraise_exception(): with excutils.save_and_reraise_exception():
if lock is not None: if lock is not None:

View File

@ -21,7 +21,6 @@ import os
import eventlet import eventlet
import json import json
from ironicclient import exceptions
from oslo_config import cfg from oslo_config import cfg
from oslo_utils import excutils from oslo_utils import excutils
@ -217,12 +216,10 @@ def process(introspection_data):
try: try:
node = node_info.node() node = node_info.node()
except exceptions.NotFound: except ir_utils.NotFound as exc:
msg = _('Node was found in cache, but is not found in Ironic') with excutils.save_and_reraise_exception():
node_info.finished(error=msg) node_info.finished(error=str(exc))
_store_logs(introspection_data, node_info) _store_logs(introspection_data, node_info)
raise utils.Error(msg, code=404, node_info=node_info,
data=introspection_data)
try: try:
result = _process_node(node, introspection_data, node_info) 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) node_info=node_info, data=introspection_data)
def reapply(uuid): def reapply(node_ident):
"""Re-apply introspection steps. """Re-apply introspection steps.
Re-apply preprocessing, postprocessing and introspection rules on Re-apply preprocessing, postprocessing and introspection rules on
stored data. stored data.
:param uuid: node uuid to use :param node_ident: node UUID or name
:raises: utils.Error :raises: utils.Error
""" """
LOG.debug('Processing re-apply introspection request for node ' LOG.debug('Processing re-apply introspection request for node '
'UUID: %s', uuid) 'UUID: %s', node_ident)
node_info = node_cache.get_node(uuid, locked=False) node_info = node_cache.get_node(node_ident, locked=False)
if not node_info.acquire_lock(blocking=False): if not node_info.acquire_lock(blocking=False):
# Note (mkovacik): it should be sufficient to check data # Note (mkovacik): it should be sufficient to check data
# presence & locking. If either introspection didn't start # presence & locking. If either introspection didn't start

View File

@ -189,12 +189,12 @@ class TestIntrospect(BaseTest):
cli = client_mock.return_value cli = client_mock.return_value
cli.node.get.side_effect = exceptions.NotFound() cli.node.get.side_effect = exceptions.NotFound()
self.assertRaisesRegexp(utils.Error, self.assertRaisesRegexp(utils.Error,
'Cannot find node', 'Node %s was not found' % self.uuid,
introspect.introspect, self.uuid) introspect.introspect, self.uuid)
cli.node.get.side_effect = exceptions.BadRequest() cli.node.get.side_effect = exceptions.BadRequest()
self.assertRaisesRegexp(utils.Error, self.assertRaisesRegexp(utils.Error,
'Cannot get node', '%s: Bad Request' % self.uuid,
introspect.introspect, self.uuid) introspect.introspect, self.uuid)
self.assertEqual(0, self.node_info.ports.call_count) self.assertEqual(0, self.node_info.ports.call_count)

View File

@ -104,12 +104,6 @@ class TestApiIntrospect(BaseAPITest):
self.assertEqual(403, res.status_code) self.assertEqual(403, res.status_code)
self.assertFalse(introspect_mock.called) 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) @mock.patch.object(process, 'process', autospec=True)
class TestApiContinue(BaseAPITest): class TestApiContinue(BaseAPITest):
@ -233,6 +227,30 @@ class TestApiGetData(BaseAPITest):
self.assertFalse(swift_conn.get_object.called) self.assertFalse(swift_conn.get_object.called)
self.assertEqual(404, res.status_code) 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) @mock.patch.object(process, 'reapply', autospec=True)
class TestApiReapply(BaseAPITest): class TestApiReapply(BaseAPITest):

View File

@ -336,7 +336,25 @@ class TestNodeCacheGetNode(test_base.NodeTest):
self.assertTrue(info._locked) self.assertTrue(info._locked)
def test_not_found(self): 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) @mock.patch.object(time, 'time', lambda: 42.0)

View File

@ -130,7 +130,7 @@ class TestProcess(BaseProcessTest):
self.cli.node.get.side_effect = exceptions.NotFound() self.cli.node.get.side_effect = exceptions.NotFound()
self.assertRaisesRegexp(utils.Error, self.assertRaisesRegexp(utils.Error,
'not found', 'Node %s was not found' % self.uuid,
process.process, self.data) process.process, self.data)
self.cli.node.get.assert_called_once_with(self.uuid) self.cli.node.get.assert_called_once_with(self.uuid)
self.assertFalse(self.process_mock.called) self.assertFalse(self.process_mock.called)

View File

@ -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.