Merge pull request #114 from stackhpc/upstream/yoga-2024-01-22

Synchronise yoga with upstream

Author: markgoddard

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

@@ -1344,7 +1344,8 @@ def sync_hostname_and_physical_networks(self, ctx):
         LOG.debug('OVN-SB Sync hostname and physical networks started')
         host_phynets_map = self.ovn_api.get_chassis_hostname_and_physnets()
         current_hosts = set(host_phynets_map)
-        previous_hosts = segments_db.get_hosts_mapped_with_segments(ctx)
+        previous_hosts = segments_db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={ovn_const.OVN_CONTROLLER_AGENT})
 
         stale_hosts = previous_hosts - current_hosts
         for host in stale_hosts:

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

@@ -666,6 +666,8 @@ def notify(self, event, row, updates=None):
 class BaseOvnSbIdl(Ml2OvnIdlBase):
     @classmethod
     def from_server(cls, connection_string, helper):
+        if 'Chassis_Private' in helper.schema_json['tables']:
+            helper.register_table('Chassis_Private')
         helper.register_table('Chassis')
         helper.register_table('Encap')
         helper.register_table('Port_Binding')

neutron/services/segments/db.py

@@ -30,6 +30,8 @@
 from oslo_log import helpers as log_helpers
 from oslo_utils import uuidutils
 
+from neutron.db.models import agent as agent_model
+from neutron.db.models import segment as segment_model
 from neutron.db import segments_db as db
 from neutron import manager
 from neutron.objects import base as base_obj
@@ -237,14 +239,43 @@ def update_segment_host_mapping(context, host, current_segment_ids):
                     entry.delete()
 
 
-def get_hosts_mapped_with_segments(context):
+def get_hosts_mapped_with_segments(context, include_agent_types=None,
+                                   exclude_agent_types=None):
     """Get hosts that are mapped with segments.
 
     L2 providers can use this method to get an overview of SegmentHostMapping,
     and then delete the stale SegmentHostMapping.
+
+    When using both include_agent_types and exclude_agent_types,
+    exclude_agent_types is most significant.
+    All hosts without agent are excluded when using any agent_type filter.
+
+    :param context: current running context information
+    :param include_agent_types: (set) List of agent types, include hosts
+        with matching agents.
+    :param exclude_agent_types: (set) List of agent types, exclude hosts
+        with matching agents.
     """
-    segment_host_mapping = network.SegmentHostMapping.get_objects(context)
-    return {row.host for row in segment_host_mapping}
+    def add_filter_by_agent_types(qry, include, exclude):
+        qry = qry.join(
+            agent_model.Agent,
+            segment_model.SegmentHostMapping.host == agent_model.Agent.host)
+        if include:
+            qry = qry.filter(agent_model.Agent.agent_type.in_(include))
+        if exclude:
+            qry = qry.filter(agent_model.Agent.agent_type.not_in(exclude))
+
+        return qry
+
+    with db_api.CONTEXT_READER.using(context):
+        query = context.session.query(segment_model.SegmentHostMapping)
+        if include_agent_types or exclude_agent_types:
+            query = add_filter_by_agent_types(query, include_agent_types,
+                                              exclude_agent_types)
+
+        res = query.all()
+
+    return {row.host for row in res}
 
 
 def _get_phys_nets(agent):

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

@@ -1794,6 +1794,22 @@ def setUp(self):
     def _sync_resources(self):
         self.sb_synchronizer.sync_hostname_and_physical_networks(self.ctx)
 
+    def create_agent(self, host, bridge_mappings=None, agent_type=None):
+        if agent_type is None:
+            agent_type = ovn_const.OVN_CONTROLLER_AGENT
+        if bridge_mappings is None:
+            bridge_mappings = {}
+        agent = {
+            'host': host,
+            'agent_type': agent_type,
+            'binary': '/bin/test',
+            'topic': 'test_topic',
+            'configurations': {'bridge_mappings': bridge_mappings}
+        }
+        _, status = self.plugin.create_or_update_agent(self.context, agent)
+
+        return status['id']
+
     def create_segment(self, network_id, physical_network, segmentation_id):
         segment_data = {'network_id': network_id,
                         'physical_network': physical_network,
@@ -1834,6 +1850,7 @@ def test_ovn_sb_sync_delete_stale_host(self):
         segment = self.create_segment(network_id, 'physnet1', 50)
         segments_db.update_segment_host_mapping(
             self.ctx, 'host1', {segment['id']})
+        _ = self.create_agent('host1', bridge_mappings={'physnet1': 'eth0'})
         segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
         self.assertEqual({'host1'}, segment_hosts)
         # Since there is no chassis in the sb DB, host1 is the stale host
@@ -1842,6 +1859,36 @@ def test_ovn_sb_sync_delete_stale_host(self):
         segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
         self.assertFalse(segment_hosts)
 
+    def test_ovn_sb_sync_host_with_no_agent_not_deleted(self):
+        with self.network() as network:
+            network_id = network['network']['id']
+        segment = self.create_segment(network_id, 'physnet1', 50)
+        segments_db.update_segment_host_mapping(
+            self.ctx, 'host1', {segment['id']})
+        _ = self.create_agent('host1', bridge_mappings={'physnet1': 'eth0'},
+                              agent_type="Not OVN Agent")
+        segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
+        self.assertEqual({'host1'}, segment_hosts)
+        # There is no chassis in the sb DB, host1 does not have an agent
+        # so it is not deleted.
+        self._sync_resources()
+        segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
+        self.assertEqual({'host1'}, segment_hosts)
+
+    def test_ovn_sb_sync_host_with_other_agent_type_not_deleted(self):
+        with self.network() as network:
+            network_id = network['network']['id']
+        segment = self.create_segment(network_id, 'physnet1', 50)
+        segments_db.update_segment_host_mapping(
+            self.ctx, 'host1', {segment['id']})
+        segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
+        self.assertEqual({'host1'}, segment_hosts)
+        # There is no chassis in the sb DB, host1 does not have an agent
+        # so it is not deleted.
+        self._sync_resources()
+        segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
+        self.assertEqual({'host1'}, segment_hosts)
+
     def test_ovn_sb_sync(self):
         with self.network() as network:
             network_id = network['network']['id']
@@ -1854,6 +1901,9 @@ def test_ovn_sb_sync(self):
         segments_db.update_segment_host_mapping(
             self.ctx, 'host3', {seg1['id']})
         segment_hosts = segments_db.get_hosts_mapped_with_segments(self.ctx)
+        _ = self.create_agent('host1')
+        _ = self.create_agent('host2', bridge_mappings={'physnet2': 'eth0'})
+        _ = self.create_agent('host3', bridge_mappings={'physnet3': 'eth0'})
         self.assertEqual({'host1', 'host2', 'host3'}, segment_hosts)
         self.add_fake_chassis('host2', ['physnet2'])
         self.add_fake_chassis('host3', ['physnet3'])

neutron/tests/unit/extensions/test_segment.py

@@ -746,10 +746,37 @@ def test_get_all_hosts_mapped_with_segments(self):
         actual_hosts = db.get_hosts_mapped_with_segments(ctx)
         self.assertEqual(hosts, actual_hosts)
 
+    def test_get_all_hosts_mapped_with_segments_agent_type_filter(self):
+        ctx = context.get_admin_context()
+        hosts = set()
+        with self.network() as network:
+            network_id = network['network']['id']
+        for i in range(1, 3):
+            host = "host%s" % i
+            segment = self._test_create_segment(
+                network_id=network_id, physical_network='physnet%s' % i,
+                segmentation_id=200 + i, network_type=constants.TYPE_VLAN)
+            db.update_segment_host_mapping(
+                ctx, host, {segment['segment']['id']})
+            hosts.add(host)
+
+        # Now they are 2 hosts with segment being mapped.
+        #  host1 does not have an agent
+        #  host2 does not have an agent
+        # Any agent_type filter excludes hosts that does not have an agent
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, exclude_agent_types={'fake-agent-type'})
+        self.assertEqual(set(), actual_hosts)
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={'fake-agent-type'})
+        self.assertEqual(set(), actual_hosts)
+
 
 class TestMl2HostSegmentMappingOVS(HostSegmentMappingTestCase):
     _mechanism_drivers = ['openvswitch', 'logger']
     mock_path = 'neutron.services.segments.db.update_segment_host_mapping'
+    agent_type_a = constants.AGENT_TYPE_OVS
+    agent_type_b = constants.AGENT_TYPE_LINUXBRIDGE
 
     def test_new_agent(self):
         host = 'host1'
@@ -869,9 +896,118 @@ def test_agent_with_no_mappings(self, mock):
         self.assertFalse(segments_host_db)
         self.assertFalse(mock.mock_calls)
 
+    def test_get_all_hosts_mapped_with_segments(self):
+        ctx = context.get_admin_context()
+        hosts = set()
+        with self.network() as network:
+            network_id = network['network']['id']
+        for i in range(1, 3):
+            host = "host%s" % i
+            segment = self._test_create_segment(
+                network_id=network_id, physical_network='physnet%s' % i,
+                segmentation_id=200 + i, network_type=constants.TYPE_VLAN)
+            self._register_agent(host, mappings={'physnet%s' % i: 'br-eth-1'},
+                                 plugin=self.plugin)
+            db.update_segment_host_mapping(
+                ctx, host, {segment['segment']['id']})
+            hosts.add(host)
+
+        # Now they are 2 hosts with segment being mapped.
+        actual_hosts = db.get_hosts_mapped_with_segments(ctx)
+        self.assertEqual(hosts, actual_hosts)
+
+    def test_get_all_hosts_mapped_with_segments_agent_type_filters(self):
+        ctx = context.get_admin_context()
+        with self.network() as network:
+            network_id = network['network']['id']
+        for i in range(1, 3):
+            host = "host%s" % i
+            segment = self._test_create_segment(
+                network_id=network_id, physical_network='physnet%s' % i,
+                segmentation_id=200 + i, network_type=constants.TYPE_VLAN)
+            if i == 2:
+                agent_type = self.agent_type_a
+            else:
+                agent_type = self.agent_type_b
+            helpers.register_ovs_agent(
+                host, agent_type=agent_type,
+                bridge_mappings={'physnet%s' % i: 'br-eth-1'},
+                plugin=self.plugin, start_flag=True)
+            db.update_segment_host_mapping(
+                ctx, host, {segment['segment']['id']})
+
+        # Now they are 2 hosts with segment being mapped.
+        #   host1 is agent_type_b
+        #   host2 is agent_type_a
+        # get all hosts (host1 and host2) when not using any filtering
+        actual_hosts = db.get_hosts_mapped_with_segments(ctx)
+        self.assertEqual({"host1", "host2"}, actual_hosts)
+        # get host1 when exclude agent_type_a agents
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, exclude_agent_types={self.agent_type_a})
+        self.assertEqual({"host1"}, actual_hosts)
+        # get host2 when exclude agent_type_b agents
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, exclude_agent_types={self.agent_type_b})
+        self.assertEqual({"host2"}, actual_hosts)
+        # get host2 when include agent_type_a agents
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={self.agent_type_a})
+        self.assertEqual({"host2"}, actual_hosts)
+        # get host1 when include agent_type_b agents
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={self.agent_type_b})
+        self.assertEqual({"host1"}, actual_hosts)
+        # get host1 and host2 when include both agent_type_a and agent_type_b
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={self.agent_type_b, self.agent_type_a})
+        self.assertEqual({"host1", "host2"}, actual_hosts)
+        # When using both include and exclude, exclude is most significant
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx,
+            include_agent_types={self.agent_type_b, self.agent_type_a},
+            exclude_agent_types={self.agent_type_b}
+        )
+        self.assertEqual({"host2"}, actual_hosts)
+        # include and exclude both agent types - exclude is most significant
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx,
+            include_agent_types={self.agent_type_b, self.agent_type_a},
+            exclude_agent_types={self.agent_type_b, self.agent_type_a}
+        )
+        self.assertEqual(set(), actual_hosts)
+
+    def test_get_all_hosts_mapped_with_segments_agent_type_filter(self):
+        ctx = context.get_admin_context()
+        hosts = set()
+        with self.network() as network:
+            network_id = network['network']['id']
+        for i in range(1, 3):
+            host = "host%s" % i
+            segment = self._test_create_segment(
+                network_id=network_id, physical_network='physnet%s' % i,
+                segmentation_id=200 + i, network_type=constants.TYPE_VLAN)
+            self._register_agent(host, mappings={'physnet%s' % i: 'br-eth-1'},
+                                 plugin=self.plugin)
+            db.update_segment_host_mapping(
+                ctx, host, {segment['segment']['id']})
+            hosts.add(host)
+
+        # Now they are 2 hosts with segment being mapped.
+        #  host1 is agent_type_a
+        #  host2 is agent_type_a
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, exclude_agent_types={self.agent_type_a})
+        self.assertEqual(set(), actual_hosts)
+        actual_hosts = db.get_hosts_mapped_with_segments(
+            ctx, include_agent_types={self.agent_type_a})
+        self.assertEqual(hosts, actual_hosts)
+
 
 class TestMl2HostSegmentMappingLinuxBridge(TestMl2HostSegmentMappingOVS):
     _mechanism_drivers = ['linuxbridge', 'logger']
+    agent_type_a = constants.AGENT_TYPE_LINUXBRIDGE
+    agent_type_b = constants.AGENT_TYPE_OVS
 
     def _register_agent(self, host, mappings=None, plugin=None):
         helpers.register_linuxbridge_agent(host=host,
@@ -881,6 +1017,8 @@ def _register_agent(self, host, mappings=None, plugin=None):
 
 class TestMl2HostSegmentMappingMacvtap(TestMl2HostSegmentMappingOVS):
     _mechanism_drivers = ['macvtap', 'logger']
+    agent_type_a = constants.AGENT_TYPE_MACVTAP
+    agent_type_b = constants.AGENT_TYPE_OVS
 
     def _register_agent(self, host, mappings=None, plugin=None):
         helpers.register_macvtap_agent(host=host, interface_mappings=mappings,
@@ -889,6 +1027,8 @@ def _register_agent(self, host, mappings=None, plugin=None):
 
 class TestMl2HostSegmentMappingSriovNicSwitch(TestMl2HostSegmentMappingOVS):
     _mechanism_drivers = ['sriovnicswitch', 'logger']
+    agent_type_a = constants.AGENT_TYPE_NIC_SWITCH
+    agent_type_b = constants.AGENT_TYPE_OVS
 
     def _register_agent(self, host, mappings=None, plugin=None):
         helpers.register_sriovnicswitch_agent(host=host,

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

@@ -1169,8 +1169,10 @@ def test_ovn_sb_sync(self):
 
         with mock.patch.object(ovn_db_sync.segments_db,
                                'get_hosts_mapped_with_segments',
-                               return_value=hosts_in_neutron):
+                               return_value=hosts_in_neutron) as mock_ghmws:
             ovn_sb_synchronizer.sync_hostname_and_physical_networks(mock.ANY)
+            mock_ghmws.assert_called_once_with(
+                mock.ANY, include_agent_types={ovn_const.OVN_CONTROLLER_AGENT})
             all_hosts = set(hostname_with_physnets.keys()) | hosts_in_neutron
             self.assertEqual(
                 len(all_hosts),

releasenotes/notes/ovn-db-sync-host-physnet-filter-agent-type-9e22942bed304807.yaml

@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    When synchronizing the OVN databases, either when running the migration
+    command or during startup, the code responsible for synchronization will
+    only clean up segment-to-host mappings for hosts with agent_type
+    ``OVN Controller agent``. Before, the synchronization would clean up
+    (delete) segment-to-host mappings for non-OVN hosts. Fixes bug:
+    `2040172 <https://bugs.launchpad.net/neutron/+bug/2040172>`_.
+