openstack/swift
Code Issues Proposed changes

tests: refactor SLO size/etag sysmeta tests

Browse Source 
We've been writing SLO manifests with size/etag sysmeta for more than 5
years, but we want our tests and code to continue to support the legacy
format forever.  This test infra refactor will make that easier for test
authors to opt-in testing of legacy manifests by reusing a common
pattern for manifest setup across tests.

This consolidation also cleans up some duplication where two TestCases
had identical manifest setup and paves the way to more tidying of
similar (but slightly different) manifest setup across TestCases and
sharing of setup across future TestCases.

This manifest setup standardization also adopts a consistent naming
scheme for manifest sysmeta values so test assertions are easier to read
as correct at a glance (e.g. slo_etag vs json_md5)

Additionally leak tracking is added to the common base; SLO was already
really good about *closing* requests, but in many cases seems to not
bother reading/draining them (even when they might be empty/small).

As part of the leak tracking investigation a couple new tests were added
to explore the behavior of SLO's SegmentedIterable in the
request_helpers module.

Drive-By: Fix SegmentedIterable docstring: the constructor has
expected an iterable yielding dicts, not tuples, since the
Related-Change [2].

Drive-By: remove FakeSwift's now unused "register_responses" interface
and provide "register_next_response" as a replacment.  This allows test
authors to extend the registered response for a given request key from a
common test setup into a "series of registered responses" by expressing
just the new/next response rather than forcing them to duplicate the
initial response in the explicit list passed to "register_responses".

Related-Bug: #2040178
Co-Authored-By: Alistair Coles <alistairncoles@gmail.com>
[1] Related-Change: Ia6ad32354105515560b005cea750aa64a88c96f9
[2] Related-Change-Id: Ib8dc216a84d370e6da7d6b819af79582b671d699
Change-Id: I54094f3d2098f56b755ec19cc9315d06a6ca8b15
This commit is contained in:
Clay Gerrard 2023-09-22 14:48:41 -05:00
parent 0bee335c6e
commit 3dab88bdf8
5 changed files with 2433 additions and 1021 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
swift/common/request_helpers.py
View File

@ -460,15 +460,17 @@ class SegmentedIterable(object):
:param app: WSGI application from which segments will come
:param listing_iter: iterable yielding the object segments to fetch,
along with the byte subranges to fetch, in the form of a 5-tuple
(object-path, object-etag, object-size, first-byte, last-byte).
along with the byte sub-ranges to fetch. Each yielded item should be a
dict with the following keys: ``path`` or ``raw_data``,
``first-byte``, ``last-byte``, ``hash`` (optional), ``bytes``
(optional).
If object-etag is None, no MD5 verification will be done.
If ``hash`` is None, no MD5 verification will be done.
If object-size is None, no length verification will be done.
If ``bytes`` is None, no length verification will be done.
If first-byte and last-byte are None, then the entire object will be
fetched.
If ``first-byte`` and ``last-byte`` are None, then the entire object
will be fetched.
:param max_get_time: maximum permitted duration of a GET request (seconds)
:param logger: logger object

32
test/unit/common/middleware/helpers.py
View File

@ -63,6 +63,7 @@ def normalize_query_string(qs):
if not qs:
return ''
else:
# N.B. sort params so app.call asserts can hard code qs
return '?%s' % parse.urlencode(sorted(parse.parse_qsl(qs)))
@ -149,14 +150,16 @@ class FakeSwift(object):
def _find_response(self, method, path):
path = normalize_path(path)
resp = self._responses[(method, path)]
if isinstance(resp, list):
try:
resp = resp.pop(0)
except IndexError:
raise IndexError("Didn't find any more %r "
"in allowed responses" % (
(method, path),))
resp_or_resps = self._responses[(method, path)]
if isinstance(resp_or_resps, list):
resps = resp_or_resps
if len(resps) > 1:
resp = resps.pop(0)
else:
# we'll return the last registered response forever
resp = resps[0]
else:
resp = resp_or_resps
return resp
def _select_response(self, env, method, path):
@ -333,11 +336,20 @@ class FakeSwift(object):
def register(self, method, path, response_class, headers, body=b''):
path = normalize_path(path)
# many historical tests assume this is "private" attribute is a simple
# map of tuple => tuple that they can go grubbing around in
self._responses[(method, path)] = (response_class, headers, body)
def register_responses(self, method, path, responses):
def register_next_response(self, method, path,
response_class, headers, body=b''):
path = normalize_path(path)
self._responses[(method, path)] = list(responses)
resp_key = (method, path)
next_resp = (response_class, headers, body)
# setdefault is weird; I hope this makes sense
maybe_resp = self._responses.setdefault(resp_key, [])
if isinstance(maybe_resp, tuple):
self._responses[resp_key] = [maybe_resp]
self._responses[resp_key].append(next_resp)
class FakeAppThatExcepts(object):

142
test/unit/common/middleware/test_helpers.py
View File

@ -508,3 +508,145 @@ class TestFakeSwift(unittest.TestCase):
self.assertEqual('bytes=0-2', req.headers.get('Range'))
self.assertEqual('bytes=0-2',
swift.calls_with_headers[-1].headers.get('Range'))
class TestFakeSwiftMultipleResponses(unittest.TestCase):
def test_register_response_is_forever(self):
swift = FakeSwift()
swift.register('GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
req = Request.blank('/v1/a/c/o')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
# you can get this response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
def test_register_response_is_last_response_wins(self):
swift = FakeSwift()
swift.register('GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
req = Request.blank('/v1/a/c/o')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
swift.register('GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Baz'}, b'other')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
# you can get this new response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
def test_register_next_response_is_last_response_wins(self):
swift = FakeSwift()
swift.register(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
swift.register_next_response(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Baz'}, b'other')
req = Request.blank('/v1/a/c/o')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
# you can get this new response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
def test_register_next_response_keeps_current_registered_response(self):
# we expect test authors will typically 'd register ALL their responses
# before you start calling FakeSwift
swift = FakeSwift()
swift.register(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
req = Request.blank('/v1/a/c/o')
# we get the registered response, obviously
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
# because before calling register_next_response, no resp are consumed
swift.register_next_response(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Baz'}, b'other')
# so, this is the "current" response, not the *next* response
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
# the *next* response is the next response
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
def test_register_next_response_first(self):
# you can just use register_next_response
swift = FakeSwift()
swift.register_next_response(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
swift.register_next_response(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Baz'}, b'other')
req = Request.blank('/v1/a/c/o')
# it works just like you'd called register
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
# you can get this new response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
def test_register_resets(self):
swift = FakeSwift()
swift.register_next_response(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Bar'}, b'stuff')
req = Request.blank('/v1/a/c/o')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
# you can get this response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Bar', resp.headers['X-Foo'])
# if you call register mid test you immediately reset the resp
swift.register(
'GET', '/v1/a/c/o',
HTTPOk, {'X-Foo': 'Baz'}, b'other')
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])
# you can get this new response as much as you want
for i in range(10):
resp = req.get_response(swift)
self.assertEqual(200, resp.status_int)
self.assertEqual('Baz', resp.headers['X-Foo'])

3205
test/unit/common/middleware/test_slo.py
View File

File diff suppressed because it is too large Load Diff

61
test/unit/common/test_request_helpers.py
View File

@ -16,13 +16,15 @@
"""Tests for swift.common.request_helpers"""
import unittest
from swift.common.swob import Request, HTTPException, HeaderKeyDict
from swift.common.swob import Request, HTTPException, HeaderKeyDict, HTTPOk
from swift.common.storage_policy import POLICIES, EC_POLICY, REPL_POLICY
from swift.common import request_helpers as rh
from swift.common.constraints import AUTO_CREATE_ACCOUNT_PREFIX
from test.debug_logger import debug_logger
from test.unit import patch_policies
from test.unit.common.test_utils import FakeResponse
from test.unit.common.middleware.helpers import FakeSwift
server_types = ['account', 'container', 'object']
@ -704,3 +706,60 @@ class TestHTTPResponseToDocumentIters(unittest.TestCase):
'X-Object-Meta-Color': 'blue',
})
self.assertIsNone(req.range)
class TestSegmentedIterable(unittest.TestCase):
def setUp(self):
self.logger = debug_logger()
self.app = FakeSwift()
self.expected_unread_requests = {}
def tearDown(self):
self.assertFalse(self.app.unclosed_requests)
self.assertEqual(self.app.unread_requests,
self.expected_unread_requests)
def test_simple_segments_app_iter(self):
self.app.register('GET', '/a/c/seg1', HTTPOk, {}, 'segment1')
self.app.register('GET', '/a/c/seg2', HTTPOk, {}, 'segment2')
req = Request.blank('/v1/a/c/mpu')
listing_iter = [
{'path': '/a/c/seg1', 'first_byte': None, 'last_byte': None},
{'path': '/a/c/seg2', 'first_byte': None, 'last_byte': None},
]
si = rh.SegmentedIterable(req, self.app, listing_iter, 60, self.logger,
'test-agent', 'test-source')
body = b''.join(si.app_iter)
self.assertEqual(b'segment1segment2', body)
def test_simple_segments_app_iter_ranges(self):
self.app.register('GET', '/a/c/seg1', HTTPOk, {}, 'segment1')
self.app.register('GET', '/a/c/seg2', HTTPOk, {}, 'segment2')
req = Request.blank('/v1/a/c/mpu')
listing_iter = [
{'path': '/a/c/seg1', 'first_byte': None, 'last_byte': None},
{'path': '/a/c/seg2', 'first_byte': None, 'last_byte': None},
]
si = rh.SegmentedIterable(req, self.app, listing_iter, 60, self.logger,
'test-agent', 'test-source')
body = b''.join(si.app_iter_ranges(
[(0, 8), (8, 16)], b'app/foo', b'bound', 16))
expected = b'\r\n'.join([
b'--bound',
b'Content-Type: app/foo',
b'Content-Range: bytes 0-7/16',
b'',
b'segment1',
b'--bound',
b'Content-Type: app/foo',
b'Content-Range: bytes 8-15/16',
b'',
b'segment2',
b'--bound--',
])
self.assertEqual(expected, body)
# XXX Spliterator stops SegementedIterable from asking to exhasut the
# segment response after it gets the last byte in app_iter_ranges
self.expected_unread_requests[
('GET', '/a/c/seg2?multipart-manifest=get')] = 1