Merge lp:~jakedahn/nova/os-quotas into lp:~hudson-openstack/nova/trunk

[Jake Dahn]

This branch implements a nova api extension which allows you to manage and update tenant/project quotas.

[Alex Meade (alex-meade)]

Nice work, but there are a number of failed tests and pep8 errors as well as:

Add yourself to the Authors file

23, 27, 28, 29, 32 : unused imports

71: unused variable

review: Needs Fixing

[Jake Dahn (jakedahn)]

Removed unused imports, and fixed a bunch of tests.

[Devin Carlen (devcamcar)]

Looks good with some style nits:

27 +from nova.auth import manager as auth_manager
28 +from nova.api.openstack import extensions

Can you arrange these alphabetically?

36 + return {'quota_set': {
37 + 'id': str(project_id),

'id' should align with 'quota_set'

38 + 'metadata_items': quota_set['metadata_items'],
39 + 'injected_file_content_bytes':
...
46 + 'injected_files': quota_set['injected_files'],
47 + 'cores': quota_set['cores'],

Closing braces are generally on previous line.

48 + }}

57 + context = req.environ['nova.context']
58 + user = req.environ.get('user')

Why use .get('user')? What does it mean to request projects with user=None, which is what this check implies you can do?

review: Needs Fixing

[Alex Meade (alex-meade)]

Couple more pep8 errors

52: calling index gave me a key error

(nova.api.openstack): TRACE: if urlparse.parse_qs(req.environ['QUERY_STRING']).get('defaults',
(nova.api.openstack): TRACE: KeyError: 'QUERY_STRING'
(nova.api.openstack): TRACE:

Update and Show seemed to work just fine though, cool stuff.

review: Needs Fixing

[Anthony Young (sleepsonthefloor)]

Few comments:

* I think it makes sense to use the name QuotaSetsController, to match resource quota_set.
* not sure if index doesn't makes sense for this controller (with keystone). perhaps ditch the listing, and move the get defaults functionality to an action?
* Probably should explicitly check for admin-ness (except for the get defaults)

Note that this controller mixes user and admin functionality - I guess that is ok since there seems to be no precedent for separating user/admin functionality by controller. Anyone else have opinions about where user vs. admin functionality should live in general?

review: Needs Fixing

[Devin Carlen (devcamcar)]

What precedents exist today for mixing admin and non admin stuff?

[Anthony Young (sleepsonthefloor)]

I believe the servers controller now does this.

[Jesse Andrews (anotherjesse)]

agree: drop index..

& fix admin0ness

[Jake Dahn (jakedahn)]

Refactored things quite a bit based on recent comments, please re-review.

[Alex Meade (alex-meade)]

Looks good, works in my installation, just a nit:

211, 229, 246: There need to be spaces around the '=' operator

[Jake Dahn (jakedahn)]

added spaces

[Alex Meade (alex-meade)]

ah I'm sorry Jake, looks like I got it wrong.

Pep8 was actually complaining about having the return character after the '=' and there should NOT be spaces. you should change those three lines to look like this.

res = req.get_response(fakes.wsgi_app(
          fake_auth_context=self.admin_context))

or close to it.

[Jake Dahn (jakedahn)]

fixed

[Alex Meade (alex-meade)]

looks good :)

review: Approve

[Devin Carlen (devcamcar)]

lgtm!

review: Approve

[Anthony Young (sleepsonthefloor)]

hey jake - you should make sure regular users can't view other tenants quotas

review: Needs Fixing

[Jake Dahn (jakedahn)]

Things have been added from anthony's suggestions. Think this thing is finally ready to ship :)

[Ben McGraw (mcgrue)]

Looks sane, has tests. LGTM.

[Jesse Andrews (anotherjesse)]

looks good to me & grue... approved.

review: Approve

[Anthony Young (sleepsonthefloor)]

LGTM!

review: Approve

[OpenStack Infra (hudson-openstack)]

Attempt to merge into lp:nova failed due to conflicts:

text conflict in nova/tests/api/openstack/test_extensions.py

[Jake Dahn (jakedahn)]

merge fixed, jenkins was frozen for a long time yesterday and it looks like some other changes snuck in before it got merged.

[Anthony Young (sleepsonthefloor)]

still looking gtm...

review: Approve

=== added file 'nova/api/openstack/contrib/quotas.py'
--- nova/api/openstack/contrib/quotas.py	1970-01-01 00:00:00 +0000
+++ nova/api/openstack/contrib/quotas.py	2011-08-16 16:18:38 +0000
@@ -0,0 +1,100 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import webob
+
+from nova import db
+from nova import exception
+from nova import quota
+from nova.api.openstack import extensions
+
+
+class QuotaSetsController(object):
+
+    def _format_quota_set(self, project_id, quota_set):
+        """Convert the quota object to a result dict"""
+
+        return {'quota_set': {
+            'id': str(project_id),
+            'metadata_items': quota_set['metadata_items'],
+            'injected_file_content_bytes':
+             quota_set['injected_file_content_bytes'],
+            'volumes': quota_set['volumes'],
+            'gigabytes': quota_set['gigabytes'],
+            'ram': quota_set['ram'],
+            'floating_ips': quota_set['floating_ips'],
+            'instances': quota_set['instances'],
+            'injected_files': quota_set['injected_files'],
+            'cores': quota_set['cores'],
+        }}
+
+    def show(self, req, id):
+        context = req.environ['nova.context']
+        try:
+            db.sqlalchemy.api.authorize_project_context(context, id)
+            return self._format_quota_set(id,
+                                        quota.get_project_quotas(context, id))
+        except exception.NotAuthorized:
+            return webob.Response(status_int=403)
+
+    def update(self, req, id, body):
+        context = req.environ['nova.context']
+        project_id = id
+        resources = ['metadata_items', 'injected_file_content_bytes',
+                'volumes', 'gigabytes', 'ram', 'floating_ips', 'instances',
+                'injected_files', 'cores']
+        for key in body['quota_set'].keys():
+            if key in resources:
+                value = int(body['quota_set'][key])
+                try:
+                    db.quota_update(context, project_id, key, value)
+                except exception.ProjectQuotaNotFound:
+                    db.quota_create(context, project_id, key, value)
+                except exception.AdminRequired:
+                    return webob.Response(status_int=403)
+        return {'quota_set': quota.get_project_quotas(context, project_id)}
+
+    def defaults(self, req, id):
+        return self._format_quota_set(id, quota._get_default_quotas())
+
+
+class Quotas(extensions.ExtensionDescriptor):
+
+    def get_name(self):
+        return "Quotas"
+
+    def get_alias(self):
+        return "os-quota-sets"
+
+    def get_description(self):
+        return "Quotas management support"
+
+    def get_namespace(self):
+        return "http://docs.openstack.org/ext/quotas-sets/api/v1.1"
+
+    def get_updated(self):
+        return "2011-08-08T00:00:00+00:00"
+
+    def get_resources(self):
+        resources = []
+
+        res = extensions.ResourceExtension('os-quota-sets',
+                                            QuotaSetsController(),
+                                            member_actions={'defaults': 'GET'})
+        resources.append(res)
+
+        return resources

=== modified file 'nova/db/sqlalchemy/api.py'
--- nova/db/sqlalchemy/api.py	2011-08-15 20:31:43 +0000
+++ nova/db/sqlalchemy/api.py	2011-08-16 16:18:38 +0000
@@ -1959,6 +1959,7 @@
 
 @require_context
 def quota_get_all_by_project(context, project_id):
+    authorize_project_context(context, project_id)
     session = get_session()
     result = {'project_id': project_id}
     rows = session.query(models.Quota).\

=== added file 'nova/tests/api/openstack/contrib/test_quotas.py'
--- nova/tests/api/openstack/contrib/test_quotas.py	1970-01-01 00:00:00 +0000
+++ nova/tests/api/openstack/contrib/test_quotas.py	2011-08-16 16:18:38 +0000
@@ -0,0 +1,152 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2011 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import json
+import webob
+
+from nova import context
+from nova import test
+from nova.tests.api.openstack import fakes
+
+from nova.api.openstack.contrib.quotas import QuotaSetsController
+
+
+def quota_set(id):
+    return {'quota_set': {'id': id, 'metadata_items': 128, 'volumes': 10,
+            'gigabytes': 1000, 'ram': 51200, 'floating_ips': 10,
+            'instances': 10, 'injected_files': 5, 'cores': 20,
+            'injected_file_content_bytes': 10240}}
+
+
+def quota_set_list():
+    return {'quota_set_list': [quota_set('1234'), quota_set('5678'),
+                               quota_set('update_me')]}
+
+
+class QuotaSetsTest(test.TestCase):
+
+    def setUp(self):
+        super(QuotaSetsTest, self).setUp()
+        self.controller = QuotaSetsController()
+        self.user_id = 'fake'
+        self.project_id = 'fake'
+        self.user_context = context.RequestContext(self.user_id,
+                                                   self.project_id)
+        self.admin_context = context.RequestContext(self.user_id,
+                                                    self.project_id,
+                                                    is_admin=True)
+
+    def test_format_quota_set(self):
+        raw_quota_set = {
+            'instances': 10,
+            'cores': 20,
+            'ram': 51200,
+            'volumes': 10,
+            'floating_ips': 10,
+            'metadata_items': 128,
+            'gigabytes': 1000,
+            'injected_files': 5,
+            'injected_file_content_bytes': 10240}
+
+        quota_set = QuotaSetsController()._format_quota_set('1234',
+                                                            raw_quota_set)
+        qs = quota_set['quota_set']
+
+        self.assertEqual(qs['id'], '1234')
+        self.assertEqual(qs['instances'], 10)
+        self.assertEqual(qs['cores'], 20)
+        self.assertEqual(qs['ram'], 51200)
+        self.assertEqual(qs['volumes'], 10)
+        self.assertEqual(qs['gigabytes'], 1000)
+        self.assertEqual(qs['floating_ips'], 10)
+        self.assertEqual(qs['metadata_items'], 128)
+        self.assertEqual(qs['injected_files'], 5)
+        self.assertEqual(qs['injected_file_content_bytes'], 10240)
+
+    def test_quotas_defaults(self):
+        req = webob.Request.blank('/v1.1/os-quota-sets/fake_tenant/defaults')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app())
+
+        self.assertEqual(res.status_int, 200)
+        expected = {'quota_set': {
+                    'id': 'fake_tenant',
+                    'instances': 10,
+                    'cores': 20,
+                    'ram': 51200,
+                    'volumes': 10,
+                    'gigabytes': 1000,
+                    'floating_ips': 10,
+                    'metadata_items': 128,
+                    'injected_files': 5,
+                    'injected_file_content_bytes': 10240}}
+
+        self.assertEqual(json.loads(res.body), expected)
+
+    def test_quotas_show_as_admin(self):
+        req = webob.Request.blank('/v1.1/os-quota-sets/1234')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app(
+                               fake_auth_context=self.admin_context))
+
+        self.assertEqual(res.status_int, 200)
+        self.assertEqual(json.loads(res.body), quota_set('1234'))
+
+    def test_quotas_show_as_unauthorized_user(self):
+        req = webob.Request.blank('/v1.1/os-quota-sets/1234')
+        req.method = 'GET'
+        req.headers['Content-Type'] = 'application/json'
+        res = req.get_response(fakes.wsgi_app(
+                               fake_auth_context=self.user_context))
+
+        self.assertEqual(res.status_int, 403)
+
+    def test_quotas_update_as_admin(self):
+        updated_quota_set = {'quota_set': {'instances': 50,
+                             'cores': 50, 'ram': 51200, 'volumes': 10,
+                             'gigabytes': 1000, 'floating_ips': 10,
+                             'metadata_items': 128, 'injected_files': 5,
+                             'injected_file_content_bytes': 10240}}
+
+        req = webob.Request.blank('/v1.1/os-quota-sets/update_me')
+        req.method = 'PUT'
+        req.body = json.dumps(updated_quota_set)
+        req.headers['Content-Type'] = 'application/json'
+
+        res = req.get_response(fakes.wsgi_app(
+                               fake_auth_context=self.admin_context))
+
+        self.assertEqual(json.loads(res.body), updated_quota_set)
+
+    def test_quotas_update_as_user(self):
+        updated_quota_set = {'quota_set': {'instances': 50,
+                             'cores': 50, 'ram': 51200, 'volumes': 10,
+                             'gigabytes': 1000, 'floating_ips': 10,
+                             'metadata_items': 128, 'injected_files': 5,
+                             'injected_file_content_bytes': 10240}}
+
+        req = webob.Request.blank('/v1.1/os-quota-sets/update_me')
+        req.method = 'PUT'
+        req.body = json.dumps(updated_quota_set)
+        req.headers['Content-Type'] = 'application/json'
+
+        res = req.get_response(fakes.wsgi_app(
+                               fake_auth_context=self.user_context))
+
+        self.assertEqual(res.status_int, 403)

=== modified file 'nova/tests/api/openstack/test_extensions.py'
--- nova/tests/api/openstack/test_extensions.py	2011-08-16 12:54:30 +0000
+++ nova/tests/api/openstack/test_extensions.py	2011-08-16 16:18:38 +0000
@@ -91,6 +91,7 @@
             "Hosts",
             "Keypairs",
             "Multinic",
+            "Quotas",
             "SecurityGroups",
             "Volumes",
             ]
@@ -110,7 +111,7 @@
         self.assertEqual(names, self.ext_list)
 
         # Make sure that at least Fox in Sox is correct.
-        (fox_ext,) = [
+        (fox_ext, ) = [
             x for x in data['extensions'] if x['alias'] == 'FOXNSOX']
         self.assertEqual(fox_ext, {
                 'namespace': 'http://www.fox.in.socks/api/ext/pie/v1.0',
@@ -155,7 +156,7 @@
         self.assertEqual(len(exts), len(self.ext_list))
 
         # Make sure that at least Fox in Sox is correct.
-        (fox_ext,) = [x for x in exts if x.get('alias') == 'FOXNSOX']
+        (fox_ext, ) = [x for x in exts if x.get('alias') == 'FOXNSOX']
         self.assertEqual(fox_ext.get('name'), 'Fox In Socks')
         self.assertEqual(fox_ext.get('namespace'),
             'http://www.fox.in.socks/api/ext/pie/v1.0')
@@ -227,6 +228,7 @@
 
 
 class InvalidExtension(object):
+
     def get_alias(self):
         return "THIRD"