Merge ~mpontillo/maas:install-kvm into maas:master

[Mike Pontillo]

Allow KVM pod deployment in MAAS using install_kvm option.

[Andres Rodriguez (andreserl)]

Does agent_name get cleared out on release?

review: Needs Information

[Mike Pontillo (mpontillo)]

Yes, agent_name is cleared on release (I checked that yesterday).

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

[Lee Trager (ltrager)]

LGTM!

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b install-kvm lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

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

30d7864... by Mike Pontillo on 2018-09-19

Fix test failures.

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index 9a95cc8..12cbc69 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -8,6 +8,7 @@ __all__ = [
     "get_storage_layout_params",
 ]
 
+from collections import namedtuple
 import json
 import re
 
@@ -193,6 +194,19 @@ DISPLAYED_ANON_MACHINE_FIELDS = (
 )
 
 
+AllocationOptions = namedtuple(
+    'AllocationOptions', (
+        'agent_name',
+        'bridge_all',
+        'bridge_fd',
+        'bridge_stp',
+        'comment',
+        'install_rackd',
+        'install_kvm',
+    )
+)
+
+
 def get_storage_layout_params(request, required=False, extract_params=False):
     """Return and validate the storage_layout parameter."""
     form = StorageLayoutForm(required=required, data=request.data)
@@ -218,11 +232,18 @@ def get_storage_layout_params(request, required=False, extract_params=False):
     return storage_layout, params
 
 
-def get_allocation_parameters(request):
-    """Returns shared parameters for deploy and allocate operations."""
+def get_allocation_options(request) -> AllocationOptions:
+    """Parses optional parameters for allocation and deployment."""
     comment = get_optional_param(request.POST, 'comment')
+    default_bridge_all = False
+    install_rackd = get_optional_param(
+        request.POST, 'install_rackd', default=False, validator=StringBool)
+    install_kvm = get_optional_param(
+        request.POST, 'install_kvm', default=False, validator=StringBool)
+    if install_kvm:
+        default_bridge_all = True
     bridge_all = get_optional_param(
-        request.POST, 'bridge_all', default=False,
+        request.POST, 'bridge_all', default=default_bridge_all,
         validator=StringBool)
     bridge_stp = get_optional_param(
         request.POST, 'bridge_stp', default=False,
@@ -230,7 +251,15 @@ def get_allocation_parameters(request):
     bridge_fd = get_optional_param(
         request.POST, 'bridge_fd', default=0, validator=Int)
     agent_name = request.data.get('agent_name', '')
-    return agent_name, bridge_all, bridge_fd, bridge_stp, comment
+    return AllocationOptions(
+        agent_name,
+        bridge_all,
+        bridge_fd,
+        bridge_stp,
+        comment,
+        install_rackd,
+        install_kvm
+    )
 
 
 def get_allocated_composed_machine(
@@ -556,6 +585,9 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
         :param install_rackd: If True, the Rack Controller will be installed on
             this machine.
         :type install_rackd: boolean
+        :param install_kvm: If True, KVM will be installed on this machine and
+            added to MAAS.
+        :type install_kvm: boolean
 
         Ideally we'd have MIME multipart and content-transfer-encoding etc.
         deal with the encapsulation of binary data, but couldn't make it work
@@ -571,27 +603,24 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
         series = request.POST.get('distro_series', None)
         license_key = request.POST.get('license_key', None)
         hwe_kernel = request.POST.get('hwe_kernel', None)
-        install_rackd = get_optional_param(
-            request.POST, 'install_rackd', default=False, validator=StringBool)
         # Acquiring a node requires VIEW permissions.
         machine = self.model.objects.get_node_or_404(
             system_id=system_id, user=request.user,
             perm=NODE_PERMISSION.VIEW)
+        options = get_allocation_options(request)
         if machine.status == NODE_STATUS.READY:
             with locks.node_acquire:
                 if machine.owner is not None and machine.owner != request.user:
                     raise NodeStateViolation(
                         "Can't allocate a machine belonging to another user.")
-                agent_name, bridge_all, bridge_fd, bridge_stp, comment = (
-                    get_allocation_parameters(request))
                 maaslog.info(
                     "Request from user %s to acquire machine: %s (%s)",
                     request.user.username, machine.fqdn, machine.system_id)
                 machine.acquire(
                     request.user, get_oauth_token(request),
-                    agent_name=agent_name, comment=comment,
-                    bridge_all=bridge_all, bridge_stp=bridge_stp,
-                    bridge_fd=bridge_fd)
+                    agent_name=options.agent_name, comment=options.comment,
+                    bridge_all=options.bridge_all,
+                    bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd)
         if NODE_STATUS.DEPLOYING not in NODE_TRANSITIONS[machine.status]:
             raise NodeStateViolation(
                 "Can't deploy a machine that is in the '{}' state".format(
@@ -600,7 +629,11 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
         if not request.user.has_perm(NODE_PERMISSION.EDIT, machine):
             raise PermissionDenied()
         # Deploying with 'install_rackd' requires ADMIN permissions.
-        if (install_rackd and not
+        if (options.install_rackd and not
+                request.user.has_perm(NODE_PERMISSION.ADMIN, machine)):
+            raise PermissionDenied()
+        # Deploying with 'install_kvm' requires ADMIN permissions.
+        if (options.install_kvm and not
                 request.user.has_perm(NODE_PERMISSION.ADMIN, machine)):
             raise PermissionDenied()
         if not machine.distro_series and not series:
@@ -613,8 +646,10 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
             form.set_license_key(license_key=license_key)
         if hwe_kernel is not None:
             form.set_hwe_kernel(hwe_kernel=hwe_kernel)
-        if install_rackd:
-            form.set_install_rackd(install_rackd=install_rackd)
+        if options.install_rackd:
+            form.set_install_rackd(install_rackd=options.install_rackd)
+        if options.install_kvm:
+            form.set_install_kvm(install_kvm=options.install_kvm)
         if form.is_valid():
             form.save()
         else:
@@ -1747,8 +1782,7 @@ class MachinesHandler(NodesHandler, PowersMixin):
         maaslog.info(
             "Request from user %s to acquire a machine with constraints: %s",
             request.user.username, str(input_constraints))
-        agent_name, bridge_all, bridge_fd, bridge_stp, comment = (
-            get_allocation_parameters(request))
+        options = get_allocation_options(request)
         verbose = get_optional_param(
             request.POST, 'verbose', default=False, validator=StringBool)
         dry_run = get_optional_param(
@@ -1814,9 +1848,9 @@ class MachinesHandler(NodesHandler, PowersMixin):
             if not dry_run:
                 machine.acquire(
                     request.user, get_oauth_token(request),
-                    agent_name=agent_name, comment=comment,
-                    bridge_all=bridge_all, bridge_stp=bridge_stp,
-                    bridge_fd=bridge_fd)
+                    agent_name=options.agent_name, comment=options.comment,
+                    bridge_all=options.bridge_all,
+                    bridge_stp=options.bridge_stp, bridge_fd=options.bridge_fd)
             machine.constraint_map = storage.get(machine.id, {})
             machine.constraints_by_type = {}
             # Need to get the interface constraints map into the proper format
diff --git a/src/maasserver/api/tests/test_machines.py b/src/maasserver/api/tests/test_machines.py
index 3ef0831..ddb305e 100644
--- a/src/maasserver/api/tests/test_machines.py
+++ b/src/maasserver/api/tests/test_machines.py
@@ -16,6 +16,10 @@ from maasserver import (
     middleware,
 )
 from maasserver.api import machines as machines_module
+from maasserver.api.machines import (
+    AllocationOptions,
+    get_allocation_options,
+)
 from maasserver.enum import (
     INTERFACE_TYPE,
     NODE_STATUS,
@@ -2774,3 +2778,35 @@ class TestPowerState(APITransactionTestCase.ForUser):
         self.assertEqual({"state": random_state}, response)
         # The machine's power state is now `random_state`.
         self.assertPowerState(machine, random_state)
+
+
+class TestGetAllocationOptions(MAASTestCase):
+
+    def test_defaults(self):
+        request = factory.make_fake_request(method="POST", data={})
+        options = get_allocation_options(request)
+        expected_options = AllocationOptions(
+            agent_name='', bridge_all=False, bridge_fd=0, bridge_stp=False,
+            comment=None, install_rackd=False, install_kvm=False)
+        self.assertThat(options, Equals(expected_options))
+
+    def test_sets_bridge_all_if_install_kvm(self):
+        request = factory.make_fake_request(
+            method="POST", data=dict(install_kvm='true'))
+        options = get_allocation_options(request)
+        expected_options = AllocationOptions(
+            agent_name='', bridge_all=True, bridge_fd=0, bridge_stp=False,
+            comment=None, install_rackd=False, install_kvm=True)
+        self.assertThat(options, Equals(expected_options))
+
+    def test_non_defaults(self):
+        request = factory.make_fake_request(method="POST", data=dict(
+            install_rackd="true", install_kvm="true", bridge_all="true",
+            bridge_stp="true", bridge_fd="42", agent_name="maas",
+            comment="don't panic"
+        ))
+        options = get_allocation_options(request)
+        expected_options = AllocationOptions(
+            agent_name='maas', bridge_all=True, bridge_fd=42, bridge_stp=True,
+            comment="don't panic", install_rackd=True, install_kvm=True)
+        self.assertThat(options, Equals(expected_options))
diff --git a/src/maasserver/forms/__init__.py b/src/maasserver/forms/__init__.py
index 69440fa..9ad9129 100644
--- a/src/maasserver/forms/__init__.py
+++ b/src/maasserver/forms/__init__.py
@@ -842,6 +842,11 @@ class MachineForm(NodeForm):
         self.is_bound = True
         self.data['install_rackd'] = install_rackd
 
+    def set_install_kvm(self, install_kvm=False):
+        """Sets whether to deploy the rack alongside this machine."""
+        self.is_bound = True
+        self.data['install_kvm'] = install_kvm
+
     class Meta:
         model = Machine
 
@@ -853,6 +858,7 @@ class MachineForm(NodeForm):
             'min_hwe_kernel',
             'hwe_kernel',
             'install_rackd',
+            'install_kvm',
         )
 
 
diff --git a/src/maasserver/forms/pods.py b/src/maasserver/forms/pods.py
index 9b457b3..4f5fa73 100644
--- a/src/maasserver/forms/pods.py
+++ b/src/maasserver/forms/pods.py
@@ -133,9 +133,11 @@ class PodForm(MAASModelForm):
         label="Default MACVLAN mode", required=False,
         choices=MACVLAN_MODE_CHOICES, initial=MACVLAN_MODE_CHOICES[0])
 
-    def __init__(self, data=None, instance=None, request=None, **kwargs):
+    def __init__(
+            self, data=None, instance=None, request=None, user=None, **kwargs):
         self.is_new = instance is None
         self.request = request
+        self.user = user
         super(PodForm, self).__init__(
             data=data, instance=instance, **kwargs)
         if data is None:
@@ -273,7 +275,11 @@ class PodForm(MAASModelForm):
             # also create it in the database.
             if not self.instance.name:
                 self.instance.set_random_name()
-            self.instance.sync(discovered_pod, self.request.user)
+            if self.request is not None:
+                user = self.request.user
+            else:
+                user = self.user
+            self.instance.sync(discovered_pod, user)
 
             # Save which rack controllers can route and which cannot.
             discovered_rack_ids = [
diff --git a/src/maasserver/forms/tests/test_machine.py b/src/maasserver/forms/tests/test_machine.py
index 4bca075..c8ed726 100644
--- a/src/maasserver/forms/tests/test_machine.py
+++ b/src/maasserver/forms/tests/test_machine.py
@@ -50,6 +50,7 @@ class TestMachineForm(MAASServerTestCase):
                 'min_hwe_kernel',
                 'hwe_kernel',
                 'install_rackd',
+                'install_kvm',
             ], list(form.fields))
 
     def test_accepts_usable_architecture(self):
@@ -366,6 +367,7 @@ class TestAdminMachineForm(MAASServerTestCase):
                 'min_hwe_kernel',
                 'hwe_kernel',
                 'install_rackd',
+                'install_kvm',
                 'cpu_count',
                 'memory',
                 'zone',
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
 new file mode 100644
index 0000000..5cce8a1
--- /dev/null
+++ b/src/maasserver/migrations/builtin/maasserver/0173_add_node_install_kvm.py
@@ -0,0 +1,23 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.11 on 2018-09-15 00:04
+from __future__ import unicode_literals
+
+from django.db import (
+    migrations,
+    models,
+)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('maasserver', '0172_partition_tags'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='node',
+            name='install_kvm',
+            field=models.BooleanField(default=False),
+        ),
+    ]
diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index a763b16..a46bacb 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -864,6 +864,8 @@ class Node(CleanSave, TimestampedModel):
     :ivar objects: The :class:`GeneralManager`.
     :ivar install_rackd: An optional flag to indicate if this node should be
         deployed with the rack controller.
+    :ivar install_kvm: An optional flag to indicate if this node should be
+        deployed with KVM and added to MAAS.
     :ivar enable_ssh: An optional flag to indicate if this node can have
         ssh enabled during commissioning, allowing the user to ssh into the
         machine's commissioning environment using the user's SSH key.
@@ -1046,6 +1048,9 @@ class Node(CleanSave, TimestampedModel):
     # Used to deploy the rack controller on a installation machine.
     install_rackd = BooleanField(default=False)
 
+    # Used to deploy the rack controller on a installation machine.
+    install_kvm = BooleanField(default=False)
+
     # Used to determine whether to:
     #  1. Import the SSH Key during commissioning and keep power on.
     #  2. Skip reconfiguring networking when a node is commissioned.
@@ -2916,6 +2921,7 @@ class Node(CleanSave, TimestampedModel):
         self.hwe_kernel = None
         self.current_installation_script_set = None
         self.install_rackd = False
+        self.install_kvm = False
         self.save()
 
         # Clear the nodes acquired filesystems.
diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
index 7359e1f..1d91a62 100644
--- a/src/maasserver/testing/factory.py
+++ b/src/maasserver/testing/factory.py
@@ -203,17 +203,29 @@ class Messages:
 
 class Factory(maastesting.factory.Factory):
 
-    def make_fake_request(self, path, method="GET", cookies={}):
+    def make_fake_request(
+            self, path="/", method="GET", cookies=None, data=None):
         """Create a fake request.
 
         :param path: The path to which to make the request.
         :param method: The method to use for the request
             ('GET' or 'POST').
-        :param cookies: A `dict` with the cookies for the request.
+        :param cookies: Optional `dict` with the cookies for the request.
+        :param data: Optional `dict` of parameters.
         """
         rf = MAASSensibleRequestFactory()
-        request = rf.get(path)
-        request.method = method
+        if data is None:
+            data = {}
+        if cookies is None:
+            cookies = {}
+        if method == "GET":
+            request = rf.get(path, data=data)
+        elif method == "POST":
+            request = rf.post(path, data=data)
+        else:
+            request = rf.get(path, data=data)
+            request.method = method
+        request.data = data
         request._messages = Messages()
         request.COOKIES = cookies.copy()
         return request
diff --git a/src/maasserver/websockets/handlers/device.py b/src/maasserver/websockets/handlers/device.py
index c3cf5c1..4c964a8 100644
--- a/src/maasserver/websockets/handlers/device.py
+++ b/src/maasserver/websockets/handlers/device.py
@@ -131,6 +131,7 @@ class DeviceHandler(NodeHandler):
             "last_image_sync",
             "default_user",
             "install_rackd",
+            "install_kvm",
             ]
         list_fields = [
             "id",
diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py
index f53a2f3..a73da82 100644
--- a/src/maasserver/websockets/handlers/machine.py
+++ b/src/maasserver/websockets/handlers/machine.py
@@ -178,6 +178,7 @@ class MachineHandler(NodeHandler):
             "managing_process",
             "last_image_sync",
             "install_rackd",
+            "install_kvm",
         ]
         list_fields = [
             "id",
diff --git a/src/metadataserver/api.py b/src/metadataserver/api.py
index b786954..e76a651 100644
--- a/src/metadataserver/api.py
+++ b/src/metadataserver/api.py
@@ -870,7 +870,20 @@ class UserDataHandler(MetadataViewHandler):
             # for user-data is when MAAS hands the node
             # off to a user.
             if node.status == NODE_STATUS.DEPLOYING:
-                node.end_deployment()
+                if node.install_kvm:
+                    # Rather than ending deployment here, note that we're
+                    # installing a KVM pod.
+                    node.agent_name = "maas-kvm-pod"
+                    node.save()
+                else:
+                    # MAAS currently considers a machine "Deployed" when the
+                    # cloud-init user data is requested. Note that this doesn't
+                    # mean the machine is ready for use yet; cloud-init will
+                    # also send a 'finish' event for the 'modules-final'
+                    # activity name. However, that check is ambiguous because
+                    # it occurs both when curtin is installing, and when
+                    # the machine reboots to finish its deployment.
+                    node.end_deployment()
             # If this node is supposed to be powered off, serve the
             # 'poweroff' userdata.
             if node.get_boot_purpose() == 'poweroff':
diff --git a/src/metadataserver/api_twisted.py b/src/metadataserver/api_twisted.py
index 08a26d3..0659d30 100644
--- a/src/metadataserver/api_twisted.py
+++ b/src/metadataserver/api_twisted.py
@@ -14,7 +14,11 @@ from maasserver.enum import (
     NODE_STATUS,
     NODE_TYPE,
 )
-from maasserver.models.node import Node
+from maasserver.forms.pods import PodForm
+from maasserver.models import (
+    Node,
+    NodeMetadata,
+)
 from maasserver.preseed import CURTIN_INSTALL_LOG
 from maasserver.utils.orm import (
     in_transaction,
@@ -175,6 +179,47 @@ class StatusHandlerResource(Resource):
         return NOT_DONE_YET
 
 
+POD_CREATION_ERROR = (
+    "Internal error while creating KVM pod. (See regiond.log for details.)"
+)
+
+
+def _create_pod_for_deployment(node):
+    virsh_password_meta = NodeMetadata.objects.filter(
+        node=node, key="virsh_password").first()
+    if virsh_password_meta is None:
+        node.mark_failed(
+            comment="Failed to deploy KVM: Password not found.", commit=False)
+    else:
+        virsh_password = virsh_password_meta.value
+        virsh_password_meta.delete()
+        # XXX: Should find the best IP to communicate with, given what the rack
+        # controller can access, or use the boot interface IP address.
+        ip = node.ip_addresses()[0]
+        if ':' in ip:
+            ip = "[%s]" % ip
+        power_address = "qemu+ssh://virsh@%s/system" % ip
+        pod_form = PodForm(data=dict(
+            type="virsh",
+            name=node.hostname,
+            power_address=power_address,
+            power_pass=virsh_password,
+            zone=node.zone.name,
+            pool=node.pool.name,
+        ), user=node.owner)
+        if pod_form.is_valid():
+            try:
+                pod_form.save()
+            except Exception:
+                node.mark_failed(comment=POD_CREATION_ERROR, commit=False)
+                log.err(None, "Exception while saving pod form.")
+            else:
+                node.status = NODE_STATUS.DEPLOYED
+        else:
+            node.mark_failed(comment=POD_CREATION_ERROR, commit=False)
+            log.msg("Error while creating KVM pod: %s" % dict(pod_form.errors))
+
+
 class StatusWorkerService(TimerService, object):
     """Service to update nodes from recieved status messages."""
 
@@ -257,7 +302,8 @@ class StatusWorkerService(TimerService, object):
         # LP:1701352 - If no exit code is given by the client default to
         # 0(pass) unless the signal is fail then set to 1(failure). This allows
         # a Curtin failure to cause the ScriptResult to fail.
-        default_exit_status = 1 if result in ['FAIL', 'FAILURE'] else 0
+        failed = result in ['FAIL', 'FAILURE']
+        default_exit_status = 1 if failed else 0
 
         # Add this event to the node event log.
         add_event_to_node_event_log(
@@ -310,7 +356,7 @@ class StatusWorkerService(TimerService, object):
                 # cloud-init may send a failure message if a script reboots
                 # the system. If a script is running which may_reboot ignore
                 # the signal.
-                if result in ['FAIL', 'FAILURE']:
+                if failed:
                     script_set = node.current_commissioning_script_set
                     if (script_set is None or not
                             script_set.scriptresult_set.filter(
@@ -323,18 +369,28 @@ class StatusWorkerService(TimerService, object):
                             script_result_status=SCRIPT_STATUS.ABORTED)
                         save_node = True
             elif node.status == NODE_STATUS.DEPLOYING:
-                if result in ['FAIL', 'FAILURE']:
+                # XXX: when activity_name == moudles-config, this currently
+                # /always/ fails, since MAAS passes two different versions
+                # for the apt configuration. The only reason why we don't
+                # see additional issues because of this is due to the node
+                # already being marked "Deployed". Right now this is prevented
+                # only in the install_kvm case, but we should make this check
+                # more general when time allows.
+                if failed and not node.install_kvm:
                     node.mark_failed(
                         comment="Installation failed (refer to the "
                                 "installation log for more information).",
                         commit=False)
                     save_node = True
+                elif (not failed and activity_name == "modules-final" and
+                      node.install_kvm and node.agent_name == "maas-kvm-pod"):
+                    save_node = True
+                    _create_pod_for_deployment(node)
             elif node.status == NODE_STATUS.DISK_ERASING:
-                if result in ['FAIL', 'FAILURE']:
+                if failed:
                     node.mark_failed(
                         comment="Failed to erase disks.", commit=False)
                     save_node = True
-
             # Deallocate the node if we enter any terminal state.
             if node.node_type == NODE_TYPE.MACHINE and node.status in [
                     NODE_STATUS.READY,
diff --git a/src/metadataserver/tests/test_api.py b/src/metadataserver/tests/test_api.py
index 998df04..2e7b6cb 100644
--- a/src/metadataserver/tests/test_api.py
+++ b/src/metadataserver/tests/test_api.py
@@ -948,6 +948,17 @@ class TestMetadataUserDataStateChanges(MAASServerTestCase):
         self.assertEqual(http.client.OK, response.status_code)
         self.assertEqual(NODE_STATUS.DEPLOYED, reload_object(node).status)
 
+    def test_skips_status_change_if_installing_kvm_and_sets_agent_name(self):
+        node = factory.make_Node(
+            status=NODE_STATUS.DEPLOYING, install_kvm=True)
+        NodeUserData.objects.set_user_data(node, sample_binary_data)
+        client = make_node_client(node)
+        response = client.get(reverse('metadata-user-data', args=['latest']))
+        self.assertEqual(http.client.OK, response.status_code)
+        self.assertEqual(NODE_STATUS.DEPLOYING, reload_object(node).status)
+        node = reload_object(node)
+        self.assertEqual(node.agent_name, "maas-kvm-pod")
+
 
 class TestCurtinMetadataUserData(
         PreseedRPCMixin, MAASTransactionServerTestCase):
diff --git a/src/metadataserver/tests/test_api_twisted.py b/src/metadataserver/tests/test_api_twisted.py
index e8c7e34..65e5014 100644
--- a/src/metadataserver/tests/test_api_twisted.py
+++ b/src/metadataserver/tests/test_api_twisted.py
@@ -24,6 +24,7 @@ from crochet import wait_for
 from maasserver.enum import NODE_STATUS
 from maasserver.models import (
     Event,
+    NodeMetadata,
     Tag,
 )
 from maasserver.models.signals.testing import SignalsDisabled
@@ -42,13 +43,19 @@ from maasserver.utils.orm import (
 )
 from maasserver.utils.threads import deferToDatabase
 from maastesting.matchers import (
+    DocTestMatches,
     MockCalledOnceWith,
     MockCallsMatch,
     MockNotCalled,
 )
 from maastesting.testcase import MAASTestCase
-from metadataserver import api
+from metadataserver import (
+    api,
+    api_twisted as api_twisted_module,
+)
 from metadataserver.api_twisted import (
+    _create_pod_for_deployment,
+    POD_CREATION_ERROR,
     StatusHandlerResource,
     StatusWorkerService,
 )
@@ -60,6 +67,7 @@ from metadataserver.models import NodeKey
 from testtools import ExpectedException
 from testtools.matchers import (
     Equals,
+    Is,
     MatchesListwise,
     MatchesSetwise,
 )
@@ -343,7 +351,7 @@ def encode_as_base64(content):
 class TestStatusWorkerService(MAASServerTestCase):
 
     def setUp(self):
-        super(TestStatusWorkerService, self).setUp()
+        super().setUp()
         self.useFixture(SignalsDisabled("power"))
 
     def processMessage(self, node, payload):
@@ -505,6 +513,23 @@ class TestStatusWorkerService(MAASServerTestCase):
             " log for more information).",
             Event.objects.filter(node=node).last().description)
 
+    def test_status_ok_for_modules_final_triggers_kvm_install(self):
+        node = factory.make_Node(
+            interface=True, status=NODE_STATUS.DEPLOYING,
+            agent_name="maas-kvm-pod", install_kvm=True)
+        payload = {
+            'event_type': 'finish',
+            'result': 'OK',
+            'origin': 'cloud-init',
+            'name': 'modules-final',
+            'description': 'America for Make Benefit Glorious Nation',
+            'timestamp': datetime.utcnow(),
+        }
+        mock_create_pod = self.patch(
+            api_twisted_module, '_create_pod_for_deployment')
+        self.processMessage(node, payload)
+        self.assertThat(mock_create_pod, MockCalledOnceWith(node))
+
     def test_status_installation_fail_leaves_node_failed(self):
         node = factory.make_Node(interface=True, status=NODE_STATUS.DEPLOYING)
         payload = {
@@ -987,3 +1012,71 @@ class TestStatusWorkerService(MAASServerTestCase):
             node.status_expires, expected_time - timedelta(minutes=1))
         self.assertLessEqual(
             node.status_expires, expected_time + timedelta(minutes=1))
+
+
+class TestCreatePodForDeployment(MAASServerTestCase):
+
+    def setUp(self):
+        super().setUp()
+        self.mock_PodForm = self.patch(api_twisted_module, "PodForm")
+
+    def test__marks_failed_if_no_virsh_password(self):
+        node = factory.make_Node(
+            interface=True, status=NODE_STATUS.DEPLOYING,
+            agent_name="maas-kvm-pod", install_kvm=True)
+        _create_pod_for_deployment(node)
+        self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT))
+        self.assertThat(node.error_description, DocTestMatches(
+            "...Password not found..."))
+
+    def test__deletes_virsh_password_metadata_and_sets_deployed(self):
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod",
+            install_kvm=True)
+        factory.make_StaticIPAddress(interface=node.boot_interface)
+        meta = NodeMetadata.objects.create(
+            node=node, key="virsh_password", value="xyz123")
+        _create_pod_for_deployment(node)
+        meta = reload_object(meta)
+        self.assertThat(meta, Is(None))
+        self.assertThat(node.status, Equals(NODE_STATUS.DEPLOYED))
+
+    def test__marks_failed_if_is_valid_returns_false(self):
+        mock_pod_form = Mock()
+        self.mock_PodForm.return_value = mock_pod_form
+        mock_pod_form.errors = {}
+        mock_pod_form.is_valid = Mock()
+        mock_pod_form.is_valid.return_value = False
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod",
+            install_kvm=True)
+        factory.make_StaticIPAddress(interface=node.boot_interface)
+        meta = NodeMetadata.objects.create(
+            node=node, key="virsh_password", value="xyz123")
+        _create_pod_for_deployment(node)
+        meta = reload_object(meta)
+        self.assertThat(meta, Is(None))
+        self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT))
+        self.assertThat(node.error_description, DocTestMatches(
+            POD_CREATION_ERROR))
+
+    def test__marks_failed_if_save_raises(self):
+        mock_pod_form = Mock()
+        self.mock_PodForm.return_value = mock_pod_form
+        mock_pod_form.errors = {}
+        mock_pod_form.is_valid = Mock()
+        mock_pod_form.is_valid.return_value = True
+        mock_pod_form.save = Mock()
+        mock_pod_form.save.side_effect = ValueError
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, agent_name="maas-kvm-pod",
+            install_kvm=True)
+        factory.make_StaticIPAddress(interface=node.boot_interface)
+        meta = NodeMetadata.objects.create(
+            node=node, key="virsh_password", value="xyz123")
+        _create_pod_for_deployment(node)
+        meta = reload_object(meta)
+        self.assertThat(meta, Is(None))
+        self.assertThat(node.status, Equals(NODE_STATUS.FAILED_DEPLOYMENT))
+        self.assertThat(node.error_description, DocTestMatches(
+            POD_CREATION_ERROR))
diff --git a/src/metadataserver/tests/test_vendor_data.py b/src/metadataserver/tests/test_vendor_data.py
index bd0475f..3b1a6d3 100644
--- a/src/metadataserver/tests/test_vendor_data.py
+++ b/src/metadataserver/tests/test_vendor_data.py
@@ -5,7 +5,10 @@
 
 __all__ = []
 
-from maasserver.models.config import Config
+from maasserver.models import (
+    Config,
+    NodeMetadata,
+)
 from maasserver.server_address import get_maas_facing_server_host
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -21,6 +24,7 @@ from testtools.matchers import (
     Contains,
     ContainsDict,
     Equals,
+    HasLength,
     Is,
     IsInstance,
     KeysEqual,
@@ -221,3 +225,20 @@ class TestGenerateRackControllerConfiguration(MAASServerTestCase):
                 "%s --maas-url %s --secret %s" % (cmd, maas_url, secret),
                 ]
             }))
+
+    def test_yields_configuration_when_machine_install_kvm_true(self):
+        node = factory.make_Node(osystem='ubuntu', netboot=False)
+        node.install_kvm = True
+        configuration = get_vendor_data(node)
+        config = str(dict(configuration))
+        self.assertThat(config, Contains("virsh"))
+        self.assertThat(config, Contains("ssh_pwauth"))
+        self.assertThat(config, Contains("rbash"))
+        self.assertThat(config, Contains("libvirt-qemu"))
+        self.assertThat(config, Contains("ForceCommand"))
+        self.assertThat(config, Contains("qemu-kvm"))
+        self.assertThat(config, Contains("libvirt-bin"))
+        # Check that a password was saved for the pod-to-be.
+        virsh_password_meta = NodeMetadata.objects.filter(
+            node=node, key="virsh_password").first()
+        self.assertThat(virsh_password_meta.value, HasLength(32))
diff --git a/src/metadataserver/vendor_data.py b/src/metadataserver/vendor_data.py
index a0c689d..b6bbf72 100644
--- a/src/metadataserver/vendor_data.py
+++ b/src/metadataserver/vendor_data.py
@@ -7,10 +7,16 @@ __all__ = [
     'get_vendor_data',
     ]
 
+from base64 import b64encode
+from crypt import crypt
 from itertools import chain
+from os import urandom
 
 from maasserver import ntp
-from maasserver.models import Config
+from maasserver.models import (
+    Config,
+    NodeMetadata,
+)
 from maasserver.server_address import get_maas_facing_server_host
 from netaddr import IPAddress
 from provisioningserver.ntp.config import normalise_address
@@ -23,6 +29,7 @@ def get_vendor_data(node):
         generate_system_info(node),
         generate_ntp_configuration(node),
         generate_rack_controller_configuration(node),
+        generate_kvm_pod_configuration(node),
     ))
 
 
@@ -91,3 +98,56 @@ def generate_rack_controller_configuration(node):
             "/snap/bin/maas init --mode rack --maas-url %s --secret %s" % (
                 maas_url, secret)
         ]
+
+
+def generate_kvm_pod_configuration(node):
+    """Generate cloud-init configuration to install the node as a KVM pod."""
+    if node.netboot is False and node.install_kvm is True:
+        yield "runcmd", [
+            # Restrict the $PATH so that rbash can be used to limit what the
+            # virsh user can do if they manage to get a shell.
+            ['mkdir', '-p', '/home/virsh/bin'],
+            ['ln', '-s', '/usr/bin/virsh', '/home/virsh/bin/virsh'],
+            ['sh', '-c', 'echo "PATH=/home/virsh/bin" >> /home/virsh/.bashrc'],
+            # Use a ForceCommand to make sure the only thing the virsh user
+            # can do with SSH is communicate with libvirt.
+            ['sh', '-c',
+                'printf "Match user virsh\\n'
+                '    X11Forwarding no\\n'
+                '    AllowTcpForwarding no\\n'
+                '    PermitTTY no\\n'
+                '    ForceCommand nc -q 0 -U /var/run/libvirt/libvirt-sock\\n"'
+                '  >> /etc/ssh/sshd_config'],
+            # Make sure the 'virsh' user is allowed to access libvirt.
+            ['/usr/sbin/usermod', '--append', '--groups',
+                'libvirt,libvirt-qemu', 'virsh'],
+            # SSH needs to be restarted in order for the above changes to
+            # take effect.
+            ['systemctl', 'restart', 'sshd'],
+            # Ensure services are ready before cloud-init finishes.
+            ['/bin/sleep', '10'],
+        ]
+        # Generate a 32-character password by encoding 24 bytes as base64.
+        virsh_password = b64encode(
+            urandom(24), altchars=b'.!').decode('ascii')
+        # Pass crypted (salted/hashed) version of the password to cloud-init.
+        encrypted_password = crypt(virsh_password)
+        # Store a cleartext version of the password so we can add a pod later.
+        NodeMetadata.objects.update_or_create(
+            node=node, key="virsh_password",
+            defaults=dict(value=virsh_password))
+        # Make sure SSH password authentication is enabled.
+        yield "ssh_pwauth", True
+        # Create a custom 'virsh' user (in addition to the default user)
+        # with the encrypted password, and a locked-down shell.
+        yield "users", [
+            'default',
+            {
+                'name': 'virsh',
+                'lock_passwd': False,
+                'passwd': encrypted_password,
+                'shell': '/bin/rbash',
+            }
+        ]
+        # XXX: Use correct packages based on architecture.
+        yield "packages", ["qemu-kvm", "libvirt-bin"]