launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21797
[Merge] lp:~cjwatson/launchpad-buildd/build-livefs-operation into lp:launchpad-buildd
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 <cjwatson@xxxxxxxxxx> 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(
