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 | 1 | # vim: tabstop=4 shiftwidth=4 softtabstop=4 | ||
6 | 2 | |||
7 | 3 | # Copyright 2011 OpenStack LLC. | ||
8 | 4 | # All Rights Reserved. | ||
9 | 5 | # | ||
10 | 6 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | ||
11 | 7 | # not use this file except in compliance with the License. You may obtain | ||
12 | 8 | # a copy of the License at | ||
13 | 9 | # | ||
14 | 10 | # http://www.apache.org/licenses/LICENSE-2.0 | ||
15 | 11 | # | ||
16 | 12 | # Unless required by applicable law or agreed to in writing, software | ||
17 | 13 | # distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
18 | 14 | # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
19 | 15 | # License for the specific language governing permissions and limitations | ||
20 | 16 | # under the License. | ||
21 | 17 | |||
22 | 18 | import webob | ||
23 | 19 | |||
24 | 20 | from nova import db | ||
25 | 21 | from nova import exception | ||
26 | 22 | from nova import quota | ||
27 | 23 | from nova.api.openstack import extensions | ||
28 | 24 | |||
29 | 25 | |||
30 | 26 | class QuotaSetsController(object): | ||
31 | 27 | |||
32 | 28 | def _format_quota_set(self, project_id, quota_set): | ||
33 | 29 | """Convert the quota object to a result dict""" | ||
34 | 30 | |||
35 | 31 | return {'quota_set': { | ||
36 | 32 | 'id': str(project_id), | ||
37 | 33 | 'metadata_items': quota_set['metadata_items'], | ||
38 | 34 | 'injected_file_content_bytes': | ||
39 | 35 | quota_set['injected_file_content_bytes'], | ||
40 | 36 | 'volumes': quota_set['volumes'], | ||
41 | 37 | 'gigabytes': quota_set['gigabytes'], | ||
42 | 38 | 'ram': quota_set['ram'], | ||
43 | 39 | 'floating_ips': quota_set['floating_ips'], | ||
44 | 40 | 'instances': quota_set['instances'], | ||
45 | 41 | 'injected_files': quota_set['injected_files'], | ||
46 | 42 | 'cores': quota_set['cores'], | ||
47 | 43 | }} | ||
48 | 44 | |||
49 | 45 | def show(self, req, id): | ||
50 | 46 | context = req.environ['nova.context'] | ||
51 | 47 | try: | ||
52 | 48 | db.sqlalchemy.api.authorize_project_context(context, id) | ||
53 | 49 | return self._format_quota_set(id, | ||
54 | 50 | quota.get_project_quotas(context, id)) | ||
55 | 51 | except exception.NotAuthorized: | ||
56 | 52 | return webob.Response(status_int=403) | ||
57 | 53 | |||
58 | 54 | def update(self, req, id, body): | ||
59 | 55 | context = req.environ['nova.context'] | ||
60 | 56 | project_id = id | ||
61 | 57 | resources = ['metadata_items', 'injected_file_content_bytes', | ||
62 | 58 | 'volumes', 'gigabytes', 'ram', 'floating_ips', 'instances', | ||
63 | 59 | 'injected_files', 'cores'] | ||
64 | 60 | for key in body['quota_set'].keys(): | ||
65 | 61 | if key in resources: | ||
66 | 62 | value = int(body['quota_set'][key]) | ||
67 | 63 | try: | ||
68 | 64 | db.quota_update(context, project_id, key, value) | ||
69 | 65 | except exception.ProjectQuotaNotFound: | ||
70 | 66 | db.quota_create(context, project_id, key, value) | ||
71 | 67 | except exception.AdminRequired: | ||
72 | 68 | return webob.Response(status_int=403) | ||
73 | 69 | return {'quota_set': quota.get_project_quotas(context, project_id)} | ||
74 | 70 | |||
75 | 71 | def defaults(self, req, id): | ||
76 | 72 | return self._format_quota_set(id, quota._get_default_quotas()) | ||
77 | 73 | |||
78 | 74 | |||
79 | 75 | class Quotas(extensions.ExtensionDescriptor): | ||
80 | 76 | |||
81 | 77 | def get_name(self): | ||
82 | 78 | return "Quotas" | ||
83 | 79 | |||
84 | 80 | def get_alias(self): | ||
85 | 81 | return "os-quota-sets" | ||
86 | 82 | |||
87 | 83 | def get_description(self): | ||
88 | 84 | return "Quotas management support" | ||
89 | 85 | |||
90 | 86 | def get_namespace(self): | ||
91 | 87 | return "http://docs.openstack.org/ext/quotas-sets/api/v1.1" | ||
92 | 88 | |||
93 | 89 | def get_updated(self): | ||
94 | 90 | return "2011-08-08T00:00:00+00:00" | ||
95 | 91 | |||
96 | 92 | def get_resources(self): | ||
97 | 93 | resources = [] | ||
98 | 94 | |||
99 | 95 | res = extensions.ResourceExtension('os-quota-sets', | ||
100 | 96 | QuotaSetsController(), | ||
101 | 97 | member_actions={'defaults': 'GET'}) | ||
102 | 98 | resources.append(res) | ||
103 | 99 | |||
104 | 100 | return resources | ||
105 | 0 | 101 | ||
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 | 1959 | 1959 | ||
111 | 1960 | @require_context | 1960 | @require_context |
112 | 1961 | def quota_get_all_by_project(context, project_id): | 1961 | def quota_get_all_by_project(context, project_id): |
113 | 1962 | authorize_project_context(context, project_id) | ||
114 | 1962 | session = get_session() | 1963 | session = get_session() |
115 | 1963 | result = {'project_id': project_id} | 1964 | result = {'project_id': project_id} |
116 | 1964 | rows = session.query(models.Quota).\ | 1965 | rows = session.query(models.Quota).\ |
117 | 1965 | 1966 | ||
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 | 1 | # vim: tabstop=4 shiftwidth=4 softtabstop=4 | ||
123 | 2 | |||
124 | 3 | # Copyright 2011 OpenStack LLC. | ||
125 | 4 | # All Rights Reserved. | ||
126 | 5 | # | ||
127 | 6 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | ||
128 | 7 | # not use this file except in compliance with the License. You may obtain | ||
129 | 8 | # a copy of the License at | ||
130 | 9 | # | ||
131 | 10 | # http://www.apache.org/licenses/LICENSE-2.0 | ||
132 | 11 | # | ||
133 | 12 | # Unless required by applicable law or agreed to in writing, software | ||
134 | 13 | # distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
135 | 14 | # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
136 | 15 | # License for the specific language governing permissions and limitations | ||
137 | 16 | # under the License. | ||
138 | 17 | |||
139 | 18 | import json | ||
140 | 19 | import webob | ||
141 | 20 | |||
142 | 21 | from nova import context | ||
143 | 22 | from nova import test | ||
144 | 23 | from nova.tests.api.openstack import fakes | ||
145 | 24 | |||
146 | 25 | from nova.api.openstack.contrib.quotas import QuotaSetsController | ||
147 | 26 | |||
148 | 27 | |||
149 | 28 | def quota_set(id): | ||
150 | 29 | return {'quota_set': {'id': id, 'metadata_items': 128, 'volumes': 10, | ||
151 | 30 | 'gigabytes': 1000, 'ram': 51200, 'floating_ips': 10, | ||
152 | 31 | 'instances': 10, 'injected_files': 5, 'cores': 20, | ||
153 | 32 | 'injected_file_content_bytes': 10240}} | ||
154 | 33 | |||
155 | 34 | |||
156 | 35 | def quota_set_list(): | ||
157 | 36 | return {'quota_set_list': [quota_set('1234'), quota_set('5678'), | ||
158 | 37 | quota_set('update_me')]} | ||
159 | 38 | |||
160 | 39 | |||
161 | 40 | class QuotaSetsTest(test.TestCase): | ||
162 | 41 | |||
163 | 42 | def setUp(self): | ||
164 | 43 | super(QuotaSetsTest, self).setUp() | ||
165 | 44 | self.controller = QuotaSetsController() | ||
166 | 45 | self.user_id = 'fake' | ||
167 | 46 | self.project_id = 'fake' | ||
168 | 47 | self.user_context = context.RequestContext(self.user_id, | ||
169 | 48 | self.project_id) | ||
170 | 49 | self.admin_context = context.RequestContext(self.user_id, | ||
171 | 50 | self.project_id, | ||
172 | 51 | is_admin=True) | ||
173 | 52 | |||
174 | 53 | def test_format_quota_set(self): | ||
175 | 54 | raw_quota_set = { | ||
176 | 55 | 'instances': 10, | ||
177 | 56 | 'cores': 20, | ||
178 | 57 | 'ram': 51200, | ||
179 | 58 | 'volumes': 10, | ||
180 | 59 | 'floating_ips': 10, | ||
181 | 60 | 'metadata_items': 128, | ||
182 | 61 | 'gigabytes': 1000, | ||
183 | 62 | 'injected_files': 5, | ||
184 | 63 | 'injected_file_content_bytes': 10240} | ||
185 | 64 | |||
186 | 65 | quota_set = QuotaSetsController()._format_quota_set('1234', | ||
187 | 66 | raw_quota_set) | ||
188 | 67 | qs = quota_set['quota_set'] | ||
189 | 68 | |||
190 | 69 | self.assertEqual(qs['id'], '1234') | ||
191 | 70 | self.assertEqual(qs['instances'], 10) | ||
192 | 71 | self.assertEqual(qs['cores'], 20) | ||
193 | 72 | self.assertEqual(qs['ram'], 51200) | ||
194 | 73 | self.assertEqual(qs['volumes'], 10) | ||
195 | 74 | self.assertEqual(qs['gigabytes'], 1000) | ||
196 | 75 | self.assertEqual(qs['floating_ips'], 10) | ||
197 | 76 | self.assertEqual(qs['metadata_items'], 128) | ||
198 | 77 | self.assertEqual(qs['injected_files'], 5) | ||
199 | 78 | self.assertEqual(qs['injected_file_content_bytes'], 10240) | ||
200 | 79 | |||
201 | 80 | def test_quotas_defaults(self): | ||
202 | 81 | req = webob.Request.blank('/v1.1/os-quota-sets/fake_tenant/defaults') | ||
203 | 82 | req.method = 'GET' | ||
204 | 83 | req.headers['Content-Type'] = 'application/json' | ||
205 | 84 | res = req.get_response(fakes.wsgi_app()) | ||
206 | 85 | |||
207 | 86 | self.assertEqual(res.status_int, 200) | ||
208 | 87 | expected = {'quota_set': { | ||
209 | 88 | 'id': 'fake_tenant', | ||
210 | 89 | 'instances': 10, | ||
211 | 90 | 'cores': 20, | ||
212 | 91 | 'ram': 51200, | ||
213 | 92 | 'volumes': 10, | ||
214 | 93 | 'gigabytes': 1000, | ||
215 | 94 | 'floating_ips': 10, | ||
216 | 95 | 'metadata_items': 128, | ||
217 | 96 | 'injected_files': 5, | ||
218 | 97 | 'injected_file_content_bytes': 10240}} | ||
219 | 98 | |||
220 | 99 | self.assertEqual(json.loads(res.body), expected) | ||
221 | 100 | |||
222 | 101 | def test_quotas_show_as_admin(self): | ||
223 | 102 | req = webob.Request.blank('/v1.1/os-quota-sets/1234') | ||
224 | 103 | req.method = 'GET' | ||
225 | 104 | req.headers['Content-Type'] = 'application/json' | ||
226 | 105 | res = req.get_response(fakes.wsgi_app( | ||
227 | 106 | fake_auth_context=self.admin_context)) | ||
228 | 107 | |||
229 | 108 | self.assertEqual(res.status_int, 200) | ||
230 | 109 | self.assertEqual(json.loads(res.body), quota_set('1234')) | ||
231 | 110 | |||
232 | 111 | def test_quotas_show_as_unauthorized_user(self): | ||
233 | 112 | req = webob.Request.blank('/v1.1/os-quota-sets/1234') | ||
234 | 113 | req.method = 'GET' | ||
235 | 114 | req.headers['Content-Type'] = 'application/json' | ||
236 | 115 | res = req.get_response(fakes.wsgi_app( | ||
237 | 116 | fake_auth_context=self.user_context)) | ||
238 | 117 | |||
239 | 118 | self.assertEqual(res.status_int, 403) | ||
240 | 119 | |||
241 | 120 | def test_quotas_update_as_admin(self): | ||
242 | 121 | updated_quota_set = {'quota_set': {'instances': 50, | ||
243 | 122 | 'cores': 50, 'ram': 51200, 'volumes': 10, | ||
244 | 123 | 'gigabytes': 1000, 'floating_ips': 10, | ||
245 | 124 | 'metadata_items': 128, 'injected_files': 5, | ||
246 | 125 | 'injected_file_content_bytes': 10240}} | ||
247 | 126 | |||
248 | 127 | req = webob.Request.blank('/v1.1/os-quota-sets/update_me') | ||
249 | 128 | req.method = 'PUT' | ||
250 | 129 | req.body = json.dumps(updated_quota_set) | ||
251 | 130 | req.headers['Content-Type'] = 'application/json' | ||
252 | 131 | |||
253 | 132 | res = req.get_response(fakes.wsgi_app( | ||
254 | 133 | fake_auth_context=self.admin_context)) | ||
255 | 134 | |||
256 | 135 | self.assertEqual(json.loads(res.body), updated_quota_set) | ||
257 | 136 | |||
258 | 137 | def test_quotas_update_as_user(self): | ||
259 | 138 | updated_quota_set = {'quota_set': {'instances': 50, | ||
260 | 139 | 'cores': 50, 'ram': 51200, 'volumes': 10, | ||
261 | 140 | 'gigabytes': 1000, 'floating_ips': 10, | ||
262 | 141 | 'metadata_items': 128, 'injected_files': 5, | ||
263 | 142 | 'injected_file_content_bytes': 10240}} | ||
264 | 143 | |||
265 | 144 | req = webob.Request.blank('/v1.1/os-quota-sets/update_me') | ||
266 | 145 | req.method = 'PUT' | ||
267 | 146 | req.body = json.dumps(updated_quota_set) | ||
268 | 147 | req.headers['Content-Type'] = 'application/json' | ||
269 | 148 | |||
270 | 149 | res = req.get_response(fakes.wsgi_app( | ||
271 | 150 | fake_auth_context=self.user_context)) | ||
272 | 151 | |||
273 | 152 | self.assertEqual(res.status_int, 403) | ||
274 | 0 | 153 | ||
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 | 91 | "Hosts", | 91 | "Hosts", |
280 | 92 | "Keypairs", | 92 | "Keypairs", |
281 | 93 | "Multinic", | 93 | "Multinic", |
282 | 94 | "Quotas", | ||
283 | 94 | "SecurityGroups", | 95 | "SecurityGroups", |
284 | 95 | "Volumes", | 96 | "Volumes", |
285 | 96 | ] | 97 | ] |
286 | @@ -110,7 +111,7 @@ | |||
287 | 110 | self.assertEqual(names, self.ext_list) | 111 | self.assertEqual(names, self.ext_list) |
288 | 111 | 112 | ||
289 | 112 | # Make sure that at least Fox in Sox is correct. | 113 | # Make sure that at least Fox in Sox is correct. |
291 | 113 | (fox_ext,) = [ | 114 | (fox_ext, ) = [ |
292 | 114 | x for x in data['extensions'] if x['alias'] == 'FOXNSOX'] | 115 | x for x in data['extensions'] if x['alias'] == 'FOXNSOX'] |
293 | 115 | self.assertEqual(fox_ext, { | 116 | self.assertEqual(fox_ext, { |
294 | 116 | 'namespace': 'http://www.fox.in.socks/api/ext/pie/v1.0', | 117 | 'namespace': 'http://www.fox.in.socks/api/ext/pie/v1.0', |
295 | @@ -155,7 +156,7 @@ | |||
296 | 155 | self.assertEqual(len(exts), len(self.ext_list)) | 156 | self.assertEqual(len(exts), len(self.ext_list)) |
297 | 156 | 157 | ||
298 | 157 | # Make sure that at least Fox in Sox is correct. | 158 | # Make sure that at least Fox in Sox is correct. |
300 | 158 | (fox_ext,) = [x for x in exts if x.get('alias') == 'FOXNSOX'] | 159 | (fox_ext, ) = [x for x in exts if x.get('alias') == 'FOXNSOX'] |
301 | 159 | self.assertEqual(fox_ext.get('name'), 'Fox In Socks') | 160 | self.assertEqual(fox_ext.get('name'), 'Fox In Socks') |
302 | 160 | self.assertEqual(fox_ext.get('namespace'), | 161 | self.assertEqual(fox_ext.get('namespace'), |
303 | 161 | 'http://www.fox.in.socks/api/ext/pie/v1.0') | 162 | 'http://www.fox.in.socks/api/ext/pie/v1.0') |
304 | @@ -227,6 +228,7 @@ | |||
305 | 227 | 228 | ||
306 | 228 | 229 | ||
307 | 229 | class InvalidExtension(object): | 230 | class InvalidExtension(object): |
308 | 231 | |||
309 | 230 | def get_alias(self): | 232 | def get_alias(self): |
310 | 231 | return "THIRD" | 233 | return "THIRD" |
311 | 232 | 234 |
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