[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

Conflicts:
    neutron/tests/unit/services/ovn_l3/test_plugin.py

Change-Id: Ib9ec45d643c6162c526cd5a02db270094b575e34
(cherry picked from commit 7b85f9c)
(cherry picked from commit 4f2de74)

Author: ralonsoh

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -32,6 +32,7 @@
 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
@@ -1125,25 +1126,30 @@ def create_floatingip(self, context, floatingip):
                 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 floatingip.get('port_id'):
-                self._create_or_update_floatingip(floatingip, txn=txn)
-                fip_status = const.FLOATINGIP_STATUS_ACTIVE
+            # 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
 
             self._qos_driver.update_floatingip(txn, floatingip)
 

neutron/services/ovn_l3/plugin.py

@@ -278,7 +278,7 @@ def delete_floatingip(self, context, id):
     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):

neutron/tests/unit/services/ovn_l3/test_plugin.py

@@ -16,6 +16,7 @@
 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,12 +27,14 @@
 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
 
 from neutron.common.ovn import constants as ovn_const
 from neutron.common.ovn import utils
 from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config
+from neutron.db import extraroute_db
 from neutron.services.revisions import revision_plugin
 from neutron.tests.unit.api import test_extensions
 from neutron.tests.unit.extensions import test_extraroute
@@ -1210,7 +1213,8 @@ def test_update_floatingip(self, uf):
         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',
@@ -1234,13 +1238,30 @@ def test_update_floatingip(self, uf):
             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'],
@@ -1276,7 +1297,8 @@ def test_update_floatingip_associate_distributed(self, uf, gn):
 
         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'],
@@ -1304,7 +1326,8 @@ def test_update_floatingip_association_empty_update(self, uf):
         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',
@@ -1339,7 +1362,8 @@ def test_update_floatingip_reassociate_to_same_port_diff_fixed_ip(
         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',