diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 75a7e8aab..79de402ed 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -289,6 +289,8 @@ class IronicPythonAgent(base.ExecuteCommandMixin): # Cached hw managers at runtime, not load time. See bug 1490008. hardware.load_managers() + # Request the hw manager to do long initializations + hardware.dispatch_to_managers('initialize_hardware') if not self.standalone: # Inspection should be started before call to lookup, otherwise diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 3e74d350d..cbd3da318 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -16,6 +16,7 @@ import abc import functools import os import shlex +import time import netifaces from oslo_concurrency import processutils @@ -38,6 +39,9 @@ UNIT_CONVERTER = pint.UnitRegistry(filename=None) UNIT_CONVERTER.define('MB = []') UNIT_CONVERTER.define('GB = 1024 MB') +_DISK_WAIT_ATTEMPTS = 5 +_DISK_WAIT_DELAY = 3 + def _get_device_vendor(dev): """Get the vendor name of a given device.""" @@ -385,17 +389,57 @@ class HardwareManager(object): 'version': getattr(self, 'HARDWARE_MANAGER_VERSION', '1.0') } + def initialize_hardware(self): + """Initialize hardware on the agent start up. + + This method will be called once on start up before any calls + to list_hardware_info are made. + + The default implementation does nothing. + """ + class GenericHardwareManager(HardwareManager): HARDWARE_MANAGER_NAME = 'generic_hardware_manager' HARDWARE_MANAGER_VERSION = '1.0' + # These modules are rarely loaded automatically + _PRELOADED_MODULES = ['ipmi_msghandler', 'ipmi_devintf', 'ipmi_si'] + def __init__(self): self.sys_path = '/sys' def evaluate_hardware_support(self): return HardwareSupport.GENERIC + def initialize_hardware(self): + LOG.debug('Initializing hardware') + self._preload_modules() + _udev_settle() + self._wait_for_disks() + + def _preload_modules(self): + # TODO(dtantsur): try to load as many kernel modules for present + # hardware as it's possible. + for mod in self._PRELOADED_MODULES: + utils.try_execute('modprobe', mod) + + def _wait_for_disks(self): + # Wait for at least one suitable disk to show up, otherwise neither + # inspection not deployment have any chances to succeed. + for attempt in range(_DISK_WAIT_ATTEMPTS): + try: + self.get_os_install_device() + except errors.DeviceNotFound: + LOG.debug('Still waiting for at least one disk to appear, ' + 'attempt %d of %d', attempt + 1, _DISK_WAIT_ATTEMPTS) + time.sleep(_DISK_WAIT_DELAY) + else: + break + else: + LOG.warning('No disks detected in %d seconds', + _DISK_WAIT_DELAY * _DISK_WAIT_ATTEMPTS) + def _get_interface_info(self, interface_name): addr_path = '{0}/class/net/{1}/address'.format(self.sys_path, interface_name) @@ -738,11 +782,6 @@ class GenericHardwareManager(HardwareManager): return True def get_bmc_address(self): - # These modules are rarely loaded automatically - utils.try_execute('modprobe', 'ipmi_msghandler') - utils.try_execute('modprobe', 'ipmi_devintf') - utils.try_execute('modprobe', 'ipmi_si') - try: out, _e = utils.execute( "ipmitool lan print | grep -e 'IP Address [^S]' " diff --git a/ironic_python_agent/tests/unit/test_agent.py b/ironic_python_agent/tests/unit/test_agent.py index b51e68518..db4f5b30b 100644 --- a/ironic_python_agent/tests/unit/test_agent.py +++ b/ironic_python_agent/tests/unit/test_agent.py @@ -167,9 +167,12 @@ class TestBaseAgent(test_base.BaseTestCase): self.assertEqual(pkg_resources.get_distribution('ironic-python-agent') .version, status.version) + @mock.patch.object(hardware.GenericHardwareManager, 'initialize_hardware', + autospec=True) @mock.patch('wsgiref.simple_server.make_server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info') - def test_run(self, mocked_list_hardware, wsgi_server_cls): + def test_run(self, mocked_list_hardware, wsgi_server_cls, + mocked_init_hardware): CONF.set_override('inspection_callback_url', '', enforce_type=True) wsgi_server = wsgi_server_cls.return_value wsgi_server.start.side_effect = KeyboardInterrupt() @@ -193,12 +196,15 @@ class TestBaseAgent(test_base.BaseTestCase): wsgi_server.serve_forever.assert_called_once_with() self.agent.heartbeater.start.assert_called_once_with() + mocked_init_hardware.assert_called_once_with(mock.ANY) @mock.patch.object(inspector, 'inspect', autospec=True) + @mock.patch.object(hardware.GenericHardwareManager, 'initialize_hardware', + autospec=True) @mock.patch('wsgiref.simple_server.make_server', autospec=True) @mock.patch.object(hardware.HardwareManager, 'list_hardware_info') def test_run_with_inspection(self, mocked_list_hardware, wsgi_server_cls, - mocked_inspector): + mocked_init_hardware, mocked_inspector): CONF.set_override('inspection_callback_url', 'http://foo/bar', enforce_type=True) @@ -231,6 +237,7 @@ class TestBaseAgent(test_base.BaseTestCase): self.agent.api_client.lookup_node.call_args[1]['node_uuid']) self.agent.heartbeater.start.assert_called_once_with() + mocked_init_hardware.assert_called_once_with(mock.ANY) def test_async_command_success(self): result = base.AsyncCommandResult('foo_command', {'fail': False}, diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index 5c0b60814..bbe88d2fb 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/ironic_python_agent/tests/unit/test_hardware.py @@ -12,9 +12,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os +import time + import mock import netifaces -import os from oslo_concurrency import processutils from oslo_utils import units from oslotest import base as test_base @@ -1085,6 +1087,66 @@ class TestGenericHardwareManager(test_base.BaseTestCase): self.hardware.get_system_vendor_info().manufacturer) +@mock.patch.object(time, 'sleep', autospec=True) +@mock.patch.object(hardware.GenericHardwareManager, + 'get_os_install_device', autospec=True) +@mock.patch.object(utils, 'execute', autospec=True) +class TestGenericHardwareManagerInitializeHardware(test_base.BaseTestCase): + def setUp(self): + super(TestGenericHardwareManagerInitializeHardware, self).setUp() + self.hardware = hardware.GenericHardwareManager() + + def test_ok(self, mocked_execute, mocked_os_dev, mocked_sleep): + self.hardware.initialize_hardware() + + expected_execute_calls = [ + mock.call('modprobe', mod) + for mod in self.hardware._PRELOADED_MODULES + ] + expected_execute_calls.append(mock.call('udevadm', 'settle')) + self.assertEqual(expected_execute_calls, mocked_execute.call_args_list) + mocked_os_dev.assert_called_once_with(mock.ANY) + self.assertFalse(mocked_sleep.called) + + def test_disk_delayed(self, mocked_execute, mocked_os_dev, mocked_sleep): + mocked_os_dev.side_effect = [ + errors.DeviceNotFound(''), + errors.DeviceNotFound(''), + None + ] + + self.hardware.initialize_hardware() + + expected_execute_calls = [ + mock.call('modprobe', mod) + for mod in self.hardware._PRELOADED_MODULES + ] + expected_execute_calls.append(mock.call('udevadm', 'settle')) + self.assertEqual(expected_execute_calls, mocked_execute.call_args_list) + mocked_os_dev.assert_called_with(mock.ANY) + self.assertEqual(3, mocked_os_dev.call_count) + self.assertEqual(2, mocked_sleep.call_count) + + @mock.patch.object(hardware.LOG, 'warning', autospec=True) + def test_no_disk(self, mocked_warn, mocked_execute, mocked_os_dev, + mocked_sleep): + mocked_os_dev.side_effect = errors.DeviceNotFound('') + + self.hardware.initialize_hardware() + + expected_execute_calls = [ + mock.call('modprobe', mod) + for mod in self.hardware._PRELOADED_MODULES + ] + expected_execute_calls.append(mock.call('udevadm', 'settle')) + self.assertEqual(expected_execute_calls, mocked_execute.call_args_list) + mocked_os_dev.assert_called_with(mock.ANY) + self.assertEqual(hardware._DISK_WAIT_ATTEMPTS, + mocked_os_dev.call_count) + self.assertEqual(hardware._DISK_WAIT_ATTEMPTS, mocked_sleep.call_count) + self.assertTrue(mocked_warn.called) + + @mock.patch.object(utils, 'execute', autospec=True) class TestModuleFunctions(test_base.BaseTestCase): diff --git a/releasenotes/notes/initialize-hardware-db00d62c3669be93.yaml b/releasenotes/notes/initialize-hardware-db00d62c3669be93.yaml new file mode 100644 index 000000000..ab5008bab --- /dev/null +++ b/releasenotes/notes/initialize-hardware-db00d62c3669be93.yaml @@ -0,0 +1,7 @@ +--- +features: + - Add a new hardware manager method "initialize_hardware" which can be + overriden to provide early oneshot hardware initialization, such as loading + kernel modules or probing hardware. The generic implementation loads the + IPMI modules, calls "udev settle" and waits for at least one suitable disk + to appear in the "lsblk" output.