[OVN] Fix DuplicateOptionError on test scope

OVN Octavia Provider is registering opts a soon modules (driver,
agent or helper) are imported, so in a test scope terms, when some
tests run the setUp they are broken by a DuplicateOptionError
because they are based on TestOVNFunctionalBase from Neutron
where same options are loaded to same oslo_config group.

This fix proposes some refactoring in a similar way to [1] and [2]
in order to avoid the registration of opts a soon a module is
imported, instead we switch to one charge per required class
whenever possible.

Additionally in order to fix gates and according to [3], we pin
version of OVS to d94cd0d3eec33e4290d7ca81918f5ac61444886e hash

[1] https://review.opendev.org/c/openstack/neutron/+/837392
[2] https://review.opendev.org/c/openstack/neutron/+/839783
[3] f93206ce40

Related-Bug: #1972278
Related-Bug: #1967472

Change-Id: I2f36af767a0a0a4c19488b6998a414b8672114f5

ovn_octavia_provider/agent.py

@@ -19,7 +19,6 @@ from ovn_octavia_provider import event as ovn_event
 from ovn_octavia_provider import helper as ovn_helper
 from ovn_octavia_provider.ovsdb import impl_idl_ovn
 
-ovn_conf.register_opts()
 
 LOG = logging.getLogger(__name__)
 
@@ -28,6 +27,13 @@ OVN_EVENT_LOCK_NAME = "neutron_ovn_octavia_event_lock"
 
 def OvnProviderAgent(exit_event):
 
+    # NOTE (froyo): Move inside class in order to avoid
+    # the issues on test scope colliding with Neutron
+    # already registered options when this register was
+    # called from outside of the class a soon this module
+    # was imported, also to cover requirement from
+    # OvnProviderHelper and intra references modules
+    ovn_conf.register_opts()
     helper = ovn_helper.OvnProviderHelper()
     events = [ovn_event.LogicalRouterPortEvent(helper),
               ovn_event.LogicalSwitchPortUpdateEvent(helper)]

ovn_octavia_provider/common/config.py

@@ -98,7 +98,17 @@ neutron_opts = [
 
 
 def register_opts():
-    cfg.CONF.register_opts(ovn_opts, group='ovn')
+    # NOTE (froyo): just to not try to re-register options already done
+    # by Neutron, specially in test scope, that will get a DuplicateOptError
+    missing_opts = ovn_opts
+    try:
+        neutron_registered_opts = [opt for opt in cfg.CONF.ovn]
+        missing_opts = [opt for opt in ovn_opts
+                        if opt.name not in neutron_registered_opts]
+    except cfg.NoSuchOptError:
+        LOG.info('Not found any opts under group ovn registered by Neutron')
+
+    cfg.CONF.register_opts(missing_opts, group='ovn')
     cfg.CONF.register_opts(neutron_opts, group='neutron')
     ks_loading.register_auth_conf_options(cfg.CONF, 'service_auth')
     ks_loading.register_session_conf_options(cfg.CONF, 'service_auth')

ovn_octavia_provider/common/utils.py

@@ -11,7 +11,9 @@
 #    under the License.
 
 from oslo_utils import netutils
+import tenacity
 
+from ovn_octavia_provider.common import config
 from ovn_octavia_provider.common import constants
 
 
@@ -58,3 +60,14 @@ def remove_macs_from_lsp_addresses(addresses):
                        (netutils.is_valid_ipv4(x) or
                         netutils.is_valid_ipv6(x))])
     return ip_list
+
+
+def retry(max_=None):
+    def inner(func):
+        def wrapper(*args, **kwargs):
+            local_max = max_ or config.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

ovn_octavia_provider/driver.py

@@ -29,7 +29,6 @@ from ovn_octavia_provider.common import exceptions as ovn_exc
 from ovn_octavia_provider import helper as ovn_helper
 from ovn_octavia_provider.i18n import _
 
-ovn_conf.register_opts()
 
 LOG = logging.getLogger(__name__)
 
@@ -38,6 +37,14 @@ class OvnProviderDriver(driver_base.ProviderDriver):
 
     def __init__(self):
         super().__init__()
+
+        # NOTE (froyo): Move inside init method in order to
+        # avoid the issues on test scope colliding with Neutron
+        # already registered options when this register was
+        # called from outside of the class a soon this module
+        # was imported, also to cover requirement from
+        # OvnProviderHelper and intra references modules
+        ovn_conf.register_opts()
         self._ovn_helper = ovn_helper.OvnProviderHelper()
 
     def __del__(self):

ovn_octavia_provider/helper.py

@@ -43,7 +43,6 @@ from ovn_octavia_provider.i18n import _
 from ovn_octavia_provider.ovsdb import impl_idl_ovn
 
 CONF = cfg.CONF  # Gets Octavia Conf as it runs under o-api domain
-ovn_conf.register_opts()
 
 LOG = logging.getLogger(__name__)
 

ovn_octavia_provider/ovsdb/impl_idl_ovn.py

@@ -27,11 +27,11 @@ import tenacity
 
 from ovn_octavia_provider.common import config
 from ovn_octavia_provider.common import exceptions as ovn_exc
+from ovn_octavia_provider.common import utils
 from ovn_octavia_provider.i18n import _
 from ovn_octavia_provider.ovsdb import impl_idl_ovn
 from ovn_octavia_provider.ovsdb import ovsdb_monitor
 
-config.register_opts()
 
 LOG = log.getLogger(__name__)
 
@@ -199,10 +199,7 @@ class OvnNbIdlForLb(ovsdb_monitor.OvnIdl):
             self.set_lock(self.event_lock_name)
         atexit.register(self.stop)
 
-    @tenacity.retry(
-        wait=tenacity.wait_exponential(
-            max=config.get_ovn_ovsdb_retry_max_interval()),
-        reraise=True)
+    @utils.retry()
     def _get_ovsdb_helper(self, connection_string):
         return idlutils.get_schema_helper(connection_string, self.SCHEMA)
 
@@ -241,10 +238,7 @@ class OvnSbIdlForLb(ovsdb_monitor.OvnIdl):
             self.set_lock(self.event_lock_name)
         atexit.register(self.stop)
 
-    @tenacity.retry(
-        wait=tenacity.wait_exponential(
-            max=config.get_ovn_ovsdb_retry_max_interval()),
-        reraise=True)
+    @utils.retry()
     def _get_ovsdb_helper(self, connection_string):
         return idlutils.get_schema_helper(connection_string, self.SCHEMA)
 

ovn_octavia_provider/tests/functional/test_agent.py

@@ -19,6 +19,7 @@ import multiprocessing as mp
 from neutron.common import utils as n_utils
 
 from ovn_octavia_provider import agent as ovn_agent
+from ovn_octavia_provider.common import config as ovn_config
 from ovn_octavia_provider.common import constants as ovn_const
 from ovn_octavia_provider import event as ovn_event
 from ovn_octavia_provider import helper as ovn_helper
@@ -37,6 +38,7 @@ class TestOvnOctaviaProviderAgent(ovn_base.TestOvnOctaviaBase):
         # with IDL running, but to make it easier for now
         # we can initialize this IDL here instead spawning
         # another process.
+        ovn_config.register_opts()
         da_helper = ovn_helper.OvnProviderHelper()
         events = [ovn_event.LogicalRouterPortEvent(da_helper),
                   ovn_event.LogicalSwitchPortUpdateEvent(da_helper)]

ovn_octavia_provider/tests/unit/common/test_utils.py

@@ -0,0 +1,68 @@
+# Copyright 2022 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 unittest import mock
+
+from neutron.tests import base
+
+from ovn_octavia_provider.common import config as ovn_config
+from ovn_octavia_provider.common import utils
+
+
+class TestRetryDecorator(base.BaseTestCase):
+    DEFAULT_RETRY_VALUE = 10
+
+    def setUp(self):
+        super().setUp()
+        mock.patch.object(
+            ovn_config, "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)

ovn_octavia_provider/tests/unit/ovsdb/test_impl_idl_ovn.py

@@ -20,6 +20,7 @@ from ovs.db import idl as ovs_idl
 from ovsdbapp.backend import ovs_idl as real_ovs_idl
 from ovsdbapp.backend.ovs_idl import idlutils
 
+from ovn_octavia_provider.common import config as ovn_config
 from ovn_octavia_provider.ovsdb import impl_idl_ovn
 
 basedir = os.path.dirname(os.path.abspath(__file__))
@@ -34,6 +35,7 @@ class TestOvnNbIdlForLb(base.BaseTestCase):
 
     def setUp(self):
         super().setUp()
+        ovn_config.register_opts()
         # TODO(haleyb) - figure out why every test in this class generates
         # this warning, think it's in relation to reading this schema file:
         # sys:1: ResourceWarning: unclosed file <_io.FileIO name=1 mode='wb'
@@ -97,6 +99,7 @@ class TestOvnSbIdlForLb(base.BaseTestCase):
 
     def setUp(self):
         super().setUp()
+        ovn_config.register_opts()
         # TODO(haleyb) - figure out why every test in this class generates
         # this warning, think it's in relation to reading this schema file:
         # sys:1: ResourceWarning: unclosed file <_io.FileIO name=1 mode='wb'

ovn_octavia_provider/tests/unit/test_helper.py

@@ -23,6 +23,7 @@ from oslo_utils import uuidutils
 from ovsdbapp.backend.ovs_idl import idlutils
 
 from ovn_octavia_provider.common import clients
+from ovn_octavia_provider.common import config as ovn_conf
 from ovn_octavia_provider.common import constants as ovn_const
 from ovn_octavia_provider import event as ovn_event
 from ovn_octavia_provider import helper as ovn_helper
@@ -34,6 +35,7 @@ class TestOvnProviderHelper(ovn_base.TestOvnOctaviaBase):
 
     def setUp(self):
         super().setUp()
+        ovn_conf.register_opts()
         self.helper = ovn_helper.OvnProviderHelper()
         self.real_helper_find_ovn_lb_with_pool_key = (
             self.helper._find_ovn_lb_with_pool_key)

zuul.d/base.yaml

@@ -51,7 +51,7 @@
       Q_BUILD_OVS_FROM_GIT: True
       INSTALL_OVN: True
       OVN_BRANCH: main
-      OVS_BRANCH: "91e1ff5dde396fbcc8623ac0726066e970e6de15"
+      OVS_BRANCH: "d94cd0d3eec33e4290d7ca81918f5ac61444886e"
 
 - job:
     name: ovn-octavia-provider-tempest-base
@@ -183,4 +183,4 @@
         Q_BUILD_OVS_FROM_GIT: True
         INSTALL_OVN: True
         OVN_BRANCH: main
-        OVS_BRANCH: "91e1ff5dde396fbcc8623ac0726066e970e6de15"
+        OVS_BRANCH: "d94cd0d3eec33e4290d7ca81918f5ac61444886e"