Ensure ovsdb_probe_interval set before connect()

Setting the ovsdb_probe_interval after Connection.start() is
called means that the probe interval is not changed from
python-ovs's default of 5s until after the initial copy of the
database is retrieved. On busy systems, this can time out and
cause infinite reconnects.

This patch passes the probe_interval argument to the ovs.db.Idl
class so that it can be set as part of creating the jsonrpc
Session.

Some unit tests were removed and replaced with a functional test
which ensures not just that set_probe_interval is called, but that
the value is actually set before the connection is established.

Conflicts:
  neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py
  neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py

Closes-bug: #1905611
Change-Id: I8c940ac8f7632c69607dea7220146ef59d55ed56
(cherry picked from commit 5783e95288)
This commit is contained in:
Terry Wilson 2020-11-25 19:55:32 +00:00
parent ac2276c396
commit 65d3f79ce6
5 changed files with 47 additions and 77 deletions

View File

@ -66,9 +66,9 @@ class MetadataAgentOvsIdl(object):
tables = ('Open_vSwitch', 'Bridge', 'Port', 'Interface')
for table in tables:
helper.register_table(table)
ovs_idl = idl.Idl(connection_string, helper)
ovs_idl._session.reconnect.set_probe_interval(
config.get_ovn_ovsdb_probe_interval())
ovs_idl = idl.Idl(
connection_string, helper,
probe_interval=config.get_ovn_ovsdb_probe_interval())
conn = connection.Connection(
ovs_idl, timeout=config.cfg.CONF.ovs.ovsdb_connection_timeout)
return idl_ovs.OvsdbIdl(conn)

View File

@ -160,8 +160,6 @@ def get_connection(db_class, trigger=None, driver=None, binding_events=False):
class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
def __init__(self, connection):
super(OvsdbNbOvnIdl, self).__init__(connection)
self.idl._session.reconnect.set_probe_interval(
cfg.get_ovn_ovsdb_probe_interval())
@property
def nb_global(self):
@ -722,10 +720,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):
def __init__(self, connection):
super(OvsdbSbOvnIdl, self).__init__(connection)
# TODO(twilson) This direct access of the idl should be removed in
# favor of a backend-agnostic method
self.idl._session.reconnect.set_probe_interval(
cfg.get_ovn_ovsdb_probe_interval())
def _get_chassis_physnets(self, chassis):
bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '')

View File

@ -352,8 +352,15 @@ class OvnDbNotifyHandler(event.RowEventHandler):
self.driver = driver
class BaseOvnIdl(connection.OvsdbIdl):
class Ml2OvnIdlBase(connection.OvsdbIdl):
def __init__(self, remote, schema, probe_interval=(), **kwargs):
if probe_interval == (): # None is a valid value to pass
probe_interval = ovn_conf.get_ovn_ovsdb_probe_interval()
super(Ml2OvnIdlBase, self).__init__(
remote, schema, probe_interval=probe_interval, **kwargs)
class BaseOvnIdl(Ml2OvnIdlBase):
def __init__(self, remote, schema):
self.notify_handler = event.RowEventHandler()
super(BaseOvnIdl, self).__init__(remote, schema)
@ -369,7 +376,7 @@ class BaseOvnIdl(connection.OvsdbIdl):
self.notify_handler.notify(event, row, updates)
class BaseOvnSbIdl(connection.OvsdbIdl):
class BaseOvnSbIdl(Ml2OvnIdlBase):
@classmethod
def from_server(cls, connection_string, schema_name):
_check_and_set_ssl_files(schema_name)

View File

@ -13,14 +13,18 @@
# under the License.
import mock
import fixtures as og_fixtures
from oslo_utils import uuidutils
from neutron.common.ovn import constants as ovn_const
from neutron.common import utils as n_utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.db import ovn_hash_ring_db as db_hash_ring
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor
from neutron.tests.functional import base
from neutron.tests.functional.resources.ovsdb import fixtures
from neutron.tests.functional.resources import process
from neutron_lib.api.definitions import portbindings
from neutron_lib.plugins import constants as plugin_constants
from neutron_lib.plugins import directory
@ -244,3 +248,34 @@ class TestNBDbMonitorOverTcp(TestNBDbMonitor):
class TestNBDbMonitorOverSsl(TestNBDbMonitor):
def get_ovsdb_server_protocol(self):
return 'ssl'
class OvnIdlProbeInterval(base.TestOVNFunctionalBase):
def setUp(self):
# skip parent setUp, we don't need it, but we do need grandparent
# pylint: disable=bad-super-call
super(base.TestOVNFunctionalBase, self).setUp()
mm = directory.get_plugin().mechanism_manager
self.mech_driver = mm.mech_drivers['ovn'].obj
self.temp_dir = self.useFixture(og_fixtures.TempDir()).path
install_share_path = self._get_install_share_path()
self.mgr = self.useFixture(
process.OvsdbServer(self.temp_dir, install_share_path,
ovn_nb_db=True, ovn_sb_db=True,
protocol='tcp'))
connection = self.mgr.get_ovsdb_connection_path
self.connections = {'OVN_Northbound': connection(),
'OVN_Southbound': connection(db_type='sb')}
def test_ovsdb_probe_interval(self):
klasses = {
ovsdb_monitor.BaseOvnIdl: ('OVN_Northbound', {}),
ovsdb_monitor.OvnNbIdl: ('OVN_Northbound',
{'driver': self.mech_driver}),
ovsdb_monitor.OvnSbIdl: ('OVN_Southbound',
{'driver': self.mech_driver})}
idls = [kls.from_server(self.connections[schema], schema, **kwargs)
for kls, (schema, kwargs) in klasses.items()]
interval = ovn_conf.get_ovn_ovsdb_probe_interval()
for idl in idls:
self.assertEqual(interval, idl._session.reconnect.probe_interval)

View File

@ -19,7 +19,6 @@ import mock
from neutron.common.ovn import constants as ovn_const
from neutron.common.ovn import utils
from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn
from neutron.tests import base
from neutron.tests.unit import fake_resources as fakes
@ -397,21 +396,6 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
fake_address_sets = TestNBImplIdlOvn.fake_set['address_sets']
self._load_ovsdb_fake_rows(self.address_set_table, fake_address_sets)
@mock.patch.object(impl_idl_ovn.OvsdbNbOvnIdl, 'ovsdb_connection', None)
@mock.patch.object(impl_idl_ovn, 'get_connection', mock.Mock())
def test_setting_ovsdb_probe_timeout_default_value(self):
inst = impl_idl_ovn.OvsdbNbOvnIdl(mock.Mock())
inst.idl._session.reconnect.set_probe_interval.assert_called_with(
60000)
@mock.patch.object(impl_idl_ovn.OvsdbNbOvnIdl, 'ovsdb_connection', None)
@mock.patch.object(impl_idl_ovn, 'get_connection', mock.Mock())
@mock.patch.object(ovn_conf, 'get_ovn_ovsdb_probe_interval')
def test_setting_ovsdb_probe_timeout(self, mock_get_probe_interval):
mock_get_probe_interval.return_value = 5000
inst = impl_idl_ovn.OvsdbNbOvnIdl(mock.Mock())
inst.idl._session.reconnect.set_probe_interval.assert_called_with(5000)
def test_get_all_logical_switches_with_ports(self):
# Test empty
mapping = self.nb_ovn_idl.get_all_logical_switches_with_ports()
@ -774,53 +758,3 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn):
self._tables.pop('Port_Group', None)
port_groups = self.nb_ovn_idl.get_port_groups()
self.assertEqual({}, port_groups)
class TestSBImplIdlOvn(TestDBImplIdlOvn):
fake_set = {
'chassis': [
{'name': 'host-1', 'hostname': 'host-1.localdomain.com',
'external_ids': {'ovn-bridge-mappings':
'public:br-ex,private:br-0'}},
{'name': 'host-2', 'hostname': 'host-2.localdomain.com',
'external_ids': {'ovn-bridge-mappings':
'public:br-ex,public2:br-ex'}},
{'name': 'host-3', 'hostname': 'host-3.localdomain.com',
'external_ids': {'ovn-bridge-mappings':
'public:br-ex'}}],
}
def setUp(self):
super(TestSBImplIdlOvn, self).setUp()
self.chassis_table = fakes.FakeOvsdbTable.create_one_ovsdb_table()
self._tables = {}
self._tables['Chassis'] = self.chassis_table
with mock.patch.object(impl_idl_ovn, 'get_connection',
return_value=mock.Mock()):
impl_idl_ovn.OvsdbSbOvnIdl.ovsdb_connection = None
self.sb_ovn_idl = impl_idl_ovn.OvsdbSbOvnIdl(mock.Mock())
self.sb_ovn_idl.idl.tables = self._tables
def _load_sb_db(self):
# Load Chassis
fake_chassis = TestSBImplIdlOvn.fake_set['chassis']
self._load_ovsdb_fake_rows(self.chassis_table, fake_chassis)
@mock.patch.object(impl_idl_ovn.OvsdbSbOvnIdl, 'ovsdb_connection', None)
@mock.patch.object(impl_idl_ovn, 'get_connection', mock.Mock())
def test_setting_ovsdb_probe_timeout_default_value(self):
inst = impl_idl_ovn.OvsdbSbOvnIdl(mock.Mock())
inst.idl._session.reconnect.set_probe_interval.assert_called_with(
60000)
@mock.patch.object(impl_idl_ovn.OvsdbSbOvnIdl, 'ovsdb_connection', None)
@mock.patch.object(impl_idl_ovn, 'get_connection', mock.Mock())
@mock.patch.object(ovn_conf, 'get_ovn_ovsdb_probe_interval')
def test_setting_ovsdb_probe_timeout(self, mock_get_probe_interval):
mock_get_probe_interval.return_value = 5000
inst = impl_idl_ovn.OvsdbSbOvnIdl(mock.Mock())
inst.idl._session.reconnect.set_probe_interval.assert_called_with(5000)