Add configuration options for DISK_WAIT

https://review.openstack.org/#/c/320295/ introduced two internal
variables: _DISK_WAIT_ATTEMPTS and _DISK_WAIT_DELAY. These values are
hardcoded. This patch adds configuration options for these so
that an operator can change them based on their own needs/fleet of
hardware.

Change-Id: I2ba97669ec710fb4a435307466cd8add9c2293ba
Closes-Bug: #1585663

ironic_python_agent/config.py

@@ -128,6 +128,18 @@ cli_opts = [
                default=APARAMS.get('ipa-hardware-initialization-delay', 0),
                help='How much time (in seconds) to wait for hardware to '
                     'initialize before proceeding with any actions.'),
+
+    cfg.IntOpt('disk_wait_attempts',
+               default=APARAMS.get('ipa-disk-wait-attempts', 10),
+               help='The number of times to try and check to see if '
+                    'at least one suitable disk has appeared in inventory '
+                    'before proceeding with any actions.'),
+
+    cfg.IntOpt('disk_wait_delay',
+               default=APARAMS.get('ipa-disk-wait-delay', 3),
+               help='How much time (in seconds) to wait between attempts '
+                    'to check if at least one suitable disk has appeared '
+                    'in inventory.'),
 ]
 
 CONF.register_cli_opts(cli_opts)

ironic_python_agent/hardware.py

@@ -20,6 +20,7 @@ import time
 
 import netifaces
 from oslo_concurrency import processutils
+from oslo_config import cfg
 from oslo_log import log
 from oslo_utils import units
 import pint
@@ -34,14 +35,12 @@ from ironic_python_agent import utils
 
 _global_managers = None
 LOG = log.getLogger()
+CONF = cfg.CONF
 
 UNIT_CONVERTER = pint.UnitRegistry(filename=None)
 UNIT_CONVERTER.define('MB = []')
 UNIT_CONVERTER.define('GB = 1024 MB')
 
-_DISK_WAIT_ATTEMPTS = 10
-_DISK_WAIT_DELAY = 3
-
 NODE = None
 
 
@@ -408,21 +407,27 @@ class GenericHardwareManager(HardwareManager):
         return HardwareSupport.GENERIC
 
     def _wait_for_disks(self):
-        # Wait for at least one suitable disk to show up, otherwise neither
+        """Wait for disk to appear
-        # inspection not deployment have any chances to succeed.
+
-        for attempt in range(_DISK_WAIT_ATTEMPTS):
+        Wait for at least one suitable disk to show up, otherwise neither
+        inspection not deployment have any chances to succeed.
+
+        """
+
+        for attempt in range(CONF.disk_wait_attempts):
             try:
                 block_devices = self.list_block_devices()
                 utils.guess_root_disk(block_devices)
             except errors.DeviceNotFound:
                 LOG.debug('Still waiting for at least one disk to appear, '
-                          'attempt %d of %d', attempt + 1, _DISK_WAIT_ATTEMPTS)
+                          'attempt %d of %d', attempt + 1,
-                time.sleep(_DISK_WAIT_DELAY)
+                          CONF.disk_wait_attempts)
+                time.sleep(CONF.disk_wait_delay)
             else:
                 break
         else:
             LOG.warning('No disks detected in %d seconds',
-                        _DISK_WAIT_DELAY * _DISK_WAIT_ATTEMPTS)
+                        CONF.disk_wait_delay * CONF.disk_wait_attempts)
 
     def _get_interface_info(self, interface_name):
         addr_path = '{0}/class/net/{1}/address'.format(self.sys_path,

ironic_python_agent/tests/unit/test_hardware.py

@@ -18,6 +18,7 @@ import time
 import mock
 import netifaces
 from oslo_concurrency import processutils
+from oslo_config import cfg
 from oslo_utils import units
 from oslotest import base as test_base
 import pyudev
@@ -27,6 +28,11 @@ from ironic_python_agent import errors
 from ironic_python_agent import hardware
 from ironic_python_agent import utils
 
+CONF = cfg.CONF
+
+CONF.import_opt('disk_wait_attempts', 'ironic_python_agent.config')
+CONF.import_opt('disk_wait_delay', 'ironic_python_agent.config')
+
 HDPARM_INFO_TEMPLATE = (
     '/dev/sda:\n'
     '\n'
@@ -275,6 +281,8 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
         self.hardware = hardware.GenericHardwareManager()
         self.node = {'uuid': 'dda135fb-732d-4742-8e72-df8f3199d244',
                      'driver_internal_info': {}}
+        CONF.clear_override('disk_wait_attempts')
+        CONF.clear_override('disk_wait_delay')
 
     @mock.patch('netifaces.ifaddresses')
     @mock.patch('os.listdir')
@@ -1155,7 +1163,83 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
         self.assertEqual(hardware.HardwareSupport.GENERIC, result)
         mocked_root_dev.assert_called_with(mocked_block_dev.return_value)
         self.assertEqual(2, mocked_root_dev.call_count)
-        mocked_sleep.assert_called_once_with(hardware._DISK_WAIT_DELAY)
+        mocked_sleep.assert_called_once_with(CONF.disk_wait_delay)
+
+    @mock.patch.object(hardware.GenericHardwareManager, 'list_block_devices',
+                       autospec=True)
+    @mock.patch.object(time, 'sleep', autospec=True)
+    @mock.patch.object(utils, 'guess_root_disk', autospec=True)
+    def test_evaluate_hw_waits_for_disks_nonconfigured(self, mocked_root_dev,
+                                                       mocked_sleep,
+                                                       mocked_block_dev):
+        mocked_root_dev.side_effect = [
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            None
+        ]
+
+        self.hardware.evaluate_hardware_support()
+
+        mocked_root_dev.assert_called_with(mocked_block_dev.return_value)
+        self.assertEqual(10, mocked_root_dev.call_count)
+
+    @mock.patch.object(hardware.GenericHardwareManager, 'list_block_devices',
+                       autospec=True)
+    @mock.patch.object(time, 'sleep', autospec=True)
+    @mock.patch.object(utils, 'guess_root_disk', autospec=True)
+    def test_evaluate_hw_waits_for_disks_configured(self, mocked_root_dev,
+                                                    mocked_sleep,
+                                                    mocked_block_dev):
+        CONF.set_override('disk_wait_attempts', '2', enforce_type=True)
+
+        mocked_root_dev.side_effect = [
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            errors.DeviceNotFound('boom'),
+            None
+        ]
+
+        self.hardware.evaluate_hardware_support()
+
+        mocked_root_dev.assert_called_with(mocked_block_dev.return_value)
+        self.assertEqual(2, mocked_root_dev.call_count)
+
+    @mock.patch.object(hardware.GenericHardwareManager, 'list_block_devices',
+                       autospec=True)
+    @mock.patch.object(time, 'sleep', autospec=True)
+    @mock.patch.object(utils, 'guess_root_disk', autospec=True)
+    def test_evaluate_hw_disks_timeout_unconfigured(self, mocked_root_dev,
+                                                    mocked_sleep,
+                                                    mocked_block_dev):
+        mocked_root_dev.side_effect = errors.DeviceNotFound('boom')
+
+        self.hardware.evaluate_hardware_support()
+
+        mocked_sleep.assert_called_with(3)
+
+    @mock.patch.object(hardware.GenericHardwareManager, 'list_block_devices',
+                       autospec=True)
+    @mock.patch.object(time, 'sleep', autospec=True)
+    @mock.patch.object(utils, 'guess_root_disk', autospec=True)
+    def test_evaluate_hw_disks_timeout_configured(self, mocked_root_dev,
+                                                  mocked_sleep,
+                                                  mocked_block_dev):
+        CONF.set_override('disk_wait_delay', '5', enforce_type=True)
+        mocked_root_dev.side_effect = errors.DeviceNotFound('boom')
+
+        self.hardware.evaluate_hardware_support()
+
+        mocked_sleep.assert_called_with(5)
 
     @mock.patch.object(hardware.GenericHardwareManager, 'list_block_devices',
                        autospec=True)
@@ -1169,9 +1253,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
 
         self.assertEqual(hardware.HardwareSupport.GENERIC, result)
         mocked_root_dev.assert_called_with(mocked_block_dev.return_value)
-        self.assertEqual(hardware._DISK_WAIT_ATTEMPTS,
+        self.assertEqual(CONF.disk_wait_attempts,
                          mocked_root_dev.call_count)
-        mocked_sleep.assert_called_with(hardware._DISK_WAIT_DELAY)
+        mocked_sleep.assert_called_with(CONF.disk_wait_delay)
 
 
 @mock.patch.object(utils, 'execute', autospec=True)

releasenotes/notes/add-disk-wait-config-opts-fe805292baca8029.yaml

@@ -0,0 +1,8 @@
+---
+features:
+  - Added `ipa-disk-wait-attempts` configuration option to configure the
+    number of times to try and check to see if at least one suitable disk
+    has appeared in inventory before proceeding with any actions.
+  - Added `ipa-disk-wait-delay` configuration option to configure how many
+    seconds to wait between attempts to check if at least one suitable disk
+    has appeared in inventory.