Merge lp:~tribaal/charms/trusty/swift-storage/backport-1496004-to-stable into lp:~openstack-charmers-archive/charms/trusty/swift-storage/trunk

Proposed by Chris Glass
Status: Merged
Merged at revision: 72
Proposed branch: lp:~tribaal/charms/trusty/swift-storage/backport-1496004-to-stable
Merge into: lp:~openstack-charmers-archive/charms/trusty/swift-storage/trunk
Diff against target: 139 lines (+72/-6)
2 files modified
hooks/swift_storage_utils.py (+39/-5)
unit_tests/test_swift_storage_utils.py (+33/-1)
To merge this branch: bzr merge lp:~tribaal/charms/trusty/swift-storage/backport-1496004-to-stable
Reviewer Review Type Date Requested Status
Liam Young Approve
David Britton (community) Approve
Review via email: mp+273547@code.launchpad.net

Description of the change

This is a backport of the fixes introduced in https://code.launchpad.net/~gnuoy/charms/trusty/swift-storage/lp1496004/+merge/273209 .

Note that the only difference in terms of diff is that the swift_storage_utils.py file is in the "hooks" folder here instead of the "lib/" folder in the -next branch.

To post a comment you must log in.
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_unit_test #10581 swift-storage for tribaal mp273547
    UNIT OK: passed

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

Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_lint_check #11390 swift-storage for tribaal mp273547
    LINT FAIL: lint-test failed
    LINT FAIL: charm-proof failed

LINT Results (max last 2 lines):
make: *** [lint] Error 100
ERROR:root:Make target returned non-zero.

Full lint test output: http://paste.ubuntu.com/12697465/
Build: http://10.245.162.77:8080/job/charm_lint_check/11390/

Revision history for this message
David Britton (dpb) wrote :

Looks like a straight backport, diff looks good. +1

review: Approve
Revision history for this message
uosci-testing-bot (uosci-testing-bot) wrote :

charm_amulet_test #7137 swift-storage for tribaal mp273547
    AMULET OK: passed

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

Revision history for this message
Liam Young (gnuoy) wrote :

Approved

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/swift_storage_utils.py'
2--- hooks/swift_storage_utils.py 2015-07-01 20:53:27 +0000
3+++ hooks/swift_storage_utils.py 2015-10-06 15:16:25 +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@@ -177,19 +177,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@@ -201,7 +235,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-04-02 18:56:27 +0000
82+++ unit_tests/test_swift_storage_utils.py 2015-10-06 15:16:25 +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