From 58cc80a2a883a13120eadd487b55b5c3b683b5b6 Mon Sep 17 00:00:00 2001 From: Sylvain Afchain Date: Tue, 17 Dec 2013 16:39:55 +0100 Subject: [PATCH] Re-submit "ML2 plugin should not delete ports on subnet deletion" 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 | 33 ++++++++++++------- .../tests/unit/cisco/n1kv/test_n1kv_plugin.py | 6 +++- neutron/tests/unit/ml2/test_ml2_plugin.py | 5 +++ neutron/tests/unit/test_db_plugin.py | 33 +++++++++++++++++++ 4 files changed, 64 insertions(+), 13 deletions(-) diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index a72c5128a..9b877862a 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -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) diff --git a/neutron/tests/unit/cisco/n1kv/test_n1kv_plugin.py b/neutron/tests/unit/cisco/n1kv/test_n1kv_plugin.py index d9dc4cf3b..642c70b8c 100644 --- a/neutron/tests/unit/cisco/n1kv/test_n1kv_plugin.py +++ b/neutron/tests/unit/cisco/n1kv/test_n1kv_plugin.py @@ -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): diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index da878b415..1abc600df 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -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): diff --git a/neutron/tests/unit/test_db_plugin.py b/neutron/tests/unit/test_db_plugin.py index 0808b6994..43dc762aa 100644 --- a/neutron/tests/unit/test_db_plugin.py +++ b/neutron/tests/unit/test_db_plugin.py @@ -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): -- 2.45.2