From 9dd0d32063f72389fd89dcf857ab3a021db73d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20J=C3=B3zefczyk?= Date: Wed, 25 Mar 2020 15:30:12 +0000 Subject: [PATCH] [OVN] Create localnet port for each created segment If new segment is created/old deleted we should update its localnet port in related Logical_Switch. Added also missing code to sync tool in order to delete provnet ports in case of leftovers. Change-Id: I6b864ba1c168643640a64bd7c25e1d0fc0ea348a Related-Bug: 1865889 (cherry picked from commit 483f468fdd5bb549f763d0507e0a7ac1106eb85a) --- neutron/cmd/ovn/neutron_ovn_db_sync_util.py | 3 +- .../drivers/ovn/mech_driver/mech_driver.py | 21 ++++ .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 6 +- .../ovn/mech_driver/ovsdb/maintenance.py | 41 +++++++ .../ovn/mech_driver/ovsdb/ovn_client.py | 51 +++++++-- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 62 +++++++--- neutron/tests/functional/base.py | 2 + .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 28 ++--- .../ovn/mech_driver/test_mech_driver.py | 78 +++++++++++++ neutron/tests/unit/fake_resources.py | 10 +- .../mech_driver/ovsdb/test_impl_idl_ovn.py | 8 +- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 63 +++++++++-- .../ovn/mech_driver/test_mech_driver.py | 106 ++++++++++++++++++ 13 files changed, 420 insertions(+), 59 deletions(-) diff --git a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py index 92bae80ae2a..39d6ac0fd1f 100644 --- a/neutron/cmd/ovn/neutron_ovn_db_sync_util.py +++ b/neutron/cmd/ovn/neutron_ovn_db_sync_util.py @@ -179,7 +179,8 @@ def main(): return cfg.CONF.set_override('mechanism_drivers', ['ovn-sync'], 'ml2') conf.service_plugins = [ - 'neutron.services.ovn_l3.plugin.OVNL3RouterPlugin'] + 'neutron.services.ovn_l3.plugin.OVNL3RouterPlugin', + 'neutron.services.segments.plugin.Plugin'] else: LOG.error('Invalid core plugin : ["%s"].', cfg.CONF.core_plugin) return diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 21bb8df5061..c45f339d84d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -22,6 +22,7 @@ import threading import types from neutron_lib.api.definitions import portbindings +from neutron_lib.api.definitions import segment as segment_def from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources @@ -173,6 +174,12 @@ class OVNMechanismDriver(api.MechanismDriver): registry.subscribe(self._add_segment_host_mapping_for_segment, resources.SEGMENT, events.AFTER_CREATE) + registry.subscribe(self.create_segment_provnet_port, + resources.SEGMENT, + events.AFTER_CREATE) + registry.subscribe(self.delete_segment_provnet_port, + resources.SEGMENT, + events.AFTER_DELETE) # Handle security group/rule notifications if self.sg_enabled: @@ -329,6 +336,20 @@ class OVNMechanismDriver(api.MechanismDriver): msg = _('Network type %s is not supported') % network_type raise n_exc.InvalidInput(error_message=msg) + def create_segment_provnet_port(self, resource, event, trigger, + context, segment, payload=None): + if not segment.get(segment_def.PHYSICAL_NETWORK): + return + self._ovn_client.create_provnet_port(segment['network_id'], segment) + + def delete_segment_provnet_port(self, resource, event, trigger, + payload): + # NOTE(mjozefcz): Get the last state of segment resource. + segment = payload.states[-1] + if segment.get(segment_def.PHYSICAL_NETWORK): + self._ovn_client.delete_provnet_port( + segment['network_id'], segment) + def create_network_precommit(self, context): """Allocate resources for a new network. 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 index a0dd2fe489a..d82f29bb406 100644 --- 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 @@ -211,17 +211,17 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): lswitch.external_ids): continue ports = [] - provnet_port = None + provnet_ports = [] for lport in getattr(lswitch, 'ports', []): if ovn_const.OVN_PORT_NAME_EXT_ID_KEY in lport.external_ids: ports.append(lport.name) # Handle provider network port elif lport.name.startswith( ovn_const.OVN_PROVNET_PORT_NAME_PREFIX): - provnet_port = lport.name + provnet_ports.append(lport.name) result.append({'name': lswitch.name, 'ports': ports, - 'provnet_port': provnet_port}) + 'provnet_ports': provnet_ports}) return result def get_all_logical_routers_with_rports(self): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index bb6d65efb2d..1384e38e5c2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -19,6 +19,7 @@ import threading from futurist import periodics from neutron_lib.api.definitions import external_net +from neutron_lib.api.definitions import segment as segment_def from neutron_lib import constants as n_const from neutron_lib import context as n_context from neutron_lib import exceptions as n_exc @@ -28,9 +29,11 @@ from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import event as row_event from neutron.common.ovn import constants as ovn_const +from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_hash_ring_db as hash_ring_db from neutron.db import ovn_revision_numbers_db as revision_numbers_db +from neutron.db import segments_db from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync @@ -607,6 +610,44 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + # 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) + def check_for_localnet_legacy_port_name(self): + if not self.has_lock: + return + + admin_context = n_context.get_admin_context() + cmds = [] + for ls in self._nb_idl.ls_list().execute(check_error=True): + network_id = ls.name.strip('neutron-') + legacy_name = utils.ovn_provnet_port_name(network_id) + legacy_port = None + segment_id = None + for lsp in ls.ports: + if legacy_name == lsp.name: + legacy_port = lsp + break + else: + continue + for segment in segments_db.get_network_segments( + admin_context, network_id): + if (segment.get(segment_def.PHYSICAL_NETWORK) == + legacy_port.options['network_name']): + segment_id = segment['id'] + break + if not segment_id: + continue + new_p_name = utils.ovn_provnet_port_name(segment_id) + cmds.append(self._nb_idl.db_set('Logical_Switch_Port', + legacy_port.uuid, + ('name', new_p_name))) + if cmds: + with self._nb_idl.transaction(check_error=True) as txn: + for cmd in cmds: + txn.add(cmd) + raise periodics.NeverAgain() + 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 index 49fc708ae3d..aa72195f435 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -21,6 +21,7 @@ from neutron_lib.api.definitions import l3 from neutron_lib.api.definitions import port_security as psec from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net as pnet +from neutron_lib.api.definitions import segment as segment_def from neutron_lib import constants as const from neutron_lib import context as n_context from neutron_lib import exceptions as n_exc @@ -39,6 +40,7 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import 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.drivers.ovn.mech_driver.ovsdb.extensions \ import qos as qos_extension from neutron.scheduler import l3_ovn_scheduler @@ -1652,15 +1654,41 @@ class OVNClient(object): for network in networks] self._transaction(commands, txn=txn) - def _create_provnet_port(self, txn, network, physnet, tag): - txn.add(self._nb_idl.create_lswitch_port( - lport_name=utils.ovn_provnet_port_name(network['id']), - lswitch_name=utils.ovn_name(network['id']), + def create_provnet_port(self, network_id, segment, txn=None): + tag = segment.get(segment_def.SEGMENTATION_ID, []) + physnet = segment.get(segment_def.PHYSICAL_NETWORK) + cmd = self._nb_idl.create_lswitch_port( + lport_name=utils.ovn_provnet_port_name(segment['id']), + lswitch_name=utils.ovn_name(network_id), addresses=[ovn_const.UNKNOWN_ADDR], external_ids={}, type=ovn_const.LSP_TYPE_LOCALNET, - tag=tag if tag else [], - options={'network_name': physnet})) + tag=tag, + options={'network_name': physnet}) + self._transaction([cmd], txn=txn) + + def delete_provnet_port(self, network_id, segment): + port_to_del = utils.ovn_provnet_port_name(segment['id']) + legacy_port_name = utils.ovn_provnet_port_name(network_id) + physnet = segment.get(segment_def.PHYSICAL_NETWORK) + lswitch = self._nb_idl.get_lswitch(utils.ovn_name(network_id)) + lports = [lp.name for lp in lswitch.ports] + + # Cover the situation where localnet ports + # were named after network_id and not segment_id. + # TODO(mjozefcz): Remove this in w-release. + if (port_to_del not in lports and + legacy_port_name in lports): + for lport in lswitch.ports: + if (legacy_port_name == lport.name and + lport.options['network_name'] == physnet): + port_to_del = legacy_port_name + break + + cmd = self._nb_idl.delete_lswitch_port( + lport_name=port_to_del, + lswitch_name=utils.ovn_name(network_id)) + self._transaction([cmd]) def _gen_network_parameters(self, network): params = {'external_ids': { @@ -1681,13 +1709,16 @@ class OVNClient(object): # without having to track what UUID OVN assigned to it. lswitch_params = self._gen_network_parameters(network) lswitch_name = utils.ovn_name(network['id']) + # NOTE(mjozefcz): Remove this workaround when bug + # 1869877 will be fixed. + segments = segments_db.get_network_segments( + context, network['id']) with self._nb_idl.transaction(check_error=True) as txn: txn.add(self._nb_idl.ls_add(lswitch_name, **lswitch_params, may_exist=True)) - physnet = network.get(pnet.PHYSICAL_NETWORK) - if physnet: - self._create_provnet_port(txn, network, physnet, - network.get(pnet.SEGMENTATION_ID)) + for segment in segments: + if segment.get(segment_def.PHYSICAL_NETWORK): + self.create_provnet_port(network['id'], segment, txn=txn) db_rev.bump_revision(context, network, ovn_const.TYPE_NETWORKS) self.create_metadata_port(context, network) return network 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 17c372f3f0b..3143a237e2b 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 @@ -16,7 +16,7 @@ import itertools from eventlet import greenthread from neutron_lib.api.definitions import l3 -from neutron_lib.api.definitions import provider_net as pnet +from neutron_lib.api.definitions import segment as segment_def from neutron_lib import constants from neutron_lib import context from neutron_lib import exceptions as n_exc @@ -73,6 +73,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer): self.mode = mode self.l3_plugin = directory.get_plugin(plugin_constants.L3) self._ovn_client = ovn_client.OVNClient(ovn_api, sb_ovn) + self.segments_plugin = directory.get_plugin('segments') def stop(self): if utils.is_ovn_l3(self.l3_plugin): @@ -970,6 +971,7 @@ class OvnNbSynchronizer(OvnDbSynchronizer): del_lswitchs_list = [] del_lports_list = [] add_provnet_ports_list = [] + del_provnet_ports_list = [] for lswitch in lswitches: if lswitch['name'] in db_networks: for lport in lswitch['ports']: @@ -981,13 +983,29 @@ class OvnNbSynchronizer(OvnDbSynchronizer): del_lports_list.append({'port': lport, 'lswitch': lswitch['name']}) db_network = db_networks[lswitch['name']] - physnet = db_network.get(pnet.PHYSICAL_NETWORK) - # Updating provider attributes is forbidden by neutron, thus - # we only need to consider missing provnet-ports in OVN DB. - if physnet and not lswitch['provnet_port']: - add_provnet_ports_list.append( - {'network': db_network, - 'lswitch': lswitch['name']}) + db_segments = self.segments_plugin.get_segments( + ctx, filters={'network_id': [db_network['id']]}) + segments_provnet_port_names = [] + for db_segment in db_segments: + physnet = db_segment.get(segment_def.PHYSICAL_NETWORK) + pname = utils.ovn_provnet_port_name(db_segment['id']) + segments_provnet_port_names.append(pname) + if physnet and pname not in lswitch['provnet_ports']: + add_provnet_ports_list.append( + {'network': db_network, + 'segment': db_segment, + 'lswitch': lswitch['name']}) + # Delete orhpaned provnet ports + for provnet_port in lswitch['provnet_ports']: + if provnet_port in segments_provnet_port_names: + continue + if provnet_port not in [ + utils.ovn_provnet_port_name(v['segment']) + for v in add_provnet_ports_list]: + del_provnet_ports_list.append( + {'network': db_network, + 'lport': provnet_port, + 'lswitch': lswitch['name']}) del db_networks[lswitch['name']] else: @@ -1043,15 +1061,33 @@ class OvnNbSynchronizer(OvnDbSynchronizer): for provnet_port_info in add_provnet_ports_list: network = provnet_port_info['network'] + segment = provnet_port_info['segment'] LOG.warning("Provider network found in Neutron but " "provider network port not found in OVN DB, " - "network_id=%s", provnet_port_info['lswitch']) + "network_id=%(net)s segment_id=%(seg)s", + {'net': network['id'], + 'seg': segment['id']}) if self.mode == SYNC_MODE_REPAIR: LOG.debug('Creating the provnet port %s in OVN NB DB', - utils.ovn_provnet_port_name(network['id'])) - self._ovn_client._create_provnet_port( - txn, network, network.get(pnet.PHYSICAL_NETWORK), - network.get(pnet.SEGMENTATION_ID)) + utils.ovn_provnet_port_name(segment['id'])) + self._ovn_client.create_provnet_port( + network['id'], segment, txn=txn) + + for provnet_port_info in del_provnet_ports_list: + network = provnet_port_info['network'] + lport = provnet_port_info['lport'] + lswitch = provnet_port_info['lswitch'] + LOG.warning("Provider network port found in OVN DB, " + "but not in neutron network_id=%(net)s " + "port_name=%(lport)s", + {'net': network, + 'seg': lport}) + if self.mode == SYNC_MODE_REPAIR: + LOG.debug('Deleting the port %s from OVN NB DB', + lport) + txn.add(self.ovn_api.delete_lswitch_port( + lport_name=lport, + lswitch_name=lswitch)) for lport_info in del_lports_list: LOG.warning("Port found in OVN but not in " diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index f1731a1a3b2..ad4b4ed6600 100644 --- a/neutron/tests/functional/base.py +++ b/neutron/tests/functional/base.py @@ -182,6 +182,7 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase, mm = directory.get_plugin().mechanism_manager self.mech_driver = mm.mech_drivers['ovn'].obj self.l3_plugin = directory.get_plugin(constants.L3) + self.segments_plugin = directory.get_plugin('segments') self.ovsdb_server_mgr = None self.ovn_northd_mgr = None self.maintenance_worker = maintenance_worker @@ -219,6 +220,7 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase, def get_additional_service_plugins(self): p = super(TestOVNFunctionalBase, self).get_additional_service_plugins() p.update({'revision_plugin_name': 'revisions'}) + p.update({'segments': 'neutron.services.segments.plugin.Plugin'}) return p @property 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 d7cd29b6a1f..f94f8f91797 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 @@ -29,7 +29,6 @@ from neutron_lib.api.definitions import l3 from neutron_lib.api.definitions import port_security as ps from neutron_lib import constants from neutron_lib import context -from neutron_lib.plugins import directory from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import idlutils @@ -379,9 +378,13 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): uuidutils.generate_uuid(), 'neutron-' + n1['network']['id'])) self.delete_lswitches.append('neutron-' + n2['network']['id']) - self.delete_lswitch_ports.append( - (utils.ovn_provnet_port_name(e1['network']['id']), - utils.ovn_name(e1['network']['id']))) + for seg in self.segments_plugin.get_segments( + self.context, + filters={'network_id': [e1['network']['id']]}): + if seg.get('physical_network'): + self.delete_lswitch_ports.append( + (utils.ovn_provnet_port_name(seg['id']), + utils.ovn_name(e1['network']['id']))) r1 = self.l3_plugin.create_router( self.context, @@ -848,9 +851,14 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): def _validate_networks(self, should_match=True): db_networks = self._list('networks') db_net_ids = [net['id'] for net in db_networks['networks']] - db_provnet_ports = [utils.ovn_provnet_port_name(net['id']) - for net in db_networks['networks'] - if net.get('provider:physical_network')] + db_provnet_ports = [] + for net in db_networks['networks']: + for seg in self.segments_plugin.get_segments( + self.context, + filters={'network_id': [net['id']]}): + if seg.get('physical_network'): + db_provnet_ports.append( + utils.ovn_provnet_port_name(seg['id'])) # Get the list of lswitch ids stored in the OVN plugin IDL _plugin_nb_ovn = self.mech_driver._nb_ovn @@ -1523,17 +1531,11 @@ class TestOvnSbSync(base.TestOVNFunctionalBase): def setUp(self): super(TestOvnSbSync, self).setUp(maintenance_worker=True) - self.segments_plugin = directory.get_plugin('segments') self.sb_synchronizer = ovn_db_sync.OvnSbSynchronizer( self.plugin, self.mech_driver._sb_ovn, self.mech_driver) self.addCleanup(self.sb_synchronizer.stop) self.ctx = context.get_admin_context() - def get_additional_service_plugins(self): - p = super(TestOvnSbSync, self).get_additional_service_plugins() - p.update({'segments': 'neutron.services.segments.plugin.Plugin'}) - return p - def _sync_resources(self): self.sb_synchronizer.sync_hostname_and_physical_networks(self.ctx) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index c70a10dd922..917efab1e9e 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -16,6 +16,7 @@ import functools import mock from neutron_lib.api.definitions import portbindings +from neutron_lib import constants from oslo_config import cfg from oslo_utils import uuidutils @@ -594,3 +595,80 @@ class TestExternalPorts(base.TestOVNFunctionalBase): def test_external_port_update_switchdev_vnic_macvtap(self): self._test_external_port_update_switchdev(portbindings.VNIC_MACVTAP) + + +class TestProvnetPorts(base.TestOVNFunctionalBase): + + def setUp(self): + super(TestProvnetPorts, self).setUp() + self._ovn_client = self.mech_driver._ovn_client + + def _find_port_row_by_name(self, name): + cmd = self.nb_api.db_find_rows( + 'Logical_Switch_Port', ('name', '=', name)) + rows = cmd.execute(check_error=True) + return rows[0] if rows else None + + def create_segment(self, network_id, physical_network, segmentation_id): + segment_data = {'network_id': network_id, + 'physical_network': physical_network, + 'segmentation_id': segmentation_id, + 'network_type': 'vlan', + 'name': constants.ATTR_NOT_SPECIFIED, + 'description': constants.ATTR_NOT_SPECIFIED} + return self.segments_plugin.create_segment( + self.context, segment={'segment': segment_data}) + + def delete_segment(self, segment_id): + return self.segments_plugin.delete_segment( + self.context, segment_id) + + def get_segments(self, network_id): + return self.segments_plugin.get_segments( + self.context, filters={'network_id': [network_id]}) + + def test_network_segments_localnet_ports(self): + n1 = self._make_network( + self.fmt, 'n1', True, + arg_list=('provider:network_type', + 'provider:segmentation_id', + 'provider:physical_network'), + **{'provider:network_type': 'vlan', + 'provider:segmentation_id': 100, + 'provider:physical_network': 'physnet1'})['network'] + ovn_port = self._find_port_row_by_name( + utils.ovn_provnet_port_name(n1['id'])) + # Assert that localnet port name is not based + # on network name. + self.assertIsNone(ovn_port) + seg_db = self.get_segments(n1['id']) + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_db[0]['id'])) + self.assertEqual(ovn_localnetport.tag, [100]) + self.assertEqual(ovn_localnetport.options, + {'network_name': 'physnet1'}) + seg_2 = self.create_segment(n1['id'], 'physnet2', '222') + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_2['id'])) + self.assertEqual(ovn_localnetport.options, + {'network_name': 'physnet2'}) + self.assertEqual(ovn_localnetport.tag, [222]) + + # Delete segments and ensure that localnet + # ports are deleted. + self.delete_segment(seg_db[0]['id']) + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_db[0]['id'])) + self.assertIsNone(ovn_localnetport) + + # Make sure that other localnet port is not touched. + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_2['id'])) + self.assertIsNotNone(ovn_localnetport) + + # Delete second segment and validate that the + # second localnet port has been deleted. + self.delete_segment(seg_2['id']) + ovn_localnetport = self._find_port_row_by_name( + utils.ovn_provnet_port_name(seg_2['id'])) + self.assertIsNone(ovn_localnetport) diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 2a53d1a2b43..6404afaf711 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -99,12 +99,14 @@ class FakeOvsdbNbOvnIdl(object): self.get_parent_port.return_value = [] self.dns_add = mock.Mock() self.get_lswitch = mock.Mock() - fake_ovs_row = FakeOvsdbRow.create_one_ovsdb_row() - self.get_lswitch.return_value = fake_ovs_row + self.fake_ls_row = FakeOvsdbRow.create_one_ovsdb_row( + attrs={'ports': []}) + fake_lsp_row = FakeOvsdbRow.create_one_ovsdb_row() + self.get_lswitch.return_value = self.fake_ls_row self.get_lswitch_port = mock.Mock() - self.get_lswitch_port.return_value = fake_ovs_row + self.get_lswitch_port.return_value = fake_lsp_row self.get_ls_and_dns_record = mock.Mock() - self.get_ls_and_dns_record.return_value = (fake_ovs_row, None) + self.get_ls_and_dns_record.return_value = (self.fake_ls_row, None) self.ls_set_dns_records = mock.Mock() self.get_floatingip = mock.Mock() self.get_floatingip.return_value = None diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index d3fbdd9b2f0..7fdbb316b9a 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -421,18 +421,18 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): mapping = self.nb_ovn_idl.get_all_logical_switches_with_ports() expected = [{'name': utils.ovn_name('ls-id-1'), 'ports': ['lsp-id-11', 'lsp-id-12', 'lsp-rp-id-1'], - 'provnet_port': 'provnet-ls-id-1'}, + 'provnet_ports': ['provnet-ls-id-1']}, {'name': utils.ovn_name('ls-id-2'), 'ports': ['lsp-id-21', 'lsp-rp-id-2'], - 'provnet_port': 'provnet-ls-id-2'}, + 'provnet_ports': ['provnet-ls-id-2']}, {'name': utils.ovn_name('ls-id-3'), 'ports': ['lsp-id-31', 'lsp-id-32', 'lsp-rp-id-3', 'lsp-vpn-id-3'], - 'provnet_port': None}, + 'provnet_ports': []}, {'name': utils.ovn_name('ls-id-5'), 'ports': ['lsp-id-51', 'lsp-id-52', 'lsp-rp-id-5', 'lsp-vpn-id-5'], - 'provnet_port': None}] + 'provnet_ports': []}] self.assertItemsEqual(mapping, expected) def test_get_all_logical_routers_with_rports(self): 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 a02f53b217b..79b0cd03f6e 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 @@ -60,6 +60,23 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): 'mtu': 1450, 'provider:physical_network': 'physnet2'}] + self.segments = [{'id': 'seg1', + 'network_id': 'n1', + 'physical_network': 'physnet1', + 'network_type': 'vlan', + 'segmentation_id': 1000}, + {'id': 'seg2', + 'network_id': 'n2', + 'physical_network': None, + 'network_type': 'geneve'}, + {'id': 'seg4', + 'network_id': 'n4', + 'physical_network': 'physnet2', + 'network_type': 'flat'}] + self.segments_map = { + k['network_id']: k + for k in self.segments} + self.subnets = [{'id': 'n1-s1', 'network_id': 'n1', 'enable_dhcp': True, @@ -324,16 +341,23 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): self.lswitches_with_ports = [{'name': 'neutron-n1', 'ports': ['p1n1', 'p3n1'], - 'provnet_port': None}, + 'provnet_ports': []}, {'name': 'neutron-n3', 'ports': ['p1n3', 'p2n3'], - 'provnet_port': None}, + 'provnet_ports': []}, {'name': 'neutron-n4', 'ports': [], - 'provnet_port': 'provnet-n4'}] + 'provnet_ports': [ + 'provnet-seg4', + 'provnet-orphaned-segment']}] self.lrport_networks = ['fdad:123:456::1/64', 'fdad:cafe:a1b2::1/64'] + def get_additional_service_plugins(self): + p = super(TestOvnNbSyncML2, self).get_additional_service_plugins() + p.update({'segments': 'neutron.services.segments.plugin.Plugin'}) + return p + def _fake_get_ovn_dhcp_options(self, subnet, network, server_mac=None): if subnet['id'] == 'n1-s1': return {'cidr': '10.0.0.0/24', @@ -368,11 +392,24 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): ovn_api = ovn_nb_synchronizer.ovn_api ovn_driver = ovn_nb_synchronizer.ovn_driver l3_plugin = ovn_nb_synchronizer.l3_plugin + segments_plugin = ovn_nb_synchronizer.segments_plugin core_plugin.get_networks = mock.Mock() core_plugin.get_networks.return_value = self.networks core_plugin.get_subnets = mock.Mock() core_plugin.get_subnets.return_value = self.subnets + + def get_segments(self, filters): + segs = [] + for segment in self.segments: + if segment['network_id'] == filters['network_id'][0]: + segs.append(segment) + return segs + + segments_plugin.get_segments = mock.Mock() + segments_plugin.get_segments.side_effect = ( + lambda x, filters: get_segments(self, filters)) + # following block is used for acl syncing unit-test # With the given set of values in the unit testing, @@ -445,7 +482,7 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): ovn_driver.validate_and_get_data_from_binding_profile = mock.Mock() ovn_nb_synchronizer._ovn_client.create_port = mock.Mock() ovn_nb_synchronizer._ovn_client.create_port.return_value = mock.ANY - ovn_nb_synchronizer._ovn_client._create_provnet_port = mock.Mock() + ovn_nb_synchronizer._ovn_client.create_provnet_port = mock.Mock() ovn_api.ls_del = mock.Mock() ovn_api.delete_lswitch_port = mock.Mock() @@ -609,14 +646,16 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): create_port_calls, any_order=True) create_provnet_port_calls = [ - mock.call(mock.ANY, mock.ANY, - network['provider:physical_network'], - network['provider:segmentation_id']) - for network in create_provnet_port_list] + mock.call( + network['id'], + self.segments_map[network['id']], + txn=mock.ANY) + for network in create_provnet_port_list + if network.get('provider:physical_network')] self.assertEqual( len(create_provnet_port_list), - ovn_nb_synchronizer._ovn_client._create_provnet_port.call_count) - ovn_nb_synchronizer._ovn_client._create_provnet_port.assert_has_calls( + ovn_nb_synchronizer._ovn_client.create_provnet_port.call_count) + ovn_nb_synchronizer._ovn_client.create_provnet_port.assert_has_calls( create_provnet_port_calls, any_order=True) self.assertEqual(len(del_network_list), @@ -746,7 +785,9 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): 'ext_ids': {}}] del_network_list = ['neutron-n3'] del_port_list = [{'id': 'p3n1', 'lswitch': 'neutron-n1'}, - {'id': 'p1n1', 'lswitch': 'neutron-n1'}] + {'id': 'p1n1', 'lswitch': 'neutron-n1'}, + {'id': 'provnet-orphaned-segment', + 'lswitch': 'neutron-n4'}] create_port_list = self.ports for port in create_port_list: if port['id'] in ['p1n1', 'fp1']: diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 65f9dd8dc48..63e38f37ef2 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -46,6 +46,7 @@ from neutron.db import db_base_plugin_v2 from neutron.db import ovn_revision_numbers_db from neutron.db import provisioning_blocks from neutron.db import securitygroups_db +from neutron.db import segments_db from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron.plugins.ml2.drivers import type_geneve # noqa @@ -589,6 +590,34 @@ class TestOVNMechanismDriver(test_plugin.Ml2PluginV2TestCase): def test_create_network_igmp_snoop_disabled(self): self._create_network_igmp_snoop(enabled=False) + def test_create_network_create_localnet_port_tunnel_network_type(self): + nb_idl = self.mech_driver._ovn_client._nb_idl + self._make_network(self.fmt, name='net1', + admin_state_up=True)['network'] + # net1 is not physical network + nb_idl.create_lswitch_port.assert_not_called() + + def test_create_network_create_localnet_port_physical_network_type(self): + nb_idl = self.mech_driver._ovn_client._nb_idl + net_arg = {pnet.NETWORK_TYPE: 'vlan', + pnet.PHYSICAL_NETWORK: 'physnet1', + pnet.SEGMENTATION_ID: '2'} + net = self._make_network(self.fmt, 'net1', True, + arg_list=(pnet.NETWORK_TYPE, + pnet.PHYSICAL_NETWORK, + pnet.SEGMENTATION_ID,), + **net_arg)['network'] + segments = segments_db.get_network_segments( + self.context, net['id']) + nb_idl.create_lswitch_port.assert_called_once_with( + addresses=[ovn_const.UNKNOWN_ADDR], + external_ids={}, + lport_name=ovn_utils.ovn_provnet_port_name(segments[0]['id']), + lswitch_name=ovn_utils.ovn_name(net['id']), + options={'network_name': 'physnet1'}, + tag=2, + type='localnet') + def test_create_port_without_security_groups(self): kwargs = {'security_groups': []} with self.network(set_context=True, tenant_id='test') as net1: @@ -1849,6 +1878,7 @@ class TestOVNMechanismDriverSegment(test_segment.HostSegmentMappingTestCase): p = mock.patch.object(ovn_utils, 'get_revision_number', return_value=1) p.start() self.addCleanup(p.stop) + self.context = context.get_admin_context() def _test_segment_host_mapping(self): # Disable the callback to update SegmentHostMapping by default, so @@ -1916,6 +1946,82 @@ class TestOVNMechanismDriverSegment(test_segment.HostSegmentMappingTestCase): segments_host_db2 = self._get_segments_for_host('hostname2') self.assertFalse(set(segments_host_db2)) + def test_create_segment_create_localnet_port(self): + ovn_nb_api = self.mech_driver._nb_ovn + with self.network() as network: + net = network['network'] + new_segment = self._test_create_segment( + network_id=net['id'], physical_network='phys_net1', + segmentation_id=200, network_type='vlan')['segment'] + ovn_nb_api.create_lswitch_port.assert_called_once_with( + addresses=[ovn_const.UNKNOWN_ADDR], + external_ids={}, + lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']), + lswitch_name=ovn_utils.ovn_name(net['id']), + options={'network_name': 'phys_net1'}, + tag=200, + type='localnet') + ovn_nb_api.create_lswitch_port.reset_mock() + new_segment = self._test_create_segment( + network_id=net['id'], physical_network='phys_net2', + segmentation_id=300, network_type='vlan')['segment'] + ovn_nb_api.create_lswitch_port.assert_called_once_with( + addresses=[ovn_const.UNKNOWN_ADDR], + external_ids={}, + lport_name=ovn_utils.ovn_provnet_port_name(new_segment['id']), + lswitch_name=ovn_utils.ovn_name(net['id']), + options={'network_name': 'phys_net2'}, + tag=300, + type='localnet') + segments = segments_db.get_network_segments( + self.context, net['id']) + self.assertEqual(len(segments), 3) + + def test_delete_segment_delete_localnet_port(self): + ovn_nb_api = self.mech_driver._nb_ovn + with self.network() as network: + net = network['network'] + segment = self._test_create_segment( + network_id=net['id'], physical_network='phys_net1', + segmentation_id=200, network_type='vlan')['segment'] + self._delete('segments', segment['id']) + ovn_nb_api.delete_lswitch_port.assert_called_once_with( + lport_name=ovn_utils.ovn_provnet_port_name(segment['id']), + lswitch_name=ovn_utils.ovn_name(net['id'])) + + def test_delete_segment_delete_localnet_port_compat_name(self): + ovn_nb_api = self.mech_driver._nb_ovn + with self.network() as network: + net = network['network'] + seg_1 = self._test_create_segment( + network_id=net['id'], physical_network='phys_net1', + segmentation_id=200, network_type='vlan')['segment'] + seg_2 = self._test_create_segment( + network_id=net['id'], physical_network='phys_net2', + segmentation_id=300, network_type='vlan')['segment'] + # Lets pretend that segment_1 is old and its localnet + # port is based on neutron network id. + ovn_nb_api.fake_ls_row.ports = [ + fakes.FakeOVNPort.create_one_port( + attrs={ + 'options': {'network_name': 'phys_net1'}, + 'tag': 200, + 'name': ovn_utils.ovn_provnet_port_name(net['id'])}), + fakes.FakeOVNPort.create_one_port( + attrs={ + 'options': {'network_name': 'phys_net2'}, + 'tag': 300, + 'name': ovn_utils.ovn_provnet_port_name(seg_2['id'])})] + self._delete('segments', seg_1['id']) + ovn_nb_api.delete_lswitch_port.assert_called_once_with( + lport_name=ovn_utils.ovn_provnet_port_name(net['id']), + lswitch_name=ovn_utils.ovn_name(net['id'])) + ovn_nb_api.delete_lswitch_port.reset_mock() + self._delete('segments', seg_2['id']) + ovn_nb_api.delete_lswitch_port.assert_called_once_with( + lport_name=ovn_utils.ovn_provnet_port_name(seg_2['id']), + lswitch_name=ovn_utils.ovn_name(net['id'])) + @mock.patch.object(n_net, 'get_random_mac', lambda *_: '01:02:03:04:05:06') class TestOVNMechanismDriverDHCPOptions(OVNMechanismDriverTestCase):