diff --git a/glanceclient/common/http.py b/glanceclient/common/http.py index 0fcd2fca..2dc28408 100644 --- a/glanceclient/common/http.py +++ b/glanceclient/common/http.py @@ -280,12 +280,36 @@ 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')): + + if 'content_length' in kwargs: + content_length = kwargs.pop('content_length') + else: + content_length = None + + if (('body' in kwargs) and (hasattr(kwargs['body'], 'read') and + method.lower() in ('post', 'put'))): + + # NOTE(dosaboy): only use chunked transfer if not setting a + # content length since setting it will implicitly disable + # chunking. + + file_content_length = utils.get_file_size(kwargs['body']) + if content_length is None: + content_length = file_content_length + elif (file_content_length and + (content_length != file_content_length)): + errmsg = ("supplied content-length (%s) does not match " + "length of supplied data (%s)" % + (content_length, file_content_length)) + raise AttributeError(errmsg) + + if content_length is None: # We use 'Transfer-Encoding: chunked' because # body size may not always be known in advance. kwargs['headers']['Transfer-Encoding'] = 'chunked' + else: + kwargs['headers']['Content-Length'] = str(content_length) + return self._http_request(url, method, **kwargs) diff --git a/glanceclient/v2/images.py b/glanceclient/v2/images.py index c8349be2..e475c50d 100644 --- a/glanceclient/v2/images.py +++ b/glanceclient/v2/images.py @@ -97,18 +97,20 @@ class Controller(object): body.set_checksum(checksum) return body - def upload(self, image_id, image_data): + def upload(self, image_id, image_data, image_size=None): """ Upload the data for an image. :param image_id: ID of the image to upload data for. :param image_data: File-like object supplying the data to upload. + :param image_size: Total size in bytes of image to be uploaded. """ url = '/v2/images/%s/file' % image_id hdrs = {'Content-Type': 'application/octet-stream'} self.http_client.raw_request('PUT', url, headers=hdrs, - body=image_data) + body=image_data, + content_length=image_size) def delete(self, image_id): """Delete an image.""" diff --git a/glanceclient/v2/shell.py b/glanceclient/v2/shell.py index 8e4813d5..b21429bc 100644 --- a/glanceclient/v2/shell.py +++ b/glanceclient/v2/shell.py @@ -218,12 +218,17 @@ def do_image_download(gc, args): help=('Local file that contains disk image to be uploaded' ' during creation. Alternatively, images can be passed' ' to the client via stdin.')) +@utils.arg('--size', metavar='', type=int, + help='Size in bytes of image to be uploaded. Default is to get ' + 'size from provided data object but this is supported in case ' + 'where size cannot be inferred.', + default=None) @utils.arg('id', metavar='', help='ID of image to upload data to.') def do_image_upload(gc, args): """Upload data for a specific image.""" image_data = utils.get_data_file(args) - gc.images.upload(args.id, image_data) + gc.images.upload(args.id, image_data, args.size) @utils.arg('id', metavar='', help='ID of image to delete.') diff --git a/tests/test_http.py b/tests/test_http.py index 3ba28d2d..b5658a0d 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -16,14 +16,17 @@ import errno import socket +import mock from mox3 import mox import six from six.moves import http_client from six.moves.urllib import parse +import tempfile import testtools import glanceclient from glanceclient.common import http +from glanceclient.common import utils as client_utils from glanceclient import exc from tests import utils @@ -196,6 +199,86 @@ class TestClient(testtools.TestCase): resp, body = client.raw_request('GET', '/v1/images/detail') self.assertEqual(resp, fake) + def test_raw_request_no_content_length(self): + with tempfile.NamedTemporaryFile() as test_file: + test_file.write('abcd') + test_file.seek(0) + data_length = 4 + self.assertEqual(client_utils.get_file_size(test_file), + data_length) + + exp_resp = {'body': test_file} + exp_resp['headers'] = {'Content-Length': str(data_length), + 'Content-Type': 'application/octet-stream'} + + def mock_request(url, method, **kwargs): + return kwargs + + rq_kwargs = {'body': test_file, 'content_length': None} + + with mock.patch.object(self.client, '_http_request') as mock_rq: + mock_rq.side_effect = mock_request + resp = self.client.raw_request('PUT', '/v1/images/detail', + **rq_kwargs) + + rq_kwargs.pop('content_length') + headers = {'Content-Length': str(data_length), + 'Content-Type': 'application/octet-stream'} + rq_kwargs['headers'] = headers + + mock_rq.assert_called_once_with('/v1/images/detail', 'PUT', + **rq_kwargs) + + self.assertEqual(exp_resp, resp) + + def test_raw_request_w_content_length(self): + with tempfile.NamedTemporaryFile() as test_file: + test_file.write('abcd') + test_file.seek(0) + data_length = 4 + self.assertEqual(client_utils.get_file_size(test_file), + data_length) + + exp_resp = {'body': test_file} + # NOTE: we expect the actual file size to be overridden by the + # supplied content length. + exp_resp['headers'] = {'Content-Length': '4', + 'Content-Type': 'application/octet-stream'} + + def mock_request(url, method, **kwargs): + return kwargs + + rq_kwargs = {'body': test_file, 'content_length': data_length} + + with mock.patch.object(self.client, '_http_request') as mock_rq: + mock_rq.side_effect = mock_request + resp = self.client.raw_request('PUT', '/v1/images/detail', + **rq_kwargs) + + rq_kwargs.pop('content_length') + headers = {'Content-Length': str(data_length), + 'Content-Type': 'application/octet-stream'} + rq_kwargs['headers'] = headers + + mock_rq.assert_called_once_with('/v1/images/detail', 'PUT', + **rq_kwargs) + + self.assertEqual(exp_resp, resp) + + def test_raw_request_w_bad_content_length(self): + with tempfile.NamedTemporaryFile() as test_file: + test_file.write('abcd') + test_file.seek(0) + self.assertEqual(client_utils.get_file_size(test_file), 4) + + def mock_request(url, method, **kwargs): + return kwargs + + with mock.patch.object(self.client, '_http_request', mock_request): + self.assertRaises(AttributeError, self.client.raw_request, + 'PUT', '/v1/images/detail', body=test_file, + content_length=32) + def test_connection_refused_raw_request(self): """ Should receive a CommunicationError if connection refused. diff --git a/tests/utils.py b/tests/utils.py index 4739622b..b7ca3aff 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -26,8 +26,11 @@ class FakeAPI(object): self.fixtures = fixtures self.calls = [] - def _request(self, method, url, headers=None, body=None): + def _request(self, method, url, headers=None, body=None, + content_length=None): call = (method, url, headers or {}, body) + if content_length is not None: + call = tuple(list(call) + [content_length]) self.calls.append(call) return self.fixtures[url][method] diff --git a/tests/v2/test_images.py b/tests/v2/test_images.py index afc7f973..4dd1e707 100644 --- a/tests/v2/test_images.py +++ b/tests/v2/test_images.py @@ -474,6 +474,15 @@ class TestController(testtools.TestCase): image_data)] self.assertEqual(self.api.calls, expect) + def test_data_upload_w_size(self): + image_data = 'CCC' + image_id = '606b0e88-7c5a-4d54-b5bb-046105d4de6f' + self.controller.upload(image_id, image_data, image_size=3) + expect = [('PUT', '/v2/images/%s/file' % image_id, + {'Content-Type': 'application/octet-stream'}, + image_data, 3)] + self.assertEqual(self.api.calls, expect) + def test_data_without_checksum(self): body = self.controller.data('5cc4bebc-db27-11e1-a1eb-080027cbe205', do_checksum=False) diff --git a/tests/v2/test_shell_v2.py b/tests/v2/test_shell_v2.py index f92765ce..ff144704 100644 --- a/tests/v2/test_shell_v2.py +++ b/tests/v2/test_shell_v2.py @@ -221,13 +221,13 @@ class ShellV2Test(testtools.TestCase): self.gc.schemas.get.assert_called_once_with('test') def test_image_upload(self): - args = self._make_args({'id': 'IMG-01', 'file': 'test'}) + args = self._make_args({'id': 'IMG-01', 'file': 'test', 'size': 1024}) with mock.patch.object(self.gc.images, 'upload') as mocked_upload: utils.get_data_file = mock.Mock(return_value='testfile') mocked_upload.return_value = None test_shell.do_image_upload(self.gc, args) - mocked_upload.assert_called_once_with('IMG-01', 'testfile') + mocked_upload.assert_called_once_with('IMG-01', 'testfile', 1024) def test_image_download(self): args = self._make_args(