Merge ~ltrager/maas:no-arch-and-mac-address-validation-with-ipmi-power-parameters into maas:master
- Git
- lp:~ltrager/maas
- no-arch-and-mac-address-validation-with-ipmi-power-parameters
- Merge into master
Status: | Merged |
---|---|
Approved by: | Lee Trager |
Approved revision: | 123adab60c94f28c336c3088382fd54d4bb37e3c |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~ltrager/maas:no-arch-and-mac-address-validation-with-ipmi-power-parameters |
Merge into: | maas:master |
Diff against target: |
615 lines (+194/-115) 12 files modified
src/maasserver/api/machines.py (+53/-27) src/maasserver/api/tests/test_enlistment.py (+52/-4) src/maasserver/api/tests/test_machines.py (+27/-0) src/maasserver/forms/__init__.py (+25/-7) src/maasserver/forms/tests/test_machine.py (+1/-1) src/maasserver/forms/tests/test_machinewithmacaddresses.py (+19/-1) src/maasserver/models/node.py (+1/-13) src/maasserver/models/tests/test_node.py (+0/-23) src/maasserver/static/js/angular/controllers/add_hardware.js (+10/-2) src/maasserver/static/js/angular/controllers/tests/test_add_hardware.js (+5/-5) src/maasserver/static/partials/nodes-list.html (+1/-1) src/maasserver/websockets/handlers/tests/test_machine.py (+0/-31) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andres Rodriguez (community) | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+348256@code.launchpad.net |
Commit message
LP: #1707216 - Allow machines with IPMI BMCs to be added without arch or MAC
Machines which use an IPMI BMC may now be added without giving an architecture and/or MAC addresses. If an incorrect MAC address is
given it will be automatically corrected.
Description of the change
Scenarios tested in the CI
* Enlistment via power on of the BMC
* Adding a machine with the correct architecture and MAC address
* Adding a machine with the correct architecture and a bad MAC address
* Adding a machine with no architecture and correct MAC address
* Adding a machine with no architecture or MAC address
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b no-arch-
STATUS: SUCCESS
COMMIT: e3bc53406b3c618
- 1e112aa... by Lee Trager
-
Merge branch 'master' into no-arch-
and-mac- address- validation- with-ipmi- power-parameter s
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b no-arch-
STATUS: SUCCESS
COMMIT: 1e112aa3f60f1e1
Andres Rodriguez (andreserl) wrote : | # |
Please see inline.
- e309910... by Lee Trager
-
Merge branch 'master' into no-arch-
and-mac- address- validation- with-ipmi- power-parameter s
Lee Trager (ltrager) wrote : | # |
The changes to the Javascript allow a user to create a machine without an architecture or MAC address. If no architecture is given but a MAC address is given the form ignores the given MAC address(see line 328 of the review). This forces the machine to go into enlistment which sets a proper architecture and commissioning sets the correct MAC address.
Andres Rodriguez (andreserl) wrote : | # |
So why ignore the MAC address. If the user is giving us a MAC address we
shouldn't be ignoring it becuase that's what the user gave us.
On Thu, Jun 21, 2018 at 12:28 PM, Lee Trager <email address hidden>
wrote:
> The changes to the Javascript allow a user to create a machine without an
> architecture or MAC address. If no architecture is given but a MAC address
> is given the form ignores the given MAC address(see line 328 of the
> review). This forces the machine to go into enlistment which sets a proper
> architecture and commissioning sets the correct MAC address.
> --
> https:/
> You are reviewing the proposed merge of ~ltrager/
> address-
>
--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b no-arch-
STATUS: SUCCESS
COMMIT: e309910f2a81d54
Lee Trager (ltrager) wrote : | # |
If the MAC address is set the machine will go into commissioning. Commissioning will fail when the preseed is fetched because the preseed needs the correct architecture to know what repositories to configure. Even if we work around that problem the architecture still needs to be set for deployments. Currently, only enlistment discovers the architecture.
When commissioning runs the MAC addresses are set correctly. If the MAC the user gave us is really on the machine it will be added.
- e73bce4... by Lee Trager
-
Merge branch 'master' into no-arch-
and-mac- address- validation- with-ipmi- power-parameter s
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b no-arch-
STATUS: FAILED
LOG: http://
COMMIT: e73bce441a7296f
- 123adab... by Lee Trager
-
Merge branch 'master' into no-arch-
and-mac- address- validation- with-ipmi- power-parameter s
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b no-arch-
STATUS: SUCCESS
COMMIT: 123adab60c94f28
Preview Diff
1 | diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py |
2 | index 9d9bad6..f8d1f84 100644 |
3 | --- a/src/maasserver/api/machines.py |
4 | +++ b/src/maasserver/api/machines.py |
5 | @@ -60,6 +60,7 @@ from maasserver.exceptions import ( |
6 | Unauthorized, |
7 | ) |
8 | from maasserver.forms import ( |
9 | + AdminMachineForm, |
10 | get_machine_create_form, |
11 | get_machine_edit_form, |
12 | ) |
13 | @@ -1064,43 +1065,46 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin): |
14 | return machine |
15 | |
16 | |
17 | -def create_machine(request): |
18 | - """Service an http request to create a machine. |
19 | - |
20 | - The machine will be in the New state. |
21 | - |
22 | - :param request: The http request for this machine to be created. |
23 | - :return: A `Machine`. |
24 | - :rtype: :class:`maasserver.models.Machine`. |
25 | - :raises: ValidationError |
26 | - """ |
27 | - |
28 | +def fix_architecture(data): |
29 | # For backwards compatibilty reasons, requests may be sent with: |
30 | # architecture with a '/' in it: use normally |
31 | # architecture without a '/' and no subarchitecture: assume 'generic' |
32 | # architecture without a '/' and a subarchitecture: use as specified |
33 | # architecture with a '/' and a subarchitecture: error |
34 | - given_arch = request.data.get('architecture', None) |
35 | - given_subarch = request.data.get('subarchitecture', None) |
36 | - given_min_hwe_kernel = request.data.get('min_hwe_kernel', None) |
37 | - altered_query_data = request.data.copy() |
38 | - if given_arch and '/' in given_arch: |
39 | - if given_subarch: |
40 | + architecture = data.get('architecture', None) |
41 | + subarchitecture = data.get('subarchitecture', None) |
42 | + if architecture and '/' in architecture: |
43 | + if subarchitecture: |
44 | # Architecture with a '/' and a subarchitecture: error. |
45 | raise MAASAPIValidationError( |
46 | 'Subarchitecture cannot be specified twice.') |
47 | - # Architecture with a '/' in it: use normally. |
48 | - elif given_arch: |
49 | - if given_subarch: |
50 | + elif architecture: |
51 | + if subarchitecture: |
52 | # Architecture without a '/' and a subarchitecture: |
53 | # use as specified. |
54 | - altered_query_data['architecture'] = '/'.join( |
55 | - [given_arch, given_subarch]) |
56 | - del altered_query_data['subarchitecture'] |
57 | + data['architecture'] = '/'.join([architecture, subarchitecture]) |
58 | + del data['subarchitecture'] |
59 | else: |
60 | # Architecture without a '/' and no subarchitecture: |
61 | # assume 'generic'. |
62 | - altered_query_data['architecture'] += '/generic' |
63 | + data['architecture'] += '/generic' |
64 | + |
65 | + |
66 | +def create_machine(request, requires_arch=False): |
67 | + """Service an http request to create a machine. |
68 | + |
69 | + The machine will be in the New state. |
70 | + |
71 | + :param request: The http request for this machine to be created. |
72 | + :return: A `Machine`. |
73 | + :rtype: :class:`maasserver.models.Machine`. |
74 | + :raises: ValidationError |
75 | + """ |
76 | + given_arch = request.data.get('architecture', None) |
77 | + given_subarch = request.data.get('subarchitecture', None) |
78 | + given_min_hwe_kernel = request.data.get('min_hwe_kernel', None) |
79 | + altered_query_data = request.data.copy() |
80 | + fix_architecture(altered_query_data) |
81 | |
82 | hwe_regex = re.compile('(hwe|ga)-.+') |
83 | has_arch_with_hwe = ( |
84 | @@ -1117,7 +1121,8 @@ def create_machine(request): |
85 | 'min_hwe_kernel must be in the form of hwe-<LETTER>.') |
86 | |
87 | Form = get_machine_create_form(request.user) |
88 | - form = Form(data=altered_query_data, request=request) |
89 | + form = Form( |
90 | + data=altered_query_data, request=request, requires_arch=requires_arch) |
91 | if form.is_valid(): |
92 | machine = form.save() |
93 | maaslog.info("%s: Enlisted new machine", machine.hostname) |
94 | @@ -1201,6 +1206,7 @@ class AnonMachinesHandler(AnonNodesHandler): |
95 | # BMC enlistment is currently only done for IPMI. |
96 | # First, check if there is a pre-existing machine within MAAS that |
97 | # matches the request. |
98 | + architecture = request.data.get('architecture', None) |
99 | power_type = request.data.get('power_type', None) |
100 | power_parameters = request.data.get('power_parameters', None) |
101 | machine = None |
102 | @@ -1210,10 +1216,30 @@ class AnonMachinesHandler(AnonNodesHandler): |
103 | status__in=[NODE_STATUS.NEW, NODE_STATUS.COMMISSIONING], |
104 | bmc__power_parameters__power_address=( |
105 | params['power_address'])).first() |
106 | + if machine is not None: |
107 | + # Update the power parameters with the new MAAS password and |
108 | + # ensure the architecture is set. |
109 | + data = { |
110 | + 'architecture': architecture, |
111 | + 'power_type': power_type, |
112 | + 'power_parameters': power_parameters, |
113 | + } |
114 | + fix_architecture(data) |
115 | + |
116 | + # AdminMachineForm must be used so the power parameters can be |
117 | + # updated. |
118 | + form = AdminMachineForm( |
119 | + data=data, instance=machine, requires_arch=True) |
120 | + if form.is_valid(): |
121 | + machine = form.save() |
122 | + else: |
123 | + raise MAASAPIValidationError(form.errors) |
124 | if machine is None: |
125 | - machine = create_machine(request) |
126 | + machine = create_machine(request, requires_arch=True) |
127 | if machine.status == NODE_STATUS.NEW: |
128 | - # Create or update NodeMetadata object for this node. |
129 | + # Make sure an enlisting NodeMetadata object exists if the machine |
130 | + # is NEW. When commissioning finishes this is how MAAS knows to |
131 | + # set the status to NEW instead of READY. |
132 | NodeMetadata.objects.update_or_create( |
133 | node=machine, key='enlisting', defaults={'value': 'True'}) |
134 | return machine |
135 | diff --git a/src/maasserver/api/tests/test_enlistment.py b/src/maasserver/api/tests/test_enlistment.py |
136 | index 07cc179..753bfdc 100644 |
137 | --- a/src/maasserver/api/tests/test_enlistment.py |
138 | +++ b/src/maasserver/api/tests/test_enlistment.py |
139 | @@ -455,20 +455,28 @@ class AnonymousEnlistmentAPITest(APITestCase.ForAnonymous): |
140 | } |
141 | machine = factory.make_Machine( |
142 | hostname=hostname, status=NODE_STATUS.NEW, |
143 | - architecture=architecture, power_type=power_type, |
144 | + architecture='', power_type=power_type, |
145 | power_parameters=power_parameters) |
146 | + # Simulate creating the MAAS IPMI user |
147 | + power_parameters["power_user"] = "maas" |
148 | + power_parameters["power_pass"] = factory.make_name("power-pass") |
149 | response = self.client.post( |
150 | reverse('machines_handler'), |
151 | { |
152 | - 'hostname': hostname, |
153 | + 'hostname': 'maas-enlistment', |
154 | 'architecture': architecture, |
155 | 'power_type': power_type, |
156 | 'mac_addresses': factory.make_mac_address(), |
157 | 'power_parameters': json.dumps(power_parameters), |
158 | }) |
159 | - node_metadata = NodeMetadata.objects.get(key='enlisting') |
160 | - self.assertEqual(node_metadata.value, 'True') |
161 | self.assertEqual(http.client.OK, response.status_code) |
162 | + machine = reload_object(machine) |
163 | + self.assertEqual(hostname, machine.hostname) |
164 | + self.assertEqual(architecture, machine.architecture) |
165 | + self.assertDictContainsSubset( |
166 | + machine.bmc.power_parameters, power_parameters) |
167 | + node_metadata = NodeMetadata.objects.get(node=machine, key='enlisting') |
168 | + self.assertEqual(node_metadata.value, 'True') |
169 | self.assertThat(mock_create_machine, MockNotCalled()) |
170 | self.assertEqual( |
171 | machine.system_id, json_load_bytes(response.content)['system_id']) |
172 | @@ -492,6 +500,46 @@ class AnonymousEnlistmentAPITest(APITestCase.ForAnonymous): |
173 | self.assertEqual( |
174 | machine.system_id, json_load_bytes(response.content)['system_id']) |
175 | |
176 | + def test_POST_create_requires_architecture(self): |
177 | + hostname = factory.make_name("hostname") |
178 | + response = self.client.post( |
179 | + reverse('machines_handler'), |
180 | + { |
181 | + 'hostname': hostname, |
182 | + 'power_type': 'manual', |
183 | + 'mac_addresses': ['aa:bb:cc:dd:ee:ff', '22:bb:cc:dd:ee:ff'], |
184 | + }) |
185 | + self.assertEqual(http.client.BAD_REQUEST, response.status_code) |
186 | + self.assertDictEqual( |
187 | + {'architecture': ['This field is required.']}, |
188 | + json_load_bytes(response.content)) |
189 | + |
190 | + def test_POST_create_validates_architecture(self): |
191 | + hostname = factory.make_name("hostname") |
192 | + power_type = 'ipmi' |
193 | + power_parameters = { |
194 | + "power_address": factory.make_ip_address(), |
195 | + "power_user": factory.make_name("power-user"), |
196 | + "power_pass": factory.make_name("power-pass"), |
197 | + "power_driver": 'LAN_2_0', |
198 | + "mac_address": '', |
199 | + "power_boot_type": 'auto', |
200 | + } |
201 | + factory.make_Machine( |
202 | + hostname=hostname, status=NODE_STATUS.NEW, |
203 | + architecture='', power_type=power_type, |
204 | + power_parameters=power_parameters) |
205 | + response = self.client.post( |
206 | + reverse('machines_handler'), |
207 | + { |
208 | + 'hostname': 'maas-enlistment', |
209 | + 'architecture': factory.make_name('arch'), |
210 | + 'power_type': power_type, |
211 | + 'mac_addresses': factory.make_mac_address(), |
212 | + 'power_parameters': json.dumps(power_parameters), |
213 | + }) |
214 | + self.assertEqual(http.client.BAD_REQUEST, response.status_code) |
215 | + |
216 | |
217 | class SimpleUserLoggedInEnlistmentAPITest(APITestCase.ForUser): |
218 | """Enlistment tests from the perspective of regular, non-admin users.""" |
219 | diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py |
220 | index c96eaa0..fc44545 100644 |
221 | --- a/src/maasserver/api/tests/test_machines.py |
222 | +++ b/src/maasserver/api/tests/test_machines.py |
223 | @@ -173,6 +173,33 @@ class TestMachinesAPI(APITestCase.ForUser): |
224 | {nic.mac_address for nic in machine.interface_set.all()}, |
225 | Equals(macs)) |
226 | |
227 | + def test_POST_creates_ipmi_machine_sets_mac_addresses_empty_no_arch(self): |
228 | + make_usable_architecture(self) |
229 | + hostname = factory.make_name('host') |
230 | + macs = { |
231 | + factory.make_mac_address() |
232 | + for _ in range(random.randint(1, 2)) |
233 | + } |
234 | + power_address = factory.make_ip_address() |
235 | + response = self.client.post( |
236 | + reverse('machines_handler'), |
237 | + { |
238 | + 'hostname': hostname, |
239 | + 'mac_addresses': macs, |
240 | + 'power_type': 'ipmi', |
241 | + 'power_parameters_power_address': power_address, |
242 | + }) |
243 | + self.assertEqual(http.client.OK, response.status_code) |
244 | + system_id = json.loads( |
245 | + response.content.decode(settings.DEFAULT_CHARSET))['system_id'] |
246 | + machine = Machine.objects.get(system_id=system_id) |
247 | + self.expectThat(machine.hostname, Equals(hostname)) |
248 | + self.expectThat( |
249 | + {nic.mac_address for nic in machine.interface_set.all()}, |
250 | + Equals(set())) |
251 | + self.assertEqual( |
252 | + power_address, machine.power_parameters['power_address']) |
253 | + |
254 | def test_POST_when_logged_in_creates_machine_in_declared_state(self): |
255 | # When a user enlists a machine, it goes into the New state. |
256 | # This will change once we start doing proper commissioning. |
257 | diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py |
258 | index 970e6a3..d769a88 100644 |
259 | --- a/src/maasserver/forms/__init__.py |
260 | +++ b/src/maasserver/forms/__init__.py |
261 | @@ -667,15 +667,18 @@ class NodeForm(MAASModelForm): |
262 | |
263 | |
264 | class MachineForm(NodeForm): |
265 | - def __init__(self, request=None, *args, **kwargs): |
266 | + def __init__(self, request=None, requires_arch=False, *args, **kwargs): |
267 | super(MachineForm, self).__init__(*args, **kwargs) |
268 | |
269 | # Even though it doesn't need it and doesn't use it, this form accepts |
270 | # a parameter named 'request' because it is used interchangingly |
271 | # with AdminMachineForm which actually uses this parameter. |
272 | + request = kwargs.get('request') |
273 | + if request is not None: |
274 | + self.data = request.data |
275 | instance = kwargs.get('instance') |
276 | |
277 | - self.set_up_architecture_field() |
278 | + self.set_up_architecture_field(requires_arch=requires_arch) |
279 | # We only want the license key field to render in the UI if the `OS` |
280 | # and `Release` fields are also present. |
281 | if self.has_owner: |
282 | @@ -688,7 +691,7 @@ class MachineForm(NodeForm): |
283 | self.fields['license_key'] = forms.CharField( |
284 | label="", required=False, widget=forms.HiddenInput()) |
285 | |
286 | - def set_up_architecture_field(self): |
287 | + def set_up_architecture_field(self, requires_arch=False): |
288 | """Create the `architecture` field. |
289 | |
290 | This needs to be done on the fly so that we can pass a dynamic list of |
291 | @@ -702,9 +705,11 @@ class MachineForm(NodeForm): |
292 | choices = list_architecture_choices(architectures) |
293 | invalid_arch_message = compose_invalid_choice_text( |
294 | 'architecture', choices) |
295 | + power_type = self.data.get('power_type', None) |
296 | self.fields['architecture'] = forms.ChoiceField( |
297 | - choices=choices, required=False, initial=default_arch, |
298 | - error_messages={'invalid_choice': invalid_arch_message}) |
299 | + choices=choices, required=(power_type != 'ipmi' or requires_arch), |
300 | + initial=default_arch, error_messages={ |
301 | + 'invalid_choice': invalid_arch_message}) |
302 | |
303 | def set_up_osystem_and_distro_series_fields(self, instance): |
304 | """Create the `osystem` and `distro_series` fields. |
305 | @@ -1205,7 +1210,8 @@ class WithMACAddressesMixin: |
306 | |
307 | def set_up_mac_addresses_field(self): |
308 | macs = [mac for mac in self.data.getlist('mac_addresses') if mac] |
309 | - self.fields['mac_addresses'] = MultipleMACAddressField(len(macs)) |
310 | + self.fields['mac_addresses'] = MultipleMACAddressField( |
311 | + len(macs), required=(self.data.get('power_type') != 'ipmi')) |
312 | self.data = self.data.copy() |
313 | self.data['mac_addresses'] = macs |
314 | |
315 | @@ -1255,7 +1261,19 @@ class WithMACAddressesMixin: |
316 | This implementation of `save` does not support the `commit` argument. |
317 | """ |
318 | node = super(WithMACAddressesMixin, self).save() |
319 | - for mac in self.cleaned_data['mac_addresses']: |
320 | + architecture = self.cleaned_data.get('architecture') |
321 | + power_type = self.cleaned_data.get('power_type') |
322 | + # If a new node with an IPMI BMC is created the user doesn't have |
323 | + # to specify the architecture or MAC addresses. Anonymous POST |
324 | + # on the machines API will find the machine the user created by |
325 | + # power address. If only the MAC address is given ignore it so the |
326 | + # machine boots into the enlistment environment and MAAS can capture |
327 | + # the architecture. |
328 | + if not architecture and power_type == 'ipmi': |
329 | + mac_addresses = [] |
330 | + else: |
331 | + mac_addresses = self.cleaned_data['mac_addresses'] |
332 | + for mac in mac_addresses: |
333 | mac_addresses_errors = [] |
334 | try: |
335 | node.add_physical_interface(mac) |
336 | diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py |
337 | index f284136..4bca075 100644 |
338 | --- a/src/maasserver/forms/tests/test_machine.py |
339 | +++ b/src/maasserver/forms/tests/test_machine.py |
340 | @@ -369,8 +369,8 @@ class TestAdminMachineForm(MAASServerTestCase): |
341 | 'cpu_count', |
342 | 'memory', |
343 | 'zone', |
344 | - 'power_type', |
345 | 'power_parameters', |
346 | + 'power_type', |
347 | 'pool', |
348 | ], |
349 | list(form.fields)) |
350 | diff --git a/src/maasserver/forms/tests/test_machinewithmacaddresses.py b/src/maasserver/forms/tests/test_machinewithmacaddresses.py |
351 | index 5e6b703..fefaaf9 100644 |
352 | --- a/src/maasserver/forms/tests/test_machinewithmacaddresses.py |
353 | +++ b/src/maasserver/forms/tests/test_machinewithmacaddresses.py |
354 | @@ -1,4 +1,4 @@ |
355 | -# Copyright 2014-2016 Canonical Ltd. This software is licensed under the |
356 | +# Copyright 2014-2018 Canonical Ltd. This software is licensed under the |
357 | # GNU Affero General Public License version 3 (see the file LICENSE). |
358 | |
359 | """Tests for `MachineWithMACAddressesForm`.""" |
360 | @@ -138,6 +138,24 @@ class MachineWithMACAddressesFormTest(MAASServerTestCase): |
361 | |
362 | self.assertTrue(form.is_valid()) |
363 | |
364 | + def test__mac_address_is_required(self): |
365 | + form = MachineWithMACAddressesForm( |
366 | + data=self.make_params(mac_addresses=[])) |
367 | + |
368 | + self.assertFalse(form.is_valid()) |
369 | + self.assertEqual(['mac_addresses'], list(form.errors)) |
370 | + self.assertEqual( |
371 | + ["This field is required."], |
372 | + form.errors['mac_addresses']) |
373 | + |
374 | + def test__no_architecture_or_mac_addresses_is_ok_for_ipmi(self): |
375 | + # No architecture or MAC addresses is okay for IPMI power types. |
376 | + params = self.make_params(mac_addresses=[]) |
377 | + params['architecture'] = None |
378 | + params['power_type'] = 'ipmi' |
379 | + form = MachineWithMACAddressesForm(data=params) |
380 | + self.assertTrue(form.is_valid()) |
381 | + |
382 | def test__save(self): |
383 | macs = ['aa:bb:cc:dd:ee:ff', '9a:bb:c3:33:e5:7f'] |
384 | form = MachineWithMACAddressesForm( |
385 | diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py |
386 | index 6f89875..3b7b9dd 100644 |
387 | --- a/src/maasserver/models/node.py |
388 | +++ b/src/maasserver/models/node.py |
389 | @@ -1567,12 +1567,6 @@ class Node(CleanSave, TimestampedModel): |
390 | ) |
391 | raise NodeStateViolation(error_text) |
392 | |
393 | - def clean_architecture(self): |
394 | - if self.architecture == '': |
395 | - raise ValidationError( |
396 | - {'architecture': |
397 | - ["Architecture must be defined for installable nodes."]}) |
398 | - |
399 | def clean_hostname_domain(self): |
400 | # If you set the hostname to a name with dots, that you mean for that |
401 | # to be the FQDN of the host. Se we check that a domain exists for |
402 | @@ -1611,8 +1605,6 @@ class Node(CleanSave, TimestampedModel): |
403 | self.clean_pool() |
404 | if self._state.has_changed('status'): |
405 | self.clean_status(self._state.get_old_value('status')) |
406 | - if self._state.has_changed('architecture'): |
407 | - self.clean_architecture() |
408 | if self._state.has_changed('boot_disk_id'): |
409 | self.clean_boot_disk() |
410 | if self._state.has_changed('boot_interface_id'): |
411 | @@ -2988,7 +2980,7 @@ class Node(CleanSave, TimestampedModel): |
412 | |
413 | def split_arch(self): |
414 | """Return architecture and subarchitecture, as a tuple.""" |
415 | - if self.architecture is None: |
416 | + if self.architecture is None or self.architecture == '': |
417 | return ("", "") |
418 | arch, subarch = self.architecture.split('/') |
419 | return (arch, subarch) |
420 | @@ -5509,10 +5501,6 @@ class Device(Node): |
421 | super(Device, self).__init__( |
422 | node_type=NODE_TYPE.DEVICE, *args, **kwargs) |
423 | |
424 | - def clean_architecture(self): |
425 | - # Devices aren't required to have a defined architecture |
426 | - pass |
427 | - |
428 | |
429 | class NodeGroupToRackController(CleanSave, Model): |
430 | """Store some of the old NodeGroup data so we can migrate it when a rack |
431 | diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py |
432 | index b5a96ef..55cd947 100644 |
433 | --- a/src/maasserver/models/tests/test_node.py |
434 | +++ b/src/maasserver/models/tests/test_node.py |
435 | @@ -1004,17 +1004,6 @@ class TestNode(MAASServerTestCase): |
436 | node = factory.make_Node() |
437 | self.assertThat(node.system_id, HasLength(6)) |
438 | |
439 | - def test_empty_architecture_rejected_for_type_node(self): |
440 | - self.assertRaises( |
441 | - ValidationError, |
442 | - factory.make_Node, node_type=NODE_TYPE.MACHINE, architecture='') |
443 | - |
444 | - def test_empty_architecture_rejected_for_type_rack_controller(self): |
445 | - self.assertRaises( |
446 | - ValidationError, |
447 | - factory.make_Node, node_type=NODE_TYPE.RACK_CONTROLLER, |
448 | - architecture='') |
449 | - |
450 | def test__set_zone(self): |
451 | zone = factory.make_Zone() |
452 | node = factory.make_Node() |
453 | @@ -3429,18 +3418,6 @@ class TestNode(MAASServerTestCase): |
454 | node = reload_object(node) |
455 | self.assertIsNone(node.status_expires) |
456 | |
457 | - def test_save_checks_architecture_for_installable_nodes(self): |
458 | - device = factory.make_Device(architecture='') |
459 | - device.node_type = factory.pick_enum( |
460 | - NODE_TYPE, but_not=[NODE_TYPE.DEVICE]) |
461 | - device._state._changed_fields['architecture'] = None |
462 | - exception = self.assertRaises( |
463 | - ValidationError, device.as_node().save) |
464 | - self.assertEqual( |
465 | - exception.message_dict, |
466 | - {'architecture': |
467 | - ['Architecture must be defined for installable nodes.']}) |
468 | - |
469 | def test_netboot_defaults_to_True(self): |
470 | node = Node() |
471 | self.assertTrue(node.netboot) |
472 | diff --git a/src/maasserver/static/js/angular/controllers/add_hardware.js b/src/maasserver/static/js/angular/controllers/add_hardware.js |
473 | index 5e1b0d8..e41cfb5 100644 |
474 | --- a/src/maasserver/static/js/angular/controllers/add_hardware.js |
475 | +++ b/src/maasserver/static/js/angular/controllers/add_hardware.js |
476 | @@ -25,6 +25,7 @@ angular.module('MAAS').controller('AddHardwareController', [ |
477 | $scope.pools = ResourcePoolsManager.getItems(); |
478 | $scope.domains = DomainsManager.getItems(); |
479 | $scope.architectures = GeneralManager.getData("architectures"); |
480 | + $scope.architectures.push("Choose an architecture"); |
481 | $scope.hwe_kernels = GeneralManager.getData("hwe_kernels"); |
482 | $scope.default_min_hwe_kernel = GeneralManager.getData( |
483 | "default_min_hwe_kernel"); |
484 | @@ -518,7 +519,8 @@ angular.module('MAAS').controller('AddHardwareController', [ |
485 | $scope.machine === null || |
486 | $scope.machine.zone === null || |
487 | $scope.machine.pool === null || |
488 | - $scope.machine.architecture === '' || |
489 | + ($scope.machine.architecture === 'Choose an architecture' && |
490 | + $scope.machine.power.type.name !== 'ipmi') || |
491 | $scope.machine.power.type === null || |
492 | $scope.invalidName($scope.machine)); |
493 | if(in_error) { |
494 | @@ -527,7 +529,8 @@ angular.module('MAAS').controller('AddHardwareController', [ |
495 | |
496 | // Make sure none of the mac addresses are in error. The first one |
497 | // cannot be blank the remaining are allowed to be empty. |
498 | - if($scope.machine.macs[0].mac === '' || |
499 | + if(($scope.machine.macs[0].mac === '' && |
500 | + $scope.machine.power.type.name !== 'ipmi') || |
501 | $scope.machine.macs[0].error) { |
502 | return true; |
503 | } |
504 | @@ -575,6 +578,11 @@ angular.module('MAAS').controller('AddHardwareController', [ |
505 | // the device. |
506 | $scope.error = null; |
507 | |
508 | + // Set the architecture to empty string if none was chosen. |
509 | + if($scope.machine.architecture === "Choose an architecture") { |
510 | + $scope.machine.architecture = ''; |
511 | + } |
512 | + |
513 | // Add the machine. |
514 | MachinesManager.create( |
515 | convertMachineToProtocol($scope.machine)).then( |
516 | diff --git a/src/maasserver/static/js/angular/controllers/tests/test_add_hardware.js b/src/maasserver/static/js/angular/controllers/tests/test_add_hardware.js |
517 | index 4f097c4..1f7b583 100644 |
518 | --- a/src/maasserver/static/js/angular/controllers/tests/test_add_hardware.js |
519 | +++ b/src/maasserver/static/js/angular/controllers/tests/test_add_hardware.js |
520 | @@ -105,7 +105,7 @@ describe("AddHardwareController", function() { |
521 | expect($scope.zones).toBe(ZonesManager.getItems()); |
522 | expect($scope.pools).toBe(ResourcePoolsManager.getItems()); |
523 | expect($scope.domains).toBe(DomainsManager.getItems()); |
524 | - expect($scope.architectures).toEqual([]); |
525 | + expect($scope.architectures).toEqual(['Choose an architecture']); |
526 | expect($scope.hwe_kernels).toEqual([]); |
527 | expect($scope.power_types).toEqual([]); |
528 | expect($scope.error).toBeNull(); |
529 | @@ -131,7 +131,7 @@ describe("AddHardwareController", function() { |
530 | architecture: '', |
531 | power: {type: makeName("power_type")} |
532 | }; |
533 | - $scope.show(); |
534 | + $scope.show() |
535 | |
536 | loadManagers_defer.resolve(); |
537 | loadItems_defer.resolve(); |
538 | @@ -139,7 +139,7 @@ describe("AddHardwareController", function() { |
539 | expect($scope.machine.architecture).toEqual(arch); |
540 | }); |
541 | |
542 | - it("initializes machine architecture with amd64 arch", function() { |
543 | + it("initializes machine arch with amd64 arch", function() { |
544 | var loadManagers_defer = $q.defer(); |
545 | var loadItems_defer = $q.defer(); |
546 | var controller = makeController(loadManagers_defer, loadItems_defer); |
547 | @@ -383,11 +383,11 @@ describe("AddHardwareController", function() { |
548 | expect($scope.machineHasError()).toBe(true); |
549 | }); |
550 | |
551 | - it("returns true if architecture is empty", function() { |
552 | + it("returns true if architecture is not chosen", function() { |
553 | var controller = makeControllerWithMachine(); |
554 | $scope.machine.zone = {}; |
555 | $scope.machine.pool = {}; |
556 | - $scope.machine.architecture = ''; |
557 | + $scope.machine.architecture = 'Choose an architecture'; |
558 | $scope.machine.power.type = {}; |
559 | $scope.machine.macs[0].mac = '00:11:22:33:44:55'; |
560 | $scope.machine.macs[0].error = false; |
561 | diff --git a/src/maasserver/static/partials/nodes-list.html b/src/maasserver/static/partials/nodes-list.html |
562 | index da3a9a2..c1cae77 100644 |
563 | --- a/src/maasserver/static/partials/nodes-list.html |
564 | +++ b/src/maasserver/static/partials/nodes-list.html |
565 | @@ -382,7 +382,7 @@ |
566 | <select name="architecture" id="architecture" |
567 | data-ng-model="machine.architecture" |
568 | data-ng-options="arch for arch in architectures"> |
569 | - <option value="" disabled>Choose an architecture</option> |
570 | + <option value="">Choose an architecture</option> |
571 | </select> |
572 | </div> |
573 | </div> |
574 | diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py |
575 | index 72fd395..183d74f 100644 |
576 | --- a/src/maasserver/websockets/handlers/tests/test_machine.py |
577 | +++ b/src/maasserver/websockets/handlers/tests/test_machine.py |
578 | @@ -1778,37 +1778,6 @@ class TestMachineHandler(MAASServerTestCase): |
579 | HandlerPermissionError, |
580 | handler.create, {}) |
581 | |
582 | - def test_create_raises_validation_error_for_missing_pxe_mac(self): |
583 | - user = factory.make_admin() |
584 | - handler = MachineHandler(user, {}) |
585 | - zone = factory.make_Zone() |
586 | - params = { |
587 | - "architecture": make_usable_architecture(self), |
588 | - "zone": { |
589 | - "name": zone.name, |
590 | - }, |
591 | - } |
592 | - error = self.assertRaises( |
593 | - HandlerValidationError, handler.create, params) |
594 | - self.assertThat(error.message_dict, Equals( |
595 | - {'mac_addresses': ['This field is required.']})) |
596 | - |
597 | - def test_create_raises_validation_error_for_missing_architecture(self): |
598 | - user = factory.make_admin() |
599 | - handler = MachineHandler(user, {}) |
600 | - zone = factory.make_Zone() |
601 | - params = { |
602 | - "pxe_mac": factory.make_mac_address(), |
603 | - "zone": { |
604 | - "name": zone.name, |
605 | - }, |
606 | - } |
607 | - error = self.assertRaises( |
608 | - HandlerValidationError, handler.create, params) |
609 | - self.assertThat(error.message_dict, Equals( |
610 | - {'architecture': [ |
611 | - 'Architecture must be defined for installable nodes.']})) |
612 | - |
613 | def test_create_creates_node(self): |
614 | user = factory.make_admin() |
615 | handler = MachineHandler(user, {}) |
UNIT TESTS and-mac- address- validation- with-ipmi- power-parameter s lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas
-b no-arch-
STATUS: SUCCESS ab35276f07cb986 76cbb928cd
COMMIT: fd05ed24e861c86