From 7733488f001739d1a37e2be4b9c1c6632c669d53 Mon Sep 17 00:00:00 2001 From: Eduardo Olivares Date: Wed, 6 Mar 2024 13:13:00 +0100 Subject: [PATCH] Refactoring tobiko lockers Tobiko has used the oslo_concurrency.lockutils.synchronized to lock execution of functions inter-process/worker. The synchronized decorator does not support to disable intra-worker locks. This is a problem for tobiko because there are some functions that need to be locked between different workers, but not inside a worker. Why? - Tobiko execution (based on pytest) is multi-worker, but not multi-thread. - The creation of Tobiko resources (which is what we need to lock to avoid concurrency issues between workers) sometimes depends on the creation of other Tobiko resources that are defined in either parent or child classes from the first ones, getting blocked due to the locks applied. Change-Id: I0b1c2d707b585fd4e45cad9968c88cedf2932eed --- lower-constraints.txt | 1 + requirements.txt | 1 + tobiko/__init__.py | 4 +- tobiko/common/_lockutils.py | 108 ++++++++++++++++++ tobiko/openstack/stacks/_cirros.py | 11 +- tobiko/openstack/stacks/_l3ha.py | 5 +- tobiko/openstack/stacks/_neutron.py | 17 +-- tobiko/openstack/stacks/_nova.py | 5 +- tobiko/openstack/stacks/_qos.py | 5 +- tobiko/openstack/stacks/_ubuntu.py | 8 -- tobiko/openstack/stacks/_vlan.py | 4 +- .../openstack/stacks/test_neutron.py | 5 +- tobiko/tests/scenario/neutron/test_router.py | 4 +- 13 files changed, 129 insertions(+), 49 deletions(-) create mode 100644 tobiko/common/_lockutils.py diff --git a/lower-constraints.txt b/lower-constraints.txt index e783e2cf5..276d29cd4 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -12,6 +12,7 @@ openstacksdk==0.31.2 oslo.concurrency==3.26.0 oslo.config==8.4.0 oslo.log==4.4.0 +oslo.utils==4.12.3 packaging==20.4 paramiko==2.9.2 pbr==5.5.1 diff --git a/requirements.txt b/requirements.txt index 18ed331a1..342567a8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -12,6 +12,7 @@ openstacksdk>=0.31.2 # Apache-2.0 oslo.concurrency>=3.26.0 # Apache-2.0 oslo.config>=8.4.0 # Apache-2.0 oslo.log>=4.4.0 # Apache-2.0 +oslo.utils>=4.12.3 # Apache-2.0 packaging>=20.4 # Apache-2.0 paramiko>=2.9.2 # LGPLv2.1 pbr>=5.5.1 # Apache-2.0 diff --git a/tobiko/__init__.py b/tobiko/__init__.py index 8d4f018e5..8e6ebe1b2 100644 --- a/tobiko/__init__.py +++ b/tobiko/__init__.py @@ -24,6 +24,7 @@ from tobiko.common import _exception from tobiko.common import _fixture from tobiko.common import _ini from tobiko.common import _loader +from tobiko.common import _lockutils from tobiko.common import _logging from tobiko.common import _operation from tobiko.common import _os @@ -104,6 +105,8 @@ CaptureLogFixture = _logging.CaptureLogFixture load_object = _loader.load_object load_module = _loader.load_module +interworker_synched = _lockutils.interworker_synched + makedirs = _os.makedirs open_output_file = _os.open_output_file @@ -171,4 +174,3 @@ load_yaml = _yaml.load_yaml from tobiko import config # noqa config.init_config() -LOCK_DIR = os.path.expanduser(config.CONF.tobiko.common.lock_dir) diff --git a/tobiko/common/_lockutils.py b/tobiko/common/_lockutils.py new file mode 100644 index 000000000..43566e556 --- /dev/null +++ b/tobiko/common/_lockutils.py @@ -0,0 +1,108 @@ +# Copyright (c) 2024 Red Hat, Inc. +# +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +from __future__ import absolute_import + +import contextlib +import functools +import os + +from oslo_concurrency import lockutils +from oslo_log import log +from oslo_utils import reflection +from oslo_utils import timeutils + + +LOG = log.getLogger(__name__) + + +def interworker_synched(name): + """Re-definition of oslo_concurrency.lockutils.synchronized. + + Tobiko needs to re-difine this decorator in order to avoid intra- + process/worker locks. This is because tobiko is executed in multiple + processes (using pytest), but each process only runs one thread. + + The intra-process lock should not be applied in tobiko because some of the + locked methods could be called recurrently. + Example: + The creation (setup_fixture) of CirrosPeerServerStackFixture depends on the + creation of CirrosServerStackFixture, which is also its parent class. + With intra-process locks, the creation of CirrosServerStackFixture could + not be started (would be locked by the creation of + CirrosPeerServerStackFixture). + """ + + def wrap(f): + + @functools.wraps(f) + def inner(*args, **kwargs): + t1 = timeutils.now() + t2 = None + gotten = True + f_name = reflection.get_callable_name(f) + try: + with lock(name): + t2 = timeutils.now() + LOG.debug('Lock "%(name)s" acquired by "%(function)s" :: ' + 'waited %(wait_secs)0.3fs', + {'name': name, + 'function': f_name, + 'wait_secs': (t2 - t1)}) + return f(*args, **kwargs) + except lockutils.AcquireLockFailedException: + gotten = False + finally: + t3 = timeutils.now() + if t2 is None: + held_secs = "N/A" + else: + held_secs = "%0.3fs" % (t3 - t2) + LOG.debug('Lock "%(name)s" "%(gotten)s" by "%(function)s" ::' + ' held %(held_secs)s', + {'name': name, + 'gotten': 'released' if gotten else 'unacquired', + 'function': f_name, + 'held_secs': held_secs}) + return inner + + return wrap + + +@contextlib.contextmanager +def lock(name): + """Re-definition of oslo_concurrency.lockutils.lock that does not apply + intra-worker locks. Only inter-worker locks are applied. + """ + from tobiko import config + lock_path = os.path.expanduser(config.CONF.tobiko.common.lock_dir) + + LOG.debug('Acquired lock "%(lock)s"', {'lock': name}) + + try: + ext_lock = lockutils.external_lock(name, + lock_file_prefix='tobiko', + lock_path=lock_path) + gotten = ext_lock.acquire(delay=0.01, blocking=True) + if not gotten: + raise lockutils.AcquireLockFailedException(name) + LOG.debug('Acquired external semaphore "%(lock)s"', + {'lock': name}) + try: + yield ext_lock + finally: + ext_lock.release() + finally: + LOG.debug('Releasing lock "%(lock)s"', {'lock': name}) diff --git a/tobiko/openstack/stacks/_cirros.py b/tobiko/openstack/stacks/_cirros.py index 4a98150ec..e28e1aaca 100644 --- a/tobiko/openstack/stacks/_cirros.py +++ b/tobiko/openstack/stacks/_cirros.py @@ -16,9 +16,9 @@ from __future__ import absolute_import import io import typing -from oslo_concurrency import lockutils from paramiko import sftp_file +import tobiko from tobiko import config from tobiko.openstack import glance from tobiko.openstack import neutron @@ -55,11 +55,6 @@ class CirrosImageFixture(glance.URLGlanceImageFixture): # when using recent Paramiko versions (>= 2.9.2) 'pubkeys': ['rsa-sha2-256', 'rsa-sha2-512']} - @lockutils.synchronized( - 'cirros_image_setup_fixture', external=True, lock_path=tobiko.LOCK_DIR) - def setup_fixture(self): - super(CirrosImageFixture, self).setup_fixture() - class CirrosFlavorStackFixture(_nova.FlavorStackFixture): ram = 128 @@ -79,6 +74,10 @@ class CirrosServerStackFixture(_nova.ServerStackFixture): #: We expect CirrOS based servers to be fast to boot is_reachable_timeout: tobiko.Seconds = 300. + @tobiko.interworker_synched('cirros_server_setup_fixture') + def setup_fixture(self): + super(CirrosServerStackFixture, self).setup_fixture() + @property def ssh_client(self) -> ssh.SSHClientFixture: ssh_client = super().ssh_client diff --git a/tobiko/openstack/stacks/_l3ha.py b/tobiko/openstack/stacks/_l3ha.py index e5ed1a859..5f397cdf6 100644 --- a/tobiko/openstack/stacks/_l3ha.py +++ b/tobiko/openstack/stacks/_l3ha.py @@ -13,8 +13,6 @@ # under the License. from __future__ import absolute_import -from oslo_concurrency import lockutils - import tobiko from tobiko.openstack import neutron from tobiko.openstack.stacks import _cirros @@ -31,8 +29,7 @@ class L3haRouterStackFixture(_neutron.RouterStackFixture): class L3haNetworkStackFixture(_neutron.NetworkBaseStackFixture): gateway_stack = tobiko.required_fixture(L3haRouterStackFixture) - @lockutils.synchronized( - 'create_l3ha_network_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_l3ha_network_stack') def setup_stack(self): super().setup_stack() diff --git a/tobiko/openstack/stacks/_neutron.py b/tobiko/openstack/stacks/_neutron.py index be4b355fe..4f762b775 100644 --- a/tobiko/openstack/stacks/_neutron.py +++ b/tobiko/openstack/stacks/_neutron.py @@ -20,7 +20,6 @@ import typing import netaddr from neutronclient.common import exceptions as nc_exceptions -from oslo_concurrency import lockutils from oslo_log import log import tobiko @@ -315,8 +314,7 @@ class SubnetPoolFixture(_fixture.ResourceFixture): def subnet_pool(self): return self.resource - @lockutils.synchronized( - 'create_subnet_pool', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_subnet_pool') def try_create_resource(self): super().try_create_resource() @@ -535,8 +533,7 @@ class NetworkBaseStackFixture(heat.HeatStackFixture): class NetworkStackFixture(NetworkBaseStackFixture): - @lockutils.synchronized( - 'create_network_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_network_stack') def setup_stack(self): super().setup_stack() @@ -545,8 +542,7 @@ class NetworkNoFipStackFixture(NetworkBaseStackFixture): """Extra Network Stack where VMs will not have FIPs""" gateway_stack = tobiko.required_fixture(RouterNoSnatStackFixture) - @lockutils.synchronized( - 'create_network_nofip_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_network_nofip_stack') def setup_stack(self): super().setup_stack() @@ -554,9 +550,7 @@ class NetworkNoFipStackFixture(NetworkBaseStackFixture): @neutron.skip_if_missing_networking_extensions('net-mtu-writable') class NetworkWithNetMtuWriteStackFixture(NetworkBaseStackFixture): - @lockutils.synchronized( - 'create_network_withnetmtuwrite_stack', - external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_network_withnetmtuwrite_stack') def setup_stack(self): super().setup_stack() @@ -629,8 +623,7 @@ class StatelessSecurityGroupFixture(_fixture.ResourceFixture): def security_group(self): return self.resource - @lockutils.synchronized( - 'create_security_group', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_security_group') def try_create_resource(self): super().try_create_resource() diff --git a/tobiko/openstack/stacks/_nova.py b/tobiko/openstack/stacks/_nova.py index 6c110ff7b..f252f2a88 100644 --- a/tobiko/openstack/stacks/_nova.py +++ b/tobiko/openstack/stacks/_nova.py @@ -20,7 +20,6 @@ import typing from abc import ABC import netaddr -from oslo_concurrency import lockutils from oslo_log import log import tobiko @@ -433,9 +432,7 @@ class CloudInitServerStackFixture(ServerStackFixture, ABC): #: nax SWAP file size in bytes swap_maxsize: typing.Optional[int] = None - @lockutils.synchronized( - 'cloudinit_server_setup_fixture', - external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('cloudinit_server_setup_fixture') def setup_fixture(self): super(CloudInitServerStackFixture, self).setup_fixture() diff --git a/tobiko/openstack/stacks/_qos.py b/tobiko/openstack/stacks/_qos.py index b0deb53b6..711e3606f 100644 --- a/tobiko/openstack/stacks/_qos.py +++ b/tobiko/openstack/stacks/_qos.py @@ -15,8 +15,6 @@ # under the License. from __future__ import absolute_import -from oslo_concurrency import lockutils - import tobiko from tobiko import config from tobiko.openstack import heat @@ -58,8 +56,7 @@ class QosNetworkStackFixture(_neutron.NetworkBaseStackFixture): value_specs = super().network_value_specs return dict(value_specs, qos_policy_id=self.qos_stack.qos_policy_id) - @lockutils.synchronized( - 'create_qos_network_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_qos_network_stack') def setup_stack(self): super().setup_stack() diff --git a/tobiko/openstack/stacks/_ubuntu.py b/tobiko/openstack/stacks/_ubuntu.py index 6e71cc89a..5f9c98be3 100644 --- a/tobiko/openstack/stacks/_ubuntu.py +++ b/tobiko/openstack/stacks/_ubuntu.py @@ -15,8 +15,6 @@ from __future__ import absolute_import import typing -from oslo_concurrency import lockutils - import tobiko from tobiko import config from tobiko.openstack import glance @@ -40,12 +38,6 @@ class UbuntuMinimalImageFixture(glance.FileGlanceImageFixture): disabled_algorithms = CONF.tobiko.ubuntu.disabled_algorithms is_reachable_timeout = CONF.tobiko.nova.ubuntu_is_reachable_timeout - @lockutils.synchronized( - 'ubuntu_minimal_image_setup_fixture', - external=True, lock_path=tobiko.LOCK_DIR) - def setup_fixture(self): - super(UbuntuMinimalImageFixture, self).setup_fixture() - IPERF3_SERVICE_FILE = """ [Unit] diff --git a/tobiko/openstack/stacks/_vlan.py b/tobiko/openstack/stacks/_vlan.py index 4a6e722dc..81c53321e 100644 --- a/tobiko/openstack/stacks/_vlan.py +++ b/tobiko/openstack/stacks/_vlan.py @@ -19,7 +19,6 @@ import abc import typing import netaddr -from oslo_concurrency import lockutils import tobiko from tobiko import config @@ -35,8 +34,7 @@ CONF = config.CONF class VlanNetworkStackFixture(_neutron.NetworkBaseStackFixture): - @lockutils.synchronized( - 'create_vlan_network_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_vlan_network_stack') def setup_stack(self): super().setup_stack() diff --git a/tobiko/tests/functional/openstack/stacks/test_neutron.py b/tobiko/tests/functional/openstack/stacks/test_neutron.py index 34a186f1e..afdb1ce3e 100644 --- a/tobiko/tests/functional/openstack/stacks/test_neutron.py +++ b/tobiko/tests/functional/openstack/stacks/test_neutron.py @@ -15,7 +15,6 @@ # under the License. from __future__ import absolute_import -from oslo_concurrency import lockutils from oslo_log import log import testtools @@ -152,9 +151,7 @@ class RouterInterfaceTestRouter(stacks.RouterStackFixture): class RouterInterfaceTestNetwork(stacks.NetworkBaseStackFixture): - @lockutils.synchronized( - 'create_router_interface_network_stack', - external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_router_interface_network_stack') def setup_stack(self): super().setup_stack() diff --git a/tobiko/tests/scenario/neutron/test_router.py b/tobiko/tests/scenario/neutron/test_router.py index c52c45d4c..de3d50b16 100644 --- a/tobiko/tests/scenario/neutron/test_router.py +++ b/tobiko/tests/scenario/neutron/test_router.py @@ -20,7 +20,6 @@ import re import typing import pytest -from oslo_concurrency import lockutils from oslo_log import log import testtools @@ -211,8 +210,7 @@ class DvrNetworkStackFixture(stacks.NetworkBaseStackFixture): gateway_stack = tobiko.required_fixture(DvrRouterStackFixture, setup=False) - @lockutils.synchronized( - 'create_dvr_network_stack', external=True, lock_path=tobiko.LOCK_DIR) + @tobiko.interworker_synched('create_dvr_network_stack') def setup_stack(self): super().setup_stack()