Merge pull request #63 from stackhpc/upstream/yoga-2023-08-28

Synchronise yoga with upstream

Author: markgoddard

neutron/agent/ovn/metadata/agent.py

@@ -339,7 +339,8 @@ def sync(self):
             for ns in ip_lib.list_network_namespaces())
         net_datapaths = self.get_networks_datapaths()
         metadata_namespaces = [
-            self._get_namespace_name(str(datapath.uuid))
+            self._get_namespace_name(
+                ovn_utils.get_network_name_from_datapath(datapath))
             for datapath in net_datapaths
         ]
         unused_namespaces = [ns for ns in system_namespaces if
@@ -348,8 +349,9 @@ def sync(self):
         for ns in unused_namespaces:
             self.teardown_datapath(self._get_datapath_name(ns))
 
-        # now that all obsolete namespaces are cleaned up, deploy required
-        # networks
+        # resync all network namespaces based on the associated datapaths,
+        # even those that are already running. This is to make sure
+        # everything within each namespace is up to date.
         for datapath in net_datapaths:
             self.provision_datapath(datapath)
 

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

@@ -42,13 +42,15 @@
 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
 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 \
@@ -272,6 +274,29 @@ def determine_bind_host(self, port, port_context=None):
                     ovn_const.VIF_DETAILS_CARD_SERIAL_NUMBER]).hostname
         return ''
 
+    @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.
 
@@ -287,8 +312,19 @@ def update_lsp_host_info(self, context, db_port, up=True):
         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/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py

@@ -446,6 +446,36 @@ def _delete_floatingip_pfs(self, context, fip_id, txn):
         self.l3_plugin.port_forwarding.db_sync_delete(
             context, fip_id, txn)
 
+    def _is_router_port_changed(self, db_router_port, lrport_nets):
+        """Check if the router port needs to be updated.
+
+        This method checks for networks and ipv6_ra_configs (if supported)
+        changes on a given router port.
+         """
+        db_lrport_nets = db_router_port['networks']
+        if db_lrport_nets != lrport_nets:
+            return True
+
+        # Check for ipv6_ra_configs changes
+        db_lrport_ra = db_router_port['ipv6_ra_configs']
+        lrport_ra = {}
+        ipv6_ra_supported = self.ovn_api.is_col_present(
+            'Logical_Router_Port', 'ipv6_ra_configs')
+        if ipv6_ra_supported:
+            lrp_name = utils.ovn_lrouter_port_name(db_router_port['id'])
+            try:
+                ovn_lrport = self.ovn_api.lrp_get(
+                    lrp_name).execute(check_error=True)
+            except idlutils.RowNotFound:
+                # If the port is not found in the OVN database the
+                # ovn-db-sync script will recreate this port later
+                # and it will have the latest information. No need
+                # to update it.
+                return False
+            lrport_ra = ovn_lrport.ipv6_ra_configs
+
+        return db_lrport_ra != lrport_ra
+
     def sync_routers_and_rports(self, ctx):
         """Sync Routers between neutron and NB.
 
@@ -525,6 +555,12 @@ def sync_routers_and_rports(self, ctx):
              constants.DEVICE_OWNER_HA_REPLICATED_INT])
         for interface in interfaces:
             db_router_ports[interface['id']] = interface
+            networks, ipv6_ra_configs = (
+                self._ovn_client._get_nets_and_ipv6_ra_confs_for_router_port(
+                    ctx, interface))
+            db_router_ports[interface['id']]['networks'] = networks
+            db_router_ports[interface['id']][
+                'ipv6_ra_configs'] = ipv6_ra_configs
 
         lrouters = self.ovn_api.get_all_logical_routers_with_rports()
 
@@ -541,11 +577,9 @@ def sync_routers_and_rports(self, ctx):
             if lrouter['name'] in db_routers:
                 for lrport, lrport_nets in lrouter['ports'].items():
                     if lrport in db_router_ports:
-                        # We dont have to check for the networks and
-                        # ipv6_ra_configs values. Lets add it to the
-                        # update_lrport_list. If they are in sync, then
-                        # update_router_port will be a no-op.
-                        update_lrport_list.append(db_router_ports[lrport])
+                        if self._is_router_port_changed(
+                                db_router_ports[lrport], lrport_nets):
+                            update_lrport_list.append(db_router_ports[lrport])
                         del db_router_ports[lrport]
                     else:
                         del_lrouter_ports_list.append(

neutron/tests/unit/agent/ovn/metadata/test_agent.py

@@ -83,7 +83,8 @@ def setUp(self):
 
         self.ports = []
         for i in range(0, 3):
-            self.ports.append(makePort(datapath=DatapathInfo(uuid=str(i),
+            self.ports.append(makePort(
+                datapath=DatapathInfo(uuid=str(uuid.uuid4()),
                 external_ids={'name': 'neutron-%d' % i})))
         self.agent.sb_idl.get_ports_on_chassis.return_value = self.ports
 

neutron/tests/unit/fake_resources.py

@@ -159,6 +159,7 @@ def __init__(self, **kwargs):
         self.ha_chassis_group_del = mock.Mock()
         self.ha_chassis_group_add_chassis = mock.Mock()
         self.ha_chassis_group_del_chassis = mock.Mock()
+        self.lrp_get = mock.Mock()
 
 
 class FakeOvsdbSbOvnIdl(object):

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

@@ -17,6 +17,7 @@
 
 from neutron.common.ovn import constants
 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 import fake_resources as fakes
@@ -25,6 +26,8 @@
 from neutron_lib.api.definitions import portbindings
 from neutron_lib.services.logapi import constants as log_const
 
+from tenacity import wait_none
+
 
 class TestOVNClientBase(base.BaseTestCase):
 
@@ -47,6 +50,9 @@ def setUp(self):
             fakes.FakeChassis.create(
                 attrs={'hostname': self.fake_smartnic_hostname}))
 
+        # Disable tenacity wait for UT
+        self.ovn_client._wait_for_port_bindings_host.retry.wait = wait_none()
+
     def test_vnic_normal_unbound_port(self):
         self.assertEqual(
             '',
@@ -139,6 +145,45 @@ def test_update_lsp_host_info_up(self):
             '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'
@@ -150,6 +195,47 @@ def test_update_lsp_host_info_down(self):
             '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):

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

@@ -23,6 +23,7 @@
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client
 from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync
 from neutron.services.ovn_l3 import plugin as ovn_plugin
+from neutron.tests.unit import fake_resources as fakes
 from neutron.tests.unit.plugins.ml2.drivers.ovn.mech_driver import \
     test_mech_driver
 
@@ -1109,6 +1110,47 @@ def test_ovn_nb_sync_calculate_routes_add_remove_keep_two_routes(self):
                                                        expected_deleted)
 
 
+class TestIsRouterPortChanged(test_mech_driver.OVNMechanismDriverTestCase):
+
+    def setUp(self):
+        super(TestIsRouterPortChanged, self).setUp()
+        self.ovn_nb_synchronizer = ovn_db_sync.OvnNbSynchronizer(
+            self.plugin, self.mech_driver.nb_ovn, self.mech_driver.sb_ovn,
+            'log', self.mech_driver)
+
+        self.db_router_port = {
+            'id': 'aa076509-915d-4b1c-8d9d-3db53d9c5faf',
+            'networks': ['fdf9:ad62:3a04::1/64'],
+            'ipv6_ra_configs': {'address_mode': 'slaac',
+                                'send_periodic': 'true',
+                                'mtu': '1442'}
+        }
+        self.lrport_nets = ['fdf9:ad62:3a04::1/64']
+        self.ovn_lrport = fakes.FakeOvsdbRow.create_one_ovsdb_row(
+            attrs={'ipv6_ra_configs': {'address_mode': 'slaac',
+                                       'send_periodic': 'true',
+                                       'mtu': '1442'}})
+
+        self.ovn_nb_synchronizer.ovn_api.is_col_present.return_value = True
+        self.ovn_nb_synchronizer.ovn_api.lrp_get().execute.return_value = (
+            self.ovn_lrport)
+
+    def test__is_router_port_changed_not_changed(self):
+        self.assertFalse(self.ovn_nb_synchronizer._is_router_port_changed(
+            self.db_router_port, self.lrport_nets))
+
+    def test__is_router_port_changed_network_changed(self):
+        self.db_router_port['networks'] = ['172.24.4.26/24',
+                                           '2001:db8::206/64']
+        self.assertTrue(self.ovn_nb_synchronizer._is_router_port_changed(
+            self.db_router_port, self.lrport_nets))
+
+    def test__is_router_port_changed_ipv6_ra_configs_changed(self):
+        self.db_router_port['ipv6_ra_configs']['mtu'] = '1500'
+        self.assertTrue(self.ovn_nb_synchronizer._is_router_port_changed(
+            self.db_router_port, self.lrport_nets))
+
+
 class TestOvnSbSyncML2(test_mech_driver.OVNMechanismDriverTestCase):
 
     def test_ovn_sb_sync(self):