From 3956c546fb725a13784f6e1441a74be1680abd9d Mon Sep 17 00:00:00 2001 From: Eoghan Glynn Date: Tue, 17 Jan 2012 23:21:15 +0000 Subject: [PATCH] Set size metadata correctly for remote images. Previously the size metadata was always zero'd for remote images, which was misleading and led to issues like: Bug 900959 We now query the remote store (HTTP, S3 or Swift) with the equivalent of a HTTP HEAD in order to determine the correct image size without downloading. Where the size is available, the metadata is set appropriately. Otherwise it falls back to zero as before. Change-Id: I3093eba1b2fa023348558c45febee39e68e1a08f --- glance/api/v1/images.py | 15 ++++++---- glance/store/__init__.py | 9 ++++++ glance/store/base.py | 11 +++++++ glance/store/http.py | 30 ++++++++++++++----- glance/store/s3.py | 29 +++++++++++++----- glance/store/swift.py | 20 +++++++++++++ .../tests/functional/test_cache_middleware.py | 24 +++++++++++---- glance/tests/functional/test_s3.py | 8 +++-- glance/tests/functional/test_swift.py | 2 +- 9 files changed, 120 insertions(+), 28 deletions(-) diff --git a/glance/api/v1/images.py b/glance/api/v1/images.py index d2447cd7..573aae3b 100644 --- a/glance/api/v1/images.py +++ b/glance/api/v1/images.py @@ -45,6 +45,7 @@ import glance.store.rbd import glance.store.s3 import glance.store.swift from glance.store import (get_from_backend, + get_size_from_backend, schedule_delete_from_backend, get_store_from_location, get_store_from_scheme, @@ -246,12 +247,16 @@ class Controller(controller.BaseController): # don't actually care what it is at this point self.get_store_or_400(req, store) - image_meta['status'] = 'queued' + # retrieve the image size from remote store (if not provided) + image_meta['size'] = image_meta.get('size', 0) \ + or get_size_from_backend(location) + else: + # Ensure that the size attribute is set to zero for uploadable + # images (if not provided). The size will be set to a non-zero + # value during upload + image_meta['size'] = image_meta.get('size', 0) - # Ensure that the size attribute is set to zero for all - # queued instances. The size will be set to a non-zero - # value during upload - image_meta['size'] = image_meta.get('size', 0) + image_meta['status'] = 'queued' try: image_meta = registry.add_image_metadata(req.context, image_meta) diff --git a/glance/store/__init__.py b/glance/store/__init__.py index 233ad1fb..3927a3bc 100644 --- a/glance/store/__init__.py +++ b/glance/store/__init__.py @@ -131,6 +131,15 @@ def get_from_backend(uri, **kwargs): return store.get(loc) +def get_size_from_backend(uri): + """Retrieves image size from backend specified by uri""" + + store = get_store_from_uri(uri) + loc = location.get_location_from_uri(uri) + + return store.get_size(loc) + + def delete_from_backend(uri, **kwargs): """Removes chunks of data from backend specified by uri""" store = get_store_from_uri(uri) diff --git a/glance/store/base.py b/glance/store/base.py index 764ec7cd..4bbd942f 100644 --- a/glance/store/base.py +++ b/glance/store/base.py @@ -76,6 +76,17 @@ class Store(object): """ raise NotImplementedError + def get_size(self, location): + """ + Takes a `glance.store.location.Location` object that indicates + where to find the image file, and returns the size + + :param location `glance.store.location.Location` object, supplied + from glance.store.location.get_location_from_uri() + :raises `glance.exception.NotFound` if image does not exist + """ + raise NotImplementedError + def add_disabled(self, *args, **kwargs): """ Add method that raises an exception because the Store was diff --git a/glance/store/http.py b/glance/store/http.py index 1fbb1455..04697c9f 100644 --- a/glance/store/http.py +++ b/glance/store/http.py @@ -113,18 +113,34 @@ class Store(glance.store.base.Store): :param location `glance.store.location.Location` object, supplied from glance.store.location.get_location_from_uri() """ - loc = location.store_location + conn, resp, content_length = self._query(location, 'GET') - conn_class = self._get_conn_class(loc) - conn = conn_class(loc.netloc) - conn.request("GET", loc.path, "", {}) - resp = conn.getresponse() - - content_length = resp.getheader('content-length', 0) iterator = http_response_iterator(conn, resp, self.CHUNKSIZE) return (iterator, content_length) + def get_size(self, location): + """ + Takes a `glance.store.location.Location` object that indicates + where to find the image file, and returns the size + + :param location `glance.store.location.Location` object, supplied + from glance.store.location.get_location_from_uri() + """ + try: + return self._query(location, 'HEAD')[2] + except Exception: + return 0 + + def _query(self, location, verb): + loc = location.store_location + conn_class = self._get_conn_class(loc) + conn = conn_class(loc.netloc) + conn.request(verb, loc.path, "", {}) + resp = conn.getresponse() + content_length = resp.getheader('content-length', 0) + return (conn, resp, content_length) + def _get_conn_class(self, loc): """ Returns connection class for accessing the resource. Useful diff --git a/glance/store/s3.py b/glance/store/s3.py index 92470ff6..4e88afbb 100644 --- a/glance/store/s3.py +++ b/glance/store/s3.py @@ -247,6 +247,27 @@ class Store(glance.store.base.Store): from glance.store.location.get_location_from_uri() :raises `glance.exception.NotFound` if image does not exist """ + key = self._retrieve_key(location) + + key.BufferSize = self.CHUNKSIZE + return (ChunkedFile(key), key.size) + + def get_size(self, location): + """ + Takes a `glance.store.location.Location` object that indicates + where to find the image file, and returns the image_size (or 0 + if unavailable) + + :param location `glance.store.location.Location` object, supplied + from glance.store.location.get_location_from_uri() + """ + try: + key = self._retrieve_key(location) + return key.size + except Exception: + return 0 + + def _retrieve_key(self, location): loc = location.store_location from boto.s3.connection import S3Connection @@ -264,13 +285,7 @@ class Store(glance.store.base.Store): 'obj_name': loc.key}) logger.debug(msg) - #if expected_size and (key.size != expected_size): - # msg = "Expected %s bytes, got %s" % (expected_size, key.size) - # logger.error(msg) - # raise glance.store.BackendException(msg) - - key.BufferSize = self.CHUNKSIZE - return (ChunkedFile(key), key.size) + return key def add(self, image_id, image_file, image_size): """ diff --git a/glance/store/swift.py b/glance/store/swift.py index 3948eaf1..d4ac71d2 100644 --- a/glance/store/swift.py +++ b/glance/store/swift.py @@ -275,6 +275,26 @@ class Store(glance.store.base.Store): return (resp_body, resp_headers.get('content-length')) + def get_size(self, location): + """ + Takes a `glance.store.location.Location` object that indicates + where to find the image file, and returns the image_size (or 0 + if unavailable) + + :param location `glance.store.location.Location` object, supplied + from glance.store.location.get_location_from_uri() + """ + loc = location.store_location + swift_conn = self._make_swift_connection( + auth_url=loc.swift_auth_url, user=loc.user, key=loc.key) + + try: + resp_headers = swift_conn.head_object(container=loc.container, + obj=loc.obj) + return resp_headers.get('content-length', 0) + except Exception: + return 0 + def _make_swift_connection(self, auth_url, user, key): """ Creates a connection using the Swift client library. diff --git a/glance/tests/functional/test_cache_middleware.py b/glance/tests/functional/test_cache_middleware.py index 35e6af05..cba79b2e 100644 --- a/glance/tests/functional/test_cache_middleware.py +++ b/glance/tests/functional/test_cache_middleware.py @@ -42,6 +42,20 @@ FIVE_KB = 5 * 1024 class RemoteImageHandler(BaseHTTPServer.BaseHTTPRequestHandler): + def do_HEAD(s): + """ + Respond to an image HEAD request fake metadata + """ + if 'images' in s.path: + s.send_response(200) + s.send_header('Content-Type', 'application/octet-stream') + s.send_header('Content-Length', FIVE_KB) + s.end_headers() + return + else: + self.send_error(404, 'File Not Found: %s' % self.path) + return + def do_GET(s): """ Respond to an image GET request with fake image content. @@ -160,7 +174,7 @@ class BaseCacheMiddlewareTest(object): thread.start_new_thread(serve_requests, (remote_server,)) - # Add a remote image and verify a 200 OK is returned + # Add a remote image and verify a 201 Created is returned remote_uri = 'http://%s:%d/images/2' % (remote_ip, remote_port) headers = {'X-Image-Meta-Name': 'Image2', 'X-Image-Meta-Is-Public': 'True', @@ -170,21 +184,19 @@ class BaseCacheMiddlewareTest(object): response, content = http.request(path, 'POST', headers=headers) self.assertEqual(response.status, 201) data = json.loads(content) - self.assertEqual(data['image']['size'], 0) + self.assertEqual(data['image']['size'], FIVE_KB) image_id = data['image']['id'] - - # Grab the image path = "http://%s:%d/v1/images/%s" % ("0.0.0.0", self.api_port, image_id) + + # Grab the image http = httplib2.Http() response, content = http.request(path, 'GET') self.assertEqual(response.status, 200) # Grab the image again to ensure it can be served out from # cache with the correct size - path = "http://%s:%d/v1/images/%s" % ("0.0.0.0", self.api_port, - image_id) http = httplib2.Http() response, content = http.request(path, 'GET') self.assertEqual(response.status, 200) diff --git a/glance/tests/functional/test_s3.py b/glance/tests/functional/test_s3.py index 4f22b928..abc25264 100644 --- a/glance/tests/functional/test_s3.py +++ b/glance/tests/functional/test_s3.py @@ -213,9 +213,13 @@ class TestS3(test_api.TestApi): http = httplib2.Http() response, content = http.request(path, 'POST', headers=headers) self.assertEqual(response.status, 201) + # ensure data is refreshed, previously the size assertion + # applied to the metadata returned from the previous GET + data = json.loads(content) self.assertEqual(data['image']['size'], FIVE_KB) - self.assertEqual(data['image']['checksum'], - hashlib.md5(image_data).hexdigest()) + # checksum is not set for a remote image, as the image data + # is not yet retrieved + self.assertEqual(data['image']['checksum'], None) # 5. GET second image and make sure it can stream the image path = "http://%s:%d/v1/images/%s" diff --git a/glance/tests/functional/test_swift.py b/glance/tests/functional/test_swift.py index 5c4cad38..4d6de1ea 100644 --- a/glance/tests/functional/test_swift.py +++ b/glance/tests/functional/test_swift.py @@ -480,7 +480,7 @@ class TestSwift(test_api.TestApi): self.assertEqual(response.status, 201, content) data = json.loads(content) self.assertEqual(data['image']['checksum'], None) - self.assertEqual(data['image']['size'], 0) + self.assertEqual(data['image']['size'], FIVE_KB) self.assertEqual(data['image']['name'], "Image1") self.assertEqual(data['image']['is_public'], True)