openstack/neutron
Code Issues Proposed changes

Merge "Revert "[OVN] Prevent Trunk creation/deletion with parent port bound"" into stable/2024.1

Browse Source
This commit is contained in:
Zuul 2024-06-18 17:10:41 +00:00 committed by Gerrit Code Review
commit e9f3825e5c
9 changed files with 18 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
neutron/common/utils.py
View File

@ -37,12 +37,9 @@ import eventlet
from eventlet.green import subprocess
import netaddr
from neutron_lib.api.definitions import availability_zone as az_def
from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import portbindings_extended
from neutron_lib import constants as n_const
from neutron_lib import context as n_context
from neutron_lib.db import api as db_api
from neutron_lib.plugins import utils as plugin_utils
from neutron_lib.services.qos import constants as qos_consts
from neutron_lib.services.trunk import constants as trunk_constants
from neutron_lib.utils import helpers
@ -1106,16 +1103,3 @@ def parse_permitted_ethertypes(permitted_ethertypes):
continue
return ret
# TODO(slaweq): this should be moved to neutron_lib.plugins.utils module
def is_port_bound(port, log_message=True):
active_binding = plugin_utils.get_port_binding_by_status_and_host(
port.get('port_bindings', []), n_const.ACTIVE)
if not active_binding:
if log_message:
LOG.warning('Binding for port %s was not found.', port)
return False
return active_binding[portbindings_extended.VIF_TYPE] not in (
portbindings.VIF_TYPE_UNBOUND,
portbindings.VIF_TYPE_BINDING_FAILED)

15
neutron/db/l3_dvr_db.py
View File

@ -17,6 +17,7 @@ import netaddr
from neutron_lib.api.definitions import external_net as extnet_apidef
from neutron_lib.api.definitions import l3 as l3_apidef
from neutron_lib.api.definitions import portbindings
from neutron_lib.api.definitions import portbindings_extended
from neutron_lib.api.definitions import router_admin_state_down_before_update
from neutron_lib.api import validators
from neutron_lib.callbacks import events
@ -70,6 +71,18 @@ def is_admin_state_down_necessary():
return _IS_ADMIN_STATE_DOWN_NECESSARY
# TODO(slaweq): this should be moved to neutron_lib.plugins.utils module
def is_port_bound(port):
active_binding = plugin_utils.get_port_binding_by_status_and_host(
port.get("port_bindings", []), const.ACTIVE)
if not active_binding:
LOG.warning("Binding for port %s was not found.", port)
return False
return active_binding[portbindings_extended.VIF_TYPE] not in [
portbindings.VIF_TYPE_UNBOUND,
portbindings.VIF_TYPE_BINDING_FAILED]
@registry.has_registry_receivers
class DVRResourceOperationHandler(object):
"""Contains callbacks for DVR operations.
@ -1422,7 +1435,7 @@ class L3_NAT_with_dvr_db_mixin(_DVRAgentInterfaceMixin,
def get_ports_under_dvr_connected_subnet(self, context, subnet_id):
ports = dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)
ports = [p for p in ports if n_utils.is_port_bound(p)]
ports = [p for p in ports if is_port_bound(p)]
# TODO(slaweq): if there would be way to pass to neutron-lib only
# list of extensions which actually should be processed, than setting
# process_extensions=True below could avoid that second loop and

26
neutron/services/trunk/drivers/ovn/trunk_driver.py
View File

@ -22,12 +22,10 @@ from oslo_config import cfg
from oslo_log import log
from neutron.common.ovn import constants as ovn_const
from neutron.common import utils as n_utils
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.services.trunk.drivers import base as trunk_base
from neutron.services.trunk import exceptions as trunk_exc
SUPPORTED_INTERFACES = (
@ -157,10 +155,6 @@ class OVNTrunkHandler(object):
LOG.debug("Done unsetting parent for subport %s", subport.port_id)
return db_port
@staticmethod
def _is_port_bound(port):
return n_utils.is_port_bound(port, log_message=False)
def trunk_created(self, resource, event, trunk_plugin, payload):
trunk = payload.states[0]
# Check if parent port is handled by OVN.
@ -176,18 +170,6 @@ class OVNTrunkHandler(object):
if trunk.sub_ports:
self._unset_sub_ports(trunk.sub_ports)
def trunk_created_precommit(self, resource, event, trunk_plugin, payload):
# payload.desired_state below is the trunk object
parent_port = payload.desired_state.db_obj.port
if self._is_port_bound(parent_port):
raise trunk_exc.ParentPortInUse(port_id=parent_port.id)
def trunk_deleted_precommit(self, resource, event, trunk_plugin, payload):
trunk = payload.states[0]
parent_port = payload.states[1]
if self._is_port_bound(parent_port):
raise trunk_exc.TrunkInUse(trunk_id=trunk.id)
def subports_added(self, resource, event, trunk_plugin, payload):
trunk = payload.states[0]
subports = payload.metadata['subports']
@ -226,14 +208,6 @@ class OVNTrunkDriver(trunk_base.DriverBase):
resource, event, trigger, payload=payload)
self._handler = OVNTrunkHandler(self.plugin_driver)
registry.subscribe(
self._handler.trunk_created_precommit,
resources.TRUNK,
events.PRECOMMIT_CREATE)
registry.subscribe(
self._handler.trunk_deleted_precommit,
resources.TRUNK,
events.PRECOMMIT_DELETE)
registry.subscribe(
self._handler.trunk_created, resources.TRUNK, events.AFTER_CREATE)
registry.subscribe(

5
neutron/services/trunk/plugin.py
View File

@ -294,7 +294,6 @@ class TrunkPlugin(service_base.ServicePluginBase):
trunk = self._get_trunk(context, trunk_id)
rules.trunk_can_be_managed(context, trunk)
trunk_port_validator = rules.TrunkPortValidator(trunk.port_id)
parent_port = trunk.db_obj.port
if trunk_port_validator.can_be_trunked_or_untrunked(context):
# NOTE(status_police): when a trunk is deleted, the logical
# object disappears from the datastore, therefore there is no
@ -308,7 +307,7 @@ class TrunkPlugin(service_base.ServicePluginBase):
'deleting trunk port %s: %s', trunk_id,
str(e))
payload = events.DBEventPayload(context, resource_id=trunk_id,
states=(trunk, parent_port))
states=(trunk,))
registry.publish(resources.TRUNK, events.PRECOMMIT_DELETE,
self, payload=payload)
else:
@ -318,7 +317,7 @@ class TrunkPlugin(service_base.ServicePluginBase):
registry.publish(resources.TRUNK, events.AFTER_DELETE, self,
payload=events.DBEventPayload(
context, resource_id=trunk_id,
states=(trunk, parent_port)))
states=(trunk,)))
@db_base_plugin_common.convert_result_to_dict
def add_subports(self, context, trunk_id, subports):

33
neutron/tests/functional/services/trunk/drivers/ovn/test_trunk_driver.py
View File

@ -14,8 +14,6 @@
import contextlib
from neutron_lib.api.definitions import portbindings
from neutron_lib.callbacks import exceptions as n_exc
from neutron_lib import constants as n_consts
from neutron_lib.objects import registry as obj_reg
from neutron_lib.plugins import utils
@ -23,7 +21,6 @@ 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
@ -108,25 +105,6 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
with self.trunk([subport]) as trunk:
self._verify_trunk_info(trunk, has_items=True)
def test_trunk_create_parent_port_bound(self):
with self.network() as network:
with self.subnet(network=network) as subnet:
with self.port(subnet=subnet) as parent_port:
pb = port_obj.PortBinding.get_objects(
self.context, port_id=parent_port['port']['id'])
port_obj.PortBinding.update_object(
self.context, {'vif_type': portbindings.VIF_TYPE_OVS},
port_id=pb[0].port_id, host=pb[0].host)
tenant_id = uuidutils.generate_uuid()
trunk = {'trunk': {
'port_id': parent_port['port']['id'],
'tenant_id': tenant_id, 'project_id': tenant_id,
'admin_state_up': True,
'name': 'trunk', 'sub_ports': []}}
self.assertRaises(n_exc.CallbackFailure,
self.trunk_plugin.create_trunk,
self.context, trunk)
def test_subport_add(self):
with self.subport() as subport:
with self.trunk() as trunk:
@ -149,14 +127,3 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
with self.trunk() as trunk:
self.trunk_plugin.delete_trunk(self.context, trunk['id'])
self._verify_trunk_info({}, has_items=False)
def test_trunk_delete_parent_port_bound(self):
with self.trunk() as trunk:
bp = port_obj.PortBinding.get_objects(
self.context, port_id=trunk['port_id'])
port_obj.PortBinding.update_object(
self.context, {'vif_type': portbindings.VIF_TYPE_OVS},
port_id=bp[0].port_id, host=bp[0].host)
self.assertRaises(n_exc.CallbackFailure,
self.trunk_plugin.delete_trunk,
self.context, trunk['id'])

3
neutron/tests/unit/db/test_l3_dvr_db.py
View File

@ -30,7 +30,6 @@ from neutron_lib.plugins import directory
from neutron_lib.plugins import utils as plugin_utils
from oslo_utils import uuidutils
from neutron.common import utils as n_utils
from neutron.db import agents_db
from neutron.db import l3_dvr_db
from neutron.db import l3_dvrscheduler_db
@ -1511,7 +1510,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
self.assertTrue(
self.mixin.is_router_distributed(self.ctx, router_id))
@mock.patch.object(n_utils, 'is_port_bound')
@mock.patch.object(l3_dvr_db, "is_port_bound")
def test_get_ports_under_dvr_connected_subnet(self, is_port_bound_mock):
router_dict = {'name': 'test_router', 'admin_state_up': True,
'distributed': True}

4
neutron/tests/unit/services/trunk/drivers/ovn/test_trunk_driver.py
View File

@ -454,10 +454,6 @@ class TestTrunkDriver(base.BaseTestCase):
with mock.patch.object(registry, 'subscribe') as mock_subscribe:
driver.register(mock.ANY, mock.ANY, mock.Mock())
calls = [
mock.call.mock_subscribe(
mock.ANY, resources.TRUNK, events.PRECOMMIT_CREATE),
mock.call.mock_subscribe(
mock.ANY, resources.TRUNK, events.PRECOMMIT_DELETE),
mock.call.mock_subscribe(
mock.ANY, resources.TRUNK, events.AFTER_CREATE),
mock.call.mock_subscribe(

3
neutron/tests/unit/services/trunk/test_plugin.py
View File

@ -162,8 +162,7 @@ class TrunkPluginTestCase(test_plugin.Ml2PluginV2TestCase):
resources.TRUNK, event, self.trunk_plugin, payload=mock.ANY)
payload = callback.mock_calls[0][2]['payload']
self.assertEqual(self.context, payload.context)
self.assertEqual(trunk_obj, payload.states[0])
self.assertEqual(parent_port['port']['id'], payload.states[1].id)
self.assertEqual(trunk_obj, payload.latest_state)
self.assertEqual(trunk['id'], payload.resource_id)
def test_delete_trunk_notify_after_delete(self):

6
releasenotes/notes/ovn-trunk-check-parent-port-eeca2eceaca9d158.yaml
View File

@ -1,6 +0,0 @@
---
fixes:
- |
Now the ML2/OVN trunk driver prevents a trunk creation if the parent port
is already bound. In the same way, if a parent port being used in a trunk
is bound, the trunk cannot be deleted.