Merge ~mpontillo/maas:install-kvm into maas:master
- Git
- lp:~mpontillo/maas
- install-kvm
- Merge into master
Proposed by
Mike Pontillo
Status: | Merged |
---|---|
Approved by: | Mike Pontillo |
Approved revision: | 30d78646ac17054d9f3a8b52ad58c5b98733a463 |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~mpontillo/maas:install-kvm |
Merge into: | maas:master |
Diff against target: |
851 lines (+417/-36) 16 files modified
src/maasserver/api/machines.py (+53/-19) src/maasserver/api/tests/test_machines.py (+36/-0) src/maasserver/forms/__init__.py (+6/-0) src/maasserver/forms/pods.py (+8/-2) src/maasserver/forms/tests/test_machine.py (+2/-0) src/maasserver/migrations/builtin/maasserver/0173_add_node_install_kvm.py (+23/-0) src/maasserver/models/node.py (+6/-0) src/maasserver/testing/factory.py (+16/-4) src/maasserver/websockets/handlers/device.py (+1/-0) src/maasserver/websockets/handlers/machine.py (+1/-0) src/metadataserver/api.py (+14/-1) src/metadataserver/api_twisted.py (+62/-6) src/metadataserver/tests/test_api.py (+11/-0) src/metadataserver/tests/test_api_twisted.py (+95/-2) src/metadataserver/tests/test_vendor_data.py (+22/-1) src/metadataserver/vendor_data.py (+61/-1) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Lee Trager (community) | Approve | ||
Blake Rouse (community) | Approve | ||
Andres Rodriguez (community) | Approve | ||
Review via email: mp+355115@code.launchpad.net |
Commit message
Allow KVM pod deployment in MAAS using install_kvm option.
Description of the change
To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote : | # |
Yes, agent_name is cleared on release (I checked that yesterday).
Revision history for this message
MAAS Lander (maas-lander) wrote : | # |
LANDING
-b install-kvm lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas
STATUS: FAILED BUILD
LOG: http://
~mpontillo/maas:install-kvm
updated
- 30d7864... by Mike Pontillo
-
Fix test failures.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py |
2 | index 9a95cc8..12cbc69 100644 |
3 | --- a/src/maasserver/api/machines.py |
4 | +++ b/src/maasserver/api/machines.py |
5 | @@ -8,6 +8,7 @@ __all__ = [ |
6 | "get_storage_layout_params", |
7 | ] |
8 | |
9 | +from collections import namedtuple |
10 | import json |
11 | import re |
12 | |
13 | @@ -193,6 +194,19 @@ DISPLAYED_ANON_MACHINE_FIELDS = ( |
14 | ) |
15 | |
16 | |
17 | +AllocationOptions = namedtuple( |
18 | + 'AllocationOptions', ( |
19 | + 'agent_name', |
20 | + 'bridge_all', |
21 | + 'bridge_fd', |
22 | + 'bridge_stp', |
23 | + 'comment', |
24 | + 'install_rackd', |
25 | + 'install_kvm', |
26 | + ) |
27 | +) |
28 | + |
29 | + |
30 | def get_storage_layout_params(request, required=False, extract_params=False): |
31 | """Return and validate the storage_layout parameter.""" |
32 | form = StorageLayoutForm(required=required, data=request.data) |
33 | @@ -218,11 +232,18 @@ def get_storage_layout_params(request, required=False, extract_params=False): |
34 | return storage_layout, params |
35 | |
36 | |
37 | -def get_allocation_parameters(request): |
38 | - """Returns shared parameters for deploy and allocate operations.""" |
39 | +def get_allocation_options(request) -> AllocationOptions: |
40 | + """Parses optional parameters for allocation and deployment.""" |
41 | comment = get_optional_param(request.POST, 'comment') |
42 | + default_bridge_all = False |
43 | + install_rackd = get_optional_param( |
44 | + request.POST, 'install_rackd', default=False, validator=StringBool) |
45 | + install_kvm = get_optional_param( |
46 | + request.POST, 'install_kvm', default=False, validator=StringBool) |
47 | + if install_kvm: |
48 | + default_bridge_all = True |
49 | bridge_all = get_optional_param( |
50 | - request.POST, 'bridge_all', default=False, |
51 | + request.POST, 'bridge_all', default=default_bridge_all, |
52 | validator=StringBool) |
53 | bridge_stp = get_optional_param( |
54 | request.POST, 'bridge_stp', default=False, |
55 | @@ -230,7 +251,15 @@ def get_allocation_parameters(request): |
56 | bridge_fd = get_optional_param( |
57 | request.POST, 'bridge_fd', default=0, validator=Int) |
58 | agent_name = request.data.get('agent_name', '') |
59 | - return agent_name, bridge_all, bridge_fd, bridge_stp, comment |
60 | + return AllocationOptions( |
61 | + agent_name, |
62 | + bridge_all, |
63 | + bridge_fd, |
64 | + bridge_stp, |
65 | + comment, |
66 | + install_rackd, |
67 | + install_kvm |
68 | + ) |
69 | |
70 | |
71 | def get_allocated_composed_machine( |
72 | @@ -556,6 +585,9 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin): |
73 | :param install_rackd: If True, the Rack Controller will be installed on |
74 | this machine. |
75 | :type install_rackd: boolean |
76 | + :param install_kvm: If True, KVM will be installed on this machine and |
77 | + added to MAAS. |
78 | + :type install_kvm: boolean |
79 | |
80 | Ideally we'd have MIME multipart and content-transfer-encoding etc. |
81 | deal with the encapsulation of binary data, but couldn't make it work |
82 | @@ -571,27 +603,24 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin): |
83 | series = request.POST.get('distro_series', None) |
84 | license_key = request.POST.get('license_key', None) |
85 | hwe_kernel = request.POST.get('hwe_kernel', None) |
86 | - install_rackd = get_optional_param( |
87 | - request.POST, 'install_rackd', default=False, validator=StringBool) |
88 | # Acquiring a node requires VIEW permissions. |
89 | machine = self.model.objects.get_node_or_404( |
90 | system_id=system_id, user=request.user, |
91 | perm=NODE_PERMISSION.VIEW) |
92 | + options = get_allocation_options(request) |
93 | if machine.status == NODE_STATUS.READY: |
94 | with locks.node_acquire: |
95 | if machine.owner is not None and machine.owner != request.user: |
96 | raise NodeStateViolation( |
97 | "Can't allocate a machine belonging to another user.") |
98 | - agent_name, bridge_all, bridge_fd, bridge_stp, comment = ( |
99 | - get_allocation_parameters(request)) |
100 | maaslog.info( |
101 | "Request from user %s to acquire machine: %s (%s)", |
102 | request.user.username, machine.fqdn, machine.system_id) |
103 | machine.acquire( |
104 | request.user, get_oauth_token(request), |
105 | - agent_name=agent_name, comment=comment, |
106 | - bridge_all=bridge_all, bridge_stp=bridge_stp, |
107 | - bridge_fd=bridge_fd) |
108 | + agent_name=options.agent_name, comment=options.comment, |
109 | + bridge_all=options.bridge_all, |
110 | + bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd) |
111 | if NODE_STATUS.DEPLOYING not in NODE_TRANSITIONS[machine.status]: |
112 | raise NodeStateViolation( |
113 | "Can't deploy a machine that is in the '{}' state".format( |
114 | @@ -600,7 +629,11 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin): |
115 | if not request.user.has_perm(NODE_PERMISSION.EDIT, machine): |
116 | raise PermissionDenied() |
117 | # Deploying with 'install_rackd' requires ADMIN permissions. |
118 | - if (install_rackd and not |
119 | + if (options.install_rackd and not |
120 | + request.user.has_perm(NODE_PERMISSION.ADMIN, machine)): |
121 | + raise PermissionDenied() |
122 | + # Deploying with 'install_kvm' requires ADMIN permissions. |
123 | + if (options.install_kvm and not |
124 | request.user.has_perm(NODE_PERMISSION.ADMIN, machine)): |
125 | raise PermissionDenied() |
126 | if not machine.distro_series and not series: |
127 | @@ -613,8 +646,10 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin): |
128 | form.set_license_key(license_key=license_key) |
129 | if hwe_kernel is not None: |
130 | form.set_hwe_kernel(hwe_kernel=hwe_kernel) |
131 | - if install_rackd: |
132 | - form.set_install_rackd(install_rackd=install_rackd) |
133 | + if options.install_rackd: |
134 | + form.set_install_rackd(install_rackd=options.install_rackd) |
135 | + if options.install_kvm: |
136 | + form.set_install_kvm(install_kvm=options.install_kvm) |
137 | if form.is_valid(): |
138 | form.save() |
139 | else: |
140 | @@ -1747,8 +1782,7 @@ class MachinesHandler(NodesHandler, PowersMixin): |
141 | maaslog.info( |
142 | "Request from user %s to acquire a machine with constraints: %s", |
143 | request.user.username, str(input_constraints)) |
144 | - agent_name, bridge_all, bridge_fd, bridge_stp, comment = ( |
145 | - get_allocation_parameters(request)) |
146 | + options = get_allocation_options(request) |
147 | verbose = get_optional_param( |
148 | request.POST, 'verbose', default=False, validator=StringBool) |
149 | dry_run = get_optional_param( |
150 | @@ -1814,9 +1848,9 @@ class MachinesHandler(NodesHandler, PowersMixin): |
151 | if not dry_run: |
152 | machine.acquire( |
153 | request.user, get_oauth_token(request), |
154 | - agent_name=agent_name, comment=comment, |
155 | - bridge_all=bridge_all, bridge_stp=bridge_stp, |
156 | - bridge_fd=bridge_fd) |
157 | + agent_name=options.agent_name, comment=options.comment, |
158 | + bridge_all=options.bridge_all, |
159 | + bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd) |
160 | machine.constraint_map = storage.get(machine.id, {}) |
161 | machine.constraints_by_type = {} |
162 | # Need to get the interface constraints map into the proper format |
163 | diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py |
164 | index 3ef0831..ddb305e 100644 |
165 | --- a/src/maasserver/api/tests/test_machines.py |
166 | +++ b/src/maasserver/api/tests/test_machines.py |
167 | @@ -16,6 +16,10 @@ from maasserver import ( |
168 | middleware, |
169 | ) |
170 | from maasserver.api import machines as machines_module |
171 | +from maasserver.api.machines import ( |
172 | + AllocationOptions, |
173 | + get_allocation_options, |
174 | +) |
175 | from maasserver.enum import ( |
176 | INTERFACE_TYPE, |
177 | NODE_STATUS, |
178 | @@ -2774,3 +2778,35 @@ class TestPowerState(APITransactionTestCase.ForUser): |
179 | self.assertEqual({"state": random_state}, response) |
180 | # The machine's power state is now `random_state`. |
181 | self.assertPowerState(machine, random_state) |
182 | + |
183 | + |
184 | +class TestGetAllocationOptions(MAASTestCase): |
185 | + |
186 | + def test_defaults(self): |
187 | + request = factory.make_fake_request(method="POST", data={}) |
188 | + options = get_allocation_options(request) |
189 | + expected_options = AllocationOptions( |
190 | + agent_name='', bridge_all=False, bridge_fd=0, bridge_stp=False, |
191 | + comment=None, install_rackd=False, install_kvm=False) |
192 | + self.assertThat(options, Equals(expected_options)) |
193 | + |
194 | + def test_sets_bridge_all_if_install_kvm(self): |
195 | + request = factory.make_fake_request( |
196 | + method="POST", data=dict(install_kvm='true')) |
197 | + options = get_allocation_options(request) |
198 | + expected_options = AllocationOptions( |
199 | + agent_name='', bridge_all=True, bridge_fd=0, bridge_stp=False, |
200 | + comment=None, install_rackd=False, install_kvm=True) |
201 | + self.assertThat(options, Equals(expected_options)) |
202 | + |
203 | + def test_non_defaults(self): |
204 | + request = factory.make_fake_request(method="POST", data=dict( |
205 | + install_rackd="true", install_kvm="true", bridge_all="true", |
206 | + bridge_stp="true", bridge_fd="42", agent_name="maas", |
207 | + comment="don't panic" |
208 | + )) |
209 | + options = get_allocation_options(request) |
210 | + expected_options = AllocationOptions( |
211 | + agent_name='maas', bridge_all=True, bridge_fd=42, bridge_stp=True, |
212 | + comment="don't panic", install_rackd=True, install_kvm=True) |
213 | + self.assertThat(options, Equals(expected_options)) |
214 | diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py |
215 | index 69440fa..9ad9129 100644 |
216 | --- a/src/maasserver/forms/__init__.py |
217 | +++ b/src/maasserver/forms/__init__.py |
218 | @@ -842,6 +842,11 @@ class MachineForm(NodeForm): |
219 | self.is_bound = True |
220 | self.data['install_rackd'] = install_rackd |
221 | |
222 | + def set_install_kvm(self, install_kvm=False): |
223 | + """Sets whether to deploy the rack alongside this machine.""" |
224 | + self.is_bound = True |
225 | + self.data['install_kvm'] = install_kvm |
226 | + |
227 | class Meta: |
228 | model = Machine |
229 | |
230 | @@ -853,6 +858,7 @@ class MachineForm(NodeForm): |
231 | 'min_hwe_kernel', |
232 | 'hwe_kernel', |
233 | 'install_rackd', |
234 | + 'install_kvm', |
235 | ) |
236 | |
237 | |
238 | diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py |
239 | index 9b457b3..4f5fa73 100644 |
240 | --- a/src/maasserver/forms/pods.py |
241 | +++ b/src/maasserver/forms/pods.py |
242 | @@ -133,9 +133,11 @@ class PodForm(MAASModelForm): |
243 | label="Default MACVLAN mode", required=False, |
244 | choices=MACVLAN_MODE_CHOICES, initial=MACVLAN_MODE_CHOICES[0]) |
245 | |
246 | - def __init__(self, data=None, instance=None, request=None, **kwargs): |
247 | + def __init__( |
248 | + self, data=None, instance=None, request=None, user=None, **kwargs): |
249 | self.is_new = instance is None |
250 | self.request = request |
251 | + self.user = user |
252 | super(PodForm, self).__init__( |
253 | data=data, instance=instance, **kwargs) |
254 | if data is None: |
255 | @@ -273,7 +275,11 @@ class PodForm(MAASModelForm): |
256 | # also create it in the database. |
257 | if not self.instance.name: |
258 | self.instance.set_random_name() |
259 | - self.instance.sync(discovered_pod, self.request.user) |
260 | + if self.request is not None: |
261 | + user = self.request.user |
262 | + else: |
263 | + user = self.user |
264 | + self.instance.sync(discovered_pod, user) |
265 | |
266 | # Save which rack controllers can route and which cannot. |
267 | discovered_rack_ids = [ |
268 | diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py |
269 | index 4bca075..c8ed726 100644 |
270 | --- a/src/maasserver/forms/tests/test_machine.py |
271 | +++ b/src/maasserver/forms/tests/test_machine.py |
272 | @@ -50,6 +50,7 @@ class TestMachineForm(MAASServerTestCase): |
273 | 'min_hwe_kernel', |
274 | 'hwe_kernel', |
275 | 'install_rackd', |
276 | + 'install_kvm', |
277 | ], list(form.fields)) |
278 | |
279 | def test_accepts_usable_architecture(self): |
280 | @@ -366,6 +367,7 @@ class TestAdminMachineForm(MAASServerTestCase): |
281 | 'min_hwe_kernel', |
282 | 'hwe_kernel', |
283 | 'install_rackd', |
284 | + 'install_kvm', |
285 | 'cpu_count', |
286 | 'memory', |
287 | 'zone', |
288 | diff --git a/src/maasserver/migrations/builtin/maasserver/0173_add_node_install_kvm.py b/src/maasserver/migrations/builtin/maasserver/0173_add_node_install_kvm.py |
289 | new file mode 100644 |
290 | index 0000000..5cce8a1 |
291 | --- /dev/null |
292 | +++ b/src/maasserver/migrations/builtin/maasserver/0173_add_node_install_kvm.py |
293 | @@ -0,0 +1,23 @@ |
294 | +# -*- coding: utf-8 -*- |
295 | +# Generated by Django 1.11.11 on 2018-09-15 00:04 |
296 | +from __future__ import unicode_literals |
297 | + |
298 | +from django.db import ( |
299 | + migrations, |
300 | + models, |
301 | +) |
302 | + |
303 | + |
304 | +class Migration(migrations.Migration): |
305 | + |
306 | + dependencies = [ |
307 | + ('maasserver', '0172_partition_tags'), |
308 | + ] |
309 | + |
310 | + operations = [ |
311 | + migrations.AddField( |
312 | + model_name='node', |
313 | + name='install_kvm', |
314 | + field=models.BooleanField(default=False), |
315 | + ), |
316 | + ] |
317 | diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py |
318 | index a763b16..a46bacb 100644 |
319 | --- a/src/maasserver/models/node.py |
320 | +++ b/src/maasserver/models/node.py |
321 | @@ -864,6 +864,8 @@ class Node(CleanSave, TimestampedModel): |
322 | :ivar objects: The :class:`GeneralManager`. |
323 | :ivar install_rackd: An optional flag to indicate if this node should be |
324 | deployed with the rack controller. |
325 | + :ivar install_kvm: An optional flag to indicate if this node should be |
326 | + deployed with KVM and added to MAAS. |
327 | :ivar enable_ssh: An optional flag to indicate if this node can have |
328 | ssh enabled during commissioning, allowing the user to ssh into the |
329 | machine's commissioning environment using the user's SSH key. |
330 | @@ -1046,6 +1048,9 @@ class Node(CleanSave, TimestampedModel): |
331 | # Used to deploy the rack controller on a installation machine. |
332 | install_rackd = BooleanField(default=False) |
333 | |
334 | + # Used to deploy the rack controller on a installation machine. |
335 | + install_kvm = BooleanField(default=False) |
336 | + |
337 | # Used to determine whether to: |
338 | # 1. Import the SSH Key during commissioning and keep power on. |
339 | # 2. Skip reconfiguring networking when a node is commissioned. |
340 | @@ -2916,6 +2921,7 @@ class Node(CleanSave, TimestampedModel): |
341 | self.hwe_kernel = None |
342 | self.current_installation_script_set = None |
343 | self.install_rackd = False |
344 | + self.install_kvm = False |
345 | self.save() |
346 | |
347 | # Clear the nodes acquired filesystems. |
348 | diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py |
349 | index 7359e1f..1d91a62 100644 |
350 | --- a/src/maasserver/testing/factory.py |
351 | +++ b/src/maasserver/testing/factory.py |
352 | @@ -203,17 +203,29 @@ class Messages: |
353 | |
354 | class Factory(maastesting.factory.Factory): |
355 | |
356 | - def make_fake_request(self, path, method="GET", cookies={}): |
357 | + def make_fake_request( |
358 | + self, path="/", method="GET", cookies=None, data=None): |
359 | """Create a fake request. |
360 | |
361 | :param path: The path to which to make the request. |
362 | :param method: The method to use for the request |
363 | ('GET' or 'POST'). |
364 | - :param cookies: A `dict` with the cookies for the request. |
365 | + :param cookies: Optional `dict` with the cookies for the request. |
366 | + :param data: Optional `dict` of parameters. |
367 | """ |
368 | rf = MAASSensibleRequestFactory() |
369 | - request = rf.get(path) |
370 | - request.method = method |
371 | + if data is None: |
372 | + data = {} |
373 | + if cookies is None: |
374 | + cookies = {} |
375 | + if method == "GET": |
376 | + request = rf.get(path, data=data) |
377 | + elif method == "POST": |
378 | + request = rf.post(path, data=data) |
379 | + else: |
380 | + request = rf.get(path, data=data) |
381 | + request.method = method |
382 | + request.data = data |
383 | request._messages = Messages() |
384 | request.COOKIES = cookies.copy() |
385 | return request |
386 | diff --git a/src/maasserver/websockets/handlers/device.py b/src/maasserver/websockets/handlers/device.py |
387 | index c3cf5c1..4c964a8 100644 |
388 | --- a/src/maasserver/websockets/handlers/device.py |
389 | +++ b/src/maasserver/websockets/handlers/device.py |
390 | @@ -131,6 +131,7 @@ class DeviceHandler(NodeHandler): |
391 | "last_image_sync", |
392 | "default_user", |
393 | "install_rackd", |
394 | + "install_kvm", |
395 | ] |
396 | list_fields = [ |
397 | "id", |
398 | diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py |
399 | index f53a2f3..a73da82 100644 |
400 | --- a/src/maasserver/websockets/handlers/machine.py |
401 | +++ b/src/maasserver/websockets/handlers/machine.py |
402 | @@ -178,6 +178,7 @@ class MachineHandler(NodeHandler): |
403 | "managing_process", |
404 | "last_image_sync", |
405 | "install_rackd", |
406 | + "install_kvm", |
407 | ] |
408 | list_fields = [ |
409 | "id", |
410 | diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py |
411 | index b786954..e76a651 100644 |
412 | --- a/src/metadataserver/api.py |
413 | +++ b/src/metadataserver/api.py |
414 | @@ -870,7 +870,20 @@ class UserDataHandler(MetadataViewHandler): |
415 | # for user-data is when MAAS hands the node |
416 | # off to a user. |
417 | if node.status == NODE_STATUS.DEPLOYING: |
418 | - node.end_deployment() |
419 | + if node.install_kvm: |
420 | + # Rather than ending deployment here, note that we're |
421 | + # installing a KVM pod. |
422 | + node.agent_name = "maas-kvm-pod" |
423 | + node.save() |
424 | + else: |
425 | + # MAAS currently considers a machine "Deployed" when the |
426 | + # cloud-init user data is requested. Note that this doesn't |
427 | + # mean the machine is ready for use yet; cloud-init will |
428 | + # also send a 'finish' event for the 'modules-final' |
429 | + # activity name. However, that check is ambiguous because |
430 | + # it occurs both when curtin is installing, and when |
431 | + # the machine reboots to finish its deployment. |
432 | + node.end_deployment() |
433 | # If this node is supposed to be powered off, serve the |
434 | # 'poweroff' userdata. |
435 | if node.get_boot_purpose() == 'poweroff': |
436 | diff --git a/src/metadataserver/api_twisted.py b/src/metadataserver/api_twisted.py |
437 | index 08a26d3..0659d30 100644 |
438 | --- a/src/metadataserver/api_twisted.py |
439 | +++ b/src/metadataserver/api_twisted.py |
440 | @@ -14,7 +14,11 @@ from maasserver.enum import ( |
441 | NODE_STATUS, |
442 | NODE_TYPE, |
443 | ) |
444 | -from maasserver.models.node import Node |
445 | +from maasserver.forms.pods import PodForm |
446 | +from maasserver.models import ( |
447 | + Node, |
448 | + NodeMetadata, |
449 | +) |
450 | from maasserver.preseed import CURTIN_INSTALL_LOG |
451 | from maasserver.utils.orm import ( |
452 | in_transaction, |
453 | @@ -175,6 +179,47 @@ class StatusHandlerResource(Resource): |
454 | return NOT_DONE_YET |
455 | |
456 | |
457 | +POD_CREATION_ERROR = ( |
458 | + "Internal error while creating KVM pod. (See regiond.log for details.)" |
459 | +) |
460 | + |
461 | + |
462 | +def _create_pod_for_deployment(node): |
463 | + virsh_password_meta = NodeMetadata.objects.filter( |
464 | + node=node, key="virsh_password").first() |
465 | + if virsh_password_meta is None: |
466 | + node.mark_failed( |
467 | + comment="Failed to deploy KVM: Password not found.", commit=False) |
468 | + else: |
469 | + virsh_password = virsh_password_meta.value |
470 | + virsh_password_meta.delete() |
471 | + # XXX: Should find the best IP to communicate with, given what the rack |
472 | + # controller can access, or use the boot interface IP address. |
473 | + ip = node.ip_addresses()[0] |
474 | + if ':' in ip: |
475 | + ip = "[%s]" % ip |
476 | + power_address = "qemu+ssh://virsh@%s/system" % ip |
477 | + pod_form = PodForm(data=dict( |
478 | + type="virsh", |
479 | + name=node.hostname, |
480 | + power_address=power_address, |
481 | + power_pass=virsh_password, |
482 | + zone=node.zone.name, |
483 | + pool=node.pool.name, |
484 | + ), user=node.owner) |
485 | + if pod_form.is_valid(): |
486 | + try: |
487 | + pod_form.save() |
488 | + except Exception: |
489 | + node.mark_failed(comment=POD_CREATION_ERROR, commit=False) |
490 | + log.err(None, "Exception while saving pod form.") |
491 | + else: |
492 | + node.status = NODE_STATUS.DEPLOYED |
493 | + else: |
494 | + node.mark_failed(comment=POD_CREATION_ERROR, commit=False) |
495 | + log.msg("Error while creating KVM pod: %s" % dict(pod_form.errors)) |
496 | + |
497 | + |
498 | class StatusWorkerService(TimerService, object): |
499 | """Service to update nodes from recieved status messages.""" |
500 | |
501 | @@ -257,7 +302,8 @@ class StatusWorkerService(TimerService, object): |
502 | # LP:1701352 - If no exit code is given by the client default to |
503 | # 0(pass) unless the signal is fail then set to 1(failure). This allows |
504 | # a Curtin failure to cause the ScriptResult to fail. |
505 | - default_exit_status = 1 if result in ['FAIL', 'FAILURE'] else 0 |
506 | + failed = result in ['FAIL', 'FAILURE'] |
507 | + default_exit_status = 1 if failed else 0 |
508 | |
509 | # Add this event to the node event log. |
510 | add_event_to_node_event_log( |
511 | @@ -310,7 +356,7 @@ class StatusWorkerService(TimerService, object): |
512 | # cloud-init may send a failure message if a script reboots |
513 | # the system. If a script is running which may_reboot ignore |
514 | # the signal. |
515 | - if result in ['FAIL', 'FAILURE']: |
516 | + if failed: |
517 | script_set = node.current_commissioning_script_set |
518 | if (script_set is None or not |
519 | script_set.scriptresult_set.filter( |
520 | @@ -323,18 +369,28 @@ class StatusWorkerService(TimerService, object): |
521 | script_result_status=SCRIPT_STATUS.ABORTED) |
522 | save_node = True |
523 | elif node.status == NODE_STATUS.DEPLOYING: |
524 | - if result in ['FAIL', 'FAILURE']: |
525 | + # XXX: when activity_name == moudles-config, this currently |
526 | + # /always/ fails, since MAAS passes two different versions |
527 | + # for the apt configuration. The only reason why we don't |
528 | + # see additional issues because of this is due to the node |
529 | + # already being marked "Deployed". Right now this is prevented |
530 | + # only in the install_kvm case, but we should make this check |
531 | + # more general when time allows. |
532 | + if failed and not node.install_kvm: |
533 | node.mark_failed( |
534 | comment="Installation failed (refer to the " |
535 | "installation log for more information).", |
536 | commit=False) |
537 | save_node = True |
538 | + elif (not failed and activity_name == "modules-final" and |
539 | + node.install_kvm and node.agent_name == "maas-kvm-pod"): |
540 | + save_node = True |
541 | + _create_pod_for_deployment(node) |
542 | elif node.status == NODE_STATUS.DISK_ERASING: |
543 | - if result in ['FAIL', 'FAILURE']: |
544 | + if failed: |
545 | node.mark_failed( |
546 | comment="Failed to erase disks.", commit=False) |
547 | save_node = True |
548 | - |
549 | # Deallocate the node if we enter any terminal state. |
550 | if node.node_type == NODE_TYPE.MACHINE and node.status in [ |
551 | NODE_STATUS.READY, |
552 | diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py |
553 | index 998df04..2e7b6cb 100644 |
554 | --- a/src/metadataserver/tests/test_api.py |
555 | +++ b/src/metadataserver/tests/test_api.py |
556 | @@ -948,6 +948,17 @@ class TestMetadataUserDataStateChanges(MAASServerTestCase): |
557 | self.assertEqual(http.client.OK, response.status_code) |
558 | self.assertEqual(NODE_STATUS.DEPLOYED, reload_object(node).status) |
559 | |
560 | + def test_skips_status_change_if_installing_kvm_and_sets_agent_name(self): |
561 | + node = factory.make_Node( |
562 | + status=NODE_STATUS.DEPLOYING, install_kvm=True) |
563 | + NodeUserData.objects.set_user_data(node, sample_binary_data) |
564 | + client = make_node_client(node) |
565 | + response = client.get(reverse('metadata-user-data', args=['latest'])) |
566 | + self.assertEqual(http.client.OK, response.status_code) |
567 | + self.assertEqual(NODE_STATUS.DEPLOYING, reload_object(node).status) |
568 | + node = reload_object(node) |
569 | + self.assertEqual(node.agent_name, "maas-kvm-pod") |
570 | + |
571 | |
572 | class TestCurtinMetadataUserData( |
573 | PreseedRPCMixin, MAASTransactionServerTestCase): |
574 | diff --git a/src/metadataserver/tests/test_api_twisted.py b/src/metadataserver/tests/test_api_twisted.py |
575 | index e8c7e34..65e5014 100644 |
576 | --- a/src/metadataserver/tests/test_api_twisted.py |
577 | +++ b/src/metadataserver/tests/test_api_twisted.py |
578 | @@ -24,6 +24,7 @@ from crochet import wait_for |
579 | from maasserver.enum import NODE_STATUS |
580 | from maasserver.models import ( |
581 | Event, |
582 | + NodeMetadata, |
583 | Tag, |
584 | ) |
585 | from maasserver.models.signals.testing import SignalsDisabled |
586 | @@ -42,13 +43,19 @@ from maasserver.utils.orm import ( |
587 | ) |
588 | from maasserver.utils.threads import deferToDatabase |
589 | from maastesting.matchers import ( |
590 | + DocTestMatches, |
591 | MockCalledOnceWith, |
592 | MockCallsMatch, |
593 | MockNotCalled, |
594 | ) |
595 | from maastesting.testcase import MAASTestCase |
596 | -from metadataserver import api |
597 | +from metadataserver import ( |
598 | + api, |
599 | + api_twisted as api_twisted_module, |
600 | +) |
601 | from metadataserver.api_twisted import ( |
602 | + _create_pod_for_deployment, |
603 | + POD_CREATION_ERROR, |
604 | StatusHandlerResource, |
605 | StatusWorkerService, |
606 | ) |
607 | @@ -60,6 +67,7 @@ from metadataserver.models import NodeKey |
608 | from testtools import ExpectedException |
609 | from testtools.matchers import ( |
610 | Equals, |
611 | + Is, |
612 | MatchesListwise, |
613 | MatchesSetwise, |
614 | ) |
615 | @@ -343,7 +351,7 @@ def encode_as_base64(content): |
616 | class TestStatusWorkerService(MAASServerTestCase): |
617 | |
618 | def setUp(self): |
619 | - super(TestStatusWorkerService, self).setUp() |
620 | + super().setUp() |
621 | self.useFixture(SignalsDisabled("power")) |
622 | |
623 | def processMessage(self, node, payload): |
624 | @@ -505,6 +513,23 @@ class TestStatusWorkerService(MAASServerTestCase): |
625 | " log for more information).", |
626 | Event.objects.filter(node=node).last().description) |
627 | |
628 | + def test_status_ok_for_modules_final_triggers_kvm_install(self): |
629 | + node = factory.make_Node( |
630 | + interface=True, status=NODE_STATUS.DEPLOYING, |
631 | + agent_name="maas-kvm-pod", install_kvm=True) |
632 | + payload = { |
633 | + 'event_type': 'finish', |
634 | + 'result': 'OK', |
635 | + 'origin': 'cloud-init', |
636 | + 'name': 'modules-final', |
637 | + 'description': 'America for Make Benefit Glorious Nation', |
638 | + 'timestamp': datetime.utcnow(), |
639 | + } |
640 | + mock_create_pod = self.patch( |
641 | + api_twisted_module, '_create_pod_for_deployment') |
642 | + self.processMessage(node, payload) |
643 | + self.assertThat(mock_create_pod, MockCalledOnceWith(node)) |
644 | + |
645 | def test_status_installation_fail_leaves_node_failed(self): |
646 | node = factory.make_Node(interface=True, status=NODE_STATUS.DEPLOYING) |
647 | payload = { |
648 | @@ -987,3 +1012,71 @@ class TestStatusWorkerService(MAASServerTestCase): |
649 | node.status_expires, expected_time - timedelta(minutes=1)) |
650 | self.assertLessEqual( |
651 | node.status_expires, expected_time + timedelta(minutes=1)) |
652 | + |
653 | + |
654 | +class TestCreatePodForDeployment(MAASServerTestCase): |
655 | + |
656 | + def setUp(self): |
657 | + super().setUp() |
658 | + self.mock_PodForm = self.patch(api_twisted_module, "PodForm") |
659 | + |
660 | + def test__marks_failed_if_no_virsh_password(self): |
661 | + node = factory.make_Node( |
662 | + interface=True, status=NODE_STATUS.DEPLOYING, |
663 | + agent_name="maas-kvm-pod", install_kvm=True) |
664 | + _create_pod_for_deployment(node) |
665 | + self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT)) |
666 | + self.assertThat(node.error_description, DocTestMatches( |
667 | + "...Password not found...")) |
668 | + |
669 | + def test__deletes_virsh_password_metadata_and_sets_deployed(self): |
670 | + node = factory.make_Node_with_Interface_on_Subnet( |
671 | + status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod", |
672 | + install_kvm=True) |
673 | + factory.make_StaticIPAddress(interface=node.boot_interface) |
674 | + meta = NodeMetadata.objects.create( |
675 | + node=node, key="virsh_password", value="xyz123") |
676 | + _create_pod_for_deployment(node) |
677 | + meta = reload_object(meta) |
678 | + self.assertThat(meta, Is(None)) |
679 | + self.assertThat(node.status, Equals(NODE_STATUS.DEPLOYED)) |
680 | + |
681 | + def test__marks_failed_if_is_valid_returns_false(self): |
682 | + mock_pod_form = Mock() |
683 | + self.mock_PodForm.return_value = mock_pod_form |
684 | + mock_pod_form.errors = {} |
685 | + mock_pod_form.is_valid = Mock() |
686 | + mock_pod_form.is_valid.return_value = False |
687 | + node = factory.make_Node_with_Interface_on_Subnet( |
688 | + status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod", |
689 | + install_kvm=True) |
690 | + factory.make_StaticIPAddress(interface=node.boot_interface) |
691 | + meta = NodeMetadata.objects.create( |
692 | + node=node, key="virsh_password", value="xyz123") |
693 | + _create_pod_for_deployment(node) |
694 | + meta = reload_object(meta) |
695 | + self.assertThat(meta, Is(None)) |
696 | + self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT)) |
697 | + self.assertThat(node.error_description, DocTestMatches( |
698 | + POD_CREATION_ERROR)) |
699 | + |
700 | + def test__marks_failed_if_save_raises(self): |
701 | + mock_pod_form = Mock() |
702 | + self.mock_PodForm.return_value = mock_pod_form |
703 | + mock_pod_form.errors = {} |
704 | + mock_pod_form.is_valid = Mock() |
705 | + mock_pod_form.is_valid.return_value = True |
706 | + mock_pod_form.save = Mock() |
707 | + mock_pod_form.save.side_effect = ValueError |
708 | + node = factory.make_Node_with_Interface_on_Subnet( |
709 | + status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod", |
710 | + install_kvm=True) |
711 | + factory.make_StaticIPAddress(interface=node.boot_interface) |
712 | + meta = NodeMetadata.objects.create( |
713 | + node=node, key="virsh_password", value="xyz123") |
714 | + _create_pod_for_deployment(node) |
715 | + meta = reload_object(meta) |
716 | + self.assertThat(meta, Is(None)) |
717 | + self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT)) |
718 | + self.assertThat(node.error_description, DocTestMatches( |
719 | + POD_CREATION_ERROR)) |
720 | diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py |
721 | index bd0475f..3b1a6d3 100644 |
722 | --- a/src/metadataserver/tests/test_vendor_data.py |
723 | +++ b/src/metadataserver/tests/test_vendor_data.py |
724 | @@ -5,7 +5,10 @@ |
725 | |
726 | __all__ = [] |
727 | |
728 | -from maasserver.models.config import Config |
729 | +from maasserver.models import ( |
730 | + Config, |
731 | + NodeMetadata, |
732 | +) |
733 | from maasserver.server_address import get_maas_facing_server_host |
734 | from maasserver.testing.factory import factory |
735 | from maasserver.testing.testcase import MAASServerTestCase |
736 | @@ -21,6 +24,7 @@ from testtools.matchers import ( |
737 | Contains, |
738 | ContainsDict, |
739 | Equals, |
740 | + HasLength, |
741 | Is, |
742 | IsInstance, |
743 | KeysEqual, |
744 | @@ -221,3 +225,20 @@ class TestGenerateRackControllerConfiguration(MAASServerTestCase): |
745 | "%s --maas-url %s --secret %s" % (cmd, maas_url, secret), |
746 | ] |
747 | })) |
748 | + |
749 | + def test_yields_configuration_when_machine_install_kvm_true(self): |
750 | + node = factory.make_Node(osystem='ubuntu', netboot=False) |
751 | + node.install_kvm = True |
752 | + configuration = get_vendor_data(node) |
753 | + config = str(dict(configuration)) |
754 | + self.assertThat(config, Contains("virsh")) |
755 | + self.assertThat(config, Contains("ssh_pwauth")) |
756 | + self.assertThat(config, Contains("rbash")) |
757 | + self.assertThat(config, Contains("libvirt-qemu")) |
758 | + self.assertThat(config, Contains("ForceCommand")) |
759 | + self.assertThat(config, Contains("qemu-kvm")) |
760 | + self.assertThat(config, Contains("libvirt-bin")) |
761 | + # Check that a password was saved for the pod-to-be. |
762 | + virsh_password_meta = NodeMetadata.objects.filter( |
763 | + node=node, key="virsh_password").first() |
764 | + self.assertThat(virsh_password_meta.value, HasLength(32)) |
765 | diff --git a/src/metadataserver/vendor_data.py b/src/metadataserver/vendor_data.py |
766 | index a0c689d..b6bbf72 100644 |
767 | --- a/src/metadataserver/vendor_data.py |
768 | +++ b/src/metadataserver/vendor_data.py |
769 | @@ -7,10 +7,16 @@ __all__ = [ |
770 | 'get_vendor_data', |
771 | ] |
772 | |
773 | +from base64 import b64encode |
774 | +from crypt import crypt |
775 | from itertools import chain |
776 | +from os import urandom |
777 | |
778 | from maasserver import ntp |
779 | -from maasserver.models import Config |
780 | +from maasserver.models import ( |
781 | + Config, |
782 | + NodeMetadata, |
783 | +) |
784 | from maasserver.server_address import get_maas_facing_server_host |
785 | from netaddr import IPAddress |
786 | from provisioningserver.ntp.config import normalise_address |
787 | @@ -23,6 +29,7 @@ def get_vendor_data(node): |
788 | generate_system_info(node), |
789 | generate_ntp_configuration(node), |
790 | generate_rack_controller_configuration(node), |
791 | + generate_kvm_pod_configuration(node), |
792 | )) |
793 | |
794 | |
795 | @@ -91,3 +98,56 @@ def generate_rack_controller_configuration(node): |
796 | "/snap/bin/maas init --mode rack --maas-url %s --secret %s" % ( |
797 | maas_url, secret) |
798 | ] |
799 | + |
800 | + |
801 | +def generate_kvm_pod_configuration(node): |
802 | + """Generate cloud-init configuration to install the node as a KVM pod.""" |
803 | + if node.netboot is False and node.install_kvm is True: |
804 | + yield "runcmd", [ |
805 | + # Restrict the $PATH so that rbash can be used to limit what the |
806 | + # virsh user can do if they manage to get a shell. |
807 | + ['mkdir', '-p', '/home/virsh/bin'], |
808 | + ['ln', '-s', '/usr/bin/virsh', '/home/virsh/bin/virsh'], |
809 | + ['sh', '-c', 'echo "PATH=/home/virsh/bin" >> /home/virsh/.bashrc'], |
810 | + # Use a ForceCommand to make sure the only thing the virsh user |
811 | + # can do with SSH is communicate with libvirt. |
812 | + ['sh', '-c', |
813 | + 'printf "Match user virsh\\n' |
814 | + ' X11Forwarding no\\n' |
815 | + ' AllowTcpForwarding no\\n' |
816 | + ' PermitTTY no\\n' |
817 | + ' ForceCommand nc -q 0 -U /var/run/libvirt/libvirt-sock\\n"' |
818 | + ' >> /etc/ssh/sshd_config'], |
819 | + # Make sure the 'virsh' user is allowed to access libvirt. |
820 | + ['/usr/sbin/usermod', '--append', '--groups', |
821 | + 'libvirt,libvirt-qemu', 'virsh'], |
822 | + # SSH needs to be restarted in order for the above changes to |
823 | + # take effect. |
824 | + ['systemctl', 'restart', 'sshd'], |
825 | + # Ensure services are ready before cloud-init finishes. |
826 | + ['/bin/sleep', '10'], |
827 | + ] |
828 | + # Generate a 32-character password by encoding 24 bytes as base64. |
829 | + virsh_password = b64encode( |
830 | + urandom(24), altchars=b'.!').decode('ascii') |
831 | + # Pass crypted (salted/hashed) version of the password to cloud-init. |
832 | + encrypted_password = crypt(virsh_password) |
833 | + # Store a cleartext version of the password so we can add a pod later. |
834 | + NodeMetadata.objects.update_or_create( |
835 | + node=node, key="virsh_password", |
836 | + defaults=dict(value=virsh_password)) |
837 | + # Make sure SSH password authentication is enabled. |
838 | + yield "ssh_pwauth", True |
839 | + # Create a custom 'virsh' user (in addition to the default user) |
840 | + # with the encrypted password, and a locked-down shell. |
841 | + yield "users", [ |
842 | + 'default', |
843 | + { |
844 | + 'name': 'virsh', |
845 | + 'lock_passwd': False, |
846 | + 'passwd': encrypted_password, |
847 | + 'shell': '/bin/rbash', |
848 | + } |
849 | + ] |
850 | + # XXX: Use correct packages based on architecture. |
851 | + yield "packages", ["qemu-kvm", "libvirt-bin"] |
Does agent_name get cleared out on release?