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

@@ -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, "),
+        LOG.debug(_("MidoClient.delete_dhcp called: bridge=%(bridge)s, "
-                  {'bridge': bridge})
+                    "cidr=%(cidr)s"),
-        dhcp = bridge.get_dhcp_subnets()
+                  {'bridge': bridge, 'cidr': cidr})
-        if not dhcp:
+        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):

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'])
+        session = context.session
-        self.client.delete_dhcp(bridge)
+        with session.begin(subtransactions=True):
 
-        # If the network is external, clean up routes, links, ports.
+            super(MidonetPluginV2, self).delete_subnet(context, id)
-        if net['router:external']:
+            bridge = self.client.get_bridge(subnet['network_id'])
-            self._unlink_bridge_from_gw_router(bridge,
+            self.client.delete_dhcp(bridge, subnet['cidr'])
-                                               self._get_provider_router())
 
-        super(MidonetPluginV2, self).delete_subnet(context, id)
+            # If the network is external, clean up routes, links, ports
-        LOG.debug(_("MidonetPluginV2.delete_subnet exiting"))
+            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.

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()