openstack/ironic
Code Issues Proposed changes

remove explicit eventlet usage around time.sleep

Browse Source 
One of the challeges with eventlet is it has helped make some of
our tests run nice and quick because time.sleep ends up getting replaced
deep under the hood when we monkey patch.

In order for us to get to a point where we can begin to consider removing
the patch, the tests need to begin to change. To that end, we have some
test reliance upon that patching and also calling helpers and then testing
the helper in the driver tests as well which increases the test time, and
really is sort of "excess" complexity.

So the goal here is to see if folks are good with such minor changes,
since we can do them now and should try to avoid making changes to these
tests when we begin to really remove eventlet.

As a note, this is the first of two, or possibly three patches to do this
overall cleanup.

Change-Id: I28a36ed1facdf4fd6ced2efceacb39d12de72e8f
This commit is contained in:
Julia Kreger
2025-05-13 16:28:54 -07:00
parent 10590b36f5
commit 5731f4fafc
14 changed files with 32 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ironic/common/console_factory.py
-2
View File
@@ -31,8 +31,6 @@ class ConsoleContainerFactory(object):
if not ConsoleContainerFactory._provider:
ConsoleContainerFactory._set_provider(**kwargs)
# Use lockutils to avoid a potential race in eventlet
# that might try to create two factories.
@classmethod
@lockutils.synchronized(EM_SEMAPHORE)
def _set_provider(cls, **kwargs):
ironic/common/dhcp_factory.py
-2
View File
@@ -34,8 +34,6 @@ class DHCPFactory(object):
if not DHCPFactory._dhcp_provider:
DHCPFactory._set_dhcp_provider(**kwargs)
# NOTE(lucasagomes): Use lockutils to avoid a potential race in eventlet
# that might try to create two dhcp factories.
@classmethod
@lockutils.synchronized(EM_SEMAPHORE)
def _set_dhcp_provider(cls, **kwargs):
ironic/conductor/base_manager.py
+2 -2
View File
@@ -15,8 +15,8 @@
import copy
import inspect
import threading
import time
import eventlet
import futurist
from futurist import periodics
from futurist import rejection
@@ -662,7 +662,7 @@ class BaseConductorManager(object):
continue
finally:
# Yield on every iteration
eventlet.sleep(0)
time.sleep(0)
def _resume_allocations(self, context):
"""Resume unfinished allocations on restart."""
ironic/conductor/manager.py
+4 -4
View File
@@ -42,8 +42,8 @@ notifying Neutron of a change, etc.
import collections
import queue
import time
import eventlet
from futurist import waiters
from oslo_log import log
import oslo_messaging as messaging
@@ -1568,7 +1568,7 @@ class ConductorManager(base_manager.BaseConductorManager):
{'node': node_uuid})
finally:
# Yield on every iteration
eventlet.sleep(0)
time.sleep(0)
@METRICS.timer('ConductorManager._power_failure_recovery')
@periodics.node_periodic(
@@ -2734,7 +2734,7 @@ class ConductorManager(base_manager.BaseConductorManager):
context, ev_type, message)
finally:
# Yield on every iteration
eventlet.sleep(0)
time.sleep(0)
def _sensors_conductor(self, context):
"""Called to collect and send metrics "sensors" for the conductor."""
@@ -3716,7 +3716,7 @@ class ConductorManager(base_manager.BaseConductorManager):
# Yield to other threads, since we also don't want to be
# looping tightly deleting rows as that will negatively
# impact DB access if done in excess.
eventlet.sleep(0)
time.sleep(0)
def _concurrent_action_limit(self, action):
"""Check Concurrency limits and block operations if needed.
ironic/conductor/periodics.py
+2 -2
View File
@@ -15,8 +15,8 @@
import collections
import functools
import inspect
import time
import eventlet
from futurist import periodics
from oslo_log import log
@@ -159,7 +159,7 @@ def node_periodic(purpose, spacing, enabled=True, filters=None,
break
finally:
# Yield on every iteration
eventlet.sleep(0)
time.sleep(0)
if (local_limit is not None
and (result is None or result)):
ironic/tests/base.py
-2
View File
@@ -28,8 +28,6 @@ import sys
import tempfile
import warnings
import eventlet
eventlet.monkey_patch(os=False)
import fixtures
from oslo_concurrency import processutils
from oslo_config import fixture as config_fixture
ironic/tests/unit/conductor/test_manager.py
+2 -2
View File
@@ -22,9 +22,9 @@ from collections import namedtuple
import datetime
import queue
import re
import time
from unittest import mock
import eventlet
from futurist import waiters
from oslo_config import cfg
import oslo_messaging as messaging
@@ -6096,7 +6096,7 @@ class ManagerSyncPowerStatesTestCase(mgr_utils.CommonMixIn,
acquire_mock.side_effect = self._get_acquire_side_effect(tasks)
sync_mock.side_effect = sync_results
with mock.patch.object(eventlet, 'sleep', autospec=True) as sleep_mock:
with mock.patch.object(time, 'sleep', autospec=True) as sleep_mock:
self.service._sync_power_states(self.context)
# Ensure we've yielded on every iteration, except for node
# not mapped to this conductor
ironic/tests/unit/drivers/modules/irmc/test_power.py
-6
View File
@@ -52,8 +52,6 @@ class IRMCPowerInternalMethodsTestCase(test_common.BaseIRMCTest):
self.assertFalse(irmc_power._is_expected_power_state(
target_state, boot_status_value))
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
@mock.patch('ironic.drivers.modules.irmc.power.snmp.SNMPClient',
spec_set=True, autospec=True)
def test__wait_power_state_soft_power_off(self, snmpclient_mock):
@@ -72,8 +70,6 @@ class IRMCPowerInternalMethodsTestCase(test_common.BaseIRMCTest):
self.assertEqual(states.POWER_OFF, task.node.power_state)
self.assertEqual(states.NOSTATE, task.node.target_power_state)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
@mock.patch('ironic.drivers.modules.irmc.power.snmp.SNMPClient',
spec_set=True, autospec=True)
def test__wait_power_state_soft_reboot(self, snmpclient_mock):
@@ -92,8 +88,6 @@ class IRMCPowerInternalMethodsTestCase(test_common.BaseIRMCTest):
self.assertEqual(states.POWER_ON, task.node.power_state)
self.assertEqual(states.NOSTATE, task.node.target_power_state)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
@mock.patch('ironic.drivers.modules.irmc.power.snmp.SNMPClient',
spec_set=True, autospec=True)
def test__wait_power_state_timeout(self, snmpclient_mock):
ironic/tests/unit/drivers/modules/redfish/test_bios.py
-4
View File
@@ -40,8 +40,6 @@ class NoBiosSystem(object):
resource=self)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishBiosTestCase(db_base.DbTestCase):
def setUp(self):
@@ -546,8 +544,6 @@ class RedfishBiosTestCase(db_base.DbTestCase):
apply_time=None)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishBiosRegistryTestCase(db_base.DbTestCase):
def setUp(self):
ironic/tests/unit/drivers/modules/redfish/test_boot.py
-6
View File
@@ -34,8 +34,6 @@ from ironic.tests.unit.objects import utils as obj_utils
INFO_DICT = db_utils.get_test_redfish_info()
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
def setUp(self):
@@ -1713,8 +1711,6 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase):
[mock_manager], sushy.VIRTUAL_MEDIA_FLOPPY, inserted=True))
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishHTTPBootTestCase(db_base.DbTestCase):
def setUp(self):
@@ -2563,8 +2559,6 @@ class RedfishHTTPBootTestCase(db_base.DbTestCase):
mock_cleanup_disk_image.assert_not_called()
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishVirtualMediaBootViaSystemTestCase(db_base.DbTestCase):
def setUp(self):
ironic/tests/unit/drivers/modules/redfish/test_firmware.py
-2
View File
@@ -34,8 +34,6 @@ from ironic.tests.unit.objects import utils as obj_utils
INFO_DICT = db_utils.get_test_redfish_info()
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
class RedfishFirmwareTestCase(db_base.DbTestCase):
def setUp(self):
ironic/tests/unit/drivers/modules/redfish/test_raid.py
-2
View File
@@ -62,8 +62,6 @@ def _mock_volume(identity, raid_type=None,
return volume
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait',
lambda *args, **kwargs: None)
@mock.patch.object(redfish_utils, 'get_system', autospec=True)
class RedfishRAIDTestCase(db_base.DbTestCase):
ironic/tests/unit/drivers/modules/test_ipmitool.py
+5 -10
View File
@@ -1800,8 +1800,7 @@ class IPMIToolPrivateMethodTestCase(
kill_on_timeout=True)
@mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True)
def test__power_on_max_retries(self, sleep_mock, mock_exec):
def test__power_on_max_retries(self, mock_exec):
self.config(command_retry_timeout=2, group='ipmi')
def side_effect(driver_info, command, **kwargs):
@@ -1823,8 +1822,7 @@ class IPMIToolPrivateMethodTestCase(
self.assertEqual(expected, mock_exec.call_args_list)
@mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True)
def test__soft_power_off(self, sleep_mock, mock_exec):
def test__soft_power_off(self, mock_exec):
def side_effect(driver_info, command, **kwargs):
resp_dict = {"power status": ["Chassis Power is off\n", None],
"power soft": [None, None]}
@@ -1842,8 +1840,7 @@ class IPMIToolPrivateMethodTestCase(
self.assertEqual(states.POWER_OFF, state)
@mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True)
def test__soft_power_off_max_retries(self, sleep_mock, mock_exec):
def test__soft_power_off_max_retries(self, mock_exec):
def side_effect(driver_info, command, **kwargs):
resp_dict = {"power status": ["Chassis Power is on\n", None],
@@ -1864,8 +1861,7 @@ class IPMIToolPrivateMethodTestCase(
self.assertEqual(expected, mock_exec.call_args_list)
@mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True)
def test__set_and_wait_explicit_reboot(self, sleep_mock, mock_exec):
def test__set_and_wait_explicit_reboot(self, mock_exec):
def side_effect(driver_info, command, **kwargs):
resp_dict = {"power status": ["Chassis Power is on\n", None],
"power reset": [None, None]}
@@ -1885,9 +1881,8 @@ class IPMIToolPrivateMethodTestCase(
@mock.patch.object(ipmi, '_power_status', autospec=True)
@mock.patch.object(ipmi, '_exec_ipmitool', autospec=True)
@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', autospec=True)
def test___set_and_wait_no_needless_status_polling(
self, sleep_mock, mock_exec, mock_status):
self, mock_exec, mock_status):
# Check that if the call to power state change fails, it doesn't
# call power_status().
self.config(command_retry_timeout=2, group='ipmi')
ironic/tests/unit/drivers/modules/test_snmp.py
+17 -23
View File
@@ -816,7 +816,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_called_once_with(driver._snmp_oid())
self.assertEqual(states.POWER_OFF, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
@mock.patch.object(time, 'sleep', autospec=True)
def test_power_on_delay(self, mock_sleep, mock_get_client):
# Ensure driver waits for the state to change following a power on
mock_client = mock_get_client.return_value
@@ -830,7 +830,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_ON, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
@mock.patch.object(time, 'sleep', autospec=True)
def test_power_off_delay(self, mock_sleep, mock_get_client):
# Ensure driver waits for the state to change following a power off
mock_client = mock_get_client.return_value
@@ -838,14 +838,14 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.side_effect = [driver.value_power_on,
driver.value_power_off]
pstate = driver.power_off()
self.assertTrue(mock_sleep.called)
mock_client.set.assert_called_once_with(driver._snmp_oid(),
driver.value_power_off)
calls = [mock.call(driver._snmp_oid())] * 2
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_OFF, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_on_invalid_state(self, mock_sleep, mock_get_client):
def test_power_on_invalid_state(self, mock_get_client):
# Ensure driver retries when querying unexpected states following a
# power on
mock_client = mock_get_client.return_value
@@ -859,8 +859,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.ERROR, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_off_invalid_state(self, mock_sleep, mock_get_client):
def test_power_off_invalid_state(self, mock_get_client):
# Ensure driver retries when querying unexpected states following a
# power off
mock_client = mock_get_client.return_value
@@ -920,8 +919,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
driver.value_power_off)
mock_client.get.assert_called_once_with(driver._snmp_oid())
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_on_timeout(self, mock_sleep, mock_get_client):
def test_power_on_timeout(self, mock_get_client):
# Ensure that a power on consistency poll timeout causes an error
mock_client = mock_get_client.return_value
driver = snmp._get_driver(self.node)
@@ -934,11 +932,10 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.ERROR, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_off_timeout(self, mock_sleep, mock_get_client):
def test_power_off_timeout(self, mock_get_client):
# Ensure that a power off consistency poll timeout causes an error
mock_client = mock_get_client.return_value
CONF.set_override('power_timeout', 5, 'snmp')
CONF.set_override('power_timeout', 1, 'snmp')
driver = snmp._get_driver(self.node)
mock_client.get.return_value = driver.value_power_on
pstate = driver.power_off()
@@ -963,8 +960,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_ON, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_reset_off_delay(self, mock_sleep, mock_get_client):
def test_power_reset_off_delay(self, mock_get_client):
# Ensure driver waits for the power off state change following a power
# reset
mock_client = mock_get_client.return_value
@@ -980,7 +976,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_ON, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
@mock.patch.object(time, 'sleep', autospec=True)
def test_power_reset_on_delay(self, mock_sleep, mock_get_client):
# Ensure driver waits for the power on state change following a power
# reset
@@ -990,6 +986,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
driver.value_power_off,
driver.value_power_on]
pstate = driver.power_reset()
self.assertTrue(mock_sleep.called)
calls = [mock.call(driver._snmp_oid(), driver.value_power_off),
mock.call(driver._snmp_oid(), driver.value_power_on)]
mock_client.set.assert_has_calls(calls)
@@ -997,7 +994,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_ON, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
@mock.patch.object(time, 'sleep', autospec=True)
def test_power_reset_off_delay_on_delay(self, mock_sleep, mock_get_client):
# Ensure driver waits for both state changes following a power reset
mock_client = mock_get_client.return_value
@@ -1007,6 +1004,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
driver.value_power_off,
driver.value_power_on]
pstate = driver.power_reset()
self.assertTrue(mock_sleep.called)
calls = [mock.call(driver._snmp_oid(), driver.value_power_off),
mock.call(driver._snmp_oid(), driver.value_power_on)]
mock_client.set.assert_has_calls(calls)
@@ -1014,8 +1012,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.POWER_ON, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_reset_off_invalid_state(self, mock_sleep, mock_get_client):
def test_power_reset_off_invalid_state(self, mock_get_client):
# Ensure driver retries when querying unexpected states following a
# power off during a reset
mock_client = mock_get_client.return_value
@@ -1029,8 +1026,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.ERROR, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_reset_on_invalid_state(self, mock_sleep, mock_get_client):
def test_power_reset_on_invalid_state(self, mock_get_client):
# Ensure driver retries when querying unexpected states following a
# power on during a reset
mock_client = mock_get_client.return_value
@@ -1046,8 +1042,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.ERROR, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_reset_off_timeout(self, mock_sleep, mock_get_client):
def test_power_reset_off_timeout(self, mock_get_client):
# Ensure that a power off consistency poll timeout during a reset
# causes an error
mock_client = mock_get_client.return_value
@@ -1061,8 +1056,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase):
mock_client.get.assert_has_calls(calls)
self.assertEqual(states.ERROR, pstate)
@mock.patch("oslo_utils.eventletutils.EventletEvent.wait", autospec=True)
def test_power_reset_on_timeout(self, mock_sleep, mock_get_client):
def test_power_reset_on_timeout(self, mock_get_client):
# Ensure that a power on consistency poll timeout during a reset
# causes an error
mock_client = mock_get_client.return_value