From d278bb6f779eb8941754b29dbb6083a732330783 Mon Sep 17 00:00:00 2001 From: space Date: Tue, 24 Oct 2017 03:36:49 -0400 Subject: [PATCH] introspection data backend: plugin layer Configurable introspection data storage backend [1] is proposed to provide flexible extension of introspection data storage instead of the single support of Swift storage backend. This patch adds plugin mechanism for loading introspection storage, creates database backend and moves Swift storage into a plugin. [1] http://specs.openstack.org/openstack/ironic-inspector-specs/specs/configurable-introspection-data-backends.html Story: 1726713 Task: 11373 Co-Authored-By: Kaifeng Wang Change-Id: Ie4d09dc0afc441b20a1e5e3bd8e742b1df918954 --- ironic_inspector/conductor/manager.py | 11 +- ironic_inspector/conf/processing.py | 8 +- ironic_inspector/main.py | 21 +-- ironic_inspector/plugins/base.py | 10 ++ .../plugins/introspection_data.py | 123 ++++++++++++++++++ ironic_inspector/process.py | 58 +++------ ironic_inspector/test/unit/test_main.py | 80 ++++++------ ironic_inspector/test/unit/test_manager.py | 79 +++++++++-- .../unit/test_plugins_introspection_data.py | 108 +++++++++++++++ ironic_inspector/test/unit/test_process.py | 52 +++++--- ironic_inspector/utils.py | 4 + ...ection-data-db-store-0586292de05cbfd7.yaml | 6 + setup.cfg | 4 + 13 files changed, 443 insertions(+), 121 deletions(-) create mode 100644 ironic_inspector/plugins/introspection_data.py create mode 100644 ironic_inspector/test/unit/test_plugins_introspection_data.py create mode 100644 releasenotes/notes/introspection-data-db-store-0586292de05cbfd7.yaml diff --git a/ironic_inspector/conductor/manager.py b/ironic_inspector/conductor/manager.py index c4ab9584f..568783f3e 100644 --- a/ironic_inspector/conductor/manager.py +++ b/ironic_inspector/conductor/manager.py @@ -22,6 +22,7 @@ from oslo_log import log import oslo_messaging as messaging from oslo_utils import reflection +from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils from ironic_inspector import db from ironic_inspector import introspect @@ -126,7 +127,15 @@ class ConductorManager(object): @messaging.expected_exceptions(utils.Error) def do_reapply(self, context, node_id, token=None): - process.reapply(node_id) + try: + data = process.get_introspection_data(node_id, processed=False, + get_json=True) + except utils.IntrospectionDataStoreDisabled: + raise utils.Error(_('Inspector is not configured to store ' + 'data. Set the [processing]store_data ' + 'configuration option to change this.'), + code=400) + process.reapply(node_id, data) def periodic_clean_up(): # pragma: no cover diff --git a/ironic_inspector/conf/processing.py b/ironic_inspector/conf/processing.py index 9840f47cd..502c284a9 100644 --- a/ironic_inspector/conf/processing.py +++ b/ironic_inspector/conf/processing.py @@ -18,7 +18,6 @@ from ironic_inspector.common.i18n import _ VALID_ADD_PORTS_VALUES = ('all', 'active', 'pxe', 'disabled') VALID_KEEP_PORTS_VALUES = ('all', 'present', 'added') -VALID_STORE_DATA_VALUES = ('none', 'swift') _OPTS = [ @@ -75,9 +74,10 @@ _OPTS = [ 'aware of. This hook is ignored by default.')), cfg.StrOpt('store_data', default='none', - choices=VALID_STORE_DATA_VALUES, - help=_('Method for storing introspection data. If set to \'none' - '\', introspection data will not be stored.')), + help=_('The storage backend for storing introspection data. ' + 'Possible values are: \'none\', \'database\' and ' + '\'swift\'. If set to \'none\', introspection data will ' + 'not be stored.')), cfg.StrOpt('store_data_location', help=_('Name of the key to store the location of stored data ' 'in the extra column of the Ironic database.')), diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index 4a4cd500e..e30da024d 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -25,7 +25,6 @@ from ironic_inspector.common import context from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils from ironic_inspector.common import rpc -from ironic_inspector.common import swift import ironic_inspector.conf from ironic_inspector.conf import opts as conf_opts from ironic_inspector import node_cache @@ -289,15 +288,15 @@ def api_introspection_abort(node_id): @api('/v1/introspection//data', rule="introspection:data", methods=['GET']) def api_introspection_data(node_id): - if CONF.processing.store_data == 'swift': + try: 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) + res = process.get_introspection_data(node_id) return res, 200, {'Content-Type': 'application/json'} - else: + except utils.IntrospectionDataStoreDisabled: return error_response(_('Inspector is not configured to store data. ' - 'Set the [processing] store_data ' + 'Set the [processing]store_data ' 'configuration option to change this.'), code=404) @@ -309,15 +308,9 @@ def api_introspection_reapply(node_id): return error_response(_('User data processing is not ' 'supported yet'), code=400) - if CONF.processing.store_data == 'swift': - client = rpc.get_client() - client.call({}, 'do_reapply', node_id=node_id) - return '', 202 - else: - return error_response(_('Inspector is not configured to store' - ' data. Set the [processing] ' - 'store_data configuration option to ' - 'change this.'), code=400) + client = rpc.get_client() + client.call({}, 'do_reapply', node_id=node_id) + return '', 202 def rule_repr(rule, short): diff --git a/ironic_inspector/plugins/base.py b/ironic_inspector/plugins/base.py index bfd8322ed..3d2a12a5b 100644 --- a/ironic_inspector/plugins/base.py +++ b/ironic_inspector/plugins/base.py @@ -142,6 +142,7 @@ _HOOKS_MGR = None _NOT_FOUND_HOOK_MGR = None _CONDITIONS_MGR = None _ACTIONS_MGR = None +_INTROSPECTION_DATA_MGR = None def missing_entrypoints_callback(names): @@ -229,3 +230,12 @@ def rule_actions_manager(): 'ironic_inspector.rules.actions', invoke_on_load=True) return _ACTIONS_MGR + + +def introspection_data_manager(): + global _INTROSPECTION_DATA_MGR + if _INTROSPECTION_DATA_MGR is None: + _INTROSPECTION_DATA_MGR = stevedore.ExtensionManager( + 'ironic_inspector.introspection_data.store', + invoke_on_load=True) + return _INTROSPECTION_DATA_MGR diff --git a/ironic_inspector/plugins/introspection_data.py b/ironic_inspector/plugins/introspection_data.py new file mode 100644 index 000000000..e0d8d6ac2 --- /dev/null +++ b/ironic_inspector/plugins/introspection_data.py @@ -0,0 +1,123 @@ +# 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. + +"""Backends for storing introspection data.""" + +import abc +import json + +from oslo_config import cfg +from oslo_utils import excutils +import six + +from ironic_inspector.common import swift +from ironic_inspector import node_cache +from ironic_inspector import utils + + +CONF = cfg.CONF + +LOG = utils.getProcessingLogger(__name__) + +_STORAGE_EXCLUDED_KEYS = {'logs'} +_UNPROCESSED_DATA_STORE_SUFFIX = 'UNPROCESSED' + + +def _filter_data_excluded_keys(data): + return {k: v for k, v in data.items() + if k not in _STORAGE_EXCLUDED_KEYS} + + +@six.add_metaclass(abc.ABCMeta) +class BaseStorageBackend(object): + + @abc.abstractmethod + def get(self, node_id, processed=True, get_json=False): + """Get introspected data from storage backend. + + :param node_id: node UUID or name. + :param processed: Specify whether the data to be retrieved is + processed or not. + :param get_json: Specify whether return the introspection data in json + format, string value is returned if False. + :returns: the introspection data. + :raises: IntrospectionDataStoreDisabled if storage backend is disabled. + """ + + @abc.abstractmethod + def save(self, node_info, data, processed=True): + """Save introspected data to storage backend. + + :param node_info: a NodeInfo object. + :param data: the introspected data to be saved, in dict format. + :param processed: Specify whether the data to be saved is processed or + not. + :raises: IntrospectionDataStoreDisabled if storage backend is disabled. + """ + + +class NoStore(BaseStorageBackend): + def get(self, node_id, processed=True, get_json=False): + raise utils.IntrospectionDataStoreDisabled( + 'Introspection data storage is disabled') + + def save(self, node_info, data, processed=True): + LOG.debug('Introspection data storage is disabled, the data will not ' + 'be saved', node_info=node_info) + + +class SwiftStore(object): + def get(self, node_id, processed=True, get_json=False): + suffix = None if processed else _UNPROCESSED_DATA_STORE_SUFFIX + LOG.debug('Fetching introspection data from Swift for %s', node_id) + data = swift.get_introspection_data(node_id, suffix=suffix) + if get_json: + return json.loads(data) + return data + + def save(self, node_info, data, processed=True): + suffix = None if processed else _UNPROCESSED_DATA_STORE_SUFFIX + swift_object_name = swift.store_introspection_data( + _filter_data_excluded_keys(data), + node_info.uuid, + suffix=suffix + ) + LOG.info('Introspection data was stored in Swift object %s', + swift_object_name, node_info=node_info) + if CONF.processing.store_data_location: + node_info.patch([{'op': 'add', 'path': '/extra/%s' % + CONF.processing.store_data_location, + 'value': swift_object_name}]) + + +class DatabaseStore(object): + def get(self, node_id, processed=True, get_json=False): + LOG.debug('Fetching introspection data from database for %(node)s', + {'node': node_id}) + data = node_cache.get_introspection_data(node_id, processed) + if get_json: + return data + return json.dumps(data) + + def save(self, node_info, data, processed=True): + introspection_data = _filter_data_excluded_keys(data) + try: + node_cache.store_introspection_data(node_info.uuid, + introspection_data, processed) + except Exception as e: + with excutils.save_and_reraise_exception(): + LOG.exception('Failed to store introspection data in ' + 'database: %(exc)s', {'exc': e}) + else: + LOG.info('Introspection data was stored in database', + node_info=node_info) diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index f21302f92..4f86bbe65 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -15,7 +15,6 @@ import copy import datetime -import json import os from oslo_config import cfg @@ -25,7 +24,6 @@ from oslo_utils import timeutils from ironic_inspector.common.i18n import _ from ironic_inspector.common import ironic as ir_utils -from ironic_inspector.common import swift from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base @@ -38,7 +36,6 @@ CONF = cfg.CONF LOG = utils.getProcessingLogger(__name__) _STORAGE_EXCLUDED_KEYS = {'logs'} -_UNPROCESSED_DATA_STORE_SUFFIX = 'UNPROCESSED' def _store_logs(introspection_data, node_info): @@ -143,48 +140,28 @@ def _filter_data_excluded_keys(data): if k not in _STORAGE_EXCLUDED_KEYS} -def _store_data(node_info, data, suffix=None): - if CONF.processing.store_data != 'swift': - LOG.debug("Swift support is disabled, introspection data " - "won't be stored", node_info=node_info) - return - - swift_object_name = swift.store_introspection_data( - _filter_data_excluded_keys(data), - node_info.uuid, - suffix=suffix - ) - LOG.info('Introspection data was stored in Swift in object ' - '%s', swift_object_name, node_info=node_info) - if CONF.processing.store_data_location: - node_info.patch([{'op': 'add', 'path': '/extra/%s' % - CONF.processing.store_data_location, - 'value': swift_object_name}]) +def _store_data(node_info, data, processed=True): + introspection_data_manager = plugins_base.introspection_data_manager() + store = CONF.processing.store_data + ext = introspection_data_manager[store].obj + ext.save(node_info, data, processed) def _store_unprocessed_data(node_info, data): # runs in background try: - _store_data(node_info, data, - suffix=_UNPROCESSED_DATA_STORE_SUFFIX) + _store_data(node_info, data, processed=False) except Exception: LOG.exception('Encountered exception saving unprocessed ' 'introspection data', node_info=node_info, data=data) -def _get_unprocessed_data(uuid): - if CONF.processing.store_data == 'swift': - LOG.debug('Fetching unprocessed introspection data from ' - 'Swift for %s', uuid) - return json.loads( - swift.get_introspection_data( - uuid, - suffix=_UNPROCESSED_DATA_STORE_SUFFIX - ) - ) - else: - raise utils.Error(_('Swift support is disabled'), code=400) +def get_introspection_data(uuid, processed=True, get_json=False): + introspection_data_manager = plugins_base.introspection_data_manager() + store = CONF.processing.store_data + ext = introspection_data_manager[store].obj + return ext.get(uuid, processed=processed, get_json=get_json) def process(introspection_data): @@ -309,7 +286,7 @@ def _finish(node_info, ironic, introspection_data, power_off=True): node_info=node_info, data=introspection_data) -def reapply(node_ident): +def reapply(node_ident, data=None): """Re-apply introspection steps. Re-apply preprocessing, postprocessing and introspection rules on @@ -331,15 +308,20 @@ def reapply(node_ident): raise utils.Error(_('Node locked, please, try again later'), node_info=node_info, code=409) - utils.executor().submit(_reapply, node_info) + utils.executor().submit(_reapply, node_info, data) -def _reapply(node_info): +def _reapply(node_info, data=None): # runs in background try: node_info.started_at = timeutils.utcnow() node_info.commit() - introspection_data = _get_unprocessed_data(node_info.uuid) + if data: + introspection_data = data + else: + introspection_data = get_introspection_data(node_info.uuid, + processed=False, + get_json=True) except Exception as exc: LOG.exception('Encountered exception while fetching ' 'stored introspection data', diff --git a/ironic_inspector/test/unit/test_main.py b/ironic_inspector/test/unit/test_main.py index 1e24f16ef..c1b45e154 100644 --- a/ironic_inspector/test/unit/test_main.py +++ b/ironic_inspector/test/unit/test_main.py @@ -22,6 +22,7 @@ from oslo_utils import uuidutils from ironic_inspector.common import ironic as ir_utils from ironic_inspector.common import rpc +from ironic_inspector.common import swift import ironic_inspector.conf from ironic_inspector.conf import opts as conf_opts from ironic_inspector import introspection_state as istate @@ -29,6 +30,7 @@ from ironic_inspector import main from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base from ironic_inspector.plugins import example as example_plugin +from ironic_inspector.plugins import introspection_data as intros_data_plugin from ironic_inspector import process from ironic_inspector import rules from ironic_inspector.test import base as test_base @@ -297,10 +299,9 @@ class TestApiListStatus(GetStatusAPIBaseTest): class TestApiGetData(BaseAPITest): - @mock.patch.object(main.swift, 'SwiftAPI', autospec=True) - def test_get_introspection_data(self, swift_mock): - CONF.set_override('store_data', 'swift', 'processing') - data = { + def setUp(self): + super(TestApiGetData, self).setUp() + self.introspection_data = { 'ipmi_address': '1.2.3.4', 'cpus': 2, 'cpu_arch': 'x86_64', @@ -310,44 +311,48 @@ class TestApiGetData(BaseAPITest): 'em1': {'mac': '11:22:33:44:55:66', 'ip': '1.2.0.1'}, } } + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_get_introspection_data_from_swift(self, swift_mock): + CONF.set_override('store_data', 'swift', 'processing') swift_conn = swift_mock.return_value - swift_conn.get_object.return_value = json.dumps(data) + swift_conn.get_object.return_value = json.dumps( + self.introspection_data) res = self.app.get('/v1/introspection/%s/data' % self.uuid) 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'))) + self.assertEqual(self.introspection_data, + json.loads(res.data.decode('utf-8'))) - @mock.patch.object(main.swift, 'SwiftAPI', autospec=True) - def test_introspection_data_not_stored(self, swift_mock): - CONF.set_override('store_data', 'none', 'processing') - swift_conn = swift_mock.return_value + @mock.patch.object(intros_data_plugin, 'DatabaseStore', + autospec=True) + def test_get_introspection_data_from_db(self, db_mock): + CONF.set_override('store_data', 'database', 'processing') + db_store = db_mock.return_value + db_store.get.return_value = json.dumps(self.introspection_data) + res = self.app.get('/v1/introspection/%s/data' % self.uuid) + db_store.get.assert_called_once_with(self.uuid, processed=True, + get_json=False) + self.assertEqual(200, res.status_code) + self.assertEqual(self.introspection_data, + json.loads(res.data.decode('utf-8'))) + + def test_introspection_data_not_stored(self): + CONF.set_override('store_data', 'none', 'processing') res = self.app.get('/v1/introspection/%s/data' % self.uuid) - 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): + @mock.patch.object(main.process, 'get_introspection_data', autospec=True) + def test_with_name(self, process_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) + process_mock.return_value = json.dumps(self.introspection_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'))) + self.assertEqual(self.introspection_data, + json.loads(res.data.decode('utf-8'))) get_mock.assert_called_once_with('name1', fields=['uuid']) @@ -361,8 +366,7 @@ class TestApiReapply(BaseAPITest): self.rpc_get_client_mock.return_value = self.client_mock CONF.set_override('store_data', 'swift', 'processing') - def test_ok(self): - + def test_api_ok(self): self.app.post('/v1/introspection/%s/data/unprocessed' % self.uuid) self.client_mock.call.assert_called_once_with({}, 'do_reapply', @@ -377,18 +381,18 @@ class TestApiReapply(BaseAPITest): message) self.assertFalse(self.client_mock.call.called) - def test_swift_disabled(self): - CONF.set_override('store_data', 'none', 'processing') + def test_get_introspection_data_error(self): + exc = utils.Error('The store is crashed', code=404) + self.client_mock.call.side_effect = exc res = self.app.post('/v1/introspection/%s/data/unprocessed' % self.uuid) - self.assertEqual(400, res.status_code) + + self.assertEqual(404, res.status_code) message = json.loads(res.data.decode())['error']['message'] - self.assertEqual('Inspector is not configured to store ' - 'data. Set the [processing] store_data ' - 'configuration option to change this.', - message) - self.assertFalse(self.client_mock.call.called) + self.assertEqual(str(exc), message) + self.client_mock.call.assert_called_once_with({}, 'do_reapply', + node_id=self.uuid) def test_generic_error(self): exc = utils.Error('Oops', code=400) diff --git a/ironic_inspector/test/unit/test_manager.py b/ironic_inspector/test/unit/test_manager.py index 71131f756..e374441db 100644 --- a/ironic_inspector/test/unit/test_manager.py +++ b/ironic_inspector/test/unit/test_manager.py @@ -11,10 +11,13 @@ # See the License for the specific language governing permissions and # limitations under the License. +import json + import fixtures import mock import oslo_messaging as messaging +from ironic_inspector.common import swift from ironic_inspector.conductor import manager import ironic_inspector.conf from ironic_inspector import introspect @@ -302,11 +305,17 @@ class TestManagerReapply(BaseManagerTest): super(TestManagerReapply, self).setUp() CONF.set_override('store_data', 'swift', 'processing') - def test_ok(self, reapply_mock): + @mock.patch.object(swift, 'store_introspection_data', autospec=True) + @mock.patch.object(swift, 'get_introspection_data', autospec=True) + def test_ok(self, swift_get_mock, swift_set_mock, reapply_mock): + swift_get_mock.return_value = json.dumps(self.data) self.manager.do_reapply(self.context, self.uuid) - reapply_mock.assert_called_once_with(self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) - def test_node_locked(self, reapply_mock): + @mock.patch.object(swift, 'store_introspection_data', autospec=True) + @mock.patch.object(swift, 'get_introspection_data', autospec=True) + def test_node_locked(self, swift_get_mock, swift_set_mock, reapply_mock): + swift_get_mock.return_value = json.dumps(self.data) exc = utils.Error('Locked.', code=409) reapply_mock.side_effect = exc @@ -317,9 +326,13 @@ class TestManagerReapply(BaseManagerTest): self.assertEqual(utils.Error, exc.exc_info[0]) self.assertIn('Locked.', str(exc.exc_info[1])) self.assertEqual(409, exc.exc_info[1].http_code) - reapply_mock.assert_called_once_with(self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) - def test_node_not_found(self, reapply_mock): + @mock.patch.object(swift, 'store_introspection_data', autospec=True) + @mock.patch.object(swift, 'get_introspection_data', autospec=True) + def test_node_not_found(self, swift_get_mock, swift_set_mock, + reapply_mock): + swift_get_mock.return_value = json.dumps(self.data) exc = utils.Error('Not found.', code=404) reapply_mock.side_effect = exc @@ -330,9 +343,11 @@ class TestManagerReapply(BaseManagerTest): self.assertEqual(utils.Error, exc.exc_info[0]) self.assertIn('Not found.', str(exc.exc_info[1])) self.assertEqual(404, exc.exc_info[1].http_code) - reapply_mock.assert_called_once_with(self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) - def test_generic_error(self, reapply_mock): + @mock.patch.object(process, 'get_introspection_data', autospec=True) + def test_generic_error(self, get_data_mock, reapply_mock): + get_data_mock.return_value = self.data exc = utils.Error('Oops', code=400) reapply_mock.side_effect = exc @@ -343,4 +358,52 @@ class TestManagerReapply(BaseManagerTest): self.assertEqual(utils.Error, exc.exc_info[0]) self.assertIn('Oops', str(exc.exc_info[1])) self.assertEqual(400, exc.exc_info[1].http_code) - reapply_mock.assert_called_once_with(self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) + get_data_mock.assert_called_once_with(self.uuid, processed=False, + get_json=True) + + @mock.patch.object(process, 'get_introspection_data', autospec=True) + def test_get_introspection_data_error(self, get_data_mock, reapply_mock): + exc = utils.Error('The store is empty', code=404) + get_data_mock.side_effect = exc + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.manager.do_reapply, + self.context, self.uuid) + + self.assertEqual(utils.Error, exc.exc_info[0]) + self.assertIn('The store is empty', str(exc.exc_info[1])) + self.assertEqual(404, exc.exc_info[1].http_code) + get_data_mock.assert_called_once_with(self.uuid, processed=False, + get_json=True) + self.assertFalse(reapply_mock.called) + + def test_store_data_disabled(self, reapply_mock): + CONF.set_override('store_data', 'none', 'processing') + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.manager.do_reapply, + self.context, self.uuid) + + self.assertEqual(utils.Error, exc.exc_info[0]) + self.assertIn('Inspector is not configured to store data', + str(exc.exc_info[1])) + self.assertEqual(400, exc.exc_info[1].http_code) + self.assertFalse(reapply_mock.called) + + @mock.patch.object(process, 'get_introspection_data', autospec=True) + def test_ok_swift(self, get_data_mock, reapply_mock): + get_data_mock.return_value = self.data + self.manager.do_reapply(self.context, self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) + get_data_mock.assert_called_once_with(self.uuid, processed=False, + get_json=True) + + @mock.patch.object(process, 'get_introspection_data', autospec=True) + def test_ok_db(self, get_data_mock, reapply_mock): + get_data_mock.return_value = self.data + CONF.set_override('store_data', 'database', 'processing') + self.manager.do_reapply(self.context, self.uuid) + reapply_mock.assert_called_once_with(self.uuid, data=self.data) + get_data_mock.assert_called_once_with(self.uuid, processed=False, + get_json=True) diff --git a/ironic_inspector/test/unit/test_plugins_introspection_data.py b/ironic_inspector/test/unit/test_plugins_introspection_data.py new file mode 100644 index 000000000..c2b74d508 --- /dev/null +++ b/ironic_inspector/test/unit/test_plugins_introspection_data.py @@ -0,0 +1,108 @@ +# 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 json + +import fixtures +import mock +from oslo_config import cfg + +from ironic_inspector.common import ironic as ir_utils +from ironic_inspector import db +from ironic_inspector import introspection_state as istate +from ironic_inspector.plugins import introspection_data +from ironic_inspector.test import base as test_base + +CONF = cfg.CONF + + +class BaseTest(test_base.NodeTest): + 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'}, + } + } + + def setUp(self): + super(BaseTest, self).setUp() + self.cli_fixture = self.useFixture( + fixtures.MockPatchObject(ir_utils, 'get_client', autospec=True)) + self.cli = self.cli_fixture.mock.return_value + + +@mock.patch.object(introspection_data.swift, 'SwiftAPI', autospec=True) +class TestSwiftStore(BaseTest): + + def setUp(self): + super(TestSwiftStore, self).setUp() + self.driver = introspection_data.SwiftStore() + + def test_get_data(self, swift_mock): + swift_conn = swift_mock.return_value + swift_conn.get_object.return_value = json.dumps(self.data) + name = 'inspector_data-%s' % self.uuid + + res_data = self.driver.get(self.uuid) + + swift_conn.get_object.assert_called_once_with(name) + self.assertEqual(self.data, json.loads(res_data)) + + def test_store_data(self, swift_mock): + swift_conn = swift_mock.return_value + name = 'inspector_data-%s' % self.uuid + + self.driver.save(self.node_info, self.data) + + data = introspection_data._filter_data_excluded_keys(self.data) + swift_conn.create_object.assert_called_once_with(name, + json.dumps(data)) + + def test_store_data_location(self, swift_mock): + CONF.set_override('store_data_location', 'inspector_data_object', + 'processing') + swift_conn = swift_mock.return_value + name = 'inspector_data-%s' % self.uuid + patch = [{'path': '/extra/inspector_data_object', + 'value': name, 'op': 'add'}] + expected = self.data + + self.driver.save(self.node_info, self.data) + + data = introspection_data._filter_data_excluded_keys(self.data) + swift_conn.create_object.assert_called_once_with(name, + json.dumps(data)) + self.assertEqual(expected, + json.loads(swift_conn.create_object.call_args[0][1])) + self.cli.node.update.assert_any_call(self.uuid, patch) + + +class TestDatabaseStore(BaseTest): + def setUp(self): + super(TestDatabaseStore, self).setUp() + self.driver = introspection_data.DatabaseStore() + session = db.get_writer_session() + with session.begin(): + db.Node(uuid=self.node_info.uuid, + state=istate.States.starting).save(session) + + def test_store_and_get_data(self): + self.driver.save(self.node_info, self.data) + + res_data = self.driver.get(self.node_info.uuid) + + self.assertEqual(self.data, json.loads(res_data)) diff --git a/ironic_inspector/test/unit/test_process.py b/ironic_inspector/test/unit/test_process.py index a23d0b193..739ae5572 100644 --- a/ironic_inspector/test/unit/test_process.py +++ b/ironic_inspector/test/unit/test_process.py @@ -28,11 +28,13 @@ from oslo_utils import uuidutils import six from ironic_inspector.common import ironic as ir_utils +from ironic_inspector.common import swift from ironic_inspector import db from ironic_inspector import introspection_state as istate from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base from ironic_inspector.plugins import example as example_plugin +from ironic_inspector.plugins import introspection_data as intros_data_plugin from ironic_inspector import process from ironic_inspector.pxe_filter import base as pxe_filter from ironic_inspector.test import base as test_base @@ -259,22 +261,13 @@ class TestUnprocessedData(BaseProcessTest): store_mock.assert_called_once_with(mock.ANY, expected) - @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) - def test_save_unprocessed_data_failure(self, swift_mock): + def test_save_unprocessed_data_failure(self): CONF.set_override('store_data', 'swift', 'processing') - name = 'inspector_data-%s-%s' % ( - self.uuid, - process._UNPROCESSED_DATA_STORE_SUFFIX - ) - - swift_conn = swift_mock.return_value - swift_conn.create_object.side_effect = utils.Error('Oops') res = process.process(self.data) # assert store failure doesn't break processing self.assertEqual(self.fake_result_json, res) - swift_conn.create_object.assert_called_once_with(name, mock.ANY) @mock.patch.object(example_plugin.ExampleProcessingHook, 'before_processing', @@ -405,6 +398,7 @@ class TestProcessNode(BaseTest): started_at=self.node_info.started_at, finished_at=self.node_info.finished_at, error=self.node_info.error).save(self.session) + plugins_base._INTROSPECTION_DATA_MGR = None def test_return_includes_uuid(self): ret_val = process._process_node(self.node_info, self.node, self.data) @@ -485,8 +479,8 @@ class TestProcessNode(BaseTest): finished_mock.assert_called_once_with( self.node_info, istate.Events.finish) - @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) - def test_store_data(self, swift_mock): + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_store_data_with_swift(self, swift_mock): CONF.set_override('store_data', 'swift', 'processing') swift_conn = swift_mock.return_value name = 'inspector_data-%s' % self.uuid @@ -498,8 +492,8 @@ class TestProcessNode(BaseTest): self.assertEqual(expected, json.loads(swift_conn.create_object.call_args[0][1])) - @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) - def test_store_data_no_logs(self, swift_mock): + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_store_data_no_logs_with_swift(self, swift_mock): CONF.set_override('store_data', 'swift', 'processing') swift_conn = swift_mock.return_value name = 'inspector_data-%s' % self.uuid @@ -511,8 +505,8 @@ class TestProcessNode(BaseTest): self.assertNotIn('logs', json.loads(swift_conn.create_object.call_args[0][1])) - @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) - def test_store_data_location(self, swift_mock): + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test_store_data_location_with_swift(self, swift_mock): CONF.set_override('store_data', 'swift', 'processing') CONF.set_override('store_data_location', 'inspector_data_object', 'processing') @@ -529,6 +523,28 @@ class TestProcessNode(BaseTest): json.loads(swift_conn.create_object.call_args[0][1])) self.cli.node.update.assert_any_call(self.uuid, patch) + @mock.patch.object(node_cache, 'store_introspection_data', autospec=True) + def test_store_data_with_database(self, store_mock): + CONF.set_override('store_data', 'database', 'processing') + + process._process_node(self.node_info, self.node, self.data) + + data = intros_data_plugin._filter_data_excluded_keys(self.data) + store_mock.assert_called_once_with(self.node_info.uuid, data, True) + self.assertEqual(data, store_mock.call_args[0][1]) + + @mock.patch.object(node_cache, 'store_introspection_data', autospec=True) + def test_store_data_no_logs_with_database(self, store_mock): + CONF.set_override('store_data', 'database', 'processing') + + self.data['logs'] = 'something' + + process._process_node(self.node_info, self.node, self.data) + + data = intros_data_plugin._filter_data_excluded_keys(self.data) + store_mock.assert_called_once_with(self.node_info.uuid, data, True) + self.assertNotIn('logs', store_mock.call_args[0][1]) + @mock.patch.object(process, '_reapply', autospec=True) @mock.patch.object(node_cache, 'get_node', autospec=True) @@ -558,7 +574,7 @@ class TestReapply(BaseTest): blocking=False ) - reapply_mock.assert_called_once_with(pop_mock.return_value) + reapply_mock.assert_called_once_with(pop_mock.return_value, data=None) @prepare_mocks def test_locking_failed(self, pop_mock, reapply_mock): @@ -575,7 +591,7 @@ class TestReapply(BaseTest): @mock.patch.object(example_plugin.ExampleProcessingHook, 'before_update') @mock.patch.object(process.rules, 'apply', autospec=True) -@mock.patch.object(process.swift, 'SwiftAPI', autospec=True) +@mock.patch.object(swift, 'SwiftAPI', autospec=True) @mock.patch.object(node_cache.NodeInfo, 'finished', autospec=True) @mock.patch.object(node_cache.NodeInfo, 'release_lock', autospec=True) class TestReapplyNode(BaseTest): diff --git a/ironic_inspector/utils.py b/ironic_inspector/utils.py index 69244a4f8..b0cfc2168 100644 --- a/ironic_inspector/utils.py +++ b/ironic_inspector/utils.py @@ -140,6 +140,10 @@ class NodeStateInvalidEvent(Error): """Invalid event attempted.""" +class IntrospectionDataStoreDisabled(Error): + """Introspection data store is disabled.""" + + class IntrospectionDataNotFound(NotFoundInCacheError): """Introspection data not found.""" diff --git a/releasenotes/notes/introspection-data-db-store-0586292de05cbfd7.yaml b/releasenotes/notes/introspection-data-db-store-0586292de05cbfd7.yaml new file mode 100644 index 000000000..0758a433a --- /dev/null +++ b/releasenotes/notes/introspection-data-db-store-0586292de05cbfd7.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Adds the support to store introspection data in ironic-inspector database. + Set the option ``[processing]store_data`` to ``database`` to use this + feature. \ No newline at end of file diff --git a/setup.cfg b/setup.cfg index 2c84cc1f2..47a98c500 100644 --- a/setup.cfg +++ b/setup.cfg @@ -43,6 +43,10 @@ ironic_inspector.hooks.processing = ironic_inspector.hooks.node_not_found = example = ironic_inspector.plugins.example:example_not_found_hook enroll = ironic_inspector.plugins.discovery:enroll_node_not_found_hook +ironic_inspector.introspection_data.store = + none = ironic_inspector.plugins.introspection_data:NoStore + swift = ironic_inspector.plugins.introspection_data:SwiftStore + database = ironic_inspector.plugins.introspection_data:DatabaseStore ironic_inspector.rules.conditions = eq = ironic_inspector.plugins.rules:EqCondition lt = ironic_inspector.plugins.rules:LtCondition