Merge "Chunked GET request support"
diff --git a/tempest/api/compute/admin/test_volume.py b/tempest/api/compute/admin/test_volume.py
index 2fcd053..e7c931e 100644
--- a/tempest/api/compute/admin/test_volume.py
+++ b/tempest/api/compute/admin/test_volume.py
@@ -13,8 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import io
-
 from tempest.api.compute import base
 from tempest.common import waiters
 from tempest import config
@@ -49,9 +47,11 @@
         :param return image_id: The UUID of the newly created image.
         """
         image = self.admin_image_client.show_image(CONF.compute.image_ref)
-        image_data = self.admin_image_client.show_image_file(
-            CONF.compute.image_ref).data
-        image_file = io.BytesIO(image_data)
+        # NOTE(danms): We need to stream this, so chunked=True means we get
+        # back a urllib3.HTTPResponse and have to carefully pass it to
+        # store_image_file() to upload it in pieces.
+        image_data_resp = self.admin_image_client.show_image_file(
+            CONF.compute.image_ref, chunked=True)
         create_dict = {
             'container_format': image['container_format'],
             'disk_format': image['disk_format'],
@@ -60,24 +60,22 @@
             'visibility': 'public',
         }
         create_dict.update(kwargs)
-        new_image = self.admin_image_client.create_image(**create_dict)
-        self.addCleanup(self.admin_image_client.wait_for_resource_deletion,
-                        new_image['id'])
-        self.addCleanup(self.admin_image_client.delete_image, new_image['id'])
-        self.admin_image_client.store_image_file(new_image['id'], image_file)
-
+        try:
+            new_image = self.admin_image_client.create_image(**create_dict)
+            self.addCleanup(self.admin_image_client.wait_for_resource_deletion,
+                            new_image['id'])
+            self.addCleanup(
+                self.admin_image_client.delete_image, new_image['id'])
+            self.admin_image_client.store_image_file(new_image['id'],
+                                                     image_data_resp)
+        finally:
+            image_data_resp.release_conn()
         return new_image['id']
 
 
 class AttachSCSIVolumeTestJSON(BaseAttachSCSIVolumeTest):
     """Test attaching scsi volume to server"""
 
-    # NOTE(gibi): https://bugs.launchpad.net/nova/+bug/2002951/comments/5 shows
-    # that calling _create_image_with_custom_property can cause excessive
-    # memory usage in the test executor as it downloads a glance image in
-    # memory. This is causing gate failures so the test is disabled. One
-    # potential fix is to do a chunked data download / upload loop instead.
-    @decorators.skip_because(bug="2002951", condition=True)
     @decorators.idempotent_id('777e468f-17ca-4da4-b93d-b7dbf56c0494')
     def test_attach_scsi_disk_with_config_drive(self):
         """Test the attach/detach volume with config drive/scsi disk
diff --git a/tempest/lib/common/http.py b/tempest/lib/common/http.py
index 33f871b..d163968 100644
--- a/tempest/lib/common/http.py
+++ b/tempest/lib/common/http.py
@@ -60,7 +60,12 @@
             retry = urllib3.util.Retry(redirect=False)
         r = super(ClosingProxyHttp, self).request(method, url, retries=retry,
                                                   *args, **new_kwargs)
-        return Response(r), r.data
+        if not kwargs.get('preload_content', True):
+            # This means we asked urllib3 for streaming content, so we
+            # need to return the raw response and not read any data yet
+            return r, b''
+        else:
+            return Response(r), r.data
 
 
 class ClosingHttp(urllib3.poolmanager.PoolManager):
@@ -109,4 +114,9 @@
             retry = urllib3.util.Retry(redirect=False)
         r = super(ClosingHttp, self).request(method, url, retries=retry,
                                              *args, **new_kwargs)
-        return Response(r), r.data
+        if not kwargs.get('preload_content', True):
+            # This means we asked urllib3 for streaming content, so we
+            # need to return the raw response and not read any data yet
+            return r, b''
+        else:
+            return Response(r), r.data
diff --git a/tempest/lib/common/rest_client.py b/tempest/lib/common/rest_client.py
index a11b7c1..6cf5b73 100644
--- a/tempest/lib/common/rest_client.py
+++ b/tempest/lib/common/rest_client.py
@@ -19,6 +19,7 @@
 import re
 import time
 import urllib
+import urllib3
 
 import jsonschema
 from oslo_log import log as logging
@@ -298,7 +299,7 @@
         """
         return self.request('POST', url, extra_headers, headers, body, chunked)
 
-    def get(self, url, headers=None, extra_headers=False):
+    def get(self, url, headers=None, extra_headers=False, chunked=False):
         """Send a HTTP GET request using keystone service catalog and auth
 
         :param str url: the relative url to send the get request to
@@ -307,11 +308,19 @@
                                    returned by the get_headers() method are to
                                    be used but additional headers are needed in
                                    the request pass them in as a dict.
+        :param bool chunked: Boolean value that indicates if we should stream
+                             the response instead of reading it all at once.
+                             If True, data will be empty and the raw urllib3
+                             response object will be returned.
+                             NB: If you pass True here, you **MUST** call
+                             release_conn() on the response object before
+                             finishing!
         :return: a tuple with the first entry containing the response headers
                  and the second the response body
         :rtype: tuple
         """
-        return self.request('GET', url, extra_headers, headers)
+        return self.request('GET', url, extra_headers, headers,
+                            chunked=chunked)
 
     def delete(self, url, headers=None, body=None, extra_headers=False):
         """Send a HTTP DELETE request using keystone service catalog and auth
@@ -480,7 +489,7 @@
         self.LOG.info(
             'Request (%s): %s %s %s%s',
             caller_name,
-            resp['status'],
+            resp.status,
             method,
             req_url,
             secs,
@@ -617,17 +626,30 @@
         """
         if headers is None:
             headers = self.get_headers()
+        # In urllib3, chunked only affects the upload. However, we may
+        # want to read large responses to GET incrementally. Re-purpose
+        # chunked=True on a GET to also control how we handle the response.
+        preload = not (method.lower() == 'get' and chunked)
+        if not preload:
+            # NOTE(danms): Not specifically necessary, but don't send
+            # chunked=True to urllib3 on a GET, since it is technically
+            # for PUT/POST type operations
+            chunked = False
         # Do the actual request, and time it
         start = time.time()
         self._log_request_start(method, url)
         resp, resp_body = self.http_obj.request(
             url, method, headers=headers,
-            body=body, chunked=chunked)
+            body=body, chunked=chunked, preload_content=preload)
         end = time.time()
         req_body = body if log_req_body is None else log_req_body
-        self._log_request(method, url, resp, secs=(end - start),
-                          req_headers=headers, req_body=req_body,
-                          resp_body=resp_body)
+        if preload:
+            # NOTE(danms): If we are reading the whole response, we can do
+            # this logging. If not, skip the logging because it will result
+            # in us reading the response data prematurely.
+            self._log_request(method, url, resp, secs=(end - start),
+                              req_headers=headers, req_body=req_body,
+                              resp_body=resp_body)
         return resp, resp_body
 
     def request(self, method, url, extra_headers=False, headers=None,
@@ -773,6 +795,10 @@
         # resp this could possibly fail
         if str(type(resp)) == "<type 'instance'>":
             ctype = resp.getheader('content-type')
+        elif isinstance(resp, urllib3.HTTPResponse):
+            # If we requested chunked=True streaming, this will be a raw
+            # urllib3.HTTPResponse
+            ctype = resp.getheaders()['content-type']
         else:
             try:
                 ctype = resp['content-type']
diff --git a/tempest/lib/services/image/v2/images_client.py b/tempest/lib/services/image/v2/images_client.py
index ae6ce25..8460b57 100644
--- a/tempest/lib/services/image/v2/images_client.py
+++ b/tempest/lib/services/image/v2/images_client.py
@@ -248,17 +248,26 @@
         self.expected_success(202, resp.status)
         return rest_client.ResponseBody(resp)
 
-    def show_image_file(self, image_id):
+    def show_image_file(self, image_id, chunked=False):
         """Download binary image data.
 
+        :param bool chunked: If True, do not read the body and return only
+                             the raw urllib3 response object for processing.
+                             NB: If you pass True here, you **MUST** call
+                             release_conn() on the response object before
+                             finishing!
+
         For a full list of available parameters, please refer to the official
         API reference:
         https://docs.openstack.org/api-ref/image/v2/#download-binary-image-data
         """
         url = 'images/%s/file' % image_id
-        resp, body = self.get(url)
+        resp, body = self.get(url, chunked=chunked)
         self.expected_success([200, 204, 206], resp.status)
-        return rest_client.ResponseBodyData(resp, body)
+        if chunked:
+            return resp
+        else:
+            return rest_client.ResponseBodyData(resp, body)
 
     def add_image_tag(self, image_id, tag):
         """Add an image tag.
diff --git a/tempest/tests/lib/common/test_http.py b/tempest/tests/lib/common/test_http.py
index a19153f..aae6ba2 100644
--- a/tempest/tests/lib/common/test_http.py
+++ b/tempest/tests/lib/common/test_http.py
@@ -149,6 +149,31 @@
              'xtra key': 'Xtra Value'},
             response)
 
+    def test_request_preload(self):
+        # Given
+        connection = self.closing_http()
+        headers = {'Xtra Key': 'Xtra Value'}
+        http_response = urllib3.HTTPResponse(headers=headers)
+        request = self.patch('urllib3.PoolManager.request',
+                             return_value=http_response)
+        retry = self.patch('urllib3.util.Retry')
+
+        # When
+        response, _ = connection.request(
+            method=REQUEST_METHOD,
+            url=REQUEST_URL,
+            headers=headers,
+            preload_content=False)
+
+        # Then
+        request.assert_called_once_with(
+            REQUEST_METHOD,
+            REQUEST_URL,
+            headers=dict(headers, connection='close'),
+            preload_content=False,
+            retries=retry(raise_on_redirect=False, redirect=5))
+        self.assertIsInstance(response, urllib3.HTTPResponse)
+
 
 class TestClosingProxyHttp(TestClosingHttp):
 
diff --git a/tempest/tests/lib/common/test_rest_client.py b/tempest/tests/lib/common/test_rest_client.py
index 910756f..81a76e0 100644
--- a/tempest/tests/lib/common/test_rest_client.py
+++ b/tempest/tests/lib/common/test_rest_client.py
@@ -55,6 +55,7 @@
     def test_get(self):
         __, return_dict = self.rest_client.get(self.url)
         self.assertEqual('GET', return_dict['method'])
+        self.assertTrue(return_dict['preload_content'])
 
     def test_delete(self):
         __, return_dict = self.rest_client.delete(self.url)
@@ -78,6 +79,17 @@
         __, return_dict = self.rest_client.copy(self.url)
         self.assertEqual('COPY', return_dict['method'])
 
+    def test_get_chunked(self):
+        self.useFixture(fixtures.MockPatchObject(self.rest_client,
+                                                 '_log_request'))
+        __, return_dict = self.rest_client.get(self.url, chunked=True)
+        # Default is preload_content=True, make sure we passed False
+        self.assertFalse(return_dict['preload_content'])
+        # Make sure we did not pass chunked=True to urllib3 for GET
+        self.assertFalse(return_dict['chunked'])
+        # Make sure we did not call _log_request() on the raw response
+        self.rest_client._log_request.assert_not_called()
+
 
 class TestRestClientNotFoundHandling(BaseRestClientTestClass):
     def setUp(self):
diff --git a/tempest/tests/lib/fake_http.py b/tempest/tests/lib/fake_http.py
index cfa4b93..5fa0c43 100644
--- a/tempest/tests/lib/fake_http.py
+++ b/tempest/tests/lib/fake_http.py
@@ -21,14 +21,17 @@
         self.return_type = return_type
 
     def request(self, uri, method="GET", body=None, headers=None,
-                redirections=5, connection_type=None, chunked=False):
+                redirections=5, connection_type=None, chunked=False,
+                preload_content=False):
         if not self.return_type:
             fake_headers = fake_http_response(headers)
             return_obj = {
                 'uri': uri,
                 'method': method,
                 'body': body,
-                'headers': headers
+                'headers': headers,
+                'chunked': chunked,
+                'preload_content': preload_content,
             }
             return (fake_headers, return_obj)
         elif isinstance(self.return_type, int):
diff --git a/tempest/tests/lib/services/image/v2/test_images_client.py b/tempest/tests/lib/services/image/v2/test_images_client.py
index 5b162f8..27a50a9 100644
--- a/tempest/tests/lib/services/image/v2/test_images_client.py
+++ b/tempest/tests/lib/services/image/v2/test_images_client.py
@@ -13,6 +13,9 @@
 #    under the License.
 
 import io
+from unittest import mock
+
+import fixtures
 
 from tempest.lib.common.utils import data_utils
 from tempest.lib.services.image.v2 import images_client
@@ -239,6 +242,21 @@
             headers={'Content-Type': 'application/octet-stream'},
             status=200)
 
+    def test_show_image_file_chunked(self):
+        # Since chunked=True on a GET should pass the response object
+        # basically untouched, we use a mock here so we get some assurances.
+        http_response = mock.MagicMock()
+        http_response.status = 200
+        self.useFixture(fixtures.MockPatch(
+            'tempest.lib.common.rest_client.RestClient.get',
+            return_value=(http_response, b'')))
+        resp = self.client.show_image_file(
+            self.FAKE_CREATE_UPDATE_SHOW_IMAGE['id'],
+            chunked=True)
+        self.assertEqual(http_response, resp)
+        resp.__contains__.assert_not_called()
+        resp.__getitem__.assert_not_called()
+
     def test_add_image_tag(self):
         self.check_service_client_function(
             self.client.add_image_tag,