Set OVS manager command timeout and inactiviy probe

When creating the OVS manager, define the command timeout
(CONF.OVS.ovsdb_timeout) and inactivity probe time
(CONF.OVS.of_inactivity_probe)

NOTE: CONF.OVS.of_inactivity_probe is defined in seconds but the
parameter should be passed to ovs-vsctl in milliseconds [1].

[1]http://www.openvswitch.org/support/dist-docs/ovs-vswitchd.conf.db.5.txt

Depends-On: https://review.opendev.org/#/c/720785

Change-Id: I8ed1fc85c2f78710bf6589ba3deca518471339b8
Closes-Bug: #1868686

lower-constraints.txt

@@ -80,7 +80,7 @@ oslo.versionedobjects==1.35.1
 oslotest==3.2.0
 osprofiler==2.3.0
 ovs==2.8.0
-ovsdbapp==1.0.0
+ovsdbapp==1.3.0
 Paste==2.0.2
 PasteDeploy==1.5.0
 pbr==4.0.0

neutron/agent/common/ovs_lib.py

@@ -119,31 +119,12 @@ class VifPort(object):
 class BaseOVS(object):
 
     def __init__(self):
-        self.ovsdb_timeout = cfg.CONF.OVS.ovsdb_timeout
         self.ovsdb = impl_idl.api_factory()
         self._hw_offload = None
 
-    def add_manager(self, connection_uri, timeout=_SENTINEL):
-        """Have ovsdb-server listen for manager connections
-
-        :param connection_uri: Manager target string
-        :param timeout: The Manager probe_interval timeout value
-                        (defaults to ovsdb_timeout)
-        """
-        if timeout is _SENTINEL:
-            timeout = cfg.CONF.OVS.ovsdb_timeout
-        with self.ovsdb.transaction() as txn:
-            txn.add(self.ovsdb.add_manager(connection_uri))
-            if timeout:
-                txn.add(
-                    self.ovsdb.db_set('Manager', connection_uri,
-                                      ('inactivity_probe', timeout * 1000)))
-
-    def get_manager(self):
-        return self.ovsdb.get_manager().execute()
-
-    def remove_manager(self, connection_uri):
-        self.ovsdb.remove_manager(connection_uri).execute()
+    @property
+    def ovsdb_timeout(self):
+        return self.ovsdb.ovsdb_connection.timeout
 
     def add_bridge(self, bridge_name,
                    datapath_type=constants.OVS_DATAPATH_SYSTEM):

neutron/agent/ovsdb/native/helpers.py

@@ -14,10 +14,19 @@
 
 import functools
 
+from oslo_config import cfg
 from ovsdbapp.schema.open_vswitch import helpers
 
 from neutron.agent.common import utils
+from neutron.conf.agent import ovs_conf as agent_ovs_conf
+from neutron.conf.plugins.ml2.drivers import ovs_conf as ml2_ovs_conf
+
+
+agent_ovs_conf.register_ovs_agent_opts(cfg.CONF)
+ml2_ovs_conf.register_ovs_opts(cfg=cfg.CONF)
 
 enable_connection_uri = functools.partial(
     helpers.enable_connection_uri, execute=utils.execute, run_as_root=True,
-    log_fail_as_error=False, check_exit_code=False)
+    log_fail_as_error=False, check_exit_code=False,
+    timeout=cfg.CONF.OVS.ovsdb_timeout,
+    inactivity_probe=cfg.CONF.OVS.of_inactivity_probe * 1000)

neutron/tests/functional/agent/ovsdb/native/test_helpers.py

@@ -0,0 +1,53 @@
+# Copyright (c) 2020 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 neutron_lib import constants as const
+
+from neutron.agent.common import ovs_lib
+from neutron.agent.ovsdb.native import helpers
+from neutron.tests.common.exclusive_resources import port
+from neutron.tests.common import net_helpers
+from neutron.tests.functional import base
+
+
+class EnableConnectionUriTestCase(base.BaseSudoTestCase):
+
+    def test_add_manager_appends(self):
+        ovs = ovs_lib.BaseOVS()
+        ovsdb_cfg_connections = []
+        manager_connections = []
+        manager_removal = []
+
+        for _ in range(5):
+            _port = self.useFixture(port.ExclusivePort(
+                const.PROTO_NAME_TCP,
+                start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
+                end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
+            ovsdb_cfg_connections.append('tcp:127.0.0.1:%s' % _port)
+            manager_connections.append('ptcp:%s:127.0.0.1' % _port)
+
+        for index, conn_uri in enumerate(ovsdb_cfg_connections):
+            helpers.enable_connection_uri(conn_uri)
+            manager_removal.append(ovs.ovsdb.remove_manager(
+                manager_connections[index]))
+            self.addCleanup(manager_removal[index].execute)
+            self.assertIn(manager_connections[index],
+                          ovs.ovsdb.get_manager().execute())
+
+        for remove in manager_removal:
+            remove.execute()
+
+        for connection in manager_connections:
+            self.assertNotIn(connection, ovs.ovsdb.get_manager().execute())

neutron/tests/functional/agent/test_ovs_lib.py

@@ -26,7 +26,6 @@ from neutron.agent.linux import ip_lib
 from neutron.common import utils
 from neutron.plugins.ml2.drivers.openvswitch.agent.common import (
     constants as agent_const)
-from neutron.tests.common.exclusive_resources import port
 from neutron.tests.common import net_helpers
 from neutron.tests.functional.agent.linux import base
 
@@ -571,41 +570,6 @@ class OVSLibTestCase(base.BaseOVSLinuxTestCase):
         super(OVSLibTestCase, self).setUp()
         self.ovs = ovs_lib.BaseOVS()
 
-    def test_add_manager_appends(self):
-        port1 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
-            start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
-            end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
-        port2 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
-            start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
-            end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
-        manager_list = ["ptcp:%s:127.0.0.1" % port1,
-                        "ptcp:%s:127.0.0.1" % port2]
-        # Verify that add_manager does not override the existing manager
-        expected_manager_list = list()
-        for conn_uri in manager_list:
-            self.ovs.add_manager(conn_uri)
-            self.addCleanup(self.ovs.remove_manager, conn_uri)
-            self.assertIn(conn_uri, self.ovs.get_manager())
-            expected_manager_list.append(conn_uri)
-
-        # Verify that switch is configured with both the managers
-        for manager_uri in expected_manager_list:
-            self.assertIn(manager_uri, manager_list)
-
-    def test_add_manager_lifecycle_baseovs(self):
-        port1 = self.useFixture(port.ExclusivePort(const.PROTO_NAME_TCP,
-            start=net_helpers.OVS_MANAGER_TEST_PORT_FIRST,
-            end=net_helpers.OVS_MANAGER_TEST_PORT_LAST)).port
-        conn_uri = "ptcp:%s:127.0.0.1" % port1
-        self.addCleanup(self.ovs.remove_manager, conn_uri)
-        self.ovs.add_manager(conn_uri)
-        self.assertIn(conn_uri, self.ovs.get_manager())
-        self.assertEqual(self.ovs.db_get_val('Manager', conn_uri,
-                                             'inactivity_probe'),
-                         self.ovs.ovsdb_timeout * 1000)
-        self.ovs.remove_manager(conn_uri)
-        self.assertNotIn(conn_uri, self.ovs.get_manager())
-
     def test_bridge_lifecycle_baseovs(self):
         name = utils.get_rand_name(prefix=net_helpers.BR_PREFIX)
         self.addCleanup(self.ovs.delete_bridge, name)

neutron/tests/unit/agent/common/test_ovs_lib.py

@@ -512,6 +512,8 @@ class OVS_Lib_Test(base.BaseTestCase):
                        if_exists=True)])
 
     def test_get_port_ofport_retry(self):
+        # Increase this value to avoid a timeout during the test execution
+        self.br.ovsdb.ovsdb_connection.timeout = 10
         with mock.patch.object(
                 self.br, 'db_get_val',
                 side_effect=[[], [], [], [], 1]):
@@ -519,7 +521,7 @@ class OVS_Lib_Test(base.BaseTestCase):
 
     def test_get_port_ofport_retry_fails(self):
         # reduce timeout for faster execution
-        self.br.ovsdb_timeout = 1
+        self.br.ovsdb.ovsdb_connection.timeout = 1
         # after 7 calls the retry will timeout and raise
         with mock.patch.object(
                 self.br, 'db_get_val',

neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/openflow/native/test_ovs_bridge.py

@@ -46,6 +46,8 @@ class OVSAgentBridgeTestCase(ovs_test_base.OVSOSKenTestBase):
         mock.patch.object(ovs_lib.OVSBridge, 'db_get_val',
                           side_effect=_mock_db_get_val).start()
         br = self.br_int_cls('br-int')
+        # Reduce timeout for faster execution
+        br.ovsdb.ovsdb_connection.timeout = 1
         # make sure that in case of any misconfiguration when no datapath is
         # found a proper exception, not a TypeError is raised
         self.assertRaises(RuntimeError, br._get_dp)

requirements.txt

@@ -46,7 +46,7 @@ oslo.versionedobjects>=1.35.1 # Apache-2.0
 osprofiler>=2.3.0 # Apache-2.0
 os-ken >= 0.3.0 # Apache-2.0
 ovs>=2.8.0 # Apache-2.0
-ovsdbapp>=1.0.0 # Apache-2.0
+ovsdbapp>=1.3.0 # Apache-2.0
 psutil>=3.2.2 # BSD
 pyroute2>=0.5.7;sys_platform!='win32' # Apache-2.0 (+ dual licensed GPL2)
 pyOpenSSL>=17.1.0 # Apache-2.0