Merge pull request #64 from stackhpc/upstream/yoga-2023-09-04

markgoddard · web-flow · commit 3dbb211c718a · 2023-09-04T09:46:01.000+01:00
Synchronise yoga with upstream
diff --git a/neutron/common/ovn/extensions.py b/neutron/common/ovn/extensions.py
@@ -65,6 +65,7 @@
 from neutron_lib.api.definitions import subnet_dns_publish_fixed_ip
 from neutron_lib.api.definitions import subnet_service_types
 from neutron_lib.api.definitions import trunk
+from neutron_lib.api.definitions import uplink_status_propagation
 from neutron_lib.api.definitions import vlantransparent
 from neutron_lib import constants
 
@@ -149,4 +150,5 @@
     floating_ip_port_forwarding.ALIAS,
     vlantransparent.ALIAS,
     logging.ALIAS,
+    uplink_status_propagation.ALIAS,
 ]
diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py
@@ -836,3 +836,10 @@ def get_ovn_chassis_other_config(chassis):
         return chassis.other_config
     except AttributeError:
         return chassis.external_ids
+
+
+def get_requested_chassis(requested_chassis):
+    """Returns a list with the items in the LSP.options:requested-chassis"""
+    if isinstance(requested_chassis, str):
+        return requested_chassis.split(',')
+    return []
diff --git a/neutron/db/ovn_hash_ring_db.py b/neutron/db/ovn_hash_ring_db.py
@@ -60,6 +60,15 @@ def remove_node_by_uuid(context, node_uuid):
     LOG.info('Node "%s" removed from the Hash Ring', node_uuid)
 
 
+@db_api.retry_if_session_inactive()
+def cleanup_old_nodes(context, days):
+    age = timeutils.utcnow() - datetime.timedelta(days=days)
+    with db_api.CONTEXT_WRITER.using(context):
+        context.session.query(ovn_models.OVNHashRing).filter(
+            ovn_models.OVNHashRing.updated_at < age).delete()
+    LOG.info('Cleaned up Hash Ring nodes older than %d days', days)
+
+
 @db_api.retry_if_session_inactive()
 def _touch(context, updated_at=None, **filter_args):
     if updated_at is None:
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
@@ -160,6 +160,10 @@ def schema_helper(cls):
                                                         cls.schema)
         return cls._schema_helper
 
+    @classmethod
+    def get_schema_version(cls):
+        return cls.schema_helper.schema_json['version']
+
     @classmethod
     def schema_has_table(cls, table_name):
         return table_name in cls.schema_helper.schema_json['tables']
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
@@ -727,7 +727,7 @@ def check_for_localnet_legacy_port_name(self):
                     txn.add(cmd)
         raise periodics.NeverAgain()
 
-    # TODO(lucasagomes): Remove this in the Z cycle
+    # TODO(lucasagomes): Remove this in the B+3 cycle
     # A static spacing value is used here, but this method will only run
     # once per lock due to the use of periodics.NeverAgain().
     @periodics.periodic(spacing=600, run_immediately=True)
@@ -738,21 +738,36 @@ def check_for_mcast_flood_reports(self):
         cmds = []
         for port in self._nb_idl.lsp_list().execute(check_error=True):
             port_type = port.type.strip()
-            if port_type in ("vtep", ovn_const.LSP_TYPE_LOCALPORT, "router"):
-                continue
-
             options = port.options
-            if port_type == ovn_const.LSP_TYPE_LOCALNET:
-                mcast_flood_value = options.get(
+            mcast_flood_reports_value = options.get(
                     ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS)
-                if mcast_flood_value == 'false':
+
+            if self._ovn_client.is_mcast_flood_broken:
+                if port_type in ("vtep", ovn_const.LSP_TYPE_LOCALPORT,
+                                 "router"):
+                    continue
+
+                if port_type == ovn_const.LSP_TYPE_LOCALNET:
+                    mcast_flood_value = options.pop(
+                        ovn_const.LSP_OPTIONS_MCAST_FLOOD, None)
+                    if mcast_flood_value:
+                        cmds.append(self._nb_idl.db_remove(
+                            'Logical_Switch_Port', port.name, 'options',
+                            ovn_const.LSP_OPTIONS_MCAST_FLOOD,
+                            if_exists=True))
+
+                if mcast_flood_reports_value == 'true':
                     continue
-                options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD: 'false'})
-            elif ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS in options:
-                continue
 
-            options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
-            cmds.append(self._nb_idl.lsp_set_options(port.name, **options))
+                options.update(
+                    {ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
+                cmds.append(self._nb_idl.lsp_set_options(port.name, **options))
+
+            elif (mcast_flood_reports_value and port_type !=
+                    ovn_const.LSP_TYPE_LOCALNET):
+                cmds.append(self._nb_idl.db_remove(
+                    'Logical_Switch_Port', port.name, 'options',
+                    ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS, if_exists=True))
 
         if cmds:
             with self._nb_idl.transaction(check_error=True) as txn:
@@ -1111,6 +1126,20 @@ def remove_duplicated_chassis_registers(self):
                     for table in ('Chassis_Private', 'Chassis'):
                         txn.add(self._sb_idl.db_destroy(table, ch.name))
 
+    @periodics.periodic(spacing=86400, run_immediately=True)
+    def cleanup_old_hash_ring_nodes(self):
+        """Daily task to cleanup old stable Hash Ring node entries.
+
+        Runs once a day and clean up Hash Ring entries that haven't
+        been updated in more than 5 days. See LP #2033281 for more
+        information.
+
+        """
+        if not self.has_lock:
+            return
+        context = n_context.get_admin_context()
+        hash_ring_db.cleanup_old_nodes(context, days=5)
+
 
 class HashRingHealthCheckPeriodics(object):
 
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
@@ -39,6 +39,7 @@
 from oslo_log import log
 from oslo_utils import excutils
 from oslo_utils import timeutils
+from oslo_utils import versionutils
 from ovsdbapp.backend.ovs_idl import idlutils
 import tenacity
 
@@ -94,6 +95,7 @@ def __init__(self, nb_idl, sb_idl):
 
         self._plugin_property = None
         self._l3_plugin_property = None
+        self._is_mcast_flood_broken = None
 
         # TODO(ralonsoh): handle the OVN client extensions with an ext. manager
         self._qos_driver = qos_extension.OVNClientQosExtension(driver=self)
@@ -338,6 +340,20 @@ def update_lsp_host_info(self, context, db_port, up=True):
 
         self._transaction(cmd)
 
+    # TODO(lucasagomes): Remove this method and the logic around the broken
+    # mcast_flood_reports configuration option on any other port that is not
+    # type "localnet" when the fixed version of OVN becomes the norm.
+    # The commit in core OVN fixing this issue is the
+    # https://github.com/ovn-org/ovn/commit/6aeeccdf272bc60630581e46aa42d97f4f56d4fa
+    @property
+    def is_mcast_flood_broken(self):
+        if self._is_mcast_flood_broken is None:
+            schema_version = self._nb_idl.get_schema_version()
+            self._is_mcast_flood_broken = (
+                versionutils.convert_version_to_tuple(schema_version) <
+                (6, 3, 0))
+        return self._is_mcast_flood_broken
+
     def _get_port_options(self, port):
         context = n_context.get_admin_context()
         binding_prof = utils.validate_and_get_data_from_binding_profile(port)
@@ -500,12 +516,8 @@ def _get_port_options(self, port):
             if port_type != ovn_const.LSP_TYPE_VIRTUAL:
                 options[ovn_const.LSP_OPTIONS_REQUESTED_CHASSIS_KEY] = chassis
 
-        # TODO(lucasagomes): Enable the mcast_flood_reports by default,
-        # according to core OVN developers it shouldn't cause any harm
-        # and will be ignored when mcast_snoop is False. We can revise
-        # this once https://bugzilla.redhat.com/show_bug.cgi?id=1933990
-        # (see comment #3) is fixed in Core OVN.
-        if port_type not in ('vtep', ovn_const.LSP_TYPE_LOCALPORT, 'router'):
+        if self.is_mcast_flood_broken and port_type not in (
+                'vtep', ovn_const.LSP_TYPE_LOCALPORT, 'router'):
             options.update({ovn_const.LSP_OPTIONS_MCAST_FLOOD_REPORTS: 'true'})
 
         device_owner = port.get('device_owner', '')
diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py
@@ -275,6 +275,15 @@ def match_fn(self, event, row, old=None):
                         {'port': row.logical_port, 'binding': row.uuid})
             return False
 
+        req_chassis = utils.get_requested_chassis(
+            row.options.get(ovn_const.LSP_OPTIONS_REQUESTED_CHASSIS_KEY, ''))
+        if len(req_chassis) > 1:
+            # This event has been issued during a LSP migration. During this
+            # process, the LSP will change the port binding but the port status
+            # will be handled by the ``LogicalSwitchPortUpdateDownEvent`` and
+            # ``LogicalSwitchPortUpdateUpEvent`` events.
+            return False
+
         return bool(lsp.up)
 
     def run(self, event, row, old=None):
diff --git a/neutron/services/trunk/drivers/ovn/trunk_driver.py b/neutron/services/trunk/drivers/ovn/trunk_driver.py
@@ -14,6 +14,7 @@
 from neutron_lib.callbacks import events
 from neutron_lib.callbacks import registry
 from neutron_lib.callbacks import resources
+from neutron_lib import constants as n_const
 from neutron_lib import context as n_context
 from neutron_lib.db import api as db_api
 from neutron_lib import exceptions as n_exc
@@ -51,7 +52,11 @@ def _set_sub_ports(self, parent_port, subports):
         context = n_context.get_admin_context()
         db_parent_port = port_obj.Port.get_object(context, id=parent_port)
         parent_port_status = db_parent_port.status
-        parent_port_bindings = db_parent_port.bindings[0]
+        try:
+            parent_port_bindings = [pb for pb in db_parent_port.bindings
+                                    if pb.status == n_const.ACTIVE][-1]
+        except IndexError:
+            parent_port_bindings = None
         for subport in subports:
             with db_api.CONTEXT_WRITER.using(context), (
                     txn(check_error=True)) as ovn_txn:
@@ -85,8 +90,10 @@ def _set_binding_profile(self, context, subport, parent_port,
             db_port.id, db_port, ovn_const.TYPE_PORTS)
         ovn_txn.add(check_rev_cmd)
         parent_binding_host = ''
-        if parent_port_bindings.host:
-            parent_binding_host = parent_port_bindings.host
+        if parent_port_bindings and parent_port_bindings.host:
+            migrating_to = parent_port_bindings.profile.get(
+                ovn_const.MIGRATING_ATTR)
+            parent_binding_host = migrating_to or parent_port_bindings.host
         try:
             # NOTE(flaviof): We expect binding's host to be set. Otherwise,
             # sub-port will not transition from DOWN to ACTIVE.
diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
@@ -1038,7 +1038,9 @@ def _verify_lb(test, protocol, vip_ext_port, vip_int_port):
             m_publish.reset_mock()
             self.pf_plugin.delete_floatingip_port_forwarding(
                 self.context, pf_obj['id'], fip_id)
-            m_publish.assert_called_once()
+            call = mock.call('port_forwarding', 'after_delete', self.pf_plugin,
+                             payload=mock.ANY)
+            m_publish.assert_has_calls([call])
 
             # Assert load balancer for port forwarding is stale
             _verify_lb(self, 'udp', 5353, 53)
diff --git a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py
@@ -97,13 +97,23 @@ def _verify_trunk_info(self, trunk, has_items, host=''):
         if trunk.get('status'):
             self.assertEqual(trunk_consts.TRUNK_ACTIVE_STATUS, trunk['status'])
 
-    def _bind_port(self, port_id, host):
+    def _bind_port(self, port_id, host_source, host_dest=None):
         with db_api.CONTEXT_WRITER.using(self.context):
-            pb = port_obj.PortBinding.get_object(self.context,
-                                                 port_id=port_id, host='')
-            pb.delete()
-            port_obj.PortBinding(self.context, port_id=port_id, host=host,
-                                 vif_type=portbindings.VIF_TYPE_OVS).create()
+            for pb in port_obj.PortBinding.get_objects(self.context,
+                                                       port_id=port_id):
+                pb.delete()
+            profile = {}
+            if host_dest:
+                # When "host_dest" there are 2 port bindings, as in a live
+                # migration; the second one (destination host) is inactive.
+                profile[ovn_const.MIGRATING_ATTR] = host_dest
+                port_obj.PortBinding(
+                    self.context, port_id=port_id, host=host_dest,
+                    vif_type=portbindings.VIF_TYPE_OVS,
+                    status=n_consts.INACTIVE).create()
+            port_obj.PortBinding(
+                self.context, port_id=port_id, host=host_source,
+                profile=profile, vif_type=portbindings.VIF_TYPE_OVS).create()
 
     def test_trunk_create(self):
         with self.trunk() as trunk:
@@ -148,6 +158,21 @@ def test_subport_add(self):
                 self._verify_trunk_info(new_trunk, has_items=True,
                                         host='host1')
 
+    def test_subport_add_live_migration_multiple_port_binding(self):
+        with self.subport() as subport:
+            with self.trunk() as trunk:
+                self.trunk_plugin.add_subports(self.context, trunk['id'],
+                                               {'sub_ports': [subport]})
+                new_trunk = self.trunk_plugin.get_trunk(self.context,
+                                                        trunk['id'])
+                self._verify_trunk_info(new_trunk, has_items=True)
+                # Bind parent port. That will trigger the binding of the
+                # trunk subports too, using the same host ID.
+                self._bind_port(trunk['port_id'], 'host1', host_dest='host2')
+                self.mech_driver.set_port_status_up(trunk['port_id'])
+                self._verify_trunk_info(new_trunk, has_items=True,
+                                        host='host2')
+
     def test_subport_delete(self):
         with self.subport() as subport:
             with self.trunk([subport]) as trunk:
diff --git a/neutron/tests/unit/db/test_ovn_hash_ring_db.py b/neutron/tests/unit/db/test_ovn_hash_ring_db.py
@@ -285,3 +285,29 @@ def test_remove_node_by_uuid(self):
             self.admin_ctx, interval=60, group_name=HASH_RING_TEST_GROUP)
         self.assertEqual(2, len(active_nodes))
         self.assertNotIn(node_to_remove, [n.node_uuid for n in active_nodes])
+
+    def test_cleanup_old_nodes(self):
+        # Add 2 new nodes
+        self._add_nodes_and_assert_exists(count=2)
+
+        # Subtract 5 days from utcnow() and touch the nodes to make
+        # them to appear stale
+        fake_utcnow = timeutils.utcnow() - datetime.timedelta(days=5)
+        with mock.patch.object(timeutils, 'utcnow') as mock_utcnow:
+            mock_utcnow.return_value = fake_utcnow
+            ovn_hash_ring_db.touch_nodes_from_host(self.admin_ctx,
+                                                   HASH_RING_TEST_GROUP)
+
+        # Add 3 new nodes
+        self._add_nodes_and_assert_exists(count=3)
+
+        # Assert we have 5 nodes in the hash ring
+        self.assertEqual(5, ovn_hash_ring_db.count_nodes_from_host(
+            self.admin_ctx, HASH_RING_TEST_GROUP))
+
+        # Clean up the 2 stale nodes
+        ovn_hash_ring_db.cleanup_old_nodes(self.admin_ctx, days=5)
+
+        # Assert we only have 3 node entries after the clean up
+        self.assertEqual(3, ovn_hash_ring_db.count_nodes_from_host(
+            self.admin_ctx, HASH_RING_TEST_GROUP))
diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py
@@ -160,6 +160,7 @@ def __init__(self, **kwargs):
         self.ha_chassis_group_add_chassis = mock.Mock()
         self.ha_chassis_group_del_chassis = mock.Mock()
         self.lrp_get = mock.Mock()
+        self.get_schema_version = mock.Mock(return_value='3.6.0')
 
 
 class FakeOvsdbSbOvnIdl(object):
@@ -182,6 +183,7 @@ def __init__(self, **kwargs):
         self.is_table_present = mock.Mock()
         self.is_table_present.return_value = False
         self.get_chassis_by_card_serial_from_cms_options = mock.Mock()
+        self.get_schema_version = mock.Mock(return_value='3.6.0')
 
 
 class FakeOvsdbTransaction(object):
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
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
diff --git a/releasenotes/notes/hash-ring-cleanup-1079d2375082cebe.yaml b/releasenotes/notes/hash-ring-cleanup-1079d2375082cebe.yaml
diff --git a/releasenotes/notes/ovn-add-extension-uplink-status-propagation-4c232954f8b4f0ef.yaml b/releasenotes/notes/ovn-add-extension-uplink-status-propagation-4c232954f8b4f0ef.yaml
diff --git a/releasenotes/notes/ovn-mcast_flood_reports-4eee20856ccfc7d7.yaml b/releasenotes/notes/ovn-mcast_flood_reports-4eee20856ccfc7d7.yaml
