Add support for image size in v2 api upload

Some backend stores e.g. RBD, will fail if told to create an
image without a valid size (RBD will fail to write to a zero-size
image). Here we add support to allow the image size to be provided
when doing an upload. The result is that the upload content-length
will be set if available either from checking the supplied file
object or as provided by user.

Closes-Bug: 1220197
Change-Id: Ia1f2ea5680a139750d931591949b3e0058148b4b

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')
+        if 'content_length' in kwargs:
-                    and method.lower() in ('post', 'put')):
+            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)
 
 

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."""

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='<IMAGE_SIZE>', 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='<IMAGE_ID>',
            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='<IMAGE_ID>', help='ID of image to delete.')

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.

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]
 

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)

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(