Create/Delete FIP Agent gateway port only if DVR Routers

A recent patch for creating FloatingIP Agent gateway
port without RPC dependency was trying to create
FloatingIP Agent Gateway port for both Legacy and
DVR routers.
"Change-id: Ieaa79c8bf2b1e03bc352f9252ce22286703e3715"

This patch fixes the problem by checking for the router,
before it tries to create or delete a FloatingIP Agent
Gateway Port.

Change-Id: I588b518d2fffd3943abe6d70fb0308adbe46e974
Closes-Bug: #1423422

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index 226b30e34f0c3b06cde257a8049e7d311db8f6fc..6b76d479d90b448d7f9e3b33a64838ef3fa5b500 100644 (file)
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -199,26 +199,40 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
         floatingIP disassociation happens.
         """
         fip_port = fip.get('port_id')
-        if fip_port and floatingip_db['id']:
-            vm_hostid = self.get_vm_port_hostid(
-                context, fip_port)
-            if vm_hostid:
-                # FIXME (Swami): This FIP Agent Gateway port should be
-                # created only once and there should not be a duplicate
-                # for the same host. Until we find a good solution for
-                # augmenting multiple server requests we should use the
-                # existing flow.
-                fip_agent_port = self.create_fip_agent_gw_port_if_not_exists(
-                    context.elevated(), external_port['network_id'], vm_hostid)
-                LOG.debug("FIP Agent gateway port: %s", fip_agent_port)
         unused_fip_agent_gw_port = (
             fip_port is None and floatingip_db['fixed_port_id'])
-        if unused_fip_agent_gw_port:
+        if unused_fip_agent_gw_port and floatingip_db.get('router_id'):
             admin_ctx = context.elevated()
-            self.clear_unused_fip_agent_gw_port(
-                admin_ctx, floatingip_db)
+            router_dict = self.get_router(
+                admin_ctx, floatingip_db['router_id'])
+            # Check if distributed router and then delete the
+            # FloatingIP agent gateway port
+            if router_dict.get('distributed'):
+                self.clear_unused_fip_agent_gw_port(
+                    admin_ctx, floatingip_db)
         super(L3_NAT_with_dvr_db_mixin, self)._update_fip_assoc(
             context, fip, floatingip_db, external_port)
+        associate_fip = fip_port and floatingip_db['id']
+        if associate_fip and floatingip_db.get('router_id'):
+            admin_ctx = context.elevated()
+            router_dict = self.get_router(
+                admin_ctx, floatingip_db['router_id'])
+            # Check if distributed router and then create the
+            # FloatingIP agent gateway port
+            if router_dict.get('distributed'):
+                vm_hostid = self.get_vm_port_hostid(
+                    context, fip_port)
+                if vm_hostid:
+                    # FIXME (Swami): This FIP Agent Gateway port should be
+                    # created only once and there should not be a duplicate
+                    # for the same host. Until we find a good solution for
+                    # augmenting multiple server requests we should use the
+                    # existing flow.
+                    fip_agent_port = (
+                        self.create_fip_agent_gw_port_if_not_exists(
+                            admin_ctx, external_port['network_id'],
+                            vm_hostid))
+                    LOG.debug("FIP Agent gateway port: %s", fip_agent_port)
 
     def clear_unused_fip_agent_gw_port(
             self, context, floatingip_db):

diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index 08cef8f6f585db7f454e9e55a0bc206d63be29e2..65d908935150eab64147981e6e61f45a3e7b4c48 100644 (file)
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -342,32 +342,33 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
         floatingip = {
             'id': _uuid(),
             'fixed_port_id': 1234,
+            'router_id': 'foo_router_id'
         }
+        router = {'id': 'foo_router_id', 'distributed': True}
         with contextlib.nested(
+            mock.patch.object(self.mixin,
+                              'get_router'),
             mock.patch.object(self.mixin,
                               'clear_unused_fip_agent_gw_port'),
             mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin,
                               '_update_fip_assoc'),
-                 ) as (vf, cf):
+                 ) as (grtr, vf, cf):
+            grtr.return_value = router
             self.mixin._update_fip_assoc(
                 self.ctx, fip, floatingip, mock.ANY)
             self.assertTrue(vf.called)
 
-    def test_create_floatingip_agent_gw_port(self):
-        fip = {
-            'id': _uuid(),
-            'port_id': _uuid(),
-        }
-        floatingip = {
-            'id': _uuid(),
-            'fixed_port_id': 1234,
-        }
+    def _setup_test_create_delete_floatingip(
+        self, fip, floatingip_db, router_db):
         port = {
-            'id': _uuid(),
+            'id': '1234',
             'binding:host_id': 'myhost',
             'network_id': 'external_net'
         }
+
         with contextlib.nested(
+            mock.patch.object(self.mixin,
+                              'get_router'),
             mock.patch.object(self.mixin,
                               'get_vm_port_hostid'),
             mock.patch.object(self.mixin,
@@ -376,11 +377,78 @@ class L3DvrTestCase(testlib_api.SqlTestCase):
                               'create_fip_agent_gw_port_if_not_exists'),
             mock.patch.object(l3_dvr_db.l3_db.L3_NAT_db_mixin,
                               '_update_fip_assoc'),
-                 ) as (vmp, vf, c_fip, cf):
+                 ) as (grtr, vmp, d_fip, c_fip, up_fip):
+            grtr.return_value = router_db
             vmp.return_value = 'my-host'
             self.mixin._update_fip_assoc(
-                self.ctx, fip, floatingip, port)
-            self.assertTrue(c_fip.called)
+                self.ctx, fip, floatingip_db, port)
+            return d_fip, c_fip
+
+    def test_create_floatingip_agent_gw_port_with_dvr_router(self):
+        floatingip = {
+            'id': _uuid(),
+            'router_id': 'foo_router_id'
+        }
+        router = {'id': 'foo_router_id', 'distributed': True}
+        fip = {
+            'id': _uuid(),
+            'port_id': _uuid()
+        }
+        delete_fip, create_fip = (
+            self._setup_test_create_delete_floatingip(
+                fip, floatingip, router))
+        self.assertTrue(create_fip.called)
+        self.assertFalse(delete_fip.called)
+
+    def test_create_floatingip_agent_gw_port_with_non_dvr_router(self):
+        floatingip = {
+            'id': _uuid(),
+            'router_id': 'foo_router_id'
+        }
+        router = {'id': 'foo_router_id', 'distributed': False}
+        fip = {
+            'id': _uuid(),
+            'port_id': _uuid()
+        }
+        delete_fip, create_fip = (
+            self._setup_test_create_delete_floatingip(
+                fip, floatingip, router))
+        self.assertFalse(create_fip.called)
+        self.assertFalse(delete_fip.called)
+
+    def test_delete_floatingip_agent_gw_port_with_dvr_router(self):
+        floatingip = {
+            'id': _uuid(),
+            'fixed_port_id': 1234,
+            'router_id': 'foo_router_id'
+        }
+        router = {'id': 'foo_router_id', 'distributed': True}
+        fip = {
+            'id': _uuid(),
+            'port_id': None
+        }
+        delete_fip, create_fip = (
+            self._setup_test_create_delete_floatingip(
+                fip, floatingip, router))
+        self.assertTrue(delete_fip.called)
+        self.assertFalse(create_fip.called)
+
+    def test_delete_floatingip_agent_gw_port_with_non_dvr_router(self):
+        floatingip = {
+            'id': _uuid(),
+            'fixed_port_id': 1234,
+            'router_id': 'foo_router_id'
+        }
+        router = {'id': 'foo_router_id', 'distributed': False}
+        fip = {
+            'id': _uuid(),
+            'port_id': None
+        }
+        delete_fip, create_fip = (
+            self._setup_test_create_delete_floatingip(
+                fip, floatingip, router))
+        self.assertFalse(create_fip.called)
+        self.assertFalse(delete_fip.called)
 
     def test__validate_router_migration_prevent_check_advanced_svc(self):
         router = {'name': 'foo_router', 'admin_state_up': True}