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>

3 files changed, 94 insertions, 11 deletions

diff --git a/gluster/swift/common/DiskDir.py b/gluster/swift/common/DiskDir.py
index 50e795c..c5793f5 100644
--- a/gluster/swift/common/DiskDir.py
+++ b/gluster/swift/common/DiskDir.py
@@ -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.
-        return dir_empty(self.datadir)
+        # 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,13 +412,16 @@ class DiskDir(DiskCommon):
                       reported_put_timestamp, reported_delete_timestamp,
                       reported_object_count, and reported_bytes_used.
         """
-        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.
-            self._update_object_count()
-        else:
-            # FIXME: to facilitate testing, we need to update all the time
-            self._update_object_count()
+        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.
+                self._update_object_count()
+            else:
+                # FIXME: to facilitate testing, we need to update all
+                # the time
+                self._update_object_count()
 
         data = {'account': self.account, 'container': self.container,
                 'object_count': self.metadata.get(
@@ -478,8 +489,11 @@ class DiskDir(DiskCommon):
 
         :param timestamp: delete timestamp
         """
-        if not dir_empty(self.datadir):
-            # FIXME: This is a failure condition here, isn't it?
+        try:
+            if not dir_empty(self.datadir):
+                # FIXME: This is a failure condition here, isn't it?
+                return
+        except FileOrDirNotFoundError:
             return
         rmdirs(self.datadir)
 
diff --git a/test/unit/common/test_diskdir.py b/test/unit/common/test_diskdir.py
index 580e76a..be0c922 100644
--- a/test/unit/common/test_diskdir.py
+++ b/test/unit/common/test_diskdir.py
@@ -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')
diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py
index a04380d..57e3111 100644
--- a/test/unit/proxy/test_server.py
+++ b/test/unit/proxy/test_server.py
@@ -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='{}')