select router with subnet's gateway_ip for floatingip

1) when a subnet is connected to multiple routers and
all these routers are connected to same external network,
then select the router with subnet's gateway_ip, if available,
for managing floatingip.

2) Otherwise go with default existing behavior i.e
select first router in internal subnet, that also present on external network.

For scenario 1), if the router with gateway ip not selected,
then for connections initiated by external agent towards floatingip
won't get response with floatingip as source address,
instead gw ip of router(i.e router with subnet's gateway_ip) as source.
Details about the bug at [1]

[1] https://bugs.launchpad.net/neutron/+bug/1470765/comments/4

Closes-bug: #1470765
Change-Id: If054945eab058c7138aabbb22cda15890ccb502c

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index 7984330247cd4e7b3a689ccccb8adaf3f2ba9a98..8713ea50dd3b76bb0c4edef02014dc787a3387d5 100644 (file)
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -788,14 +788,6 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                'status': floatingip['status']}
         return self._fields(res, fields)
 
-    def _get_interface_ports_for_network(self, context, network_id):
-        router_intf_qry = context.session.query(RouterPort)
-        router_intf_qry = router_intf_qry.join(models_v2.Port)
-        return router_intf_qry.filter(
-            models_v2.Port.network_id == network_id,
-            RouterPort.port_type == DEVICE_OWNER_ROUTER_INTF
-        )
-
     def _get_router_for_floatingip(self, context, internal_port,
                                    internal_subnet_id,
                                    external_network_id):
@@ -805,24 +797,29 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                      'which has no gateway_ip') % internal_subnet_id)
             raise n_exc.BadRequest(resource='floatingip', msg=msg)
 
-        router_intf_ports = self._get_interface_ports_for_network(
-            context, internal_port['network_id'])
-
-        # This joins on port_id so is not a cross-join
-        routerport_qry = router_intf_ports.join(models_v2.IPAllocation)
-        routerport_qry = routerport_qry.filter(
+        # Find routers(with router_id and interface address) that
+        # connect given internal subnet and the external network.
+        # Among them, if the router's interface address matches
+        # with subnet's gateway-ip, return that router.
+        # Otherwise return the first router.
+        gw_port = orm.aliased(models_v2.Port, name="gw_port")
+        routerport_qry = context.session.query(
+            RouterPort.router_id, models_v2.IPAllocation.ip_address).join(
+            models_v2.Port, models_v2.IPAllocation).filter(
+            models_v2.Port.network_id == internal_port['network_id'],
+            RouterPort.port_type.in_(l3_constants.ROUTER_INTERFACE_OWNERS),
             models_v2.IPAllocation.subnet_id == internal_subnet_id
-        )
+        ).join(gw_port, gw_port.device_id == RouterPort.router_id).filter(
+            gw_port.network_id == external_network_id).distinct()
 
-        for router_port in routerport_qry:
-            router_id = router_port.router.id
-            router_gw_qry = context.session.query(models_v2.Port)
-            has_gw_port = router_gw_qry.filter_by(
-                network_id=external_network_id,
-                device_id=router_id,
-                device_owner=DEVICE_OWNER_ROUTER_GW).count()
-            if has_gw_port:
+        first_router_id = None
+        for router_id, interface_ip in routerport_qry:
+            if interface_ip == subnet['gateway_ip']:
                 return router_id
+            if not first_router_id:
+                first_router_id = router_id
+        if first_router_id:
+            return first_router_id
 
         raise l3.ExternalGatewayForFloatingIPNotFound(
             subnet_id=internal_subnet_id,

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index 5470bb9d3c7a354fe490b14c4d38440c7cbfc024..c276c607ab68bd1086cce45ac356e0b8b1823c67 100644 (file)
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -30,7 +30,6 @@ from neutron.common import utils as n_utils
 from neutron.db import l3_attrs_db
 from neutron.db import l3_db
 from neutron.db import l3_dvrscheduler_db as l3_dvrsched_db
-from neutron.db import models_v2
 from neutron.extensions import l3
 from neutron.extensions import portbindings
 from neutron import manager
@@ -202,15 +201,6 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
         return super(L3_NAT_with_dvr_db_mixin,
                      self)._get_device_owner(context, router)
 
-    def _get_interface_ports_for_network(self, context, network_id):
-        router_intf_qry = context.session.query(l3_db.RouterPort)
-        router_intf_qry = router_intf_qry.join(models_v2.Port)
-
-        return router_intf_qry.filter(
-            models_v2.Port.network_id == network_id,
-            l3_db.RouterPort.port_type.in_(l3_const.ROUTER_INTERFACE_OWNERS)
-        )
-
     def _update_fip_assoc(self, context, fip, floatingip_db, external_port):
         """Override to create floating agent gw port for DVR.
 

diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py
index ac0e0ef4eb99a18af2e792eae32744e77a0a3887..3df2ac9fd9114113b88eeb07dfbd7e9c88588f51 100644 (file)
--- a/neutron/tests/unit/extensions/test_l3.py
+++ b/neutron/tests/unit/extensions/test_l3.py
@@ -2349,6 +2349,42 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                     self.assertEqual(fp2['floatingip']['router_id'],
                                      r2['router']['id'])
 
+    def test_floatingip_same_external_and_internal(self):
+        # Select router with subnet's gateway_ip for floatingip when
+        # routers connected to same subnet and external network.
+        with self.subnet(cidr="10.0.0.0/24") as exs,\
+                self.subnet(cidr="12.0.0.0/24", gateway_ip="12.0.0.50") as ins:
+            network_ex_id = exs['subnet']['network_id']
+            self._set_net_external(network_ex_id)
+
+            r2i_fixed_ips = [{'ip_address': '12.0.0.2'}]
+            with self.router() as r1,\
+                    self.router() as r2,\
+                    self.port(subnet=ins,
+                              fixed_ips=r2i_fixed_ips) as r2i_port:
+                self._add_external_gateway_to_router(
+                    r1['router']['id'],
+                    network_ex_id)
+                self._router_interface_action('add', r2['router']['id'],
+                                              None,
+                                              r2i_port['port']['id'])
+                self._router_interface_action('add', r1['router']['id'],
+                                              ins['subnet']['id'],
+                                              None)
+                self._add_external_gateway_to_router(
+                    r2['router']['id'],
+                    network_ex_id)
+
+                with self.port(subnet=ins,
+                               fixed_ips=[{'ip_address': '12.0.0.8'}]
+                               ) as private_port:
+
+                    fp = self._make_floatingip(self.fmt, network_ex_id,
+                                            private_port['port']['id'],
+                                            floating_ip='10.0.0.8')
+                    self.assertEqual(r1['router']['id'],
+                                     fp['floatingip']['router_id'])
+
     def test_floatingip_delete_router_intf_with_subnet_id_returns_409(self):
         found = False
         with self.floatingip_with_assoc():