From 727aadbc257ec3c99dd1621202948d288d45c8cc Mon Sep 17 00:00:00 2001 From: Stuart McLaren Date: Wed, 26 Sep 2012 12:56:51 +0000 Subject: [PATCH] Handle create/update of images with unknown size It may not be possible to know in advance the total size of image data which is to be uploaded, for example if the data is being piped to stdin. To handle this we use HTTP Transfer-Encoding: chunked and do not set any image size headers. Various subtly different cases needed to be handled for both image-create and image-update, including: * input from named pipe * piped input of zero size * regular file of zero length Fix for bug 1056220. Change-Id: I0c7f0a64d883e058993b954a1c465c5b057f2bcf --- glanceclient/common/http.py | 20 +++++++++++++++++++- glanceclient/v1/images.py | 20 ++++++++++---------- glanceclient/v1/shell.py | 17 ++++++++++++++--- tests/v1/test_images.py | 2 +- 4 files changed, 44 insertions(+), 15 deletions(-) diff --git a/glanceclient/common/http.py b/glanceclient/common/http.py index db1acefa..bc8d23a5 100644 --- a/glanceclient/common/http.py +++ b/glanceclient/common/http.py @@ -146,7 +146,19 @@ class HTTPClient(object): try: conn_url = os.path.normpath('%s/%s' % (self.endpoint_path, url)) - conn.request(method, conn_url, **kwargs) + if kwargs['headers'].get('Transfer-Encoding') == 'chunked': + conn.putrequest(method, conn_url) + for header, value in kwargs['headers'].items(): + conn.putheader(header, value) + conn.endheaders() + chunk = kwargs['body'].read(CHUNKSIZE) + # Chunk it, baby... + while chunk: + conn.send('%x\r\n%s\r\n' % (len(chunk), chunk)) + chunk = kwargs['body'].read(CHUNKSIZE) + conn.send('0\r\n\r\n') + else: + conn.request(method, conn_url, **kwargs) resp = conn.getresponse() except socket.gaierror as e: message = "Error finding address for %(url)s: %(e)s" % locals() @@ -201,6 +213,12 @@ class HTTPClient(object): kwargs.setdefault('headers', {}) kwargs['headers'].setdefault('Content-Type', 'application/octet-stream') + if 'body' in kwargs: + if (hasattr(kwargs['body'], 'read') + and method.lower() in ('post', 'put')): + # We use 'Transfer-Encoding: chunked' because + # body size may not always be known in advance. + kwargs['headers']['Transfer-Encoding'] = 'chunked' return self._http_request(url, method, **kwargs) diff --git a/glanceclient/v1/images.py b/glanceclient/v1/images.py index e2b661dc..e8337322 100644 --- a/glanceclient/v1/images.py +++ b/glanceclient/v1/images.py @@ -177,10 +177,15 @@ class ImageManager(base.Manager): # Illegal seek. This means the user is trying # to pipe image data to the client, e.g. # echo testdata | bin/glance add blah..., or - # that stdin is empty - return 0 + # that stdin is empty, or that a file-like + # object which doesn't support 'seek/tell' has + # been supplied. + return None else: raise + else: + # Cannot determine size of input image + return None def create(self, **kwargs): """Create an image @@ -190,10 +195,8 @@ class ImageManager(base.Manager): image_data = kwargs.pop('data', None) if image_data is not None: image_size = self._get_file_size(image_data) - if image_size != 0: + if image_size is not None: kwargs.setdefault('size', image_size) - else: - image_data = None fields = {} for field in kwargs: @@ -218,16 +221,13 @@ class ImageManager(base.Manager): TODO(bcwaldon): document accepted params """ - hdrs = {} image_data = kwargs.pop('data', None) if image_data is not None: image_size = self._get_file_size(image_data) - if image_size != 0: + if image_size is not None: kwargs.setdefault('size', image_size) - hdrs['Content-Length'] = image_size - else: - image_data = None + hdrs = {} try: purge_props = 'true' if kwargs.pop('purge_props') else 'false' except KeyError: diff --git a/glanceclient/v1/shell.py b/glanceclient/v1/shell.py index df2dbd7a..4704dc1b 100644 --- a/glanceclient/v1/shell.py +++ b/glanceclient/v1/shell.py @@ -81,9 +81,20 @@ def _set_data_field(fields, args): if args.file: fields['data'] = open(args.file, 'rb') else: - if msvcrt: - msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY) - fields['data'] = sys.stdin + # We distinguish between cases where image data is pipelined: + # (1) glance ... < /tmp/file or cat /tmp/file | glance ... + # and cases where no image data is provided: + # (2) glance ... + if (sys.stdin.isatty() is not True): + # Our input is from stdin, and we are part of + # a pipeline, so data may be present. (We are of + # type (1) above.) + if msvcrt: + msvcrt.setmode(sys.stdin.fileno(), os.O_BINARY) + fields['data'] = sys.stdin + else: + # We are of type (2) above, no image data supplied + fields['data'] = None @utils.arg('id', metavar='', help='ID of image to describe.') diff --git a/tests/v1/test_images.py b/tests/v1/test_images.py index 0aa8b2b8..759010f2 100644 --- a/tests/v1/test_images.py +++ b/tests/v1/test_images.py @@ -390,7 +390,7 @@ class ImageManagerTest(unittest.TestCase): def test_update_with_data(self): image_data = StringIO.StringIO('XXX') self.mgr.update('1', data=image_data) - expect_headers = {'x-image-meta-size': '3', 'Content-Length': 3} + expect_headers = {'x-image-meta-size': '3'} expect = [('PUT', '/v1/images/1', expect_headers, image_data)] self.assertEqual(self.api.calls, expect)