From 5f28762ae2a96b8062628d0cac81f5da94c6ed34 Mon Sep 17 00:00:00 2001 From: John Schwarz Date: Thu, 16 Apr 2015 12:01:26 +0300 Subject: [PATCH] Add callback prior to deleting a subnet When using LBaaS and trying to delete a subnet, neutron has no way of knowing if the subnet is associated to some pool. As a result, the subnet is deleted but the pool remains associated to the (now nonexistent) subnet_id. This patch lays the ground-work for adding a check in LBaaS' side to prevent such cases. Related-Bug: #1413817 Change-Id: I3d5e231b67c72ffd919c92d65b57da56c63e053c --- neutron/callbacks/resources.py | 2 ++ neutron/common/exceptions.py | 8 ++++- neutron/db/db_base_plugin_v2.py | 17 ++++++++++ neutron/plugins/ml2/plugin.py | 2 ++ .../plugins/opencontrail/contrail_plugin.py | 2 ++ .../tests/unit/db/test_db_base_plugin_v2.py | 31 +++++++++++++++++++ 6 files changed, 61 insertions(+), 1 deletion(-) diff --git a/neutron/callbacks/resources.py b/neutron/callbacks/resources.py index f7831b8efa5..d796faf4960 100644 --- a/neutron/callbacks/resources.py +++ b/neutron/callbacks/resources.py @@ -16,6 +16,7 @@ ROUTER_GATEWAY = 'router_gateway' ROUTER_INTERFACE = 'router_interface' SECURITY_GROUP = 'security_group' SECURITY_GROUP_RULE = 'security_group_rule' +SUBNET = 'subnet' VALID = ( PORT, @@ -24,4 +25,5 @@ VALID = ( ROUTER_INTERFACE, SECURITY_GROUP, SECURITY_GROUP_RULE, + SUBNET, ) diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index 227cf082150..aa164b9ac1d 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -119,7 +119,13 @@ class NetworkInUse(InUse): class SubnetInUse(InUse): message = _("Unable to complete operation on subnet %(subnet_id)s. " - "One or more ports have an IP allocation from this subnet.") + "%(reason)s") + + def __init__(self, **kwargs): + if 'reason' not in kwargs: + kwargs['reason'] = _("One or more ports have an IP allocation " + "from this subnet.") + super(SubnetInUse, self).__init__(**kwargs) class PortInUse(InUse): diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 29edc707d9c..e28aa3fde65 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -25,6 +25,10 @@ from sqlalchemy import orm from sqlalchemy.orm import exc from neutron.api.v2 import attributes +from neutron.callbacks import events +from neutron.callbacks import exceptions +from neutron.callbacks import registry +from neutron.callbacks import resources from neutron.common import constants from neutron.common import exceptions as n_exc from neutron.common import ipv6_utils @@ -56,6 +60,15 @@ LOG = logging.getLogger(__name__) AUTO_DELETE_PORT_OWNERS = [constants.DEVICE_OWNER_DHCP] +def _check_subnet_not_used(context, subnet_id): + try: + kwargs = {'context': context, 'subnet_id': subnet_id} + registry.notify( + resources.SUBNET, events.BEFORE_DELETE, None, **kwargs) + except exceptions.CallbackFailure as e: + raise n_exc.SubnetInUse(subnet_id=subnet_id, reason=e) + + class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, common_db_mixin.CommonDbMixin): """V2 Neutron plugin interface implementation using SQLAlchemy models. @@ -1572,6 +1585,10 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, def delete_subnet(self, context, id): with context.session.begin(subtransactions=True): subnet = self._get_subnet(context, id) + + # Make sure the subnet isn't used by other resources + _check_subnet_not_used(context, id) + # Delete all network owned ports qry_network_ports = ( context.session.query(models_v2.IPAllocation). diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 2f209db7723..bf184f1ddc4 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -900,6 +900,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, raise os_db_exception.RetryRequest( exc.SubnetInUse(subnet_id=id)) + db_base_plugin_v2._check_subnet_not_used(context, id) + # If allocated is None, then all the IPAllocation were # correctly deleted during the previous pass. if not allocated: diff --git a/neutron/plugins/opencontrail/contrail_plugin.py b/neutron/plugins/opencontrail/contrail_plugin.py index 50baba60867..caf97a233ba 100644 --- a/neutron/plugins/opencontrail/contrail_plugin.py +++ b/neutron/plugins/opencontrail/contrail_plugin.py @@ -20,6 +20,7 @@ import requests from neutron.api.v2 import attributes as attr from neutron.common import exceptions as exc +from neutron.db import db_base_plugin_v2 from neutron.db import portbindings_base from neutron.extensions import external_net from neutron.extensions import portbindings @@ -345,6 +346,7 @@ class NeutronPluginContrailCoreV2(neutron_plugin_base_v2.NeutronPluginBaseV2, belonging to the specified tenant. """ + db_base_plugin_v2._check_subnet_not_used(context, subnet_id) self._delete_resource('subnet', context, subnet_id) def get_subnets(self, context, filters=None, fields=None): diff --git a/neutron/tests/unit/db/test_db_base_plugin_v2.py b/neutron/tests/unit/db/test_db_base_plugin_v2.py index 41624788472..1b3d7e2f57d 100644 --- a/neutron/tests/unit/db/test_db_base_plugin_v2.py +++ b/neutron/tests/unit/db/test_db_base_plugin_v2.py @@ -31,6 +31,8 @@ from neutron.api import api_common from neutron.api import extensions from neutron.api.v2 import attributes from neutron.api.v2 import router +from neutron.callbacks import exceptions +from neutron.callbacks import registry from neutron.common import constants from neutron.common import exceptions as n_exc from neutron.common import ipv6_utils @@ -4609,6 +4611,35 @@ class TestSubnetsV2(NeutronDbPluginV2TestCase): res = req.get_response(self.api) self.assertEqual(res.status_int, webob.exc.HTTPNoContent.code) + def test_delete_subnet_with_callback(self): + with contextlib.nested( + self.subnet(), + mock.patch.object(registry, 'notify')) as (subnet, notify): + + errors = [ + exceptions.NotificationError( + 'fake_id', n_exc.NeutronException()), + ] + notify.side_effect = [ + exceptions.CallbackFailure(errors=errors), None + ] + + # Make sure the delete request fails + delete_request = self.new_delete_request('subnets', + subnet['subnet']['id']) + delete_response = delete_request.get_response(self.api) + + self.assertTrue('NeutronError' in delete_response.json) + self.assertEqual('SubnetInUse', + delete_response.json['NeutronError']['type']) + + # Make sure the subnet wasn't deleted + list_request = self.new_list_request( + 'subnets', params="id=%s" % subnet['subnet']['id']) + list_response = list_request.get_response(self.api) + self.assertEqual(subnet['subnet']['id'], + list_response.json['subnets'][0]['id']) + def _helper_test_validate_subnet(self, option, exception): cfg.CONF.set_override(option, 0) with self.network() as network: