From d83e243c6d54f29ded7a0bea6e5538772fff3a8e Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 9 Feb 2023 12:57:25 +0100 Subject: [PATCH] [OVN] Add missing LSP device_owner info in trunk driver The ``OVNTrunkHandler`` class updates the port binding profile of the trunk subports. The methods ``_set_binding_profile`` and ``_unset_binding_profile`` update both the OVN LSP register and the Neutron DB port register. This patch adds the missing field "neutron:device_owner" from the LSP external_ids dictionary. This patch also updates ``OvsdbNbOvnIdl.set_lswitch_port`` API method. The method now accepts "external_ids_update" kwarg. This dictionary allows to update (or add) individually each LSP.external_ids dictionary key, instead of overwritting the whole variable. NOTE: ``set_lswitch_port`` is not used outside Neutron now so this is safe to change the API method signature. Closes-Bug: #2006735 Change-Id: I985f3294b2ca7ab5989022ec1b904c8e29354e07 --- .../ml2/drivers/ovn/mech_driver/ovsdb/api.py | 13 ++++++++++--- .../drivers/ovn/mech_driver/ovsdb/commands.py | 9 ++++++++- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 5 +++-- .../services/trunk/drivers/ovn/trunk_driver.py | 17 ++++++++++++----- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 16 ++++++++-------- .../trunk/drivers/ovn/test_trunk_driver.py | 17 +++++++++++++---- .../ovn/mech_driver/ovsdb/test_commands.py | 13 ++++++++----- .../trunk/drivers/ovn/test_trunk_driver.py | 10 +++++++--- 8 files changed, 69 insertions(+), 31 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py index c172b9e4d22..0e2f8cbbdf4 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py @@ -40,16 +40,23 @@ class API(api.API, metaclass=abc.ABCMeta): """ @abc.abstractmethod - def set_lswitch_port(self, lport_name, if_exists=True, **columns): + def set_lswitch_port(self, lport_name, external_ids_update=None, + if_exists=True, **columns): """Create a command to set OVN logical switch port fields :param lport_name: The name of the lport :type lport_name: string + :param external_ids_update: Dictionary of keys to be updated + individually in the external IDs dictionary. + If "external_ids" is defined in "columns", + "external_ids" will override any + "external_ids_update" value. + :type external_ids_update: dictionary + :param if_exists: Do not fail if lport does not exist + :type if_exists: bool :param columns: Dictionary of port columns Supported columns: macs, external_ids, parent_name, tag, enabled - :param if_exists: Do not fail if lport does not exist - :type if_exists: bool :type columns: dictionary :returns: :class:`Command` with no result """ diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 213f6f87395..10cdc3f031c 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -148,9 +148,10 @@ class AddLSwitchPortCommand(command.BaseCommand): class SetLSwitchPortCommand(command.BaseCommand): - def __init__(self, api, lport, if_exists, **columns): + def __init__(self, api, lport, external_ids_update, if_exists, **columns): super(SetLSwitchPortCommand, self).__init__(api) self.lport = lport + self.external_ids_update = external_ids_update self.columns = columns self.if_exists = if_exists @@ -195,6 +196,12 @@ class SetLSwitchPortCommand(command.BaseCommand): for uuid in cur_port_dhcp_opts - new_port_dhcp_opts: self.api._tables['DHCP_Options'].rows[uuid].delete() + external_ids_update = self.external_ids_update or {} + external_ids = getattr(port, 'external_ids', {}) + for k, v in external_ids_update.items(): + external_ids[k] = v + port.external_ids = external_ids + for col, val in self.columns.items(): setattr(port, col, val) 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 4f69484ad55..fbf0134afe3 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 @@ -277,8 +277,9 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): return cmd.AddLSwitchPortCommand(self, lport_name, lswitch_name, may_exist, **columns) - def set_lswitch_port(self, lport_name, if_exists=True, **columns): - return cmd.SetLSwitchPortCommand(self, lport_name, + def set_lswitch_port(self, lport_name, external_ids_update=None, + if_exists=True, **columns): + return cmd.SetLSwitchPortCommand(self, lport_name, external_ids_update, if_exists, **columns) def update_lswitch_qos_options(self, port, if_exists=True, **qos): diff --git a/neutron/services/trunk/drivers/ovn/trunk_driver.py b/neutron/services/trunk/drivers/ovn/trunk_driver.py index 520c35e3164..0056edf2b7a 100644 --- a/neutron/services/trunk/drivers/ovn/trunk_driver.py +++ b/neutron/services/trunk/drivers/ovn/trunk_driver.py @@ -21,7 +21,7 @@ from neutron_lib.services.trunk import constants as trunk_consts from oslo_config import cfg from oslo_log import log -from neutron.common.ovn.constants import OVN_ML2_MECH_DRIVER_NAME +from neutron.common.ovn import constants as ovn_const from neutron.objects import ports as port_obj from neutron.services.trunk.drivers import base as trunk_base @@ -94,10 +94,13 @@ class OVNTrunkHandler(object): LOG.debug("Port not found while trying to set " "binding_profile: %s", subport.port_id) return + ext_ids = {ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: db_port.device_owner} ovn_txn.add(self.plugin_driver.nb_ovn.set_lswitch_port( lport_name=subport.port_id, parent_name=parent_port, - tag=subport.segmentation_id)) + tag=subport.segmentation_id, + external_ids_update=ext_ids, + )) LOG.debug("Done setting parent %s for subport %s", parent_port, subport.port_id) @@ -128,11 +131,14 @@ class OVNTrunkHandler(object): LOG.debug("Port not found while trying to unset " "binding_profile: %s", subport.port_id) return + ext_ids = {ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY: db_port.device_owner} ovn_txn.add(self.plugin_driver.nb_ovn.set_lswitch_port( lport_name=subport.port_id, parent_name=[], up=False, - tag=[])) + tag=[], + external_ids_update=ext_ids, + )) LOG.debug("Done unsetting parent for subport %s", subport.port_id) def trunk_created(self, trunk): @@ -185,7 +191,8 @@ class OVNTrunkDriver(trunk_base.DriverBase): @property def is_loaded(self): try: - return OVN_ML2_MECH_DRIVER_NAME in cfg.CONF.ml2.mechanism_drivers + return (ovn_const.OVN_ML2_MECH_DRIVER_NAME in + cfg.CONF.ml2.mechanism_drivers) except cfg.NoSuchOptError: return False @@ -205,7 +212,7 @@ class OVNTrunkDriver(trunk_base.DriverBase): @classmethod def create(cls, plugin_driver): cls.plugin_driver = plugin_driver - return cls(OVN_ML2_MECH_DRIVER_NAME, + return cls(ovn_const.OVN_ML2_MECH_DRIVER_NAME, SUPPORTED_INTERFACES, SUPPORTED_SEGMENTATION_TYPES, None, 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 26d8959871a..5e12d7556a5 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 @@ -833,12 +833,12 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): txn.add(self.nb_api.pg_del(pg)) for lport_name in self.reset_lport_dhcpv4_options: - txn.add(self.nb_api.set_lswitch_port(lport_name, True, - dhcpv4_options=[])) + txn.add(self.nb_api.set_lswitch_port( + lport_name, if_exists=True, dhcpv4_options=[])) for lport_name in self.reset_lport_dhcpv6_options: - txn.add(self.nb_api.set_lswitch_port(lport_name, True, - dhcpv6_options=[])) + txn.add(self.nb_api.set_lswitch_port( + lport_name, if_exists=True, dhcpv6_options=[])) for dhcp_opts in self.stale_lport_dhcpv4_options: dhcpv4_opts = txn.add(self.nb_api.add_dhcp_options( @@ -851,7 +851,7 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): if dhcp_opts['port_id'] in self.orphaned_lport_dhcp_options: continue txn.add(self.nb_api.set_lswitch_port( - lport_name, True, dhcpv4_options=dhcpv4_opts)) + lport_name, if_exists=True, dhcpv4_options=dhcpv4_opts)) for dhcp_opts in self.stale_lport_dhcpv6_options: dhcpv6_opts = txn.add(self.nb_api.add_dhcp_options( @@ -864,7 +864,7 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): if dhcp_opts['port_id'] in self.orphaned_lport_dhcp_options: continue txn.add(self.nb_api.set_lswitch_port( - lport_name, True, dhcpv6_options=dhcpv6_opts)) + lport_name, if_exists=True, dhcpv6_options=dhcpv6_opts)) for row_uuid in self.missed_dhcp_options: txn.add(self.nb_api.delete_dhcp_options(row_uuid)) @@ -881,12 +881,12 @@ class TestOvnNbSync(base.TestOVNFunctionalBase): for port_id in self.lport_dhcpv4_disabled: txn.add(self.nb_api.set_lswitch_port( - port_id, True, + port_id, if_exists=True, dhcpv4_options=[self.lport_dhcpv4_disabled[port_id]])) for port_id in self.lport_dhcpv6_disabled: txn.add(self.nb_api.set_lswitch_port( - port_id, True, + port_id, if_exists=True, dhcpv6_options=[self.lport_dhcpv6_disabled[port_id]])) # Delete the first DNS record and clear the second row records 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 index 4927bbd5c61..0c42fbff3dc 100644 --- a/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py +++ b/neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py @@ -22,6 +22,8 @@ from neutron_lib.plugins import utils from neutron_lib.services.trunk import constants as trunk_consts from oslo_utils import uuidutils +from neutron.common.ovn import constants as ovn_const + class TestOVNTrunkDriver(base.TestOVNFunctionalBase): @@ -60,18 +62,25 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): for row in self.nb_api.tables[ 'Logical_Switch_Port'].rows.values(): if row.parent_name and row.tag: + device_owner = row.external_ids[ + ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY] ovn_trunk_info.append({'port_id': row.name, 'parent_port_id': row.parent_name, - 'tag': row.tag}) + 'tag': row.tag, + 'device_owner': device_owner, + }) return ovn_trunk_info def _verify_trunk_info(self, trunk, has_items): ovn_subports_info = self._get_ovn_trunk_info() neutron_subports_info = [] for subport in trunk.get('sub_ports', []): - neutron_subports_info.append({'port_id': subport['port_id'], - 'parent_port_id': [trunk['port_id']], - 'tag': [subport['segmentation_id']]}) + neutron_subports_info.append( + {'port_id': subport['port_id'], + 'parent_port_id': [trunk['port_id']], + 'tag': [subport['segmentation_id']], + 'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER, + }) # Check that the subport has the binding is active. binding = obj_reg.load_class('PortBinding').get_object( self.context, port_id=subport['port_id'], host='') diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 387ded939d2..39bf2a2334b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -200,7 +200,8 @@ class TestSetLSwitchPortCommand(TestBaseCommand): with mock.patch.object(idlutils, 'row_by_value', side_effect=idlutils.RowNotFound): cmd = commands.SetLSwitchPortCommand( - self.ovn_api, 'fake-lsp', if_exists=if_exists) + self.ovn_api, 'fake-lsp', external_ids_update=None, + if_exists=if_exists) if if_exists: cmd.run_idl(self.transaction) else: @@ -220,8 +221,8 @@ class TestSetLSwitchPortCommand(TestBaseCommand): with mock.patch.object(idlutils, 'row_by_value', return_value=fake_lsp): cmd = commands.SetLSwitchPortCommand( - self.ovn_api, fake_lsp.name, if_exists=True, - external_ids=new_ext_ids) + self.ovn_api, fake_lsp.name, external_ids_update=new_ext_ids, + if_exists=True, external_ids=new_ext_ids) cmd.run_idl(self.transaction) self.assertEqual(new_ext_ids, fake_lsp.external_ids) @@ -255,7 +256,8 @@ class TestSetLSwitchPortCommand(TestBaseCommand): with mock.patch.object(idlutils, 'row_by_value', return_value=fake_lsp): cmd = commands.SetLSwitchPortCommand( - self.ovn_api, fake_lsp.name, if_exists=True, **columns) + self.ovn_api, fake_lsp.name, external_ids_update=None, + if_exists=True, **columns) cmd.run_idl(self.transaction) if clear_v4_opts and clear_v6_opts: @@ -307,7 +309,8 @@ class TestSetLSwitchPortCommand(TestBaseCommand): with mock.patch.object(idlutils, 'row_by_value', return_value=fake_lsp): cmd = commands.SetLSwitchPortCommand( - self.ovn_api, fake_lsp.name, if_exists=True, + self.ovn_api, fake_lsp.name, external_ids_update=ext_ids, + if_exists=True, external_ids=ext_ids, dhcpv4_options=dhcpv4_opts, dhcpv6_options=dhcpv6_opts) if not isinstance(dhcpv4_opts, list): diff --git a/neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py b/neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py index 621b815f448..986380e598e 100644 --- a/neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py +++ b/neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py @@ -129,7 +129,9 @@ class TestTrunkHandler(base.BaseTestCase): calls = [mock.call(lport_name=s_port.port_id, parent_name=trunk.port_id, - tag=s_port.segmentation_id) + tag=s_port.segmentation_id, + external_ids_update={ + 'neutron:device_owner': 'trunk:subport'}) for trunk, s_port in [(self.trunk_1, self.sub_port_1), (self.trunk_1, self.sub_port_2)]] self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls) @@ -196,7 +198,8 @@ class TestTrunkHandler(base.BaseTestCase): calls = [mock.call(lport_name=s_port.port_id, parent_name=[], tag=[], - up=False) + up=False, + external_ids_update={'neutron:device_owner': ''}) for trunk, s_port in [(self.trunk_1, self.sub_port_1), (self.trunk_1, self.sub_port_2)]] self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls) @@ -225,7 +228,8 @@ class TestTrunkHandler(base.BaseTestCase): calls = [mock.call(lport_name=s_port.port_id, parent_name=[], tag=[], - up=False) + up=False, + external_ids_update={'neutron:device_owner': ''}) for trunk, s_port in [(self.trunk_1, self.sub_port_1)]] self._assert_calls(self.plugin_driver.nb_ovn.set_lswitch_port, calls)