From 42cc227798d912cd1edd254f3ea228bab3225b24 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Wed, 21 Sep 2016 05:31:31 -0400 Subject: [PATCH] Change default exception in wait_until_true By default, wait_until_true uses default exception from eventlet which is eventlet.TimeoutError. This class is not subclass of Exception but BaseException. In case wait_until_true times out in any test, the whole test executor worker is stopped leaving scheduled tests not executed. This patch replaces eventlet.TimeoutError with new WaitTimeout exception, that inherits from Exception and thus won't break execution of other test cases in case it's raised. Related-Bug: 1625221 Change-Id: I44c0c22f427f61d84963e6e59393b90fbaa8f058 --- neutron/agent/l3/ha_router.py | 3 +- neutron/agent/linux/async_process.py | 4 +-- neutron/agent/linux/ovsdb_monitor.py | 6 ++-- neutron/common/utils.py | 36 ++++++++++++++++--- .../openvswitch/agent/ovsdb_handler.py | 2 +- neutron/tests/fullstack/test_trunk.py | 5 +-- .../agent/linux/test_ovsdb_monitor.py | 3 +- .../functional/agent/test_l2_ovs_agent.py | 6 ++-- neutron/tests/functional/common/test_utils.py | 17 +++++++-- .../unit/agent/linux/test_async_process.py | 3 +- .../openvswitch/agent/test_ovsdb_handler.py | 6 ++-- 11 files changed, 62 insertions(+), 29 deletions(-) diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index b87929faa57..83b828309f2 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -16,7 +16,6 @@ import os import shutil import signal -import eventlet import netaddr from neutron_lib import constants as n_consts from oslo_log import log as logging @@ -349,7 +348,7 @@ class HaRouter(router.RouterInfo): try: common_utils.wait_until_true(lambda: not pm.active, timeout=SIGTERM_TIMEOUT) - except eventlet.timeout.Timeout: + except common_utils.WaitTimeout: pm.disable(sig=str(int(signal.SIGKILL))) def update_initial_state(self, callback): diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py index bcc8b7f5e4d..a29011ee6e7 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -104,7 +104,7 @@ class AsyncProcess(object): """Launch a process and monitor it asynchronously. :param block: Block until the process has started. - :raises eventlet.timeout.Timeout if blocking is True and the process + :raises utils.WaitTimeout if blocking is True and the process did not start in time. """ LOG.debug('Launching async process [%s].', self.cmd) @@ -122,7 +122,7 @@ class AsyncProcess(object): :param block: Block until the process has stopped. :param kill_signal: Number of signal that will be sent to the process when terminating the process - :raises eventlet.timeout.Timeout if blocking is True and the process + :raises utils.WaitTimeout if blocking is True and the process did not stop in time. """ if self._is_running: diff --git a/neutron/agent/linux/ovsdb_monitor.py b/neutron/agent/linux/ovsdb_monitor.py index a830de740e3..a3c16bec3ec 100644 --- a/neutron/agent/linux/ovsdb_monitor.py +++ b/neutron/agent/linux/ovsdb_monitor.py @@ -12,13 +12,13 @@ # License for the specific language governing permissions and limitations # under the License. -import eventlet from oslo_log import log as logging from oslo_serialization import jsonutils from neutron._i18n import _LE from neutron.agent.linux import async_process from neutron.agent.ovsdb import api as ovsdb +from neutron.common import utils LOG = logging.getLogger(__name__) @@ -113,6 +113,4 @@ class SimpleInterfaceMonitor(OvsdbMonitor): def start(self, block=False, timeout=5): super(SimpleInterfaceMonitor, self).start() if block: - with eventlet.timeout.Timeout(timeout): - while not self.is_active(): - eventlet.sleep() + utils.wait_until_true(self.is_active) diff --git a/neutron/common/utils.py b/neutron/common/utils.py index dc7351329af..9b31b59bbf0 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -28,6 +28,7 @@ import sys import time import uuid +import debtcollector from debtcollector import removals import eventlet from eventlet.green import subprocess @@ -58,6 +59,17 @@ SYNCHRONIZED_PREFIX = 'neutron-' synchronized = lockutils.synchronized_with_prefix(SYNCHRONIZED_PREFIX) +class WaitTimeout(Exception, eventlet.TimeoutError): + """Default exception coming from wait_until_true() function. + + The reason is that eventlet.TimeoutError inherits from BaseException and + testtools.TestCase consumes only Exceptions. Which means in case + TimeoutError is raised, test runner stops and exits while it still has test + cases scheduled for execution. + """ + pass + + @removals.remove( message="Use ensure_tree(path, 0o755) from oslo_utils.fileutils") def ensure_dir(dir_path): @@ -696,12 +708,26 @@ def wait_until_true(predicate, timeout=60, sleep=1, exception=None): Best practice is to instantiate predicate with functools.partial() :param timeout: Timeout in seconds how long should function wait. :param sleep: Polling interval for results in seconds. - :param exception: Exception class for eventlet.Timeout. - (see doc for eventlet.Timeout for more information) + :param exception: Exception instance to raise on timeout. If None is passed + (default) then WaitTimeout exception is raised. """ - with eventlet.timeout.Timeout(timeout, exception): - while not predicate(): - eventlet.sleep(sleep) + try: + with eventlet.timeout.Timeout(timeout): + while not predicate(): + eventlet.sleep(sleep) + except eventlet.TimeoutError: + if exception is None: + debtcollector.deprecate( + "Raising eventlet.TimeoutError by default has been deprecated", + message="wait_until_true() now raises WaitTimeout error by " + "default.", + version="Ocata", + removal_version="Pike") + exception = WaitTimeout("Timed out after %d seconds" % timeout) + #NOTE(jlibosva): In case None is passed exception is instantiated on + # the line above. + #pylint: disable=raising-bad-type + raise exception class _AuthenticBase(object): diff --git a/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py b/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py index 6bcc1777693..3418e7064df 100644 --- a/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py +++ b/neutron/services/trunk/drivers/openvswitch/agent/ovsdb_handler.py @@ -465,7 +465,7 @@ class OVSDBHandler(object): bridge_has_port_predicate, timeout=self.timeout) return True - except eventlet.TimeoutError: + except common_utils.WaitTimeout: LOG.error( _LE('No port present on trunk bridge %(br_name)s ' 'in %(timeout)d seconds.'), diff --git a/neutron/tests/fullstack/test_trunk.py b/neutron/tests/fullstack/test_trunk.py index 1c04e56919b..d1d35f31a90 100644 --- a/neutron/tests/fullstack/test_trunk.py +++ b/neutron/tests/fullstack/test_trunk.py @@ -14,7 +14,6 @@ import functools -import eventlet import netaddr from neutron_lib import constants from oslo_utils import uuidutils @@ -293,7 +292,9 @@ class TestTrunkPlugin(base.BaseFullStackTestCase): ) try: utils.wait_until_true(no_patch_ports_predicate) - except eventlet.TimeoutError: + except utils.WaitTimeout: + # Create exception object after timeout to provide up-to-date list + # of interfaces raise TrunkTestException( "Integration bridge %s still has following ports while some of" " them are patch ports for trunk that were supposed to be " diff --git a/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py b/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py index 020029b5576..6450934436c 100644 --- a/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py +++ b/neutron/tests/functional/agent/linux/test_ovsdb_monitor.py @@ -22,7 +22,6 @@ Tests in this module will be skipped unless: - sudo testing is enabled (see neutron.tests.functional.base for details) """ -import eventlet from oslo_config import cfg from neutron.agent.common import ovs_lib @@ -135,7 +134,7 @@ class TestSimpleInterfaceMonitor(BaseMonitorTest): try: utils.wait_until_true( lambda: self.monitor.get_events().get('added')) - except eventlet.timeout.Timeout: + except utils.WaitTimeout: raise AssertionError('Initial call should always be true') def test_get_events_includes_ofport(self): diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index 3c7bb25e2d0..7e5288bfbe3 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -16,8 +16,6 @@ import time -from eventlet.timeout import Timeout - from neutron.common import utils from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants from neutron.tests.common import net_helpers @@ -319,8 +317,8 @@ class TestOVSAgent(base.OVSAgentTestFramework): unplug_ports=[self.ports[1]]) self.wait_until_ports_state([self.ports[0]], up=True) self.assertRaises( - Timeout, self.wait_until_ports_state, [self.ports[1]], up=True, - timeout=10) + utils.WaitTimeout, self.wait_until_ports_state, [self.ports[1]], + up=True, timeout=10) class TestOVSAgentExtensionConfig(base.OVSAgentTestFramework): diff --git a/neutron/tests/functional/common/test_utils.py b/neutron/tests/functional/common/test_utils.py index bbbb37bf006..7e7713c07a4 100644 --- a/neutron/tests/functional/common/test_utils.py +++ b/neutron/tests/functional/common/test_utils.py @@ -10,9 +10,10 @@ # License for the specific language governing permissions and limitations # under the License. -import eventlet import testtools +import eventlet + from neutron.common import utils from neutron.tests import base @@ -22,5 +23,17 @@ class TestWaitUntilTrue(base.BaseTestCase): utils.wait_until_true(lambda: True) def test_wait_until_true_predicate_fails(self): - with testtools.ExpectedException(eventlet.timeout.Timeout): + with testtools.ExpectedException(utils.WaitTimeout): utils.wait_until_true(lambda: False, 2) + + def test_wait_until_true_predicate_fails_compatibility_test(self): + """This test makes sure that eventlet.TimeoutError can still be caught. + """ + try: + utils.wait_until_true(lambda: False, 2) + except eventlet.TimeoutError: + return + except Exception: + pass + self.fail('wait_until_true() does not raise eventlet.TimeoutError ' + 'compatible exception by default.') diff --git a/neutron/tests/unit/agent/linux/test_async_process.py b/neutron/tests/unit/agent/linux/test_async_process.py index 0512556d715..73d54f1235d 100644 --- a/neutron/tests/unit/agent/linux/test_async_process.py +++ b/neutron/tests/unit/agent/linux/test_async_process.py @@ -16,7 +16,6 @@ import signal import eventlet.event import eventlet.queue -import eventlet.timeout import mock import testtools @@ -88,7 +87,7 @@ class TestAsyncProcess(base.BaseTestCase): self.proc._is_running = True self.proc._kill_event = kill_event # Ensure the test times out eventually if the watcher loops endlessly - with eventlet.timeout.Timeout(5): + with self.assert_max_execution_time(): with mock.patch.object(self.proc, '_handle_process_error') as func: self.proc._watch_process(callback, kill_event) diff --git a/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py b/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py index 1468d025cf5..14dd051790e 100644 --- a/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py +++ b/neutron/tests/unit/services/trunk/drivers/openvswitch/agent/test_ovsdb_handler.py @@ -15,13 +15,13 @@ import mock -import eventlet import oslo_messaging from oslo_serialization import jsonutils from oslo_utils import uuidutils import testtools from neutron.api.rpc.handlers import resources_rpc +from neutron.common import utils from neutron.objects import trunk as trunk_obj from neutron.services.trunk import constants from neutron.services.trunk.drivers.openvswitch.agent import exceptions @@ -141,8 +141,8 @@ class TestOVSDBHandler(base.BaseTestCase): 'mac_address': 'mac'} for subport in self.fake_subports]} @mock.patch('neutron.agent.common.ovs_lib.OVSBridge') - @mock.patch('neutron.common.utils.wait_until_true', - side_effect=eventlet.TimeoutError) + @mock.patch.object(utils, 'wait_until_true', + side_effect=utils.WaitTimeout) def test_handle_trunk_add_interface_wont_appear(self, wut, br): self.ovsdb_handler.handle_trunk_add('foo') self.assertTrue(self.trunk_manager.dispose_trunk.called)