Merge lp:~blake-rouse/maas/fix-1441837 into lp:~maas-committers/maas/trunk
- fix-1441837
- Merge into trunk
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 3947 |
Proposed branch: | lp:~blake-rouse/maas/fix-1441837 |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
1501 lines (+722/-273) 8 files modified
src/maasserver/context_processors.py (+1/-0) src/maasserver/static/js/angular/controllers/add_device.js (+107/-41) src/maasserver/static/js/angular/controllers/tests/test_add_device.js (+209/-95) src/maasserver/static/js/angular/directives/placeholder.js (+22/-0) src/maasserver/static/js/angular/directives/tests/test_placeholder.js (+55/-0) src/maasserver/static/partials/nodes-list.html (+57/-39) src/maasserver/websockets/handlers/device.py (+115/-73) src/maasserver/websockets/handlers/tests/test_device.py (+156/-25) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-1441837 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Raphaël Badin (community) | Approve | ||
Carla Berkers | Pending | ||
Review via email: mp+259046@code.launchpad.net |
Commit message
Add the ability to add multiple interfaces when creating a device. Update the implementation to match design.
Description of the change
This is a large change since it completely changed the implementation of how the AddDeviceController was created. The websocket handler code also needed to be updated to support the new logic.
Design wanted the static range to be in the placeholder text of the input when doing the static ip assignment. This logic required a directive that is very small, so it is included in this branch.
Blake Rouse (blake-rouse) wrote : | # |
Blake Rouse (blake-rouse) wrote : | # |
I have QA'd this as well from packaging to make sure that devices with multiple interfaces are created as expected.
Raphaël Badin (rvb) wrote : | # |
Looks okay. Some of it clearly is potato programming but I guess we will have to land this and refactor the backend side (the part that allocates IP addresses) when we get a chance.
AFAIK commit_
Blake Rouse (blake-rouse) wrote : | # |
Thanks for the review.
Yeah I agree I hate the backend side of this code. Yes commit_
Preview Diff
1 | === modified file 'src/maasserver/context_processors.py' |
2 | --- src/maasserver/context_processors.py 2015-05-13 11:33:20 +0000 |
3 | +++ src/maasserver/context_processors.py 2015-05-27 00:50:29 +0000 |
4 | @@ -64,6 +64,7 @@ |
5 | 'js/angular/directives/dbl_click_overlay.js', |
6 | 'js/angular/directives/contenteditable.js', |
7 | 'js/angular/directives/sticky_header.js', |
8 | + 'js/angular/directives/placeholder.js', |
9 | 'js/angular/filters/nodes.js', |
10 | 'js/angular/controllers/error.js', |
11 | 'js/angular/controllers/nodes_list.js', |
12 | |
13 | === modified file 'src/maasserver/static/js/angular/controllers/add_device.js' |
14 | --- src/maasserver/static/js/angular/controllers/add_device.js 2015-05-13 21:10:11 +0000 |
15 | +++ src/maasserver/static/js/angular/controllers/add_device.js 2015-05-27 00:50:29 +0000 |
16 | @@ -35,10 +35,9 @@ |
17 | } |
18 | ]; |
19 | |
20 | - // Makes a new device. |
21 | - function makeDevice() { |
22 | + // Makes a new interface. |
23 | + function makeInterface() { |
24 | return { |
25 | - name: "", |
26 | mac: "", |
27 | ipAssignment: null, |
28 | clusterInterfaceId: null, |
29 | @@ -46,6 +45,14 @@ |
30 | }; |
31 | } |
32 | |
33 | + // Makes a new device. |
34 | + function makeDevice() { |
35 | + return { |
36 | + name: "", |
37 | + interfaces: [makeInterface()] |
38 | + }; |
39 | + } |
40 | + |
41 | // Initial device. |
42 | $scope.device = makeDevice(); |
43 | |
44 | @@ -53,13 +60,25 @@ |
45 | // how it is handled over the websocket. |
46 | function convertDeviceToProtocol(device) { |
47 | // Return the new object. |
48 | - return { |
49 | + var convertedDevice = { |
50 | hostname: device.name, |
51 | - primary_mac: device.mac, |
52 | - ip_assignment: device.ipAssignment.name, |
53 | - ip_address: device.ipAddress, |
54 | - "interface": device.clusterInterfaceId |
55 | + primary_mac: device.interfaces[0].mac, |
56 | + extra_macs: [], |
57 | + interfaces: [] |
58 | }; |
59 | + var i; |
60 | + for(i = 1; i < device.interfaces.length; i++) { |
61 | + convertedDevice.extra_macs.push(device.interfaces[i].mac); |
62 | + } |
63 | + angular.forEach(device.interfaces, function(nic) { |
64 | + convertedDevice.interfaces.push({ |
65 | + mac: nic.mac, |
66 | + ip_assignment: nic.ipAssignment.name, |
67 | + ip_address: nic.ipAddress, |
68 | + "interface": nic.clusterInterfaceId |
69 | + }); |
70 | + }); |
71 | + return convertedDevice; |
72 | } |
73 | |
74 | // Gets the cluster interface by id from the managed cluster |
75 | @@ -106,9 +125,16 @@ |
76 | }; |
77 | |
78 | // Return text to show an interfaces static range. |
79 | - $scope.getInterfaceStaticRange = function(nic) { |
80 | - return nic.network + " (" + |
81 | - nic.static_range.low + " - " + nic.static_range.high + ")"; |
82 | + $scope.getInterfaceStaticRange = function(cInterfaceId) { |
83 | + if(!angular.isNumber(cInterfaceId)) { |
84 | + return ""; |
85 | + } |
86 | + var clusterInterface = getInterfaceById(cInterfaceId); |
87 | + if(!angular.isObject(clusterInterface)) { |
88 | + return ""; |
89 | + } |
90 | + return clusterInterface.static_range.low + " - " + |
91 | + clusterInterface.static_range.high + " (Optional)"; |
92 | }; |
93 | |
94 | // Returns true if the name is in error. |
95 | @@ -121,47 +147,60 @@ |
96 | }; |
97 | |
98 | // Returns true if the MAC is in error. |
99 | - $scope.macHasError = function() { |
100 | + $scope.macHasError = function(deviceInterface) { |
101 | // If the MAC is empty don't show error. |
102 | - if($scope.device.mac.length === 0) { |
103 | + if(deviceInterface.mac.length === 0) { |
104 | return false; |
105 | } |
106 | - return !ValidationService.validateMAC($scope.device.mac); |
107 | + // If the MAC is invalid show error. |
108 | + if(!ValidationService.validateMAC(deviceInterface.mac)) { |
109 | + return true; |
110 | + } |
111 | + // If the MAC is the same as another MAC show error. |
112 | + var i; |
113 | + for(i = 0; i < $scope.device.interfaces.length; i++) { |
114 | + var isSelf = $scope.device.interfaces[i] === deviceInterface; |
115 | + if(!isSelf && |
116 | + $scope.device.interfaces[i].mac === deviceInterface.mac) { |
117 | + return true; |
118 | + } |
119 | + } |
120 | + return false; |
121 | }; |
122 | |
123 | // Returns true if the IP address is in error. |
124 | - $scope.ipHasError = function() { |
125 | + $scope.ipHasError = function(deviceInterface) { |
126 | // If the IP is empty don't show error. |
127 | - if($scope.device.ipAddress.length === 0) { |
128 | + if(deviceInterface.ipAddress.length === 0) { |
129 | return false; |
130 | } |
131 | // If ip address is invalid, then exit early. |
132 | - if(!ValidationService.validateIP($scope.device.ipAddress)) { |
133 | + if(!ValidationService.validateIP(deviceInterface.ipAddress)) { |
134 | return true; |
135 | } |
136 | var i, inNetwork, managedInterfaces = $scope.getManagedInterfaces(); |
137 | - if(angular.isObject($scope.device.ipAssignment)){ |
138 | - if($scope.device.ipAssignment.name === "external") { |
139 | + if(angular.isObject(deviceInterface.ipAssignment)){ |
140 | + if(deviceInterface.ipAssignment.name === "external") { |
141 | // External IP address cannot be within a managed interface |
142 | // on one of the clusters. |
143 | for(i = 0; i < managedInterfaces.length; i++) { |
144 | inNetwork = ValidationService.validateIPInNetwork( |
145 | - $scope.device.ipAddress, |
146 | + deviceInterface.ipAddress, |
147 | managedInterfaces[i].network); |
148 | if(inNetwork) { |
149 | return true; |
150 | } |
151 | } |
152 | - } else if($scope.device.ipAssignment.name === "static" && |
153 | - angular.isNumber($scope.device.clusterInterfaceId)) { |
154 | + } else if(deviceInterface.ipAssignment.name === "static" && |
155 | + angular.isNumber(deviceInterface.clusterInterfaceId)) { |
156 | // Static IP address must be within the static range |
157 | // of the selected clusterInterface. |
158 | var clusterInterface = getInterfaceById( |
159 | - $scope.device.clusterInterfaceId); |
160 | + deviceInterface.clusterInterfaceId); |
161 | inNetwork = ValidationService.validateIPInNetwork( |
162 | - $scope.device.ipAddress, clusterInterface.network); |
163 | + deviceInterface.ipAddress, clusterInterface.network); |
164 | var inDynamicRange = ValidationService.validateIPInRange( |
165 | - $scope.device.ipAddress, clusterInterface.network, |
166 | + deviceInterface.ipAddress, clusterInterface.network, |
167 | clusterInterface.dynamic_range.low, |
168 | clusterInterface.dynamic_range.high); |
169 | if(!inNetwork || inDynamicRange) { |
170 | @@ -175,26 +214,53 @@ |
171 | // Return true when the device is missing information or invalid |
172 | // information. |
173 | $scope.deviceHasError = function() { |
174 | - // Early-out for errors. |
175 | - in_error = ( |
176 | - $scope.device.name === '' || |
177 | - $scope.device.mac === '' || |
178 | - !angular.isObject($scope.device.ipAssignment) || |
179 | - $scope.nameHasError() || |
180 | - $scope.macHasError()); |
181 | - if(in_error) { |
182 | - return in_error; |
183 | - } |
184 | - if($scope.device.ipAssignment.name === "external") { |
185 | - return $scope.device.ipAddress === '' || $scope.ipHasError(); |
186 | - } |
187 | - if($scope.device.ipAssignment.name === "static") { |
188 | - return !angular.isNumber($scope.device.clusterInterfaceId) || |
189 | - $scope.ipHasError(); |
190 | + if($scope.device.name === '' || $scope.nameHasError()) { |
191 | + return true; |
192 | + } |
193 | + |
194 | + var i; |
195 | + for(i = 0; i < $scope.device.interfaces.length; i++) { |
196 | + var deviceInterface = $scope.device.interfaces[i]; |
197 | + if(deviceInterface.mac === '' || |
198 | + $scope.macHasError(deviceInterface) || |
199 | + !angular.isObject(deviceInterface.ipAssignment)) { |
200 | + return true; |
201 | + } |
202 | + var externalIpError = ( |
203 | + deviceInterface.ipAssignment.name === "external" && ( |
204 | + deviceInterface.ipAddress === '' || |
205 | + $scope.ipHasError(deviceInterface))); |
206 | + var staticIpError = ( |
207 | + deviceInterface.ipAssignment.name === "static" && ( |
208 | + !angular.isNumber(deviceInterface.clusterInterfaceId) || |
209 | + $scope.ipHasError(deviceInterface))); |
210 | + if(externalIpError || staticIpError) { |
211 | + return true; |
212 | + } |
213 | } |
214 | return false; |
215 | }; |
216 | |
217 | + // Adds new interface to device. |
218 | + $scope.addInterface = function() { |
219 | + $scope.device.interfaces.push(makeInterface()); |
220 | + }; |
221 | + |
222 | + // Returns true if the first interface in the device interfaces array. |
223 | + $scope.isPrimaryInterface = function(deviceInterface) { |
224 | + return $scope.device.interfaces.indexOf(deviceInterface) === 0; |
225 | + }; |
226 | + |
227 | + // Removes the interface from the devices interfaces array. |
228 | + $scope.deleteInterface = function(deviceInterface) { |
229 | + // Don't remove the primary. |
230 | + if($scope.isPrimaryInterface(deviceInterface)) { |
231 | + return; |
232 | + } |
233 | + $scope.device.interfaces.splice( |
234 | + $scope.device.interfaces.indexOf(deviceInterface), 1); |
235 | + }; |
236 | + |
237 | // Called when cancel clicked. |
238 | $scope.cancel = function() { |
239 | $scope.error = null; |
240 | |
241 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_add_device.js' |
242 | --- src/maasserver/static/js/angular/controllers/tests/test_add_device.js 2015-05-13 21:22:16 +0000 |
243 | +++ src/maasserver/static/js/angular/controllers/tests/test_add_device.js 2015-05-27 00:50:29 +0000 |
244 | @@ -158,6 +158,28 @@ |
245 | }; |
246 | } |
247 | |
248 | + // Make a interface |
249 | + function makeInterface(mac, ipAssignment, clusterInterfaceId, ipAddress) { |
250 | + if(angular.isUndefined(mac)) { |
251 | + mac = ""; |
252 | + } |
253 | + if(angular.isUndefined(ipAssignment)) { |
254 | + ipAssignment = null; |
255 | + } |
256 | + if(angular.isUndefined(clusterInterfaceId)) { |
257 | + clusterInterfaceId = null; |
258 | + } |
259 | + if(angular.isUndefined(ipAddress)) { |
260 | + ipAddress = ""; |
261 | + } |
262 | + return { |
263 | + mac: mac, |
264 | + ipAssignment: ipAssignment, |
265 | + clusterInterfaceId: clusterInterfaceId, |
266 | + ipAddress: ipAddress |
267 | + }; |
268 | + } |
269 | + |
270 | it("sets addDeviceScope on $scope.$parent", function() { |
271 | var controller = makeController(); |
272 | expect(parentScope.addDeviceScope).toBe($scope); |
273 | @@ -184,10 +206,12 @@ |
274 | ]); |
275 | expect($scope.device).toEqual({ |
276 | name: "", |
277 | - mac: "", |
278 | - ipAssignment: null, |
279 | - clusterInterfaceId: null, |
280 | - ipAddress: "" |
281 | + interfaces: [{ |
282 | + mac: "", |
283 | + ipAssignment: null, |
284 | + clusterInterfaceId: null, |
285 | + ipAddress: "" |
286 | + }] |
287 | }); |
288 | }); |
289 | |
290 | @@ -263,9 +287,12 @@ |
291 | it("returns text including low and high of static range", function() { |
292 | var controller = makeController(); |
293 | var nic = makeManagedClusterInterface(); |
294 | - expect($scope.getInterfaceStaticRange(nic)).toEqual( |
295 | - nic.network + " (" + nic.static_range.low + |
296 | - " - " + nic.static_range.high + ")"); |
297 | + $scope.clusters = [ |
298 | + makeCluster([nic]) |
299 | + ]; |
300 | + expect($scope.getInterfaceStaticRange(nic.id)).toEqual( |
301 | + nic.static_range.low + " - " + nic.static_range.high + |
302 | + " (Optional)"); |
303 | }); |
304 | }); |
305 | |
306 | @@ -293,19 +320,44 @@ |
307 | |
308 | it("returns false if mac is empty", function() { |
309 | var controller = makeController(); |
310 | - expect($scope.macHasError()).toBe(false); |
311 | + var nic = makeInterface(); |
312 | + expect($scope.macHasError(nic)).toBe(false); |
313 | }); |
314 | |
315 | it("returns false if valid mac", function() { |
316 | var controller = makeController(); |
317 | - $scope.device.mac = "00:00:11:22:33:44"; |
318 | - expect($scope.macHasError()).toBe(false); |
319 | + var nic = makeInterface("00:00:11:22:33:44"); |
320 | + expect($scope.macHasError(nic)).toBe(false); |
321 | + }); |
322 | + |
323 | + it("returns false if not repeat mac", function() { |
324 | + var controller = makeController(); |
325 | + var nic = makeInterface("00:00:11:22:33:44"); |
326 | + var nic2 = makeInterface("00:00:11:22:33:55"); |
327 | + $scope.device.interfaces = [ |
328 | + nic, |
329 | + nic2 |
330 | + ]; |
331 | + expect($scope.macHasError(nic)).toBe(false); |
332 | + expect($scope.macHasError(nic2)).toBe(false); |
333 | }); |
334 | |
335 | it("returns true if invalid mac", function() { |
336 | var controller = makeController(); |
337 | - $scope.device.mac = "00:00:11:22:33"; |
338 | - expect($scope.macHasError()).toBe(true); |
339 | + var nic = makeInterface("00:00:11:22:33"); |
340 | + expect($scope.macHasError(nic)).toBe(true); |
341 | + }); |
342 | + |
343 | + it("returns true if repeat mac", function() { |
344 | + var controller = makeController(); |
345 | + var nic = makeInterface("00:00:11:22:33:44"); |
346 | + var nic2 = makeInterface("00:00:11:22:33:44"); |
347 | + $scope.device.interfaces = [ |
348 | + nic, |
349 | + nic2 |
350 | + ]; |
351 | + expect($scope.macHasError(nic)).toBe(true); |
352 | + expect($scope.macHasError(nic2)).toBe(true); |
353 | }); |
354 | }); |
355 | |
356 | @@ -313,31 +365,36 @@ |
357 | |
358 | it("returns false if ip is empty", function() { |
359 | var controller = makeController(); |
360 | - expect($scope.ipHasError()).toBe(false); |
361 | + var nic = makeInterface(); |
362 | + expect($scope.ipHasError(nic)).toBe(false); |
363 | }); |
364 | |
365 | it("returns false if valid ipv4", function() { |
366 | var controller = makeController(); |
367 | - $scope.device.ipAddress = "192.168.1.1"; |
368 | - expect($scope.ipHasError()).toBe(false); |
369 | + var nic = makeInterface(); |
370 | + nic.ipAddress = "192.168.1.1"; |
371 | + expect($scope.ipHasError(nic)).toBe(false); |
372 | }); |
373 | |
374 | it("returns false if valid ipv6", function() { |
375 | var controller = makeController(); |
376 | - $scope.device.ipAddress = "2001:db8::1"; |
377 | - expect($scope.ipHasError()).toBe(false); |
378 | + var nic = makeInterface(); |
379 | + nic.ipAddress = "2001:db8::1"; |
380 | + expect($scope.ipHasError(nic)).toBe(false); |
381 | }); |
382 | |
383 | it("returns true if invalid ipv4", function() { |
384 | var controller = makeController(); |
385 | - $scope.device.ipAddress = "192.168.1"; |
386 | - expect($scope.ipHasError()).toBe(true); |
387 | + var nic = makeInterface(); |
388 | + nic.ipAddress = "192.168.1"; |
389 | + expect($scope.ipHasError(nic)).toBe(true); |
390 | }); |
391 | |
392 | it("returns true if invalid ipv6", function() { |
393 | var controller = makeController(); |
394 | - $scope.device.ipAddress = "2001::db8::1"; |
395 | - expect($scope.ipHasError()).toBe(true); |
396 | + var nic = makeInterface(); |
397 | + nic.ipAddress = "2001::db8::1"; |
398 | + expect($scope.ipHasError(nic)).toBe(true); |
399 | }); |
400 | |
401 | it("returns false if external ip out of managed network", function() { |
402 | @@ -346,11 +403,12 @@ |
403 | var cluster = makeCluster([nic]); |
404 | $scope.clusters = [cluster]; |
405 | // No class A address is in the fake networks. |
406 | - $scope.device.ipAddress = "10.0.1.1"; |
407 | - $scope.device.ipAssignment = { |
408 | + var deviceInterface = makeInterface(); |
409 | + deviceInterface.ipAddress = "10.0.1.1"; |
410 | + deviceInterface.ipAssignment = { |
411 | name: "external" |
412 | }; |
413 | - expect($scope.ipHasError()).toBe(false); |
414 | + expect($scope.ipHasError(deviceInterface)).toBe(false); |
415 | }); |
416 | |
417 | it("returns true if external ip in managed network", function() { |
418 | @@ -358,11 +416,12 @@ |
419 | var nic = makeManagedClusterInterface(); |
420 | var cluster = makeCluster([nic]); |
421 | $scope.clusters = [cluster]; |
422 | - $scope.device.ipAddress = nic.static_range.low; |
423 | - $scope.device.ipAssignment = { |
424 | + var deviceInterface = makeInterface(); |
425 | + deviceInterface.ipAddress = nic.static_range.low; |
426 | + deviceInterface.ipAssignment = { |
427 | name: "external" |
428 | }; |
429 | - expect($scope.ipHasError()).toBe(true); |
430 | + expect($scope.ipHasError(deviceInterface)).toBe(true); |
431 | }); |
432 | |
433 | it("returns false if static in managed network", function() { |
434 | @@ -370,11 +429,12 @@ |
435 | var nic = makeManagedClusterInterface(); |
436 | var cluster = makeCluster([nic]); |
437 | $scope.clusters = [cluster]; |
438 | - $scope.device.ipAddress = nic.static_range.low; |
439 | - $scope.device.ipAssignment = { |
440 | + var deviceInterface = makeInterface(); |
441 | + deviceInterface.ipAddress = nic.static_range.low; |
442 | + deviceInterface.ipAssignment = { |
443 | name: "static" |
444 | }; |
445 | - expect($scope.ipHasError()).toBe(false); |
446 | + expect($scope.ipHasError(deviceInterface)).toBe(false); |
447 | }); |
448 | |
449 | it("returns false if static ip in select network", function() { |
450 | @@ -382,12 +442,13 @@ |
451 | var nic = makeManagedClusterInterface(); |
452 | var cluster = makeCluster([nic]); |
453 | $scope.clusters = [cluster]; |
454 | - $scope.device.ipAddress = nic.static_range.low; |
455 | - $scope.device.clusterInterfaceId = nic.id; |
456 | - $scope.device.ipAssignment = { |
457 | + var deviceInterface = makeInterface(); |
458 | + deviceInterface.ipAddress = nic.static_range.low; |
459 | + deviceInterface.clusterInterfaceId = nic.id; |
460 | + deviceInterface.ipAssignment = { |
461 | name: "static" |
462 | }; |
463 | - expect($scope.ipHasError()).toBe(false); |
464 | + expect($scope.ipHasError(deviceInterface)).toBe(false); |
465 | }); |
466 | |
467 | it("returns true if static ip out of select network", function() { |
468 | @@ -396,12 +457,13 @@ |
469 | var otherNic = makeManagedClusterInterface(); |
470 | var cluster = makeCluster([nic]); |
471 | $scope.clusters = [cluster]; |
472 | - $scope.device.ipAddress = otherNic.static_range.low; |
473 | - $scope.device.clusterInterfaceId = nic.id; |
474 | - $scope.device.ipAssignment = { |
475 | + var deviceInterface = makeInterface(); |
476 | + deviceInterface.ipAddress = otherNic.static_range.low; |
477 | + deviceInterface.clusterInterfaceId = nic.id; |
478 | + deviceInterface.ipAssignment = { |
479 | name: "static" |
480 | }; |
481 | - expect($scope.ipHasError()).toBe(true); |
482 | + expect($scope.ipHasError(deviceInterface)).toBe(true); |
483 | }); |
484 | |
485 | it("returns true if static ip in dynamic range of network", function() { |
486 | @@ -409,12 +471,13 @@ |
487 | var nic = makeManagedClusterInterface(); |
488 | var cluster = makeCluster([nic]); |
489 | $scope.clusters = [cluster]; |
490 | - $scope.device.ipAddress = nic.dynamic_range.low; |
491 | - $scope.device.clusterInterfaceId = nic.id; |
492 | - $scope.device.ipAssignment = { |
493 | + var deviceInterface = makeInterface(); |
494 | + deviceInterface.ipAddress = nic.dynamic_range.low; |
495 | + deviceInterface.clusterInterfaceId = nic.id; |
496 | + deviceInterface.ipAssignment = { |
497 | name: "static" |
498 | }; |
499 | - expect($scope.ipHasError()).toBe(true); |
500 | + expect($scope.ipHasError(deviceInterface)).toBe(true); |
501 | }); |
502 | }); |
503 | |
504 | @@ -422,8 +485,8 @@ |
505 | |
506 | it("returns true if name empty", function() { |
507 | var controller = makeController(); |
508 | - $scope.device.mac = '00:11:22:33:44:55'; |
509 | - $scope.device.ipAssignment = { |
510 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
511 | + $scope.device.interfaces[0].ipAssignment = { |
512 | name: "dynamic" |
513 | }; |
514 | expect($scope.deviceHasError()).toBe(true); |
515 | @@ -432,7 +495,7 @@ |
516 | it("returns true if mac empty", function() { |
517 | var controller = makeController(); |
518 | $scope.device.name = "abc"; |
519 | - $scope.device.ipAssignment = { |
520 | + $scope.device.interfaces[0].ipAssignment = { |
521 | name: "dynamic" |
522 | }; |
523 | expect($scope.deviceHasError()).toBe(true); |
524 | @@ -441,8 +504,8 @@ |
525 | it("returns true if name invalid", function() { |
526 | var controller = makeController(); |
527 | $scope.device.name = "ab_c.local"; |
528 | - $scope.device.mac = '00:11:22:33:44:55'; |
529 | - $scope.device.ipAssignment = { |
530 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
531 | + $scope.device.interfaces[0].ipAssignment = { |
532 | name: "dynamic" |
533 | }; |
534 | expect($scope.deviceHasError()).toBe(true); |
535 | @@ -451,8 +514,8 @@ |
536 | it("returns true if mac invalid", function() { |
537 | var controller = makeController(); |
538 | $scope.device.name = "abc"; |
539 | - $scope.device.mac = '00:11:22:33:44'; |
540 | - $scope.device.ipAssignment = { |
541 | + $scope.device.interfaces[0].mac = '00:11:22:33:44'; |
542 | + $scope.device.interfaces[0].ipAssignment = { |
543 | name: "dynamic" |
544 | }; |
545 | expect($scope.deviceHasError()).toBe(true); |
546 | @@ -461,15 +524,15 @@ |
547 | it("returns true if missing ip assignment selection", function() { |
548 | var controller = makeController(); |
549 | $scope.device.name = "abc"; |
550 | - $scope.device.mac = '00:11:22:33:44:55'; |
551 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
552 | expect($scope.deviceHasError()).toBe(true); |
553 | }); |
554 | |
555 | it("returns false if dynamic ip assignment selection", function() { |
556 | var controller = makeController(); |
557 | $scope.device.name = "abc"; |
558 | - $scope.device.mac = '00:11:22:33:44:55'; |
559 | - $scope.device.ipAssignment = { |
560 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
561 | + $scope.device.interfaces[0].ipAssignment = { |
562 | name: "dynamic" |
563 | }; |
564 | expect($scope.deviceHasError()).toBe(false); |
565 | @@ -478,33 +541,33 @@ |
566 | it("returns true if external ip assignment and ip empty", function() { |
567 | var controller = makeController(); |
568 | $scope.device.name = "abc"; |
569 | - $scope.device.mac = '00:11:22:33:44:55'; |
570 | - $scope.device.ipAssignment = { |
571 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
572 | + $scope.device.interfaces[0].ipAssignment = { |
573 | name: "external" |
574 | }; |
575 | - $scope.device.ipAddress = ""; |
576 | + $scope.device.interfaces[0].ipAddress = ""; |
577 | expect($scope.deviceHasError()).toBe(true); |
578 | }); |
579 | |
580 | it("returns true if external ip assignment and ip invalid", function() { |
581 | var controller = makeController(); |
582 | $scope.device.name = "abc"; |
583 | - $scope.device.mac = '00:11:22:33:44:55'; |
584 | - $scope.device.ipAssignment = { |
585 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
586 | + $scope.device.interfaces[0].ipAssignment = { |
587 | name: "external" |
588 | }; |
589 | - $scope.device.ipAddress = "192.168"; |
590 | + $scope.device.interfaces[0].ipAddress = "192.168"; |
591 | expect($scope.deviceHasError()).toBe(true); |
592 | }); |
593 | |
594 | it("returns false if external ip assignment and ip valid", function() { |
595 | var controller = makeController(); |
596 | $scope.device.name = "abc"; |
597 | - $scope.device.mac = '00:11:22:33:44:55'; |
598 | - $scope.device.ipAssignment = { |
599 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
600 | + $scope.device.interfaces[0].ipAssignment = { |
601 | name: "external" |
602 | }; |
603 | - $scope.device.ipAddress = "192.168.1.1"; |
604 | + $scope.device.interfaces[0].ipAddress = "192.168.1.1"; |
605 | expect($scope.deviceHasError()).toBe(false); |
606 | }); |
607 | |
608 | @@ -512,8 +575,8 @@ |
609 | function() { |
610 | var controller = makeController(); |
611 | $scope.device.name = "abc"; |
612 | - $scope.device.mac = '00:11:22:33:44:55'; |
613 | - $scope.device.ipAssignment = { |
614 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
615 | + $scope.device.interfaces[0].ipAssignment = { |
616 | name: "static" |
617 | }; |
618 | expect($scope.deviceHasError()).toBe(true); |
619 | @@ -526,11 +589,11 @@ |
620 | var cluster = makeCluster([nic]); |
621 | $scope.clusters = [cluster]; |
622 | $scope.device.name = "abc"; |
623 | - $scope.device.mac = '00:11:22:33:44:55'; |
624 | - $scope.device.ipAssignment = { |
625 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
626 | + $scope.device.interfaces[0].ipAssignment = { |
627 | name: "static" |
628 | }; |
629 | - $scope.device.clusterInterfaceId = nic.id; |
630 | + $scope.device.interfaces[0].clusterInterfaceId = nic.id; |
631 | expect($scope.deviceHasError()).toBe(false); |
632 | }); |
633 | |
634 | @@ -542,12 +605,12 @@ |
635 | var cluster = makeCluster([nic]); |
636 | $scope.clusters = [cluster]; |
637 | $scope.device.name = "abc"; |
638 | - $scope.device.mac = '00:11:22:33:44:55'; |
639 | - $scope.device.ipAssignment = { |
640 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
641 | + $scope.device.interfaces[0].ipAssignment = { |
642 | name: "static" |
643 | }; |
644 | - $scope.device.clusterInterfaceId = nic.id; |
645 | - $scope.device.ipAddress = "192.168"; |
646 | + $scope.device.interfaces[0].clusterInterfaceId = nic.id; |
647 | + $scope.device.interfaces[0].ipAddress = "192.168"; |
648 | expect($scope.deviceHasError()).toBe(true); |
649 | }); |
650 | |
651 | @@ -560,12 +623,13 @@ |
652 | var cluster = makeCluster([nic]); |
653 | $scope.clusters = [cluster]; |
654 | $scope.device.name = "abc"; |
655 | - $scope.device.mac = '00:11:22:33:44:55'; |
656 | - $scope.device.ipAssignment = { |
657 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
658 | + $scope.device.interfaces[0].ipAssignment = { |
659 | name: "static" |
660 | }; |
661 | - $scope.device.clusterInterfaceId = nic.id; |
662 | - $scope.device.ipAddress = otherNic.static_range.low; |
663 | + $scope.device.interfaces[0].clusterInterfaceId = nic.id; |
664 | + $scope.device.interfaces[0].ipAddress = |
665 | + otherNic.static_range.low; |
666 | expect($scope.deviceHasError()).toBe(true); |
667 | }); |
668 | |
669 | @@ -577,16 +641,62 @@ |
670 | var cluster = makeCluster([nic]); |
671 | $scope.clusters = [cluster]; |
672 | $scope.device.name = "abc"; |
673 | - $scope.device.mac = '00:11:22:33:44:55'; |
674 | - $scope.device.ipAssignment = { |
675 | + $scope.device.interfaces[0].mac = '00:11:22:33:44:55'; |
676 | + $scope.device.interfaces[0].ipAssignment = { |
677 | name: "static" |
678 | }; |
679 | - $scope.device.clusterInterfaceId = nic.id; |
680 | - $scope.device.ipAddress = nic.static_range.low; |
681 | + $scope.device.interfaces[0].clusterInterfaceId = nic.id; |
682 | + $scope.device.interfaces[0].ipAddress = nic.static_range.low; |
683 | expect($scope.deviceHasError()).toBe(false); |
684 | }); |
685 | }); |
686 | |
687 | + describe("addInterface", function() { |
688 | + |
689 | + it("adds another interface", function() { |
690 | + var controller = makeController(); |
691 | + $scope.addInterface(); |
692 | + expect($scope.device.interfaces.length).toBe(2); |
693 | + }); |
694 | + }); |
695 | + |
696 | + describe("isPrimaryInterface", function() { |
697 | + |
698 | + it("returns true for first interface", function() { |
699 | + var controller = makeController(); |
700 | + $scope.addInterface(); |
701 | + expect( |
702 | + $scope.isPrimaryInterface( |
703 | + $scope.device.interfaces[0])).toBe(true); |
704 | + }); |
705 | + |
706 | + it("returns false for second interface", function() { |
707 | + var controller = makeController(); |
708 | + $scope.addInterface(); |
709 | + expect( |
710 | + $scope.isPrimaryInterface( |
711 | + $scope.device.interfaces[1])).toBe(false); |
712 | + }); |
713 | + }); |
714 | + |
715 | + describe("deleteInterface", function() { |
716 | + |
717 | + it("doesnt remove primary interface", function() { |
718 | + var controller = makeController(); |
719 | + var nic = $scope.device.interfaces[0]; |
720 | + $scope.deleteInterface(nic); |
721 | + expect($scope.device.interfaces[0]).toBe(nic); |
722 | + }); |
723 | + |
724 | + it("removes interface", function() { |
725 | + var controller = makeController(); |
726 | + $scope.addInterface(); |
727 | + var nic = $scope.device.interfaces[1]; |
728 | + $scope.deleteInterface(nic); |
729 | + expect($scope.device.interfaces.indexOf(nic)).toBe(-1); |
730 | + }); |
731 | + }); |
732 | + |
733 | describe("cancel", function() { |
734 | |
735 | it("clears error", function() { |
736 | @@ -629,10 +739,8 @@ |
737 | spyOn($scope, "deviceHasError").and.returnValue(false); |
738 | spyOn(DevicesManager, "create").and.returnValue( |
739 | $q.defer().promise); |
740 | - $scope.device = { |
741 | - ipAssignment: { |
742 | - name: "dynamic" |
743 | - } |
744 | + $scope.device.interfaces[0].ipAssignment = { |
745 | + name: "dynamic" |
746 | }; |
747 | $scope.save(); |
748 | expect($scope.error).toBeNull(); |
749 | @@ -651,20 +759,26 @@ |
750 | var ipAddress = makeName("ip"); |
751 | $scope.device = { |
752 | name: name, |
753 | - mac: mac, |
754 | - ipAssignment: { |
755 | - name: assignment |
756 | - }, |
757 | - clusterInterfaceId: nicId, |
758 | - ipAddress: ipAddress |
759 | + interfaces: [{ |
760 | + mac: mac, |
761 | + ipAssignment: { |
762 | + name: assignment |
763 | + }, |
764 | + clusterInterfaceId: nicId, |
765 | + ipAddress: ipAddress |
766 | + }] |
767 | }; |
768 | $scope.save(); |
769 | expect(DevicesManager.create).toHaveBeenCalledWith({ |
770 | hostname: name, |
771 | primary_mac: mac, |
772 | - ip_assignment: assignment, |
773 | - ip_address: ipAddress, |
774 | - "interface": nicId |
775 | + extra_macs: [], |
776 | + interfaces: [{ |
777 | + mac: mac, |
778 | + ip_assignment: assignment, |
779 | + ip_address: ipAddress, |
780 | + "interface": nicId |
781 | + }] |
782 | }); |
783 | }); |
784 | |
785 | @@ -675,7 +789,7 @@ |
786 | var defer = $q.defer(); |
787 | spyOn(DevicesManager, "create").and.returnValue(defer.promise); |
788 | $scope.device.name = makeName("name"); |
789 | - $scope.device.ipAssignment = { |
790 | + $scope.device.interfaces[0].ipAssignment = { |
791 | name: "dynamic" |
792 | }; |
793 | $scope.save(); |
794 | @@ -692,7 +806,7 @@ |
795 | var defer = $q.defer(); |
796 | spyOn(DevicesManager, "create").and.returnValue(defer.promise); |
797 | $scope.device.name = makeName("name"); |
798 | - $scope.device.ipAssignment = { |
799 | + $scope.device.interfaces[0].ipAssignment = { |
800 | name: "dynamic" |
801 | }; |
802 | spyOn($scope, "hide"); |
803 | @@ -710,7 +824,7 @@ |
804 | var defer = $q.defer(); |
805 | spyOn(DevicesManager, "create").and.returnValue(defer.promise); |
806 | $scope.device.name = makeName("name"); |
807 | - $scope.device.ipAssignment = { |
808 | + $scope.device.interfaces[0].ipAssignment = { |
809 | name: "dynamic" |
810 | }; |
811 | spyOn($scope, "hide"); |
812 | @@ -728,7 +842,7 @@ |
813 | var defer = $q.defer(); |
814 | spyOn(DevicesManager, "create").and.returnValue(defer.promise); |
815 | $scope.device.name = makeName("name"); |
816 | - $scope.device.ipAssignment = { |
817 | + $scope.device.interfaces[0].ipAssignment = { |
818 | name: "dynamic" |
819 | }; |
820 | $scope.save(); |
821 | |
822 | === added file 'src/maasserver/static/js/angular/directives/placeholder.js' |
823 | --- src/maasserver/static/js/angular/directives/placeholder.js 1970-01-01 00:00:00 +0000 |
824 | +++ src/maasserver/static/js/angular/directives/placeholder.js 2015-05-27 00:50:29 +0000 |
825 | @@ -0,0 +1,22 @@ |
826 | +/* Copyright 2015 Canonical Ltd. This software is licensed under the |
827 | + * GNU Affero General Public License version 3 (see the file LICENSE). |
828 | + * |
829 | + * Placeholder directive. |
830 | + * |
831 | + * Allows the placeholder attribute on an element to be dynamic. |
832 | + */ |
833 | + |
834 | + |
835 | +angular.module('MAAS').directive('ngPlaceholder', function() { |
836 | + return { |
837 | + restrict: "A", |
838 | + scope: { |
839 | + ngPlaceholder: "=" |
840 | + }, |
841 | + link: function(scope, element, attrs) { |
842 | + scope.$watch('ngPlaceholder', function() { |
843 | + element[0].placeholder = scope.ngPlaceholder; |
844 | + }); |
845 | + } |
846 | + }; |
847 | +}); |
848 | |
849 | === added file 'src/maasserver/static/js/angular/directives/tests/test_placeholder.js' |
850 | --- src/maasserver/static/js/angular/directives/tests/test_placeholder.js 1970-01-01 00:00:00 +0000 |
851 | +++ src/maasserver/static/js/angular/directives/tests/test_placeholder.js 2015-05-27 00:50:29 +0000 |
852 | @@ -0,0 +1,55 @@ |
853 | +/* Copyright 2015 Canonical Ltd. This software is licensed under the |
854 | + * GNU Affero General Public License version 3 (see the file LICENSE). |
855 | + * |
856 | + * Unit tests for placeholder directive. |
857 | + */ |
858 | + |
859 | +describe("ngPlaceholder", function() { |
860 | + |
861 | + // Load the MAAS module. |
862 | + beforeEach(module("MAAS")); |
863 | + |
864 | + // Create a new scope before each test. |
865 | + var $scope; |
866 | + beforeEach(inject(function($rootScope) { |
867 | + $scope = $rootScope.$new(); |
868 | + })); |
869 | + |
870 | + // Return the compiled directive with the items from the scope. |
871 | + function compileDirective(ngPlaceholder) { |
872 | + var directive; |
873 | + var html = [ |
874 | + '<div>', |
875 | + '<input ng-placeholder="' + ngPlaceholder + '" />', |
876 | + '</div>' |
877 | + ].join(''); |
878 | + |
879 | + // Compile the directive. |
880 | + inject(function($compile) { |
881 | + directive = $compile(html)($scope); |
882 | + }); |
883 | + |
884 | + // Perform the digest cycle to finish the compile. |
885 | + $scope.$digest(); |
886 | + return directive.find("input"); |
887 | + } |
888 | + |
889 | + it("sets placeholder attribute on input", function() { |
890 | + var placeholderText = makeName("placeholder"); |
891 | + $scope.placeholder = placeholderText; |
892 | + var directive = compileDirective("placeholder"); |
893 | + expect(directive[0].placeholder).toEqual(placeholderText); |
894 | + }); |
895 | + |
896 | + it("sets placeholder attribute on input when changed", function() { |
897 | + var placeholderText = makeName("placeholder"); |
898 | + $scope.placeholder = placeholderText; |
899 | + var directive = compileDirective("placeholder"); |
900 | + |
901 | + // Change the text. |
902 | + placeholderText = makeName("placeholder"); |
903 | + $scope.placeholder = placeholderText; |
904 | + $scope.$digest(); |
905 | + expect(directive[0].placeholder).toEqual(placeholderText); |
906 | + }); |
907 | +}); |
908 | |
909 | === modified file 'src/maasserver/static/partials/nodes-list.html' |
910 | --- src/maasserver/static/partials/nodes-list.html 2015-05-22 16:16:00 +0000 |
911 | +++ src/maasserver/static/partials/nodes-list.html 2015-05-27 00:50:29 +0000 |
912 | @@ -278,51 +278,69 @@ |
913 | </div> |
914 | <div class="page-header__dropdown ng-hide" data-ng-show="addDeviceScope.viewable" data-ng-controller="AddDeviceController"> |
915 | <h3>Add device</h3> |
916 | - <form action="post" class="twelve-col"> |
917 | - <fieldset class="six-col no-padding"> |
918 | + <form class="twelve-col"> |
919 | + <fieldset class="twelve-col last-col no-padding"> |
920 | <div class="inline"> |
921 | <label for="device-name" class="two-col">Name</label> |
922 | <input type="text" id="device-name" class="three-col" placeholder="Name your device" |
923 | data-ng-model="device.name" |
924 | data-ng-class="{ invalid: nameHasError() }"> |
925 | </div> |
926 | - <div class="inline"> |
927 | - <label for="mac-address" class="two-col">MAC address</label> |
928 | - <input type="text" id="mac-address" class="three-col" placeholder="00:00:00:00:00:00" |
929 | - data-ng-model="device.mac" |
930 | - data-ng-class="{ invalid: macHasError() }"> |
931 | - </div> |
932 | - </fieldset> |
933 | - <fieldset class="six-col last-col no-padding"> |
934 | - <div class="inline"> |
935 | - <label for="ip-assignment" class="two-col">IP assigment</label> |
936 | - <select name="ip-assignment" id="ip-assignment" class="three-col" |
937 | - placeholder="Select IP assignment" data-ng-model="device.ipAssignment" |
938 | - data-ng-options="assigment.title for assigment in ipAssignments"> |
939 | - <option value="" disabled selected>Select IP assignment</option> |
940 | - </select> |
941 | - </div> |
942 | - <div class="inline ng-hide" data-ng-show="device.ipAssignment.name === 'external'"> |
943 | - <label for="ip-address" class="two-col">IP address</label> |
944 | - <input type="text" id="ip-address" class="three-col" placeholder="000.000.000.000" |
945 | - data-ng-model="device.ipAddress" |
946 | - data-ng-class="{ invalid: ipHasError() }"> |
947 | - </div> |
948 | - <div class="inline ng-hide" data-ng-show="device.ipAssignment.name === 'static'"> |
949 | - <label for="network" class="two-col">Network</label> |
950 | - <select name="network" id="network" class="three-col" |
951 | - placeholder="Select network" data-ng-model="device.clusterInterfaceId" |
952 | - data-ng-options="interface.id as getInterfaceStaticRange(interface) for interface in getManagedInterfaces()"> |
953 | - <option value="" disabled selected>Select network</option> |
954 | - </select> |
955 | - </div> |
956 | - <div class="inline ng-hide" data-ng-show="device.ipAssignment.name === 'static'"> |
957 | - <label for="ip-address" class="two-col">IP address</label> |
958 | - <input type="text" id="ip-address" class="three-col" placeholder="000.000.000.000 (Optional)" |
959 | - data-ng-model="device.ipAddress" |
960 | - data-ng-class="{ invalid: ipHasError() }"> |
961 | - </div> |
962 | - </fieldset> |
963 | + </fieldset> |
964 | + <table class="no-margin-bottom table-listing"> |
965 | + <thead> |
966 | + <tr class="table-listing__row"> |
967 | + <th class="t20">MAC address</th> |
968 | + <th class="t20">IP assignment</th> |
969 | + <th class="t20">Network</th> |
970 | + <th class="t35">IP address</th> |
971 | + <th class="t5"></th> |
972 | + </tr> |
973 | + </thead> |
974 | + <tbody> |
975 | + <tr class="table-listing__row" |
976 | + data-ng-repeat="interface in device.interfaces"> |
977 | + <td class="t20 table-listing__cell"> |
978 | + <input type="text" id="mac-address" placeholder="00:00:00:00:00:00" |
979 | + data-ng-model="interface.mac" |
980 | + data-ng-class="{ invalid: macHasError(interface) }"> |
981 | + </td> |
982 | + <td class="t20 table-listing__cell"> |
983 | + <select name="ip-assignment" id="ip-assignment" |
984 | + placeholder="Select IP assignment" data-ng-model="interface.ipAssignment" |
985 | + data-ng-options="assigment.title for assigment in ipAssignments"> |
986 | + <option value="" disabled selected>Select IP assignment</option> |
987 | + </select> |
988 | + </td> |
989 | + <td class="t20 table-listing__cell"> |
990 | + <select name="network" id="network" |
991 | + placeholder="Select network" data-ng-model="interface.clusterInterfaceId" |
992 | + data-ng-options="cInterface.id as cInterface.network for cInterface in getManagedInterfaces()" |
993 | + data-ng-show="interface.ipAssignment.name === 'static'"> |
994 | + <option value="" disabled selected>Select network</option> |
995 | + </select> |
996 | + </td> |
997 | + <td class="t35 table-listing__cell"> |
998 | + <input type="text" id="ip-address" placeholder="000.000.000.000" |
999 | + data-ng-model="interface.ipAddress" |
1000 | + data-ng-class="{ invalid: ipHasError(interface) }" |
1001 | + data-ng-show="interface.ipAssignment.name === 'external'"> |
1002 | + <input type="text" id="ip-address" ng-placeholder="getInterfaceStaticRange(interface.clusterInterfaceId)" |
1003 | + data-ng-model="interface.ipAddress" |
1004 | + data-ng-class="{ invalid: ipHasError(interface) }" |
1005 | + data-ng-show="interface.ipAssignment.name === 'static' && interface.clusterInterfaceId"> |
1006 | + </td> |
1007 | + <td class="t5 align-right"> |
1008 | + <!-- space needed to set correct height --> |
1009 | + <span data-ng-show="isPrimaryInterface(interface)"> </span> |
1010 | + <a title="Delete interface" class="icon delete" |
1011 | + data-ng-click="deleteInterface(interface)" |
1012 | + data-ng-hide="isPrimaryInterface(interface)">delete</a> |
1013 | + </td> |
1014 | + </tr> |
1015 | + </tbody> |
1016 | + </table> |
1017 | + <button class="cta-ubuntu secondary" data-ng-click="addInterface()">+ Add another interface</button> |
1018 | </form> |
1019 | <div class="page-header__feedback twelve-col no-margin-bottom"> |
1020 | <div class="twelve-col ng-hide" data-ng-show="error"> |
1021 | |
1022 | === modified file 'src/maasserver/websockets/handlers/device.py' |
1023 | --- src/maasserver/websockets/handlers/device.py 2015-05-14 17:25:57 +0000 |
1024 | +++ src/maasserver/websockets/handlers/device.py 2015-05-27 00:50:29 +0000 |
1025 | @@ -30,7 +30,6 @@ |
1026 | from maasserver.models.node import Device |
1027 | from maasserver.models.nodegroup import NodeGroup |
1028 | from maasserver.models.nodegroupinterface import NodeGroupInterface |
1029 | -from maasserver.models.staticipaddress import StaticIPAddress |
1030 | from maasserver.node_action import compile_node_actions |
1031 | from maasserver.utils.orm import commit_within_atomic_block |
1032 | from maasserver.websockets.base import ( |
1033 | @@ -70,6 +69,45 @@ |
1034 | return list(dhcp.update_host_maps(static_mappings)) |
1035 | |
1036 | |
1037 | +def get_MACAddress_from_list(mac_addresses, mac): |
1038 | + """Return the `MACAddress` object based on the mac value.""" |
1039 | + for mac_obj in mac_addresses: |
1040 | + if mac_obj.mac_address == mac: |
1041 | + return mac_obj |
1042 | + return None |
1043 | + |
1044 | + |
1045 | +def delete_device_and_static_ip_addresses( |
1046 | + device, external_static_ips, assigned_sticky_ips): |
1047 | + """Delete the created external and sticky ips and the created device. |
1048 | + |
1049 | + This function calls `commit_within_atomic_block` to force the transaction |
1050 | + to be saved. |
1051 | + """ |
1052 | + for static_ip, _ in external_static_ips: |
1053 | + static_ip.deallocate() |
1054 | + for static_ip, _ in assigned_sticky_ips: |
1055 | + static_ip.deallocate() |
1056 | + device.delete() |
1057 | + commit_within_atomic_block() |
1058 | + |
1059 | + |
1060 | +def log_static_allocations(device, external_static_ips, assigned_sticky_ips): |
1061 | + """Log the allocation of the static ip address.""" |
1062 | + all_ips = [ |
1063 | + static_ip.ip |
1064 | + for static_ip, _ in external_static_ips |
1065 | + ] |
1066 | + all_ips.extend([ |
1067 | + static_ip.ip |
1068 | + for static_ip, _ in assigned_sticky_ips |
1069 | + ]) |
1070 | + if len(all_ips) > 0: |
1071 | + maaslog.info( |
1072 | + "%s: Sticky IP address(es) allocated: %s", |
1073 | + device.hostname, ', '.join(all_ips)) |
1074 | + |
1075 | + |
1076 | class DeviceHandler(TimestampedModelHandler): |
1077 | |
1078 | class Meta: |
1079 | @@ -260,98 +298,102 @@ |
1080 | # XXX blake_r 03-04-15 bug=1440102: This is very ugly and a repeat |
1081 | # of code in other places. Needs to be refactored. |
1082 | |
1083 | - # Create the object, then link ip_address to the device based |
1084 | - # on the users choice. |
1085 | + # Create the object with the form and then create all of the interfaces |
1086 | + # based on the users choices. |
1087 | data = super(DeviceHandler, self).create(params) |
1088 | device_obj = Device.objects.get(system_id=data['system_id']) |
1089 | - mac_address = device_obj.get_pxe_mac() |
1090 | - ip_assignment = params["ip_assignment"] |
1091 | - if ip_assignment == DEVICE_IP_ASSIGNMENT.EXTERNAL: |
1092 | - sticky_ip = mac_address.set_static_ip( |
1093 | - params["ip_address"], self.user) |
1094 | + mac_addresses = list(device_obj.macaddress_set.all()) |
1095 | + external_static_ips = [] |
1096 | + assigned_sticky_ips = [] |
1097 | + |
1098 | + # Acquire all of the needed ip address based on the user selection. |
1099 | + for nic in params["interfaces"]: |
1100 | + mac_address = get_MACAddress_from_list(mac_addresses, nic["mac"]) |
1101 | + ip_assignment = nic["ip_assignment"] |
1102 | + if ip_assignment == DEVICE_IP_ASSIGNMENT.EXTERNAL: |
1103 | + sticky_ip = mac_address.set_static_ip( |
1104 | + nic["ip_address"], self.user) |
1105 | + external_static_ips.append( |
1106 | + (sticky_ip, mac_address)) |
1107 | + elif ip_assignment == DEVICE_IP_ASSIGNMENT.DYNAMIC: |
1108 | + # Nothing needs to be done. It will get an ip address in the |
1109 | + # dynamic range the first time it DHCP from a cluster |
1110 | + # interface. |
1111 | + pass |
1112 | + elif ip_assignment == DEVICE_IP_ASSIGNMENT.STATIC: |
1113 | + # Link the MAC address to the cluster interface. |
1114 | + cluster_interface = NodeGroupInterface.objects.get( |
1115 | + id=nic["interface"]) |
1116 | + mac_address.cluster_interface = cluster_interface |
1117 | + mac_address.save() |
1118 | + |
1119 | + # Convert an empty string to None. |
1120 | + ip_address = nic.get("ip_address") |
1121 | + if not ip_address: |
1122 | + ip_address = None |
1123 | + |
1124 | + # Claim the static ip. |
1125 | + sticky_ips = mac_address.claim_static_ips( |
1126 | + alloc_type=IPADDRESS_TYPE.STICKY, |
1127 | + requested_address=ip_address) |
1128 | + assigned_sticky_ips.extend([ |
1129 | + (static_ip, mac_address) |
1130 | + for static_ip in sticky_ips |
1131 | + ]) |
1132 | + |
1133 | + # Commit all of the changes before telling the clusters to |
1134 | + # update. |
1135 | + commit_within_atomic_block() |
1136 | + |
1137 | + # Update the managed clusters with all the external static ip |
1138 | + # addresses. |
1139 | + if len(external_static_ips) > 0: |
1140 | dhcp_managed_clusters = [ |
1141 | cluster |
1142 | for cluster in NodeGroup.objects.all() |
1143 | if cluster.manages_dhcp() |
1144 | ] |
1145 | - |
1146 | - # Commit the change and tell all the managed clusters to |
1147 | - # update thier host maps. |
1148 | - commit_within_atomic_block() |
1149 | + claims = [ |
1150 | + (static_ip.ip, mac.mac_address.get_raw()) |
1151 | + for static_ip, mac in external_static_ips |
1152 | + ] |
1153 | failures = update_host_maps( |
1154 | - [(sticky_ip.ip, mac_address.mac_address.get_raw())], |
1155 | - dhcp_managed_clusters) |
1156 | + claims, dhcp_managed_clusters) |
1157 | if len(failures) > 0: |
1158 | - # Failed to update the cluster. Remove the allocated static |
1159 | - # ip address and delete the newly created device. Since it |
1160 | - # was committed before the call to update_host_maps. |
1161 | - sticky_ip.deallocate() |
1162 | - device_obj.delete() |
1163 | - |
1164 | - # Commit again so this raised failure will not roll back |
1165 | - # the deletions above. |
1166 | - commit_within_atomic_block() |
1167 | - |
1168 | - # Now raise the first error to show a reason to the user. It |
1169 | - # is possible to have multiple errors, but at the moment |
1170 | + # Delete the created device and ip addresses. |
1171 | + delete_device_and_static_ip_addresses( |
1172 | + device_obj, external_static_ips, assigned_sticky_ips) |
1173 | + |
1174 | + # Now raise the first error to show a reason to the user. |
1175 | + # It is possible to have multiple errors, but at the moment |
1176 | # we only raise the first error. |
1177 | raise HandlerError(failures[0].value) |
1178 | - sticky_ips = [sticky_ip] |
1179 | - |
1180 | - elif ip_assignment == DEVICE_IP_ASSIGNMENT.DYNAMIC: |
1181 | - # Nothing needs to be done. It will get an ip address in the |
1182 | - # dynamic range the first time it DHCP from a cluster interface. |
1183 | - pass |
1184 | - |
1185 | - elif ip_assignment == DEVICE_IP_ASSIGNMENT.STATIC: |
1186 | - # Link the MAC address to the cluster interface. |
1187 | - cluster_interface = NodeGroupInterface.objects.get( |
1188 | - id=params["interface"]) |
1189 | - mac_address.cluster_interface = cluster_interface |
1190 | - mac_address.save() |
1191 | - |
1192 | - # Convert an empty string to None. |
1193 | - ip_address = params.get("ip_address") |
1194 | - if not ip_address: |
1195 | - ip_address = None |
1196 | - |
1197 | - # Claim the static ip. |
1198 | - sticky_ips = mac_address.claim_static_ips( |
1199 | - alloc_type=IPADDRESS_TYPE.STICKY, |
1200 | - requested_address=ip_address) |
1201 | + |
1202 | + # Update the devices cluster with all the assigned sticky ip |
1203 | + # addresses. |
1204 | + if len(assigned_sticky_ips) > 0: |
1205 | claims = [ |
1206 | - (static_ip.ip, mac_address.mac_address.get_raw()) |
1207 | - for static_ip in sticky_ips |
1208 | + (static_ip.ip, mac.mac_address.get_raw()) |
1209 | + for static_ip, mac in assigned_sticky_ips |
1210 | ] |
1211 | - |
1212 | - # Commit the change and tell all the cluster for this device to |
1213 | - # update its host map. |
1214 | - commit_within_atomic_block() |
1215 | - failures = update_host_maps(claims, [device_obj.nodegroup]) |
1216 | + failures = update_host_maps( |
1217 | + claims, [device_obj.nodegroup]) |
1218 | if len(failures) > 0: |
1219 | - # Failed to update the cluster. Remove all allocated static |
1220 | - # ip address and delete the newly created device. Since it |
1221 | - # was committed before the call to update_host_maps. |
1222 | - StaticIPAddress.objects.delete_by_node(device_obj) |
1223 | - device_obj.delete() |
1224 | - |
1225 | - # Commit again so this raised failure will not roll back |
1226 | - # the deletions above. |
1227 | - commit_within_atomic_block() |
1228 | - |
1229 | - # Now raise the first error to show a reason to the user. It |
1230 | - # is possible to have multiple errors, but at the moment |
1231 | + # Delete the created device and ip addresses. |
1232 | + delete_device_and_static_ip_addresses( |
1233 | + device_obj, external_static_ips, assigned_sticky_ips) |
1234 | + |
1235 | + # Now raise the first error to show a reason to the user. |
1236 | + # It is possible to have multiple errors, but at the moment |
1237 | # we only raise the first error. |
1238 | raise HandlerError(failures[0].value) |
1239 | |
1240 | # Update the DNS zone for the master cluster as all device entries |
1241 | # go into that cluster. |
1242 | - if ip_assignment in [ |
1243 | - DEVICE_IP_ASSIGNMENT.EXTERNAL, DEVICE_IP_ASSIGNMENT.STATIC]: |
1244 | + log_static_allocations( |
1245 | + device_obj, external_static_ips, assigned_sticky_ips) |
1246 | + if len(external_static_ips) > 0 or len(assigned_sticky_ips) > 0: |
1247 | dns_update_zones([NodeGroup.objects.ensure_master()]) |
1248 | - maaslog.info( |
1249 | - "%s: Sticky IP address(es) allocated: %s", device_obj.hostname, |
1250 | - ', '.join(allocation.ip for allocation in sticky_ips)) |
1251 | |
1252 | return self.full_dehydrate(device_obj) |
1253 | |
1254 | |
1255 | === modified file 'src/maasserver/websockets/handlers/tests/test_device.py' |
1256 | --- src/maasserver/websockets/handlers/tests/test_device.py 2015-04-03 18:45:06 +0000 |
1257 | +++ src/maasserver/websockets/handlers/tests/test_device.py 2015-05-27 00:50:29 +0000 |
1258 | @@ -282,8 +282,11 @@ |
1259 | created_device = handler.create({ |
1260 | "hostname": hostname, |
1261 | "primary_mac": mac, |
1262 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.DYNAMIC, |
1263 | - }) |
1264 | + "interfaces": [{ |
1265 | + "mac": mac, |
1266 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.DYNAMIC, |
1267 | + }], |
1268 | + }) |
1269 | self.expectThat(created_device["hostname"], Equals(hostname)) |
1270 | self.expectThat(created_device["primary_mac"], Equals(mac)) |
1271 | self.expectThat(created_device["extra_macs"], Equals([])) |
1272 | @@ -303,9 +306,12 @@ |
1273 | created_device = handler.create({ |
1274 | "hostname": hostname, |
1275 | "primary_mac": mac, |
1276 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1277 | - "ip_address": ip_address, |
1278 | - }) |
1279 | + "interfaces": [{ |
1280 | + "mac": mac, |
1281 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1282 | + "ip_address": ip_address, |
1283 | + }], |
1284 | + }) |
1285 | self.expectThat( |
1286 | created_device["ip_assignment"], |
1287 | Equals(DEVICE_IP_ASSIGNMENT.EXTERNAL)) |
1288 | @@ -325,9 +331,12 @@ |
1289 | handler.create({ |
1290 | "hostname": hostname, |
1291 | "primary_mac": mac, |
1292 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1293 | - "ip_address": ip_address, |
1294 | - }) |
1295 | + "interfaces": [{ |
1296 | + "mac": mac, |
1297 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1298 | + "ip_address": ip_address, |
1299 | + }], |
1300 | + }) |
1301 | self.assertThat( |
1302 | mock_dns_update_zones, |
1303 | MockCalledOnceWith([NodeGroup.objects.ensure_master()])) |
1304 | @@ -345,9 +354,12 @@ |
1305 | self.assertRaises(HandlerError, handler.create, { |
1306 | "hostname": hostname, |
1307 | "primary_mac": mac, |
1308 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1309 | - "ip_address": ip_address, |
1310 | - }) |
1311 | + "interfaces": [{ |
1312 | + "mac": mac, |
1313 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1314 | + "ip_address": ip_address, |
1315 | + }], |
1316 | + }) |
1317 | self.expectThat( |
1318 | Node.objects.filter(hostname=hostname).count(), |
1319 | Equals(0), "Created Node was not deleted.") |
1320 | @@ -369,9 +381,12 @@ |
1321 | created_device = handler.create({ |
1322 | "hostname": hostname, |
1323 | "primary_mac": mac, |
1324 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1325 | - "interface": nodegroup_interface.id, |
1326 | - }) |
1327 | + "interfaces": [{ |
1328 | + "mac": mac, |
1329 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1330 | + "interface": nodegroup_interface.id, |
1331 | + }], |
1332 | + }) |
1333 | self.expectThat( |
1334 | created_device["ip_assignment"], |
1335 | Equals(DEVICE_IP_ASSIGNMENT.STATIC)) |
1336 | @@ -396,10 +411,13 @@ |
1337 | created_device = handler.create({ |
1338 | "hostname": hostname, |
1339 | "primary_mac": mac, |
1340 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1341 | - "interface": nodegroup_interface.id, |
1342 | - "ip_address": ip_address, |
1343 | - }) |
1344 | + "interfaces": [{ |
1345 | + "mac": mac, |
1346 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1347 | + "interface": nodegroup_interface.id, |
1348 | + "ip_address": ip_address, |
1349 | + }], |
1350 | + }) |
1351 | self.expectThat( |
1352 | created_device["ip_assignment"], |
1353 | Equals(DEVICE_IP_ASSIGNMENT.STATIC)) |
1354 | @@ -424,9 +442,12 @@ |
1355 | handler.create({ |
1356 | "hostname": hostname, |
1357 | "primary_mac": mac, |
1358 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1359 | - "interface": nodegroup_interface.id, |
1360 | - }) |
1361 | + "interfaces": [{ |
1362 | + "mac": mac, |
1363 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1364 | + "interface": nodegroup_interface.id, |
1365 | + }], |
1366 | + }) |
1367 | self.assertThat( |
1368 | mock_dns_update_zones, |
1369 | MockCalledOnceWith([NodeGroup.objects.ensure_master()])) |
1370 | @@ -446,10 +467,13 @@ |
1371 | self.assertRaises(HandlerError, handler.create, { |
1372 | "hostname": hostname, |
1373 | "primary_mac": mac, |
1374 | - "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1375 | - "interface": nodegroup_interface.id, |
1376 | - "ip_address": ip_address, |
1377 | - }) |
1378 | + "interfaces": [{ |
1379 | + "mac": mac, |
1380 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1381 | + "interface": nodegroup_interface.id, |
1382 | + "ip_address": ip_address, |
1383 | + }], |
1384 | + }) |
1385 | self.expectThat( |
1386 | Node.objects.filter(hostname=hostname).count(), |
1387 | Equals(0), "Created Node was not deleted.") |
1388 | @@ -460,6 +484,113 @@ |
1389 | StaticIPAddress.objects.filter(ip=ip_address).count(), |
1390 | Equals(0), "Created StaticIPAddress was not deleted.") |
1391 | |
1392 | + def test_create_creates_device_with_static_and_external_ip(self): |
1393 | + user = factory.make_User() |
1394 | + handler = DeviceHandler(user, {}) |
1395 | + hostname = factory.make_name("hostname") |
1396 | + nodegroup = factory.make_NodeGroup() |
1397 | + nodegroup_interface = factory.make_NodeGroupInterface(nodegroup) |
1398 | + mac_static = factory.make_mac_address() |
1399 | + static_ip_address = nodegroup_interface.static_ip_range_low |
1400 | + mac_external = factory.make_mac_address() |
1401 | + external_ip_address = factory.make_ipv4_address() |
1402 | + self.patch(dhcp_module, "update_host_maps").return_value = [] |
1403 | + created_device = handler.create({ |
1404 | + "hostname": hostname, |
1405 | + "primary_mac": mac_static, |
1406 | + "extra_macs": [ |
1407 | + mac_external |
1408 | + ], |
1409 | + "interfaces": [ |
1410 | + { |
1411 | + "mac": mac_static, |
1412 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1413 | + "interface": nodegroup_interface.id, |
1414 | + "ip_address": static_ip_address, |
1415 | + }, |
1416 | + { |
1417 | + "mac": mac_external, |
1418 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1419 | + "ip_address": external_ip_address, |
1420 | + }, |
1421 | + ], |
1422 | + }) |
1423 | + self.expectThat( |
1424 | + created_device["primary_mac"], |
1425 | + Equals(mac_static)) |
1426 | + self.expectThat( |
1427 | + created_device["extra_macs"], |
1428 | + Equals([mac_external])) |
1429 | + self.expectThat( |
1430 | + created_device["ip_assignment"], |
1431 | + Equals(DEVICE_IP_ASSIGNMENT.STATIC)) |
1432 | + self.expectThat( |
1433 | + created_device["ip_address"], Equals(static_ip_address)) |
1434 | + self.expectThat( |
1435 | + MACAddress.objects.get( |
1436 | + mac_address=MAC(mac_static)).cluster_interface, |
1437 | + Equals(nodegroup_interface), |
1438 | + "Link between MACAddress and NodeGroupInterface was not created.") |
1439 | + self.expectThat( |
1440 | + StaticIPAddress.objects.filter(ip=static_ip_address).count(), |
1441 | + Equals(1), "Static StaticIPAddress was not created.") |
1442 | + self.expectThat( |
1443 | + StaticIPAddress.objects.filter(ip=external_ip_address).count(), |
1444 | + Equals(1), "External StaticIPAddress was not created.") |
1445 | + |
1446 | + def test_create_deletes_device_and_ips_when_only_one_errors(self): |
1447 | + user = factory.make_User() |
1448 | + handler = DeviceHandler(user, {}) |
1449 | + hostname = factory.make_name("hostname") |
1450 | + nodegroup = factory.make_NodeGroup() |
1451 | + nodegroup_interface = factory.make_NodeGroupInterface(nodegroup) |
1452 | + mac_static = factory.make_mac_address() |
1453 | + static_ip_address = nodegroup_interface.static_ip_range_low |
1454 | + mac_external = factory.make_mac_address() |
1455 | + external_ip_address = factory.make_ipv4_address() |
1456 | + self.patch(dhcp_module, "update_host_maps").side_effect = [ |
1457 | + [], [Failure(factory.make_exception())], |
1458 | + ] |
1459 | + self.assertRaises(HandlerError, handler.create, { |
1460 | + "hostname": hostname, |
1461 | + "primary_mac": mac_static, |
1462 | + "extra_macs": [ |
1463 | + mac_external |
1464 | + ], |
1465 | + "interfaces": [ |
1466 | + { |
1467 | + "mac": mac_static, |
1468 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.STATIC, |
1469 | + "interface": nodegroup_interface.id, |
1470 | + "ip_address": static_ip_address, |
1471 | + }, |
1472 | + { |
1473 | + "mac": mac_external, |
1474 | + "ip_assignment": DEVICE_IP_ASSIGNMENT.EXTERNAL, |
1475 | + "ip_address": external_ip_address, |
1476 | + }, |
1477 | + ], |
1478 | + }) |
1479 | + self.expectThat( |
1480 | + Node.objects.filter(hostname=hostname).count(), |
1481 | + Equals(0), "Created Node was not deleted.") |
1482 | + self.expectThat( |
1483 | + MACAddress.objects.filter(mac_address=MAC(mac_static)).count(), |
1484 | + Equals(0), |
1485 | + "Created MACAddress for static ip address was not deleted.") |
1486 | + self.expectThat( |
1487 | + MACAddress.objects.filter(mac_address=MAC(mac_external)).count(), |
1488 | + Equals(0), |
1489 | + "Created MACAddress for external ip address was not deleted.") |
1490 | + self.expectThat( |
1491 | + StaticIPAddress.objects.filter(ip=static_ip_address).count(), |
1492 | + Equals(0), |
1493 | + "Created StaticIPAddress for static ip address was not deleted.") |
1494 | + self.expectThat( |
1495 | + StaticIPAddress.objects.filter(ip=external_ip_address).count(), |
1496 | + Equals(0), |
1497 | + "Created StaticIPAddress for external ip address was not deleted.") |
1498 | + |
1499 | def test_missing_action_raises_error(self): |
1500 | user = factory.make_User() |
1501 | device = self.make_device_with_ip_address(owner=user) |
I know this is a large change, sorry about that. Its large because it completely changes how the create operation works and all the logic in the AddDeviceContro ller.
If I was to split this up it would break the ability to create devices until all branches have been landed in trunk. Since this is very late in the cycle I decided not to do this.