Merge ~mpontillo/maas:preseed-per-interface-routes--bug-1758919--2.3 into maas:2.3
- Git
- lp:~mpontillo/maas
- preseed-per-interface-routes--bug-1758919--2.3
- Merge into 2.3
Proposed by
Mike Pontillo
Status: | Merged |
---|---|
Approved by: | Mike Pontillo |
Approved revision: | 434ff94a620afd73b314baa08e7e95fa36ec41ca |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~mpontillo/maas:preseed-per-interface-routes--bug-1758919--2.3 |
Merge into: | maas:2.3 |
Diff against target: |
381 lines (+233/-61) 2 files modified
src/maasserver/preseed_network.py (+25/-16) src/maasserver/tests/test_preseed_network.py (+208/-45) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Review via email: mp+342243@code.launchpad.net |
Commit message
LP: #1758919 - Move routes to interface context within preseed.
Backports: 9fbf7eb682c3c3f
Description of the change
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 | diff --git a/src/maasserver/preseed_network.py b/src/maasserver/preseed_network.py |
2 | index 1be72d6..bb56765 100644 |
3 | --- a/src/maasserver/preseed_network.py |
4 | +++ b/src/maasserver/preseed_network.py |
5 | @@ -52,8 +52,6 @@ def _generate_route_operation(route, version=1): |
6 | """Generate route operation place in `network_config`.""" |
7 | if version == 1: |
8 | route_operation = { |
9 | - "id": route.id, |
10 | - "type": "route", |
11 | "destination": route.destination.cidr, |
12 | "gateway": route.gateway_ip, |
13 | "metric": route.metric, |
14 | @@ -99,6 +97,19 @@ class InterfaceConfiguration: |
15 | else: |
16 | raise ValueError("Unknown interface type: %s" % self.type) |
17 | |
18 | + if version == 2: |
19 | + routes = self._generate_route_operations( |
20 | + self.matching_routes, version=version) |
21 | + if len(routes) > 0: |
22 | + self.config['routes'] = routes |
23 | + |
24 | + def _generate_route_operations(self, matching_routes, version=1): |
25 | + """Generate all route operations.""" |
26 | + routes = [] |
27 | + for route in sorted(matching_routes, key=attrgetter("id")): |
28 | + routes.append(_generate_route_operation(route, version=version)) |
29 | + return routes |
30 | + |
31 | def _generate_physical_operation(self, version=1): |
32 | """Generate physical interface operation for `interface` and place in |
33 | `network_config`.""" |
34 | @@ -241,7 +252,8 @@ class InterfaceConfiguration: |
35 | self._set_default_gateway( |
36 | subnet, |
37 | v1_subnet_operation if version == 1 else v2_config) |
38 | - if subnet.dns_servers is not None: |
39 | + if (subnet.dns_servers is not None and |
40 | + len(subnet.dns_servers) > 0): |
41 | v1_subnet_operation["dns_nameservers"] = ( |
42 | subnet.dns_servers) |
43 | if "nameservers" not in v2_config: |
44 | @@ -251,8 +263,16 @@ class InterfaceConfiguration: |
45 | v2_nameservers["addresses"] = [] |
46 | v2_nameservers["addresses"].extend( |
47 | [server for server in subnet.dns_servers]) |
48 | - self.matching_routes.update( |
49 | - self._get_matching_routes(subnet)) |
50 | + matching_subnet_routes = self._get_matching_routes(subnet) |
51 | + if len(matching_subnet_routes) > 0 and version == 1: |
52 | + # For the v1 YAML, the list of routes is rendered |
53 | + # within the context of each subnet. |
54 | + routes = self._generate_route_operations( |
55 | + matching_subnet_routes, version=version) |
56 | + v1_subnet_operation['routes'] = routes |
57 | + # Keep track of routes which apply to the context of this |
58 | + # interface for rendering the v2 YAML. |
59 | + self.matching_routes.update(matching_subnet_routes) |
60 | if dhcp_type: |
61 | v1_config.append( |
62 | {"type": dhcp_type} |
63 | @@ -479,7 +499,6 @@ class NodeNetworkConfiguration: |
64 | for name in sorted(get_dns_search_paths()) |
65 | if name != self.node.domain.name |
66 | ] |
67 | - self._generate_route_operations(version=version) |
68 | self.v1_config.append({ |
69 | "type": "nameserver", |
70 | "address": default_dns_servers, |
71 | @@ -517,16 +536,6 @@ class NodeNetworkConfiguration: |
72 | # v2_config.update({"nameservers": nameservers}) |
73 | self.config = network_config |
74 | |
75 | - def _generate_route_operations(self, version=1): |
76 | - """Generate all route operations.""" |
77 | - routes = [] |
78 | - for route in sorted(self.matching_routes, key=attrgetter("id")): |
79 | - routes.append(_generate_route_operation(route, version=version)) |
80 | - if version == 1: |
81 | - self.v1_config.extend(routes) |
82 | - elif version == 2 and len(routes) > 0: |
83 | - self.v2_config["routes"] = routes |
84 | - |
85 | |
86 | def compose_curtin_network_config(node, version=1): |
87 | """Compose the network configuration for curtin.""" |
88 | diff --git a/src/maasserver/tests/test_preseed_network.py b/src/maasserver/tests/test_preseed_network.py |
89 | index 8a509dc..cc66f9f 100644 |
90 | --- a/src/maasserver/tests/test_preseed_network.py |
91 | +++ b/src/maasserver/tests/test_preseed_network.py |
92 | @@ -5,7 +5,6 @@ |
93 | |
94 | __all__ = [] |
95 | |
96 | -from operator import attrgetter |
97 | import random |
98 | from textwrap import dedent |
99 | |
100 | @@ -15,7 +14,6 @@ from maasserver.enum import ( |
101 | IPADDRESS_FAMILY, |
102 | IPADDRESS_TYPE, |
103 | ) |
104 | -from maasserver.models.staticroute import StaticRoute |
105 | from maasserver.preseed_network import ( |
106 | compose_curtin_network_config, |
107 | NodeNetworkConfiguration, |
108 | @@ -228,31 +226,6 @@ class AssertNetworkConfigMixin: |
109 | ret += " - type: dhcp6\n" |
110 | return ret |
111 | |
112 | - def collectRoutesConfig(self, node): |
113 | - routes = set() |
114 | - interfaces = node.interface_set.filter(enabled=True).order_by('name') |
115 | - for iface in interfaces: |
116 | - addresses = iface.ip_addresses.exclude( |
117 | - alloc_type__in=[ |
118 | - IPADDRESS_TYPE.DISCOVERED, |
119 | - IPADDRESS_TYPE.DHCP, |
120 | - ]).order_by('id') |
121 | - for address in addresses: |
122 | - subnet = address.subnet |
123 | - if subnet is not None: |
124 | - routes.update( |
125 | - StaticRoute.objects.filter( |
126 | - source=subnet).order_by('id')) |
127 | - config = "" |
128 | - for route in sorted(routes, key=attrgetter('id')): |
129 | - config += self.ROUTE_CONFIG % { |
130 | - 'id': route.id, |
131 | - 'destination': route.destination.cidr, |
132 | - 'gateway': route.gateway_ip, |
133 | - 'metric': route.metric, |
134 | - } |
135 | - return config |
136 | - |
137 | def collectDNSConfig(self, node, ipv4=True, ipv6=True): |
138 | config = "- type: nameserver\n address: %s\n search:\n" % ( |
139 | repr(node.get_default_dns_servers(ipv4=ipv4, ipv6=ipv6))) |
140 | @@ -466,29 +439,14 @@ class TestBridgeNetworkLayout(MAASServerTestCase, AssertNetworkConfigMixin): |
141 | self.assertNetworkConfig(net_config, config) |
142 | |
143 | |
144 | -class TestNetworkLayoutWithRoutes( |
145 | - MAASServerTestCase, AssertNetworkConfigMixin): |
146 | - |
147 | - def test__renders_expected_output(self): |
148 | - node = factory.make_Node_with_Interface_on_Subnet( |
149 | - interface_count=2) |
150 | - for iface in node.interface_set.filter(enabled=True): |
151 | - subnet = iface.vlan.subnet_set.first() |
152 | - factory.make_StaticRoute(source=subnet) |
153 | - factory.make_StaticIPAddress( |
154 | - interface=iface, subnet=subnet) |
155 | - net_config = self.collect_interface_config(node) |
156 | - net_config += self.collectRoutesConfig(node) |
157 | - net_config += self.collectDNSConfig(node) |
158 | - config = compose_curtin_network_config(node) |
159 | - self.assertNetworkConfig(net_config, config) |
160 | - |
161 | - |
162 | class TestNetplan(MAASServerTestCase): |
163 | |
164 | def _render_netplan_dict(self, node): |
165 | return NodeNetworkConfiguration(node, version=2).config |
166 | |
167 | + def _render_v1_dict(self, node): |
168 | + return NodeNetworkConfiguration(node, version=1).config |
169 | + |
170 | def test__single_ethernet_interface(self): |
171 | node = factory.make_Node() |
172 | factory.make_Interface( |
173 | @@ -824,3 +782,208 @@ class TestNetplan(MAASServerTestCase): |
174 | } |
175 | } |
176 | self.expectThat(netplan, Equals(expected_netplan)) |
177 | + |
178 | + def test__multiple_ethernet_interfaces_with_routes(self): |
179 | + node = factory.make_Node() |
180 | + vlan = factory.make_VLAN() |
181 | + subnet = factory.make_Subnet( |
182 | + cidr='10.0.0.0/24', gateway_ip='10.0.0.1', |
183 | + dns_servers=[]) |
184 | + subnet2 = factory.make_Subnet( |
185 | + cidr='10.0.1.0/24', gateway_ip='10.0.1.1', |
186 | + dns_servers=[]) |
187 | + dest_subnet = factory.make_Subnet(cidr='192.168.0.0/24') |
188 | + eth0 = factory.make_Interface( |
189 | + node=node, name='eth0', mac_address="00:01:02:03:04:05", vlan=vlan) |
190 | + eth1 = factory.make_Interface( |
191 | + node=node, name='eth1', mac_address="02:01:02:03:04:05") |
192 | + node.boot_interface = eth0 |
193 | + node.save() |
194 | + factory.make_StaticIPAddress( |
195 | + interface=eth0, subnet=subnet, ip='10.0.0.4', |
196 | + alloc_type=IPADDRESS_TYPE.STICKY) |
197 | + factory.make_StaticIPAddress( |
198 | + interface=eth1, subnet=subnet2, ip='10.0.1.4', |
199 | + alloc_type=IPADDRESS_TYPE.STICKY) |
200 | + factory.make_StaticRoute( |
201 | + source=subnet, gateway_ip='10.0.0.3', destination=dest_subnet, |
202 | + metric=42) |
203 | + factory.make_StaticRoute( |
204 | + source=subnet2, gateway_ip='10.0.1.3', destination=dest_subnet, |
205 | + metric=43) |
206 | + # Make sure we know when and where the default DNS server will be used. |
207 | + get_default_dns_servers_mock = self.patch( |
208 | + node, 'get_default_dns_servers') |
209 | + get_default_dns_servers_mock.return_value = ['127.0.0.2'] |
210 | + netplan = self._render_netplan_dict(node) |
211 | + expected_netplan = { |
212 | + 'network': { |
213 | + 'version': 2, |
214 | + 'ethernets': { |
215 | + 'eth0': { |
216 | + 'gateway': '10.0.0.1', |
217 | + 'match': {'macaddress': '00:01:02:03:04:05'}, |
218 | + 'mtu': 1500, |
219 | + 'set-name': 'eth0', |
220 | + 'addresses': ['10.0.0.4/24'], |
221 | + 'routes': [{ |
222 | + 'to': '192.168.0.0/24', |
223 | + 'via': '10.0.0.3', |
224 | + 'metric': 42, |
225 | + }], |
226 | + }, |
227 | + 'eth1': { |
228 | + 'match': {'macaddress': '02:01:02:03:04:05'}, |
229 | + 'mtu': 1500, |
230 | + 'set-name': 'eth1', |
231 | + 'addresses': ['10.0.1.4/24'], |
232 | + 'routes': [{ |
233 | + 'to': '192.168.0.0/24', |
234 | + 'via': '10.0.1.3', |
235 | + 'metric': 43, |
236 | + }], |
237 | + }, |
238 | + }, |
239 | + }, |
240 | + } |
241 | + self.expectThat(netplan, Equals(expected_netplan)) |
242 | + v1 = self._render_v1_dict(node) |
243 | + expected_v1 = { |
244 | + 'network': { |
245 | + 'version': 1, |
246 | + 'config': [ |
247 | + { |
248 | + 'id': 'eth0', |
249 | + 'mac_address': '00:01:02:03:04:05', |
250 | + 'mtu': 1500, |
251 | + 'name': 'eth0', |
252 | + 'subnets': [{ |
253 | + 'address': '10.0.0.4/24', |
254 | + 'gateway': '10.0.0.1', |
255 | + 'type': 'static', |
256 | + 'routes': [{ |
257 | + 'destination': '192.168.0.0/24', |
258 | + 'gateway': '10.0.0.3', |
259 | + 'metric': 42 |
260 | + }], |
261 | + }], |
262 | + 'type': 'physical' |
263 | + }, |
264 | + { |
265 | + 'id': 'eth1', |
266 | + 'mac_address': '02:01:02:03:04:05', |
267 | + 'mtu': 1500, |
268 | + 'name': 'eth1', |
269 | + 'subnets': [{ |
270 | + 'address': '10.0.1.4/24', |
271 | + 'type': 'static', |
272 | + 'routes': [{ |
273 | + 'destination': '192.168.0.0/24', |
274 | + 'gateway': '10.0.1.3', |
275 | + 'metric': 43, |
276 | + }], |
277 | + }], |
278 | + 'type': 'physical' |
279 | + }, |
280 | + { |
281 | + 'address': ['127.0.0.2'], |
282 | + 'search': ['maas'], |
283 | + 'type': 'nameserver' |
284 | + } |
285 | + ], |
286 | + } |
287 | + } |
288 | + self.expectThat(v1, Equals(expected_v1)) |
289 | + |
290 | + def test__multiple_ethernet_interfaces_with_dns(self): |
291 | + node = factory.make_Node() |
292 | + vlan = factory.make_VLAN() |
293 | + subnet = factory.make_Subnet( |
294 | + cidr='10.0.0.0/24', gateway_ip='10.0.0.1', |
295 | + dns_servers=['10.0.0.2']) |
296 | + subnet2 = factory.make_Subnet( |
297 | + cidr='10.0.1.0/24', gateway_ip='10.0.1.1', |
298 | + dns_servers=['10.0.1.2']) |
299 | + eth0 = factory.make_Interface( |
300 | + node=node, name='eth0', mac_address="00:01:02:03:04:05", vlan=vlan) |
301 | + eth1 = factory.make_Interface( |
302 | + node=node, name='eth1', mac_address="02:01:02:03:04:05") |
303 | + node.boot_interface = eth0 |
304 | + node.save() |
305 | + factory.make_StaticIPAddress( |
306 | + interface=eth0, subnet=subnet, ip='10.0.0.4', |
307 | + alloc_type=IPADDRESS_TYPE.STICKY) |
308 | + factory.make_StaticIPAddress( |
309 | + interface=eth1, subnet=subnet2, ip='10.0.1.4', |
310 | + alloc_type=IPADDRESS_TYPE.STICKY) |
311 | + # Make sure we know when and where the default DNS server will be used. |
312 | + get_default_dns_servers_mock = self.patch( |
313 | + node, 'get_default_dns_servers') |
314 | + get_default_dns_servers_mock.return_value = ['127.0.0.2'] |
315 | + netplan = self._render_netplan_dict(node) |
316 | + expected_netplan = { |
317 | + 'network': { |
318 | + 'version': 2, |
319 | + 'ethernets': { |
320 | + 'eth0': { |
321 | + 'gateway': '10.0.0.1', |
322 | + 'nameservers': { |
323 | + 'addresses': ['10.0.0.2'] |
324 | + }, |
325 | + 'match': {'macaddress': '00:01:02:03:04:05'}, |
326 | + 'mtu': 1500, |
327 | + 'set-name': 'eth0', |
328 | + 'addresses': ['10.0.0.4/24'], |
329 | + }, |
330 | + 'eth1': { |
331 | + 'match': {'macaddress': '02:01:02:03:04:05'}, |
332 | + 'nameservers': { |
333 | + 'addresses': ['10.0.1.2'] |
334 | + }, |
335 | + 'mtu': 1500, |
336 | + 'set-name': 'eth1', |
337 | + 'addresses': ['10.0.1.4/24'], |
338 | + }, |
339 | + }, |
340 | + }, |
341 | + } |
342 | + self.expectThat(netplan, Equals(expected_netplan)) |
343 | + v1 = self._render_v1_dict(node) |
344 | + expected_v1 = { |
345 | + 'network': { |
346 | + 'version': 1, |
347 | + 'config': [ |
348 | + { |
349 | + 'id': 'eth0', |
350 | + 'mac_address': '00:01:02:03:04:05', |
351 | + 'mtu': 1500, |
352 | + 'name': 'eth0', |
353 | + 'subnets': [{ |
354 | + 'address': '10.0.0.4/24', |
355 | + 'dns_nameservers': ['10.0.0.2'], |
356 | + 'gateway': '10.0.0.1', |
357 | + 'type': 'static', |
358 | + }], |
359 | + 'type': 'physical' |
360 | + }, |
361 | + { |
362 | + 'id': 'eth1', |
363 | + 'mac_address': '02:01:02:03:04:05', |
364 | + 'mtu': 1500, |
365 | + 'name': 'eth1', |
366 | + 'subnets': [{ |
367 | + 'address': '10.0.1.4/24', |
368 | + 'dns_nameservers': ['10.0.1.2'], |
369 | + 'type': 'static', |
370 | + }], |
371 | + 'type': 'physical' |
372 | + }, |
373 | + { |
374 | + 'address': ['127.0.0.2'], |
375 | + 'search': ['maas'], |
376 | + 'type': 'nameserver' |
377 | + } |
378 | + ], |
379 | + } |
380 | + } |
381 | + self.expectThat(v1, Equals(expected_v1)) |
Self-approve backport.