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