Merge ~mpontillo/maas:move-vlans-between-fabrics into maas:master

[Mike Pontillo]

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

[Newell Jensen (newell-jensen)]

lgtm +1

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b move-vlans-between-fabrics lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/2252/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b move-vlans-between-fabrics lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/2262/consoleText

7dd47ed... by Mike Pontillo on 2018-04-03

Fix off-by-one random failures in maasserver.tests.test_stats:TestMAASStats.

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 e25658d..d520e16 100644
--- a/src/maasserver/models/vlan.py
+++ b/src/maasserver/models/vlan.py
@@ -14,6 +14,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 59ebb6e..8a2826a 100644
--- a/src/maasserver/static/partials/vlan-details.html
+++ b/src/maasserver/static/partials/vlan-details.html
@@ -326,14 +326,8 @@
                             label-width="2" input-width="5" blur-on-enter="true"></maas-obj-field>
                     </div>
                     <div class="col-6">
-                        <div class="p-form__group">
-                            <div class="p-form__label">
-                                <p><strong>Fabric</strong></p>
-                            </div>
-                            <div class="p-form__control">
-                                <p><a href="#/fabric/{$ vlanDetails.fabric.id $}">{$ vlanDetails.fabric.name $}</a></p>
-                            </div>
-                        </div>
+                        <maas-obj-field type="options" key="fabric" label="Fabric"
+                                        options="fabric.id as fabric.name for fabric in vlanDetails.fabrics"></maas-obj-field>
                         <div class="p-form__group" data-ng-if="vlanDetails.relatedControllers">
                             <div class="p-form__label">
                                 <p><strong>Rack controllers</strong>
@@ -351,6 +345,9 @@
                     </div>
                 </div>
                 <div class="row u-no-margin--top">
+                    <div class="col-8 u-vertically-center">
+                        <maas-obj-errors></maas-obj-errors>
+                    </div>
                     <div class="row u-align--right">
                         <button class="p-button--base" type="button"
                         data-ng-click="vlanDetails.exitEditSummary()">Cancel</button>
diff --git a/src/maasserver/stats.py b/src/maasserver/stats.py
index f6da73e..978af87 100644
--- a/src/maasserver/stats.py
+++ b/src/maasserver/stats.py
@@ -30,16 +30,20 @@ from maasserver.utils import get_maas_user_agent
 import requests
 
 
-def get_maas_stats():
+def get_machine_stats():
     nodes = Node.objects.all()
     machines = nodes.filter(node_type=NODE_TYPE.MACHINE)
     # Rather overall amount of stats for machines.
-    stats = machines.aggregate(
+    return machines.aggregate(
         total_cpu=Sum('cpu_count'), total_mem=Sum('memory'),
         total_storage=Sum('blockdevice__size'))
+
+
+def get_maas_stats():
     # Get all node types to get count values
     node_types = Node.objects.values_list('node_type', flat=True)
     node_types = Counter(node_types)
+    stats = get_machine_stats()
 
     return json.dumps({
         "controllers": {
diff --git a/src/maasserver/tests/test_stats.py b/src/maasserver/tests/test_stats.py
index 68cc2b1..d0200f7 100644
--- a/src/maasserver/tests/test_stats.py
+++ b/src/maasserver/tests/test_stats.py
@@ -13,6 +13,7 @@ from maasserver import stats
 from maasserver.models import Config
 from maasserver.stats import (
     get_maas_stats,
+    get_machine_stats,
     get_request_params,
     make_maas_user_agent_request,
 )
@@ -40,12 +41,18 @@ class TestMAASStats(MAASServerTestCase):
         factory.make_RegionRackController()
         factory.make_RegionController()
         factory.make_RackController()
-        m1 = factory.make_Machine(cpu_count=2, memory=200)
-        m2 = factory.make_Machine(cpu_count=3, memory=100)
+        factory.make_Machine(cpu_count=2, memory=200)
+        factory.make_Machine(cpu_count=3, memory=100)
         factory.make_Device()
 
         stats = get_maas_stats()
-        total_storage = int((m1.storage + m2.storage) * 1000000)
+        machine_stats = get_machine_stats()
+
+        # Due to floating point calculation subtleties, sometimes the value the
+        # database returns is off by one compared to the value Python
+        # calculates, so just get it directly from the database for the test.
+        total_storage = machine_stats['total_storage']
+
         compare = {
             "controllers": {
                 "regionracks": 1,