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

Proposed by Mike Pontillo on 2018-04-03
Status: Merged
Approved by: Mike Pontillo on 2018-04-03
Approved revision: cd5f92a6481eeae6240a1e0f3a955cfb3233daab
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:bug-1704501--2.3
Merge into: maas:2.3
Diff against target: 190 lines (+94/-7)
4 files modified
src/maasserver/forms/tests/test_vlan.py (+65/-0)
src/maasserver/forms/vlan.py (+5/-0)
src/maasserver/models/vlan.py (+16/-0)
src/maasserver/static/partials/vlan-details.html (+8/-7)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve on 2018-04-03
Review via email: mp+342604@code.launchpad.net

Commit message

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

Backports: 4083dc0635958109a28ef44a84df63e855007335

To post a comment you must log in.
~mpontillo/maas:bug-1704501--2.3 updated on 2018-04-03
cd5f92a... by Mike Pontillo on 2018-04-03

Fix fabric field formatting.

Mike Pontillo (mpontillo) wrote :

Self-approve backport.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/forms/tests/test_vlan.py b/src/maasserver/forms/tests/test_vlan.py
2index 8db5f5a..5c5aacb 100644
3--- a/src/maasserver/forms/tests/test_vlan.py
4+++ b/src/maasserver/forms/tests/test_vlan.py
5@@ -13,6 +13,11 @@ from maasserver.models.vlan import DEFAULT_MTU
6 from maasserver.testing.factory import factory
7 from maasserver.testing.testcase import MAASServerTestCase
8 from maasserver.utils.orm import reload_object
9+from testtools import ExpectedException
10+from testtools.matchers import (
11+ Equals,
12+ Not,
13+)
14
15
16 class TestVLANForm(MAASServerTestCase):
17@@ -371,3 +376,63 @@ class TestVLANForm(MAASServerTestCase):
18 self.assertEqual(secondary_rack, vlan.primary_rack)
19 self.assertEqual(None, vlan.secondary_rack)
20 self.assertTrue(vlan.dhcp_on)
21+
22+
23+class TestVLANFormFabricModification(MAASServerTestCase):
24+
25+ def test__cannot_move_vlan_with_overlapping_vid(self):
26+ fabric0 = Fabric.objects.get_default_fabric()
27+ fabric1 = factory.make_Fabric()
28+ fabric1_untagged = fabric1.get_default_vlan()
29+ form = VLANForm(instance=fabric1_untagged, data={
30+ "fabric": fabric0.id
31+ })
32+ is_valid = form.is_valid()
33+ self.assertThat(is_valid, Equals(False))
34+ self.assertThat(dict(form.errors), Equals(
35+ {'__all__': [
36+ 'A VLAN with the specified VID already '
37+ 'exists in the destination fabric.'
38+ ]}
39+ ))
40+ with ExpectedException(ValueError):
41+ form.save()
42+
43+ def test__allows_moving_vlan_to_new_fabric_if_vid_is_unique(self):
44+ fabric0 = Fabric.objects.get_default_fabric()
45+ fabric1 = factory.make_Fabric()
46+ fabric1_untagged = fabric1.get_default_vlan()
47+ form = VLANForm(instance=fabric1_untagged, data={
48+ "fabric": fabric0.id,
49+ "vid": 10
50+ })
51+ is_valid = form.is_valid()
52+ self.assertThat(is_valid, Equals(True))
53+ form.save()
54+
55+ def test__deletes_empty_fabrics(self):
56+ fabric0 = Fabric.objects.get_default_fabric()
57+ fabric1 = factory.make_Fabric()
58+ fabric1_untagged = fabric1.get_default_vlan()
59+ form = VLANForm(instance=fabric1_untagged, data={
60+ "fabric": fabric0.id,
61+ "vid": 10
62+ })
63+ is_valid = form.is_valid()
64+ self.assertThat(is_valid, Equals(True))
65+ form.save()
66+ self.assertThat(reload_object(fabric1), Equals(None))
67+
68+ def test__does_not_delete_non_empty_fabrics(self):
69+ fabric0 = Fabric.objects.get_default_fabric()
70+ fabric1 = factory.make_Fabric()
71+ factory.make_VLAN(fabric=fabric1)
72+ fabric1_untagged = fabric1.get_default_vlan()
73+ form = VLANForm(instance=fabric1_untagged, data={
74+ "fabric": fabric0.id,
75+ "vid": 10
76+ })
77+ is_valid = form.is_valid()
78+ form.save()
79+ self.assertThat(is_valid, Equals(True))
80+ self.assertThat(reload_object(fabric1), Not(Equals(None)))
81diff --git a/src/maasserver/forms/vlan.py b/src/maasserver/forms/vlan.py
82index 13fad27..89e5f4d 100644
83--- a/src/maasserver/forms/vlan.py
84+++ b/src/maasserver/forms/vlan.py
85@@ -15,6 +15,7 @@ from maasserver.fields import (
86 )
87 from maasserver.forms import MAASModelForm
88 from maasserver.models import (
89+ Fabric,
90 RackController,
91 Space,
92 )
93@@ -30,6 +31,9 @@ class VLANForm(MAASModelForm):
94 space = SpecifierOrModelChoiceField(
95 queryset=Space.objects.all(), required=False, empty_label="")
96
97+ fabric = SpecifierOrModelChoiceField(
98+ queryset=Fabric.objects.all(), required=False, empty_label="")
99+
100 class Meta:
101 model = VLAN
102 fields = (
103@@ -42,6 +46,7 @@ class VLANForm(MAASModelForm):
104 'secondary_rack',
105 'relay_vlan',
106 'space',
107+ 'fabric',
108 )
109
110 def __init__(self, *args, **kwargs):
111diff --git a/src/maasserver/models/vlan.py b/src/maasserver/models/vlan.py
112index 28e9f42..35e34a9 100644
113--- a/src/maasserver/models/vlan.py
114+++ b/src/maasserver/models/vlan.py
115@@ -15,6 +15,7 @@ from django.db.models import (
116 BooleanField,
117 CASCADE,
118 CharField,
119+ Count,
120 deletion,
121 ForeignKey,
122 IntegerField,
123@@ -243,6 +244,15 @@ class VLAN(CleanSave, TimestampedModel):
124 subnet.vlan = self.fabric.get_default_vlan()
125 subnet.save()
126
127+ def unique_error_message(self, model_class, unique_check):
128+ if set(unique_check) == {'vid', 'fabric'}:
129+ return (
130+ 'A VLAN with the specified VID already exists in the '
131+ 'destination fabric.'
132+ )
133+ else:
134+ return super().unique_error_message(model_class, unique_check)
135+
136 def delete(self):
137 if self.is_fabric_default():
138 raise ValidationError(
139@@ -268,3 +278,9 @@ class VLAN(CleanSave, TimestampedModel):
140 "DHCP server is being used.",
141 ident="dhcp_disabled_all_vlans")
142 super().save(*args, **kwargs)
143+ # Circular dependencies.
144+ from maasserver.models import Fabric
145+ # Delete any now-empty fabrics.
146+ fabrics_with_vlan_count = Fabric.objects.annotate(
147+ vlan_count=Count("vlan"))
148+ fabrics_with_vlan_count.filter(vlan_count=0).delete()
149diff --git a/src/maasserver/static/partials/vlan-details.html b/src/maasserver/static/partials/vlan-details.html
150index a526ed9..b02043f 100644
151--- a/src/maasserver/static/partials/vlan-details.html
152+++ b/src/maasserver/static/partials/vlan-details.html
153@@ -220,7 +220,7 @@
154 </div>
155 </header>
156 <section class="row">
157- <div class="wrapper--inner">
158+ <fieldset class="wrapper--inner">
159 <div class="twelve-col">
160 <h2 class="u-float--left">VLAN Summary</h2>
161 <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>
162@@ -268,12 +268,10 @@
163 <maas-obj-field type="textarea" key="description" label="Description" placeholder="VLAN description"
164 label-width="two" input-width="three" blur-on-enter="true"></maas-obj-field>
165 </fieldset>
166- <div class="six-col last-col">
167+ <fieldset class="form__fieldset six-col last-col">
168 <dl>
169- <dt class="two-col">Fabric</dt>
170- <dd class="four-col last-col">
171- <a href="#/fabric/{$ vlanDetails.fabric.id $}">{$ vlanDetails.fabric.name $}</a>
172- </dd>
173+ <maas-obj-field type="options" key="fabric" label="Fabric" label-width="two" input-width="three"
174+ options="fabric.id as fabric.name for fabric in vlanDetails.fabrics"></maas-obj-field>
175 <div data-ng-if="vlanDetails.relatedControllers">
176 <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>
177 <dd class="four-col last-col">
178@@ -283,8 +281,11 @@
179 </dd>
180 </div>
181 </dl>
182+ </fieldset>
183+ <div class="six-col">
184+ <maas-obj-errors></maas-obj-errors>
185 </div>
186- <div class="twelve-col u-align--right">
187+ <div class="six-col last-col u-align--right">
188 <button class="button--base button--inline"
189 data-ng-click="vlanDetails.exitEditSummary()">Cancel</button>
190 <button class="button--positive button--inline" maas-obj-save>Save summary</button>

Subscribers

People subscribed via source and target branches

to all changes: