From da00ed76e6008bd06dada0f0441ae436dd759cdf Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Wed, 27 Nov 2013 14:09:25 +0000 Subject: [PATCH] Revert "ML2 plugin should not delete ports on subnet deletion" 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 | 33 ++++++++------------- neutron/tests/unit/ml2/test_ml2_plugin.py | 5 ---- neutron/tests/unit/test_db_plugin.py | 35 ----------------------- 3 files changed, 12 insertions(+), 61 deletions(-) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 16307ef9b9f..973a9fe14d3 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -490,10 +490,10 @@ def update_subnet(self, context, id, subnet): 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 @@ def delete_subnet(self, context, id): 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 @@ def delete_subnet(self, context, id): 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) diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index 0bc625367c1..5334262a72b 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -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): diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index 554d9f4ec44..1025a80b52d 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -2497,41 +2497,6 @@ def test_delete_subnet_port_exists_owned_by_network(self): 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):