diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 412489cf67f..3ac808ba5aa 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -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) diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py index 5b4f1d99532..1f3b3eb8962 100644 --- a/neutron/db/l3_dvr_db.py +++ b/neutron/db/l3_dvr_db.py @@ -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 diff --git a/neutron/services/trunk/drivers/ovn/trunk_driver.py b/neutron/services/trunk/drivers/ovn/trunk_driver.py index d0228844ddb..f57e45a858d 100644 --- a/neutron/services/trunk/drivers/ovn/trunk_driver.py +++ b/neutron/services/trunk/drivers/ovn/trunk_driver.py @@ -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( diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index d19515777db..edb98ef5dea 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -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): 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 fa281232c42..39d5eb6cd6e 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,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']) diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py index 6767127bc0d..78171d8366f 100644 --- a/neutron/tests/unit/db/test_l3_dvr_db.py +++ b/neutron/tests/unit/db/test_l3_dvr_db.py @@ -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} 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 b036778877e..f98e16556be 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 @@ -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( diff --git a/neutron/tests/unit/services/trunk/test_plugin.py b/neutron/tests/unit/services/trunk/test_plugin.py index 271c16e66db..f2c26cdcef3 100644 --- a/neutron/tests/unit/services/trunk/test_plugin.py +++ b/neutron/tests/unit/services/trunk/test_plugin.py @@ -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): diff --git a/releasenotes/notes/ovn-trunk-check-parent-port-eeca2eceaca9d158.yaml b/releasenotes/notes/ovn-trunk-check-parent-port-eeca2eceaca9d158.yaml deleted file mode 100644 index 111dc99ca12..00000000000 --- a/releasenotes/notes/ovn-trunk-check-parent-port-eeca2eceaca9d158.yaml +++ /dev/null @@ -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.