Merge ~ltrager/maas:no-arch-and-mac-address-validation-with-ipmi-power-parameters into maas:master

Proposed by Lee Trager
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)
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

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: fd05ed24e861c86ab35276f07cb98676cbb928cd

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e3bc53406b3c6188943bdd710ecb77ce915e127e

review: Approve
1e112aa... by Lee Trager

Merge branch 'master' into no-arch-and-mac-address-validation-with-ipmi-power-parameters

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 1e112aa3f60f1e132e4774a1edbd0898f8d2b397

review: Approve
Revision history for this message
Andres Rodriguez (andreserl) wrote :

Please see inline.

review: Needs Information
e309910... by Lee Trager

Merge branch 'master' into no-arch-and-mac-address-validation-with-ipmi-power-parameters

Revision history for this message
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.

Revision history for this message
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://code.launchpad.net/~ltrager/maas/+git/maas/+merge/348256
> You are reviewing the proposed merge of ~ltrager/maas:no-arch-and-mac-
> address-validation-with-ipmi-power-parameters into maas:master.
>

--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: e309910f2a81d542c9bf26477797b92db091046c

review: Approve
Revision history for this message
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-parameters

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/3277/console
COMMIT: e73bce441a7296fc77441b0c7c5b2dfb0dd67956

review: Needs Fixing
123adab... by Lee Trager

Merge branch 'master' into no-arch-and-mac-address-validation-with-ipmi-power-parameters

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b no-arch-and-mac-address-validation-with-ipmi-power-parameters lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 123adab60c94f28c336c3088382fd54d4bb37e3c

review: Approve
Revision history for this message
Andres Rodriguez (andreserl) wrote :

lgtm!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
2index 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
135diff --git a/src/maasserver/api/tests/test_enlistment.py b/src/maasserver/api/tests/test_enlistment.py
136index 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."""
219diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
220index 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.
257diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
258index 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)
336diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py
337index 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))
350diff --git a/src/maasserver/forms/tests/test_machinewithmacaddresses.py b/src/maasserver/forms/tests/test_machinewithmacaddresses.py
351index 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(
385diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
386index 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
431diff --git a/src/maasserver/models/tests/test_node.py b/src/maasserver/models/tests/test_node.py
432index 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)
472diff --git a/src/maasserver/static/js/angular/controllers/add_hardware.js b/src/maasserver/static/js/angular/controllers/add_hardware.js
473index 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(
516diff --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
517index 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;
561diff --git a/src/maasserver/static/partials/nodes-list.html b/src/maasserver/static/partials/nodes-list.html
562index 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>
574diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
575index 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, {})

Subscribers

People subscribed via source and target branches