openstack/swift
Code Issues Proposed changes

Fix checkmount error parsing in swift-recon

Browse Source 
- swift-recon now handles parsing instances where 'mounted' key (in unmounted
  and disk_usage) is an error message instead of a bool.
- Add's checkmount exception handling to the recon umounted endpoint.
- Updates existing unittest to have ismount throw an error.
- Updates unittests to cover the corner cases

Change-Id: Id51d14a8b98de69faaac84b2b34b7404b7df69e9
This commit is contained in:
Florian Hines 2013-12-18 14:03:23 -06:00 committed by John Dickinson
parent c72c10a1fe
commit 62254e42c4
3 changed files with 50 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
bin/swift-recon
View File

@ -242,20 +242,29 @@ class SwiftRecon(object):
:param hosts: set of hosts to check. in the format of:
set([('127.0.0.1', 6020), ('127.0.0.2', 6030)])
"""
stats = {}
unmounted = {}
errors = {}
recon = Scout("unmounted", self.verbose, self.suppress_errors,
self.timeout)
print "[%s] Getting unmounted drives from %s hosts..." % \
(self._ptime(), len(hosts))
for url, response, status in self.pool.imap(recon.scout, hosts):
if status == 200:
stats[url] = []
unmounted[url] = []
errors[url] = []
for i in response:
stats[url].append(i['device'])
for host in stats:
if not isinstance(i['mounted'], bool):
errors[url].append(i['device'])
else:
unmounted[url].append(i['device'])
for host in unmounted:
node = urlparse(host).netloc
for entry in stats[host]:
for entry in unmounted[host]:
print "Not mounted: %s on %s" % (entry, node)
for host in errors:
node = urlparse(host).netloc
for entry in errors[host]:
print "Device errors: %s on %s" % (entry, node)
print "=" * 79
def expirer_check(self, hosts):
@ -655,7 +664,10 @@ class SwiftRecon(object):
if status == 200:
hostusage = []
for entry in response:
if entry['mounted']:
if not isinstance(entry['mounted'], bool):
print "-> %s/%s: Error: %s" % (url, entry['device'],
entry['mounted'])
elif entry['mounted']:
used = float(entry['used']) / float(entry['size']) \
* 100.0
raw_total_used.append(entry['used'])
@ -672,7 +684,7 @@ class SwiftRecon(object):
for url in stats:
if len(stats[url]) > 0:
#get per host hi/los for another day
# get per host hi/los for another day
low = min(stats[url])
high = max(stats[url])
highs.append(high)
@ -685,7 +697,7 @@ class SwiftRecon(object):
if len(lows) > 0:
low = min(lows)
high = max(highs)
#dist graph shamelessly stolen from https://github.com/gholt/tcod
# dist graph shamelessly stolen from https://github.com/gholt/tcod
print "Distribution Graph:"
mul = 69.0 / max(percents.values())
for percent in sorted(percents):

9
swift/common/middleware/recon.py
View File

@ -191,9 +191,12 @@ class ReconMiddleware(object):
"""list unmounted (failed?) devices"""
mountlist = []
for entry in os.listdir(self.devices):
mpoint = {'device': entry,
'mounted': check_mount(self.devices, entry)}
if not mpoint['mounted']:
try:
mounted = check_mount(self.devices, entry)
except OSError as err:
mounted = str(err)
mpoint = {'device': entry, 'mounted': mounted}
if mpoint['mounted'] is not True:
mountlist.append(mpoint)
return mountlist

28
test/unit/common/middleware/test_recon.py
View File

@ -101,7 +101,10 @@ class MockOS(object):
def fake_ismount(self, *args, **kwargs):
self.ismount_calls.append((args, kwargs))
return self.ismount_output
if isinstance(self.ismount_output, Exception):
raise self.ismount_output
else:
return self.ismount_output
def fake_statvfs(self, *args, **kwargs):
self.statvfs_calls.append((args, kwargs))
@ -560,7 +563,6 @@ class TestReconSuccess(TestCase):
"quarantined": 0}})
def test_get_unmounted(self):
unmounted_resp = [{'device': 'fakeone', 'mounted': False},
{'device': 'faketwo', 'mounted': False}]
self.mockos.ls_output = ['fakeone', 'faketwo']
@ -569,6 +571,24 @@ class TestReconSuccess(TestCase):
self.assertEquals(self.mockos.listdir_calls, [(('/srv/node/',), {})])
self.assertEquals(rv, unmounted_resp)
def test_get_unmounted_everything_normal(self):
unmounted_resp = []
self.mockos.ls_output = ['fakeone', 'faketwo']
self.mockos.ismount_output = True
rv = self.app.get_unmounted()
self.assertEquals(self.mockos.listdir_calls, [(('/srv/node/',), {})])
self.assertEquals(rv, unmounted_resp)
def test_get_unmounted_checkmount_fail(self):
unmounted_resp = [{'device': 'fakeone', 'mounted': 'brokendrive'}]
self.mockos.ls_output = ['fakeone']
self.mockos.ismount_output = OSError('brokendrive')
rv = self.app.get_unmounted()
self.assertEquals(self.mockos.listdir_calls, [(('/srv/node/',), {})])
self.assertEquals(self.mockos.ismount_calls,
[(('/srv/node/fakeone',), {})])
self.assertEquals(rv, unmounted_resp)
def test_no_get_unmounted(self):
def fake_checkmount_true(*args):
@ -601,9 +621,9 @@ class TestReconSuccess(TestCase):
def test_get_diskusage_checkmount_fail(self):
du_resp = [{'device': 'canhazdrive1', 'avail': '',
'mounted': False, 'used': '', 'size': ''}]
'mounted': 'brokendrive', 'used': '', 'size': ''}]
self.mockos.ls_output = ['canhazdrive1']
self.mockos.ismount_output = False
self.mockos.ismount_output = OSError('brokendrive')
rv = self.app.get_diskusage()
self.assertEquals(self.mockos.listdir_calls, [(('/srv/node/',), {})])
self.assertEquals(self.mockos.ismount_calls,