From: Lars Kellogg-Stedman Date: Fri, 4 Apr 2014 17:35:32 +0000 (-0400) Subject: allow delete_port to work when there are multiple floating ips X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=bad87ce76ad27f2a4b3c72e26e2eccc2c9564b85;p=openstack-build%2Fneutron-build.git allow delete_port to work when there are multiple floating ips It is possible to associate multiple floating ip addresses with a single port through the use of multiple *fixed* ip addresses, e.g.: nova boot ... --nic net-id=my-net-id myserver nova add-fixed-ip myserver my-net-id nova add-floating-ip --fixed-address x.x.x.1 myserver y.y.y.y.1 nova add-floating-ip --fixed-address x.x.x.2 myserver y.y.y.y.2 In this situation, neutron.db.l3_db.disassociate_floatingips would throw an exception: Exception: Multiple floating IPs found for port This would prevent someone from deleting an instance that was associated with multiple floating ips. This patch corrects disassociate_floatingips so that it will disassociate all floating ips associated with the port, allowing the delete operation to proceed correctly. Change-Id: I68a2131fa8ee80828354c9db4ac405c3f77c3b90 Closes-bug: 1302701 --- diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py index 947bba0b5..167729b27 100644 --- a/neutron/db/l3_db.py +++ b/neutron/db/l3_db.py @@ -828,23 +828,21 @@ class L3_NAT_db_mixin(l3.RouterPluginBase): 'port_owner': port_db['device_owner']}) def disassociate_floatingips(self, context, port_id): + router_ids = set() + with context.session.begin(subtransactions=True): - try: - fip_qry = context.session.query(FloatingIP) - floating_ip = fip_qry.filter_by(fixed_port_id=port_id).one() - router_id = floating_ip['router_id'] + fip_qry = context.session.query(FloatingIP) + floating_ips = fip_qry.filter_by(fixed_port_id=port_id) + for floating_ip in floating_ips: + router_ids.add(floating_ip['router_id']) floating_ip.update({'fixed_port_id': None, 'fixed_ip_address': None, 'router_id': None}) - except exc.NoResultFound: - return - except exc.MultipleResultsFound: - # should never happen - raise Exception(_('Multiple floating IPs found for port %s') - % port_id) - if router_id: + + if router_ids: self.l3_rpc_notifier.routers_updated( - context, [router_id]) + context, list(router_ids), + 'disassociate_floatingips') def _build_routers_list(self, routers, gw_ports): gw_port_id_gw_port_dict = dict((gw_port['id'], gw_port) diff --git a/neutron/plugins/midonet/plugin.py b/neutron/plugins/midonet/plugin.py index 5188ba036..e47021221 100644 --- a/neutron/plugins/midonet/plugin.py +++ b/neutron/plugins/midonet/plugin.py @@ -1110,8 +1110,9 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2, """Disassociate floating IPs (if any) from this port.""" try: fip_qry = context.session.query(l3_db.FloatingIP) - fip_db = fip_qry.filter_by(fixed_port_id=port_id).one() - self._remove_nat_rules(context, fip_db) + fip_dbs = fip_qry.filter_by(fixed_port_id=port_id) + for fip_db in fip_dbs: + self._remove_nat_rules(context, fip_db) except sa_exc.NoResultFound: pass diff --git a/neutron/plugins/vmware/plugins/base.py b/neutron/plugins/vmware/plugins/base.py index 8ac138a0b..7d57e1feb 100644 --- a/neutron/plugins/vmware/plugins/base.py +++ b/neutron/plugins/vmware/plugins/base.py @@ -1980,15 +1980,17 @@ class NsxPluginV2(addr_pair_db.AllowedAddressPairsMixin, def disassociate_floatingips(self, context, port_id): try: fip_qry = context.session.query(l3_db.FloatingIP) - fip_db = fip_qry.filter_by(fixed_port_id=port_id).one() - nsx_router_id = nsx_utils.get_nsx_router_id( - context.session, self.cluster, fip_db.router_id) - self._retrieve_and_delete_nat_rules(context, - fip_db.floating_ip_address, - fip_db.fixed_ip_address, - nsx_router_id, - min_num_rules_expected=1) - self._remove_floatingip_address(context, fip_db) + fip_dbs = fip_qry.filter_by(fixed_port_id=port_id) + + for fip_db in fip_dbs: + nsx_router_id = nsx_utils.get_nsx_router_id( + context.session, self.cluster, fip_db.router_id) + self._retrieve_and_delete_nat_rules(context, + fip_db.floating_ip_address, + fip_db.fixed_ip_address, + nsx_router_id, + min_num_rules_expected=1) + self._remove_floatingip_address(context, fip_db) except sa_exc.NoResultFound: LOG.debug(_("The port '%s' is not associated with floating IPs"), port_id) diff --git a/neutron/plugins/vmware/plugins/service.py b/neutron/plugins/vmware/plugins/service.py index 24be47f6f..d9b8b0587 100644 --- a/neutron/plugins/vmware/plugins/service.py +++ b/neutron/plugins/vmware/plugins/service.py @@ -753,20 +753,25 @@ class NsxAdvancedPlugin(sr_db.ServiceRouter_mixin, self._update_nat_rules(context, router) def disassociate_floatingips(self, context, port_id): + routers = set() + try: fip_qry = context.session.query(l3_db.FloatingIP) - fip_db = fip_qry.filter_by(fixed_port_id=port_id).one() - router_id = fip_db.router_id + fip_dbs = fip_qry.filter_by(fixed_port_id=port_id) + for fip_db in fip_dbs: + routers.add(fip_db.router_id) except sa_exc.NoResultFound: - router_id = None + pass super(NsxAdvancedPlugin, self).disassociate_floatingips(context, port_id) - if router_id and self._is_advanced_service_router(context, router_id): - router = self._get_router(context, router_id) - # TODO(fank): do rollback on error, or have a dedicated thread - # do sync work (rollback, re-configure, or make router down) - self._update_interface(context, router) - self._update_nat_rules(context, router) + + for router_id in routers: + if self._is_advanced_service_router(context, router_id): + router = self._get_router(context, router_id) + # TODO(fank): do rollback on error, or have a dedicated thread + # do sync work (rollback, re-configure, or make router down) + self._update_interface(context, router) + self._update_nat_rules(context, router) # # FWaaS plugin implementation diff --git a/neutron/tests/unit/test_l3_plugin.py b/neutron/tests/unit/test_l3_plugin.py index 4c81aa807..be5b177c4 100644 --- a/neutron/tests/unit/test_l3_plugin.py +++ b/neutron/tests/unit/test_l3_plugin.py @@ -1285,6 +1285,72 @@ class L3NatTestCaseBase(L3NatTestCaseMixin): self.assertEqual(body['floatingip']['fixed_ip_address'], ip_address) + def test_floatingip_create_different_fixed_ip_same_port(self): + '''This tests that it is possible to delete a port that has + multiple floating ip addresses associated with it (each floating + address associated with a unique fixed address). + ''' + + with self.router() as r: + with self.subnet(cidr='11.0.0.0/24') as public_sub: + self._set_net_external(public_sub['subnet']['network_id']) + self._add_external_gateway_to_router( + r['router']['id'], + public_sub['subnet']['network_id']) + + with self.subnet() as private_sub: + ip_range = list(netaddr.IPNetwork( + private_sub['subnet']['cidr'])) + fixed_ips = [{'ip_address': str(ip_range[-3])}, + {'ip_address': str(ip_range[-2])}] + + self._router_interface_action( + 'add', r['router']['id'], + private_sub['subnet']['id'], None) + + with self.port(subnet=private_sub, + fixed_ips=fixed_ips) as p: + + fip1 = self._make_floatingip( + self.fmt, + public_sub['subnet']['network_id'], + p['port']['id'], + fixed_ip=str(ip_range[-2])) + fip2 = self._make_floatingip( + self.fmt, + public_sub['subnet']['network_id'], + p['port']['id'], + fixed_ip=str(ip_range[-3])) + + # Test that floating ips are assigned successfully. + body = self._show('floatingips', + fip1['floatingip']['id']) + self.assertEqual( + body['floatingip']['port_id'], + fip1['floatingip']['port_id']) + + body = self._show('floatingips', + fip2['floatingip']['id']) + self.assertEqual( + body['floatingip']['port_id'], + fip2['floatingip']['port_id']) + + # Test that port has been successfully deleted. + body = self._show('ports', p['port']['id'], + expected_code=exc.HTTPNotFound.code) + + for fip in [fip1, fip2]: + self._delete('floatingips', + fip['floatingip']['id']) + + self._router_interface_action( + 'remove', r['router']['id'], + private_sub['subnet']['id'], None) + + self._remove_external_gateway_from_router( + r['router']['id'], + public_sub['subnet']['network_id']) + def test_floatingip_update_different_fixed_ip_same_port(self): with self.subnet() as s: ip_range = list(netaddr.IPNetwork(s['subnet']['cidr']))