bug 1057844: improve floating-ip association checks

allow multiple floating ips to be associated with the same internal port
as long as they map to different external nets (not yet supported in
Folsom) or different internal fixed IPs.  With Quantum, there is no
need to disallow either scenario.

Also improve check for a valid external network to router to internal
subnet path when a floating IP is bound.

Change-Id: Iced675e1f064172ee8a5bb6b9e37032e83af5711

diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py
index f448a68e5766ffa21739e77fd8c8b5c1752dd907..9b17b8598ffe7bfd25935bafbb568a1ada276a89 100644 (file)
--- a/quantum/db/l3_db.py
+++ b/quantum/db/l3_db.py
@@ -416,34 +416,35 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
                'fixed_ip_address': floatingip['fixed_ip_address']}
         return self._fields(res, fields)
 
-    def _get_router_for_internal_subnet(self, context, internal_port,
-                                        internal_subnet_id):
+    def _get_router_for_floatingip(self, context, internal_port,
+                                   internal_subnet_id,
+                                   external_network_id):
         subnet_db = self._get_subnet(context, internal_subnet_id)
         if not subnet_db['gateway_ip']:
             msg = ('Cannot add floating IP to port on subnet %s '
                    'which has no gateway_ip' % internal_subnet_id)
             raise q_exc.BadRequest(resource='floatingip', msg=msg)
 
-        #FIXME(danwent): can do join, but cannot use standard F-K syntax?
-        # just do it inefficiently for now
-        port_qry = context.session.query(models_v2.Port)
-        ports = port_qry.filter_by(network_id=internal_port['network_id'])
-        for p in ports:
-            ips = [ip['ip_address'] for ip in p['fixed_ips']]
-            if len(ips) != 1:
-                continue
-            fixed = p['fixed_ips'][0]
-            if (fixed['ip_address'] == subnet_db['gateway_ip'] and
-                    fixed['subnet_id'] == internal_subnet_id):
-                router_qry = context.session.query(Router)
-                try:
-                    router = router_qry.filter_by(id=p['device_id']).one()
-                    return router['id']
-                except exc.NoResultFound:
-                    pass
+        # find router interface ports on this network
+        router_intf_qry = context.session.query(models_v2.Port)
+        router_intf_ports = router_intf_qry.filter_by(
+            network_id=internal_port['network_id'],
+            device_owner=DEVICE_OWNER_ROUTER_INTF)
+
+        for intf_p in router_intf_ports:
+            if intf_p['fixed_ips'][0]['subnet_id'] == internal_subnet_id:
+                router_id = intf_p['device_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:
+                    return router_id
 
         raise l3.ExternalGatewayForFloatingIPNotFound(
             subnet_id=internal_subnet_id,
+            external_network_id=external_network_id,
             port_id=internal_port['id'])
 
     def get_assoc_data(self, context, fip, floating_network_id):
@@ -491,9 +492,10 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
             internal_ip_address = internal_port['fixed_ips'][0]['ip_address']
             internal_subnet_id = internal_port['fixed_ips'][0]['subnet_id']
 
-        router_id = self._get_router_for_internal_subnet(context,
-                                                         internal_port,
-                                                         internal_subnet_id)
+        router_id = self._get_router_for_floatingip(context,
+                                                    internal_port,
+                                                    internal_subnet_id,
+                                                    floating_network_id)
         # confirm that this router has a floating
         # ip enabled gateway with support for this floating IP network
         try:
@@ -516,17 +518,24 @@ class L3_NAT_db_mixin(l3.RouterPluginBase):
             msg = "fixed_ip_address cannot be specified without a port_id"
             raise q_exc.BadRequest(resource='floatingip', msg=msg)
         if 'port_id' in fip and fip['port_id']:
-            port_qry = context.session.query(FloatingIP)
-            try:
-                port_qry.filter_by(fixed_port_id=fip['port_id']).one()
-                raise l3.FloatingIPPortAlreadyAssociated(
-                    port_id=fip['port_id'])
-            except exc.NoResultFound:
-                pass
             port_id, internal_ip_address, router_id = self.get_assoc_data(
                 context,
                 fip,
                 floatingip_db['floating_network_id'])
+            fip_qry = context.session.query(FloatingIP)
+            try:
+                fip_qry.filter_by(
+                    fixed_port_id=fip['port_id'],
+                    floating_network_id=floatingip_db['floating_network_id'],
+                    fixed_ip_address=internal_ip_address).one()
+                raise l3.FloatingIPPortAlreadyAssociated(
+                    port_id=fip['port_id'],
+                    fip_id=floatingip_db['id'],
+                    floating_ip_address=floatingip_db['floating_ip_address'],
+                    fixed_ip=internal_ip_address,
+                    net_id=floatingip_db['floating_network_id'])
+            except exc.NoResultFound:
+                pass
         floatingip_db.update({'fixed_ip_address': internal_ip_address,
                               'fixed_port_id': port_id,
                               'router_id': router_id})

diff --git a/quantum/extensions/l3.py b/quantum/extensions/l3.py
index f7e0e35c732ae685b79374fa72823f457be2d604..0b2189cd9c7d9afc3000a1e8f1fc7acf25e0bff0 100644 (file)
--- a/quantum/extensions/l3.py
+++ b/quantum/extensions/l3.py
@@ -44,14 +44,16 @@ class FloatingIPNotFound(qexception.NotFound):
 
 
 class ExternalGatewayForFloatingIPNotFound(qexception.NotFound):
-    message = _("Could not find an external network gateway reachable "
+    message = _("External network %(external_network_id)s is not reachable "
                 "from subnet %(subnet_id)s.  Therefore, cannot associate "
                 "Port %(port_id)s with a Floating IP.")
 
 
 class FloatingIPPortAlreadyAssociated(qexception.InUse):
-    message = _("Port %(port_id)s already has a floating IP"
-                " associated with it")
+    message = _("Cannot associate floating IP %(floating_ip_address)s "
+                "(%(fip_id)s) with port %(port_id)s "
+                "using fixed IP %(fixed_ip)s, as that fixed IP already "
+                "has a floating IP on external network %(net_id)s.")
 
 
 class L3PortInUse(qexception.InUse):

diff --git a/quantum/tests/unit/test_l3_plugin.py b/quantum/tests/unit/test_l3_plugin.py
index 0b00b1b78b51fce298930d3a4b2de5cc1b2709f4..7b1417348e77debc0c9289fd4042f321e1b53b84 100644 (file)
--- a/quantum/tests/unit/test_l3_plugin.py
+++ b/quantum/tests/unit/test_l3_plugin.py
@@ -949,16 +949,13 @@ class L3NatDBTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
                 self.assertEquals(body['floatingip']['fixed_ip_address'], None)
                 self.assertEquals(body['floatingip']['router_id'], None)
 
-    def test_double_floating_assoc(self):
+    def test_two_fips_one_port_invalid_return_409(self):
         with self.floatingip_with_assoc() as fip1:
-            with self.subnet() as s:
-                with self.floatingip_no_assoc(s) as fip2:
-                    port_id = fip1['floatingip']['port_id']
-                    body = self._update('floatingips',
-                                        fip2['floatingip']['id'],
-                                        {'floatingip':
-                                         {'port_id': port_id}},
-                                        expected_code=exc.HTTPConflict.code)
+            res = self._create_floatingip(
+                'json',
+                fip1['floatingip']['floating_network_id'],
+                fip1['floatingip']['port_id'])
+            self.assertEqual(res.status_int, exc.HTTPConflict.code)
 
     def test_floating_ip_direct_port_delete_returns_409(self):
         found = False