Merge ~adam-collard/maas:websocket-create-deployed-machine into maas:master
- Git
- lp:~adam-collard/maas
- websocket-create-deployed-machine
- Merge into master
Status: | Merged |
---|---|
Approved by: | Adam Collard |
Approved revision: | e038d4d1b1cfb3659236e242f96bc48845536fbf |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~adam-collard/maas:websocket-create-deployed-machine |
Merge into: | maas:master |
Diff against target: |
518 lines (+170/-128) 9 files modified
src/maasserver/api/machines.py (+13/-19) src/maasserver/forms/__init__.py (+20/-2) src/maasserver/forms/ephemeral.py (+2/-30) src/maasserver/forms/tests/test_ephemeral.py (+2/-58) src/maasserver/forms/tests/test_machine.py (+23/-2) src/maasserver/websockets/handlers/machine.py (+18/-10) src/maasserver/websockets/handlers/tests/test_machine.py (+28/-0) src/metadataserver/models/scriptset.py (+21/-6) src/metadataserver/models/tests/test_scriptset.py (+43/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
MAAS Lander | Needs Fixing | ||
Alberto Donato (community) | Approve | ||
Review via email: mp+407385@code.launchpad.net |
Commit message
Allow creating a machine as deployed via websocket handler
Description of the change
Adam Collard (adam-collard) : | # |
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b websocket-
STATUS: SUCCESS
COMMIT: 1633be5330aca16
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b websocket-
STATUS: FAILED
LOG: http://
COMMIT: 08c2fc6141b4500
MAAS Lander (maas-lander) wrote : | # |
LANDING
-b websocket-
STATUS: FAILED BUILD
LOG: http://
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b websocket-
STATUS: FAILED
LOG: http://
COMMIT: 6de8d707da29ec7
- e038d4d... by Adam Collard
-
Flaky test - we don't care about the order
Preview Diff
1 | diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py | |||
2 | index fe60680..5c234f2 100644 | |||
3 | --- a/src/maasserver/api/machines.py | |||
4 | +++ b/src/maasserver/api/machines.py | |||
5 | @@ -71,10 +71,7 @@ from maasserver.forms import ( | |||
6 | 71 | MachineForm, | 71 | MachineForm, |
7 | 72 | ) | 72 | ) |
8 | 73 | from maasserver.forms.clone import CloneForm | 73 | from maasserver.forms.clone import CloneForm |
13 | 74 | from maasserver.forms.ephemeral import ( | 74 | from maasserver.forms.ephemeral import CommissionForm |
10 | 75 | CommissionForm, | ||
11 | 76 | CreateScriptsForDeployedForm, | ||
12 | 77 | ) | ||
14 | 78 | from maasserver.forms.filesystem import ( | 75 | from maasserver.forms.filesystem import ( |
15 | 79 | MountNonStorageFilesystemForm, | 76 | MountNonStorageFilesystemForm, |
16 | 80 | UnmountNonStorageFilesystemForm, | 77 | UnmountNonStorageFilesystemForm, |
17 | @@ -1968,6 +1965,10 @@ class MachinesHandler(NodesHandler, PowersMixin): | |||
18 | 1968 | time out. Machines created by administrators will be commissioned | 1965 | time out. Machines created by administrators will be commissioned |
19 | 1969 | unless set to false. | 1966 | unless set to false. |
20 | 1970 | 1967 | ||
21 | 1968 | @param (boolean) "deployed" [required=false,formatting=true] Request | ||
22 | 1969 | the newly created machine to be created with status set to | ||
23 | 1970 | DEPLOYED. | ||
24 | 1971 | |||
25 | 1971 | @param (int) "enable_ssh" [required=false] Whether to enable SSH for | 1972 | @param (int) "enable_ssh" [required=false] Whether to enable SSH for |
26 | 1972 | the commissioning environment using the user's SSH key(s). '1' == True, | 1973 | the commissioning environment using the user's SSH key(s). '1' == True, |
27 | 1973 | '0' == False. | 1974 | '0' == False. |
28 | @@ -2011,21 +2012,14 @@ class MachinesHandler(NodesHandler, PowersMixin): | |||
29 | 2011 | request.data, "deployed", default=False, validator=StringBool | 2012 | request.data, "deployed", default=False, validator=StringBool |
30 | 2012 | ) | 2013 | ) |
31 | 2013 | machine = create_machine(request) | 2014 | machine = create_machine(request) |
47 | 2014 | if request.user.is_superuser: | 2015 | if request.user.is_superuser and commission and not deployed: |
48 | 2015 | if deployed: | 2016 | form = CommissionForm( |
49 | 2016 | form = CreateScriptsForDeployedForm( | 2017 | instance=machine, user=request.user, data=request.data |
50 | 2017 | instance=machine, | 2018 | ) |
51 | 2018 | data=request.data, | 2019 | # Silently ignore errors to prevent 500 errors. The commissioning |
52 | 2019 | ) | 2020 | # callbacks have their own logging. This fixes LP:1600328. |
53 | 2020 | form.save() | 2021 | if form.is_valid(): |
54 | 2021 | elif commission: | 2022 | machine = form.save() |
40 | 2022 | form = CommissionForm( | ||
41 | 2023 | instance=machine, user=request.user, data=request.data | ||
42 | 2024 | ) | ||
43 | 2025 | # Silently ignore errors to prevent 500 errors. The commissioning | ||
44 | 2026 | # callbacks have their own logging. This fixes LP:1600328. | ||
45 | 2027 | if form.is_valid(): | ||
46 | 2028 | machine = form.save() | ||
55 | 2029 | 2023 | ||
56 | 2030 | return machine | 2024 | return machine |
57 | 2031 | 2025 | ||
58 | diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py | |||
59 | index d4d35df..f5feb77 100644 | |||
60 | --- a/src/maasserver/forms/__init__.py | |||
61 | +++ b/src/maasserver/forms/__init__.py | |||
62 | @@ -748,6 +748,7 @@ class MachineForm(NodeForm): | |||
63 | 748 | "architecture", choices | 748 | "architecture", choices |
64 | 749 | ) | 749 | ) |
65 | 750 | required = requires_macs_and_architecture(self.data) or requires_arch | 750 | required = requires_macs_and_architecture(self.data) or requires_arch |
66 | 751 | self._need_boot_images = required | ||
67 | 751 | self.fields["architecture"] = forms.ChoiceField( | 752 | self.fields["architecture"] = forms.ChoiceField( |
68 | 752 | choices=choices, | 753 | choices=choices, |
69 | 753 | required=required, | 754 | required=required, |
70 | @@ -853,7 +854,7 @@ class MachineForm(NodeForm): | |||
71 | 853 | is_valid = super().is_valid() | 854 | is_valid = super().is_valid() |
72 | 854 | if not is_valid: | 855 | if not is_valid: |
73 | 855 | return False | 856 | return False |
75 | 856 | if len(list_all_usable_architectures()) == 0: | 857 | if self._need_boot_images and not list_all_usable_architectures(): |
76 | 857 | set_form_error(self, "architecture", NO_ARCHITECTURES_AVAILABLE) | 858 | set_form_error(self, "architecture", NO_ARCHITECTURES_AVAILABLE) |
77 | 858 | is_valid = False | 859 | is_valid = False |
78 | 859 | return is_valid | 860 | return is_valid |
79 | @@ -1172,6 +1173,23 @@ class AdminMachineForm(MachineForm, AdminNodeForm, WithPowerTypeMixin): | |||
80 | 1172 | cleaned_data = WithPowerTypeMixin.check_driver(self, cleaned_data) | 1173 | cleaned_data = WithPowerTypeMixin.check_driver(self, cleaned_data) |
81 | 1173 | return cleaned_data | 1174 | return cleaned_data |
82 | 1174 | 1175 | ||
83 | 1176 | def _setup_deployed_machine(self, machine): | ||
84 | 1177 | """Configure the Machine before it has been saved.""" | ||
85 | 1178 | from metadataserver.models import NodeKey | ||
86 | 1179 | from metadataserver.models.scriptset import ScriptSet | ||
87 | 1180 | |||
88 | 1181 | machine.status = NODE_STATUS.DEPLOYED | ||
89 | 1182 | # Foreign relations need to have an id to relate to, have to | ||
90 | 1183 | # save here | ||
91 | 1184 | machine.save() | ||
92 | 1185 | script_set = ScriptSet.objects.create_deployed_machine_script_set( | ||
93 | 1186 | machine | ||
94 | 1187 | ) | ||
95 | 1188 | machine.current_commissioning_script_set = script_set | ||
96 | 1189 | |||
97 | 1190 | # ensure a token is available for the machine | ||
98 | 1191 | NodeKey.objects.get_token_for_node(machine) | ||
99 | 1192 | |||
100 | 1175 | def save(self, *args, **kwargs): | 1193 | def save(self, *args, **kwargs): |
101 | 1176 | """Persist the node into the database.""" | 1194 | """Persist the node into the database.""" |
102 | 1177 | machine = super().save(commit=False) | 1195 | machine = super().save(commit=False) |
103 | @@ -1182,7 +1200,7 @@ class AdminMachineForm(MachineForm, AdminNodeForm, WithPowerTypeMixin): | |||
104 | 1182 | if pool: | 1200 | if pool: |
105 | 1183 | machine.pool = pool | 1201 | machine.pool = pool |
106 | 1184 | if self.cleaned_data.get("deployed"): | 1202 | if self.cleaned_data.get("deployed"): |
108 | 1185 | machine.status = NODE_STATUS.DEPLOYED | 1203 | self._setup_deployed_machine(machine) |
109 | 1186 | WithPowerTypeMixin.set_values(self, machine) | 1204 | WithPowerTypeMixin.set_values(self, machine) |
110 | 1187 | if kwargs.get("commit", True): | 1205 | if kwargs.get("commit", True): |
111 | 1188 | machine.save(*args, **kwargs) | 1206 | machine.save(*args, **kwargs) |
112 | diff --git a/src/maasserver/forms/ephemeral.py b/src/maasserver/forms/ephemeral.py | |||
113 | index 39d28e2..b736eaa 100644 | |||
114 | --- a/src/maasserver/forms/ephemeral.py | |||
115 | +++ b/src/maasserver/forms/ephemeral.py | |||
116 | @@ -21,8 +21,8 @@ from django.http import QueryDict | |||
117 | 21 | from maasserver.enum import NODE_STATUS | 21 | from maasserver.enum import NODE_STATUS |
118 | 22 | from maasserver.node_action import compile_node_actions | 22 | from maasserver.node_action import compile_node_actions |
119 | 23 | from maasserver.utils.forms import set_form_error | 23 | from maasserver.utils.forms import set_form_error |
122 | 24 | from metadataserver.enum import RESULT_TYPE, SCRIPT_TYPE | 24 | from metadataserver.enum import SCRIPT_TYPE |
123 | 25 | from metadataserver.models import NodeKey, Script, ScriptSet | 25 | from metadataserver.models import Script |
124 | 26 | 26 | ||
125 | 27 | 27 | ||
126 | 28 | class TestForm(Form): | 28 | class TestForm(Form): |
127 | @@ -322,31 +322,3 @@ class CommissionForm(TestForm): | |||
128 | 322 | ), | 322 | ), |
129 | 323 | ) | 323 | ) |
130 | 324 | return self.instance | 324 | return self.instance |
131 | 325 | |||
132 | 326 | |||
133 | 327 | class CreateScriptsForDeployedForm(Form): | ||
134 | 328 | def __init__(self, instance, **kwargs): | ||
135 | 329 | super().__init__(**kwargs) | ||
136 | 330 | self.instance = instance | ||
137 | 331 | |||
138 | 332 | def save(self): | ||
139 | 333 | node = self.instance | ||
140 | 334 | scripts = list( | ||
141 | 335 | Script.objects.filter( | ||
142 | 336 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
143 | 337 | default=True, | ||
144 | 338 | tags__contains=["deploy-info"], | ||
145 | 339 | ) | ||
146 | 340 | ) | ||
147 | 341 | script_set = ScriptSet.objects.create( | ||
148 | 342 | node=node, | ||
149 | 343 | result_type=RESULT_TYPE.COMMISSIONING, | ||
150 | 344 | requested_scripts=[script.name for script in scripts], | ||
151 | 345 | ) | ||
152 | 346 | for script in scripts: | ||
153 | 347 | script_set.add_pending_script(script) | ||
154 | 348 | node.current_commissioning_script_set = script_set | ||
155 | 349 | node.save() | ||
156 | 350 | # ensure a token is available for the machine | ||
157 | 351 | NodeKey.objects.get_token_for_node(node) | ||
158 | 352 | return script_set | ||
159 | diff --git a/src/maasserver/forms/tests/test_ephemeral.py b/src/maasserver/forms/tests/test_ephemeral.py | |||
160 | index a5c2e56..fde9528 100644 | |||
161 | --- a/src/maasserver/forms/tests/test_ephemeral.py | |||
162 | +++ b/src/maasserver/forms/tests/test_ephemeral.py | |||
163 | @@ -12,16 +12,11 @@ from maasserver.enum import ( | |||
164 | 12 | NODE_TYPE_CHOICES, | 12 | NODE_TYPE_CHOICES, |
165 | 13 | POWER_STATE, | 13 | POWER_STATE, |
166 | 14 | ) | 14 | ) |
172 | 15 | from maasserver.forms.ephemeral import ( | 15 | from maasserver.forms.ephemeral import CommissionForm, TestForm |
168 | 16 | CommissionForm, | ||
169 | 17 | CreateScriptsForDeployedForm, | ||
170 | 18 | TestForm, | ||
171 | 19 | ) | ||
173 | 20 | from maasserver.testing.factory import factory | 16 | from maasserver.testing.factory import factory |
174 | 21 | from maasserver.testing.testcase import MAASServerTestCase | 17 | from maasserver.testing.testcase import MAASServerTestCase |
175 | 22 | from maastesting.matchers import MockCalledOnceWith | 18 | from maastesting.matchers import MockCalledOnceWith |
178 | 23 | from metadataserver.enum import SCRIPT_STATUS, SCRIPT_TYPE | 19 | from metadataserver.enum import SCRIPT_TYPE |
177 | 24 | from metadataserver.models import NodeKey | ||
179 | 25 | 20 | ||
180 | 26 | 21 | ||
181 | 27 | class TestTestForm(MAASServerTestCase): | 22 | class TestTestForm(MAASServerTestCase): |
182 | @@ -988,54 +983,3 @@ class TestCommissionForm(MAASServerTestCase): | |||
183 | 988 | script_input={}, | 983 | script_input={}, |
184 | 989 | ), | 984 | ), |
185 | 990 | ) | 985 | ) |
186 | 991 | |||
187 | 992 | |||
188 | 993 | class TestCreateScriptsForDeployedForm(MAASServerTestCase): | ||
189 | 994 | def test_create_scripts(self): | ||
190 | 995 | script1 = factory.make_Script( | ||
191 | 996 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
192 | 997 | default=True, | ||
193 | 998 | tags=["foo", "deploy-info"], | ||
194 | 999 | ) | ||
195 | 1000 | script2 = factory.make_Script( | ||
196 | 1001 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
197 | 1002 | default=True, | ||
198 | 1003 | tags=["bar", "deploy-info"], | ||
199 | 1004 | ) | ||
200 | 1005 | # other scripts that are not matched | ||
201 | 1006 | factory.make_Script( # not for commissioning | ||
202 | 1007 | script_type=SCRIPT_TYPE.TESTING, | ||
203 | 1008 | default=True, | ||
204 | 1009 | tags=["bar", "deploy-info"], | ||
205 | 1010 | ) | ||
206 | 1011 | factory.make_Script( # not a builtin script | ||
207 | 1012 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
208 | 1013 | default=False, | ||
209 | 1014 | tags=["deploy-info"], | ||
210 | 1015 | ) | ||
211 | 1016 | factory.make_Script( # no deploy-info tag | ||
212 | 1017 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
213 | 1018 | default=True, | ||
214 | 1019 | tags=["foo"], | ||
215 | 1020 | ) | ||
216 | 1021 | node = factory.make_Node(status=NODE_STATUS.DEPLOYED) | ||
217 | 1022 | form = CreateScriptsForDeployedForm(instance=node, data={}) | ||
218 | 1023 | self.assertTrue(form.is_valid()) | ||
219 | 1024 | script_set = form.save() | ||
220 | 1025 | self.assertEqual(script_set.node, node) | ||
221 | 1026 | script_results = list(script_set.scriptresult_set.order_by("id")) | ||
222 | 1027 | self.assertEqual( | ||
223 | 1028 | [script_result.script for script_result in script_results], | ||
224 | 1029 | [script1, script2], | ||
225 | 1030 | ) | ||
226 | 1031 | for script_result in script_results: | ||
227 | 1032 | self.assertEqual(script_result.status, SCRIPT_STATUS.PENDING) | ||
228 | 1033 | self.assertIs(node.current_commissioning_script_set, script_set) | ||
229 | 1034 | |||
230 | 1035 | def test_create_node_token(self): | ||
231 | 1036 | node = factory.make_Node(status=NODE_STATUS.DEPLOYED) | ||
232 | 1037 | self.assertFalse(NodeKey.objects.filter(node=node).exists()) | ||
233 | 1038 | form = CreateScriptsForDeployedForm(instance=node, data={}) | ||
234 | 1039 | self.assertTrue(form.is_valid()) | ||
235 | 1040 | form.save() | ||
236 | 1041 | self.assertTrue(NodeKey.objects.filter(node=node).exists()) | ||
237 | diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py | |||
238 | index cef7cbe..bf61313 100644 | |||
239 | --- a/src/maasserver/forms/tests/test_machine.py | |||
240 | +++ b/src/maasserver/forms/tests/test_machine.py | |||
241 | @@ -24,6 +24,7 @@ from maasserver.testing.osystems import ( | |||
242 | 24 | patch_usable_osystems, | 24 | patch_usable_osystems, |
243 | 25 | ) | 25 | ) |
244 | 26 | from maasserver.testing.testcase import MAASServerTestCase | 26 | from maasserver.testing.testcase import MAASServerTestCase |
245 | 27 | from metadataserver.models import NodeKey | ||
246 | 27 | from provisioningserver.rpc.exceptions import ( | 28 | from provisioningserver.rpc.exceptions import ( |
247 | 28 | NoConnectionsAvailable, | 29 | NoConnectionsAvailable, |
248 | 29 | NoSuchOperatingSystem, | 30 | NoSuchOperatingSystem, |
249 | @@ -409,11 +410,9 @@ class TestAdminMachineForm(MAASServerTestCase): | |||
250 | 409 | 410 | ||
251 | 410 | def test_AdminMachineForm_new_machine_deployed(self): | 411 | def test_AdminMachineForm_new_machine_deployed(self): |
252 | 411 | hostname = factory.make_string() | 412 | hostname = factory.make_string() |
253 | 412 | arch = make_usable_architecture(self) | ||
254 | 413 | form = AdminMachineForm( | 413 | form = AdminMachineForm( |
255 | 414 | data={ | 414 | data={ |
256 | 415 | "hostname": hostname, | 415 | "hostname": hostname, |
257 | 416 | "architecture": arch, | ||
258 | 417 | "deployed": True, | 416 | "deployed": True, |
259 | 418 | }, | 417 | }, |
260 | 419 | ) | 418 | ) |
261 | @@ -518,3 +517,25 @@ class TestAdminMachineForm(MAASServerTestCase): | |||
262 | 518 | ] | 517 | ] |
263 | 519 | }, | 518 | }, |
264 | 520 | ) | 519 | ) |
265 | 520 | |||
266 | 521 | def test_AdminMachineForm_creates_scriptset_for_deployed(self): | ||
267 | 522 | hostname = factory.make_string() | ||
268 | 523 | form = AdminMachineForm( | ||
269 | 524 | data={ | ||
270 | 525 | "hostname": hostname, | ||
271 | 526 | "deployed": True, | ||
272 | 527 | }, | ||
273 | 528 | ) | ||
274 | 529 | machine = form.save() | ||
275 | 530 | self.assertIsNotNone(machine.current_commissioning_script_set) | ||
276 | 531 | |||
277 | 532 | def test_AdminMachineForm_creates_node_token_for_deployed(self): | ||
278 | 533 | hostname = factory.make_string() | ||
279 | 534 | form = AdminMachineForm( | ||
280 | 535 | data={ | ||
281 | 536 | "hostname": hostname, | ||
282 | 537 | "deployed": True, | ||
283 | 538 | }, | ||
284 | 539 | ) | ||
285 | 540 | machine = form.save() | ||
286 | 541 | self.assertTrue(NodeKey.objects.filter(node=machine).exists()) | ||
287 | diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py | |||
288 | index 3236647..4425c78 100644 | |||
289 | --- a/src/maasserver/websockets/handlers/machine.py | |||
290 | +++ b/src/maasserver/websockets/handlers/machine.py | |||
291 | @@ -22,6 +22,7 @@ from maasserver.enum import ( | |||
292 | 22 | from maasserver.exceptions import NodeActionError, NodeStateViolation | 22 | from maasserver.exceptions import NodeActionError, NodeStateViolation |
293 | 23 | from maasserver.forms import ( | 23 | from maasserver.forms import ( |
294 | 24 | AddPartitionForm, | 24 | AddPartitionForm, |
295 | 25 | AdminMachineForm, | ||
296 | 25 | AdminMachineWithMACAddressesForm, | 26 | AdminMachineWithMACAddressesForm, |
297 | 26 | CreateBcacheForm, | 27 | CreateBcacheForm, |
298 | 27 | CreateCacheSetForm, | 28 | CreateCacheSetForm, |
299 | @@ -246,6 +247,10 @@ class MachineHandler(NodeHandler): | |||
300 | 246 | edit_permission = NodePermission.admin | 247 | edit_permission = NodePermission.admin |
301 | 247 | delete_permission = NodePermission.admin | 248 | delete_permission = NodePermission.admin |
302 | 248 | 249 | ||
303 | 250 | def __init__(self, *args, **kwargs): | ||
304 | 251 | super().__init__(*args, **kwargs) | ||
305 | 252 | self._deployed = False | ||
306 | 253 | |||
307 | 249 | def get_queryset(self, for_list=False): | 254 | def get_queryset(self, for_list=False): |
308 | 250 | """Return `QuerySet` for devices only viewable by `user`.""" | 255 | """Return `QuerySet` for devices only viewable by `user`.""" |
309 | 251 | return Machine.objects.get_nodes( | 256 | return Machine.objects.get_nodes( |
310 | @@ -391,6 +396,8 @@ class MachineHandler(NodeHandler): | |||
311 | 391 | 396 | ||
312 | 392 | def get_form_class(self, action): | 397 | def get_form_class(self, action): |
313 | 393 | """Return the form class used for `action`.""" | 398 | """Return the form class used for `action`.""" |
314 | 399 | if action == "create" and self._deployed: | ||
315 | 400 | return AdminMachineForm | ||
316 | 394 | if action in ("create", "update"): | 401 | if action in ("create", "update"): |
317 | 395 | return AdminMachineWithMACAddressesForm | 402 | return AdminMachineWithMACAddressesForm |
318 | 396 | else: | 403 | else: |
319 | @@ -408,6 +415,7 @@ class MachineHandler(NodeHandler): | |||
320 | 408 | new_params["description"] = params.get("description") | 415 | new_params["description"] = params.get("description") |
321 | 409 | new_params["power_type"] = params.get("power_type") | 416 | new_params["power_type"] = params.get("power_type") |
322 | 410 | new_params["power_parameters"] = params.get("power_parameters") | 417 | new_params["power_parameters"] = params.get("power_parameters") |
323 | 418 | new_params["deployed"] = params.get("deployed") | ||
324 | 411 | if "zone" in params: | 419 | if "zone" in params: |
325 | 412 | new_params["zone"] = params["zone"]["name"] | 420 | new_params["zone"] = params["zone"]["name"] |
326 | 413 | if params.get("pool"): | 421 | if params.get("pool"): |
327 | @@ -427,18 +435,18 @@ class MachineHandler(NodeHandler): | |||
328 | 427 | return super().preprocess_form(action, new_params) | 435 | return super().preprocess_form(action, new_params) |
329 | 428 | 436 | ||
330 | 429 | def create(self, params): | 437 | def create(self, params): |
332 | 430 | """Create the object from params.""" | 438 | self._deployed = bool(params.get("deployed", False)) |
333 | 431 | data = super().create(params) | 439 | data = super().create(params) |
335 | 432 | node_obj = Node.objects.get(system_id=data["system_id"]) | 440 | if not self._deployed: |
336 | 441 | machine = Node.objects.get(system_id=data["system_id"]) | ||
337 | 442 | # Start the commissioning process right away, which has the | ||
338 | 443 | # desired side effect of initializing the node's power state. | ||
339 | 444 | d = machine.start_commissioning(self.user) | ||
340 | 445 | # Silently ignore errors to prevent tracebacks. The commissioning | ||
341 | 446 | # callbacks have their own logging. This fixes LP1600328. | ||
342 | 447 | d.addErrback(lambda _: None) | ||
343 | 433 | 448 | ||
352 | 434 | # Start the commissioning process right away, which has the | 449 | return data |
345 | 435 | # desired side effect of initializing the node's power state. | ||
346 | 436 | d = node_obj.start_commissioning(self.user) | ||
347 | 437 | # Silently ignore errors to prevent tracebacks. The commissioning | ||
348 | 438 | # callbacks have their own logging. This fixes LP1600328. | ||
349 | 439 | d.addErrback(lambda _: None) | ||
350 | 440 | |||
351 | 441 | return self.full_dehydrate(node_obj) | ||
353 | 442 | 450 | ||
354 | 443 | def update(self, params): | 451 | def update(self, params): |
355 | 444 | """Update the object from params.""" | 452 | """Update the object from params.""" |
356 | diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py | |||
357 | index 391127b..b13f953 100644 | |||
358 | --- a/src/maasserver/websockets/handlers/tests/test_machine.py | |||
359 | +++ b/src/maasserver/websockets/handlers/tests/test_machine.py | |||
360 | @@ -114,6 +114,7 @@ from metadataserver.enum import ( | |||
361 | 114 | SCRIPT_STATUS_FAILED, | 114 | SCRIPT_STATUS_FAILED, |
362 | 115 | SCRIPT_TYPE, | 115 | SCRIPT_TYPE, |
363 | 116 | ) | 116 | ) |
364 | 117 | from metadataserver.models.nodekey import NodeKey | ||
365 | 117 | from metadataserver.models.scriptset import get_status_from_qs | 118 | from metadataserver.models.scriptset import get_status_from_qs |
366 | 118 | from provisioningserver.refresh.node_info_scripts import ( | 119 | from provisioningserver.refresh.node_info_scripts import ( |
367 | 119 | LIST_MODALIASES_OUTPUT_NAME, | 120 | LIST_MODALIASES_OUTPUT_NAME, |
368 | @@ -2416,6 +2417,33 @@ class TestMachineHandler(MAASServerTestCase): | |||
369 | 2416 | ) | 2417 | ) |
370 | 2417 | self.assertThat(mock_start_commissioning, MockCalledOnceWith(user)) | 2418 | self.assertThat(mock_start_commissioning, MockCalledOnceWith(user)) |
371 | 2418 | 2419 | ||
372 | 2420 | def test_create_creates_deployed_node(self): | ||
373 | 2421 | user = factory.make_admin() | ||
374 | 2422 | handler = MachineHandler(user, {}, None) | ||
375 | 2423 | hostname = factory.make_name("hostname") | ||
376 | 2424 | description = factory.make_name("description") | ||
377 | 2425 | zone = factory.make_Zone() | ||
378 | 2426 | |||
379 | 2427 | mock_start_commissioning = self.patch( | ||
380 | 2428 | node_model, "start_commissioning" | ||
381 | 2429 | ) | ||
382 | 2430 | |||
383 | 2431 | created_node = handler.create( | ||
384 | 2432 | { | ||
385 | 2433 | "hostname": hostname, | ||
386 | 2434 | "description": description, | ||
387 | 2435 | "zone": {"name": zone.name}, | ||
388 | 2436 | "deployed": True, | ||
389 | 2437 | } | ||
390 | 2438 | ) | ||
391 | 2439 | # the commissioning process is not started | ||
392 | 2440 | mock_start_commissioning.assert_not_called() | ||
393 | 2441 | self.assertEqual(created_node["status"], "Deployed") | ||
394 | 2442 | node = Node.objects.get(system_id=created_node["system_id"]) | ||
395 | 2443 | self.assertIsNotNone(node.current_commissioning_script_set) | ||
396 | 2444 | self.assertTrue(NodeKey.objects.filter(node=node).exists()) | ||
397 | 2445 | self.assertEqual(node.status, NODE_STATUS.DEPLOYED) | ||
398 | 2446 | |||
399 | 2419 | def test_update_raise_permissions_error_for_non_admin(self): | 2447 | def test_update_raise_permissions_error_for_non_admin(self): |
400 | 2420 | user = factory.make_User() | 2448 | user = factory.make_User() |
401 | 2421 | node = factory.make_Node() | 2449 | node = factory.make_Node() |
402 | diff --git a/src/metadataserver/models/scriptset.py b/src/metadataserver/models/scriptset.py | |||
403 | index e9eed4a..2e8dea6 100644 | |||
404 | --- a/src/metadataserver/models/scriptset.py | |||
405 | +++ b/src/metadataserver/models/scriptset.py | |||
406 | @@ -227,6 +227,25 @@ class ScriptSetManager(Manager): | |||
407 | 227 | self._clean_old(node, RESULT_TYPE.INSTALLATION, script_set) | 227 | self._clean_old(node, RESULT_TYPE.INSTALLATION, script_set) |
408 | 228 | return script_set | 228 | return script_set |
409 | 229 | 229 | ||
410 | 230 | def create_deployed_machine_script_set(self, machine): | ||
411 | 231 | """Setup ScriptSet for a brown-field deployment. | ||
412 | 232 | |||
413 | 233 | Built-in scripts with deploy-info tag are queued up. | ||
414 | 234 | """ | ||
415 | 235 | scripts = list( | ||
416 | 236 | Script.objects.filter( | ||
417 | 237 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
418 | 238 | default=True, | ||
419 | 239 | tags__contains=["deploy-info"], | ||
420 | 240 | ) | ||
421 | 241 | ) | ||
422 | 242 | script_set = machine.scriptset_set.create( | ||
423 | 243 | requested_scripts=[script.name for script in scripts] | ||
424 | 244 | ) | ||
425 | 245 | for script in scripts: | ||
426 | 246 | script_set.add_pending_script(script) | ||
427 | 247 | return script_set | ||
428 | 248 | |||
429 | 230 | def _find_scripts(self, script_qs, hw_pairs, modaliases): | 249 | def _find_scripts(self, script_qs, hw_pairs, modaliases): |
430 | 231 | for script in script_qs: | 250 | for script in script_qs: |
431 | 232 | # If a script is not for specific hardware, it is always included | 251 | # If a script is not for specific hardware, it is always included |
432 | @@ -444,13 +463,10 @@ class ScriptSet(CleanSave, Model): | |||
433 | 444 | def add_pending_script(self, script, script_input=None): | 463 | def add_pending_script(self, script, script_input=None): |
434 | 445 | """Create and add a new ScriptResult for the given Script. | 464 | """Create and add a new ScriptResult for the given Script. |
435 | 446 | 465 | ||
437 | 447 | Creates a new ScriptResult for the given script and assoicates it with | 466 | Creates a new ScriptResult for the given script and associates it with |
438 | 448 | this ScriptSet. Raises a ValidationError if ParametersForm validation | 467 | this ScriptSet. Raises a ValidationError if ParametersForm validation |
439 | 449 | fails. | 468 | fails. |
440 | 450 | """ | 469 | """ |
441 | 451 | # Avoid circular dependencies. | ||
442 | 452 | from metadataserver.models import ScriptResult | ||
443 | 453 | |||
444 | 454 | if script_input is None: | 470 | if script_input is None: |
445 | 455 | script_input = {} | 471 | script_input = {} |
446 | 456 | form = ParametersForm( | 472 | form = ParametersForm( |
447 | @@ -461,8 +477,7 @@ class ScriptSet(CleanSave, Model): | |||
448 | 461 | if not form.is_valid(): | 477 | if not form.is_valid(): |
449 | 462 | raise ValidationError(form.errors) | 478 | raise ValidationError(form.errors) |
450 | 463 | for param in form.cleaned_data["input"]: | 479 | for param in form.cleaned_data["input"]: |
453 | 464 | ScriptResult.objects.create( | 480 | self.scriptresult_set.create( |
452 | 465 | script_set=self, | ||
454 | 466 | status=SCRIPT_STATUS.PENDING, | 481 | status=SCRIPT_STATUS.PENDING, |
455 | 467 | script=script, | 482 | script=script, |
456 | 468 | script_name=script.name, | 483 | script_name=script.name, |
457 | diff --git a/src/metadataserver/models/tests/test_scriptset.py b/src/metadataserver/models/tests/test_scriptset.py | |||
458 | index 68a5b9f..ad37878 100644 | |||
459 | --- a/src/metadataserver/models/tests/test_scriptset.py | |||
460 | +++ b/src/metadataserver/models/tests/test_scriptset.py | |||
461 | @@ -10,7 +10,7 @@ from unittest.mock import PropertyMock | |||
462 | 10 | from django.core.exceptions import ValidationError | 10 | from django.core.exceptions import ValidationError |
463 | 11 | from django.db.models import Q | 11 | from django.db.models import Q |
464 | 12 | 12 | ||
466 | 13 | from maasserver.enum import NODE_TYPE | 13 | from maasserver.enum import NODE_STATUS, NODE_TYPE |
467 | 14 | from maasserver.exceptions import NoScriptsFound | 14 | from maasserver.exceptions import NoScriptsFound |
468 | 15 | from maasserver.models import Config, Event, EventType, Node | 15 | from maasserver.models import Config, Event, EventType, Node |
469 | 16 | from maasserver.preseed import CURTIN_INSTALL_LOG | 16 | from maasserver.preseed import CURTIN_INSTALL_LOG |
470 | @@ -1012,6 +1012,48 @@ class TestScriptSetManager(MAASServerTestCase): | |||
471 | 1012 | ).all(), | 1012 | ).all(), |
472 | 1013 | ) | 1013 | ) |
473 | 1014 | 1014 | ||
474 | 1015 | def test_create_deployed_machine_script_set_scripts(self): | ||
475 | 1016 | # Avoid builtins muddying the test | ||
476 | 1017 | Script.objects.all().delete() | ||
477 | 1018 | script1 = factory.make_Script( | ||
478 | 1019 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
479 | 1020 | default=True, | ||
480 | 1021 | tags=["foo", "deploy-info"], | ||
481 | 1022 | ) | ||
482 | 1023 | script2 = factory.make_Script( | ||
483 | 1024 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
484 | 1025 | default=True, | ||
485 | 1026 | tags=["bar", "deploy-info"], | ||
486 | 1027 | ) | ||
487 | 1028 | # other scripts that are not matched | ||
488 | 1029 | factory.make_Script( # not for commissioning | ||
489 | 1030 | script_type=SCRIPT_TYPE.TESTING, | ||
490 | 1031 | default=True, | ||
491 | 1032 | tags=["bar", "deploy-info"], | ||
492 | 1033 | ) | ||
493 | 1034 | factory.make_Script( # not a builtin script | ||
494 | 1035 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
495 | 1036 | default=False, | ||
496 | 1037 | tags=["deploy-info"], | ||
497 | 1038 | ) | ||
498 | 1039 | factory.make_Script( # no deploy-info tag | ||
499 | 1040 | script_type=SCRIPT_TYPE.COMMISSIONING, | ||
500 | 1041 | default=True, | ||
501 | 1042 | tags=["foo"], | ||
502 | 1043 | ) | ||
503 | 1044 | node = factory.make_Node(status=NODE_STATUS.DEPLOYED) | ||
504 | 1045 | script_set = ScriptSet.objects.create_deployed_machine_script_set(node) | ||
505 | 1046 | self.assertItemsEqual( | ||
506 | 1047 | [ | ||
507 | 1048 | (script_result.script, script_result.status) | ||
508 | 1049 | for script_result in script_set.scriptresult_set.all() | ||
509 | 1050 | ], | ||
510 | 1051 | [ | ||
511 | 1052 | (script1, SCRIPT_STATUS.PENDING), | ||
512 | 1053 | (script2, SCRIPT_STATUS.PENDING), | ||
513 | 1054 | ], | ||
514 | 1055 | ) | ||
515 | 1056 | |||
516 | 1015 | 1057 | ||
517 | 1016 | class TestScriptSet(MAASServerTestCase): | 1058 | class TestScriptSet(MAASServerTestCase): |
518 | 1017 | """Test the ScriptSet model.""" | 1059 | """Test the ScriptSet model.""" |
nice!
just one question inline