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

@@ -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

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

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

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

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:

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',

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')):