Merge "[OVN] Retry retrieving LSP hosting information"

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -41,6 +41,7 @@ from oslo_utils import timeutils
 from ovsdbapp.backend.ovs_idl import idlutils
 import tenacity
 
+from neutron._i18n import _
 from neutron.common.ovn import acl as ovn_acl
 from neutron.common.ovn import constants as ovn_const
 from neutron.common.ovn import utils
@@ -48,6 +49,7 @@ from neutron.common import utils as common_utils
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
 from neutron.db import ovn_revision_numbers_db as db_rev
 from neutron.db import segments_db
+from neutron.plugins.ml2 import db as ml2_db
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions \
     import placement as placement_extension
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions \
@@ -231,6 +233,29 @@ class OVNClient(object):
             external_ids=subnet_dhcp_options['external_ids'])
         return {'cmd': add_dhcp_opts_cmd}
 
+    @tenacity.retry(retry=tenacity.retry_if_exception_type(RuntimeError),
+                    wait=tenacity.wait_random(min=2, max=3),
+                    stop=tenacity.stop_after_attempt(3),
+                    reraise=True)
+    def _wait_for_port_bindings_host(self, context, port_id):
+        db_port = ml2_db.get_port(context, port_id)
+        # This is already checked previously but, just to stay on
+        # the safe side in case the port is deleted mid-operation
+        if not db_port:
+            raise RuntimeError(
+                _('No port found with ID %s') % port_id)
+
+        if not db_port.port_bindings:
+            raise RuntimeError(
+                _('No port bindings information found for  '
+                  'port %s') % port_id)
+
+        if not db_port.port_bindings[0].host:
+            raise RuntimeError(
+                _('No hosting information found for port %s') % port_id)
+
+        return db_port
+
     def update_lsp_host_info(self, context, db_port, up=True):
         """Update the binding hosting information for the LSP.
 
@@ -246,8 +271,19 @@ class OVNClient(object):
         if up:
             if not db_port.port_bindings:
                 return
-            host = db_port.port_bindings[0].host
 
+            if not db_port.port_bindings[0].host:
+                # NOTE(lucasgomes): There might be a sync issue between
+                # the moment that this port was fetched from the database
+                # and the hosting information being set, retry a few times
+                try:
+                    db_port = self._wait_for_port_bindings_host(
+                        context, db_port.id)
+                except RuntimeError as e:
+                    LOG.warning(e)
+                    return
+
+            host = db_port.port_bindings[0].host
             ext_ids = ('external_ids',
                        {ovn_const.OVN_HOST_ID_EXT_ID_KEY: host})
             cmd.append(

neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py

@@ -18,6 +18,7 @@ from unittest import mock
 from neutron.common.ovn import constants
 from neutron.conf.plugins.ml2 import config as ml2_conf
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf
+from neutron.plugins.ml2 import db as ml2_db
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
 from neutron.tests import base
 from neutron.tests.unit.services.logapi.drivers.ovn \
@@ -26,6 +27,8 @@ from neutron_lib.api.definitions import l3
 from neutron_lib import constants as const
 from neutron_lib.services.logapi import constants as log_const
 
+from tenacity import wait_none
+
 
 class TestOVNClientBase(base.BaseTestCase):
 
@@ -45,6 +48,9 @@ class TestOVNClient(TestOVNClientBase):
         self.get_plugin = mock.patch(
             'neutron_lib.plugins.directory.get_plugin').start()
 
+        # Disable tenacity wait for UT
+        self.ovn_client._wait_for_port_bindings_host.retry.wait = wait_none()
+
     def test__add_router_ext_gw_default_route(self):
         plugin = mock.MagicMock()
         self.get_plugin.return_value = plugin
@@ -116,6 +122,45 @@ class TestOVNClient(TestOVNClientBase):
             'Logical_Switch_Port', port_id,
             ('external_ids', {constants.OVN_HOST_ID_EXT_ID_KEY: host_id}))
 
+    def test_update_lsp_host_info_up_retry(self):
+        context = mock.MagicMock()
+        host_id = 'fake-binding-host-id'
+        port_id = 'fake-port-id'
+        db_port_no_host = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host="")])
+        db_port = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host=host_id)])
+
+        with mock.patch.object(
+                self.ovn_client, '_wait_for_port_bindings_host') as mock_wait:
+            mock_wait.return_value = db_port
+            self.ovn_client.update_lsp_host_info(context, db_port_no_host)
+
+            # Assert _wait_for_port_bindings_host was called
+            mock_wait.assert_called_once_with(context, port_id)
+
+        # Assert host_id was set
+        self.nb_idl.db_set.assert_called_once_with(
+            'Logical_Switch_Port', port_id,
+            ('external_ids', {constants.OVN_HOST_ID_EXT_ID_KEY: host_id}))
+
+    def test_update_lsp_host_info_up_retry_fail(self):
+        context = mock.MagicMock()
+        port_id = 'fake-port-id'
+        db_port_no_host = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host="")])
+
+        with mock.patch.object(
+                self.ovn_client, '_wait_for_port_bindings_host') as mock_wait:
+            mock_wait.side_effect = RuntimeError("boom")
+            self.ovn_client.update_lsp_host_info(context, db_port_no_host)
+
+            # Assert _wait_for_port_bindings_host was called
+            mock_wait.assert_called_once_with(context, port_id)
+
+        # Assert host_id was NOT set
+        self.assertFalse(self.nb_idl.db_set.called)
+
     def test_update_lsp_host_info_down(self):
         context = mock.MagicMock()
         port_id = 'fake-port-id'
@@ -127,6 +172,47 @@ class TestOVNClient(TestOVNClientBase):
             'Logical_Switch_Port', port_id, 'external_ids',
             constants.OVN_HOST_ID_EXT_ID_KEY, if_exists=True)
 
+    @mock.patch.object(ml2_db, 'get_port')
+    def test__wait_for_port_bindings_host(self, mock_get_port):
+        context = mock.MagicMock()
+        host_id = 'fake-binding-host-id'
+        port_id = 'fake-port-id'
+        db_port_no_host = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host="")])
+        db_port = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host=host_id)])
+
+        mock_get_port.side_effect = (db_port_no_host, db_port)
+
+        ret = self.ovn_client._wait_for_port_bindings_host(
+            context, port_id)
+
+        self.assertEqual(ret, db_port)
+
+        expected_calls = [mock.call(context, port_id),
+                          mock.call(context, port_id)]
+        mock_get_port.assert_has_calls(expected_calls)
+
+    @mock.patch.object(ml2_db, 'get_port')
+    def test__wait_for_port_bindings_host_fail(self, mock_get_port):
+        context = mock.MagicMock()
+        port_id = 'fake-port-id'
+        db_port_no_pb = mock.Mock(id=port_id, port_bindings=[])
+        db_port_no_host = mock.Mock(
+            id=port_id, port_bindings=[mock.Mock(host="")])
+
+        mock_get_port.side_effect = (
+            db_port_no_pb, db_port_no_host, db_port_no_host)
+
+        self.assertRaises(
+            RuntimeError, self.ovn_client._wait_for_port_bindings_host,
+            context, port_id)
+
+        expected_calls = [mock.call(context, port_id),
+                          mock.call(context, port_id),
+                          mock.call(context, port_id)]
+        mock_get_port.assert_has_calls(expected_calls)
+
 
 class TestOVNClientFairMeter(TestOVNClientBase,
                              test_log_driver.TestOVNDriverBase):