From 7cb40d5fec9e78b68223d5efdbc4bee30b02c3ee Mon Sep 17 00:00:00 2001 From: dparalen Date: Tue, 1 Nov 2016 10:52:31 +0100 Subject: [PATCH] Add API for listing all introspection statuses This patch introduces an API endpoint to list introspection statuses. The endpoint supports pagination with an uuid-marker and a limit query string fields. Due to the pagination, this change introduces a new configuration option: ``api_max_limit``. APIImpact Change-Id: I74d02698801d5290619161b2d8d7181ab51a0a5e Partial-Bug: #1525238 --- doc/source/http-api.rst | 44 ++++++ example.conf | 5 + ironic_inspector/api_tools.py | 83 +++++++++++ ironic_inspector/conf.py | 2 + ironic_inspector/main.py | 19 ++- ironic_inspector/node_cache.py | 27 ++++ ironic_inspector/test/functional.py | 76 ++++++++++ ironic_inspector/test/unit/test_api_tools.py | 136 ++++++++++++++++++ ironic_inspector/test/unit/test_main.py | 27 ++++ ironic_inspector/test/unit/test_node_cache.py | 33 +++++ ...trospection-statuses-2a3d4379c3854894.yaml | 9 ++ 11 files changed, 460 insertions(+), 1 deletion(-) create mode 100644 ironic_inspector/api_tools.py create mode 100644 ironic_inspector/test/unit/test_api_tools.py create mode 100644 releasenotes/notes/add-support-for-listing-all-introspection-statuses-2a3d4379c3854894.yaml diff --git a/doc/source/http-api.rst b/doc/source/http-api.rst index b5c386478..c95ce7c5f 100644 --- a/doc/source/http-api.rst +++ b/doc/source/http-api.rst @@ -56,6 +56,49 @@ Response body: JSON dictionary with keys: * ``uuid`` node UUID * ``started_at`` a UTC ISO8601 timestamp * ``finished_at`` a UTC ISO8601 timestamp or ``null`` +* ``links`` containing a self URL + +Get All Introspection Statuses +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +``GET /v1/introspection`` get all hardware introspection statuses. + +Requires X-Auth-Token header with Keystone token for authentication. + +Returned status list is sorted by the ``started_at, uuid`` attribute pair, +newer items first, and is paginated with these query string fields: + +* ``marker`` the UUID of the last node returned previously +* ``limit`` default, max: ``CONF.api_max_limit`` + +Response: + +* 200 - OK +* 400 - bad request +* 401, 403 - missing or invalid authentication + +Response body: a JSON object containing a list of status objects:: + + { + 'introspection': [ + { + 'finished': true, + 'error': null, + ... + }, + ... + ] + } + +Each status object contains these keys: + +* ``finished`` (boolean) whether introspection is finished + (``true`` on introspection completion or if it ends because of an error) +* ``error`` error string or ``null``; ``Canceled by operator`` in + case introspection was aborted +* ``uuid`` node UUID +* ``started_at`` an UTC ISO8601 timestamp +* ``finished_at`` an UTC ISO8601 timestamp or ``null`` Abort Running Introspection @@ -338,3 +381,4 @@ Version History * **1.5** support for Ironic node names. * **1.6** endpoint for rules creating returns 201 instead of 200 on success. * **1.7** UUID, started_at, finished_at in the introspection status API. +* **1.8** support for listing all introspection statuses. diff --git a/example.conf b/example.conf index c2d8a6b00..8fff7e519 100644 --- a/example.conf +++ b/example.conf @@ -70,6 +70,11 @@ # as root (string value) #rootwrap_config = /etc/ironic-inspector/rootwrap.conf +# Limit the number of elements an API list-call returns (integer +# value) +# Minimum value: 1 +#api_max_limit = 1000 + # # From oslo.log # diff --git a/ironic_inspector/api_tools.py b/ironic_inspector/api_tools.py new file mode 100644 index 000000000..a8aa755c3 --- /dev/null +++ b/ironic_inspector/api_tools.py @@ -0,0 +1,83 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Generic Rest Api tools.""" + +import flask +from oslo_config import cfg +from oslo_utils import uuidutils +import six + +from ironic_inspector.common.i18n import _ +from ironic_inspector import utils + +CONF = cfg.CONF + + +def raises_coercion_exceptions(fn): + """Convert coercion function exceptions to utils.Error. + + :raises: utils.Error when the coercion function raises an + AssertionError or a ValueError + """ + @six.wraps(fn) + def inner(*args, **kwargs): + try: + ret = fn(*args, **kwargs) + except (AssertionError, ValueError) as exc: + raise utils.Error(_('Bad request: %s') % exc, code=400) + return ret + return inner + + +def request_field(field_name): + """Decorate a function that coerces the specified field. + + :param field_name: name of the field to fetch + :returns: a decorator + """ + def outer(fn): + @six.wraps(fn) + def inner(*args, **kwargs): + default = kwargs.pop('default', None) + field = flask.request.args.get(field_name, default=default) + if field == default: + # field not found or the same as the default, just return + return default + return fn(field, *args, **kwargs) + return inner + return outer + + +@request_field('marker') +@raises_coercion_exceptions +def marker_field(value): + """Fetch the pagination marker field from flask.request.args. + + :returns: an uuid + """ + assert uuidutils.is_uuid_like(value), _('Marker not UUID-like') + return value + + +@request_field('limit') +@raises_coercion_exceptions +def limit_field(value): + """Fetch the pagination limit field from flask.request.args. + + :returns: the limit + """ + # limit of zero means the default limit + value = int(value) or CONF.api_max_limit + assert value >= 0, _('Limit cannot be negative') + assert value <= CONF.api_max_limit, _('Limit over %s') % CONF.api_max_limit + return value diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index 0ebc95a18..bde68ba12 100644 --- a/ironic_inspector/conf.py +++ b/ironic_inspector/conf.py @@ -215,6 +215,8 @@ SERVICE_OPTS = [ default="/etc/ironic-inspector/rootwrap.conf", help=_('Path to the rootwrap configuration file to use for ' 'running commands as root')), + cfg.IntOpt('api_max_limit', default=1000, min=1, + help=_('Limit the number of elements an API list-call returns')) ] diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index ccc49e670..fe3906458 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -27,6 +27,7 @@ from oslo_log import log from oslo_utils import uuidutils import werkzeug +from ironic_inspector import api_tools from ironic_inspector import db from ironic_inspector.common.i18n import _, _LC, _LE, _LI, _LW from ironic_inspector.common import ironic as ir_utils @@ -47,7 +48,7 @@ app = flask.Flask(__name__) LOG = utils.getProcessingLogger(__name__) MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 7) +CURRENT_API_VERSION = (1, 8) _LOGGING_EXCLUDED_KEYS = ('logs',) @@ -226,6 +227,22 @@ def api_introspection(node_id): return flask.json.jsonify(generate_introspection_status(node_info)) +@app.route('/v1/introspection', methods=['GET']) +@convert_exceptions +def api_introspection_statuses(): + utils.check_auth(flask.request) + + nodes = node_cache.get_node_list( + marker=api_tools.marker_field(), + limit=api_tools.limit_field(default=CONF.api_max_limit) + ) + data = { + 'introspection': [generate_introspection_status(node) + for node in nodes] + } + return flask.json.jsonify(data) + + @app.route('/v1/introspection//abort', methods=['POST']) @convert_exceptions def api_introspection_abort(node_id): diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 361afc1f4..09f5d10f6 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -21,6 +21,7 @@ from ironicclient import exceptions from oslo_concurrency import lockutils from oslo_config import cfg from oslo_db import exception as db_exc +from oslo_db.sqlalchemy import utils as db_utils from oslo_utils import excutils from oslo_utils import uuidutils from sqlalchemy import text @@ -624,3 +625,29 @@ def create_node(driver, ironic=None, **attributes): else: LOG.info(_LI('Node %s was created successfully'), node.uuid) return add_node(node.uuid, ironic=ironic) + + +def get_node_list(ironic=None, marker=None, limit=None): + """Get node list from the cache. + + The list of the nodes is ordered based on the (started_at, uuid) + attribute pair, newer items first. + + :param ironic: optional ironic client instance + :param marker: pagination marker (an UUID or None) + :param limit: pagination limit; None for default CONF.api_max_limit + :returns: a list of NodeInfo instances. + """ + if marker is not None: + # uuid marker -> row marker for pagination + marker = db.model_query(db.Node).get(marker) + if marker is None: + raise utils.Error(_('Node not found for marker: %s') % marker, + code=404) + + rows = db.model_query(db.Node) + # ordered based on (started_at, uuid); newer first + rows = db_utils.paginate_query(rows, db.Node, limit, + ('started_at', 'uuid'), + marker=marker, sort_dir='desc') + return [NodeInfo.from_row(row, ironic=ironic) for row in rows] diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index f44c279f7..dcb92c8ac 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -23,6 +23,8 @@ import json import os import pytz import shutil +import six +from six.moves import urllib import tempfile import unittest @@ -34,6 +36,7 @@ import requests from ironic_inspector.common import ironic as ir_utils from ironic_inspector.common import swift +from ironic_inspector import db from ironic_inspector import dbsync from ironic_inspector import main from ironic_inspector import rules @@ -78,6 +81,24 @@ def get_error(response): return response.json()['error']['message'] +def _query_string(*field_names): + def outer(func): + @six.wraps(func) + def inner(*args, **kwargs): + queries = [] + for field_name in field_names: + field = kwargs.pop(field_name, None) + if field is not None: + queries.append('%s=%s' % (field_name, field)) + + query_string = '&'.join(queries) + if query_string: + query_string = '?' + query_string + return func(*args, query_string=query_string, **kwargs) + return inner + return outer + + class Base(base.NodeTest): ROOT_URL = 'http://127.0.0.1:5050' IS_FUNCTIONAL = True @@ -143,6 +164,11 @@ class Base(base.NodeTest): def call_get_status(self, uuid, **kwargs): return self.call('get', '/v1/introspection/%s' % uuid, **kwargs).json() + @_query_string('marker', 'limit') + def call_get_statuses(self, query_string='', **kwargs): + path = '/v1/introspection' + return self.call('get', path + query_string, **kwargs).json() + def call_abort_introspect(self, uuid, **kwargs): return self.call('post', '/v1/introspection/%s/abort' % uuid, **kwargs) @@ -248,6 +274,56 @@ class Test(Base): status = self.call_get_status(self.uuid) self.check_status(status, finished=True) + def test_introspection_statuses(self): + self.call_introspect(self.uuid) + eventlet.greenthread.sleep(DEFAULT_SLEEP) + + # NOTE(zhenguo): only test finished=False here, as we don't know + # other nodes status in this thread. + statuses = self.call_get_statuses().get('introspection') + self.assertIn(self._fake_status(finished=False), statuses) + + # check we've got 1 status with a limit of 1 + statuses = self.call_get_statuses(limit=1).get('introspection') + self.assertEqual(1, len(statuses)) + + all_statuses = self.call_get_statuses().get('introspection') + marker_statuses = self.call_get_statuses( + marker=self.uuid, limit=1).get('introspection') + marker_index = all_statuses.index(self.call_get_status(self.uuid)) + # marker is the last row on previous page + self.assertEqual(all_statuses[marker_index+1:marker_index+2], + marker_statuses) + + self.call_continue(self.data) + eventlet.greenthread.sleep(DEFAULT_SLEEP) + + status = self.call_get_status(self.uuid) + self.check_status(status, finished=True) + + # fetch all statuses and db nodes to assert pagination + statuses = self.call_get_statuses().get('introspection') + nodes = db.model_query(db.Node).order_by( + db.Node.started_at.desc()).all() + + # assert ordering + self.assertEqual([node.uuid for node in nodes], + [status_.get('uuid') for status_ in statuses]) + + # assert pagination + half = len(nodes) // 2 + marker = nodes[half].uuid + statuses = self.call_get_statuses(marker=marker).get('introspection') + self.assertEqual([node.uuid for node in nodes[half + 1:]], + [status_.get('uuid') for status_ in statuses]) + + # assert status links work + self.assertEqual([self.call_get_status(status_.get('uuid')) + for status_ in statuses], + [self.call('GET', urllib.parse.urlparse( + status_.get('links')[0].get('href')).path).json() + for status_ in statuses]) + def test_rules_api(self): res = self.call_list_rules() self.assertEqual([], res) diff --git a/ironic_inspector/test/unit/test_api_tools.py b/ironic_inspector/test/unit/test_api_tools.py new file mode 100644 index 000000000..979e12325 --- /dev/null +++ b/ironic_inspector/test/unit/test_api_tools.py @@ -0,0 +1,136 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import flask +import mock +from oslo_config import cfg +from oslo_utils import uuidutils +import six + +from ironic_inspector import api_tools +import ironic_inspector.test.base as test_base +from ironic_inspector import utils + +CONF = cfg.CONF +app = flask.Flask(__name__) +app.testing = True + + +def mock_test_field(return_value=None, side_effect=None): + """Mock flask.request.args.get""" + def outer(func): + @six.wraps(func) + def inner(self, *args, **kwargs): + with app.test_request_context('/'): + get_mock = flask.request.args.get = mock.Mock() + get_mock.return_value = return_value + get_mock.side_effect = side_effect + ret = func(self, get_mock, *args, **kwargs) + return ret + return inner + return outer + + +class RaisesCoercionExceptionTestCase(test_base.BaseTest): + def test_ok(self): + @api_tools.raises_coercion_exceptions + def fn(): + return True + self.assertIs(True, fn()) + + def test_assertion_error(self): + @api_tools.raises_coercion_exceptions + def fn(): + assert False, 'Oops!' + + six.assertRaisesRegex(self, utils.Error, 'Bad request: Oops!', fn) + + def test_value_error(self): + @api_tools.raises_coercion_exceptions + def fn(): + raise ValueError('Oops!') + + six.assertRaisesRegex(self, utils.Error, 'Bad request: Oops!', fn) + + +class RequestFieldTestCase(test_base.BaseTest): + @mock_test_field(return_value='42') + def test_request_field_ok(self, get_mock): + @api_tools.request_field('foo') + def fn(value): + self.assertEqual(get_mock.return_value, value) + + fn() + get_mock.assert_called_once_with('foo', default=None) + + @mock_test_field(return_value='42') + def test_request_field_with_default(self, get_mock): + @api_tools.request_field('foo') + def fn(value): + self.assertEqual(get_mock.return_value, value) + + fn(default='bar') + get_mock.assert_called_once_with('foo', default='bar') + + @mock_test_field(return_value=42) + def test_request_field_with_default_returns_default(self, get_mock): + @api_tools.request_field('foo') + def fn(value): + self.assertEqual(get_mock.return_value, value) + + fn(default=42) + get_mock.assert_called_once_with('foo', default=42) + + +class MarkerFieldTestCase(test_base.BaseTest): + @mock_test_field(return_value=uuidutils.generate_uuid()) + def test_marker_ok(self, get_mock): + value = api_tools.marker_field() + self.assertEqual(get_mock.return_value, value) + + @mock.patch.object(uuidutils, 'is_uuid_like', autospec=True) + @mock_test_field(return_value='foo') + def test_marker_check_fails(self, get_mock, like_mock): + like_mock.return_value = False + six.assertRaisesRegex(self, utils.Error, '.*(Marker not UUID-like)', + api_tools.marker_field) + like_mock.assert_called_once_with(get_mock.return_value) + + +class LimitFieldTestCase(test_base.BaseTest): + @mock_test_field(return_value=42) + def test_limit_ok(self, get_mock): + value = api_tools.limit_field() + self.assertEqual(get_mock.return_value, value) + + @mock_test_field(return_value=str(CONF.api_max_limit + 1)) + def test_limit_over(self, get_mock): + six.assertRaisesRegex(self, utils.Error, + '.*(Limit over %s)' % CONF.api_max_limit, + api_tools.limit_field) + + @mock_test_field(return_value='0') + def test_limit_zero(self, get_mock): + value = api_tools.limit_field() + self.assertEqual(CONF.api_max_limit, value) + + @mock_test_field(return_value='-1') + def test_limit_negative(self, get_mock): + six.assertRaisesRegex(self, utils.Error, + '.*(Limit cannot be negative)', + api_tools.limit_field) + + @mock_test_field(return_value='foo') + def test_limit_invalid_value(self, get_mock): + six.assertRaisesRegex(self, utils.Error, 'Bad request', + api_tools.limit_field) diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index e496fa45a..21df05b67 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -232,6 +232,33 @@ class TestApiGetStatus(GetStatusAPIBaseTest): json.loads(res.data.decode('utf-8'))) +@mock.patch.object(node_cache, 'get_node_list', autospec=True) +class TestApiListStatus(GetStatusAPIBaseTest): + + def test_list_introspection(self, list_mock): + list_mock.return_value = [self.finished_node, self.unfinished_node] + res = self.app.get('/v1/introspection') + self.assertEqual(200, res.status_code) + statuses = json.loads(res.data.decode('utf-8')).get('introspection') + + self.assertEqual([self.finished_node.status, + self.unfinished_node.status], statuses) + list_mock.assert_called_once_with(marker=None, + limit=CONF.api_max_limit) + + def test_list_introspection_limit(self, list_mock): + res = self.app.get('/v1/introspection?limit=1000') + self.assertEqual(200, res.status_code) + list_mock.assert_called_once_with(marker=None, limit=1000) + + def test_list_introspection_makrer(self, list_mock): + res = self.app.get('/v1/introspection?marker=%s' % + self.finished_node.uuid) + self.assertEqual(200, res.status_code) + list_mock.assert_called_once_with(marker=self.finished_node.uuid, + limit=CONF.api_max_limit) + + class TestApiGetData(BaseAPITest): @mock.patch.object(main.swift, 'SwiftAPI', autospec=True) def test_get_introspection_data(self, swift_mock): diff --git a/ironic_inspector/test/unit/test_node_cache.py b/ironic_inspector/test/unit/test_node_cache.py index bf481834b..036f5047d 100644 --- a/ironic_inspector/test/unit/test_node_cache.py +++ b/ironic_inspector/test/unit/test_node_cache.py @@ -725,3 +725,36 @@ class TestNodeCreate(test_base.NodeTest): mock_get_client.assert_called_once_with() self.mock_client.node.create.assert_called_once_with(driver='fake') self.assertFalse(mock_add_node.called) + + +class TestNodeCacheListNode(test_base.NodeTest): + def setUp(self): + super(TestNodeCacheListNode, self).setUp() + self.uuid2 = uuidutils.generate_uuid() + session = db.get_session() + with session.begin(): + db.Node(uuid=self.uuid, started_at=42.42).save(session) + db.Node(uuid=self.uuid2, started_at=42.24, + finished_at=100.0).save(session) + + # mind please node(self.uuid).started_at > node(self.uuid2).started_at + # and the result ordering is strict in node_cache.get_node_list newer first + + def test_list_node(self): + nodes = node_cache.get_node_list() + + self.assertEqual([self.uuid, self.uuid2], + [node.uuid for node in nodes]) + + def test_list_node_limit(self): + nodes = node_cache.get_node_list(limit=1) + self.assertEqual([self.uuid], [node.uuid for node in nodes]) + + def test_list_node_marker(self): + # get nodes started_at after node(self.uuid) + nodes = node_cache.get_node_list(marker=self.uuid) + self.assertEqual([self.uuid2], [node.uuid for node in nodes]) + + def test_list_node_wrong_marker(self): + self.assertRaises(utils.Error, node_cache.get_node_list, + marker='foo-bar') diff --git a/releasenotes/notes/add-support-for-listing-all-introspection-statuses-2a3d4379c3854894.yaml b/releasenotes/notes/add-support-for-listing-all-introspection-statuses-2a3d4379c3854894.yaml new file mode 100644 index 000000000..ab5b24a21 --- /dev/null +++ b/releasenotes/notes/add-support-for-listing-all-introspection-statuses-2a3d4379c3854894.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Add an API endpoint for listing introspection statuses, operators can get + status for all running or previously run introspection processing. + + - | + Introduced a new configuration option ``api_max_limit`` that defines the + maximum number of items per page when API results are paginated.