MAAS

Merge lp:~trapnine/maas/fix-1526542-1.10 into lp:maas/1.10

  1. fix-1526542-1.10
  2. Merge into 1.10
Proposed by Jeffrey C Jones
Status: Merged
Approved by: Andres Rodriguez
Approved revision: no longer in the source branch.
Merged at revision: 4576
Proposed branch: lp:~trapnine/maas/fix-1526542-1.10
Merge into: lp:maas/1.10
Diff against target: 282 lines (+252/-2)
2 files modified
src/metadataserver/models/commissioningscript.py (+16/-2)
src/metadataserver/models/tests/test_commissioningscript.py (+236/-0)
To merge this branch: bzr merge lp:~trapnine/maas/fix-1526542-1.10
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve
Review via email: mp+285062@code.launchpad.net

Commit message

Skip block devices with duplicate serial numbers to fix multipath issue.

Description of the change

Skip block devices with duplicate serial numbers to fix multipath issue.

Backported from trunk.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Pending.

Revision history for this message
Andres Rodriguez (andreserl) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/metadataserver/models/commissioningscript.py'
2--- src/metadataserver/models/commissioningscript.py 2015-12-17 19:55:54 +0000
3+++ src/metadataserver/models/commissioningscript.py 2016-02-04 14:08:57 +0000
4@@ -468,7 +468,8 @@
5 "ID_ATA_SATA": "SATA",
6 "ID_ATA_ROTATION_RATE_RPM": "RPM"
7 }
8- del_blocks = []
9+ del_blocks = set()
10+ seen_devices = set()
11 for block_info in blockdevs:
12 # Some RAID devices return the name of the device seperated with "!",
13 # but udevadm expects it to be a "/".
14@@ -489,7 +490,20 @@
15 if k == "ID_CDROM" and v == "1":
16 # Remove any type of CDROM from the blockdevs, as we
17 # cannot use this device for installation.
18- del_blocks.append(block_name)
19+ del_blocks.add(block_name)
20+ break
21+
22+ if block_name in del_blocks:
23+ continue
24+
25+ # Skip duplicate (serial, model) for multipath.
26+ serial = block_info.get("SERIAL")
27+ if serial:
28+ model = block_info.get("MODEL", "").strip()
29+ if (serial, model) in seen_devices:
30+ del_blocks.add(block_name)
31+ continue
32+ seen_devices.add((serial, model))
33
34 # Remove any devices that need to be removed.
35 blockdevs = [
36
37=== modified file 'src/metadataserver/models/tests/test_commissioningscript.py'
38--- src/metadataserver/models/tests/test_commissioningscript.py 2015-12-16 23:51:21 +0000
39+++ src/metadataserver/models/tests/test_commissioningscript.py 2016-02-04 14:08:57 +0000
40@@ -804,6 +804,242 @@
41 "RPM": "5400",
42 }], self.call_gather_physical_block_devices(byidroot))
43
44+ def test__removes_duplicate_block_device_same_serial_and_model(self):
45+ """Multipath disks get multiple IDs, but same serial/model is same
46+ device and should only be enumerated once."""
47+ name = factory.make_name('name')
48+ model = factory.make_name('model')
49+ serial = factory.make_name('serial')
50+ size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
51+ block_size = random.choice([512, 1024, 4096])
52+ check_output = self.patch(subprocess, "check_output")
53+
54+ name2 = factory.make_name('name')
55+
56+ # Create simulated /dev tree.
57+ devroot = self.make_dir()
58+ os.mkdir(os.path.join(devroot, 'disk'))
59+ byidroot = os.path.join(devroot, 'disk', 'by_id')
60+ os.mkdir(byidroot)
61+
62+ os.mknod(os.path.join(devroot, name))
63+ os.symlink(os.path.join(devroot, name),
64+ os.path.join(byidroot, 'deviceid'))
65+
66+ os.mknod(os.path.join(devroot, name2))
67+ os.symlink(os.path.join(devroot, name2),
68+ os.path.join(byidroot, 'deviceid2'))
69+
70+ check_output.side_effect = [
71+ b"\n".join([
72+ self.make_lsblk_output(name=name, model=model),
73+ self.make_lsblk_output(name=name2, model=model)]),
74+ self.make_udevadm_output(name, serial=serial, dev=devroot),
75+ self.make_udevadm_output(name2, serial=serial, dev=devroot),
76+ b'%d' % size,
77+ b'%d' % block_size,
78+ b'%d' % size,
79+ b'%d' % block_size,
80+ ]
81+
82+ self.assertEqual([{
83+ "NAME": name,
84+ "PATH": os.path.join(devroot, name),
85+ "ID_PATH": os.path.join(byidroot, 'deviceid'),
86+ "RO": "0",
87+ "RM": "0",
88+ "MODEL": model,
89+ "ROTA": "1",
90+ "SATA": "1",
91+ "SERIAL": serial,
92+ "SIZE": "%s" % size,
93+ "BLOCK_SIZE": "%s" % block_size,
94+ "RPM": "5400",
95+ }], self.call_gather_physical_block_devices(byidroot))
96+
97+ def test__removes_duplicate_block_device_same_serial_blank_model(self):
98+ """Multipath disks get multiple IDs, but same serial is same device."""
99+ name = factory.make_name('name')
100+ model = ""
101+ serial = factory.make_name('serial')
102+ size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
103+ block_size = random.choice([512, 1024, 4096])
104+ check_output = self.patch(subprocess, "check_output")
105+
106+ name2 = factory.make_name('name')
107+
108+ # Create simulated /dev tree.
109+ devroot = self.make_dir()
110+ os.mkdir(os.path.join(devroot, 'disk'))
111+ byidroot = os.path.join(devroot, 'disk', 'by_id')
112+ os.mkdir(byidroot)
113+
114+ os.mknod(os.path.join(devroot, name))
115+ os.symlink(os.path.join(devroot, name),
116+ os.path.join(byidroot, 'deviceid'))
117+
118+ os.mknod(os.path.join(devroot, name2))
119+ os.symlink(os.path.join(devroot, name2),
120+ os.path.join(byidroot, 'deviceid2'))
121+
122+ check_output.side_effect = [
123+ b"\n".join([
124+ self.make_lsblk_output(name=name, model=model),
125+ self.make_lsblk_output(name=name2, model=model)]),
126+ self.make_udevadm_output(name, serial=serial, dev=devroot),
127+ self.make_udevadm_output(name2, serial=serial, dev=devroot),
128+ b'%d' % size,
129+ b'%d' % block_size,
130+ b'%d' % size,
131+ b'%d' % block_size,
132+ ]
133+
134+ self.assertEqual([{
135+ "NAME": name,
136+ "PATH": os.path.join(devroot, name),
137+ "ID_PATH": os.path.join(byidroot, 'deviceid'),
138+ "RO": "0",
139+ "RM": "0",
140+ "MODEL": model,
141+ "ROTA": "1",
142+ "SATA": "1",
143+ "SERIAL": serial,
144+ "SIZE": "%s" % size,
145+ "BLOCK_SIZE": "%s" % block_size,
146+ "RPM": "5400",
147+ }], self.call_gather_physical_block_devices(byidroot))
148+
149+ def test__keeps_block_device_same_serial_different_model(self):
150+ """Multipath disks get multiple IDs, but same serial is same device."""
151+ name = factory.make_name('name')
152+ model = factory.make_name('model')
153+ serial = factory.make_name('serial')
154+ size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
155+ block_size = random.choice([512, 1024, 4096])
156+ check_output = self.patch(subprocess, "check_output")
157+
158+ name2 = factory.make_name('name')
159+ model2 = factory.make_name('model')
160+
161+ # Create simulated /dev tree.
162+ devroot = self.make_dir()
163+ os.mkdir(os.path.join(devroot, 'disk'))
164+ byidroot = os.path.join(devroot, 'disk', 'by_id')
165+ os.mkdir(byidroot)
166+
167+ os.mknod(os.path.join(devroot, name))
168+ os.symlink(os.path.join(devroot, name),
169+ os.path.join(byidroot, 'deviceid'))
170+
171+ os.mknod(os.path.join(devroot, name2))
172+ os.symlink(os.path.join(devroot, name2),
173+ os.path.join(byidroot, 'deviceid2'))
174+
175+ check_output.side_effect = [
176+ b"\n".join([
177+ self.make_lsblk_output(name=name, model=model),
178+ self.make_lsblk_output(name=name2, model=model2)]),
179+ self.make_udevadm_output(name, serial=serial, dev=devroot),
180+ self.make_udevadm_output(name2, serial=serial, dev=devroot),
181+ b'%d' % size,
182+ b'%d' % block_size,
183+ b'%d' % size,
184+ b'%d' % block_size,
185+ ]
186+
187+ self.assertEqual([{
188+ "NAME": name,
189+ "PATH": os.path.join(devroot, name),
190+ "ID_PATH": os.path.join(byidroot, 'deviceid'),
191+ "RO": "0",
192+ "RM": "0",
193+ "MODEL": model,
194+ "ROTA": "1",
195+ "SATA": "1",
196+ "SERIAL": serial,
197+ "SIZE": "%s" % size,
198+ "BLOCK_SIZE": "%s" % block_size,
199+ "RPM": "5400",
200+ }, {
201+ "NAME": name2,
202+ "PATH": os.path.join(devroot, name2),
203+ "ID_PATH": os.path.join(byidroot, 'deviceid2'),
204+ "RO": "0",
205+ "RM": "0",
206+ "MODEL": model2,
207+ "ROTA": "1",
208+ "SATA": "1",
209+ "SERIAL": serial,
210+ "SIZE": "%s" % size,
211+ "BLOCK_SIZE": "%s" % block_size,
212+ "RPM": "5400",
213+ }], self.call_gather_physical_block_devices(byidroot))
214+
215+ def test__keeps_block_device_blank_serial_same_model(self):
216+ """Multipath disks get multiple IDs, but same serial is same device."""
217+ name = factory.make_name('name')
218+ model = factory.make_name('model')
219+ serial = ''
220+ size = random.randint(3000 * 1000, 1000 * 1000 * 1000)
221+ block_size = random.choice([512, 1024, 4096])
222+ check_output = self.patch(subprocess, "check_output")
223+
224+ name2 = factory.make_name('name')
225+
226+ # Create simulated /dev tree.
227+ devroot = self.make_dir()
228+ os.mkdir(os.path.join(devroot, 'disk'))
229+ byidroot = os.path.join(devroot, 'disk', 'by_id')
230+ os.mkdir(byidroot)
231+
232+ os.mknod(os.path.join(devroot, name))
233+ os.symlink(os.path.join(devroot, name),
234+ os.path.join(byidroot, 'deviceid'))
235+
236+ os.mknod(os.path.join(devroot, name2))
237+ os.symlink(os.path.join(devroot, name2),
238+ os.path.join(byidroot, 'deviceid2'))
239+
240+ check_output.side_effect = [
241+ b"\n".join([
242+ self.make_lsblk_output(name=name, model=model),
243+ self.make_lsblk_output(name=name2, model=model)]),
244+ self.make_udevadm_output(name, serial=serial, dev=devroot),
245+ self.make_udevadm_output(name2, serial=serial, dev=devroot),
246+ b'%d' % size,
247+ b'%d' % block_size,
248+ b'%d' % size,
249+ b'%d' % block_size,
250+ ]
251+
252+ self.assertEqual([{
253+ "NAME": name,
254+ "PATH": os.path.join(devroot, name),
255+ "ID_PATH": os.path.join(byidroot, 'deviceid'),
256+ "RO": "0",
257+ "RM": "0",
258+ "MODEL": model,
259+ "ROTA": "1",
260+ "SATA": "1",
261+ "SERIAL": serial,
262+ "SIZE": "%s" % size,
263+ "BLOCK_SIZE": "%s" % block_size,
264+ "RPM": "5400",
265+ }, {
266+ "NAME": name2,
267+ "PATH": os.path.join(devroot, name2),
268+ "ID_PATH": os.path.join(byidroot, 'deviceid2'),
269+ "RO": "0",
270+ "RM": "0",
271+ "MODEL": model,
272+ "ROTA": "1",
273+ "SATA": "1",
274+ "SERIAL": serial,
275+ "SIZE": "%s" % size,
276+ "BLOCK_SIZE": "%s" % block_size,
277+ "RPM": "5400",
278+ }], self.call_gather_physical_block_devices(byidroot))
279+
280 def test__returns_block_device_without_id_path(self):
281 """Block devices without by-id links should not have ID_PATH key"""
282 name = factory.make_name('name')

Subscribers

People subscribed via source and target branches