Merge "Handle os.listdir failures in container-updater"

swift/container/updater.py

@@ -70,6 +70,14 @@ class ContainerUpdater(Daemon):
             self.account_ring = Ring(self.swift_dir, ring_name='account')
         return self.account_ring
 
+    def _listdir(self, path):
+        try:
+            return os.listdir(path)
+        except OSError as e:
+            self.logger.error(_('ERROR:  Failed to get paths to drive '
+                                'partitions: %s') % e)
+            return []
+
     def get_paths(self):
         """
         Get paths to all of the partitions on each drive to be processed.
@@ -77,7 +85,7 @@ class ContainerUpdater(Daemon):
         :returns: a list of paths
         """
         paths = []
-        for device in os.listdir(self.devices):
+        for device in self._listdir(self.devices):
             dev_path = os.path.join(self.devices, device)
             if self.mount_check and not ismount(dev_path):
                 self.logger.warn(_('%s is not mounted'), device)
@@ -85,7 +93,7 @@ class ContainerUpdater(Daemon):
             con_path = os.path.join(dev_path, DATADIR)
             if not os.path.exists(con_path):
                 continue
-            for partition in os.listdir(con_path):
+            for partition in self._listdir(con_path):
                 paths.append(os.path.join(con_path, partition))
         shuffle(paths)
         return paths

test/unit/container/test_updater.py

@@ -14,12 +14,14 @@
 # limitations under the License.
 
 import cPickle as pickle
+import mock
 import os
 import unittest
 from contextlib import closing
 from gzip import GzipFile
 from shutil import rmtree
 from tempfile import mkdtemp
+from test.unit import FakeLogger
 
 from eventlet import spawn, Timeout, listen
 
@@ -157,6 +159,44 @@ class TestContainerUpdater(unittest.TestCase):
         self.assertEquals(info['reported_object_count'], 1)
         self.assertEquals(info['reported_bytes_used'], 3)
 
+    @mock.patch('os.listdir')
+    def test_listdir_with_exception(self, mock_listdir):
+        e = OSError('permission_denied')
+        mock_listdir.side_effect = e
+        cu = container_updater.ContainerUpdater({
+            'devices': self.devices_dir,
+            'mount_check': 'false',
+            'swift_dir': self.testdir,
+            'interval': '1',
+            'concurrency': '1',
+            'node_timeout': '15',
+            'account_suppression_time': 0
+        })
+        cu.logger = FakeLogger()
+        paths = cu.get_paths()
+        self.assertEqual(paths, [])
+        log_lines = cu.logger.get_lines_for_level('error')
+        msg = ('ERROR:  Failed to get paths to drive partitions: '
+               'permission_denied')
+        self.assertEqual(log_lines[0], msg)
+
+    @mock.patch('os.listdir', return_value='bar/')
+    def test_listdir_without_exception(self, mock_listdir):
+        cu = container_updater.ContainerUpdater({
+            'devices': self.devices_dir,
+            'mount_check': 'false',
+            'swift_dir': self.testdir,
+            'interval': '1',
+            'concurrency': '1',
+            'node_timeout': '15',
+            'account_suppression_time': 0
+        })
+        cu.logger = FakeLogger()
+        path = cu._listdir('foo/bar/')
+        self.assertEqual(path, 'bar/')
+        log_lines = cu.logger.get_lines_for_level('error')
+        self.assertEqual(len(log_lines), 0)
+
     def test_unicode(self):
         cu = container_updater.ContainerUpdater({
             'devices': self.devices_dir,