ML2 plugin should not delete ports on subnet deletion

On subnet deletion ports are deleted asynchronously by dhcp agent
so plugin doesn't need to delete them itself.
Instead Ml2Plugin.delete_subnet() method should call update_port()
for each allocation to remove the IP from the port and call the MechanismDrivers.
The patch also adds subnets test suite from test_db_plugin to test_ml2_plugin.

Closes-Bug: #1246737
Change-Id: I7cf0461e9a3cfec4921e2de41fb1ab3fc119fddc

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 973a9fe14d342e5408d487ec6f02a82053b28005..16307ef9b9f6fd459f2a8f412565568ce18a1699 100644 (file)
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -490,10 +490,10 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
     def delete_subnet(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_subnet()
-        # function is not used because it auto-deletes ports from the
-        # DB without invoking the derived class's delete_port(),
-        # preventing mechanism drivers from being called. This
-        # approach should be revisited when the API layer is reworked
+        # function is not used because it deallocates the subnet's addresses
+        # from ports in the DB without invoking the derived class's
+        # update_port(), preventing mechanism drivers from being called.
+        # This approach should be revisited when the API layer is reworked
         # during icehouse.
 
         LOG.debug(_("Deleting subnet %s"), id)
@@ -501,13 +501,13 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         while True:
             with session.begin(subtransactions=True):
                 subnet = self.get_subnet(context, id)
-                # Get ports to auto-delete.
+                # Get ports to auto-deallocate
                 allocated = (session.query(models_v2.IPAllocation).
                              filter_by(subnet_id=id).
                              join(models_v2.Port).
                              filter_by(network_id=subnet['network_id']).
                              with_lockmode('update').all())
-                LOG.debug(_("Ports to auto-delete: %s"), allocated)
+                LOG.debug(_("Ports to auto-deallocate: %s"), allocated)
                 only_auto_del = all(not a.port_id or
                                     a.ports.device_owner in db_base_plugin_v2.
                                     AUTO_DELETE_PORT_OWNERS
@@ -530,12 +530,21 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     break
 
             for a in allocated:
-                try:
-                    self.delete_port(context, a.port_id)
-                except Exception:
-                    LOG.exception(_("Exception auto-deleting port %s"),
-                                  a.port_id)
-                    raise
+                if a.port_id:
+                    # calling update_port() for each allocation to remove the
+                    # IP from the port and call the MechanismDrivers
+                    data = {'port':
+                            {'fixed_ips': [{'subnet_id': ip.subnet_id,
+                                            'ip_address': ip.ip_address}
+                                           for ip in a.ports.fixed_ips
+                                           if ip.subnet_id != id]}}
+                    try:
+                        self.update_port(context, a.port_id, data)
+                    except Exception:
+                        LOG.exception(_("Exception deleting fixed_ip from "
+                                        "port %s"), a.port_id)
+                        raise
+                session.delete(a)
 
         try:
             self.mechanism_manager.delete_subnet_postcommit(mech_context)

diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py
index 5334262a72b50bae516fa9cda23a22670ee2dced..0bc625367c143e9af324a8f7728acead679c45ad 100644 (file)
--- a/neutron/tests/unit/ml2/test_ml2_plugin.py
+++ b/neutron/tests/unit/ml2/test_ml2_plugin.py
@@ -67,6 +67,11 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2,
     pass
 
 
+class TestMl2SubnetsV2(test_plugin.TestSubnetsV2,
+                       Ml2PluginV2TestCase):
+    pass
+
+
 class TestMl2PortsV2(test_plugin.TestPortsV2, Ml2PluginV2TestCase):
 
     def test_update_port_status_build(self):

diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py
index 1025a80b52da469f00e7add98f0a1c06b02d6541..554d9f4ec4487d37259b77af00f72662eacd1013 100644 (file)
--- a/neutron/tests/unit/test_db_plugin.py
+++ b/neutron/tests/unit/test_db_plugin.py
@@ -2497,6 +2497,41 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase):
         res = req.get_response(self.api)
         self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code)
 
+    def test_delete_subnet_dhcp_port_associated_with_other_subnets(self):
+        # Create new network
+        res = self._create_network(fmt=self.fmt, name='net',
+                                   admin_state_up=True)
+        network = self.deserialize(self.fmt, res)
+        subnet1 = self._make_subnet(self.fmt, network, '10.0.0.1',
+                                    '10.0.0.0/24', ip_version=4)
+        subnet2 = self._make_subnet(self.fmt, network, '10.0.1.1',
+                                    '10.0.1.0/24', ip_version=4)
+        res = self._create_port(self.fmt,
+                                network['network']['id'],
+                                device_owner='network:dhcp',
+                                fixed_ips=[
+                                    {'subnet_id': subnet1['subnet']['id']},
+                                    {'subnet_id': subnet2['subnet']['id']}
+                                ])
+        port = self.deserialize(self.fmt, res)
+        expected_subnets = [subnet1['subnet']['id'], subnet2['subnet']['id']]
+        self.assertEqual(expected_subnets,
+                         [s['subnet_id'] for s in port['port']['fixed_ips']])
+        req = self.new_delete_request('subnets', subnet1['subnet']['id'])
+        res = req.get_response(self.api)
+        self.assertEqual(res.status_int, 204)
+        port = self._show('ports', port['port']['id'])
+
+        expected_subnets = [subnet2['subnet']['id']]
+        self.assertEqual(expected_subnets,
+                         [s['subnet_id'] for s in port['port']['fixed_ips']])
+        req = self.new_delete_request('subnets', subnet2['subnet']['id'])
+        res = req.get_response(self.api)
+        self.assertEqual(res.status_int, 204)
+        port = self._show('ports', port['port']['id'])
+        self.assertEqual([],
+                         [s['subnet_id'] for s in port['port']['fixed_ips']])
+
     def test_delete_subnet_port_exists_owned_by_other(self):
         with self.subnet() as subnet:
             with self.port(subnet=subnet):