Skip to content

Commit

Permalink
Revert "ML2 plugin should not delete ports on subnet deletion"
Browse files Browse the repository at this point in the history
This reverts commit 0d131ff

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
  • Loading branch information
salv-orlando committed Nov 29, 2013
1 parent 0d131ff commit da00ed7
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 61 deletions.
33 changes: 12 additions & 21 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 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)
session = context.session
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
Expand All @@ -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)
Expand Down
5 changes: 0 additions & 5 deletions neutron/tests/unit/ml2/test_ml2_plugin.py
Expand Up @@ -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):
Expand Down
35 changes: 0 additions & 35 deletions neutron/tests/unit/test_db_plugin.py
Expand Up @@ -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):
Expand Down

0 comments on commit da00ed7

Please sign in to comment.