Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/build-livefs-operation into lp:launchpad-buildd with lp:~cjwatson/launchpad-buildd/gather-results-via-backend as a prerequisite.
Commit message: Convert buildlivefs to the new Operation framework and add unit tests. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad-buildd/build-livefs-operation/+merge/328657 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/build-livefs-operation into lp:launchpad-buildd.
=== added file 'bin/buildlivefs' --- bin/buildlivefs 1970-01-01 00:00:00 +0000 +++ bin/buildlivefs 2017-08-07 10:26:10 +0000 @@ -0,0 +1,24 @@ +#! /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). + +"""Build a live file system.""" + +from __future__ import print_function + +__metaclass__ = type + +import sys + +from lpbuildd.target.build_livefs import BuildLiveFS +from lpbuildd.target.operation import configure_logging + + +def main(): + configure_logging() + return BuildLiveFS().run() + + +if __name__ == "__main__": + sys.exit(main()) === modified file 'debian/changelog' --- debian/changelog 2017-08-07 10:26:10 +0000 +++ debian/changelog 2017-08-07 10:26:10 +0000 @@ -20,6 +20,7 @@ * Rewrite mount-chroot and umount-chroot in Python, allowing them to have unit tests. * Rewrite scan-for-processes in Python, allowing it to have unit tests. + * Convert buildlivefs to the new Operation framework and add unit tests. -- Colin Watson <cjwat...@ubuntu.com> Tue, 25 Jul 2017 23:07:58 +0100 === modified file 'lpbuildd/livefs.py' --- lpbuildd/livefs.py 2017-08-07 10:26:10 +0000 +++ lpbuildd/livefs.py 2017-08-07 10:26:10 +0000 @@ -45,17 +45,12 @@ def doRunBuild(self): """Run the process to build the live filesystem.""" - args = [ - "buildlivefs", - "--build-id", self._buildid, - "--arch", self.arch_tag, - ] + args = ["buildlivefs"] 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 +61,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(self.build_livefs_path, args) def iterate_BUILD_LIVEFS(self, retcode): """Finished building the live filesystem.""" === modified file 'lpbuildd/target/backend.py' --- lpbuildd/target/backend.py 2017-08-07 10:26:10 +0000 +++ lpbuildd/target/backend.py 2017-08-07 10:26:10 +0000 @@ -50,13 +50,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-07 10:26:10 +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." + + def make_parser(self): + parser = super(BuildLiveFS, self).make_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") + return parser 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-07 10:26:10 +0000 +++ lpbuildd/target/chroot.py 2017-08-07 10:26:10 +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) === 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-07 10:26:10 +0000 @@ -0,0 +1,175 @@ +# 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 testtools import TestCase +from testtools.matchers import ( + AnyMatch, + Equals, + Is, + MatchesAll, + MatchesDict, + MatchesListwise, + ) + +from lpbuildd.target.build_livefs import ( + BuildLiveFS, + RETCODE_FAILURE_BUILD, + RETCODE_FAILURE_INSTALL, + ) +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 = ["--backend=fake", "--series=xenial", "--arch=amd64", "1"] + build_livefs = BuildLiveFS(args=args) + 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 = ["--backend=fake", "--series=xenial", "--arch=amd64", "1"] + build_livefs = BuildLiveFS(args=args) + 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 = ["--backend=fake", "--series=xenial", "--arch=amd64", "1"] + build_livefs = BuildLiveFS(args=args) + build_livefs.install() + self.assertThat(build_livefs.backend.run.calls, MatchesListwise([ + RanAptGet("install", "livecd-rootfs"), + ])) + + def test_install_i386(self): + args = ["--backend=fake", "--series=xenial", "--arch=i386", "1"] + build_livefs = BuildLiveFS(args=args) + 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 = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--locale=zh_CN", + ] + build_livefs = BuildLiveFS(args=args) + 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 = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--project=ubuntu", + ] + build_livefs = BuildLiveFS(args=args) + 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 = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--locale=zh_CN", + ] + build_livefs = BuildLiveFS(args=args) + 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 = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--project=ubuntu", + ] + build_livefs = BuildLiveFS(args=args) + 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) + + args = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--project=ubuntu", + ] + build_livefs = BuildLiveFS(args=args) + 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) + + args = [ + "--backend=fake", "--series=xenial", "--arch=amd64", "1", + "--project=ubuntu", + ] + build_livefs = BuildLiveFS(args=args) + 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-07 10:26:10 +0000 +++ lpbuildd/tests/test_livefs.py 2017-08-07 10:26:10 +0000 @@ -75,9 +75,10 @@ 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/buildlivefs", "buildlivefs", + "--backend=chroot", "--series=saucy", "--arch=i386", + self.buildid, + "--project", "ubuntu", ] self.assertEqual(expected_command, self.buildmanager.commands[-1]) self.assertEqual(
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp