From c13d722f3913137945c27fcc74371d3316129f30 Mon Sep 17 00:00:00 2001 From: Jakub Libosvar Date: Mon, 16 May 2016 18:07:32 +0000 Subject: [PATCH] ovsdb: Don't let block() wait indefinitely Poller.block() calls select() on defined file descriptors. This patch adds timeout to select() so it doesn't get stuck in case no fd is ready. Also timeout was added when reading transaction results from queue. Closes-Bug: 1567668 Change-Id: I7dbddd01409430ce93d8c23f04f02c46fb2a68c4 --- neutron/agent/ovsdb/api.py | 4 ++ neutron/agent/ovsdb/impl_idl.py | 8 +++- neutron/agent/ovsdb/native/commands.py | 2 + neutron/agent/ovsdb/native/connection.py | 3 ++ .../functional/agent/test_l2_ovs_agent.py | 8 ---- .../tests/unit/agent/ovsdb/test_impl_idl.py | 37 +++++++++++++++++++ 6 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 neutron/tests/unit/agent/ovsdb/test_impl_idl.py diff --git a/neutron/agent/ovsdb/api.py b/neutron/agent/ovsdb/api.py index a690ffdd4a1..a750dc3eb49 100644 --- a/neutron/agent/ovsdb/api.py +++ b/neutron/agent/ovsdb/api.py @@ -356,6 +356,10 @@ class API(object): """ +class TimeoutException(Exception): + pass + + def val_to_py(val): """Convert a json ovsdb return value to native python object""" if isinstance(val, collections.Sequence) and len(val) == 2: diff --git a/neutron/agent/ovsdb/impl_idl.py b/neutron/agent/ovsdb/impl_idl.py index f25ac78e2a8..2aa5dda0a6e 100644 --- a/neutron/agent/ovsdb/impl_idl.py +++ b/neutron/agent/ovsdb/impl_idl.py @@ -54,7 +54,13 @@ class Transaction(api.Transaction): def commit(self): self.ovsdb_connection.queue_txn(self) - result = self.results.get() + try: + result = self.results.get(self.timeout) + except Queue.Empty: + raise api.TimeoutException( + _("Commands %(commands)s exceeded timeout %(timeout)d " + "seconds") % {'commands': self.commands, + 'timeout': self.timeout}) if self.check_error: if isinstance(result, idlutils.ExceptionResult): if self.log_errors: diff --git a/neutron/agent/ovsdb/native/commands.py b/neutron/agent/ovsdb/native/commands.py index 8a138d6f49b..97649342fdc 100644 --- a/neutron/agent/ovsdb/native/commands.py +++ b/neutron/agent/ovsdb/native/commands.py @@ -48,6 +48,8 @@ class BaseCommand(api.Command): ", ".join("%s=%s" % (k, v) for k, v in command_info.items() if k not in ['api', 'result'])) + __repr__ = __str__ + class AddBridgeCommand(BaseCommand): def __init__(self, api, name, may_exist, datapath_type): diff --git a/neutron/agent/ovsdb/native/connection.py b/neutron/agent/ovsdb/native/connection.py index 6bf2c607ebf..82a177090c6 100644 --- a/neutron/agent/ovsdb/native/connection.py +++ b/neutron/agent/ovsdb/native/connection.py @@ -104,6 +104,9 @@ class Connection(object): while True: self.idl.wait(self.poller) self.poller.fd_wait(self.txns.alert_fileno, poller.POLLIN) + #TODO(jlibosva): Remove next line once losing connection to ovsdb + # is solved. + self.poller.timer_wait(self.timeout) self.poller.block() self.idl.run() txn = self.txns.get_nowait() diff --git a/neutron/tests/functional/agent/test_l2_ovs_agent.py b/neutron/tests/functional/agent/test_l2_ovs_agent.py index 15efcc269dd..cbb2cb47622 100644 --- a/neutron/tests/functional/agent/test_l2_ovs_agent.py +++ b/neutron/tests/functional/agent/test_l2_ovs_agent.py @@ -24,14 +24,6 @@ from neutron.tests.functional.agent.l2 import base class TestOVSAgent(base.OVSAgentTestFramework): - def setUp(self): - # NOTE(jlibosva): Tests are getting stuck when running with native - # ovsdb interface in this test suite. We skip until the root cause is - # found to unblock CI jobs. - if self.ovsdb_interface == 'native': - self.skipTest('bug/1567668') - super(TestOVSAgent, self).setUp() - def test_port_creation_and_deletion(self): self.setup_agent_and_ports( port_dicts=self.create_test_ports()) diff --git a/neutron/tests/unit/agent/ovsdb/test_impl_idl.py b/neutron/tests/unit/agent/ovsdb/test_impl_idl.py new file mode 100644 index 00000000000..3e4e927cbe9 --- /dev/null +++ b/neutron/tests/unit/agent/ovsdb/test_impl_idl.py @@ -0,0 +1,37 @@ +# Copyright (c) 2016 Red Hat, Inc. +# +# 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. + +import mock +from six.moves import queue +import testtools +import unittest2 + +try: + from ovs.db import idl # noqa +except ImportError: + raise unittest2.SkipTest( + "Skip test since ovs requirement for PY3 doesn't support yet.") + +from neutron.agent.ovsdb import api +from neutron.agent.ovsdb import impl_idl +from neutron.tests import base + + +class TransactionTestCase(base.BaseTestCase): + def test_commit_raises_exception_on_timeout(self): + with mock.patch.object(queue, 'Queue') as mock_queue: + transaction = impl_idl.Transaction(mock.sentinel, mock.Mock(), 0) + mock_queue.return_value.get.side_effect = queue.Empty + with testtools.ExpectedException(api.TimeoutException): + transaction.commit()