Merge lp:~rvb/maas/poweroff-redux into lp:~maas-committers/maas/trunk

[Raphaël Badin]

In order to power a node off, don't use poweroff.c32/poweroff.com. Instead, bring up the node in an ephemeral environment and issue a `poweroff` command. This will enable MAAS to power off nodes without APM support.

I tested this manually on my NUCs and I got it through the CI (http://d-jenkins.ubuntu-ci:8080/view/MAAS/job/utopic-trunk-adt-maas-manual/30/).

Once this lands, we can update the CI code to use the NUCs we have in the CI lab and that we couldn't use so far because of the bug this branch is fixing.

[Gavin Panella (allenap)]

Cool. Should work everywhere, as long as the machine can power itself off. Machines that can't aren't really MAAS's core audience (or peripheral either). Lots of comments but none of them are blockers.

review: Approve

[Raphaël Badin (rvb)]

Thanks for the review. I think I've addressed most of your comments but I confess I didn't do a full refactoring of the get_boot_purpose() code.

[MAAS Lander (maas-lander)]

There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions.

=== added file 'etc/maas/templates/commissioning-user-data/user_data_poweroff.template'
--- etc/maas/templates/commissioning-user-data/user_data_poweroff.template	1970-01-01 00:00:00 +0000
+++ etc/maas/templates/commissioning-user-data/user_data_poweroff.template	2014-12-05 16:13:51 +0000
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+#### script setup ######
+PATH="$BIN_D:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+
+write_poweroff_job() {
+   cat >/etc/init/maas-poweroff.conf <<EOF
+   description "Power-off when MAAS task is done"
+   start on stopped cloud-final
+   console output
+   task
+   script
+     [ ! -e /tmp/block-poweroff ] || exit 0
+     /sbin/poweroff
+   end script
+EOF
+   # reload required due to lack of inotify in overlayfs (LP: #882147)
+   initctl reload-configuration
+}
+
+main() {
+   write_poweroff_job
+
+   echo "Powering node off."
+}
+
+main
+exit

=== modified file 'src/maasserver/api/pxeconfig.py'
--- src/maasserver/api/pxeconfig.py	2014-11-28 15:38:33 +0000
+++ src/maasserver/api/pxeconfig.py	2014-12-05 16:13:51 +0000
@@ -226,24 +226,19 @@
         osystem = Config.objects.get_config('commissioning_osystem')
         series = Config.objects.get_config('commissioning_distro_series')
 
-    # If we are powering off the node, then we only need to return enough
-    # to turn the machine off. Calculation of the label, kernel parameters,
-    # server address, and cluster address is not needed.
     if purpose == 'poweroff':
-        params = KernelParameters(
-            osystem="", arch=arch, subarch=subarch, release="",
-            label="", purpose=purpose, hostname=hostname, domain=domain,
-            preseed_url="", log_host="", fs_host="", extra_opts="")
-        return HttpResponse(
-            json.dumps(params._asdict()),
-            content_type="application/json")
+        # In order to power the node off, we need to get it booted in the
+        # commissioning environment and issue a `poweroff` command.
+        boot_purpose = 'commissioning'
+    else:
+        boot_purpose = purpose
 
     # We use as our default label the label of the most recent image for
     # the criteria we've assembled above. If there is no latest image
     # (which should never happen in reality but may happen in tests), we
     # fall back to using 'no-such-image' as our default.
     latest_image = get_boot_image(
-        nodegroup, osystem, arch, subarch, series, purpose)
+        nodegroup, osystem, arch, subarch, series, boot_purpose)
     if latest_image is None:
         # XXX 2014-03-18 gmb bug=1294131:
         #     We really ought to raise an exception here so that client
@@ -291,7 +286,7 @@
 
     params = KernelParameters(
         osystem=osystem, arch=arch, subarch=subarch, release=series,
-        label=label, purpose=purpose, hostname=hostname, domain=domain,
+        label=label, purpose=boot_purpose, hostname=hostname, domain=domain,
         preseed_url=preseed_url, log_host=server_address,
         fs_host=cluster_address, extra_opts=extra_kernel_opts)
 

=== modified file 'src/maasserver/api/tests/test_pxeconfig.py'
--- src/maasserver/api/tests/test_pxeconfig.py	2014-11-28 15:44:21 +0000
+++ src/maasserver/api/tests/test_pxeconfig.py	2014-12-05 16:13:51 +0000
@@ -419,21 +419,26 @@
         pxe_config = self.get_pxeconfig(params)
         self.assertEqual(None, pxe_config['extra_opts'])
 
-    def test_pxeconfig_returns_poweroff_for_insane_state(self):
+    def test_pxeconfig_returns_commissioning_for_insane_state(self):
         mac = factory.make_MACAddress_with_Node()
         params = self.get_default_params()
         params['mac'] = mac.mac_address
         pxe_config = self.get_pxeconfig(params)
-        self.assertEqual('poweroff', pxe_config['purpose'])
+        # The 'purpose' of the PXE config is 'commissioning' here
+        # even if the 'purpose' returned by node.get_boot_purpose
+        # is 'poweroff' because MAAS needs to bring the machine
+        # up in a commissioning environment in order to power
+        # the machine down.
+        self.assertEqual('commissioning', pxe_config['purpose'])
 
-    def test_pxeconfig_returns_poweroff_for_ready_node(self):
+    def test_pxeconfig_returns_commissioning_for_ready_node(self):
         mac = factory.make_MACAddress_with_Node()
         mac.node.status = NODE_STATUS.READY
         mac.node.save()
         params = self.get_default_params()
         params['mac'] = mac.mac_address
         pxe_config = self.get_pxeconfig(params)
-        self.assertEqual('poweroff', pxe_config['purpose'])
+        self.assertEqual('commissioning', pxe_config['purpose'])
 
     def test_pxeconfig_returns_image_subarch_not_node_subarch(self):
         # In the scenario such as deploying trusty on an hwe-s subarch

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-11-26 10:11:28 +0000
+++ src/maasserver/models/node.py	2014-12-05 16:13:51 +0000
@@ -1396,8 +1396,8 @@
             # otherwise boot locally.
             if self.netboot:
                 # Avoid circular imports.
-                from maasserver.preseed import get_preseed_type_for
-                preseed_type = get_preseed_type_for(self)
+                from maasserver.preseed import get_deploying_preseed_type_for
+                preseed_type = get_deploying_preseed_type_for(self)
                 if preseed_type == PRESEED_TYPE.CURTIN:
                     return "xinstall"
                 else:

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2014-11-07 13:16:58 +0000
+++ src/maasserver/preseed.py	2014-12-05 16:13:51 +0000
@@ -338,8 +338,17 @@
     using the default installer but there is no boot image that supports
     that method then it will boot using the fast-path installer.
     """
-    if node.status in COMMISSIONING_LIKE_STATUSES:
+    is_commissioning_preseed = (
+        node.status in COMMISSIONING_LIKE_STATUSES or
+        node.get_boot_purpose() == 'poweroff'
+        )
+    if is_commissioning_preseed:
         return PRESEED_TYPE.COMMISSIONING
+    else:
+        return get_deploying_preseed_type_for(node)
+
+
+def get_deploying_preseed_type_for(node):
     if node.boot_type == NODE_BOOT.FASTPATH:
         purpose_order = ['xinstall', 'install']
     elif node.boot_type == NODE_BOOT.DEBIAN:

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2014-10-09 16:32:04 +0000
+++ src/maasserver/tests/test_preseed.py	2014-12-05 16:13:51 +0000
@@ -666,7 +666,8 @@
         self.return_windows_specific_preseed_data()
         node = factory.make_Node(
             nodegroup=self.rpc_nodegroup, osystem='windows',
-            architecture='amd64/generic', distro_series=self.release)
+            architecture='amd64/generic', distro_series=self.release,
+            status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'install')
         preseed = render_preseed(
             node, '', osystem='windows', release=self.release)
@@ -1207,29 +1208,42 @@
             PRESEED_TYPE.COMMISSIONING, get_preseed_type_for(node))
 
     def test_get_preseed_type_for_default(self):
-        node = factory.make_Node(boot_type=NODE_BOOT.DEBIAN)
+        node = factory.make_Node(
+            boot_type=NODE_BOOT.DEBIAN, status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'install')
         self.assertEqual(
             PRESEED_TYPE.DEFAULT, get_preseed_type_for(node))
 
     def test_get_preseed_type_for_curtin(self):
-        node = factory.make_Node(boot_type=NODE_BOOT.FASTPATH)
+        node = factory.make_Node(
+            boot_type=NODE_BOOT.FASTPATH, status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'xinstall')
         self.assertEqual(
             PRESEED_TYPE.CURTIN, get_preseed_type_for(node))
 
     def test_get_preseed_type_for_default_when_curtin_not_supported(self):
-        node = factory.make_Node(boot_type=NODE_BOOT.FASTPATH)
+        node = factory.make_Node(
+            boot_type=NODE_BOOT.FASTPATH, status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'install')
         self.assertEqual(
             PRESEED_TYPE.DEFAULT, get_preseed_type_for(node))
 
     def test_get_preseed_type_for_curtin_when_default_not_supported(self):
-        node = factory.make_Node(boot_type=NODE_BOOT.DEBIAN)
+        node = factory.make_Node(
+            boot_type=NODE_BOOT.DEBIAN, status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'xinstall')
         self.assertEqual(
             PRESEED_TYPE.CURTIN, get_preseed_type_for(node))
 
+    def test_get_preseed_type_for_poweroff(self):
+        # A 'ready' node isn't supposed to be powered on and thus
+        # will get a 'commissioning' preseed in order to be powered
+        # down.
+        node = factory.make_Node(
+            boot_type=NODE_BOOT.DEBIAN, status=NODE_STATUS.READY)
+        self.assertEqual(
+            PRESEED_TYPE.COMMISSIONING, get_preseed_type_for(node))
+
 
 class TestRenderPreseedArchives(
         PreseedRPCMixin, BootImageHelperMixin, MAASServerTestCase):
@@ -1239,10 +1253,12 @@
         nodes = [
             factory.make_Node(
                 nodegroup=self.rpc_nodegroup,
+                status=NODE_STATUS.DEPLOYING,
                 architecture=make_usable_architecture(
                     self, arch_name="i386", subarch_name="generic")),
             factory.make_Node(
                 nodegroup=self.rpc_nodegroup,
+                status=NODE_STATUS.DEPLOYING,
                 architecture=make_usable_architecture(
                     self, arch_name="amd64", subarch_name="generic")),
             ]
@@ -1314,14 +1330,16 @@
 
     def test_get_preseed_returns_default_preseed(self):
         node = factory.make_Node(
-            nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.DEBIAN)
+            nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.DEBIAN,
+            status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'install')
         preseed = get_preseed(node)
         self.assertIn('preseed/late_command', preseed)
 
     def test_get_preseed_returns_curtin_preseed(self):
         node = factory.make_Node(
-            nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.FASTPATH)
+            nodegroup=self.rpc_nodegroup, boot_type=NODE_BOOT.FASTPATH,
+            status=NODE_STATUS.DEPLOYING)
         self.configure_get_boot_images_for_node(node, 'xinstall')
         preseed = get_preseed(node)
         curtin_url = reverse('curtin-metadata')

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2014-11-12 03:27:06 +0000
+++ src/metadataserver/api.py	2014-12-05 16:13:51 +0000
@@ -78,6 +78,7 @@
 from metadataserver.models.commissioningscript import (
     BUILTIN_COMMISSIONING_SCRIPTS,
     )
+from metadataserver.user_data import poweroff
 from piston.utils import rc
 from provisioningserver.events import (
     EVENT_DETAILS,
@@ -424,9 +425,14 @@
             # off to a user.
             if node.status == NODE_STATUS.DEPLOYING:
                 node.end_deployment()
+            # If this node is supposed to be powered off, serve the
+            # 'poweroff' userdata.
+            if node.get_boot_purpose() == 'poweroff':
+                user_data = poweroff.generate_user_data(node=node)
+            else:
+                user_data = NodeUserData.objects.get_user_data(node)
             return HttpResponse(
-                NodeUserData.objects.get_user_data(node),
-                mimetype='application/octet-stream')
+                user_data, mimetype='application/octet-stream')
         except NodeUserData.DoesNotExist:
             logger.info(
                 "No user data registered for node named %s" % node.hostname)

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2014-11-25 13:06:34 +0000
+++ src/metadataserver/tests/test_api.py	2014-12-05 16:13:51 +0000
@@ -62,6 +62,7 @@
     make_list_response,
     make_text_response,
     MetaDataHandler,
+    poweroff as api_poweroff,
     UnknownMetadataVersion,
     )
 from metadataserver.models import (
@@ -383,7 +384,7 @@
     """Tests for the metadata user-data API endpoint."""
 
     def test_user_data_view_returns_binary_data(self):
-        node = factory.make_Node()
+        node = factory.make_Node(status=NODE_STATUS.COMMISSIONING)
         NodeUserData.objects.set_user_data(node, sample_binary_data)
         client = make_node_client(node)
         response = client.get(reverse('metadata-user-data', args=['latest']))
@@ -393,8 +394,22 @@
             (httplib.OK, sample_binary_data),
             (response.status_code, response.content))
 
+    def test_poweroff_user_data_returned_if_unexpected_status(self):
+        node = factory.make_Node(status=NODE_STATUS.READY)
+        NodeUserData.objects.set_user_data(node, sample_binary_data)
+        client = make_node_client(node)
+        user_data = factory.make_name('user data').encode("ascii")
+        self.patch(api_poweroff, 'generate_user_data').return_value = user_data
+        response = client.get(reverse('metadata-user-data', args=['latest']))
+        self.assertEqual('application/octet-stream', response['Content-Type'])
+        self.assertIsInstance(response.content, bytes)
+        self.assertEqual(
+            (httplib.OK, user_data),
+            (response.status_code, response.content))
+
     def test_user_data_for_node_without_user_data_returns_not_found(self):
-        client = make_node_client()
+        client = make_node_client(
+            factory.make_Node(status=NODE_STATUS.COMMISSIONING))
         response = client.get(reverse('metadata-user-data', args=['latest']))
         self.assertEqual(httplib.NOT_FOUND, response.status_code)
 
@@ -938,7 +953,8 @@
             (response.status_code, response.content))
 
     def test_api_retrieves_node_userdata_by_mac(self):
-        mac = factory.make_MACAddress_with_Node()
+        mac = factory.make_MACAddress_with_Node(
+            node=factory.make_Node(status=NODE_STATUS.COMMISSIONING))
         user_data = factory.make_string().encode('ascii')
         NodeUserData.objects.set_user_data(mac.node, user_data)
         url = reverse(

=== added file 'src/metadataserver/user_data/poweroff.py'
--- src/metadataserver/user_data/poweroff.py	1970-01-01 00:00:00 +0000
+++ src/metadataserver/user_data/poweroff.py	2014-12-05 16:13:51 +0000
@@ -0,0 +1,34 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Poweroff userdata generation."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = [
+    "generate_user_data",
+]
+
+from metadataserver.user_data.snippets import get_userdata_template_dir
+from metadataserver.user_data.utils import (
+    generate_user_data as _generate_user_data,
+    )
+
+
+def generate_user_data(node):
+    """Produce the poweroff script.
+
+    :rtype: `bytes`
+    """
+    userdata_dir = get_userdata_template_dir()
+    result = _generate_user_data(
+        node, userdata_dir, 'user_data_poweroff.template',
+        'user_data_config.template')
+    return result

=== added file 'src/metadataserver/user_data/tests/test_poweroff.py'
--- src/metadataserver/user_data/tests/test_poweroff.py	1970-01-01 00:00:00 +0000
+++ src/metadataserver/user_data/tests/test_poweroff.py	2014-12-05 16:13:51 +0000
@@ -0,0 +1,31 @@
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test generation of poweroff user data."""
+
+from __future__ import (
+    absolute_import,
+    print_function,
+    unicode_literals,
+    )
+
+str = None
+
+__metaclass__ = type
+__all__ = []
+
+from maasserver.testing.factory import factory
+from maasserver.testing.testcase import MAASServerTestCase
+from metadataserver.user_data.poweroff import generate_user_data
+from testtools.matchers import ContainsAll
+
+
+class TestPoweroffUserData(MAASServerTestCase):
+
+    def test_generate_user_data_produces_poweroff_script(self):
+        node = factory.make_Node()
+        self.assertThat(
+            generate_user_data(node), ContainsAll({
+                'Powering node off',
+                'poweroff',
+            }))

=== modified file 'src/provisioningserver/boot/__init__.py'
--- src/provisioningserver/boot/__init__.py	2014-11-17 11:56:49 +0000
+++ src/provisioningserver/boot/__init__.py	2014-12-05 16:13:51 +0000
@@ -26,7 +26,6 @@
 from io import BytesIO
 from os import path
 
-from provisioningserver import config
 from provisioningserver.boot.tftppath import compose_image_path
 from provisioningserver.kernel_opts import compose_kernel_command_line
 from provisioningserver.utils import locate_config
@@ -107,18 +106,6 @@
     return find_mac_via_arp(remote_host)
 
 
-def compose_poweroff_command():
-    """Composes the poweroff command depending on the version of
-    syslinux that is avaliable in the tftproot."""
-    com32_path = path.join(
-        config.BOOT_RESOURCES_STORAGE, "current", "syslinux", "poweroff.c32")
-    if path.exists(com32_path):
-        # syslinux 6 uses a com32 module for poweroff
-        return "COM32 /syslinux/poweroff.c32"
-    # syslinux 4 uses the older type
-    return "KERNEL /syslinux/poweroff.com"
-
-
 class BootMethod:
     """Skeleton for a boot method."""
 
@@ -245,9 +232,6 @@
             "kernel_path": kernel_path,
             }
 
-        if kernel_params.purpose == 'poweroff':
-            namespace['poweroff'] = compose_poweroff_command()
-
         return namespace
 
 

=== modified file 'src/provisioningserver/boot/tests/test_boot.py'
--- src/provisioningserver/boot/tests/test_boot.py	2014-09-13 11:38:50 +0000
+++ src/provisioningserver/boot/tests/test_boot.py	2014-12-05 16:13:51 +0000
@@ -25,18 +25,13 @@
     MAASTwistedRunTest,
     )
 import mock
-from provisioningserver import (
-    boot,
-    config,
-    )
+from provisioningserver import boot
 from provisioningserver.boot import (
     BootMethod,
     BytesReader,
-    compose_poweroff_command,
     gen_template_filenames,
     get_remote_mac,
     )
-from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
 import tempita
 from twisted.internet.defer import inlineCallbacks
 from twisted.python import context
@@ -155,32 +150,3 @@
         self.assertRaises(
             IOError, method.get_template,
             *factory.make_names("purpose", "arch", "subarch"))
-
-    def test_compose_poweroff_command_for_syslinux_6(self):
-        storage_dir = self.make_dir()
-        syslinux_path = os.path.join(storage_dir, "current", "syslinux")
-        os.makedirs(syslinux_path)
-        factory.make_file(syslinux_path, "poweroff.c32", contents=None)
-        self.patch(config, 'BOOT_RESOURCES_STORAGE', storage_dir)
-        self.assertEqual(
-            "COM32 /syslinux/poweroff.c32",
-            compose_poweroff_command())
-
-    def test_compose_poweroff_command_for_syslinux_4(self):
-        storage_dir = self.make_dir()
-        syslinux_path = os.path.join(storage_dir, "current", "syslinux")
-        os.makedirs(syslinux_path)
-        factory.make_file(syslinux_path, "poweroff.com", contents=None)
-        self.patch(config, 'BOOT_RESOURCES_STORAGE', storage_dir)
-        self.assertEqual(
-            "KERNEL /syslinux/poweroff.com",
-            compose_poweroff_command())
-
-    def test_compose_template_namespace_includes_poweroff(self):
-        fake_poweroff = factory.make_name('poweroff')
-        self.patch(
-            boot, 'compose_poweroff_command').return_value = fake_poweroff
-        method = FakeBootMethod()
-        kernel_params = make_kernel_parameters(purpose="poweroff")
-        namespace = method.compose_template_namespace(kernel_params)
-        self.assertEqual(fake_poweroff, namespace['poweroff'])