Merge "[OVN][L3] Optimize FIP update operation" into stable/zed

Author: Zuul

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
@@ -1159,25 +1160,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

@@ -285,7 +285,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
@@ -1216,7 +1219,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',
@@ -1240,13 +1244,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'],
@@ -1282,7 +1303,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'],
@@ -1310,7 +1332,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',
@@ -1345,7 +1368,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',