Merge lp:~jakedahn/nova/os-quotas into lp:~hudson-openstack/nova/trunk
- os-quotas
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Jesse Andrews |
Approved revision: | 1456 |
Merged at revision: | 1444 |
Proposed branch: | lp:~jakedahn/nova/os-quotas |
Merge into: | lp:~hudson-openstack/nova/trunk |
Diff against target: |
310 lines (+257/-2) 4 files modified
nova/api/openstack/contrib/quotas.py (+100/-0) nova/db/sqlalchemy/api.py (+1/-0) nova/tests/api/openstack/contrib/test_quotas.py (+152/-0) nova/tests/api/openstack/test_extensions.py (+4/-2) |
To merge this branch: | bzr merge lp:~jakedahn/nova/os-quotas |
Related bugs: | |
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Anthony Young (community) | Approve | ||
Jesse Andrews (community) | Approve | ||
Devin Carlen (community) | Approve | ||
Alex Meade (community) | Approve | ||
Review via email: mp+70812@code.launchpad.net |
Commit message
Description of the change
This branch implements a nova api extension which allows you to manage and update tenant/project quotas.
Jake Dahn (jakedahn) wrote : | # |
Removed unused imports, and fixed a bunch of tests.
Devin Carlen (devcamcar) wrote : | # |
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[
39 + 'injected_
...
46 + 'injected_files': quota_set[
47 + 'cores': quota_set['cores'],
Closing braces are generally on previous line.
48 + }}
57 + context = req.environ[
58 + user = req.environ.
Why use .get('user')? What does it mean to request projects with user=None, which is what this check implies you can do?
Alex Meade (alex-meade) wrote : | # |
Couple more pep8 errors
52: calling index gave me a key error
(nova.api.
(nova.api.
(nova.api.
Update and Show seemed to work just fine though, cool stuff.
Anthony Young (sleepsonthefloor) wrote : | # |
Few comments:
* I think it makes sense to use the name QuotaSetsContro
* 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?
Devin Carlen (devcamcar) wrote : | # |
What precedents exist today for mixing admin and non admin stuff?
Anthony Young (sleepsonthefloor) wrote : | # |
I believe the servers controller now does this.
Jesse Andrews (anotherjesse) wrote : | # |
agree: drop index..
& fix admin0ness
Jake Dahn (jakedahn) wrote : | # |
Refactored things quite a bit based on recent comments, please re-review.
Alex Meade (alex-meade) wrote : | # |
Looks good, works in my installation, just a nit:
211, 229, 246: There need to be spaces around the '=' operator
Jake Dahn (jakedahn) wrote : | # |
added spaces
Alex Meade (alex-meade) wrote : | # |
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_
or close to it.
Jake Dahn (jakedahn) wrote : | # |
fixed
Anthony Young (sleepsonthefloor) wrote : | # |
hey jake - you should make sure regular users can't view other tenants quotas
Jake Dahn (jakedahn) wrote : | # |
Things have been added from anthony's suggestions. Think this thing is finally ready to ship :)
Ben McGraw (mcgrue) wrote : | # |
Looks sane, has tests. LGTM.
Jesse Andrews (anotherjesse) wrote : | # |
looks good to me & grue... approved.
OpenStack Infra (hudson-openstack) wrote : | # |
Attempt to merge into lp:nova failed due to conflicts:
text conflict in nova/tests/
Jake Dahn (jakedahn) wrote : | # |
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) wrote : | # |
still looking gtm...
Preview Diff
1 | === added file 'nova/api/openstack/contrib/quotas.py' |
2 | --- nova/api/openstack/contrib/quotas.py 1970-01-01 00:00:00 +0000 |
3 | +++ nova/api/openstack/contrib/quotas.py 2011-08-16 16:18:38 +0000 |
4 | @@ -0,0 +1,100 @@ |
5 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
6 | + |
7 | +# Copyright 2011 OpenStack LLC. |
8 | +# All Rights Reserved. |
9 | +# |
10 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
11 | +# not use this file except in compliance with the License. You may obtain |
12 | +# a copy of the License at |
13 | +# |
14 | +# http://www.apache.org/licenses/LICENSE-2.0 |
15 | +# |
16 | +# Unless required by applicable law or agreed to in writing, software |
17 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
18 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
19 | +# License for the specific language governing permissions and limitations |
20 | +# under the License. |
21 | + |
22 | +import webob |
23 | + |
24 | +from nova import db |
25 | +from nova import exception |
26 | +from nova import quota |
27 | +from nova.api.openstack import extensions |
28 | + |
29 | + |
30 | +class QuotaSetsController(object): |
31 | + |
32 | + def _format_quota_set(self, project_id, quota_set): |
33 | + """Convert the quota object to a result dict""" |
34 | + |
35 | + return {'quota_set': { |
36 | + 'id': str(project_id), |
37 | + 'metadata_items': quota_set['metadata_items'], |
38 | + 'injected_file_content_bytes': |
39 | + quota_set['injected_file_content_bytes'], |
40 | + 'volumes': quota_set['volumes'], |
41 | + 'gigabytes': quota_set['gigabytes'], |
42 | + 'ram': quota_set['ram'], |
43 | + 'floating_ips': quota_set['floating_ips'], |
44 | + 'instances': quota_set['instances'], |
45 | + 'injected_files': quota_set['injected_files'], |
46 | + 'cores': quota_set['cores'], |
47 | + }} |
48 | + |
49 | + def show(self, req, id): |
50 | + context = req.environ['nova.context'] |
51 | + try: |
52 | + db.sqlalchemy.api.authorize_project_context(context, id) |
53 | + return self._format_quota_set(id, |
54 | + quota.get_project_quotas(context, id)) |
55 | + except exception.NotAuthorized: |
56 | + return webob.Response(status_int=403) |
57 | + |
58 | + def update(self, req, id, body): |
59 | + context = req.environ['nova.context'] |
60 | + project_id = id |
61 | + resources = ['metadata_items', 'injected_file_content_bytes', |
62 | + 'volumes', 'gigabytes', 'ram', 'floating_ips', 'instances', |
63 | + 'injected_files', 'cores'] |
64 | + for key in body['quota_set'].keys(): |
65 | + if key in resources: |
66 | + value = int(body['quota_set'][key]) |
67 | + try: |
68 | + db.quota_update(context, project_id, key, value) |
69 | + except exception.ProjectQuotaNotFound: |
70 | + db.quota_create(context, project_id, key, value) |
71 | + except exception.AdminRequired: |
72 | + return webob.Response(status_int=403) |
73 | + return {'quota_set': quota.get_project_quotas(context, project_id)} |
74 | + |
75 | + def defaults(self, req, id): |
76 | + return self._format_quota_set(id, quota._get_default_quotas()) |
77 | + |
78 | + |
79 | +class Quotas(extensions.ExtensionDescriptor): |
80 | + |
81 | + def get_name(self): |
82 | + return "Quotas" |
83 | + |
84 | + def get_alias(self): |
85 | + return "os-quota-sets" |
86 | + |
87 | + def get_description(self): |
88 | + return "Quotas management support" |
89 | + |
90 | + def get_namespace(self): |
91 | + return "http://docs.openstack.org/ext/quotas-sets/api/v1.1" |
92 | + |
93 | + def get_updated(self): |
94 | + return "2011-08-08T00:00:00+00:00" |
95 | + |
96 | + def get_resources(self): |
97 | + resources = [] |
98 | + |
99 | + res = extensions.ResourceExtension('os-quota-sets', |
100 | + QuotaSetsController(), |
101 | + member_actions={'defaults': 'GET'}) |
102 | + resources.append(res) |
103 | + |
104 | + return resources |
105 | |
106 | === modified file 'nova/db/sqlalchemy/api.py' |
107 | --- nova/db/sqlalchemy/api.py 2011-08-15 20:31:43 +0000 |
108 | +++ nova/db/sqlalchemy/api.py 2011-08-16 16:18:38 +0000 |
109 | @@ -1959,6 +1959,7 @@ |
110 | |
111 | @require_context |
112 | def quota_get_all_by_project(context, project_id): |
113 | + authorize_project_context(context, project_id) |
114 | session = get_session() |
115 | result = {'project_id': project_id} |
116 | rows = session.query(models.Quota).\ |
117 | |
118 | === added file 'nova/tests/api/openstack/contrib/test_quotas.py' |
119 | --- nova/tests/api/openstack/contrib/test_quotas.py 1970-01-01 00:00:00 +0000 |
120 | +++ nova/tests/api/openstack/contrib/test_quotas.py 2011-08-16 16:18:38 +0000 |
121 | @@ -0,0 +1,152 @@ |
122 | +# vim: tabstop=4 shiftwidth=4 softtabstop=4 |
123 | + |
124 | +# Copyright 2011 OpenStack LLC. |
125 | +# All Rights Reserved. |
126 | +# |
127 | +# Licensed under the Apache License, Version 2.0 (the "License"); you may |
128 | +# not use this file except in compliance with the License. You may obtain |
129 | +# a copy of the License at |
130 | +# |
131 | +# http://www.apache.org/licenses/LICENSE-2.0 |
132 | +# |
133 | +# Unless required by applicable law or agreed to in writing, software |
134 | +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT |
135 | +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the |
136 | +# License for the specific language governing permissions and limitations |
137 | +# under the License. |
138 | + |
139 | +import json |
140 | +import webob |
141 | + |
142 | +from nova import context |
143 | +from nova import test |
144 | +from nova.tests.api.openstack import fakes |
145 | + |
146 | +from nova.api.openstack.contrib.quotas import QuotaSetsController |
147 | + |
148 | + |
149 | +def quota_set(id): |
150 | + return {'quota_set': {'id': id, 'metadata_items': 128, 'volumes': 10, |
151 | + 'gigabytes': 1000, 'ram': 51200, 'floating_ips': 10, |
152 | + 'instances': 10, 'injected_files': 5, 'cores': 20, |
153 | + 'injected_file_content_bytes': 10240}} |
154 | + |
155 | + |
156 | +def quota_set_list(): |
157 | + return {'quota_set_list': [quota_set('1234'), quota_set('5678'), |
158 | + quota_set('update_me')]} |
159 | + |
160 | + |
161 | +class QuotaSetsTest(test.TestCase): |
162 | + |
163 | + def setUp(self): |
164 | + super(QuotaSetsTest, self).setUp() |
165 | + self.controller = QuotaSetsController() |
166 | + self.user_id = 'fake' |
167 | + self.project_id = 'fake' |
168 | + self.user_context = context.RequestContext(self.user_id, |
169 | + self.project_id) |
170 | + self.admin_context = context.RequestContext(self.user_id, |
171 | + self.project_id, |
172 | + is_admin=True) |
173 | + |
174 | + def test_format_quota_set(self): |
175 | + raw_quota_set = { |
176 | + 'instances': 10, |
177 | + 'cores': 20, |
178 | + 'ram': 51200, |
179 | + 'volumes': 10, |
180 | + 'floating_ips': 10, |
181 | + 'metadata_items': 128, |
182 | + 'gigabytes': 1000, |
183 | + 'injected_files': 5, |
184 | + 'injected_file_content_bytes': 10240} |
185 | + |
186 | + quota_set = QuotaSetsController()._format_quota_set('1234', |
187 | + raw_quota_set) |
188 | + qs = quota_set['quota_set'] |
189 | + |
190 | + self.assertEqual(qs['id'], '1234') |
191 | + self.assertEqual(qs['instances'], 10) |
192 | + self.assertEqual(qs['cores'], 20) |
193 | + self.assertEqual(qs['ram'], 51200) |
194 | + self.assertEqual(qs['volumes'], 10) |
195 | + self.assertEqual(qs['gigabytes'], 1000) |
196 | + self.assertEqual(qs['floating_ips'], 10) |
197 | + self.assertEqual(qs['metadata_items'], 128) |
198 | + self.assertEqual(qs['injected_files'], 5) |
199 | + self.assertEqual(qs['injected_file_content_bytes'], 10240) |
200 | + |
201 | + def test_quotas_defaults(self): |
202 | + req = webob.Request.blank('/v1.1/os-quota-sets/fake_tenant/defaults') |
203 | + req.method = 'GET' |
204 | + req.headers['Content-Type'] = 'application/json' |
205 | + res = req.get_response(fakes.wsgi_app()) |
206 | + |
207 | + self.assertEqual(res.status_int, 200) |
208 | + expected = {'quota_set': { |
209 | + 'id': 'fake_tenant', |
210 | + 'instances': 10, |
211 | + 'cores': 20, |
212 | + 'ram': 51200, |
213 | + 'volumes': 10, |
214 | + 'gigabytes': 1000, |
215 | + 'floating_ips': 10, |
216 | + 'metadata_items': 128, |
217 | + 'injected_files': 5, |
218 | + 'injected_file_content_bytes': 10240}} |
219 | + |
220 | + self.assertEqual(json.loads(res.body), expected) |
221 | + |
222 | + def test_quotas_show_as_admin(self): |
223 | + req = webob.Request.blank('/v1.1/os-quota-sets/1234') |
224 | + req.method = 'GET' |
225 | + req.headers['Content-Type'] = 'application/json' |
226 | + res = req.get_response(fakes.wsgi_app( |
227 | + fake_auth_context=self.admin_context)) |
228 | + |
229 | + self.assertEqual(res.status_int, 200) |
230 | + self.assertEqual(json.loads(res.body), quota_set('1234')) |
231 | + |
232 | + def test_quotas_show_as_unauthorized_user(self): |
233 | + req = webob.Request.blank('/v1.1/os-quota-sets/1234') |
234 | + req.method = 'GET' |
235 | + req.headers['Content-Type'] = 'application/json' |
236 | + res = req.get_response(fakes.wsgi_app( |
237 | + fake_auth_context=self.user_context)) |
238 | + |
239 | + self.assertEqual(res.status_int, 403) |
240 | + |
241 | + def test_quotas_update_as_admin(self): |
242 | + updated_quota_set = {'quota_set': {'instances': 50, |
243 | + 'cores': 50, 'ram': 51200, 'volumes': 10, |
244 | + 'gigabytes': 1000, 'floating_ips': 10, |
245 | + 'metadata_items': 128, 'injected_files': 5, |
246 | + 'injected_file_content_bytes': 10240}} |
247 | + |
248 | + req = webob.Request.blank('/v1.1/os-quota-sets/update_me') |
249 | + req.method = 'PUT' |
250 | + req.body = json.dumps(updated_quota_set) |
251 | + req.headers['Content-Type'] = 'application/json' |
252 | + |
253 | + res = req.get_response(fakes.wsgi_app( |
254 | + fake_auth_context=self.admin_context)) |
255 | + |
256 | + self.assertEqual(json.loads(res.body), updated_quota_set) |
257 | + |
258 | + def test_quotas_update_as_user(self): |
259 | + updated_quota_set = {'quota_set': {'instances': 50, |
260 | + 'cores': 50, 'ram': 51200, 'volumes': 10, |
261 | + 'gigabytes': 1000, 'floating_ips': 10, |
262 | + 'metadata_items': 128, 'injected_files': 5, |
263 | + 'injected_file_content_bytes': 10240}} |
264 | + |
265 | + req = webob.Request.blank('/v1.1/os-quota-sets/update_me') |
266 | + req.method = 'PUT' |
267 | + req.body = json.dumps(updated_quota_set) |
268 | + req.headers['Content-Type'] = 'application/json' |
269 | + |
270 | + res = req.get_response(fakes.wsgi_app( |
271 | + fake_auth_context=self.user_context)) |
272 | + |
273 | + self.assertEqual(res.status_int, 403) |
274 | |
275 | === modified file 'nova/tests/api/openstack/test_extensions.py' |
276 | --- nova/tests/api/openstack/test_extensions.py 2011-08-16 12:54:30 +0000 |
277 | +++ nova/tests/api/openstack/test_extensions.py 2011-08-16 16:18:38 +0000 |
278 | @@ -91,6 +91,7 @@ |
279 | "Hosts", |
280 | "Keypairs", |
281 | "Multinic", |
282 | + "Quotas", |
283 | "SecurityGroups", |
284 | "Volumes", |
285 | ] |
286 | @@ -110,7 +111,7 @@ |
287 | self.assertEqual(names, self.ext_list) |
288 | |
289 | # Make sure that at least Fox in Sox is correct. |
290 | - (fox_ext,) = [ |
291 | + (fox_ext, ) = [ |
292 | x for x in data['extensions'] if x['alias'] == 'FOXNSOX'] |
293 | self.assertEqual(fox_ext, { |
294 | 'namespace': 'http://www.fox.in.socks/api/ext/pie/v1.0', |
295 | @@ -155,7 +156,7 @@ |
296 | self.assertEqual(len(exts), len(self.ext_list)) |
297 | |
298 | # Make sure that at least Fox in Sox is correct. |
299 | - (fox_ext,) = [x for x in exts if x.get('alias') == 'FOXNSOX'] |
300 | + (fox_ext, ) = [x for x in exts if x.get('alias') == 'FOXNSOX'] |
301 | self.assertEqual(fox_ext.get('name'), 'Fox In Socks') |
302 | self.assertEqual(fox_ext.get('namespace'), |
303 | 'http://www.fox.in.socks/api/ext/pie/v1.0') |
304 | @@ -227,6 +228,7 @@ |
305 | |
306 | |
307 | class InvalidExtension(object): |
308 | + |
309 | def get_alias(self): |
310 | return "THIRD" |
311 |
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