Merge lp:~justin-fathomdb/nova/ec2-filters into lp:~hudson-openstack/nova/trunk
- ec2-filters
- Merge into trunk
Status: | Work in progress |
---|---|
Proposed branch: | lp:~justin-fathomdb/nova/ec2-filters |
Merge into: | lp:~hudson-openstack/nova/trunk |
Diff against target: |
417 lines (+270/-10) 7 files modified
nova/api/ec2/apirequest.py (+7/-4) nova/api/ec2/cloud.py (+27/-2) nova/api/predicates.py (+133/-0) nova/db/api.py (+5/-0) nova/db/sqlalchemy/api.py (+49/-0) nova/tests/test_cloud.py (+39/-0) nova/volume/api.py (+10/-4) |
To merge this branch: | bzr merge lp:~justin-fathomdb/nova/ec2-filters |
Related bugs: | |
Related blueprints: |
Support EC2 filters
(Undefined)
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nova Core security contacts | Pending | ||
Devin Carlen | Pending | ||
Review via email: mp+53266@code.launchpad.net |
This proposal supersedes a proposal from 2011-03-11.
Commit message
Description of the change
Initial support for EC2 filters
Targeted for diablo, but I can't get LP to understand that!
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal | # |
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal | # |
Hi Justin,
Nice work here but I think it's a bit overkill for the bug. Excerpt from bug 732924:
"This makes it difficult to figure out which project a particular volume is associated with. Instances, on the other hand specifically return a project as the owner (in a separate attribute at that):
[ownerId] => testproject
Similarly, addresses also report the project as an owner:
[instanceId] => i-00000002 (testproject)"
The bug isn't that we need any advanced filtering. This only affects EC2 API and how we have to work around a few of its sharp edges.
And it looks like Ryan Lane already made this fix:
https:/
So what is the need for this merge? It doesn't seem to have anything to do with the bug you linked.
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal | # |
Ryan raised his issue on the mailing list and I said that this was one of the three solutions I saw, and that I was working on it. The other options were to put the projectId into the metadata, or to change the status field. To be honest, I didn't think we'd be willing to change the status field, because we've just broken anyone that was previously using that to get the user.
The fact that an alternative solution was merged does rather reduce the need for the patch, but this is definitely a solution that Ryan and I discussed to his issue, which is why I linked the bug.
This patch is still the "right thing to do", and we'll have to do it for true EC2 API compatibility, which I thought was already an approved goal. We'll probably also have to implement filtering for the OpenStack API once users have a sufficient number of VMs (which I presume is why AWS implemented it), which is why the patch is architected with a separation of the AWS filterspec from an underlying predicate. Finally, it would dramatically simplify and harmonize a lot of our code, by using functions that take predicates rather than a long list of similar functions varying only in their arguments.
But maybe not in Cactus? If we're locking down the cactus branch for release, should I propose a merge into the Diablo branch?
Thierry Carrez (ttx) wrote : Posted in a previous version of this proposal | # |
The referenced bug is already fixed. That sounds like a new feature we could easily rubberstamp for Diablo.
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal | # |
Agreed; let's bring this in for Diablo.
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal | # |
Marked as rejected for now. We'll re-review in Diablo.
justinsb (justin-fathomdb) wrote : Posted in a previous version of this proposal | # |
How do I propose for merge into Diablo, so that the code doesn't get lost?
Is the release process (in terms of version control branches) documented
anywhere? I'm finding it all a bit "unusual"
Thierry Carrez (ttx) wrote : | # |
You should keep the branch merge proposal "work in progress" until Diablo branches open. You'll probably have to merge with trunk before reproposing for merging.
I'd suggest filing a blueprint for a review (rubberstamp) session at the summit, linking to the branch.
justinsb (justin-fathomdb) wrote : | # |
That seems a highly unusual "process". Is this process documented and
agreed? (And can you send me a link please!)
I think perhaps at Diablo we should have a discussion about which of
the successful open source release/development models we should use.
Devin Carlen (devcamcar) wrote : | # |
Maybe its a bit unusual but there's no diablo branch right now to propose merging into. Just means you'll have to babysit the branch for a little while. :)
Thierry Carrez (ttx) wrote : Posted in a previous version of this proposal | # |
Keep your branch until the Diablo merge window opens. You don't need to propose for merging. The code won't get lost. The branch will stay.
You can also set up your own "development branch" where you merge all those "feature branches" for more advanced testing.
Preview Diff
1 | === modified file 'nova/api/ec2/apirequest.py' | |||
2 | --- nova/api/ec2/apirequest.py 2011-03-04 19:01:25 +0000 | |||
3 | +++ nova/api/ec2/apirequest.py 2011-03-14 15:58:21 +0000 | |||
4 | @@ -133,11 +133,14 @@ | |||
5 | 133 | # NOTE(vish): Automatically convert strings back | 133 | # NOTE(vish): Automatically convert strings back |
6 | 134 | # into their respective values | 134 | # into their respective values |
7 | 135 | value = _try_convert(value) | 135 | value = _try_convert(value) |
8 | 136 | |||
9 | 136 | if len(parts) > 1: | 137 | if len(parts) > 1: |
14 | 137 | d = args.get(key, {}) | 138 | parent = args.setdefault(key, {}) |
15 | 138 | d[parts[1]] = value | 139 | for i in xrange(1, len(parts) - 1): |
16 | 139 | value = d | 140 | parent = parent.setdefault(parts[i], {}) |
17 | 140 | args[key] = value | 141 | parent[parts[-1]] = value |
18 | 142 | else: | ||
19 | 143 | args[key] = value | ||
20 | 141 | 144 | ||
21 | 142 | for key in args.keys(): | 145 | for key in args.keys(): |
22 | 143 | # NOTE(vish): Turn numeric dict keys into lists | 146 | # NOTE(vish): Turn numeric dict keys into lists |
23 | 144 | 147 | ||
24 | === modified file 'nova/api/ec2/cloud.py' | |||
25 | --- nova/api/ec2/cloud.py 2011-03-03 23:05:00 +0000 | |||
26 | +++ nova/api/ec2/cloud.py 2011-03-14 15:58:21 +0000 | |||
27 | @@ -2,6 +2,7 @@ | |||
28 | 2 | 2 | ||
29 | 3 | # Copyright 2010 United States Government as represented by the | 3 | # Copyright 2010 United States Government as represented by the |
30 | 4 | # Administrator of the National Aeronautics and Space Administration. | 4 | # Administrator of the National Aeronautics and Space Administration. |
31 | 5 | # Copyright 2011 Justin Santa Barbara | ||
32 | 5 | # All Rights Reserved. | 6 | # All Rights Reserved. |
33 | 6 | # | 7 | # |
34 | 7 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | 8 | # Licensed under the Apache License, Version 2.0 (the "License"); you may |
35 | @@ -39,6 +40,7 @@ | |||
36 | 39 | from nova import network | 40 | from nova import network |
37 | 40 | from nova import utils | 41 | from nova import utils |
38 | 41 | from nova import volume | 42 | from nova import volume |
39 | 43 | from nova.api import predicates | ||
40 | 42 | from nova.api.ec2 import ec2utils | 44 | from nova.api.ec2 import ec2utils |
41 | 43 | from nova.compute import instance_types | 45 | from nova.compute import instance_types |
42 | 44 | from nova.image import s3 | 46 | from nova.image import s3 |
43 | @@ -533,15 +535,38 @@ | |||
44 | 533 | return self.compute_api.get_ajax_console(context, | 535 | return self.compute_api.get_ajax_console(context, |
45 | 534 | instance_id=instance_id) | 536 | instance_id=instance_id) |
46 | 535 | 537 | ||
48 | 536 | def describe_volumes(self, context, volume_id=None, **kwargs): | 538 | def _map_ec2_filter_to_predicate(self, ec2_filters): |
49 | 539 | if ec2_filters is None: | ||
50 | 540 | return predicates.TruePredicate() | ||
51 | 541 | value_predicates = [] | ||
52 | 542 | for ec2_filter in ec2_filters: | ||
53 | 543 | name = ec2_filter['name'] | ||
54 | 544 | acceptable_values = [] | ||
55 | 545 | for v in ec2_filter['value'].values(): | ||
56 | 546 | acceptable_values.append(v) | ||
57 | 547 | if not acceptable_values: | ||
58 | 548 | return predicates.FalsePredicate() | ||
59 | 549 | value_predicates.append(predicates.ValuePredicate( | ||
60 | 550 | name, | ||
61 | 551 | acceptable_values)) | ||
62 | 552 | if not value_predicates: | ||
63 | 553 | return predicates.TruePredicate() | ||
64 | 554 | |||
65 | 555 | return predicates.CompoundAndPredicate(value_predicates) | ||
66 | 556 | |||
67 | 557 | def describe_volumes(self, context, volume_id=None, filter=None, | ||
68 | 558 | **kwargs): | ||
69 | 559 | predicate = self._map_ec2_filter_to_predicate(filter) | ||
70 | 537 | if volume_id: | 560 | if volume_id: |
71 | 561 | #TODO(justinsb): We probably should support filters here | ||
72 | 562 | # but filters with specified ids are a very odd request | ||
73 | 538 | volumes = [] | 563 | volumes = [] |
74 | 539 | for ec2_id in volume_id: | 564 | for ec2_id in volume_id: |
75 | 540 | internal_id = ec2utils.ec2_id_to_id(ec2_id) | 565 | internal_id = ec2utils.ec2_id_to_id(ec2_id) |
76 | 541 | volume = self.volume_api.get(context, internal_id) | 566 | volume = self.volume_api.get(context, internal_id) |
77 | 542 | volumes.append(volume) | 567 | volumes.append(volume) |
78 | 543 | else: | 568 | else: |
80 | 544 | volumes = self.volume_api.get_all(context) | 569 | volumes = self.volume_api.get_all(context, predicate) |
81 | 545 | volumes = [self._format_volume(context, v) for v in volumes] | 570 | volumes = [self._format_volume(context, v) for v in volumes] |
82 | 546 | return {'volumeSet': volumes} | 571 | return {'volumeSet': volumes} |
83 | 547 | 572 | ||
84 | 548 | 573 | ||
85 | === added file 'nova/api/predicates.py' | |||
86 | --- nova/api/predicates.py 1970-01-01 00:00:00 +0000 | |||
87 | +++ nova/api/predicates.py 2011-03-14 15:58:21 +0000 | |||
88 | @@ -0,0 +1,133 @@ | |||
89 | 1 | # vim: tabstop=4 shiftwidth=4 softtabstop=4 | ||
90 | 2 | |||
91 | 3 | # Copyright 2011 Justin Santa Barbara | ||
92 | 4 | # All Rights Reserved. | ||
93 | 5 | # | ||
94 | 6 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | ||
95 | 7 | # not use this file except in compliance with the License. You may obtain | ||
96 | 8 | # a copy of the License at | ||
97 | 9 | # | ||
98 | 10 | # http://www.apache.org/licenses/LICENSE-2.0 | ||
99 | 11 | # | ||
100 | 12 | # Unless required by applicable law or agreed to in writing, software | ||
101 | 13 | # distributed under the License is distributed on an "AS IS" BASIS, WITHOUT | ||
102 | 14 | # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the | ||
103 | 15 | # License for the specific language governing permissions and limitations | ||
104 | 16 | # under the License. | ||
105 | 17 | |||
106 | 18 | """Model classes that represent predicates""" | ||
107 | 19 | |||
108 | 20 | from nova import log as logging | ||
109 | 21 | |||
110 | 22 | |||
111 | 23 | LOG = logging.getLogger('nova.api.predicates') | ||
112 | 24 | |||
113 | 25 | # Well known properties | ||
114 | 26 | PROPERTY_PROJECT_ID = 'project_id' | ||
115 | 27 | |||
116 | 28 | |||
117 | 29 | class Predicate(object): | ||
118 | 30 | """A predicate is a logical function which is either true or false. | ||
119 | 31 | |||
120 | 32 | We use it for filtering values""" | ||
121 | 33 | def __init__(self): | ||
122 | 34 | pass | ||
123 | 35 | |||
124 | 36 | def join_and(self, rhs): | ||
125 | 37 | """Returns a predicate which is true iff both this and rhs are true""" | ||
126 | 38 | return CompoundAndPredicate([self, rhs]) | ||
127 | 39 | |||
128 | 40 | def filter_by_project(self, project_id): | ||
129 | 41 | """Adds a predicate so we only match the specified project""" | ||
130 | 42 | project_filter = ValuePredicate(PROPERTY_PROJECT_ID, project_id) | ||
131 | 43 | return self.join_and(project_filter) | ||
132 | 44 | |||
133 | 45 | def collapse_to_filters(self): | ||
134 | 46 | """Collapse to a dictionary of key => value_list filters. | ||
135 | 47 | |||
136 | 48 | If the predicate is unsatisfiable, return False. | ||
137 | 49 | If the predicate cannot be simplified in this way, raise an exception. | ||
138 | 50 | The predicate should be true for an object o iff | ||
139 | 51 | for each k,v in map: o[k] in v | ||
140 | 52 | """ | ||
141 | 53 | raise NotImplementedError() | ||
142 | 54 | |||
143 | 55 | |||
144 | 56 | class ValuePredicate(Predicate): | ||
145 | 57 | """A predicate on a single property | ||
146 | 58 | |||
147 | 59 | The predicate is satisfied if the value is a member of acceptable_values | ||
148 | 60 | """ | ||
149 | 61 | def __init__(self, key, acceptable_values): | ||
150 | 62 | super(ValuePredicate, self).__init__() | ||
151 | 63 | self.key = key | ||
152 | 64 | if not isinstance(acceptable_values, list): | ||
153 | 65 | raise Exception("acceptable_values must be a list") | ||
154 | 66 | self.acceptable_values = acceptable_values | ||
155 | 67 | |||
156 | 68 | def collapse_to_filters(self): | ||
157 | 69 | collapsed = {} | ||
158 | 70 | collapsed[self.key] = self.acceptable_values | ||
159 | 71 | return collapsed | ||
160 | 72 | |||
161 | 73 | |||
162 | 74 | def _intersect(lhs, rhs): | ||
163 | 75 | """Returns the list of items in both lhs and rhs""" | ||
164 | 76 | return list(set(lhs) & set(rhs)) | ||
165 | 77 | |||
166 | 78 | |||
167 | 79 | class CompoundAndPredicate(Predicate): | ||
168 | 80 | """A predicate where all children must be satisfied""" | ||
169 | 81 | def __init__(self, children): | ||
170 | 82 | super(CompoundAndPredicate, self).__init__() | ||
171 | 83 | self.children = children | ||
172 | 84 | |||
173 | 85 | def join_and(self, rhs): | ||
174 | 86 | #NOTE(justinsb): We could do some immediate-simplification here | ||
175 | 87 | # e.g. if rhs is a CompoundAndPredicate | ||
176 | 88 | conjunction = self.children + rhs | ||
177 | 89 | return CompoundAndPredicate(conjunction) | ||
178 | 90 | |||
179 | 91 | def collapse_to_filters(self): | ||
180 | 92 | collapsed = {} | ||
181 | 93 | for child in self.children: | ||
182 | 94 | child_map = child.collapse_to_filters() | ||
183 | 95 | if child_map == False: | ||
184 | 96 | return False | ||
185 | 97 | |||
186 | 98 | for k, v in child_map.iteritems(): | ||
187 | 99 | values = collapsed.get(k) | ||
188 | 100 | if values is None: | ||
189 | 101 | values = v | ||
190 | 102 | else: | ||
191 | 103 | values = _intersect(values, v) | ||
192 | 104 | if not values: | ||
193 | 105 | # Unsatisfiable (something like X=A and X=B, A != B) | ||
194 | 106 | return False | ||
195 | 107 | |||
196 | 108 | collapsed[k] = values | ||
197 | 109 | |||
198 | 110 | return collapsed | ||
199 | 111 | |||
200 | 112 | |||
201 | 113 | class FalsePredicate(Predicate): | ||
202 | 114 | """A predicate which is always False""" | ||
203 | 115 | |||
204 | 116 | def join_and(self, rhs): | ||
205 | 117 | # We are never true, so our conjunction can never be true | ||
206 | 118 | return FalsePredicate() | ||
207 | 119 | |||
208 | 120 | def collapse_to_filters(self): | ||
209 | 121 | # Unsatisfiable, return False | ||
210 | 122 | return False | ||
211 | 123 | |||
212 | 124 | |||
213 | 125 | class TruePredicate(Predicate): | ||
214 | 126 | """A predicate which is always True""" | ||
215 | 127 | |||
216 | 128 | def join_and(self, rhs): | ||
217 | 129 | return rhs | ||
218 | 130 | |||
219 | 131 | def collapse_to_filters(self): | ||
220 | 132 | # No criteria | ||
221 | 133 | return {} | ||
222 | 0 | 134 | ||
223 | === modified file 'nova/db/api.py' | |||
224 | --- nova/db/api.py 2011-03-09 21:27:38 +0000 | |||
225 | +++ nova/db/api.py 2011-03-14 15:58:21 +0000 | |||
226 | @@ -770,6 +770,11 @@ | |||
227 | 770 | return IMPL.volume_get_all_by_project(context, project_id) | 770 | return IMPL.volume_get_all_by_project(context, project_id) |
228 | 771 | 771 | ||
229 | 772 | 772 | ||
230 | 773 | def volume_get_predicated(context, predicate): | ||
231 | 774 | """Get a list of volumes matching the specified predicate.""" | ||
232 | 775 | return IMPL.volume_get_predicated(context, predicate) | ||
233 | 776 | |||
234 | 777 | |||
235 | 773 | def volume_get_by_ec2_id(context, ec2_id): | 778 | def volume_get_by_ec2_id(context, ec2_id): |
236 | 774 | """Get a volume by ec2 id.""" | 779 | """Get a volume by ec2 id.""" |
237 | 775 | return IMPL.volume_get_by_ec2_id(context, ec2_id) | 780 | return IMPL.volume_get_by_ec2_id(context, ec2_id) |
238 | 776 | 781 | ||
239 | === modified file 'nova/db/sqlalchemy/api.py' | |||
240 | --- nova/db/sqlalchemy/api.py 2011-03-11 23:05:02 +0000 | |||
241 | +++ nova/db/sqlalchemy/api.py 2011-03-14 15:58:21 +0000 | |||
242 | @@ -2,6 +2,7 @@ | |||
243 | 2 | 2 | ||
244 | 3 | # Copyright 2010 United States Government as represented by the | 3 | # Copyright 2010 United States Government as represented by the |
245 | 4 | # Administrator of the National Aeronautics and Space Administration. | 4 | # Administrator of the National Aeronautics and Space Administration. |
246 | 5 | # Copyright 2011 Justin Santa Barbara | ||
247 | 5 | # All Rights Reserved. | 6 | # All Rights Reserved. |
248 | 6 | # | 7 | # |
249 | 7 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | 8 | # Licensed under the Apache License, Version 2.0 (the "License"); you may |
250 | @@ -25,7 +26,9 @@ | |||
251 | 25 | from nova import db | 26 | from nova import db |
252 | 26 | from nova import exception | 27 | from nova import exception |
253 | 27 | from nova import flags | 28 | from nova import flags |
254 | 29 | from nova import log as logging | ||
255 | 28 | from nova import utils | 30 | from nova import utils |
256 | 31 | from nova.api import predicates | ||
257 | 29 | from nova.db.sqlalchemy import models | 32 | from nova.db.sqlalchemy import models |
258 | 30 | from nova.db.sqlalchemy.session import get_session | 33 | from nova.db.sqlalchemy.session import get_session |
259 | 31 | from sqlalchemy import or_ | 34 | from sqlalchemy import or_ |
260 | @@ -36,8 +39,11 @@ | |||
261 | 36 | from sqlalchemy.sql import func | 39 | from sqlalchemy.sql import func |
262 | 37 | from sqlalchemy.sql.expression import literal_column | 40 | from sqlalchemy.sql.expression import literal_column |
263 | 38 | 41 | ||
264 | 42 | |||
265 | 39 | FLAGS = flags.FLAGS | 43 | FLAGS = flags.FLAGS |
266 | 40 | 44 | ||
267 | 45 | LOG = logging.getLogger('nova.db.sqlalchemy') | ||
268 | 46 | |||
269 | 41 | 47 | ||
270 | 42 | def is_admin_context(context): | 48 | def is_admin_context(context): |
271 | 43 | """Indicates if the request context is an administrator.""" | 49 | """Indicates if the request context is an administrator.""" |
272 | @@ -1511,6 +1517,49 @@ | |||
273 | 1511 | return result | 1517 | return result |
274 | 1512 | 1518 | ||
275 | 1513 | 1519 | ||
276 | 1520 | @require_context | ||
277 | 1521 | def volume_get_predicated(context, predicate=None): | ||
278 | 1522 | session = get_session() | ||
279 | 1523 | query = session.query(models.Volume).\ | ||
280 | 1524 | options(joinedload('instance')).\ | ||
281 | 1525 | filter_by(deleted=can_read_deleted(context)) | ||
282 | 1526 | |||
283 | 1527 | if predicate: | ||
284 | 1528 | filters = predicate.collapse_to_filters() | ||
285 | 1529 | LOG.debug("filters=%s", filters) | ||
286 | 1530 | else: | ||
287 | 1531 | filters = {} | ||
288 | 1532 | |||
289 | 1533 | # If the predicate is known-unsatisfiable, don't bother hitting the DB | ||
290 | 1534 | if filters == False: | ||
291 | 1535 | return [] | ||
292 | 1536 | |||
293 | 1537 | filtered_project_id = None | ||
294 | 1538 | |||
295 | 1539 | # For now, we whitelist/hard-code the filters... | ||
296 | 1540 | for k, v in filters.iteritems(): | ||
297 | 1541 | if k == predicates.PROPERTY_PROJECT_ID: | ||
298 | 1542 | for project_id in v: | ||
299 | 1543 | authorize_project_context(context, project_id) | ||
300 | 1544 | |||
301 | 1545 | #TODO(justinsb): Does SA simplify a single-item IN clause? | ||
302 | 1546 | if len(v) == 1: | ||
303 | 1547 | query = query.filter(models.Volume.project_id == v[0]) | ||
304 | 1548 | else: | ||
305 | 1549 | query = query.filter(models.Volume.project_id.in_(v)) | ||
306 | 1550 | |||
307 | 1551 | filtered_project_id = v | ||
308 | 1552 | else: | ||
309 | 1553 | raise exception.ApiError(_("Unknown predicate: %s") % k) | ||
310 | 1554 | |||
311 | 1555 | # Only an admin can do a query without a project filter | ||
312 | 1556 | if not filtered_project_id: | ||
313 | 1557 | if not is_admin_context(context): | ||
314 | 1558 | raise exception.NotAuthorized() | ||
315 | 1559 | |||
316 | 1560 | return query.all() | ||
317 | 1561 | |||
318 | 1562 | |||
319 | 1514 | @require_admin_context | 1563 | @require_admin_context |
320 | 1515 | def volume_get_all(context): | 1564 | def volume_get_all(context): |
321 | 1516 | session = get_session() | 1565 | session = get_session() |
322 | 1517 | 1566 | ||
323 | === modified file 'nova/tests/test_cloud.py' | |||
324 | --- nova/tests/test_cloud.py 2011-03-10 04:42:11 +0000 | |||
325 | +++ nova/tests/test_cloud.py 2011-03-14 15:58:21 +0000 | |||
326 | @@ -30,6 +30,7 @@ | |||
327 | 30 | from nova import context | 30 | from nova import context |
328 | 31 | from nova import crypto | 31 | from nova import crypto |
329 | 32 | from nova import db | 32 | from nova import db |
330 | 33 | from nova import exception | ||
331 | 33 | from nova import flags | 34 | from nova import flags |
332 | 34 | from nova import log as logging | 35 | from nova import log as logging |
333 | 35 | from nova import rpc | 36 | from nova import rpc |
334 | @@ -176,6 +177,44 @@ | |||
335 | 176 | db.volume_destroy(self.context, vol1['id']) | 177 | db.volume_destroy(self.context, vol1['id']) |
336 | 177 | db.volume_destroy(self.context, vol2['id']) | 178 | db.volume_destroy(self.context, vol2['id']) |
337 | 178 | 179 | ||
338 | 180 | def test_describe_volumes_filters(self): | ||
339 | 181 | """Makes sure describe_volumes works and filters results.""" | ||
340 | 182 | vol1 = db.volume_create(self.context, {'project_id': 'proj1'}) | ||
341 | 183 | vol2 = db.volume_create(self.context, {'project_id': 'proj2'}) | ||
342 | 184 | |||
343 | 185 | filtr = [{ 'name': 'project_id', 'value': {'1': 'proj1'} }] | ||
344 | 186 | result = self.cloud.describe_volumes(self.context, filter=filtr) | ||
345 | 187 | self.assertEqual(len(result['volumeSet']), 1) | ||
346 | 188 | |||
347 | 189 | filtr = [{ 'name': 'project_id', 'value': {'1': 'proj2'} }] | ||
348 | 190 | result = self.cloud.describe_volumes(self.context, filter=filtr) | ||
349 | 191 | self.assertEqual(len(result['volumeSet']), 1) | ||
350 | 192 | |||
351 | 193 | filtr = [{ 'name': 'project_id', 'value': {'1': 'proj3'} }] | ||
352 | 194 | result = self.cloud.describe_volumes(self.context, filter=filtr) | ||
353 | 195 | self.assertEqual(len(result['volumeSet']), 0) | ||
354 | 196 | |||
355 | 197 | filtr = [{ 'name': 'project_id', 'value': {'1': 'proj1', | ||
356 | 198 | '2': 'proj2'} }] | ||
357 | 199 | result = self.cloud.describe_volumes(self.context, filter=filtr) | ||
358 | 200 | self.assertEqual(len(result['volumeSet']), 2) | ||
359 | 201 | |||
360 | 202 | # Unsatisfiable (shouldn't even hit DB!) | ||
361 | 203 | filtr = [{ 'name': 'project_id', 'value': {'1': 'proj1'} }, | ||
362 | 204 | { 'name': 'project_id', 'value': {'1': 'proj2'} }, | ||
363 | 205 | ] | ||
364 | 206 | result = self.cloud.describe_volumes(self.context, filter=filtr) | ||
365 | 207 | self.assertEqual(len(result['volumeSet']), 0) | ||
366 | 208 | |||
367 | 209 | # Not a supported value | ||
368 | 210 | filtr = [{ 'name': 'notso_key', 'value': {'1': 'notso_value'} }] | ||
369 | 211 | self.assertRaises(exception.ApiError, | ||
370 | 212 | self.cloud.describe_volumes, | ||
371 | 213 | self.context, filter=filtr) | ||
372 | 214 | |||
373 | 215 | db.volume_destroy(self.context, vol1['id']) | ||
374 | 216 | db.volume_destroy(self.context, vol2['id']) | ||
375 | 217 | |||
376 | 179 | def test_describe_availability_zones(self): | 218 | def test_describe_availability_zones(self): |
377 | 180 | """Makes sure describe_availability_zones works and filters results.""" | 219 | """Makes sure describe_availability_zones works and filters results.""" |
378 | 181 | service1 = db.service_create(self.context, {'host': 'host1_zones', | 220 | service1 = db.service_create(self.context, {'host': 'host1_zones', |
379 | 182 | 221 | ||
380 | === modified file 'nova/volume/api.py' | |||
381 | --- nova/volume/api.py 2011-02-19 07:15:42 +0000 | |||
382 | +++ nova/volume/api.py 2011-03-14 15:58:21 +0000 | |||
383 | @@ -2,6 +2,7 @@ | |||
384 | 2 | 2 | ||
385 | 3 | # Copyright 2010 United States Government as represented by the | 3 | # Copyright 2010 United States Government as represented by the |
386 | 4 | # Administrator of the National Aeronautics and Space Administration. | 4 | # Administrator of the National Aeronautics and Space Administration. |
387 | 5 | # Copyright 2011 Justin Santa Barbara | ||
388 | 5 | # All Rights Reserved. | 6 | # All Rights Reserved. |
389 | 6 | # | 7 | # |
390 | 7 | # Licensed under the Apache License, Version 2.0 (the "License"); you may | 8 | # Licensed under the Apache License, Version 2.0 (the "License"); you may |
391 | @@ -28,6 +29,7 @@ | |||
392 | 28 | from nova import log as logging | 29 | from nova import log as logging |
393 | 29 | from nova import quota | 30 | from nova import quota |
394 | 30 | from nova import rpc | 31 | from nova import rpc |
395 | 32 | from nova.api import predicates | ||
396 | 31 | from nova.db import base | 33 | from nova.db import base |
397 | 32 | 34 | ||
398 | 33 | FLAGS = flags.FLAGS | 35 | FLAGS = flags.FLAGS |
399 | @@ -84,10 +86,14 @@ | |||
400 | 84 | def get(self, context, volume_id): | 86 | def get(self, context, volume_id): |
401 | 85 | return self.db.volume_get(context, volume_id) | 87 | return self.db.volume_get(context, volume_id) |
402 | 86 | 88 | ||
407 | 87 | def get_all(self, context): | 89 | def get_all(self, context, predicate=None): |
408 | 88 | if context.is_admin: | 90 | if predicate is None: |
409 | 89 | return self.db.volume_get_all(context) | 91 | predicate = predicates.TruePredicate() |
410 | 90 | return self.db.volume_get_all_by_project(context, context.project_id) | 92 | |
411 | 93 | if not context.is_admin: | ||
412 | 94 | predicate = predicate.filter_by_project(context.project_id) | ||
413 | 95 | |||
414 | 96 | return self.db.volume_get_predicated(context, predicate) | ||
415 | 91 | 97 | ||
416 | 92 | def check_attach(self, context, volume_id): | 98 | def check_attach(self, context, volume_id): |
417 | 93 | volume = self.get(context, volume_id) | 99 | volume = self.get(context, volume_id) |
I could use some pointers on how to unit-test the EC2 API, so I can check this still works when I'm not cheating and going direct to the CloudController...