MAAS

Merge lp:~blake-rouse/maas/force-untagged-api into lp:~maas-committers/maas/trunk

  1. force-untagged-api
  2. Merge into trunk
Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: no longer in the source branch.
Merged at revision: 4731
Proposed branch: lp:~blake-rouse/maas/force-untagged-api
Merge into: lp:~maas-committers/maas/trunk
Prerequisite: lp:~blake-rouse/maas/rack-nic-fabric-change
Diff against target: 840 lines (+227/-91)
4 files modified
src/maasserver/api/tests/test_interfaces.py (+21/-13)
src/maasserver/forms_interface.py (+55/-12)
src/maasserver/tests/test_forms_interface.py (+137/-57)
src/maasserver/websockets/handlers/tests/test_machine.py (+14/-9)
To merge this branch: bzr merge lp:~blake-rouse/maas/force-untagged-api
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
Review via email: mp+288026@code.launchpad.net

Commit message

Prevent allowing tagged VLAN from being attached to a physical or bond interface. Prevent untagged VLAN from being attached to a VLAN interface. This causes the API to match what the UI does.

Description of the change

Not having the API not match the UI is wrong and invalid. Its either we support one way or the other way, we should not support 50% one way and 50% another way. That makes for a confusing and incomplete product.

To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Please see my comments here:

https://code.launchpad.net/~blake-rouse/maas/rack-nic-fabric-change/+merge/287852

Most of these validations are okay, but the ones that force the model to require physical interfaces to be on the default ("untagged") VLAN are deal-breakers for me.

I understand that we are making MAAS work out-of-the-box with switch fabrics where all ports always have the same default VLAN. But this change goes a step further and forces that to be true. The underlying model was designed to handle both scenarios, so forcing the issue in this way prevents a large percentage of customers from using MAAS, with no possible workaround. (I would argue that it is also a regression, since MAAS 1.8 supports this. MAAS 1.9 and 1.10 support this as well, though it is not the default behavior.)

review: Disapprove
Revision history for this message
Blake Rouse (blake-rouse) wrote :

My comments are documented in the bug. https://bugs.launchpad.net/maas/+bug/1552923

Revision history for this message
Mike Pontillo (mpontillo) wrote :

I still strongly disagree with this additional validation. However, I am changing my vote to "Approve" based on discussions with our management team; the preferred approach is to add this validation despite the noted risks.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/tests/test_interfaces.py'
2--- src/maasserver/api/tests/test_interfaces.py 2016-02-26 18:39:26 +0000
3+++ src/maasserver/api/tests/test_interfaces.py 2016-03-03 21:30:31 +0000
4@@ -48,14 +48,14 @@
5 def make_complex_interface(node, name=None):
6 """Makes interface with parents and children."""
7 fabric = factory.make_Fabric()
8- vlan_5 = factory.make_VLAN(vid=5, fabric=fabric)
9+ untagged = fabric.get_default_vlan()
10 nic_0 = factory.make_Interface(
11- INTERFACE_TYPE.PHYSICAL, vlan=vlan_5, node=node)
12+ INTERFACE_TYPE.PHYSICAL, vlan=untagged, node=node)
13 nic_1 = factory.make_Interface(
14- INTERFACE_TYPE.PHYSICAL, vlan=vlan_5, node=node)
15+ INTERFACE_TYPE.PHYSICAL, vlan=untagged, node=node)
16 parents = [nic_0, nic_1]
17 bond_interface = factory.make_Interface(
18- INTERFACE_TYPE.BOND, mac_address=nic_0.mac_address, vlan=vlan_5,
19+ INTERFACE_TYPE.BOND, mac_address=nic_0.mac_address, vlan=untagged,
20 parents=parents, name=name)
21 vlan_10 = factory.make_VLAN(vid=10, fabric=fabric)
22 vlan_nic_10 = factory.make_Interface(
23@@ -112,7 +112,8 @@
24 node = factory.make_Node(status=status)
25 mac = factory.make_mac_address()
26 name = factory.make_name("eth")
27- vlan = factory.make_VLAN()
28+ fabric = factory.make_Fabric()
29+ vlan = fabric.get_default_vlan()
30 tags = [
31 factory.make_name("tag")
32 for _ in range(3)
33@@ -145,7 +146,8 @@
34 owner=self.logged_in_user, parent=parent)
35 mac = factory.make_mac_address()
36 name = factory.make_name("eth")
37- vlan = factory.make_VLAN()
38+ fabric = factory.make_Fabric()
39+ vlan = fabric.get_default_vlan()
40 tags = [
41 factory.make_name("tag")
42 for _ in range(3)
43@@ -178,7 +180,8 @@
44 node = factory.make_Node(status=status)
45 mac = factory.make_mac_address()
46 name = factory.make_name("eth")
47- vlan = factory.make_VLAN()
48+ fabric = factory.make_Fabric()
49+ vlan = fabric.get_default_vlan()
50 tags = [
51 factory.make_name("tag")
52 for _ in range(3)
53@@ -274,7 +277,8 @@
54 interface_on_other_node = factory.make_Interface(
55 INTERFACE_TYPE.PHYSICAL)
56 name = factory.make_name("eth")
57- vlan = factory.make_VLAN()
58+ fabric = factory.make_Fabric()
59+ vlan = fabric.get_default_vlan()
60 uri = get_interfaces_uri(node)
61 response = self.client.post(uri, {
62 "op": "create_physical",
63@@ -294,7 +298,8 @@
64 self.become_admin()
65 for status in (NODE_STATUS.READY, NODE_STATUS.BROKEN):
66 node = factory.make_Node(status=status)
67- vlan = factory.make_VLAN()
68+ fabric = factory.make_Fabric()
69+ vlan = fabric.get_default_vlan()
70 parent_1_iface = factory.make_Interface(
71 INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=node)
72 parent_2_iface = factory.make_Interface(
73@@ -398,7 +403,7 @@
74 self.assertEqual(
75 http.client.CONFLICT, response.status_code, response.content)
76
77- def test_create_bond_requires_name_vlan_and_parents(self):
78+ def test_create_bond_requires_name_and_parents(self):
79 self.become_admin()
80 node = factory.make_Node(status=NODE_STATUS.READY)
81 uri = get_interfaces_uri(node)
82@@ -411,7 +416,6 @@
83 self.assertEqual({
84 "mac_address": ["This field cannot be blank."],
85 "name": ["This field is required."],
86- "vlan": ["This field is required."],
87 "parents": ["A Bond interface must have one or more parents."],
88 }, json_load_bytes(response.content))
89
90@@ -635,7 +639,8 @@
91 interface = factory.make_Interface(
92 INTERFACE_TYPE.PHYSICAL, node=node)
93 new_name = factory.make_name("name")
94- new_vlan = factory.make_VLAN()
95+ new_fabric = factory.make_Fabric()
96+ new_vlan = new_fabric.get_default_vlan()
97 uri = get_interface_uri(interface)
98 response = self.client.put(uri, {
99 "name": new_name,
100@@ -654,7 +659,8 @@
101 interface = factory.make_Interface(
102 INTERFACE_TYPE.PHYSICAL, node=device)
103 new_name = factory.make_name("name")
104- new_vlan = factory.make_VLAN()
105+ new_fabric = factory.make_Fabric()
106+ new_vlan = new_fabric.get_default_vlan()
107 uri = get_interface_uri(interface)
108 response = self.client.put(uri, {
109 "name": new_name,
110@@ -689,9 +695,11 @@
111 node)
112 physical_interface = factory.make_Interface(
113 INTERFACE_TYPE.PHYSICAL, node=node)
114+ new_vlan = factory.make_VLAN(fabric=physical_interface.vlan.fabric)
115 uri = get_interface_uri(vlan_10)
116 response = self.client.put(uri, {
117 "parent": physical_interface.id,
118+ "vlan": new_vlan.id,
119 })
120 self.assertEqual(
121 http.client.OK, response.status_code, response.content)
122
123=== modified file 'src/maasserver/forms_interface.py'
124--- src/maasserver/forms_interface.py 2015-12-01 18:12:59 +0000
125+++ src/maasserver/forms_interface.py 2016-03-03 21:30:31 +0000
126@@ -186,6 +186,13 @@
127 if len(parents) > 0:
128 raise ValidationError("A physical interface cannot have parents.")
129
130+ def clean_vlan(self):
131+ new_vlan = self.cleaned_data.get('vlan')
132+ if new_vlan and new_vlan.fabric.get_default_vlan() != new_vlan:
133+ raise ValidationError(
134+ "A physical interface can only belong to an untagged VLAN.")
135+ return new_vlan
136+
137 def clean(self):
138 cleaned_data = super(PhysicalInterfaceForm, self).clean()
139 new_name = cleaned_data.get('name')
140@@ -223,11 +230,25 @@
141 "in a bond.")
142 return parents
143
144+ def clean_vlan(self):
145+ new_vlan = self.cleaned_data.get('vlan')
146+ if new_vlan and new_vlan.fabric.get_default_vlan() == new_vlan:
147+ raise ValidationError(
148+ "A VLAN interface can only belong to a tagged VLAN.")
149+ return new_vlan
150+
151 def clean(self):
152 cleaned_data = super(VLANInterfaceForm, self).clean()
153 if self.fields_ok(['vlan', 'parents']):
154 new_vlan = self.cleaned_data.get('vlan')
155 if new_vlan:
156+ # VLAN needs to be the in the same fabric as the parent.
157+ parent = self.cleaned_data.get('parents')[0]
158+ if parent.vlan.fabric_id != new_vlan.fabric_id:
159+ set_form_error(
160+ self, "vlan",
161+ "A VLAN interface can only belong to a tagged VLAN on "
162+ "the same fabric as its parent interface.")
163 name = build_vlan_interface_name(
164 self.cleaned_data.get('parents').first(), new_vlan)
165 self.clean_interface_name_uniqueness(name)
166@@ -271,6 +292,15 @@
167 'name',
168 )
169
170+ def __init__(self, *args, **kwargs):
171+ super(BondInterfaceForm, self).__init__(*args, **kwargs)
172+ # Allow VLAN to be blank when creating.
173+ instance = kwargs.get("instance", None)
174+ if instance is not None and instance.id is not None:
175+ self.fields['vlan'].required = True
176+ else:
177+ self.fields['vlan'].required = False
178+
179 def clean_parents(self):
180 parents = self.get_clean_parents()
181 if parents is None:
182@@ -280,9 +310,16 @@
183 "A Bond interface must have one or more parents.")
184 return parents
185
186+ def clean_vlan(self):
187+ new_vlan = self.cleaned_data.get('vlan')
188+ if new_vlan and new_vlan.fabric.get_default_vlan() != new_vlan:
189+ raise ValidationError(
190+ "A bond interface can only belong to an untagged VLAN.")
191+ return new_vlan
192+
193 def clean(self):
194 cleaned_data = super(BondInterfaceForm, self).clean()
195- if self.fields_ok(['parents']):
196+ if self.fields_ok(['vlan', 'parents']):
197 parents = self.cleaned_data.get('parents')
198 # Set the mac_address if its missing and the interface is being
199 # created.
200@@ -323,6 +360,23 @@
201 self, 'parents',
202 "%s is already in-use by another interface." % (
203 ', '.join(sorted(parents_with_other_children))))
204+
205+ # When creating the bond set VLAN to the same as the parents
206+ # and check that the parents all belong to the same VLAN.
207+ if self.instance.id is None:
208+ vlan = self.cleaned_data.get('vlan')
209+ if vlan is None:
210+ vlan = parents[0].vlan
211+ self.cleaned_data['vlan'] = vlan
212+ parent_vlans = {
213+ parent.vlan
214+ for parent in parents
215+ }
216+ if parent_vlans != set([vlan]):
217+ set_form_error(
218+ self, 'parents',
219+ "All parents must belong to the same VLAN.")
220+
221 return cleaned_data
222
223 def set_extra_parameters(self, interface, created):
224@@ -346,17 +400,6 @@
225 elif created:
226 interface.params[bond_field] = self.fields[bond_field].initial
227
228- def save(self, *args, **kwargs):
229- """Persist the interface into the database."""
230- created = self.instance.id is None
231- interface = super(BondInterfaceForm, self).save()
232- if created:
233- # Bond was created we remove all the links on the parent interfaces
234- # and ensure that the bond has atleast a LINK_UP.
235- for parent in interface.parents.all():
236- parent.clear_all_links(clearing_config=True)
237- return interface
238-
239
240 INTERFACE_FORM_MAPPING = {
241 INTERFACE_TYPE.PHYSICAL: PhysicalInterfaceForm,
242
243=== modified file 'src/maasserver/tests/test_forms_interface.py'
244--- src/maasserver/tests/test_forms_interface.py 2015-12-01 18:12:59 +0000
245+++ src/maasserver/tests/test_forms_interface.py 2016-03-03 21:30:31 +0000
246@@ -25,6 +25,7 @@
247 from maasserver.testing.factory import factory
248 from maasserver.testing.testcase import MAASServerTestCase
249 from maasserver.utils.forms import compose_invalid_choice_text
250+from maasserver.utils.orm import reload_object
251 from testtools import ExpectedException
252 from testtools.matchers import MatchesStructure
253
254@@ -58,7 +59,8 @@
255 node = factory.make_Node()
256 mac_address = factory.make_mac_address()
257 interface_name = 'eth0'
258- vlan = factory.make_VLAN()
259+ fabric = factory.make_Fabric()
260+ vlan = fabric.get_default_vlan()
261 tags = [
262 factory.make_name("tag")
263 for _ in range(3)
264@@ -84,7 +86,8 @@
265 node = factory.make_Node()
266 mac_address = factory.make_mac_address()
267 interface_name = 'eth0'
268- vlan = factory.make_VLAN()
269+ fabric = factory.make_Fabric()
270+ vlan = fabric.get_default_vlan()
271 tags = [
272 factory.make_name("tag")
273 for _ in range(3)
274@@ -104,7 +107,8 @@
275
276 def test__requires_mac_address(self):
277 interface_name = 'eth0'
278- vlan = factory.make_VLAN()
279+ fabric = factory.make_Fabric()
280+ vlan = fabric.get_default_vlan()
281 form = PhysicalInterfaceForm(
282 node=factory.make_Node(),
283 data={
284@@ -119,7 +123,9 @@
285 form.errors['mac_address'][0])
286
287 def test_rejects_interface_with_duplicate_name(self):
288- interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
289+ fabric = factory.make_Fabric()
290+ vlan = fabric.get_default_vlan()
291+ interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan)
292 mac_address = factory.make_mac_address()
293 form = PhysicalInterfaceForm(
294 node=interface.node,
295@@ -135,9 +141,30 @@
296 "already has an interface named '%s'." % interface.name,
297 form.errors['name'][0])
298
299+ def test_rejects_interface_on_tagged_vlan(self):
300+ fabric = factory.make_Fabric()
301+ interface = factory.make_Interface(
302+ INTERFACE_TYPE.PHYSICAL, vlan=fabric.get_default_vlan())
303+ vlan = factory.make_VLAN(fabric=fabric)
304+ mac_address = factory.make_mac_address()
305+ form = PhysicalInterfaceForm(
306+ node=interface.node,
307+ data={
308+ 'name': factory.make_name("eth"),
309+ 'mac_address': mac_address,
310+ 'vlan': vlan.id,
311+ })
312+ self.assertFalse(form.is_valid(), form.errors)
313+ self.assertItemsEqual(
314+ ['vlan'], form.errors.keys(), form.errors)
315+ self.assertIn(
316+ "A physical interface can only belong to an untagged VLAN.",
317+ form.errors['vlan'][0])
318+
319 def test__rejects_parents(self):
320 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
321- vlan = factory.make_VLAN()
322+ fabric = factory.make_Fabric()
323+ vlan = fabric.get_default_vlan()
324 form = PhysicalInterfaceForm(
325 node=parent.node,
326 data={
327@@ -157,7 +184,8 @@
328 interface = factory.make_Interface(
329 INTERFACE_TYPE.PHYSICAL, name='eth0')
330 new_name = 'eth1'
331- new_vlan = factory.make_VLAN(vid=33)
332+ new_fabric = factory.make_Fabric()
333+ new_vlan = new_fabric.get_default_vlan()
334 form = PhysicalInterfaceForm(
335 instance=interface,
336 data={
337@@ -178,7 +206,8 @@
338 node = factory.make_Node()
339 mac_address = factory.make_mac_address()
340 interface_name = 'eth0'
341- vlan = factory.make_VLAN()
342+ fabric = factory.make_Fabric()
343+ vlan = fabric.get_default_vlan()
344 tags = [
345 factory.make_name("tag")
346 for _ in range(3)
347@@ -217,7 +246,8 @@
348 "autoconf": autoconf,
349 }
350 new_name = 'eth1'
351- new_vlan = factory.make_VLAN(vid=33)
352+ new_fabric = factory.make_Fabric()
353+ new_vlan = new_fabric.get_default_vlan()
354 form = PhysicalInterfaceForm(
355 instance=interface,
356 data={
357@@ -290,7 +320,7 @@
358
359 def test__creates_vlan_interface(self):
360 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
361- vlan = factory.make_VLAN(vid=10)
362+ vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
363 form = VLANInterfaceForm(
364 node=parent.node,
365 data={
366@@ -308,7 +338,7 @@
367
368 def test__create_ensures_link_up(self):
369 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
370- vlan = factory.make_VLAN(vid=10)
371+ vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
372 form = VLANInterfaceForm(
373 node=parent.node,
374 data={
375@@ -321,13 +351,10 @@
376 interface.ip_addresses.filter(alloc_type=IPADDRESS_TYPE.STICKY))
377
378 def test_rejects_interface_with_duplicate_name(self):
379- vlan = factory.make_VLAN(vid=10)
380- parent = factory.make_Interface(
381- INTERFACE_TYPE.PHYSICAL,
382- vlan=vlan)
383+ parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
384+ vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
385 interface = factory.make_Interface(
386- INTERFACE_TYPE.VLAN,
387- vlan=vlan, parents=[parent])
388+ INTERFACE_TYPE.VLAN, vlan=vlan, parents=[parent])
389 form = VLANInterfaceForm(
390 node=parent.node,
391 data={
392@@ -341,6 +368,22 @@
393 "already has an interface named '%s'." % interface.name,
394 form.errors['name'][0])
395
396+ def test_rejects_interface_on_default_fabric(self):
397+ parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
398+ vlan = parent.vlan.fabric.get_default_vlan()
399+ form = VLANInterfaceForm(
400+ node=parent.node,
401+ data={
402+ 'vlan': vlan.id,
403+ 'parents': [parent.id],
404+ })
405+ self.assertFalse(form.is_valid(), form.errors)
406+ self.assertItemsEqual(
407+ ['vlan'], form.errors.keys(), form.errors)
408+ self.assertIn(
409+ "A VLAN interface can only belong to a tagged VLAN.",
410+ form.errors['vlan'][0])
411+
412 def test__rejects_no_parents(self):
413 vlan = factory.make_VLAN(vid=10)
414 form = VLANInterfaceForm(
415@@ -356,13 +399,14 @@
416
417 def test__rejects_vlan_parent(self):
418 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
419+ vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
420 vlan_parent = factory.make_Interface(
421- INTERFACE_TYPE.VLAN, parents=[parent])
422- vlan = factory.make_VLAN(vid=10)
423+ INTERFACE_TYPE.VLAN, vlan=vlan, parents=[parent])
424+ other_vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=11)
425 form = VLANInterfaceForm(
426 node=parent.node,
427 data={
428- 'vlan': vlan.id,
429+ 'vlan': other_vlan.id,
430 'parents': [vlan_parent.id],
431 })
432 self.assertFalse(form.is_valid(), form.errors)
433@@ -371,10 +415,27 @@
434 "VLAN interface can't have another VLAN interface as parent.",
435 form.errors['parents'][0])
436
437+ def test__rejects_vlan_not_on_same_fabric(self):
438+ parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
439+ factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
440+ other_vlan = factory.make_VLAN()
441+ form = VLANInterfaceForm(
442+ node=parent.node,
443+ data={
444+ 'vlan': other_vlan.id,
445+ 'parents': [parent.id],
446+ })
447+ self.assertFalse(form.is_valid(), form.errors)
448+ self.assertItemsEqual(['vlan'], form.errors.keys())
449+ self.assertIn(
450+ "A VLAN interface can only belong to a tagged VLAN on "
451+ "the same fabric as its parent interface.",
452+ form.errors['vlan'][0])
453+
454 def test__rejects_parent_on_bond(self):
455 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
456- factory.make_Interface(INTERFACE_TYPE.BOND, parents=[parent])
457- vlan = factory.make_VLAN(vid=10)
458+ bond = factory.make_Interface(INTERFACE_TYPE.BOND, parents=[parent])
459+ vlan = factory.make_VLAN(fabric=bond.vlan.fabric, vid=10)
460 form = VLANInterfaceForm(
461 node=parent.node,
462 data={
463@@ -408,7 +469,7 @@
464 parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
465 interface = factory.make_Interface(
466 INTERFACE_TYPE.VLAN, parents=[parent])
467- new_vlan = factory.make_VLAN(vid=33)
468+ new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric, vid=33)
469 form = VLANInterfaceForm(
470 instance=interface,
471 data={
472@@ -429,15 +490,13 @@
473 def test__error_with_invalid_bond_mode(self):
474 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
475 parent2 = factory.make_Interface(
476- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
477+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
478 interface_name = factory.make_name()
479- vlan = factory.make_VLAN(vid=10)
480 bond_mode = factory.make_name("bond_mode")
481 form = BondInterfaceForm(
482 node=parent1.node,
483 data={
484 'name': interface_name,
485- 'vlan': vlan.id,
486 'parents': [parent1.id, parent2.id],
487 'bond_mode': bond_mode,
488 })
489@@ -451,14 +510,12 @@
490 def test__creates_bond_interface(self):
491 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
492 parent2 = factory.make_Interface(
493- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
494+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
495 interface_name = factory.make_name()
496- vlan = factory.make_VLAN(vid=10)
497 form = BondInterfaceForm(
498 node=parent1.node,
499 data={
500 'name': interface_name,
501- 'vlan': vlan.id,
502 'parents': [parent1.id, parent2.id],
503 })
504 self.assertTrue(form.is_valid(), form.errors)
505@@ -475,15 +532,13 @@
506 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
507 parent1.ensure_link_up()
508 parent2 = factory.make_Interface(
509- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
510+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
511 parent2.ensure_link_up()
512 interface_name = factory.make_name()
513- vlan = factory.make_VLAN(vid=10)
514 form = BondInterfaceForm(
515 node=parent1.node,
516 data={
517 'name': interface_name,
518- 'vlan': vlan.id,
519 'parents': [parent1.id, parent2.id],
520 })
521 self.assertTrue(form.is_valid(), form.errors)
522@@ -502,14 +557,12 @@
523 def test__creates_bond_interface_with_parent_mac_address(self):
524 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
525 parent2 = factory.make_Interface(
526- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
527+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
528 interface_name = factory.make_name()
529- vlan = factory.make_VLAN(vid=10)
530 form = BondInterfaceForm(
531 node=parent1.node,
532 data={
533 'name': interface_name,
534- 'vlan': vlan.id,
535 'parents': [parent1.id, parent2.id],
536 'mac_address': parent1.mac_address,
537 })
538@@ -525,14 +578,12 @@
539 def test__creates_bond_interface_with_default_bond_params(self):
540 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
541 parent2 = factory.make_Interface(
542- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
543+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
544 interface_name = factory.make_name()
545- vlan = factory.make_VLAN(vid=10)
546 form = BondInterfaceForm(
547 node=parent1.node,
548 data={
549 'name': interface_name,
550- 'vlan': vlan.id,
551 'parents': [parent1.id, parent2.id],
552 })
553 self.assertTrue(form.is_valid(), form.errors)
554@@ -549,9 +600,8 @@
555 def test__creates_bond_interface_with_bond_params(self):
556 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
557 parent2 = factory.make_Interface(
558- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
559+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
560 interface_name = factory.make_name()
561- vlan = factory.make_VLAN(vid=10)
562 bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
563 bond_miimon = random.randint(0, 1000)
564 bond_downdelay = random.randint(0, 1000)
565@@ -563,7 +613,6 @@
566 node=parent1.node,
567 data={
568 'name': interface_name,
569- 'vlan': vlan.id,
570 'parents': [parent1.id, parent2.id],
571 'bond_mode': bond_mode,
572 'bond_miimon': bond_miimon,
573@@ -584,13 +633,11 @@
574 }, interface.params)
575
576 def test__rejects_no_parents(self):
577- vlan = factory.make_VLAN(vid=10)
578 interface_name = factory.make_name()
579 form = BondInterfaceForm(
580 node=factory.make_Node(),
581 data={
582 'name': interface_name,
583- 'vlan': vlan.id,
584 })
585 self.assertFalse(form.is_valid(), form.errors)
586 self.assertItemsEqual(['parents', 'mac_address'], form.errors.keys())
587@@ -598,21 +645,37 @@
588 "A Bond interface must have one or more parents.",
589 form.errors['parents'][0])
590
591+ def test__rejects_when_vlan_not_untagged(self):
592+ interface_name = factory.make_name()
593+ parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
594+ vlan = factory.make_VLAN(fabric=parent.vlan.fabric)
595+ form = BondInterfaceForm(
596+ node=parent.node,
597+ data={
598+ 'name': interface_name,
599+ 'parents': [parent.id],
600+ 'mac_address': parent.mac_address,
601+ 'vlan': vlan.id,
602+ })
603+ self.assertFalse(form.is_valid(), form.errors)
604+ self.assertItemsEqual(['vlan'], form.errors.keys())
605+ self.assertIn(
606+ "A bond interface can only belong to an untagged VLAN.",
607+ form.errors['vlan'][0])
608+
609 def test__rejects_when_parents_already_have_children(self):
610 node = factory.make_Node()
611 parent1 = factory.make_Interface(
612 INTERFACE_TYPE.PHYSICAL, node=node, name="eth0")
613 factory.make_Interface(INTERFACE_TYPE.VLAN, parents=[parent1])
614 parent2 = factory.make_Interface(
615- INTERFACE_TYPE.PHYSICAL, node=node, name="eth1")
616+ INTERFACE_TYPE.PHYSICAL, node=node, name="eth1", vlan=parent1.vlan)
617 factory.make_Interface(INTERFACE_TYPE.VLAN, parents=[parent2])
618- vlan = factory.make_VLAN(vid=10)
619 interface_name = factory.make_name()
620 form = BondInterfaceForm(
621 node=node,
622 data={
623 'name': interface_name,
624- 'vlan': vlan.id,
625 'parents': [parent1.id, parent2.id]
626 })
627 self.assertFalse(form.is_valid(), form.errors)
628@@ -620,17 +683,36 @@
629 "eth0, eth1 is already in-use by another interface.",
630 form.errors['parents'][0])
631
632+ def test__rejects_when_parents_not_in_same_vlan(self):
633+ node = factory.make_Node()
634+ parent1 = factory.make_Interface(
635+ INTERFACE_TYPE.PHYSICAL, node=node, name="eth0")
636+ parent2 = factory.make_Interface(
637+ INTERFACE_TYPE.PHYSICAL, node=node, name="eth1")
638+ interface_name = factory.make_name()
639+ form = BondInterfaceForm(
640+ node=node,
641+ data={
642+ 'name': interface_name,
643+ 'parents': [parent1.id, parent2.id]
644+ })
645+ self.assertFalse(form.is_valid(), form.errors)
646+ self.assertEquals(
647+ "All parents must belong to the same VLAN.",
648+ form.errors['parents'][0])
649+
650 def test__edits_interface(self):
651 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
652 parent2 = factory.make_Interface(
653- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
654+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
655 interface = factory.make_Interface(
656 INTERFACE_TYPE.BOND,
657 parents=[parent1, parent2])
658- new_vlan = factory.make_VLAN(vid=33)
659+ new_fabric = factory.make_Fabric()
660+ new_vlan = new_fabric.get_default_vlan()
661 new_name = factory.make_name()
662 new_parent = factory.make_Interface(
663- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
664+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
665 form = BondInterfaceForm(
666 instance=interface,
667 data={
668@@ -647,6 +729,10 @@
669 vlan=new_vlan, type=INTERFACE_TYPE.BOND))
670 self.assertItemsEqual(
671 [parent1, parent2, new_parent], interface.parents.all())
672+ self.assertItemsEqual([new_vlan], set(
673+ reload_object(parent).vlan
674+ for parent in [parent1, parent2, new_parent]
675+ ))
676
677 def test__edits_interface_removes_parents(self):
678 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
679@@ -704,7 +790,7 @@
680 def test__edit_doesnt_overwrite_params(self):
681 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
682 parent2 = factory.make_Interface(
683- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
684+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
685 interface = factory.make_Interface(
686 INTERFACE_TYPE.BOND,
687 parents=[parent1, parent2])
688@@ -724,12 +810,10 @@
689 "bond_xmit_hash_policy": bond_xmit_hash_policy,
690 }
691 interface.save()
692- new_vlan = factory.make_VLAN(vid=33)
693 new_name = factory.make_name()
694 form = BondInterfaceForm(
695 instance=interface,
696 data={
697- 'vlan': new_vlan.id,
698 'name': new_name,
699 })
700 self.assertTrue(form.is_valid(), form.errors)
701@@ -746,7 +830,7 @@
702 def test__edit_does_overwrite_params(self):
703 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
704 parent2 = factory.make_Interface(
705- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
706+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
707 interface = factory.make_Interface(
708 INTERFACE_TYPE.BOND,
709 parents=[parent1, parent2])
710@@ -766,7 +850,6 @@
711 "bond_xmit_hash_policy": bond_xmit_hash_policy,
712 }
713 interface.save()
714- new_vlan = factory.make_VLAN(vid=33)
715 new_name = factory.make_name()
716 new_bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
717 new_bond_miimon = random.randint(0, 1000)
718@@ -778,7 +861,6 @@
719 form = BondInterfaceForm(
720 instance=interface,
721 data={
722- 'vlan': new_vlan.id,
723 'name': new_name,
724 'bond_mode': new_bond_mode,
725 'bond_miimon': new_bond_miimon,
726@@ -801,7 +883,7 @@
727 def test__edit_allows_zero_params(self):
728 parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
729 parent2 = factory.make_Interface(
730- INTERFACE_TYPE.PHYSICAL, node=parent1.node)
731+ INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
732 interface = factory.make_Interface(
733 INTERFACE_TYPE.BOND,
734 parents=[parent1, parent2])
735@@ -821,7 +903,6 @@
736 "bond_xmit_hash_policy": bond_xmit_hash_policy,
737 }
738 interface.save()
739- new_vlan = factory.make_VLAN(vid=33)
740 new_name = factory.make_name()
741 new_bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
742 new_bond_miimon = 0
743@@ -833,7 +914,6 @@
744 form = BondInterfaceForm(
745 instance=interface,
746 data={
747- 'vlan': new_vlan.id,
748 'name': new_name,
749 'bond_mode': new_bond_mode,
750 'bond_miimon': new_bond_miimon,
751
752=== modified file 'src/maasserver/websockets/handlers/tests/test_machine.py'
753--- src/maasserver/websockets/handlers/tests/test_machine.py 2016-03-02 10:23:02 +0000
754+++ src/maasserver/websockets/handlers/tests/test_machine.py 2016-03-03 21:30:31 +0000
755@@ -1843,7 +1843,8 @@
756 handler = MachineHandler(user, {})
757 name = factory.make_name("eth")
758 mac_address = factory.make_mac_address()
759- vlan = factory.make_VLAN()
760+ fabric = factory.make_Fabric()
761+ vlan = fabric.get_default_vlan()
762 handler.create_physical({
763 "system_id": node.system_id,
764 "name": name,
765@@ -1860,7 +1861,8 @@
766 handler = MachineHandler(user, {})
767 name = factory.make_name("eth")
768 mac_address = factory.make_mac_address()
769- vlan = factory.make_VLAN()
770+ fabric = factory.make_Fabric()
771+ vlan = fabric.get_default_vlan()
772 subnet = factory.make_Subnet(vlan=vlan)
773 handler.create_physical({
774 "system_id": node.system_id,
775@@ -1882,7 +1884,8 @@
776 handler = MachineHandler(user, {})
777 name = factory.make_name("eth")
778 mac_address = factory.make_mac_address()
779- vlan = factory.make_VLAN()
780+ fabric = factory.make_Fabric()
781+ vlan = fabric.get_default_vlan()
782 handler.create_physical({
783 "system_id": node.system_id,
784 "name": name,
785@@ -1902,7 +1905,8 @@
786 handler = MachineHandler(user, {})
787 name = factory.make_name("eth")
788 mac_address = factory.make_mac_address()
789- vlan = factory.make_VLAN()
790+ fabric = factory.make_Fabric()
791+ vlan = fabric.get_default_vlan()
792 subnet = factory.make_Subnet(vlan=vlan)
793 handler.create_physical({
794 "system_id": node.system_id,
795@@ -1923,7 +1927,7 @@
796 node = factory.make_Node()
797 handler = MachineHandler(user, {})
798 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
799- new_vlan = factory.make_VLAN()
800+ new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
801 handler.create_vlan({
802 "system_id": node.system_id,
803 "parent": interface.id,
804@@ -1939,7 +1943,7 @@
805 node = factory.make_Node()
806 handler = MachineHandler(user, {})
807 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
808- new_vlan = factory.make_VLAN()
809+ new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
810 new_subnet = factory.make_Subnet(vlan=new_vlan)
811 handler.create_vlan({
812 "system_id": node.system_id,
813@@ -1961,7 +1965,7 @@
814 node = factory.make_Node()
815 handler = MachineHandler(user, {})
816 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
817- new_vlan = factory.make_VLAN()
818+ new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
819 handler.create_vlan({
820 "system_id": node.system_id,
821 "parent": interface.id,
822@@ -1981,7 +1985,7 @@
823 node = factory.make_Node()
824 handler = MachineHandler(user, {})
825 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
826- new_vlan = factory.make_VLAN()
827+ new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
828 new_subnet = factory.make_Subnet(vlan=new_vlan)
829 handler.create_vlan({
830 "system_id": node.system_id,
831@@ -2041,7 +2045,8 @@
832 handler = MachineHandler(user, {})
833 interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
834 new_name = factory.make_name("name")
835- new_vlan = factory.make_VLAN()
836+ new_fabric = factory.make_Fabric()
837+ new_vlan = new_fabric.get_default_vlan()
838 handler.update_interface({
839 "system_id": node.system_id,
840 "interface_id": interface.id,