Merge "Fix checkmount error parsing in swift-recon"

bin/swift-recon

@@ -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):

swift/common/middleware/recon.py

@@ -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
 

test/unit/common/middleware/test_recon.py

@@ -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,