From 1989d7e88bf0db1580b232e339e460b6cf64255a Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 12 May 2020 12:11:37 +0200 Subject: [PATCH] Avoid using introspection start delays with non-managed boot When boot is managed by the ironic side, a node is powered on right after starting introspection. Inspector's own delays play no role and may actually prevent introspectin from happening if a node boots faster than it's whitelisted in the PXE filter. This changes moves the delay handling later in the process and only does it when manage_boot is True. Conflicts: ironic_inspector/test/unit/test_introspect.py Change-Id: If7de8b66ea42eff2966c62a9a0529ab9a5c06f26 Story: #2007658 Task: #39745 (cherry picked from commit 3d1bf55b3594b3ea35febd003131d118139f2200) --- ironic_inspector/conf/default.py | 4 +- ironic_inspector/introspect.py | 43 ++++++++++--------- ironic_inspector/test/unit/test_introspect.py | 18 ++++++++ .../unmanaged-delay-d39871e1346d9448.yaml | 6 +++ 4 files changed, 50 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/unmanaged-delay-d39871e1346d9448.yaml diff --git a/ironic_inspector/conf/default.py b/ironic_inspector/conf/default.py index 9eaffacb1..2c54426ab 100644 --- a/ironic_inspector/conf/default.py +++ b/ironic_inspector/conf/default.py @@ -58,7 +58,9 @@ _OPTS = [ help=_('The green thread pool size.')), cfg.IntOpt('introspection_delay', default=5, - help=_('Delay (in seconds) between two introspections.')), + help=_('Delay (in seconds) between two introspections. Only ' + 'applies when boot is managed by ironic-inspector (i.e. ' + 'manage_boot==True).')), cfg.ListOpt('ipmi_address_fields', default=['ilo_address', 'drac_host', 'drac_address', 'cimc_address'], diff --git a/ironic_inspector/introspect.py b/ironic_inspector/introspect.py index ad9e295b3..25efdd679 100644 --- a/ironic_inspector/introspect.py +++ b/ironic_inspector/introspect.py @@ -64,25 +64,6 @@ def introspect(node_id, manage_boot=True, token=None): utils.executor().submit(_background_introspect, node_info, ironic) -@node_cache.release_lock -@node_cache.fsm_transition(istate.Events.wait) -def _background_introspect(node_info, ironic): - global _LAST_INTROSPECTION_TIME - - LOG.debug('Attempting to acquire lock on last introspection time') - with _LAST_INTROSPECTION_LOCK: - delay = (_LAST_INTROSPECTION_TIME - time.time() - + CONF.introspection_delay) - if delay > 0: - LOG.debug('Waiting %d seconds before sending the next ' - 'node on introspection', delay) - time.sleep(delay) - _LAST_INTROSPECTION_TIME = time.time() - - node_info.acquire_lock() - _background_introspect_locked(node_info, ironic) - - def _persistent_ramdisk_boot(node): """If the ramdisk should be configured as a persistent boot device.""" value = node.driver_info.get('force_persistent_boot_device', 'Default') @@ -92,7 +73,27 @@ def _persistent_ramdisk_boot(node): return strutils.bool_from_string(value, False) -def _background_introspect_locked(node_info, ironic): +def _wait_for_turn(node_info): + """Wait for the node's turn to be introspected.""" + global _LAST_INTROSPECTION_TIME + + LOG.debug('Attempting to acquire lock on last introspection time', + node_info=node_info) + with _LAST_INTROSPECTION_LOCK: + delay = (_LAST_INTROSPECTION_TIME - time.time() + + CONF.introspection_delay) + if delay > 0: + LOG.debug('Waiting %d seconds before sending the next ' + 'node on introspection', delay, node_info=node_info) + time.sleep(delay) + _LAST_INTROSPECTION_TIME = time.time() + + +@node_cache.release_lock +@node_cache.fsm_transition(istate.Events.wait) +def _background_introspect(node_info, ironic): + node_info.acquire_lock() + # TODO(dtantsur): pagination macs = list(node_info.ports()) if macs: @@ -121,6 +122,8 @@ def _background_introspect_locked(node_info, ironic): raise utils.Error(_('Failed to set boot device to PXE: %s') % exc, node_info=node_info) + _wait_for_turn(node_info) + try: ironic.node.set_power_state(node_info.uuid, 'reboot') except Exception as exc: diff --git a/ironic_inspector/test/unit/test_introspect.py b/ironic_inspector/test/unit/test_introspect.py index f3f452a04..6d1945e69 100644 --- a/ironic_inspector/test/unit/test_introspect.py +++ b/ironic_inspector/test/unit/test_introspect.py @@ -401,6 +401,24 @@ class TestIntrospect(BaseTest): # updated to the current time.time() self.assertEqual(42, introspect._LAST_INTROSPECTION_TIME) + @mock.patch.object(time, 'time', autospec=True) + def test_introspection_no_delay_without_manage_boot(self, time_mock, + client_mock, + start_mock): + time_mock.return_value = 42 + introspect._LAST_INTROSPECTION_TIME = 40 + CONF.set_override('introspection_delay', 10) + + self._prepare(client_mock) + start_mock.return_value = self.node_info + self.node_info.manage_boot = False + + introspect.introspect(self.uuid, manage_boot=False) + + self.assertFalse(self.sleep_fixture.mock.called) + # not updated + self.assertEqual(40, introspect._LAST_INTROSPECTION_TIME) + @mock.patch.object(time, 'time') def test_introspection_delay_not_needed(self, time_mock, client_mock, start_mock): diff --git a/releasenotes/notes/unmanaged-delay-d39871e1346d9448.yaml b/releasenotes/notes/unmanaged-delay-d39871e1346d9448.yaml new file mode 100644 index 000000000..0218a763b --- /dev/null +++ b/releasenotes/notes/unmanaged-delay-d39871e1346d9448.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + No longer uses introspection delay for nodes with ``manage_boot==False`` + (i.e. boot is managed by ironic). It is useless and may actually break + introspection if a node boots before it gets whitelisted in the PXE filter.