From fd3acd2e5944251b3a473d2936f33f11101d56e4 Mon Sep 17 00:00:00 2001 From: David Hadas Date: Wed, 24 Apr 2013 21:58:15 +0300 Subject: [PATCH] Autocreate cleanups Autocreate was messy - now cleaned. Auto-create now occurs at account POST, and container PUT only. A method for autocreation was added Autocreation was removed from account_info and container_info. Fake-it as if the account exists on account HEAD and account GET. Return 404 on everything else when the account does not exist. Fix: Bug #1172223 Fix: Bug #1179140 Change-Id: Iac54c1438eb09883fbc29a1ad2ac2245b95efc92 --- swift/account/server.py | 6 +- swift/proxy/controllers/account.py | 60 +++++----- swift/proxy/controllers/base.py | 40 +++---- swift/proxy/controllers/container.py | 14 ++- swift/proxy/controllers/obj.py | 6 +- test/unit/common/test_utils.py | 9 +- test/unit/proxy/test_server.py | 162 +++++++++++++-------------- 7 files changed, 149 insertions(+), 148 deletions(-) diff --git a/swift/account/server.py b/swift/account/server.py index 2ba0f75ea7..4c08fc1e78 100644 --- a/swift/account/server.py +++ b/swift/account/server.py @@ -267,7 +267,11 @@ class AccountController(object): account_list = '\n'.join(output_list) else: if not account_list: - return HTTPNoContent(request=req, headers=resp_headers) + resp_headers['Content-Type'] = req.accept.best_match( + ['text/plain', 'application/json', + 'application/xml', 'text/xml']) + return HTTPNoContent(request=req, headers=resp_headers, + charset='utf-8') account_list = '\n'.join(r[0] for r in account_list) + '\n' ret = Response(body=account_list, request=req, headers=resp_headers) ret.content_type = out_content_type diff --git a/swift/proxy/controllers/account.py b/swift/proxy/controllers/account.py index d4fd4f6286..959b336a78 100644 --- a/swift/proxy/controllers/account.py +++ b/swift/proxy/controllers/account.py @@ -24,13 +24,15 @@ # These shenanigans are to ensure all related objects can be garbage # collected. We've seen objects hang around forever otherwise. +import time from urllib import unquote -from swift.common.utils import public +from swift.common.utils import normalize_timestamp, public from swift.common.constraints import check_metadata, MAX_ACCOUNT_NAME_LENGTH -from swift.common.http import is_success, HTTP_NOT_FOUND +from swift.common.http import HTTP_NOT_FOUND from swift.proxy.controllers.base import Controller, get_account_memcache_key -from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed, Request +from swift.common.swob import HTTPBadRequest, HTTPMethodNotAllowed, \ + HTTPNoContent class AccountController(Controller): @@ -46,28 +48,26 @@ class AccountController(Controller): def GETorHEAD(self, req): """Handler for HTTP GET/HEAD requests.""" + if len(self.account_name) > MAX_ACCOUNT_NAME_LENGTH: + resp = HTTPBadRequest(request=req) + resp.body = 'Account name length of %d longer than %d' % \ + (len(self.account_name), MAX_ACCOUNT_NAME_LENGTH) + return resp + partition, nodes = self.app.account_ring.get_nodes(self.account_name) resp = self.GETorHEAD_base( req, _('Account'), self.app.account_ring, partition, req.path_info.rstrip('/')) if resp.status_int == HTTP_NOT_FOUND and self.app.account_autocreate: - if len(self.account_name) > MAX_ACCOUNT_NAME_LENGTH: - resp = HTTPBadRequest(request=req) - resp.body = 'Account name length of %d longer than %d' % \ - (len(self.account_name), MAX_ACCOUNT_NAME_LENGTH) - return resp - headers = self.generate_request_headers(req) - resp = self.make_requests( - Request.blank('/v1/' + self.account_name), - self.app.account_ring, partition, 'PUT', - '/' + self.account_name, [headers] * len(nodes)) - if not is_success(resp.status_int): - self.app.logger.warning('Could not autocreate account %r' % - self.account_name) - return resp - resp = self.GETorHEAD_base( - req, _('Account'), self.app.account_ring, partition, - req.path_info.rstrip('/')) + # Fake a response + headers = {'Content-Length': '0', + 'Accept-Ranges': 'bytes', + 'Content-Type': 'text/plain; charset=utf-8', + 'X-Timestamp': normalize_timestamp(time.time()), + 'X-Account-Bytes-Used': '0', + 'X-Account-Container-Count': '0', + 'X-Account-Object-Count': '0'} + resp = HTTPNoContent(request=req, headers=headers) return resp @public @@ -99,6 +99,11 @@ class AccountController(Controller): @public def POST(self, req): """HTTP POST request handler.""" + if len(self.account_name) > MAX_ACCOUNT_NAME_LENGTH: + resp = HTTPBadRequest(request=req) + resp.body = 'Account name length of %d longer than %d' % \ + (len(self.account_name), MAX_ACCOUNT_NAME_LENGTH) + return resp error_response = check_metadata(req, 'account') if error_response: return error_response @@ -112,19 +117,10 @@ class AccountController(Controller): req, self.app.account_ring, account_partition, 'POST', req.path_info, [headers] * len(accounts)) if resp.status_int == HTTP_NOT_FOUND and self.app.account_autocreate: - if len(self.account_name) > MAX_ACCOUNT_NAME_LENGTH: - resp = HTTPBadRequest(request=req) - resp.body = 'Account name length of %d longer than %d' % \ - (len(self.account_name), MAX_ACCOUNT_NAME_LENGTH) - return resp + self.autocreate_account(self.account_name) resp = self.make_requests( - Request.blank('/v1/' + self.account_name), - self.app.account_ring, account_partition, 'PUT', - '/' + self.account_name, [headers] * len(accounts)) - if not is_success(resp.status_int): - self.app.logger.warning('Could not autocreate account %r' % - self.account_name) - return resp + req, self.app.account_ring, account_partition, 'POST', + req.path_info, [headers] * len(accounts)) return resp @public diff --git a/swift/proxy/controllers/base.py b/swift/proxy/controllers/base.py index a7e12762e6..5c056cd3fe 100644 --- a/swift/proxy/controllers/base.py +++ b/swift/proxy/controllers/base.py @@ -37,7 +37,6 @@ from swift.common.wsgi import make_pre_authed_request from swift.common.utils import normalize_timestamp, config_true_value, \ public, split_path, cache_from_env, list_from_csv from swift.common.bufferedhttp import http_connect -from swift.common.constraints import MAX_ACCOUNT_NAME_LENGTH from swift.common.exceptions import ChunkReadTimeout, ConnectionTimeout from swift.common.http import is_informational, is_success, is_redirection, \ is_server_error, HTTP_OK, HTTP_PARTIAL_CONTENT, HTTP_MULTIPLE_CHOICES, \ @@ -379,7 +378,7 @@ class Controller(object): {'msg': msg, 'ip': node['ip'], 'port': node['port'], 'device': node['device']}) - def account_info(self, account, req=None, autocreate=False): + def account_info(self, account, req=None): """ Get account information, and also verify that the account exists. @@ -411,7 +410,7 @@ class Controller(object): container_count = 0 if result_code == HTTP_OK: return partition, nodes, container_count - elif result_code == HTTP_NOT_FOUND and not autocreate: + elif result_code == HTTP_NOT_FOUND: return None, None, None result_code = 0 path = '/%s' % account @@ -452,18 +451,6 @@ class Controller(object): self.exception_occurred(node, _('Account'), _('Trying to get account info for %s') % path) - if result_code == HTTP_NOT_FOUND and autocreate: - if len(account) > MAX_ACCOUNT_NAME_LENGTH: - return None, None, None - headers = self.generate_request_headers(req) - resp = self.make_requests(Request.blank('/v1' + path), - self.app.account_ring, partition, 'PUT', - path, [headers] * len(nodes)) - if not is_success(resp.status_int): - self.app.logger.warning('Could not autocreate account %r' % - path) - return None, None, None - result_code = HTTP_OK if self.app.memcache and result_code in (HTTP_OK, HTTP_NOT_FOUND): if result_code == HTTP_OK: cache_timeout = self.app.recheck_account_existence @@ -481,8 +468,7 @@ class Controller(object): return partition, nodes, container_count return None, None, None - def container_info(self, account, container, req=None, - account_autocreate=False): + def container_info(self, account, container, req=None): """ Get container information and thusly verify container existence. This will also make a call to account_info to verify that the @@ -517,8 +503,7 @@ class Controller(object): container_info['partition'] = part container_info['nodes'] = nodes return container_info - if not self.account_info(account, req, - autocreate=account_autocreate)[1]: + if not self.account_info(account, req)[1]: return container_info headers = self.generate_request_headers(req) for node in self.iter_nodes(self.app.container_ring, part): @@ -790,6 +775,23 @@ class Controller(object): """ return is_success(src.status) or is_redirection(src.status) + def autocreate_account(self, account): + partition, nodes = self.app.account_ring.get_nodes(account) + path = '/%s' % account + headers = {'X-Timestamp': normalize_timestamp(time.time()), + 'X-Trans-Id': self.trans_id, + 'Connection': 'close'} + resp = self.make_requests(Request.blank('/v1' + path), + self.app.account_ring, partition, 'PUT', + path, [headers] * len(nodes)) + if is_success(resp.status_int): + self.app.logger.info('autocreate account %r' % path) + if self.app.memcache: + self.app.memcache.delete( + get_account_memcache_key(account)) + else: + self.app.logger.warning('Could not autocreate account %r' % path) + def GETorHEAD_base(self, req, server_type, ring, partition, path): """ Base handler for HTTP GET or HEAD requests. diff --git a/swift/proxy/controllers/container.py b/swift/proxy/controllers/container.py index 3229c69036..b62afc4570 100644 --- a/swift/proxy/controllers/container.py +++ b/swift/proxy/controllers/container.py @@ -124,8 +124,13 @@ class ContainerController(Controller): (len(self.container_name), MAX_CONTAINER_NAME_LENGTH) return resp account_partition, accounts, container_count = \ - self.account_info(self.account_name, req, - autocreate=self.app.account_autocreate) + self.account_info(self.account_name) + if not accounts and self.app.account_autocreate: + self.autocreate_account(self.account_name) + account_partition, accounts, container_count = \ + self.account_info(self.account_name) + if not accounts: + return HTTPNotFound(request=req) if self.app.max_containers_per_account > 0 and \ container_count >= self.app.max_containers_per_account and \ self.account_name not in self.app.max_containers_whitelist: @@ -133,8 +138,6 @@ class ContainerController(Controller): resp.body = 'Reached container limit of %s' % \ self.app.max_containers_per_account return resp - if not accounts: - return HTTPNotFound(request=req) container_partition, containers = self.app.container_ring.get_nodes( self.account_name, self.container_name) headers = self._backend_requests(req, len(containers), @@ -157,8 +160,7 @@ class ContainerController(Controller): if error_response: return error_response account_partition, accounts, container_count = \ - self.account_info(self.account_name, req, - autocreate=self.app.account_autocreate) + self.account_info(self.account_name) if not accounts: return HTTPNotFound(request=req) container_partition, containers = self.app.container_ring.get_nodes( diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py index b92347fd3c..f27bece0d7 100644 --- a/swift/proxy/controllers/obj.py +++ b/swift/proxy/controllers/obj.py @@ -568,8 +568,7 @@ class ObjectController(Controller): if error_response: return error_response container_info = self.container_info( - self.account_name, self.container_name, req, - account_autocreate=self.app.account_autocreate) + self.account_name, self.container_name) container_partition = container_info['partition'] containers = container_info['nodes'] req.acl = container_info['write_acl'] @@ -692,8 +691,7 @@ class ObjectController(Controller): def PUT(self, req): """HTTP PUT request handler.""" container_info = self.container_info( - self.account_name, self.container_name, req, - account_autocreate=self.app.account_autocreate) + self.account_name, self.container_name) container_partition = container_info['partition'] containers = container_info['nodes'] req.acl = container_info['write_acl'] diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 0f5a0eb1bf..7ab21212d7 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -528,18 +528,23 @@ class TestUtils(unittest.TestCase): self.assert_('my error message' in log_msg) # test eventlet.Timeout - log_exception(ConnectionTimeout(42, 'my error message')) + connection_timeout = ConnectionTimeout(42, 'my error message') + log_exception(connection_timeout) log_msg = strip_value(sio) self.assert_('Traceback' not in log_msg) self.assert_('ConnectionTimeout' in log_msg) self.assert_('(42s)' in log_msg) self.assert_('my error message' not in log_msg) - log_exception(MessageTimeout(42, 'my error message')) + connection_timeout.cancel() + + message_timeout = MessageTimeout(42, 'my error message') + log_exception(message_timeout) log_msg = strip_value(sio) self.assert_('Traceback' not in log_msg) self.assert_('MessageTimeout' in log_msg) self.assert_('(42s)' in log_msg) self.assert_('my error message' in log_msg) + message_timeout.cancel() # test unhandled log_exception(Exception('my error message')) diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py index f02561826e..603e5643ad 100644 --- a/test/unit/proxy/test_server.py +++ b/test/unit/proxy/test_server.py @@ -444,17 +444,8 @@ class TestController(unittest.TestCase): test(404, 507, 503) test(503, 503, 503) - def test_account_info_account_autocreate(self): + def test_account_info_no_account(self): with save_globals(): - self.memcache.store = {} - # account_info returns after 3 non 2xx responses unless autocreate - # is True - set_http_connect(404, 404, 404) - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=False) - self.check_account_info_return(partition, nodes, is_none=True) - self.assertEquals(count, None) - self.memcache.store = {} set_http_connect(404, 404, 404) partition, nodes, count = \ @@ -462,44 +453,6 @@ class TestController(unittest.TestCase): self.check_account_info_return(partition, nodes, is_none=True) self.assertEquals(count, None) - self.memcache.store = {} - set_http_connect(404, 404, 404, 201, 201, 201) - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=True) - self.check_account_info_return(partition, nodes) - self.assertEquals(count, 0) - - self.memcache.store = {} - set_http_connect(404, 404, 404, 503, 201, 201) - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=True) - self.check_account_info_return(partition, nodes) - self.assertEquals(count, 0) - - self.memcache.store = {} - set_http_connect(404, 404, 404, 503, 201, 503) - exc = None - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=True) - self.check_account_info_return(partition, nodes, is_none=True) - self.assertEquals(None, count) - - self.memcache.store = {} - set_http_connect(404, 404, 404, 403, 403, 403) - exc = None - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=True) - self.check_account_info_return(partition, nodes, is_none=True) - self.assertEquals(None, count) - - self.memcache.store = {} - set_http_connect(404, 404, 404, 409, 409, 409) - exc = None - partition, nodes, count = \ - self.controller.account_info(self.account, self.request, autocreate=True) - self.check_account_info_return(partition, nodes, is_none=True) - self.assertEquals(None, count) - def check_container_info_return(self, ret, is_none=False): if is_none: partition, nodes, read_acl, write_acl = None, None, None, None @@ -4534,6 +4487,7 @@ class TestContainerController(unittest.TestCase): if expected < 400: self.assert_('x-works' in res.headers) self.assertEquals(res.headers['x-works'], 'yes') + test_status_map((200, 200, 404, 404), 200) test_status_map((200, 200, 500, 404), 200) test_status_map((200, 304, 500, 404), 304) @@ -4545,7 +4499,7 @@ class TestContainerController(unittest.TestCase): self.app.account_autocreate = True test_status_map((404, 404, 404), 404) - def test_PUT_POST(self): + def test_PUT(self): with save_globals(): controller = proxy_server.ContainerController(self.app, 'account', 'container') @@ -4560,6 +4514,55 @@ class TestContainerController(unittest.TestCase): expected = str(expected) self.assertEquals(res.status[:len(expected)], expected) + test_status_map((200, 201, 201, 201), 201, missing_container=True) + test_status_map((200, 201, 201, 500), 201, missing_container=True) + test_status_map((200, 204, 404, 404), 404, missing_container=True) + test_status_map((200, 204, 500, 404), 503, missing_container=True) + self.assertFalse(self.app.account_autocreate) + test_status_map((404, 404, 404), 404, missing_container=True) + self.app.account_autocreate = True + #fail to retrieve account info + test_status_map( + (503, 503, 503), # account_info fails on 503 + 404, missing_container=True) + # account fail after creation + test_status_map( + (404, 404, 404, # account_info fails on 404 + 201, 201, 201, # PUT account + 404, 404, 404), # account_info fail + 404, missing_container=True) + test_status_map( + (503, 503, 404, # account_info fails on 404 + 503, 503, 503, # PUT account + 503, 503, 404), # account_info fail + 404, missing_container=True) + #put fails + test_status_map( + (404, 404, 404, # account_info fails on 404 + 201, 201, 201, # PUT account + 200, # account_info success + 503, 503, 201), # put container fail + 503, missing_container=True) + # all goes according to plan + test_status_map( + (404, 404, 404, # account_info fails on 404 + 201, 201, 201, # PUT account + 200, # account_info success + 201, 201, 201), # put container success + 201, missing_container=True) + test_status_map( + (503, 404, 404, # account_info fails on 404 + 503, 201, 201, # PUT account + 503, 200, # account_info success + 503, 201, 201), # put container success + 201, missing_container=True) + + def test_POST(self): + with save_globals(): + controller = proxy_server.ContainerController(self.app, 'account', + 'container') + + def test_status_map(statuses, expected, **kwargs): set_http_connect(*statuses, **kwargs) self.app.memcache.store = {} req = Request.blank('/a/c', {}) @@ -4568,6 +4571,7 @@ class TestContainerController(unittest.TestCase): res = controller.POST(req) expected = str(expected) self.assertEquals(res.status[:len(expected)], expected) + test_status_map((200, 201, 201, 201), 201, missing_container=True) test_status_map((200, 201, 201, 500), 201, missing_container=True) test_status_map((200, 204, 404, 404), 404, missing_container=True) @@ -4575,12 +4579,7 @@ class TestContainerController(unittest.TestCase): self.assertFalse(self.app.account_autocreate) test_status_map((404, 404, 404), 404, missing_container=True) self.app.account_autocreate = True - test_status_map((404, 404, 404, 201, 201, 201, 200, 200, 200), 200, - missing_container=True) - test_status_map((404, 404, 404, 403, 403, 403), 404, - missing_container=True) - test_status_map((404, 404, 404, 403, 201, 201, 403, 201, 201), 201, - missing_container=True) + test_status_map((404, 404, 404), 404, missing_container=True) def test_PUT_max_containers_per_account(self): with save_globals(): @@ -4737,12 +4736,6 @@ class TestContainerController(unittest.TestCase): (200, 404, 404, 404), 404) self.assert_status_map(controller.DELETE, (200, 204, 503, 404), 503) - self.assertFalse(self.app.account_autocreate) - self.assert_status_map(controller.DELETE, - (404, 404, 404), 404) - self.app.account_autocreate = True - self.assert_status_map(controller.DELETE, - (404, 404, 404), 404) self.app.memcache = FakeMemcacheReturnsNone() # 200: Account check, 404x3: Container check @@ -5355,32 +5348,30 @@ class TestAccountController(unittest.TestCase): self.app.memcache = FakeMemcacheReturnsNone() self.assert_status_map(controller.GET, (404, 404, 404), 404) + def test_GET_autocreate(self): with save_globals(): controller = proxy_server.AccountController(self.app, 'account') self.app.memcache = FakeMemcacheReturnsNone() + self.assertFalse(self.app.account_autocreate) # Repeat the test for autocreate = False and 404 by all self.assert_status_map(controller.GET, - (404, 404, 404, 201, 201, 201, 204), 404) + (404, 404, 404), 404) self.assert_status_map(controller.GET, - (404, 503, 404, 201, 201, 201, 204), 404) + (404, 503, 404), 404) # When autocreate is True, if none of the nodes respond 2xx # And quorum of the nodes responded 404, # ALL nodes are asked to create the account # If successful, the GET request is repeated. controller.app.account_autocreate = True self.assert_status_map(controller.GET, - (404, 404, 404, 201, 201, 201, 204), 204) + (404, 404, 404), 204) self.assert_status_map(controller.GET, - (404, 503, 404, 201, 503, 201, 204), 204) - # Test a case when less than quorum responded 2xx during - # account create + (404, 503, 404), 204) + + # We always return 503 if no majority between 4xx, 3xx or 2xx found self.assert_status_map(controller.GET, - (404, 404, 404, 201, 409, 409), 409) - self.assert_status_map(controller.GET, - (404, 404, 404, 403, 403, 403), 403) - self.assert_status_map(controller.GET, - (404, 503, 404, 403, 503, 403), 403) + (500, 500, 400), 503) def test_HEAD(self): # Same behaviour as GET @@ -5405,33 +5396,36 @@ class TestAccountController(unittest.TestCase): with save_globals(): controller = proxy_server.AccountController(self.app, 'account') self.app.memcache = FakeMemcacheReturnsNone() + self.assertFalse(self.app.account_autocreate) self.assert_status_map(controller.HEAD, - (404, 404, 404, 201, 201, 201, 204), 404) + (404, 404, 404), 404) controller.app.account_autocreate = True self.assert_status_map(controller.HEAD, - (404, 404, 404, 201, 201, 201, 204), 204) + (404, 404, 404), 204) self.assert_status_map(controller.HEAD, - (404, 404, 404, 201, 409, 409), 409) + (500, 404, 404), 204) + # We always return 503 if no majority between 4xx, 3xx or 2xx found self.assert_status_map(controller.HEAD, - (404, 404, 404, 201, 403, 201, 204), 204) + (500, 500, 400), 503) def test_POST_autocreate(self): with save_globals(): controller = proxy_server.AccountController(self.app, 'account') self.app.memcache = FakeMemcacheReturnsNone() + # first test with autocreate being False + self.assertFalse(self.app.account_autocreate) self.assert_status_map(controller.POST, - (404, 404, 404, 201, 201, 201), 404) + (404, 404, 404), 404) + # next turn it on and test account being created than updated controller.app.account_autocreate = True self.assert_status_map(controller.POST, - (404, 404, 404, 201, 201, 201), 201) + (404, 404, 404, 202, 202, 202, 201, 201, 201), 201) + # account_info PUT account POST account self.assert_status_map(controller.POST, - (404, 404, 404, 403, 403, 403), 403) + (404, 404, 503, 201, 201, 503, 204, 204, 504), 204) + # what if create fails self.assert_status_map(controller.POST, - (404, 404, 404, 409, 409, 409), 409) - self.assert_status_map(controller.POST, - (404, 404, 404, 201, 409, 409), 409) - self.assert_status_map(controller.POST, - (404, 404, 404, 201, 201, 409), 201) + (404, 404, 404, 403, 403, 403, 400, 400, 400), 400) def test_connection_refused(self): self.app.account_ring.get_nodes('account')