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
This commit is contained in:
parent
b4624ec242
commit
3956c546fb
@ -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)
|
||||
|
@ -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)
|
||||
|
@ -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
|
||||
|
@ -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
|
||||
|
@ -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):
|
||||
"""
|
||||
|
@ -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.
|
||||
|
@ -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)
|
||||
|
@ -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"
|
||||
|
@ -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)
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user