]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
allow delete_port to work when there are multiple floating ips
authorLars Kellogg-Stedman <lars@redhat.com>
Fri, 4 Apr 2014 17:35:32 +0000 (13:35 -0400)
committerLars Kellogg-Stedman <lars@redhat.com>
Fri, 18 Apr 2014 21:20:55 +0000 (17:20 -0400)
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 <port-id>

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

neutron/db/l3_db.py
neutron/plugins/midonet/plugin.py
neutron/plugins/vmware/plugins/base.py
neutron/plugins/vmware/plugins/service.py
neutron/tests/unit/test_l3_plugin.py

index 947bba0b55f65d21112df661f20d3ded4a790a07..167729b273eee742794d4ef60f5a49bdee977360 100644 (file)
@@ -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)
index 5188ba0363bcf5981139678bbe700d9c622e31bf..e47021221d8f2a43d1060c3effbda368767216e7 100644 (file)
@@ -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
 
index 8ac138a0b7d02bc229c675600b66534094a8cf85..7d57e1feb42860dd7168096da3ae181134d08dab 100644 (file)
@@ -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)
index 24be47f6f61d1678ff3e9ca371a63c896a0c0ad9..d9b8b0587685d851560b847db136c8d7a87e2cf0 100644 (file)
@@ -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
index 4c81aa80720de5532d008c474f8af4e5da6396ad..be5b177c4b3aa23dda2f1100f6bbecca80aae732 100644 (file)
@@ -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']))