launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15822
[Merge] lp:~wgrant/launchpad/builderinteractor-updateBuild into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/builderinteractor-updateBuild into lp:launchpad.
Commit message:
Move updateBuild from BuildFarmJobBehavior to BuilderInteractor.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/builderinteractor-updateBuild/+merge/182600
Move updateBuild from BuildFarmJobBehavior to BuilderInteractor. BuildFarmJobBehaviors should override handleStatus instead, as all but TranslationTemplatesBuildBehavior already did.
This gets us closer to running BuilderInteractor without touching the database most of the time.
--
https://code.launchpad.net/~wgrant/launchpad/builderinteractor-updateBuild/+merge/182600
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/builderinteractor-updateBuild into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py 2013-08-27 11:35:09 +0000
+++ lib/lp/buildmaster/interactor.py 2013-08-28 11:11:24 +0000
@@ -15,6 +15,7 @@
from urlparse import urlparse
import xmlrpclib
+import transaction
from twisted.internet import defer
from twisted.web import xmlrpc
from twisted.web.client import downloadPage
@@ -35,9 +36,12 @@
IBuildFarmJobBehavior,
)
from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
+from lp.services import encoding
from lp.services.config import config
from lp.services.helpers import filenameToContentType
+from lp.services.job.interfaces.job import JobStatus
from lp.services.librarian.interfaces import ILibraryFileAliasSet
+from lp.services.librarian.interfaces.client import ILibrarianClient
from lp.services.librarian.utils import copy_and_close
from lp.services.twistedsupport import cancel_on_timeout
from lp.services.twistedsupport.processmonitor import ProcessWithTimeout
@@ -610,7 +614,133 @@
:return: A Deferred that fires when the slave dialog is finished.
"""
- return self._current_build_behavior.updateBuild(queueItem)
+ logger = logging.getLogger('slave-scanner')
+
+ d = self.slaveStatus()
+
+ def got_failure(failure):
+ failure.trap(xmlrpclib.Fault, socket.error)
+ info = failure.value
+ info = ("Could not contact the builder %s, caught a (%s)"
+ % (queueItem.builder.url, info))
+ raise BuildSlaveFailure(info)
+
+ def got_status(slave_status):
+ builder_status_handlers = {
+ 'BuilderStatus.IDLE': self.updateBuild_IDLE,
+ 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
+ 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
+ 'BuilderStatus.ABORTED': self.updateBuild_ABORTED,
+ 'BuilderStatus.WAITING': self.updateBuild_WAITING,
+ }
+
+ builder_status = slave_status['builder_status']
+ if builder_status not in builder_status_handlers:
+ logger.critical(
+ "Builder on %s returned unknown status %s, failing it"
+ % (self.builder.url, builder_status))
+ self.builder.failBuilder(
+ "Unknown status code (%s) returned from status() probe."
+ % builder_status)
+ # XXX: This will leave the build and job in a bad state, but
+ # should never be possible, since our builder statuses are
+ # known.
+ queueItem._builder = None
+ queueItem.setDateStarted(None)
+ transaction.commit()
+ return
+
+ # Since logtail is a xmlrpclib.Binary container and it is
+ # returned from the IBuilder content class, it arrives
+ # protected by a Zope Security Proxy, which is not declared,
+ # thus empty. Before passing it to the status handlers we
+ # will simply remove the proxy.
+ logtail = removeSecurityProxy(slave_status.get('logtail'))
+
+ method = builder_status_handlers[builder_status]
+ return defer.maybeDeferred(
+ method, queueItem, slave_status, logtail, logger)
+
+ d.addErrback(got_failure)
+ d.addCallback(got_status)
+ return d
+
+ def updateBuild_IDLE(self, queueItem, slave_status, logtail, logger):
+ """Somehow the builder forgot about the build job.
+
+ Log this and reset the record.
+ """
+ logger.warn(
+ "Builder %s forgot about buildqueue %d -- resetting buildqueue "
+ "record" % (queueItem.builder.url, queueItem.id))
+ queueItem.reset()
+ transaction.commit()
+
+ def updateBuild_BUILDING(self, queueItem, slave_status, logtail, logger):
+ """Build still building, collect the logtail"""
+ if queueItem.job.status != JobStatus.RUNNING:
+ queueItem.job.start()
+ queueItem.logtail = encoding.guess(str(logtail))
+ transaction.commit()
+
+ def updateBuild_ABORTING(self, queueItem, slave_status, logtail, logger):
+ """Build was ABORTED.
+
+ Master-side should wait until the slave finish the process correctly.
+ """
+ queueItem.logtail = "Waiting for slave process to be terminated"
+ transaction.commit()
+
+ def updateBuild_ABORTED(self, queueItem, slave_status, logtail, logger):
+ """ABORTING process has successfully terminated.
+
+ Clean the builder for another jobs.
+ """
+ d = self.cleanSlave()
+
+ def got_cleaned(ignored):
+ queueItem.builder = None
+ if queueItem.job.status != JobStatus.FAILED:
+ queueItem.job.fail()
+ queueItem.specific_job.jobAborted()
+ transaction.commit()
+ return d.addCallback(got_cleaned)
+
+ def extractBuildStatus(self, slave_status):
+ """Read build status name.
+
+ :param slave_status: build status dict as passed to the
+ updateBuild_* methods.
+ :return: the unqualified status name, e.g. "OK".
+ """
+ status_string = slave_status['build_status']
+ lead_string = 'BuildStatus.'
+ assert status_string.startswith(lead_string), (
+ "Malformed status string: '%s'" % status_string)
+
+ return status_string[len(lead_string):]
+
+ def updateBuild_WAITING(self, queueItem, slave_status, logtail, logger):
+ """Perform the actions needed for a slave in a WAITING state
+
+ Buildslave can be WAITING in five situations:
+
+ * Build has failed, no filemap is received (PACKAGEFAIL, DEPFAIL,
+ CHROOTFAIL, BUILDERFAIL,
+ ABORTED)
+
+ * Build has been built successfully (BuildStatus.OK), in this case
+ we have a 'filemap', so we can retrieve those files and store in
+ Librarian with getFileFromSlave() and then pass the binaries to
+ the uploader for processing.
+ """
+ librarian = getUtility(ILibrarianClient)
+ build_status = self.extractBuildStatus(slave_status)
+
+ # XXX: dsilvers 2005-03-02: Confirm the builder has the right build?
+ d = self._current_build_behavior.handleStatus(
+ build_status, librarian, slave_status)
+ return d
def transferSlaveFileToLibrarian(self, file_sha1, filename, private):
"""Transfer a file from the slave to the librarian.
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2013-08-22 04:07:24 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2013-08-28 11:11:24 +0000
@@ -66,11 +66,10 @@
supposed to be.
"""
- def updateBuild(queueItem):
- """Verify the current build job status.
-
- Perform the required actions for each state.
-
- :param queueItem: The `BuildQueue` for the build.
- :return: A Deferred that fires when the update is done.
+ def handleStatus(status, librarian, slave_status):
+ """Update the build from a WAITING slave result.
+
+ :param status: The tail of the BuildStatus (eg. OK or PACKAGEFAIL).
+ :param librarian: An `ILibrarianClient` to use for file uploads.
+ :param slave_status: Slave status from `BuilderInteractor.slaveStatus`.
"""
=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py 2013-08-28 04:40:32 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py 2013-08-28 11:11:24 +0000
@@ -13,14 +13,10 @@
import datetime
import logging
import os.path
-import socket
-import xmlrpclib
import transaction
from twisted.internet import defer
-from zope.component import getUtility
from zope.interface import implements
-from zope.security.proxy import removeSecurityProxy
from lp.buildmaster.enums import (
BuildFarmJobType,
@@ -34,10 +30,7 @@
BuildBehaviorMismatch,
IBuildFarmJobBehavior,
)
-from lp.services import encoding
from lp.services.config import config
-from lp.services.job.interfaces.job import JobStatus
-from lp.services.librarian.interfaces.client import ILibrarianClient
SLAVE_LOG_FILENAME = 'buildlog'
@@ -105,135 +98,6 @@
self.build.setLog(lfa_id)
transaction.commit()
- def updateBuild(self, queueItem):
- """See `IBuildFarmJobBehavior`."""
- logger = logging.getLogger('slave-scanner')
-
- d = self._interactor.slaveStatus()
-
- def got_failure(failure):
- failure.trap(xmlrpclib.Fault, socket.error)
- info = failure.value
- info = ("Could not contact the builder %s, caught a (%s)"
- % (queueItem.builder.url, info))
- raise BuildSlaveFailure(info)
-
- def got_status(slave_status):
- builder_status_handlers = {
- 'BuilderStatus.IDLE': self.updateBuild_IDLE,
- 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
- 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
- 'BuilderStatus.ABORTED': self.updateBuild_ABORTED,
- 'BuilderStatus.WAITING': self.updateBuild_WAITING,
- }
-
- builder_status = slave_status['builder_status']
- if builder_status not in builder_status_handlers:
- logger.critical(
- "Builder on %s returned unknown status %s, failing it"
- % (self._builder.url, builder_status))
- self._builder.failBuilder(
- "Unknown status code (%s) returned from status() probe."
- % builder_status)
- # XXX: This will leave the build and job in a bad state, but
- # should never be possible, since our builder statuses are
- # known.
- queueItem._builder = None
- queueItem.setDateStarted(None)
- transaction.commit()
- return
-
- # Since logtail is a xmlrpclib.Binary container and it is
- # returned from the IBuilder content class, it arrives
- # protected by a Zope Security Proxy, which is not declared,
- # thus empty. Before passing it to the status handlers we
- # will simply remove the proxy.
- logtail = removeSecurityProxy(slave_status.get('logtail'))
-
- method = builder_status_handlers[builder_status]
- return defer.maybeDeferred(
- method, queueItem, slave_status, logtail, logger)
-
- d.addErrback(got_failure)
- d.addCallback(got_status)
- return d
-
- def updateBuild_IDLE(self, queueItem, slave_status, logtail, logger):
- """Somehow the builder forgot about the build job.
-
- Log this and reset the record.
- """
- logger.warn(
- "Builder %s forgot about buildqueue %d -- resetting buildqueue "
- "record" % (queueItem.builder.url, queueItem.id))
- queueItem.reset()
- transaction.commit()
-
- def updateBuild_BUILDING(self, queueItem, slave_status, logtail, logger):
- """Build still building, collect the logtail"""
- if queueItem.job.status != JobStatus.RUNNING:
- queueItem.job.start()
- queueItem.logtail = encoding.guess(str(logtail))
- transaction.commit()
-
- def updateBuild_ABORTING(self, queueItem, slave_status, logtail, logger):
- """Build was ABORTED.
-
- Master-side should wait until the slave finish the process correctly.
- """
- queueItem.logtail = "Waiting for slave process to be terminated"
- transaction.commit()
-
- def updateBuild_ABORTED(self, queueItem, slave_status, logtail, logger):
- """ABORTING process has successfully terminated.
-
- Clean the builder for another jobs.
- """
- d = self._interactor.cleanSlave()
-
- def got_cleaned(ignored):
- queueItem.builder = None
- if queueItem.job.status != JobStatus.FAILED:
- queueItem.job.fail()
- queueItem.specific_job.jobAborted()
- transaction.commit()
- return d.addCallback(got_cleaned)
-
- def extractBuildStatus(self, slave_status):
- """Read build status name.
-
- :param slave_status: build status dict as passed to the
- updateBuild_* methods.
- :return: the unqualified status name, e.g. "OK".
- """
- status_string = slave_status['build_status']
- lead_string = 'BuildStatus.'
- assert status_string.startswith(lead_string), (
- "Malformed status string: '%s'" % status_string)
-
- return status_string[len(lead_string):]
-
- def updateBuild_WAITING(self, queueItem, slave_status, logtail, logger):
- """Perform the actions needed for a slave in a WAITING state
-
- Buildslave can be WAITING in five situations:
-
- * Build has failed, no filemap is received (PACKAGEFAIL, DEPFAIL,
- CHROOTFAIL, BUILDERFAIL,
- ABORTED)
-
- * Build has been built successfully (BuildStatus.OK), in this case
- we have a 'filemap', so we can retrieve those files and store in
- Librarian with getFileFromSlave() and then pass the binaries to
- the uploader for processing.
- """
- librarian = getUtility(ILibrarianClient)
- build_status = self.extractBuildStatus(slave_status)
-
- # XXX: dsilvers 2005-03-02: Confirm the builder has the right build?
- d = self.handleStatus(build_status, librarian, slave_status)
- return d
-
# The list of build status values for which email notifications are
# 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
@@ -241,7 +105,7 @@
ALLOWED_STATUS_NOTIFICATIONS = ['OK', 'PACKAGEFAIL', 'CHROOTFAIL']
def handleStatus(self, status, librarian, slave_status):
- """See `IPackageBuild`."""
+ """See `IBuildFarmJobBehavior`."""
from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
send_notification = status in self.ALLOWED_STATUS_NOTIFICATIONS
=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py 2013-08-27 10:33:01 +0000
+++ lib/lp/buildmaster/tests/test_builder.py 2013-08-28 11:11:24 +0000
@@ -251,6 +251,23 @@
run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
+ def test_extractBuildStatus_baseline(self):
+ # extractBuildStatus picks the name of the build status out of a
+ # dict describing the slave's status.
+ slave_status = {'build_status': 'BuildStatus.BUILDING'}
+ interactor = BuilderInteractor(MockBuilder())
+ self.assertEqual(
+ BuildStatus.BUILDING.name,
+ interactor.extractBuildStatus(slave_status))
+
+ def test_extractBuildStatus_malformed(self):
+ # extractBuildStatus errors out when the status string is not
+ # of the form it expects.
+ slave_status = {'build_status': 'BUILDING'}
+ interactor = BuilderInteractor(MockBuilder())
+ self.assertRaises(
+ AssertionError, interactor.extractBuildStatus, slave_status)
+
def test_updateStatus_aborts_lost_and_broken_slave(self):
# A slave that's 'lost' should be aborted; when the slave is
# broken then abort() should also throw a fault.
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-27 11:35:09 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-08-28 11:11:24 +0000
@@ -78,23 +78,6 @@
"""Create a `BuildQueue` object."""
return self.factory.makeSourcePackageRecipeBuildJob()
- def test_extractBuildStatus_baseline(self):
- # extractBuildStatus picks the name of the build status out of a
- # dict describing the slave's status.
- slave_status = {'build_status': 'BuildStatus.BUILDING'}
- behavior = self._makeBehavior()
- self.assertEqual(
- BuildStatus.BUILDING.name,
- behavior.extractBuildStatus(slave_status))
-
- def test_extractBuildStatus_malformed(self):
- # extractBuildStatus errors out when the status string is not
- # of the form it expects.
- slave_status = {'build_status': 'BUILDING'}
- behavior = self._makeBehavior()
- self.assertRaises(
- AssertionError, behavior.extractBuildStatus, slave_status)
-
def test_cookie_baseline(self):
buildfarmjob = self.factory.makeTranslationTemplatesBuildJob()
=== modified file 'lib/lp/code/model/recipebuilder.py'
--- lib/lp/code/model/recipebuilder.py 2013-08-19 23:23:19 +0000
+++ lib/lp/code/model/recipebuilder.py 2013-08-28 11:11:24 +0000
@@ -43,8 +43,6 @@
ALLOWED_STATUS_NOTIFICATIONS = [
'OK', 'PACKAGEFAIL', 'DEPFAIL', 'CHROOTFAIL']
- status = None
-
@property
def build(self):
return self.buildfarmjob.build
=== modified file 'lib/lp/translations/model/translationtemplatesbuildbehavior.py'
--- lib/lp/translations/model/translationtemplatesbuildbehavior.py 2013-08-19 23:23:19 +0000
+++ lib/lp/translations/model/translationtemplatesbuildbehavior.py 2013-08-28 11:11:24 +0000
@@ -11,6 +11,7 @@
'TranslationTemplatesBuildBehavior',
]
+import logging
import os
import tempfile
@@ -118,8 +119,8 @@
status['filemap'] = raw_slave_status[3]
@defer.inlineCallbacks
- def updateBuild_WAITING(self, queue_item, slave_status, logtail, logger):
- """Deal with a finished ("WAITING" state, perversely) build job.
+ def handleStatus(self, status, librarian, slave_status, queue_item=None):
+ """Deal with a finished build job.
Retrieves tarball and logs from the slave, then cleans up the
slave so it's ready for a next job and destroys the queue item.
@@ -127,15 +128,17 @@
If this fails for whatever unforeseen reason, a future run will
retry it.
"""
- build_status = self.extractBuildStatus(slave_status)
-
+ from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
+ logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
+ if queue_item is None:
+ queue_item = self.build.buildqueue_record
logger.info(
"Templates generation job %s for %s finished with status %s." % (
queue_item.specific_job.getName(),
queue_item.specific_job.branch.bzr_identity,
- build_status))
+ status))
- if build_status == 'OK':
+ if status == 'OK':
self.build.updateStatus(
BuildStatus.UPLOADING, builder=queue_item.builder)
transaction.commit()
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py'
--- lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2013-08-27 09:17:07 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py 2013-08-28 11:11:24 +0000
@@ -188,7 +188,7 @@
d = behavior._readTarball(buildqueue, {path: path}, logging)
return d.addCallback(got_tarball)
- def test_updateBuild_WAITING_OK(self):
+ def test_handleStatus_OK(self):
# Hopefully, a build will succeed and produce a tarball.
behavior = self.makeBehavior()
behavior._uploadTarball = FakeMethod()
@@ -212,8 +212,11 @@
'build_status': status[1],
'filemap': {'translation-templates.tar.gz': 'foo'},
}
- return behavior.updateBuild_WAITING(
- queue_item, slave_status, None, logging), slave_call_log
+ return (
+ behavior.handleStatus(
+ behavior._interactor.extractBuildStatus(slave_status),
+ None, slave_status, queue_item=queue_item),
+ slave_call_log)
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behavior.build.status)
@@ -229,7 +232,7 @@
d.addCallback(build_updated)
return d
- def test_updateBuild_WAITING_failed(self):
+ def test_handleStatus_failed(self):
# Builds may also fail (and produce no tarball).
behavior = self.makeBehavior()
behavior._uploadTarball = FakeMethod()
@@ -247,15 +250,15 @@
'BuildStatus.FAILEDTOBUILD',
status[2],
)
- status_dict = {
+ slave_status = {
'builder_status': raw_status[0],
'build_status': raw_status[1],
}
- behavior.updateSlaveStatus(raw_status, status_dict)
- self.assertNotIn('filemap', status_dict)
-
- return behavior.updateBuild_WAITING(
- queue_item, status_dict, None, logging)
+ behavior.updateSlaveStatus(raw_status, slave_status)
+ self.assertNotIn('filemap', slave_status)
+ return behavior.handleStatus(
+ behavior._interactor.extractBuildStatus(slave_status),
+ None, slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FAILEDTOBUILD, behavior.build.status)
@@ -270,7 +273,7 @@
d.addCallback(build_updated)
return d
- def test_updateBuild_WAITING_notarball(self):
+ def test_handleStatus_notarball(self):
# Even if the build status is "OK," absence of a tarball will
# not faze the Behavior class.
behavior = self.makeBehavior()
@@ -288,14 +291,15 @@
'BuildStatus.OK',
status[2],
)
- status_dict = {
+ slave_status = {
'builder_status': raw_status[0],
'build_status': raw_status[1],
}
- behavior.updateSlaveStatus(raw_status, status_dict)
- self.assertFalse('filemap' in status_dict)
- return behavior.updateBuild_WAITING(
- queue_item, status_dict, None, logging)
+ behavior.updateSlaveStatus(raw_status, slave_status)
+ self.assertFalse('filemap' in slave_status)
+ return behavior.handleStatus(
+ behavior._interactor.extractBuildStatus(slave_status),
+ None, slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behavior.build.status)
@@ -308,7 +312,7 @@
d.addCallback(build_updated)
return d
- def test_updateBuild_WAITING_uploads(self):
+ def test_handleStatus_uploads(self):
productseries = self.makeProductSeriesWithBranchForTranslation()
branch = productseries.branch
behavior = self.makeBehavior(branch=branch)
@@ -336,8 +340,9 @@
'build_id': status[2],
}
behavior.updateSlaveStatus(status, slave_status)
- return behavior.updateBuild_WAITING(
- queue_item, slave_status, None, logging)
+ return behavior.handleStatus(
+ behavior._interactor.extractBuildStatus(slave_status),
+ None, slave_status, queue_item=queue_item),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behavior.build.status)
Follow ups
