]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Revert "ML2 plugin should not delete ports on subnet deletion"
authorSalvatore Orlando <salv.orlando@gmail.com>
Wed, 27 Nov 2013 14:09:25 +0000 (14:09 +0000)
committerSalvatore Orlando <salv.orlando@gmail.com>
Fri, 29 Nov 2013 17:22:17 +0000 (09:22 -0800)
This reverts commit 0d131ff0e9964cb6a65f64809270f9d597c2d5d1

There is really no problem with this change. However, it is probably
triggering a port_update notification to the agent for each port
with an allocated IP.
The agent handles that notification in a way which might be improved
from a scalability perspective.

I don't actually want this change to removed, I am just checking
whether neutron without it passess jobs.

Change-Id: I5494b607127b261043dcddfdc10c93a28ec20af5
Related-Bug: 1253896
Related-Bug: 1254236

neutron/plugins/ml2/plugin.py
neutron/tests/unit/ml2/test_ml2_plugin.py
neutron/tests/unit/test_db_plugin.py

index 16307ef9b9f6fd459f2a8f412565568ce18a1699..973a9fe14d342e5408d487ec6f02a82053b28005 100644 (file)
@@ -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 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
+        # 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
         # 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-deallocate
+                # Get ports to auto-delete.
                 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-deallocate: %s"), allocated)
+                LOG.debug(_("Ports to auto-delete: %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,21 +530,12 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     break
 
             for a in allocated:
-                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.delete_port(context, a.port_id)
+                except Exception:
+                    LOG.exception(_("Exception auto-deleting port %s"),
+                                  a.port_id)
+                    raise
 
         try:
             self.mechanism_manager.delete_subnet_postcommit(mech_context)
index 0bc625367c143e9af324a8f7728acead679c45ad..5334262a72b50bae516fa9cda23a22670ee2dced 100644 (file)
@@ -67,11 +67,6 @@ 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 554d9f4ec4487d37259b77af00f72662eacd1013..1025a80b52da469f00e7add98f0a1c06b02d6541 100644 (file)
@@ -2497,41 +2497,6 @@ 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):