Avoid register config options on imports

fernandoroyosanchez · cubeek · commit 44ec1f150338 · 2022-11-22T05:09:37.000Z
Continue similar approach following in [1], where some project imports collide with config options. As part of the change, a wrapped decorator has been implemented to cover those functions that include any of the ovn config options as value to the decorators arguments (e.g. tenacity retry). This way we avoid requiring the options to be registered as soon as the module is imported, where they have not yet been registered by a main process. [1] https://review.opendev.org/c/openstack/neutron/+/837392 Co-authored-by: Jakub Libosvar <libosvar@redhat.com> Co-authored-by: Fernando Royo <froyo@redhat.com> Conflicts: neutron/agent/ovn/metadata_agent.py neutron/cmd/ovn/neutron_ovn_db_sync_util.py neutron/common/ovn/utils.py neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py Change-Id: I4bccb094ee7f690cbc352c38b5b39d505e6ea460 (cherry picked from commit 227c5f8)
diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py
@@ -24,7 +24,6 @@
 from oslo_utils import netutils
 from ovsdbapp.backend.ovs_idl import event as row_event
 from ovsdbapp.backend.ovs_idl import vlog
-import tenacity
 
 from neutron.agent.linux import external_process
 from neutron.agent.linux import ip_lib
@@ -280,10 +279,7 @@ def start(self):
 
         self._proxy.wait()
 
-    @tenacity.retry(
-        wait=tenacity.wait_exponential(
-            max=config.get_ovn_ovsdb_retry_max_interval()),
-        reraise=True)
+    @ovn_utils.retry()
     def register_metadata_agent(self):
         # NOTE(lucasagomes): db_add() will not overwrite the UUID if
         # it's already set.
diff --git a/neutron/agent/ovn/metadata/ovsdb.py b/neutron/agent/ovn/metadata/ovsdb.py
@@ -17,8 +17,8 @@
 from ovsdbapp.backend.ovs_idl import connection
 from ovsdbapp.backend.ovs_idl import idlutils
 from ovsdbapp.schema.open_vswitch import impl_idl as idl_ovs
-import tenacity
 
+from neutron.common.ovn import utils as ovn_utils
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovsdb_monitor
@@ -54,16 +54,11 @@ def __init__(self, chassis=None, events=None, tables=None):
         if events:
             self.notify_handler.watch_events(events)
 
-    @tenacity.retry(
-        wait=tenacity.wait_exponential(max=180),
-        reraise=True)
+    @ovn_utils.retry(max_=180)
     def _get_ovsdb_helper(self, connection_string):
         return idlutils.get_schema_helper(connection_string, self.SCHEMA)
 
-    @tenacity.retry(
-        wait=tenacity.wait_exponential(
-            max=config.get_ovn_ovsdb_retry_max_interval()),
-        reraise=True)
+    @ovn_utils.retry()
     def start(self):
         LOG.info('Getting OvsdbSbOvnIdl for MetadataAgent with retry')
         conn = connection.Connection(
diff --git a/neutron/agent/ovn/metadata_agent.py b/neutron/agent/ovn/metadata_agent.py
@@ -21,11 +21,13 @@
 from neutron.agent.ovn.metadata import agent
 from neutron.conf.agent.metadata import config as meta
 from neutron.conf.agent.ovn.metadata import config as ovn_meta
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 
 LOG = logging.getLogger(__name__)
 
 
 def main():
+    ovn_conf.register_opts()
     ovn_meta.register_meta_conf_opts(meta.SHARED_OPTS)
     ovn_meta.register_meta_conf_opts(meta.UNIX_DOMAIN_METADATA_PROXY_OPTS)
     ovn_meta.register_meta_conf_opts(meta.METADATA_PROXY_HANDLER_OPTS)
diff --git a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py
@@ -137,6 +137,7 @@ def security_groups_provider_updated(self, context,
 
 def setup_conf():
     conf = cfg.CONF
+    ovn_conf.register_opts()
     ml2_group, ml2_opts = neutron_options.list_ml2_conf_opts()[0]
     cfg.CONF.register_cli_opts(ml2_opts, ml2_group)
     cfg.CONF.register_cli_opts(securitygroups_rpc.security_group_opts,
diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py
@@ -33,6 +33,7 @@
 from oslo_utils import netutils
 from oslo_utils import strutils
 from ovsdbapp import constants as ovsdbapp_const
+import tenacity
 
 from neutron._i18n import _
 from neutron.common.ovn import constants
@@ -685,3 +686,14 @@ def is_port_external(port):
 
     return (vnic_type in constants.EXTERNAL_PORT_TYPES and
             constants.PORT_CAP_SWITCHDEV not in capabilities)
+
+
+def retry(max_=None):
+    def inner(func):
+        def wrapper(*args, **kwargs):
+            local_max = max_ or ovn_conf.get_ovn_ovsdb_retry_max_interval()
+            return tenacity.retry(
+                wait=tenacity.wait_exponential(max=local_max),
+                reraise=True)(func)(*args, **kwargs)
+        return wrapper
+    return inner
diff --git a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py
@@ -199,8 +199,10 @@
                        'br-int | grep "Check pkt length action".')),
 ]
 
-cfg.CONF.register_opts(ovn_opts, group='ovn')
-ovs_conf.register_ovs_agent_opts()
+
+def register_opts():
+    cfg.CONF.register_opts(ovn_opts, group='ovn')
+    ovs_conf.register_ovs_agent_opts()
 
 
 def list_opts():
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py
@@ -117,6 +117,7 @@ def initialize(self):
         self.node_uuid = None
         self.hash_ring_group = ovn_const.HASH_RING_ML2_GROUP
         self.sg_enabled = ovn_acl.is_sg_enabled()
+        ovn_conf.register_opts()
         self._post_fork_event = threading.Event()
         if cfg.CONF.SECURITYGROUP.firewall_driver:
             LOG.warning('Firewall driver configuration is ignored')
diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py
@@ -184,6 +184,7 @@ def setUp(self, maintenance_worker=False, service_plugins=None):
         # ensure viable minimum is set for OVN's Geneve
         ml2_config.cfg.CONF.set_override('max_header_size', 38,
                                          group='ml2_type_geneve')
+        ovn_conf.register_opts()
         ovn_conf.cfg.CONF.set_override('dns_servers',
                                        ['10.10.10.10'],
                                        group='ovn')
diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py
@@ -22,6 +22,7 @@
 from ovsdbapp.tests import utils
 
 from neutron.common.ovn import constants as ovn_const
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb \
     import impl_idl_ovn as impl
 from neutron.services.portforwarding import constants as pf_const
@@ -38,6 +39,7 @@ class BaseOvnIdlTest(n_base.BaseLoggingTestCase,
 
     def setUp(self):
         super(BaseOvnIdlTest, self).setUp()
+        ovn_conf.register_opts()
         self.api = impl.OvsdbSbOvnIdl(self.connection['OVN_Southbound'])
         self.nbapi = impl.OvsdbNbOvnIdl(self.connection['OVN_Northbound'])
         self.handler = ovsdb_event.RowEventHandler()
diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
@@ -801,6 +801,7 @@ class TestCreateDefaultDropPortGroup(base.BaseLoggingTestCase,
 
     def setUp(self):
         super(TestCreateDefaultDropPortGroup, self).setUp()
+        ovn_conf.register_opts()
         self.api = impl_idl_ovn.OvsdbNbOvnIdl(
             self.connection['OVN_Northbound'])
         self.addCleanup(self.api.pg_del(self.PG_NAME, if_exists=True).execute,
diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py
@@ -30,6 +30,7 @@
 from neutron.agent.ovn.metadata import driver
 from neutron.conf.agent.metadata import config as meta_conf
 from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.tests import base
 
 
@@ -53,6 +54,7 @@ def setUp(self):
             meta_conf.METADATA_PROXY_HANDLER_OPTS, self.conf)
         ovn_meta_conf.register_meta_conf_opts(
             ovn_meta_conf.OVS_OPTS, self.conf, group='ovs')
+        ovn_conf.register_opts()
 
 
 class TestMetadataAgent(base.BaseTestCase):
diff --git a/neutron/tests/unit/agent/ovn/metadata/test_driver.py b/neutron/tests/unit/agent/ovn/metadata/test_driver.py
@@ -24,6 +24,7 @@
 from neutron.agent.ovn.metadata import driver as metadata_driver
 from neutron.conf.agent.metadata import config as meta_conf
 from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.tests import base
 from neutron.tests.unit.agent.linux import test_utils
 
@@ -44,6 +45,7 @@ def setUp(self):
         mock.patch('eventlet.spawn').start()
 
         ovn_meta_conf.register_meta_conf_opts(meta_conf.SHARED_OPTS, cfg.CONF)
+        ovn_conf.register_opts()
 
     def test_spawn_metadata_proxy(self):
         datapath_id = _uuid()
diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py
@@ -26,6 +26,7 @@
 
 from neutron.common.ovn import constants
 from neutron.common.ovn import utils
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.tests import base
 from neutron.tests.unit import fake_resources as fakes
 
@@ -375,6 +376,10 @@ def test_get_lsp_dhcp_opts(self):
 
 class TestGetDhcpDnsServers(base.BaseTestCase):
 
+    def setUp(self):
+        ovn_conf.register_opts()
+        super(TestGetDhcpDnsServers, self).setUp()
+
     def test_ipv4(self):
         # DNS servers from subnet.
         dns_servers = utils.get_dhcp_dns_servers(
@@ -701,3 +706,49 @@ def test_capability_only_allowed(self):
             utils.validate_and_get_data_from_binding_profile(
                 {portbindings.VNIC_TYPE: self.VNIC_FAKE_OTHER,
                  constants.OVN_PORT_BINDING_PROFILE: binding_profile}))
+
+
+class TestRetryDecorator(base.BaseTestCase):
+    DEFAULT_RETRY_VALUE = 10
+
+    def setUp(self):
+        super().setUp()
+        mock.patch.object(
+            ovn_conf, "get_ovn_ovsdb_retry_max_interval",
+            return_value=self.DEFAULT_RETRY_VALUE).start()
+
+    def test_default_retry_value(self):
+        with mock.patch('tenacity.wait_exponential') as m_wait:
+            @utils.retry()
+            def decorated_method():
+                pass
+
+            decorated_method()
+        m_wait.assert_called_with(max=self.DEFAULT_RETRY_VALUE)
+
+    def test_custom_retry_value(self):
+        custom_value = 3
+        with mock.patch('tenacity.wait_exponential') as m_wait:
+            @utils.retry(max_=custom_value)
+            def decorated_method():
+                pass
+
+            decorated_method()
+        m_wait.assert_called_with(max=custom_value)
+
+    def test_positive_result(self):
+        number_of_exceptions = 3
+        method = mock.Mock(
+            side_effect=[Exception() for i in range(number_of_exceptions)])
+
+        @utils.retry(max_=0.001)
+        def decorated_method():
+            try:
+                method()
+            except StopIteration:
+                return
+
+        decorated_method()
+
+        # number of exceptions + one successful call
+        self.assertEqual(number_of_exceptions + 1, method.call_count)
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py
@@ -26,6 +26,7 @@
 
 from neutron.api import extensions
 from neutron.common.ovn import constants as ovn_const
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.core_extensions import qos as core_qos
 from neutron.objects import network as network_obj
 from neutron.objects import ports as port_obj
@@ -61,6 +62,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase):
                  'TestFloatingIPQoSL3NatServicePlugin')
 
     def setUp(self):
+        ovn_conf.register_opts()
         cfg.CONF.set_override('extension_drivers', self._extension_drivers,
                               group='ml2')
         cfg.CONF.set_override('enable_distributed_floating_ip', 'False',
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
@@ -83,6 +83,7 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
                                      test_sg.Ml2SecurityGroupsTestCase):
 
     def setUp(self):
+        ovn_conf.register_opts()
         super(TestDBInconsistenciesPeriodics, self).setUp()
         self.net = self._make_network(
             self.fmt, name='net1', admin_state_up=True)['network']
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py
@@ -16,6 +16,7 @@
 from unittest import mock
 
 from neutron.common.ovn import constants
+from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
 from neutron.tests import base
 from neutron.tests.unit import fake_resources as fakes
@@ -25,6 +26,7 @@
 class TestOVNClientBase(base.BaseTestCase):
 
     def setUp(self):
+        ovn_conf.register_opts()
         super(TestOVNClientBase, self).setUp()
         self.nb_idl = mock.MagicMock()
         self.sb_idl = mock.MagicMock()
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py
@@ -198,6 +198,7 @@ def test_connection_sb_start(self):
 class TestOvnIdlDistributedLock(base.BaseTestCase):
 
     def setUp(self):
+        ovn_conf.register_opts()
         super(TestOvnIdlDistributedLock, self).setUp()
         self.node_uuid = uuidutils.generate_uuid()
         self.fake_driver = mock.Mock()
@@ -724,6 +725,10 @@ def test_handle_ha_chassis_group_changes_update_new_gw(self):
 
 
 class TestShortLivingOvsdbApi(base.BaseTestCase):
+    def setUp(self):
+        ovn_conf.register_opts()
+        super().setUp()
+
     def test_context(self):
         api_class = mock.Mock()
         idl = mock.Mock()
diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py
@@ -140,6 +140,7 @@ def setUp(self):
         # ensure viable minimum is set for OVN's Geneve
         cfg.CONF.set_override('max_header_size', 38,
                               group='ml2_type_geneve')
+        ovn_conf.register_opts()
         ovn_conf.cfg.CONF.set_override('ovn_metadata_enabled', False,
                                        group='ovn')
         ovn_conf.cfg.CONF.set_override('dns_servers', ['8.8.8.8'],
@@ -2588,6 +2589,7 @@ class OVNMechanismDriverTestCase(MechDriverSetupBase,
     _mechanism_drivers = ['logger', 'ovn']
 
     def setUp(self):
+        ovn_conf.register_opts()
         cfg.CONF.set_override('global_physnet_mtu', 1550)
         cfg.CONF.set_override('tenant_network_types',
                               ['geneve'],
