Merge lp:~blake-rouse/maas/fix-1598175 into lp:~maas-committers/maas/trunk
- fix-1598175
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 5494 |
Proposed branch: | lp:~blake-rouse/maas/fix-1598175 |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
644 lines (+337/-23) 13 files modified
src/maasserver/api/blockdevices.py (+24/-9) src/maasserver/api/interfaces.py (+25/-3) src/maasserver/api/tests/test_blockdevice.py (+26/-0) src/maasserver/api/tests/test_interfaces.py (+24/-2) src/maasserver/forms.py (+16/-0) src/maasserver/forms_interface.py (+11/-0) src/maasserver/static/js/angular/controllers/node_details_networking.js (+35/-0) src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js (+92/-0) src/maasserver/static/partials/node-details.html (+8/-8) src/maasserver/tests/test_forms_blockdevice.py (+35/-0) src/maasserver/tests/test_forms_interface.py (+22/-0) src/maasserver/websockets/handlers/machine.py (+5/-1) src/maasserver/websockets/handlers/tests/test_machine.py (+14/-0) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-1598175 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Review via email:
|
Commit message
Allow changing the name and MAC address of an interface on a deployed machine. Allow changing the name, model, serial, and id_path of a physical block device on a deployed machine.
Description of the change
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Blake Rouse (blake-rouse) : | # |
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Blake Rouse (blake-rouse) wrote : | # |
Fixed issues and updated the docstrings. This is ready for another review.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Mike Pontillo (mpontillo) wrote : | # |
Looks good. Thanks for refactoring the ng-if statements; it's much clearer now.
Just one comment about the docstring below.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/maasserver/api/blockdevices.py' | |||
2 | --- src/maasserver/api/blockdevices.py 2016-08-18 17:31:05 +0000 | |||
3 | +++ src/maasserver/api/blockdevices.py 2016-10-19 18:09:00 +0000 | |||
4 | @@ -22,6 +22,7 @@ | |||
5 | 22 | from maasserver.forms import ( | 22 | from maasserver.forms import ( |
6 | 23 | CreatePhysicalBlockDeviceForm, | 23 | CreatePhysicalBlockDeviceForm, |
7 | 24 | FormatBlockDeviceForm, | 24 | FormatBlockDeviceForm, |
8 | 25 | UpdateDeployedPhysicalBlockDeviceForm, | ||
9 | 25 | UpdatePhysicalBlockDeviceForm, | 26 | UpdatePhysicalBlockDeviceForm, |
10 | 26 | UpdateVirtualBlockDeviceForm, | 27 | UpdateVirtualBlockDeviceForm, |
11 | 27 | ) | 28 | ) |
12 | @@ -227,6 +228,11 @@ | |||
13 | 227 | def update(self, request, system_id, device_id): | 228 | def update(self, request, system_id, device_id): |
14 | 228 | """Update block device on a machine. | 229 | """Update block device on a machine. |
15 | 229 | 230 | ||
16 | 231 | Machines must have a status of Ready to have access to all options. | ||
17 | 232 | Machines with Deployed status can only have the name, model, serial, | ||
18 | 233 | and/or id_path updated for a block device. This is intented to allow a | ||
19 | 234 | bad block device to be replaced while the machine remains deployed. | ||
20 | 235 | |||
21 | 230 | Fields for physical block device: | 236 | Fields for physical block device: |
22 | 231 | 237 | ||
23 | 232 | :param name: Name of the block device. | 238 | :param name: Name of the block device. |
24 | @@ -252,18 +258,27 @@ | |||
25 | 252 | device = BlockDevice.objects.get_block_device_or_404( | 258 | device = BlockDevice.objects.get_block_device_or_404( |
26 | 253 | system_id, device_id, request.user, NODE_PERMISSION.ADMIN) | 259 | system_id, device_id, request.user, NODE_PERMISSION.ADMIN) |
27 | 254 | node = device.get_node() | 260 | node = device.get_node() |
29 | 255 | if node.status != NODE_STATUS.READY: | 261 | if node.status not in [NODE_STATUS.READY, NODE_STATUS.DEPLOYED]: |
30 | 256 | raise NodeStateViolation( | 262 | raise NodeStateViolation( |
31 | 257 | "Cannot update block device because the machine is not Ready.") | 263 | "Cannot update block device because the machine is not Ready.") |
38 | 258 | if device.type == 'physical': | 264 | if node.status == NODE_STATUS.DEPLOYED: |
39 | 259 | form = UpdatePhysicalBlockDeviceForm( | 265 | if device.type == 'physical': |
40 | 260 | instance=device, data=request.data) | 266 | form = UpdateDeployedPhysicalBlockDeviceForm( |
41 | 261 | elif device.type == 'virtual': | 267 | instance=device, data=request.data) |
42 | 262 | form = UpdateVirtualBlockDeviceForm( | 268 | else: |
43 | 263 | instance=device, data=request.data) | 269 | raise NodeStateViolation( |
44 | 270 | "Cannot update virtual block device because the machine " | ||
45 | 271 | "is Deployed.") | ||
46 | 264 | else: | 272 | else: |
49 | 265 | raise ValueError( | 273 | if device.type == 'physical': |
50 | 266 | 'Cannot update block device of type %s' % device.type) | 274 | form = UpdatePhysicalBlockDeviceForm( |
51 | 275 | instance=device, data=request.data) | ||
52 | 276 | elif device.type == 'virtual': | ||
53 | 277 | form = UpdateVirtualBlockDeviceForm( | ||
54 | 278 | instance=device, data=request.data) | ||
55 | 279 | else: | ||
56 | 280 | raise ValueError( | ||
57 | 281 | 'Cannot update block device of type %s' % device.type) | ||
58 | 267 | if form.is_valid(): | 282 | if form.is_valid(): |
59 | 268 | return form.save() | 283 | return form.save() |
60 | 269 | else: | 284 | else: |
61 | 270 | 285 | ||
62 | === modified file 'src/maasserver/api/interfaces.py' | |||
63 | --- src/maasserver/api/interfaces.py 2016-08-18 17:31:05 +0000 | |||
64 | +++ src/maasserver/api/interfaces.py 2016-10-19 18:09:00 +0000 | |||
65 | @@ -26,6 +26,7 @@ | |||
66 | 26 | BondInterfaceForm, | 26 | BondInterfaceForm, |
67 | 27 | BridgeInterfaceForm, | 27 | BridgeInterfaceForm, |
68 | 28 | ControllerInterfaceForm, | 28 | ControllerInterfaceForm, |
69 | 29 | DeployedInterfaceForm, | ||
70 | 29 | InterfaceForm, | 30 | InterfaceForm, |
71 | 30 | PhysicalInterfaceForm, | 31 | PhysicalInterfaceForm, |
72 | 31 | VLANInterfaceForm, | 32 | VLANInterfaceForm, |
73 | @@ -73,8 +74,21 @@ | |||
74 | 73 | 74 | ||
75 | 74 | 75 | ||
76 | 75 | def raise_error_for_invalid_state_on_allocated_operations( | 76 | def raise_error_for_invalid_state_on_allocated_operations( |
79 | 76 | node, user, operation): | 77 | node, user, operation, extra_states=None): |
80 | 77 | if node.status not in ALLOWED_STATES: | 78 | """Raises `NodeStateViolation` when the status of the node is not |
81 | 79 | READY or BROKEN. | ||
82 | 80 | |||
83 | 81 | :param node: Node to check status. | ||
84 | 82 | :param user: User performing the operation. | ||
85 | 83 | :param operation: Nice name of the operation. | ||
86 | 84 | :param extra_states: Extra states that the node can be in when checking | ||
87 | 85 | the status of the node. | ||
88 | 86 | :type extra_states: `Iterable`. | ||
89 | 87 | """ | ||
90 | 88 | allowed = list(ALLOWED_STATES) | ||
91 | 89 | if extra_states is not None: | ||
92 | 90 | allowed.extend(extra_states) | ||
93 | 91 | if node.status not in allowed: | ||
94 | 78 | raise NodeStateViolation( | 92 | raise NodeStateViolation( |
95 | 79 | "Cannot %s interface because the machine is not Ready or " | 93 | "Cannot %s interface because the machine is not Ready or " |
96 | 80 | "Broken." % operation) | 94 | "Broken." % operation) |
97 | @@ -378,6 +392,11 @@ | |||
98 | 378 | def update(self, request, system_id, interface_id): | 392 | def update(self, request, system_id, interface_id): |
99 | 379 | """Update interface on node. | 393 | """Update interface on node. |
100 | 380 | 394 | ||
101 | 395 | Machines must has status of Ready or Broken to have access to all | ||
102 | 396 | options. Machines with Deployed status can only have the name and/or | ||
103 | 397 | mac_address updated for an interface. This is intented to allow a bad | ||
104 | 398 | interface to be replaced while the machine remains deployed. | ||
105 | 399 | |||
106 | 381 | Fields for physical interface: | 400 | Fields for physical interface: |
107 | 382 | 401 | ||
108 | 383 | :param name: Name of the interface. | 402 | :param name: Name of the interface. |
109 | @@ -477,12 +496,15 @@ | |||
110 | 477 | # This node needs to be in the correct state to modify | 496 | # This node needs to be in the correct state to modify |
111 | 478 | # the interface. | 497 | # the interface. |
112 | 479 | raise_error_for_invalid_state_on_allocated_operations( | 498 | raise_error_for_invalid_state_on_allocated_operations( |
114 | 480 | node, request.user, "update interface") | 499 | node, request.user, "update interface", |
115 | 500 | extra_states=[NODE_STATUS.DEPLOYED]) | ||
116 | 481 | if node.is_controller: | 501 | if node.is_controller: |
117 | 482 | if interface.type == INTERFACE_TYPE.VLAN: | 502 | if interface.type == INTERFACE_TYPE.VLAN: |
118 | 483 | raise MAASAPIForbidden( | 503 | raise MAASAPIForbidden( |
119 | 484 | "Cannot modify VLAN interface on controller.") | 504 | "Cannot modify VLAN interface on controller.") |
120 | 485 | interface_form = ControllerInterfaceForm | 505 | interface_form = ControllerInterfaceForm |
121 | 506 | elif node.status == NODE_STATUS.DEPLOYED: | ||
122 | 507 | interface_form = DeployedInterfaceForm | ||
123 | 486 | else: | 508 | else: |
124 | 487 | interface_form = InterfaceForm.get_interface_form(interface.type) | 509 | interface_form = InterfaceForm.get_interface_form(interface.type) |
125 | 488 | # For VLAN interface we cast parents to parent. As a VLAN can only | 510 | # For VLAN interface we cast parents to parent. As a VLAN can only |
126 | 489 | 511 | ||
127 | === modified file 'src/maasserver/api/tests/test_blockdevice.py' | |||
128 | --- src/maasserver/api/tests/test_blockdevice.py 2016-08-14 17:36:30 +0000 | |||
129 | +++ src/maasserver/api/tests/test_blockdevice.py 2016-10-19 18:09:00 +0000 | |||
130 | @@ -908,6 +908,32 @@ | |||
131 | 908 | self.assertEqual('mynewname', parsed_device['name']) | 908 | self.assertEqual('mynewname', parsed_device['name']) |
132 | 909 | self.assertEqual(4096, parsed_device['block_size']) | 909 | self.assertEqual(4096, parsed_device['block_size']) |
133 | 910 | 910 | ||
134 | 911 | def test_update_deployed_physical_block_device_as_admin(self): | ||
135 | 912 | """Update deployed physical block device. | ||
136 | 913 | |||
137 | 914 | PUT /api/2.0/nodes/{system_id}/blockdevice/{id} | ||
138 | 915 | """ | ||
139 | 916 | self.become_admin() | ||
140 | 917 | node = factory.make_Node( | ||
141 | 918 | status=NODE_STATUS.DEPLOYED, owner=self.user) | ||
142 | 919 | block_device = factory.make_PhysicalBlockDevice( | ||
143 | 920 | node=node, | ||
144 | 921 | name='myblockdevice', | ||
145 | 922 | size=MIN_BLOCK_DEVICE_SIZE, | ||
146 | 923 | block_size=1024) | ||
147 | 924 | uri = get_blockdevice_uri(block_device) | ||
148 | 925 | response = self.client.put(uri, { | ||
149 | 926 | 'name': 'mynewname', | ||
150 | 927 | 'block_size': 4096 | ||
151 | 928 | }) | ||
152 | 929 | block_device = reload_object(block_device) | ||
153 | 930 | self.assertEqual( | ||
154 | 931 | http.client.OK, response.status_code, response.content) | ||
155 | 932 | parsed_device = json_load_bytes(response.content) | ||
156 | 933 | self.assertEqual(parsed_device['id'], block_device.id) | ||
157 | 934 | self.assertEqual('mynewname', parsed_device['name']) | ||
158 | 935 | self.assertEqual(1024, parsed_device['block_size']) | ||
159 | 936 | |||
160 | 911 | def test_update_virtual_block_device_as_admin(self): | 937 | def test_update_virtual_block_device_as_admin(self): |
161 | 912 | """Check update block device with a virtual one. | 938 | """Check update block device with a virtual one. |
162 | 913 | 939 | ||
163 | 914 | 940 | ||
164 | === modified file 'src/maasserver/api/tests/test_interfaces.py' | |||
165 | --- src/maasserver/api/tests/test_interfaces.py 2016-10-18 08:00:37 +0000 | |||
166 | +++ src/maasserver/api/tests/test_interfaces.py 2016-10-19 18:09:00 +0000 | |||
167 | @@ -877,6 +877,24 @@ | |||
168 | 877 | self.assertEqual( | 877 | self.assertEqual( |
169 | 878 | http.client.NOT_FOUND, response.status_code, response.content) | 878 | http.client.NOT_FOUND, response.status_code, response.content) |
170 | 879 | 879 | ||
171 | 880 | def test_update_deployed_machine_interface(self): | ||
172 | 881 | self.become_admin() | ||
173 | 882 | node = factory.make_Node(status=NODE_STATUS.DEPLOYED) | ||
174 | 883 | interface = factory.make_Interface( | ||
175 | 884 | INTERFACE_TYPE.PHYSICAL, node=node) | ||
176 | 885 | new_name = factory.make_name("name") | ||
177 | 886 | new_mac = factory.make_mac_address() | ||
178 | 887 | uri = get_interface_uri(interface) | ||
179 | 888 | response = self.client.put(uri, { | ||
180 | 889 | "name": new_name, | ||
181 | 890 | "mac_address": new_mac, | ||
182 | 891 | }) | ||
183 | 892 | self.assertEqual( | ||
184 | 893 | http.client.OK, response.status_code, response.content) | ||
185 | 894 | parsed_interface = json_load_bytes(response.content) | ||
186 | 895 | self.assertEqual(new_name, parsed_interface["name"]) | ||
187 | 896 | self.assertEqual(new_mac, parsed_interface["mac_address"]) | ||
188 | 897 | |||
189 | 880 | def test_update_physical_interface(self): | 898 | def test_update_physical_interface(self): |
190 | 881 | self.become_admin() | 899 | self.become_admin() |
191 | 882 | for status in (NODE_STATUS.READY, NODE_STATUS.BROKEN): | 900 | for status in (NODE_STATUS.READY, NODE_STATUS.BROKEN): |
192 | @@ -963,9 +981,13 @@ | |||
193 | 963 | self.assertEqual( | 981 | self.assertEqual( |
194 | 964 | http.client.FORBIDDEN, response.status_code, response.content) | 982 | http.client.FORBIDDEN, response.status_code, response.content) |
195 | 965 | 983 | ||
197 | 966 | def test_read_409_when_not_ready_or_broken(self): | 984 | def test_update_409_when_not_ready_broken_or_deployed(self): |
198 | 967 | self.become_admin() | 985 | self.become_admin() |
200 | 968 | for status in STATUSES: | 986 | statuses = list(STATUSES) |
201 | 987 | # Update is the only call that a deployed node can have called on | ||
202 | 988 | # its interface. | ||
203 | 989 | statuses.remove(NODE_STATUS.DEPLOYED) | ||
204 | 990 | for status in statuses: | ||
205 | 969 | node = factory.make_Node(interface=True, status=status) | 991 | node = factory.make_Node(interface=True, status=status) |
206 | 970 | interface = factory.make_Interface( | 992 | interface = factory.make_Interface( |
207 | 971 | INTERFACE_TYPE.PHYSICAL, node=node) | 993 | INTERFACE_TYPE.PHYSICAL, node=node) |
208 | 972 | 994 | ||
209 | === modified file 'src/maasserver/forms.py' | |||
210 | --- src/maasserver/forms.py 2016-09-25 03:40:16 +0000 | |||
211 | +++ src/maasserver/forms.py 2016-10-19 18:09:00 +0000 | |||
212 | @@ -2537,6 +2537,22 @@ | |||
213 | 2537 | ] | 2537 | ] |
214 | 2538 | 2538 | ||
215 | 2539 | 2539 | ||
216 | 2540 | class UpdateDeployedPhysicalBlockDeviceForm(MAASModelForm): | ||
217 | 2541 | """For updating physical block device on deployed machine.""" | ||
218 | 2542 | |||
219 | 2543 | name = forms.CharField(required=False) | ||
220 | 2544 | id_path = AbsolutePathField(required=False) | ||
221 | 2545 | |||
222 | 2546 | class Meta: | ||
223 | 2547 | model = PhysicalBlockDevice | ||
224 | 2548 | fields = [ | ||
225 | 2549 | "name", | ||
226 | 2550 | "model", | ||
227 | 2551 | "serial", | ||
228 | 2552 | "id_path", | ||
229 | 2553 | ] | ||
230 | 2554 | |||
231 | 2555 | |||
232 | 2540 | class UpdateVirtualBlockDeviceForm(MAASModelForm): | 2556 | class UpdateVirtualBlockDeviceForm(MAASModelForm): |
233 | 2541 | """For updating virtual block device.""" | 2557 | """For updating virtual block device.""" |
234 | 2542 | 2558 | ||
235 | 2543 | 2559 | ||
236 | === modified file 'src/maasserver/forms_interface.py' | |||
237 | --- src/maasserver/forms_interface.py 2016-09-27 00:47:54 +0000 | |||
238 | +++ src/maasserver/forms_interface.py 2016-10-19 18:09:00 +0000 | |||
239 | @@ -187,6 +187,17 @@ | |||
240 | 187 | return interface | 187 | return interface |
241 | 188 | 188 | ||
242 | 189 | 189 | ||
243 | 190 | class DeployedInterfaceForm(MAASModelForm): | ||
244 | 191 | """Interface update form for machines when deployed.""" | ||
245 | 192 | |||
246 | 193 | class Meta: | ||
247 | 194 | model = Interface | ||
248 | 195 | fields = ( | ||
249 | 196 | 'name', | ||
250 | 197 | 'mac_address', | ||
251 | 198 | ) | ||
252 | 199 | |||
253 | 200 | |||
254 | 190 | class PhysicalInterfaceForm(InterfaceForm): | 201 | class PhysicalInterfaceForm(InterfaceForm): |
255 | 191 | """Form used to create/edit a physical interface.""" | 202 | """Form used to create/edit a physical interface.""" |
256 | 192 | 203 | ||
257 | 193 | 204 | ||
258 | === modified file 'src/maasserver/static/js/angular/controllers/node_details_networking.js' | |||
259 | --- src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-09-21 14:49:23 +0000 | |||
260 | +++ src/maasserver/static/js/angular/controllers/node_details_networking.js 2016-10-19 18:09:00 +0000 | |||
261 | @@ -556,6 +556,23 @@ | |||
262 | 556 | return true; | 556 | return true; |
263 | 557 | }; | 557 | }; |
264 | 558 | 558 | ||
265 | 559 | // Return true if only the name or mac address of an interface can | ||
266 | 560 | // be edited. | ||
267 | 561 | $scope.isLimitedEditingAllowed = function(nic) { | ||
268 | 562 | if (!$scope.isSuperUser()) { | ||
269 | 563 | // If the user is not the superuser, pretend it's not Ready. | ||
270 | 564 | return false; | ||
271 | 565 | } | ||
272 | 566 | if ($scope.$parent.isController) { | ||
273 | 567 | // Controllers never in limited mode. | ||
274 | 568 | return false; | ||
275 | 569 | } | ||
276 | 570 | return ( | ||
277 | 571 | angular.isObject($scope.node) && | ||
278 | 572 | $scope.node.status === "Deployed" && | ||
279 | 573 | nic.type !== "vlan"); | ||
280 | 574 | }; | ||
281 | 575 | |||
282 | 559 | // Return true if the networking information cannot be edited. | 576 | // Return true if the networking information cannot be edited. |
283 | 560 | // (it can't be changed when the node is in any state other | 577 | // (it can't be changed when the node is in any state other |
284 | 561 | // than Ready or Broken and the user is not a superuser) | 578 | // than Ready or Broken and the user is not a superuser) |
285 | @@ -903,6 +920,17 @@ | |||
286 | 903 | return $scope.selectedMode === SELECTION_MODE.ADD; | 920 | return $scope.selectedMode === SELECTION_MODE.ADD; |
287 | 904 | }; | 921 | }; |
288 | 905 | 922 | ||
289 | 923 | // Return true if either an alias or VLAN can be added. | ||
290 | 924 | $scope.canAddAliasOrVLAN = function(nic) { | ||
291 | 925 | if($scope.$parent.isController) { | ||
292 | 926 | return false; | ||
293 | 927 | } else if (!$scope.isNodeEditingAllowed()) { | ||
294 | 928 | return false; | ||
295 | 929 | } else { | ||
296 | 930 | return $scope.canAddAlias(nic) || $scope.canAddVLAN(nic); | ||
297 | 931 | } | ||
298 | 932 | }; | ||
299 | 933 | |||
300 | 906 | // Return true if the alias can be added to interface. | 934 | // Return true if the alias can be added to interface. |
301 | 907 | $scope.canAddAlias = function(nic) { | 935 | $scope.canAddAlias = function(nic) { |
302 | 908 | if(!angular.isObject(nic)) { | 936 | if(!angular.isObject(nic)) { |
303 | @@ -950,6 +978,13 @@ | |||
304 | 950 | } | 978 | } |
305 | 951 | }; | 979 | }; |
306 | 952 | 980 | ||
307 | 981 | // Return true if the interface can be removed. | ||
308 | 982 | $scope.canBeRemoved = function() { | ||
309 | 983 | return ( | ||
310 | 984 | !$scope.$parent.isController && | ||
311 | 985 | $scope.isNodeEditingAllowed()); | ||
312 | 986 | }; | ||
313 | 987 | |||
314 | 953 | // Enter remove mode. | 988 | // Enter remove mode. |
315 | 954 | $scope.remove = function() { | 989 | $scope.remove = function() { |
316 | 955 | $scope.selectedMode = SELECTION_MODE.DELETE; | 990 | $scope.selectedMode = SELECTION_MODE.DELETE; |
317 | 956 | 991 | ||
318 | === modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js' | |||
319 | --- src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js 2016-08-18 17:31:05 +0000 | |||
320 | +++ src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js 2016-10-19 18:09:00 +0000 | |||
321 | @@ -2027,6 +2027,45 @@ | |||
322 | 2027 | }); | 2027 | }); |
323 | 2028 | }); | 2028 | }); |
324 | 2029 | 2029 | ||
325 | 2030 | describe("canAddAliasOrVLAN", function() { | ||
326 | 2031 | |||
327 | 2032 | it("returns false if isController", function() { | ||
328 | 2033 | var controller = makeController(); | ||
329 | 2034 | $parentScope.isController = true; | ||
330 | 2035 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(true); | ||
331 | 2036 | spyOn($scope, "canAddAlias").and.returnValue(true); | ||
332 | 2037 | spyOn($scope, "canAddVLAN").and.returnValue(true); | ||
333 | 2038 | expect($scope.canAddAliasOrVLAN({})).toBe(false); | ||
334 | 2039 | }); | ||
335 | 2040 | |||
336 | 2041 | it("returns false if no node editing", function() { | ||
337 | 2042 | var controller = makeController(); | ||
338 | 2043 | $parentScope.isController = false; | ||
339 | 2044 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(false); | ||
340 | 2045 | spyOn($scope, "canAddAlias").and.returnValue(true); | ||
341 | 2046 | spyOn($scope, "canAddVLAN").and.returnValue(true); | ||
342 | 2047 | expect($scope.canAddAliasOrVLAN({})).toBe(false); | ||
343 | 2048 | }); | ||
344 | 2049 | |||
345 | 2050 | it("returns true if can edit alias", function() { | ||
346 | 2051 | var controller = makeController(); | ||
347 | 2052 | $parentScope.isController = false; | ||
348 | 2053 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(true); | ||
349 | 2054 | spyOn($scope, "canAddAlias").and.returnValue(true); | ||
350 | 2055 | spyOn($scope, "canAddVLAN").and.returnValue(false); | ||
351 | 2056 | expect($scope.canAddAliasOrVLAN({})).toBe(true); | ||
352 | 2057 | }); | ||
353 | 2058 | |||
354 | 2059 | it("returns true if can edit VLAN", function() { | ||
355 | 2060 | var controller = makeController(); | ||
356 | 2061 | $parentScope.isController = false; | ||
357 | 2062 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(true); | ||
358 | 2063 | spyOn($scope, "canAddAlias").and.returnValue(false); | ||
359 | 2064 | spyOn($scope, "canAddVLAN").and.returnValue(true); | ||
360 | 2065 | expect($scope.canAddAliasOrVLAN({})).toBe(true); | ||
361 | 2066 | }); | ||
362 | 2067 | }); | ||
363 | 2068 | |||
364 | 2030 | describe("canAddAlias", function() { | 2069 | describe("canAddAlias", function() { |
365 | 2031 | 2070 | ||
366 | 2032 | it("returns false if nic undefined", function() { | 2071 | it("returns false if nic undefined", function() { |
367 | @@ -2365,6 +2404,30 @@ | |||
368 | 2365 | }); | 2404 | }); |
369 | 2366 | }); | 2405 | }); |
370 | 2367 | 2406 | ||
371 | 2407 | describe("canBeRemoved", function() { | ||
372 | 2408 | |||
373 | 2409 | it("false if isController", function() { | ||
374 | 2410 | var controller = makeController(); | ||
375 | 2411 | $parentScope.isController = true; | ||
376 | 2412 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(true); | ||
377 | 2413 | expect($scope.canBeRemoved()).toBe(false); | ||
378 | 2414 | }); | ||
379 | 2415 | |||
380 | 2416 | it("false if no node editing", function() { | ||
381 | 2417 | var controller = makeController(); | ||
382 | 2418 | $parentScope.isController = false; | ||
383 | 2419 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(false); | ||
384 | 2420 | expect($scope.canBeRemoved()).toBe(false); | ||
385 | 2421 | }); | ||
386 | 2422 | |||
387 | 2423 | it("true if node can be edited", function() { | ||
388 | 2424 | var controller = makeController(); | ||
389 | 2425 | $parentScope.isController = false; | ||
390 | 2426 | spyOn($scope, "isNodeEditingAllowed").and.returnValue(true); | ||
391 | 2427 | expect($scope.canBeRemoved()).toBe(true); | ||
392 | 2428 | }); | ||
393 | 2429 | }); | ||
394 | 2430 | |||
395 | 2368 | describe("remove", function() { | 2431 | describe("remove", function() { |
396 | 2369 | 2432 | ||
397 | 2370 | it("sets selectedMode to delete", function() { | 2433 | it("sets selectedMode to delete", function() { |
398 | @@ -2935,6 +2998,35 @@ | |||
399 | 2935 | }); | 2998 | }); |
400 | 2936 | }); | 2999 | }); |
401 | 2937 | 3000 | ||
402 | 3001 | describe("isLimitedEditingAllowed", function() { | ||
403 | 3002 | |||
404 | 3003 | it("returns false when not superuser", function() { | ||
405 | 3004 | var controller = makeController(); | ||
406 | 3005 | $scope.isSuperUser = function() { return false; }; | ||
407 | 3006 | expect($scope.isLimitedEditingAllowed()).toBe(false); | ||
408 | 3007 | }); | ||
409 | 3008 | |||
410 | 3009 | it("returns false when isController", function() { | ||
411 | 3010 | var controller = makeController(); | ||
412 | 3011 | $scope.isSuperUser = function() { return true; }; | ||
413 | 3012 | $parentScope.isController = true; | ||
414 | 3013 | expect($scope.isLimitedEditingAllowed()).toBe(false); | ||
415 | 3014 | }); | ||
416 | 3015 | |||
417 | 3016 | it("returns true when deployed and not vlan", function() { | ||
418 | 3017 | var controller = makeController(); | ||
419 | 3018 | $scope.isSuperUser = function() { return true; }; | ||
420 | 3019 | $parentScope.isController = false; | ||
421 | 3020 | $scope.node = { | ||
422 | 3021 | status: "Deployed" | ||
423 | 3022 | }; | ||
424 | 3023 | var nic = { | ||
425 | 3024 | type: "physical" | ||
426 | 3025 | }; | ||
427 | 3026 | expect($scope.isLimitedEditingAllowed(nic)).toBe(true); | ||
428 | 3027 | }); | ||
429 | 3028 | }); | ||
430 | 3029 | |||
431 | 2938 | describe("isAllNetworkingDisabled", function() { | 3030 | describe("isAllNetworkingDisabled", function() { |
432 | 2939 | 3031 | ||
433 | 2940 | it("returns true if the user is not a superuser " + | 3032 | it("returns true if the user is not a superuser " + |
434 | 2941 | 3033 | ||
435 | === modified file 'src/maasserver/static/partials/node-details.html' | |||
436 | --- src/maasserver/static/partials/node-details.html 2016-09-30 12:50:08 +0000 | |||
437 | +++ src/maasserver/static/partials/node-details.html 2016-10-19 18:09:00 +0000 | |||
438 | @@ -574,7 +574,7 @@ | |||
439 | 574 | </header> | 574 | </header> |
440 | 575 | <main class="table__body" data-selected-rows> | 575 | <main class="table__body" data-selected-rows> |
441 | 576 | <div class="table__row" | 576 | <div class="table__row" |
443 | 577 | data-ng-class="{ disabled: isDisabled(), 'is-active': isInterfaceSelected(interface) && isNodeEditingAllowed(), noEdit: cannotEditInterface(interface) }" | 577 | data-ng-class="{ disabled: isDisabled(), 'is-active': isInterfaceSelected(interface) && (isNodeEditingAllowed() || isLimitedEditingAllowed(interface)), noEdit: cannotEditInterface(interface) }" |
444 | 578 | data-ng-repeat="interface in interfaces | removeInterfaceParents:newBondInterface:!isNodeEditingAllowed() | removeInterfaceParents:newBridgeInterface:!isNodeEditingAllowed()"> | 578 | data-ng-repeat="interface in interfaces | removeInterfaceParents:newBondInterface:!isNodeEditingAllowed() | removeInterfaceParents:newBridgeInterface:!isNodeEditingAllowed()"> |
445 | 579 | <div class="table__data table-col--3"> | 579 | <div class="table__data table-col--3"> |
446 | 580 | <input type="checkbox" class="checkbox" id="{$ getUniqueKey(interface) $}" | 580 | <input type="checkbox" class="checkbox" id="{$ getUniqueKey(interface) $}" |
447 | @@ -588,7 +588,7 @@ | |||
448 | 588 | <div class="table__data table-col--12" data-ng-show="column == 'name'"> | 588 | <div class="table__data table-col--12" data-ng-show="column == 'name'"> |
449 | 589 | <span data-ng-if="!isEditing(interface)">{$ interface.name $}</span> | 589 | <span data-ng-if="!isEditing(interface)">{$ interface.name $}</span> |
450 | 590 | <input type="text" class="table__input" | 590 | <input type="text" class="table__input" |
452 | 591 | data-ng-if="isEditing(interface)" | 591 | data-ng-if="isEditing(interface) && interface.type != 'vlan'" |
453 | 592 | data-ng-model="editInterface.name" | 592 | data-ng-model="editInterface.name" |
454 | 593 | data-ng-class="{ 'has-error': isInterfaceNameInvalid(editInterface) }"> | 593 | data-ng-class="{ 'has-error': isInterfaceNameInvalid(editInterface) }"> |
455 | 594 | </div> | 594 | </div> |
456 | @@ -624,17 +624,17 @@ | |||
457 | 624 | </span> | 624 | </span> |
458 | 625 | </div> | 625 | </div> |
459 | 626 | <div class="table__data table-col--8"> | 626 | <div class="table__data table-col--8"> |
461 | 627 | <div class="table__controls u-align--right" data-ng-if="isNodeEditingAllowed()"> | 627 | <div class="table__controls u-align--right" data-ng-if="isNodeEditingAllowed() || isLimitedEditingAllowed(interface)"> |
462 | 628 | <a class="icon icon--add tooltip" | 628 | <a class="icon icon--add tooltip" |
463 | 629 | aria-label="Add Alias or VLAN" | 629 | aria-label="Add Alias or VLAN" |
465 | 630 | data-ng-if="!isController && (canAddAlias(interface) || canAddVLAN(interface))" | 630 | data-ng-if="canAddAliasOrVLAN(interface)" |
466 | 631 | data-ng-click="quickAdd(interface)"></a> | 631 | data-ng-click="quickAdd(interface)"></a> |
467 | 632 | <a class="icon icon--edit tooltip" | 632 | <a class="icon icon--edit tooltip" |
468 | 633 | data-ng-if="!cannotEditInterface(interface)" | 633 | data-ng-if="!cannotEditInterface(interface)" |
469 | 634 | aria-label="Edit" | 634 | aria-label="Edit" |
470 | 635 | data-ng-click="edit(interface)"></a> | 635 | data-ng-click="edit(interface)"></a> |
471 | 636 | <a class="icon icon--delete tooltip u-margin--left" | 636 | <a class="icon icon--delete tooltip u-margin--left" |
473 | 637 | data-ng-if="!isController" | 637 | data-ng-if="canBeRemoved()" |
474 | 638 | aria-label="Remove" | 638 | aria-label="Remove" |
475 | 639 | data-ng-click="quickRemove(interface)"></a> | 639 | data-ng-click="quickRemove(interface)"></a> |
476 | 640 | </div> | 640 | </div> |
477 | @@ -653,7 +653,7 @@ | |||
478 | 653 | </div> | 653 | </div> |
479 | 654 | </div> | 654 | </div> |
480 | 655 | </div> | 655 | </div> |
482 | 656 | <div data-ng-show="isNodeEditingAllowed()"> | 656 | <div data-ng-if="isNodeEditingAllowed() || isLimitedEditingAllowed(interface)"> |
483 | 657 | <div class="table__dropdown"> | 657 | <div class="table__dropdown"> |
484 | 658 | <div class="table__row form form--stack u-padding--top u-padding--right u-padding--left box-sizing" data-ng-class="{ 'is-active': isEditing(interface) || isShowingAdd() }"> | 658 | <div class="table__row form form--stack u-padding--top u-padding--right u-padding--left box-sizing" data-ng-class="{ 'is-active': isEditing(interface) || isShowingAdd() }"> |
485 | 659 | <div data-ng-if="isShowingAdd()" class="table__row--indent"> | 659 | <div data-ng-if="isShowingAdd()" class="table__row--indent"> |
486 | @@ -742,14 +742,14 @@ | |||
487 | 742 | data-ng-model="editInterface.mac_address" | 742 | data-ng-model="editInterface.mac_address" |
488 | 743 | data-ng-class="{ 'has-error': isMACAddressInvalid(editInterface.mac_address, true) }"> | 743 | data-ng-class="{ 'has-error': isMACAddressInvalid(editInterface.mac_address, true) }"> |
489 | 744 | </div> | 744 | </div> |
491 | 745 | <div class="form__group"> | 745 | <div class="form__group" data-ng-if="!isLimitedEditingAllowed(interface)"> |
492 | 746 | <label class="two-col" data-ng-if="interface.type != 'alias'">Tags</label> | 746 | <label class="two-col" data-ng-if="interface.type != 'alias'">Tags</label> |
493 | 747 | <div class="form__group-input three-col last-col" data-ng-if="interface.type != 'alias'"> | 747 | <div class="form__group-input three-col last-col" data-ng-if="interface.type != 'alias'"> |
494 | 748 | <tags-input ng-model="editInterface.tags" allow-tags-pattern="[\w-]+"></tags-input> | 748 | <tags-input ng-model="editInterface.tags" allow-tags-pattern="[\w-]+"></tags-input> |
495 | 749 | </div> | 749 | </div> |
496 | 750 | </div> | 750 | </div> |
497 | 751 | </div> | 751 | </div> |
499 | 752 | <div class="form__fieldset six-col last-col"> | 752 | <div class="form__fieldset six-col last-col" data-ng-if="!isLimitedEditingAllowed(interface)"> |
500 | 753 | <div class="form__group"> | 753 | <div class="form__group"> |
501 | 754 | <label for="fabric" class="two-col">Fabric</label> | 754 | <label for="fabric" class="two-col">Fabric</label> |
502 | 755 | <select name="fabric" class="three-col" | 755 | <select name="fabric" class="three-col" |
503 | 756 | 756 | ||
504 | === modified file 'src/maasserver/tests/test_forms_blockdevice.py' | |||
505 | --- src/maasserver/tests/test_forms_blockdevice.py 2016-03-28 13:54:47 +0000 | |||
506 | +++ src/maasserver/tests/test_forms_blockdevice.py 2016-10-19 18:09:00 +0000 | |||
507 | @@ -12,6 +12,7 @@ | |||
508 | 12 | from maasserver.forms import ( | 12 | from maasserver.forms import ( |
509 | 13 | CreatePhysicalBlockDeviceForm, | 13 | CreatePhysicalBlockDeviceForm, |
510 | 14 | FormatBlockDeviceForm, | 14 | FormatBlockDeviceForm, |
511 | 15 | UpdateDeployedPhysicalBlockDeviceForm, | ||
512 | 15 | UpdatePhysicalBlockDeviceForm, | 16 | UpdatePhysicalBlockDeviceForm, |
513 | 16 | UpdateVirtualBlockDeviceForm, | 17 | UpdateVirtualBlockDeviceForm, |
514 | 17 | ) | 18 | ) |
515 | @@ -245,6 +246,40 @@ | |||
516 | 245 | )) | 246 | )) |
517 | 246 | 247 | ||
518 | 247 | 248 | ||
519 | 249 | class TestUpdateDeployedPhysicalBlockDeviceForm(MAASServerTestCase): | ||
520 | 250 | |||
521 | 251 | def test_requires_no_fields(self): | ||
522 | 252 | block_device = factory.make_PhysicalBlockDevice() | ||
523 | 253 | form = UpdateDeployedPhysicalBlockDeviceForm( | ||
524 | 254 | instance=block_device, data={}) | ||
525 | 255 | self.assertTrue(form.is_valid(), form.errors) | ||
526 | 256 | self.assertItemsEqual([], form.errors.keys()) | ||
527 | 257 | |||
528 | 258 | def test_updates_deployed_physical_block_device(self): | ||
529 | 259 | block_device = factory.make_PhysicalBlockDevice() | ||
530 | 260 | name = factory.make_name("sd") | ||
531 | 261 | model = factory.make_name("model") | ||
532 | 262 | serial = factory.make_name("serial") | ||
533 | 263 | id_path = factory.make_absolute_path() | ||
534 | 264 | form = UpdateDeployedPhysicalBlockDeviceForm( | ||
535 | 265 | instance=block_device, data={ | ||
536 | 266 | 'name': name, | ||
537 | 267 | 'model': model, | ||
538 | 268 | 'serial': serial, | ||
539 | 269 | 'id_path': id_path, | ||
540 | 270 | }) | ||
541 | 271 | self.assertTrue(form.is_valid(), form.errors) | ||
542 | 272 | block_device = form.save() | ||
543 | 273 | self.assertThat(block_device, MatchesStructure.byEquality( | ||
544 | 274 | name=name, | ||
545 | 275 | model=model, | ||
546 | 276 | serial=serial, | ||
547 | 277 | id_path=id_path, | ||
548 | 278 | size=block_device.size, | ||
549 | 279 | block_size=block_device.block_size, | ||
550 | 280 | )) | ||
551 | 281 | |||
552 | 282 | |||
553 | 248 | class TestUpdateVirtualBlockDeviceForm(MAASServerTestCase): | 283 | class TestUpdateVirtualBlockDeviceForm(MAASServerTestCase): |
554 | 249 | 284 | ||
555 | 250 | def test_requires_no_fields(self): | 285 | def test_requires_no_fields(self): |
556 | 251 | 286 | ||
557 | === modified file 'src/maasserver/tests/test_forms_interface.py' | |||
558 | --- src/maasserver/tests/test_forms_interface.py 2016-09-27 14:24:19 +0000 | |||
559 | +++ src/maasserver/tests/test_forms_interface.py 2016-10-19 18:09:00 +0000 | |||
560 | @@ -20,6 +20,7 @@ | |||
561 | 20 | BondInterfaceForm, | 20 | BondInterfaceForm, |
562 | 21 | BridgeInterfaceForm, | 21 | BridgeInterfaceForm, |
563 | 22 | ControllerInterfaceForm, | 22 | ControllerInterfaceForm, |
564 | 23 | DeployedInterfaceForm, | ||
565 | 23 | InterfaceForm, | 24 | InterfaceForm, |
566 | 24 | PhysicalInterfaceForm, | 25 | PhysicalInterfaceForm, |
567 | 25 | VLANInterfaceForm, | 26 | VLANInterfaceForm, |
568 | @@ -104,6 +105,27 @@ | |||
569 | 104 | name=interface.name, vlan=None, enabled=interface.enabled)) | 105 | name=interface.name, vlan=None, enabled=interface.enabled)) |
570 | 105 | 106 | ||
571 | 106 | 107 | ||
572 | 108 | class DeployedInterfaceFormTest(MAASServerTestCase): | ||
573 | 109 | |||
574 | 110 | def test__updates_interface(self): | ||
575 | 111 | interface = factory.make_Interface( | ||
576 | 112 | INTERFACE_TYPE.PHYSICAL, name='eth0') | ||
577 | 113 | new_name = 'eth1' | ||
578 | 114 | new_mac = factory.make_mac_address() | ||
579 | 115 | form = DeployedInterfaceForm( | ||
580 | 116 | instance=interface, | ||
581 | 117 | data={ | ||
582 | 118 | 'name': new_name, | ||
583 | 119 | 'mac_address': new_mac, | ||
584 | 120 | }) | ||
585 | 121 | self.assertTrue(form.is_valid(), form.errors) | ||
586 | 122 | interface = form.save() | ||
587 | 123 | self.assertThat( | ||
588 | 124 | interface, | ||
589 | 125 | MatchesStructure.byEquality( | ||
590 | 126 | name=new_name, mac_address=new_mac)) | ||
591 | 127 | |||
592 | 128 | |||
593 | 107 | class PhysicalInterfaceFormTest(MAASServerTestCase): | 129 | class PhysicalInterfaceFormTest(MAASServerTestCase): |
594 | 108 | 130 | ||
595 | 109 | def test__creates_physical_interface(self): | 131 | def test__creates_physical_interface(self): |
596 | 110 | 132 | ||
597 | === modified file 'src/maasserver/websockets/handlers/machine.py' | |||
598 | --- src/maasserver/websockets/handlers/machine.py 2016-10-13 01:35:42 +0000 | |||
599 | +++ src/maasserver/websockets/handlers/machine.py 2016-10-19 18:09:00 +0000 | |||
600 | @@ -45,6 +45,7 @@ | |||
601 | 45 | AcquiredBridgeInterfaceForm, | 45 | AcquiredBridgeInterfaceForm, |
602 | 46 | BondInterfaceForm, | 46 | BondInterfaceForm, |
603 | 47 | BridgeInterfaceForm, | 47 | BridgeInterfaceForm, |
604 | 48 | DeployedInterfaceForm, | ||
605 | 48 | InterfaceForm, | 49 | InterfaceForm, |
606 | 49 | PhysicalInterfaceForm, | 50 | PhysicalInterfaceForm, |
607 | 50 | VLANInterfaceForm, | 51 | VLANInterfaceForm, |
608 | @@ -811,7 +812,10 @@ | |||
609 | 811 | 812 | ||
610 | 812 | node = self.get_object(params) | 813 | node = self.get_object(params) |
611 | 813 | interface = Interface.objects.get(node=node, id=params["interface_id"]) | 814 | interface = Interface.objects.get(node=node, id=params["interface_id"]) |
613 | 814 | interface_form = InterfaceForm.get_interface_form(interface.type) | 815 | if node.status == NODE_STATUS.DEPLOYED: |
614 | 816 | interface_form = DeployedInterfaceForm | ||
615 | 817 | else: | ||
616 | 818 | interface_form = InterfaceForm.get_interface_form(interface.type) | ||
617 | 815 | form = interface_form(instance=interface, data=params) | 819 | form = interface_form(instance=interface, data=params) |
618 | 816 | if form.is_valid(): | 820 | if form.is_valid(): |
619 | 817 | interface = form.save() | 821 | interface = form.save() |
620 | 818 | 822 | ||
621 | === modified file 'src/maasserver/websockets/handlers/tests/test_machine.py' | |||
622 | --- src/maasserver/websockets/handlers/tests/test_machine.py 2016-10-12 16:34:23 +0000 | |||
623 | +++ src/maasserver/websockets/handlers/tests/test_machine.py 2016-10-19 18:09:00 +0000 | |||
624 | @@ -2326,6 +2326,20 @@ | |||
625 | 2326 | self.assertEqual(new_name, interface.name) | 2326 | self.assertEqual(new_name, interface.name) |
626 | 2327 | self.assertEqual(new_vlan, interface.vlan) | 2327 | self.assertEqual(new_vlan, interface.vlan) |
627 | 2328 | 2328 | ||
628 | 2329 | def test_update_interface_for_deployed_node(self): | ||
629 | 2330 | user = factory.make_admin() | ||
630 | 2331 | node = factory.make_Node(status=NODE_STATUS.DEPLOYED) | ||
631 | 2332 | handler = MachineHandler(user, {}) | ||
632 | 2333 | interface = factory.make_Interface(INTERFACE_TYPE.PHYSICAL, node=node) | ||
633 | 2334 | new_name = factory.make_name("name") | ||
634 | 2335 | handler.update_interface({ | ||
635 | 2336 | "system_id": node.system_id, | ||
636 | 2337 | "interface_id": interface.id, | ||
637 | 2338 | "name": new_name, | ||
638 | 2339 | }) | ||
639 | 2340 | interface = reload_object(interface) | ||
640 | 2341 | self.assertEqual(new_name, interface.name) | ||
641 | 2342 | |||
642 | 2329 | def test_update_interface_raises_ValidationError(self): | 2343 | def test_update_interface_raises_ValidationError(self): |
643 | 2330 | user = factory.make_admin() | 2344 | user = factory.make_admin() |
644 | 2331 | node = factory.make_Node() | 2345 | node = factory.make_Node() |
Code looks pretty good (see a few comments below, though).
"Needs fixing", though, because I don't see any docstring updates. We need to explain to API users what [new] states editing is allowed in, and which values can be updated.