From 955e621167987626806128d259a29d4e62acf102 Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Mon, 8 May 2023 16:51:41 +0200 Subject: [PATCH] [OVN][Trunk] Add port binding info on subport when parent is bound The host ID and VIF details are added on the subport when the trunk is created, if it's created when it's not attached to any VM this fields will remain empty and be filled on the parent port when it gets bound to a host, but there's no callback to add this info on the subport. Closes-Bug: #2018289 Closes-Bug: #2024160 Change-Id: I34bb6f178c314907bdf9f76789777f6736938b67 --- .../trunk/drivers/ovn/trunk_driver.py | 30 +++++++++-- .../trunk/drivers/ovn/test_trunk_driver.py | 50 +++++++++++++------ .../trunk/drivers/ovn/test_trunk_driver.py | 2 + 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/neutron/services/trunk/drivers/ovn/trunk_driver.py b/neutron/services/trunk/drivers/ovn/trunk_driver.py index 650fe1d119c..6d248aff79e 100644 --- a/neutron/services/trunk/drivers/ovn/trunk_driver.py +++ b/neutron/services/trunk/drivers/ovn/trunk_driver.py @@ -49,11 +49,13 @@ class OVNTrunkHandler(object): 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] for subport in subports: with db_api.CONTEXT_WRITER.using(context), ( txn(check_error=True)) as ovn_txn: port = self._set_binding_profile(context, subport, parent_port, - parent_port_status, ovn_txn) + parent_port_status, + parent_port_bindings, ovn_txn) db_rev.bump_revision(context, port, ovn_const.TYPE_PORTS) def _unset_sub_ports(self, subports): @@ -67,7 +69,8 @@ class OVNTrunkHandler(object): @db_base_plugin_common.convert_result_to_dict def _set_binding_profile(self, context, subport, parent_port, - parent_port_status, ovn_txn): + parent_port_status, + parent_port_bindings, ovn_txn): LOG.debug("Setting parent %s for subport %s", parent_port, subport.port_id) db_port = port_obj.Port.get_object(context, id=subport.port_id) @@ -79,6 +82,9 @@ class OVNTrunkHandler(object): check_rev_cmd = self.plugin_driver.nb_ovn.check_revision_number( 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 try: # NOTE(flaviof): We expect binding's host to be set. Otherwise, # sub-port will not transition from DOWN to ACTIVE. @@ -94,6 +100,7 @@ class OVNTrunkHandler(object): port_obj.PortBinding.update_object( context, {'profile': binding.profile, + 'host': parent_binding_host, 'vif_type': portbindings.VIF_TYPE_OVS}, port_id=subport.port_id, host=binding.host) @@ -155,6 +162,14 @@ class OVNTrunkHandler(object): LOG.debug("Done unsetting parent for subport %s", subport.port_id) return db_port + def trunk_updated(self, trunk): + # Check if parent port is handled by OVN. + if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port', + trunk.port_id, default=None): + return + if trunk.sub_ports: + self._set_sub_ports(trunk.port_id, trunk.sub_ports) + def trunk_created(self, trunk): # Check if parent port is handled by OVN. if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port', @@ -189,6 +204,8 @@ class OVNTrunkHandler(object): def trunk_event(self, resource, event, trunk_plugin, payload): if event == events.AFTER_CREATE: self.trunk_created(payload.states[0]) + elif event == events.AFTER_UPDATE: + self.trunk_updated(payload.states[0]) elif event == events.AFTER_DELETE: self.trunk_deleted(payload.states[0]) @@ -215,13 +232,16 @@ class OVNTrunkDriver(trunk_base.DriverBase): super(OVNTrunkDriver, self).register( resource, event, trigger, payload=payload) self._handler = OVNTrunkHandler(self.plugin_driver) - for trunk_event in (events.AFTER_CREATE, events.AFTER_DELETE): + for _event in (events.AFTER_CREATE, events.AFTER_UPDATE, + events.AFTER_DELETE): registry.subscribe(self._handler.trunk_event, resources.TRUNK, - trunk_event) + _event) + + for _event in (events.AFTER_CREATE, events.AFTER_DELETE): registry.subscribe(self._handler.subport_event, resources.SUBPORTS, - trunk_event) + _event) @classmethod def create(cls, plugin_driver): 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 fed86fabbb4..1c19b38b282 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 @@ -14,21 +14,23 @@ import contextlib -from neutron.services.trunk import plugin as trunk_plugin -from neutron.tests.functional import base +from neutron_lib.api.definitions import portbindings from neutron_lib import constants as n_consts -from neutron_lib.objects import registry as obj_reg +from neutron_lib.db import api as db_api 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 +from neutron.objects import ports as port_obj +from neutron.services.trunk import plugin as trunk_plugin +from neutron.tests.functional import base class TestOVNTrunkDriver(base.TestOVNFunctionalBase): - def setUp(self): - super(TestOVNTrunkDriver, self).setUp() + def setUp(self, **kwargs): + super().setUp(**kwargs) self.trunk_plugin = trunk_plugin.TrunkPlugin() self.trunk_plugin.add_segmentation_type( trunk_consts.SEGMENTATION_TYPE_VLAN, @@ -39,7 +41,8 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): sub_ports = sub_ports or [] with self.network() as network: with self.subnet(network=network) as subnet: - with self.port(subnet=subnet) as parent_port: + with self.port(subnet=subnet, + device_owner='compute:nova') as parent_port: tenant_id = uuidutils.generate_uuid() trunk = {'trunk': { 'port_id': parent_port['port']['id'], @@ -64,17 +67,14 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): if row.parent_name and row.tag: device_owner = row.external_ids[ ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY] - revision_number = row.external_ids[ - ovn_const.OVN_REV_NUM_EXT_ID_KEY] ovn_trunk_info.append({'port_id': row.name, 'parent_port_id': row.parent_name, 'tag': row.tag, 'device_owner': device_owner, - 'revision_number': revision_number, }) return ovn_trunk_info - def _verify_trunk_info(self, trunk, has_items): + def _verify_trunk_info(self, trunk, has_items, host=''): ovn_subports_info = self._get_ovn_trunk_info() neutron_subports_info = [] for subport in trunk.get('sub_ports', []): @@ -83,12 +83,12 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): 'parent_port_id': [trunk['port_id']], 'tag': [subport['segmentation_id']], 'device_owner': trunk_consts.TRUNK_SUBPORT_OWNER, - 'revision_number': '2', }) - # 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='') - self.assertEqual(n_consts.PORT_STATUS_ACTIVE, binding['status']) + # Check the subport binding. + pb = port_obj.PortBinding.get_object( + self.context, port_id=subport['port_id'], host=host) + self.assertEqual(n_consts.PORT_STATUS_ACTIVE, pb.status) + self.assertEqual(host, pb.host) self.assertCountEqual(ovn_subports_info, neutron_subports_info) self.assertEqual(has_items, len(neutron_subports_info) != 0) @@ -96,6 +96,14 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): if trunk.get('status'): self.assertEqual(trunk_consts.TRUNK_ACTIVE_STATUS, trunk['status']) + def _bind_port(self, port_id, host): + 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() + def test_trunk_create(self): with self.trunk() as trunk: self._verify_trunk_info(trunk, has_items=False) @@ -113,10 +121,22 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase): 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') + self.mech_driver.set_port_status_up(trunk['port_id']) + self._verify_trunk_info(new_trunk, has_items=True, + host='host1') def test_subport_delete(self): with self.subport() as subport: with self.trunk([subport]) as trunk: + # Bind parent port. + self._bind_port(trunk['port_id'], 'host1') + self.mech_driver.set_port_status_up(trunk['port_id']) + self._verify_trunk_info(trunk, has_items=True, + host='host1') + self.trunk_plugin.remove_subports(self.context, trunk['id'], {'sub_ports': [subport]}) new_trunk = self.trunk_plugin.get_trunk(self.context, 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 894520bf860..5891c2ab2c4 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 @@ -122,6 +122,7 @@ class TestTrunkHandler(base.BaseTestCase): mock.call(mock.ANY, {'profile': {'parent_name': trunk.port_id, 'tag': s_port.segmentation_id}, + 'host': mock.ANY, 'vif_type': portbindings.VIF_TYPE_OVS}, host=mock.ANY, port_id=s_port.port_id) @@ -152,6 +153,7 @@ class TestTrunkHandler(base.BaseTestCase): self.mock_update_pb.assert_called_once_with( mock.ANY, {'profile': {'parent_name': self.sub_port_1.trunk_id, 'tag': self.sub_port_1.segmentation_id}, + 'host': 'foo.com', 'vif_type': portbindings.VIF_TYPE_OVS}, host='foo.com', port_id=self.sub_port_1.port_id) self.mock_port_update.assert_not_called()