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

Proposed by Liam Young on 2015-10-02
Status: Merged
Merged at revision: 85
Proposed branch: lp:~gnuoy/charms/trusty/swift-storage/lp1496004
Merge into: lp:~openstack-charmers-archive/charms/trusty/swift-storage/next
Diff against target: 139 lines (+72/-6)
2 files modified
lib/swift_storage_utils.py (+39/-5)
unit_tests/test_swift_storage_utils.py (+33/-1)
To merge this branch: bzr merge lp:~gnuoy/charms/trusty/swift-storage/lp1496004
Reviewer Review Type Date Requested Status
Chris Glass (community) 2015-10-02 Approve on 2015-10-06
Review via email: mp+273209@code.launchpad.net
To post a comment you must log in.
87. By Liam Young on 2015-10-02

Add comment to get_mount_point

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/

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/

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/

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

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

88. By Liam Young on 2015-10-05

Empty commit to kick osci

Chris Glass (tribaal) wrote :

This looks good, thanks a lot!

+1, will merge once OSCI ran over it.

review: Approve

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/

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/

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

Good stuff, merging.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/swift_storage_utils.py'
2--- lib/swift_storage_utils.py 2015-09-02 10:52:38 +0000
3+++ lib/swift_storage_utils.py 2015-10-05 12:40:16 +0000
4@@ -3,7 +3,7 @@
5 import shutil
6 import tempfile
7
8-from subprocess import check_call, call, CalledProcessError
9+from subprocess import check_call, call, CalledProcessError, check_output
10
11 # Stuff copied from cinder py charm, needs to go somewhere
12 # common.
13@@ -165,19 +165,53 @@
14 return is_block_device(partition) and not is_device_mounted(partition)
15
16
17-def find_block_devices():
18+def get_mount_point(device):
19+ mnt_point = None
20+ try:
21+ out = check_output(['findmnt', device])
22+ mnt_points = []
23+ for line in out.split('\n'):
24+ if line and not line.startswith('TARGET'):
25+ mnt_points.append(line.split()[0])
26+ if len(mnt_points) > 1:
27+ log('Device {} mounted in multiple times, ignoring'.format(device))
28+ else:
29+ mnt_point = mnt_points[0]
30+ except CalledProcessError:
31+ # findmnt returns non-zero rc if dev not mounted
32+ pass
33+ return mnt_point
34+
35+
36+def find_block_devices(include_mounted=False):
37 found = []
38 incl = ['sd[a-z]', 'vd[a-z]', 'cciss\/c[0-9]d[0-9]']
39
40 with open('/proc/partitions') as proc:
41- print proc
42 partitions = [p.split() for p in proc.readlines()[2:]]
43 for partition in [p[3] for p in partitions if p]:
44 for inc in incl:
45 _re = re.compile(r'^(%s)$' % inc)
46 if _re.match(partition):
47 found.append(os.path.join('/dev', partition))
48- return [f for f in found if _is_storage_ready(f)]
49+ if include_mounted:
50+ devs = [f for f in found if is_block_device(f)]
51+ else:
52+ devs = [f for f in found if _is_storage_ready(f)]
53+ return devs
54+
55+
56+def guess_block_devices():
57+ bdevs = find_block_devices(include_mounted=True)
58+ gdevs = []
59+ for dev in bdevs:
60+ if is_device_mounted(dev):
61+ mnt_point = get_mount_point(dev)
62+ if mnt_point and mnt_point.startswith('/srv/node'):
63+ gdevs.append(dev)
64+ else:
65+ gdevs.append(dev)
66+ return gdevs
67
68
69 def determine_block_devices():
70@@ -189,7 +223,7 @@
71 return None
72
73 if block_device == 'guess':
74- bdevs = find_block_devices()
75+ bdevs = guess_block_devices()
76 else:
77 bdevs = block_device.split(' ')
78
79
80=== modified file 'unit_tests/test_swift_storage_utils.py'
81--- unit_tests/test_swift_storage_utils.py 2015-07-17 15:52:38 +0000
82+++ unit_tests/test_swift_storage_utils.py 2015-10-05 12:40:16 +0000
83@@ -75,6 +75,11 @@
84 8 16 119454720 sdb1
85 """
86
87+FINDMNT_FOUND_TEMPLATE = """
88+TARGET SOURCE FSTYPE OPTIONS
89+{} /dev/{} xfs rw,relatime,attr2,inode64,noquota
90+"""
91+
92
93 class SwiftStorageUtilsTests(CharmTestCase):
94
95@@ -153,9 +158,17 @@
96 ex = ['/dev/vdb', '/srv/swift.img']
97 self.assertEqual(ex, result)
98
99+ @patch.object(swift_utils, 'check_output')
100 @patch.object(swift_utils, 'find_block_devices')
101 @patch.object(swift_utils, 'ensure_block_device')
102- def test_determine_block_device_guess_dev(self, _ensure, _find):
103+ def test_determine_block_device_guess_dev(self, _ensure, _find,
104+ _check_output):
105+ "Devices already mounted under /srv/node/ should be returned"
106+ def _findmnt(cmd):
107+ dev = cmd[1].split('/')[-1]
108+ mnt_point = '/srv/node/' + dev
109+ return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
110+ _check_output.side_effect = _findmnt
111 _ensure.side_effect = self._fake_ensure
112 self.test_config.set('block-device', 'guess')
113 _find.return_value = ['/dev/vdb', '/dev/sdb']
114@@ -163,6 +176,25 @@
115 self.assertTrue(_find.called)
116 self.assertEquals(result, ['/dev/vdb', '/dev/sdb'])
117
118+ @patch.object(swift_utils, 'check_output')
119+ @patch.object(swift_utils, 'find_block_devices')
120+ @patch.object(swift_utils, 'ensure_block_device')
121+ def test_determine_block_device_guess_dev_not_eligable(self, _ensure,
122+ _find,
123+ _check_output):
124+ "Devices not mounted under /srv/node/ should not be returned"
125+ def _findmnt(cmd):
126+ dev = cmd[1].split('/')[-1]
127+ mnt_point = '/'
128+ return FINDMNT_FOUND_TEMPLATE.format(mnt_point, dev)
129+ _check_output.side_effect = _findmnt
130+ _ensure.side_effect = self._fake_ensure
131+ self.test_config.set('block-device', 'guess')
132+ _find.return_value = ['/dev/vdb']
133+ result = swift_utils.determine_block_devices()
134+ self.assertTrue(_find.called)
135+ self.assertEquals(result, [])
136+
137 def test_mkfs_xfs(self):
138 swift_utils.mkfs_xfs('/dev/sdb')
139 self.check_call.assert_called_with(

Subscribers

People subscribed via source and target branches