From 4af2163bd41648d64cf1c3c838990737955d2133 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 10 Dec 2013 04:20:26 -0800 Subject: [PATCH] Make timeout for ovs-vsctl configurable This patch adds a new configuration variable for the timeout on ovs-vsctl commands, and sets the default timeout to 10 seconds. This is aimed at allowing users to tune the agents in order to avoid timeout errors on their deployments. Change-Id: I73ea0d0de49a4b4a118bc2d68ad9c093ea122717 Closes-Bug: #1254520 --- etc/dhcp_agent.ini | 4 ++ etc/l3_agent.ini | 4 ++ neutron/agent/dhcp_agent.py | 1 + neutron/agent/l3_agent.py | 1 + neutron/agent/linux/ovs_lib.py | 20 +++++-- neutron/plugins/ryu/common/config.py | 2 +- .../tests/unit/openvswitch/test_ovs_lib.py | 53 +++++++++++++++---- 7 files changed, 71 insertions(+), 14 deletions(-) diff --git a/etc/dhcp_agent.ini b/etc/dhcp_agent.ini index e062beb2f08..425684b04f6 100644 --- a/etc/dhcp_agent.ini +++ b/etc/dhcp_agent.ini @@ -81,3 +81,7 @@ # you are sure that your version of iproute does not suffer from the problem. # If True, namespaces will be deleted when a dhcp server is disabled. # dhcp_delete_namespaces = False + +# Timeout for ovs-vsctl commands. +# If the timeout expires, ovs commands will fail with ALARMCLOCK error. +# ovs_vsctl_timeout = 10 diff --git a/etc/l3_agent.ini b/etc/l3_agent.ini index 777e10ad7fc..96dcc34ebf4 100644 --- a/etc/l3_agent.ini +++ b/etc/l3_agent.ini @@ -71,3 +71,7 @@ # you are sure that your version of iproute does not suffer from the problem. # If True, namespaces will be deleted when a router is destroyed. # router_delete_namespaces = False + +# Timeout for ovs-vsctl commands. +# If the timeout expires, ovs commands will fail with ALARMCLOCK error. +# ovs_vsctl_timeout = 10 diff --git a/neutron/agent/dhcp_agent.py b/neutron/agent/dhcp_agent.py index 777668f674d..605a369f243 100644 --- a/neutron/agent/dhcp_agent.py +++ b/neutron/agent/dhcp_agent.py @@ -25,6 +25,7 @@ from neutron.agent.common import config from neutron.agent.linux import dhcp from neutron.agent.linux import external_process from neutron.agent.linux import interface +from neutron.agent.linux import ovs_lib # noqa from neutron.agent import rpc as agent_rpc from neutron.common import constants from neutron.common import exceptions diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 8853fb2c685..a61045005fd 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -26,6 +26,7 @@ from neutron.agent.linux import external_process from neutron.agent.linux import interface from neutron.agent.linux import ip_lib from neutron.agent.linux import iptables_manager +from neutron.agent.linux import ovs_lib # noqa from neutron.agent.linux import utils from neutron.agent import rpc as agent_rpc from neutron.common import constants as l3_constants diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index b6fcf927014..0ff48e39c1c 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -20,6 +20,8 @@ import re +from oslo.config import cfg + from neutron.agent.linux import ip_lib from neutron.agent.linux import utils from neutron.openstack.common import jsonutils @@ -28,6 +30,15 @@ from neutron.plugins.common import constants as p_const # TODO(JLH) Should we remove the explicit include of the ovs plugin here from neutron.plugins.openvswitch.common import constants +# Default timeout for ovs-vsctl command +DEFAULT_OVS_VSCTL_TIMEOUT = 10 +OPTS = [ + cfg.IntOpt('ovs_vsctl_timeout', + default=DEFAULT_OVS_VSCTL_TIMEOUT, + help=_('Timeout in seconds for ovs-vsctl commands')), +] +cfg.CONF.register_opts(OPTS) + LOG = logging.getLogger(__name__) @@ -50,9 +61,10 @@ class BaseOVS(object): def __init__(self, root_helper): self.root_helper = root_helper + self.vsctl_timeout = cfg.CONF.ovs_vsctl_timeout def run_vsctl(self, args, check_error=False): - full_args = ["ovs-vsctl", "--timeout=2"] + args + full_args = ["ovs-vsctl", "--timeout=%d" % self.vsctl_timeout] + args try: return utils.execute(full_args, root_helper=self.root_helper) except Exception as e: @@ -393,7 +405,8 @@ class OVSBridge(BaseOVS): def get_bridge_for_iface(root_helper, iface): - args = ["ovs-vsctl", "--timeout=2", "iface-to-br", iface] + args = ["ovs-vsctl", "--timeout=%d" % cfg.CONF.ovs_vsctl_timeout, + "iface-to-br", iface] try: return utils.execute(args, root_helper=root_helper).strip() except Exception: @@ -402,7 +415,8 @@ def get_bridge_for_iface(root_helper, iface): def get_bridges(root_helper): - args = ["ovs-vsctl", "--timeout=2", "list-br"] + args = ["ovs-vsctl", "--timeout=%d" % cfg.CONF.ovs_vsctl_timeout, + "list-br"] try: return utils.execute(args, root_helper=root_helper).strip().split("\n") except Exception as e: diff --git a/neutron/plugins/ryu/common/config.py b/neutron/plugins/ryu/common/config.py index 37b369df205..be6766d1bed 100644 --- a/neutron/plugins/ryu/common/config.py +++ b/neutron/plugins/ryu/common/config.py @@ -17,7 +17,7 @@ from oslo.config import cfg from neutron.agent.common import config - +from neutron.agent.linux import ovs_lib # noqa ovs_opts = [ cfg.StrOpt('integration_bridge', default='br-int', diff --git a/neutron/tests/unit/openvswitch/test_ovs_lib.py b/neutron/tests/unit/openvswitch/test_ovs_lib.py index dd3f1137500..9ec041bb6ba 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_lib.py +++ b/neutron/tests/unit/openvswitch/test_ovs_lib.py @@ -16,6 +16,7 @@ # @author: Dan Wendlandt, Nicira, Inc. import mock +from oslo.config import cfg import testtools from neutron.agent.linux import ovs_lib @@ -109,11 +110,12 @@ class OVS_Lib_Test(base.BaseTestCase): def setUp(self): super(OVS_Lib_Test, self).setUp() self.BR_NAME = "br-int" - self.TO = "--timeout=2" + self.TO = "--timeout=10" self.root_helper = 'sudo' self.br = ovs_lib.OVSBridge(self.BR_NAME, self.root_helper) - self.execute = mock.patch.object(utils, "execute").start() + self.execute = mock.patch.object( + utils, "execute", spec=utils.execute).start() self.addCleanup(mock.patch.stopall) def test_vifport(self): @@ -151,14 +153,29 @@ class OVS_Lib_Test(base.BaseTestCase): self.br.reset_bridge() - def test_delete_port(self): + def _build_timeout_opt(self, exp_timeout): + return "--timeout=%d" % exp_timeout if exp_timeout else self.TO + + def _test_delete_port(self, exp_timeout=None): + exp_timeout_str = self._build_timeout_opt(exp_timeout) pname = "tap5" self.br.delete_port(pname) self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "--", "--if-exists", + ["ovs-vsctl", exp_timeout_str, "--", "--if-exists", "del-port", self.BR_NAME, pname], root_helper=self.root_helper) + def test_delete_port(self): + self._test_delete_port() + + def test_call_command_non_default_timeput(self): + # This test is only for verifying a non-default timeout + # is correctly applied. Does not need to be repeated for + # every ovs_lib method + new_timeout = 5 + self.br.vsctl_timeout = new_timeout + self._test_delete_port(new_timeout) + def test_add_flow(self): ofport = "99" vid = 4000 @@ -503,17 +520,25 @@ class OVS_Lib_Test(base.BaseTestCase): self.assertEqual(port_name, 'dhc5c1321a7-c7') self.assertEqual(ofport, 2) - def test_iface_to_br(self): + def _test_iface_to_br(self, exp_timeout=None): iface = 'tap0' br = 'br-int' root_helper = 'sudo' self.execute.return_value = 'br-int' - + exp_timeout_str = self._build_timeout_opt(exp_timeout) self.assertEqual(ovs_lib.get_bridge_for_iface(root_helper, iface), br) self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "iface-to-br", iface], + ["ovs-vsctl", exp_timeout_str, "iface-to-br", iface], root_helper=root_helper) + def test_iface_to_br(self): + self._test_iface_to_br() + + def test_iface_to_br_non_default_timeout(self): + new_timeout = 5 + cfg.CONF.set_override('ovs_vsctl_timeout', new_timeout) + self._test_iface_to_br(new_timeout) + def test_iface_to_br_handles_ovs_vsctl_exception(self): iface = 'tap0' root_helper = 'sudo' @@ -547,16 +572,24 @@ class OVS_Lib_Test(base.BaseTestCase): mock.call('tap5678') ]) - def test_get_bridges(self): + def _test_get_bridges(self, exp_timeout=None): bridges = ['br-int', 'br-ex'] root_helper = 'sudo' self.execute.return_value = 'br-int\nbr-ex\n' - + timeout_str = self._build_timeout_opt(exp_timeout) self.assertEqual(ovs_lib.get_bridges(root_helper), bridges) self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "list-br"], + ["ovs-vsctl", timeout_str, "list-br"], root_helper=root_helper) + def test_get_bridges(self): + self._test_get_bridges() + + def test_get_bridges_not_default_timeout(self): + new_timeout = 5 + cfg.CONF.set_override('ovs_vsctl_timeout', new_timeout) + self._test_get_bridges(new_timeout) + def test_get_local_port_mac_succeeds(self): with mock.patch('neutron.agent.linux.ip_lib.IpLinkCommand', return_value=mock.Mock(address='foo')):