Merge lp:~raharper/curtin/fix-network-package-install into lp:~curtin-dev/curtin/trunk
- fix-network-package-install
- Merge into trunk
Proposed by
Ryan Harper
Status: | Merged |
---|---|
Merged at revision: | 269 |
Proposed branch: | lp:~raharper/curtin/fix-network-package-install |
Merge into: | lp:~curtin-dev/curtin/trunk |
Diff against target: |
476 lines (+339/-23) 6 files modified
curtin/commands/curthooks.py (+32/-17) curtin/net/__init__.py (+15/-2) curtin/net/network_state.py (+28/-0) examples/tests/bonding_network.yaml (+27/-0) tests/unittests/test_net.py (+2/-4) tests/vmtests/test_bonding.py (+235/-0) |
To merge this branch: | bzr merge lp:~raharper/curtin/fix-network-package-install |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Scott Moser (community) | Approve | ||
Review via email:
|
Commit message
Description of the change
- Fixes lp:1491994
- Enforce order on rendering of /etc/network/
- Don't emit hwaddress for anything except bonds or bridges
- Introduce a bonding vmtest
- lint and unit-test fixes.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'curtin/commands/curthooks.py' |
2 | --- curtin/commands/curthooks.py 2015-09-04 17:01:28 +0000 |
3 | +++ curtin/commands/curthooks.py 2015-09-04 19:23:11 +0000 |
4 | @@ -573,24 +573,39 @@ |
5 | update_initramfs(target, all_kernels=True) |
6 | |
7 | |
8 | -def install_missing_storage_packages(cfg, target): |
9 | - # Do nothing if no custom storage. |
10 | - if 'storage' not in cfg: |
11 | - return |
12 | - |
13 | - all_types = set( |
14 | - operation['type'] |
15 | - for operation in cfg['storage']['config'] |
16 | - ) |
17 | +def install_missing_packages(cfg, target): |
18 | + ''' describe which operation types will require specific packages |
19 | + |
20 | + 'custom_config_key': { |
21 | + 'pkg1': ['op_name_1', 'op_name_2', ...] |
22 | + } |
23 | + ''' |
24 | + custom_configs = { |
25 | + 'storage': { |
26 | + 'lvm2': ['lvm_volgroup', 'lvm_partition'], |
27 | + 'mdadm': ['raid'], |
28 | + 'bcache-tools': ['bcache']}, |
29 | + 'network': { |
30 | + 'vlan': ['vlan'], |
31 | + 'ifenslave': ['bond'], |
32 | + 'bridge-utils': ['bridge']}, |
33 | + } |
34 | + |
35 | needed_packages = [] |
36 | installed_packages = get_installed_packages(target) |
37 | - if ('lvm_volgroup' in all_types or 'lvm_partition' in all_types) and \ |
38 | - 'lvm2' not in installed_packages: |
39 | - needed_packages.append('lvm2') |
40 | - if 'raid' in all_types and 'mdadm' not in installed_packages: |
41 | - needed_packages.append('mdadm') |
42 | - if 'bcache' in all_types and 'bcache-tools' not in installed_packages: |
43 | - needed_packages.append('bcache-tools') |
44 | + for cust_cfg, pkg_reqs in custom_configs.items(): |
45 | + if cust_cfg not in cfg: |
46 | + continue |
47 | + |
48 | + all_types = set( |
49 | + operation['type'] |
50 | + for operation in cfg[cust_cfg]['config'] |
51 | + ) |
52 | + for pkg, types in pkg_reqs.items(): |
53 | + if set(types).intersection(all_types) and \ |
54 | + pkg not in installed_packages: |
55 | + needed_packages.append(pkg) |
56 | + |
57 | if needed_packages: |
58 | util.install_packages(needed_packages, target=target) |
59 | |
60 | @@ -630,7 +645,7 @@ |
61 | |
62 | detect_and_handle_multipath(cfg, target) |
63 | |
64 | - install_missing_storage_packages(cfg, target) |
65 | + install_missing_packages(cfg, target) |
66 | |
67 | # If a crypttab file was created by block_meta than it needs to be copied |
68 | # onto the target system, and update_initramfs() needs to be run, so that |
69 | |
70 | === modified file 'curtin/net/__init__.py' |
71 | --- curtin/net/__init__.py 2015-09-02 13:55:39 +0000 |
72 | +++ curtin/net/__init__.py 2015-09-04 19:23:11 +0000 |
73 | @@ -265,7 +265,6 @@ |
74 | 'metric', |
75 | 'gateway', |
76 | 'pointopoint', |
77 | - 'hwaddress', |
78 | 'mtu', |
79 | 'scope', |
80 | ] |
81 | @@ -289,6 +288,9 @@ |
82 | 'index', |
83 | 'subnets', |
84 | ] |
85 | + if iface['type'] not in ['bond', 'bridge']: |
86 | + ignore_map.append('mac_address') |
87 | + |
88 | for key, value in iface.items(): |
89 | if value and key not in ignore_map: |
90 | if type(value) == list: |
91 | @@ -319,7 +321,18 @@ |
92 | |
93 | content = "" |
94 | interfaces = network_state.get('interfaces') |
95 | - for iface in interfaces.values(): |
96 | + ''' Apply a sort order to ensure that we write out |
97 | + the physical interfaces first; this is critical for |
98 | + bonding |
99 | + ''' |
100 | + order = { |
101 | + 'physical': 0, |
102 | + 'bond': 1, |
103 | + 'bridge': 2, |
104 | + 'vlan': 3, |
105 | + } |
106 | + for iface in sorted(interfaces.values(), |
107 | + key=lambda k: (order[k['type']], k['name'])): |
108 | content += "auto {name}\n".format(**iface) |
109 | |
110 | subnets = iface.get('subnets', {}) |
111 | |
112 | === modified file 'curtin/net/network_state.py' |
113 | --- curtin/net/network_state.py 2015-09-02 13:55:39 +0000 |
114 | +++ curtin/net/network_state.py 2015-09-04 19:23:11 +0000 |
115 | @@ -116,6 +116,8 @@ |
116 | |
117 | interfaces = self.network_state.get('interfaces') |
118 | iface = interfaces.get(command['name'], {}) |
119 | + for param, val in command.get('params', {}).items(): |
120 | + iface.update({param: val}) |
121 | iface.update({ |
122 | 'name': command.get('name'), |
123 | 'type': command.get('type'), |
124 | @@ -160,15 +162,20 @@ |
125 | #/etc/network/interfaces |
126 | auto eth0 |
127 | iface eth0 inet manual |
128 | + bond-master bond0 |
129 | + bond-mode 802.3ad |
130 | |
131 | auto eth1 |
132 | iface eth1 inet manual |
133 | + bond-master bond0 |
134 | + bond-mode 802.3ad |
135 | |
136 | auto bond0 |
137 | iface bond0 inet static |
138 | address 192.168.0.10 |
139 | gateway 192.168.0.1 |
140 | netmask 255.255.255.0 |
141 | + bond-slaves none |
142 | bond-mode 802.3ad |
143 | bond-miimon 100 |
144 | bond-downdelay 200 |
145 | @@ -190,6 +197,7 @@ |
146 | iface = interfaces.get(command.get('name'), {}) |
147 | for param, val in command.get('params').items(): |
148 | iface.update({param: val}) |
149 | + iface.update({'bond-slaves': 'none'}) |
150 | self.network_state['interfaces'].update({iface['name']: iface}) |
151 | |
152 | # handle bond slaves |
153 | @@ -205,6 +213,9 @@ |
154 | interfaces = self.network_state.get('interfaces') |
155 | bond_if = interfaces.get(ifname) |
156 | bond_if['bond-master'] = command.get('name') |
157 | + # copy in bond config into slave |
158 | + for param, val in command.get('params').items(): |
159 | + bond_if.update({param: val}) |
160 | self.network_state['interfaces'].update({ifname: bond_if}) |
161 | |
162 | def handle_bridge(self, command): |
163 | @@ -354,7 +365,24 @@ |
164 | print("NS1 == NS2 ?=> {}".format( |
165 | ns1.network_state == ns2.network_state)) |
166 | |
167 | + def test_output(network_config): |
168 | + (version, config) = load_config(network_config) |
169 | + ns1 = NetworkState(version=version, config=config) |
170 | + ns1.parse_config() |
171 | + random.shuffle(config) |
172 | + ns2 = NetworkState(version=version, config=config) |
173 | + ns2.parse_config() |
174 | + print("NS1 == NS2 ?=> {}".format( |
175 | + ns1.network_state == ns2.network_state)) |
176 | + eni_1 = net.render_interfaces(ns1.network_state) |
177 | + eni_2 = net.render_interfaces(ns2.network_state) |
178 | + print(eni_1) |
179 | + print(eni_2) |
180 | + print("eni_1 == eni_2 ?=> {}".format( |
181 | + eni_1 == eni_2)) |
182 | + |
183 | y = curtin_config.load_config(sys.argv[1]) |
184 | network_config = y.get('network') |
185 | test_parse(network_config) |
186 | test_dump_and_load(network_config) |
187 | + test_output(network_config) |
188 | |
189 | === added file 'examples/tests/bonding_network.yaml' |
190 | --- examples/tests/bonding_network.yaml 1970-01-01 00:00:00 +0000 |
191 | +++ examples/tests/bonding_network.yaml 2015-09-04 19:23:11 +0000 |
192 | @@ -0,0 +1,27 @@ |
193 | +network: |
194 | + version: 1 |
195 | + config: |
196 | + # Physical interfaces. |
197 | + - type: physical |
198 | + name: eth0 |
199 | + mac_address: "52:54:00:12:34:00" |
200 | + subnets: |
201 | + - type: dhcp4 |
202 | + - type: physical |
203 | + name: eth1 |
204 | + mac_address: "52:54:00:12:34:02" |
205 | + - type: physical |
206 | + name: eth2 |
207 | + mac_address: "52:54:00:12:34:04" |
208 | + # Bond. |
209 | + - type: bond |
210 | + name: bond0 |
211 | + mac_address: "52:54:00:12:34:06" |
212 | + bond_interfaces: |
213 | + - eth1 |
214 | + - eth2 |
215 | + params: |
216 | + bond-mode: active-backup |
217 | + subnets: |
218 | + - type: static |
219 | + address: 10.23.23.2/24 |
220 | |
221 | === modified file 'tests/unittests/test_net.py' |
222 | --- tests/unittests/test_net.py 2015-09-02 13:55:39 +0000 |
223 | +++ tests/unittests/test_net.py 2015-09-04 19:23:11 +0000 |
224 | @@ -307,10 +307,8 @@ |
225 | |
226 | def test_render_interfaces(self): |
227 | ns = self.get_net_state() |
228 | - ifaces = ('auto eth1\n' + 'iface eth1 inet manual\n' + |
229 | - ' hwaddress cf:d6:af:48:e8:80\n\n' + |
230 | - 'auto eth0\n' + 'iface eth0 inet dhcp\n' + |
231 | - ' hwaddress c0:d6:9f:2c:e8:80\n\n') |
232 | + ifaces = ('auto eth0\n' + 'iface eth0 inet dhcp\n\n' + |
233 | + 'auto eth1\n' + 'iface eth1 inet manual\n\n') |
234 | net_ifaces = net.render_interfaces(ns.network_state) |
235 | self.assertEqual(sorted(ifaces.split('\n')), |
236 | sorted(net_ifaces.split('\n'))) |
237 | |
238 | === added file 'tests/vmtests/test_bonding.py' |
239 | --- tests/vmtests/test_bonding.py 1970-01-01 00:00:00 +0000 |
240 | +++ tests/vmtests/test_bonding.py 2015-09-04 19:23:11 +0000 |
241 | @@ -0,0 +1,235 @@ |
242 | +from . import VMBaseClass |
243 | +from unittest import TestCase |
244 | + |
245 | +import ipaddress |
246 | +import os |
247 | +import re |
248 | +import textwrap |
249 | +import yaml |
250 | + |
251 | + |
252 | +def iface_extract(input): |
253 | + mo = re.search(r'^(?P<interface>\w+|\w+:\d+)\s+' + |
254 | + r'Link encap:(?P<link_encap>\S+)\s+' + |
255 | + r'(HWaddr\s+(?P<mac_address>\S+))?' + |
256 | + r'(\s+inet addr:(?P<address>\S+))?' + |
257 | + r'(\s+Bcast:(?P<broadcast>\S+)\s+)?' + |
258 | + r'(Mask:(?P<netmask>\S+)\s+)?', |
259 | + input, re.MULTILINE) |
260 | + |
261 | + mtu = re.search(r'(\s+MTU:(?P<mtu>\d+)\s+)\s+', input, re.MULTILINE) |
262 | + mtu_info = mtu.groupdict('') |
263 | + mtu_info['mtu'] = int(mtu_info['mtu']) |
264 | + |
265 | + if mo: |
266 | + info = mo.groupdict('') |
267 | + info['running'] = False |
268 | + info['up'] = False |
269 | + info['multicast'] = False |
270 | + if 'RUNNING' in input: |
271 | + info['running'] = True |
272 | + if 'UP' in input: |
273 | + info['up'] = True |
274 | + if 'MULTICAST' in input: |
275 | + info['multicast'] = True |
276 | + info.update(mtu_info) |
277 | + return info |
278 | + return {} |
279 | + |
280 | + |
281 | +def ifconfig_to_dict(ifconfig): |
282 | + interfaces = {} |
283 | + for iface in [iface_extract(iface) for iface in ifconfig.split('\n\n') |
284 | + if iface.strip()]: |
285 | + interfaces[iface['interface']] = iface |
286 | + |
287 | + return interfaces |
288 | + |
289 | + |
290 | +class TestNetworkAbs(VMBaseClass): |
291 | + __test__ = False |
292 | + interactive = False |
293 | + conf_file = "examples/tests/bonding_network.yaml" |
294 | + install_timeout = 600 |
295 | + boot_timeout = 600 |
296 | + extra_disks = [] |
297 | + extra_nics = [] |
298 | + user_data = textwrap.dedent("""\ |
299 | + #cloud-config |
300 | + password: passw0rd |
301 | + chpasswd: { expire: False } |
302 | + bootcmd: |
303 | + - mkdir -p /media/output |
304 | + - mount /dev/vdb /media/output |
305 | + runcmd: |
306 | + - ifconfig -a > /media/output/ifconfig_a |
307 | + - cp -av /etc/network/interfaces /media/output |
308 | + - cp -av /etc/udev/rules.d/70-persistent-net.rules /media/output |
309 | + - ip -o route show > /media/output/ip_route_show |
310 | + - route -n > /media/output/route_n |
311 | + - dpkg-query -W -f '${Status}' ifenslave > \ |
312 | + /media/output/ifenslave_installed |
313 | + power_state: |
314 | + mode: poweroff |
315 | + """) |
316 | + |
317 | + def test_output_files_exist(self): |
318 | + self.output_files_exist(["ifconfig_a", |
319 | + "interfaces", |
320 | + "70-persistent-net.rules", |
321 | + "ip_route_show", |
322 | + "ifenslave_installed", |
323 | + "route_n"]) |
324 | + |
325 | + def test_ifenslave_installed(self): |
326 | + with open(os.path.join(self.td.mnt, "ifenslave_installed")) as fp: |
327 | + status = fp.read().strip() |
328 | + print('ifenslave installed: {}'.format(status)) |
329 | + self.assertEqual('install ok installed', status) |
330 | + |
331 | + def test_etc_network_interfaces(self): |
332 | + with open(os.path.join(self.td.mnt, "interfaces")) as fp: |
333 | + eni = fp.read() |
334 | + print('etc/network/interfaces:\n{}'.format(eni)) |
335 | + |
336 | + expected_eni = self.get_expected_etc_network_interfaces() |
337 | + eni_lines = eni.split('\n') |
338 | + for line in expected_eni.split('\n'): |
339 | + self.assertTrue(line in eni_lines) |
340 | + |
341 | + def test_ifconfig_output(self): |
342 | + '''check ifconfig output with test input''' |
343 | + network_state = self.get_network_state() |
344 | + print('expected_network_state:\n{}'.format( |
345 | + yaml.dump(network_state, default_flow_style=False, indent=4))) |
346 | + |
347 | + with open(os.path.join(self.td.mnt, "ifconfig_a")) as fp: |
348 | + ifconfig_a = fp.read() |
349 | + print('ifconfig -a:\n{}'.format(ifconfig_a)) |
350 | + |
351 | + ifconfig_dict = ifconfig_to_dict(ifconfig_a) |
352 | + print('parsed ifcfg dict:\n{}'.format( |
353 | + yaml.dump(ifconfig_dict, default_flow_style=False, indent=4))) |
354 | + |
355 | + with open(os.path.join(self.td.mnt, "ip_route_show")) as fp: |
356 | + ip_route_show = fp.read() |
357 | + print("ip route show:\n{}".format(ip_route_show)) |
358 | + for line in [line for line in ip_route_show.split('\n') |
359 | + if 'src' in line]: |
360 | + m = re.search(r'^(?P<network>\S+)\sdev\s' + |
361 | + r'(?P<devname>\S+)\s+' + |
362 | + r'proto kernel\s+scope link' + |
363 | + r'\s+src\s(?P<src_ip>\S+)', |
364 | + line) |
365 | + route_info = m.groupdict('') |
366 | + print(route_info) |
367 | + |
368 | + with open(os.path.join(self.td.mnt, "route_n")) as fp: |
369 | + route_n = fp.read() |
370 | + print("route -n:\n{}".format(route_n)) |
371 | + |
372 | + interfaces = network_state.get('interfaces') |
373 | + for iface in interfaces.values(): |
374 | + subnets = iface.get('subnets', {}) |
375 | + if subnets: |
376 | + for index, subnet in zip(range(0, len(subnets)), subnets): |
377 | + iface['index'] = index |
378 | + if index == 0: |
379 | + ifname = "{name}".format(**iface) |
380 | + else: |
381 | + ifname = "{name}:{index}".format(**iface) |
382 | + |
383 | + self.check_interface(iface, |
384 | + ifconfig_dict.get(ifname), |
385 | + route_n) |
386 | + else: |
387 | + iface['index'] = 0 |
388 | + self.check_interface(iface, |
389 | + ifconfig_dict.get(iface['name']), |
390 | + route_n) |
391 | + |
392 | + def check_interface(self, iface, ifconfig, route_n): |
393 | + print('testing iface:\n{}\n\nifconfig:\n{}'.format( |
394 | + iface, ifconfig)) |
395 | + subnets = iface.get('subnets', {}) |
396 | + if subnets and iface['index'] != 0: |
397 | + ifname = "{name}:{index}".format(**iface) |
398 | + else: |
399 | + ifname = "{name}".format(**iface) |
400 | + |
401 | + # initial check, do we have the correct iface ? |
402 | + print('ifname={}'.format(ifname)) |
403 | + print("ifconfig['interface']={}".format(ifconfig['interface'])) |
404 | + self.assertEqual(ifname, ifconfig['interface']) |
405 | + |
406 | + # check physical interface attributes |
407 | + # FIXME: can't check mac_addr under bonding since |
408 | + # the bond might change slave mac addrs |
409 | + for key in ['mtu']: |
410 | + if key in iface and iface[key]: |
411 | + self.assertEqual(iface[key], |
412 | + ifconfig[key]) |
413 | + |
414 | + def __get_subnet(subnets, subidx): |
415 | + for index, subnet in zip(range(0, len(subnets)), subnets): |
416 | + if index == subidx: |
417 | + break |
418 | + return subnet |
419 | + |
420 | + # check subnet related attributes, and specifically only |
421 | + # the subnet specified by iface['index'] |
422 | + subnets = iface.get('subnets', {}) |
423 | + if subnets: |
424 | + subnet = __get_subnet(subnets, iface['index']) |
425 | + if 'address' in subnet and subnet['address']: |
426 | + if ':' in subnet['address']: |
427 | + inet_iface = ipaddress.IPv6Interface( |
428 | + subnet['address']) |
429 | + else: |
430 | + inet_iface = ipaddress.IPv4Interface( |
431 | + subnet['address']) |
432 | + |
433 | + # check ip addr |
434 | + self.assertEqual(str(inet_iface.ip), |
435 | + ifconfig['address']) |
436 | + |
437 | + self.assertEqual(str(inet_iface.netmask), |
438 | + ifconfig['netmask']) |
439 | + |
440 | + self.assertEqual( |
441 | + str(inet_iface.network.broadcast_address), |
442 | + ifconfig['broadcast']) |
443 | + |
444 | + # handle gateway by looking at routing table |
445 | + if 'gateway' in subnet and subnet['gateway']: |
446 | + gw_ip = subnet['gateway'] |
447 | + gateways = [line for line in route_n.split('\n') |
448 | + if 'UG' in line and gw_ip in line] |
449 | + print('matching gateways:\n{}'.format(gateways)) |
450 | + self.assertEqual(len(gateways), 1) |
451 | + [gateways] = gateways |
452 | + (dest, gw, genmask, flags, metric, ref, use, iface) = \ |
453 | + gateways.split() |
454 | + print('expected gw:{} found gw:{}'.format(gw_ip, gw)) |
455 | + self.assertEqual(gw_ip, gw) |
456 | + |
457 | + |
458 | +class TrustyTestBasic(TestNetworkAbs, TestCase): |
459 | + __test__ = False |
460 | + repo = "maas-daily" |
461 | + release = "trusty" |
462 | + arch = "amd64" |
463 | + |
464 | + |
465 | +class WilyTestBasic(TestNetworkAbs, TestCase): |
466 | + __test__ = True |
467 | + repo = "maas-daily" |
468 | + release = "wily" |
469 | + arch = "amd64" |
470 | + |
471 | + |
472 | +class VividTestBasic(TestNetworkAbs, TestCase): |
473 | + __test__ = True |
474 | + repo = "maas-daily" |
475 | + release = "vivid" |
476 | + arch = "amd64" |
llooks good.