From 3f903a7b1e13f432d785cb709d82e7e0c601bfae Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Fri, 9 Jan 2015 13:29:55 +0100 Subject: [PATCH] Implement get status endpoint * New endpoint /v1/introspection/ * Backed by new function node_cache.get_node * New client function get_status * Functional test Change-Id: Ib072b2ff711e7df232a793bf5e17c7880e97b92d Implements: blueprint get-status-api --- README.rst | 6 ++++ functest/run.py | 7 +++++ ironic_discoverd/client.py | 35 ++++++++++++++++++++---- ironic_discoverd/main.py | 11 +++++++- ironic_discoverd/node_cache.py | 24 +++++++++++++++- ironic_discoverd/test/test_client.py | 24 +++++++++++++--- ironic_discoverd/test/test_main.py | 22 +++++++++++++++ ironic_discoverd/test/test_node_cache.py | 27 ++++++++++++++---- 8 files changed, 139 insertions(+), 17 deletions(-) diff --git a/README.rst b/README.rst index 411b69882..1a854501c 100644 --- a/README.rst +++ b/README.rst @@ -230,6 +230,11 @@ See `1.0.0 release tracking page`_ for details. * Add support for plugins that hook into data processing pipeline, see `plugin-architecture blueprint`_ for details. +* Add new API ``GET /v1/introspection/`` and ``client.get_status`` for + getting discovery status. + + See `get-status-api blueprint`_ for details. + **Configuration** * Cache nodes under discovery in a local SQLite database. Set ``database`` @@ -247,6 +252,7 @@ See `1.0.0 release tracking page`_ for details. .. _1.0.0 release tracking page: https://bugs.launchpad.net/ironic-discoverd/+milestone/1.0.0 .. _setup-ipmi-credentials blueprint: https://blueprints.launchpad.net/ironic-discoverd/+spec/setup-ipmi-credentials .. _plugin-architecture blueprint: https://blueprints.launchpad.net/ironic-discoverd/+spec/plugin-architecture +.. _get-status-api blueprint: https://blueprints.launchpad.net/ironic-discoverd/+spec/get-status-api 0.2 Series ~~~~~~~~~~ diff --git a/functest/run.py b/functest/run.py index caf391582..8896d8320 100644 --- a/functest/run.py +++ b/functest/run.py @@ -100,6 +100,10 @@ class Test(base.NodeTest): client.discover([self.uuid], auth_token='token') eventlet.greenthread.sleep(1) + status = client.get_status(self.uuid, auth_token='token') + self.assertEqual({'finished': False, 'error': None}, status) + + self.node.power_state = 'power off' self.call_ramdisk() eventlet.greenthread.sleep(1) @@ -107,6 +111,9 @@ class Test(base.NodeTest): self.cli.port.create.assert_called_once_with( node_uuid=self.uuid, address='11:22:33:44:55:66') + status = client.get_status(self.uuid, auth_token='token') + self.assertEqual({'finished': True, 'error': None}, status) + @mock.patch.object(utils, 'get_keystone') @mock.patch.object(utils, 'get_client') diff --git a/ironic_discoverd/client.py b/ironic_discoverd/client.py index 9e1f10e08..112119314 100644 --- a/ironic_discoverd/client.py +++ b/ironic_discoverd/client.py @@ -21,6 +21,14 @@ import six _DEFAULT_URL = 'http://127.0.0.1:5050/v1' +def _prepare(base_url, auth_token): + base_url = base_url.rstrip('/') + if not base_url.endswith('v1'): + base_url += '/v1' + headers = {'X-Auth-Token': auth_token} + return base_url, headers + + def discover(uuids, base_url=_DEFAULT_URL, auth_token=''): """Post node UUID's for discovery. @@ -34,17 +42,32 @@ def discover(uuids, base_url=_DEFAULT_URL, auth_token=''): raise TypeError("Expected list of strings for uuids argument, got %s" % uuids) - base_url = base_url.rstrip('/') - if not base_url.endswith('v1'): - base_url += '/v1' - - headers = {'Content-Type': 'application/json', - 'X-Auth-Token': auth_token} + base_url, headers = _prepare(base_url, auth_token) + headers['Content-Type'] = 'application/json' res = requests.post(base_url + "/discover", data=json.dumps(uuids), headers=headers) res.raise_for_status() +def get_status(uuid, base_url=_DEFAULT_URL, auth_token=''): + """Get introspection status for a node. + + New in ironic-discoverd version 1.0.0. + :param uuid: node uuid. + :param base_url: *ironic-discoverd* URL in form: http://host:port[/ver], + defaults to ``http://127.0.0.1:5050/v1``. + :param auth_token: Keystone authentication token. + :raises: *requests* library HTTP errors. + """ + if not isinstance(uuid, six.string_types): + raise TypeError("Expected string for uuid argument, got %r" % uuid) + + base_url, headers = _prepare(base_url, auth_token) + res = requests.get(base_url + "/introspection/%s" % uuid, headers=headers) + res.raise_for_status() + return res.json() + + if __name__ == '__main__': parser = argparse.ArgumentParser(description='Discover nodes.') parser.add_argument('uuids', metavar='UUID', type=str, nargs='+', diff --git a/ironic_discoverd/main.py b/ironic_discoverd/main.py index 50275b1ed..69464db99 100644 --- a/ironic_discoverd/main.py +++ b/ironic_discoverd/main.py @@ -18,7 +18,7 @@ import json import logging import sys -from flask import Flask, request # noqa +from flask import Flask, request, json as flask_json # noqa from keystoneclient import exceptions @@ -47,6 +47,15 @@ def post_continue(): return json.dumps(res), 200, {'Content-Type': 'applications/json'} +@app.route('/v1/introspection/') +def introspection(uuid): + # NOTE(dtantsur): in the future this method will also accept PUT + # to initiate introspection. + node_info = node_cache.get_node(uuid) + return flask_json.jsonify(finished=bool(node_info.finished_at), + error=node_info.error or None) + + @app.route('/v1/discover', methods=['POST']) def post_discover(): if conf.getboolean('discoverd', 'authenticate'): diff --git a/ironic_discoverd/node_cache.py b/ironic_discoverd/node_cache.py index ff4c24c85..a82a040d0 100644 --- a/ironic_discoverd/node_cache.py +++ b/ironic_discoverd/node_cache.py @@ -62,6 +62,13 @@ class NodeInfo(object): (self.finished_at, error, self.uuid)) db.execute("delete from attributes where uuid=?", (self.uuid,)) + @classmethod + def from_row(cls, row): + """Construct NodeInfo from a database row.""" + fields = {key: row[key] + for key in ('uuid', 'started_at', 'finished_at', 'error')} + return cls(**fields) + def init(): """Initialize the database.""" @@ -78,7 +85,9 @@ def init(): def _db(): if _DB_NAME is None: init() - return sqlite3.connect(_DB_NAME) + conn = sqlite3.connect(_DB_NAME) + conn.row_factory = sqlite3.Row + return conn def add_node(uuid, **attributes): @@ -121,6 +130,19 @@ def macs_on_discovery(): "where name='mac'")} +def get_node(uuid): + """Get node from cache by it's UUID. + + :param uuid: node UUID. + :returns: structure NodeInfo. + """ + row = _db().execute('select * from nodes where uuid=?', (uuid,)).fetchone() + if row is None: + raise utils.DiscoveryFailed('Could not find node %s in cache' % uuid, + code=404) + return NodeInfo.from_row(row) + + def find_node(**attributes): """Find node in cache. diff --git a/ironic_discoverd/test/test_client.py b/ironic_discoverd/test/test_client.py index ddaddebcc..5f7c0e7ec 100644 --- a/ironic_discoverd/test/test_client.py +++ b/ironic_discoverd/test/test_client.py @@ -19,8 +19,8 @@ from ironic_discoverd import client @mock.patch.object(client.requests, 'post', autospec=True) -class TestClient(unittest.TestCase): - def test_client(self, mock_post): +class TestDiscover(unittest.TestCase): + def test(self, mock_post): client.discover(['uuid1', 'uuid2'], base_url="http://host:port", auth_token="token") mock_post.assert_called_once_with( @@ -30,7 +30,7 @@ class TestClient(unittest.TestCase): 'X-Auth-Token': 'token'} ) - def test_client_full_url(self, mock_post): + def test_full_url(self, mock_post): client.discover(['uuid1', 'uuid2'], base_url="http://host:port/v1/", auth_token="token") mock_post.assert_called_once_with( @@ -40,7 +40,7 @@ class TestClient(unittest.TestCase): 'X-Auth-Token': 'token'} ) - def test_client_default_url(self, mock_post): + def test_default_url(self, mock_post): client.discover(['uuid1', 'uuid2'], auth_token="token") mock_post.assert_called_once_with( @@ -49,3 +49,19 @@ class TestClient(unittest.TestCase): headers={'Content-Type': 'application/json', 'X-Auth-Token': 'token'} ) + + +@mock.patch.object(client.requests, 'get', autospec=True) +class TestGetStatus(unittest.TestCase): + def test(self, mock_get): + mock_get.return_value.json.return_value = 'json' + + client.get_status('uuid', auth_token='token') + + mock_get.assert_called_once_with( + "http://127.0.0.1:5050/v1/introspection/uuid", + headers={'X-Auth-Token': 'token'} + ) + + def test_invalid_input(self, _): + self.assertRaises(TypeError, client.get_status, 42) diff --git a/ironic_discoverd/test/test_main.py b/ironic_discoverd/test/test_main.py index dd5a15bb3..98d9ee23a 100644 --- a/ironic_discoverd/test/test_main.py +++ b/ironic_discoverd/test/test_main.py @@ -11,6 +11,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json import unittest import eventlet @@ -21,6 +22,7 @@ import mock from ironic_discoverd import conf from ironic_discoverd import discover from ironic_discoverd import main +from ironic_discoverd import node_cache from ironic_discoverd.plugins import base as plugins_base from ironic_discoverd.plugins import example as example_plugin from ironic_discoverd import process @@ -85,6 +87,26 @@ class TestApi(test_base.BaseTest): process_mock.assert_called_once_with("JSON") self.assertEqual(b'boom', res.data) + @mock.patch.object(node_cache, 'get_node', autospec=True) + def test_get_introspection_in_progress(self, get_mock): + get_mock.return_value = node_cache.NodeInfo(uuid='uuid', + started_at=42.0) + res = self.app.get('/v1/introspection/uuid') + self.assertEqual(200, res.status_code) + self.assertEqual({'finished': False, 'error': None}, + json.loads(res.data.decode('utf-8'))) + + @mock.patch.object(node_cache, 'get_node', autospec=True) + def test_get_introspection_finished(self, get_mock): + get_mock.return_value = node_cache.NodeInfo(uuid='uuid', + started_at=42.0, + finished_at=100.1, + error='boom') + res = self.app.get('/v1/introspection/uuid') + self.assertEqual(200, res.status_code) + self.assertEqual({'finished': True, 'error': 'boom'}, + json.loads(res.data.decode('utf-8'))) + @mock.patch.object(eventlet.greenthread, 'sleep', autospec=True) @mock.patch.object(utils, 'get_client') diff --git a/ironic_discoverd/test/test_node_cache.py b/ironic_discoverd/test/test_node_cache.py index f14115c8b..a23082352 100644 --- a/ironic_discoverd/test/test_node_cache.py +++ b/ironic_discoverd/test/test_node_cache.py @@ -47,7 +47,7 @@ class TestNodeCache(test_base.NodeTest): self.assertEqual([('bmc_address', '1.2.3.4', self.uuid), ('mac', self.macs[0], self.uuid), ('mac', self.macs[1], self.uuid)], - res) + [tuple(row) for row in res]) def test_add_node_duplicate_mac(self): with self.db: @@ -157,6 +157,23 @@ class TestNodeCacheCleanUp(test_base.NodeTest): 'select * from attributes').fetchall()) +class TestNodeCacheGetNode(test_base.NodeTest): + def test_ok(self): + started_at = time.time() - 42 + with self.db: + self.db.execute('insert into nodes(uuid, started_at) ' + 'values(?, ?)', (self.uuid, started_at)) + info = node_cache.get_node(self.uuid) + + self.assertEqual(self.uuid, info.uuid) + self.assertEqual(started_at, info.started_at) + self.assertIsNone(info.finished_at) + self.assertIsNone(info.error) + + def test_not_found(self): + self.assertRaises(utils.DiscoveryFailed, node_cache.get_node, 'foo') + + @mock.patch.object(time, 'time', lambda: 42.0) class TestNodeInfoFinished(test_base.NodeTest): def setUp(self): @@ -169,16 +186,16 @@ class TestNodeInfoFinished(test_base.NodeTest): def test_success(self): self.node_info.finished() - self.assertEqual((42.0, None), self.db.execute( - 'select finished_at, error from nodes').fetchone()) + self.assertEqual((42.0, None), tuple(self.db.execute( + 'select finished_at, error from nodes').fetchone())) self.assertEqual([], self.db.execute( "select * from attributes").fetchall()) def test_error(self): self.node_info.finished(error='boom') - self.assertEqual((42.0, 'boom'), self.db.execute( - 'select finished_at, error from nodes').fetchone()) + self.assertEqual((42.0, 'boom'), tuple(self.db.execute( + 'select finished_at, error from nodes').fetchone())) self.assertEqual([], self.db.execute( "select * from attributes").fetchall())