Merge "Add hardware manager interface for hardware initialization"

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

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]' "

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},

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

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.