Merge "Revert "Add hardware manager interface for hardware initialization""

ironic_python_agent/agent.py

@@ -289,8 +289,6 @@ 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,7 +16,6 @@ import abc
 import functools
 import os
 import shlex
-import time
 
 import netifaces
 from oslo_concurrency import processutils
@@ -39,9 +38,6 @@ 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."""
@@ -389,57 +385,17 @@ 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)
@@ -782,6 +738,11 @@ 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,12 +167,9 @@ 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()
@@ -196,15 +193,12 @@ 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_init_hardware, mocked_inspector):
+                                 mocked_inspector):
         CONF.set_override('inspection_callback_url', 'http://foo/bar',
                           enforce_type=True)
 
@@ -237,7 +231,6 @@ 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,11 +12,9 @@
 # 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
@@ -1087,66 +1085,6 @@ 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

@@ -1,7 +0,0 @@
----
-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.