Merge ~mpontillo/maas:bug-1704501--2.3 into maas:2.3

[Mike Pontillo]

LP: #1704501 - Allow users to change which Fabric a VLAN is on.

Backports: 4083dc0635958109a28ef44a84df63e855007335

cd5f92a... by Mike Pontillo on 2018-04-03

Fix fabric field formatting.

[Mike Pontillo (mpontillo)]

Self-approve backport.

review: Approve

diff --git a/src/maasserver/forms/tests/test_vlan.py b/src/maasserver/forms/tests/test_vlan.py
index 8db5f5a..5c5aacb 100644
--- a/src/maasserver/forms/tests/test_vlan.py
+++ b/src/maasserver/forms/tests/test_vlan.py
@@ -13,6 +13,11 @@ from maasserver.models.vlan import DEFAULT_MTU
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils.orm import reload_object
+from testtools import ExpectedException
+from testtools.matchers import (
+    Equals,
+    Not,
+)
 
 
 class TestVLANForm(MAASServerTestCase):
@@ -371,3 +376,63 @@ class TestVLANForm(MAASServerTestCase):
         self.assertEqual(secondary_rack, vlan.primary_rack)
         self.assertEqual(None, vlan.secondary_rack)
         self.assertTrue(vlan.dhcp_on)
+
+
+class TestVLANFormFabricModification(MAASServerTestCase):
+
+    def test__cannot_move_vlan_with_overlapping_vid(self):
+        fabric0 = Fabric.objects.get_default_fabric()
+        fabric1 = factory.make_Fabric()
+        fabric1_untagged = fabric1.get_default_vlan()
+        form = VLANForm(instance=fabric1_untagged, data={
+            "fabric": fabric0.id
+        })
+        is_valid = form.is_valid()
+        self.assertThat(is_valid, Equals(False))
+        self.assertThat(dict(form.errors), Equals(
+            {'__all__': [
+                'A VLAN with the specified VID already '
+                'exists in the destination fabric.'
+            ]}
+        ))
+        with ExpectedException(ValueError):
+            form.save()
+
+    def test__allows_moving_vlan_to_new_fabric_if_vid_is_unique(self):
+        fabric0 = Fabric.objects.get_default_fabric()
+        fabric1 = factory.make_Fabric()
+        fabric1_untagged = fabric1.get_default_vlan()
+        form = VLANForm(instance=fabric1_untagged, data={
+            "fabric": fabric0.id,
+            "vid": 10
+        })
+        is_valid = form.is_valid()
+        self.assertThat(is_valid, Equals(True))
+        form.save()
+
+    def test__deletes_empty_fabrics(self):
+        fabric0 = Fabric.objects.get_default_fabric()
+        fabric1 = factory.make_Fabric()
+        fabric1_untagged = fabric1.get_default_vlan()
+        form = VLANForm(instance=fabric1_untagged, data={
+            "fabric": fabric0.id,
+            "vid": 10
+        })
+        is_valid = form.is_valid()
+        self.assertThat(is_valid, Equals(True))
+        form.save()
+        self.assertThat(reload_object(fabric1), Equals(None))
+
+    def test__does_not_delete_non_empty_fabrics(self):
+        fabric0 = Fabric.objects.get_default_fabric()
+        fabric1 = factory.make_Fabric()
+        factory.make_VLAN(fabric=fabric1)
+        fabric1_untagged = fabric1.get_default_vlan()
+        form = VLANForm(instance=fabric1_untagged, data={
+            "fabric": fabric0.id,
+            "vid": 10
+        })
+        is_valid = form.is_valid()
+        form.save()
+        self.assertThat(is_valid, Equals(True))
+        self.assertThat(reload_object(fabric1), Not(Equals(None)))
diff --git a/src/maasserver/forms/vlan.py b/src/maasserver/forms/vlan.py
index 13fad27..89e5f4d 100644
--- a/src/maasserver/forms/vlan.py
+++ b/src/maasserver/forms/vlan.py
@@ -15,6 +15,7 @@ from maasserver.fields import (
 )
 from maasserver.forms import MAASModelForm
 from maasserver.models import (
+    Fabric,
     RackController,
     Space,
 )
@@ -30,6 +31,9 @@ class VLANForm(MAASModelForm):
     space = SpecifierOrModelChoiceField(
         queryset=Space.objects.all(), required=False, empty_label="")
 
+    fabric = SpecifierOrModelChoiceField(
+        queryset=Fabric.objects.all(), required=False, empty_label="")
+
     class Meta:
         model = VLAN
         fields = (
@@ -42,6 +46,7 @@ class VLANForm(MAASModelForm):
             'secondary_rack',
             'relay_vlan',
             'space',
+            'fabric',
             )
 
     def __init__(self, *args, **kwargs):
diff --git a/src/maasserver/models/vlan.py b/src/maasserver/models/vlan.py
index 28e9f42..35e34a9 100644
--- a/src/maasserver/models/vlan.py
+++ b/src/maasserver/models/vlan.py
@@ -15,6 +15,7 @@ from django.db.models import (
     BooleanField,
     CASCADE,
     CharField,
+    Count,
     deletion,
     ForeignKey,
     IntegerField,
@@ -243,6 +244,15 @@ class VLAN(CleanSave, TimestampedModel):
             subnet.vlan = self.fabric.get_default_vlan()
             subnet.save()
 
+    def unique_error_message(self, model_class, unique_check):
+        if set(unique_check) == {'vid', 'fabric'}:
+            return (
+                'A VLAN with the specified VID already exists in the '
+                'destination fabric.'
+            )
+        else:
+            return super().unique_error_message(model_class, unique_check)
+
     def delete(self):
         if self.is_fabric_default():
             raise ValidationError(
@@ -268,3 +278,9 @@ class VLAN(CleanSave, TimestampedModel):
                 "DHCP server is being used.",
                 ident="dhcp_disabled_all_vlans")
         super().save(*args, **kwargs)
+        # Circular dependencies.
+        from maasserver.models import Fabric
+        # Delete any now-empty fabrics.
+        fabrics_with_vlan_count = Fabric.objects.annotate(
+            vlan_count=Count("vlan"))
+        fabrics_with_vlan_count.filter(vlan_count=0).delete()
diff --git a/src/maasserver/static/partials/vlan-details.html b/src/maasserver/static/partials/vlan-details.html
index a526ed9..b02043f 100644
--- a/src/maasserver/static/partials/vlan-details.html
+++ b/src/maasserver/static/partials/vlan-details.html
@@ -220,7 +220,7 @@
         </div>
     </header>
     <section class="row">
-        <div class="wrapper--inner">
+        <fieldset class="wrapper--inner">
             <div class="twelve-col">
                 <h2 class="u-float--left">VLAN Summary</h2>
                 <button type="button" name="button" class="button--secondary button--inline u-float--right" data-ng-click="vlanDetails.enterEditSummary()" data-ng-if="!vlanDetails.editSummary">Edit</button>
@@ -268,12 +268,10 @@
                     <maas-obj-field type="textarea" key="description" label="Description" placeholder="VLAN description"
                         label-width="two" input-width="three" blur-on-enter="true"></maas-obj-field>
                 </fieldset>
-                <div class="six-col last-col">
+                <fieldset class="form__fieldset six-col last-col">
                     <dl>
-                        <dt class="two-col">Fabric</dt>
-                        <dd class="four-col last-col">
-                            <a href="#/fabric/{$ vlanDetails.fabric.id $}">{$ vlanDetails.fabric.name $}</a>
-                        </dd>
+                        <maas-obj-field type="options" key="fabric" label="Fabric" label-width="two" input-width="three"
+                                        options="fabric.id as fabric.name for fabric in vlanDetails.fabrics"></maas-obj-field>
                         <div data-ng-if="vlanDetails.relatedControllers">
                             <dt class="two-col">Rack controllers <span class="icon icon--help tooltip" aria-label="A rack controller controls hosts and images and runs network services&#xa;like DHCP for connected VLANs."></span></dt>
                             <dd class="four-col last-col">
@@ -283,8 +281,11 @@
                             </dd>
                         </div>
                     </dl>
+                </fieldset>
+                <div class="six-col">
+                    <maas-obj-errors></maas-obj-errors>
                 </div>
-                <div class="twelve-col u-align--right">
+                <div class="six-col last-col u-align--right">
                     <button class="button--base button--inline"
                        data-ng-click="vlanDetails.exitEditSummary()">Cancel</button>
                     <button class="button--positive button--inline" maas-obj-save>Save summary</button>