From 28c5e648e3f9744ea7677ae1658b3148b0990f04 Mon Sep 17 00:00:00 2001
From: Rossella Sblendido <rossella@midokura.com>
Date: Mon, 30 Sep 2013 16:09:59 +0000
Subject: [PATCH] MidoNet plugin, clean up dhcp entries correctly

When a subnet gets deleted, remove the proper dhcp entry, not
always the first one.

Change-Id: I4f152eed5a7dd408bda866cb304838989bdc363c
Solves-bug: #1233259
---
 neutron/plugins/midonet/midonet_lib.py        | 18 +++++++++++------
 neutron/plugins/midonet/plugin.py             | 20 +++++++++++--------
 .../tests/unit/midonet/test_midonet_lib.py    | 12 +++++++++++
 3 files changed, 36 insertions(+), 14 deletions(-)

diff --git a/neutron/plugins/midonet/midonet_lib.py b/neutron/plugins/midonet/midonet_lib.py
index 6b96e3812f..8f40b3986d 100644
--- a/neutron/plugins/midonet/midonet_lib.py
+++ b/neutron/plugins/midonet/midonet_lib.py
@@ -187,18 +187,24 @@ class MidoClient:
         self.remove_dhcp_host(bridge, net_util.subnet_str(cidr), ip, mac)
 
     @handle_api_error
-    def delete_dhcp(self, bridge):
+    def delete_dhcp(self, bridge, cidr):
         """Delete a DHCP entry
 
         :param bridge: bridge to remove DHCP from
+        :param cidr: subnet represented as x.x.x.x/y
         """
-        LOG.debug(_("MidoClient.delete_dhcp called: bridge=%(bridge)s, "),
-                  {'bridge': bridge})
-        dhcp = bridge.get_dhcp_subnets()
-        if not dhcp:
+        LOG.debug(_("MidoClient.delete_dhcp called: bridge=%(bridge)s, "
+                    "cidr=%(cidr)s"),
+                  {'bridge': bridge, 'cidr': cidr})
+        dhcp_subnets = bridge.get_dhcp_subnets()
+        net_addr, net_len = net_util.net_addr(cidr)
+        if not dhcp_subnets:
             raise MidonetApiException(
                 msg=_("Tried to delete non-existent DHCP"))
-        dhcp[0].delete()
+        for dhcp in dhcp_subnets:
+            if dhcp.get_subnet_prefix() == net_addr:
+                dhcp.delete()
+                break
 
     @handle_api_error
     def delete_port(self, id, delete_chains=False):
diff --git a/neutron/plugins/midonet/plugin.py b/neutron/plugins/midonet/plugin.py
index cd12ec5ce7..a1790d488d 100644
--- a/neutron/plugins/midonet/plugin.py
+++ b/neutron/plugins/midonet/plugin.py
@@ -38,6 +38,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 external_net as ext_net
 from neutron.extensions import securitygroup as ext_sg
 from neutron.openstack.common import excutils
 from neutron.openstack.common import log as logging
@@ -412,16 +413,19 @@ class MidonetPluginV2(db_base_plugin_v2.NeutronDbPluginV2,
         net = super(MidonetPluginV2, self).get_network(context,
                                                        subnet['network_id'],
                                                        fields=None)
-        bridge = self.client.get_bridge(subnet['network_id'])
-        self.client.delete_dhcp(bridge)
+        session = context.session
+        with session.begin(subtransactions=True):
 
-        # If the network is external, clean up routes, links, ports.
-        if net['router:external']:
-            self._unlink_bridge_from_gw_router(bridge,
-                                               self._get_provider_router())
+            super(MidonetPluginV2, self).delete_subnet(context, id)
+            bridge = self.client.get_bridge(subnet['network_id'])
+            self.client.delete_dhcp(bridge, subnet['cidr'])
 
-        super(MidonetPluginV2, self).delete_subnet(context, id)
-        LOG.debug(_("MidonetPluginV2.delete_subnet exiting"))
+            # If the network is external, clean up routes, links, ports
+            if net[ext_net.EXTERNAL]:
+                self._unlink_bridge_from_gw_router(
+                    bridge, self._get_provider_router())
+
+            LOG.debug(_("MidonetPluginV2.delete_subnet exiting"))
 
     def create_network(self, context, network):
         """Create Neutron network.
diff --git a/neutron/tests/unit/midonet/test_midonet_lib.py b/neutron/tests/unit/midonet/test_midonet_lib.py
index 0fe95f99f1..1406cdf045 100644
--- a/neutron/tests/unit/midonet/test_midonet_lib.py
+++ b/neutron/tests/unit/midonet/test_midonet_lib.py
@@ -95,6 +95,18 @@ class MidoClientTestCase(testtools.TestCase):
 
         bridge.assert_has_call(dhcp_call)
 
+    def test_delete_dhcp(self):
+
+        bridge = mock.Mock()
+        subnet = mock.Mock()
+        subnet.get_subnet_prefix.return_value = "10.0.0.0"
+        subnets = mock.MagicMock(return_value=[subnet])
+        bridge.get_dhcp_subnets.side_effect = subnets
+        self.client.delete_dhcp(bridge, "10.0.0.0/24")
+        bridge.assert_has_calls(mock.call.get_dhcp_subnets)
+        subnet.assert_has_calls([mock.call.get_subnet_prefix(),
+                                mock.call.delete()])
+
     def test_add_dhcp_host(self):
 
         bridge = mock.Mock()