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,