From 82169ead1cc1c60318507fc3f0fbd7deff0f15e9 Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Mon, 24 Jun 2019 16:55:59 -0500 Subject: [PATCH] Don't handle object without container Closes-Bug: #1833616 Change-Id: I16ca1589abcea2bca942da7e97719286a8961ea6 --- swift/proxy/controllers/base.py | 4 +- swift/proxy/server.py | 3 + test/unit/proxy/test_server.py | 108 ++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 2 deletions(-) diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index a56a81fe04..e3e0ad4e23 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -367,7 +367,7 @@ def get_container_info(env, app, swift_source=None): if info: info = deepcopy(info) # avoid mutating what's in swift.infocache else: - info = headers_to_container_info({}, 0) + info = headers_to_container_info({}, 503) # Old data format in memcache immediately after a Swift upgrade; clean # it up so consumers of get_container_info() aren't exposed to it. @@ -432,7 +432,7 @@ def get_account_info(env, app, swift_source=None): if info: info = info.copy() # avoid mutating what's in swift.infocache else: - info = headers_to_account_info({}, 0) + info = headers_to_account_info({}, 503) for field in ('container_count', 'bytes', 'total_object_count'): if info.get(field) is None: diff --git a/swift/proxy/server.py b/swift/proxy/server.py index 7c8fbef502..4415a82571 100644 --- a/swift/proxy/server.py +++ b/swift/proxy/server.py @@ -29,6 +29,7 @@ from eventlet import Timeout from swift import __canonical_version__ as swift_version from swift.common import constraints +from swift.common.http import is_server_error from swift.common.storage_policy import POLICIES from swift.common.ring import Ring from swift.common.utils import cache_from_env, get_logger, \ @@ -403,6 +404,8 @@ class Application(object): raise APIVersionError('Invalid path') if obj and container and account: info = get_container_info(req.environ, self) + if is_server_error(info.get('status')): + raise HTTPServiceUnavailable(request=req) policy_index = req.headers.get('X-Backend-Storage-Policy-Index', info['storage_policy']) policy = POLICIES.get_by_index(policy_index) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index 6732b2a491..5c5fa54abe 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -4625,6 +4625,114 @@ class TestReplicatedObjectController( resp = controller.POST(req) self.assertEqual(resp.status_int, 404) + def test_PUT_object_to_container_does_not_exist(self): + self.app.container_ring.max_more_nodes = 3 # that's 3 handoffs + + # no container found anywhere! + req = Request.blank('/v1/a/c/o', method='PUT') + with mocked_http_conn(*([200] + [404] * 6)) as fake_conn: + resp = req.get_response(self.app) + # object create returns error + self.assertEqual(resp.status_int, 404) + self.assertEqual(['HEAD'] * 7, + [r['method'] for r in fake_conn.requests]) + self.assertEqual(['/a'] + ['/a/c'] * 6, [ + r['path'][len('/sdX/0'):] for r in fake_conn.requests]) + + def test_PUT_object_to_container_exist_on_handoff(self): + self.app.container_ring.max_more_nodes = 3 # that's 3 handoffs + + # finally get info after three requests + req = Request.blank('/v1/a/c/o', method='PUT', content_length=0) + account_status = [200] + container_status = ([404] * 5) + [200] + object_status = [201, 201, 201] + status = account_status + container_status + object_status + with mocked_http_conn(*status) as fake_conn: + resp = req.get_response(self.app) + # object created + self.assertEqual(resp.status_int, 201) + + account_requests = fake_conn.requests[:len(account_status)] + self.assertEqual(['HEAD'], + [r['method'] for r in account_requests]) + self.assertEqual(['/a'], [ + r['path'][len('/sdX/0'):] for r in account_requests]) + + container_requests = fake_conn.requests[ + len(account_status):len(account_status) + len(container_status)] + self.assertEqual(['HEAD'] * 6, + [r['method'] for r in container_requests]) + self.assertEqual(['/a/c'] * 6, [ + r['path'][len('/sdX/0'):] for r in container_requests]) + + obj_requests = fake_conn.requests[ + len(account_status) + len(container_status):] + self.assertEqual(['PUT'] * 3, + [r['method'] for r in obj_requests]) + self.assertEqual(['/a/c/o'] * 3, [ + r['path'][len('/sdX/0'):] for r in obj_requests]) + + def test_PUT_object_to_primary_timeout_container_exist(self): + self.app.container_ring.max_more_nodes = 3 # that's 3 handoffs + + req = Request.blank('/v1/a/c/o', method='PUT', content_length=0) + account_status = [200] + # no response from primaries but container exists on a handoff! + container_status = ([Timeout()] * 3) + [200] + object_status = [201, 201, 201] + status = account_status + container_status + object_status + with mocked_http_conn(*status) as fake_conn: + resp = req.get_response(self.app) + # object created + self.assertEqual(resp.status_int, 201) + + account_requests = fake_conn.requests[:len(account_status)] + self.assertEqual(['HEAD'], + [r['method'] for r in account_requests]) + self.assertEqual(['/a'], [ + r['path'][len('/sdX/0'):] for r in account_requests]) + + container_requests = fake_conn.requests[ + len(account_status):len(account_status) + len(container_status)] + self.assertEqual(['HEAD'] * 4, + [r['method'] for r in container_requests]) + self.assertEqual(['/a/c'] * 4, [ + r['path'][len('/sdX/0'):] for r in container_requests]) + + obj_requests = fake_conn.requests[ + len(account_status) + len(container_status):] + self.assertEqual(['PUT'] * 3, + [r['method'] for r in obj_requests]) + self.assertEqual(['/a/c/o'] * 3, [ + r['path'][len('/sdX/0'):] for r in obj_requests]) + + def test_PUT_object_to_all_containers_error(self): + self.app.container_ring.max_more_nodes = 2 # 2 handoffs + + req = Request.blank('/v1/a/c/o', method='PUT', content_length=0) + account_status = [200] + container_status = [503] * 5 # 3 replicas + 2 handoffs + status = account_status + container_status + with mocked_http_conn(*status) as fake_conn: + resp = req.get_response(self.app) + + account_requests = fake_conn.requests[:len(account_status)] + self.assertEqual(['HEAD'], + [r['method'] for r in account_requests]) + self.assertEqual(['/a'], [ + r['path'][len('/sdX/0'):] for r in account_requests]) + + container_requests = fake_conn.requests[ + len(account_status):len(account_status) + len(container_status)] + self.assertEqual(['HEAD'] * 5, + [r['method'] for r in container_requests]) + self.assertEqual(['/a/c'] * 5, [ + r['path'][len('/sdX/0'):] for r in container_requests]) + + # object is not created! + self.assertEqual(resp.status_int, 503) + def test_bad_metadata(self): with save_globals(): controller = ReplicatedObjectController(