Merge lp:~blake-rouse/maas/nic-no-fabric into lp:~maas-committers/maas/trunk
- nic-no-fabric
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 5205 |
Proposed branch: | lp:~blake-rouse/maas/nic-no-fabric |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
858 lines (+389/-69) 16 files modified
src/maasserver/api/interfaces.py (+8/-4) src/maasserver/api/tests/test_interfaces.py (+2/-3) src/maasserver/forms_interface.py (+22/-9) src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py (+1/-1) src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py (+23/-0) src/maasserver/models/interface.py (+27/-16) src/maasserver/models/node.py (+4/-0) src/maasserver/models/signals/interfaces.py (+18/-15) src/maasserver/models/signals/tests/test_interfaces.py (+22/-0) src/maasserver/models/tests/test_interface.py (+31/-0) src/maasserver/static/js/angular/controllers/node_details_networking.js (+13/-3) src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js (+67/-0) src/maasserver/static/partials/node-details.html (+4/-1) src/maasserver/testing/factory.py (+16/-15) src/maasserver/tests/test_forms_interface.py (+129/-1) src/maasserver/websockets/handlers/node.py (+2/-1) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/nic-no-fabric |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Review via email: mp+300381@code.launchpad.net |
Commit message
Add the ability for an interface to be disconnected (not connected to any VLAN).
Description of the change
MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~blake-rouse/maas/nic-no-fabric into lp:maas failed. Below is the output from the failed tests.
Hit:1 http://
Hit:2 http://
Get:3 http://
Hit:4 http://
Get:5 http://
Fetched 395 kB in 0s (795 kB/s)
Reading package lists...
sudo DEBIAN_
--no-
Reading package lists...
Building dependency tree...
Reading state information...
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
build-essential is already the newest version (12.1ubuntu2).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubun
distro-info is already the newest version (0.14build1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libjs-yui3-full is already the newest version (3.5.1-1ubuntu3).
libjs-yui3-min is already the newest version (3.5.1-1ubuntu3).
make is already the newest version (4.1-6).
postgresql is already the newest version (9.5+173).
pxelinux is already the newest version (3:6.03+
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5....
Preview Diff
1 | === modified file 'src/maasserver/api/interfaces.py' |
2 | --- src/maasserver/api/interfaces.py 2016-05-12 19:07:37 +0000 |
3 | +++ src/maasserver/api/interfaces.py 2016-07-26 10:11:30 +0000 |
4 | @@ -109,7 +109,8 @@ |
5 | :param name: Name of the interface. |
6 | :param mac_address: MAC address of the interface. |
7 | :param tags: Tags for the interface. |
8 | - :param vlan: Untagged VLAN the interface is connected to. |
9 | + :param vlan: Untagged VLAN the interface is connected to. If not |
10 | + provided then the interface is considered disconnected. |
11 | |
12 | Following are extra parameters that can be set on the interface: |
13 | |
14 | @@ -151,7 +152,8 @@ |
15 | :param name: Name of the interface. |
16 | :param mac_address: MAC address of the interface. |
17 | :param tags: Tags for the interface. |
18 | - :param vlan: VLAN the interface is connected to. |
19 | + :param vlan: VLAN the interface is connected to. If not |
20 | + provided then the interface is considered disconnected. |
21 | :param parents: Parent interfaces that make this bond. |
22 | |
23 | Following are parameters specific to bonds: |
24 | @@ -318,13 +320,15 @@ |
25 | :param name: Name of the interface. |
26 | :param mac_address: MAC address of the interface. |
27 | :param tags: Tags for the interface. |
28 | - :param vlan: Untagged VLAN the interface is connected to. |
29 | + :param vlan: Untagged VLAN the interface is connected to. If not set |
30 | + then the interface is considered disconnected. |
31 | |
32 | Fields for bond interface: |
33 | :param name: Name of the interface. |
34 | :param mac_address: MAC address of the interface. |
35 | :param tags: Tags for the interface. |
36 | - :param vlan: Tagged VLAN the interface is connected to. |
37 | + :param vlan: Tagged VLAN the interface is connected to. If not set |
38 | + then the interface is considered disconnected. |
39 | :param parents: Parent interfaces that make this bond. |
40 | |
41 | Fields for VLAN interface: |
42 | |
43 | === modified file 'src/maasserver/api/tests/test_interfaces.py' |
44 | --- src/maasserver/api/tests/test_interfaces.py 2016-05-26 13:28:46 +0000 |
45 | +++ src/maasserver/api/tests/test_interfaces.py 2016-07-26 10:11:30 +0000 |
46 | @@ -257,7 +257,7 @@ |
47 | self.assertEqual( |
48 | http.client.CONFLICT, response.status_code, response.content) |
49 | |
50 | - def test_create_physical_requires_mac_name_and_vlan(self): |
51 | + def test_create_physical_requires_mac_and_name(self): |
52 | self.become_admin() |
53 | node = factory.make_Node(status=NODE_STATUS.READY) |
54 | uri = get_interfaces_uri(node) |
55 | @@ -269,7 +269,6 @@ |
56 | self.assertEqual({ |
57 | "mac_address": ["This field is required."], |
58 | "name": ["This field is required."], |
59 | - "vlan": ["This field is required."], |
60 | }, json_load_bytes(response.content)) |
61 | |
62 | def test_create_physical_doesnt_allow_mac_already_register(self): |
63 | @@ -490,7 +489,7 @@ |
64 | self.assertEqual( |
65 | http.client.BAD_REQUEST, response.status_code, response.content) |
66 | self.assertEqual({ |
67 | - "vlan": ["This field is required."], |
68 | + "vlan": ["A VLAN interface must be connected to a tagged VLAN."], |
69 | "parent": ["A VLAN interface must have exactly one parent."], |
70 | }, json_load_bytes(response.content)) |
71 | |
72 | |
73 | === modified file 'src/maasserver/forms_interface.py' |
74 | --- src/maasserver/forms_interface.py 2016-04-22 17:28:15 +0000 |
75 | +++ src/maasserver/forms_interface.py 2016-07-26 10:11:30 +0000 |
76 | @@ -99,6 +99,11 @@ |
77 | rel = interface.parent_relationships.filter( |
78 | parent=parent_to_del) |
79 | rel.delete() |
80 | + # Allow setting the VLAN to None. |
81 | + new_vlan = self.cleaned_data.get('vlan') |
82 | + vlan_was_set = 'vlan' in self.data |
83 | + if new_vlan is None and vlan_was_set: |
84 | + interface.vlan = new_vlan |
85 | self.set_extra_parameters(interface, created) |
86 | interface.save() |
87 | if created: |
88 | @@ -168,6 +173,17 @@ |
89 | 'vlan', |
90 | ) |
91 | |
92 | + def save(self, *args, **kwargs): |
93 | + """Persist the interface into the database.""" |
94 | + interface = super(ControllerInterfaceForm, self).save(commit=False) |
95 | + # Allow setting the VLAN to None. |
96 | + new_vlan = self.cleaned_data.get('vlan') |
97 | + vlan_was_set = 'vlan' in self.data |
98 | + if new_vlan is None and vlan_was_set: |
99 | + interface.vlan = new_vlan |
100 | + interface.save() |
101 | + return interface |
102 | + |
103 | |
104 | class PhysicalInterfaceForm(InterfaceForm): |
105 | """Form used to create/edit a physical interface.""" |
106 | @@ -251,7 +267,13 @@ |
107 | return parents |
108 | |
109 | def clean_vlan(self): |
110 | + created = self.instance.id is None |
111 | new_vlan = self.cleaned_data.get('vlan') |
112 | + vlan_was_set = 'vlan' in self.data |
113 | + if (created and new_vlan is None) or ( |
114 | + not created and new_vlan is None and vlan_was_set): |
115 | + raise ValidationError( |
116 | + "A VLAN interface must be connected to a tagged VLAN.") |
117 | if new_vlan and new_vlan.fabric.get_default_vlan() == new_vlan: |
118 | raise ValidationError( |
119 | "A VLAN interface can only belong to a tagged VLAN.") |
120 | @@ -281,15 +303,6 @@ |
121 | bridges. |
122 | """ |
123 | |
124 | - def __init__(self, *args, **kwargs): |
125 | - super().__init__(*args, **kwargs) |
126 | - # Allow VLAN to be blank when creating. |
127 | - instance = kwargs.get("instance", None) |
128 | - if instance is not None and instance.id is not None: |
129 | - self.fields['vlan'].required = True |
130 | - else: |
131 | - self.fields['vlan'].required = False |
132 | - |
133 | def clean_parents(self): |
134 | """Validate that child interfaces cannot be created unless at least one |
135 | parent is present. |
136 | |
137 | === modified file 'src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py' |
138 | --- src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py 2016-05-11 19:01:48 +0000 |
139 | +++ src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py 2016-07-26 10:11:30 +0000 |
140 | @@ -39,7 +39,7 @@ |
141 | migrations.AlterField( |
142 | model_name='interface', |
143 | name='vlan', |
144 | - field=models.ForeignKey(to='maasserver.VLAN', default=maasserver.models.interface.get_default_vlan, on_delete=django.db.models.deletion.PROTECT), |
145 | + field=models.ForeignKey(to='maasserver.VLAN', default=None, on_delete=django.db.models.deletion.PROTECT), |
146 | ), |
147 | migrations.AlterField( |
148 | model_name='subnet', |
149 | |
150 | === added file 'src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py' |
151 | --- src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py 1970-01-01 00:00:00 +0000 |
152 | +++ src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py 2016-07-26 10:11:30 +0000 |
153 | @@ -0,0 +1,23 @@ |
154 | +# -*- coding: utf-8 -*- |
155 | +from __future__ import unicode_literals |
156 | + |
157 | +from django.db import ( |
158 | + migrations, |
159 | + models, |
160 | +) |
161 | +import django.db.models.deletion |
162 | + |
163 | + |
164 | +class Migration(migrations.Migration): |
165 | + |
166 | + dependencies = [ |
167 | + ('maasserver', '0069_add_previous_node_status_to_node'), |
168 | + ] |
169 | + |
170 | + operations = [ |
171 | + migrations.AlterField( |
172 | + model_name='interface', |
173 | + name='vlan', |
174 | + field=models.ForeignKey(null=True, blank=True, to='maasserver.VLAN', on_delete=django.db.models.deletion.PROTECT), |
175 | + ), |
176 | + ] |
177 | |
178 | === modified file 'src/maasserver/models/interface.py' |
179 | --- src/maasserver/models/interface.py 2016-05-12 19:07:37 +0000 |
180 | +++ src/maasserver/models/interface.py 2016-07-26 10:11:30 +0000 |
181 | @@ -67,11 +67,6 @@ |
182 | INTERFACE_NAME_REGEXP = '^[\w\-_.:]+$' |
183 | |
184 | |
185 | -def get_default_vlan(): |
186 | - from maasserver.models.vlan import VLAN |
187 | - return VLAN.objects.get_default_vlan().id |
188 | - |
189 | - |
190 | def get_subnet_family(subnet): |
191 | """Return the IPADDRESS_FAMILY for the `subnet`.""" |
192 | if subnet is not None: |
193 | @@ -375,8 +370,7 @@ |
194 | through='InterfaceRelationship', symmetrical=False) |
195 | |
196 | vlan = ForeignKey( |
197 | - 'VLAN', default=get_default_vlan, editable=True, blank=False, |
198 | - null=False, on_delete=PROTECT) |
199 | + 'VLAN', editable=True, blank=True, null=True, on_delete=PROTECT) |
200 | |
201 | ip_addresses = ManyToManyField( |
202 | 'StaticIPAddress', editable=True, blank=True) |
203 | @@ -438,8 +432,13 @@ |
204 | mtu = None |
205 | if self.params: |
206 | mtu = self.params.get('mtu', None) |
207 | + if mtu is None and self.vlan is not None: |
208 | + mtu = self.vlan.mtu |
209 | if mtu is None: |
210 | - mtu = self.vlan.mtu |
211 | + # Use default MTU for the interface when the interface has no |
212 | + # MTU set and it is disconnected. |
213 | + from maasserver.models.vlan import DEFAULT_MTU |
214 | + mtu = DEFAULT_MTU |
215 | return mtu |
216 | |
217 | def get_links(self): |
218 | @@ -752,11 +751,12 @@ |
219 | # subnets into the default VLAN, this assumption might be incorrect in |
220 | # many cases, leading to interfaces being configured as AUTO when |
221 | # they should be configured as DHCP. |
222 | - found_subnet = self.vlan.subnet_set.first() |
223 | - if found_subnet is not None: |
224 | - return self.link_subnet(INTERFACE_LINK_TYPE.AUTO, found_subnet) |
225 | - else: |
226 | - return self.link_subnet(INTERFACE_LINK_TYPE.DHCP, None) |
227 | + if self.vlan is not None: |
228 | + found_subnet = self.vlan.subnet_set.first() |
229 | + if found_subnet is not None: |
230 | + return self.link_subnet(INTERFACE_LINK_TYPE.AUTO, found_subnet) |
231 | + else: |
232 | + return self.link_subnet(INTERFACE_LINK_TYPE.DHCP, None) |
233 | |
234 | def ensure_link_up(self): |
235 | """Ensure that if no subnet links exists that at least a LINK_UP |
236 | @@ -766,7 +766,7 @@ |
237 | if has_links: |
238 | # Nothing to do, already has links. |
239 | return |
240 | - else: |
241 | + elif self.vlan is not None: |
242 | # Use an associated subnet if it exists and its on the same VLAN |
243 | # the interface is currently connected, else it will just be a |
244 | # LINK_UP without a subnet. |
245 | @@ -1265,20 +1265,31 @@ |
246 | "parents": ["VLAN interface must have exactly one parent."] |
247 | }) |
248 | parent = parents[0] |
249 | + # We do not allow a bridge interface to be a parent for a VLAN |
250 | + # interface. |
251 | allowed_vlan_parent_types = ( |
252 | INTERFACE_TYPE.PHYSICAL, |
253 | INTERFACE_TYPE.BOND, |
254 | INTERFACE_TYPE.BRIDGE |
255 | ) |
256 | if parent.get_type() not in allowed_vlan_parent_types: |
257 | - # XXX mpontillo 2016-06-23: we won't mention bridges in this |
258 | - # error message, since users can't configure bridges on nodes. |
259 | + # XXX blake_r 2016-07-18: we won't mention bridges in this |
260 | + # error message, since users can't configure VLAN interfaces |
261 | + # on bridges. |
262 | raise ValidationError({ |
263 | "parents": [ |
264 | "VLAN interface can only be created on a physical " |
265 | "or bond interface." |
266 | ] |
267 | }) |
268 | + # VLAN interface must be connected to a VLAN, it cannot be |
269 | + # disconnected like physical and bond interfaces. |
270 | + if self.vlan is None: |
271 | + raise ValidationError({ |
272 | + "vlan": [ |
273 | + "VLAN interface requires connection to a VLAN." |
274 | + ] |
275 | + }) |
276 | |
277 | def save(self, *args, **kwargs): |
278 | # Set the node of this VLAN to the same as its parents. |
279 | |
280 | === modified file 'src/maasserver/models/node.py' |
281 | --- src/maasserver/models/node.py 2016-07-25 20:13:54 +0000 |
282 | +++ src/maasserver/models/node.py 2016-07-26 10:11:30 +0000 |
283 | @@ -3407,6 +3407,10 @@ |
284 | new_vlan = new_fabric.get_default_vlan() |
285 | interface.vlan = new_vlan |
286 | interface.save() |
287 | + else: |
288 | + interface.vlan = ( |
289 | + Fabric.objects.get_default_fabric().get_default_vlan()) |
290 | + interface.save() |
291 | else: |
292 | if interface.node.id != self.id: |
293 | # MAC address was on a different node. We need to move |
294 | |
295 | === modified file 'src/maasserver/models/signals/interfaces.py' |
296 | --- src/maasserver/models/signals/interfaces.py 2016-05-12 19:07:37 +0000 |
297 | +++ src/maasserver/models/signals/interfaces.py 2016-07-26 10:11:30 +0000 |
298 | @@ -197,22 +197,25 @@ |
299 | NODE_TYPE.RACK_CONTROLLER, |
300 | NODE_TYPE.REGION_AND_RACK_CONTROLLER): |
301 | # Interface VLAN was changed on a controller. Move all linked subnets |
302 | - # to that new VLAN. |
303 | - for ip_address in instance.ip_addresses.all(): |
304 | - if ip_address.subnet is not None: |
305 | - ip_address.subnet.vlan = new_vlan |
306 | - ip_address.subnet.save() |
307 | + # to that new VLAN, unless the new VLAN is None. When the VLAN is |
308 | + # None then the administrator is say that the interface is now |
309 | + # disconnected. |
310 | + if new_vlan is not None: |
311 | + for ip_address in instance.ip_addresses.all(): |
312 | + if ip_address.subnet is not None: |
313 | + ip_address.subnet.vlan = new_vlan |
314 | + ip_address.subnet.save() |
315 | |
316 | - # If any children are VLAN interfaces then we need to move those |
317 | - # VLANs into the same fabric as the parent. |
318 | - for rel in instance.children_relationships.all(): |
319 | - if rel.child.type == INTERFACE_TYPE.VLAN: |
320 | - new_child_vlan, _ = VLAN.objects.get_or_create( |
321 | - fabric=new_vlan.fabric, vid=rel.child.vlan.vid) |
322 | - rel.child.vlan = new_child_vlan |
323 | - rel.child.save() |
324 | - # No need to update the IP addresses here this function |
325 | - # will be called again because the child has been saved. |
326 | + # If any children are VLAN interfaces then we need to move those |
327 | + # VLANs into the same fabric as the parent. |
328 | + for rel in instance.children_relationships.all(): |
329 | + if rel.child.type == INTERFACE_TYPE.VLAN: |
330 | + new_child_vlan, _ = VLAN.objects.get_or_create( |
331 | + fabric=new_vlan.fabric, vid=rel.child.vlan.vid) |
332 | + rel.child.vlan = new_child_vlan |
333 | + rel.child.save() |
334 | + # No need to update the IP addresses here this function |
335 | + # will be called again because the child has been saved. |
336 | |
337 | else: |
338 | # Interface VLAN was changed on a machine or device. Remove all its |
339 | |
340 | === modified file 'src/maasserver/models/signals/tests/test_interfaces.py' |
341 | --- src/maasserver/models/signals/tests/test_interfaces.py 2016-04-11 16:23:26 +0000 |
342 | +++ src/maasserver/models/signals/tests/test_interfaces.py 2016-07-26 10:11:30 +0000 |
343 | @@ -194,6 +194,17 @@ |
344 | self.assertIsNone(reload_object(static_ip)) |
345 | self.assertIsNotNone(reload_object(discovered_ip)) |
346 | |
347 | + def test__removes_links_when_goes_to_disconnected(self): |
348 | + node = self.maker() |
349 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node) |
350 | + static_ip = factory.make_StaticIPAddress(interface=interface) |
351 | + discovered_ip = factory.make_StaticIPAddress( |
352 | + alloc_type=IPADDRESS_TYPE.DISCOVERED, interface=interface) |
353 | + interface.vlan = None |
354 | + interface.save() |
355 | + self.assertIsNone(reload_object(static_ip)) |
356 | + self.assertIsNotNone(reload_object(discovered_ip)) |
357 | + |
358 | |
359 | class TestInterfaceVLANUpdateController(MAASServerTestCase): |
360 | |
361 | @@ -222,6 +233,17 @@ |
362 | interface.save() |
363 | self.assertEquals(new_vlan, reload_object(subnet).vlan) |
364 | |
365 | + def test__doesnt_move_link_subnets_when_vlan_is_None(self): |
366 | + node = self.maker() |
367 | + interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node) |
368 | + old_vlan = interface.vlan |
369 | + subnet = factory.make_Subnet(vlan=old_vlan) |
370 | + factory.make_StaticIPAddress( |
371 | + subnet=subnet, interface=interface) |
372 | + interface.vlan = None |
373 | + interface.save() |
374 | + self.assertEquals(old_vlan, reload_object(subnet).vlan) |
375 | + |
376 | def test__moves_children_vlans_to_same_fabric(self): |
377 | node = self.maker() |
378 | parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node) |
379 | |
380 | === modified file 'src/maasserver/models/tests/test_interface.py' |
381 | --- src/maasserver/models/tests/test_interface.py 2016-05-12 19:07:37 +0000 |
382 | +++ src/maasserver/models/tests/test_interface.py 2016-07-26 10:11:30 +0000 |
383 | @@ -42,6 +42,7 @@ |
384 | UnknownInterface, |
385 | VLANInterface, |
386 | ) |
387 | +from maasserver.models.vlan import DEFAULT_MTU |
388 | from maasserver.testing.factory import factory |
389 | from maasserver.testing.orm import reload_objects |
390 | from maasserver.testing.testcase import ( |
391 | @@ -527,6 +528,17 @@ |
392 | name=name, node=node, mac_address=mac, |
393 | type=INTERFACE_TYPE.PHYSICAL)) |
394 | |
395 | + def test_allows_null_vlan(self): |
396 | + name = factory.make_name('name') |
397 | + node = factory.make_Node() |
398 | + mac = factory.make_MAC() |
399 | + interface = factory.make_Interface( |
400 | + INTERFACE_TYPE.PHYSICAL, |
401 | + name=name, node=node, mac_address=mac, disconnected=True) |
402 | + self.assertThat(interface, MatchesStructure.byEquality( |
403 | + name=name, node=node, mac_address=mac, |
404 | + type=INTERFACE_TYPE.PHYSICAL, vlan=None)) |
405 | + |
406 | def test_string_representation_contains_essential_data(self): |
407 | name = factory.make_name('name') |
408 | node = factory.make_Node() |
409 | @@ -570,6 +582,11 @@ |
410 | nic1.vlan.save() |
411 | self.assertEqual(vlan_mtu, nic1.get_effective_mtu()) |
412 | |
413 | + def test_get_effective_mtu_returns_default_mtu(self): |
414 | + nic1 = factory.make_Interface( |
415 | + INTERFACE_TYPE.PHYSICAL, disconnected=True) |
416 | + self.assertEqual(DEFAULT_MTU, nic1.get_effective_mtu()) |
417 | + |
418 | def test_get_links_returns_links_for_each_type(self): |
419 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
420 | links = [] |
421 | @@ -1595,6 +1612,11 @@ |
422 | class TestForceAutoOrDHCPLink(MAASServerTestCase): |
423 | """Tests for `Interface.force_auto_or_dhcp_link`.""" |
424 | |
425 | + def test__does_nothing_when_disconnected(self): |
426 | + interface = factory.make_Interface( |
427 | + INTERFACE_TYPE.PHYSICAL, disconnected=True) |
428 | + self.assertIsNone(interface.force_auto_or_dhcp_link()) |
429 | + |
430 | def test__sets_to_AUTO_on_subnet(self): |
431 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
432 | subnet = factory.make_Subnet(vlan=interface.vlan) |
433 | @@ -1622,6 +1644,15 @@ |
434 | 1, interface.ip_addresses.count(), |
435 | "Should only have one IP address assigned.") |
436 | |
437 | + def test__does_nothing_if_no_vlan(self): |
438 | + interface = factory.make_Interface( |
439 | + INTERFACE_TYPE.PHYSICAL, disconnected=True) |
440 | + interface.ensure_link_up() |
441 | + interface = reload_object(interface) |
442 | + self.assertEqual( |
443 | + 0, interface.ip_addresses.count(), |
444 | + "Should only have no IP address assigned.") |
445 | + |
446 | def test__creates_link_up_to_discovered_subnet_on_same_vlan(self): |
447 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
448 | subnet = factory.make_Subnet(vlan=interface.vlan) |
449 | |
450 | === modified file 'src/maasserver/static/js/angular/controllers/node_details_networking.js' |
451 | --- src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-07-11 18:29:06 +0000 |
452 | +++ src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-07-26 10:11:30 +0000 |
453 | @@ -654,11 +654,17 @@ |
454 | if($scope.isInterfaceNameInvalid(nic)) { |
455 | nic.name = originalInteface.name; |
456 | } else if(originalInteface.name !== nic.name || |
457 | + (originalInteface.vlan_id === null && nic.vlan !== null) || |
458 | + (originalInteface.vlan_id !== null && nic.vlan === null) || |
459 | originalInteface.vlan_id !== nic.vlan.id) { |
460 | var params = { |
461 | - "name": nic.name, |
462 | - "vlan": nic.vlan.id |
463 | + "name": nic.name |
464 | }; |
465 | + if(nic.vlan !== null) { |
466 | + params.vlan = nic.vlan.id; |
467 | + } else { |
468 | + params.vlan = null; |
469 | + } |
470 | $scope.$parent.nodesManager.updateInterface( |
471 | $scope.node, nic.id, params).then(null, function(error) { |
472 | // XXX blake_r: Just log the error in the console, but |
473 | @@ -716,7 +722,11 @@ |
474 | $scope.fabricChanged = function(nic) { |
475 | // Update the VLAN on the node to be the default VLAN for that |
476 | // fabric. The first VLAN for the fabric is the default. |
477 | - nic.vlan = getDefaultVLAN(nic.fabric); |
478 | + if(nic.fabric !== null) { |
479 | + nic.vlan = getDefaultVLAN(nic.fabric); |
480 | + } else { |
481 | + nic.vlan = null; |
482 | + } |
483 | $scope.saveInterface(nic); |
484 | }; |
485 | |
486 | |
487 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js' |
488 | --- src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js 2016-07-11 18:29:06 +0000 |
489 | +++ src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js 2016-07-26 10:11:30 +0000 |
490 | @@ -1685,6 +1685,62 @@ |
491 | "vlan": vlan.id |
492 | }); |
493 | }); |
494 | + |
495 | + it("calls MachinesManager.updateInterface if vlan set", function() { |
496 | + var controller = makeController(); |
497 | + var id = makeInteger(0, 100); |
498 | + var name = makeName("nic"); |
499 | + var vlan = { id: makeInteger(0, 100) }; |
500 | + var original_nic = { |
501 | + id: id, |
502 | + name: name, |
503 | + vlan_id: null |
504 | + }; |
505 | + var nic = { |
506 | + id: id, |
507 | + name: name, |
508 | + vlan: vlan |
509 | + }; |
510 | + $scope.originalInterfaces[id] = original_nic; |
511 | + $scope.interfaces = [nic]; |
512 | + |
513 | + spyOn(MachinesManager, "updateInterface").and.returnValue( |
514 | + $q.defer().promise); |
515 | + $scope.saveInterface(nic); |
516 | + expect(MachinesManager.updateInterface).toHaveBeenCalledWith( |
517 | + node, id, { |
518 | + "name": name, |
519 | + "vlan": vlan.id |
520 | + }); |
521 | + }); |
522 | + |
523 | + it("calls MachinesManager.updateInterface if vlan unset", function() { |
524 | + var controller = makeController(); |
525 | + var id = makeInteger(0, 100); |
526 | + var name = makeName("nic"); |
527 | + var vlan = { id: makeInteger(0, 100) }; |
528 | + var original_nic = { |
529 | + id: id, |
530 | + name: name, |
531 | + vlan_id: makeInteger(200, 300) |
532 | + }; |
533 | + var nic = { |
534 | + id: id, |
535 | + name: name, |
536 | + vlan: null |
537 | + }; |
538 | + $scope.originalInterfaces[id] = original_nic; |
539 | + $scope.interfaces = [nic]; |
540 | + |
541 | + spyOn(MachinesManager, "updateInterface").and.returnValue( |
542 | + $q.defer().promise); |
543 | + $scope.saveInterface(nic); |
544 | + expect(MachinesManager.updateInterface).toHaveBeenCalledWith( |
545 | + node, id, { |
546 | + "name": name, |
547 | + "vlan": null |
548 | + }); |
549 | + }); |
550 | }); |
551 | |
552 | describe("setFocusInterface", function() { |
553 | @@ -1847,6 +1903,17 @@ |
554 | expect(nic.vlan).toBe(vlan); |
555 | }); |
556 | |
557 | + it("sets vlan to null", function() { |
558 | + var controller = makeController(); |
559 | + var nic = { |
560 | + vlan: {}, |
561 | + fabric: null |
562 | + }; |
563 | + spyOn($scope, "saveInterface"); |
564 | + $scope.fabricChanged(nic); |
565 | + expect(nic.vlan).toBeNull(); |
566 | + }); |
567 | + |
568 | it("calls saveInterface", function() { |
569 | var controller = makeController(); |
570 | var fabric = { |
571 | |
572 | === modified file 'src/maasserver/static/partials/node-details.html' |
573 | --- src/maasserver/static/partials/node-details.html 2016-07-15 00:42:25 +0000 |
574 | +++ src/maasserver/static/partials/node-details.html 2016-07-26 10:11:30 +0000 |
575 | @@ -617,11 +617,13 @@ |
576 | data-ng-disabled="interface.type == 'alias' || interface.type == 'vlan' || !isNodeEditingAllowed()" |
577 | data-ng-change="fabricChanged(interface)" |
578 | data-ng-options="fabric as fabric.name for fabric in fabrics"> |
579 | + <option value="">Disconnected</option> |
580 | </select> |
581 | </div> |
582 | <div class="table__data table-col--14"> |
583 | <select class="table__input" name="vlan" id="vlan" |
584 | data-ng-model="interface.vlan" |
585 | + data-ng-if="interface.fabric" |
586 | data-ng-disabled="isController || interface.type == 'alias' || interface.vlan.vid === 0 || !isNodeEditingAllowed()" |
587 | data-ng-change="saveInterface(interface)" |
588 | data-ng-options="vlan as getVLANText(vlan) for vlan in vlans | removeDefaultVLANIfVLAN:interface.type | filterByFabric:interface.fabric"> |
589 | @@ -629,6 +631,7 @@ |
590 | </div> |
591 | <div class="table__data table-col--18"> |
592 | <select class="table__input" name="subnet" id="subnet" |
593 | + data-ng-if="interface.fabric" |
594 | data-ng-hide="isAllNetworkingDisabled() && interface.discovered[0].subnet_id" |
595 | data-ng-disabled="isController || !isNodeEditingAllowed()" |
596 | data-ng-model="interface.subnet" |
597 | @@ -641,7 +644,7 @@ |
598 | </span> |
599 | </div> |
600 | <div class="table__data table-col--21"> |
601 | - <ul class="no-bullets no-margin-bottom" data-ng-if="!isController && !isAllNetworkingDisabled()"> |
602 | + <ul class="no-bullets no-margin-bottom" data-ng-if="!isController && !isAllNetworkingDisabled() && interface.fabric"> |
603 | <li class="no-margin"> |
604 | <select class="table__input" name="link-mode" id="link-mode" |
605 | data-ng-model="interface.mode" |
606 | |
607 | === modified file 'src/maasserver/testing/factory.py' |
608 | --- src/maasserver/testing/factory.py 2016-07-20 19:27:20 +0000 |
609 | +++ src/maasserver/testing/factory.py 2016-07-26 10:11:30 +0000 |
610 | @@ -902,7 +902,7 @@ |
611 | def make_Interface( |
612 | self, iftype=INTERFACE_TYPE.PHYSICAL, node=None, mac_address=None, |
613 | vlan=None, parents=None, name=None, cluster_interface=None, |
614 | - ip=None, enabled=True, fabric=None, tags=None): |
615 | + ip=None, enabled=True, fabric=None, tags=None, disconnected=False): |
616 | if name is None: |
617 | if iftype in (INTERFACE_TYPE.PHYSICAL, INTERFACE_TYPE.UNKNOWN): |
618 | name = self.make_name('eth') |
619 | @@ -919,20 +919,21 @@ |
620 | name = None |
621 | if iftype is None: |
622 | iftype = INTERFACE_TYPE.PHYSICAL |
623 | - if vlan is None: |
624 | - if fabric is not None: |
625 | - if iftype == INTERFACE_TYPE.VLAN: |
626 | - vlan = self.make_VLAN(fabric=fabric) |
627 | - else: |
628 | - vlan = fabric.get_default_vlan() |
629 | - else: |
630 | - if iftype == INTERFACE_TYPE.VLAN and parents: |
631 | - vlan = self.make_VLAN(fabric=parents[0].vlan.fabric) |
632 | - elif iftype == INTERFACE_TYPE.BOND and parents: |
633 | - vlan = parents[0].vlan |
634 | - else: |
635 | - fabric = self.make_Fabric() |
636 | - vlan = fabric.get_default_vlan() |
637 | + if not disconnected: |
638 | + if vlan is None: |
639 | + if fabric is not None: |
640 | + if iftype == INTERFACE_TYPE.VLAN: |
641 | + vlan = self.make_VLAN(fabric=fabric) |
642 | + else: |
643 | + vlan = fabric.get_default_vlan() |
644 | + else: |
645 | + if iftype == INTERFACE_TYPE.VLAN and parents: |
646 | + vlan = self.make_VLAN(fabric=parents[0].vlan.fabric) |
647 | + elif iftype == INTERFACE_TYPE.BOND and parents: |
648 | + vlan = parents[0].vlan |
649 | + else: |
650 | + fabric = self.make_Fabric() |
651 | + vlan = fabric.get_default_vlan() |
652 | if (mac_address is None and |
653 | iftype in [ |
654 | INTERFACE_TYPE.PHYSICAL, |
655 | |
656 | === modified file 'src/maasserver/tests/test_forms_interface.py' |
657 | --- src/maasserver/tests/test_forms_interface.py 2016-04-22 17:28:15 +0000 |
658 | +++ src/maasserver/tests/test_forms_interface.py 2016-07-26 10:11:30 +0000 |
659 | @@ -86,6 +86,22 @@ |
660 | MatchesStructure.byEquality( |
661 | name=interface.name, vlan=new_vlan, enabled=interface.enabled)) |
662 | |
663 | + def test__allows_no_vlan(self): |
664 | + node = self.maker() |
665 | + interface = factory.make_Interface( |
666 | + INTERFACE_TYPE.PHYSICAL, node=node) |
667 | + form = ControllerInterfaceForm( |
668 | + instance=interface, |
669 | + data={ |
670 | + 'vlan': None, |
671 | + }) |
672 | + self.assertTrue(form.is_valid(), form.errors) |
673 | + interface = form.save() |
674 | + self.assertThat( |
675 | + interface, |
676 | + MatchesStructure.byEquality( |
677 | + name=interface.name, vlan=None, enabled=interface.enabled)) |
678 | + |
679 | |
680 | class PhysicalInterfaceFormTest(MAASServerTestCase): |
681 | |
682 | @@ -116,6 +132,30 @@ |
683 | type=INTERFACE_TYPE.PHYSICAL, tags=tags)) |
684 | self.assertItemsEqual([], interface.parents.all()) |
685 | |
686 | + def test__creates_physical_interface_disconnected(self): |
687 | + node = factory.make_Node() |
688 | + mac_address = factory.make_mac_address() |
689 | + interface_name = 'eth0' |
690 | + tags = [ |
691 | + factory.make_name("tag") |
692 | + for _ in range(3) |
693 | + ] |
694 | + form = PhysicalInterfaceForm( |
695 | + node=node, |
696 | + data={ |
697 | + 'name': interface_name, |
698 | + 'mac_address': mac_address, |
699 | + 'tags': ",".join(tags), |
700 | + }) |
701 | + self.assertTrue(form.is_valid(), form.errors) |
702 | + interface = form.save() |
703 | + self.assertThat( |
704 | + interface, |
705 | + MatchesStructure.byEquality( |
706 | + node=node, mac_address=mac_address, name=interface_name, |
707 | + type=INTERFACE_TYPE.PHYSICAL, tags=tags, vlan=None)) |
708 | + self.assertItemsEqual([], interface.parents.all()) |
709 | + |
710 | def test__create_ensures_link_up(self): |
711 | node = factory.make_Node() |
712 | mac_address = factory.make_mac_address() |
713 | @@ -255,6 +295,26 @@ |
714 | name=new_name, vlan=new_vlan, enabled=False, tags=[])) |
715 | self.assertItemsEqual([], interface.parents.all()) |
716 | |
717 | + def test__edits_interface_disconnected(self): |
718 | + interface = factory.make_Interface( |
719 | + INTERFACE_TYPE.PHYSICAL, name='eth0') |
720 | + new_name = 'eth1' |
721 | + form = PhysicalInterfaceForm( |
722 | + instance=interface, |
723 | + data={ |
724 | + 'name': new_name, |
725 | + 'vlan': None, |
726 | + 'enabled': False, |
727 | + 'tags': "", |
728 | + }) |
729 | + self.assertTrue(form.is_valid(), form.errors) |
730 | + interface = form.save() |
731 | + self.assertThat( |
732 | + interface, |
733 | + MatchesStructure.byEquality( |
734 | + name=new_name, vlan=None, enabled=False, tags=[])) |
735 | + self.assertItemsEqual([], interface.parents.all()) |
736 | + |
737 | def test__create_sets_interface_parameters(self): |
738 | node = factory.make_Node() |
739 | mac_address = factory.make_mac_address() |
740 | @@ -403,6 +463,20 @@ |
741 | self.assertIsNotNone( |
742 | interface.ip_addresses.filter(alloc_type=IPADDRESS_TYPE.STICKY)) |
743 | |
744 | + def test__create_rejects_interface_without_vlan(self): |
745 | + parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
746 | + form = VLANInterfaceForm( |
747 | + node=parent.node, |
748 | + data={ |
749 | + 'parents': [parent.id], |
750 | + }) |
751 | + self.assertFalse(form.is_valid(), form.errors) |
752 | + self.assertItemsEqual( |
753 | + ['vlan'], form.errors.keys(), form.errors) |
754 | + self.assertIn( |
755 | + "A VLAN interface must be connected to a tagged VLAN.", |
756 | + form.errors['vlan'][0]) |
757 | + |
758 | def test_rejects_interface_with_duplicate_name(self): |
759 | parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
760 | vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10) |
761 | @@ -468,6 +542,20 @@ |
762 | "VLAN interface can't have another VLAN interface as parent.", |
763 | form.errors['parents'][0]) |
764 | |
765 | + def test__rejects_no_vlan(self): |
766 | + parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
767 | + form = VLANInterfaceForm( |
768 | + node=parent.node, |
769 | + data={ |
770 | + 'vlan': None, |
771 | + 'parents': [parent.id], |
772 | + }) |
773 | + self.assertFalse(form.is_valid(), form.errors) |
774 | + self.assertItemsEqual(['vlan'], form.errors.keys()) |
775 | + self.assertIn( |
776 | + "A VLAN interface must be connected to a tagged VLAN.", |
777 | + form.errors['vlan'][0]) |
778 | + |
779 | def test__rejects_vlan_not_on_same_fabric(self): |
780 | parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
781 | factory.make_VLAN(fabric=parent.vlan.fabric, vid=10) |
782 | @@ -576,7 +664,8 @@ |
783 | self.assertThat( |
784 | interface, |
785 | MatchesStructure.byEquality( |
786 | - name=interface_name, type=INTERFACE_TYPE.BOND)) |
787 | + name=interface_name, type=INTERFACE_TYPE.BOND, |
788 | + vlan=parent1.vlan)) |
789 | self.assertIn( |
790 | interface.mac_address, [parent1.mac_address, parent2.mac_address]) |
791 | self.assertItemsEqual([parent1, parent2], interface.parents.all()) |
792 | @@ -787,6 +876,25 @@ |
793 | for parent in [parent1, parent2, new_parent] |
794 | )) |
795 | |
796 | + def test__edits_interface_allows_disconnected(self): |
797 | + parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
798 | + parent2 = factory.make_Interface( |
799 | + INTERFACE_TYPE.PHYSICAL, node=parent1.node) |
800 | + interface = factory.make_Interface( |
801 | + INTERFACE_TYPE.BOND, parents=[parent1, parent2]) |
802 | + form = BondInterfaceForm( |
803 | + instance=interface, |
804 | + data={ |
805 | + 'vlan': None, |
806 | + }) |
807 | + self.assertTrue(form.is_valid(), form.errors) |
808 | + interface = form.save() |
809 | + self.assertThat( |
810 | + interface, |
811 | + MatchesStructure.byEquality( |
812 | + mac_address=interface.mac_address, vlan=None, |
813 | + type=INTERFACE_TYPE.BOND)) |
814 | + |
815 | def test__edits_interface_removes_parents(self): |
816 | parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
817 | parent2 = factory.make_Interface( |
818 | @@ -1119,6 +1227,26 @@ |
819 | self.assertItemsEqual( |
820 | [parent1, parent2, new_parent], interface.parents.all()) |
821 | |
822 | + def test__edits_interface_allows_disconnected(self): |
823 | + parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
824 | + parent2 = factory.make_Interface( |
825 | + INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan) |
826 | + interface = factory.make_Interface( |
827 | + INTERFACE_TYPE.BRIDGE, |
828 | + parents=[parent1, parent2]) |
829 | + form = BridgeInterfaceForm( |
830 | + instance=interface, |
831 | + data={ |
832 | + 'vlan': None, |
833 | + }) |
834 | + self.assertTrue(form.is_valid(), form.errors) |
835 | + interface = form.save() |
836 | + self.assertThat( |
837 | + interface, |
838 | + MatchesStructure.byEquality( |
839 | + mac_address=interface.mac_address, |
840 | + vlan=None, type=INTERFACE_TYPE.BRIDGE)) |
841 | + |
842 | def test__edits_interface_removes_parents(self): |
843 | parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL) |
844 | parent2 = factory.make_Interface( |
845 | |
846 | === modified file 'src/maasserver/websockets/handlers/node.py' |
847 | --- src/maasserver/websockets/handlers/node.py 2016-05-12 19:07:37 +0000 |
848 | +++ src/maasserver/websockets/handlers/node.py 2016-07-26 10:11:30 +0000 |
849 | @@ -455,7 +455,8 @@ |
850 | def get_all_fabric_names(self, obj, subnets): |
851 | fabric_names = set() |
852 | for interface in obj.interface_set.all(): |
853 | - fabric_names.add(interface.vlan.fabric.name) |
854 | + if interface.vlan is not None: |
855 | + fabric_names.add(interface.vlan.fabric.name) |
856 | for subnet in subnets: |
857 | fabric_names.add(subnet.vlan.fabric.name) |
858 | return list(fabric_names) |
Thanks for taking on this much-needed change.
I've been testing this code out on my local MAAS, and it's working great. I really like how we don't drop new nodes into a default fabric/VLAN any more. Land it!