Merge lp:~jaypipes/glance/teller-api into lp:~hudson-openstack/glance/trunk
- teller-api
- Merge into trunk
Proposed by
Jay Pipes
Status: | Merged |
---|---|
Approved by: | Jay Pipes |
Approved revision: | 21 |
Merged at revision: | 20 |
Proposed branch: | lp:~jaypipes/glance/teller-api |
Merge into: | lp:~hudson-openstack/glance/trunk |
Diff against target: |
779 lines (+322/-159) 7 files modified
glance/client.py (+30/-11) glance/parallax/controllers.py (+28/-27) glance/teller/controllers.py (+18/-20) glance/teller/registries.py (+19/-33) tests/stubs.py (+115/-25) tests/unit/test_clients.py (+76/-11) tests/unit/test_teller_api.py (+36/-32) |
To merge this branch: | bzr merge lp:~jaypipes/glance/teller-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Michael Gundlach (community) | Approve | ||
Rick Harris (community) | Approve | ||
Christopher MacGown | Pending | ||
Review via email: mp+42392@code.launchpad.net |
Commit message
Description of the change
* Changes Teller API to use REST with opaque ID sent in
API calls instead of a "parallax URI". This hides the
URI stuff behind the API layer in communication between
Parallax and Teller.
* Adds unit tests for the only complete Teller API call so
far: GET images/<ID>, which returns a gzip'd string of
image data
I want to get feedback on these new unit tests and the
changes to the Teller API to remove the parallax URI from
the API calls.
To post a comment you must log in.
Revision history for this message
Rick Harris (rconradharris) wrote : | # |
Revision history for this message
Rick Harris (rconradharris) wrote : | # |
Marking as approved.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'glance/client.py' |
2 | --- glance/client.py 2010-11-23 18:12:11 +0000 |
3 | +++ glance/client.py 2010-12-01 17:19:12 +0000 |
4 | @@ -73,6 +73,20 @@ |
5 | self.protocol = url.scheme |
6 | self.connection = None |
7 | |
8 | + def get_connection_type(self): |
9 | + """ |
10 | + Returns the proper connection type |
11 | + """ |
12 | + try: |
13 | + connection_type = {'http': httplib.HTTPConnection, |
14 | + 'https': httplib.HTTPSConnection}\ |
15 | + [self.protocol] |
16 | + return connection_type |
17 | + except KeyError: |
18 | + raise UnsupportedProtocolError("Unsupported protocol %s. Unable " |
19 | + " to connect to server." |
20 | + % self.protocol) |
21 | + |
22 | def do_request(self, method, action, body=None): |
23 | """ |
24 | Connects to the server and issues a request. Handles converting |
25 | @@ -86,14 +100,7 @@ |
26 | :param data: string of data to send, or None (default) |
27 | """ |
28 | try: |
29 | - connection_type = {'http': httplib.HTTPConnection, |
30 | - 'https': httplib.HTTPSConnection}\ |
31 | - [self.protocol] |
32 | - except KeyError: |
33 | - raise UnsupportedProtocolError("Unsupported protocol %s. Unable " |
34 | - " to connect to server." |
35 | - % self.protocol) |
36 | - try: |
37 | + connection_type = self.get_connection_type() |
38 | c = connection_type(self.netloc, self.port) |
39 | c.request(method, action, body) |
40 | res = c.getresponse() |
41 | @@ -116,8 +123,6 @@ |
42 | except (socket.error, IOError), e: |
43 | raise ClientConnectionError("Unable to connect to " |
44 | "server. Got error: %s" % e) |
45 | - finally: |
46 | - c.close() |
47 | |
48 | def get_status_code(self, response): |
49 | """ |
50 | @@ -132,7 +137,7 @@ |
51 | |
52 | class TellerClient(BaseClient): |
53 | |
54 | - """A client for the Teller image caching service""" |
55 | + """A client for the Teller image caching and delivery service""" |
56 | |
57 | DEFAULT_ADDRESS = 'http://127.0.0.1' |
58 | DEFAULT_PORT = 9191 |
59 | @@ -148,6 +153,20 @@ |
60 | """ |
61 | super(TellerClient, self).__init__(**kwargs) |
62 | |
63 | + def get_image(self, image_id): |
64 | + """ |
65 | + Returns the raw disk image as a mime-encoded blob stream for the |
66 | + supplied opaque image identifier. |
67 | + |
68 | + :param image_id: The opaque image identifier |
69 | + |
70 | + :raises exception.NotFound if image is not found |
71 | + """ |
72 | + # TODO(jaypipes): Handle other registries than Parallax... |
73 | + |
74 | + res = self.do_request("GET", "images/%s" % image_id) |
75 | + return res.read() |
76 | + |
77 | |
78 | class ParallaxClient(BaseClient): |
79 | |
80 | |
81 | === modified file 'glance/parallax/controllers.py' |
82 | --- glance/parallax/controllers.py 2010-11-23 18:12:11 +0000 |
83 | +++ glance/parallax/controllers.py 2010-12-01 17:19:12 +0000 |
84 | @@ -61,7 +61,7 @@ |
85 | |
86 | """ |
87 | images = db.image_get_all_public(None) |
88 | - image_dicts = [self._make_image_dict(i) for i in images] |
89 | + image_dicts = [make_image_dict(i) for i in images] |
90 | return dict(images=image_dicts) |
91 | |
92 | def show(self, req, id): |
93 | @@ -71,7 +71,7 @@ |
94 | except exception.NotFound: |
95 | raise exc.HTTPNotFound() |
96 | |
97 | - return dict(image=self._make_image_dict(image)) |
98 | + return dict(image=make_image_dict(image)) |
99 | |
100 | def delete(self, req, id): |
101 | """Deletes an existing image with the registry. |
102 | @@ -133,31 +133,6 @@ |
103 | except exception.NotFound: |
104 | return exc.HTTPNotFound() |
105 | |
106 | - @staticmethod |
107 | - def _make_image_dict(image): |
108 | - """Create a dict representation of an image which we can use to |
109 | - serialize the image. |
110 | - |
111 | - """ |
112 | - |
113 | - def _fetch_attrs(d, attrs): |
114 | - return dict([(a, d[a]) for a in attrs]) |
115 | - |
116 | - files = [_fetch_attrs(f, db.IMAGE_FILE_ATTRS) for f in image['files']] |
117 | - |
118 | - # TODO(sirp): should this be a dict, or a list of dicts? |
119 | - # A plain dict is more convenient, but list of dicts would provide |
120 | - # access to created_at, etc |
121 | - properties = dict((p['key'], p['value']) |
122 | - for p in image['properties'] |
123 | - if not p['deleted']) |
124 | - |
125 | - image_dict = _fetch_attrs(image, db.IMAGE_ATTRS) |
126 | - |
127 | - image_dict['files'] = files |
128 | - image_dict['properties'] = properties |
129 | - return image_dict |
130 | - |
131 | |
132 | class API(wsgi.Router): |
133 | """WSGI entry point for all Parallax requests.""" |
134 | @@ -169,3 +144,29 @@ |
135 | collection={'detail': 'GET'}) |
136 | mapper.connect("/", controller=ImageController(), action="index") |
137 | super(API, self).__init__(mapper) |
138 | + |
139 | + |
140 | +def make_image_dict(image): |
141 | + """ |
142 | + Create a dict representation of an image which we can use to |
143 | + serialize the image. |
144 | + """ |
145 | + |
146 | + def _fetch_attrs(d, attrs): |
147 | + return dict([(a, d[a]) for a in attrs |
148 | + if a in d.keys()]) |
149 | + |
150 | + files = [_fetch_attrs(f, db.IMAGE_FILE_ATTRS) for f in image['files']] |
151 | + |
152 | + # TODO(sirp): should this be a dict, or a list of dicts? |
153 | + # A plain dict is more convenient, but list of dicts would provide |
154 | + # access to created_at, etc |
155 | + properties = dict((p['key'], p['value']) |
156 | + for p in image['properties'] |
157 | + if 'deleted' in p.keys() and not p['deleted']) |
158 | + |
159 | + image_dict = _fetch_attrs(image, db.IMAGE_ATTRS) |
160 | + |
161 | + image_dict['files'] = files |
162 | + image_dict['properties'] = properties |
163 | + return image_dict |
164 | |
165 | === modified file 'glance/teller/controllers.py' |
166 | --- glance/teller/controllers.py 2010-10-12 18:01:12 +0000 |
167 | +++ glance/teller/controllers.py 2010-12-01 17:19:12 +0000 |
168 | @@ -31,9 +31,9 @@ |
169 | |
170 | |
171 | class ImageController(wsgi.Controller): |
172 | - """Image Controller """ |
173 | + """Image Controller""" |
174 | |
175 | - def index(self, req): |
176 | + def show(self, req, id): |
177 | """ |
178 | Query the parallax service for the image registry for the passed in |
179 | req['uri']. If it exists, we connect to the appropriate backend as |
180 | @@ -43,44 +43,40 @@ |
181 | Optionally, we can pass in 'registry' which will use a given |
182 | RegistryAdapter for the request. This is useful for testing. |
183 | """ |
184 | - try: |
185 | - uri = req.str_GET['uri'] |
186 | - except KeyError: |
187 | - return exc.HTTPBadRequest(body="Missing uri", request=req, |
188 | - content_type="text/plain") |
189 | |
190 | registry = req.str_GET.get('registry', 'parallax') |
191 | |
192 | try: |
193 | - image = registries.lookup_by_registry(registry, uri) |
194 | - logging.debug("Found image registry for URI: %s. Got: %s", uri, image) |
195 | + image = registries.lookup_by_registry(registry, id) |
196 | except registries.UnknownImageRegistry: |
197 | + logging.debug("Could not find image registry: %s.", registry) |
198 | return exc.HTTPBadRequest(body="Unknown registry '%s'" % registry, |
199 | request=req, |
200 | content_type="text/plain") |
201 | - |
202 | - if not image: |
203 | + except exception.NotFound: |
204 | raise exc.HTTPNotFound(body='Image not found', request=req, |
205 | content_type='text/plain') |
206 | |
207 | def image_iterator(): |
208 | for file in image['files']: |
209 | - chunks = backends.get_from_backend( |
210 | - file['location'], expected_size=file['size']) |
211 | + chunks = backends.get_from_backend(file['location'], |
212 | + expected_size=file['size']) |
213 | |
214 | for chunk in chunks: |
215 | yield chunk |
216 | |
217 | - return req.get_response(Response(app_iter=image_iterator())) |
218 | + res = Response(app_iter=image_iterator(), |
219 | + content_type="text/plain") |
220 | + return req.get_response(res) |
221 | |
222 | + def index(self, req): |
223 | + """Index is not currently supported """ |
224 | + raise exc.HTTPNotImplemented() |
225 | + |
226 | def detail(self, req): |
227 | """Detail is not currently supported """ |
228 | raise exc.HTTPNotImplemented() |
229 | |
230 | - def show(self, req): |
231 | - """Show is not currently supported """ |
232 | - raise exc.HTTPNotImplemented() |
233 | - |
234 | def delete(self, req, id): |
235 | """Delete is not currently supported """ |
236 | raise exc.HTTPNotImplemented() |
237 | @@ -95,10 +91,12 @@ |
238 | |
239 | |
240 | class API(wsgi.Router): |
241 | + |
242 | """WSGI entry point for all Teller requests.""" |
243 | |
244 | def __init__(self): |
245 | mapper = routes.Mapper() |
246 | - mapper.resource("image", "image", controller=ImageController(), |
247 | - collection={'detail': 'GET'}) |
248 | + mapper.resource("image", "images", controller=ImageController(), |
249 | + collection={'detail': 'GET'}) |
250 | + mapper.connect("/", controller=ImageController(), action="index") |
251 | super(API, self).__init__(mapper) |
252 | |
253 | === modified file 'glance/teller/registries.py' |
254 | --- glance/teller/registries.py 2010-10-11 18:28:35 +0000 |
255 | +++ glance/teller/registries.py 2010-12-01 17:19:12 +0000 |
256 | @@ -19,6 +19,8 @@ |
257 | import json |
258 | import urlparse |
259 | |
260 | +from glance import client |
261 | + |
262 | |
263 | class ImageRegistryException(Exception): |
264 | """ Base class for all RegistryAdapter exceptions """ |
265 | @@ -47,38 +49,17 @@ |
266 | """ |
267 | |
268 | @classmethod |
269 | - def lookup(cls, parsed_uri): |
270 | + def lookup(cls, image_id): |
271 | """ |
272 | - Takes a parsed_uri, checks if that image is registered in Parallax, |
273 | + Takes an image ID and checks if that image is registered in Parallax, |
274 | and if so, returns the image metadata. If the image does not exist, |
275 | - we return None. |
276 | + we raise NotFound |
277 | """ |
278 | - scheme = parsed_uri.scheme |
279 | - if scheme == 'http': |
280 | - conn_class = httplib.HTTPConnection |
281 | - elif scheme == 'https': |
282 | - conn_class = httplib.HTTPSConnection |
283 | - else: |
284 | - raise ImageRegistryException( |
285 | - "Unrecognized scheme '%s'" % scheme) |
286 | - |
287 | - conn = conn_class(parsed_uri.netloc) |
288 | - try: |
289 | - conn.request('GET', parsed_uri.path, "", {}) |
290 | - response = conn.getresponse() |
291 | - |
292 | - # The image exists |
293 | - if response.status == 200: |
294 | - result = response.read() |
295 | - image_json = json.loads(result) |
296 | - try: |
297 | - return image_json["image"] |
298 | - except KeyError: |
299 | - raise ImageRegistryException("Missing 'image' key") |
300 | - except Exception: # gaierror |
301 | - return None |
302 | - finally: |
303 | - conn.close() |
304 | + # TODO(jaypipes): Make parallax client configurable via options. |
305 | + # Unfortunately, the decision to make all adapters have no state |
306 | + # hinders this... |
307 | + c = client.ParallaxClient() |
308 | + return c.get_image(image_id) |
309 | |
310 | |
311 | REGISTRY_ADAPTERS = { |
312 | @@ -86,12 +67,17 @@ |
313 | } |
314 | |
315 | |
316 | -def lookup_by_registry(registry, image_uri): |
317 | - """ Convenience function to lookup based on a registry protocol """ |
318 | +def lookup_by_registry(registry, image_id): |
319 | + """ |
320 | + Convenience function to lookup image metadata for the given |
321 | + opaque image identifier and registry. |
322 | + |
323 | + :param registry: String name of registry to use for lookups |
324 | + :param image_id: Opaque image identifier |
325 | + """ |
326 | try: |
327 | adapter = REGISTRY_ADAPTERS[registry] |
328 | except KeyError: |
329 | raise UnknownImageRegistry("'%s' not found" % registry) |
330 | |
331 | - parsed_uri = urlparse.urlparse(image_uri) |
332 | - return adapter.lookup(parsed_uri) |
333 | + return adapter.lookup(image_id) |
334 | |
335 | === modified file 'tests/stubs.py' |
336 | --- tests/stubs.py 2010-11-23 18:12:11 +0000 |
337 | +++ tests/stubs.py 2010-12-01 17:19:12 +0000 |
338 | @@ -19,18 +19,26 @@ |
339 | |
340 | import datetime |
341 | import httplib |
342 | +import os |
343 | +import shutil |
344 | import StringIO |
345 | import sys |
346 | +import gzip |
347 | |
348 | import stubout |
349 | import webob |
350 | |
351 | from glance.common import exception |
352 | from glance.parallax import controllers as parallax_controllers |
353 | +from glance.teller import controllers as teller_controllers |
354 | +import glance.teller.backends |
355 | import glance.teller.backends.swift |
356 | import glance.parallax.db.sqlalchemy.api |
357 | |
358 | |
359 | +FAKE_FILESYSTEM_ROOTDIR = os.path.join('/tmp', 'glance-tests') |
360 | + |
361 | + |
362 | def stub_out_http_backend(stubs): |
363 | """Stubs out the httplib.HTTPRequest.getresponse to return |
364 | faked-out data instead of grabbing actual contents of a resource |
365 | @@ -63,24 +71,61 @@ |
366 | fake_http_conn.getresponse) |
367 | |
368 | |
369 | +def clean_out_fake_filesystem_backend(): |
370 | + """ |
371 | + Removes any leftover directories used in fake filesystem |
372 | + backend |
373 | + """ |
374 | + if os.path.exists(FAKE_FILESYSTEM_ROOTDIR): |
375 | + shutil.rmtree(FAKE_FILESYSTEM_ROOTDIR, ignore_errors=True) |
376 | + |
377 | + |
378 | def stub_out_filesystem_backend(stubs): |
379 | - """Stubs out the Filesystem Teller service to return fake |
380 | + """ |
381 | + Stubs out the Filesystem Teller service to return fake |
382 | data from files. |
383 | |
384 | - The stubbed service always yields the following fixture:: |
385 | - |
386 | - //chunk0 |
387 | - //chunk1 |
388 | + We establish a few fake images in a directory under /tmp/glance-tests |
389 | + and ensure that this directory contains the following files: |
390 | + |
391 | + /acct/2.gz.0 <-- zipped tarfile containing "chunk0" |
392 | + /acct/2.gz.1 <-- zipped tarfile containing "chunk42" |
393 | + |
394 | + The stubbed service yields the data in the above files. |
395 | |
396 | :param stubs: Set of stubout stubs |
397 | |
398 | """ |
399 | + |
400 | class FakeFilesystemBackend(object): |
401 | |
402 | + CHUNKSIZE = 100 |
403 | + |
404 | @classmethod |
405 | - def get(cls, parsed_uri, expected_size, conn_class=None): |
406 | - |
407 | - return StringIO.StringIO(parsed_uri.path) |
408 | + def get(cls, parsed_uri, expected_size, opener=None): |
409 | + filepath = os.path.join('/', |
410 | + parsed_uri.netloc, |
411 | + parsed_uri.path.strip(os.path.sep)) |
412 | + f = gzip.open(filepath, 'rb') |
413 | + data = f.read() |
414 | + f.close() |
415 | + return data |
416 | + |
417 | + # Establish a clean faked filesystem with dummy images |
418 | + if os.path.exists(FAKE_FILESYSTEM_ROOTDIR): |
419 | + shutil.rmtree(FAKE_FILESYSTEM_ROOTDIR, ignore_errors=True) |
420 | + os.mkdir(FAKE_FILESYSTEM_ROOTDIR) |
421 | + os.mkdir(os.path.join(FAKE_FILESYSTEM_ROOTDIR, 'acct')) |
422 | + |
423 | + f = gzip.open(os.path.join(FAKE_FILESYSTEM_ROOTDIR, 'acct', '2.gz.0'), |
424 | + "wb") |
425 | + f.write("chunk0") |
426 | + f.close() |
427 | + |
428 | + f = gzip.open(os.path.join(FAKE_FILESYSTEM_ROOTDIR, 'acct', '2.gz.1'), |
429 | + "wb") |
430 | + f.write("chunk42") |
431 | + f.close() |
432 | |
433 | fake_filesystem_backend = FakeFilesystemBackend() |
434 | stubs.Set(glance.teller.backends.FilesystemBackend, 'get', |
435 | @@ -156,25 +201,16 @@ |
436 | fake_parallax_registry.lookup) |
437 | |
438 | |
439 | -def stub_out_parallax_server(stubs): |
440 | +def stub_out_parallax_and_teller_server(stubs): |
441 | """ |
442 | - Mocks httplib calls to 127.0.0.1:9292 for testing so |
443 | - that a real Parallax server does not need to be up and |
444 | + Mocks calls to 127.0.0.1 on 9191 and 9292 for testing so |
445 | + that a real Teller server does not need to be up and |
446 | running |
447 | """ |
448 | |
449 | - def fake_http_connection_constructor(address, port): |
450 | - """ |
451 | - Returns either a faked connection or a real |
452 | - one depending on if the connection is to a parallax |
453 | - server or not... |
454 | - """ |
455 | - return FakeParallaxConnection() |
456 | - |
457 | - |
458 | class FakeParallaxConnection(object): |
459 | |
460 | - def __init__(self): |
461 | + def __init__(self, *args, **kwargs): |
462 | pass |
463 | |
464 | def connect(self): |
465 | @@ -199,8 +235,54 @@ |
466 | setattr(res, 'read', fake_reader) |
467 | return res |
468 | |
469 | - stubs.Set(httplib, 'HTTPConnection', |
470 | - fake_http_connection_constructor) |
471 | + class FakeTellerConnection(object): |
472 | + |
473 | + def __init__(self, *args, **kwargs): |
474 | + pass |
475 | + |
476 | + def connect(self): |
477 | + return True |
478 | + |
479 | + def close(self): |
480 | + return True |
481 | + |
482 | + def request(self, method, url, body=None): |
483 | + self.req = webob.Request.blank("/" + url.lstrip("/")) |
484 | + self.req.method = method |
485 | + if body: |
486 | + self.req.body = body |
487 | + |
488 | + def getresponse(self): |
489 | + res = self.req.get_response(teller_controllers.API()) |
490 | + |
491 | + # httplib.Response has a read() method...fake it out |
492 | + def fake_reader(): |
493 | + return res.body |
494 | + |
495 | + setattr(res, 'read', fake_reader) |
496 | + return res |
497 | + |
498 | + def fake_get_connection_type(client): |
499 | + """ |
500 | + Returns the proper connection type |
501 | + """ |
502 | + if client.port == 9191 and client.netloc == '127.0.0.1': |
503 | + return FakeTellerConnection |
504 | + if client.port == 9292 and client.netloc == '127.0.0.1': |
505 | + return FakeParallaxConnection |
506 | + else: |
507 | + try: |
508 | + connection_type = {'http': httplib.HTTPConnection, |
509 | + 'https': httplib.HTTPSConnection}\ |
510 | + [client.protocol] |
511 | + return connection_type |
512 | + except KeyError: |
513 | + raise UnsupportedProtocolError("Unsupported protocol %s. Unable " |
514 | + " to connect to server." |
515 | + % self.protocol) |
516 | + |
517 | + stubs.Set(glance.client.BaseClient, 'get_connection_type', |
518 | + fake_get_connection_type) |
519 | |
520 | |
521 | def stub_out_parallax_db_image_api(stubs): |
522 | @@ -225,7 +307,11 @@ |
523 | 'updated_at': datetime.datetime.utcnow(), |
524 | 'deleted_at': None, |
525 | 'deleted': False, |
526 | - 'files': [], |
527 | + 'files': [ |
528 | + {"location": "swift://user:passwd@acct/container/obj.tar.gz.0", |
529 | + "size": 6}, |
530 | + {"location": "swift://user:passwd@acct/container/obj.tar.gz.1", |
531 | + "size": 7}], |
532 | 'properties': []}, |
533 | {'id': 2, |
534 | 'name': 'fake image #2', |
535 | @@ -236,7 +322,11 @@ |
536 | 'updated_at': datetime.datetime.utcnow(), |
537 | 'deleted_at': None, |
538 | 'deleted': False, |
539 | - 'files': [], |
540 | + 'files': [ |
541 | + {"location": "file://tmp/glance-tests/acct/2.gz.0", |
542 | + "size": 6}, |
543 | + {"location": "file://tmp/glance-tests/acct/2.gz.1", |
544 | + "size": 7}], |
545 | 'properties': []}] |
546 | |
547 | VALID_STATUSES = ('available', 'disabled', 'pending') |
548 | |
549 | === modified file 'tests/unit/test_clients.py' |
550 | --- tests/unit/test_clients.py 2010-11-23 18:12:11 +0000 |
551 | +++ tests/unit/test_clients.py 2010-12-01 17:19:12 +0000 |
552 | @@ -47,13 +47,16 @@ |
553 | |
554 | class TestParallaxClient(unittest.TestCase): |
555 | |
556 | - """Test proper actions made for both valid and invalid requests""" |
557 | + """ |
558 | + Test proper actions made for both valid and invalid requests |
559 | + against a Parallax service |
560 | + """ |
561 | |
562 | def setUp(self): |
563 | """Establish a clean test environment""" |
564 | self.stubs = stubout.StubOutForTesting() |
565 | stubs.stub_out_parallax_db_image_api(self.stubs) |
566 | - stubs.stub_out_parallax_server(self.stubs) |
567 | + stubs.stub_out_parallax_and_teller_server(self.stubs) |
568 | self.client = client.ParallaxClient() |
569 | |
570 | def tearDown(self): |
571 | @@ -76,27 +79,61 @@ |
572 | 'name': 'fake image #2', |
573 | 'is_public': True, |
574 | 'image_type': 'kernel', |
575 | - 'status': 'available' |
576 | - } |
577 | + 'status': 'available', |
578 | + 'files': [ |
579 | + {"location": "file://tmp/glance-tests/acct/2.gz.0", |
580 | + "size": 6}, |
581 | + {"location": "file://tmp/glance-tests/acct/2.gz.1", |
582 | + "size": 7}], |
583 | + 'properties': []} |
584 | + |
585 | + expected = {'id': 2, |
586 | + 'name': 'fake image #2', |
587 | + 'is_public': True, |
588 | + 'image_type': 'kernel', |
589 | + 'status': 'available', |
590 | + 'files': [ |
591 | + {"location": "file://tmp/glance-tests/acct/2.gz.0", |
592 | + "size": 6}, |
593 | + {"location": "file://tmp/glance-tests/acct/2.gz.1", |
594 | + "size": 7}], |
595 | + 'properties': {}} |
596 | |
597 | images = self.client.get_images_detailed() |
598 | self.assertEquals(len(images), 1) |
599 | |
600 | - for k,v in fixture.iteritems(): |
601 | + for k,v in expected.iteritems(): |
602 | self.assertEquals(v, images[0][k]) |
603 | |
604 | - def test_get_image_metadata(self): |
605 | + def test_get_image(self): |
606 | """Tests that the detailed info about an image returned""" |
607 | fixture = {'id': 2, |
608 | 'name': 'fake image #2', |
609 | 'is_public': True, |
610 | 'image_type': 'kernel', |
611 | - 'status': 'available' |
612 | - } |
613 | + 'status': 'available', |
614 | + 'files': [ |
615 | + {"location": "file://tmp/glance-tests/acct/2.gz.0", |
616 | + "size": 6}, |
617 | + {"location": "file://tmp/glance-tests/acct/2.gz.1", |
618 | + "size": 7}], |
619 | + 'properties': []} |
620 | + |
621 | + expected = {'id': 2, |
622 | + 'name': 'fake image #2', |
623 | + 'is_public': True, |
624 | + 'image_type': 'kernel', |
625 | + 'status': 'available', |
626 | + 'files': [ |
627 | + {"location": "file://tmp/glance-tests/acct/2.gz.0", |
628 | + "size": 6}, |
629 | + {"location": "file://tmp/glance-tests/acct/2.gz.1", |
630 | + "size": 7}], |
631 | + 'properties': {}} |
632 | |
633 | data = self.client.get_image(2) |
634 | |
635 | - for k,v in fixture.iteritems(): |
636 | + for k,v in expected.iteritems(): |
637 | self.assertEquals(v, data[k]) |
638 | |
639 | def test_get_image_non_existing(self): |
640 | @@ -106,7 +143,7 @@ |
641 | self.client.get_image, |
642 | 42) |
643 | |
644 | - def test_add_image_metadata_basic(self): |
645 | + def test_add_image_basic(self): |
646 | """Tests that we can add image metadata and returns the new id""" |
647 | fixture = {'name': 'fake public image', |
648 | 'is_public': True, |
649 | @@ -128,7 +165,7 @@ |
650 | self.assertTrue('status' in data.keys()) |
651 | self.assertEquals('available', data['status']) |
652 | |
653 | - def test_add_image_metadata_with_properties(self): |
654 | + def test_add_image_with_properties(self): |
655 | """Tests that we can add image metadata with properties""" |
656 | fixture = {'name': 'fake public image', |
657 | 'is_public': True, |
658 | @@ -231,3 +268,31 @@ |
659 | self.assertRaises(exception.NotFound, |
660 | self.client.delete_image, |
661 | 3) |
662 | + |
663 | + |
664 | +class TestTellerClient(unittest.TestCase): |
665 | + |
666 | + """ |
667 | + Test proper actions made for both valid and invalid requests |
668 | + against a Teller service |
669 | + """ |
670 | + |
671 | + def setUp(self): |
672 | + """Establish a clean test environment""" |
673 | + self.stubs = stubout.StubOutForTesting() |
674 | + stubs.stub_out_parallax_db_image_api(self.stubs) |
675 | + stubs.stub_out_parallax_and_teller_server(self.stubs) |
676 | + stubs.stub_out_filesystem_backend(self.stubs) |
677 | + self.client = client.TellerClient() |
678 | + |
679 | + def tearDown(self): |
680 | + """Clear the test environment""" |
681 | + stubs.clean_out_fake_filesystem_backend() |
682 | + self.stubs.UnsetAll() |
683 | + |
684 | + def test_get_image(self): |
685 | + """Test a simple file backend retrieval works as expected""" |
686 | + expected = 'chunk0chunk42' |
687 | + image = self.client.get_image(2) |
688 | + |
689 | + self.assertEquals(expected, image) |
690 | |
691 | === modified file 'tests/unit/test_teller_api.py' |
692 | --- tests/unit/test_teller_api.py 2010-10-11 19:34:10 +0000 |
693 | +++ tests/unit/test_teller_api.py 2010-12-01 17:19:12 +0000 |
694 | @@ -15,9 +15,10 @@ |
695 | # License for the specific language governing permissions and limitations |
696 | # under the License. |
697 | |
698 | +import unittest |
699 | + |
700 | import stubout |
701 | -import unittest |
702 | -from webob import Request, exc |
703 | +import webob |
704 | |
705 | from glance.teller import controllers |
706 | from tests import stubs |
707 | @@ -27,39 +28,42 @@ |
708 | def setUp(self): |
709 | """Establish a clean test environment""" |
710 | self.stubs = stubout.StubOutForTesting() |
711 | + stubs.stub_out_parallax_and_teller_server(self.stubs) |
712 | + stubs.stub_out_parallax_db_image_api(self.stubs) |
713 | + stubs.stub_out_filesystem_backend(self.stubs) |
714 | self.image_controller = controllers.ImageController() |
715 | |
716 | def tearDown(self): |
717 | """Clear the test environment""" |
718 | + stubs.clean_out_fake_filesystem_backend() |
719 | self.stubs.UnsetAll() |
720 | |
721 | - def test_index_image_with_no_uri_should_raise_http_bad_request(self): |
722 | - # uri must be specified |
723 | - request = Request.blank("/image") |
724 | - response = self.image_controller.index(request) |
725 | - self.assertEqual(response.status_int, 400) # should be 422? |
726 | - |
727 | - def test_index_image_unrecognized_registry_adapter(self): |
728 | - # FIXME: need urllib.quote here? |
729 | - image_uri = "http://parallax-success/myacct/my-image" |
730 | - request = self._make_request(image_uri, "unknownregistry") |
731 | - response = self.image_controller.index(request) |
732 | - self.assertEqual(response.status_int, 400) # should be 422? |
733 | - |
734 | - def test_index_image_where_image_exists_should_return_the_data(self): |
735 | - # FIXME: need urllib.quote here? |
736 | - stubs.stub_out_parallax(self.stubs) |
737 | - stubs.stub_out_filesystem_backend(self.stubs) |
738 | - image_uri = "http://parallax/myacct/my-image" |
739 | - request = self._make_request(image_uri) |
740 | - response = self.image_controller.index(request) |
741 | - self.assertEqual("/chunk0/chunk1", response.body) |
742 | - |
743 | - def test_index_image_where_image_doesnt_exist_should_raise_not_found(self): |
744 | - image_uri = "http://bad-parallax-uri/myacct/does-not-exist" |
745 | - request = self._make_request(image_uri) |
746 | - self.assertRaises(exc.HTTPNotFound, self.image_controller.index, |
747 | - request) |
748 | - |
749 | - def _make_request(self, image_uri, registry="parallax"): |
750 | - return Request.blank("/image?uri=%s®istry=%s" % (image_uri, registry)) |
751 | + def test_index_raises_not_implemented(self): |
752 | + req = webob.Request.blank("/images") |
753 | + res = req.get_response(controllers.API()) |
754 | + self.assertEquals(res.status_int, webob.exc.HTTPNotImplemented.code) |
755 | + |
756 | + def test_blank_raises_not_implemented(self): |
757 | + req = webob.Request.blank("/") |
758 | + res = req.get_response(controllers.API()) |
759 | + self.assertEquals(res.status_int, webob.exc.HTTPNotImplemented.code) |
760 | + |
761 | + def test_detail_raises_not_implemented(self): |
762 | + req = webob.Request.blank("/images/detail") |
763 | + res = req.get_response(controllers.API()) |
764 | + self.assertEquals(res.status_int, webob.exc.HTTPNotImplemented.code) |
765 | + |
766 | + def test_show_image_unrecognized_registry_adapter(self): |
767 | + req = webob.Request.blank("/images/1?registry=unknown") |
768 | + res = req.get_response(controllers.API()) |
769 | + self.assertEquals(res.status_int, webob.exc.HTTPBadRequest.code) |
770 | + |
771 | + def test_show_image_basic(self): |
772 | + req = webob.Request.blank("/images/2") |
773 | + res = req.get_response(controllers.API()) |
774 | + self.assertEqual('chunk0chunk42', res.body) |
775 | + |
776 | + def test_show_non_exists_image(self): |
777 | + req = webob.Request.blank("/images/42") |
778 | + res = req.get_response(controllers.API()) |
779 | + self.assertEquals(res.status_int, webob.exc.HTTPNotFound.code) |
lgtm
I'm fine with the idea of using sequential id's for now and one registry per installation. However, down the line (prob beyond cactus), we're going to want to discuss how to federate the registries, whether it's using the registry URI as the ID (which has some neat interoperability implications), or using a UUID with some peer-lookup algorithm.
Tests look good and still run fast.