Return correct status when deleting non-existing container
The code was raising an exception when the container (which happens to be a directory) did not exist. To be compatible with OpenStack Swift, we need to handle an object which its container/directory does not exist. BUG: 960944 (https://bugzilla.redhat.com/show_bug.cgi?id=960944) Change-Id: Ibb2db354a655e040fb70ebbe6a7d8f815d33dc0f Signed-off-by: Luis Pabon <lpabon@redhat.com> Reviewed-on: http://review.gluster.org/5201 Reviewed-by: Peter Portante <pportant@redhat.com> Tested-by: Peter Portante <pportant@redhat.com>
This commit is contained in:
parent
a88accbe27
commit
b00e479637
@ -25,6 +25,7 @@ from gluster.swift.common.utils import validate_account, validate_container, \
|
||||
X_CONTENT_LENGTH, X_TIMESTAMP, X_PUT_TIMESTAMP, X_ETAG, X_OBJECTS_COUNT, \
|
||||
X_BYTES_USED, X_CONTAINER_COUNT, DIR_TYPE
|
||||
from gluster.swift.common import Glusterfs
|
||||
from gluster.swift.common.exceptions import FileOrDirNotFoundError
|
||||
|
||||
|
||||
DATADIR = 'containers'
|
||||
@ -182,8 +183,15 @@ class DiskCommon(object):
|
||||
return not os_path.exists(self.datadir)
|
||||
|
||||
def empty(self):
|
||||
# FIXME: Common because ported swift AccountBroker unit tests use it.
|
||||
# If it does not exist, then it is empty. A value of True is
|
||||
# what is expected by OpenStack Swift when the directory does
|
||||
# not exist. Check swift/common/db.py:ContainerBroker.empty()
|
||||
# and swift/container/server.py:ContainerController.DELETE()
|
||||
# for more information
|
||||
try:
|
||||
return dir_empty(self.datadir)
|
||||
except FileOrDirNotFoundError:
|
||||
return True
|
||||
|
||||
def update_metadata(self, metadata):
|
||||
assert self.metadata, "Valid container/account metadata should have " \
|
||||
@ -404,12 +412,15 @@ class DiskDir(DiskCommon):
|
||||
reported_put_timestamp, reported_delete_timestamp,
|
||||
reported_object_count, and reported_bytes_used.
|
||||
"""
|
||||
if self._dir_exists:
|
||||
if not Glusterfs.OBJECT_ONLY:
|
||||
# If we are not configured for object only environments, we should
|
||||
# update the object counts in case they changed behind our back.
|
||||
# If we are not configured for object only environments,
|
||||
# we should update the object counts in case they changed
|
||||
# behind our back.
|
||||
self._update_object_count()
|
||||
else:
|
||||
# FIXME: to facilitate testing, we need to update all the time
|
||||
# FIXME: to facilitate testing, we need to update all
|
||||
# the time
|
||||
self._update_object_count()
|
||||
|
||||
data = {'account': self.account, 'container': self.container,
|
||||
@ -478,9 +489,12 @@ class DiskDir(DiskCommon):
|
||||
|
||||
:param timestamp: delete timestamp
|
||||
"""
|
||||
try:
|
||||
if not dir_empty(self.datadir):
|
||||
# FIXME: This is a failure condition here, isn't it?
|
||||
return
|
||||
except FileOrDirNotFoundError:
|
||||
return
|
||||
rmdirs(self.datadir)
|
||||
|
||||
def set_x_container_sync_points(self, sync_point1, sync_point2):
|
||||
|
@ -344,6 +344,22 @@ class TestDiskCommon(unittest.TestCase):
|
||||
del dc.metadata['X-Container-Meta-foo']
|
||||
assert dc.metadata == md_copy
|
||||
|
||||
def test_empty_dir_is_not_empty(self):
|
||||
dc = dd.DiskCommon(self.td, self.fake_drives[0],
|
||||
self.fake_accounts[0], self.fake_logger)
|
||||
os.makedirs(os.path.join(self.td, self.fake_drives[0], 'aaabbbccc'))
|
||||
self.assertFalse(dc.empty())
|
||||
|
||||
def test_empty_dir_is_empty(self):
|
||||
dc = dd.DiskCommon(self.td, self.fake_drives[0],
|
||||
self.fake_accounts[0], self.fake_logger)
|
||||
self.assertTrue(dc.empty())
|
||||
|
||||
def test_empty_dir_does_not_exist(self):
|
||||
dc = dd.DiskCommon(self.td, 'non_existent_drive',
|
||||
self.fake_accounts[0], self.fake_logger)
|
||||
self.assertTrue(dc.empty())
|
||||
|
||||
|
||||
class TestContainerBroker(unittest.TestCase):
|
||||
"""
|
||||
@ -498,6 +514,24 @@ class TestContainerBroker(unittest.TestCase):
|
||||
self.assertEquals(info['x_container_sync_point1'], -1)
|
||||
self.assertEquals(info['x_container_sync_point2'], -1)
|
||||
|
||||
def test_get_info_nonexistent_container(self):
|
||||
broker = dd.DiskDir(self.path, self.drive, account='no_account',
|
||||
container='no_container', logger=FakeLogger())
|
||||
info = broker.get_info()
|
||||
|
||||
#
|
||||
# Because broker._dir_exists is False and _update_object_count()
|
||||
# has not been called yet, the values returned for
|
||||
# object_count, bytes_used, and put_timestamp are '0' as
|
||||
# a string. OpenStack Swift handles this situation by
|
||||
# passing the value to float().
|
||||
#
|
||||
self.assertEquals(info['account'], 'no_account')
|
||||
self.assertEquals(info['container'], 'no_container')
|
||||
self.assertEquals(info['object_count'], '0')
|
||||
self.assertEquals(info['bytes_used'], '0')
|
||||
self.assertEquals(info['put_timestamp'], '0')
|
||||
|
||||
def test_set_x_syncs(self):
|
||||
broker = self._get_broker(account='test1',
|
||||
container='test2')
|
||||
|
@ -4512,6 +4512,41 @@ class TestContainerController(unittest.TestCase):
|
||||
self.assert_status_map(controller.DELETE,
|
||||
(200, 404, 404, 404), 404)
|
||||
|
||||
def test_DELETE_container_that_does_not_exist(self):
|
||||
prolis = _test_sockets[0]
|
||||
# Create a container
|
||||
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
|
||||
fd = sock.makefile()
|
||||
fd.write('PUT /v1/a/aaabbbccc HTTP/1.1\r\nHost: localhost\r\n'
|
||||
'Connection: close\r\nX-Storage-Token: t\r\n'
|
||||
'Content-Length: 0\r\n\r\n')
|
||||
fd.flush()
|
||||
headers = readuntil2crlfs(fd)
|
||||
exp = 'HTTP/1.1 201'
|
||||
self.assertEquals(headers[:len(exp)], exp)
|
||||
|
||||
# Delete container
|
||||
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
|
||||
fd = sock.makefile()
|
||||
fd.write('DELETE /v1/a/aaabbbccc HTTP/1.1\r\nHost: localhost\r\n'
|
||||
'Connection: close\r\nX-Storage-Token: t\r\n'
|
||||
'Content-Length: 0\r\n\r\n')
|
||||
fd.flush()
|
||||
headers = readuntil2crlfs(fd)
|
||||
exp = 'HTTP/1.1 204'
|
||||
self.assertEquals(headers[:len(exp)], exp)
|
||||
|
||||
# Delete again
|
||||
sock = connect_tcp(('localhost', prolis.getsockname()[1]))
|
||||
fd = sock.makefile()
|
||||
fd.write('DELETE /v1/a/aaabbbccc HTTP/1.1\r\nHost: localhost\r\n'
|
||||
'Connection: close\r\nX-Storage-Token: t\r\n'
|
||||
'Content-Length: 0\r\n\r\n')
|
||||
fd.flush()
|
||||
headers = readuntil2crlfs(fd)
|
||||
exp = 'HTTP/1.1 404'
|
||||
self.assertEquals(headers[:len(exp)], exp)
|
||||
|
||||
def test_response_get_accept_ranges_header(self):
|
||||
with save_globals():
|
||||
set_http_connect(200, 200, body='{}')
|
||||
|
Loading…
Reference in New Issue
Block a user