From 227c5f8568d0b2499db0d20c36eb65a07e56a893 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Thu, 28 Apr 2022 17:52:55 +0200 Subject: [PATCH] Avoid register config options on imports 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 Co-authored-by: Fernando Royo Change-Id: I4bccb094ee7f690cbc352c38b5b39d505e6ea460 --- neutron/agent/ovn/metadata/agent.py | 6 +-- neutron/agent/ovn/metadata/ovsdb.py | 11 ++-- neutron/agent/ovn/metadata_agent.py | 2 + neutron/cmd/ovn/neutron_ovn_db_sync_util.py | 1 + neutron/common/ovn/utils.py | 12 +++++ .../conf/plugins/ml2/drivers/ovn/ovn_conf.py | 6 ++- .../drivers/ovn/mech_driver/mech_driver.py | 1 + neutron/tests/functional/base.py | 1 + .../ovn/mech_driver/ovsdb/test_impl_idl.py | 2 + .../ovn/mech_driver/test_mech_driver.py | 1 + .../unit/agent/ovn/metadata/test_agent.py | 2 + .../unit/agent/ovn/metadata/test_driver.py | 2 + neutron/tests/unit/common/ovn/test_utils.py | 51 +++++++++++++++++++ .../mech_driver/ovsdb/extensions/test_qos.py | 2 + .../ovn/mech_driver/ovsdb/test_maintenance.py | 1 + .../ovn/mech_driver/ovsdb/test_ovn_client.py | 2 + .../mech_driver/ovsdb/test_ovsdb_monitor.py | 5 ++ .../ovn/mech_driver/test_mech_driver.py | 2 + 18 files changed, 95 insertions(+), 15 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index 5654026d365..9b18c08e807 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -24,7 +24,6 @@ from oslo_log import log 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 @@ class MetadataAgent(object): 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 index 7817bdaf85c..d5ba2d66803 100644 --- a/neutron/agent/ovn/metadata/ovsdb.py +++ b/neutron/agent/ovn/metadata/ovsdb.py @@ -17,8 +17,8 @@ from ovs.db import idl 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 @@ class MetadataAgentOvnSbIdl(ovsdb_monitor.OvnIdl): 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 index 8afaf0cef5b..df188809bd7 100644 --- a/neutron/agent/ovn/metadata_agent.py +++ b/neutron/agent/ovn/metadata_agent.py @@ -21,12 +21,14 @@ from oslo_log import log as logging 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(): config.register_common_config_options() + 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 index 9370cabe4fd..3cc1eed31db 100644 --- a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py +++ b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py @@ -139,6 +139,7 @@ class AgentNotifierApi(object): def setup_conf(): conf = cfg.CONF common_config.register_common_config_options() + 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 index 1d6a219e26f..0f615cdd4b6 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -34,6 +34,7 @@ from oslo_serialization import jsonutils 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 @@ -693,3 +694,14 @@ def connection_config_to_target_string(connection_config): _dict['ip']) elif _dict['file']: return 'p' + _dict['proto'] + ':' + _dict['file'] + + +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 index 22411b31b87..db07b160e79 100644 --- a/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py +++ b/neutron/conf/plugins/ml2/drivers/ovn/ovn_conf.py @@ -209,8 +209,10 @@ ovn_opts = [ '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 index 9e9d9c94669..54c5261881e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -118,6 +118,7 @@ class OVNMechanismDriver(api.MechanismDriver): # NOTE(lucasagomes): _clean_hash_ring() must be called before # self.subscribe() to avoid processes racing when adding or # deleting nodes from the Hash Ring during service initialization + ovn_conf.register_opts() self._clean_hash_ring() self._post_fork_event = threading.Event() if cfg.CONF.SECURITYGROUP.firewall_driver: diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index dfac36d8571..5f3d1f74395 100644 --- a/neutron/tests/functional/base.py +++ b/neutron/tests/functional/base.py @@ -183,6 +183,7 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase, # 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 index 219c0f53dc6..17b5fef250a 100644 --- 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.functional import base 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 index 95fe9303f2d..6844154faa5 100644 --- 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 @@ -731,6 +731,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 index 1a01469f8c1..041433a3758 100644 --- 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 agent 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 @@ class ConfFixture(config_fixture.Config): 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 index 2a497c7aeb6..e5b2abd28d4 100644 --- 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 agent as metadata_agent 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 @@ class TestMetadataDriverProcess(base.BaseTestCase): 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 index 43cf62e1a8b..180e53e66ce 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -26,6 +26,7 @@ from oslo_config import cfg 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 @@ -407,6 +408,10 @@ class TestConnectionConfigToTargetString(base.BaseTestCase): 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( @@ -710,3 +715,49 @@ class TestValidateAndGetDataFromBindingProfile(base.BaseTestCase): 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 index 793f8488619..4174f2b1ee9 100644 --- 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 @@ -29,6 +29,7 @@ from oslo_utils import uuidutils 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.db import l3_fip_qos from neutron.db import l3_gateway_ip_qos @@ -76,6 +77,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): 'test_qos.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 index ea189b6a403..ed8500618b1 100644 --- 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 index 8333d2281be..aff755e0696 100644 --- 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 @@ from neutron_lib.api.definitions import portbindings 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 index 2f5329119a9..e00eeb2f289 100644 --- 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 @@ -206,6 +206,7 @@ class TestOvnConnection(base.BaseTestCase): 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() @@ -737,6 +738,10 @@ class TestChassisEvent(base.BaseTestCase): 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 index 93bce25d511..4eaa2d6723c 100644 --- 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 @@ -104,6 +104,7 @@ class TestOVNMechanismDriverBase(MechDriverSetupBase, # 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'], @@ -2397,6 +2398,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'],