From 5d11608b75bba8edece9d961d3445d1777efcc7b Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 24 Sep 2020 22:03:20 +0000 Subject: [PATCH] [OVN] Simplify connection creation logic It's a bit cleaner to have the API classes define their own factory methods instead of having so many if statements in get_connection(). Conflicts: neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py Change-Id: I36f1f16d9eef32f4012b18685ee155cc4886e0c4 (cherry picked from commit cd91e1576836ae0a44cb643f97be321bf7081c65) --- neutron/cmd/ovn/neutron_ovn_db_sync_util.py | 8 +-- .../drivers/ovn/mech_driver/mech_driver.py | 3 +- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 49 ++++++++++--------- .../mech_driver/ovsdb/test_impl_idl_ovn.py | 2 +- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 2 +- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py index 9a51d4e61ab..1111c97939b 100644 --- a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py +++ b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py @@ -27,6 +27,7 @@ from neutron import opts as neutron_options from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync +from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import worker from neutron.plugins.ml2 import plugin as ml2_plugin LOG = logging.getLogger(__name__) @@ -188,16 +189,15 @@ def main(): LOG.error('Invalid core plugin : ["%s"].', cfg.CONF.core_plugin) return + mech_worker = worker.MaintenanceWorker try: - conn = impl_idl_ovn.get_connection(impl_idl_ovn.OvsdbNbOvnIdl) - ovn_api = impl_idl_ovn.OvsdbNbOvnIdl(conn) + ovn_api = impl_idl_ovn.OvsdbNbOvnIdl.from_worker(mech_worker) except RuntimeError: LOG.error('Invalid --ovn-ovn_nb_connection parameter provided.') return try: - sb_conn = impl_idl_ovn.get_connection(impl_idl_ovn.OvsdbSbOvnIdl) - ovn_sb_api = impl_idl_ovn.OvsdbSbOvnIdl(sb_conn) + ovn_sb_api = impl_idl_ovn.OvsdbSbOvnIdl.from_worker(mech_worker) except RuntimeError: LOG.error('Invalid --ovn-ovn_sb_connection parameter provided.') return diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 3650702e8d0..dae183bafa9 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -233,8 +233,7 @@ class OVNMechanismDriver(api.MechanismDriver): self.node_uuid = ovn_hash_ring_db.add_node(admin_context, self.hash_ring_group) - self._nb_ovn, self._sb_ovn = impl_idl_ovn.get_ovn_idls( - self, trigger, binding_events=not is_maintenance) + self._nb_ovn, self._sb_ovn = impl_idl_ovn.get_ovn_idls(self, trigger) if self._sb_ovn.is_table_present('Chassis_Private'): self.agent_chassis_table = 'Chassis_Private' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index 08c54c2b40b..42d9245c0c0 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -33,6 +33,7 @@ from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as cfg from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import commands as cmd from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor +from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import worker LOG = log.getLogger(__name__) @@ -122,45 +123,35 @@ class OvsdbConnectionUnavailable(n_exc.ServiceUnavailable): # Retry forever to get the OVN NB and SB IDLs. Wait 2^x * 1 seconds between # each retry, up to 'max_interval' seconds, then interval will be fixed # to 'max_interval' seconds afterwards. The default 'max_interval' is 180. -def get_ovn_idls(driver, trigger, binding_events=False): +def get_ovn_idls(driver, trigger): @tenacity.retry( wait=tenacity.wait_exponential( max=cfg.get_ovn_ovsdb_retry_max_interval()), reraise=True) - def get_ovn_idl_retry(cls): + def get_ovn_idl_retry(api_cls): trigger_class = utils.get_method_class(trigger) LOG.info('Getting %(cls)s for %(trigger)s with retry', - {'cls': cls.__name__, 'trigger': trigger_class.__name__}) - return cls(get_connection(cls, trigger, driver, binding_events)) + {'cls': api_cls.__name__, 'trigger': trigger_class.__name__}) + return api_cls.from_worker(trigger_class, driver) vlog.use_python_logger(max_level=cfg.get_ovn_ovsdb_log_level()) return tuple(get_ovn_idl_retry(c) for c in (OvsdbNbOvnIdl, OvsdbSbOvnIdl)) -def get_connection(db_class, trigger=None, driver=None, binding_events=False): - if db_class == OvsdbNbOvnIdl: - args = (cfg.get_ovn_nb_connection(), 'OVN_Northbound') - elif db_class == OvsdbSbOvnIdl: - args = (cfg.get_ovn_sb_connection(), 'OVN_Southbound') - - if binding_events: - if db_class == OvsdbNbOvnIdl: - idl_ = ovsdb_monitor.OvnNbIdl.from_server(*args, driver=driver) - else: - idl_ = ovsdb_monitor.OvnSbIdl.from_server(*args, driver=driver) - else: - if db_class == OvsdbNbOvnIdl: - idl_ = ovsdb_monitor.BaseOvnIdl.from_server(*args) - else: - idl_ = ovsdb_monitor.BaseOvnSbIdl.from_server(*args) - - return connection.Connection(idl_, timeout=cfg.get_ovn_ovsdb_timeout()) - - class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): def __init__(self, connection): super(OvsdbNbOvnIdl, self).__init__(connection) + @classmethod + def from_worker(cls, worker_class, driver=None): + args = (cfg.get_ovn_nb_connection(), 'OVN_Northbound') + if worker_class == worker.MaintenanceWorker: + idl_ = ovsdb_monitor.BaseOvnIdl.from_server(*args) + else: + idl_ = ovsdb_monitor.OvnNbIdl.from_server(*args, driver=driver) + conn = connection.Connection(idl_, timeout=cfg.get_ovn_ovsdb_timeout()) + return cls(conn) + @property def nb_global(self): return next(iter(self.tables['NB_Global'].rows.values())) @@ -721,6 +712,16 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): def __init__(self, connection): super(OvsdbSbOvnIdl, self).__init__(connection) + @classmethod + def from_worker(cls, worker_class, driver=None): + args = (cfg.get_ovn_sb_connection(), 'OVN_Southbound') + if worker_class == worker.MaintenanceWorker: + idl_ = ovsdb_monitor.BaseOvnSbIdl.from_server(*args) + else: + idl_ = ovsdb_monitor.OvnSbIdl.from_server(*args, driver=driver) + conn = connection.Connection(idl_, timeout=cfg.get_ovn_ovsdb_timeout()) + return cls(conn) + def _get_chassis_physnets(self, chassis): bridge_mappings = chassis.external_ids.get('ovn-bridge-mappings', '') mapping_dict = helpers.parse_mappings(bridge_mappings.split(','), diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index 7bd9b75d168..673f4171afe 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -337,7 +337,7 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): self._tables['DHCP_Options'] = self.dhcp_table self._tables['Address_Set'] = self.address_set_table - with mock.patch.object(impl_idl_ovn, 'get_connection', + with mock.patch.object(impl_idl_ovn.OvsdbNbOvnIdl, 'from_worker', return_value=mock.Mock()): impl_idl_ovn.OvsdbNbOvnIdl.ovsdb_connection = None self.nb_ovn_idl = impl_idl_ovn.OvsdbNbOvnIdl(mock.Mock()) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 4cc9599ac54..a27eb83f084 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -547,7 +547,7 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): self._test_mocks_helper(ovn_nb_synchronizer) ovn_api = ovn_nb_synchronizer.ovn_api - mock.patch.object(impl_idl_ovn, 'get_connection').start() + mock.patch.object(impl_idl_ovn.OvsdbNbOvnIdl, 'from_worker').start() ovn_nb_synchronizer.do_sync()