Merge lp:~raharper/curtin/fix-lp1531520 into lp:~curtin-dev/curtin/trunk
- fix-lp1531520
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 336 |
Proposed branch: | lp:~raharper/curtin/fix-lp1531520 |
Merge into: | lp:~curtin-dev/curtin/trunk |
Diff against target: |
340 lines (+84/-53) 3 files modified
curtin/block/mdadm.py (+38/-22) curtin/commands/block_meta.py (+7/-7) tests/unittests/test_block_mdadm.py (+39/-24) |
To merge this branch: | bzr merge lp:~raharper/curtin/fix-lp1531520 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve | |
curtin developers | Pending | ||
Review via email: mp+281805@code.launchpad.net |
Commit message
Description of the change
mdadm: fix issues exposed by use via block_meta
Using mdadm via block_meta exposed a few issues.
- block_meta calls block.md_check, this should be mdadm.md_check
- mdadm.sysfs_
an attribute and avoiding reading files
- mdadm.sysfs_
- ensure all entrypoints to mdadm code check that device names are device paths.
Server Team CI bot (server-team-bot) wrote : | # |
Scott Moser (smoser) wrote : | # |
I'm surprised / disapppointed that pyflakes didn't catch the curtin.block / curtin.mdadm issue.
pylint does:
% pylint curtin/
No config file found, using default configuration
E:919,15: Module 'curtin.block' has no 'md_check' member (no-member)
Ryan Harper (raharper) wrote : | # |
On Thu, Jan 7, 2016 at 8:59 AM, Scott Moser <email address hidden> wrote:
> I'm surprised / disapppointed that pyflakes didn't catch the curtin.block
> / curtin.mdadm issue.
> pylint does:
> % pylint curtin/
> No config file found, using default configuration
> E:919,15: Module 'curtin.block' has no 'md_check' member (no-member)
>
Same here.
>
>
> Diff comments:
>
> > === modified file 'curtin/
> > --- curtin/
> > +++ curtin/
> > @@ -201,16 +208,16 @@
> >
> >
> > def mdadm_stop(
> > - if not devpath:
> > - raise ValueError(
> > + if not valid_devpath(
> > + raise ValueError('Invalid devpath', devpath)
>
> Is there a reason:
> raise ValueError('Invalid devpath', devpath)
> rather than:
> raise ValueError('Invalid devpath: %s' % devpath)
>
Yeah, I suppose we should have a common style, but the parameter produces
better output because
we're not string formatting.
>>> raise ValueError('My Error %s' % '')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: My Error
Empty variable output is hidden, but if we pass it as parameter not only
are we typing less but it's quoted properly
>>> raise ValueError('My Error', '')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: ('My Error', '')
> The difference can be seen here
> >>> v1 = ValueError('Invalid devpath', devpath)
> >>> v1.message
>
>
In python3, my ValueError doesn't have a .message
but it does have args
>>> v1.args
('Invalid devpath', 'foo')
> >>> str(v1)
> "('Invalid devpath', 'foo')"
>
> Versus:
> >>> v2 = ValueError('Invalid devpath: %s' % devpath)
> >>> v2.message
> 'Invalid devpath: foo'
> >>> str(v2)
> 'Invalid devpath: foo'
>
str(v1)
"('Invalid devpath', 'foo')"
>
> >
> > LOG.info("mdadm stopping: %s" % devpath)
> > util.subp(["mdadm", "--stop", devpath], rcs=[0, 1], capture=True)
> >
> >
> > def mdadm_remove(
> > - if not devpath:
> > - raise ValueError(
> > + if not valid_devpath(
> > + raise ValueError('Invalid devpath', devpath)
> >
> > LOG.info("mdadm removing: %s" % devpath)
> > util.subp(["mdadm", "--remove", devpath], rcs=[0, 1], capture=True)
> > @@ -469,6 +483,9 @@
> >
> >
> > def md_get_
> > + if not valid_devpath(
> > + raise ValueError('Invalid devpath: [{}]'.format(
>
> then you changed to another message here...
>
I missed the conversion. Thanks,
>
> maybe you could just:
> def assert_
> if not valid_devpath(
> raise ValueError('Invalid devpath: %s' % devpath)
>
> then just use the one line:
> assert_
>
> > +
> > sysfs_md = sys_block_
> > if not os.path.
> > raise ValueError('Cannot find md sysfs directory: ' +
>
>
> --
> https:/
- 338. By Ryan Harper
-
Use common message formatting when raising ValueError exceptions
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:338
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 339. By Ryan Harper
-
Introduce assert_
valid_devpath Reduce duplicate code of raising ValueError when provided an invalid devpath.
- 340. By Ryan Harper
-
from trunk
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:340
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'curtin/block/mdadm.py' | |||
2 | --- curtin/block/mdadm.py 2016-01-04 16:24:45 +0000 | |||
3 | +++ curtin/block/mdadm.py 2016-01-07 17:16:36 +0000 | |||
4 | @@ -126,7 +126,7 @@ | |||
5 | 126 | cmd += ['--scan'] | 126 | cmd += ['--scan'] |
6 | 127 | else: | 127 | else: |
7 | 128 | valid_mdname(md_devname) | 128 | valid_mdname(md_devname) |
9 | 129 | cmd += [dev_path(md_devname), "--run"] + devices | 129 | cmd += [md_devname, "--run"] + devices |
10 | 130 | if spares: | 130 | if spares: |
11 | 131 | cmd += spares | 131 | cmd += spares |
12 | 132 | 132 | ||
13 | @@ -139,8 +139,10 @@ | |||
14 | 139 | 'md_name=%s raidlevel=%s ' % (md_devname, raidlevel) + | 139 | 'md_name=%s raidlevel=%s ' % (md_devname, raidlevel) + |
15 | 140 | ' devices=%s spares=%s name=%s' % (devices, spares, md_name)) | 140 | ' devices=%s spares=%s name=%s' % (devices, spares, md_name)) |
16 | 141 | 141 | ||
17 | 142 | assert_valid_devpath(md_devname) | ||
18 | 143 | |||
19 | 142 | if raidlevel not in VALID_RAID_LEVELS: | 144 | if raidlevel not in VALID_RAID_LEVELS: |
21 | 143 | raise ValueError('Invalid raidlevel: ' + str(raidlevel)) | 145 | raise ValueError('Invalid raidlevel: [{}]'.format(raidlevel)) |
22 | 144 | 146 | ||
23 | 145 | min_devices = md_minimum_devices(raidlevel) | 147 | min_devices = md_minimum_devices(raidlevel) |
24 | 146 | if len(devices) < min_devices: | 148 | if len(devices) < min_devices: |
25 | @@ -152,7 +154,7 @@ | |||
26 | 152 | err = ('Raidlevel does not support spare devices: ' + str(raidlevel)) | 154 | err = ('Raidlevel does not support spare devices: ' + str(raidlevel)) |
27 | 153 | raise ValueError(err) | 155 | raise ValueError(err) |
28 | 154 | 156 | ||
30 | 155 | cmd = ["mdadm", "--create", dev_path(md_devname), "--run", | 157 | cmd = ["mdadm", "--create", md_devname, "--run", |
31 | 156 | "--level=%s" % raidlevel, "--raid-devices=%s" % len(devices)] | 158 | "--level=%s" % raidlevel, "--raid-devices=%s" % len(devices)] |
32 | 157 | if md_name: | 159 | if md_name: |
33 | 158 | cmd.append("--name=%s" % md_name) | 160 | cmd.append("--name=%s" % md_name) |
34 | @@ -174,13 +176,16 @@ | |||
35 | 174 | util.subp(["udevadm", "control", "--stop-exec-queue"]) | 176 | util.subp(["udevadm", "control", "--stop-exec-queue"]) |
36 | 175 | util.subp(cmd, capture=True) | 177 | util.subp(cmd, capture=True) |
37 | 176 | util.subp(["udevadm", "control", "--start-exec-queue"]) | 178 | util.subp(["udevadm", "control", "--start-exec-queue"]) |
39 | 177 | util.subp(["udevadm", "settle", "--exit-if-exists=%s" % md_devname]) | 179 | util.subp(["udevadm", "settle", |
40 | 180 | "--exit-if-exists=%s" % md_devname]) | ||
41 | 178 | 181 | ||
42 | 179 | 182 | ||
43 | 180 | def mdadm_examine(devpath, export=MDADM_USE_EXPORT): | 183 | def mdadm_examine(devpath, export=MDADM_USE_EXPORT): |
44 | 181 | ''' exectute mdadm --examine, and optionally | 184 | ''' exectute mdadm --examine, and optionally |
45 | 182 | append --export. | 185 | append --export. |
46 | 183 | Parse and return dict of key=val from output''' | 186 | Parse and return dict of key=val from output''' |
47 | 187 | assert_valid_devpath(devpath) | ||
48 | 188 | |||
49 | 184 | cmd = ["mdadm", "--examine"] | 189 | cmd = ["mdadm", "--examine"] |
50 | 185 | if export: | 190 | if export: |
51 | 186 | cmd.extend(["--export"]) | 191 | cmd.extend(["--export"]) |
52 | @@ -201,16 +206,14 @@ | |||
53 | 201 | 206 | ||
54 | 202 | 207 | ||
55 | 203 | def mdadm_stop(devpath): | 208 | def mdadm_stop(devpath): |
58 | 204 | if not devpath: | 209 | assert_valid_devpath(devpath) |
57 | 205 | raise ValueError('mdadm_stop: missing parameter devpath') | ||
59 | 206 | 210 | ||
60 | 207 | LOG.info("mdadm stopping: %s" % devpath) | 211 | LOG.info("mdadm stopping: %s" % devpath) |
61 | 208 | util.subp(["mdadm", "--stop", devpath], rcs=[0, 1], capture=True) | 212 | util.subp(["mdadm", "--stop", devpath], rcs=[0, 1], capture=True) |
62 | 209 | 213 | ||
63 | 210 | 214 | ||
64 | 211 | def mdadm_remove(devpath): | 215 | def mdadm_remove(devpath): |
67 | 212 | if not devpath: | 216 | assert_valid_devpath(devpath) |
66 | 213 | raise ValueError('mdadm_remove: missing parameter devpath') | ||
68 | 214 | 217 | ||
69 | 215 | LOG.info("mdadm removing: %s" % devpath) | 218 | LOG.info("mdadm removing: %s" % devpath) |
70 | 216 | util.subp(["mdadm", "--remove", devpath], rcs=[0, 1], capture=True) | 219 | util.subp(["mdadm", "--remove", devpath], rcs=[0, 1], capture=True) |
71 | @@ -241,21 +244,29 @@ | |||
72 | 241 | 244 | ||
73 | 242 | # ------------------------------ # | 245 | # ------------------------------ # |
74 | 243 | def valid_mdname(md_devname): | 246 | def valid_mdname(md_devname): |
78 | 244 | if md_devname is None: | 247 | assert_valid_devpath(md_devname) |
76 | 245 | raise ValueError('Parameter: md_devname is None') | ||
77 | 246 | return False | ||
79 | 247 | 248 | ||
83 | 248 | if not is_valid_device(dev_path(md_devname)): | 249 | if not is_valid_device(md_devname): |
84 | 249 | raise ValueError('Specified md device does not exist: ' + | 250 | raise ValueError('Specified md device does not exist: ' + md_devname) |
82 | 250 | dev_path(md_devname)) | ||
85 | 251 | return False | 251 | return False |
86 | 252 | 252 | ||
87 | 253 | return True | 253 | return True |
88 | 254 | 254 | ||
89 | 255 | 255 | ||
90 | 256 | def valid_devpath(devpath): | ||
91 | 257 | if devpath: | ||
92 | 258 | return devpath.startswith('/dev') | ||
93 | 259 | return False | ||
94 | 260 | |||
95 | 261 | |||
96 | 262 | def assert_valid_devpath(devpath): | ||
97 | 263 | if not valid_devpath(devpath): | ||
98 | 264 | raise ValueError('Invalid devpath: %s' % devpath) | ||
99 | 265 | |||
100 | 266 | |||
101 | 256 | def md_sysfs_attr(md_devname, attrname): | 267 | def md_sysfs_attr(md_devname, attrname): |
102 | 257 | if not valid_mdname(md_devname): | 268 | if not valid_mdname(md_devname): |
104 | 258 | raise ValueError('Invalid md devicename') | 269 | raise ValueError('Invalid md devicename: [{}]'.format(md_devname)) |
105 | 259 | 270 | ||
106 | 260 | attrdata = '' | 271 | attrdata = '' |
107 | 261 | # /sys/class/block/<md_short>/md | 272 | # /sys/class/block/<md_short>/md |
108 | @@ -263,9 +274,8 @@ | |||
109 | 263 | 274 | ||
110 | 264 | # /sys/class/block/<md_short>/md/attrname | 275 | # /sys/class/block/<md_short>/md/attrname |
111 | 265 | sysfs_attr_path = os.path.join(sysmd, attrname) | 276 | sysfs_attr_path = os.path.join(sysmd, attrname) |
115 | 266 | if not os.path.isfile(sysfs_attr_path): | 277 | if os.path.isfile(sysfs_attr_path): |
116 | 267 | with open(sysfs_attr_path) as fp: | 278 | attrdata = util.load_file(sysfs_attr_path).strip() |
114 | 268 | attrdata = fp.read().strip() | ||
117 | 269 | 279 | ||
118 | 270 | return attrdata | 280 | return attrdata |
119 | 271 | 281 | ||
120 | @@ -452,6 +462,8 @@ | |||
121 | 452 | 462 | ||
122 | 453 | 463 | ||
123 | 454 | def md_get_spares_list(devpath): | 464 | def md_get_spares_list(devpath): |
124 | 465 | assert_valid_devpath(devpath) | ||
125 | 466 | |||
126 | 455 | sysfs_md = sys_block_path(devpath) + '/md' | 467 | sysfs_md = sys_block_path(devpath) + '/md' |
127 | 456 | 468 | ||
128 | 457 | if not os.path.exists(sysfs_md): | 469 | if not os.path.exists(sysfs_md): |
129 | @@ -469,6 +481,8 @@ | |||
130 | 469 | 481 | ||
131 | 470 | 482 | ||
132 | 471 | def md_get_devices_list(devpath): | 483 | def md_get_devices_list(devpath): |
133 | 484 | assert_valid_devpath(devpath) | ||
134 | 485 | |||
135 | 472 | sysfs_md = sys_block_path(devpath) + '/md' | 486 | sysfs_md = sys_block_path(devpath) + '/md' |
136 | 473 | if not os.path.exists(sysfs_md): | 487 | if not os.path.exists(sysfs_md): |
137 | 474 | raise ValueError('Cannot find md sysfs directory: ' + | 488 | raise ValueError('Cannot find md sysfs directory: ' + |
138 | @@ -483,6 +497,8 @@ | |||
139 | 483 | 497 | ||
140 | 484 | 498 | ||
141 | 485 | def md_check_array_uuid(md_devname, md_uuid): | 499 | def md_check_array_uuid(md_devname, md_uuid): |
142 | 500 | valid_mdname(md_devname) | ||
143 | 501 | |||
144 | 486 | # confirm we have /dev/{mdname} by following the udev symlink | 502 | # confirm we have /dev/{mdname} by following the udev symlink |
145 | 487 | mduuid_path = ('/dev/disk/by-id/md-uuid-' + md_uuid) | 503 | mduuid_path = ('/dev/disk/by-id/md-uuid-' + md_uuid) |
146 | 488 | mdlink_devname = dev_path(os.path.realpath(mduuid_path)) | 504 | mdlink_devname = dev_path(os.path.realpath(mduuid_path)) |
147 | @@ -495,8 +511,7 @@ | |||
148 | 495 | 511 | ||
149 | 496 | 512 | ||
150 | 497 | def md_get_uuid(md_devname): | 513 | def md_get_uuid(md_devname): |
153 | 498 | if not valid_mdname(md_devname): | 514 | valid_mdname(md_devname) |
152 | 499 | raise ValueError('Invalid md devicename') | ||
154 | 500 | 515 | ||
155 | 501 | md_query = mdadm_query_detail(md_devname) | 516 | md_query = mdadm_query_detail(md_devname) |
156 | 502 | return md_query.get('MD_UUID', None) | 517 | return md_query.get('MD_UUID', None) |
157 | @@ -543,12 +558,12 @@ | |||
158 | 543 | # check array state | 558 | # check array state |
159 | 544 | 559 | ||
160 | 545 | writable = md_check_array_state_rw(md_devname) | 560 | writable = md_check_array_state_rw(md_devname) |
162 | 546 | degraded = int(md_sysfs_attr(md_devname, 'degraded')) | 561 | degraded = md_sysfs_attr(md_devname, 'degraded') |
163 | 547 | sync_action = md_sysfs_attr(md_devname, 'sync_action') | 562 | sync_action = md_sysfs_attr(md_devname, 'sync_action') |
164 | 548 | 563 | ||
165 | 549 | if not writable: | 564 | if not writable: |
166 | 550 | raise ValueError('Array not in writable state: ' + md_devname) | 565 | raise ValueError('Array not in writable state: ' + md_devname) |
168 | 551 | if degraded > 0: | 566 | if degraded != "0": |
169 | 552 | raise ValueError('Array in degraded state: ' + md_devname) | 567 | raise ValueError('Array in degraded state: ' + md_devname) |
170 | 553 | if sync_action != "idle": | 568 | if sync_action != "idle": |
171 | 554 | raise ValueError('Array syncing, not idle state: ' + md_devname) | 569 | raise ValueError('Array syncing, not idle state: ' + md_devname) |
172 | @@ -615,6 +630,7 @@ | |||
173 | 615 | raidlevel, | 630 | raidlevel, |
174 | 616 | devices, | 631 | devices, |
175 | 617 | spares)) | 632 | spares)) |
176 | 633 | assert_valid_devpath(md_devname) | ||
177 | 618 | 634 | ||
178 | 619 | md_check_array_state(md_devname) | 635 | md_check_array_state(md_devname) |
179 | 620 | md_check_raidlevel(raidlevel) | 636 | md_check_raidlevel(raidlevel) |
180 | 621 | 637 | ||
181 | === modified file 'curtin/commands/block_meta.py' | |||
182 | --- curtin/commands/block_meta.py 2016-01-06 22:18:33 +0000 | |||
183 | +++ curtin/commands/block_meta.py 2016-01-07 17:16:36 +0000 | |||
184 | @@ -896,6 +896,7 @@ | |||
185 | 896 | devices = info.get('devices') | 896 | devices = info.get('devices') |
186 | 897 | raidlevel = info.get('raidlevel') | 897 | raidlevel = info.get('raidlevel') |
187 | 898 | spare_devices = info.get('spare_devices') | 898 | spare_devices = info.get('spare_devices') |
188 | 899 | md_devname = block.dev_path(info.get('name')) | ||
189 | 899 | if not devices: | 900 | if not devices: |
190 | 900 | raise ValueError("devices for raid must be specified") | 901 | raise ValueError("devices for raid must be specified") |
191 | 901 | if raidlevel not in ['linear', 'raid0', 0, 'stripe', 'raid1', 1, 'mirror', | 902 | if raidlevel not in ['linear', 'raid0', 0, 'stripe', 'raid1', 1, 'mirror', |
192 | @@ -916,23 +917,22 @@ | |||
193 | 916 | # Handle preserve flag | 917 | # Handle preserve flag |
194 | 917 | if info.get('preserve'): | 918 | if info.get('preserve'): |
195 | 918 | # check if the array is already up, if not try to assemble | 919 | # check if the array is already up, if not try to assemble |
197 | 919 | if not block.md_check(info.get('name'), raidlevel, | 920 | if not mdadm.md_check(md_devname, raidlevel, |
198 | 920 | device_paths, spare_device_paths): | 921 | device_paths, spare_device_paths): |
199 | 921 | LOG.info("assembling preserved raid for " | 922 | LOG.info("assembling preserved raid for " |
201 | 922 | "{}".format(info.get('name'))) | 923 | "{}".format(md_devname)) |
202 | 923 | 924 | ||
205 | 924 | mdadm.mdadm_assemble(info.get('name'), | 925 | mdadm.mdadm_assemble(md_devname, device_paths, spare_device_paths) |
204 | 925 | device_paths, spare_device_paths) | ||
206 | 926 | 926 | ||
207 | 927 | # try again after attempting to assemble | 927 | # try again after attempting to assemble |
209 | 928 | if not mdadm.md_check(info.get('name'), raidlevel, | 928 | if not mdadm.md_check(md_devname, raidlevel, |
210 | 929 | devices, spare_device_paths): | 929 | devices, spare_device_paths): |
211 | 930 | raise ValueError("Unable to confirm preserved raid array: " | 930 | raise ValueError("Unable to confirm preserved raid array: " |
213 | 931 | " {}".format(info.get('name'))) | 931 | " {}".format(md_devname)) |
214 | 932 | # raid is all OK | 932 | # raid is all OK |
215 | 933 | return | 933 | return |
216 | 934 | 934 | ||
218 | 935 | mdadm.mdadm_create(info.get('name'), raidlevel, | 935 | mdadm.mdadm_create(md_devname, raidlevel, |
219 | 936 | device_paths, spare_device_paths, | 936 | device_paths, spare_device_paths, |
220 | 937 | info.get('mdname', '')) | 937 | info.get('mdname', '')) |
221 | 938 | 938 | ||
222 | 939 | 939 | ||
223 | === modified file 'tests/unittests/test_block_mdadm.py' | |||
224 | --- tests/unittests/test_block_mdadm.py 2015-12-19 19:48:06 +0000 | |||
225 | +++ tests/unittests/test_block_mdadm.py 2016-01-07 17:16:36 +0000 | |||
226 | @@ -5,12 +5,6 @@ | |||
227 | 5 | import os | 5 | import os |
228 | 6 | import subprocess | 6 | import subprocess |
229 | 7 | 7 | ||
230 | 8 | from sys import version_info | ||
231 | 9 | if version_info.major == 2: | ||
232 | 10 | import __builtin__ as builtins | ||
233 | 11 | else: | ||
234 | 12 | import builtins | ||
235 | 13 | |||
236 | 14 | 8 | ||
237 | 15 | class MdadmTestBase(TestCase): | 9 | class MdadmTestBase(TestCase): |
238 | 16 | def setUp(self): | 10 | def setUp(self): |
239 | @@ -60,15 +54,9 @@ | |||
240 | 60 | self.mock_util.subp.assert_has_calls(expected_calls) | 54 | self.mock_util.subp.assert_has_calls(expected_calls) |
241 | 61 | 55 | ||
242 | 62 | def test_mdadm_assemble_md_devname_short(self): | 56 | def test_mdadm_assemble_md_devname_short(self): |
252 | 63 | md_devname = "md0" | 57 | with self.assertRaises(ValueError): |
253 | 64 | mdadm.mdadm_assemble(md_devname=md_devname) | 58 | md_devname = "md0" |
254 | 65 | 59 | mdadm.mdadm_assemble(md_devname=md_devname) | |
246 | 66 | expected_calls = [ | ||
247 | 67 | call(["mdadm", "--assemble", "/dev/md0", "--run"], capture=True, | ||
248 | 68 | rcs=[0, 1, 2]), | ||
249 | 69 | call(["udevadm", "settle"]), | ||
250 | 70 | ] | ||
251 | 71 | self.mock_util.subp.assert_has_calls(expected_calls) | ||
255 | 72 | 60 | ||
256 | 73 | def test_mdadm_assemble_md_devname_none(self): | 61 | def test_mdadm_assemble_md_devname_none(self): |
257 | 74 | with self.assertRaises(ValueError): | 62 | with self.assertRaises(ValueError): |
258 | @@ -149,6 +137,15 @@ | |||
259 | 149 | devices=devices, spares=spares) | 137 | devices=devices, spares=spares) |
260 | 150 | self.mock_util.subp.assert_has_calls(expected_calls) | 138 | self.mock_util.subp.assert_has_calls(expected_calls) |
261 | 151 | 139 | ||
262 | 140 | def test_mdadm_create_raid0_devshort(self): | ||
263 | 141 | md_devname = "md0" | ||
264 | 142 | raidlevel = 0 | ||
265 | 143 | devices = ["/dev/vdc1", "/dev/vdd1"] | ||
266 | 144 | spares = [] | ||
267 | 145 | with self.assertRaises(ValueError): | ||
268 | 146 | mdadm.mdadm_create(md_devname=md_devname, raidlevel=raidlevel, | ||
269 | 147 | devices=devices, spares=spares) | ||
270 | 148 | |||
271 | 152 | def test_mdadm_create_raid0_with_spares(self): | 149 | def test_mdadm_create_raid0_with_spares(self): |
272 | 153 | md_devname = "/dev/md0" | 150 | md_devname = "/dev/md0" |
273 | 154 | raidlevel = 0 | 151 | raidlevel = 0 |
274 | @@ -488,12 +485,8 @@ | |||
275 | 488 | 485 | ||
276 | 489 | def test_valid_mdname_short(self): | 486 | def test_valid_mdname_short(self): |
277 | 490 | mdname = "md0" | 487 | mdname = "md0" |
284 | 491 | result = mdadm.valid_mdname(mdname) | 488 | with self.assertRaises(ValueError): |
285 | 492 | expected_calls = [ | 489 | mdadm.valid_mdname(mdname) |
280 | 493 | call("/dev/md0") | ||
281 | 494 | ] | ||
282 | 495 | self.mock_valid.assert_has_calls(expected_calls) | ||
283 | 496 | self.assertTrue(result) | ||
286 | 497 | 490 | ||
287 | 498 | def test_valid_mdname_none(self): | 491 | def test_valid_mdname_none(self): |
288 | 499 | mdname = None | 492 | mdname = None |
289 | @@ -506,14 +499,15 @@ | |||
290 | 506 | with self.assertRaises(ValueError): | 499 | with self.assertRaises(ValueError): |
291 | 507 | mdadm.valid_mdname(mdname) | 500 | mdadm.valid_mdname(mdname) |
292 | 508 | 501 | ||
295 | 509 | @patch.object(builtins, "open") | 502 | @patch('curtin.block.mdadm.os.path.isfile') |
296 | 510 | def test_md_sysfs_attr(self, mock_open): | 503 | def test_md_sysfs_attr(self, mock_isfile): |
297 | 511 | mdname = "/dev/md0" | 504 | mdname = "/dev/md0" |
298 | 512 | attr_name = 'array_state' | 505 | attr_name = 'array_state' |
299 | 513 | sysfs_path = '/sys/class/block/{}/md/{}'.format(dev_short(mdname), | 506 | sysfs_path = '/sys/class/block/{}/md/{}'.format(dev_short(mdname), |
300 | 514 | attr_name) | 507 | attr_name) |
301 | 508 | mock_isfile.return_value = True | ||
302 | 515 | mdadm.md_sysfs_attr(mdname, attr_name) | 509 | mdadm.md_sysfs_attr(mdname, attr_name) |
304 | 516 | mock_open.assert_called_with(sysfs_path) | 510 | self.mock_util.load_file.assert_called_with(sysfs_path) |
305 | 517 | 511 | ||
306 | 518 | def test_md_sysfs_attr_devname_none(self): | 512 | def test_md_sysfs_attr_devname_none(self): |
307 | 519 | mdname = None | 513 | mdname = None |
308 | @@ -729,6 +723,17 @@ | |||
309 | 729 | mdadm.md_check_array_state(mdname) | 723 | mdadm.md_check_array_state(mdname) |
310 | 730 | 724 | ||
311 | 731 | @patch('curtin.block.mdadm.md_sysfs_attr') | 725 | @patch('curtin.block.mdadm.md_sysfs_attr') |
312 | 726 | def test_md_check_array_state_degraded_empty(self, mock_attr): | ||
313 | 727 | mdname = '/dev/md0' | ||
314 | 728 | mock_attr.side_effect = [ | ||
315 | 729 | 'clean', # array_state | ||
316 | 730 | '', # unknown | ||
317 | 731 | 'idle', # sync_action | ||
318 | 732 | ] | ||
319 | 733 | with self.assertRaises(ValueError): | ||
320 | 734 | mdadm.md_check_array_state(mdname) | ||
321 | 735 | |||
322 | 736 | @patch('curtin.block.mdadm.md_sysfs_attr') | ||
323 | 732 | def test_md_check_array_state_sync(self, mock_attr): | 737 | def test_md_check_array_state_sync(self, mock_attr): |
324 | 733 | mdname = '/dev/md0' | 738 | mdname = '/dev/md0' |
325 | 734 | mock_attr.side_effect = [ | 739 | mock_attr.side_effect = [ |
326 | @@ -888,4 +893,14 @@ | |||
327 | 888 | mock_spare.assert_has_calls([call(md_devname, spares)]) | 893 | mock_spare.assert_has_calls([call(md_devname, spares)]) |
328 | 889 | mock_member.assert_has_calls([call(md_devname, devices + spares)]) | 894 | mock_member.assert_has_calls([call(md_devname, devices + spares)]) |
329 | 890 | 895 | ||
330 | 896 | def test_md_check_all_good_devshort(self): | ||
331 | 897 | md_devname = 'md0' | ||
332 | 898 | raidlevel = 1 | ||
333 | 899 | devices = ['/dev/vda', '/dev/vdb'] | ||
334 | 900 | spares = ['/dev/vdc'] | ||
335 | 901 | |||
336 | 902 | with self.assertRaises(ValueError): | ||
337 | 903 | mdadm.md_check(md_devname, raidlevel, devices=devices, | ||
338 | 904 | spares=spares) | ||
339 | 905 | |||
340 | 891 | # vi: ts=4 expandtab syntax=python | 906 | # vi: ts=4 expandtab syntax=python |
PASSED: Continuous integration, rev:337 /server- team-jenkins. canonical. com/job/ curtin- ci/95/ /server- team-jenkins. canonical. com/job/ generic- update- mp/92/console
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild: /server- team-jenkins. canonical. com/job/ curtin- ci/95/rebuild
https:/