Merge ~raharper/curtin:fix/mpath-partition-wiping into curtin:master
- Git
- lp:~raharper/curtin
- fix/mpath-partition-wiping
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Dan Watkins | ||||
Approved revision: | 6a07152bfbed01ea2589320128d431de543e5baa | ||||
Merge reported by: | Server Team CI bot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~raharper/curtin:fix/mpath-partition-wiping | ||||
Merge into: | curtin:master | ||||
Diff against target: |
640 lines (+236/-25) 6 files modified
curtin/block/clear_holders.py (+32/-11) curtin/block/multipath.py (+74/-2) examples/tests/dirty_disks_config.yaml (+8/-0) tests/unittests/test_block_multipath.py (+87/-6) tests/unittests/test_clear_holders.py (+34/-6) tests/vmtests/test_multipath.py (+1/-0) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Dan Watkins (community) | Approve | ||
Server Team CI bot | continuous-integration | Approve | |
Review via email: mp+377927@code.launchpad.net |
Commit message
multipath: handle removal of multipath partitions correctly
Curtin could handle multipath-partition removal if it was given
the device-mapper device (dm-x) however, if it was fed the
underying scsi device (e.g. sda2) this would fail when curtin
would try to clear the partition requesting exclusive access
which it could not obtain due to the device-mapper device.
This branch implements a kernel scsi device lookup to map
sdXN -> dm-N mapping which resolves the issue by allowing
curtin to wipe the dm-X device even if it has been
provided the sdXN device.
Additional Changes
- Enable dirty_disk mode on multipath test to reproduce
failure
LP: #1857042
Description of the change
- 5514b58... by Ryan Harper
-
Handle multipath not installed in dirty_disk setup.
Server Team CI bot (server-team-bot) wrote : | # |
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:5514b585ae4
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Dan Watkins (oddbloke) wrote : | # |
A few inline comments/questions. Thanks!
- cddfb4d... by Ryan Harper
-
Update comment and remove unneeded variable initialization.
- 79dd492... by Ryan Harper
-
Refactor logic to drop unneeded mpath_is_member call
- 10b2611... by Ryan Harper
-
multipath: move up no device found check
- 79748dd... by Ryan Harper
-
Handle dmsetup names with spaces
Ryan Harper (raharper) wrote : | # |
Working on fixes, thanks for the review, update coming in just a bit
- 36cdc45... by Ryan Harper
-
Add an additional path to make test more clear.
- 9f40323... by Ryan Harper
-
Use assertIsNone
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:9f403230ba2
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
Server Team CI bot (server-team-bot) wrote : | # |
FAILED: Continuous integration, rev:9f403230ba2
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 1acb477... by Ryan Harper
-
multipath: add docstrings to unittests
Also, update the negative case for dmsetup ls output.
- 4b17684... by Ryan Harper
-
multipath: update docstrings and add supported check
Bionic and older systems do not have multipath installed by
default so curtin will check if multipath tools are present
and only issues multipath commands if they are needed. This
fixes the bionic basic test on this branch. - dd507cc... by Ryan Harper
-
Add footer to multipath.py
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:dd507cc6916
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- 6a07152... by Ryan Harper
-
multipath: try harder when removing multipath devices, add verbosity
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:6a07152bfbe
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | diff --git a/curtin/block/clear_holders.py b/curtin/block/clear_holders.py |
2 | index 10e7dc5..08aa70a 100644 |
3 | --- a/curtin/block/clear_holders.py |
4 | +++ b/curtin/block/clear_holders.py |
5 | @@ -285,7 +285,25 @@ def wipe_superblock(device): |
6 | else: |
7 | bcache._stop_device(stop_path) |
8 | |
9 | - _wipe_superblock(blockdev) |
10 | + # the blockdev (e.g. /dev/sda2) may be a multipath partition which can |
11 | + # only be wiped via its device mapper device (e.g. /dev/dm-4) |
12 | + # check for this and determine the correct device mapper value to use. |
13 | + mp_dev = None |
14 | + mp_support = multipath.multipath_supported() |
15 | + if mp_support: |
16 | + parent, partnum = block.get_blockdev_for_partition(blockdev) |
17 | + parent_mpath_id = multipath.find_mpath_id_by_path(parent) |
18 | + if parent_mpath_id is not None: |
19 | + # construct multipath dmsetup id |
20 | + # <mpathid>-part%d -> /dev/dm-1 |
21 | + mp_id, mp_dev = multipath.find_mpath_id_by_parent(parent_mpath_id, |
22 | + partnum=partnum) |
23 | + # if we don't find a mapping then the mp partition has already been |
24 | + # wiped/removed |
25 | + if mp_dev: |
26 | + _wipe_superblock(mp_dev) |
27 | + else: |
28 | + _wipe_superblock(blockdev) |
29 | |
30 | # if we had partitions, make sure they've been removed |
31 | if partitions: |
32 | @@ -308,16 +326,19 @@ def wipe_superblock(device): |
33 | device, attempt + 1, len(retries), wait) |
34 | time.sleep(wait) |
35 | |
36 | - # multipath partitions are separate block devices (disks) |
37 | - if multipath.is_mpath_partition(blockdev): |
38 | - multipath.remove_partition(blockdev) |
39 | - # multipath devices must be hidden to utilize a single member (path) |
40 | - elif multipath.is_mpath_device(blockdev): |
41 | - mp_id = multipath.find_mpath_id(blockdev) |
42 | - if mp_id: |
43 | - multipath.remove_map(mp_id) |
44 | - else: |
45 | - raise RuntimeError('Failed to find multipath id for %s' % blockdev) |
46 | + if mp_support: |
47 | + # multipath partitions are separate block devices (disks) |
48 | + if mp_dev or multipath.is_mpath_partition(blockdev): |
49 | + multipath.remove_partition(mp_dev if mp_dev else blockdev) |
50 | + # multipath devices must be hidden to utilize a single member (path) |
51 | + elif multipath.is_mpath_device(blockdev): |
52 | + mp_id = multipath.find_mpath_id(blockdev) |
53 | + multipath.remove_partition(blockdev) |
54 | + if mp_id: |
55 | + multipath.remove_map(mp_id) |
56 | + else: |
57 | + raise RuntimeError( |
58 | + 'Failed to find multipath id for %s' % blockdev) |
59 | |
60 | |
61 | def _wipe_superblock(blockdev, exclusive=True, strict=True): |
62 | diff --git a/curtin/block/multipath.py b/curtin/block/multipath.py |
63 | index d1e8441..8ce0509 100644 |
64 | --- a/curtin/block/multipath.py |
65 | +++ b/curtin/block/multipath.py |
66 | @@ -11,6 +11,7 @@ SHOW_MAPS_FMT = "name=%n multipath='%w' sysfs='%d' paths='%N'" |
67 | |
68 | |
69 | def _extract_mpath_data(cmd, show_verb): |
70 | + """ Parse output from specifed command output via load_shell_content.""" |
71 | data, _err = util.subp(cmd, capture=True) |
72 | result = [] |
73 | for line in data.splitlines(): |
74 | @@ -23,16 +24,34 @@ def _extract_mpath_data(cmd, show_verb): |
75 | |
76 | |
77 | def show_paths(): |
78 | + """ Query multipathd for paths output and return a dict of the values.""" |
79 | cmd = ['multipathd', 'show', 'paths', 'raw', 'format', SHOW_PATHS_FMT] |
80 | return _extract_mpath_data(cmd, 'paths') |
81 | |
82 | |
83 | def show_maps(): |
84 | + """ Query multipathd for maps output and return a dict of the values.""" |
85 | cmd = ['multipathd', 'show', 'maps', 'raw', 'format', SHOW_MAPS_FMT] |
86 | return _extract_mpath_data(cmd, 'maps') |
87 | |
88 | |
89 | +def dmname_to_blkdev_mapping(): |
90 | + """ Use dmsetup ls output to build a dict of DM_NAME, /dev/dm-x values.""" |
91 | + data, _err = util.subp(['dmsetup', 'ls', '-o', 'blkdevname'], capture=True) |
92 | + mapping = {} |
93 | + if data and data.strip() != "No devices found": |
94 | + LOG.debug('multipath: dmsetup ls output:\n%s', data) |
95 | + for line in data.splitlines(): |
96 | + if line: |
97 | + dm_name, blkdev = line.split('\t') |
98 | + # (dm-1) -> /dev/dm-1 |
99 | + mapping[dm_name] = '/dev/' + blkdev.strip('()') |
100 | + |
101 | + return mapping |
102 | + |
103 | + |
104 | def is_mpath_device(devpath): |
105 | + """ Check if devpath is a multipath device, returns boolean. """ |
106 | info = udev.udevadm_info(devpath) |
107 | if info.get('DM_UUID', '').startswith('mpath-'): |
108 | return True |
109 | @@ -41,6 +60,7 @@ def is_mpath_device(devpath): |
110 | |
111 | |
112 | def is_mpath_member(devpath): |
113 | + """ Check if a device is a multipath member (a path), returns boolean. """ |
114 | try: |
115 | util.subp(['multipath', '-c', devpath], capture=True) |
116 | return True |
117 | @@ -49,6 +69,7 @@ def is_mpath_member(devpath): |
118 | |
119 | |
120 | def is_mpath_partition(devpath): |
121 | + """ Check if a device is a multipath partition, returns boolean. """ |
122 | if devpath.startswith('/dev/dm-'): |
123 | if 'DM_PART' in udev.udevadm_info(devpath): |
124 | LOG.debug("%s is multipath device partition", devpath) |
125 | @@ -58,6 +79,7 @@ def is_mpath_partition(devpath): |
126 | |
127 | |
128 | def mpath_partition_to_mpath_id(devpath): |
129 | + """ Return the mpath id of a multipath partition. """ |
130 | info = udev.udevadm_info(devpath) |
131 | if 'DM_MPATH' in info: |
132 | return info['DM_MPATH'] |
133 | @@ -66,9 +88,10 @@ def mpath_partition_to_mpath_id(devpath): |
134 | |
135 | |
136 | def remove_partition(devpath, retries=10): |
137 | + """ Remove a multipath partition mapping. """ |
138 | LOG.debug('multipath: removing multipath partition: %s', devpath) |
139 | for _ in range(0, retries): |
140 | - util.subp(['dmsetup', 'remove', devpath], rcs=[0, 1]) |
141 | + util.subp(['dmsetup', 'remove', '--force', '--retry', devpath]) |
142 | udev.udevadm_settle() |
143 | if not os.path.exists(devpath): |
144 | return |
145 | @@ -77,10 +100,11 @@ def remove_partition(devpath, retries=10): |
146 | |
147 | |
148 | def remove_map(map_id, retries=10): |
149 | + """ Remove a multipath device mapping. """ |
150 | LOG.debug('multipath: removing multipath map: %s', map_id) |
151 | devpath = '/dev/mapper/%s' % map_id |
152 | for _ in range(0, retries): |
153 | - util.subp(['multipath', '-f', map_id], rcs=[0, 1]) |
154 | + util.subp(['multipath', '-v3', '-R3', '-f', map_id], rcs=[0, 1]) |
155 | udev.udevadm_settle() |
156 | if not os.path.exists(devpath): |
157 | return |
158 | @@ -89,6 +113,7 @@ def remove_map(map_id, retries=10): |
159 | |
160 | |
161 | def find_mpath_members(multipath_id, paths=None): |
162 | + """ Return a list of device path for each member of aspecified mpath_id.""" |
163 | if not paths: |
164 | paths = show_paths() |
165 | |
166 | @@ -98,6 +123,7 @@ def find_mpath_members(multipath_id, paths=None): |
167 | |
168 | |
169 | def find_mpath_id(devpath, maps=None): |
170 | + """ Return the mpath_id associated with a specified device path. """ |
171 | if not maps: |
172 | maps = show_maps() |
173 | |
174 | @@ -109,3 +135,49 @@ def find_mpath_id(devpath, maps=None): |
175 | return mpmap['multipath'] |
176 | |
177 | return None |
178 | + |
179 | + |
180 | +def find_mpath_id_by_path(devpath, paths=None): |
181 | + """ Return the mpath_id associated with a specified device path. """ |
182 | + if not paths: |
183 | + paths = show_paths() |
184 | + |
185 | + for path in paths: |
186 | + if devpath == '/dev/' + path['device']: |
187 | + return path['multipath'] |
188 | + |
189 | + return None |
190 | + |
191 | + |
192 | +def find_mpath_id_by_parent(multipath_id, partnum=None): |
193 | + """ Return the mpath_id associated with a specified device path. """ |
194 | + devmap = dmname_to_blkdev_mapping() |
195 | + LOG.debug('multipath: dm_name blk map: %s', devmap) |
196 | + dm_name = multipath_id |
197 | + if partnum: |
198 | + dm_name += "-part%d" % int(partnum) |
199 | + |
200 | + return (dm_name, devmap.get(dm_name)) |
201 | + |
202 | + |
203 | +def multipath_supported(): |
204 | + """Return a boolean indicating if multipath is supported.""" |
205 | + try: |
206 | + multipath_assert_supported() |
207 | + return True |
208 | + except RuntimeError: |
209 | + return False |
210 | + |
211 | + |
212 | +def multipath_assert_supported(): |
213 | + """ Determine if the runtime system supports multipath. |
214 | + returns: True if system supports multipath |
215 | + raises: RuntimeError: if system does not support multipath |
216 | + """ |
217 | + missing_progs = [p for p in ('multipath', 'multipathd') |
218 | + if not util.which(p)] |
219 | + if missing_progs: |
220 | + raise RuntimeError( |
221 | + "Missing multipath utils: %s" % ','.join(missing_progs)) |
222 | + |
223 | +# vi: ts=4 expandtab syntax=python |
224 | diff --git a/examples/tests/dirty_disks_config.yaml b/examples/tests/dirty_disks_config.yaml |
225 | index 9fd68ab..2f86f9e 100644 |
226 | --- a/examples/tests/dirty_disks_config.yaml |
227 | +++ b/examples/tests/dirty_disks_config.yaml |
228 | @@ -22,6 +22,13 @@ bucket: |
229 | done |
230 | swapon --show |
231 | exit 0 |
232 | + - &multipath_on | |
233 | + #!/bin/sh |
234 | + if command -v multipath; then |
235 | + multipath -ll |
236 | + multipath -r |
237 | + multipath -ll |
238 | + fi |
239 | - &zpool_export | |
240 | #!/bin/sh |
241 | # disable any rpools to trigger disks with zfs_member label but inactive |
242 | @@ -80,6 +87,7 @@ early_commands: |
243 | WORKING_DIR=/tmp/my.bdir/work.d, |
244 | curtin, --showtrace, -v, block-meta, --umount, custom] |
245 | 02-enable_swaps: [sh, -c, *swapon] |
246 | + 02-multipath_on: [sh, -c, *multipath_on] |
247 | 03-disable_rpool: [sh, -c, *zpool_export] |
248 | 04-lvm_stop: [sh, -c, *lvm_stop] |
249 | 05-mdadm_stop: [sh, -c, *mdadm_stop] |
250 | diff --git a/tests/unittests/test_block_multipath.py b/tests/unittests/test_block_multipath.py |
251 | index 0964349..b0a8e32 100644 |
252 | --- a/tests/unittests/test_block_multipath.py |
253 | +++ b/tests/unittests/test_block_multipath.py |
254 | @@ -4,6 +4,14 @@ from curtin.block import multipath |
255 | from .helpers import CiTestCase, raise_pexec_error |
256 | |
257 | |
258 | +# dmsetup uses tabs as separators |
259 | +DMSETUP_LS_BLKDEV_OUTPUT = '''\ |
260 | +mpatha (dm-0) |
261 | +mpatha-part1 (dm-1) |
262 | +1gb zero (dm-2) |
263 | +''' |
264 | + |
265 | + |
266 | class TestMultipath(CiTestCase): |
267 | |
268 | def setUp(self): |
269 | @@ -14,6 +22,7 @@ class TestMultipath(CiTestCase): |
270 | self.m_subp.return_value = ("", "") |
271 | |
272 | def test_show_paths(self): |
273 | + """verify show_paths extracts mulitpath path data correctly.""" |
274 | self.m_subp.return_value = ("foo=bar wark=2", "") |
275 | expected = ['multipathd', 'show', 'paths', 'raw', 'format', |
276 | multipath.SHOW_PATHS_FMT] |
277 | @@ -22,6 +31,7 @@ class TestMultipath(CiTestCase): |
278 | self.m_subp.assert_called_with(expected, capture=True) |
279 | |
280 | def test_show_maps(self): |
281 | + """verify show_maps extracts mulitpath map data correctly.""" |
282 | self.m_subp.return_value = ("foo=bar wark=2", "") |
283 | expected = ['multipathd', 'show', 'maps', 'raw', 'format', |
284 | multipath.SHOW_MAPS_FMT] |
285 | @@ -30,29 +40,36 @@ class TestMultipath(CiTestCase): |
286 | self.m_subp.assert_called_with(expected, capture=True) |
287 | |
288 | def test_is_mpath_device_true(self): |
289 | + """is_mpath_device returns true if dev DM_UUID starts with mpath-""" |
290 | self.m_udev.udevadm_info.return_value = {'DM_UUID': 'mpath-mpatha-foo'} |
291 | self.assertTrue(multipath.is_mpath_device(self.random_string())) |
292 | |
293 | def test_is_mpath_device_false(self): |
294 | + """is_mpath_device returns false when DM_UUID doesnt start w/ mpath-""" |
295 | self.m_udev.udevadm_info.return_value = {'DM_UUID': 'lvm-vg-foo-lv1'} |
296 | self.assertFalse(multipath.is_mpath_device(self.random_string())) |
297 | |
298 | def test_is_mpath_member_true(self): |
299 | + """is_mpath_device returns false when DM_UUID doesnt start w/ mpath-""" |
300 | self.assertTrue(multipath.is_mpath_member(self.random_string())) |
301 | |
302 | def test_is_mpath_member_false(self): |
303 | + """is_mpath_member returns false if 'multipath -c <dev>' exits err.""" |
304 | self.m_subp.side_effect = raise_pexec_error |
305 | self.assertFalse(multipath.is_mpath_member(self.random_string())) |
306 | |
307 | def test_is_mpath_partition_true(self): |
308 | + """is_mpath_member returns true if 'multipath -c <dev>' exits 0.""" |
309 | dm_device = "/dev/dm-" + self.random_string() |
310 | self.m_udev.udevadm_info.return_value = {'DM_PART': '1'} |
311 | self.assertTrue(multipath.is_mpath_partition(dm_device)) |
312 | |
313 | def test_is_mpath_partition_false(self): |
314 | + """is_mpath_member returns false if DM_PART is not present for dev.""" |
315 | self.assertFalse(multipath.is_mpath_partition(self.random_string())) |
316 | |
317 | def test_mpath_partition_to_mpath_id(self): |
318 | + """mpath_part_to_mpath_id extracts MD_MPATH value from mp partition.""" |
319 | dev = self.random_string() |
320 | mpath_id = self.random_string() |
321 | self.m_udev.udevadm_info.return_value = {'DM_MPATH': mpath_id} |
322 | @@ -60,18 +77,20 @@ class TestMultipath(CiTestCase): |
323 | multipath.mpath_partition_to_mpath_id(dev)) |
324 | |
325 | def test_mpath_partition_to_mpath_id_none(self): |
326 | + """mpath_part_to_mpath_id returns none if DM_MPATH missing.""" |
327 | dev = self.random_string() |
328 | self.m_udev.udevadm_info.return_value = {} |
329 | - self.assertEqual(None, |
330 | - multipath.mpath_partition_to_mpath_id(dev)) |
331 | + self.assertIsNone(multipath.mpath_partition_to_mpath_id(dev)) |
332 | |
333 | @mock.patch('curtin.block.multipath.os.path.exists') |
334 | @mock.patch('curtin.block.multipath.util.wait_for_removal') |
335 | def test_remove_partition(self, m_wait, m_exists): |
336 | + """multipath.remove_partition runs dmsetup skips wait if dev gone.""" |
337 | devpath = self.random_string() |
338 | m_exists.side_effect = iter([True, True, False]) |
339 | multipath.remove_partition(devpath) |
340 | - expected = mock.call(['dmsetup', 'remove', devpath], rcs=[0, 1]) |
341 | + expected = mock.call( |
342 | + ['dmsetup', 'remove', '--force', '--retry', devpath]) |
343 | self.m_subp.assert_has_calls([expected] * 3) |
344 | m_wait.assert_not_called() |
345 | self.assertEqual(3, self.m_udev.udevadm_settle.call_count) |
346 | @@ -79,10 +98,12 @@ class TestMultipath(CiTestCase): |
347 | @mock.patch('curtin.block.multipath.os.path.exists') |
348 | @mock.patch('curtin.block.multipath.util.wait_for_removal') |
349 | def test_remove_partition_waits(self, m_wait, m_exists): |
350 | + """multipath.remove_partition runs dmsetup waits if dev still there.""" |
351 | devpath = self.random_string() |
352 | m_exists.side_effect = iter([True, True, True]) |
353 | multipath.remove_partition(devpath, retries=3) |
354 | - expected = mock.call(['dmsetup', 'remove', devpath], rcs=[0, 1]) |
355 | + expected = mock.call( |
356 | + ['dmsetup', 'remove', '--force', '--retry', devpath]) |
357 | self.m_subp.assert_has_calls([expected] * 3) |
358 | self.assertEqual(3, self.m_udev.udevadm_settle.call_count) |
359 | self.assertEqual(1, m_wait.call_count) |
360 | @@ -90,11 +111,13 @@ class TestMultipath(CiTestCase): |
361 | @mock.patch('curtin.block.multipath.os.path.exists') |
362 | @mock.patch('curtin.block.multipath.util.wait_for_removal') |
363 | def test_remove_map(self, m_wait, m_exists): |
364 | + """multipath.remove_map runs multipath -f skips wait if map gone.""" |
365 | map_id = self.random_string() |
366 | devpath = '/dev/mapper/%s' % map_id |
367 | m_exists.side_effect = iter([True, True, False]) |
368 | multipath.remove_map(devpath) |
369 | - expected = mock.call(['multipath', '-f', devpath], rcs=[0, 1]) |
370 | + expected = mock.call( |
371 | + ['multipath', '-v3', '-R3', '-f', devpath], rcs=[0, 1]) |
372 | self.m_subp.assert_has_calls([expected] * 3) |
373 | m_wait.assert_not_called() |
374 | self.assertEqual(3, self.m_udev.udevadm_settle.call_count) |
375 | @@ -102,16 +125,19 @@ class TestMultipath(CiTestCase): |
376 | @mock.patch('curtin.block.multipath.os.path.exists') |
377 | @mock.patch('curtin.block.multipath.util.wait_for_removal') |
378 | def test_remove_map_wait(self, m_wait, m_exists): |
379 | + """multipath.remove_map runs multipath -f wait if map remains.""" |
380 | map_id = self.random_string() |
381 | devpath = '/dev/mapper/%s' % map_id |
382 | m_exists.side_effect = iter([True, True, True]) |
383 | multipath.remove_map(devpath, retries=3) |
384 | - expected = mock.call(['multipath', '-f', devpath], rcs=[0, 1]) |
385 | + expected = mock.call( |
386 | + ['multipath', '-v3', '-R3', '-f', devpath], rcs=[0, 1]) |
387 | self.m_subp.assert_has_calls([expected] * 3) |
388 | self.assertEqual(3, self.m_udev.udevadm_settle.call_count) |
389 | self.assertEqual(1, m_wait.call_count) |
390 | |
391 | def test_find_mpath_members(self): |
392 | + """find_mpath_members enumerates kernel block devs of a mpath_id.""" |
393 | mp_id = 'mpatha' |
394 | paths = ['device=bar multipath=mpatha', |
395 | 'device=wark multipath=mpatha'] |
396 | @@ -120,6 +146,7 @@ class TestMultipath(CiTestCase): |
397 | sorted(multipath.find_mpath_members(mp_id))) |
398 | |
399 | def test_find_mpath_members_empty(self): |
400 | + """find_mpath_members returns empty list if mpath_id not found.""" |
401 | mp_id = self.random_string() |
402 | paths = ['device=bar multipath=mpatha', |
403 | 'device=wark multipath=mpatha'] |
404 | @@ -127,6 +154,7 @@ class TestMultipath(CiTestCase): |
405 | self.assertEqual([], multipath.find_mpath_members(mp_id)) |
406 | |
407 | def test_find_mpath_id(self): |
408 | + """find_mpath_id returns mpath_id if device is part of mpath group.""" |
409 | mp_id = 'mpatha' |
410 | maps = ['sysfs=bar multipath=mpatha', |
411 | 'sysfs=wark multipath=mpatha'] |
412 | @@ -134,16 +162,69 @@ class TestMultipath(CiTestCase): |
413 | self.assertEqual(mp_id, multipath.find_mpath_id('/dev/bar')) |
414 | |
415 | def test_find_mpath_id_name(self): |
416 | + """find_mpath_id_name returns the name if present in maps on match.""" |
417 | maps = ['sysfs=bar multipath=mpatha name=friendly', |
418 | 'sysfs=wark multipath=mpatha'] |
419 | self.m_subp.return_value = ("\n".join(maps), "") |
420 | self.assertEqual('friendly', multipath.find_mpath_id('/dev/bar')) |
421 | |
422 | def test_find_mpath_id_none(self): |
423 | + """find_mpath_id_name returns none when device is not part of maps.""" |
424 | maps = ['sysfs=bar multipath=mpatha', |
425 | 'sysfs=wark multipath=mpatha'] |
426 | self.m_subp.return_value = ("\n".join(maps), "") |
427 | self.assertEqual(None, multipath.find_mpath_id('/dev/foo')) |
428 | |
429 | + def test_dmname_to_blkdev_mapping(self): |
430 | + """dmname_to_blkdev_mapping returns dmname to blkdevice dictionary.""" |
431 | + self.m_subp.return_value = (DMSETUP_LS_BLKDEV_OUTPUT, "") |
432 | + expected_mapping = { |
433 | + 'mpatha': '/dev/dm-0', |
434 | + 'mpatha-part1': '/dev/dm-1', |
435 | + '1gb zero': '/dev/dm-2', |
436 | + } |
437 | + self.assertEqual(expected_mapping, |
438 | + multipath.dmname_to_blkdev_mapping()) |
439 | + |
440 | + def test_dmname_to_blkdev_mapping_empty(self): |
441 | + """dmname_to_blkdev_mapping returns empty dict when dmsetup is empty. |
442 | + """ |
443 | + self.m_subp.return_value = ("No devices found", "") |
444 | + expected_mapping = {} |
445 | + self.assertEqual(expected_mapping, |
446 | + multipath.dmname_to_blkdev_mapping()) |
447 | + |
448 | + @mock.patch('curtin.block.multipath.dmname_to_blkdev_mapping') |
449 | + def test_find_mpath_id_by_parent(self, m_dmmap): |
450 | + """find_mpath_id_by_parent returns device mapper blk for given DM_NAME. |
451 | + """ |
452 | + m_dmmap.return_value = { |
453 | + 'mpatha': '/dev/dm-0', 'mpatha-part1': '/dev/dm-1'} |
454 | + mpath_id = 'mpatha' |
455 | + expected_result = ('mpatha-part1', '/dev/dm-1') |
456 | + self.assertEqual( |
457 | + expected_result, |
458 | + multipath.find_mpath_id_by_parent(mpath_id, partnum=1)) |
459 | + |
460 | + def test_find_mpath_id_by_path(self): |
461 | + """find_mpath_id_by_path returns the mp_id if specified device is |
462 | + member. |
463 | + """ |
464 | + mp_id = 'mpatha' |
465 | + paths = ['device=bar multipath=mpatha', |
466 | + 'device=wark multipath=mpathb'] |
467 | + self.m_subp.return_value = ("\n".join(paths), "") |
468 | + self.assertEqual(mp_id, multipath.find_mpath_id_by_path('/dev/bar')) |
469 | + |
470 | + def test_find_mpath_id_by_path_returns_none_not_found(self): |
471 | + """find_mpath_id_by_path returns None if specified device is not a |
472 | + member. |
473 | + """ |
474 | + mp_id = 'mpatha' |
475 | + paths = ['device=bar multipath=%s' % mp_id, |
476 | + 'device=wark multipath=%s' % mp_id] |
477 | + self.m_subp.return_value = ("\n".join(paths), "") |
478 | + self.assertIsNone(multipath.find_mpath_id_by_path('/dev/xxx')) |
479 | + |
480 | |
481 | # vi: ts=4 expandtab syntax=python |
482 | diff --git a/tests/unittests/test_clear_holders.py b/tests/unittests/test_clear_holders.py |
483 | index b454ae4..0a8f2da 100644 |
484 | --- a/tests/unittests/test_clear_holders.py |
485 | +++ b/tests/unittests/test_clear_holders.py |
486 | @@ -305,17 +305,22 @@ class TestClearHolders(CiTestCase): |
487 | self.assertTrue(mock_log.debug.called) |
488 | self.assertTrue(mock_log.critical.called) |
489 | |
490 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
491 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
492 | @mock.patch('curtin.block.clear_holders.os.path.exists') |
493 | @mock.patch('curtin.block.clear_holders.LOG') |
494 | @mock.patch('curtin.block.clear_holders.block') |
495 | def test_clear_holders_wipe_superblock(self, mock_block, mock_log, |
496 | - mock_os_path, mock_swap): |
497 | + mock_os_path, mock_swap, |
498 | + mock_mp_bp): |
499 | """test clear_holders.wipe_superblock handles errors right""" |
500 | mock_swap.return_value = False |
501 | mock_os_path.return_value = False |
502 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
503 | mock_block.is_extended_partition.return_value = True |
504 | + mock_block.get_blockdev_for_partition.return_value = ( |
505 | + self.test_blockdev, None) |
506 | + mock_mp_bp.return_value = None |
507 | clear_holders.wipe_superblock(self.test_syspath) |
508 | self.assertFalse(mock_block.wipe_volume.called) |
509 | mock_block.is_extended_partition.return_value = False |
510 | @@ -325,16 +330,21 @@ class TestClearHolders(CiTestCase): |
511 | mock_block.wipe_volume.assert_called_with( |
512 | self.test_blockdev, exclusive=True, mode='superblock', strict=True) |
513 | |
514 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
515 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
516 | @mock.patch('curtin.block.clear_holders.zfs') |
517 | @mock.patch('curtin.block.clear_holders.LOG') |
518 | @mock.patch('curtin.block.clear_holders.block') |
519 | def test_clear_holders_wipe_superblock_zfs(self, mock_block, mock_log, |
520 | - mock_zfs, mock_swap): |
521 | + mock_zfs, mock_swap, |
522 | + mock_mp_bp): |
523 | """test clear_holders.wipe_superblock handles zfs member""" |
524 | mock_swap.return_value = False |
525 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
526 | mock_block.is_extended_partition.return_value = True |
527 | + mock_block.get_blockdev_for_partition.return_value = ( |
528 | + self.test_blockdev, None) |
529 | + mock_mp_bp.return_value = None |
530 | clear_holders.wipe_superblock(self.test_syspath) |
531 | self.assertFalse(mock_block.wipe_volume.called) |
532 | mock_block.is_extended_partition.return_value = False |
533 | @@ -348,16 +358,21 @@ class TestClearHolders(CiTestCase): |
534 | mock_block.wipe_volume.assert_called_with( |
535 | self.test_blockdev, exclusive=True, mode='superblock', strict=True) |
536 | |
537 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
538 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
539 | @mock.patch('curtin.block.clear_holders.zfs') |
540 | @mock.patch('curtin.block.clear_holders.LOG') |
541 | @mock.patch('curtin.block.clear_holders.block') |
542 | def test_clear_holders_wipe_superblock_no_zfs(self, mock_block, mock_log, |
543 | - mock_zfs, mock_swap): |
544 | + mock_zfs, mock_swap, |
545 | + mock_mp_bp): |
546 | """test clear_holders.wipe_superblock checks zfs supported""" |
547 | mock_swap.return_value = False |
548 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
549 | mock_block.is_extended_partition.return_value = True |
550 | + mock_block.get_blockdev_for_partition.return_value = ( |
551 | + self.test_blockdev, None) |
552 | + mock_mp_bp.return_value = None |
553 | clear_holders.wipe_superblock(self.test_syspath) |
554 | self.assertFalse(mock_block.wipe_volume.called) |
555 | mock_block.is_extended_partition.return_value = False |
556 | @@ -372,13 +387,14 @@ class TestClearHolders(CiTestCase): |
557 | mock_block.wipe_volume.assert_called_with( |
558 | self.test_blockdev, exclusive=True, mode='superblock', strict=True) |
559 | |
560 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
561 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
562 | @mock.patch('curtin.block.clear_holders.zfs') |
563 | @mock.patch('curtin.block.clear_holders.LOG') |
564 | @mock.patch('curtin.block.clear_holders.block') |
565 | def test_clear_holders_wipe_superblock_zfs_no_utils(self, mock_block, |
566 | mock_log, mock_zfs, |
567 | - mock_swap): |
568 | + mock_swap, mock_mp_bp): |
569 | """test clear_holders.wipe_superblock handles missing zpool cmd""" |
570 | mock_swap.return_value = False |
571 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
572 | @@ -386,6 +402,9 @@ class TestClearHolders(CiTestCase): |
573 | clear_holders.wipe_superblock(self.test_syspath) |
574 | self.assertFalse(mock_block.wipe_volume.called) |
575 | mock_block.is_extended_partition.return_value = False |
576 | + mock_block.get_blockdev_for_partition.return_value = ( |
577 | + self.test_blockdev, None) |
578 | + mock_mp_bp.return_value = None |
579 | mock_block.is_zfs_member.return_value = True |
580 | mock_zfs.zfs_supported.return_value = True |
581 | mock_zfs.device_to_poolname.return_value = 'fake_pool' |
582 | @@ -400,17 +419,21 @@ class TestClearHolders(CiTestCase): |
583 | mock_block.wipe_volume.assert_called_with( |
584 | self.test_blockdev, exclusive=True, mode='superblock', strict=True) |
585 | |
586 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
587 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
588 | @mock.patch('curtin.block.clear_holders.time') |
589 | @mock.patch('curtin.block.clear_holders.LOG') |
590 | @mock.patch('curtin.block.clear_holders.block') |
591 | def test_clear_holders_wipe_superblock_rereads_pt(self, mock_block, |
592 | mock_log, m_time, |
593 | - mock_swap): |
594 | + mock_swap, mock_mp_bp): |
595 | """test clear_holders.wipe_superblock re-reads partition table""" |
596 | mock_swap.return_value = False |
597 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
598 | mock_block.is_extended_partition.return_value = False |
599 | + mock_block.get_blockdev_for_partition.return_value = ( |
600 | + self.test_blockdev, None) |
601 | + mock_mp_bp.return_value = None |
602 | mock_block.is_zfs_member.return_value = False |
603 | mock_block.get_sysfs_partitions.side_effect = iter([ |
604 | ['p1', 'p2'], # has partitions before wipe |
605 | @@ -426,17 +449,22 @@ class TestClearHolders(CiTestCase): |
606 | mock_block.rescan_block_devices.assert_has_calls( |
607 | [mock.call(devices=[self.test_blockdev])] * 2) |
608 | |
609 | + @mock.patch('curtin.block.multipath.find_mpath_id_by_path') |
610 | @mock.patch('curtin.block.clear_holders.is_swap_device') |
611 | @mock.patch('curtin.block.clear_holders.time') |
612 | @mock.patch('curtin.block.clear_holders.LOG') |
613 | @mock.patch('curtin.block.clear_holders.block') |
614 | def test_clear_holders_wipe_superblock_rereads_pt_oserr(self, mock_block, |
615 | mock_log, m_time, |
616 | - mock_swap): |
617 | + mock_swap, |
618 | + mock_mp_bp): |
619 | """test clear_holders.wipe_superblock re-reads ptable handles oserr""" |
620 | mock_swap.return_value = False |
621 | mock_block.sysfs_to_devpath.return_value = self.test_blockdev |
622 | mock_block.is_extended_partition.return_value = False |
623 | + mock_block.get_blockdev_for_partition.return_value = ( |
624 | + self.test_blockdev, None) |
625 | + mock_mp_bp.return_value = None |
626 | mock_block.is_zfs_member.return_value = False |
627 | mock_block.get_sysfs_partitions.side_effect = iter([ |
628 | ['p1', 'p2'], # has partitions before wipe |
629 | diff --git a/tests/vmtests/test_multipath.py b/tests/vmtests/test_multipath.py |
630 | index 339441e..c807a9b 100644 |
631 | --- a/tests/vmtests/test_multipath.py |
632 | +++ b/tests/vmtests/test_multipath.py |
633 | @@ -11,6 +11,7 @@ import textwrap |
634 | |
635 | class TestMultipathBasicAbs(VMBaseClass): |
636 | conf_file = "examples/tests/multipath.yaml" |
637 | + dirty_disks = True |
638 | test_type = 'storage' |
639 | multipath = True |
640 | disk_driver = 'scsi-hd' |
FAILED: Continuous integration, rev:0bf2887ad59 57495c63b08c4e8 a4001003e82702 /jenkins. ubuntu. com/server/ job/curtin- ci/3812/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-arm64/ 3812/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-ppc64el/ 3812/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-s390x/ 3812/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= torkoal/ 3812/
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
Click here to trigger a rebuild: /jenkins. ubuntu. com/server/ job/curtin- ci/3812/ /rebuild
https:/