launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22954
[Merge] lp:~cjwatson/launchpad-buildd/gatherResults-in-thread into lp:launchpad-buildd
Colin Watson has proposed merging lp:~cjwatson/launchpad-buildd/gatherResults-in-thread into lp:launchpad-buildd.
Commit message:
Call gatherResults in a different thread so that it doesn't block responses to XML-RPC requests.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1795877 in launchpad-buildd: "livefs builds fail without logs (possibly due to multiple large artifacts)"
https://bugs.launchpad.net/launchpad-buildd/+bug/1795877
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad-buildd/gatherResults-in-thread/+merge/356196
The test changes here really just keep the existing tests working. I tried to get proper tests going for this, but I couldn't quite manage to hook up all the callbacks in a way that didn't crash or hang, and ran out of time for it. However, I've done a fair amount of local testing of these changes, including manually inserting time.sleep calls into gatherResults for various build types and testing that I can usefully call status() and abort() while gatherResults is in progress.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad-buildd/gatherResults-in-thread into lp:launchpad-buildd.
=== modified file 'debian/changelog'
--- debian/changelog 2018-08-16 15:35:27 +0000
+++ debian/changelog 2018-10-05 15:49:20 +0000
@@ -1,3 +1,10 @@
+launchpad-buildd (165) UNRELEASED; urgency=medium
+
+ * Call gatherResults in a different thread so that it doesn't block
+ responses to XML-RPC requests (LP: #1795877).
+
+ -- Colin Watson <cjwatson@xxxxxxxxxx> Fri, 05 Oct 2018 16:43:11 +0100
+
launchpad-buildd (164) xenial; urgency=medium
* Configure snap proxy settings for Subversion (LP: #1668358).
=== modified file 'lpbuildd/binarypackage.py'
--- lpbuildd/binarypackage.py 2017-08-03 13:13:08 +0000
+++ lpbuildd/binarypackage.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from __future__ import absolute_import, print_function
@@ -17,6 +17,10 @@
PkgRelation,
)
from debian.debian_support import Version
+from twisted.internet import (
+ defer,
+ threads,
+ )
from lpbuildd.debian import (
DebianBuildManager,
@@ -343,14 +347,26 @@
"""Finished the sbuild run."""
if success == SBuildExitCodes.OK:
print("Returning build status: OK")
- try:
- self.gatherResults()
- except Exception as e:
- self._slave.log("Failed to gather results: %s" % e)
- self._slave.buildFail()
+
+ # XXX cjwatson 2018-10-04: Refactor using inlineCallbacks once
+ # we're on Twisted >= 18.7.0
+ # (https://twistedmatrix.com/trac/ticket/4632).
+ def failed_to_gather(failure):
+ if failure.check(defer.CancelledError):
+ if not self.alreadyfailed:
+ self._slave.log("Build cancelled unexpectedly!")
+ self._slave.buildFail()
+ else:
+ self._slave.log(
+ "Failed to gather results: %s" % failure.value)
+ self._slave.buildFail()
self.alreadyfailed = True
- self.doReapProcesses(self._state)
- return
+
+ def reap(ignored):
+ self.doReapProcesses(self._state)
+
+ return threads.deferToThread(self.gatherResults).addErrback(
+ failed_to_gather).addCallback(reap)
log_patterns = []
stop_patterns = [["^Toolchain package versions:", re.M]]
=== modified file 'lpbuildd/debian.py'
--- lpbuildd/debian.py 2018-05-02 09:00:10 +0000
+++ lpbuildd/debian.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# Authors: Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
@@ -13,6 +13,7 @@
import re
import signal
+from twisted.internet import defer
from twisted.python import log
from lpbuildd.slave import (
@@ -41,6 +42,7 @@
self._state = DebianBuildState.INIT
slave.emptyLog()
self.alreadyfailed = False
+ self._iterator = None
@property
def initial_build_state(self):
@@ -115,6 +117,7 @@
finally:
chfile.close()
+ @defer.inlineCallbacks
def iterate(self, success, quiet=False):
# When a Twisted ProcessControl class is killed by SIGTERM,
# which we call 'build process aborted', 'None' is returned as
@@ -129,7 +132,9 @@
func = getattr(self, "iterate_" + self._state, None)
if func is None:
raise ValueError("Unknown internal state " + self._state)
- func(success)
+ self._iterator = func(success)
+ yield self._iterator
+ self._iterator = None
def iterateReap(self, state, success):
log.msg("Iterating with success flag %s against stage %s after "
@@ -305,6 +310,13 @@
"""
self.doReapProcesses(self._state, notify=False)
+ def abort(self):
+ """See `BuildManager`."""
+ super(DebianBuildManager, self).abort()
+ if self._iterator is not None:
+ self._iterator.cancel()
+ self._iterator = None
+
def get_build_path(home, build_id, *extra):
"""Generate a path within the build directory.
=== modified file 'lpbuildd/livefs.py'
--- lpbuildd/livefs.py 2018-01-09 00:50:02 +0000
+++ lpbuildd/livefs.py 2018-10-05 15:49:20 +0000
@@ -1,10 +1,15 @@
-# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
import os
+from twisted.internet import (
+ defer,
+ threads,
+ )
+
from lpbuildd.debian import (
DebianBuildManager,
DebianBuildState,
@@ -69,8 +74,23 @@
def iterate_BUILD_LIVEFS(self, retcode):
"""Finished building the live filesystem."""
if retcode == RETCODE_SUCCESS:
- self.gatherResults()
print("Returning build status: OK")
+
+ # XXX cjwatson 2018-10-04: Refactor using inlineCallbacks once
+ # we're on Twisted >= 18.7.0
+ # (https://twistedmatrix.com/trac/ticket/4632).
+ def failed_to_gather(failure):
+ failure.trap(defer.CancelledError)
+ if not self.alreadyfailed:
+ self._slave.log("Build cancelled unexpectedly!")
+ self._slave.buildFail()
+ self.alreadyfailed = True
+
+ def reap(ignored):
+ self.doReapProcesses(self._state)
+
+ return threads.deferToThread(self.gatherResults).addErrback(
+ failed_to_gather).addCallback(reap)
elif (retcode >= RETCODE_FAILURE_INSTALL and
retcode <= RETCODE_FAILURE_BUILD):
if not self.alreadyfailed:
=== modified file 'lpbuildd/snap.py'
--- lpbuildd/snap.py 2018-06-07 20:08:26 +0000
+++ lpbuildd/snap.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from __future__ import print_function
@@ -30,7 +30,11 @@
from urlparse import urlparse
from twisted.application import strports
-from twisted.internet import reactor
+from twisted.internet import (
+ defer,
+ reactor,
+ threads,
+ )
from twisted.internet.interfaces import IHalfCloseableProtocol
from twisted.python.compat import intToBytes
from twisted.web import (
@@ -356,8 +360,23 @@
self.stopProxy()
self.revokeProxyToken()
if retcode == RETCODE_SUCCESS:
- self.gatherResults()
print("Returning build status: OK")
+
+ # XXX cjwatson 2018-10-04: Refactor using inlineCallbacks once
+ # we're on Twisted >= 18.7.0
+ # (https://twistedmatrix.com/trac/ticket/4632).
+ def failed_to_gather(failure):
+ failure.trap(defer.CancelledError)
+ if not self.alreadyfailed:
+ self._slave.log("Build cancelled unexpectedly!")
+ self._slave.buildFail()
+ self.alreadyfailed = True
+
+ def reap(ignored):
+ self.doReapProcesses(self._state)
+
+ return threads.deferToThread(self.gatherResults).addErrback(
+ failed_to_gather).addCallback(reap)
elif (retcode >= RETCODE_FAILURE_INSTALL and
retcode <= RETCODE_FAILURE_BUILD):
if not self.alreadyfailed:
=== modified file 'lpbuildd/sourcepackagerecipe.py'
--- lpbuildd/sourcepackagerecipe.py 2016-01-05 14:05:42 +0000
+++ lpbuildd/sourcepackagerecipe.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E1002
@@ -7,11 +7,18 @@
import os
import re
+from twisted.internet import (
+ defer,
+ threads,
+ )
+
from lpbuildd.debian import (
DebianBuildManager,
DebianBuildState,
get_build_path,
)
+
+
RETCODE_SUCCESS = 0
RETCODE_FAILURE_INSTALL = 200
RETCODE_FAILURE_BUILD_TREE = 201
@@ -98,8 +105,23 @@
def iterate_BUILD_RECIPE(self, retcode):
"""Move from BUILD_RECIPE to the next logical state."""
if retcode == RETCODE_SUCCESS:
- self.gatherResults()
print("Returning build status: OK")
+
+ # XXX cjwatson 2018-10-04: Refactor using inlineCallbacks once
+ # we're on Twisted >= 18.7.0
+ # (https://twistedmatrix.com/trac/ticket/4632).
+ def failed_to_gather(failure):
+ failure.trap(defer.CancelledError)
+ if not self.alreadyfailed:
+ self._slave.log("Build cancelled unexpectedly!")
+ self._slave.buildFail()
+ self.alreadyfailed = True
+
+ def reap(ignored):
+ self.doReapProcesses(self._state)
+
+ return threads.deferToThread(self.gatherResults).addErrback(
+ failed_to_gather).addCallback(reap)
elif retcode == RETCODE_FAILURE_INSTALL_BUILD_DEPS:
if not self.alreadyfailed:
rx = (
=== modified file 'lpbuildd/tests/test_binarypackage.py'
--- lpbuildd/tests/test_binarypackage.py 2017-08-22 14:53:29 +0000
+++ lpbuildd/tests/test_binarypackage.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -13,6 +13,7 @@
from debian.deb822 import PkgRelation
from fixtures import MonkeyPatch
from testtools import TestCase
+from testtools.deferredruntest import AsynchronousDeferredRunTest
from testtools.matchers import (
Contains,
ContainsDict,
@@ -21,6 +22,7 @@
MatchesListwise,
Not,
)
+from twisted.internet import defer
from twisted.internet.task import Clock
from lpbuildd.binarypackage import (
@@ -85,6 +87,9 @@
class TestBinaryPackageBuildManagerIteration(TestCase):
"""Run BinaryPackageBuildManager through its iteration steps."""
+
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
def setUp(self):
super(TestBinaryPackageBuildManagerIteration, self).setUp()
self.useFixture(DisableSudo())
@@ -108,6 +113,7 @@
"""Retrieve build manager's state."""
return self.buildmanager._state
+ @defer.inlineCallbacks
def startBuild(self, dscname=''):
# The build manager's iterate() kicks off the consecutive states
# after INIT.
@@ -123,7 +129,7 @@
self.buildmanager._state = BinaryPackageBuildState.UPDATE
# SBUILD: Build the package.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertState(
BinaryPackageBuildState.SBUILD,
[
@@ -147,9 +153,10 @@
self.assertNotEqual(
self.buildmanager.iterate, self.buildmanager.iterators[-1])
+ @defer.inlineCallbacks
def assertScansSanely(self, exit_code):
# After building the package, reap processes.
- self.buildmanager.iterate(exit_code)
+ yield self.buildmanager.iterate(exit_code)
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/in-target', 'in-target',
@@ -166,9 +173,10 @@
'--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=True)
+ @defer.inlineCallbacks
def test_iterate(self):
# The build manager iterates a normal build from start to finish.
- self.startBuild()
+ yield self.startBuild()
write_file(
os.path.join(self.buildmanager._cachepath, 'buildlog'),
@@ -179,7 +187,7 @@
write_file(changes_path, "I am a changes file.")
# After building the package, reap processes.
- self.assertScansSanely(SBuildExitCodes.OK)
+ yield self.assertScansSanely(SBuildExitCodes.OK)
self.assertFalse(self.slave.wasCalled('buildFail'))
self.assertThat(self.slave, HasWaitingFiles.byEquality({
'foo_1_i386.changes': b'I am a changes file.',
@@ -189,6 +197,7 @@
self.assertUnmountsSanely()
self.assertFalse(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_with_debug_symbols(self):
# A build with debug symbols sets up /CurrentlyBuilding
# appropriately, and does not pass DEB_BUILD_OPTIONS.
@@ -199,7 +208,7 @@
'build_debug_symbols': True})
os.makedirs(self.chrootdir)
self.buildmanager._state = BinaryPackageBuildState.UPDATE
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/sbuild-package', 'sbuild-package',
@@ -218,6 +227,7 @@
Build-Debug-Symbols: yes
"""), cb.read())
+ @defer.inlineCallbacks
def test_without_debug_symbols(self):
# A build with debug symbols sets up /CurrentlyBuilding
# appropriately, and passes DEB_BUILD_OPTIONS=noautodbgsym.
@@ -228,7 +238,7 @@
'build_debug_symbols': False})
os.makedirs(self.chrootdir)
self.buildmanager._state = BinaryPackageBuildState.UPDATE
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertState(
BinaryPackageBuildState.SBUILD,
['sharepath/slavebin/sbuild-package', 'sbuild-package',
@@ -248,9 +258,10 @@
Purpose: PRIMARY
"""), cb.read())
+ @defer.inlineCallbacks
def test_abort_sbuild(self):
# Aborting sbuild kills processes in the chroot.
- self.startBuild()
+ yield self.startBuild()
# Send an abort command. The build manager reaps processes.
self.buildmanager.abort()
@@ -267,10 +278,11 @@
self.assertUnmountsSanely()
self.assertFalse(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_abort_sbuild_fail(self):
# If killing processes in the chroot hangs, the build manager does
# its best to clean up and fails the builder.
- self.startBuild()
+ yield self.startBuild()
sbuild_subprocess = self.buildmanager._subprocess
# Send an abort command. The build manager reaps processes.
@@ -300,7 +312,7 @@
# When sbuild exits, it does not reap processes again, but proceeds
# directly to UMOUNT.
- self.buildmanager.iterate(128 + 9) # SIGKILL
+ yield self.buildmanager.iterate(128 + 9) # SIGKILL
self.assertState(
BinaryPackageBuildState.UMOUNT,
['sharepath/slavebin/in-target', 'in-target',
@@ -308,6 +320,7 @@
'--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=True)
+ @defer.inlineCallbacks
def test_abort_between_subprocesses(self):
# If a build is aborted between subprocesses, the build manager
# pretends that it was terminated by a signal.
@@ -324,7 +337,7 @@
'--backend=chroot', '--series=warty', '--arch=i386',
self.buildid], final=False)
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertState(
BinaryPackageBuildState.CLEANUP,
['sharepath/slavebin/in-target', 'in-target',
@@ -334,10 +347,11 @@
final=True)
self.assertFalse(self.slave.wasCalled('builderFail'))
+ @defer.inlineCallbacks
def test_missing_changes(self):
# The build manager recovers if the expected .changes file does not
# exist, and considers it a package build failure.
- self.startBuild()
+ yield self.startBuild()
write_file(
os.path.join(self.buildmanager._cachepath, 'buildlog'),
"I am a build log.")
@@ -348,7 +362,7 @@
"I am a changes file.")
# After building the package, reap processes.
- self.assertScansSanely(SBuildExitCodes.OK)
+ yield self.assertScansSanely(SBuildExitCodes.OK)
self.assertTrue(self.slave.wasCalled('buildFail'))
self.assertThat(self.slave, HasWaitingFiles({}))
@@ -564,8 +578,9 @@
PkgRelation.parse_relations("foo <!nocheck>, bar <stage1>"),
{}))
+ @defer.inlineCallbacks
def startDepFail(self, error, dscname=''):
- self.startBuild(dscname=dscname)
+ yield self.startBuild(dscname=dscname)
write_file(
os.path.join(self.buildmanager._cachepath, 'buildlog'),
"The following packages have unmet dependencies:\n"
@@ -574,9 +589,10 @@
+ ("a" * 4096) + "\n"
+ "Fail-Stage: install-deps\n")
+ @defer.inlineCallbacks
def assertMatchesDepfail(self, error, dep):
- self.startDepFail(error)
- self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+ yield self.startDepFail(error)
+ yield self.assertScansSanely(SBuildExitCodes.GIVENBACK)
self.assertUnmountsSanely()
if dep is not None:
self.assertFalse(self.slave.wasCalled('buildFail'))
@@ -585,27 +601,32 @@
self.assertFalse(self.slave.wasCalled('depFail'))
self.assertTrue(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_detects_depfail(self):
# The build manager detects dependency installation failures.
- self.assertMatchesDepfail(
+ yield self.assertMatchesDepfail(
"enoent but it is not installable", "enoent")
+ @defer.inlineCallbacks
def test_detects_versioned_depfail(self):
# The build manager detects dependency installation failures.
- self.assertMatchesDepfail(
+ yield self.assertMatchesDepfail(
"ebadver (< 2.0) but 3.0 is to be installed", "ebadver (< 2.0)")
+ @defer.inlineCallbacks
def test_detects_versioned_current_depfail(self):
# The build manager detects dependency installation failures.
- self.assertMatchesDepfail(
+ yield self.assertMatchesDepfail(
"ebadver (< 2.0) but 3.0 is installed", "ebadver (< 2.0)")
+ @defer.inlineCallbacks
def test_strips_depfail(self):
# The build manager strips qualifications and restrictions from
# dependency installation failures.
- self.assertMatchesDepfail(
+ yield self.assertMatchesDepfail(
"ebadver:any (>= 3.0) but 2.0 is installed", "ebadver (>= 3.0)")
+ @defer.inlineCallbacks
def test_uninstallable_deps_analysis_failure(self):
# If there are uninstallable build-dependencies and analysis can't
# find any missing direct build-dependencies, the build manager
@@ -618,7 +639,7 @@
Version: 1
Build-Depends: uninstallable (>= 1)
"""))
- self.startDepFail(
+ yield self.startDepFail(
"uninstallable (>= 1) but it is not going to be installed",
dscname="123")
apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
@@ -629,11 +650,12 @@
Package: uninstallable
Version: 1
"""))
- self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+ yield self.assertScansSanely(SBuildExitCodes.GIVENBACK)
self.assertUnmountsSanely()
self.assertFalse(self.slave.wasCalled('depFail'))
self.assertTrue(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_uninstallable_deps_analysis_depfail(self):
# If there are uninstallable build-dependencies and analysis reports
# some missing direct build-dependencies, the build manager marks
@@ -645,7 +667,7 @@
Version: 1
Build-Depends: ebadver (>= 2)
"""))
- self.startDepFail(
+ yield self.startDepFail(
"ebadver (>= 2) but it is not going to be installed",
dscname="123")
apt_lists = os.path.join(self.chrootdir, "var", "lib", "apt", "lists")
@@ -656,11 +678,12 @@
Package: ebadver
Version: 1
"""))
- self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+ yield self.assertScansSanely(SBuildExitCodes.GIVENBACK)
self.assertUnmountsSanely()
self.assertFalse(self.slave.wasCalled('buildFail'))
self.assertEqual([(("ebadver (>= 2)",), {})], self.slave.depFail.calls)
+ @defer.inlineCallbacks
def test_uninstallable_deps_analysis_mixed_depfail(self):
# If there is a mix of definite and dubious dep-wait output, then
# the build manager analyses the situation rather than trusting just
@@ -672,7 +695,7 @@
Version: 1
Build-Depends: ebadver (>= 2), uninstallable
"""))
- self.startBuild(dscname="123")
+ yield self.startBuild(dscname="123")
write_file(
os.path.join(self.buildmanager._cachepath, 'buildlog'),
"The following packages have unmet dependencies:\n"
@@ -694,19 +717,20 @@
Package: uninstallable
Version: 1
"""))
- self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+ yield self.assertScansSanely(SBuildExitCodes.GIVENBACK)
self.assertUnmountsSanely()
self.assertFalse(self.slave.wasCalled('buildFail'))
self.assertEqual([(("ebadver (>= 2)",), {})], self.slave.depFail.calls)
+ @defer.inlineCallbacks
def test_depfail_with_unknown_error_converted_to_packagefail(self):
# The build manager converts a DEPFAIL to a PACKAGEFAIL if the
# missing dependency can't be determined from the log.
- self.startBuild()
+ yield self.startBuild()
write_file(
os.path.join(self.buildmanager._cachepath, 'buildlog'),
"E: Everything is broken.\n")
- self.assertScansSanely(SBuildExitCodes.GIVENBACK)
+ yield self.assertScansSanely(SBuildExitCodes.GIVENBACK)
self.assertTrue(self.slave.wasCalled('buildFail'))
self.assertFalse(self.slave.wasCalled('depFail'))
=== modified file 'lpbuildd/tests/test_livefs.py'
--- lpbuildd/tests/test_livefs.py 2017-08-26 09:51:15 +0000
+++ lpbuildd/tests/test_livefs.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -10,6 +10,8 @@
TempDir,
)
from testtools import TestCase
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from twisted.internet import defer
from lpbuildd.livefs import (
LiveFilesystemBuildManager,
@@ -35,6 +37,9 @@
class TestLiveFilesystemBuildManagerIteration(TestCase):
"""Run LiveFilesystemBuildManager through its iteration steps."""
+
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
def setUp(self):
super(TestLiveFilesystemBuildManagerIteration, self).setUp()
self.working_dir = self.useFixture(TempDir()).path
@@ -52,6 +57,7 @@
"""Retrieve build manager's state."""
return self.buildmanager._state
+ @defer.inlineCallbacks
def startBuild(self):
# The build manager's iterate() kicks off the consecutive states
# after INIT.
@@ -71,7 +77,7 @@
self.buildmanager._state = LiveFilesystemBuildState.UPDATE
# BUILD_LIVEFS: Run the slave's payload to build the live filesystem.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertEqual(
LiveFilesystemBuildState.BUILD_LIVEFS, self.getState())
expected_command = [
@@ -85,9 +91,10 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled("chrootFail"))
+ @defer.inlineCallbacks
def test_iterate(self):
# The build manager iterates a normal build from start to finish.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
with open(log_path, "w") as log:
@@ -97,7 +104,7 @@
"/build/livecd.ubuntu.manifest", b"I am a manifest file.")
# After building the package, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/in-target", "in-target",
"scan-for-processes",
@@ -126,9 +133,10 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled("buildFail"))
+ @defer.inlineCallbacks
def test_omits_symlinks(self):
# Symlinks in the build output are not included in gathered results.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
with open(log_path, "w") as log:
@@ -139,7 +147,7 @@
self.buildmanager.backend.add_link(
"/build/livecd.ubuntu.kernel", "livefs.ubuntu.kernel-generic")
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertThat(self.slave, HasWaitingFiles.byEquality({
"livecd.ubuntu.kernel-generic": b"I am a kernel.",
}))
=== modified file 'lpbuildd/tests/test_snap.py'
--- lpbuildd/tests/test_snap.py 2018-06-05 20:59:48 +0000
+++ lpbuildd/tests/test_snap.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -70,6 +70,7 @@
"""Retrieve build manager's state."""
return self.buildmanager._state
+ @defer.inlineCallbacks
def startBuild(self, args=None, options=None):
# The build manager's iterate() kicks off the consecutive states
# after INIT.
@@ -92,7 +93,7 @@
self.buildmanager._state = SnapBuildState.UPDATE
# BUILD_SNAP: Run the slave's payload to build the snap package.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertEqual(SnapBuildState.BUILD_SNAP, self.getState())
expected_command = [
"sharepath/slavebin/in-target", "in-target",
@@ -119,9 +120,10 @@
status_file.write('{"revision_id": "dummy"}')
self.assertEqual({"revision_id": "dummy"}, self.buildmanager.status())
+ @defer.inlineCallbacks
def test_iterate(self):
# The build manager iterates a normal build from start to finish.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
with open(log_path, "w") as log:
@@ -131,7 +133,7 @@
"/build/test-snap/test-snap_0_all.snap", b"I am a snap package.")
# After building the package, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/in-target", "in-target",
"scan-for-processes",
@@ -159,10 +161,11 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled("buildFail"))
+ @defer.inlineCallbacks
def test_iterate_with_manifest(self):
# The build manager iterates a build that uploads a manifest from
# start to finish.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
with open(log_path, "w") as log:
@@ -174,7 +177,7 @@
"/build/test-snap/test-snap_0_all.manifest", b"I am a manifest.")
# After building the package, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/in-target", "in-target",
"scan-for-processes",
@@ -203,10 +206,11 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled("buildFail"))
+ @defer.inlineCallbacks
def test_iterate_with_build_source_tarball(self):
# The build manager iterates a build that uploads a source tarball
# from start to finish.
- self.startBuild(
+ yield self.startBuild(
{"build_source_tarball": True}, ["--build-source-tarball"])
log_path = os.path.join(self.buildmanager._cachepath, "buildlog")
@@ -219,7 +223,7 @@
"/build/test-snap.tar.gz", b"I am a source tarball.")
# After building the package, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
"sharepath/slavebin/in-target", "in-target",
"scan-for-processes",
=== modified file 'lpbuildd/tests/test_sourcepackagerecipe.py'
--- lpbuildd/tests/test_sourcepackagerecipe.py 2017-08-22 14:53:29 +0000
+++ lpbuildd/tests/test_sourcepackagerecipe.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2013-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -9,6 +9,8 @@
from textwrap import dedent
from testtools import TestCase
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from twisted.internet import defer
from lpbuildd.sourcepackagerecipe import (
RETCODE_FAILURE_INSTALL_BUILD_DEPS,
@@ -35,6 +37,9 @@
class TestSourcePackageRecipeBuildManagerIteration(TestCase):
"""Run SourcePackageRecipeBuildManager through its iteration steps."""
+
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
def setUp(self):
super(TestSourcePackageRecipeBuildManagerIteration, self).setUp()
self.working_dir = tempfile.mkdtemp()
@@ -55,6 +60,7 @@
"""Retrieve build manager's state."""
return self.buildmanager._state
+ @defer.inlineCallbacks
def startBuild(self, git=False):
# The build manager's iterate() kicks off the consecutive states
# after INIT.
@@ -84,7 +90,7 @@
self.buildmanager._state = SourcePackageRecipeBuildState.UPDATE
# BUILD_RECIPE: Run the slave's payload to build the source package.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertEqual(
SourcePackageRecipeBuildState.BUILD_RECIPE, self.getState())
expected_command = [
@@ -101,9 +107,10 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled('chrootFail'))
+ @defer.inlineCallbacks
def test_iterate(self):
# The build manager iterates a normal build from start to finish.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
with open(log_path, 'w') as log:
@@ -121,7 +128,7 @@
manifest.write("I am a manifest file.")
# After building the package, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
'sharepath/slavebin/in-target', 'in-target',
'scan-for-processes',
@@ -153,9 +160,10 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_iterate_BUILD_RECIPE_install_build_deps_depfail(self):
# The build manager can detect dependency wait states.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
with open(log_path, 'w') as log:
@@ -166,7 +174,7 @@
" but it is not going to be installed.\n")
# The buildmanager calls depFail correctly and reaps processes.
- self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
+ yield self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
expected_command = [
'sharepath/slavebin/in-target', 'in-target',
'scan-for-processes',
@@ -196,17 +204,18 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_iterate_BUILD_RECIPE_install_build_deps_buildfail(self):
# If the build manager cannot detect a dependency wait from a
# build-dependency installation failure, it fails the build.
- self.startBuild()
+ yield self.startBuild()
log_path = os.path.join(self.buildmanager._cachepath, 'buildlog')
with open(log_path, 'w') as log:
log.write("I am a failing build log.")
# The buildmanager calls buildFail correctly and reaps processes.
- self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
+ yield self.buildmanager.iterate(RETCODE_FAILURE_INSTALL_BUILD_DEPS)
expected_command = [
'sharepath/slavebin/in-target', 'in-target',
'scan-for-processes',
@@ -234,8 +243,9 @@
self.assertEqual(
self.buildmanager.iterate, self.buildmanager.iterators[-1])
+ @defer.inlineCallbacks
def test_iterate_git(self):
# Starting a git-based recipe build passes the correct option. (The
# rest of the build is identical to bzr-based recipe builds from the
# build manager's point of view.)
- self.startBuild(git=True)
+ yield self.startBuild(git=True)
=== modified file 'lpbuildd/tests/test_translationtemplatesbuildmanager.py'
--- lpbuildd/tests/test_translationtemplatesbuildmanager.py 2017-09-08 15:57:18 +0000
+++ lpbuildd/tests/test_translationtemplatesbuildmanager.py 2018-10-05 15:49:20 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -10,6 +10,8 @@
TempDir,
)
from testtools import TestCase
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from twisted.internet import defer
from lpbuildd.target.generate_translation_templates import (
RETCODE_FAILURE_BUILD,
@@ -39,6 +41,9 @@
class TestTranslationTemplatesBuildManagerIteration(TestCase):
"""Run TranslationTemplatesBuildManager through its iteration steps."""
+
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
def setUp(self):
super(TestTranslationTemplatesBuildManagerIteration, self).setUp()
self.working_dir = self.useFixture(TempDir()).path
@@ -57,6 +62,7 @@
"""Retrieve build manager's state."""
return self.buildmanager._state
+ @defer.inlineCallbacks
def test_iterate(self):
# Two iteration steps are specific to this build manager.
url = 'lp:~my/branch'
@@ -74,7 +80,7 @@
# GENERATE: Run the slave's payload, the script that generates
# templates.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
self.assertEqual(
TranslationTemplatesBuildState.GENERATE, self.getState())
expected_command = [
@@ -95,7 +101,7 @@
outfile_path, b"I am a template tarball. Seriously.")
# After generating templates, reap processes.
- self.buildmanager.iterate(0)
+ yield self.buildmanager.iterate(0)
expected_command = [
'sharepath/slavebin/in-target', 'in-target',
'scan-for-processes',
@@ -128,6 +134,7 @@
self.buildmanager.iterate, self.buildmanager.iterators[-1])
self.assertFalse(self.slave.wasCalled('buildFail'))
+ @defer.inlineCallbacks
def test_iterate_fail_GENERATE_install(self):
# See that a GENERATE that fails at the install step is handled
# properly.
@@ -141,7 +148,7 @@
self.buildmanager._state = TranslationTemplatesBuildState.GENERATE
# The buildmanager fails and reaps processes.
- self.buildmanager.iterate(RETCODE_FAILURE_INSTALL)
+ yield self.buildmanager.iterate(RETCODE_FAILURE_INSTALL)
self.assertEqual(
TranslationTemplatesBuildState.GENERATE, self.getState())
expected_command = [
@@ -169,6 +176,7 @@
self.assertEqual(
self.buildmanager.iterate, self.buildmanager.iterators[-1])
+ @defer.inlineCallbacks
def test_iterate_fail_GENERATE_build(self):
# See that a GENERATE that fails at the build step is handled
# properly.
@@ -182,7 +190,7 @@
self.buildmanager._state = TranslationTemplatesBuildState.GENERATE
# The buildmanager fails and reaps processes.
- self.buildmanager.iterate(RETCODE_FAILURE_BUILD)
+ yield self.buildmanager.iterate(RETCODE_FAILURE_BUILD)
expected_command = [
'sharepath/slavebin/in-target', 'in-target',
'scan-for-processes',
=== modified file 'lpbuildd/translationtemplates.py'
--- lpbuildd/translationtemplates.py 2017-09-08 15:57:18 +0000
+++ lpbuildd/translationtemplates.py 2018-10-05 15:49:20 +0000
@@ -1,10 +1,15 @@
-# Copyright 2010-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
import os
+from twisted.internet import (
+ defer,
+ threads,
+ )
+
from lpbuildd.debian import (
DebianBuildManager,
DebianBuildState,
@@ -63,7 +68,21 @@
"""Template generation finished."""
if retcode == 0:
# It worked! Now let's bring in the harvest.
- self.gatherResults()
+ # XXX cjwatson 2018-10-04: Refactor using inlineCallbacks once
+ # we're on Twisted >= 18.7.0
+ # (https://twistedmatrix.com/trac/ticket/4632).
+ def failed_to_gather(failure):
+ failure.trap(defer.CancelledError)
+ if not self.alreadyfailed:
+ self._slave.log("Build cancelled unexpectedly!")
+ self._slave.buildFail()
+ self.alreadyfailed = True
+
+ def reap(ignored):
+ self.doReapProcesses(self._state)
+
+ return threads.deferToThread(self.gatherResults).addErrback(
+ failed_to_gather).addCallback(reap)
else:
if not self.alreadyfailed:
if retcode == RETCODE_FAILURE_INSTALL:
