diff --git a/neutron/services/trunk/drivers/ovn/trunk_driver.py b/neutron/services/trunk/drivers/ovn/trunk_driver.py index fcc260ff4e0..637aa6de8b0 100644 --- a/neutron/services/trunk/drivers/ovn/trunk_driver.py +++ b/neutron/services/trunk/drivers/ovn/trunk_driver.py @@ -14,6 +14,7 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources +from neutron_lib import constants from neutron_lib import context as n_context from neutron_lib.db import api as db_api from neutron_lib import exceptions as n_exc @@ -26,6 +27,7 @@ from neutron.db import db_base_plugin_common from neutron.db import ovn_revision_numbers_db as db_rev from neutron.objects import ports as port_obj +from neutron.objects import trunk as trunk_objects from neutron.services.trunk.drivers import base as trunk_base from neutron.services.trunk import exceptions as trunk_exc @@ -216,6 +218,46 @@ def subport_event(self, resource, event, trunk_plugin, payload): self.subports_deleted(payload.states[0], payload.metadata['subports']) + def port_updated(self, resource, event, trunk_plugin, payload): + '''Propagate trunk parent port ACTIVE to trunk ACTIVE + + During a live migration with a trunk the only way we found to update + the trunk to ACTIVE is to do this when the trunk's parent port gets + updated to ACTIVE. This is clearly suboptimal, because the trunk's + ACTIVE status should mean that all of its ports (parent and sub) are + active. But in ml2/ovn the parent port's binding is not cascaded to the + subports. Actually the subports' binding:host is left empty. This way + here we don't know anything about the subports' state changes during a + live migration. If we don't want to leave the trunk in DOWN this is + what we have. + + Please note that this affects trunk create as well. Because of this we + move ml2/ovn trunks to ACTIVE way early. But at least here we don't + affect other mechanism drivers and their corresponding trunk drivers. + + See also: + https://bugs.launchpad.net/neutron/+bug/1988549 + https://review.opendev.org/c/openstack/neutron/+/853779 + https://bugs.launchpad.net/neutron/+bug/2095152 + ''' + updated_port = payload.latest_state + trunk_details = updated_port.get('trunk_details') + # If no trunk_details, the port is not the parent of a trunk. + if not trunk_details: + return + + original_port = payload.states[0] + orig_status = original_port.get('status') + new_status = updated_port.get('status') + context = payload.context + trunk_id = trunk_details['trunk_id'] + if (new_status == constants.PORT_STATUS_ACTIVE and + new_status != orig_status): + trunk = trunk_objects.Trunk.get_object(context, id=trunk_id) + if trunk is None: + raise trunk_exc.TrunkNotFound(trunk_id=trunk_id) + trunk.update(status=trunk_consts.TRUNK_ACTIVE_STATUS) + class OVNTrunkDriver(trunk_base.DriverBase): @property @@ -242,6 +284,11 @@ def register(self, resource, event, trigger, payload=None): resources.SUBPORTS, _event) + registry.subscribe( + self._handler.port_updated, + resources.PORT, + events.AFTER_UPDATE) + @classmethod def create(cls, plugin_driver): cls.plugin_driver = plugin_driver diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index ffcf1a8a2c3..fe314cbc2b9 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -21,7 +21,6 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources -from neutron_lib import constants as const from neutron_lib import context from neutron_lib.db import api as db_api from neutron_lib.db import resource_extend @@ -465,19 +464,12 @@ def _trigger_trunk_status_change(self, resource, event, trigger, payload): original_port = payload.states[0] orig_vif_type = original_port.get(portbindings.VIF_TYPE) new_vif_type = updated_port.get(portbindings.VIF_TYPE) - orig_status = original_port.get('status') - new_status = updated_port.get('status') vif_type_changed = orig_vif_type != new_vif_type - trunk_id = trunk_details['trunk_id'] if vif_type_changed and new_vif_type == portbindings.VIF_TYPE_UNBOUND: + trunk_id = trunk_details['trunk_id'] # NOTE(status_police) Trunk status goes to DOWN when the parent # port is unbound. This means there are no more physical resources # associated with the logical resource. self.update_trunk( context, trunk_id, {'trunk': {'status': constants.TRUNK_DOWN_STATUS}}) - elif new_status == const.PORT_STATUS_ACTIVE and \ - new_status != orig_status: - self.update_trunk( - context, trunk_id, - {'trunk': {'status': constants.TRUNK_ACTIVE_STATUS}}) 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..a4f703fd3e3 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 @@ -18,6 +18,7 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources +from neutron_lib import constants as nlib_consts from neutron_lib import exceptions as n_exc from neutron_lib.services.trunk import constants as trunk_consts from oslo_config import cfg @@ -25,9 +26,13 @@ from neutron.common.ovn.constants import OVN_ML2_MECH_DRIVER_NAME from neutron.objects.ports import Port from neutron.objects.ports import PortBinding +from neutron.objects import trunk as trunk_objects +from neutron.services.trunk import drivers from neutron.services.trunk.drivers.ovn import trunk_driver +from neutron.services.trunk import plugin as trunk_plugin from neutron.tests import base from neutron.tests.unit import fake_resources +from neutron.tests.unit.plugins.ml2 import test_plugin class TestTrunkHandler(base.BaseTestCase): @@ -363,6 +368,70 @@ def test_subport_event_invalid(self, s_deleted, s_added): s_deleted.assert_not_called() +class TestTrunkHandlerWithPlugin(test_plugin.Ml2PluginV2TestCase): + def setUp(self): + super().setUp() + self.drivers_patch = mock.patch.object(drivers, 'register').start() + self.compat_patch = mock.patch.object( + trunk_plugin.TrunkPlugin, 'check_compatibility').start() + self.trunk_plugin = trunk_plugin.TrunkPlugin() + self.trunk_plugin.add_segmentation_type('vlan', lambda x: True) + self.plugin_driver = mock.Mock() + self.trunk_handler = trunk_driver.OVNTrunkHandler(self.plugin_driver) + + def _create_test_trunk(self, port, subports=None): + subports = subports if subports else [] + trunk = {'port_id': port['port']['id'], + 'project_id': 'test_tenant', + 'sub_ports': subports} + response = ( + self.trunk_plugin.create_trunk(self.context, {'trunk': trunk})) + return response + + def _get_trunk_obj(self, trunk_id): + return trunk_objects.Trunk.get_object(self.context, id=trunk_id) + + def test_parent_active_triggers_trunk_active(self): + with self.port() as new_parent: + new_parent['status'] = nlib_consts.PORT_STATUS_ACTIVE + old_parent = {'status': nlib_consts.PORT_STATUS_DOWN} + old_trunk = self._create_test_trunk(new_parent) + old_trunk = self._get_trunk_obj(old_trunk['id']) + old_trunk.update(status=trunk_consts.TRUNK_DOWN_STATUS) + trunk_details = {'trunk_id': old_trunk.id} + new_parent['trunk_details'] = trunk_details + old_parent['trunk_details'] = trunk_details + self.trunk_handler.port_updated( + resources.PORT, + events.AFTER_UPDATE, + None, + payload=events.DBEventPayload( + self.context, states=(old_parent, new_parent))) + new_trunk = self._get_trunk_obj(old_trunk.id) + self.assertEqual( + trunk_consts.TRUNK_ACTIVE_STATUS, new_trunk.status) + + def test_parent_build_does_not_trigger_trunk_active(self): + with self.port() as new_parent: + new_parent['status'] = nlib_consts.PORT_STATUS_BUILD + old_parent = {'status': nlib_consts.PORT_STATUS_DOWN} + old_trunk = self._create_test_trunk(new_parent) + old_trunk = self._get_trunk_obj(old_trunk['id']) + old_trunk.update(status=trunk_consts.TRUNK_DOWN_STATUS) + trunk_details = {'trunk_id': old_trunk.id} + new_parent['trunk_details'] = trunk_details + old_parent['trunk_details'] = trunk_details + self.trunk_handler.port_updated( + resources.PORT, + events.AFTER_UPDATE, + None, + payload=events.DBEventPayload( + self.context, states=(old_parent, new_parent))) + new_trunk = self._get_trunk_obj(old_trunk.id) + self.assertNotEqual( + trunk_consts.TRUNK_ACTIVE_STATUS, new_trunk.status) + + class TestTrunkDriver(base.BaseTestCase): def setUp(self): super(TestTrunkDriver, self).setUp() diff --git a/neutron/tests/unit/services/trunk/test_plugin.py b/neutron/tests/unit/services/trunk/test_plugin.py index 271c16e66db..ca7f07e9de4 100644 --- a/neutron/tests/unit/services/trunk/test_plugin.py +++ b/neutron/tests/unit/services/trunk/test_plugin.py @@ -19,7 +19,6 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources -from neutron_lib import constants as neutron_const from neutron_lib.plugins import directory from neutron_lib.services.trunk import constants import testtools @@ -287,32 +286,6 @@ def test_remove_subports_trunk_goes_to_down(self): {'sub_ports': [{'port_id': subport['port']['id']}]}) self.assertEqual(constants.TRUNK_DOWN_STATUS, trunk['status']) - def test__trigger_trunk_status_change_parent_port_status_down(self): - callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) - with self.port() as parent: - parent['status'] = neutron_const.PORT_STATUS_DOWN - original_port = {'status': neutron_const.PORT_STATUS_DOWN} - _, _ = ( - self._test__trigger_trunk_status_change( - parent, original_port, - constants.TRUNK_DOWN_STATUS, - constants.TRUNK_DOWN_STATUS)) - callback.assert_not_called() - - def test__trigger_trunk_status_change_parent_port_status_up(self): - callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) - with self.port() as parent: - parent['status'] = neutron_const.PORT_STATUS_ACTIVE - original_port = {'status': neutron_const.PORT_STATUS_DOWN} - _, _ = ( - self._test__trigger_trunk_status_change( - parent, original_port, - constants.TRUNK_DOWN_STATUS, - constants.TRUNK_ACTIVE_STATUS)) - callback.assert_called_once_with( - resources.TRUNK, events.AFTER_UPDATE, - self.trunk_plugin, payload=mock.ANY) - def test__trigger_trunk_status_change_vif_type_changed_unbound(self): callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) with self.port() as parent: