From 86436a661b4cdb2aedc3c55ce37ba38c03f37e3f Mon Sep 17 00:00:00 2001 From: Dan Wendlandt Date: Tue, 20 Nov 2012 14:09:19 -0800 Subject: [PATCH] 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 --- quantum/db/l3_db.py | 67 ++++++++++++++++------------ quantum/extensions/l3.py | 8 ++-- quantum/tests/unit/test_l3_plugin.py | 15 +++---- 3 files changed, 49 insertions(+), 41 deletions(-) diff --git a/quantum/db/l3_db.py b/quantum/db/l3_db.py index f448a68e5..9b17b8598 100644 --- 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 f7e0e35c7..0b2189cd9 100644 --- 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 0b00b1b78..7b1417348 100644 --- 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