From 31b73b498453ff25275a43c46c5bfa023585327a Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 21 Jan 2020 15:23:18 +0100 Subject: [PATCH] Expose collector and hardware manager names via introspection data This change adds a new introspection data field 'configuration' with two lists: managers and collectors. Change-Id: Ice0d7e6ecff3f319bc3a4f41617059fd6914e31c --- doc/source/admin/how_it_works.rst | 6 ++++ ironic_python_agent/agent.py | 2 +- ironic_python_agent/hardware.py | 22 +++--------- ironic_python_agent/inspector.py | 12 +++++-- ironic_python_agent/tests/unit/test_agent.py | 28 +++++++-------- .../tests/unit/test_hardware.py | 36 +++++++++---------- .../tests/unit/test_inspector.py | 20 +++++++++-- .../inventory-conf-29b59ebe97aefbde.yaml | 9 +++++ 8 files changed, 81 insertions(+), 54 deletions(-) create mode 100644 releasenotes/notes/inventory-conf-29b59ebe97aefbde.yaml diff --git a/doc/source/admin/how_it_works.rst b/doc/source/admin/how_it_works.rst index 952b2798e..05bbd1a4e 100644 --- a/doc/source/admin/how_it_works.rst +++ b/doc/source/admin/how_it_works.rst @@ -106,6 +106,12 @@ collectors are: * ``inventory`` - `Hardware Inventory`_. * ``root_disk`` - The default root device for this machine, which will be used for deployment if root device hints are not provided. + * ``configuration`` - Inspection configuration, an object with two keys: + + * ``collectors`` - List of enabled collectors. + * ``managers`` - List of enabled :ref:`Hardware Managers`: items with + keys ``name`` and ``version``. + * ``boot_interface`` - Deprecated, use the ``inventory.boot.pxe_interface`` field. diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 8adb96ead..d86b950ca 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -371,7 +371,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin): self.started_at = _time() # Cached hw managers at runtime, not load time. See bug 1490008. - hardware.load_managers() + hardware.get_managers() # Operator-settable delay before hardware actually comes up. # Helps with slow RAID drivers - see bug 1582797. if self.hardware_initialization_delay > 0: diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 06cf3def0..14c2cceef 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -1847,7 +1847,7 @@ def _compare_extensions(ext1, ext2): return mgr2.evaluate_hardware_support() - mgr1.evaluate_hardware_support() -def _get_managers(): +def get_managers(): """Get a list of hardware managers in priority order. Use stevedore to find all eligible hardware managers, sort them based on @@ -1889,7 +1889,7 @@ def dispatch_to_all_managers(method, *args, **kwargs): """Dispatch a method to all hardware managers. Dispatches the given method in priority order as sorted by - `_get_managers`. If the method doesn't exist or raises + `get_managers`. If the method doesn't exist or raises IncompatibleHardwareMethodError, it continues to the next hardware manager. All managers that have hardware support for this node will be called, and their responses will be added to a dictionary of the form @@ -1905,7 +1905,7 @@ def dispatch_to_all_managers(method, *args, **kwargs): manager. """ responses = {} - managers = _get_managers() + managers = get_managers() for manager in managers: if getattr(manager, method, None): try: @@ -1934,7 +1934,7 @@ def dispatch_to_managers(method, *args, **kwargs): """Dispatch a method to best suited hardware manager. Dispatches the given method in priority order as sorted by - `_get_managers`. If the method doesn't exist or raises + `get_managers`. If the method doesn't exist or raises IncompatibleHardwareMethodError, it is attempted again with a more generic hardware manager. This continues until a method executes that returns any result without raising an IncompatibleHardwareMethodError. @@ -1947,7 +1947,7 @@ def dispatch_to_managers(method, *args, **kwargs): :raises HardwareManagerMethodNotFound: if all managers failed the method :raises HardwareManagerNotFound: if no valid hardware managers found """ - managers = _get_managers() + managers = get_managers() for manager in managers: if getattr(manager, method, None): try: @@ -1967,18 +1967,6 @@ def dispatch_to_managers(method, *args, **kwargs): raise errors.HardwareManagerMethodNotFound(method) -def load_managers(): - """Preload hardware managers into the cache. - - This method is to help warm up the cache for hardware managers when - called. Used to resolve bug 1490008, where agents can crash the first - time a hardware manager is needed. - - :raises HardwareManagerNotFound: if no valid hardware managers found - """ - _get_managers() - - def cache_node(node): """Store the node object in the hardware module. diff --git a/ironic_python_agent/inspector.py b/ironic_python_agent/inspector.py index 6b6458619..8e8b06d92 100644 --- a/ironic_python_agent/inspector.py +++ b/ironic_python_agent/inspector.py @@ -50,6 +50,11 @@ def extension_manager(names): on_missing_entrypoints_callback=_extension_manager_err_callback) +def _get_collector_names(): + return [x.strip() for x in CONF.inspection_collectors.split(',') + if x.strip()] + + def inspect(): """Optionally run inspection on the current node. @@ -72,8 +77,7 @@ def inspect(): url.rstrip('/') + '/v1/continue') config.override(params) - collector_names = [x.strip() for x in CONF.inspection_collectors.split(',') - if x.strip()] + collector_names = _get_collector_names() LOG.info('inspection is enabled with collectors %s', collector_names) # NOTE(dtantsur): inspection process tries to delay raising any exceptions @@ -219,6 +223,10 @@ def collect_default(data, failures): data['boot_interface'] = inventory['boot'].pxe_interface LOG.debug('boot devices was %s', data['boot_interface']) LOG.debug('BMC IP address: %s', inventory.get('bmc_address')) + data['configuration'] = { + 'collectors': _get_collector_names(), + 'managers': [mgr.get_version() for mgr in hardware.get_managers()], + } def collect_logs(data, failures): diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index 8a3a7c552..7aaf45c1b 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -181,8 +181,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) - @mock.patch.object(hardware, 'load_managers', autospec=True) - def test_run(self, mock_load_managers, mock_wsgi, + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_run(self, mock_get_managers, mock_wsgi, mock_wait, mock_dispatch): CONF.set_override('inspection_callback_url', '') @@ -223,8 +223,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) - @mock.patch.object(hardware, 'load_managers', autospec=True) - def test_url_from_mdns_by_default(self, mock_load_managers, mock_wsgi, + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_url_from_mdns_by_default(self, mock_get_managers, mock_wsgi, mock_wait, mock_dispatch, mock_mdns): CONF.set_override('inspection_callback_url', '') mock_mdns.return_value = 'https://example.com', {} @@ -276,8 +276,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) - @mock.patch.object(hardware, 'load_managers', autospec=True) - def test_url_from_mdns_explicitly(self, mock_load_managers, mock_wsgi, + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_url_from_mdns_explicitly(self, mock_get_managers, mock_wsgi, mock_wait, mock_dispatch, mock_mdns): CONF.set_override('inspection_callback_url', '') CONF.set_override('disk_wait_attempts', 0) @@ -335,8 +335,8 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): '_wait_for_interface', autospec=True) @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) - @mock.patch.object(hardware, 'load_managers', autospec=True) - def test_run_raise_keyboard_interrupt(self, mock_load_managers, mock_wsgi, + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_run_raise_keyboard_interrupt(self, mock_get_managers, mock_wsgi, mock_dispatch, mock_wait, mock_sleep): CONF.set_override('inspection_callback_url', '') @@ -548,14 +548,14 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): mock_dispatch.assert_has_calls(expected_dispatch_calls) mock_sleep.assert_has_calls(expected_sleep_calls) - @mock.patch.object(hardware, 'load_managers', autospec=True) + @mock.patch.object(hardware, 'get_managers', autospec=True) @mock.patch.object(time, 'sleep', autospec=True) @mock.patch.object(agent.IronicPythonAgent, '_wait_for_interface', autospec=True) @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @mock.patch('oslo_service.wsgi.Server', autospec=True) def test_run_with_sleep(self, mock_wsgi, mock_dispatch, - mock_wait, mock_sleep, mock_load_managers): + mock_wait, mock_sleep, mock_get_managers): CONF.set_override('inspection_callback_url', '') def set_serve_api(): @@ -584,7 +584,7 @@ class TestBaseAgent(ironic_agent_base.IronicAgentTest): self.agent.heartbeater.start.assert_called_once_with() mock_sleep.assert_called_once_with(10) - self.assertTrue(mock_load_managers.called) + self.assertTrue(mock_get_managers.called) self.assertTrue(mock_wait.called) self.assertEqual([mock.call('list_hardware_info'), mock.call('wait_for_disks')], @@ -712,8 +712,8 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest): @mock.patch('oslo_service.wsgi.Server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info', autospec=True) - @mock.patch.object(hardware, 'load_managers', autospec=True) - def test_run(self, mock_load_managers, mock_list_hardware, + @mock.patch.object(hardware, 'get_managers', autospec=True) + def test_run(self, mock_get_managers, mock_list_hardware, mock_wsgi): wsgi_server_request = mock_wsgi.return_value @@ -727,7 +727,7 @@ class TestAgentStandalone(ironic_agent_base.IronicAgentTest): self.agent.run() - self.assertTrue(mock_load_managers.called) + self.assertTrue(mock_get_managers.called) mock_wsgi.assert_called_once_with(CONF, 'ironic-python-agent', app=self.agent.api, host=mock.ANY, port=9999) diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 407f2fae3..af2045b66 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -928,7 +928,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertIn(if_names[0], result) self.assertEqual(expected_lldp_data, result) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @mock.patch('os.path.exists', autospec=True) @@ -944,8 +944,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_exists, mocked_listdir, mocked_ifaddresses, - mocked_get_managers): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers): + mockedget_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s @@ -969,7 +969,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertTrue(interfaces[0].has_carrier) self.assertEqual('em0', interfaces[0].biosdevname) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @mock.patch('os.path.exists', autospec=True) @@ -985,8 +985,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_exists, mocked_listdir, mocked_ifaddresses, - mocked_get_managers): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers): + mockedget_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s @@ -1061,7 +1061,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): mock_execute.assert_called_once_with('biosdevname', '-i', interface_name) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('ironic_python_agent.netutils.get_lldp_info', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @@ -1079,8 +1079,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_listdir, mocked_ifaddresses, mocked_lldp_info, - mocked_get_managers): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers): + mockedget_managers.return_value = [hardware.GenericHardwareManager()] CONF.set_override('collect_lldp', True) mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] @@ -1119,7 +1119,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): @mock.patch.object(netutils, 'interface_has_carrier', autospec=True) @mock.patch.object(netutils, 'get_mac_addr', autospec=True) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('ironic_python_agent.netutils.get_lldp_info', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @@ -1128,9 +1128,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): @mock.patch.object(utils, 'execute', autospec=True) def test_list_network_interfaces_with_lldp_error( self, mocked_execute, mocked_open, mocked_exists, mocked_listdir, - mocked_ifaddresses, mocked_lldp_info, mocked_get_managers, + mocked_ifaddresses, mocked_lldp_info, mockedget_managers, mock_get_mac, mock_has_carrier): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers.return_value = [hardware.GenericHardwareManager()] CONF.set_override('collect_lldp', True) mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] @@ -1156,7 +1156,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertTrue(interfaces[0].has_carrier) self.assertEqual('em0', interfaces[0].biosdevname) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @mock.patch('os.path.exists', autospec=True) @@ -1172,9 +1172,9 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_exists, mocked_listdir, mocked_ifaddresses, - mocked_get_managers): + mockedget_managers): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s @@ -1198,7 +1198,7 @@ class TestGenericHardwareManager(base.IronicAgentTest): self.assertIsNone(interfaces[0].vendor) self.assertEqual('em0', interfaces[0].biosdevname) - @mock.patch('ironic_python_agent.hardware._get_managers', autospec=True) + @mock.patch('ironic_python_agent.hardware.get_managers', autospec=True) @mock.patch('netifaces.ifaddresses', autospec=True) @mock.patch('os.listdir', autospec=True) @mock.patch('os.path.exists', autospec=True) @@ -1214,8 +1214,8 @@ class TestGenericHardwareManager(base.IronicAgentTest): mocked_exists, mocked_listdir, mocked_ifaddresses, - mocked_get_managers): - mocked_get_managers.return_value = [hardware.GenericHardwareManager()] + mockedget_managers): + mockedget_managers.return_value = [hardware.GenericHardwareManager()] mocked_listdir.return_value = ['lo', 'eth0'] mocked_exists.side_effect = [False, True] mocked_open.return_value.__enter__ = lambda s: s diff --git a/ironic_python_agent/tests/unit/test_inspector.py b/ironic_python_agent/tests/unit/test_inspector.py index 978ce971b..9155cf56c 100644 --- a/ironic_python_agent/tests/unit/test_inspector.py +++ b/ironic_python_agent/tests/unit/test_inspector.py @@ -226,11 +226,17 @@ class BaseDiscoverTest(base.IronicAgentTest): self.data = {} +@mock.patch.object(hardware, 'get_managers', autospec=True) @mock.patch.object(inspector, 'wait_for_dhcp', autospec=True) @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) class TestCollectDefault(BaseDiscoverTest): - def test_ok(self, mock_dispatch, mock_wait_for_dhcp): + def test_ok(self, mock_dispatch, mock_wait_for_dhcp, mock_get_mgrs): + mgrs = [{'name': 'extra', 'version': '1.42'}, + {'name': 'generic', 'version': '1.1'}] mock_dispatch.return_value = self.inventory + mock_get_mgrs.return_value = [ + mock.Mock(**{'get_version.return_value': item}) for item in mgrs + ] inspector.collect_default(self.data, self.failures) @@ -240,13 +246,21 @@ class TestCollectDefault(BaseDiscoverTest): self.assertEqual('boot:if', self.data['boot_interface']) self.assertEqual(self.inventory['disks'][2].name, self.data['root_disk'].name) + self.assertEqual({'collectors': ['default'], 'managers': mgrs}, + self.data['configuration']) mock_dispatch.assert_called_once_with('list_hardware_info') mock_wait_for_dhcp.assert_called_once_with() - def test_no_root_disk(self, mock_dispatch, mock_wait_for_dhcp): + def test_no_root_disk(self, mock_dispatch, mock_wait_for_dhcp, + mock_get_mgrs): + mgrs = [{'name': 'extra', 'version': '1.42'}, + {'name': 'generic', 'version': '1.1'}] mock_dispatch.return_value = self.inventory self.inventory['disks'] = [] + mock_get_mgrs.return_value = [ + mock.Mock(**{'get_version.return_value': item}) for item in mgrs + ] inspector.collect_default(self.data, self.failures) @@ -255,6 +269,8 @@ class TestCollectDefault(BaseDiscoverTest): self.assertEqual('boot:if', self.data['boot_interface']) self.assertNotIn('root_disk', self.data) + self.assertEqual({'collectors': ['default'], 'managers': mgrs}, + self.data['configuration']) mock_dispatch.assert_called_once_with('list_hardware_info') mock_wait_for_dhcp.assert_called_once_with() diff --git a/releasenotes/notes/inventory-conf-29b59ebe97aefbde.yaml b/releasenotes/notes/inventory-conf-29b59ebe97aefbde.yaml new file mode 100644 index 000000000..f5189a57e --- /dev/null +++ b/releasenotes/notes/inventory-conf-29b59ebe97aefbde.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + Adds a new field ``configuration`` to the introspection data collected by + the ``default`` collector. It contains two fields: + + * ``collectors`` - list of the enabled inspection collectors. + * ``managers`` - list of the enabled hardware managers in their priority + order.