Merge "Reorganise Inventory Storage"

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
+        return inspect_utils.get_introspection_data(node, api.request.context)
-        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)
 
 
 class NodesController(rest.RestController):

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}

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")
+        inspect_utils.store_introspection_data(node, introspection_data,
-        plugin_data = introspection_data
+                                               task.context)
-        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})
 
 
 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}

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')

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)

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):
+    @mock.patch.object(inspect_utils, 'store_introspection_data',
-        CONF.set_override('data_backend', 'database',
+                       autospec=True)
-                          group='inventory')
+    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
+        fake_introspection_data = {
-        mock_get_data.return_value = {
             "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"}
+        mock_get_data.return_value = fake_introspection_data
-        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)
         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'
+        mock_store_data.assert_called_once_with(self.node,
-        swift_obj_mock.create_object_from_data.assert_has_calls([
+                                                fake_introspection_data,
-            mock.call(object_name + '-inventory', fake_inventory_data,
+                                                self.task.context)
-                      container),
-            mock.call(object_name + '-plugin', fake_plugin_data, container)])
 
     def test_status_ok_store_inventory_nostore(self, mock_client):
         CONF.set_override('data_backend', 'none', group='inventory')