]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Disassociate floating IPs from port on terminate
authorDave Cahill <dcahill@midokura.com>
Fri, 27 Sep 2013 10:44:00 +0000 (10:44 +0000)
committerDave Cahill <dcahill@midokura.com>
Thu, 10 Oct 2013 01:37:17 +0000 (01:37 +0000)
Bugfix - floating IPs were left associated after VM
was terminated. Now call disassociate_floatingips
within delete_port as in other networking plugins.

Add L3NatDBIntTestCase suite to cover the
floating IP disassociation case, and fix all failing
tests from that suite.

Change-Id: I856c46631e495d513065fc9e987898408441a21e
Closes-Bug: #1231913
(cherry picked from commit f4b78c7f17e29448ed54b136eeb4ac700b324120)

neutron/plugins/midonet/plugin.py
neutron/tests/unit/midonet/mock_lib.py
neutron/tests/unit/midonet/test_midonet_plugin.py

index 196fa3936f7862b7521a5fc4a789fe19f17f81fa..c9d6dee4fd011b001fce55a03f2ed81125141aff 100644 (file)
@@ -23,6 +23,7 @@
 
 from midonetclient import api
 from oslo.config import cfg
+from sqlalchemy.orm import exc as sa_exc
 
 from neutron.api.v2 import attributes
 from neutron.common import constants
@@ -38,6 +39,7 @@ from neutron.db import external_net_db
 from neutron.db import l3_db
 from neutron.db import models_v2
 from neutron.db import securitygroups_db
+from neutron.extensions import l3
 from neutron.extensions import securitygroup as ext_sg
 from neutron.openstack.common import excutils
 from neutron.openstack.common import log as logging
@@ -48,6 +50,8 @@ from neutron.plugins.midonet import midonet_lib
 
 LOG = logging.getLogger(__name__)
 
+EXTERNAL_GW_INFO = l3.EXTERNAL_GW_INFO
+
 METADATA_DEFAULT_IP = "169.254.169.254/32"
 OS_FLOATING_IP_RULE_KEY = 'OS_FLOATING_IP'
 OS_SG_RULE_KEY = 'OS_SG_RULE_ID'
@@ -350,6 +354,17 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
             dl_type=net_util.get_ethertype_value(sg_rule["ethertype"]),
             properties=props)
 
+    def _remove_nat_rules(self, context, fip):
+        router = self.client.get_router(fip["router_id"])
+        self.client.remove_static_route(self._get_provider_router(),
+                                        fip["floating_ip_address"])
+
+        chain_names = _nat_chain_names(router.get_id())
+        for _type, name in chain_names.iteritems():
+            self.client.remove_rules_by_property(
+                router.get_tenant_id(), name,
+                OS_FLOATING_IP_RULE_KEY, fip["id"])
+
     def setup_rpc(self):
         # RPC support
         self.topic = topics.PLUGIN
@@ -457,6 +472,7 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         with session.begin(subtransactions=True):
             net = super(MidonetPluginV2, self).update_network(
                 context, id, network)
+            self._process_l3_update(context, net, network['network'])
             self.client.update_bridge(id, net['name'])
 
         LOG.debug(_("MidonetPluginV2.update_network exiting: net=%r"), net)
@@ -587,6 +603,7 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         if l3_port_check:
             self.prevent_l3_port_deletion(context, id)
 
+        self.disassociate_floatingips(context, id)
         port = self.get_port(context, id)
         device_id = port['device_id']
         # If this port is for router interface/gw, unlink and delete.
@@ -668,6 +685,11 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
         :param router: Router information provided to create a new router.
         """
+
+        # NOTE(dcahill): Similar to the Nicira plugin, we completely override
+        # this method in order to be able to use the MidoNet ID as Neutron ID
+        # TODO(dcahill): Propose upstream patch for allowing
+        # 3rd parties to specify IDs as we do with l2 plugin
         LOG.debug(_("MidonetPluginV2.create_router called: router=%(router)s"),
                   {"router": router})
         tenant_id = self._get_tenant_id_for_create(context, router['router'])
@@ -676,15 +698,29 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         mido_router_id = mido_router.get_id()
 
         try:
+            r = router['router']
+            has_gw_info = False
+            if EXTERNAL_GW_INFO in r:
+                has_gw_info = True
+                gw_info = r[EXTERNAL_GW_INFO]
+                del r[EXTERNAL_GW_INFO]
+            tenant_id = self._get_tenant_id_for_create(context, r)
             with context.session.begin(subtransactions=True):
+                # pre-generate id so it will be available when
+                # configuring external gw port
+                router_db = l3_db.Router(id=mido_router_id,
+                                         tenant_id=tenant_id,
+                                         name=r['name'],
+                                         admin_state_up=r['admin_state_up'],
+                                         status="ACTIVE")
+                context.session.add(router_db)
+                if has_gw_info:
+                    self._update_router_gw_info(context, router_db['id'],
+                                                gw_info)
+
+            router_data = self._make_router_dict(router_db,
+                                                 process_extensions=False)
 
-                router_data = super(MidonetPluginV2, self).create_router(
-                    context, router)
-
-                # get entry from the DB and update 'id' with MidoNet router id.
-                router_db = self._get_router(context, router_data['id'])
-                router_data['id'] = mido_router_id
-                router_db.update(router_data)
         except Exception:
             # Try removing the midonet router
             with excutils.save_and_reraise_exception():
@@ -797,7 +833,8 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
                 if (l3_db.EXTERNAL_GW_INFO in r and
                         r[l3_db.EXTERNAL_GW_INFO] is not None):
                     # Gateway created
-                    gw_port = self._get_port(context, r["gw_port_id"])
+                    gw_port = self._get_port(context.elevated(),
+                                             r["gw_port_id"])
                     gw_ip = gw_port['fixed_ips'][0]['ip_address']
 
                     # First link routers and set up the routes
@@ -1012,24 +1049,25 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
 
             # disassociate floating IP
             elif floatingip['floatingip']['port_id'] is None:
-
                 fip = super(MidonetPluginV2, self).get_floatingip(context, id)
-                router = self.client.get_router(fip["router_id"])
-                self.client.remove_static_route(self._get_provider_router(),
-                                                fip["floating_ip_address"])
-
-                chain_names = _nat_chain_names(router.get_id())
-                for _type, name in chain_names.iteritems():
-                    self.client.remove_rules_by_property(
-                        router.get_tenant_id(), name, OS_FLOATING_IP_RULE_KEY,
-                        id)
-
+                self._remove_nat_rules(context, fip)
                 super(MidonetPluginV2, self).update_floatingip(context, id,
                                                                floatingip)
 
         LOG.debug(_("MidonetPluginV2.update_floating_ip exiting: fip=%s"), fip)
         return fip
 
+    def disassociate_floatingips(self, context, port_id):
+        """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)
+        except sa_exc.NoResultFound:
+            pass
+
+        super(MidonetPluginV2, self).disassociate_floatingips(context, port_id)
+
     def create_security_group(self, context, security_group, default_sg=False):
         """Create security group.
 
index 6628b50f0c376b2403b6ccacef2cf6a5cd4ada41..dc1d5ed25f0891fe68de64c3c0cb7a2d529459af 100644 (file)
@@ -29,6 +29,8 @@ def get_bridge_mock(id=None, tenant_id='test-tenant', name='net'):
     bridge.get_id.return_value = id
     bridge.get_tenant_id.return_value = tenant_id
     bridge.get_name.return_value = name
+    bridge.get_ports.return_value = []
+    bridge.get_peer_ports.return_value = []
     return bridge
 
 
@@ -81,6 +83,9 @@ def get_router_mock(id=None, tenant_id='test-tenant', name='router'):
     router.get_id.return_value = id
     router.get_tenant_id.return_value = tenant_id
     router.get_name.return_value = name
+    router.get_ports.return_value = []
+    router.get_peer_ports.return_value = []
+    router.get_routes.return_value = []
     return router
 
 
@@ -123,6 +128,9 @@ class MidonetLibMockConfig():
     def _create_bridge(self, tenant_id, name):
         return get_bridge_mock(tenant_id=tenant_id, name=name)
 
+    def _create_router(self, tenant_id, name):
+        return get_router_mock(tenant_id=tenant_id, name=name)
+
     def _create_subnet(self, bridge, gateway_ip, subnet_prefix, subnet_len):
         return get_subnet_mock(bridge.get_id(), gateway_ip=gateway_ip,
                                subnet_prefix=subnet_prefix,
@@ -158,6 +166,7 @@ class MidonetLibMockConfig():
         self.inst.get_port.side_effect = self._get_port
 
         # Router methods side effects
+        self.inst.create_router.side_effect = self._create_router
         self.inst.get_router.side_effect = self._get_router
 
 
index a6d3f8340e15fbf8cdcdf73fdd79c4d875bf5759..708be7360cb75f9bdc9cf9fe93ea4ed8a18e9cba 100644 (file)
@@ -28,10 +28,10 @@ import neutron.common.test_lib as test_lib
 import neutron.tests.unit.midonet.mock_lib as mock_lib
 import neutron.tests.unit.test_db_plugin as test_plugin
 import neutron.tests.unit.test_extension_security_group as sg
-
+import neutron.tests.unit.test_l3_plugin as test_l3_plugin
 
 MIDOKURA_PKG_PATH = "neutron.plugins.midonet.plugin"
-
+MIDONET_PLUGIN_NAME = ('%s.MidonetPluginV2' % MIDOKURA_PKG_PATH)
 
 # Need to mock the midonetclient module since the plugin will try to load it.
 sys.modules["midonetclient"] = mock.Mock()
@@ -39,9 +39,10 @@ sys.modules["midonetclient"] = mock.Mock()
 
 class MidonetPluginV2TestCase(test_plugin.NeutronDbPluginV2TestCase):
 
-    _plugin_name = ('%s.MidonetPluginV2' % MIDOKURA_PKG_PATH)
-
-    def setUp(self):
+    def setUp(self,
+              plugin=MIDONET_PLUGIN_NAME,
+              ext_mgr=None,
+              service_plugins=None):
         self.mock_api = mock.patch(
             'neutron.plugins.midonet.midonet_lib.MidoClient')
         etc_path = os.path.join(os.path.dirname(__file__), 'etc')
@@ -51,7 +52,8 @@ class MidonetPluginV2TestCase(test_plugin.NeutronDbPluginV2TestCase):
         self.instance = self.mock_api.start()
         mock_cfg = mock_lib.MidonetLibMockConfig(self.instance.return_value)
         mock_cfg.setup()
-        super(MidonetPluginV2TestCase, self).setUp(self._plugin_name)
+        super(MidonetPluginV2TestCase, self).setUp(plugin=plugin,
+                                                   ext_mgr=ext_mgr)
 
     def tearDown(self):
         super(MidonetPluginV2TestCase, self).tearDown()
@@ -64,6 +66,20 @@ class TestMidonetNetworksV2(test_plugin.TestNetworksV2,
     pass
 
 
+class TestMidonetL3NatTestCase(test_l3_plugin.L3NatDBIntTestCase,
+                               MidonetPluginV2TestCase):
+    def setUp(self,
+              plugin=MIDONET_PLUGIN_NAME,
+              ext_mgr=None,
+              service_plugins=None):
+        super(TestMidonetL3NatTestCase, self).setUp(plugin=plugin,
+                                                    ext_mgr=None,
+                                                    service_plugins=None)
+
+    def test_floatingip_with_invalid_create_port(self):
+        self._test_floatingip_with_invalid_create_port(MIDONET_PLUGIN_NAME)
+
+
 class TestMidonetSecurityGroupsTestCase(sg.SecurityGroupDBTestCase):
 
     _plugin_name = ('%s.MidonetPluginV2' % MIDOKURA_PKG_PATH)