From 6e3cbee84578eee3714659498ba334693b617f41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Tue, 24 Oct 2023 09:38:33 +0200 Subject: [PATCH 1/3] get_hosts_mapped_with_segments add filter agt_type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the get_hosts_mapped_with_segments method to add optional filters to include/exclude based on agent type. Uses a joined query, when both include and exclude filtering is used togheter the exclude filter is most significant. Partial-Bug: #2040172 Change-Id: I2cfd52a2657fad989e24e974fda470ecd960262b Signed-off-by: Harald Jensås (cherry picked from commit 64b5787c3283b29cd36dde46ae298ae332f972d5) --- neutron/services/segments/db.py | 37 ++++- neutron/tests/unit/extensions/test_segment.py | 140 ++++++++++++++++++ 2 files changed, 174 insertions(+), 3 deletions(-) diff --git a/neutron/services/segments/db.py b/neutron/services/segments/db.py index 77d1b02b179..094e8807f41 100644 --- a/neutron/services/segments/db.py +++ b/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): diff --git a/neutron/tests/unit/extensions/test_segment.py b/neutron/tests/unit/extensions/test_segment.py index 906532ab016..e2811d9bd58 100644 --- a/neutron/tests/unit/extensions/test_segment.py +++ b/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, From e86d8fc63a14b9e60f1e46472b5578b06eb51ec8 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 11 Jan 2024 18:44:23 -0600 Subject: [PATCH 2/3] Register Chassis_Private table in BaseOvnSbIdl As of Ib3c6f0dc01efd31430691e720ba23ccb4ede65fa, the MaintenanceWorker checks for Chassis_Private table support and uses it to remove duplicate Chassis/Chassis_Private entries. The Chassis_Private table was not monitored in the BaseOvnSbIdl class which the MaintenanceWorker uses. Closes-Bug: #2049265 Change-Id: I711996b7644e80bc195833e4429e4d745728f9cf (cherry picked from commit 60eb15ed301751c3f6825f165fd36188e86dd642) --- .../plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py | 2 ++ 1 file changed, 2 insertions(+) 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 index fda944e61d6..988cc666ba5 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/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') From 37abd72fed99525c302182d276d1c175f5d01a70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Harald=20Jens=C3=A5s?= Date: Mon, 23 Oct 2023 17:29:00 +0200 Subject: [PATCH 3/3] [OVN] DB sync host/physnet - filter on agent_type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When syncing hostname and physical networks, filter neutron hosts on agent_type. Only segmenthostmappings for hosts with agent 'OVN Controller agent' should be cleaned up. Since change: I935186b6ee95f0cae8dc05869d9742c8fb3353c3 there is de-duplication of segmenthostmapping updates from agents. If the OVN DB sync clears/deletes mappings for hosts owned by other agents/plugins the mappings are never re-created. Closes-Bug: #2040172 Change-Id: Iaf15e560e1b1ec31618b2ebc6206a938463c1094 Signed-off-by: Harald Jensås (cherry picked from commit 71d69cf6277ba553354512209a2bff61c013f8ea) (cherry picked from commit 4eb234a4476761ca2581dba45655caceb21fc1ee) --- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 3 +- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 50 +++++++++++++++++++ .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 4 +- ...et-filter-agent-type-9e22942bed304807.yaml | 10 ++++ 4 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/ovn-db-sync-host-physnet-filter-agent-type-9e22942bed304807.yaml diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index e3231d9d109..594267ca678 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/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: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 382b1a38eb3..d4430578d0c 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/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']) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 6eb48bcb08e..0c81c6a0576 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/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), diff --git a/releasenotes/notes/ovn-db-sync-host-physnet-filter-agent-type-9e22942bed304807.yaml b/releasenotes/notes/ovn-db-sync-host-physnet-filter-agent-type-9e22942bed304807.yaml new file mode 100644 index 00000000000..79a2593174a --- /dev/null +++ b/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 `_. +