openstack/swift
Code Issues Proposed changes

Deleted account respond as non existing accounts

Browse Source 
Currently clients can not distinguish between non existing accounts
(which can be created) and accounts marked for deletion, which has
not yet been reaped and therefore cannot be re-created until reaped.

Following this patch, if an account is marked as deleted but hasn't
been reaped and is still on disk, responses will include a status
header:
    'X-Account-Status' = 'Deleted'

Fixes:Bug #1188609
Change-Id: Ibd39965ae3f5d45fd78f130e0e31f5a0141a8633
This commit is contained in:
David Hadas 2013-06-26 08:23:00 +03:00
parent edf4068c8b
commit 8226761889
5 changed files with 105 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
swift/account/server.py
View File

@ -24,7 +24,7 @@ from eventlet import Timeout
import swift.common.db
from swift.account.utils import account_listing_response, \
account_listing_content_type
from swift.common.db import AccountBroker
from swift.common.db import AccountBroker, DatabaseConnectionError
from swift.common.utils import get_logger, get_param, hash_path, public, \
normalize_timestamp, storage_directory, config_true_value, \
validate_device_partition, json, timing_stats
@ -74,6 +74,20 @@ class AccountController(object):
db_path = os.path.join(self.root, drive, db_dir, hsh + '.db')
return AccountBroker(db_path, account=account, logger=self.logger)
def _deleted_response(self, broker, req, resp, body=''):
# We are here since either the account does not exist or
# it exists but marked for deletion.
headers = {}
# Try to check if account exists and is marked for deletion
try:
if broker.is_status_deleted():
# Account does exist and is marked for deletion
headers = {'X-Account-Status': 'Deleted'}
except DatabaseConnectionError:
# Account does not exist!
pass
return resp(request=req, headers=headers, charset='utf-8', body=body)
@public
@timing_stats()
def DELETE(self, req):
@ -92,9 +106,9 @@ class AccountController(object):
content_type='text/plain')
broker = self._get_account_broker(drive, part, account)
if broker.is_deleted():
return HTTPNotFound(request=req)
return self._deleted_response(broker, req, HTTPNotFound)
broker.delete_db(req.headers['x-timestamp'])
return HTTPNoContent(request=req)
return self._deleted_response(broker, req, HTTPNoContent)
@public
@timing_stats()
@ -140,7 +154,8 @@ class AccountController(object):
except swift.common.db.DatabaseAlreadyExists:
pass
elif broker.is_status_deleted():
return HTTPForbidden(request=req, body='Recently deleted')
return self._deleted_response(broker, req, HTTPForbidden,
body='Recently deleted')
else:
created = broker.is_deleted()
broker.update_put_timestamp(timestamp)
@ -185,7 +200,7 @@ class AccountController(object):
broker.pending_timeout = 0.1
broker.stale_reads_ok = True
if broker.is_deleted():
return HTTPNotFound(request=req)
return self._deleted_response(broker, req, HTTPNotFound)
info = broker.get_info()
headers = {
'X-Account-Container-Count': info['container_count'],
@ -238,7 +253,7 @@ class AccountController(object):
broker.pending_timeout = 0.1
broker.stale_reads_ok = True
if broker.is_deleted():
return HTTPNotFound(request=req)
return self._deleted_response(broker, req, HTTPNotFound)
return account_listing_response(account, req, out_content_type, broker,
limit, marker, end_marker, prefix,
delimiter)
@ -286,7 +301,7 @@ class AccountController(object):
return HTTPInsufficientStorage(drive=drive, request=req)
broker = self._get_account_broker(drive, part, account)
if broker.is_deleted():
return HTTPNotFound(request=req)
return self._deleted_response(broker, req, HTTPNotFound)
timestamp = normalize_timestamp(req.headers['x-timestamp'])
metadata = {}
metadata.update((key, (value, timestamp))

22
swift/proxy/controllers/base.py
View File

@ -689,7 +689,8 @@ class Controller(object):
resp = conn.getresponse()
if not is_informational(resp.status) and \
not is_server_error(resp.status):
return resp.status, resp.reason, resp.read()
return resp.status, resp.reason, resp.getheaders(), \
resp.read()
elif resp.status == HTTP_INSUFFICIENT_STORAGE:
self.error_limit(node, _('ERROR Insufficient Storage'))
except (Exception, Timeout):
@ -722,13 +723,14 @@ class Controller(object):
head, query_string, self.app.logger.thread_locals)
response = [resp for resp in pile if resp]
while len(response) < len(start_nodes):
response.append((HTTP_SERVICE_UNAVAILABLE, '', ''))
statuses, reasons, bodies = zip(*response)
response.append((HTTP_SERVICE_UNAVAILABLE, '', '', ''))
statuses, reasons, resp_headers, bodies = zip(*response)
return self.best_response(req, statuses, reasons, bodies,
'%s %s' % (self.server_type, req.method))
'%s %s' % (self.server_type, req.method),
headers=resp_headers)
def best_response(self, req, statuses, reasons, bodies, server_type,
etag=None):
etag=None, headers=None):
"""
Given a list of responses from several servers, choose the best to
return to the API.
@ -739,6 +741,7 @@ class Controller(object):
:param bodies: bodies of each response
:param server_type: type of server the responses came from
:param etag: etag
:param headers: headers of each response
:returns: swob.Response object with the correct status, body, etc. set
"""
resp = Response(request=req)
@ -751,6 +754,8 @@ class Controller(object):
status_index = statuses.index(status)
resp.status = '%s %s' % (status, reasons[status_index])
resp.body = bodies[status_index]
if headers:
update_headers(resp, headers[status_index])
if etag:
resp.headers['etag'] = etag.strip('"')
return resp
@ -922,6 +927,7 @@ class Controller(object):
statuses = []
reasons = []
bodies = []
source_headers = []
sources = []
newest = config_true_value(req.headers.get('x-newest', 'f'))
for node in self.iter_nodes(ring, partition):
@ -950,11 +956,13 @@ class Controller(object):
statuses.append(HTTP_NOT_FOUND)
reasons.append('')
bodies.append('')
source_headers.append('')
self.close_swift_conn(possible_source)
else:
statuses.append(possible_source.status)
reasons.append(possible_source.reason)
bodies.append('')
source_headers.append('')
sources.append((possible_source, node))
if not newest: # one good source is enough
break
@ -962,6 +970,7 @@ class Controller(object):
statuses.append(possible_source.status)
reasons.append(possible_source.reason)
bodies.append(possible_source.read())
source_headers.append(possible_source.getheaders())
if possible_source.status == HTTP_INSUFFICIENT_STORAGE:
self.error_limit(node, _('ERROR Insufficient Storage'))
elif is_server_error(possible_source.status):
@ -995,7 +1004,8 @@ class Controller(object):
res.content_type = source.getheader('Content-Type')
if not res:
res = self.best_response(req, statuses, reasons, bodies,
'%s %s' % (server_type, req.method))
'%s %s' % (server_type, req.method),
headers=source_headers)
try:
(account, container) = split_path(req.path_info, 1, 2)
_set_info_cache(self.app, req.environ, account, container, res)

6
test/unit/__init__.py
View File

@ -402,8 +402,10 @@ def fake_http_connect(*code_iter, **kwargs):
'x-object-meta-test': 'testing',
'x-delete-at': '9876543210',
'etag': etag,
'x-works': 'yes',
'x-account-container-count': kwargs.get('count', 12345)}
'x-works': 'yes'}
if self.status // 100 == 2:
headers['x-account-container-count'] = \
kwargs.get('count', 12345)
if not self.timestamp:
del headers['x-timestamp']
try:

47
test/unit/account/test_server.py
View File

@ -49,6 +49,7 @@ class TestAccountController(unittest.TestCase):
'HTTP_X_TIMESTAMP': '0'})
resp = self.controller.DELETE(req)
self.assertEquals(resp.status_int, 404)
self.assertTrue('X-Account-Status' not in resp.headers)
def test_DELETE_empty(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@ -58,6 +59,7 @@ class TestAccountController(unittest.TestCase):
'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.DELETE(req)
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_DELETE_not_empty(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@ -74,6 +76,7 @@ class TestAccountController(unittest.TestCase):
resp = self.controller.DELETE(req)
# We now allow deleting non-empty accounts
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_DELETE_now_empty(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@ -99,6 +102,7 @@ class TestAccountController(unittest.TestCase):
'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.DELETE(req)
self.assertEquals(resp.status_int, 204)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_DELETE_invalid_partition(self):
req = Request.blank('/sda1/./a', environ={'REQUEST_METHOD': 'DELETE',
@ -123,9 +127,29 @@ class TestAccountController(unittest.TestCase):
self.assertEquals(resp.status_int, 507)
def test_HEAD_not_found(self):
# Test the case in which account does not exist (can be recreated)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'HEAD'})
resp = self.controller.HEAD(req)
self.assertEquals(resp.status_int, 404)
self.assertTrue('X-Account-Status' not in resp.headers)
# Test the case in which account was deleted but not yet reaped
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
'HTTP_X_TIMESTAMP': '0'})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a/c1', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Put-Timestamp': '1',
'X-Delete-Timestamp': '0',
'X-Object-Count': '0',
'X-Bytes-Used': '0'})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'DELETE',
'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.DELETE(req)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'HEAD'})
resp = self.controller.HEAD(req)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_HEAD_empty_account(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@ -219,6 +243,7 @@ class TestAccountController(unittest.TestCase):
'X-Timestamp': normalize_timestamp(0)})
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 404)
self.assertTrue('X-Account-Status' not in resp.headers)
def test_PUT(self):
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
@ -244,6 +269,7 @@ class TestAccountController(unittest.TestCase):
resp = self.controller.PUT(req)
self.assertEquals(resp.status_int, 403)
self.assertEquals(resp.body, 'Recently deleted')
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_PUT_GET_metadata(self):
# Set metadata header
@ -388,11 +414,32 @@ class TestAccountController(unittest.TestCase):
'HTTP_X_TIMESTAMP': '2'})
resp = self.controller.POST(req)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_GET_not_found_plain(self):
# Test the case in which account does not exist (can be recreated)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 404)
self.assertTrue('X-Account-Status' not in resp.headers)
# Test the case in which account was deleted but not yet reaped
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'PUT',
'HTTP_X_TIMESTAMP': '0'})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a/c1', environ={'REQUEST_METHOD': 'PUT'},
headers={'X-Put-Timestamp': '1',
'X-Delete-Timestamp': '0',
'X-Object-Count': '0',
'X-Bytes-Used': '0'})
self.controller.PUT(req)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'DELETE',
'HTTP_X_TIMESTAMP': '1'})
resp = self.controller.DELETE(req)
req = Request.blank('/sda1/p/a', environ={'REQUEST_METHOD': 'GET'})
resp = self.controller.GET(req)
self.assertEquals(resp.status_int, 404)
self.assertEquals(resp.headers['X-Account-Status'], 'Deleted')
def test_GET_not_found_json(self):
req = Request.blank('/sda1/p/a?format=json',

14
test/unit/proxy/test_server.py
View File

@ -350,12 +350,12 @@ class TestController(unittest.TestCase):
# Test the internal representation in memcache
# 'container_count' changed from 0 to None
cache_key = get_account_memcache_key(self.account)
container_info = {'status': 404,
account_info = {'status': 404,
'container_count': None, # internally keep None
'total_object_count': None,
'bytes': None,
'meta': {}}
self.assertEquals(container_info,
self.assertEquals(account_info,
self.memcache.get(cache_key))
set_http_connect()
@ -2279,6 +2279,16 @@ class TestObjectController(unittest.TestCase):
node_iter=iter(node_list)))
self.assertEqual(node_list, got_nodes)
def test_best_response_sets_headers(self):
controller = proxy_server.ObjectController(self.app, 'account',
'container', 'object')
req = Request.blank('/a/c/o', environ={'REQUEST_METHOD': 'GET'})
resp = controller.best_response(req, [200] * 3, ['OK'] * 3, [''] * 3,
'Object', headers=[{'X-Test': '1'},
{'X-Test': '2'},
{'X-Test': '3'}])
self.assertEquals(resp.headers['X-Test'], '1')
def test_best_response_sets_etag(self):
controller = proxy_server.ObjectController(self.app, 'account',
'container', 'object')