Merge lp:~cjwatson/launchpad-buildd/build-livefs-operation into lp:launchpad-buildd

[Colin Watson]

Convert buildlivefs to the new Operation framework and add unit tests.

[lp:~cjwatson/launchpad-buildd/build-livefs-operation]

259. By Colin Watson on 2017-08-07

Silence a logging warning.

[William Grant (wgrant)]

review: Approve (code)

[lp:~cjwatson/launchpad-buildd/build-livefs-operation]

260. By Colin Watson on 2017-08-22

Merge trunk.

=== modified file 'MANIFEST.in'
--- MANIFEST.in	2017-08-22 14:46:10 +0000
+++ MANIFEST.in	2017-08-22 16:37:45 +0000
@@ -1,6 +1,5 @@
 include LICENSE
 include Makefile
-include bin/buildlivefs
 include bin/buildrecipe
 include bin/buildsnap
 include bin/generate-translation-templates

=== modified file 'debian/changelog'
--- debian/changelog	2017-08-22 16:27:41 +0000
+++ debian/changelog	2017-08-22 16:37:45 +0000
@@ -22,6 +22,7 @@
   * Rewrite scan-for-processes in Python, allowing it to have unit tests.
   * Improve "RUN:" log messages to be copy-and-pasteable as shell commands,
     which is sometimes useful while debugging.
+  * Convert buildlivefs to the new Operation framework and add unit tests.
 
  -- Colin Watson <[e-mail]>  Tue, 25 Jul 2017 23:07:58 +0100
 

=== modified file 'debian/launchpad-buildd.install'
--- debian/launchpad-buildd.install	2017-08-22 14:46:10 +0000
+++ debian/launchpad-buildd.install	2017-08-22 16:37:45 +0000
@@ -3,7 +3,6 @@
 debian/upgrade-config			usr/share/launchpad-buildd
 sbuildrc				usr/share/launchpad-buildd
 template-buildd-slave.conf		usr/share/launchpad-buildd
-bin/buildlivefs				usr/share/launchpad-buildd/slavebin
 bin/buildrecipe				usr/share/launchpad-buildd/slavebin
 bin/buildsnap				usr/share/launchpad-buildd/slavebin
 bin/generate-translation-templates	usr/share/launchpad-buildd/slavebin

=== modified file 'lpbuildd/livefs.py'
--- lpbuildd/livefs.py	2017-08-05 09:43:43 +0000
+++ lpbuildd/livefs.py	2017-08-22 16:37:45 +0000
@@ -25,10 +25,6 @@
 
     initial_build_state = LiveFilesystemBuildState.BUILD_LIVEFS
 
-    def __init__(self, slave, buildid, **kwargs):
-        DebianBuildManager.__init__(self, slave, buildid, **kwargs)
-        self.build_livefs_path = os.path.join(self._slavebin, "buildlivefs")
-
     def initiate(self, files, chroot, extra_args):
         """Initiate a build with a given set of files and chroot."""
         self.subarch = extra_args.get("subarch")
@@ -45,17 +41,12 @@
 
     def doRunBuild(self):
         """Run the process to build the live filesystem."""
-        args = [
-            "buildlivefs",
-            "--build-id", self._buildid,
-            "--arch", self.arch_tag,
-            ]
+        args = []
         if self.subarch:
             args.extend(["--subarch", self.subarch])
         args.extend(["--project", self.project])
         if self.subproject:
             args.extend(["--subproject", self.subproject])
-        args.extend(["--series", self.series])
         if self.datestamp:
             args.extend(["--datestamp", self.datestamp])
         if self.image_format:
@@ -66,7 +57,7 @@
             args.extend(["--locale", self.locale])
         for ppa in self.extra_ppas:
             args.extend(["--extra-ppa", ppa])
-        self.runSubProcess(self.build_livefs_path, args)
+        self.runTargetSubProcess("buildlivefs", *args)
 
     def iterate_BUILD_LIVEFS(self, retcode):
         """Finished building the live filesystem."""

=== modified file 'lpbuildd/target/backend.py'
--- lpbuildd/target/backend.py	2017-08-22 15:55:44 +0000
+++ lpbuildd/target/backend.py	2017-08-22 16:37:45 +0000
@@ -37,13 +37,14 @@
         raise NotImplementedError
 
     def run(self, args, env=None, input_text=None, get_output=False,
-            **kwargs):
+            echo=False, **kwargs):
         """Run a command in the target environment.
 
         :param args: the command and arguments to run.
         :param env: additional environment variables to set.
         :param input_text: input text to pass on the command's stdin.
         :param get_output: if True, return the output from the command.
+        :param echo: if True, print the command before executing it.
         :param kwargs: additional keyword arguments for `subprocess.Popen`.
         """
         raise NotImplementedError

=== renamed file 'bin/buildlivefs' => 'lpbuildd/target/build_livefs.py' (properties changed: +x to -x)
--- bin/buildlivefs	2017-07-28 11:15:51 +0000
+++ lpbuildd/target/build_livefs.py	2017-08-22 16:37:45 +0000
@@ -1,30 +1,25 @@
-#! /usr/bin/python -u
 # Copyright 2013-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""A script that builds a live file system."""
-
 from __future__ import print_function
 
 __metaclass__ = type
 
-from optparse import OptionParser
+from collections import OrderedDict
+import logging
 import os
-import subprocess
-import sys
-import traceback
-
-from lpbuildd.util import (
-    set_personality,
-    shell_escape,
-    )
-
-
-RETCODE_SUCCESS = 0
+
+from lpbuildd.target.operation import Operation
+from lpbuildd.util import shell_escape
+
+
 RETCODE_FAILURE_INSTALL = 200
 RETCODE_FAILURE_BUILD = 201
 
 
+logger = logging.getLogger(__name__)
+
+
 def get_build_path(build_id, *extra):
     """Generate a path within the build directory.
 
@@ -35,27 +30,34 @@
     return os.path.join(os.environ["HOME"], "build-" + build_id, *extra)
 
 
-class LiveFSBuilder:
-    """Builds a live file system."""
-
-    def __init__(self, options):
-        self.options = options
-        self.chroot_path = get_build_path(
-            self.options.build_id, 'chroot-autobuild')
-
-    def chroot(self, args, echo=False):
-        """Run a command in the chroot.
-
-        :param args: the command and arguments to run.
-        """
-        args = set_personality(
-            args, self.options.arch, series=self.options.series)
-        if echo:
-            print("Running in chroot: %s" %
-                  ' '.join("'%s'" % arg for arg in args))
-            sys.stdout.flush()
-        subprocess.check_call([
-            "/usr/bin/sudo", "/usr/sbin/chroot", self.chroot_path] + args)
+class BuildLiveFS(Operation):
+
+    description = "Build a live file system."
+
+    @classmethod
+    def add_arguments(cls, parser):
+        super(BuildLiveFS, cls).add_arguments(parser)
+        parser.add_argument(
+            "--subarch", metavar="SUBARCH",
+            help="build for subarchitecture SUBARCH")
+        parser.add_argument(
+            "--project", metavar="PROJECT", help="build for project PROJECT")
+        parser.add_argument(
+            "--subproject", metavar="SUBPROJECT",
+            help="build for subproject SUBPROJECT")
+        parser.add_argument("--datestamp", help="date stamp")
+        parser.add_argument(
+            "--image-format", metavar="FORMAT",
+            help="produce an image in FORMAT")
+        parser.add_argument(
+            "--proposed", default=False, action="store_true",
+            help="enable use of -proposed pocket")
+        parser.add_argument(
+            "--locale", metavar="LOCALE",
+            help="use ubuntu-defaults-image to build an image for LOCALE")
+        parser.add_argument(
+            "--extra-ppa", dest="extra_ppas", default=[], action="append",
+            help="use this additional PPA")
 
     def run_build_command(self, args, env=None, echo=False):
         """Run a build command in the chroot.
@@ -67,6 +69,7 @@
 
         :param args: the command and arguments to run.
         :param env: dictionary of additional environment variables to set.
+        :param echo: if True, print the command before executing it.
         """
         args = [shell_escape(arg) for arg in args]
         if env:
@@ -74,28 +77,28 @@
                 "%s=%s" % (key, shell_escape(value))
                 for key, value in env.items()] + args
         command = "cd /build && %s" % " ".join(args)
-        self.chroot(["/bin/sh", "-c", command], echo=echo)
+        self.backend.run(["/bin/sh", "-c", command], echo=echo)
 
     def install(self):
-        self.chroot(["apt-get", "-y", "install", "livecd-rootfs"])
-        if self.options.arch == "i386":
-            self.chroot([
+        self.backend.run(["apt-get", "-y", "install", "livecd-rootfs"])
+        if self.args.arch == "i386":
+            self.backend.run([
                 "apt-get", "-y", "--no-install-recommends", "install",
                 "ltsp-server",
                 ])
-        if self.options.locale is not None:
-            self.chroot([
+        if self.args.locale is not None:
+            self.backend.run([
                 "apt-get", "-y", "--install-recommends", "install",
                 "ubuntu-defaults-builder",
                 ])
 
     def build(self):
-        if self.options.locale is not None:
+        if self.args.locale is not None:
             self.run_build_command([
                 "ubuntu-defaults-image",
-                "--locale", self.options.locale,
-                "--arch", self.options.arch,
-                "--release", self.options.series,
+                "--locale", self.args.locale,
+                "--arch", self.args.arch,
+                "--release", self.args.series,
                 ])
         else:
             self.run_build_command(["rm", "-rf", "auto"])
@@ -106,70 +109,35 @@
                 self.run_build_command(["ln", "-s", lb_script_path, "auto/"])
             self.run_build_command(["lb", "clean", "--purge"])
 
-            base_lb_env = {
-                "PROJECT": self.options.project,
-                "ARCH": self.options.arch,
-                }
-            if self.options.subproject is not None:
-                base_lb_env["SUBPROJECT"] = self.options.subproject
-            if self.options.subarch is not None:
-                base_lb_env["SUBARCH"] = self.options.subarch
-            lb_env = dict(base_lb_env)
-            lb_env["SUITE"] = self.options.series
-            if self.options.datestamp is not None:
-                lb_env["NOW"] = self.options.datestamp
-            if self.options.image_format is not None:
-                lb_env["IMAGEFORMAT"] = self.options.image_format
-            if self.options.proposed:
+            base_lb_env = OrderedDict()
+            base_lb_env["PROJECT"] = self.args.project
+            base_lb_env["ARCH"] = self.args.arch
+            if self.args.subproject is not None:
+                base_lb_env["SUBPROJECT"] = self.args.subproject
+            if self.args.subarch is not None:
+                base_lb_env["SUBARCH"] = self.args.subarch
+            lb_env = base_lb_env.copy()
+            lb_env["SUITE"] = self.args.series
+            if self.args.datestamp is not None:
+                lb_env["NOW"] = self.args.datestamp
+            if self.args.image_format is not None:
+                lb_env["IMAGEFORMAT"] = self.args.image_format
+            if self.args.proposed:
                 lb_env["PROPOSED"] = "1"
-            if self.options.extra_ppas:
-                lb_env["EXTRA_PPAS"] = " ".join(self.options.extra_ppas)
+            if self.args.extra_ppas:
+                lb_env["EXTRA_PPAS"] = " ".join(self.args.extra_ppas)
             self.run_build_command(["lb", "config"], env=lb_env)
             self.run_build_command(["lb", "build"], env=base_lb_env)
 
-
-def main():
-    parser = OptionParser()
-    parser.add_option("--build-id", help="build identifier")
-    parser.add_option(
-        "--arch", metavar="ARCH", help="build for architecture ARCH")
-    parser.add_option(
-        "--subarch", metavar="SUBARCH",
-        help="build for subarchitecture SUBARCH")
-    parser.add_option(
-        "--project", metavar="PROJECT", help="build for project PROJECT")
-    parser.add_option(
-        "--subproject", metavar="SUBPROJECT",
-        help="build for subproject SUBPROJECT")
-    parser.add_option(
-        "--series", metavar="SERIES", help="build for series SERIES")
-    parser.add_option("--datestamp", help="date stamp")
-    parser.add_option(
-        "--image-format", metavar="FORMAT", help="produce an image in FORMAT")
-    parser.add_option(
-        "--proposed", default=False, action="store_true",
-        help="enable use of -proposed pocket")
-    parser.add_option(
-        "--locale", metavar="LOCALE",
-        help="use ubuntu-defaults-image to build an image for LOCALE")
-    parser.add_option(
-        "--extra-ppa", dest="extra_ppas", default=[], action="append",
-        help="use this additional PPA")
-    options, _ = parser.parse_args()
-
-    builder = LiveFSBuilder(options)
-    try:
-        builder.install()
-    except Exception:
-        traceback.print_exc()
-        return RETCODE_FAILURE_INSTALL
-    try:
-        builder.build()
-    except Exception:
-        traceback.print_exc()
-        return RETCODE_FAILURE_BUILD
-    return RETCODE_SUCCESS
-
-
-if __name__ == "__main__":
-    sys.exit(main())
+    def run(self):
+        try:
+            self.install()
+        except Exception:
+            logger.exception('Install failed')
+            return RETCODE_FAILURE_INSTALL
+        try:
+            self.build()
+        except Exception:
+            logger.exception('Build failed')
+            return RETCODE_FAILURE_BUILD
+        return 0

=== modified file 'lpbuildd/target/chroot.py'
--- lpbuildd/target/chroot.py	2017-08-22 16:01:39 +0000
+++ lpbuildd/target/chroot.py	2017-08-22 16:37:45 +0000
@@ -53,7 +53,7 @@
             self.copy_in(path, path)
 
     def run(self, args, env=None, input_text=None, get_output=False,
-            **kwargs):
+            echo=False, **kwargs):
         """See `Backend`."""
         if env:
             args = ["env"] + [
@@ -61,6 +61,9 @@
                 for key, value in env.items()] + args
         if self.arch is not None:
             args = set_personality(args, self.arch, series=self.series)
+        if echo:
+            print("Running in chroot: %s" % ' '.join(
+                shell_escape(arg) for arg in args))
         cmd = ["sudo", "/usr/sbin/chroot", self.chroot_path] + args
         if input_text is None and not get_output:
             subprocess.check_call(cmd, cwd=self.chroot_path, **kwargs)

=== modified file 'lpbuildd/target/cli.py'
--- lpbuildd/target/cli.py	2017-08-22 14:52:43 +0000
+++ lpbuildd/target/cli.py	2017-08-22 16:37:45 +0000
@@ -14,6 +14,7 @@
     OverrideSourcesList,
     Update,
     )
+from lpbuildd.target.build_livefs import BuildLiveFS
 from lpbuildd.target.lifecycle import (
     Create,
     KillProcesses,
@@ -44,6 +45,7 @@
 
 operations = {
     "add-trusted-keys": AddTrustedKeys,
+    "buildlivefs": BuildLiveFS,
     "override-sources-list": OverrideSourcesList,
     "mount-chroot": Start,
     "remove-build": Remove,

=== added file 'lpbuildd/target/tests/test_build_livefs.py'
--- lpbuildd/target/tests/test_build_livefs.py	1970-01-01 00:00:00 +0000
+++ lpbuildd/target/tests/test_build_livefs.py	2017-08-22 16:37:45 +0000
@@ -0,0 +1,196 @@
+# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+import subprocess
+
+from fixtures import FakeLogger
+from testtools import TestCase
+from testtools.matchers import (
+    AnyMatch,
+    Equals,
+    Is,
+    MatchesAll,
+    MatchesDict,
+    MatchesListwise,
+    )
+
+from lpbuildd.target.build_livefs import (
+    RETCODE_FAILURE_BUILD,
+    RETCODE_FAILURE_INSTALL,
+    )
+from lpbuildd.target.cli import parse_args
+from lpbuildd.tests.fakeslave import FakeMethod
+
+
+class RanCommand(MatchesListwise):
+
+    def __init__(self, args, echo=None, **env):
+        kwargs_matcher = {}
+        if echo is not None:
+            kwargs_matcher["echo"] = Is(echo)
+        if env:
+            kwargs_matcher["env"] = MatchesDict(env)
+        super(RanCommand, self).__init__(
+            [Equals((args,)), MatchesDict(kwargs_matcher)])
+
+
+class RanAptGet(RanCommand):
+
+    def __init__(self, *args):
+        super(RanAptGet, self).__init__(["apt-get", "-y"] + list(args))
+
+
+class RanBuildCommand(RanCommand):
+
+    def __init__(self, command):
+        super(RanBuildCommand, self).__init__(
+            ["/bin/sh", "-c", "cd /build && " + command], echo=False)
+
+
+class TestBuildLiveFS(TestCase):
+
+    def test_run_build_command_no_env(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.run_build_command(["echo", "hello world"])
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanBuildCommand("echo 'hello world'"),
+            ]))
+
+    def test_run_build_command_env(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.run_build_command(
+            ["echo", "hello world"], env={"FOO": "bar baz"})
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanBuildCommand("env FOO='bar baz' echo 'hello world'"),
+            ]))
+
+    def test_install(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.install()
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "livecd-rootfs"),
+            ]))
+
+    def test_install_i386(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=i386", "1",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.install()
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "livecd-rootfs"),
+            RanAptGet("--no-install-recommends", "install", "ltsp-server"),
+            ]))
+
+    def test_install_locale(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--locale=zh_CN",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.install()
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanAptGet("install", "livecd-rootfs"),
+            RanAptGet(
+                "--install-recommends", "install", "ubuntu-defaults-builder"),
+            ]))
+
+    def test_build(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--project=ubuntu",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.build()
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanBuildCommand("rm -rf auto"),
+            RanBuildCommand("mkdir -p auto"),
+            RanBuildCommand(
+                "ln -s /usr/share/livecd-rootfs/live-build/auto/config auto/"),
+            RanBuildCommand(
+                "ln -s /usr/share/livecd-rootfs/live-build/auto/build auto/"),
+            RanBuildCommand(
+                "ln -s /usr/share/livecd-rootfs/live-build/auto/clean auto/"),
+            RanBuildCommand("lb clean --purge"),
+            RanBuildCommand(
+                "env PROJECT=ubuntu ARCH=amd64 SUITE=xenial lb config"),
+            RanBuildCommand("env PROJECT=ubuntu ARCH=amd64 lb build"),
+            ]))
+
+    def test_build_locale(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--locale=zh_CN",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.build()
+        self.assertThat(build_livefs.backend.run.calls, MatchesListwise([
+            RanBuildCommand(
+                "ubuntu-defaults-image --locale zh_CN --arch amd64 "
+                "--release xenial"),
+            ]))
+
+    def test_run_succeeds(self):
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--project=ubuntu",
+            ]
+        build_livefs = parse_args(args=args).operation
+        self.assertEqual(0, build_livefs.run())
+        self.assertThat(build_livefs.backend.run.calls, MatchesAll(
+            AnyMatch(RanAptGet("install", "livecd-rootfs")),
+            AnyMatch(RanBuildCommand(
+                "env PROJECT=ubuntu ARCH=amd64 lb build"))))
+
+    def test_run_install_fails(self):
+        class FailInstall(FakeMethod):
+            def __call__(self, run_args, *args, **kwargs):
+                super(FailInstall, self).__call__(run_args, *args, **kwargs)
+                if run_args[0] == "apt-get":
+                    raise subprocess.CalledProcessError(1, run_args)
+
+        self.useFixture(FakeLogger())
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--project=ubuntu",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.backend.run = FailInstall()
+        self.assertEqual(RETCODE_FAILURE_INSTALL, build_livefs.run())
+
+    def test_run_build_fails(self):
+        class FailBuild(FakeMethod):
+            def __call__(self, run_args, *args, **kwargs):
+                super(FailBuild, self).__call__(run_args, *args, **kwargs)
+                if run_args[0] == "/bin/sh":
+                    raise subprocess.CalledProcessError(1, run_args)
+
+        self.useFixture(FakeLogger())
+        args = [
+            "buildlivefs",
+            "--backend=fake", "--series=xenial", "--arch=amd64", "1",
+            "--project=ubuntu",
+            ]
+        build_livefs = parse_args(args=args).operation
+        build_livefs.backend.run = FailBuild()
+        self.assertEqual(RETCODE_FAILURE_BUILD, build_livefs.run())

=== modified file 'lpbuildd/tests/test_livefs.py'
--- lpbuildd/tests/test_livefs.py	2017-08-22 15:55:44 +0000
+++ lpbuildd/tests/test_livefs.py	2017-08-22 16:37:45 +0000
@@ -75,9 +75,11 @@
         self.assertEqual(
             LiveFilesystemBuildState.BUILD_LIVEFS, self.getState())
         expected_command = [
-            "sharepath/slavebin/buildlivefs", "buildlivefs", "--build-id",
-            self.buildid, "--arch", "i386", "--project", "ubuntu",
-            "--series", "saucy",
+            "sharepath/slavebin/in-target", "in-target",
+            "buildlivefs",
+            "--backend=chroot", "--series=saucy", "--arch=i386",
+            self.buildid,
+            "--project", "ubuntu",
             ]
         self.assertEqual(expected_command, self.buildmanager.commands[-1])
         self.assertEqual(