diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index d4ae22f101..312143500c 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -48,7 +48,7 @@ from ironic.common import states as ir_states from ironic.conductor import steps as conductor_steps import ironic.conf from ironic.drivers import base as driver_base -from ironic.drivers.modules import inspector as inspector +from ironic.drivers.modules import inspect_utils from ironic import objects @@ -1951,11 +1951,6 @@ class NodeInventoryController(rest.RestController): super(NodeInventoryController).__init__() self.node_ident = node_ident - def _node_inventory_convert(self, node_inventory): - inventory_data = node_inventory['inventory_data'] - plugin_data = node_inventory['plugin_data'] - return {"inventory": inventory_data, "plugin_data": plugin_data} - @METRICS.timer('NodeInventoryController.get') @method.expose() @args.validate(node_ident=args.uuid_or_name) @@ -1966,16 +1961,7 @@ class NodeInventoryController(rest.RestController): """ node = api_utils.check_node_policy_and_retrieve( 'baremetal:node:inventory:get', self.node_ident) - store_data = CONF.inventory.data_backend - if store_data == 'none': - raise exception.NotFound( - (_("Cannot obtain node inventory because it was not stored"))) - if store_data == 'database': - node_inventory = objects.NodeInventory.get_by_node_id( - api.request.context, node.id) - return self._node_inventory_convert(node_inventory) - if store_data == 'swift': - return inspector.get_introspection_data(node.uuid) + return inspect_utils.get_introspection_data(node, api.request.context) class NodesController(rest.RestController): diff --git a/ironic/drivers/modules/inspect_utils.py b/ironic/drivers/modules/inspect_utils.py index 89a13e658e..1340d0d7d8 100644 --- a/ironic/drivers/modules/inspect_utils.py +++ b/ironic/drivers/modules/inspect_utils.py @@ -17,9 +17,14 @@ from oslo_log import log as logging from oslo_utils import netutils from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import swift +from ironic.conf import CONF from ironic import objects +from ironic.objects import node_inventory LOG = logging.getLogger(__name__) +_OBJECT_NAME_PREFIX = 'inspector_data' def create_ports_if_not_exist(task, macs): @@ -51,3 +56,85 @@ def create_ports_if_not_exist(task, macs): except exception.MACAlreadyExists: LOG.info("Port already exists for MAC address %(address)s " "for node %(node)s", {'address': mac, 'node': node.uuid}) + + +def store_introspection_data(node, introspection_data, context): + # If store_data == 'none', do not store the data + store_data = CONF.inventory.data_backend + if store_data == 'none': + LOG.debug('Introspection data storage is disabled, the data will ' + 'not be saved for node %(node)s', {'node': node.uuid}) + return + inventory_data = introspection_data.pop("inventory") + plugin_data = introspection_data + if store_data == 'database': + node_inventory.NodeInventory( + context, + node_id=node.id, + inventory_data=inventory_data, + plugin_data=plugin_data).create() + LOG.info('Introspection data was stored in database for node ' + '%(node)s', {'node': node.uuid}) + if store_data == 'swift': + swift_object_name = _store_introspection_data_in_swift( + node_uuid=node.uuid, + inventory_data=inventory_data, + plugin_data=plugin_data) + LOG.info('Introspection data was stored for node %(node)s in Swift' + ' object %(obj_name)s-inventory and %(obj_name)s-plugin', + {'node': node.uuid, 'obj_name': swift_object_name}) + + +def _node_inventory_convert(node_inventory): + inventory_data = node_inventory['inventory_data'] + plugin_data = node_inventory['plugin_data'] + return {"inventory": inventory_data, "plugin_data": plugin_data} + + +def get_introspection_data(node, context): + store_data = CONF.inventory.data_backend + if store_data == 'none': + raise exception.NotFound( + (_("Cannot obtain node inventory because it was not stored"))) + if store_data == 'database': + node_inventory = objects.NodeInventory.get_by_node_id( + context, node.id) + return _node_inventory_convert(node_inventory) + if store_data == 'swift': + return _get_introspection_data_from_swift(node.uuid) + + +def _store_introspection_data_in_swift(node_uuid, inventory_data, plugin_data): + """Uploads introspection data to Swift. + + :param data: data to store in Swift + :param node_id: ID of the Ironic node that the data came from + :returns: name of the Swift object that the data is stored in + """ + swift_api = swift.SwiftAPI() + swift_object_name = '%s-%s' % (_OBJECT_NAME_PREFIX, node_uuid) + container = CONF.inventory.swift_data_container + swift_api.create_object_from_data(swift_object_name + '-inventory', + inventory_data, + container) + swift_api.create_object_from_data(swift_object_name + '-plugin', + plugin_data, + container) + return swift_object_name + + +def _get_introspection_data_from_swift(node_uuid): + """Uploads introspection data to Swift. + + :param data: data to store in Swift + :param node_id: ID of the Ironic node that the data came from + :returns: name of the Swift object that the data is stored in + """ + swift_api = swift.SwiftAPI() + swift_object_name = '%s-%s' % (_OBJECT_NAME_PREFIX, node_uuid) + container = CONF.inventory.swift_data_container + inventory_data = swift_api.get_object(swift_object_name + '-inventory', + container) + plugin_data = swift_api.get_object(swift_object_name + '-plugin', + container) + return {"inventory": inventory_data, "plugin_data": plugin_data} diff --git a/ironic/drivers/modules/inspector.py b/ironic/drivers/modules/inspector.py index 543eaba48c..dbf1717145 100644 --- a/ironic/drivers/modules/inspector.py +++ b/ironic/drivers/modules/inspector.py @@ -28,7 +28,6 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import keystone from ironic.common import states -from ironic.common import swift from ironic.common import utils from ironic.conductor import periodics from ironic.conductor import task_manager @@ -37,14 +36,12 @@ from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import inspect_utils -from ironic.objects import node_inventory LOG = logging.getLogger(__name__) _INSPECTOR_SESSION = None # Internal field to mark whether ironic or inspector manages boot for the node _IRONIC_MANAGES_BOOT = 'inspector_manage_boot' -_OBJECT_NAME_PREFIX = 'inspector_data' def _get_inspector_session(**kwargs): @@ -367,7 +364,6 @@ def _check_status(task): _inspection_error_handler(task, error) elif status.is_finished: _clean_up(task) - # If store_data == 'none', do not store the data store_data = CONF.inventory.data_backend if store_data == 'none': LOG.debug('Introspection data storage is disabled, the data will ' @@ -375,23 +371,8 @@ def _check_status(task): return introspection_data = inspector_client.get_introspection_data( node.uuid, processed=True) - inventory_data = introspection_data.pop("inventory") - plugin_data = introspection_data - if store_data == 'database': - node_inventory.NodeInventory( - node_id=node.id, - inventory_data=inventory_data, - plugin_data=plugin_data).create() - LOG.info('Introspection data was stored in database for node ' - '%(node)s', {'node': node.uuid}) - if store_data == 'swift': - swift_object_name = store_introspection_data( - node_uuid=node.uuid, - inventory_data=inventory_data, - plugin_data=plugin_data) - LOG.info('Introspection data was stored for node %(node)s in Swift' - ' object %(obj_name)s-inventory and %(obj_name)s-plugin', - {'node': node.uuid, 'obj_name': swift_object_name}) + inspect_utils.store_introspection_data(node, introspection_data, + task.context) def _clean_up(task): @@ -406,39 +387,3 @@ def _clean_up(task): LOG.info('Inspection finished successfully for node %s', task.node.uuid) task.process_event('done') - - -def store_introspection_data(node_uuid, inventory_data, plugin_data): - """Uploads introspection data to Swift. - - :param data: data to store in Swift - :param node_id: ID of the Ironic node that the data came from - :returns: name of the Swift object that the data is stored in - """ - swift_api = swift.SwiftAPI() - swift_object_name = '%s-%s' % (_OBJECT_NAME_PREFIX, node_uuid) - container = CONF.inventory.swift_data_container - swift_api.create_object_from_data(swift_object_name + '-inventory', - inventory_data, - container) - swift_api.create_object_from_data(swift_object_name + '-plugin', - plugin_data, - container) - return swift_object_name - - -def get_introspection_data(node_uuid): - """Uploads introspection data to Swift. - - :param data: data to store in Swift - :param node_id: ID of the Ironic node that the data came from - :returns: name of the Swift object that the data is stored in - """ - swift_api = swift.SwiftAPI() - swift_object_name = '%s-%s' % (_OBJECT_NAME_PREFIX, node_uuid) - container = CONF.inventory.swift_data_container - inventory_data = swift_api.get_object(swift_object_name + '-inventory', - container) - plugin_data = swift_api.get_object(swift_object_name + '-plugin', - container) - return {"inventory": inventory_data, "plugin_data": plugin_data} diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index b4f59be8a7..3c0049a737 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -43,6 +43,7 @@ from ironic.common import indicator_states from ironic.common import policy from ironic.common import states from ironic.conductor import rpcapi +from ironic.drivers.modules import inspect_utils from ironic.drivers.modules import inspector from ironic import objects from ironic.objects import fields as obj_fields @@ -7956,7 +7957,8 @@ class TestNodeInventory(test_api_base.BaseApiTest): self.assertEqual({'inventory': self.fake_inventory_data, 'plugin_data': self.fake_plugin_data}, ret) - @mock.patch.object(inspector, 'get_introspection_data', autospec=True) + @mock.patch.object(inspect_utils, '_get_introspection_data_from_swift', + autospec=True) def test_get_inventory_swift(self, mock_get_data): CONF.set_override('data_backend', 'swift', group='inventory') diff --git a/ironic/tests/unit/drivers/modules/test_inspect_utils.py b/ironic/tests/unit/drivers/modules/test_inspect_utils.py index 3c636a1b19..e12bbb7437 100644 --- a/ironic/tests/unit/drivers/modules/test_inspect_utils.py +++ b/ironic/tests/unit/drivers/modules/test_inspect_utils.py @@ -18,14 +18,18 @@ from unittest import mock from oslo_utils import importutils +from ironic.common import context as ironic_context from ironic.common import exception +from ironic.common import swift from ironic.conductor import task_manager from ironic.drivers.modules import inspect_utils as utils +from ironic.drivers.modules import inspector from ironic import objects from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils sushy = importutils.try_import('sushy') +CONF = inspector.CONF @mock.patch('time.sleep', lambda sec: None) @@ -88,3 +92,120 @@ class InspectFunctionTestCase(db_base.DbTestCase): mock.call(task.context, **port_dict2)] port_mock.assert_has_calls(expected_calls, any_order=True) self.assertEqual(2, port_mock.return_value.create.call_count) + + +class IntrospectionDataStorageFunctionsTestCase(db_base.DbTestCase): + fake_inventory_data = {"cpu": "amd"} + fake_plugin_data = {"disks": [{"name": "/dev/vda"}]} + + def setUp(self): + super(IntrospectionDataStorageFunctionsTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context) + + def test_store_introspection_data_db(self): + CONF.set_override('data_backend', 'database', + group='inventory') + fake_introspection_data = {'inventory': self.fake_inventory_data, + **self.fake_plugin_data} + fake_context = ironic_context.RequestContext() + utils.store_introspection_data(self.node, fake_introspection_data, + fake_context) + stored = objects.NodeInventory.get_by_node_id(self.context, + self.node.id) + self.assertEqual(self.fake_inventory_data, stored["inventory_data"]) + self.assertEqual(self.fake_plugin_data, stored["plugin_data"]) + + @mock.patch.object(utils, '_store_introspection_data_in_swift', + autospec=True) + def test_store_introspection_data_swift(self, mock_store_data): + CONF.set_override('data_backend', 'swift', group='inventory') + CONF.set_override( + 'swift_data_container', 'introspection_data', + group='inventory') + fake_introspection_data = { + "inventory": self.fake_inventory_data, **self.fake_plugin_data} + fake_context = ironic_context.RequestContext() + utils.store_introspection_data(self.node, fake_introspection_data, + fake_context) + mock_store_data.assert_called_once_with( + self.node.uuid, inventory_data=self.fake_inventory_data, + plugin_data=self.fake_plugin_data) + + def test_store_introspection_data_nostore(self): + CONF.set_override('data_backend', 'none', group='inventory') + fake_introspection_data = { + "inventory": self.fake_inventory_data, **self.fake_plugin_data} + fake_context = ironic_context.RequestContext() + ret = utils.store_introspection_data(self.node, + fake_introspection_data, + fake_context) + self.assertIsNone(ret) + + def test__node_inventory_convert(self): + required_output = {"inventory": self.fake_inventory_data, + "plugin_data": self.fake_plugin_data} + input_given = {} + input_given["inventory_data"] = self.fake_inventory_data + input_given["plugin_data"] = self.fake_plugin_data + input_given["booom"] = "boom" + ret = utils._node_inventory_convert(input_given) + self.assertEqual(required_output, ret) + + @mock.patch.object(utils, '_node_inventory_convert', autospec=True) + @mock.patch.object(objects, 'NodeInventory', spec_set=True, autospec=True) + def test_get_introspection_data_db(self, mock_inventory, mock_convert): + CONF.set_override('data_backend', 'database', + group='inventory') + fake_introspection_data = {'inventory': self.fake_inventory_data, + 'plugin_data': self.fake_plugin_data} + fake_context = ironic_context.RequestContext() + mock_inventory.get_by_node_id.return_value = fake_introspection_data + utils.get_introspection_data(self.node, fake_context) + mock_convert.assert_called_once_with(fake_introspection_data) + + @mock.patch.object(utils, '_get_introspection_data_from_swift', + autospec=True) + def test_get_introspection_data_swift(self, mock_get_data): + CONF.set_override('data_backend', 'swift', group='inventory') + CONF.set_override( + 'swift_data_container', 'introspection_data', + group='inventory') + fake_context = ironic_context.RequestContext() + utils.get_introspection_data(self.node, fake_context) + mock_get_data.assert_called_once_with( + self.node.uuid) + + def test_get_introspection_data_nostore(self): + CONF.set_override('data_backend', 'none', group='inventory') + fake_context = ironic_context.RequestContext() + self.assertRaises( + exception.NotFound, utils.get_introspection_data, + self.node, fake_context) + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test__store_introspection_data_in_swift(self, swift_api_mock): + container = 'introspection_data' + CONF.set_override('swift_data_container', container, group='inventory') + utils._store_introspection_data_in_swift( + self.node.uuid, self.fake_inventory_data, self.fake_plugin_data) + swift_obj_mock = swift_api_mock.return_value + object_name = 'inspector_data-' + str(self.node.uuid) + swift_obj_mock.create_object_from_data.assert_has_calls([ + mock.call(object_name + '-inventory', self.fake_inventory_data, + container), + mock.call(object_name + '-plugin', self.fake_plugin_data, + container)]) + + @mock.patch.object(swift, 'SwiftAPI', autospec=True) + def test__get_introspection_data_from_swift(self, swift_api_mock): + container = 'introspection_data' + CONF.set_override('swift_data_container', container, group='inventory') + swift_obj_mock = swift_api_mock.return_value + swift_obj_mock.get_object.side_effect = [ + self.fake_inventory_data, + self.fake_plugin_data + ] + ret = utils._get_introspection_data_from_swift(self.node.uuid) + req_ret = {"inventory": self.fake_inventory_data, + "plugin_data": self.fake_plugin_data} + self.assertEqual(req_ret, ret) diff --git a/ironic/tests/unit/drivers/modules/test_inspector.py b/ironic/tests/unit/drivers/modules/test_inspector.py index fcac0b6325..75ccc3ebfb 100644 --- a/ironic/tests/unit/drivers/modules/test_inspector.py +++ b/ironic/tests/unit/drivers/modules/test_inspector.py @@ -19,17 +19,14 @@ import openstack from ironic.common import context from ironic.common import exception from ironic.common import states -from ironic.common import swift from ironic.common import utils from ironic.conductor import task_manager from ironic.drivers.modules import inspect_utils from ironic.drivers.modules import inspector from ironic.drivers.modules.redfish import utils as redfish_utils -from ironic import objects from ironic.tests.unit.db import base as db_base from ironic.tests.unit.objects import utils as obj_utils - CONF = inspector.CONF @@ -554,52 +551,23 @@ class CheckStatusTestCase(BaseTestCase): self.task) self.driver.boot.clean_up_ramdisk.assert_called_once_with(self.task) - def test_status_ok_store_inventory_in_db(self, mock_client): - CONF.set_override('data_backend', 'database', - group='inventory') + @mock.patch.object(inspect_utils, 'store_introspection_data', + autospec=True) + def test_status_ok_store_inventory(self, mock_store_data, mock_client): mock_get = mock_client.return_value.get_introspection mock_get.return_value = mock.Mock(is_finished=True, error=None, spec=['is_finished', 'error']) - mock_get_data = mock_client.return_value.get_introspection_data - mock_get_data.return_value = { + fake_introspection_data = { "inventory": {"cpu": "amd"}, "disks": [{"name": "/dev/vda"}]} - inspector._check_status(self.task) - mock_get.assert_called_once_with(self.node.uuid) - mock_get_data.assert_called_once_with(self.node.uuid, processed=True) - - stored = objects.NodeInventory.get_by_node_id(self.context, - self.node.id) - self.assertEqual({"cpu": "amd"}, stored["inventory_data"]) - self.assertEqual({"disks": [{"name": "/dev/vda"}]}, - stored["plugin_data"]) - - @mock.patch.object(swift, 'SwiftAPI', autospec=True) - def test_status_ok_store_inventory_in_swift(self, - swift_api_mock, mock_client): - CONF.set_override('data_backend', 'swift', group='inventory') - CONF.set_override( - 'swift_data_container', 'introspection_data', - group='inventory') - mock_get = mock_client.return_value.get_introspection - mock_get.return_value = mock.Mock(is_finished=True, - error=None, - spec=['is_finished', 'error']) mock_get_data = mock_client.return_value.get_introspection_data - fake_inventory_data = {"cpu": "amd"} - fake_plugin_data = {"disks": [{"name": "/dev/vda"}]} - mock_get_data.return_value = { - "inventory": fake_inventory_data, **fake_plugin_data} - swift_obj_mock = swift_api_mock.return_value - object_name = 'inspector_data-' + str(self.node.uuid) + mock_get_data.return_value = fake_introspection_data inspector._check_status(self.task) mock_get.assert_called_once_with(self.node.uuid) mock_get_data.assert_called_once_with(self.node.uuid, processed=True) - container = 'introspection_data' - swift_obj_mock.create_object_from_data.assert_has_calls([ - mock.call(object_name + '-inventory', fake_inventory_data, - container), - mock.call(object_name + '-plugin', fake_plugin_data, container)]) + mock_store_data.assert_called_once_with(self.node, + fake_introspection_data, + self.task.context) def test_status_ok_store_inventory_nostore(self, mock_client): CONF.set_override('data_backend', 'none', group='inventory')