]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Re-submit "ML2 plugin should not delete ports on subnet deletion"
authorSylvain Afchain <sylvain.afchain@enovance.com>
Tue, 17 Dec 2013 15:39:55 +0000 (16:39 +0100)
committerSylvain Afchain <sylvain.afchain@enovance.com>
Thu, 24 Apr 2014 09:39:58 +0000 (11:39 +0200)
This patch was previously merged:

Commit: 0d131ff0e9964cb6a65f64809270f9d597c2d5d1

And reverted by this commit da00ed76e6008bd06dada0f0441ae436dd759cdf
in order to check if there was a relation with the gate failures.

Change-Id: Iacb4de8d9aa6a6cbe32c4f41fcf2657f2d09e6e2

neutron/plugins/ml2/plugin.py
neutron/tests/unit/cisco/n1kv/test_n1kv_plugin.py
neutron/tests/unit/ml2/test_ml2_plugin.py
neutron/tests/unit/test_db_plugin.py

index a72c5128abaefaab6d6f5f52fc577b58b09d783a..9b877862afbc6d5161b26ee8bb8d9f93cfe6c9d2 100644 (file)
@@ -565,10 +565,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)
@@ -576,13 +576,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
@@ -605,12 +605,21 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     break
 
             for a in allocated:
-                try:
-                    self.delete_port(context, a.port_id)
-                except Exception:
-                    with excutils.save_and_reraise_exception():
-                        LOG.exception(_("Exception auto-deleting port %s"),
-                                      a.port_id)
+                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:
+                        with excutils.save_and_reraise_exception():
+                            LOG.exception(_("Exception deleting fixed_ip from "
+                                            "port %s"), a.port_id)
+                session.delete(a)
 
         try:
             self.mechanism_manager.delete_subnet_postcommit(mech_context)
index d9dc4cf3b2c49791c5e64b58b00c7e8986fa9eaf..642c70b8c874de9224d9a680cddf096899762b5d 100644 (file)
@@ -547,7 +547,11 @@ class TestN1kvNetworks(test_plugin.TestNetworksV2,
 class TestN1kvSubnets(test_plugin.TestSubnetsV2,
                       N1kvPluginTestCase):
 
-    pass
+    def setUp(self):
+        super(TestN1kvSubnets, self).setUp()
+
+        # Create some of the database entries that we require.
+        self._make_test_policy_profile(name='dhcp_pp')
 
 
 class TestN1kvL3Test(test_l3_plugin.L3NatExtensionTestCase):
index da878b4152bf9f4f8a77a5473084bbef86c8a395..1abc600dfeae1105bd6284fa07fa3151e2ed275a 100644 (file)
@@ -97,6 +97,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):
index 0808b69943b5046586653235d4f70c5d53d10909..43dc762aacd09fe4bc644286b4199f89f4123dd6 100644 (file)
@@ -2460,6 +2460,39 @@ 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):
+        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=constants.DEVICE_OWNER_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.assertFalse(port['port']['fixed_ips'])
+
     def test_delete_subnet_port_exists_owned_by_other(self):
         with self.subnet() as subnet:
             with self.port(subnet=subnet):