From 1947fd65d22bad286ffee987181d0c93772d1a85 Mon Sep 17 00:00:00 2001
From: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Date: Mon, 9 Sep 2019 10:58:46 +0000
Subject: [PATCH] Implement Floating IP association logic only once

Implement the Floating IP association logic only in one single place,
L3_NAT_dbonly_mixin._update_fip_assoc(). The dictionary returned will
include a new key, "association_event", with values:
- None: there is no association event. The internal port does not
  change.
- True: a new internal port is added to the FIP register. An
  association event can imply a disassociation event if the FIP register
  had an existing internal port.
- False: the previous internal port is removed and no one is added.

Change-Id: I775aee178cf56f842b3c0a375eda01577840e227
Related-Bug: #1842327
---
 neutron/db/l3_db.py                      | 25 ++++++++++++------------
 neutron/db/l3_dvr_db.py                  |  5 ++---
 neutron/tests/unit/db/test_l3_dvr_db.py  |  2 +-
 neutron/tests/unit/extensions/test_l3.py |  6 ++++--
 4 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index f69235add77..fc93c75c27d 100644
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -1239,10 +1239,13 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
         # {'port_id': null}, then the floating IP cloud also be dissociated.
         return port_id, internal_ip_address, router_id
 
-    def _update_fip_assoc(self, context, fip, floatingip_obj, external_port):
+    def _update_fip_assoc(self, context, fip, floatingip_obj):
         previous_router_id = floatingip_obj.router_id
         port_id, internal_ip_address, router_id = (
             self._check_and_get_fip_assoc(context, fip, floatingip_obj))
+        association_event = None
+        if floatingip_obj.fixed_port_id != port_id:
+            association_event = bool(port_id)
         floatingip_obj.fixed_ip_address = (
             netaddr.IPAddress(internal_ip_address)
             if internal_ip_address else None)
@@ -1260,7 +1263,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                 'floating_ip_address': floating_ip_address,
                 'floating_network_id': floatingip_obj.floating_network_id,
                 'floating_ip_id': floatingip_obj.id,
-                'context': context}
+                'context': context,
+                'association_event': association_event}
 
     def _is_ipv4_network(self, context, net_id):
         net = self._core_plugin._get_network(context, net_id)
@@ -1339,8 +1343,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                 description=fip.get('description'))
             # Update association with internal port
             # and define external IP address
-            assoc_result = self._update_fip_assoc(
-                context, fip, floatingip_obj, external_port)
+            assoc_result = self._update_fip_assoc(context, fip, floatingip_obj)
             floatingip_obj.create()
             floatingip_dict = self._make_floatingip_dict(
                 floatingip_obj, process_extensions=False)
@@ -1366,7 +1369,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                         events.AFTER_UPDATE,
                         self._update_fip_assoc,
                         **assoc_result)
-        if assoc_result['fixed_ip_address'] and assoc_result['fixed_port_id']:
+        if assoc_result['association_event']:
             LOG.info(FIP_ASSOC_MSG,
                      {'fip_id': assoc_result['floating_ip_id'],
                       'ext_ip': assoc_result['floating_ip_address'],
@@ -1402,11 +1405,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
         with context.session.begin(subtransactions=True):
             floatingip_obj = self._get_floatingip(context, id)
             old_floatingip = self._make_floatingip_dict(floatingip_obj)
-            fip_port_id = floatingip_obj.floating_port_id
             old_fixed_port_id = floatingip_obj.fixed_port_id
-            assoc_result = self._update_fip_assoc(
-                context, fip, floatingip_obj,
-                self._core_plugin.get_port(context.elevated(), fip_port_id))
+            assoc_result = self._update_fip_assoc(context, fip, floatingip_obj)
 
             floatingip_obj.update()
             floatingip_dict = self._make_floatingip_dict(floatingip_obj)
@@ -1433,10 +1433,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                         events.AFTER_UPDATE,
                         self._update_fip_assoc,
                         **assoc_result)
-        if old_fixed_port_id != assoc_result['fixed_port_id']:
-            assoc = ('associated' if assoc_result['fixed_port_id']
-                     else 'disassociated')
+        if assoc_result['association_event'] is not None:
             port_id = old_fixed_port_id or assoc_result['fixed_port_id']
+            assoc = ('associated' if assoc_result['association_event']
+                     else 'disassociated')
             LOG.info(FIP_ASSOC_MSG,
                      {'fip_id': assoc_result['floating_ip_id'],
                       'ext_ip': assoc_result['floating_ip_address'],
@@ -1625,6 +1625,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
                 'floating_ip_id': fip.id,
                 'context': context,
                 'router_ids': router_ids,
+                'association_event': False,
             }
             registry.notify(resources.FLOATING_IP, events.AFTER_UPDATE, self,
                             **assoc_result)
diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index a5f9490dfc5..1529fd80376 100644
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -391,14 +391,13 @@ class DVRResourceOperationHandler(object):
     def _create_dvr_floating_gw_port(self, resource, event, trigger, context,
                                      router_id, fixed_port_id, floating_ip_id,
                                      floating_network_id, fixed_ip_address,
-                                     **kwargs):
+                                     association_event, **kwargs):
         """Create floating agent gw port for DVR.
 
         Floating IP Agent gateway port will be created when a
         floatingIP association happens.
         """
-        associate_fip = fixed_port_id and floating_ip_id
-        if associate_fip and router_id:
+        if association_event and router_id:
             admin_ctx = context.elevated()
             router_dict = self.get_router(admin_ctx, router_id)
             # Check if distributed router and then create the
diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index 37ad3e6da85..d4975568b35 100644
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -605,7 +605,7 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
                             context=mock.Mock(), router_id=router_db['id'],
                             fixed_port_id=port['id'], floating_ip_id=fip['id'],
                             floating_network_id=fip['floating_network_id'],
-                            fixed_ip_address='1.2.3.4')
+                            fixed_ip_address='1.2.3.4', association_event=True)
             return c_fip
 
     def test_create_floatingip_agent_gw_port_with_dvr_router(self):
diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py
index 409caa483d0..aa33954b55b 100644
--- a/neutron/tests/unit/extensions/test_l3.py
+++ b/neutron/tests/unit/extensions/test_l3.py
@@ -2668,7 +2668,8 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                                            floating_network_id=fip_network_id,
                                            last_known_router_id=None,
                                            floating_ip_id=fip_id,
-                                           router_id=router_id)
+                                           router_id=router_id,
+                                           association_event=True)
 
     def test_floatingip_disassociate_notification(self):
         with self.port() as p:
@@ -2697,7 +2698,8 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                                            floating_network_id=fip_network_id,
                                            last_known_router_id=router_id,
                                            floating_ip_id=fip_id,
-                                           router_id=None)
+                                           router_id=None,
+                                           association_event=False)
 
     def test_floatingip_association_on_unowned_router(self):
         # create a router owned by one tenant and associate the FIP with a