From 7b85f9c244cde25d2dad81f51a0c96a429ddc488 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 26 Jun 2023 10:05:06 +0000 Subject: [PATCH] [OVN][L3] Optimize FIP update operation If the floating IP updates only the QoS policy, the method now skips the OVN NAT rules update and updates only the QoS policy. That avoids the OVN NAT rules deletion and creation and the ``FIPAddDeleteEvent`` event that deletes the MAC binding entries for an active floating IP, causing a disruption. Closes-Bug: #2025144 Change-Id: Ib9ec45d643c6162c526cd5a02db270094b575e34 --- .../ovn/mech_driver/ovsdb/ovn_client.py | 26 ++++++++------ neutron/services/ovn_l3/plugin.py | 2 +- .../tests/unit/services/ovn_l3/test_plugin.py | 34 ++++++++++++++++--- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 6503e67efd9..484a9acc8c9 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -31,6 +31,7 @@ from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory from neutron_lib.plugins import utils as p_utils from neutron_lib.services.logapi import constants as log_const +from neutron_lib.services.qos import constants as qos_consts from neutron_lib.utils import helpers from neutron_lib.utils import net as n_net from oslo_config import cfg @@ -997,25 +998,30 @@ class OVNClient(object): n_context.get_admin_context(), floatingip['id'], const.FLOATINGIP_STATUS_ACTIVE) - def update_floatingip(self, context, floatingip): + def update_floatingip(self, context, floatingip, fip_request=None): fip_status = None router_id = None ovn_fip = self._nb_idl.get_floatingip(floatingip['id']) + fip_request = fip_request[l3.FLOATINGIP] if fip_request else {} + qos_update_only = (len(fip_request.keys()) == 1 and + qos_consts.QOS_POLICY_ID in fip_request) check_rev_cmd = self._nb_idl.check_revision_number( floatingip['id'], floatingip, ovn_const.TYPE_FLOATINGIPS) with self._nb_idl.transaction(check_error=True) as txn: txn.add(check_rev_cmd) - if ovn_fip: - lrouter = ovn_fip['external_ids'].get( - ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY, - utils.ovn_name(router_id)) - self._delete_floatingip(ovn_fip, lrouter, txn=txn) - fip_status = const.FLOATINGIP_STATUS_DOWN + # If FIP updates the QoS policy only, skip the OVN NAT rules update + if not qos_update_only: + if ovn_fip: + lrouter = ovn_fip['external_ids'].get( + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY, + utils.ovn_name(router_id)) + self._delete_floatingip(ovn_fip, lrouter, txn=txn) + fip_status = const.FLOATINGIP_STATUS_DOWN - if floatingip.get('port_id'): - self._create_or_update_floatingip(floatingip, txn=txn) - fip_status = const.FLOATINGIP_STATUS_ACTIVE + if floatingip.get('port_id'): + self._create_or_update_floatingip(floatingip, txn=txn) + fip_status = const.FLOATINGIP_STATUS_ACTIVE self._qos_driver.update_floatingip(txn, floatingip) diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index a708765b32b..2439c26d493 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -293,7 +293,7 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, def update_floatingip(self, context, id, floatingip): fip = super(OVNL3RouterPlugin, self).update_floatingip(context, id, floatingip) - self._ovn_client.update_floatingip(context, fip) + self._ovn_client.update_floatingip(context, fip, floatingip) return fip def update_floatingip_status(self, context, floatingip_id, status): diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index 04a65b4e202..1dbc3626896 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -16,6 +16,7 @@ import copy from unittest import mock from neutron_lib.api.definitions import external_net +from neutron_lib.api.definitions import l3 as l3_def from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net as pnet from neutron_lib.callbacks import events @@ -26,6 +27,7 @@ from neutron_lib.exceptions import availability_zone as az_exc from neutron_lib.exceptions import l3 as l3_exc from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory +from neutron_lib.services.qos import constants as qos_consts from oslo_config import cfg from oslo_utils import uuidutils @@ -34,6 +36,7 @@ from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers import driver_type as driver_type_conf from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config +from neutron.db import extraroute_db from neutron import manager as neutron_manager from neutron.plugins.ml2 import managers from neutron.services.ovn_l3 import exceptions as ovn_l3_exc @@ -1216,7 +1219,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): uf.return_value = self.fake_floating_ip_new nb_ovn.get_floatingip.return_value = ( self.fake_ovn_nat_rule) - self.l3_inst.update_floatingip(self.context, 'id', 'floatingip') + fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}} + self.l3_inst.update_floatingip(self.context, 'id', fip) nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with( 'neutron-router-id', type='dnat_and_snat', @@ -1240,13 +1244,30 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): logical_port='new-port_id', external_ids=expected_ext_ids) + @mock.patch.object(extraroute_db.ExtraRoute_dbonly_mixin, + 'update_floatingip') + def test_update_floatingip_qos(self, uf): + nb_ovn = self.l3_inst._nb_ovn + nb_ovn.is_col_present.return_value = True + uf.return_value = self.fake_floating_ip_new + nb_ovn.get_floatingip.return_value = ( + self.fake_ovn_nat_rule) + fip = {l3_def.FLOATINGIP: {qos_consts.QOS_POLICY_ID: 'qos_id_1'}} + with mock.patch.object(self.l3_inst._ovn_client._qos_driver, + 'update_floatingip') as ufip: + self.l3_inst.update_floatingip(self.context, 'id', fip) + nb_ovn.delete_nat_rule_in_lrouter.assert_not_called() + nb_ovn.add_nat_rule_in_lrouter.assert_not_called() + ufip.assert_called_once_with(mock.ANY, self.fake_floating_ip_new) + @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_floatingip') def test_update_floatingip_associate(self, uf): self.l3_inst._nb_ovn.is_col_present.return_value = True self.fake_floating_ip.update({'fixed_port_id': None}) uf.return_value = self.fake_floating_ip_new - self.l3_inst.update_floatingip(self.context, 'id', 'floatingip') + fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}} + self.l3_inst.update_floatingip(self.context, 'id', fip) self.l3_inst._nb_ovn.delete_nat_rule_in_lrouter.assert_not_called() expected_ext_ids = { ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip_new['id'], @@ -1282,7 +1303,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): config.cfg.CONF.set_override( 'enable_distributed_floating_ip', True, group='ovn') - self.l3_inst.update_floatingip(self.context, 'id', 'floatingip') + fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}} + self.l3_inst.update_floatingip(self.context, 'id', fip) self.l3_inst._nb_ovn.delete_nat_rule_in_lrouter.assert_not_called() expected_ext_ids = { ovn_const.OVN_FIP_EXT_ID_KEY: self.fake_floating_ip_new['id'], @@ -1310,7 +1332,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): self.fake_floating_ip.update({'fixed_port_id': 'foo'}) self.fake_floating_ip_new.update({'port_id': 'foo'}) uf.return_value = self.fake_floating_ip_new - self.l3_inst.update_floatingip(self.context, 'id', 'floatingip') + fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}} + self.l3_inst.update_floatingip(self.context, 'id', fip) nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with( 'neutron-router-id', type='dnat_and_snat', @@ -1345,7 +1368,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): self.fake_floating_ip_new.update({'port_id': 'port_id', 'fixed_port_id': 'port_id'}) uf.return_value = self.fake_floating_ip_new - self.l3_inst.update_floatingip(self.context, 'id', 'floatingip') + fip = {l3_def.FLOATINGIP: {'port_id': 'port1'}} + self.l3_inst.update_floatingip(self.context, 'id', fip) nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with( 'neutron-router-id',