Chunked GET request support
In one test, we are downloading the entire image (into memory) and
re-uploading it. That works when the image is 16MiB but not when it
is 1GiB. This adds support to the internal http client for chunked
downloads (similar to upload), makes the image client able to take
that flag, and finally makes the offending test do a chunked upload/
download streaming operation.
Note this un-skips the test, effectively reverting a6b7e334c
because the test should no longer consume large amounts of memory.
Related-Bug: #2002951
Change-Id: I31e537538a1862e71091aa470da3b8e9c799bf15
This commit is contained in:
parent
1982a60dbb
commit
2c192f46db
|
@ -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 @@ class BaseAttachSCSIVolumeTest(base.BaseV2ComputeAdminTest):
|
|||
: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 @@ class BaseAttachSCSIVolumeTest(base.BaseV2ComputeAdminTest):
|
|||
'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
|
||||
|
|
|
@ -60,7 +60,12 @@ class ClosingProxyHttp(urllib3.ProxyManager):
|
|||
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 @@ class ClosingHttp(urllib3.poolmanager.PoolManager):
|
|||
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
|
||||
|
|
|
@ -19,6 +19,7 @@ import email.utils
|
|||
import re
|
||||
import time
|
||||
import urllib
|
||||
import urllib3
|
||||
|
||||
import jsonschema
|
||||
from oslo_log import log as logging
|
||||
|
@ -298,7 +299,7 @@ class RestClient(object):
|
|||
"""
|
||||
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 @@ class RestClient(object):
|
|||
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 @@ class RestClient(object):
|
|||
self.LOG.info(
|
||||
'Request (%s): %s %s %s%s',
|
||||
caller_name,
|
||||
resp['status'],
|
||||
resp.status,
|
||||
method,
|
||||
req_url,
|
||||
secs,
|
||||
|
@ -617,17 +626,30 @@ class RestClient(object):
|
|||
"""
|
||||
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 @@ class RestClient(object):
|
|||
# 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']
|
||||
|
|
|
@ -248,17 +248,26 @@ class ImagesClient(rest_client.RestClient):
|
|||
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.
|
||||
|
|
|
@ -149,6 +149,31 @@ class TestClosingHttp(base.TestCase):
|
|||
'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):
|
||||
|
||||
|
|
|
@ -55,6 +55,7 @@ class TestRestClientHTTPMethods(BaseRestClientTestClass):
|
|||
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 @@ class TestRestClientHTTPMethods(BaseRestClientTestClass):
|
|||
__, 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):
|
||||
|
|
|
@ -21,14 +21,17 @@ class fake_httplib2(object):
|
|||
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):
|
||||
|
|
|
@ -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 @@ class TestImagesClient(base.BaseServiceTest):
|
|||
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,
|
||||
|
|
Loading…
Reference in New Issue