From 2f0919f7df9a7e6a7e22c428e40f324fff88ef24 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 1 Sep 2022 12:22:43 +0200 Subject: [PATCH] Accept a port deletion with missing port binding information If the database "port" child register "ml2_port_bindings" has been manually deleted from the database, now is possible to delete the Neutron Port resource (that implies the "port" database register and all other child registers) Before this patch, the port deletion CLI command succeeded (no exception was raised) but the Port resource was not deleted. Closes-Bug: #1988323 Change-Id: I02de276d0cd8e4ae27355d4aee5f48e92634f318 --- neutron/db/l3_dvrscheduler_db.py | 5 +- neutron/plugins/ml2/plugin.py | 64 +++++++++++-------- .../functional/plugins/ml2/test_plugin.py | 39 +++++++++++ 3 files changed, 80 insertions(+), 28 deletions(-) diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index 3a85b1eb260..0c5a126e624 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -207,7 +207,10 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin): return [] admin_context = context.elevated() - port_host = deleted_port[portbindings.HOST_ID] + port_host = deleted_port.get(portbindings.HOST_ID) + if not port_host: + return [] + subnet_ids = [ip['subnet_id'] for ip in deleted_port['fixed_ips']] router_ids = self.get_dvr_routers_by_subnet_ids(admin_context, subnet_ids) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index c4ce9061b5a..eacfb722eef 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -2062,6 +2062,26 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, @utils.transaction_guard @db_api.retry_if_session_inactive() def delete_port(self, context, id, l3_port_check=True): + + def handle_distributed_port_bindings(mech_contexts): + bindings = db.get_distributed_port_bindings(context, id) + for bind in bindings: + levels = db.get_binding_level_objs(context, id, bind.host) + metadata['bind'] = bind + metadata['levels'] = levels + registry.publish(resources.PORT, + events.PRECOMMIT_DELETE, + self, + payload=events.DBEventPayload( + context, + resource_id=id, + metadata=metadata, + states=(port,))) + mech_context = driver_context.PortContext( + self, context, port, network, bind, levels) + self.mechanism_manager.delete_port_precommit(mech_context) + mech_contexts.append(mech_context) + with db_api.CONTEXT_READER.using(context): try: port_db = self._get_port(context, id) @@ -2077,8 +2097,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, with db_api.CONTEXT_WRITER.using(context): binding = p_utils.get_port_binding_by_status_and_host( - port_db.port_bindings, const.ACTIVE, - raise_if_not_found=True, port_id=id) + port_db.port_bindings, const.ACTIVE, port_id=id) network = self.get_network(context, port['network_id']) bound_mech_contexts = [] @@ -2086,39 +2105,30 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, metadata = {'network': network, 'port_db': port_db, 'bindings': binding} - if device_owner == const.DEVICE_OWNER_DVR_INTERFACE: - bindings = db.get_distributed_port_bindings(context, - id) - for bind in bindings: - levels = db.get_binding_level_objs(context, id, bind.host) - metadata['bind'] = bind + + if not binding: + LOG.warning('The port %s has no binding information, the ' + '"ml2_port_bindings" register is not present', id) + if device_owner == const.DEVICE_OWNER_DVR_INTERFACE: + handle_distributed_port_bindings(bound_mech_contexts) + else: + if device_owner == const.DEVICE_OWNER_DVR_INTERFACE: + handle_distributed_port_bindings(bound_mech_contexts) + else: + levels = db.get_binding_level_objs(context, id, + binding.host) + metadata['bind'] = None metadata['levels'] = levels - registry.publish(resources.PORT, - events.PRECOMMIT_DELETE, - self, - payload=events.DBEventPayload( + registry.publish(resources.PORT, events.PRECOMMIT_DELETE, + self, payload=events.DBEventPayload( context, resource_id=id, metadata=metadata, states=(port,))) mech_context = driver_context.PortContext( - self, context, port, network, bind, levels) + self, context, port, network, binding, levels) self.mechanism_manager.delete_port_precommit(mech_context) bound_mech_contexts.append(mech_context) - else: - levels = db.get_binding_level_objs(context, id, binding.host) - metadata['bind'] = None - metadata['levels'] = levels - registry.publish(resources.PORT, events.PRECOMMIT_DELETE, self, - payload=events.DBEventPayload( - context, - resource_id=id, - metadata=metadata, - states=(port,))) - mech_context = driver_context.PortContext( - self, context, port, network, binding, levels) - self.mechanism_manager.delete_port_precommit(mech_context) - bound_mech_contexts.append(mech_context) if l3plugin: router_ids = l3plugin.disassociate_floatingips( context, id, do_notify=False) diff --git a/neutron/tests/functional/plugins/ml2/test_plugin.py b/neutron/tests/functional/plugins/ml2/test_plugin.py index 548258fe47a..667ef651a4b 100644 --- a/neutron/tests/functional/plugins/ml2/test_plugin.py +++ b/neutron/tests/functional/plugins/ml2/test_plugin.py @@ -13,11 +13,16 @@ # License for the specific language governing permissions and limitations # under the License. +from unittest import mock + from neutron_lib.api.definitions import portbindings from neutron_lib import constants from neutron_lib import context +from neutron_lib.db import api as db_api from neutron.db import agents_db +from neutron.plugins.ml2 import models +from neutron.plugins.ml2 import plugin as ml2_plugin from neutron.tests.common import helpers from neutron.tests.unit.plugins.ml2 import base as ml2_test_base @@ -32,6 +37,12 @@ class TestMl2PortBinding(ml2_test_base.ML2TestFramework, self.admin_context = context.get_admin_context() self.host_args = {portbindings.HOST_ID: helpers.HOST, 'admin_state_up': True} + self._max_bind_retries = ml2_plugin.MAX_BIND_TRIES + ml2_plugin.MAX_BIND_TRIES = 1 + self.addCleanup(self._restore_max_bind_retries) + + def _restore_max_bind_retries(self): + ml2_plugin.MAX_BIND_TRIES = self._max_bind_retries def test_port_bind_successfully(self): helpers.register_ovs_agent(host=helpers.HOST) @@ -70,3 +81,31 @@ class TestMl2PortBinding(ml2_test_base.ML2TestFramework, portbindings.VIF_TYPE_OVS) self.assertEqual(bound_context.current['binding:vif_type'], portbindings.VIF_TYPE_OVS) + + @mock.patch.object(ml2_plugin, 'LOG') + def test_delete_port_no_binding_register(self, mock_log): + with self.network() as network: + with self.subnet(network=network) as subnet: + with self.port( + subnet=subnet, device_owner=DEVICE_OWNER_COMPUTE, + arg_list=(portbindings.HOST_ID, 'admin_state_up',), + **self.host_args) as port: + pass + + port_id = port['port']['id'] + ports = self._list('ports')['ports'] + self.assertEqual(1, len(ports)) + self.assertEqual(port_id, ports[0]['id']) + with db_api.CONTEXT_WRITER.using(self.context): + port_binding = self.context.session.query( + models.PortBinding).filter( + models.PortBinding.port_id == port_id).one() + self.context.session.delete(port_binding) + + req = self.new_delete_request('ports', port['port']['id']) + req.get_response(self.api) + ports = self._list('ports')['ports'] + self.assertEqual(0, len(ports)) + mock_log.warning.assert_called_once_with( + 'The port %s has no binding information, the "ml2_port_bindings" ' + 'register is not present', port_id)