MAAS

Merge ~newell-jensen/maas:lp1548402 into maas:master

  1. Git
  2. lp:~newell-jensen/maas
  3. lp1548402
  4. Merge into master
Proposed by Newell Jensen
Status: Merged
Approved by: Newell Jensen
Approved revision: 9a16d511e3f3ed9b56fd2890e85fceed1c711b61
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~newell-jensen/maas:lp1548402
Merge into: maas:master
Diff against target: 352 lines (+108/-11)
5 files modified
src/maasserver/api/machines.py (+7/-0)
src/maasserver/api/tests/test_machine.py (+34/-1)
src/maasserver/node_action.py (+7/-0)
src/maasserver/tests/test_node_action.py (+58/-10)
src/maasserver/websockets/handlers/tests/test_machine.py (+2/-0)
Reviewer Review Type Date Requested Status
Andres Rodriguez (community) Approve
Review via email: mp+327766@code.launchpad.net

Commit message

LP: #1548402 - Handle errors in preseeds

Ensure that MAAS prevents starting the deployment of a machine if the preseeds fail to render.

To post a comment you must log in.
Revision history for this message
Andres Rodriguez (andreserl) wrote :

lgtm!

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

LANDING
-b lp1548402 lp:~newell-jensen/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci-jenkins.internal:8080/job/maas/job/branch-tester/124/console

~newell-jensen/maas:lp1548402 updated
9a16d51... by Newell Jensen

Add fix for failing test in websocket handler machine_test

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 58d9dfd..dfca107 100644
3--- a/src/maasserver/api/machines.py
4+++ b/src/maasserver/api/machines.py
5@@ -443,6 +443,13 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
6 form.save()
7 else:
8 raise MAASAPIValidationError(form.errors)
9+ # Check that the curtin preseeds renders correctly.
10+ try:
11+ get_curtin_merged_config(machine)
12+ except Exception as e:
13+ raise MAASAPIBadRequest(
14+ "Failed to render preseed: %s" % e)
15+
16 return self.power_on(request, system_id)
17
18 @operation(idempotent=False)
19diff --git a/src/maasserver/api/tests/test_machine.py b/src/maasserver/api/tests/test_machine.py
20index 4f9fa5d..9d4abf0 100644
21--- a/src/maasserver/api/tests/test_machine.py
22+++ b/src/maasserver/api/tests/test_machine.py
23@@ -1,4 +1,4 @@
24-# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
25+# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
26 # GNU Affero General Public License version 3 (see the file LICENSE).
27
28 """Tests for the Machine API."""
29@@ -326,6 +326,7 @@ class TestMachineAPI(APITestCase.ForUser):
30
31 def test_POST_deploy_sets_osystem_and_distro_series(self):
32 self.patch(node_module.Node, "_start")
33+ self.patch(machines_module, "get_curtin_merged_config")
34 machine = factory.make_Node(
35 owner=self.user, interface=True,
36 power_type='manual',
37@@ -367,6 +368,7 @@ class TestMachineAPI(APITestCase.ForUser):
38
39 def test_POST_deploy_sets_license_key(self):
40 self.patch(node_module.Node, "_start")
41+ self.patch(machines_module, "get_curtin_merged_config")
42 machine = factory.make_Node(
43 owner=self.user, interface=True,
44 power_type='manual',
45@@ -415,6 +417,7 @@ class TestMachineAPI(APITestCase.ForUser):
46
47 def test_POST_deploy_sets_default_distro_series(self):
48 self.patch(node_module.Node, "_start")
49+ self.patch(machines_module, "get_curtin_merged_config")
50 machine = factory.make_Node(
51 owner=self.user, interface=True,
52 power_type='manual',
53@@ -432,6 +435,7 @@ class TestMachineAPI(APITestCase.ForUser):
54
55 def test_POST_deploy_works_if_series_already_set(self):
56 self.patch(node_module.Node, "_start")
57+ self.patch(machines_module, "get_curtin_merged_config")
58 osystem = Config.objects.get_config('default_osystem')
59 distro_series = Config.objects.get_config('default_distro_series')
60 make_usable_osystem(
61@@ -467,6 +471,26 @@ class TestMachineAPI(APITestCase.ForUser):
62 ),
63 (response.status_code, json_load_bytes(response.content)))
64
65+ def test_POST_deploy_fails_when_preseed_not_rendered(self):
66+ mock_get_curtin_merged_config = self.patch(
67+ machines_module, "get_curtin_merged_config")
68+ mock_get_curtin_merged_config.side_effect = Exception('error')
69+ osystem = Config.objects.get_config('default_osystem')
70+ distro_series = Config.objects.get_config('default_distro_series')
71+ make_usable_osystem(
72+ self, osystem_name=osystem, releases=[distro_series])
73+ machine = factory.make_Node(
74+ owner=self.user, interface=True,
75+ status=NODE_STATUS.ALLOCATED,
76+ power_type='manual',
77+ distro_series=distro_series,
78+ osystem=osystem,
79+ architecture=make_usable_architecture(self))
80+ response = self.client.post(
81+ self.get_machine_uri(machine), {'op': 'deploy'})
82+ self.assertEqual(http.client.BAD_REQUEST, response.status_code)
83+ self.assertEqual(b"Failed to render preseed: error", response.content)
84+
85 def test_POST_deploy_validates_hwe_kernel_with_default_distro_series(self):
86 architecture = make_usable_architecture(self, subarch_name="generic")
87 machine = factory.make_Node(
88@@ -495,6 +519,7 @@ class TestMachineAPI(APITestCase.ForUser):
89
90 def test_POST_deploy_may_be_repeated(self):
91 self.patch(node_module.Node, "_start")
92+ self.patch(machines_module, "get_curtin_merged_config")
93 machine = factory.make_Node(
94 owner=self.user, interface=True,
95 power_type='manual',
96@@ -515,6 +540,7 @@ class TestMachineAPI(APITestCase.ForUser):
97 node_module.RackControllerManager, "filter_by_url_accessible"
98 ).return_value = [rack_controller]
99 self.patch(node_module.Node, "_power_control_node")
100+ self.patch(machines_module, "get_curtin_merged_config")
101 machine = factory.make_Node(
102 owner=self.user, interface=True,
103 power_type='virsh', architecture=make_usable_architecture(self),
104@@ -536,6 +562,7 @@ class TestMachineAPI(APITestCase.ForUser):
105
106 def test_POST_deploy_passes_comment(self):
107 self.patch(node_module.Node, "_start")
108+ self.patch(machines_module, "get_curtin_merged_config")
109 rack_controller = factory.make_RackController()
110 machine = factory.make_Node(
111 owner=self.user, interface=True,
112@@ -565,6 +592,7 @@ class TestMachineAPI(APITestCase.ForUser):
113 osystem = make_usable_osystem(self)
114 distro_series = osystem['default_release']
115 machine_start = self.patch(node_module.Machine, 'start')
116+ self.patch(machines_module, "get_curtin_merged_config")
117 machine_start.return_value = False
118 self.client.post(
119 self.get_machine_uri(machine), {
120@@ -578,6 +606,7 @@ class TestMachineAPI(APITestCase.ForUser):
121 def test_POST_deploy_doesnt_reset_power_options_bug_1569102(self):
122 self.become_admin()
123 self.patch(node_module.Node, "_start")
124+ self.patch(machines_module, "get_curtin_merged_config")
125 rack_controller = factory.make_RackController()
126 machine = factory.make_Node(
127 owner=self.user, interface=True,
128@@ -600,6 +629,7 @@ class TestMachineAPI(APITestCase.ForUser):
129
130 def test_POST_deploy_allocates_ready_machines(self):
131 self.patch(node_module.Node, "_start")
132+ self.patch(machines_module, "get_curtin_merged_config")
133 machine = factory.make_Node(
134 status=NODE_STATUS.READY, interface=True,
135 power_type='manual',
136@@ -631,6 +661,7 @@ class TestMachineAPI(APITestCase.ForUser):
137
138 def test_POST_deploy_passes_agent_name(self):
139 self.patch(node_module.Node, "_start")
140+ self.patch(machines_module, "get_curtin_merged_config")
141 machine = factory.make_Node(
142 status=NODE_STATUS.READY, interface=True,
143 power_type='manual',
144@@ -650,6 +681,7 @@ class TestMachineAPI(APITestCase.ForUser):
145
146 def test_POST_deploy_passes_comment_on_acquire(self):
147 self.patch(node_module.Node, "_start")
148+ self.patch(machines_module, "get_curtin_merged_config")
149 machine_method = self.patch(node_module.Machine, 'acquire')
150 machine = factory.make_Node(
151 status=NODE_STATUS.READY, owner=self.user, interface=True,
152@@ -673,6 +705,7 @@ class TestMachineAPI(APITestCase.ForUser):
153
154 def test_POST_deploy_passes_bridge_settings(self):
155 self.patch(node_module.Node, "_start")
156+ self.patch(machines_module, "get_curtin_merged_config")
157 machine_method = self.patch(node_module.Machine, 'acquire')
158 machine = factory.make_Node(
159 status=NODE_STATUS.READY, owner=self.user, interface=True,
160diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
161index fc48bb0..564abd5 100644
162--- a/src/maasserver/node_action.py
163+++ b/src/maasserver/node_action.py
164@@ -44,6 +44,7 @@ from maasserver.node_status import (
165 is_failed_status,
166 NON_MONITORED_STATUSES,
167 )
168+from maasserver.preseed import get_curtin_config
169 from maasserver.utils.orm import post_commit_do
170 from maasserver.utils.osystems import (
171 validate_hwe_kernel,
172@@ -346,6 +347,12 @@ class Deploy(NodeAction):
173 raise NodeActionError(e)
174
175 try:
176+ get_curtin_config(self.node)
177+ except Exception as e:
178+ raise NodeActionError(
179+ "Failed to retrieve curtin config: %s" % e)
180+
181+ try:
182 self.node.start(self.user)
183 except StaticIPAddressExhaustion:
184 raise NodeActionError(
185diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
186index 4f0c0bd..bad38a2 100644
187--- a/src/maasserver/tests/test_node_action.py
188+++ b/src/maasserver/tests/test_node_action.py
189@@ -49,6 +49,7 @@ from maasserver.node_action import (
190 SetZone,
191 Test,
192 )
193+import maasserver.node_action as node_action_module
194 from maasserver.node_status import (
195 MONITORED_STATUSES,
196 NODE_TESTING_RESET_READY_TRANSITIONS,
197@@ -499,10 +500,27 @@ class TestDeployAction(MAASServerTestCase):
198 node = factory.make_Node(
199 interface=True, status=NODE_STATUS.ALLOCATED,
200 power_type='manual', owner=user)
201- node_start = self.patch(node, 'start')
202+ mock_get_curtin_config = self.patch(
203+ node_action_module, 'get_curtin_config')
204+ mock_node_start = self.patch(node, 'start')
205 Deploy(node, user).execute()
206- self.assertThat(
207- node_start, MockCalledOnceWith(user))
208+ self.expectThat(
209+ mock_get_curtin_config, MockCalledOnceWith(node))
210+ self.expectThat(
211+ mock_node_start, MockCalledOnceWith(user))
212+
213+ def test_Deploy_raises_NodeActionError_for_no_curtin_config(self):
214+ user = factory.make_User()
215+ node = factory.make_Node(
216+ interface=True, status=NODE_STATUS.ALLOCATED,
217+ power_type='manual', owner=user)
218+ mock_get_curtin_config = self.patch(
219+ node_action_module, 'get_curtin_config')
220+ mock_get_curtin_config.side_effect = NodeActionError('error')
221+ error = self.assertRaises(
222+ NodeActionError, Deploy(node, user).execute)
223+ self.assertEqual(
224+ "Failed to retrieve curtin config: error", str(error))
225
226 def test_Deploy_raises_NodeActionError_for_invalid_os(self):
227 user = factory.make_User()
228@@ -527,7 +545,9 @@ class TestDeployAction(MAASServerTestCase):
229 node = factory.make_Node(
230 interface=True, status=NODE_STATUS.ALLOCATED,
231 power_type='manual', owner=user)
232- self.patch(node, 'start')
233+ mock_get_curtin_config = self.patch(
234+ node_action_module, 'get_curtin_config')
235+ mock_node_start = self.patch(node, 'start')
236 osystem = make_usable_osystem(self)
237 os_name = osystem["name"]
238 release_name = osystem["releases"][0]["name"]
239@@ -536,6 +556,10 @@ class TestDeployAction(MAASServerTestCase):
240 "distro_series": release_name
241 }
242 Deploy(node, user).execute(**extra)
243+ self.expectThat(
244+ mock_get_curtin_config, MockCalledOnceWith(node))
245+ self.expectThat(
246+ mock_node_start, MockCalledOnceWith(user))
247 self.expectThat(node.osystem, Equals(os_name))
248 self.expectThat(
249 node.distro_series, Equals(release_name))
250@@ -545,7 +569,9 @@ class TestDeployAction(MAASServerTestCase):
251 node = factory.make_Node(
252 interface=True, status=NODE_STATUS.ALLOCATED,
253 power_type='manual', owner=user)
254- self.patch(node, 'start')
255+ mock_get_curtin_config = self.patch(
256+ node_action_module, 'get_curtin_config')
257+ mock_node_start = self.patch(node, 'start')
258 osystem = make_usable_osystem(self)
259 os_name = osystem["name"]
260 release_name = osystem["releases"][0]["name"]
261@@ -554,6 +580,10 @@ class TestDeployAction(MAASServerTestCase):
262 "distro_series": release_name + '*'
263 }
264 Deploy(node, user).execute(**extra)
265+ self.expectThat(
266+ mock_get_curtin_config, MockCalledOnceWith(node))
267+ self.expectThat(
268+ mock_node_start, MockCalledOnceWith(user))
269 self.expectThat(node.osystem, Equals(os_name))
270 self.expectThat(
271 node.distro_series, Equals(release_name))
272@@ -563,12 +593,18 @@ class TestDeployAction(MAASServerTestCase):
273 node = factory.make_Node(
274 interface=True, status=NODE_STATUS.ALLOCATED,
275 power_type='manual', owner=user)
276- self.patch(node, 'start')
277+ mock_get_curtin_config = self.patch(
278+ node_action_module, 'get_curtin_config')
279+ mock_node_start = self.patch(node, 'start')
280 osystem = make_osystem_with_releases(self)
281 extra = {
282 "distro_series": osystem["releases"][0]["name"],
283 }
284 Deploy(node, user).execute(**extra)
285+ self.expectThat(
286+ mock_get_curtin_config, MockCalledOnceWith(node))
287+ self.expectThat(
288+ mock_node_start, MockCalledOnceWith(user))
289 self.expectThat(node.osystem, Equals(""))
290 self.expectThat(node.distro_series, Equals(""))
291
292@@ -577,24 +613,36 @@ class TestDeployAction(MAASServerTestCase):
293 node = factory.make_Node(
294 interface=True, status=NODE_STATUS.ALLOCATED,
295 power_type='manual', owner=user)
296- self.patch(node, 'start')
297+ mock_get_curtin_config = self.patch(
298+ node_action_module, 'get_curtin_config')
299+ mock_node_start = self.patch(node, 'start')
300 osystem = make_osystem_with_releases(self)
301 extra = {
302 "osystem": osystem["name"],
303 }
304 Deploy(node, user).execute(**extra)
305+ self.expectThat(
306+ mock_get_curtin_config, MockCalledOnceWith(node))
307+ self.expectThat(
308+ mock_node_start, MockCalledOnceWith(user))
309 self.expectThat(node.osystem, Equals(""))
310 self.expectThat(node.distro_series, Equals(""))
311
312 def test_Deploy_allocates_node_if_node_not_already_allocated(self):
313 user = factory.make_User()
314 node = factory.make_Node(status=NODE_STATUS.READY, with_boot_disk=True)
315- self.patch(node, 'start')
316+ mock_get_curtin_config = self.patch(
317+ node_action_module, 'get_curtin_config')
318+ mock_node_start = self.patch(node, 'start')
319 action = Deploy(node, user)
320 action.execute()
321
322- self.assertEqual(user, node.owner)
323- self.assertEqual(NODE_STATUS.ALLOCATED, node.status)
324+ self.expectThat(
325+ mock_get_curtin_config, MockCalledOnceWith(node))
326+ self.expectThat(
327+ mock_node_start, MockCalledOnceWith(user))
328+ self.expectThat(user, Equals(node.owner))
329+ self.expectThat(NODE_STATUS.ALLOCATED, Equals(node.status))
330
331
332 class TestDeployActionTransactional(MAASTransactionServerTestCase):
333diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
334index 80ab253..0732ba1 100644
335--- a/src/maasserver/websockets/handlers/tests/test_machine.py
336+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
337@@ -56,6 +56,7 @@ from maasserver.models.partition import (
338 PARTITION_ALIGNMENT_SIZE,
339 )
340 from maasserver.node_action import compile_node_actions
341+import maasserver.node_action as node_action_module
342 from maasserver.testing.architecture import make_usable_architecture
343 from maasserver.testing.factory import factory
344 from maasserver.testing.osystems import make_usable_osystem
345@@ -2059,6 +2060,7 @@ class TestMachineHandler(MAASServerTestCase):
346 self.patch(Machine, 'on_network').return_value = True
347 node = factory.make_Node(status=NODE_STATUS.ALLOCATED, owner=user)
348 self.patch(Machine, "_start").return_value = None
349+ self.patch(node_action_module, 'get_curtin_config')
350 osystem = make_usable_osystem(self)
351 handler = MachineHandler(user, {})
352 handler.action({

Subscribers

People subscribed via source and target branches