Handle os.listdir failures in object-updater

While investigating bug 1375348 I discovered the problem
reported there was not limited to the object-auditor.  The
object-updater has similar bugs.

This patch catches the unhandled exception that can be thrown
by os.listdir if the self.devices directory is inaccessible.

Change-Id: I6293b840916bb63cf9eebbc05068d9a3c871bdc3
Related-bug: 1375348
This commit is contained in:
Jay S. Bryant 2014-10-02 14:10:04 -05:00
parent e970a1ae57
commit 1c9bc0b522
2 changed files with 48 additions and 5 deletions

View File

@ -55,6 +55,15 @@ class ObjectUpdater(Daemon):
'/var/cache/swift') '/var/cache/swift')
self.rcache = os.path.join(self.recon_cache_path, 'object.recon') self.rcache = os.path.join(self.recon_cache_path, 'object.recon')
def _listdir(self, path):
try:
return os.listdir(path)
except OSError as e:
self.logger.error(_('ERROR: Unable to access %(path)s: '
'%(error)s') %
{'path': path, 'error': e})
return []
def get_container_ring(self): def get_container_ring(self):
"""Get the container ring. Load it, if it hasn't been yet.""" """Get the container ring. Load it, if it hasn't been yet."""
if not self.container_ring: if not self.container_ring:
@ -70,7 +79,7 @@ class ObjectUpdater(Daemon):
pids = [] pids = []
# read from container ring to ensure it's fresh # read from container ring to ensure it's fresh
self.get_container_ring().get_nodes('') self.get_container_ring().get_nodes('')
for device in os.listdir(self.devices): for device in self._listdir(self.devices):
if self.mount_check and \ if self.mount_check and \
not ismount(os.path.join(self.devices, device)): not ismount(os.path.join(self.devices, device)):
self.logger.increment('errors') self.logger.increment('errors')
@ -113,7 +122,7 @@ class ObjectUpdater(Daemon):
begin = time.time() begin = time.time()
self.successes = 0 self.successes = 0
self.failures = 0 self.failures = 0
for device in os.listdir(self.devices): for device in self._listdir(self.devices):
if self.mount_check and \ if self.mount_check and \
not ismount(os.path.join(self.devices, device)): not ismount(os.path.join(self.devices, device)):
self.logger.increment('errors') self.logger.increment('errors')
@ -138,7 +147,7 @@ class ObjectUpdater(Daemon):
""" """
start_time = time.time() start_time = time.time()
# loop through async pending dirs for all policies # loop through async pending dirs for all policies
for asyncdir in os.listdir(device): for asyncdir in self._listdir(device):
# skip stuff like "accounts", "containers", etc. # skip stuff like "accounts", "containers", etc.
if not (asyncdir == ASYNCDIR_BASE or if not (asyncdir == ASYNCDIR_BASE or
asyncdir.startswith(ASYNCDIR_BASE + '-')): asyncdir.startswith(ASYNCDIR_BASE + '-')):
@ -161,12 +170,12 @@ class ObjectUpdater(Daemon):
'valid policy') % asyncdir) 'valid policy') % asyncdir)
continue continue
for prefix in os.listdir(async_pending): for prefix in self._listdir(async_pending):
prefix_path = os.path.join(async_pending, prefix) prefix_path = os.path.join(async_pending, prefix)
if not os.path.isdir(prefix_path): if not os.path.isdir(prefix_path):
continue continue
last_obj_hash = None last_obj_hash = None
for update in sorted(os.listdir(prefix_path), reverse=True): for update in sorted(self._listdir(prefix_path), reverse=True):
update_path = os.path.join(prefix_path, update) update_path = os.path.join(prefix_path, update)
if not os.path.isfile(update_path): if not os.path.isfile(update_path):
continue continue

View File

@ -23,6 +23,7 @@ from contextlib import closing
from gzip import GzipFile from gzip import GzipFile
from tempfile import mkdtemp from tempfile import mkdtemp
from shutil import rmtree from shutil import rmtree
from test.unit import FakeLogger
from time import time from time import time
from distutils.dir_util import mkpath from distutils.dir_util import mkpath
@ -91,6 +92,39 @@ class TestObjectUpdater(unittest.TestCase):
self.assertEquals(cu.node_timeout, 5) self.assertEquals(cu.node_timeout, 5)
self.assert_(cu.get_container_ring() is not None) self.assert_(cu.get_container_ring() is not None)
@mock.patch('os.listdir')
def test_listdir_with_exception(self, mock_listdir):
e = OSError('permission_denied')
mock_listdir.side_effect = e
# setup updater
conf = {
'devices': self.devices_dir,
'mount_check': 'false',
'swift_dir': self.testdir,
}
daemon = object_updater.ObjectUpdater(conf)
daemon.logger = FakeLogger()
paths = daemon._listdir('foo/bar')
self.assertEqual([], paths)
log_lines = daemon.logger.get_lines_for_level('error')
msg = ('ERROR: Unable to access foo/bar: permission_denied')
self.assertEqual(log_lines[0], msg)
@mock.patch('os.listdir', return_value=['foo', 'bar'])
def test_listdir_without_exception(self, mock_listdir):
# setup updater
conf = {
'devices': self.devices_dir,
'mount_check': 'false',
'swift_dir': self.testdir,
}
daemon = object_updater.ObjectUpdater(conf)
daemon.logger = FakeLogger()
path = daemon._listdir('foo/bar/')
log_lines = daemon.logger.get_lines_for_level('error')
self.assertEqual(len(log_lines), 0)
self.assertEqual(path, ['foo', 'bar'])
def test_object_sweep(self): def test_object_sweep(self):
def check_with_idx(index, warn, should_skip): def check_with_idx(index, warn, should_skip):
if int(index) > 0: if int(index) > 0: