From 13a8c6321ef466505e815bfad34912f371d08d77 Mon Sep 17 00:00:00 2001 From: Yosef Hoffman Date: Thu, 2 Jun 2016 13:34:10 -0400 Subject: [PATCH] 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 | 12 +++ ironic_python_agent/hardware.py | 23 +++-- .../tests/unit/test_hardware.py | 90 ++++++++++++++++++- ...isk-wait-config-opts-fe805292baca8029.yaml | 8 ++ 4 files changed, 121 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/add-disk-wait-config-opts-fe805292baca8029.yaml diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 482174a52..eae43b7a7 100644 --- a/ironic_python_agent/config.py +++ b/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) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index 0e1eda091..e44a73d6e 100644 --- a/ironic_python_agent/hardware.py +++ b/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 - # inspection not deployment have any chances to succeed. - for attempt in range(_DISK_WAIT_ATTEMPTS): + """Wait for disk to appear + + 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) - time.sleep(_DISK_WAIT_DELAY) + 'attempt %d of %d', attempt + 1, + 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, diff --git a/ironic_python_agent/tests/unit/test_hardware.py b/ironic_python_agent/tests/unit/test_hardware.py index a05bdbd71..671de9682 100644 --- a/ironic_python_agent/tests/unit/test_hardware.py +++ b/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) diff --git a/releasenotes/notes/add-disk-wait-config-opts-fe805292baca8029.yaml b/releasenotes/notes/add-disk-wait-config-opts-fe805292baca8029.yaml new file mode 100644 index 000000000..4b6335efc --- /dev/null +++ b/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.