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

diff --git a/neutron/callbacks/resources.py b/neutron/callbacks/resources.py
index f7831b8efa542ad8e1d70ddf94bfd0388ae99882..d796faf49608487a45c4231fcaf23a380dc7e0ff 100644 (file)
--- 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 227cf0821509ef060018e67ca7dc1e49f3321b72..aa164b9ac1da6667f4392a0f99320ca569fa8265 100644 (file)
--- 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 29edc707d9cc38e20ca89ce36c1ebcba9ad07ea0..e28aa3fde65ae267752ed43f8164ce2565d4ae35 100644 (file)
--- 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 2f209db7723a58d27d69a9d8ce9882c69e05a5aa..bf184f1ddc4adc108a3386119fdafc63bdc9e8b5 100644 (file)
--- 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 50baba608673159bb97ca61f0f4fd31b990ebbd0..caf97a233baf80d1dba4349182d64e581199c5d9 100644 (file)
--- 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 4162478847267d39650ee8ca1d6856edae1bd3a0..1b3d7e2f57d8cfb5b5dd3588ea979ab163047cd7 100644 (file)
--- 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: