Return a 404 on quota delete if entry not found

If a tenant quota is not found, the delete operation should fail
with a 404 error. It currently returns 204 even if the delete
operation actually does not delete anything which is inconsistent
with other delete operations in the Neutron API.

APIImpact

Change-Id: I1cd91b5e06bd17f9aac97bba71228f2e5c48879b
Closes-Bug: 1307506
Co-Authored-By: Salvatore Orlando <salv.orlando@gmail.com>

neutron/common/exceptions.py

@@ -334,6 +334,10 @@ class CTZoneExhaustedError(e.NeutronException):
                 "be applied.")
 
 
+class TenantQuotaNotFound(e.NotFound):
+    message = _("Quota for tenant %(tenant_id)s could not be found.")
+
+
 # Neutron-lib migration shim. This will wrap any exceptionss that are moved
 # to that library in a deprecation warning, until they can be updated to
 # import directly from their new location.

neutron/db/quota/driver.py

@@ -61,11 +61,15 @@ class DbQuotaDriver(object):
         """Delete the quota entries for a given tenant_id.
 
         After deletion, this tenant will use default quota values in conf.
+        Raise a "not found" error if the quota for the given tenant was
+        never defined.
         """
         with context.session.begin():
             tenant_quotas = context.session.query(quota_models.Quota)
             tenant_quotas = tenant_quotas.filter_by(tenant_id=tenant_id)
-            tenant_quotas.delete()
+            if not tenant_quotas.delete():
+                # No record deleted means the quota was not found
+                raise exceptions.TenantQuotaNotFound(tenant_id=tenant_id)
 
     @staticmethod
     def get_all_quotas(context, resources):

neutron/tests/tempest/api/admin/test_quotas.py

@@ -15,6 +15,7 @@
 
 import six
 from tempest.lib.common.utils import data_utils
+from tempest.lib import exceptions as lib_exc
 from tempest import test
 
 from neutron.tests.tempest.api import base
@@ -60,7 +61,7 @@ class QuotasTest(base.BaseAdminNetworkTest):
         # Change quotas for tenant
         quota_set = self.admin_client.update_quotas(tenant_id,
                                                     **new_quotas)
-        self.addCleanup(self.admin_client.reset_quotas, tenant_id)
+        self.addCleanup(self._cleanup_quotas, tenant_id)
         for key, value in six.iteritems(new_quotas):
             self.assertEqual(value, quota_set[key])
 
@@ -83,3 +84,12 @@ class QuotasTest(base.BaseAdminNetworkTest):
         non_default_quotas = self.admin_client.list_quotas()
         for q in non_default_quotas['quotas']:
             self.assertNotEqual(tenant_id, q['tenant_id'])
+
+    def _cleanup_quotas(self, project_id):
+        # Try to clean up the resources. If it fails, then
+        # assume that everything was already deleted, so
+        # it is OK to continue.
+        try:
+            self.admin_client.reset_quotas(project_id)
+        except lib_exc.NotFound:
+            pass

neutron/tests/unit/extensions/test_quotasv2.py

@@ -280,6 +280,10 @@ class QuotaExtensionDbTestCase(QuotaExtensionTestCase):
         tenant_id = 'tenant_id1'
         env = {'neutron.context': context.Context('', tenant_id + '2',
                                                   is_admin=True)}
+        # Create a quota to ensure we have something to delete
+        quotas = {'quota': {'network': 100}}
+        self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt),
+                     self.serialize(quotas), extra_environ=env)
         res = self.api.delete(_get_path('quotas', id=tenant_id, fmt=self.fmt),
                               extra_environ=env)
         self.assertEqual(204, res.status_int)
@@ -292,6 +296,14 @@ class QuotaExtensionDbTestCase(QuotaExtensionTestCase):
                               extra_environ=env, expect_errors=True)
         self.assertEqual(403, res.status_int)
 
+    def test_delete_quota_with_unknown_tenant_returns_404(self):
+        tenant_id = 'idnotexist'
+        env = {'neutron.context': context.Context('', tenant_id + '2',
+                                                  is_admin=True)}
+        res = self.api.delete(_get_path('quotas', id=tenant_id, fmt=self.fmt),
+                              extra_environ=env, expect_errors=True)
+        self.assertEqual(exc.HTTPNotFound.code, res.status_int)
+
     def test_quotas_loaded_bad_returns_404(self):
         try:
             res = self.api.get(_get_path('quotas'), expect_errors=True)

releasenotes/notes/404-for-quota-tenant-2c09c16759269b21.yaml

@@ -0,0 +1,6 @@
+---
+features:
+  - |
+    Return code for `quota delete` for a tenant whose
+    quota has not been previously defined has been
+    changed from 204 to 404.