From: Oleg Bondarev Date: Fri, 1 Nov 2013 11:20:27 +0000 (+0400) Subject: ML2 plugin should not delete ports on subnet deletion X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0d131ff0e9964cb6a65f64809270f9d597c2d5d1;p=openstack-build%2Fneutron-build.git 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 973a9fe14..16307ef9b 100644 --- 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 5334262a7..0bc625367 100644 --- 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 1025a80b5..554d9f4ec 100644 --- 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):