Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions neutron/services/trunk/drivers/ovn/trunk_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
10 changes: 1 addition & 9 deletions neutron/services/trunk/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}})
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,21 @@
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

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):
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 0 additions & 27 deletions neutron/tests/unit/services/trunk/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down