Enhance the validation of the quotas update

Need check whether the already used and reserved exceeds the new quota before update it. DocImpact Implements a validation to validate whether already used and reserved quota exceeds the new quota when run 'nova quota-update', it will throw error if the quota exceeds. This check will be ignored if admin want to force update when run 'nova quota-update' with additional option '--force'. This validation help admin to be aware of whether the quotas are oversold when they try to update quota and also provide an option '--force' to allow admin force update the quotas. Fix bug 1160749 Change-Id: Iba3cee0f0d92cf2e6d64bc83830b0091992d1ee9
openstack · Jun 13, 2013 · d5bbfad · d5bbfad
1 parent e0142d0
commit d5bbfad
Show file tree

Hide file tree

Showing 16 changed files with 267 additions and 23 deletions.
diff --git a/doc/api_samples/all_extensions/extensions-get-resp.json b/doc/api_samples/all_extensions/extensions-get-resp.json
@@ -426,11 +426,11 @@
         },
         {
             "alias": "os-extended-quotas",
-            "description": "Adds ability for admins to delete quota",
+            "description": "Adds ability for admins to delete quota and optionally force the update Quota command.",
             "links": [],
             "name": "ExtendedQuotas",
-            "namespace": "http://docs.openstack.org/compute/ext/quota-delete/api/v1.1",
-            "updated": "2013-05-23T00:00:00+00:00"
+            "namespace": "http://docs.openstack.org/compute/ext/extended_quotas/api/v1.1",
+            "updated": "2013-06-09T00:00:00+00:00"
         },
         {
             "alias": "os-quota-sets",

diff --git a/doc/api_samples/all_extensions/extensions-get-resp.xml b/doc/api_samples/all_extensions/extensions-get-resp.xml
@@ -177,8 +177,8 @@
   <extension alias="os-quota-class-sets" updated="2012-03-12T00:00:00+00:00" namespace="http://docs.openstack.org/compute/ext/quota-classes-sets/api/v1.1" name="QuotaClasses">
     <description>Quota classes management support.</description>
   </extension>
-  <extension alias="os-extended-quotas" updated="2013-05-23T00:00:00+00:00" namespace="http://docs.openstack.org/compute/ext/quota-delete/api/v1.1" name="ExtendedQuotas">
-    <description>Adds ability for admins to delete quota.</description>
+  <extension alias="os-extended-quotas" updated="2013-06-09T00:00:00+00:00" namespace="http://docs.openstack.org/compute/ext/extended_quotas/api/v1.1" name="ExtendedQuotas">
+    <description>Adds ability for admins to delete quota and optionally force the update Quota command.</description>
   </extension>
   <extension alias="os-quota-sets" updated="2011-08-08T00:00:00+00:00" namespace="http://docs.openstack.org/compute/ext/quotas-sets/api/v1.1" name="Quotas">
     <description>Quotas management support.</description>

diff --git a/doc/api_samples/os-extended-quotas/quotas-update-post-req.json b/doc/api_samples/os-extended-quotas/quotas-update-post-req.json
@@ -0,0 +1,6 @@
+{
+    "quota_set": {
+        "force": "True",
+        "instances": 45
+    }
+}
diff --git a/doc/api_samples/os-extended-quotas/quotas-update-post-req.xml b/doc/api_samples/os-extended-quotas/quotas-update-post-req.xml
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<quota_set id="fake_tenant">
+  <force>True</force>
+  <instances>45</instances>
+</quota_set>
diff --git a/doc/api_samples/os-extended-quotas/quotas-update-post-resp.json b/doc/api_samples/os-extended-quotas/quotas-update-post-resp.json
@@ -0,0 +1,16 @@
+{
+    "quota_set": {
+        "cores": 20,
+        "fixed_ips": -1,
+        "floating_ips": 10,
+        "injected_file_content_bytes": 10240,
+        "injected_file_path_bytes": 255,
+        "injected_files": 5,
+        "instances": 45,
+        "key_pairs": 100,
+        "metadata_items": 128,
+        "ram": 51200,
+        "security_group_rules": 20,
+        "security_groups": 10
+    }
+}
diff --git a/doc/api_samples/os-extended-quotas/quotas-update-post-resp.xml b/doc/api_samples/os-extended-quotas/quotas-update-post-resp.xml
@@ -0,0 +1,15 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<quota_set>
+  <cores>20</cores>
+  <fixed_ips>-1</fixed_ips>
+  <floating_ips>10</floating_ips>
+  <injected_file_content_bytes>10240</injected_file_content_bytes>
+  <injected_file_path_bytes>255</injected_file_path_bytes>
+  <injected_files>5</injected_files>
+  <instances>45</instances>
+  <key_pairs>100</key_pairs>
+  <metadata_items>128</metadata_items>
+  <ram>51200</ram>
+  <security_group_rules>20</security_group_rules>
+  <security_groups>10</security_groups>
+</quota_set>
diff --git a/nova/api/openstack/compute/contrib/extended_quotas.py b/nova/api/openstack/compute/contrib/extended_quotas.py
@@ -17,9 +17,12 @@
 
 
 class Extended_quotas(extensions.ExtensionDescriptor):
-    """Adds ability for admins to delete quota."""
+    """Adds ability for admins to delete quota
+    and optionally force the update Quota command.
+    """
 
     name = "ExtendedQuotas"
     alias = "os-extended-quotas"
-    namespace = "http://docs.openstack.org/compute/ext/quota-delete/api/v1.1"
-    updated = "2013-05-23T00:00:00+00:00"
+    namespace = ("http://docs.openstack.org/compute/ext/extended_quotas"
+                 "/api/v1.1")
+    updated = "2013-06-09T00:00:00+00:00"
diff --git a/nova/api/openstack/compute/contrib/quotas.py b/nova/api/openstack/compute/contrib/quotas.py
@@ -24,11 +24,13 @@
 from nova import db
 from nova import exception
 from nova.openstack.common import log as logging
+from nova.openstack.common import strutils
 from nova import quota
 
 
 QUOTAS = quota.QUOTAS
 LOG = logging.getLogger(__name__)
+NON_QUOTA_KEYS = ['tenant_id', 'id', 'force']
 
 
 authorize_update = extensions.extension_authorizer('compute', 'quotas:update')
@@ -94,26 +96,71 @@ def update(self, req, id, body):
         project_id = id
 
         bad_keys = []
-        for key in body['quota_set'].keys():
+
+        # By default, we can force update the quota if the extended
+        # is not loaded
+        force_update = True
+        extended_loaded = False
+        if self.ext_mgr.is_loaded('os-extended-quotas'):
+            # force optional has been enabled, the default value of
+            # force_update need to be changed to False
+            extended_loaded = True
+            force_update = False
+
+        for key, value in body['quota_set'].items():
             if (key not in QUOTAS and
-                    key != 'tenant_id' and
-                    key != 'id'):
+                    key not in NON_QUOTA_KEYS):
                 bad_keys.append(key)
+                continue
+            if key == 'force' and extended_loaded:
+                # only check the force optional when the extended has
+                # been loaded
+                force_update = strutils.bool_from_string(value)
+            elif key not in NON_QUOTA_KEYS and value:
+                try:
+                    value = int(value)
+                except (ValueError, TypeError):
+                    msg = _("Quota '%(value)s' for %(key)s should be "
+                            "integer.") % locals()
+                    LOG.warn(msg)
+                    raise webob.exc.HTTPBadRequest(explanation=msg)
+                self._validate_quota_limit(value)
+
+        LOG.debug(_("force update quotas: %s") % force_update)
 
         if len(bad_keys) > 0:
             msg = _("Bad key(s) %s in quota_set") % ",".join(bad_keys)
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
-        for key in body['quota_set'].keys():
-            try:
-                value = int(body['quota_set'][key])
-            except (ValueError, TypeError):
-                LOG.warn(_("Quota for %s should be integer.") % key)
-                # NOTE(hzzhoushaoyu): Do not prevent valid value to be
-                # updated. If raise BadRequest, some may be updated and
-                # others may be not.
+        try:
+            project_quota = self._get_quotas(context, id, True)
+        except exception.NotAuthorized:
+            raise webob.exc.HTTPForbidden()
+
+        for key, value in body['quota_set'].items():
+            if key in NON_QUOTA_KEYS or not value:
                 continue
-            self._validate_quota_limit(value)
+            # validate whether already used and reserved exceeds the new
+            # quota, this check will be ignored if admin want to force
+            # update
+            value = int(value)
+            if force_update is not True and value >= 0:
+                quota_value = project_quota.get(key)
+                if quota_value and quota_value['limit'] >= 0:
+                    quota_used = (quota_value['in_use'] +
+                                  quota_value['reserved'])
+                    LOG.debug(_("Quota %(key)s used: %(quota_used)s, "
+                                "value: %(value)s."),
+                              {'key': key, 'quota_used': quota_used,
+                               'value': value})
+                    if quota_used > value:
+                        msg = (_("Quota value %(value)s for %(key)s are "
+                                "greater than already used and reserved "
+                                "%(quota_used)s") %
+                                {'value': value, 'key': key,
+                                 'quota_used': quota_used})
+                        raise webob.exc.HTTPBadRequest(explanation=msg)
+
             try:
                 db.quota_update(context, project_id, key, value)
             except exception.ProjectQuotaNotFound:

diff --git a/nova/tests/api/openstack/compute/contrib/test_quotas.py b/nova/tests/api/openstack/compute/contrib/test_quotas.py
@@ -1,6 +1,7 @@
 # vim: tabstop=4 shiftwidth=4 softtabstop=4
 
 # Copyright 2011 OpenStack Foundation
+# Copyright 2013 IBM Corp.
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -111,6 +112,8 @@ def test_quotas_show_as_unauthorized_user(self):
                           req, 1234)
 
     def test_quotas_update_as_admin(self):
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         body = {'quota_set': {'instances': 50, 'cores': 50,
                               'ram': 51200, 'floating_ips': 10,
                               'fixed_ips': -1, 'metadata_items': 128,
@@ -128,6 +131,8 @@ def test_quotas_update_as_admin(self):
         self.assertEqual(res_dict, body)
 
     def test_quotas_update_as_user(self):
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         body = {'quota_set': {'instances': 50, 'cores': 50,
                               'ram': 51200, 'floating_ips': 10,
                               'fixed_ips': -1, 'metadata_items': 128,
@@ -142,6 +147,8 @@ def test_quotas_update_as_user(self):
                           req, 'update_me', body)
 
     def test_quotas_update_invalid_key(self):
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         body = {'quota_set': {'instances2': -2, 'cores': -2,
                               'ram': -2, 'floating_ips': -2,
                               'metadata_items': -2, 'injected_files': -2,
@@ -153,6 +160,8 @@ def test_quotas_update_invalid_key(self):
                           req, 'update_me', body)
 
     def test_quotas_update_invalid_limit(self):
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         body = {'quota_set': {'instances': -2, 'cores': -2,
                               'ram': -2, 'floating_ips': -2, 'fixed_ips': -2,
                               'metadata_items': -2, 'injected_files': -2,
@@ -163,7 +172,7 @@ def test_quotas_update_invalid_limit(self):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           req, 'update_me', body)
 
-    def test_quotas_update_invalid_value(self):
+    def test_quotas_update_invalid_value_json_fromat_empty_string(self):
         expected_resp = {'quota_set': {
                               'instances': 50, 'cores': 50,
                               'ram': 51200, 'floating_ips': 10,
@@ -187,9 +196,22 @@ def test_quotas_update_invalid_value(self):
                               'key_pairs': 100}}
         req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
                                       use_admin_context=True)
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         res_dict = self.controller.update(req, 'update_me', body)
         self.assertEqual(res_dict, expected_resp)
 
+    def test_quotas_update_invalid_value_xml_fromat_empty_string(self):
+        expected_resp = {'quota_set': {
+                              'instances': 50, 'cores': 50,
+                              'ram': 51200, 'floating_ips': 10,
+                              'fixed_ips': -1, 'metadata_items': 128,
+                              'injected_files': 5,
+                              'injected_file_content_bytes': 10240,
+                              'injected_file_path_bytes': 255,
+                              'security_groups': 10,
+                              'security_group_rules': 20,
+                              'key_pairs': 100}}
         # when PUT XML format with empty string for quota
         body = {'quota_set': {'instances': 50, 'cores': 50,
                               'ram': {}, 'floating_ips': 10,
@@ -202,9 +224,29 @@ def test_quotas_update_invalid_value(self):
                               'key_pairs': 100}}
         req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
                                       use_admin_context=True)
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
         res_dict = self.controller.update(req, 'update_me', body)
         self.assertEqual(res_dict, expected_resp)
 
+    def test_quotas_update_invalid_value_non_int(self):
+        # when PUT non integer value
+        body = {'quota_set': {'instances': test, 'cores': 50,
+                              'ram': {}, 'floating_ips': 10,
+                              'fixed_ips': -1, 'metadata_items': 128,
+                              'injected_files': 5,
+                              'injected_file_content_bytes': 10240,
+                              'injected_file_path_bytes': 255,
+                              'security_groups': 10,
+                              'security_group_rules': 20,
+                              'key_pairs': 100}}
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+                                      use_admin_context=True)
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          req, 'update_me', body)
+
     def test_delete_quotas_when_extension_not_loaded(self):
         self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(False)
         self.mox.ReplayAll()
@@ -296,3 +338,62 @@ def test_deserializer(self):
 
         result = self.deserializer.deserialize(intext)['body']
         self.assertEqual(result, exemplar)
+
+
+fake_quotas = {'ram': {'limit': 51200,
+                       'in_use': 12800,
+                       'reserved': 12800},
+               'cores': {'limit': 20,
+                         'in_use': 10,
+                         'reserved': 5},
+               'instances': {'limit': 100,
+                             'in_use': 0,
+                             'reserved': 0}}
+
+
+def fake_get_quotas(self, context, id, usages=False):
+    if usages:
+        return fake_quotas
+    else:
+        return dict((k, v['limit']) for k, v in fake_quotas.items())
+
+
+class ExtendedQuotasTest(test.TestCase):
+
+    def setUp(self):
+        super(ExtendedQuotasTest, self).setUp()
+        self.ext_mgr = self.mox.CreateMock(extensions.ExtensionManager)
+        self.controller = quotas.QuotaSetsController(self.ext_mgr)
+
+    def test_quotas_update_exceed_in_used(self):
+
+        body = {'quota_set': {'cores': 10}}
+
+        self.stubs.Set(quotas.QuotaSetsController, '_get_quotas',
+                       fake_get_quotas)
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+                                      use_admin_context=True)
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
+
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          req, 'update_me', body)
+
+    def test_quotas_force_update_exceed_in_used(self):
+        self.stubs.Set(quotas.QuotaSetsController, '_get_quotas',
+                       fake_get_quotas)
+        req = fakes.HTTPRequest.blank('/v2/fake4/os-quota-sets/update_me',
+                                      use_admin_context=True)
+        expected = {'quota_set': {'ram': 25600, 'instances': 200, 'cores': 10}}
+        body = {'quota_set': {'ram': 25600,
+                              'instances': 200,
+                              'cores': 10,
+                              'force': 'True'}}
+        fake_quotas.get('ram')['limit'] = 25600
+        fake_quotas.get('cores')['limit'] = 10
+        fake_quotas.get('instances')['limit'] = 200
+
+        self.ext_mgr.is_loaded('os-extended-quotas').AndReturn(True)
+        self.mox.ReplayAll()
+        res_dict = self.controller.update(req, 'update_me', body)
+        self.assertEqual(res_dict, expected)
diff --git a/nova/tests/integrated/api_samples/all_extensions/extensions-get-resp.json.tpl b/nova/tests/integrated/api_samples/all_extensions/extensions-get-resp.json.tpl
@@ -437,7 +437,7 @@
             "description": "%(text)s",
             "links": [],
             "name": "ExtendedQuotas",
-            "namespace": "http://docs.openstack.org/compute/ext/quota-delete/api/v1.1",
+            "namespace": "http://docs.openstack.org/compute/ext/extended_quotas/api/v1.1",
             "updated": "%(timestamp)s"
         },
         {

diff --git a/nova/tests/integrated/api_samples/all_extensions/extensions-get-resp.xml.tpl b/nova/tests/integrated/api_samples/all_extensions/extensions-get-resp.xml.tpl
@@ -162,7 +162,7 @@
   <extension alias="os-quota-class-sets" updated="%(timestamp)s" namespace="http://docs.openstack.org/compute/ext/quota-classes-sets/api/v1.1" name="QuotaClasses">
     <description>%(text)s</description>
   </extension>
-  <extension alias="os-extended-quotas" updated="%(timestamp)s" namespace="http://docs.openstack.org/compute/ext/quota-delete/api/v1.1" name="ExtendedQuotas">
+  <extension alias="os-extended-quotas" updated="%(timestamp)s" namespace="http://docs.openstack.org/compute/ext/extended_quotas/api/v1.1" name="ExtendedQuotas">
     <description>%(text)s</description>
   </extension>
   <extension alias="os-quota-sets" updated="%(timestamp)s" namespace="http://docs.openstack.org/compute/ext/quotas-sets/api/v1.1" name="Quotas">

diff --git a/nova/tests/integrated/api_samples/os-extended-quotas/quotas-update-post-req.json.tpl b/nova/tests/integrated/api_samples/os-extended-quotas/quotas-update-post-req.json.tpl
@@ -0,0 +1,6 @@
+{
+    "quota_set": {
+        "force": "True",
+        "instances": 45
+    }
+}
diff --git a/nova/tests/integrated/api_samples/os-extended-quotas/quotas-update-post-req.xml.tpl b/nova/tests/integrated/api_samples/os-extended-quotas/quotas-update-post-req.xml.tpl
@@ -0,0 +1,5 @@
+<?xml version='1.0' encoding='UTF-8'?>
+<quota_set id="fake_tenant">
+  <force>True</force>
+  <instances>45</instances>
+</quota_set>