Merge lp:~gnuoy/charms/trusty/swift-storage/lp1496004 into lp:~openstack-charmers-archive/charms/trusty/swift-storage/next

[Liam Young]

[lp:~gnuoy/charms/trusty/swift-storage/lp1496004]

87. By Liam Young on 2015-10-02

Add comment to get_mount_point

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10361 swift-storage-next for gnuoy mp273209
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10361/

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11158 swift-storage-next for gnuoy mp273209
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11158/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6988 swift-storage-next for gnuoy mp273209
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 124
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12639377/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6988/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #6993 swift-storage-next for gnuoy mp273209
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12640819/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6993/

[David Britton (dpb)]

I have tested a full deploy in the autopilot with swift/ceph and this charm fixes the problem (with devices: guess).

[lp:~gnuoy/charms/trusty/swift-storage/lp1496004]

88. By Liam Young on 2015-10-05

Empty commit to kick osci

[Chris Glass (tribaal)]

This looks good, thanks a lot!

+1, will merge once OSCI ran over it.

review: Approve

[uosci-testing-bot (uosci-testing-bot)]

charm_lint_check #11326 swift-storage-next for gnuoy mp273209
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/11326/

[uosci-testing-bot (uosci-testing-bot)]

charm_unit_test #10520 swift-storage-next for gnuoy mp273209
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/10520/

[uosci-testing-bot (uosci-testing-bot)]

charm_amulet_test #7110 swift-storage-next for gnuoy mp273209
    AMULET OK: passed

Build: http://10.245.162.77:8080/job/charm_amulet_test/7110/

[Chris Glass (tribaal)]

Good stuff, merging.

review: Approve

=== modified file 'lib/swift_storage_utils.py'
--- lib/swift_storage_utils.py	2015-09-02 10:52:38 +0000
+++ lib/swift_storage_utils.py	2015-10-05 12:40:16 +0000
@@ -3,7 +3,7 @@
 import shutil
 import tempfile
 
-from subprocess import check_call, call, CalledProcessError
+from subprocess import check_call, call, CalledProcessError, check_output
 
 # Stuff copied from cinder py charm, needs to go somewhere
 # common.
@@ -165,19 +165,53 @@
     return is_block_device(partition) and not is_device_mounted(partition)
 
 
-def find_block_devices():
+def get_mount_point(device):
+    mnt_point = None
+    try:
+        out = check_output(['findmnt', device])
+        mnt_points = []
+        for line in out.split('\n'):
+            if line and not line.startswith('TARGET'):
+                mnt_points.append(line.split()[0])
+        if len(mnt_points) > 1:
+            log('Device {} mounted in multiple times, ignoring'.format(device))
+        else:
+            mnt_point = mnt_points[0]
+    except CalledProcessError:
+        # findmnt returns non-zero rc if dev not mounted
+        pass
+    return mnt_point
+
+
+def find_block_devices(include_mounted=False):
     found = []
     incl = ['sd[a-z]', 'vd[a-z]', 'cciss\/c[0-9]d[0-9]']
 
     with open('/proc/partitions') as proc:
-        print proc
         partitions = [p.split() for p in proc.readlines()[2:]]
     for partition in [p[3] for p in partitions if p]:
         for inc in incl:
             _re = re.compile(r'^(%s)$' % inc)
             if _re.match(partition):
                 found.append(os.path.join('/dev', partition))
-    return [f for f in found if _is_storage_ready(f)]
+    if include_mounted:
+        devs = [f for f in found if is_block_device(f)]
+    else:
+        devs = [f for f in found if _is_storage_ready(f)]
+    return devs
+
+
+def guess_block_devices():
+    bdevs = find_block_devices(include_mounted=True)
+    gdevs = []
+    for dev in bdevs:
+        if is_device_mounted(dev):
+            mnt_point = get_mount_point(dev)
+            if mnt_point and mnt_point.startswith('/srv/node'):
+                gdevs.append(dev)
+        else:
+            gdevs.append(dev)
+    return gdevs
 
 
 def determine_block_devices():
@@ -189,7 +223,7 @@
         return None
 
     if block_device == 'guess':
-        bdevs = find_block_devices()
+        bdevs = guess_block_devices()
     else:
         bdevs = block_device.split(' ')
 

=== modified file 'unit_tests/test_swift_storage_utils.py'
--- unit_tests/test_swift_storage_utils.py	2015-07-17 15:52:38 +0000
+++ unit_tests/test_swift_storage_utils.py	2015-10-05 12:40:16 +0000
@@ -75,6 +75,11 @@
    8       16  119454720 sdb1
 """
 
+FINDMNT_FOUND_TEMPLATE = """
+TARGET        SOURCE   FSTYPE OPTIONS
+{}           /dev/{} xfs    rw,relatime,attr2,inode64,noquota
+"""
+
 
 class SwiftStorageUtilsTests(CharmTestCase):
 
@@ -153,9 +158,17 @@
         ex = ['/dev/vdb', '/srv/swift.img']
         self.assertEqual(ex, result)
 
+    @patch.object(swift_utils, 'check_output')
     @patch.object(swift_utils, 'find_block_devices')
     @patch.object(swift_utils, 'ensure_block_device')
-    def test_determine_block_device_guess_dev(self, _ensure, _find):
+    def test_determine_block_device_guess_dev(self, _ensure, _find,
+                                              _check_output):
+        "Devices already mounted under /srv/node/ should be returned"
+        def _findmnt(cmd):
+            dev = cmd[1].split('/')[-1]
+            mnt_point = '/srv/node/' + dev
+            return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
+        _check_output.side_effect = _findmnt
         _ensure.side_effect = self._fake_ensure
         self.test_config.set('block-device', 'guess')
         _find.return_value = ['/dev/vdb', '/dev/sdb']
@@ -163,6 +176,25 @@
         self.assertTrue(_find.called)
         self.assertEquals(result, ['/dev/vdb', '/dev/sdb'])
 
+    @patch.object(swift_utils, 'check_output')
+    @patch.object(swift_utils, 'find_block_devices')
+    @patch.object(swift_utils, 'ensure_block_device')
+    def test_determine_block_device_guess_dev_not_eligable(self, _ensure,
+                                                           _find,
+                                                           _check_output):
+        "Devices not mounted under /srv/node/ should not be returned"
+        def _findmnt(cmd):
+            dev = cmd[1].split('/')[-1]
+            mnt_point = '/'
+            return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
+        _check_output.side_effect = _findmnt
+        _ensure.side_effect = self._fake_ensure
+        self.test_config.set('block-device', 'guess')
+        _find.return_value = ['/dev/vdb']
+        result = swift_utils.determine_block_devices()
+        self.assertTrue(_find.called)
+        self.assertEquals(result, [])
+
     def test_mkfs_xfs(self):
         swift_utils.mkfs_xfs('/dev/sdb')
         self.check_call.assert_called_with(