Merge lp:~blake-rouse/maas/fix-1566336-1.9 into lp:maas/1.9

[Blake Rouse]

Remove links on parents when added to a bond. Only allow physical interface and bond interface to be attached to the untagged VLAN on a fabric.

[Blake Rouse (blake-rouse)]

Self-approving backport.

review: Approve

=== modified file 'src/maasserver/api/tests/test_interfaces.py'
--- src/maasserver/api/tests/test_interfaces.py	2015-12-04 21:23:28 +0000
+++ src/maasserver/api/tests/test_interfaces.py	2016-04-05 16:02:24 +0000
@@ -56,14 +56,14 @@
 def make_complex_interface(node, name=None):
     """Makes interface with parents and children."""
     fabric = factory.make_Fabric()
-    vlan_5 = factory.make_VLAN(vid=5, fabric=fabric)
+    untagged = fabric.get_default_vlan()
     nic_0 = factory.make_Interface(
-        INTERFACE_TYPE.PHYSICAL, vlan=vlan_5, node=node)
+        INTERFACE_TYPE.PHYSICAL, vlan=untagged, node=node)
     nic_1 = factory.make_Interface(
-        INTERFACE_TYPE.PHYSICAL, vlan=vlan_5, node=node)
+        INTERFACE_TYPE.PHYSICAL, vlan=untagged, node=node)
     parents = [nic_0, nic_1]
     bond_interface = factory.make_Interface(
-        INTERFACE_TYPE.BOND, mac_address=nic_0.mac_address, vlan=vlan_5,
+        INTERFACE_TYPE.BOND, mac_address=nic_0.mac_address, vlan=untagged,
         parents=parents, name=name)
     vlan_10 = factory.make_VLAN(vid=10, fabric=fabric)
     vlan_nic_10 = factory.make_Interface(
@@ -117,7 +117,8 @@
             node = factory.make_Node(status=status)
             mac = factory.make_mac_address()
             name = factory.make_name("eth")
-            vlan = factory.make_VLAN()
+            fabric = factory.make_Fabric()
+            vlan = fabric.get_default_vlan()
             tags = [
                 factory.make_name("tag")
                 for _ in range(3)
@@ -150,7 +151,8 @@
             owner=self.logged_in_user, installable=False, parent=parent)
         mac = factory.make_mac_address()
         name = factory.make_name("eth")
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         tags = [
             factory.make_name("tag")
             for _ in range(3)
@@ -183,7 +185,8 @@
             node = factory.make_Node(status=status)
             mac = factory.make_mac_address()
             name = factory.make_name("eth")
-            vlan = factory.make_VLAN()
+            fabric = factory.make_Fabric()
+            vlan = fabric.get_default_vlan()
             tags = [
                 factory.make_name("tag")
                 for _ in range(3)
@@ -279,7 +282,8 @@
         interface_on_other_node = factory.make_Interface(
             INTERFACE_TYPE.PHYSICAL)
         name = factory.make_name("eth")
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         uri = get_interfaces_uri(node)
         response = self.client.post(uri, {
             "op": "create_physical",
@@ -299,7 +303,8 @@
         self.become_admin()
         for status in (NODE_STATUS.READY, NODE_STATUS.BROKEN):
             node = factory.make_Node(status=status)
-            vlan = factory.make_VLAN()
+            fabric = factory.make_Fabric()
+            vlan = fabric.get_default_vlan()
             parent_1_iface = factory.make_Interface(
                 INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=node)
             parent_2_iface = factory.make_Interface(
@@ -402,7 +407,7 @@
             self.assertEqual(
                 httplib.CONFLICT, response.status_code, response.content)
 
-    def test_create_bond_requires_name_vlan_and_parents(self):
+    def test_create_bond_requires_name_and_parents(self):
         self.become_admin()
         node = factory.make_Node(status=NODE_STATUS.READY)
         uri = get_interfaces_uri(node)
@@ -415,7 +420,6 @@
         self.assertEquals({
             "mac_address": ["This field cannot be blank."],
             "name": ["This field is required."],
-            "vlan": ["This field is required."],
             "parents": ["A Bond interface must have one or more parents."],
             }, json.loads(response.content))
 
@@ -634,7 +638,8 @@
             interface = factory.make_Interface(
                 INTERFACE_TYPE.PHYSICAL, node=node)
             new_name = factory.make_name("name")
-            new_vlan = factory.make_VLAN()
+            new_fabric = factory.make_Fabric()
+            new_vlan = new_fabric.get_default_vlan()
             uri = get_interface_uri(interface)
             response = self.client.put(uri, {
                 "name": new_name,
@@ -653,7 +658,8 @@
         interface = factory.make_Interface(
             INTERFACE_TYPE.PHYSICAL, node=device)
         new_name = factory.make_name("name")
-        new_vlan = factory.make_VLAN()
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
         uri = get_interface_uri(interface)
         response = self.client.put(uri, {
             "name": new_name,
@@ -688,9 +694,11 @@
                 node)
             physical_interface = factory.make_Interface(
                 INTERFACE_TYPE.PHYSICAL, node=node)
+            new_vlan = factory.make_VLAN(fabric=physical_interface.vlan.fabric)
             uri = get_interface_uri(vlan_10)
             response = self.client.put(uri, {
                 "parent": physical_interface.id,
+                "vlan": new_vlan.id,
                 })
             self.assertEqual(
                 httplib.OK, response.status_code, response.content)
 
=== modified file 'src/maasserver/forms_interface.py'
--- src/maasserver/forms_interface.py	2015-10-31 23:55:19 +0000
+++ src/maasserver/forms_interface.py	2016-04-05 16:02:24 +0000
@@ -196,6 +196,13 @@
             msg = "A physical interface cannot have parents."
             raise ValidationError({'parents': [msg]})
 
+    def clean_vlan(self):
+        new_vlan = self.cleaned_data.get('vlan')
+        if new_vlan and new_vlan.fabric.get_default_vlan() != new_vlan:
+            raise ValidationError(
+                "A physical interface can only belong to an untagged VLAN.")
+        return new_vlan
+
     def clean(self):
         cleaned_data = super(PhysicalInterfaceForm, self).clean()
         new_name = cleaned_data.get('name')
@@ -233,11 +240,25 @@
                 "in a bond.")
         return parents
 
+    def clean_vlan(self):
+        new_vlan = self.cleaned_data.get('vlan')
+        if new_vlan and new_vlan.fabric.get_default_vlan() == new_vlan:
+            raise ValidationError(
+                "A VLAN interface can only belong to a tagged VLAN.")
+        return new_vlan
+
     def clean(self):
         cleaned_data = super(VLANInterfaceForm, self).clean()
         if self.fields_ok(['vlan', 'parents']):
             new_vlan = self.cleaned_data.get('vlan')
             if new_vlan:
+                # VLAN needs to be the in the same fabric as the parent.
+                parent = self.cleaned_data.get('parents')[0]
+                if parent.vlan.fabric_id != new_vlan.fabric_id:
+                    set_form_error(
+                        self, "vlan",
+                        "A VLAN interface can only belong to a tagged VLAN on "
+                        "the same fabric as its parent interface.")
                 name = build_vlan_interface_name(
                     self.cleaned_data.get('parents').first(), new_vlan)
                 self.clean_interface_name_uniqueness(name)
@@ -281,6 +302,15 @@
             'name',
         )
 
+    def __init__(self, *args, **kwargs):
+        super(BondInterfaceForm, self).__init__(*args, **kwargs)
+        # Allow VLAN to be blank when creating.
+        instance = kwargs.get("instance", None)
+        if instance is not None and instance.id is not None:
+            self.fields['vlan'].required = True
+        else:
+            self.fields['vlan'].required = False
+
     def clean_parents(self):
         parents = self.get_clean_parents()
         if parents is None:
@@ -290,9 +320,16 @@
             raise ValidationError({'parents': [msg]})
         return parents
 
+    def clean_vlan(self):
+        new_vlan = self.cleaned_data.get('vlan')
+        if new_vlan and new_vlan.fabric.get_default_vlan() != new_vlan:
+            raise ValidationError(
+                "A bond interface can only belong to an untagged VLAN.")
+        return new_vlan
+
     def clean(self):
         cleaned_data = super(BondInterfaceForm, self).clean()
-        if self.fields_ok(['parents']):
+        if self.fields_ok(['vlan', 'parents']):
             parents = self.cleaned_data.get('parents')
             # Set the mac_address if its missing and the interface is being
             # created.
@@ -333,6 +370,23 @@
                         self, 'parents',
                         "%s is already in-use by another interface." % (
                             ', '.join(sorted(parents_with_other_children))))
+
+                # When creating the bond set VLAN to the same as the parents
+                # and check that the parents all belong to the same VLAN.
+                if self.instance.id is None:
+                    vlan = self.cleaned_data.get('vlan')
+                    if vlan is None:
+                        vlan = parents[0].vlan
+                        self.cleaned_data['vlan'] = vlan
+                    parent_vlans = {
+                        parent.vlan
+                        for parent in parents
+                    }
+                    if parent_vlans != set([vlan]):
+                        set_form_error(
+                            self, 'parents',
+                            "All parents must belong to the same VLAN.")
+
         return cleaned_data
 
     def set_extra_parameters(self, interface, created):
@@ -356,17 +410,6 @@
             elif created:
                 interface.params[bond_field] = self.fields[bond_field].initial
 
-    def save(self, *args, **kwargs):
-        """Persist the interface into the database."""
-        created = self.instance.id is None
-        interface = super(BondInterfaceForm, self).save()
-        if created:
-            # Bond was created we remove all the links on the parent interfaces
-            # and ensure that the bond has atleast a LINK_UP.
-            for parent in interface.parents.all():
-                parent.clear_all_links(clearing_config=True)
-        return interface
-
 
 INTERFACE_FORM_MAPPING = {
     INTERFACE_TYPE.PHYSICAL: PhysicalInterfaceForm,
 
=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2015-12-11 00:24:18 +0000
+++ src/maasserver/models/interface.py	2016-04-05 16:02:24 +0000
@@ -922,11 +922,12 @@
             # Nothing to do, already has links.
             return
         else:
-            # Use an associated subnet if it exists, else it will just be a
+            # Use an associated subnet if it exists and its on the same VLAN
+            # the interface is currently connected, else it will just be a
             # LINK_UP without a subnet.
             discovered_address = self.ip_addresses.filter(
                 alloc_type=IPADDRESS_TYPE.DISCOVERED,
-                subnet__isnull=False).first()
+                subnet__vlan=self.vlan).first()
             if discovered_address is not None:
                 subnet = discovered_address.subnet
             else:
 
=== modified file 'src/maasserver/models/signals/interfaces.py'
--- src/maasserver/models/signals/interfaces.py	2015-11-03 12:38:17 +0000
+++ src/maasserver/models/signals/interfaces.py	2016-04-05 16:02:24 +0000
@@ -14,6 +14,7 @@
 __metaclass__ = type
 __all__ = []
 
+from django.db.models.signals import post_save
 from maasserver.enum import (
     INTERFACE_TYPE,
     IPADDRESS_TYPE,
@@ -27,6 +28,14 @@
 from maasserver.utils.signals import connect_to_field_change
 
 
+INTERFACE_CLASSES = [
+    Interface,
+    PhysicalInterface,
+    BondInterface,
+    VLANInterface,
+]
+
+
 def interface_enabled_or_disabled(instance, old_values, **kwargs):
     """When an interface is enabled be sure at minimum a LINK_UP is created.
     When an interface is disabled make sure that all its links are removed,
@@ -52,12 +61,10 @@
                         ip_address, clearing_config=True)
 
 
-connect_to_field_change(
-    interface_enabled_or_disabled,
-    Interface, ['enabled'], delete=False)
-connect_to_field_change(
-    interface_enabled_or_disabled,
-    PhysicalInterface, ['enabled'], delete=False)
+for klass in INTERFACE_CLASSES:
+    connect_to_field_change(
+        interface_enabled_or_disabled,
+        klass, ['enabled'], delete=False)
 
 
 def interface_mtu_params_update(instance, old_values, **kwargs):
@@ -118,15 +125,48 @@
             parent.save()
 
 
-connect_to_field_change(
-    interface_mtu_params_update,
-    Interface, ['params'], delete=False)
-connect_to_field_change(
-    interface_mtu_params_update,
-    PhysicalInterface, ['params'], delete=False)
-connect_to_field_change(
-    interface_mtu_params_update,
-    BondInterface, ['params'], delete=False)
-connect_to_field_change(
-    interface_mtu_params_update,
-    VLANInterface, ['params'], delete=False)
+for klass in INTERFACE_CLASSES:
+    connect_to_field_change(
+        interface_mtu_params_update,
+        klass, ['params'], delete=False)
+
+
+def update_bond_parents(sender, instance, created, **kwargs):
+    """Update bond parents when interface created."""
+    if instance.type == INTERFACE_TYPE.BOND:
+        for parent in instance.parents.all():
+            # Make sure the parent has not links as well, just to be sure.
+            parent.clear_all_links(clearing_config=True)
+            if parent.vlan != instance.vlan:
+                parent.vlan = instance.vlan
+                parent.save()
+
+
+for klass in INTERFACE_CLASSES:
+    post_save.connect(
+        update_bond_parents, sender=klass)
+
+
+def interface_vlan_update(instance, old_values, **kwargs):
+    """When an interfaces VLAN is changed we need to remove all
+    links if the VLAN is different.
+    """
+    new_vlan_id = instance.vlan_id
+    [old_vlan_id] = old_values
+    if new_vlan_id == old_vlan_id:
+        # Nothing changed do nothing.
+        return
+    if instance.node is None:
+        # Not assigned to a node. Nothing to do.
+        return
+
+    # Interface VLAN was changed on a machine or device. Remove all its
+    # links except the DISCOVERED ones.
+    instance.ip_addresses.exclude(
+        alloc_type=IPADDRESS_TYPE.DISCOVERED).delete()
+
+
+for klass in INTERFACE_CLASSES:
+    connect_to_field_change(
+        interface_vlan_update,
+        klass, ['vlan_id'], delete=False)
 
=== modified file 'src/maasserver/models/signals/tests/test_interfaces.py'
--- src/maasserver/models/signals/tests/test_interfaces.py	2015-11-03 12:38:17 +0000
+++ src/maasserver/models/signals/tests/test_interfaces.py	2016-04-05 16:02:24 +0000
@@ -132,3 +132,49 @@
         self.assertEquals({
             'mtu': bond_mtu,
             }, reload_object(physical3_interface).params)
+
+
+class TestUpdateBondParents(MAASServerTestCase):
+
+    def test__updates_bond_parents(self):
+        parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+        bond = factory.make_Interface(
+            INTERFACE_TYPE.BOND, parents=[parent1, parent2])
+        self.assertEqual(bond.vlan, reload_object(parent1).vlan)
+        self.assertEqual(bond.vlan, reload_object(parent2).vlan)
+
+    def test__update_bond_clears_parent_links(self):
+        parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+        static_ip = factory.make_StaticIPAddress(interface=parent1)
+        factory.make_Interface(
+            INTERFACE_TYPE.BOND, parents=[parent1, parent2])
+        self.assertIsNone(reload_object(static_ip))
+
+
+class TestInterfaceVLANUpdate(MAASServerTestCase):
+
+    scenarios = (
+        ("node", {
+            "maker": factory.make_Node,
+        }),
+        ("device", {
+            "maker": factory.make_Device,
+        }),
+    )
+
+    def test__removes_links(self):
+        node = self.maker()
+        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
+        static_ip = factory.make_StaticIPAddress(interface=interface)
+        discovered_ip = factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, interface=interface)
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
+        interface.vlan = new_vlan
+        interface.save()
+        self.assertIsNone(reload_object(static_ip))
+        self.assertIsNotNone(reload_object(discovered_ip))
 
=== modified file 'src/maasserver/models/tests/test_interface.py'
--- src/maasserver/models/tests/test_interface.py	2015-12-11 00:24:23 +0000
+++ src/maasserver/models/tests/test_interface.py	2016-04-05 16:02:24 +0000
@@ -230,31 +230,51 @@
                 ["id:%s" % iface1.id, "id:%s" % iface2.id]), [iface1, iface2])
 
     def test__filter_by_specifiers_matches_vid(self):
-        iface1 = factory.make_Interface()
-        iface2 = factory.make_Interface()
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                "vid:%s" % iface1.vlan.vid), [iface1])
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                "vid:%s" % iface2.vlan.vid), [iface2])
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                ["vid:%s" % iface1.vlan.vid, "vid:%s" % iface2.vlan.vid]),
+        fabric1 = factory.make_Fabric()
+        parent1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=fabric1.get_default_vlan())
+        vlan1 = factory.make_VLAN(fabric=fabric1)
+        iface1 = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, vlan=vlan1, parents=[parent1])
+        fabric2 = factory.make_Fabric()
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=fabric2.get_default_vlan())
+        vlan2 = factory.make_VLAN(fabric=fabric2)
+        iface2 = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, vlan=vlan2, parents=[parent2])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                "vid:%s" % vlan1.vid), [iface1])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                "vid:%s" % vlan2.vid), [iface2])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                ["vid:%s" % vlan1.vid, "vid:%s" % vlan2.vid]),
             [iface1, iface2])
 
     def test__filter_by_specifiers_matches_vlan(self):
-        iface1 = factory.make_Interface()
-        iface2 = factory.make_Interface()
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                "vlan:%s" % iface1.vlan.vid), [iface1])
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                "vlan:%s" % iface2.vlan.vid), [iface2])
-        self.assertItemsEqual(
-            Interface.objects.filter_by_specifiers(
-                ["vlan:%s" % iface1.vlan.vid, "vlan:%s" % iface2.vlan.vid]),
+        fabric1 = factory.make_Fabric()
+        parent1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=fabric1.get_default_vlan())
+        vlan1 = factory.make_VLAN(fabric=fabric1)
+        iface1 = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, vlan=vlan1, parents=[parent1])
+        fabric2 = factory.make_Fabric()
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=fabric2.get_default_vlan())
+        vlan2 = factory.make_VLAN(fabric=fabric2)
+        iface2 = factory.make_Interface(
+            INTERFACE_TYPE.VLAN, vlan=vlan2, parents=[parent2])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                "vlan:%s" % vlan1.vid), [iface1])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                "vlan:%s" % vlan2.vid), [iface2])
+        self.assertItemsEqual(
+            Interface.objects.filter_by_specifiers(
+                ["vlan:%s" % vlan1.vid, "vlan:%s" % vlan2.vid]),
             [iface1, iface2])
 
     def test__filter_by_specifiers_matches_subnet_specifier(self):
@@ -1515,7 +1535,7 @@
             1, interface.ip_addresses.count(),
             "Should only have one IP address assigned.")
 
-    def test__creates_link_up_to_discovered_subnet(self):
+    def test__creates_link_up_to_discovered_subnet_on_same_vlan(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         subnet = factory.make_Subnet(vlan=interface.vlan)
         factory.make_StaticIPAddress(
@@ -1527,6 +1547,18 @@
         self.assertIsNone(link_ip.ip)
         self.assertEquals(subnet, link_ip.subnet)
 
+    def test__creates_link_up_to_no_subnet_when_on_different_vlan(self):
+        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        subnet = factory.make_Subnet()
+        factory.make_StaticIPAddress(
+            alloc_type=IPADDRESS_TYPE.DISCOVERED, ip="",
+            subnet=subnet, interface=interface)
+        interface.ensure_link_up()
+        link_ip = interface.ip_addresses.filter(
+            alloc_type=IPADDRESS_TYPE.STICKY).first()
+        self.assertIsNone(link_ip.ip)
+        self.assertIsNone(link_ip.subnet)
+
     def test__creates_link_up_to_no_subnet(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         interface.ensure_link_up()
 
=== modified file 'src/maasserver/models/tests/test_staticipaddress.py'
--- src/maasserver/models/tests/test_staticipaddress.py	2015-11-17 00:33:35 +0000
+++ src/maasserver/models/tests/test_staticipaddress.py	2016-04-05 16:02:24 +0000
@@ -805,6 +805,7 @@
 
     def test_get_hostname_ip_mapping_prefers_bond_with_no_boot_interface(self):
         self.patch_autospec(interface_module, "update_host_maps")
+        self.patch_autospec(interface_module, "remove_host_maps")
         subnet = factory.make_Subnet(
             cidr=unicode(factory.make_ipv4_network().cidr))
         node = factory.make_Node_with_Interface_on_Subnet(
@@ -835,6 +836,7 @@
 
     def test_get_hostname_ip_mapping_prefers_bond_with_boot_interface(self):
         self.patch_autospec(interface_module, "update_host_maps")
+        self.patch_autospec(interface_module, "remove_host_maps")
         subnet = factory.make_Subnet(
             cidr=unicode(factory.make_ipv4_network().cidr))
         node = factory.make_Node_with_Interface_on_Subnet(
@@ -859,6 +861,7 @@
 
     def test_get_hostname_ip_mapping_ignores_bond_without_boot_interface(self):
         self.patch_autospec(interface_module, "update_host_maps")
+        self.patch_autospec(interface_module, "remove_host_maps")
         subnet = factory.make_Subnet(
             cidr=unicode(factory.make_ipv4_network().cidr))
         node = factory.make_Node_with_Interface_on_Subnet(
 
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2015-12-11 00:24:23 +0000
+++ src/maasserver/testing/factory.py	2016-04-05 16:02:24 +0000
@@ -335,7 +335,6 @@
             acquired = node.status in ALLOCATED_NODE_STATUSES
             self.make_Filesystem(
                 partition=root_partition, mount_point='/', acquired=acquired)
-
         # Update the 'updated'/'created' fields with a call to 'update'
         # preventing a call to save() from overriding the values.
         if updated is not None:
@@ -591,7 +590,9 @@
         node = self.make_Node(
             nodegroup=nodegroup, fabric=fabric, **kwargs)
         if vlan is None:
-            vlan = self.make_VLAN(fabric=fabric)
+            if fabric is None:
+                fabric = factory.make_Fabric()
+            vlan = fabric.get_default_vlan()
         if subnet is None:
             subnet = self.make_Subnet(vlan=vlan, cidr=cidr)
         # Check if the subnet already has a managed interface.
@@ -630,7 +631,7 @@
                 self.make_StaticIPAddress(
                     alloc_type=IPADDRESS_TYPE.STICKY, ip="",
                     subnet=subnet, interface=interface)
-        return node
+        return reload_object(node)
 
     UNDEFINED = float('NaN')
 
@@ -786,7 +787,19 @@
         if iftype is None:
             iftype = INTERFACE_TYPE.PHYSICAL
         if vlan is None:
-            vlan = self.make_VLAN(fabric=fabric)
+            if fabric is not None:
+                if iftype == INTERFACE_TYPE.VLAN:
+                    vlan = self.make_VLAN(fabric=fabric)
+                else:
+                    vlan = fabric.get_default_vlan()
+            else:
+                if iftype == INTERFACE_TYPE.VLAN and parents:
+                    vlan = self.make_VLAN(fabric=parents[0].vlan.fabric)
+                elif iftype == INTERFACE_TYPE.BOND and parents:
+                    vlan = parents[0].vlan
+                else:
+                    fabric = self.make_Fabric()
+                    vlan = fabric.get_default_vlan()
         if (mac_address is None and
                 iftype in [
                     INTERFACE_TYPE.PHYSICAL,
 
=== modified file 'src/maasserver/tests/test_forms_interface.py'
--- src/maasserver/tests/test_forms_interface.py	2015-10-31 23:55:19 +0000
+++ src/maasserver/tests/test_forms_interface.py	2016-04-05 16:02:24 +0000
@@ -32,6 +32,7 @@
 )
 from maasserver.models.interface import build_vlan_interface_name
 from maasserver.testing.factory import factory
+from maasserver.testing.orm import reload_object
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.forms import compose_invalid_choice_text
 from testtools import ExpectedException
@@ -67,7 +68,8 @@
         node = factory.make_Node()
         mac_address = factory.make_mac_address()
         interface_name = 'eth0'
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         tags = [
             factory.make_name("tag")
             for _ in range(3)
@@ -93,7 +95,8 @@
         node = factory.make_Node()
         mac_address = factory.make_mac_address()
         interface_name = 'eth0'
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         tags = [
             factory.make_name("tag")
             for _ in range(3)
@@ -113,7 +116,8 @@
 
     def test__requires_mac_address(self):
         interface_name = 'eth0'
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         form = PhysicalInterfaceForm(
             node=factory.make_Node(),
             data={
@@ -128,7 +132,9 @@
             form.errors['mac_address'][0])
 
     def test_rejects_interface_with_duplicate_name(self):
-        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
+        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, vlan=vlan)
         mac_address = factory.make_mac_address()
         form = PhysicalInterfaceForm(
             node=interface.node,
@@ -144,9 +150,30 @@
             "already has an interface named '%s'." % interface.name,
             form.errors['name'][0])
 
+    def test_rejects_interface_on_tagged_vlan(self):
+        fabric = factory.make_Fabric()
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, vlan=fabric.get_default_vlan())
+        vlan = factory.make_VLAN(fabric=fabric)
+        mac_address = factory.make_mac_address()
+        form = PhysicalInterfaceForm(
+            node=interface.node,
+            data={
+                'name': factory.make_name("eth"),
+                'mac_address': mac_address,
+                'vlan': vlan.id,
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(
+            ['vlan'], form.errors.keys(), form.errors)
+        self.assertIn(
+            "A physical interface can only belong to an untagged VLAN.",
+            form.errors['vlan'][0])
+
     def test__rejects_parents(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         form = PhysicalInterfaceForm(
             node=parent.node,
             data={
@@ -166,7 +193,8 @@
         interface = factory.make_Interface(
             INTERFACE_TYPE.PHYSICAL, name='eth0')
         new_name = 'eth1'
-        new_vlan = factory.make_VLAN(vid=33)
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
         form = PhysicalInterfaceForm(
             instance=interface,
             data={
@@ -187,7 +215,8 @@
         node = factory.make_Node()
         mac_address = factory.make_mac_address()
         interface_name = 'eth0'
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         tags = [
             factory.make_name("tag")
             for _ in range(3)
@@ -226,7 +255,8 @@
             "autoconf": autoconf,
         }
         new_name = 'eth1'
-        new_vlan = factory.make_VLAN(vid=33)
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
         form = PhysicalInterfaceForm(
             instance=interface,
             data={
@@ -299,7 +329,7 @@
 
     def test__creates_vlan_interface(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
-        vlan = factory.make_VLAN(vid=10)
+        vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
         form = VLANInterfaceForm(
             node=parent.node,
             data={
@@ -317,7 +347,7 @@
 
     def test__create_ensures_link_up(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
-        vlan = factory.make_VLAN(vid=10)
+        vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
         form = VLANInterfaceForm(
             node=parent.node,
             data={
@@ -330,13 +360,10 @@
             interface.ip_addresses.filter(alloc_type=IPADDRESS_TYPE.STICKY))
 
     def test_rejects_interface_with_duplicate_name(self):
-        vlan = factory.make_VLAN(vid=10)
-        parent = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL,
-            vlan=vlan)
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
         interface = factory.make_Interface(
-            INTERFACE_TYPE.VLAN,
-            vlan=vlan, parents=[parent])
+            INTERFACE_TYPE.VLAN, vlan=vlan, parents=[parent])
         form = VLANInterfaceForm(
             node=parent.node,
             data={
@@ -350,6 +377,22 @@
             "already has an interface named '%s'." % interface.name,
             form.errors['name'][0])
 
+    def test_rejects_interface_on_default_fabric(self):
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        vlan = parent.vlan.fabric.get_default_vlan()
+        form = VLANInterfaceForm(
+            node=parent.node,
+            data={
+                'vlan': vlan.id,
+                'parents': [parent.id],
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(
+            ['vlan'], form.errors.keys(), form.errors)
+        self.assertIn(
+            "A VLAN interface can only belong to a tagged VLAN.",
+            form.errors['vlan'][0])
+
     def test__rejects_no_parents(self):
         vlan = factory.make_VLAN(vid=10)
         form = VLANInterfaceForm(
@@ -365,13 +408,14 @@
 
     def test__rejects_vlan_parent(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
         vlan_parent = factory.make_Interface(
-            INTERFACE_TYPE.VLAN, parents=[parent])
-        vlan = factory.make_VLAN(vid=10)
+            INTERFACE_TYPE.VLAN, vlan=vlan, parents=[parent])
+        other_vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=11)
         form = VLANInterfaceForm(
             node=parent.node,
             data={
-                'vlan': vlan.id,
+                'vlan': other_vlan.id,
                 'parents': [vlan_parent.id],
             })
         self.assertFalse(form.is_valid(), form.errors)
@@ -380,10 +424,27 @@
             "VLAN interface can't have another VLAN interface as parent.",
             form.errors['parents'][0])
 
+    def test__rejects_vlan_not_on_same_fabric(self):
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
+        other_vlan = factory.make_VLAN()
+        form = VLANInterfaceForm(
+            node=parent.node,
+            data={
+                'vlan': other_vlan.id,
+                'parents': [parent.id],
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(['vlan'], form.errors.keys())
+        self.assertIn(
+            "A VLAN interface can only belong to a tagged VLAN on "
+            "the same fabric as its parent interface.",
+            form.errors['vlan'][0])
+
     def test__rejects_parent_on_bond(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
-        factory.make_Interface(INTERFACE_TYPE.BOND, parents=[parent])
-        vlan = factory.make_VLAN(vid=10)
+        bond = factory.make_Interface(INTERFACE_TYPE.BOND, parents=[parent])
+        vlan = factory.make_VLAN(fabric=bond.vlan.fabric, vid=10)
         form = VLANInterfaceForm(
             node=parent.node,
             data={
@@ -417,7 +478,7 @@
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         interface = factory.make_Interface(
             INTERFACE_TYPE.VLAN, parents=[parent])
-        new_vlan = factory.make_VLAN(vid=33)
+        new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric, vid=33)
         form = VLANInterfaceForm(
             instance=interface,
             data={
@@ -438,15 +499,13 @@
     def test__error_with_invalid_bond_mode(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         bond_mode = factory.make_name("bond_mode")
         form = BondInterfaceForm(
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
                 'bond_mode': bond_mode,
             })
@@ -460,14 +519,12 @@
     def test__creates_bond_interface(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         form = BondInterfaceForm(
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
             })
         self.assertTrue(form.is_valid(), form.errors)
@@ -484,15 +541,13 @@
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent1.ensure_link_up()
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         parent2.ensure_link_up()
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         form = BondInterfaceForm(
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
             })
         self.assertTrue(form.is_valid(), form.errors)
@@ -511,14 +566,12 @@
     def test__creates_bond_interface_with_parent_mac_address(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         form = BondInterfaceForm(
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
                 'mac_address': parent1.mac_address,
             })
@@ -534,14 +587,12 @@
     def test__creates_bond_interface_with_default_bond_params(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         form = BondInterfaceForm(
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
             })
         self.assertTrue(form.is_valid(), form.errors)
@@ -558,9 +609,8 @@
     def test__creates_bond_interface_with_bond_params(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface_name = factory.make_name()
-        vlan = factory.make_VLAN(vid=10)
         bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
         bond_miimon = random.randint(0, 1000)
         bond_downdelay = random.randint(0, 1000)
@@ -572,7 +622,6 @@
             node=parent1.node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id],
                 'bond_mode': bond_mode,
                 'bond_miimon': bond_miimon,
@@ -593,13 +642,11 @@
             }, interface.params)
 
     def test__rejects_no_parents(self):
-        vlan = factory.make_VLAN(vid=10)
         interface_name = factory.make_name()
         form = BondInterfaceForm(
             node=factory.make_Node(),
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
             })
         self.assertFalse(form.is_valid(), form.errors)
         self.assertItemsEqual(['parents', 'mac_address'], form.errors.keys())
@@ -607,21 +654,37 @@
             "A Bond interface must have one or more parents.",
             form.errors['parents'][0])
 
+    def test__rejects_when_vlan_not_untagged(self):
+        interface_name = factory.make_name()
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        vlan = factory.make_VLAN(fabric=parent.vlan.fabric)
+        form = BondInterfaceForm(
+            node=parent.node,
+            data={
+                'name': interface_name,
+                'parents': [parent.id],
+                'mac_address': parent.mac_address,
+                'vlan': vlan.id,
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(['vlan'], form.errors.keys())
+        self.assertIn(
+            "A bond interface can only belong to an untagged VLAN.",
+            form.errors['vlan'][0])
+
     def test__rejects_when_parents_already_have_children(self):
         node = factory.make_Node()
         parent1 = factory.make_Interface(
             INTERFACE_TYPE.PHYSICAL, node=node, name="eth0")
         factory.make_Interface(INTERFACE_TYPE.VLAN, parents=[parent1])
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=node, name="eth1")
+            INTERFACE_TYPE.PHYSICAL, node=node, name="eth1", vlan=parent1.vlan)
         factory.make_Interface(INTERFACE_TYPE.VLAN, parents=[parent2])
-        vlan = factory.make_VLAN(vid=10)
         interface_name = factory.make_name()
         form = BondInterfaceForm(
             node=node,
             data={
                 'name': interface_name,
-                'vlan': vlan.id,
                 'parents': [parent1.id, parent2.id]
             })
         self.assertFalse(form.is_valid(), form.errors)
@@ -629,17 +692,36 @@
             "eth0, eth1 is already in-use by another interface.",
             form.errors['parents'][0])
 
+    def test__rejects_when_parents_not_in_same_vlan(self):
+        node = factory.make_Node()
+        parent1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name="eth0")
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node, name="eth1")
+        interface_name = factory.make_name()
+        form = BondInterfaceForm(
+            node=node,
+            data={
+                'name': interface_name,
+                'parents': [parent1.id, parent2.id]
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertEquals(
+            "All parents must belong to the same VLAN.",
+            form.errors['parents'][0])
+
     def test__edits_interface(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface = factory.make_Interface(
             INTERFACE_TYPE.BOND,
             parents=[parent1, parent2])
-        new_vlan = factory.make_VLAN(vid=33)
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
         new_name = factory.make_name()
         new_parent = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         form = BondInterfaceForm(
             instance=interface,
             data={
@@ -656,6 +738,10 @@
                 vlan=new_vlan, type=INTERFACE_TYPE.BOND))
         self.assertItemsEqual(
             [parent1, parent2, new_parent], interface.parents.all())
+        self.assertItemsEqual([new_vlan], set(
+            reload_object(parent).vlan
+            for parent in [parent1, parent2, new_parent]
+        ))
 
     def test__edits_interface_removes_parents(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
@@ -713,7 +799,7 @@
     def test__edit_doesnt_overwrite_params(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface = factory.make_Interface(
             INTERFACE_TYPE.BOND,
             parents=[parent1, parent2])
@@ -733,12 +819,10 @@
             "bond_xmit_hash_policy": bond_xmit_hash_policy,
         }
         interface.save()
-        new_vlan = factory.make_VLAN(vid=33)
         new_name = factory.make_name()
         form = BondInterfaceForm(
             instance=interface,
             data={
-                'vlan': new_vlan.id,
                 'name': new_name,
             })
         self.assertTrue(form.is_valid(), form.errors)
@@ -755,7 +839,7 @@
     def test__edit_does_overwrite_params(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface = factory.make_Interface(
             INTERFACE_TYPE.BOND,
             parents=[parent1, parent2])
@@ -775,7 +859,6 @@
             "bond_xmit_hash_policy": bond_xmit_hash_policy,
         }
         interface.save()
-        new_vlan = factory.make_VLAN(vid=33)
         new_name = factory.make_name()
         new_bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
         new_bond_miimon = random.randint(0, 1000)
@@ -787,7 +870,6 @@
         form = BondInterfaceForm(
             instance=interface,
             data={
-                'vlan': new_vlan.id,
                 'name': new_name,
                 'bond_mode': new_bond_mode,
                 'bond_miimon': new_bond_miimon,
@@ -810,7 +892,7 @@
     def test__edit_allows_zero_params(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
-            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
         interface = factory.make_Interface(
             INTERFACE_TYPE.BOND,
             parents=[parent1, parent2])
@@ -830,7 +912,6 @@
             "bond_xmit_hash_policy": bond_xmit_hash_policy,
         }
         interface.save()
-        new_vlan = factory.make_VLAN(vid=33)
         new_name = factory.make_name()
         new_bond_mode = factory.pick_choice(BOND_MODE_CHOICES)
         new_bond_miimon = 0
@@ -842,7 +923,6 @@
         form = BondInterfaceForm(
             instance=interface,
             data={
-                'vlan': new_vlan.id,
                 'name': new_name,
                 'bond_mode': new_bond_mode,
                 'bond_miimon': new_bond_miimon,
 
=== modified file 'src/maasserver/websockets/handlers/tests/test_node.py'
--- src/maasserver/websockets/handlers/tests/test_node.py	2015-11-18 18:14:30 +0000
+++ src/maasserver/websockets/handlers/tests/test_node.py	2016-04-05 16:02:24 +0000
@@ -1912,7 +1912,8 @@
         handler = NodeHandler(user, {})
         name = factory.make_name("eth")
         mac_address = factory.make_mac_address()
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         handler.create_physical({
             "system_id": node.system_id,
             "name": name,
@@ -1929,7 +1930,8 @@
         handler = NodeHandler(user, {})
         name = factory.make_name("eth")
         mac_address = factory.make_mac_address()
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         subnet = factory.make_Subnet(vlan=vlan)
         handler.create_physical({
             "system_id": node.system_id,
@@ -1951,7 +1953,8 @@
         handler = NodeHandler(user, {})
         name = factory.make_name("eth")
         mac_address = factory.make_mac_address()
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         handler.create_physical({
             "system_id": node.system_id,
             "name": name,
@@ -1971,7 +1974,8 @@
         handler = NodeHandler(user, {})
         name = factory.make_name("eth")
         mac_address = factory.make_mac_address()
-        vlan = factory.make_VLAN()
+        fabric = factory.make_Fabric()
+        vlan = fabric.get_default_vlan()
         subnet = factory.make_Subnet(vlan=vlan)
         handler.create_physical({
             "system_id": node.system_id,
@@ -1992,7 +1996,7 @@
         node = factory.make_Node()
         handler = NodeHandler(user, {})
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
-        new_vlan = factory.make_VLAN()
+        new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
         handler.create_vlan({
             "system_id": node.system_id,
             "parent": interface.id,
@@ -2008,7 +2012,7 @@
         node = factory.make_Node()
         handler = NodeHandler(user, {})
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
-        new_vlan = factory.make_VLAN()
+        new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
         new_subnet = factory.make_Subnet(vlan=new_vlan)
         handler.create_vlan({
             "system_id": node.system_id,
@@ -2030,7 +2034,7 @@
         node = factory.make_Node()
         handler = NodeHandler(user, {})
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
-        new_vlan = factory.make_VLAN()
+        new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
         handler.create_vlan({
             "system_id": node.system_id,
             "parent": interface.id,
@@ -2050,7 +2054,7 @@
         node = factory.make_Node()
         handler = NodeHandler(user, {})
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
-        new_vlan = factory.make_VLAN()
+        new_vlan = factory.make_VLAN(fabric=interface.vlan.fabric)
         new_subnet = factory.make_Subnet(vlan=new_vlan)
         handler.create_vlan({
             "system_id": node.system_id,
@@ -2110,7 +2114,8 @@
         handler = NodeHandler(user, {})
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
         new_name = factory.make_name("name")
-        new_vlan = factory.make_VLAN()
+        new_fabric = factory.make_Fabric()
+        new_vlan = new_fabric.get_default_vlan()
         handler.update_interface({
             "system_id": node.system_id,
             "interface_id": interface.id,