launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17024
[Merge] lp:~wgrant/launchpad/handleStatus-crash into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/handleStatus-crash into lp:launchpad.
Commit message:
Refactor IBuildFarmJobBehaviour result handling, eliminating lots of duplication.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #47353 in Launchpad itself: "give-back-loop detection would be desireable"
https://bugs.launchpad.net/launchpad/+bug/47353
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/handleStatus-crash/+merge/224617
This branch pretty seriously refactors BuildFarmJobBehaviourBase's result handling methods, to the tune of +46/-120. It makes use of the new robust failure counting and builder cleanup to safely raise exceptions on slave errors without risking the builder's life (each build gets a second chance on another builder, and a builder gets several chances). It's also a good bit less confusing to work with.
All the dynamic method names are gone. Failure results that occur in fairly normal operation (PACKAGEFAIL, DEPFAIL, CHROOTFAIL and ABORTED) are now all handled by a single codepath that sets the status and stores the log. The other failures (BUILDERFAIL and GIVENBACK) cause a BuildDaemonError that invokes failure counting. OK calls a handleSuccess method that replaces _handleStatus_OK. The default handleSuccess has minimal changes, except that it'll now crash rather than do a partial upload when some of the files attempt to escape the temporary directory.
TranslationTemplatesBuildBehaviour was the only implementation that overrode handleStatus. I ported it to override handleSuccess instead, removing another chunk of code.
Switching BUILDERFAIL to use failure counting meant that the only callsite of Builder.handleFailure was SlaveScanner._scanFailed, so I inlined it.
--
https://code.launchpad.net/~wgrant/launchpad/handleStatus-crash/+merge/224617
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/handleStatus-crash into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
--- lib/lp/buildmaster/interfaces/builder.py 2014-06-23 14:52:02 +0000
+++ lib/lp/buildmaster/interfaces/builder.py 2014-06-26 13:11:35 +0000
@@ -229,12 +229,6 @@
this code will need some sort of mutex.
"""
- def handleFailure(logger):
- """Handle buildd slave failures.
-
- Increment builder and (if possible) job failure counts.
- """
-
class IBuilderEdit(Interface):
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2014-06-26 08:29:52 +0000
+++ lib/lp/buildmaster/manager.py 2014-06-26 13:11:35 +0000
@@ -350,7 +350,9 @@
vitals = self.builder_factory.getVitals(self.builder_name)
builder = self.builder_factory[self.builder_name]
try:
- builder.handleFailure(self.logger)
+ builder.gotFailure()
+ if builder.current_build is not None:
+ builder.current_build.gotFailure()
recover_failure(self.logger, vitals, builder, retry, failure.value)
transaction.commit()
except Exception:
=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py 2014-06-26 08:29:52 +0000
+++ lib/lp/buildmaster/model/builder.py 2014-06-26 13:11:35 +0000
@@ -293,12 +293,6 @@
return None
- def handleFailure(self, logger):
- """See IBuilder."""
- self.gotFailure()
- if self.current_build is not None:
- self.current_build.gotFailure()
-
class BuilderProcessor(StormBase):
__storm_table__ = 'BuilderProcessor'
=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/model/buildfarmjobbehaviour.py 2014-06-26 07:39:03 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehaviour.py 2014-06-26 13:11:35 +0000
@@ -24,7 +24,10 @@
BuildFarmJobType,
BuildStatus,
)
-from lp.buildmaster.interfaces.builder import CannotBuild
+from lp.buildmaster.interfaces.builder import (
+ BuildDaemonError,
+ CannotBuild,
+ )
from lp.services.config import config
from lp.services.helpers import filenameToContentType
from lp.services.librarian.interfaces import ILibraryFileAliasSet
@@ -194,8 +197,9 @@
# allowed to be sent. It is up to each callback as to whether it will
# consider sending a notification but it won't do so if the status is not
# in this list.
- ALLOWED_STATUS_NOTIFICATIONS = ['OK', 'PACKAGEFAIL', 'CHROOTFAIL']
+ ALLOWED_STATUS_NOTIFICATIONS = ['PACKAGEFAIL', 'CHROOTFAIL']
+ @defer.inlineCallbacks
def handleStatus(self, bq, status, slave_status):
"""See `IBuildFarmJobBehaviour`."""
if bq != self.build.buildqueue_record:
@@ -204,22 +208,39 @@
from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
- method = getattr(self, '_handleStatus_' + status, None)
- if method is None:
- logger.critical(
- "Unknown BuildStatus '%s' for builder '%s'"
- % (status, self.build.buildqueue_record.builder.url))
- return
+ fail_status_map = {
+ 'PACKAGEFAIL': BuildStatus.FAILEDTOBUILD,
+ 'DEPFAIL': BuildStatus.MANUALDEPWAIT,
+ 'CHROOTFAIL': BuildStatus.CHROOTWAIT,
+ }
+ if self.build.status == BuildStatus.CANCELLING:
+ fail_status_map['ABORTED'] = BuildStatus.CANCELLED
+
logger.info(
- 'Processing finished %s build %s (%s) from builder %s'
- % (status, self.build.build_cookie,
- self.build.buildqueue_record.specific_build.title,
- self.build.buildqueue_record.builder.name))
- d = method(slave_status, logger, notify)
- return d
+ 'Processing finished job %s (%s) from builder %s: %s'
+ % (self.build.build_cookie, self.build.title,
+ self.build.buildqueue_record.builder.name, status))
+ if status == 'OK':
+ yield self.handleSuccess(slave_status, logger)
+ elif status in fail_status_map:
+ # XXX wgrant: The builder should be set long before here, but
+ # currently isn't.
+ self.build.updateStatus(
+ fail_status_map[status],
+ builder=self.build.buildqueue_record.builder,
+ slave_status=slave_status)
+ transaction.commit()
+ else:
+ raise BuildDaemonError(
+ "Build returned unexpected status: %r" % status)
+ yield self.storeLogFromSlave()
+ if notify:
+ self.build.notify()
+ self.build.buildqueue_record.destroySelf()
+ transaction.commit()
@defer.inlineCallbacks
- def _handleStatus_OK(self, slave_status, logger, notify):
+ def handleSuccess(self, slave_status, logger):
"""Handle a package that built successfully.
Once built successfully, we pull the files, store them in a
@@ -235,7 +256,6 @@
build = build.buildqueue_record.specific_build
if not build.current_source_publication:
build.updateStatus(BuildStatus.SUPERSEDED)
- self.build.buildqueue_record.destroySelf()
return
self.verifySuccessfulBuild()
@@ -257,7 +277,6 @@
grab_dir, str(build.archive.id), build.distribution.name)
os.makedirs(upload_path)
- successful_copy_from_slave = True
filenames_to_download = []
for filename in filemap:
logger.info("Grabbing file: %s" % filename)
@@ -266,118 +285,25 @@
# upload path, then we don't try to copy this or any
# subsequent files.
if not os.path.realpath(out_file_name).startswith(upload_path):
- successful_copy_from_slave = False
- logger.warning(
- "A slave tried to upload the file '%s' "
- "for the build %d." % (filename, build.id))
- break
+ raise BuildDaemonError(
+ "Build returned a file named %r." % filename)
filenames_to_download.append((filemap[filename], out_file_name))
yield self._slave.getFiles(filenames_to_download)
- status = (
- BuildStatus.UPLOADING if successful_copy_from_slave
- else BuildStatus.FAILEDTOUPLOAD)
# XXX wgrant: The builder should be set long before here, but
# currently isn't.
build.updateStatus(
- status, builder=build.buildqueue_record.builder,
+ BuildStatus.UPLOADING, builder=build.buildqueue_record.builder,
slave_status=slave_status)
transaction.commit()
- yield self.storeLogFromSlave()
-
- # We only attempt the upload if we successfully copied all the
- # files from the slave.
- if successful_copy_from_slave:
- logger.info(
- "Gathered %s %d completely. Moving %s to uploader queue."
- % (build.__class__.__name__, build.id, upload_leaf))
- target_dir = os.path.join(root, "incoming")
- else:
- logger.warning(
- "Copy from slave for build %s was unsuccessful.", build.id)
- if notify:
- build.notify(
- extra_info='Copy from slave was unsuccessful.')
- target_dir = os.path.join(root, "failed")
-
+ # Move the directory used to grab the binaries into incoming
+ # atomically, so other bits don't have to deal with incomplete
+ # uploads.
+ logger.info(
+ "Gathered %s completely. Moving %s to uploader queue."
+ % (build.build_cookie, upload_leaf))
+ target_dir = os.path.join(root, "incoming")
if not os.path.exists(target_dir):
os.mkdir(target_dir)
-
- self.build.buildqueue_record.destroySelf()
- transaction.commit()
-
- # Move the directory used to grab the binaries into
- # the incoming directory so the upload processor never
- # sees half-finished uploads.
os.rename(grab_dir, os.path.join(target_dir, upload_leaf))
-
- @defer.inlineCallbacks
- def _handleStatus_generic_fail(self, status, slave_status, logger, notify):
- """Handle a generic build failure.
-
- The build, not the builder, has failed. Set its status, store
- available information, and remove the queue entry.
- """
- # XXX wgrant: The builder should be set long before here, but
- # currently isn't.
- self.build.updateStatus(
- status, builder=self.build.buildqueue_record.builder,
- slave_status=slave_status)
- transaction.commit()
- yield self.storeLogFromSlave()
- if notify:
- self.build.notify()
- self.build.buildqueue_record.destroySelf()
- transaction.commit()
-
- def _handleStatus_PACKAGEFAIL(self, slave_status, logger, notify):
- """Handle a package that had failed to build."""
- return self._handleStatus_generic_fail(
- BuildStatus.FAILEDTOBUILD, slave_status, logger, notify)
-
- def _handleStatus_DEPFAIL(self, slave_status, logger, notify):
- """Handle a package that had missing dependencies."""
- return self._handleStatus_generic_fail(
- BuildStatus.MANUALDEPWAIT, slave_status, logger, notify)
-
- def _handleStatus_CHROOTFAIL(self, slave_status, logger, notify):
- """Handle a package that had failed when unpacking the CHROOT."""
- return self._handleStatus_generic_fail(
- BuildStatus.CHROOTWAIT, slave_status, logger, notify)
-
- def _handleStatus_BUILDERFAIL(self, slave_status, logger, notify):
- """Handle builder failures.
-
- Fail the builder, and reset the job.
- """
- self.build.buildqueue_record.builder.failBuilder(
- "Builder returned BUILDERFAIL when asked for its status")
- self.build.buildqueue_record.reset()
- transaction.commit()
-
- @defer.inlineCallbacks
- def _handleStatus_ABORTED(self, slave_status, logger, notify):
- """Handle aborted builds.
-
- If the build was explicitly cancelled, then mark it as such.
- Otherwise, the build has failed in some unexpected way; we'll
- reset it it and clean up the slave.
- """
- if self.build.status == BuildStatus.CANCELLING:
- yield self.storeLogFromSlave()
- self.build.buildqueue_record.markAsCancelled()
- else:
- self._builder.handleFailure(logger)
- self.build.buildqueue_record.reset()
- transaction.commit()
-
- def _handleStatus_GIVENBACK(self, slave_status, logger, notify):
- """Handle automatic retry requested by builder.
-
- GIVENBACK pseudo-state represents a request for automatic retry
- later, the build records is delayed by reducing the lastscore to
- ZERO.
- """
- self.build.buildqueue_record.reset()
- transaction.commit()
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2014-06-20 12:21:49 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2014-06-26 13:11:35 +0000
@@ -19,7 +19,6 @@
from lp.buildmaster.tests.mock_slaves import make_publisher
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import flush_database_updates
-from lp.services.log.logger import BufferLogger
from lp.soyuz.enums import (
ArchivePurpose,
PackagePublishingStatus,
@@ -77,12 +76,6 @@
builder.setCleanStatus(BuilderCleanStatus.CLEAN)
self.assertEqual(BuilderCleanStatus.CLEAN, builder.clean_status)
- def test_handleFailure_increments_failure_count(self):
- builder = self.factory.makeBuilder()
- self.assertEqual(0, builder.failure_count)
- builder.handleFailure(BufferLogger())
- self.assertEqual(1, builder.failure_count)
-
def test_set_processors(self):
builder = self.factory.makeBuilder()
proc1 = self.factory.makeProcessor()
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py 2014-06-26 07:39:03 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py 2014-06-26 13:11:35 +0000
@@ -11,6 +11,7 @@
import shutil
import tempfile
+from testtools import ExpectedException
from testtools.deferredruntest import AsynchronousDeferredRunTest
from twisted.internet import defer
from zope.component import getUtility
@@ -19,6 +20,7 @@
from lp.archiveuploader.uploadprocessor import parse_build_upload_leaf_name
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interactor import BuilderInteractor
+from lp.buildmaster.interfaces.builder import BuildDaemonError
from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
IBuildFarmJobBehaviour,
)
@@ -282,30 +284,27 @@
{'filemap': {'myfile.py': 'test_file_hash'}})
return d.addCallback(got_status)
+ @defer.inlineCallbacks
def test_handleStatus_OK_absolute_filepath(self):
- # A filemap that tries to write to files outside of
- # the upload directory will result in a failed upload.
- def got_status(ignored):
- self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
- self.assertResultCount(0, "failed")
- self.assertIdentical(None, self.build.buildqueue_record)
-
- d = self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
- return d.addCallback(got_status)
-
+ # A filemap that tries to write to files outside of the upload
+ # directory will not be collected.
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned a file named '/tmp/myfile.py'."):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, 'OK',
+ {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
+
+ @defer.inlineCallbacks
def test_handleStatus_OK_relative_filepath(self):
# A filemap that tries to write to files outside of
- # the upload directory will result in a failed upload.
- def got_status(ignored):
- self.assertEqual(BuildStatus.FAILEDTOUPLOAD, self.build.status)
- self.assertResultCount(0, "failed")
-
- d = self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'../myfile.py': 'test_file_hash'}})
- return d.addCallback(got_status)
+ # the upload directory will not be collected.
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned a file named '../myfile.py'."):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, 'OK',
+ {'filemap': {'../myfile.py': 'test_file_hash'}})
def test_handleStatus_OK_sets_build_log(self):
# The build log is set during handleStatus.
@@ -361,22 +360,18 @@
self.build.buildqueue_record, "ABORTED", {})
return d.addCallback(got_status)
- def test_handleStatus_ABORTED_recovers_building(self):
+ @defer.inlineCallbacks
+ def test_handleStatus_ABORTED_illegal_when_building(self):
self.builder.vm_host = "fake_vm_host"
self.behaviour = self.interactor.getBuildBehaviour(
self.build.buildqueue_record, self.builder, self.slave)
self.build.updateStatus(BuildStatus.BUILDING)
- def got_status(ignored):
- self.assertEqual(
- 0, len(pop_notifications()), "Notifications received")
- self.assertEqual(BuildStatus.NEEDSBUILD, self.build.status)
- self.assertEqual(1, self.builder.failure_count)
- self.assertEqual(1, self.build.failure_count)
-
- d = self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
- return d.addCallback(got_status)
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned unexpected status: 'ABORTED'"):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, "ABORTED", {})
@defer.inlineCallbacks
def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
@@ -398,3 +393,27 @@
self.assertNotEqual(None, self.build.date_finished)
return d.addCallback(got_status)
+
+ @defer.inlineCallbacks
+ def test_givenback_collection(self):
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned unexpected status: 'GIVENBACK'"):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, "GIVENBACK", {})
+
+ @defer.inlineCallbacks
+ def test_builderfail_collection(self):
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned unexpected status: 'BUILDERFAIL'"):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, "BUILDERFAIL", {})
+
+ @defer.inlineCallbacks
+ def test_invalid_status_collection(self):
+ with ExpectedException(
+ BuildDaemonError,
+ "Build returned unexpected status: 'BORKED'"):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, "BORKED", {})
=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py 2014-06-26 08:29:52 +0000
+++ lib/lp/buildmaster/tests/test_manager.py 2014-06-26 13:11:35 +0000
@@ -559,6 +559,12 @@
self.assertEqual(BuilderCleanStatus.DIRTY, builder.clean_status)
self.assertEqual(BuildStatus.CANCELLED, build.status)
+
+class TestSlaveScannerWithLibrarian(TestCaseWithFactory):
+
+ layer = LaunchpadZopelessLayer
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=20)
+
@defer.inlineCallbacks
def test_end_to_end(self):
# Test that SlaveScanner.scan() successfully finds, dispatches,
@@ -573,18 +579,14 @@
processors=[bq.processor], manual=False, vm_host='VMHOST')
transaction.commit()
- # Mock out the build behaviour's _handleStatus_OK so it doesn't
+ # Mock out the build behaviour's handleSuccess so it doesn't
# try to upload things to the librarian or queue.
- @defer.inlineCallbacks
- def handleStatus_OK(self, slave_status, logger, notify):
+ def handleSuccess(self, slave_status, logger):
build.updateStatus(
BuildStatus.UPLOADING, builder, slave_status=slave_status)
transaction.commit()
- yield self._slave.clean()
- bq.destroySelf()
- transaction.commit()
self.patch(
- BinaryPackageBuildBehaviour, '_handleStatus_OK', handleStatus_OK)
+ BinaryPackageBuildBehaviour, 'handleSuccess', handleSuccess)
# And create a SlaveScanner with a slave and a clock that we
# control.
@@ -627,14 +629,14 @@
self.assertEqual(
['status', 'status', 'status'], get_slave.result.method_log)
- # When the build finishes, the scanner will notice, call
- # handleStatus(), and then clean the builder. Our fake
- # _handleStatus_OK doesn't do anything special, but there'd
- # usually be file retrievals in the middle. The builder remains
- # dirty afterward.
+ # When the build finishes, the scanner will notice and call
+ # handleStatus(). Our fake handleSuccess() doesn't do anything
+ # special, but there'd usually be file retrievals in the middle,
+ # and the log is retrieved by handleStatus() afterwards.
+ # The builder remains dirty afterward.
get_slave.result = WaitingSlave(build_id=build.build_cookie)
yield scanner.scan()
- self.assertEqual(['status', 'clean'], get_slave.result.method_log)
+ self.assertEqual(['status', 'getFile'], get_slave.result.method_log)
self.assertIs(None, builder.currentjob)
self.assertEqual(BuildStatus.UPLOADING, build.status)
self.assertEqual(builder, build.builder)
=== modified file 'lib/lp/code/model/recipebuilder.py'
--- lib/lp/code/model/recipebuilder.py 2014-06-25 11:37:48 +0000
+++ lib/lp/code/model/recipebuilder.py 2014-06-26 13:11:35 +0000
@@ -41,8 +41,7 @@
# allowed to be sent. It is up to each callback as to whether it will
# consider sending a notification but it won't do so if the status is not
# in this list.
- ALLOWED_STATUS_NOTIFICATIONS = [
- 'OK', 'PACKAGEFAIL', 'DEPFAIL', 'CHROOTFAIL']
+ ALLOWED_STATUS_NOTIFICATIONS = ['PACKAGEFAIL', 'DEPFAIL', 'CHROOTFAIL']
def _extraBuildArgs(self, distroarchseries, logger=None):
"""
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py'
--- lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py 2014-06-26 07:39:03 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuildbehaviour.py 2014-06-26 13:11:35 +0000
@@ -405,20 +405,6 @@
self.candidate, WaitingSlave('BuildStatus.CHROOTFAIL'))
return d.addCallback(got_update)
- def test_builderfail_collection(self):
- # The builder failed after we dispatched the build.
- def got_update(ignored):
- self.assertEqual(
- "Builder returned BUILDERFAIL when asked for its status",
- self.builder.failnotes)
- self.assertIs(None, self.candidate.builder)
- self.assertEqual(BuildStatus.NEEDSBUILD, self.build.status)
- self.assertEqual(BuildQueueStatus.WAITING, self.candidate.status)
-
- d = self.updateBuild(
- self.candidate, WaitingSlave('BuildStatus.BUILDERFAIL'))
- return d.addCallback(got_update)
-
def test_building_collection(self):
# The builder is still building the package.
def got_update(ignored):
@@ -464,20 +450,6 @@
d = self.updateBuild(self.candidate, WaitingSlave('BuildStatus.OK'))
return d.addCallback(got_update)
- def test_givenback_collection(self):
- score = self.candidate.lastscore
-
- def got_update(ignored):
- self.assertIs(None, self.candidate.builder)
- self.assertIs(None, self.candidate.date_started)
- self.assertEqual(score, self.candidate.lastscore)
- self.assertEqual(BuildStatus.NEEDSBUILD, self.build.status)
- self.assertEqual(BuildQueueStatus.WAITING, self.candidate.status)
-
- d = self.updateBuild(
- self.candidate, WaitingSlave('BuildStatus.GIVENBACK'))
- return d.addCallback(got_update)
-
def test_log_file_collection(self):
self.build.updateStatus(BuildStatus.FULLYBUILT)
old_tmps = sorted(os.listdir('/tmp'))
=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehaviour.py'
--- lib/lp/translations/model/translationtemplatesbuildbehaviour.py 2014-06-26 07:17:13 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehaviour.py 2014-06-26 13:11:35 +0000
@@ -11,7 +11,6 @@
'TranslationTemplatesBuildBehaviour',
]
-import logging
import os
import re
import tempfile
@@ -45,6 +44,8 @@
unsafe_chars = '[^a-zA-Z0-9_+-]'
+ ALLOWED_STATUS_NOTIFICATIONS = []
+
def getLogFileName(self):
"""See `IBuildFarmJob`."""
safe_name = re.sub(
@@ -91,7 +92,7 @@
approver_factory=TranslationBuildApprover)
@defer.inlineCallbacks
- def handleStatus(self, queue_item, status, slave_status):
+ def handleSuccess(self, slave_status, logger):
"""Deal with a finished build job.
Retrieves tarball and logs from the slave, then cleans up the
@@ -100,50 +101,37 @@
If this fails for whatever unforeseen reason, a future run will
retry it.
"""
- from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
- logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
- logger.info(
- "Processing finished %s build %s (%s) from builder %s" % (
- status, self.build.build_cookie,
- queue_item.specific_build.branch.bzr_identity,
- queue_item.builder.name))
-
- if status == 'OK':
- self.build.updateStatus(
- BuildStatus.UPLOADING, builder=queue_item.builder)
- transaction.commit()
- logger.debug("Processing successful templates build.")
- filemap = slave_status.get('filemap')
- filename = yield self._readTarball(queue_item, filemap, logger)
-
- # XXX 2010-11-12 bug=674575
- # Please make addOrUpdateEntriesFromTarball() take files on
- # disk; reading arbitrarily sized files into memory is
- # dangerous.
- if filename is None:
- logger.error("Build produced no tarball.")
+ self.build.updateStatus(
+ BuildStatus.UPLOADING,
+ builder=self.build.buildqueue_record.builder)
+ transaction.commit()
+ logger.debug("Processing successful templates build.")
+ filemap = slave_status.get('filemap')
+ filename = yield self._readTarball(
+ self.build.buildqueue_record, filemap, logger)
+
+ # XXX 2010-11-12 bug=674575
+ # Please make addOrUpdateEntriesFromTarball() take files on
+ # disk; reading arbitrarily sized files into memory is
+ # dangerous.
+ if filename is None:
+ logger.error("Build produced no tarball.")
+ self.build.updateStatus(BuildStatus.FULLYBUILT)
+ else:
+ tarball_file = open(filename)
+ try:
+ tarball = tarball_file.read()
+ if tarball is None:
+ logger.error("Build produced empty tarball.")
+ else:
+ logger.debug(
+ "Uploading translation templates tarball.")
+ self._uploadTarball(
+ self.build.buildqueue_record.specific_build.branch,
+ tarball, logger)
+ logger.debug("Upload complete.")
+ finally:
self.build.updateStatus(BuildStatus.FULLYBUILT)
- else:
- tarball_file = open(filename)
- try:
- tarball = tarball_file.read()
- if tarball is None:
- logger.error("Build produced empty tarball.")
- else:
- logger.debug(
- "Uploading translation templates tarball.")
- self._uploadTarball(
- queue_item.specific_build.branch, tarball, logger)
- logger.debug("Upload complete.")
- finally:
- self.build.updateStatus(BuildStatus.FULLYBUILT)
- tarball_file.close()
- os.remove(filename)
- else:
- self.build.updateStatus(
- BuildStatus.FAILEDTOBUILD, builder=queue_item.builder)
- transaction.commit()
-
- yield self.storeLogFromSlave(build_queue=queue_item)
- queue_item.destroySelf()
+ tarball_file.close()
+ os.remove(filename)
transaction.commit()
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py 2014-06-25 11:06:04 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py 2014-06-26 13:11:35 +0000
@@ -139,7 +139,8 @@
behaviour = self.makeBehaviour(
filemap={'translation-templates.tar.gz': 'foo'})
behaviour._uploadTarball = FakeMethod()
- queue_item = FakeBuildQueue(behaviour)
+ queue_item = behaviour.build.queueBuild()
+ queue_item.markAsBuilding(self.factory.makeBuilder())
slave = behaviour._slave
d = slave.status()
@@ -155,7 +156,7 @@
self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
# Log file is stored.
self.assertIsNotNone(behaviour.build.log)
- self.assertEqual(1, queue_item.destroySelf.call_count)
+ self.assertIs(None, behaviour.build.buildqueue_record)
self.assertEqual(1, behaviour._uploadTarball.call_count)
d.addCallback(got_status)
@@ -164,9 +165,10 @@
def test_handleStatus_failed(self):
# Builds may also fail (and produce no tarball).
- behaviour = self.makeBehaviour(state='BuildStatus.FAILEDTOBUILD')
+ behaviour = self.makeBehaviour(state='BuildStatus.PACKAGEFAIL')
behaviour._uploadTarball = FakeMethod()
- queue_item = FakeBuildQueue(behaviour)
+ queue_item = behaviour.build.queueBuild()
+ queue_item.markAsBuilding(self.factory.makeBuilder())
slave = behaviour._slave
d = slave.status()
@@ -182,7 +184,7 @@
self.assertEqual(BuildStatus.FAILEDTOBUILD, behaviour.build.status)
# Log file is stored.
self.assertIsNotNone(behaviour.build.log)
- self.assertEqual(1, queue_item.destroySelf.call_count)
+ self.assertIs(None, behaviour.build.buildqueue_record)
self.assertEqual(0, behaviour._uploadTarball.call_count)
d.addCallback(got_status)
@@ -194,7 +196,8 @@
# not faze the Behaviour class.
behaviour = self.makeBehaviour()
behaviour._uploadTarball = FakeMethod()
- queue_item = FakeBuildQueue(behaviour)
+ queue_item = behaviour.build.queueBuild()
+ queue_item.markAsBuilding(self.factory.makeBuilder())
slave = behaviour._slave
d = slave.status()
@@ -208,7 +211,7 @@
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
- self.assertEqual(1, queue_item.destroySelf.call_count)
+ self.assertIs(None, behaviour.build.buildqueue_record)
self.assertEqual(0, behaviour._uploadTarball.call_count)
d.addCallback(got_status)
@@ -220,7 +223,8 @@
branch = productseries.branch
behaviour = self.makeBehaviour(
branch=branch, filemap={'translation-templates.tar.gz': 'foo'})
- queue_item = FakeBuildQueue(behaviour)
+ queue_item = behaviour.build.queueBuild()
+ queue_item.markAsBuilding(self.factory.makeBuilder())
slave = behaviour._slave
def fake_getFile(sum, file):
Follow ups
