From 26c3d819b0c14422b8e0bd5ec8c3c6f5d4fdb3ab Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Wed, 7 Jul 2021 14:31:38 -0500 Subject: [PATCH] Drain and close more internal client requests Specifically avoid logging a 499 when object-expirer calls swift.delete_container after clearing a bucket Related-Change-Id: I1d5c6e40e1833c53994f8e15b4850871789413ac Change-Id: Ic82f8ba706a8b6889b37ec4fa40b24bdad3c3521 --- swift/common/internal_client.py | 27 +++--- test/unit/common/test_internal_client.py | 108 +++++++++++++++++++++++ 2 files changed, 123 insertions(+), 12 deletions(-) diff --git a/swift/common/internal_client.py b/swift/common/internal_client.py index 89a26175a8..bd9452ec54 100644 --- a/swift/common/internal_client.py +++ b/swift/common/internal_client.py @@ -234,6 +234,12 @@ class InternalClient(object): # To make pep8 tool happy, in place of raise t, v, tb: six.reraise(exc_type, exc_value, exc_traceback) + def handle_request(self, *args, **kwargs): + resp = self.make_request(*args, **kwargs) + # Drain the response body to prevent unexpected disconnect + # in proxy-server + drain_and_close(resp) + def _get_metadata( self, path, metadata_prefix='', acceptable_statuses=(2,), headers=None, params=None): @@ -363,7 +369,7 @@ class InternalClient(object): headers[k] = v else: headers['%s%s' % (metadata_prefix, k)] = v - self.make_request('POST', path, headers, acceptable_statuses) + self.handle_request('POST', path, headers, acceptable_statuses) # account methods @@ -402,7 +408,7 @@ class InternalClient(object): unexpected way. """ path = self.make_path(account) - self.make_request('PUT', path, {}, (201, 202)) + self.handle_request('PUT', path, {}, (201, 202)) def delete_account(self, account, acceptable_statuses=(2, HTTP_NOT_FOUND)): """ @@ -417,7 +423,7 @@ class InternalClient(object): unexpected way. """ path = self.make_path(account) - self.make_request('DELETE', path, {}, acceptable_statuses) + self.handle_request('DELETE', path, {}, acceptable_statuses) def get_account_info( self, account, acceptable_statuses=(2, HTTP_NOT_FOUND)): @@ -531,7 +537,7 @@ class InternalClient(object): headers = headers or {} path = self.make_path(account, container) - self.make_request('PUT', path, headers, acceptable_statuses) + self.handle_request('PUT', path, headers, acceptable_statuses) def delete_container( self, account, container, headers=None, @@ -552,7 +558,7 @@ class InternalClient(object): headers = headers or {} path = self.make_path(account, container) - self.make_request('DELETE', path, headers, acceptable_statuses) + self.handle_request('DELETE', path, headers, acceptable_statuses) def get_container_metadata( self, account, container, metadata_prefix='', @@ -655,11 +661,8 @@ class InternalClient(object): """ path = self.make_path(account, container, obj) - resp = self.make_request('DELETE', path, (headers or {}), - acceptable_statuses) - # Drain the response body to prevent unexpected disconnect - # in proxy-server - drain_and_close(resp) + self.handle_request('DELETE', path, (headers or {}), + acceptable_statuses) def get_object_metadata( self, account, container, obj, metadata_prefix='', @@ -812,8 +815,8 @@ class InternalClient(object): if 'Content-Length' not in headers: headers['Transfer-Encoding'] = 'chunked' path = self.make_path(account, container, obj) - self.make_request('PUT', path, headers, acceptable_statuses, fobj, - params=params) + self.handle_request('PUT', path, headers, acceptable_statuses, fobj, + params=params) def get_auth(url, user, key, auth_version='1.0', **kwargs): diff --git a/test/unit/common/test_internal_client.py b/test/unit/common/test_internal_client.py index e22fea4aa7..a3bc6c7a7d 100644 --- a/test/unit/common/test_internal_client.py +++ b/test/unit/common/test_internal_client.py @@ -1019,6 +1019,20 @@ class TestInternalClient(unittest.TestCase): ret_items.append(container) self.assertEqual(items, ret_items) + def test_create_account(self): + account, container, obj = path_parts() + path = make_path_info(account) + client, app = get_client_app() + app.register('PUT', path, swob.HTTPCreated, {}) + client.create_account(account) + self.assertEqual([('PUT', path, { + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'User-Agent': 'test' + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + def test_delete_account(self): account, container, obj = path_parts() path = make_path_info(account) @@ -1026,6 +1040,8 @@ class TestInternalClient(unittest.TestCase): app.register('DELETE', path, swob.HTTPNoContent, {}) client.delete_account(account) self.assertEqual(1, len(app._calls)) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) def test_get_account_info(self): class Response(object): @@ -1115,6 +1131,22 @@ class TestInternalClient(unittest.TestCase): acceptable_statuses=(2, 4)) def test_set_account_metadata(self): + account, container, obj = path_parts() + path = make_path_info(account) + client, app = get_client_app() + app.register('POST', path, swob.HTTPAccepted, {}) + client.set_account_metadata(account, {'Color': 'Blue'}, + metadata_prefix='X-Account-Meta-') + self.assertEqual([('POST', path, { + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'X-Account-Meta-Color': 'Blue', + 'User-Agent': 'test', + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_set_account_metadata_plumbing(self): account, container, obj = path_parts() path = make_path(account) metadata = 'some_metadata' @@ -1161,6 +1193,20 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(1, client.make_request_called) def test_create_container(self): + account, container, obj = path_parts() + path = make_path_info(account, container) + client, app = get_client_app() + app.register('PUT', path, swob.HTTPCreated, {}) + client.create_container(account, container) + self.assertEqual([('PUT', path, { + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'User-Agent': 'test' + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_create_container_plumbing(self): class InternalClient(internal_client.InternalClient): def __init__(self, test, path, headers): self.test = test @@ -1186,6 +1232,16 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(1, client.make_request_called) def test_delete_container(self): + account, container, obj = path_parts() + path = make_path_info(account, container) + client, app = get_client_app() + app.register('DELETE', path, swob.HTTPNoContent, {}) + client.delete_container(account, container) + self.assertEqual(1, len(app._calls)) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_delete_container_plumbing(self): class InternalClient(internal_client.InternalClient): def __init__(self, test, path): self.test = test @@ -1238,6 +1294,22 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(items, ret_items) def test_set_container_metadata(self): + account, container, obj = path_parts() + path = make_path_info(account, container) + client, app = get_client_app() + app.register('POST', path, swob.HTTPAccepted, {}) + client.set_container_metadata(account, container, {'Color': 'Blue'}, + metadata_prefix='X-Container-Meta-') + self.assertEqual([('POST', path, { + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'X-Container-Meta-Color': 'Blue', + 'User-Agent': 'test', + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_set_container_metadata_plumbing(self): account, container, obj = path_parts() path = make_path(account, container) metadata = 'some_metadata' @@ -1257,10 +1329,15 @@ class TestInternalClient(unittest.TestCase): client.delete_object(account, container, obj) self.assertEqual(app.unclosed_requests, {}) self.assertEqual(1, len(app._calls)) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + app.register('DELETE', path, swob.HTTPNotFound, {}) client.delete_object(account, container, obj) self.assertEqual(app.unclosed_requests, {}) self.assertEqual(2, len(app._calls)) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) def test_get_object_metadata(self): account, container, obj = path_parts() @@ -1383,6 +1460,22 @@ class TestInternalClient(unittest.TestCase): self.assertEqual([], lines) def test_set_object_metadata(self): + account, container, obj = path_parts() + path = make_path_info(account, container, obj) + client, app = get_client_app() + app.register('POST', path, swob.HTTPAccepted, {}) + client.set_object_metadata(account, container, obj, {'Color': 'Blue'}, + metadata_prefix='X-Object-Meta-') + self.assertEqual([('POST', path, { + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'X-Object-Meta-Color': 'Blue', + 'User-Agent': 'test', + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_set_object_metadata_plumbing(self): account, container, obj = path_parts() path = make_path(account, container, obj) metadata = 'some_metadata' @@ -1396,6 +1489,21 @@ class TestInternalClient(unittest.TestCase): self.assertEqual(1, client.set_metadata_called) def test_upload_object(self): + account, container, obj = path_parts() + path = make_path_info(account, container, obj) + client, app = get_client_app() + app.register('PUT', path, swob.HTTPCreated, {}) + client.upload_object(BytesIO(b'fobj'), account, container, obj) + self.assertEqual([('PUT', path, { + 'Transfer-Encoding': 'chunked', + 'X-Backend-Allow-Reserved-Names': 'true', + 'Host': 'localhost:80', + 'User-Agent': 'test' + })], app._calls) + self.assertEqual({}, app.unread_requests) + self.assertEqual({}, app.unclosed_requests) + + def test_upload_object_plumbing(self): class InternalClient(internal_client.InternalClient): def __init__(self, test, path, headers, fobj): self.test = test