Merge lp:~blake-rouse/maas/node-gateway-api into lp:~maas-committers/maas/trunk
- node-gateway-api
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 4267 |
Proposed branch: | lp:~blake-rouse/maas/node-gateway-api |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
789 lines (+590/-15) 7 files modified
src/maasserver/api/interfaces.py (+27/-0) src/maasserver/api/nodes.py (+26/-0) src/maasserver/api/tests/test_doc.py (+6/-4) src/maasserver/api/tests/test_interfaces.py (+64/-0) src/maasserver/api/tests/test_node.py (+43/-0) src/maasserver/forms_interface_link.py (+162/-10) src/maasserver/tests/test_forms_interface_link.py (+262/-1) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/node-gateway-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andres Rodriguez (community) | Approve | ||
Review via email: mp+271006@code.launchpad.net |
Commit message
Add API to set the default gateway for IPv4 and IPv6 on the node. Add API to clear the default gatway allowing MAAS to pick the best gateway. Includes drive-by fix for the InterfaceLinkForm and the new AUTO link type.
Description of the change
Blake Rouse (blake-rouse) wrote : | # |
Thanks for the review.
That might be a good addition we want to add, but I think in general this is way more flexible. This should allow anyone using MAAS to get there network configuration just right.
If we fell that is something that should be added to the subnet model, please file a bug and we can adjust the pick_best_
MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~blake-rouse/maas/node-gateway-api into lp:maas failed. Below is the output from the failed tests.
Ign http://
Ign http://
Hit http://
Ign http://
Hit http://
Hit http://
Get:1 http://
Hit http://
Get:2 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:3 http://
Get:4 http://
Get:5 http://
Get:6 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 1,365 kB in 3s (373 kB/s)
Reading package lists...
sudo DEBIAN_
--
Preview Diff
1 | === modified file 'src/maasserver/api/interfaces.py' |
2 | --- src/maasserver/api/interfaces.py 2015-09-15 03:14:28 +0000 |
3 | +++ src/maasserver/api/interfaces.py 2015-09-15 19:08:34 +0000 |
4 | @@ -30,6 +30,7 @@ |
5 | ) |
6 | from maasserver.forms_interface_link import ( |
7 | InterfaceLinkForm, |
8 | + InterfaceSetDefaultGatwayForm, |
9 | InterfaceUnlinkForm, |
10 | ) |
11 | from maasserver.models.interface import ( |
12 | @@ -268,6 +269,9 @@ |
13 | :param ip_address: IP address for the interface in subnet. Only used |
14 | when mode is STATIC. If not provided an IP address from subnet |
15 | will be auto selected. |
16 | + :param default_gateway: True sets the gateway IP address for the subnet |
17 | + as the default gateway for the node this interface belongs to. |
18 | + Option can only be used with the AUTO and STATIC modes. |
19 | |
20 | Mode definitions: |
21 | AUTO - Assign this interface a static IP address from the provided |
22 | @@ -311,6 +315,29 @@ |
23 | else: |
24 | raise MAASAPIValidationError(form.errors) |
25 | |
26 | + @operation(idempotent=False) |
27 | + def set_default_gateway(self, request, system_id, interface_id): |
28 | + """Set the node to use this interface as the default gateway. |
29 | + |
30 | + If this interface has more than one subnet with a gateway IP in the |
31 | + same IP address family then specifying the ID of the link on |
32 | + this interface is required. |
33 | + |
34 | + :param link_id: ID of the link on this interface to select the |
35 | + default gateway IP address from. |
36 | + |
37 | + Returns 400 if the interface has not AUTO or STATIC links. |
38 | + Returns 404 if the node or interface is not found. |
39 | + """ |
40 | + interface = Interface.objects.get_interface_or_404( |
41 | + system_id, interface_id, request.user, NODE_PERMISSION.ADMIN) |
42 | + form = InterfaceSetDefaultGatwayForm( |
43 | + instance=interface, data=request.data) |
44 | + if form.is_valid(): |
45 | + return form.save() |
46 | + else: |
47 | + raise MAASAPIValidationError(form.errors) |
48 | + |
49 | |
50 | class PhysicalInterfaceHandler(NodeInterfaceHandler): |
51 | """ |
52 | |
53 | === modified file 'src/maasserver/api/nodes.py' |
54 | --- src/maasserver/api/nodes.py 2015-09-15 13:25:33 +0000 |
55 | +++ src/maasserver/api/nodes.py 2015-09-15 19:08:34 +0000 |
56 | @@ -855,6 +855,32 @@ |
57 | storage_layout, e.message)) |
58 | return node |
59 | |
60 | + @operation(idempotent=False) |
61 | + def clear_default_gateways(self, request, system_id): |
62 | + """Clear any set default gateways on the node. |
63 | + |
64 | + This will clear both IPv4 and IPv6 gateways on the node. This will |
65 | + transition the logic of identifing the best gateway to MAAS. This logic |
66 | + is determined based the following criteria: |
67 | + |
68 | + 1. Managed subnets over unmanaged subnets. |
69 | + 2. Bond interfaces over physical interfaces. |
70 | + 3. Node's boot interface over all other interfaces except bonds. |
71 | + 4. Physical interfaces over VLAN interfaces. |
72 | + 5. Sticky IP links over user reserved IP links. |
73 | + 6. User reserved IP links over auto IP links. |
74 | + |
75 | + If the default gateways need to be specific for this node you can set |
76 | + which interface and subnet's gateway to use when this node is deployed |
77 | + with the `node-interfaces set-default-gateway` API. |
78 | + """ |
79 | + node = Node.nodes.get_node_or_404( |
80 | + system_id=system_id, user=request.user, perm=NODE_PERMISSION.ADMIN) |
81 | + node.gateway_link_ipv4 = None |
82 | + node.gateway_link_ipv6 = None |
83 | + node.save() |
84 | + return node |
85 | + |
86 | |
87 | def create_node(request): |
88 | """Service an http request to create a node. |
89 | |
90 | === modified file 'src/maasserver/api/tests/test_doc.py' |
91 | --- src/maasserver/api/tests/test_doc.py 2015-08-21 13:59:15 +0000 |
92 | +++ src/maasserver/api/tests/test_doc.py 2015-09-15 19:08:34 +0000 |
93 | @@ -317,10 +317,12 @@ |
94 | "restful=False", |
95 | "POST release_sticky_ip_address op=release_sticky_ip_address " |
96 | "restful=False", |
97 | - 'POST mark_fixed op=mark_fixed restful=False', |
98 | - 'POST mark_broken op=mark_broken restful=False', |
99 | - 'POST abort_operation op=abort_operation restful=False', |
100 | - 'POST set_storage_layout op=set_storage_layout restful=False', |
101 | + "POST mark_fixed op=mark_fixed restful=False", |
102 | + "POST mark_broken op=mark_broken restful=False", |
103 | + "POST abort_operation op=abort_operation restful=False", |
104 | + "POST set_storage_layout op=set_storage_layout restful=False", |
105 | + "POST clear_default_gateways op=clear_default_gateways " |
106 | + "restful=False", |
107 | } |
108 | observed_actions = { |
109 | "%(method)s %(name)s op=%(op)s restful=%(restful)s" % action |
110 | |
111 | === modified file 'src/maasserver/api/tests/test_interfaces.py' |
112 | --- src/maasserver/api/tests/test_interfaces.py 2015-09-09 18:35:17 +0000 |
113 | +++ src/maasserver/api/tests/test_interfaces.py 2015-09-15 19:08:34 +0000 |
114 | @@ -611,3 +611,67 @@ |
115 | }) |
116 | self.assertEqual( |
117 | httplib.FORBIDDEN, response.status_code, response.content) |
118 | + |
119 | + def test_set_default_gateway_sets_gateway_link_ipv4_on_node(self): |
120 | + # The form that is used is fully tested in test_forms_interface_link. |
121 | + # This just tests that the form is saved and the node link is created. |
122 | + self.become_admin() |
123 | + node = factory.make_Node(interface=True) |
124 | + interface = node.get_boot_interface() |
125 | + network = factory.make_ipv4_network() |
126 | + subnet = factory.make_Subnet( |
127 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
128 | + link_ip = factory.make_StaticIPAddress( |
129 | + alloc_type=IPADDRESS_TYPE.AUTO, ip="", |
130 | + subnet=subnet, interface=interface) |
131 | + uri = get_node_interface_uri(interface) |
132 | + response = self.client.post(uri, { |
133 | + "op": "set_default_gateway", |
134 | + "link_id": link_ip.id |
135 | + }) |
136 | + self.assertEqual(httplib.OK, response.status_code, response.content) |
137 | + self.assertEqual(link_ip, reload_object(node).gateway_link_ipv4) |
138 | + |
139 | + def test_set_default_gateway_sets_gateway_link_ipv6_on_node(self): |
140 | + # The form that is used is fully tested in test_forms_interface_link. |
141 | + # This just tests that the form is saved and the node link is created. |
142 | + self.become_admin() |
143 | + node = factory.make_Node(interface=True) |
144 | + interface = node.get_boot_interface() |
145 | + network = factory.make_ipv6_network() |
146 | + subnet = factory.make_Subnet( |
147 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
148 | + link_ip = factory.make_StaticIPAddress( |
149 | + alloc_type=IPADDRESS_TYPE.AUTO, ip="", |
150 | + subnet=subnet, interface=interface) |
151 | + uri = get_node_interface_uri(interface) |
152 | + response = self.client.post(uri, { |
153 | + "op": "set_default_gateway", |
154 | + "link_id": link_ip.id |
155 | + }) |
156 | + self.assertEqual(httplib.OK, response.status_code, response.content) |
157 | + self.assertEqual(link_ip, reload_object(node).gateway_link_ipv6) |
158 | + |
159 | + def test_set_default_gateway_raises_error(self): |
160 | + self.become_admin() |
161 | + node = factory.make_Node(interface=True) |
162 | + interface = node.get_boot_interface() |
163 | + uri = get_node_interface_uri(interface) |
164 | + response = self.client.post(uri, { |
165 | + "op": "set_default_gateway", |
166 | + }) |
167 | + self.assertEqual( |
168 | + httplib.BAD_REQUEST, response.status_code, response.content) |
169 | + self.assertEquals({ |
170 | + "__all__": ["This interface has no usable gateways."] |
171 | + }, json.loads(response.content)) |
172 | + |
173 | + def test_set_default_gateway_requries_admin(self): |
174 | + node = factory.make_Node(interface=True) |
175 | + interface = node.get_boot_interface() |
176 | + uri = get_node_interface_uri(interface) |
177 | + response = self.client.post(uri, { |
178 | + "op": "set_default_gateway", |
179 | + }) |
180 | + self.assertEqual( |
181 | + httplib.FORBIDDEN, response.status_code, response.content) |
182 | |
183 | === modified file 'src/maasserver/api/tests/test_node.py' |
184 | --- src/maasserver/api/tests/test_node.py 2015-09-08 21:59:19 +0000 |
185 | +++ src/maasserver/api/tests/test_node.py 2015-09-15 19:08:34 +0000 |
186 | @@ -1810,3 +1810,46 @@ |
187 | self.assertThat( |
188 | mock_set_storage_layout, |
189 | MockCalledOnceWith('flat', params=ANY, allow_fallback=False)) |
190 | + |
191 | + |
192 | +class TestClearDefaultGateways(APITestCase): |
193 | + |
194 | + def get_node_uri(self, node): |
195 | + """Get the API URI for `node`.""" |
196 | + return reverse('node_handler', args=[node.system_id]) |
197 | + |
198 | + def test__403_when_not_admin(self): |
199 | + node = factory.make_Node( |
200 | + owner=self.logged_in_user, status=NODE_STATUS.ALLOCATED) |
201 | + response = self.client.post( |
202 | + self.get_node_uri(node), {'op': 'clear_default_gateways'}) |
203 | + self.assertEquals( |
204 | + httplib.FORBIDDEN, response.status_code, response.content) |
205 | + |
206 | + def test__clears_default_gateways(self): |
207 | + self.become_admin() |
208 | + node = factory.make_Node( |
209 | + owner=self.logged_in_user, status=NODE_STATUS.ALLOCATED) |
210 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node) |
211 | + network_v4 = factory.make_ipv4_network() |
212 | + subnet_v4 = factory.make_Subnet( |
213 | + cidr=unicode(network_v4.cidr), vlan=interface.vlan) |
214 | + link_v4 = factory.make_StaticIPAddress( |
215 | + alloc_type=IPADDRESS_TYPE.AUTO, ip="", |
216 | + subnet=subnet_v4, interface=interface) |
217 | + node.gateway_link_ipv4 = link_v4 |
218 | + network_v6 = factory.make_ipv6_network() |
219 | + subnet_v6 = factory.make_Subnet( |
220 | + cidr=unicode(network_v6.cidr), vlan=interface.vlan) |
221 | + link_v6 = factory.make_StaticIPAddress( |
222 | + alloc_type=IPADDRESS_TYPE.AUTO, ip="", |
223 | + subnet=subnet_v6, interface=interface) |
224 | + node.gateway_link_ipv6 = link_v6 |
225 | + node.save() |
226 | + response = self.client.post( |
227 | + self.get_node_uri(node), {'op': 'clear_default_gateways'}) |
228 | + self.assertEquals( |
229 | + httplib.OK, response.status_code, response.content) |
230 | + node = reload_object(node) |
231 | + self.assertIsNone(node.gateway_link_ipv4) |
232 | + self.assertIsNone(node.gateway_link_ipv6) |
233 | |
234 | === modified file 'src/maasserver/forms_interface_link.py' |
235 | --- src/maasserver/forms_interface_link.py 2015-09-15 14:15:07 +0000 |
236 | +++ src/maasserver/forms_interface_link.py 2015-09-15 19:08:34 +0000 |
237 | @@ -16,14 +16,19 @@ |
238 | "InterfaceLinkForm", |
239 | ] |
240 | |
241 | +from collections import Counter |
242 | + |
243 | from django import forms |
244 | +from django.core.exceptions import ValidationError |
245 | from maasserver.enum import ( |
246 | INTERFACE_LINK_TYPE, |
247 | INTERFACE_LINK_TYPE_CHOICES, |
248 | + IPADDRESS_FAMILY, |
249 | IPADDRESS_TYPE, |
250 | ) |
251 | from maasserver.fields import CaseInsensitiveChoiceField |
252 | from maasserver.models.interface import Interface |
253 | +from maasserver.models.staticipaddress import StaticIPAddress |
254 | from maasserver.utils.forms import ( |
255 | compose_invalid_choice_text, |
256 | set_form_error, |
257 | @@ -31,6 +36,12 @@ |
258 | from maasserver.utils.orm import get_one |
259 | from netaddr import IPAddress |
260 | |
261 | +# Link modes that support the default_gateway option. |
262 | +GATEWAY_OPTION_MODES = [ |
263 | + INTERFACE_LINK_TYPE.AUTO, |
264 | + INTERFACE_LINK_TYPE.STATIC, |
265 | +] |
266 | + |
267 | |
268 | class InterfaceLinkForm(forms.Form): |
269 | """Interface link form.""" |
270 | @@ -46,6 +57,8 @@ |
271 | |
272 | ip_address = forms.GenericIPAddressField(required=False) |
273 | |
274 | + default_gateway = forms.BooleanField(initial=False, required=False) |
275 | + |
276 | def __init__(self, *args, **kwargs): |
277 | self.instance = kwargs.pop("instance") |
278 | super(InterfaceLinkForm, self).__init__(*args, **kwargs) |
279 | @@ -54,14 +67,24 @@ |
280 | def clean(self): |
281 | cleaned_data = super(InterfaceLinkForm, self).clean() |
282 | mode = cleaned_data.get("mode", None) |
283 | - if mode == INTERFACE_LINK_TYPE.DHCP: |
284 | + if mode is None: |
285 | + return cleaned_data |
286 | + elif mode == INTERFACE_LINK_TYPE.AUTO: |
287 | + self._clean_mode_auto(cleaned_data) |
288 | + elif mode == INTERFACE_LINK_TYPE.DHCP: |
289 | self._clean_mode_dhcp() |
290 | elif mode == INTERFACE_LINK_TYPE.STATIC: |
291 | self._clean_mode_static(cleaned_data) |
292 | elif mode == INTERFACE_LINK_TYPE.LINK_UP: |
293 | self._clean_mode_link_up() |
294 | + self._clean_default_gateway(cleaned_data) |
295 | return cleaned_data |
296 | |
297 | + def _clean_mode_auto(self, cleaned_data): |
298 | + subnet = cleaned_data.get("subnet", None) |
299 | + if subnet is None: |
300 | + set_form_error(self, "subnet", "This field is required.") |
301 | + |
302 | def _clean_mode_dhcp(self): |
303 | # Can only have one DHCP link on an interface. |
304 | dhcp_address = get_one( |
305 | @@ -102,29 +125,55 @@ |
306 | if self.instance.ip_addresses.count() > 0: |
307 | set_form_error( |
308 | self, "mode", |
309 | - "Cannot configure interface to link up (with no IP address)" |
310 | + "Cannot configure interface to link up (with no IP address) " |
311 | "while other links are already configured.") |
312 | |
313 | + def _clean_default_gateway(self, cleaned_data): |
314 | + mode = cleaned_data.get("mode", None) |
315 | + subnet = cleaned_data.get("subnet", None) |
316 | + default_gateway = cleaned_data.get("default_gateway", False) |
317 | + if not default_gateway: |
318 | + return |
319 | + if mode not in GATEWAY_OPTION_MODES: |
320 | + set_form_error( |
321 | + self, "default_gateway", "Cannot use in mode '%s'." % mode) |
322 | + else: |
323 | + if subnet is None: |
324 | + set_form_error( |
325 | + self, "default_gateway", |
326 | + "Subnet is required when default_gateway is True.") |
327 | + elif not subnet.gateway_ip: |
328 | + set_form_error( |
329 | + self, "default_gateway", |
330 | + "Cannot set as default gateway because subnet " |
331 | + "%s doesn't provide a gateway IP address." % subnet) |
332 | + |
333 | def save(self): |
334 | mode = self.cleaned_data.get("mode", None) |
335 | subnet = self.cleaned_data.get("subnet", None) |
336 | ip_address = self.cleaned_data.get("ip_address", None) |
337 | + default_gateway = self.cleaned_data.get("default_gateway", False) |
338 | if not ip_address: |
339 | ip_address = None |
340 | - self.instance.link_subnet(mode, subnet, ip_address=ip_address) |
341 | + link_ip = self.instance.link_subnet( |
342 | + mode, subnet, ip_address=ip_address) |
343 | + if default_gateway: |
344 | + node = self.instance.get_node() |
345 | + network = subnet.get_ipnetwork() |
346 | + if network.version == IPADDRESS_FAMILY.IPv4: |
347 | + node.gateway_link_ipv4 = link_ip |
348 | + elif network.version == IPADDRESS_FAMILY.IPv6: |
349 | + node.gateway_link_ipv6 = link_ip |
350 | + else: |
351 | + raise ValueError( |
352 | + "Unknown subnet IP version: %s" % network.version) |
353 | + node.save() |
354 | return Interface.objects.get(id=self.instance.id) |
355 | |
356 | |
357 | class InterfaceUnlinkForm(forms.Form): |
358 | """Interface unlink form.""" |
359 | |
360 | - id = forms.ChoiceField( |
361 | - choices=INTERFACE_LINK_TYPE_CHOICES, required=True, |
362 | - error_messages={ |
363 | - 'invalid_choice': compose_invalid_choice_text( |
364 | - 'mode', INTERFACE_LINK_TYPE_CHOICES), |
365 | - }) |
366 | - |
367 | def __init__(self, *args, **kwargs): |
368 | self.instance = kwargs.pop("instance") |
369 | super(InterfaceUnlinkForm, self).__init__(*args, **kwargs) |
370 | @@ -148,3 +197,106 @@ |
371 | link_id = self.cleaned_data.get("id", None) |
372 | self.instance.unlink_subnet_by_id(link_id) |
373 | return Interface.objects.get(id=self.instance.id) |
374 | + |
375 | + |
376 | +class InterfaceSetDefaultGatwayForm(forms.Form): |
377 | + """Interface set default gateway form.""" |
378 | + |
379 | + def __init__(self, *args, **kwargs): |
380 | + self.instance = kwargs.pop("instance") |
381 | + super(InterfaceSetDefaultGatwayForm, self).__init__(*args, **kwargs) |
382 | + self.links = self.get_valid_links() |
383 | + self.set_up_link_id_field() |
384 | + |
385 | + def get_valid_links(self): |
386 | + """Return IP links on the instance that are of the correct type, |
387 | + have a subnet, and has a gateway_ip set.""" |
388 | + links = self.instance.ip_addresses.filter( |
389 | + alloc_type__in=[IPADDRESS_TYPE.AUTO, IPADDRESS_TYPE.STICKY], |
390 | + subnet__isnull=False, subnet__gateway_ip__isnull=False) |
391 | + links = links.select_related("subnet") |
392 | + return [ |
393 | + link |
394 | + for link in links.all() |
395 | + if link.subnet.gateway_ip |
396 | + ] |
397 | + |
398 | + def set_up_link_id_field(self): |
399 | + link_choices = [ |
400 | + (link.id, link.id) |
401 | + for link in self.links |
402 | + ] |
403 | + invalid_choice = compose_invalid_choice_text('link_id', link_choices) |
404 | + self.fields["link_id"] = forms.ChoiceField( |
405 | + choices=link_choices, required=False, |
406 | + error_messages={ |
407 | + 'invalid_choice': invalid_choice, |
408 | + }) |
409 | + |
410 | + def _clean_has_gateways(self): |
411 | + """Sets error if the interface has not available gateways.""" |
412 | + if len(self.links) == 0: |
413 | + raise ValidationError("This interface has no usable gateways.") |
414 | + |
415 | + def _clean_ipv4_and_ipv6_gateways(self): |
416 | + """Sets error if the interface doesn't have only one IPv4 and one |
417 | + IPv6 gateway.""" |
418 | + unique_gateways = set( |
419 | + link.subnet.gateway_ip |
420 | + for link in self.links |
421 | + ) |
422 | + gateway_versions = Counter( |
423 | + IPAddress(gateway).version |
424 | + for gateway in unique_gateways |
425 | + ) |
426 | + too_many = [ |
427 | + ip_family |
428 | + for ip_family, count in gateway_versions.items() |
429 | + if count > 1 |
430 | + ] |
431 | + if len(too_many) > 0: |
432 | + set_form_error( |
433 | + self, "link_id", |
434 | + "This field is required; Interface has more than " |
435 | + "one usable %s gateway%s." % ( |
436 | + ' and '.join( |
437 | + map(lambda version: "IPv%d" % version, too_many)), |
438 | + "s" if len(too_many) > 1 else "")) |
439 | + |
440 | + def clean(self): |
441 | + self._clean_has_gateways() |
442 | + cleaned_data = super(InterfaceSetDefaultGatwayForm, self).clean() |
443 | + link_id = cleaned_data.get("link_id", None) |
444 | + if not link_id: |
445 | + self._clean_ipv4_and_ipv6_gateways() |
446 | + return cleaned_data |
447 | + |
448 | + def get_first_link_by_ip_family(self, ip_family): |
449 | + for link in self.links: |
450 | + if link.subnet.get_ipnetwork().version == ip_family: |
451 | + return link |
452 | + return None |
453 | + |
454 | + def save(self): |
455 | + link_id = self.cleaned_data.get("link_id", None) |
456 | + node = self.instance.get_node() |
457 | + if link_id: |
458 | + link = StaticIPAddress.objects.get(id=int(link_id)) |
459 | + network = link.subnet.get_ipnetwork() |
460 | + if network.version == IPADDRESS_FAMILY.IPv4: |
461 | + node.gateway_link_ipv4 = link |
462 | + elif network.version == IPADDRESS_FAMILY.IPv6: |
463 | + node.gateway_link_ipv6 = link |
464 | + else: |
465 | + raise ValueError( |
466 | + "Unknown subnet IP version: %s" % network.version) |
467 | + node.save() |
468 | + else: |
469 | + ipv4_link = self.get_first_link_by_ip_family(IPADDRESS_FAMILY.IPv4) |
470 | + if ipv4_link is not None: |
471 | + node.gateway_link_ipv4 = ipv4_link |
472 | + ipv6_link = self.get_first_link_by_ip_family(IPADDRESS_FAMILY.IPv6) |
473 | + if ipv6_link is not None: |
474 | + node.gateway_link_ipv6 = ipv6_link |
475 | + node.save() |
476 | + return self.instance |
477 | |
478 | === modified file 'src/maasserver/tests/test_forms_interface_link.py' |
479 | --- src/maasserver/tests/test_forms_interface_link.py 2015-09-15 14:15:07 +0000 |
480 | +++ src/maasserver/tests/test_forms_interface_link.py 2015-09-15 19:08:34 +0000 |
481 | @@ -25,6 +25,7 @@ |
482 | ) |
483 | from maasserver.forms_interface_link import ( |
484 | InterfaceLinkForm, |
485 | + InterfaceSetDefaultGatwayForm, |
486 | InterfaceUnlinkForm, |
487 | ) |
488 | from maasserver.models import interface as interface_module |
489 | @@ -61,6 +62,90 @@ |
490 | form = InterfaceLinkForm(instance=interface, data={}) |
491 | self.assertItemsEqual(subnets, form.fields["subnet"].queryset) |
492 | |
493 | + def test__AUTO_requires_subnet(self): |
494 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
495 | + form = InterfaceLinkForm(instance=interface, data={ |
496 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
497 | + }) |
498 | + self.assertFalse(form.is_valid(), form.errors) |
499 | + self.assertEquals({ |
500 | + "subnet": ["This field is required."], |
501 | + }, form.errors) |
502 | + |
503 | + def test__AUTO_creates_link_to_AUTO_with_subnet(self): |
504 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
505 | + auto_subnet = factory.make_Subnet(vlan=interface.vlan) |
506 | + form = InterfaceLinkForm(instance=interface, data={ |
507 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
508 | + "subnet": auto_subnet.id, |
509 | + }) |
510 | + self.assertTrue(form.is_valid(), form.errors) |
511 | + interface = form.save() |
512 | + auto_ip = interface.ip_addresses.get(alloc_type=IPADDRESS_TYPE.AUTO) |
513 | + self.assertEquals(auto_subnet, auto_ip.subnet) |
514 | + |
515 | + def test__AUTO_sets_node_gateway_link_v4(self): |
516 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
517 | + network = factory.make_ipv4_network() |
518 | + auto_subnet = factory.make_Subnet( |
519 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
520 | + form = InterfaceLinkForm(instance=interface, data={ |
521 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
522 | + "subnet": auto_subnet.id, |
523 | + "default_gateway": True, |
524 | + }) |
525 | + self.assertTrue(form.is_valid(), form.errors) |
526 | + interface = form.save() |
527 | + auto_ip = interface.ip_addresses.get(alloc_type=IPADDRESS_TYPE.AUTO) |
528 | + node = interface.get_node() |
529 | + self.assertEquals(auto_ip, node.gateway_link_ipv4) |
530 | + |
531 | + def test__AUTO_sets_node_gateway_link_v6(self): |
532 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
533 | + network = factory.make_ipv6_network() |
534 | + auto_subnet = factory.make_Subnet( |
535 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
536 | + form = InterfaceLinkForm(instance=interface, data={ |
537 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
538 | + "subnet": auto_subnet.id, |
539 | + "default_gateway": True, |
540 | + }) |
541 | + self.assertTrue(form.is_valid(), form.errors) |
542 | + interface = form.save() |
543 | + auto_ip = interface.ip_addresses.get(alloc_type=IPADDRESS_TYPE.AUTO) |
544 | + node = interface.get_node() |
545 | + self.assertEquals(auto_ip, node.gateway_link_ipv6) |
546 | + |
547 | + def test__AUTO_default_gateway_requires_subnet(self): |
548 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
549 | + form = InterfaceLinkForm(instance=interface, data={ |
550 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
551 | + "default_gateway": True, |
552 | + }) |
553 | + self.assertFalse(form.is_valid(), form.errors) |
554 | + self.assertEquals({ |
555 | + "default_gateway": [ |
556 | + "Subnet is required when default_gateway is True."], |
557 | + "subnet": ["This field is required."], |
558 | + }, form.errors) |
559 | + |
560 | + def test__AUTO_default_gateway_requires_subnet_with_gateway_ip(self): |
561 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
562 | + auto_subnet = factory.make_Subnet(vlan=interface.vlan) |
563 | + auto_subnet.gateway_ip = None |
564 | + auto_subnet.save() |
565 | + form = InterfaceLinkForm(instance=interface, data={ |
566 | + "mode": INTERFACE_LINK_TYPE.AUTO, |
567 | + "subnet": auto_subnet.id, |
568 | + "default_gateway": True, |
569 | + }) |
570 | + self.assertFalse(form.is_valid(), form.errors) |
571 | + self.assertEquals({ |
572 | + "default_gateway": [ |
573 | + "Cannot set as default gateway because subnet " |
574 | + "%s doesn't provide a gateway IP address." % auto_subnet], |
575 | + }, form.errors) |
576 | + |
577 | def test__DHCP_not_allowed_if_already_DHCP_with_subnet(self): |
578 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
579 | dhcp_subnet = factory.make_Subnet() |
580 | @@ -92,6 +177,18 @@ |
581 | "Interface is already set to DHCP."] |
582 | }, form.errors) |
583 | |
584 | + def test__DHCP_not_allowed_default_gateway(self): |
585 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
586 | + form = InterfaceLinkForm(instance=interface, data={ |
587 | + "mode": INTERFACE_LINK_TYPE.DHCP, |
588 | + "default_gateway": True, |
589 | + }) |
590 | + self.assertFalse(form.is_valid(), form.errors) |
591 | + self.assertEquals({ |
592 | + "default_gateway": [ |
593 | + "Cannot use in mode '%s'." % (INTERFACE_LINK_TYPE.DHCP)] |
594 | + }, form.errors) |
595 | + |
596 | def test__DHCP_creates_link_to_DHCP_with_subnet(self): |
597 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
598 | dhcp_subnet = factory.make_Subnet(vlan=interface.vlan) |
599 | @@ -237,6 +334,54 @@ |
600 | self.assertIsNotNone(ip_address) |
601 | self.assertIn(IPAddress(ip_address.ip), ngi.get_static_ip_range()) |
602 | |
603 | + def test__STATIC_sets_node_gateway_link_ipv4(self): |
604 | + # Silence update_host_maps. |
605 | + self.patch_autospec(interface_module, "update_host_maps") |
606 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
607 | + network = factory.make_ipv4_network() |
608 | + subnet = factory.make_Subnet( |
609 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
610 | + nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED) |
611 | + factory.make_NodeGroupInterface( |
612 | + nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP, |
613 | + subnet=subnet) |
614 | + form = InterfaceLinkForm(instance=interface, data={ |
615 | + "mode": INTERFACE_LINK_TYPE.STATIC, |
616 | + "subnet": subnet.id, |
617 | + "default_gateway": True, |
618 | + }) |
619 | + self.assertTrue(form.is_valid(), form.errors) |
620 | + interface = form.save() |
621 | + ip_address = get_one( |
622 | + interface.ip_addresses.filter( |
623 | + alloc_type=IPADDRESS_TYPE.STICKY, subnet=subnet)) |
624 | + node = interface.get_node() |
625 | + self.assertEquals(ip_address, node.gateway_link_ipv4) |
626 | + |
627 | + def test__STATIC_sets_node_gateway_link_ipv6(self): |
628 | + # Silence update_host_maps. |
629 | + self.patch_autospec(interface_module, "update_host_maps") |
630 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
631 | + network = factory.make_ipv6_network() |
632 | + subnet = factory.make_Subnet( |
633 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
634 | + nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED) |
635 | + factory.make_NodeGroupInterface( |
636 | + nodegroup, management=NODEGROUPINTERFACE_MANAGEMENT.DHCP, |
637 | + subnet=subnet) |
638 | + form = InterfaceLinkForm(instance=interface, data={ |
639 | + "mode": INTERFACE_LINK_TYPE.STATIC, |
640 | + "subnet": subnet.id, |
641 | + "default_gateway": True, |
642 | + }) |
643 | + self.assertTrue(form.is_valid(), form.errors) |
644 | + interface = form.save() |
645 | + ip_address = get_one( |
646 | + interface.ip_addresses.filter( |
647 | + alloc_type=IPADDRESS_TYPE.STICKY, subnet=subnet)) |
648 | + node = interface.get_node() |
649 | + self.assertEquals(ip_address, node.gateway_link_ipv6) |
650 | + |
651 | def test__LINK_UP_not_allowed_with_other_ip_addresses(self): |
652 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
653 | factory.make_StaticIPAddress( |
654 | @@ -247,7 +392,7 @@ |
655 | self.assertFalse(form.is_valid(), form.errors) |
656 | self.assertEquals({ |
657 | "mode": [ |
658 | - "Cannot configure interface to link up (with no IP address)" |
659 | + "Cannot configure interface to link up (with no IP address) " |
660 | "while other links are already configured."] |
661 | }, form.errors) |
662 | |
663 | @@ -276,6 +421,18 @@ |
664 | self.assertIsNotNone(link_ip) |
665 | self.assertIsNone(link_ip.ip) |
666 | |
667 | + def test__LINK_UP_not_allowed_default_gateway(self): |
668 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
669 | + form = InterfaceLinkForm(instance=interface, data={ |
670 | + "mode": INTERFACE_LINK_TYPE.LINK_UP, |
671 | + "default_gateway": True, |
672 | + }) |
673 | + self.assertFalse(form.is_valid(), form.errors) |
674 | + self.assertEquals({ |
675 | + "default_gateway": [ |
676 | + "Cannot use in mode '%s'." % (INTERFACE_LINK_TYPE.LINK_UP)] |
677 | + }, form.errors) |
678 | + |
679 | |
680 | class TestInterfaceUnlinkForm(MAASServerTestCase): |
681 | |
682 | @@ -382,3 +539,107 @@ |
683 | self.assertTrue(form.is_valid(), form.errors) |
684 | form.save() |
685 | self.assertIsNone(reload_object(link_ip)) |
686 | + |
687 | + |
688 | +class TestInterfaceSetDefaultGatwayForm(MAASServerTestCase): |
689 | + |
690 | + def make_ip_family_link( |
691 | + self, interface, network, alloc_type=IPADDRESS_TYPE.STICKY): |
692 | + subnet = factory.make_Subnet( |
693 | + cidr=unicode(network.cidr), vlan=interface.vlan) |
694 | + if alloc_type == IPADDRESS_TYPE.STICKY: |
695 | + ip = factory.pick_ip_in_network(network) |
696 | + else: |
697 | + ip = "" |
698 | + return factory.make_StaticIPAddress( |
699 | + alloc_type=alloc_type, ip=ip, subnet=subnet, interface=interface) |
700 | + |
701 | + def test__interface_needs_gateways(self): |
702 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
703 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={}) |
704 | + self.assertFalse(form.is_valid(), form.errors) |
705 | + self.assertEquals({ |
706 | + "__all__": ["This interface has no usable gateways."], |
707 | + }, form.errors) |
708 | + |
709 | + def test__doesnt_require_link_id_if_only_one_gateway_per_family(self): |
710 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
711 | + self.make_ip_family_link(interface, factory.make_ipv4_network()) |
712 | + self.make_ip_family_link(interface, factory.make_ipv6_network()) |
713 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={}) |
714 | + self.assertTrue(form.is_valid(), form.errors) |
715 | + |
716 | + def test__requires_link_id_if_more_than_one_gateway_per_family(self): |
717 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
718 | + self.make_ip_family_link(interface, factory.make_ipv4_network()) |
719 | + self.make_ip_family_link(interface, factory.make_ipv6_network()) |
720 | + self.make_ip_family_link(interface, factory.make_ipv4_network()) |
721 | + self.make_ip_family_link(interface, factory.make_ipv6_network()) |
722 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={}) |
723 | + self.assertFalse(form.is_valid(), form.errors) |
724 | + self.assertEquals({ |
725 | + "link_id": [ |
726 | + "This field is required; Interface has more than one " |
727 | + "usable IPv4 and IPv6 gateways."], |
728 | + }, form.errors) |
729 | + |
730 | + def test__link_id_fields_setup_correctly(self): |
731 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
732 | + links = [] |
733 | + for _ in range(2): |
734 | + links.append( |
735 | + self.make_ip_family_link( |
736 | + interface, factory.make_ipv4_network())) |
737 | + for _ in range(2): |
738 | + links.append( |
739 | + self.make_ip_family_link( |
740 | + interface, factory.make_ipv6_network())) |
741 | + link_ids = [ |
742 | + link.id |
743 | + for link in links |
744 | + ] |
745 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={}) |
746 | + choice_ids = [ |
747 | + choice[0] |
748 | + for choice in form.fields["link_id"].choices |
749 | + ] |
750 | + self.assertItemsEqual(link_ids, choice_ids) |
751 | + |
752 | + def test__sets_gateway_links_on_node_when_no_link_id(self): |
753 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
754 | + ipv4_link = self.make_ip_family_link( |
755 | + interface, factory.make_ipv4_network()) |
756 | + ipv6_link = self.make_ip_family_link( |
757 | + interface, factory.make_ipv6_network()) |
758 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={}) |
759 | + self.assertTrue(form.is_valid(), form.errors) |
760 | + interface = form.save() |
761 | + node = interface.get_node() |
762 | + self.assertEquals(ipv4_link, node.gateway_link_ipv4) |
763 | + self.assertEquals(ipv6_link, node.gateway_link_ipv6) |
764 | + |
765 | + def test__sets_gateway_link_v4_on_node_when_link_id(self): |
766 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
767 | + ipv4_link = self.make_ip_family_link( |
768 | + interface, factory.make_ipv4_network()) |
769 | + self.make_ip_family_link(interface, factory.make_ipv4_network()) |
770 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={ |
771 | + "link_id": ipv4_link.id, |
772 | + }) |
773 | + self.assertTrue(form.is_valid(), form.errors) |
774 | + interface = form.save() |
775 | + node = interface.get_node() |
776 | + self.assertEquals(ipv4_link, node.gateway_link_ipv4) |
777 | + |
778 | + def test__sets_gateway_link_v6_on_node_when_link_id(self): |
779 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
780 | + ipv6_link = self.make_ip_family_link( |
781 | + interface, factory.make_ipv6_network()) |
782 | + self.make_ip_family_link(interface, factory.make_ipv6_network()) |
783 | + form = InterfaceSetDefaultGatwayForm(instance=interface, data={ |
784 | + "link_id": ipv6_link.id, |
785 | + }) |
786 | + self.assertTrue(form.is_valid(), form.errors) |
787 | + interface = form.save() |
788 | + node = interface.get_node() |
789 | + self.assertEquals(ipv6_link, node.gateway_link_ipv6) |
lgtm! However, the only question / suggestion I have... and I think this is something we've kind of discussed in the past:
Wouldn't it be better to simply say "Subnetwork A, is the Internet" (or the one that provides upstream connectivity), and that would simply automatically mean that the interface with the internet subnet would be the one with the default gateway?