[OVN] Prevent Trunk creation/deletion with parent port bound
This patch imitates the ML2/OVS Trunk driver behaviour. When the trunk parent port is bound: * A new trunk cannot be created using this parent port. * If the port is assigned as parent port of a trunk, this trunk cannot be deleted. Closes-Bug: #2022059 Change-Id: I8cfa7e67524a42224cbb4b3c3cec3cfa49b795fd
This commit is contained in:
parent
dec4435c9d
commit
833a6d82cd
|
@ -37,9 +37,12 @@ import eventlet
|
||||||
from eventlet.green import subprocess
|
from eventlet.green import subprocess
|
||||||
import netaddr
|
import netaddr
|
||||||
from neutron_lib.api.definitions import availability_zone as az_def
|
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 constants as n_const
|
||||||
from neutron_lib import context as n_context
|
from neutron_lib import context as n_context
|
||||||
from neutron_lib.db import api as db_api
|
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.qos import constants as qos_consts
|
||||||
from neutron_lib.services.trunk import constants as trunk_constants
|
from neutron_lib.services.trunk import constants as trunk_constants
|
||||||
from neutron_lib.utils import helpers
|
from neutron_lib.utils import helpers
|
||||||
|
@ -1108,3 +1111,16 @@ def parse_permitted_ethertypes(permitted_ethertypes):
|
||||||
continue
|
continue
|
||||||
|
|
||||||
return ret
|
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)
|
||||||
|
|
|
@ -17,7 +17,6 @@ import netaddr
|
||||||
from neutron_lib.api.definitions import external_net as extnet_apidef
|
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 l3 as l3_apidef
|
||||||
from neutron_lib.api.definitions import portbindings
|
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.definitions import router_admin_state_down_before_update
|
||||||
from neutron_lib.api import validators
|
from neutron_lib.api import validators
|
||||||
from neutron_lib.callbacks import events
|
from neutron_lib.callbacks import events
|
||||||
|
@ -71,18 +70,6 @@ def is_admin_state_down_necessary():
|
||||||
return _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
|
@registry.has_registry_receivers
|
||||||
class DVRResourceOperationHandler(object):
|
class DVRResourceOperationHandler(object):
|
||||||
"""Contains callbacks for DVR operations.
|
"""Contains callbacks for DVR operations.
|
||||||
|
@ -1428,7 +1415,7 @@ class L3_NAT_with_dvr_db_mixin(_DVRAgentInterfaceMixin,
|
||||||
|
|
||||||
def get_ports_under_dvr_connected_subnet(self, context, subnet_id):
|
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 = dvr_mac_db.get_ports_query_by_subnet_and_ip(context, subnet_id)
|
||||||
ports = [p for p in ports if is_port_bound(p)]
|
ports = [p for p in ports if n_utils.is_port_bound(p)]
|
||||||
# TODO(slaweq): if there would be way to pass to neutron-lib only
|
# TODO(slaweq): if there would be way to pass to neutron-lib only
|
||||||
# list of extensions which actually should be processed, than setting
|
# list of extensions which actually should be processed, than setting
|
||||||
# process_extensions=True below could avoid that second loop and
|
# process_extensions=True below could avoid that second loop and
|
||||||
|
|
|
@ -22,10 +22,12 @@ from oslo_config import cfg
|
||||||
from oslo_log import log
|
from oslo_log import log
|
||||||
|
|
||||||
from neutron.common.ovn import constants as ovn_const
|
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 db_base_plugin_common
|
||||||
from neutron.db import ovn_revision_numbers_db as db_rev
|
from neutron.db import ovn_revision_numbers_db as db_rev
|
||||||
from neutron.objects import ports as port_obj
|
from neutron.objects import ports as port_obj
|
||||||
from neutron.services.trunk.drivers import base as trunk_base
|
from neutron.services.trunk.drivers import base as trunk_base
|
||||||
|
from neutron.services.trunk import exceptions as trunk_exc
|
||||||
|
|
||||||
|
|
||||||
SUPPORTED_INTERFACES = (
|
SUPPORTED_INTERFACES = (
|
||||||
|
@ -162,6 +164,10 @@ class OVNTrunkHandler(object):
|
||||||
LOG.debug("Done unsetting parent for subport %s", subport.port_id)
|
LOG.debug("Done unsetting parent for subport %s", subport.port_id)
|
||||||
return db_port
|
return db_port
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _is_port_bound(port):
|
||||||
|
return n_utils.is_port_bound(port, log_message=False)
|
||||||
|
|
||||||
def trunk_updated(self, trunk):
|
def trunk_updated(self, trunk):
|
||||||
# Check if parent port is handled by OVN.
|
# Check if parent port is handled by OVN.
|
||||||
if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
|
if not self.plugin_driver.nb_ovn.lookup('Logical_Switch_Port',
|
||||||
|
@ -208,6 +214,16 @@ class OVNTrunkHandler(object):
|
||||||
self.trunk_updated(payload.states[0])
|
self.trunk_updated(payload.states[0])
|
||||||
elif event == events.AFTER_DELETE:
|
elif event == events.AFTER_DELETE:
|
||||||
self.trunk_deleted(payload.states[0])
|
self.trunk_deleted(payload.states[0])
|
||||||
|
elif event == events.PRECOMMIT_CREATE:
|
||||||
|
trunk = payload.desired_state
|
||||||
|
parent_port = trunk.db_obj.port
|
||||||
|
if self._is_port_bound(parent_port):
|
||||||
|
raise trunk_exc.ParentPortInUse(port_id=parent_port.id)
|
||||||
|
elif event == events.PRECOMMIT_DELETE:
|
||||||
|
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 subport_event(self, resource, event, trunk_plugin, payload):
|
def subport_event(self, resource, event, trunk_plugin, payload):
|
||||||
if event == events.AFTER_CREATE:
|
if event == events.AFTER_CREATE:
|
||||||
|
@ -233,7 +249,8 @@ class OVNTrunkDriver(trunk_base.DriverBase):
|
||||||
resource, event, trigger, payload=payload)
|
resource, event, trigger, payload=payload)
|
||||||
self._handler = OVNTrunkHandler(self.plugin_driver)
|
self._handler = OVNTrunkHandler(self.plugin_driver)
|
||||||
for _event in (events.AFTER_CREATE, events.AFTER_UPDATE,
|
for _event in (events.AFTER_CREATE, events.AFTER_UPDATE,
|
||||||
events.AFTER_DELETE):
|
events.AFTER_DELETE, events.PRECOMMIT_CREATE,
|
||||||
|
events.PRECOMMIT_DELETE):
|
||||||
registry.subscribe(self._handler.trunk_event,
|
registry.subscribe(self._handler.trunk_event,
|
||||||
resources.TRUNK,
|
resources.TRUNK,
|
||||||
_event)
|
_event)
|
||||||
|
|
|
@ -294,6 +294,7 @@ class TrunkPlugin(service_base.ServicePluginBase):
|
||||||
trunk = self._get_trunk(context, trunk_id)
|
trunk = self._get_trunk(context, trunk_id)
|
||||||
rules.trunk_can_be_managed(context, trunk)
|
rules.trunk_can_be_managed(context, trunk)
|
||||||
trunk_port_validator = rules.TrunkPortValidator(trunk.port_id)
|
trunk_port_validator = rules.TrunkPortValidator(trunk.port_id)
|
||||||
|
parent_port = trunk.db_obj.port
|
||||||
if trunk_port_validator.can_be_trunked_or_untrunked(context):
|
if trunk_port_validator.can_be_trunked_or_untrunked(context):
|
||||||
# NOTE(status_police): when a trunk is deleted, the logical
|
# NOTE(status_police): when a trunk is deleted, the logical
|
||||||
# object disappears from the datastore, therefore there is no
|
# object disappears from the datastore, therefore there is no
|
||||||
|
@ -307,7 +308,7 @@ class TrunkPlugin(service_base.ServicePluginBase):
|
||||||
'deleting trunk port %s: %s', trunk_id,
|
'deleting trunk port %s: %s', trunk_id,
|
||||||
str(e))
|
str(e))
|
||||||
payload = events.DBEventPayload(context, resource_id=trunk_id,
|
payload = events.DBEventPayload(context, resource_id=trunk_id,
|
||||||
states=(trunk,))
|
states=(trunk, parent_port))
|
||||||
registry.publish(resources.TRUNK, events.PRECOMMIT_DELETE,
|
registry.publish(resources.TRUNK, events.PRECOMMIT_DELETE,
|
||||||
self, payload=payload)
|
self, payload=payload)
|
||||||
else:
|
else:
|
||||||
|
@ -317,7 +318,7 @@ class TrunkPlugin(service_base.ServicePluginBase):
|
||||||
registry.publish(resources.TRUNK, events.AFTER_DELETE, self,
|
registry.publish(resources.TRUNK, events.AFTER_DELETE, self,
|
||||||
payload=events.DBEventPayload(
|
payload=events.DBEventPayload(
|
||||||
context, resource_id=trunk_id,
|
context, resource_id=trunk_id,
|
||||||
states=(trunk,)))
|
states=(trunk, parent_port)))
|
||||||
|
|
||||||
@db_base_plugin_common.convert_result_to_dict
|
@db_base_plugin_common.convert_result_to_dict
|
||||||
def add_subports(self, context, trunk_id, subports):
|
def add_subports(self, context, trunk_id, subports):
|
||||||
|
|
|
@ -15,6 +15,7 @@
|
||||||
import contextlib
|
import contextlib
|
||||||
|
|
||||||
from neutron_lib.api.definitions import portbindings
|
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 import constants as n_consts
|
||||||
from neutron_lib.db import api as db_api
|
from neutron_lib.db import api as db_api
|
||||||
from neutron_lib.plugins import utils
|
from neutron_lib.plugins import utils
|
||||||
|
@ -113,6 +114,25 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
with self.trunk([subport]) as trunk:
|
with self.trunk([subport]) as trunk:
|
||||||
self._verify_trunk_info(trunk, has_items=True)
|
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):
|
def test_subport_add(self):
|
||||||
with self.subport() as subport:
|
with self.subport() as subport:
|
||||||
with self.trunk() as trunk:
|
with self.trunk() as trunk:
|
||||||
|
@ -147,3 +167,14 @@ class TestOVNTrunkDriver(base.TestOVNFunctionalBase):
|
||||||
with self.trunk() as trunk:
|
with self.trunk() as trunk:
|
||||||
self.trunk_plugin.delete_trunk(self.context, trunk['id'])
|
self.trunk_plugin.delete_trunk(self.context, trunk['id'])
|
||||||
self._verify_trunk_info({}, has_items=False)
|
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'])
|
||||||
|
|
|
@ -30,6 +30,7 @@ from neutron_lib.plugins import directory
|
||||||
from neutron_lib.plugins import utils as plugin_utils
|
from neutron_lib.plugins import utils as plugin_utils
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
|
||||||
|
from neutron.common import utils as n_utils
|
||||||
from neutron.db import agents_db
|
from neutron.db import agents_db
|
||||||
from neutron.db import l3_dvr_db
|
from neutron.db import l3_dvr_db
|
||||||
from neutron.db import l3_dvrscheduler_db
|
from neutron.db import l3_dvrscheduler_db
|
||||||
|
@ -1521,7 +1522,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
|
||||||
self.assertTrue(
|
self.assertTrue(
|
||||||
self.mixin.is_router_distributed(self.ctx, router_id))
|
self.mixin.is_router_distributed(self.ctx, router_id))
|
||||||
|
|
||||||
@mock.patch.object(l3_dvr_db, "is_port_bound")
|
@mock.patch.object(n_utils, 'is_port_bound')
|
||||||
def test_get_ports_under_dvr_connected_subnet(self, is_port_bound_mock):
|
def test_get_ports_under_dvr_connected_subnet(self, is_port_bound_mock):
|
||||||
router_dict = {'name': 'test_router', 'admin_state_up': True,
|
router_dict = {'name': 'test_router', 'admin_state_up': True,
|
||||||
'distributed': True}
|
'distributed': True}
|
||||||
|
|
|
@ -162,7 +162,8 @@ class TrunkPluginTestCase(test_plugin.Ml2PluginV2TestCase):
|
||||||
resources.TRUNK, event, self.trunk_plugin, payload=mock.ANY)
|
resources.TRUNK, event, self.trunk_plugin, payload=mock.ANY)
|
||||||
payload = callback.mock_calls[0][2]['payload']
|
payload = callback.mock_calls[0][2]['payload']
|
||||||
self.assertEqual(self.context, payload.context)
|
self.assertEqual(self.context, payload.context)
|
||||||
self.assertEqual(trunk_obj, payload.latest_state)
|
self.assertEqual(trunk_obj, payload.states[0])
|
||||||
|
self.assertEqual(parent_port['port']['id'], payload.states[1].id)
|
||||||
self.assertEqual(trunk['id'], payload.resource_id)
|
self.assertEqual(trunk['id'], payload.resource_id)
|
||||||
|
|
||||||
def test_delete_trunk_notify_after_delete(self):
|
def test_delete_trunk_notify_after_delete(self):
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
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.
|
Loading…
Reference in New Issue