Skip to content

Commit

Permalink
ML2 plugin should not delete ports on subnet deletion
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Oleg Bondarev committed Nov 22, 2013
1 parent 67fa30f commit 0d131ff
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 12 deletions.
33 changes: 21 additions & 12 deletions neutron/plugins/ml2/plugin.py
Expand Up @@ -490,24 +490,24 @@ 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 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)
session = context.session
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
Expand All @@ -530,12 +530,21 @@ def delete_subnet(self, context, id):
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)
Expand Down
5 changes: 5 additions & 0 deletions neutron/tests/unit/ml2/test_ml2_plugin.py
Expand Up @@ -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):
Expand Down
35 changes: 35 additions & 0 deletions neutron/tests/unit/test_db_plugin.py
Expand Up @@ -2497,6 +2497,41 @@ 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):
Expand Down

0 comments on commit 0d131ff

Please sign in to comment.