From 6eb9f58c8796243cf5132cb06160ea11ecddfacc Mon Sep 17 00:00:00 2001 From: John Trowbridge Date: Fri, 14 Aug 2015 09:46:26 -0400 Subject: [PATCH] Store and expose introspection data This adds the ability to store all of the data collected during introspection. The configuration option "[processing] store_data" (defaults to 'none'), determines this behavior. Initially, only 'none' and 'swift' are supported. If 'swift' is used, the data is stored in Swift with the object name of "inspector_data-". Adds an endpoint /v1/introspection//data which retrieves the data according to the method in "[processing] store_data". Returns 404 if this option is disabled. There is a further option to store the location of the data in the Ironic Node.extra column. For 'swift', this will be the name of the swift object. The option, "[processing] store_data_location" determines the key name in the Node.extra column. (defaults to not storing the location). Change-Id: Ibc38064f7ea56f85b9f5a77ef6f62a50f0381ff4 Implements: blueprint store-introspection-data --- CONTRIBUTING.rst | 1 + HTTP-API.rst | 18 +++++++ devstack/exercise.sh | 17 ++++++ devstack/plugin.sh | 11 ++++ example.conf | 9 ++++ ironic_inspector/common/swift.py | 54 +++++++++++++++++++ ironic_inspector/conf.py | 12 ++++- ironic_inspector/main.py | 17 +++++- ironic_inspector/process.py | 12 +++++ ironic_inspector/test/test_main.py | 31 +++++++++++ ironic_inspector/test/test_process.py | 35 ++++++++++++ ironic_inspector/test/test_swift.py | 78 ++++++++++++++++++++++++--- plugin-requirements.txt | 2 - requirements.txt | 1 + 14 files changed, 288 insertions(+), 10 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index e84bb2426..7363c8c8f 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -83,6 +83,7 @@ Example local.conf enable_service ironic ir-api ir-cond disable_service n-net n-novnc enable_service neutron q-svc q-agt q-dhcp q-l3 q-meta + enable_service s-proxy s-object s-container s-account disable_service heat h-api h-api-cfn h-api-cw h-eng disable_service cinder c-sch c-api c-vol diff --git a/HTTP-API.rst b/HTTP-API.rst index 85e092ba6..9dcf45509 100644 --- a/HTTP-API.rst +++ b/HTTP-API.rst @@ -50,6 +50,23 @@ Response body: JSON dictionary with keys: * ``finished`` (boolean) whether introspection is finished * ``error`` error string or ``null`` +Get Introspection Data +~~~~~~~~~~~~~~~~~~~~~~ + +``GET /v1/introspection//data`` get stored data from successful +introspection. + +Requires X-Auth-Token header with Keystone token for authentication. + +Response: + +* 200 - OK +* 400 - bad request +* 401, 403 - missing or invalid authentication +* 404 - data cannot be found or data storage not configured + +Response body: JSON dictionary with introspection data + Ramdisk Callback ~~~~~~~~~~~~~~~~ @@ -151,3 +168,4 @@ Version History ^^^^^^^^^^^^^^^ **1.0** version of API at the moment of introducing versioning. +**1.1** adds endpoint to retrieve stored introspection data. diff --git a/devstack/exercise.sh b/devstack/exercise.sh index 92dd6d65f..aff5c4487 100755 --- a/devstack/exercise.sh +++ b/devstack/exercise.sh @@ -71,6 +71,21 @@ function curl_ir { curl -H "X-Auth-Token: $token" -X $1 "$ironic_url/$2" } +function curl_ins { + curl -H "X-Auth-Token: $token" -X $1 "http://127.0.0.1:5050/$2" +} + +function test_swift { + # Basic sanity check of the data stored in Swift + stored_data_json=$(curl_ins GET v1/introspection/$uuid/data) + stored_cpu_arch=$(echo $stored_data_json | jq -r '.cpu_arch') + echo CPU arch for $uuid from stored data: $stored_cpu_arch + if [ "$stored_cpu_arch" != "$expected_cpu_arch" ]; then + echo "The data stored in Swift does not match the expected data." + exit 1 + fi +} + for uuid in $nodes; do node_json=$(curl_ir GET v1/nodes/$uuid) properties=$(echo $node_json | jq '.properties') @@ -93,6 +108,8 @@ for uuid in $nodes; do exit 1 fi + openstack service list | grep swift && test_swift + for attempt in {1..12}; do node_json=$(curl_ir GET v1/nodes/$uuid) provision_state=$(echo $node_json | jq -r '.provision_state') diff --git a/devstack/plugin.sh b/devstack/plugin.sh index 3f58e5319..223678bcb 100644 --- a/devstack/plugin.sh +++ b/devstack/plugin.sh @@ -137,10 +137,21 @@ function configure_inspector { inspector_iniset firewall dnsmasq_interface $IRONIC_INSPECTOR_INTERFACE inspector_iniset database connection sqlite:///$IRONIC_INSPECTOR_DATA_DIR/inspector.sqlite + is_service_enabled swift && configure_inspector_swift + iniset "$IRONIC_CONF_FILE" inspector enabled True iniset "$IRONIC_CONF_FILE" inspector service_url $IRONIC_INSPECTOR_URI } +function configure_inspector_swift { + inspector_iniset swift os_auth_url "$KEYSTONE_SERVICE_URI/v2.0" + inspector_iniset swift username $IRONIC_INSPECTOR_ADMIN_USER + inspector_iniset swift password $SERVICE_PASSWORD + inspector_iniset swift tenant_name $SERVICE_TENANT_NAME + + inspector_iniset processing store_data swift +} + function configure_inspector_dhcp { mkdir_chown_stack "$IRONIC_INSPECTOR_CONF_DIR" diff --git a/example.conf b/example.conf index 42999c025..2bd2103b9 100644 --- a/example.conf +++ b/example.conf @@ -589,6 +589,15 @@ # ignored by default. (string value) #node_not_found_hook = +# Method for storing introspection data. If set to 'none', +# introspection data will not be stored (string value) +# Allowed values: none, swift +#store_data = none + +# Name of the key to store the location of stored data in the extra +# column of the Ironic database. (string value) +#store_data_location = + [swift] diff --git a/ironic_inspector/common/swift.py b/ironic_inspector/common/swift.py index 57e061f57..bf323b8c6 100644 --- a/ironic_inspector/common/swift.py +++ b/ironic_inspector/common/swift.py @@ -13,6 +13,8 @@ # Mostly copied from ironic/common/swift.py +import json + from oslo_config import cfg from oslo_log import log from swiftclient import client as swift_client @@ -71,6 +73,8 @@ def list_opts(): CONF.register_opts(SWIFT_OPTS, group='swift') +OBJECT_NAME_PREFIX = 'inspector_data' + class SwiftAPI(object): """API for communicating with Swift.""" @@ -137,3 +141,53 @@ class SwiftAPI(object): raise utils.Error(err_msg) return obj_uuid + + def get_object(self, object, container=CONF.swift.container): + """Downloads a given object from Swift. + + :param object: The name of the object in Swift + :param container: The name of the container for the object. + :returns: Swift object + :raises: utils.Error, if the Swift operation fails. + """ + try: + headers, obj = self.connection.get_object(container, object) + except swift_exceptions.ClientException as e: + err_msg = (_('Swift failed to get object %(object)s in ' + 'container %(container)s. Error was: %(error)s') % + {'object': object, 'container': container, 'error': e}) + raise utils.Error(err_msg) + + return obj + + +def store_introspection_data(data, uuid): + """Uploads introspection data to Swift. + + :param data: data to store in Swift + :param uuid: UUID of the Ironic node that the data came from + :returns: name of the Swift object that the data is stored in + """ + swift_api = SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) + swift_object_name = '%s-%s' % (OBJECT_NAME_PREFIX, uuid) + swift_api.create_object(swift_object_name, json.dumps(data)) + return swift_object_name + + +def get_introspection_data(uuid): + """Downloads introspection data from Swift. + + :param uuid: UUID of the Ironic node that the data came from + :returns: Swift object with the introspection data + """ + swift_api = SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) + swift_object_name = '%s-%s' % (OBJECT_NAME_PREFIX, uuid) + return swift_api.get_object(swift_object_name) diff --git a/ironic_inspector/conf.py b/ironic_inspector/conf.py index 8da964388..24da3dc02 100644 --- a/ironic_inspector/conf.py +++ b/ironic_inspector/conf.py @@ -16,6 +16,7 @@ from oslo_config import cfg VALID_ADD_PORTS_VALUES = ('all', 'active', 'pxe') VALID_KEEP_PORTS_VALUES = ('all', 'present', 'added') +VALID_STORE_DATA_VALUES = ('none', 'swift') IRONIC_OPTS = [ @@ -160,7 +161,16 @@ PROCESSING_OPTS = [ default=None, help='The name of the hook to run when inspector receives ' 'inspection information from a node it isn\'t already ' - 'aware of. This hook is ignored by default.') + '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.'), + cfg.StrOpt('store_data_location', + default=None, + 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 3bf0d30f8..dee71a3ef 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -26,6 +26,7 @@ from oslo_utils import uuidutils from ironic_inspector import db from ironic_inspector.common.i18n import _, _LC, _LE, _LI, _LW +from ironic_inspector.common import swift from ironic_inspector import conf # noqa from ironic_inspector import firewall from ironic_inspector import introspect @@ -41,7 +42,7 @@ app = flask.Flask(__name__) LOG = log.getLogger('ironic_inspector.main') MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 0) +CURRENT_API_VERSION = (1, 1) _MIN_VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Minimum-Version' _MAX_VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Maximum-Version' _VERSION_HEADER = 'X-OpenStack-Ironic-Inspector-API-Version' @@ -152,6 +153,20 @@ def api_introspection(uuid): error=node_info.error or None) +@app.route('/v1/introspection//data', methods=['GET']) +@convert_exceptions +def api_introspection_data(uuid): + utils.check_auth(flask.request) + if CONF.processing.store_data == 'swift': + res = swift.get_introspection_data(uuid) + return res, 200, {'Content-Type': 'applications/json'} + else: + return error_response(_('Inspector is not configured to store data. ' + 'Set the [processing] store_data ' + 'configuration option to change this.'), + code=404) + + @app.errorhandler(404) def handle_404(error): return error_response(error, code=404) diff --git a/ironic_inspector/process.py b/ironic_inspector/process.py index 155747867..82c280f35 100644 --- a/ironic_inspector/process.py +++ b/ironic_inspector/process.py @@ -15,14 +15,17 @@ import eventlet from ironicclient import exceptions +from oslo_config import cfg from oslo_log import log from ironic_inspector.common.i18n import _, _LE, _LI +from ironic_inspector.common import swift from ironic_inspector import firewall from ironic_inspector import node_cache from ironic_inspector.plugins import base as plugins_base from ironic_inspector import utils +CONF = cfg.CONF LOG = log.getLogger("ironic_inspector.process") @@ -142,6 +145,15 @@ def _process_node(ironic, node, introspection_data, node_info): node_patches, port_patches = _run_post_hooks(node_info, introspection_data) + if CONF.processing.store_data == 'swift': + swift_object_name = swift.store_introspection_data(introspection_data, + node_info.uuid) + if CONF.processing.store_data_location: + node_patches.append({'op': 'add', + 'path': '/extra/%s' % + CONF.processing.store_data_location, + 'value': swift_object_name}) + node = ironic.node.update(node.uuid, node_patches) for mac, patches in port_patches.items(): port = node_info.ports(ironic)[mac] diff --git a/ironic_inspector/test/test_main.py b/ironic_inspector/test/test_main.py index e57761607..3b521a955 100644 --- a/ironic_inspector/test/test_main.py +++ b/ironic_inspector/test/test_main.py @@ -150,6 +150,37 @@ class TestApiGetStatus(BaseAPITest): json.loads(res.data.decode('utf-8'))) +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 = { + '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/%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'))) + + @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 + res = self.app.get('/v1/introspection/%s/data' % self.uuid) + self.assertFalse(swift_conn.get_object.called) + self.assertEqual(404, res.status_code) + + class TestApiMisc(BaseAPITest): @mock.patch.object(node_cache, 'get_node', autospec=True) def test_404_expected(self, get_mock): diff --git a/ironic_inspector/test/test_process.py b/ironic_inspector/test/test_process.py index 037d997ae..e6514d067 100644 --- a/ironic_inspector/test/test_process.py +++ b/ironic_inspector/test/test_process.py @@ -12,6 +12,7 @@ # limitations under the License. import functools +import json import time import eventlet @@ -578,6 +579,40 @@ class TestProcessNode(BaseTest): self.cli.port.delete.assert_any_call(port.uuid) self.assertEqual(2, self.cli.port.delete.call_count) + @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) + def test_store_data(self, swift_mock, filters_mock, post_hook_mock): + CONF.set_override('store_data', 'swift', 'processing') + swift_conn = swift_mock.return_value + name = 'inspector_data-%s' % self.uuid + expected = json.dumps(self.data) + + self.call() + + swift_conn.create_object.assert_called_once_with(name, expected) + self.cli.node.update.assert_called_once_with(self.uuid, + self.patch_props) + + @mock.patch.object(process.swift, 'SwiftAPI', autospec=True) + def test_store_data_location(self, swift_mock, filters_mock, + post_hook_mock): + CONF.set_override('store_data', 'swift', 'processing') + CONF.set_override('store_data_location', 'inspector_data_object', + 'processing') + swift_conn = swift_mock.return_value + name = 'inspector_data-%s' % self.uuid + self.patch_props.append( + {'path': '/extra/inspector_data_object', + 'value': name, + 'op': 'add'} + ) + expected = json.dumps(self.data) + + self.call() + + swift_conn.create_object.assert_called_once_with(name, expected) + self.cli.node.update.assert_called_once_with(self.uuid, + self.patch_props) + class TestValidateInterfacesHook(test_base.BaseTest): def test_wrong_add_ports(self): diff --git a/ironic_inspector/test/test_swift.py b/ironic_inspector/test/test_swift.py index f0f0622af..a0f29bdf5 100644 --- a/ironic_inspector/test/test_swift.py +++ b/ironic_inspector/test/test_swift.py @@ -26,14 +26,34 @@ from swiftclient import client as swift_client from swiftclient import exceptions as swift_exception from ironic_inspector.common import swift -from ironic_inspector.test import base +from ironic_inspector.test import base as test_base from ironic_inspector import utils CONF = cfg.CONF +class BaseTest(test_base.NodeTest): + def setUp(self): + super(BaseTest, self).setUp() + self.all_macs = self.macs + ['DE:AD:BE:EF:DE:AD'] + self.pxe_mac = self.macs[1] + self.data = { + 'ipmi_address': self.bmc_address, + 'cpus': 2, + 'cpu_arch': 'x86_64', + 'memory_mb': 1024, + 'local_gb': 20, + 'interfaces': { + 'em1': {'mac': self.macs[0], 'ip': '1.2.0.1'}, + 'em2': {'mac': self.macs[1], 'ip': '1.2.0.2'}, + 'em3': {'mac': self.all_macs[2]}, + }, + 'boot_interface': '01-' + self.pxe_mac.replace(':', '-'), + } + + @mock.patch.object(swift_client, 'Connection', autospec=True) -class SwiftTestCase(base.BaseTest): +class SwiftTestCase(BaseTest): def setUp(self): super(SwiftTestCase, self).setUp() @@ -54,7 +74,11 @@ class SwiftTestCase(base.BaseTest): reload_module(sys.modules['ironic_inspector.common.swift']) def test___init__(self, connection_mock): - swift.SwiftAPI() + swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) params = {'retries': 2, 'user': 'swift', 'tenant_name': 'tenant', @@ -66,7 +90,11 @@ class SwiftTestCase(base.BaseTest): connection_mock.assert_called_once_with(**params) def test_create_object(self, connection_mock): - swiftapi = swift.SwiftAPI() + swiftapi = swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) connection_obj_mock = connection_mock.return_value connection_obj_mock.put_object.return_value = 'object-uuid' @@ -80,7 +108,11 @@ class SwiftTestCase(base.BaseTest): self.assertEqual('object-uuid', object_uuid) def test_create_object_create_container_fails(self, connection_mock): - swiftapi = swift.SwiftAPI() + swiftapi = swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) connection_obj_mock = connection_mock.return_value connection_obj_mock.put_container.side_effect = self.swift_exception self.assertRaises(utils.Error, swiftapi.create_object, 'object', @@ -90,7 +122,11 @@ class SwiftTestCase(base.BaseTest): self.assertFalse(connection_obj_mock.put_object.called) def test_create_object_put_object_fails(self, connection_mock): - swiftapi = swift.SwiftAPI() + swiftapi = swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) connection_obj_mock = connection_mock.return_value connection_obj_mock.put_object.side_effect = self.swift_exception self.assertRaises(utils.Error, swiftapi.create_object, 'object', @@ -99,3 +135,33 @@ class SwiftTestCase(base.BaseTest): 'inspector') connection_obj_mock.put_object.assert_called_once_with( 'ironic-inspector', 'object', 'some-string-data', headers=None) + + def test_get_object(self, connection_mock): + swiftapi = swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) + connection_obj_mock = connection_mock.return_value + + expected_obj = self.data + connection_obj_mock.get_object.return_value = ('headers', expected_obj) + + swift_obj = swiftapi.get_object('object') + + connection_obj_mock.get_object.assert_called_once_with( + 'ironic-inspector', 'object') + self.assertEqual(expected_obj, swift_obj) + + def test_get_object_fails(self, connection_mock): + swiftapi = swift.SwiftAPI(user=CONF.swift.username, + tenant_name=CONF.swift.tenant_name, + key=CONF.swift.password, + auth_url=CONF.swift.os_auth_url, + auth_version=CONF.swift.os_auth_version) + connection_obj_mock = connection_mock.return_value + connection_obj_mock.get_object.side_effect = self.swift_exception + self.assertRaises(utils.Error, swiftapi.get_object, + 'object') + connection_obj_mock.get_object.assert_called_once_with( + 'ironic-inspector', 'object') diff --git a/plugin-requirements.txt b/plugin-requirements.txt index 9dc610c94..e69de29bb 100644 --- a/plugin-requirements.txt +++ b/plugin-requirements.txt @@ -1,2 +0,0 @@ -# required for extra_hardware plugin -python-swiftclient>=2.2.0 diff --git a/requirements.txt b/requirements.txt index 2dbc507f6..d48a98c48 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,6 +8,7 @@ keystonemiddleware>=2.0.0 pbr<2.0,>=1.6 python-ironicclient>=0.6.0 python-keystoneclient>=1.6.0 +python-swiftclient>=2.2.0 oslo.config>=2.3.0 # Apache-2.0 oslo.db>=2.4.1 # Apache-2.0 oslo.i18n>=1.5.0 # Apache-2.0