openstack/ironic-python-agent
Code Issues Proposed changes

Expose collector and hardware manager names via introspection data

Browse Source 
This change adds a new introspection data field 'configuration'
with two lists: managers and collectors.

Change-Id: Ice0d7e6ecff3f319bc3a4f41617059fd6914e31c
This commit is contained in:
Dmitry Tantsur 2020-01-21 15:23:18 +01:00
parent 4b602771e9
commit 31b73b4984
8 changed files with 81 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
doc/source/admin/how_it_works.rst
View File

@ -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.

2
ironic_python_agent/agent.py
View File

@ -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:

22
ironic_python_agent/hardware.py
View File

@ -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.

12
ironic_python_agent/inspector.py
View File

@ -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):

28
ironic_python_agent/tests/unit/test_agent.py
View File

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

36
ironic_python_agent/tests/unit/test_hardware.py
View File

@ -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

20
ironic_python_agent/tests/unit/test_inspector.py
View File

@ -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()

9
releasenotes/notes/inventory-conf-29b59ebe97aefbde.yaml Normal file
View File

@ -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.