Merge ~raphael-glon/cloud-init:master into cloud-init:master
- Git
- lp:~raphael-glon/cloud-init
- master
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Dan Watkins | ||||
Approved revision: | 1af82aaadd9639ea6e32884a2dfcaa4cb205a5fd | ||||
Merge reported by: | Server Team CI bot | ||||
Merged at revision: | not available | ||||
Proposed branch: | ~raphael-glon/cloud-init:master | ||||
Merge into: | cloud-init:master | ||||
Diff against target: |
233 lines (+143/-20) 2 files modified
cloudinit/net/eni.py (+11/-5) tests/unittests/test_net.py (+132/-15) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Ryan Harper | Approve | ||
Server Team CI bot | continuous-integration | Approve | |
Dan Watkins | Abstain | ||
Review via email: mp+363970@code.launchpad.net |
Commit message
net: Fix ipv6 static routes when using eni renderer
When rendering ipv6 static routes in eni format the
post-up/pre down commands were not correct for ipv6.
LP: #1818669
Description of the change
- 1af82aa... by raphael.glon
-
Unittests: add bond rendering test for eni config
Ryan Harper (raharper) wrote : | # |
I've taken a look and I can confirm that for v6 routes, route needs the network with prefix.
This is information is present in the internal network_state of the route so a little juggling is needed to always use network/prefix when rendering route and ensure that on v6 routes we use -A inet6.
I've added the test-case from the bug and a few others.
This passes tox and does not regress anything.
Please consider applying this instead
raphael.glon (raphael-glon) wrote : | # |
Ok for the considered patch
-> but there is still one problem with it:
-net is mandatory for route -4 but unsupported for ipv6
so we need to remove the -net param for ipv6 only
- aa64677... by raphael.glon
-
Fix net/eni static routes for ipv6
this fixes:
https://bugs.launchpad .net/cloud- init/+bug/ 1818669
eni: post-up/pre down commands were not correct for ipv6unittests: additional test to check the
rendered static routes for eni config
Dan Watkins (oddbloke) wrote : | # |
CLA has been signed; thanks Raphael!
(I'm going to abstain as Ryan has started working with you on this.)
Ryan Harper (raharper) wrote : | # |
Yes; that's a good catch on the -net | -A inet6.
Ryan Harper (raharper) wrote : | # |
I've pointed CI at this branch. If that comes out clean, I'll mark approve and land.
Server Team CI bot (server-team-bot) wrote : | # |
PASSED: Continuous integration, rev:1af82aaadd9
https:/
Executed test runs:
SUCCESS: Checkout
SUCCESS: Unit & Style Tests
SUCCESS: Ubuntu LTS: Build
SUCCESS: Ubuntu LTS: Integration
IN_PROGRESS: Declarative: Post Actions
Click here to trigger a rebuild:
https:/
raphael.glon (raphael-glon) wrote : | # |
Hello Ryan,
Not sure what I should do next about this merge request. Is there something more needed for approval ?
Regards
Ryan Harper (raharper) wrote : | # |
Sorry for not replying. The branch is good to go. I just need to mark it for landing.
Ryan Harper (raharper) : | # |
Preview Diff
1 | diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py | |||
2 | index 6423632..b129bb6 100644 | |||
3 | --- a/cloudinit/net/eni.py | |||
4 | +++ b/cloudinit/net/eni.py | |||
5 | @@ -366,8 +366,6 @@ class Renderer(renderer.Renderer): | |||
6 | 366 | down = indent + "pre-down route del" | 366 | down = indent + "pre-down route del" |
7 | 367 | or_true = " || true" | 367 | or_true = " || true" |
8 | 368 | mapping = { | 368 | mapping = { |
9 | 369 | 'network': '-net', | ||
10 | 370 | 'netmask': 'netmask', | ||
11 | 371 | 'gateway': 'gw', | 369 | 'gateway': 'gw', |
12 | 372 | 'metric': 'metric', | 370 | 'metric': 'metric', |
13 | 373 | } | 371 | } |
14 | @@ -379,13 +377,21 @@ class Renderer(renderer.Renderer): | |||
15 | 379 | default_gw = ' -A inet6 default' | 377 | default_gw = ' -A inet6 default' |
16 | 380 | 378 | ||
17 | 381 | route_line = '' | 379 | route_line = '' |
20 | 382 | for k in ['network', 'netmask', 'gateway', 'metric']: | 380 | for k in ['network', 'gateway', 'metric']: |
21 | 383 | if default_gw and k in ['network', 'netmask']: | 381 | if default_gw and k == 'network': |
22 | 384 | continue | 382 | continue |
23 | 385 | if k == 'gateway': | 383 | if k == 'gateway': |
24 | 386 | route_line += '%s %s %s' % (default_gw, mapping[k], route[k]) | 384 | route_line += '%s %s %s' % (default_gw, mapping[k], route[k]) |
25 | 387 | elif k in route: | 385 | elif k in route: |
27 | 388 | route_line += ' %s %s' % (mapping[k], route[k]) | 386 | if k == 'network': |
28 | 387 | if ':' in route[k]: | ||
29 | 388 | route_line += ' -A inet6' | ||
30 | 389 | else: | ||
31 | 390 | route_line += ' -net' | ||
32 | 391 | if 'prefix' in route: | ||
33 | 392 | route_line += ' %s/%s' % (route[k], route['prefix']) | ||
34 | 393 | else: | ||
35 | 394 | route_line += ' %s %s' % (mapping[k], route[k]) | ||
36 | 389 | content.append(up + route_line + or_true) | 395 | content.append(up + route_line + or_true) |
37 | 390 | content.append(down + route_line + or_true) | 396 | content.append(down + route_line + or_true) |
38 | 391 | return content | 397 | return content |
39 | diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py | |||
40 | index e3b9e02..f725a4b 100644 | |||
41 | --- a/tests/unittests/test_net.py | |||
42 | +++ b/tests/unittests/test_net.py | |||
43 | @@ -1113,8 +1113,8 @@ iface eth0.101 inet static | |||
44 | 1113 | iface eth0.101 inet static | 1113 | iface eth0.101 inet static |
45 | 1114 | address 192.168.2.10/24 | 1114 | address 192.168.2.10/24 |
46 | 1115 | 1115 | ||
49 | 1116 | post-up route add -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true | 1116 | post-up route add -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true |
50 | 1117 | pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true | 1117 | pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true |
51 | 1118 | """), | 1118 | """), |
52 | 1119 | 'expected_netplan': textwrap.dedent(""" | 1119 | 'expected_netplan': textwrap.dedent(""" |
53 | 1120 | network: | 1120 | network: |
54 | @@ -1505,17 +1505,18 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true | |||
55 | 1505 | - gateway: 192.168.0.3 | 1505 | - gateway: 192.168.0.3 |
56 | 1506 | netmask: 255.255.255.0 | 1506 | netmask: 255.255.255.0 |
57 | 1507 | network: 10.1.3.0 | 1507 | network: 10.1.3.0 |
58 | 1508 | - gateway: 2001:67c:1562:1 | ||
59 | 1509 | network: 2001:67c:1 | ||
60 | 1510 | netmask: ffff:ffff:0 | ||
61 | 1511 | - gateway: 3001:67c:1562:1 | ||
62 | 1512 | network: 3001:67c:1 | ||
63 | 1513 | netmask: ffff:ffff:0 | ||
64 | 1514 | metric: 10000 | ||
65 | 1515 | - type: static | 1508 | - type: static |
66 | 1516 | address: 192.168.1.2/24 | 1509 | address: 192.168.1.2/24 |
67 | 1517 | - type: static | 1510 | - type: static |
68 | 1518 | address: 2001:1::1/92 | 1511 | address: 2001:1::1/92 |
69 | 1512 | routes: | ||
70 | 1513 | - gateway: 2001:67c:1562:1 | ||
71 | 1514 | network: 2001:67c:1 | ||
72 | 1515 | netmask: ffff:ffff:0 | ||
73 | 1516 | - gateway: 3001:67c:1562:1 | ||
74 | 1517 | network: 3001:67c:1 | ||
75 | 1518 | netmask: ffff:ffff:0 | ||
76 | 1519 | metric: 10000 | ||
77 | 1519 | """), | 1520 | """), |
78 | 1520 | 'expected_netplan': textwrap.dedent(""" | 1521 | 'expected_netplan': textwrap.dedent(""" |
79 | 1521 | network: | 1522 | network: |
80 | @@ -1554,6 +1555,51 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true | |||
81 | 1554 | to: 3001:67c:1/32 | 1555 | to: 3001:67c:1/32 |
82 | 1555 | via: 3001:67c:1562:1 | 1556 | via: 3001:67c:1562:1 |
83 | 1556 | """), | 1557 | """), |
84 | 1558 | 'expected_eni': textwrap.dedent("""\ | ||
85 | 1559 | auto lo | ||
86 | 1560 | iface lo inet loopback | ||
87 | 1561 | |||
88 | 1562 | auto bond0s0 | ||
89 | 1563 | iface bond0s0 inet manual | ||
90 | 1564 | bond-master bond0 | ||
91 | 1565 | bond-mode active-backup | ||
92 | 1566 | bond-xmit-hash-policy layer3+4 | ||
93 | 1567 | bond_miimon 100 | ||
94 | 1568 | |||
95 | 1569 | auto bond0s1 | ||
96 | 1570 | iface bond0s1 inet manual | ||
97 | 1571 | bond-master bond0 | ||
98 | 1572 | bond-mode active-backup | ||
99 | 1573 | bond-xmit-hash-policy layer3+4 | ||
100 | 1574 | bond_miimon 100 | ||
101 | 1575 | |||
102 | 1576 | auto bond0 | ||
103 | 1577 | iface bond0 inet static | ||
104 | 1578 | address 192.168.0.2/24 | ||
105 | 1579 | gateway 192.168.0.1 | ||
106 | 1580 | bond-mode active-backup | ||
107 | 1581 | bond-slaves none | ||
108 | 1582 | bond-xmit-hash-policy layer3+4 | ||
109 | 1583 | bond_miimon 100 | ||
110 | 1584 | hwaddress aa:bb:cc:dd:e8:ff | ||
111 | 1585 | mtu 9000 | ||
112 | 1586 | post-up route add -net 10.1.3.0/24 gw 192.168.0.3 || true | ||
113 | 1587 | pre-down route del -net 10.1.3.0/24 gw 192.168.0.3 || true | ||
114 | 1588 | |||
115 | 1589 | # control-alias bond0 | ||
116 | 1590 | iface bond0 inet static | ||
117 | 1591 | address 192.168.1.2/24 | ||
118 | 1592 | |||
119 | 1593 | # control-alias bond0 | ||
120 | 1594 | iface bond0 inet6 static | ||
121 | 1595 | address 2001:1::1/92 | ||
122 | 1596 | post-up route add -A inet6 2001:67c:1/32 gw 2001:67c:1562:1 || true | ||
123 | 1597 | pre-down route del -A inet6 2001:67c:1/32 gw 2001:67c:1562:1 || true | ||
124 | 1598 | post-up route add -A inet6 3001:67c:1/32 gw 3001:67c:1562:1 metric 10000 \ | ||
125 | 1599 | || true | ||
126 | 1600 | pre-down route del -A inet6 3001:67c:1/32 gw 3001:67c:1562:1 metric 10000 \ | ||
127 | 1601 | || true | ||
128 | 1602 | """), | ||
129 | 1557 | 'yaml-v2': textwrap.dedent(""" | 1603 | 'yaml-v2': textwrap.dedent(""" |
130 | 1558 | version: 2 | 1604 | version: 2 |
131 | 1559 | ethernets: | 1605 | ethernets: |
132 | @@ -3570,17 +3616,17 @@ class TestEniRoundTrip(CiTestCase): | |||
133 | 3570 | 'iface eth0 inet static', | 3616 | 'iface eth0 inet static', |
134 | 3571 | ' address 172.23.31.42/26', | 3617 | ' address 172.23.31.42/26', |
135 | 3572 | ' gateway 172.23.31.2', | 3618 | ' gateway 172.23.31.2', |
137 | 3573 | ('post-up route add -net 10.0.0.0 netmask 255.240.0.0 gw ' | 3619 | ('post-up route add -net 10.0.0.0/12 gw ' |
138 | 3574 | '172.23.31.1 metric 0 || true'), | 3620 | '172.23.31.1 metric 0 || true'), |
140 | 3575 | ('pre-down route del -net 10.0.0.0 netmask 255.240.0.0 gw ' | 3621 | ('pre-down route del -net 10.0.0.0/12 gw ' |
141 | 3576 | '172.23.31.1 metric 0 || true'), | 3622 | '172.23.31.1 metric 0 || true'), |
143 | 3577 | ('post-up route add -net 192.168.2.0 netmask 255.255.0.0 gw ' | 3623 | ('post-up route add -net 192.168.2.0/16 gw ' |
144 | 3578 | '172.23.31.1 metric 0 || true'), | 3624 | '172.23.31.1 metric 0 || true'), |
146 | 3579 | ('pre-down route del -net 192.168.2.0 netmask 255.255.0.0 gw ' | 3625 | ('pre-down route del -net 192.168.2.0/16 gw ' |
147 | 3580 | '172.23.31.1 metric 0 || true'), | 3626 | '172.23.31.1 metric 0 || true'), |
149 | 3581 | ('post-up route add -net 10.0.200.0 netmask 255.255.0.0 gw ' | 3627 | ('post-up route add -net 10.0.200.0/16 gw ' |
150 | 3582 | '172.23.31.1 metric 1 || true'), | 3628 | '172.23.31.1 metric 1 || true'), |
152 | 3583 | ('pre-down route del -net 10.0.200.0 netmask 255.255.0.0 gw ' | 3629 | ('pre-down route del -net 10.0.200.0/16 gw ' |
153 | 3584 | '172.23.31.1 metric 1 || true'), | 3630 | '172.23.31.1 metric 1 || true'), |
154 | 3585 | ] | 3631 | ] |
155 | 3586 | found = files['/etc/network/interfaces'].splitlines() | 3632 | found = files['/etc/network/interfaces'].splitlines() |
156 | @@ -3588,6 +3634,77 @@ class TestEniRoundTrip(CiTestCase): | |||
157 | 3588 | self.assertEqual( | 3634 | self.assertEqual( |
158 | 3589 | expected, [line for line in found if line]) | 3635 | expected, [line for line in found if line]) |
159 | 3590 | 3636 | ||
160 | 3637 | def test_ipv6_static_routes(self): | ||
161 | 3638 | # as reported in bug 1818669 | ||
162 | 3639 | conf = [ | ||
163 | 3640 | {'name': 'eno3', 'type': 'physical', | ||
164 | 3641 | 'subnets': [{ | ||
165 | 3642 | 'address': 'fd00::12/64', | ||
166 | 3643 | 'dns_nameservers': ['fd00:2::15'], | ||
167 | 3644 | 'gateway': 'fd00::1', | ||
168 | 3645 | 'ipv6': True, | ||
169 | 3646 | 'type': 'static', | ||
170 | 3647 | 'routes': [{'netmask': '32', | ||
171 | 3648 | 'network': 'fd00:12::', | ||
172 | 3649 | 'gateway': 'fd00::2'}, | ||
173 | 3650 | {'network': 'fd00:14::', | ||
174 | 3651 | 'gateway': 'fd00::3'}, | ||
175 | 3652 | {'destination': 'fe00:14::/48', | ||
176 | 3653 | 'gateway': 'fe00::4', | ||
177 | 3654 | 'metric': 500}, | ||
178 | 3655 | {'gateway': '192.168.23.1', | ||
179 | 3656 | 'metric': 999, | ||
180 | 3657 | 'netmask': 24, | ||
181 | 3658 | 'network': '192.168.23.0'}, | ||
182 | 3659 | {'destination': '10.23.23.0/24', | ||
183 | 3660 | 'gateway': '10.23.23.2', | ||
184 | 3661 | 'metric': 300}]}]}, | ||
185 | 3662 | ] | ||
186 | 3663 | |||
187 | 3664 | files = self._render_and_read( | ||
188 | 3665 | network_config={'config': conf, 'version': 1}) | ||
189 | 3666 | expected = [ | ||
190 | 3667 | 'auto lo', | ||
191 | 3668 | 'iface lo inet loopback', | ||
192 | 3669 | 'auto eno3', | ||
193 | 3670 | 'iface eno3 inet6 static', | ||
194 | 3671 | ' address fd00::12/64', | ||
195 | 3672 | ' dns-nameservers fd00:2::15', | ||
196 | 3673 | ' gateway fd00::1', | ||
197 | 3674 | (' post-up route add -A inet6 fd00:12::/32 gw ' | ||
198 | 3675 | 'fd00::2 || true'), | ||
199 | 3676 | (' pre-down route del -A inet6 fd00:12::/32 gw ' | ||
200 | 3677 | 'fd00::2 || true'), | ||
201 | 3678 | (' post-up route add -A inet6 fd00:14::/64 gw ' | ||
202 | 3679 | 'fd00::3 || true'), | ||
203 | 3680 | (' pre-down route del -A inet6 fd00:14::/64 gw ' | ||
204 | 3681 | 'fd00::3 || true'), | ||
205 | 3682 | (' post-up route add -A inet6 fe00:14::/48 gw ' | ||
206 | 3683 | 'fe00::4 metric 500 || true'), | ||
207 | 3684 | (' pre-down route del -A inet6 fe00:14::/48 gw ' | ||
208 | 3685 | 'fe00::4 metric 500 || true'), | ||
209 | 3686 | (' post-up route add -net 192.168.23.0/24 gw ' | ||
210 | 3687 | '192.168.23.1 metric 999 || true'), | ||
211 | 3688 | (' pre-down route del -net 192.168.23.0/24 gw ' | ||
212 | 3689 | '192.168.23.1 metric 999 || true'), | ||
213 | 3690 | (' post-up route add -net 10.23.23.0/24 gw ' | ||
214 | 3691 | '10.23.23.2 metric 300 || true'), | ||
215 | 3692 | (' pre-down route del -net 10.23.23.0/24 gw ' | ||
216 | 3693 | '10.23.23.2 metric 300 || true'), | ||
217 | 3694 | |||
218 | 3695 | ] | ||
219 | 3696 | found = files['/etc/network/interfaces'].splitlines() | ||
220 | 3697 | |||
221 | 3698 | self.assertEqual( | ||
222 | 3699 | expected, [line for line in found if line]) | ||
223 | 3700 | |||
224 | 3701 | def testsimple_render_bond(self): | ||
225 | 3702 | entry = NETWORK_CONFIGS['bond'] | ||
226 | 3703 | files = self._render_and_read(network_config=yaml.load(entry['yaml'])) | ||
227 | 3704 | self.assertEqual( | ||
228 | 3705 | entry['expected_eni'].splitlines(), | ||
229 | 3706 | files['/etc/network/interfaces'].splitlines()) | ||
230 | 3707 | |||
231 | 3591 | 3708 | ||
232 | 3592 | class TestNetRenderers(CiTestCase): | 3709 | class TestNetRenderers(CiTestCase): |
233 | 3593 | @mock.patch("cloudinit.net.renderers.sysconfig.available") | 3710 | @mock.patch("cloudinit.net.renderers.sysconfig.available") |
Hi Raphael,
Thank you for contributing to cloud-init.
To contribute, you must sign the Canonical Contributor License Agreement
(CLA) [1].
If you have already signed it as an individual, your Launchpad user will agreement- canonical launchpad group [2].
be listed in the contributor-
Unfortunately there is no easy way to check if an organization or company
you are doing work for has signed. If you are unsure or have questions,
email <email address hidden> or ping powersj in #cloud-init channel
via freenode.
For information on how to sign, please see the HACKING document [3].
Thanks again, and please feel free to reach out with any questions.
– www.canonical. com/contributor s /launchpad. net/~contributo r-agreement- canonical/ +members cloudinit. readthedocs. io/en/latest/ topics/ hacking. html
[1] http://
[2] https:/
[3] http://