Merge lp:~blake-rouse/maas/fix-1620478 into lp:~maas-committers/maas/trunk
- fix-1620478
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Blake Rouse | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 5497 | ||||
Proposed branch: | lp:~blake-rouse/maas/fix-1620478 | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
564 lines (+88/-174) 7 files modified
src/maasserver/models/tests/test_vlan.py (+1/-71) src/maasserver/models/vlan.py (+0/-47) src/maasserver/static/js/angular/controllers/node_details_networking.js (+4/-4) src/maasserver/static/js/angular/controllers/tests/test_vlan_details.js (+4/-12) src/maasserver/static/js/angular/controllers/vlan_details.js (+4/-4) src/maasserver/websockets/handlers/tests/test_vlan.py (+51/-23) src/maasserver/websockets/handlers/vlan.py (+24/-13) |
||||
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-1620478 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Review via email: mp+308846@code.launchpad.net |
Commit message
Fix websocket handler to use VLANForm for configure_dhcp. Return system_id instead of ID for primary and secondary racks. Remove the un-needed configure_dhcp on the VLAN model.
Description of the change
To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote : | # |
One other note below, non-blocking.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/maasserver/models/tests/test_vlan.py' |
2 | --- src/maasserver/models/tests/test_vlan.py 2016-05-12 19:07:37 +0000 |
3 | +++ src/maasserver/models/tests/test_vlan.py 2016-10-20 19:40:35 +0000 |
4 | @@ -6,7 +6,6 @@ |
5 | __all__ = [] |
6 | |
7 | import random |
8 | -import re |
9 | |
10 | from django.core.exceptions import ValidationError |
11 | from django.db.models import ProtectedError |
12 | @@ -19,11 +18,7 @@ |
13 | from maasserver.testing.factory import factory |
14 | from maasserver.testing.testcase import MAASServerTestCase |
15 | from maasserver.utils.orm import reload_object |
16 | -from testtools.matchers import ( |
17 | - Equals, |
18 | - Is, |
19 | - MatchesStructure, |
20 | -) |
21 | +from testtools.matchers import MatchesStructure |
22 | from testtools.testcase import ExpectedException |
23 | |
24 | |
25 | @@ -184,71 +179,6 @@ |
26 | reload_object(subnet).vlan, fabric.get_default_vlan()) |
27 | |
28 | |
29 | -class TestVLANConfigureDHCP(MAASServerTestCase): |
30 | - |
31 | - def _regex(self, string): |
32 | - """Returns an escaped regular expression for the given string, which |
33 | - will match the given string anywhere in the input to the regex. |
34 | - """ |
35 | - return ".*" + re.escape(string) + ".*" |
36 | - |
37 | - def test__unconfigures_dhcp(self): |
38 | - primary = factory.make_RackController() |
39 | - secondary = factory.make_RackController() |
40 | - vlan = factory.make_VLAN() |
41 | - vlan.dhcp_on = True |
42 | - vlan.primary_rack = primary |
43 | - vlan.secondary_rack = secondary |
44 | - vlan.configure_dhcp([]) |
45 | - self.assertThat(vlan.dhcp_on, Equals(False)) |
46 | - self.assertThat(vlan.primary_rack, Is(None)) |
47 | - self.assertThat(vlan.secondary_rack, Is(None)) |
48 | - |
49 | - def test__configures_dhcp_with_one_controller(self): |
50 | - primary = factory.make_RackController() |
51 | - secondary = factory.make_RackController() |
52 | - vlan = factory.make_VLAN() |
53 | - vlan.dhcp_on = False |
54 | - vlan.primary_rack = primary |
55 | - vlan.secondary_rack = secondary |
56 | - vlan.configure_dhcp([primary]) |
57 | - self.assertThat(vlan.dhcp_on, Equals(True)) |
58 | - self.assertThat(vlan.primary_rack, Is(primary)) |
59 | - self.assertThat(vlan.secondary_rack, Is(None)) |
60 | - |
61 | - def test__configures_dhcp_with_two_controllers(self): |
62 | - primary = factory.make_RackController() |
63 | - secondary = factory.make_RackController() |
64 | - vlan = factory.make_VLAN() |
65 | - vlan.configure_dhcp([primary, secondary]) |
66 | - self.assertThat(vlan.dhcp_on, Equals(True)) |
67 | - self.assertThat(vlan.primary_rack, Is(primary)) |
68 | - self.assertThat(vlan.secondary_rack, Is(secondary)) |
69 | - |
70 | - def test__rejects_non_list(self): |
71 | - vlan = factory.make_VLAN() |
72 | - with ExpectedException( |
73 | - AssertionError, self._regex(VLAN.MUST_SPECIFY_LIST_ERROR)): |
74 | - vlan.configure_dhcp(1) |
75 | - |
76 | - def test__rejects_three_item_list(self): |
77 | - rack1 = factory.make_RackController() |
78 | - rack2 = factory.make_RackController() |
79 | - rack3 = factory.make_RackController() |
80 | - vlan = factory.make_VLAN() |
81 | - with ExpectedException( |
82 | - ValueError, self._regex( |
83 | - VLAN.INVALID_NUMBER_OF_CONTROLLERS_ERROR % 3)): |
84 | - vlan.configure_dhcp([rack1, rack2, rack3]) |
85 | - |
86 | - def test__rejects_list_with_duplicate_items(self): |
87 | - rack = factory.make_RackController() |
88 | - vlan = factory.make_VLAN() |
89 | - with ExpectedException( |
90 | - ValidationError, self._regex(VLAN.DUPLICATE_CONTROLLER_ERROR)): |
91 | - vlan.configure_dhcp([rack, rack]) |
92 | - |
93 | - |
94 | class TestVLANVidValidation(MAASServerTestCase): |
95 | |
96 | scenarios = [ |
97 | |
98 | === modified file 'src/maasserver/models/vlan.py' |
99 | --- src/maasserver/models/vlan.py 2016-09-04 06:45:58 +0000 |
100 | +++ src/maasserver/models/vlan.py 2016-10-20 19:40:35 +0000 |
101 | @@ -134,12 +134,6 @@ |
102 | :ivar fabric: The `Fabric` this VLAN belongs to. |
103 | """ |
104 | |
105 | - MUST_SPECIFY_LIST_ERROR = "Must specify a list of controllers." |
106 | - DUPLICATE_CONTROLLER_ERROR = "Duplicate rack controller specified." |
107 | - INVALID_NUMBER_OF_CONTROLLERS_ERROR = ( |
108 | - "Number of controllers must be zero, one, or two. (Got %d.)" |
109 | - ) |
110 | - |
111 | objects = VLANManager() |
112 | |
113 | class Meta(DefaultMeta): |
114 | @@ -195,47 +189,6 @@ |
115 | self.clean_vid() |
116 | self.clean_mtu() |
117 | |
118 | - def configure_dhcp(self, controllers): |
119 | - """Configures DHCP, based on the specified list of controllers. |
120 | - |
121 | - If the list contains zero items, DHCP will be disabled and the primary |
122 | - and secondary controllers will be cleared from this VLAN. |
123 | - |
124 | - If the list contains one item, DHCP will be enabled and the primary |
125 | - controller will be set on this VLAN. The secondary controller will be |
126 | - cleared. |
127 | - |
128 | - If the list contains two items, DHCP will be enabled and the primary |
129 | - and secondary controllers will be set on this VLAN. |
130 | - |
131 | - If the argument is not a list, a duplicate controller is specified, |
132 | - or more than two controllers are specified, raises ValidationError. |
133 | - |
134 | - Note that this method does not save the model object; that is left up |
135 | - to the caller. |
136 | - |
137 | - :param: controllers: list |
138 | - :raises: ValidationError |
139 | - """ |
140 | - assert isinstance(controllers, list), self.MUST_SPECIFY_LIST_ERROR |
141 | - if len(controllers) == 0: |
142 | - self.dhcp_on = False |
143 | - self.primary_rack = None |
144 | - self.secondary_rack = None |
145 | - elif len(controllers) == 1: |
146 | - self.dhcp_on = True |
147 | - self.primary_rack = controllers[0] |
148 | - self.secondary_rack = None |
149 | - elif len(controllers) == 2: |
150 | - if len(set(controllers)) != 2: |
151 | - raise ValidationError(self.DUPLICATE_CONTROLLER_ERROR) |
152 | - self.dhcp_on = True |
153 | - self.primary_rack = controllers[0] |
154 | - self.secondary_rack = controllers[1] |
155 | - elif len(controllers) > 2: |
156 | - raise ValueError( |
157 | - self.INVALID_NUMBER_OF_CONTROLLERS_ERROR % (len(controllers))) |
158 | - |
159 | def is_fabric_default(self): |
160 | """Is this the default VLAN in the fabric?""" |
161 | return self.fabric.get_default_vlan() == self |
162 | |
163 | === modified file 'src/maasserver/static/js/angular/controllers/node_details_networking.js' |
164 | --- src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-10-18 14:53:10 +0000 |
165 | +++ src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-10-20 19:40:35 +0000 |
166 | @@ -300,15 +300,15 @@ |
167 | "sort_key": nic.fabric.name + "|" + |
168 | $scope.getVLANText(nic.vlan) |
169 | }; |
170 | - if(nic.vlan.primary_rack_sid) { |
171 | + if(nic.vlan.primary_rack) { |
172 | vlanRecord.primary_rack = |
173 | ControllersManager.getItemFromList( |
174 | - nic.vlan.primary_rack_sid); |
175 | + nic.vlan.primary_rack); |
176 | } |
177 | - if(nic.vlan.secondary_rack_sid) { |
178 | + if(nic.vlan.secondary_rack) { |
179 | vlanRecord.secondary_rack = |
180 | ControllersManager.getItemFromList( |
181 | - nic.vlan.secondary_rack_sid); |
182 | + nic.vlan.secondary_rack); |
183 | } |
184 | vlanTable.push(vlanRecord); |
185 | } |
186 | |
187 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_vlan_details.js' |
188 | --- src/maasserver/static/js/angular/controllers/tests/test_vlan_details.js 2016-09-22 15:05:38 +0000 |
189 | +++ src/maasserver/static/js/angular/controllers/tests/test_vlan_details.js 2016-10-20 19:40:35 +0000 |
190 | @@ -20,10 +20,8 @@ |
191 | name: null, |
192 | dhcp_on: true, |
193 | space_ids: [2001], |
194 | - primary_rack: primaryController.id, |
195 | - secondary_rack: secondaryController.id, |
196 | - primary_rack_sid: primaryController.system_id, |
197 | - secondary_rack_sid: secondaryController.system_id, |
198 | + primary_rack: primaryController.system_id, |
199 | + secondary_rack: secondaryController.system_id, |
200 | rack_sids: [] |
201 | }; |
202 | VLANsManager._items.push(vlan); |
203 | @@ -281,24 +279,20 @@ |
204 | |
205 | it("updates primaryRack variable when controller changes", function() { |
206 | vlan.primary_rack = 0; |
207 | - vlan.primary_rack_sid = null; |
208 | var controller = makeControllerResolveSetActiveItem(); |
209 | expect(controller.primaryRack).toBe(null); |
210 | expect(controller.secondaryRack).toBe(secondaryController); |
211 | - vlan.primary_rack = primaryController.id; |
212 | - vlan.primary_rack_sid = primaryController.system_id; |
213 | + vlan.primary_rack = primaryController.system_id; |
214 | $scope.$digest(); |
215 | expect(controller.primaryRack).toBe(primaryController); |
216 | }); |
217 | |
218 | it("updates secondaryRack variable when controller changes", function() { |
219 | vlan.secondary_rack = 0; |
220 | - vlan.secondary_rack_sid = null; |
221 | var controller = makeControllerResolveSetActiveItem(); |
222 | expect(controller.primaryRack).toBe(primaryController); |
223 | expect(controller.secondaryRack).toBe(null); |
224 | - vlan.secondary_rack = secondaryController.id; |
225 | - vlan.secondary_rack_sid = secondaryController.system_id; |
226 | + vlan.secondary_rack = secondaryController.system_id; |
227 | $scope.$digest(); |
228 | expect(controller.secondaryRack).toBe(secondaryController); |
229 | }); |
230 | @@ -375,9 +369,7 @@ |
231 | it("performAction for enable_dhcp not called if racks are missing", |
232 | function() { |
233 | vlan.primary_rack = 0; |
234 | - vlan.primary_rack_sid = null; |
235 | vlan.secondary_rack = 0; |
236 | - vlan.secondary_rack_sid = null; |
237 | var controller = makeControllerResolveSetActiveItem(); |
238 | controller.actionOption = controller.PROVIDE_DHCP_ACTION; |
239 | // This will populate the default values for the racks with |
240 | |
241 | === modified file 'src/maasserver/static/js/angular/controllers/vlan_details.js' |
242 | --- src/maasserver/static/js/angular/controllers/vlan_details.js 2016-09-21 14:49:23 +0000 |
243 | +++ src/maasserver/static/js/angular/controllers/vlan_details.js 2016-10-20 19:40:35 +0000 |
244 | @@ -344,15 +344,15 @@ |
245 | if(!angular.isObject(vlan)) { |
246 | return; |
247 | } |
248 | - if(vlan.primary_rack_sid) { |
249 | + if(vlan.primary_rack) { |
250 | vm.primaryRack = ControllersManager.getItemFromList( |
251 | - vlan.primary_rack_sid); |
252 | + vlan.primary_rack); |
253 | } else { |
254 | vm.primaryRack = null; |
255 | } |
256 | - if(vlan.secondary_rack_sid) { |
257 | + if(vlan.secondary_rack) { |
258 | vm.secondaryRack = ControllersManager.getItemFromList( |
259 | - vlan.secondary_rack_sid); |
260 | + vlan.secondary_rack); |
261 | } else { |
262 | vm.secondaryRack = null; |
263 | } |
264 | |
265 | === modified file 'src/maasserver/websockets/handlers/tests/test_vlan.py' |
266 | --- src/maasserver/websockets/handlers/tests/test_vlan.py 2016-09-22 02:53:33 +0000 |
267 | +++ src/maasserver/websockets/handlers/tests/test_vlan.py 2016-10-20 19:40:35 +0000 |
268 | @@ -7,12 +7,15 @@ |
269 | |
270 | import random |
271 | |
272 | -from django.core.exceptions import ValidationError |
273 | +from maasserver.enum import INTERFACE_TYPE |
274 | from maasserver.models.vlan import VLAN |
275 | from maasserver.testing.factory import factory |
276 | from maasserver.testing.testcase import MAASServerTestCase |
277 | from maasserver.utils.orm import reload_object |
278 | -from maasserver.websockets.base import dehydrate_datetime |
279 | +from maasserver.websockets.base import ( |
280 | + dehydrate_datetime, |
281 | + HandlerValidationError, |
282 | +) |
283 | from maasserver.websockets.handlers.vlan import VLANHandler |
284 | from testtools import ExpectedException |
285 | from testtools.matchers import ( |
286 | @@ -139,10 +142,11 @@ |
287 | class TestVLANHandlerConfigureDHCP(MAASServerTestCase): |
288 | |
289 | def test__configure_dhcp_with_one_parameter(self): |
290 | - rack = factory.make_RackController() |
291 | user = factory.make_admin() |
292 | handler = VLANHandler(user, {}) |
293 | vlan = factory.make_VLAN() |
294 | + rack = factory.make_RackController() |
295 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
296 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
297 | handler.configure_dhcp({ |
298 | "id": vlan.id, |
299 | @@ -153,11 +157,13 @@ |
300 | self.assertThat(vlan.primary_rack, Equals(rack)) |
301 | |
302 | def test__configure_dhcp_with_two_parameters(self): |
303 | + user = factory.make_admin() |
304 | + handler = VLANHandler(user, {}) |
305 | + vlan = factory.make_VLAN() |
306 | rack = factory.make_RackController() |
307 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
308 | rack2 = factory.make_RackController() |
309 | - user = factory.make_admin() |
310 | - handler = VLANHandler(user, {}) |
311 | - vlan = factory.make_VLAN() |
312 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack2, vlan=vlan) |
313 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
314 | handler.configure_dhcp({ |
315 | "id": vlan.id, |
316 | @@ -169,22 +175,27 @@ |
317 | self.assertThat(vlan.secondary_rack, Equals(rack2)) |
318 | |
319 | def test__configure_dhcp_with_duplicate_raises(self): |
320 | - rack = factory.make_RackController() |
321 | user = factory.make_admin() |
322 | handler = VLANHandler(user, {}) |
323 | vlan = factory.make_VLAN() |
324 | + rack = factory.make_RackController() |
325 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
326 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
327 | - with ExpectedException(ValidationError): |
328 | + with ExpectedException(HandlerValidationError): |
329 | handler.configure_dhcp({ |
330 | "id": vlan.id, |
331 | "controllers": [rack.system_id, rack.system_id] |
332 | }) |
333 | |
334 | def test__configure_dhcp_with_no_parameters_disables_dhcp(self): |
335 | - rack = factory.make_RackController() |
336 | user = factory.make_admin() |
337 | handler = VLANHandler(user, {}) |
338 | - vlan = factory.make_VLAN(dhcp_on=True, primary_rack=rack) |
339 | + rack = factory.make_RackController() |
340 | + vlan = factory.make_VLAN() |
341 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
342 | + vlan.dhcp_on = True |
343 | + vlan.primary_rack = rack |
344 | + vlan.save() |
345 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
346 | handler.configure_dhcp({ |
347 | "id": vlan.id, |
348 | @@ -196,10 +207,14 @@ |
349 | self.assertThat(vlan.secondary_rack, Is(None)) |
350 | |
351 | def test__non_superuser_asserts(self): |
352 | - rack = factory.make_RackController() |
353 | user = factory.make_User() |
354 | handler = VLANHandler(user, {}) |
355 | - vlan = factory.make_VLAN(dhcp_on=True, primary_rack=rack) |
356 | + rack = factory.make_RackController() |
357 | + vlan = factory.make_VLAN() |
358 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
359 | + vlan.dhcp_on = True |
360 | + vlan.primary_rack = rack |
361 | + vlan.save() |
362 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
363 | with ExpectedException(AssertionError, "Permission denied."): |
364 | handler.configure_dhcp({ |
365 | @@ -208,12 +223,16 @@ |
366 | }) |
367 | |
368 | def test__non_superuser_reloads_user(self): |
369 | - rack = factory.make_RackController() |
370 | user = factory.make_admin() |
371 | handler = VLANHandler(user, {}) |
372 | user.is_superuser = False |
373 | user.save() |
374 | - vlan = factory.make_VLAN(dhcp_on=True, primary_rack=rack) |
375 | + rack = factory.make_RackController() |
376 | + vlan = factory.make_VLAN() |
377 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
378 | + vlan.dhcp_on = True |
379 | + vlan.primary_rack = rack |
380 | + vlan.save() |
381 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
382 | with ExpectedException(AssertionError, "Permission denied."): |
383 | handler.configure_dhcp({ |
384 | @@ -222,10 +241,11 @@ |
385 | }) |
386 | |
387 | def test__configure_dhcp_optionally_creates_iprange(self): |
388 | - rack = factory.make_RackController() |
389 | user = factory.make_admin() |
390 | handler = VLANHandler(user, {}) |
391 | vlan = factory.make_VLAN() |
392 | + rack = factory.make_RackController() |
393 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
394 | subnet = factory.make_Subnet( |
395 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
396 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
397 | @@ -251,10 +271,11 @@ |
398 | self.assertThat(dynamic_range.comment, Contains("Web UI")) |
399 | |
400 | def test__configure_dhcp_optionally_defines_gateway(self): |
401 | - rack = factory.make_RackController() |
402 | user = factory.make_admin() |
403 | handler = VLANHandler(user, {}) |
404 | vlan = factory.make_VLAN() |
405 | + rack = factory.make_RackController() |
406 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
407 | subnet = factory.make_Subnet( |
408 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
409 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
410 | @@ -275,10 +296,11 @@ |
411 | self.assertThat(subnet.gateway_ip, Equals("10.0.0.1")) |
412 | |
413 | def test__configure_dhcp_optionally_defines_gateway_and_range(self): |
414 | - rack = factory.make_RackController() |
415 | user = factory.make_admin() |
416 | handler = VLANHandler(user, {}) |
417 | vlan = factory.make_VLAN() |
418 | + rack = factory.make_RackController() |
419 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
420 | subnet = factory.make_Subnet( |
421 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
422 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
423 | @@ -307,10 +329,11 @@ |
424 | self.assertThat(dynamic_range.comment, Contains("Web UI")) |
425 | |
426 | def test__configure_dhcp_ignores_empty_gateway(self): |
427 | - rack = factory.make_RackController() |
428 | user = factory.make_admin() |
429 | handler = VLANHandler(user, {}) |
430 | vlan = factory.make_VLAN() |
431 | + rack = factory.make_RackController() |
432 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
433 | subnet = factory.make_Subnet( |
434 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
435 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
436 | @@ -339,10 +362,11 @@ |
437 | self.assertThat(dynamic_range.comment, Contains("Web UI")) |
438 | |
439 | def test__configure_dhcp_gateway_outside_subnet_raises(self): |
440 | - rack = factory.make_RackController() |
441 | user = factory.make_admin() |
442 | handler = VLANHandler(user, {}) |
443 | vlan = factory.make_VLAN() |
444 | + rack = factory.make_RackController() |
445 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
446 | subnet = factory.make_Subnet( |
447 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
448 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
449 | @@ -359,10 +383,11 @@ |
450 | }) |
451 | |
452 | def test__configure_dhcp_gateway_fe80_allowed(self): |
453 | - rack = factory.make_RackController() |
454 | user = factory.make_admin() |
455 | handler = VLANHandler(user, {}) |
456 | vlan = factory.make_VLAN() |
457 | + rack = factory.make_RackController() |
458 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
459 | subnet = factory.make_Subnet( |
460 | vlan=vlan, cidr="2001:db8::/64", gateway_ip="") |
461 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
462 | @@ -380,10 +405,11 @@ |
463 | self.assertEqual(subnet.gateway_ip, 'fe80::1') |
464 | |
465 | def test__configure_dhcp_gateway_inside_range_raises(self): |
466 | - rack = factory.make_RackController() |
467 | user = factory.make_admin() |
468 | handler = VLANHandler(user, {}) |
469 | vlan = factory.make_VLAN() |
470 | + rack = factory.make_RackController() |
471 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
472 | subnet = factory.make_Subnet( |
473 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
474 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
475 | @@ -401,10 +427,11 @@ |
476 | vlan = reload_object(vlan) |
477 | |
478 | def test__configure_dhcp_gateway_raises_if_dynamic_range_required(self): |
479 | - rack = factory.make_RackController() |
480 | user = factory.make_admin() |
481 | handler = VLANHandler(user, {}) |
482 | vlan = factory.make_VLAN() |
483 | + rack = factory.make_RackController() |
484 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
485 | subnet = factory.make_Subnet( |
486 | vlan=vlan, cidr="10.0.0.0/24", gateway_ip="") |
487 | self.assertThat(subnet.get_dynamic_ranges().count(), Equals(0)) |
488 | @@ -421,10 +448,11 @@ |
489 | }) |
490 | |
491 | def test__configure_dhcp_ignores_undefined_subnet(self): |
492 | - rack = factory.make_RackController() |
493 | user = factory.make_admin() |
494 | handler = VLANHandler(user, {}) |
495 | vlan = factory.make_VLAN() |
496 | + rack = factory.make_RackController() |
497 | + factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=rack, vlan=vlan) |
498 | factory.make_ipv4_Subnet_with_IPRanges(vlan=vlan) |
499 | handler.configure_dhcp({ |
500 | "id": vlan.id, |
501 | |
502 | === modified file 'src/maasserver/websockets/handlers/vlan.py' |
503 | --- src/maasserver/websockets/handlers/vlan.py 2016-09-22 02:53:33 +0000 |
504 | +++ src/maasserver/websockets/handlers/vlan.py 2016-10-20 19:40:35 +0000 |
505 | @@ -16,11 +16,11 @@ |
506 | from maasserver.models import ( |
507 | Fabric, |
508 | IPRange, |
509 | - RackController, |
510 | Subnet, |
511 | VLAN, |
512 | ) |
513 | from maasserver.utils.orm import reload_object |
514 | +from maasserver.websockets.base import HandlerValidationError |
515 | from maasserver.websockets.handlers.timestampedmodel import ( |
516 | TimestampedModelHandler, |
517 | ) |
518 | @@ -56,13 +56,19 @@ |
519 | "vlan", |
520 | ] |
521 | |
522 | + def dehydrate_primary_rack(self, rack): |
523 | + if rack is None: |
524 | + return None |
525 | + else: |
526 | + return rack.system_id |
527 | + |
528 | + def dehydrate_secondary_rack(self, rack): |
529 | + if rack is None: |
530 | + return None |
531 | + else: |
532 | + return rack.system_id |
533 | + |
534 | def dehydrate(self, obj, data, for_list=False): |
535 | - # We need the system_id for each controller, since that's how we |
536 | - # need to look them up inside the Javascript controller. |
537 | - if obj.primary_rack is not None: |
538 | - data["primary_rack_sid"] = obj.primary_rack.system_id |
539 | - if obj.secondary_rack is not None: |
540 | - data["secondary_rack_sid"] = obj.secondary_rack.system_id |
541 | nodes = { |
542 | interface.node |
543 | for interface in obj.interface_set.all() |
544 | @@ -171,9 +177,14 @@ |
545 | raise ValueError( |
546 | "Cannot configure DHCP: At least one dynamic range is " |
547 | "required.") |
548 | - controllers = [ |
549 | - RackController.objects.get(system_id=system_id) |
550 | - for system_id in parameters['controllers'] |
551 | - ] |
552 | - vlan.configure_dhcp(controllers) |
553 | - vlan.save() |
554 | + controllers = parameters.get('controllers', []) |
555 | + data = { |
556 | + "dhcp_on": True if len(controllers) > 0 else False, |
557 | + "primary_rack": controllers[0] if len(controllers) > 0 else None, |
558 | + "secondary_rack": controllers[1] if len(controllers) > 1 else None, |
559 | + } |
560 | + form = VLANForm(instance=vlan, data=data) |
561 | + if form.is_valid(): |
562 | + form.save() |
563 | + else: |
564 | + raise HandlerValidationError(form.errors) |
Looks good, thanks for the cleanup.
Looks like these constants in the VLAN model can be probably be removed too:
MUST_ SPECIFY_ LIST_ERROR CONTROLLER_ ERROR NUMBER_ OF_CONTROLLERS_ ERROR
DUPLICATE_
INVALID_
One other comment below in the VLAN handler.