diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 38a246f1c76..55ef12cac12 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -30,6 +30,7 @@ from neutron_lib.services import base as service_base from oslo_log import log from oslo_utils import excutils +from neutron._i18n import _ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import extensions from neutron.common.ovn import utils @@ -460,15 +461,33 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, context, router_id, add, remove, txn=txn) @staticmethod - @registry.receives(resources.PORT, [events.AFTER_UPDATE]) + @registry.receives( + resources.PORT, + [events.BEFORE_UPDATE, events.AFTER_UPDATE] + ) def _port_update(resource, event, trigger, **kwargs): l3plugin = directory.get_plugin(plugin_constants.L3) if not l3plugin: return current = kwargs['port'] + original = kwargs['original_port'] - if utils.is_lsp_router_port(current): + # The OVN NB DB has a constraint where network has to be + # greater than 0. Updating it with an empty network would + # cause a constraint violation error. This problem happens + # when the last IP of a LRP is deleted, in order to avoid it + # an exception needs to be thrown before any write is performed + # to the DB, since if not it would leave the Neutron DB and the + # OVN DB unsync. + # https://bugs.launchpad.net/neutron/+bug/1948457 + if (event == events.BEFORE_UPDATE and + 'fixed_ips' in current and not current['fixed_ips'] and + utils.is_lsp_router_port(original)): + reason = _("Router port must have at least one IP.") + raise n_exc.ServicePortInUse(port_id=original['id'], reason=reason) + + if event == events.AFTER_UPDATE and utils.is_lsp_router_port(current): # We call the update_router port with if_exists, because neutron, # internally creates the port, and then calls update, which will # trigger this callback even before we had the chance to create diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index 4e621bde011..32d4f0ef33c 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1339,12 +1339,14 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): 'ovn_client.OVNClient.update_router_port') def test_port_update_postcommit(self, update_rp_mock): kwargs = {'port': {'device_owner': 'foo'}, + 'original_port': '', 'context': 'fake_context'} self.l3_inst._port_update(resources.PORT, events.AFTER_UPDATE, None, **kwargs) update_rp_mock.assert_not_called() kwargs = {'port': {'device_owner': constants.DEVICE_OWNER_ROUTER_INTF}, + 'original_port': '', 'context': 'fake_context'} self.l3_inst._port_update(resources.PORT, events.AFTER_UPDATE, None, **kwargs) @@ -1353,6 +1355,26 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): kwargs['port'], if_exists=True) + def test_port_update_before_update_router_port_without_ip(self): + context = 'fake_context' + og_port = {'device_owner': constants.DEVICE_OWNER_ROUTER_INTF, + 'fixed_ips': ['10.0.0.10'], + 'id': 'port-id'} + port = {'device_owner': constants.DEVICE_OWNER_ROUTER_INTF, + 'fixed_ips': [], + 'id': 'port-id'} + kwargs = {'original_port': og_port, + 'port': port, + 'context': context} + self.assertRaises( + n_exc.ServicePortInUse, + self.l3_inst._port_update, + resources.PORT, + events.BEFORE_UPDATE, + None, + **kwargs, + ) + @mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.update_port_status') @mock.patch('neutron.plugins.ml2.plugin.Ml2Plugin.update_port') @mock.patch('neutron.db.db_base_plugin_v2.NeutronDbPluginV2.get_ports') diff --git a/releasenotes/notes/bug-817525-eef68687dafa97fd.yaml b/releasenotes/notes/bug-817525-eef68687dafa97fd.yaml new file mode 100644 index 00000000000..3d0c5f5b101 --- /dev/null +++ b/releasenotes/notes/bug-817525-eef68687dafa97fd.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Changes the API behaviour while using OVN driver to enforce that it's not possible to delete + all the IPs from a router port. + For more info see `bug LP#1948457 `_