Merge lp:~blake-rouse/maas/nic-no-fabric into lp:~maas-committers/maas/trunk

[Blake Rouse]

Add the ability for an interface to be disconnected (not connected to any VLAN).

[Mike Pontillo (mpontillo)]

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!

review: Approve

[MAAS Lander (maas-lander)]

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://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Hit:2 http://security.ubuntu.com/ubuntu xenial-security InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [95.7 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [300 kB]
Fetched 395 kB in 0s (795 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-netaddr python3-netifaces python3-novaclient python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
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.20160115ubuntu3).
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+dfsg-11ubuntu1).
python-formencode is already the newest version (1.3.0-0ubuntu5).
python-lxml is already the newest version (3.5....

=== modified file 'src/maasserver/api/interfaces.py'
--- src/maasserver/api/interfaces.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/api/interfaces.py	2016-07-26 10:11:30 +0000
@@ -109,7 +109,8 @@
         :param name: Name of the interface.
         :param mac_address: MAC address of the interface.
         :param tags: Tags for the interface.
-        :param vlan: Untagged VLAN the interface is connected to.
+        :param vlan: Untagged VLAN the interface is connected to.  If not
+            provided then the interface is considered disconnected.
 
         Following are extra parameters that can be set on the interface:
 
@@ -151,7 +152,8 @@
         :param name: Name of the interface.
         :param mac_address: MAC address of the interface.
         :param tags: Tags for the interface.
-        :param vlan: VLAN the interface is connected to.
+        :param vlan: VLAN the interface is connected to.  If not
+            provided then the interface is considered disconnected.
         :param parents: Parent interfaces that make this bond.
 
         Following are parameters specific to bonds:
@@ -318,13 +320,15 @@
         :param name: Name of the interface.
         :param mac_address: MAC address of the interface.
         :param tags: Tags for the interface.
-        :param vlan: Untagged VLAN the interface is connected to.
+        :param vlan: Untagged VLAN the interface is connected to.  If not set
+        then the interface is considered disconnected.
 
         Fields for bond interface:
         :param name: Name of the interface.
         :param mac_address: MAC address of the interface.
         :param tags: Tags for the interface.
-        :param vlan: Tagged VLAN the interface is connected to.
+        :param vlan: Tagged VLAN the interface is connected to.  If not set
+        then the interface is considered disconnected.
         :param parents: Parent interfaces that make this bond.
 
         Fields for VLAN interface:
 
=== modified file 'src/maasserver/api/tests/test_interfaces.py'
--- src/maasserver/api/tests/test_interfaces.py	2016-05-26 13:28:46 +0000
+++ src/maasserver/api/tests/test_interfaces.py	2016-07-26 10:11:30 +0000
@@ -257,7 +257,7 @@
             self.assertEqual(
                 http.client.CONFLICT, response.status_code, response.content)
 
-    def test_create_physical_requires_mac_name_and_vlan(self):
+    def test_create_physical_requires_mac_and_name(self):
         self.become_admin()
         node = factory.make_Node(status=NODE_STATUS.READY)
         uri = get_interfaces_uri(node)
@@ -269,7 +269,6 @@
         self.assertEqual({
             "mac_address": ["This field is required."],
             "name": ["This field is required."],
-            "vlan": ["This field is required."],
             }, json_load_bytes(response.content))
 
     def test_create_physical_doesnt_allow_mac_already_register(self):
@@ -490,7 +489,7 @@
         self.assertEqual(
             http.client.BAD_REQUEST, response.status_code, response.content)
         self.assertEqual({
-            "vlan": ["This field is required."],
+            "vlan": ["A VLAN interface must be connected to a tagged VLAN."],
             "parent": ["A VLAN interface must have exactly one parent."],
             }, json_load_bytes(response.content))
 
 
=== modified file 'src/maasserver/forms_interface.py'
--- src/maasserver/forms_interface.py	2016-04-22 17:28:15 +0000
+++ src/maasserver/forms_interface.py	2016-07-26 10:11:30 +0000
@@ -99,6 +99,11 @@
                     rel = interface.parent_relationships.filter(
                         parent=parent_to_del)
                     rel.delete()
+        # Allow setting the VLAN to None.
+        new_vlan = self.cleaned_data.get('vlan')
+        vlan_was_set = 'vlan' in self.data
+        if new_vlan is None and vlan_was_set:
+            interface.vlan = new_vlan
         self.set_extra_parameters(interface, created)
         interface.save()
         if created:
@@ -168,6 +173,17 @@
             'vlan',
             )
 
+    def save(self, *args, **kwargs):
+        """Persist the interface into the database."""
+        interface = super(ControllerInterfaceForm, self).save(commit=False)
+        # Allow setting the VLAN to None.
+        new_vlan = self.cleaned_data.get('vlan')
+        vlan_was_set = 'vlan' in self.data
+        if new_vlan is None and vlan_was_set:
+            interface.vlan = new_vlan
+        interface.save()
+        return interface
+
 
 class PhysicalInterfaceForm(InterfaceForm):
     """Form used to create/edit a physical interface."""
@@ -251,7 +267,13 @@
         return parents
 
     def clean_vlan(self):
+        created = self.instance.id is None
         new_vlan = self.cleaned_data.get('vlan')
+        vlan_was_set = 'vlan' in self.data
+        if (created and new_vlan is None) or (
+                not created and new_vlan is None and vlan_was_set):
+            raise ValidationError(
+                "A VLAN interface must be connected to a tagged 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.")
@@ -281,15 +303,6 @@
     bridges.
     """
 
-    def __init__(self, *args, **kwargs):
-        super().__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):
         """Validate that child interfaces cannot be created unless at least one
         parent is present.
 
=== modified file 'src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py'
--- src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py	2016-05-11 19:01:48 +0000
+++ src/maasserver/migrations/builtin/maasserver/0056_add_description_to_fabric_and_space.py	2016-07-26 10:11:30 +0000
@@ -39,7 +39,7 @@
         migrations.AlterField(
             model_name='interface',
             name='vlan',
-            field=models.ForeignKey(to='maasserver.VLAN', default=maasserver.models.interface.get_default_vlan, on_delete=django.db.models.deletion.PROTECT),
+            field=models.ForeignKey(to='maasserver.VLAN', default=None, on_delete=django.db.models.deletion.PROTECT),
         ),
         migrations.AlterField(
             model_name='subnet',
 
=== added file 'src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py'
--- src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py	1970-01-01 00:00:00 +0000
+++ src/maasserver/migrations/builtin/maasserver/0070_allow_null_vlan_on_interface.py	2016-07-26 10:11:30 +0000
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.db import (
+    migrations,
+    models,
+)
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('maasserver', '0069_add_previous_node_status_to_node'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='interface',
+            name='vlan',
+            field=models.ForeignKey(null=True, blank=True, to='maasserver.VLAN', on_delete=django.db.models.deletion.PROTECT),
+        ),
+    ]
 
=== modified file 'src/maasserver/models/interface.py'
--- src/maasserver/models/interface.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/interface.py	2016-07-26 10:11:30 +0000
@@ -67,11 +67,6 @@
 INTERFACE_NAME_REGEXP = '^[\w\-_.:]+$'
 
 
-def get_default_vlan():
-    from maasserver.models.vlan import VLAN
-    return VLAN.objects.get_default_vlan().id
-
-
 def get_subnet_family(subnet):
     """Return the IPADDRESS_FAMILY for the `subnet`."""
     if subnet is not None:
@@ -375,8 +370,7 @@
         through='InterfaceRelationship', symmetrical=False)
 
     vlan = ForeignKey(
-        'VLAN', default=get_default_vlan, editable=True, blank=False,
-        null=False, on_delete=PROTECT)
+        'VLAN', editable=True, blank=True, null=True, on_delete=PROTECT)
 
     ip_addresses = ManyToManyField(
         'StaticIPAddress', editable=True, blank=True)
@@ -438,8 +432,13 @@
         mtu = None
         if self.params:
             mtu = self.params.get('mtu', None)
+        if mtu is None and self.vlan is not None:
+            mtu = self.vlan.mtu
         if mtu is None:
-            mtu = self.vlan.mtu
+            # Use default MTU for the interface when the interface has no
+            # MTU set and it is disconnected.
+            from maasserver.models.vlan import DEFAULT_MTU
+            mtu = DEFAULT_MTU
         return mtu
 
     def get_links(self):
@@ -752,11 +751,12 @@
         # subnets into the default VLAN, this assumption might be incorrect in
         # many cases, leading to interfaces being configured as AUTO when
         # they should be configured as DHCP.
-        found_subnet = self.vlan.subnet_set.first()
-        if found_subnet is not None:
-            return self.link_subnet(INTERFACE_LINK_TYPE.AUTO, found_subnet)
-        else:
-            return self.link_subnet(INTERFACE_LINK_TYPE.DHCP, None)
+        if self.vlan is not None:
+            found_subnet = self.vlan.subnet_set.first()
+            if found_subnet is not None:
+                return self.link_subnet(INTERFACE_LINK_TYPE.AUTO, found_subnet)
+            else:
+                return self.link_subnet(INTERFACE_LINK_TYPE.DHCP, None)
 
     def ensure_link_up(self):
         """Ensure that if no subnet links exists that at least a LINK_UP
@@ -766,7 +766,7 @@
         if has_links:
             # Nothing to do, already has links.
             return
-        else:
+        elif self.vlan is not None:
             # 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.
@@ -1265,20 +1265,31 @@
                     "parents": ["VLAN interface must have exactly one parent."]
                     })
             parent = parents[0]
+            # We do not allow a bridge interface to be a parent for a VLAN
+            # interface.
             allowed_vlan_parent_types = (
                 INTERFACE_TYPE.PHYSICAL,
                 INTERFACE_TYPE.BOND,
                 INTERFACE_TYPE.BRIDGE
             )
             if parent.get_type() not in allowed_vlan_parent_types:
-                # XXX mpontillo 2016-06-23: we won't mention bridges in this
-                # error message, since users can't configure bridges on nodes.
+                # XXX blake_r 2016-07-18: we won't mention bridges in this
+                # error message, since users can't configure VLAN interfaces
+                # on bridges.
                 raise ValidationError({
                     "parents": [
                         "VLAN interface can only be created on a physical "
                         "or bond interface."
                     ]
                 })
+            # VLAN interface must be connected to a VLAN, it cannot be
+            # disconnected like physical and bond interfaces.
+            if self.vlan is None:
+                raise ValidationError({
+                    "vlan": [
+                        "VLAN interface requires connection to a VLAN."
+                    ]
+                })
 
     def save(self, *args, **kwargs):
         # Set the node of this VLAN to the same as its parents.
 
=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2016-07-25 20:13:54 +0000
+++ src/maasserver/models/node.py	2016-07-26 10:11:30 +0000
@@ -3407,6 +3407,10 @@
                     new_vlan = new_fabric.get_default_vlan()
                     interface.vlan = new_vlan
                     interface.save()
+                else:
+                    interface.vlan = (
+                        Fabric.objects.get_default_fabric().get_default_vlan())
+                    interface.save()
         else:
             if interface.node.id != self.id:
                 # MAC address was on a different node. We need to move
 
=== modified file 'src/maasserver/models/signals/interfaces.py'
--- src/maasserver/models/signals/interfaces.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/signals/interfaces.py	2016-07-26 10:11:30 +0000
@@ -197,22 +197,25 @@
             NODE_TYPE.RACK_CONTROLLER,
             NODE_TYPE.REGION_AND_RACK_CONTROLLER):
         # Interface VLAN was changed on a controller. Move all linked subnets
-        # to that new VLAN.
-        for ip_address in instance.ip_addresses.all():
-            if ip_address.subnet is not None:
-                ip_address.subnet.vlan = new_vlan
-                ip_address.subnet.save()
+        # to that new VLAN, unless the new VLAN is None. When the VLAN is
+        # None then the administrator is say that the interface is now
+        # disconnected.
+        if new_vlan is not None:
+            for ip_address in instance.ip_addresses.all():
+                if ip_address.subnet is not None:
+                    ip_address.subnet.vlan = new_vlan
+                    ip_address.subnet.save()
 
-        # If any children are VLAN interfaces then we need to move those
-        # VLANs into the same fabric as the parent.
-        for rel in instance.children_relationships.all():
-            if rel.child.type == INTERFACE_TYPE.VLAN:
-                new_child_vlan, _ = VLAN.objects.get_or_create(
-                    fabric=new_vlan.fabric, vid=rel.child.vlan.vid)
-                rel.child.vlan = new_child_vlan
-                rel.child.save()
-                # No need to update the IP addresses here this function
-                # will be called again because the child has been saved.
+            # If any children are VLAN interfaces then we need to move those
+            # VLANs into the same fabric as the parent.
+            for rel in instance.children_relationships.all():
+                if rel.child.type == INTERFACE_TYPE.VLAN:
+                    new_child_vlan, _ = VLAN.objects.get_or_create(
+                        fabric=new_vlan.fabric, vid=rel.child.vlan.vid)
+                    rel.child.vlan = new_child_vlan
+                    rel.child.save()
+                    # No need to update the IP addresses here this function
+                    # will be called again because the child has been saved.
 
     else:
         # Interface VLAN was changed on a machine or device. Remove all its
 
=== modified file 'src/maasserver/models/signals/tests/test_interfaces.py'
--- src/maasserver/models/signals/tests/test_interfaces.py	2016-04-11 16:23:26 +0000
+++ src/maasserver/models/signals/tests/test_interfaces.py	2016-07-26 10:11:30 +0000
@@ -194,6 +194,17 @@
         self.assertIsNone(reload_object(static_ip))
         self.assertIsNotNone(reload_object(discovered_ip))
 
+    def test__removes_links_when_goes_to_disconnected(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)
+        interface.vlan = None
+        interface.save()
+        self.assertIsNone(reload_object(static_ip))
+        self.assertIsNotNone(reload_object(discovered_ip))
+
 
 class TestInterfaceVLANUpdateController(MAASServerTestCase):
 
@@ -222,6 +233,17 @@
         interface.save()
         self.assertEquals(new_vlan, reload_object(subnet).vlan)
 
+    def test__doesnt_move_link_subnets_when_vlan_is_None(self):
+        node = self.maker()
+        interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
+        old_vlan = interface.vlan
+        subnet = factory.make_Subnet(vlan=old_vlan)
+        factory.make_StaticIPAddress(
+            subnet=subnet, interface=interface)
+        interface.vlan = None
+        interface.save()
+        self.assertEquals(old_vlan, reload_object(subnet).vlan)
+
     def test__moves_children_vlans_to_same_fabric(self):
         node = self.maker()
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node)
 
=== modified file 'src/maasserver/models/tests/test_interface.py'
--- src/maasserver/models/tests/test_interface.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/models/tests/test_interface.py	2016-07-26 10:11:30 +0000
@@ -42,6 +42,7 @@
     UnknownInterface,
     VLANInterface,
 )
+from maasserver.models.vlan import DEFAULT_MTU
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_objects
 from maasserver.testing.testcase import (
@@ -527,6 +528,17 @@
             name=name, node=node, mac_address=mac,
             type=INTERFACE_TYPE.PHYSICAL))
 
+    def test_allows_null_vlan(self):
+        name = factory.make_name('name')
+        node = factory.make_Node()
+        mac = factory.make_MAC()
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL,
+            name=name, node=node, mac_address=mac, disconnected=True)
+        self.assertThat(interface, MatchesStructure.byEquality(
+            name=name, node=node, mac_address=mac,
+            type=INTERFACE_TYPE.PHYSICAL, vlan=None))
+
     def test_string_representation_contains_essential_data(self):
         name = factory.make_name('name')
         node = factory.make_Node()
@@ -570,6 +582,11 @@
         nic1.vlan.save()
         self.assertEqual(vlan_mtu, nic1.get_effective_mtu())
 
+    def test_get_effective_mtu_returns_default_mtu(self):
+        nic1 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, disconnected=True)
+        self.assertEqual(DEFAULT_MTU, nic1.get_effective_mtu())
+
     def test_get_links_returns_links_for_each_type(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         links = []
@@ -1595,6 +1612,11 @@
 class TestForceAutoOrDHCPLink(MAASServerTestCase):
     """Tests for `Interface.force_auto_or_dhcp_link`."""
 
+    def test__does_nothing_when_disconnected(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, disconnected=True)
+        self.assertIsNone(interface.force_auto_or_dhcp_link())
+
     def test__sets_to_AUTO_on_subnet(self):
         interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         subnet = factory.make_Subnet(vlan=interface.vlan)
@@ -1622,6 +1644,15 @@
             1, interface.ip_addresses.count(),
             "Should only have one IP address assigned.")
 
+    def test__does_nothing_if_no_vlan(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, disconnected=True)
+        interface.ensure_link_up()
+        interface = reload_object(interface)
+        self.assertEqual(
+            0, interface.ip_addresses.count(),
+            "Should only have no IP address assigned.")
+
     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)
 
=== modified file 'src/maasserver/static/js/angular/controllers/node_details_networking.js'
--- src/maasserver/static/js/angular/controllers/node_details_networking.js	2016-07-11 18:29:06 +0000
+++ src/maasserver/static/js/angular/controllers/node_details_networking.js	2016-07-26 10:11:30 +0000
@@ -654,11 +654,17 @@
             if($scope.isInterfaceNameInvalid(nic)) {
                 nic.name = originalInteface.name;
             } else if(originalInteface.name !== nic.name ||
+                (originalInteface.vlan_id === null && nic.vlan !== null) ||
+                (originalInteface.vlan_id !== null && nic.vlan === null) ||
                 originalInteface.vlan_id !== nic.vlan.id) {
                 var params = {
-                    "name": nic.name,
-                    "vlan": nic.vlan.id
+                    "name": nic.name
                 };
+                if(nic.vlan !== null) {
+                    params.vlan = nic.vlan.id;
+                } else {
+                    params.vlan = null;
+                }
                 $scope.$parent.nodesManager.updateInterface(
                     $scope.node, nic.id, params).then(null, function(error) {
                         // XXX blake_r: Just log the error in the console, but
@@ -716,7 +722,11 @@
         $scope.fabricChanged = function(nic) {
             // Update the VLAN on the node to be the default VLAN for that
             // fabric. The first VLAN for the fabric is the default.
-            nic.vlan = getDefaultVLAN(nic.fabric);
+            if(nic.fabric !== null) {
+                nic.vlan = getDefaultVLAN(nic.fabric);
+            } else {
+                nic.vlan = null;
+            }
             $scope.saveInterface(nic);
         };
 
 
=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js'
--- src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js	2016-07-11 18:29:06 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js	2016-07-26 10:11:30 +0000
@@ -1685,6 +1685,62 @@
                     "vlan": vlan.id
                 });
         });
+
+        it("calls MachinesManager.updateInterface if vlan set", function() {
+            var controller = makeController();
+            var id = makeInteger(0, 100);
+            var name = makeName("nic");
+            var vlan = { id: makeInteger(0, 100) };
+            var original_nic = {
+                id: id,
+                name: name,
+                vlan_id: null
+            };
+            var nic = {
+                id: id,
+                name: name,
+                vlan: vlan
+            };
+            $scope.originalInterfaces[id] = original_nic;
+            $scope.interfaces = [nic];
+
+            spyOn(MachinesManager, "updateInterface").and.returnValue(
+                $q.defer().promise);
+            $scope.saveInterface(nic);
+            expect(MachinesManager.updateInterface).toHaveBeenCalledWith(
+                node, id, {
+                    "name": name,
+                    "vlan": vlan.id
+                });
+        });
+
+        it("calls MachinesManager.updateInterface if vlan unset", function() {
+            var controller = makeController();
+            var id = makeInteger(0, 100);
+            var name = makeName("nic");
+            var vlan = { id: makeInteger(0, 100) };
+            var original_nic = {
+                id: id,
+                name: name,
+                vlan_id: makeInteger(200, 300)
+            };
+            var nic = {
+                id: id,
+                name: name,
+                vlan: null
+            };
+            $scope.originalInterfaces[id] = original_nic;
+            $scope.interfaces = [nic];
+
+            spyOn(MachinesManager, "updateInterface").and.returnValue(
+                $q.defer().promise);
+            $scope.saveInterface(nic);
+            expect(MachinesManager.updateInterface).toHaveBeenCalledWith(
+                node, id, {
+                    "name": name,
+                    "vlan": null
+                });
+        });
     });
 
     describe("setFocusInterface", function() {
@@ -1847,6 +1903,17 @@
             expect(nic.vlan).toBe(vlan);
         });
 
+        it("sets vlan to null", function() {
+            var controller = makeController();
+            var nic = {
+                vlan: {},
+                fabric: null
+            };
+            spyOn($scope, "saveInterface");
+            $scope.fabricChanged(nic);
+            expect(nic.vlan).toBeNull();
+        });
+
         it("calls saveInterface", function() {
             var controller = makeController();
             var fabric = {
 
=== modified file 'src/maasserver/static/partials/node-details.html'
--- src/maasserver/static/partials/node-details.html	2016-07-15 00:42:25 +0000
+++ src/maasserver/static/partials/node-details.html	2016-07-26 10:11:30 +0000
@@ -617,11 +617,13 @@
                                             data-ng-disabled="interface.type == 'alias' || interface.type == 'vlan' || !isNodeEditingAllowed()"
                                             data-ng-change="fabricChanged(interface)"
                                             data-ng-options="fabric as fabric.name for fabric in fabrics">
+                                                <option value="">Disconnected</option>
                                         </select>
                                     </div>
                                     <div class="table__data table-col--14">
                                         <select class="table__input" name="vlan" id="vlan"
                                             data-ng-model="interface.vlan"
+                                            data-ng-if="interface.fabric"
                                             data-ng-disabled="isController || interface.type == 'alias' || interface.vlan.vid === 0 || !isNodeEditingAllowed()"
                                             data-ng-change="saveInterface(interface)"
                                             data-ng-options="vlan as getVLANText(vlan) for vlan in vlans | removeDefaultVLANIfVLAN:interface.type | filterByFabric:interface.fabric">
@@ -629,6 +631,7 @@
                                     </div>
                                     <div class="table__data table-col--18">
                                         <select class="table__input" name="subnet" id="subnet"
+                                            data-ng-if="interface.fabric"
                                             data-ng-hide="isAllNetworkingDisabled() && interface.discovered[0].subnet_id"
                                             data-ng-disabled="isController || !isNodeEditingAllowed()"
                                             data-ng-model="interface.subnet"
@@ -641,7 +644,7 @@
                                         </span>
                                     </div>
                                     <div class="table__data table-col--21">
-                                        <ul class="no-bullets no-margin-bottom" data-ng-if="!isController && !isAllNetworkingDisabled()">
+                                        <ul class="no-bullets no-margin-bottom" data-ng-if="!isController && !isAllNetworkingDisabled() && interface.fabric">
                                             <li class="no-margin">
                                                 <select class="table__input" name="link-mode" id="link-mode"
                                                     data-ng-model="interface.mode"
 
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2016-07-20 19:27:20 +0000
+++ src/maasserver/testing/factory.py	2016-07-26 10:11:30 +0000
@@ -902,7 +902,7 @@
     def make_Interface(
             self, iftype=INTERFACE_TYPE.PHYSICAL, node=None, mac_address=None,
             vlan=None, parents=None, name=None, cluster_interface=None,
-            ip=None, enabled=True, fabric=None, tags=None):
+            ip=None, enabled=True, fabric=None, tags=None, disconnected=False):
         if name is None:
             if iftype in (INTERFACE_TYPE.PHYSICAL, INTERFACE_TYPE.UNKNOWN):
                 name = self.make_name('eth')
@@ -919,20 +919,21 @@
                 name = None
         if iftype is None:
             iftype = INTERFACE_TYPE.PHYSICAL
-        if vlan is None:
-            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 not disconnected:
+            if vlan is None:
+                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	2016-04-22 17:28:15 +0000
+++ src/maasserver/tests/test_forms_interface.py	2016-07-26 10:11:30 +0000
@@ -86,6 +86,22 @@
             MatchesStructure.byEquality(
                 name=interface.name, vlan=new_vlan, enabled=interface.enabled))
 
+    def test__allows_no_vlan(self):
+        node = self.maker()
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=node)
+        form = ControllerInterfaceForm(
+            instance=interface,
+            data={
+                'vlan': None,
+            })
+        self.assertTrue(form.is_valid(), form.errors)
+        interface = form.save()
+        self.assertThat(
+            interface,
+            MatchesStructure.byEquality(
+                name=interface.name, vlan=None, enabled=interface.enabled))
+
 
 class PhysicalInterfaceFormTest(MAASServerTestCase):
 
@@ -116,6 +132,30 @@
                 type=INTERFACE_TYPE.PHYSICAL, tags=tags))
         self.assertItemsEqual([], interface.parents.all())
 
+    def test__creates_physical_interface_disconnected(self):
+        node = factory.make_Node()
+        mac_address = factory.make_mac_address()
+        interface_name = 'eth0'
+        tags = [
+            factory.make_name("tag")
+            for _ in range(3)
+        ]
+        form = PhysicalInterfaceForm(
+            node=node,
+            data={
+                'name': interface_name,
+                'mac_address': mac_address,
+                'tags': ",".join(tags),
+            })
+        self.assertTrue(form.is_valid(), form.errors)
+        interface = form.save()
+        self.assertThat(
+            interface,
+            MatchesStructure.byEquality(
+                node=node, mac_address=mac_address, name=interface_name,
+                type=INTERFACE_TYPE.PHYSICAL, tags=tags, vlan=None))
+        self.assertItemsEqual([], interface.parents.all())
+
     def test__create_ensures_link_up(self):
         node = factory.make_Node()
         mac_address = factory.make_mac_address()
@@ -255,6 +295,26 @@
                 name=new_name, vlan=new_vlan, enabled=False, tags=[]))
         self.assertItemsEqual([], interface.parents.all())
 
+    def test__edits_interface_disconnected(self):
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, name='eth0')
+        new_name = 'eth1'
+        form = PhysicalInterfaceForm(
+            instance=interface,
+            data={
+                'name': new_name,
+                'vlan': None,
+                'enabled': False,
+                'tags': "",
+            })
+        self.assertTrue(form.is_valid(), form.errors)
+        interface = form.save()
+        self.assertThat(
+            interface,
+            MatchesStructure.byEquality(
+                name=new_name, vlan=None, enabled=False, tags=[]))
+        self.assertItemsEqual([], interface.parents.all())
+
     def test__create_sets_interface_parameters(self):
         node = factory.make_Node()
         mac_address = factory.make_mac_address()
@@ -403,6 +463,20 @@
         self.assertIsNotNone(
             interface.ip_addresses.filter(alloc_type=IPADDRESS_TYPE.STICKY))
 
+    def test__create_rejects_interface_without_vlan(self):
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        form = VLANInterfaceForm(
+            node=parent.node,
+            data={
+                'parents': [parent.id],
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(
+            ['vlan'], form.errors.keys(), form.errors)
+        self.assertIn(
+            "A VLAN interface must be connected to a tagged VLAN.",
+            form.errors['vlan'][0])
+
     def test_rejects_interface_with_duplicate_name(self):
         parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         vlan = factory.make_VLAN(fabric=parent.vlan.fabric, vid=10)
@@ -468,6 +542,20 @@
             "VLAN interface can't have another VLAN interface as parent.",
             form.errors['parents'][0])
 
+    def test__rejects_no_vlan(self):
+        parent = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        form = VLANInterfaceForm(
+            node=parent.node,
+            data={
+                'vlan': None,
+                'parents': [parent.id],
+            })
+        self.assertFalse(form.is_valid(), form.errors)
+        self.assertItemsEqual(['vlan'], form.errors.keys())
+        self.assertIn(
+            "A VLAN interface must be connected to a tagged VLAN.",
+            form.errors['vlan'][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)
@@ -576,7 +664,8 @@
         self.assertThat(
             interface,
             MatchesStructure.byEquality(
-                name=interface_name, type=INTERFACE_TYPE.BOND))
+                name=interface_name, type=INTERFACE_TYPE.BOND,
+                vlan=parent1.vlan))
         self.assertIn(
             interface.mac_address, [parent1.mac_address, parent2.mac_address])
         self.assertItemsEqual([parent1, parent2], interface.parents.all())
@@ -787,6 +876,25 @@
             for parent in [parent1, parent2, new_parent]
         ))
 
+    def test__edits_interface_allows_disconnected(self):
+        parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node)
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.BOND, parents=[parent1, parent2])
+        form = BondInterfaceForm(
+            instance=interface,
+            data={
+                'vlan': None,
+            })
+        self.assertTrue(form.is_valid(), form.errors)
+        interface = form.save()
+        self.assertThat(
+            interface,
+            MatchesStructure.byEquality(
+                mac_address=interface.mac_address, vlan=None,
+                type=INTERFACE_TYPE.BOND))
+
     def test__edits_interface_removes_parents(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
@@ -1119,6 +1227,26 @@
         self.assertItemsEqual(
             [parent1, parent2, new_parent], interface.parents.all())
 
+    def test__edits_interface_allows_disconnected(self):
+        parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
+        parent2 = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=parent1.node, vlan=parent1.vlan)
+        interface = factory.make_Interface(
+            INTERFACE_TYPE.BRIDGE,
+            parents=[parent1, parent2])
+        form = BridgeInterfaceForm(
+            instance=interface,
+            data={
+                'vlan': None,
+            })
+        self.assertTrue(form.is_valid(), form.errors)
+        interface = form.save()
+        self.assertThat(
+            interface,
+            MatchesStructure.byEquality(
+                mac_address=interface.mac_address,
+                vlan=None, type=INTERFACE_TYPE.BRIDGE))
+
     def test__edits_interface_removes_parents(self):
         parent1 = factory.make_Interface(INTERFACE_TYPE.PHYSICAL)
         parent2 = factory.make_Interface(
 
=== modified file 'src/maasserver/websockets/handlers/node.py'
--- src/maasserver/websockets/handlers/node.py	2016-05-12 19:07:37 +0000
+++ src/maasserver/websockets/handlers/node.py	2016-07-26 10:11:30 +0000
@@ -455,7 +455,8 @@
     def get_all_fabric_names(self, obj, subnets):
         fabric_names = set()
         for interface in obj.interface_set.all():
-            fabric_names.add(interface.vlan.fabric.name)
+            if interface.vlan is not None:
+                fabric_names.add(interface.vlan.fabric.name)
         for subnet in subnets:
             fabric_names.add(subnet.vlan.fabric.name)
         return list(fabric_names)