diff --git a/master/buildbot/data/buildrequests.py b/master/buildbot/data/buildrequests.py index fe84b78aa0d..f84bbe19e43 100644 --- a/master/buildbot/data/buildrequests.py +++ b/master/buildbot/data/buildrequests.py @@ -18,6 +18,8 @@ from buildbot.db.buildrequests import AlreadyClaimedError from buildbot.db.buildrequests import NotClaimedError +from buildbot.status import results + from twisted.internet import defer from twisted.internet import reactor @@ -62,6 +64,39 @@ def startConsuming(self, callback, options, kwargs): return self.master.mq.startConsuming(callback, ('buildrequests', buildrequestid, None)) + @defer.inlineCallbacks + def control(self, action, args, kwargs): + if action != "cancel": + raise ValueError("action: {} is not supported".format(action)) + brid = kwargs['buildrequestid'] + # first, try to claim the request; if this fails, then it's too late to + # cancel the build anyway + try: + b = yield self.master.db.buildrequests.claimBuildRequests(brids=[brid]) + except AlreadyClaimedError: + # XXX race condition + # - After a buildrequest was claimed, and + # - Before creating a build, + # the claiming master still + # needs to do some processing, (send a message to the message queue, + # call maybeStartBuild on the related builder). + # In that case we won't have the related builds here. We don't have + # an alternative to letting them run without stopping them for now. + builds = yield self.master.data.get(("buildrequests", brid, "builds")) + + # Don't call the data API here, as the buildrequests might have been + # taken by another master. We just send the stop message and forget about those. + for b in builds: + self.master.mq.produce(("control", "builds", str(b['buildid']), "stop"), + dict(reason=kwargs.get('reason'))) + defer.returnValue(None) + + # then complete it with 'CANCELLED'; this is the closest we can get to + # cancelling a request without running into trouble with dangling + # references. + yield self.master.data.updates.completeBuildRequests([brid], + results.CANCELLED) + class BuildRequestsEndpoint(Db2DataMixin, base.Endpoint): diff --git a/master/buildbot/data/builds.py b/master/buildbot/data/builds.py index 56e4ed2dbfb..63cdfa7550f 100644 --- a/master/buildbot/data/builds.py +++ b/master/buildbot/data/builds.py @@ -67,6 +67,14 @@ def startConsuming(self, callback, options, kwargs): return self.master.mq.startConsuming(callback, ('builds', str(buildid), None)) + def control(self, action, args, kwargs): + if action != "stop": + raise ValueError("action: {} is not supported".format(action)) + self.master.mq.produce(("control", "builds", + str(kwargs['buildid']), 'stop'), + dict(reason=kwargs.get('reason', 'no reason'))) + return defer.succeed(None) + class BuildsEndpoint(Db2DataMixin, base.Endpoint): @@ -151,6 +159,7 @@ def addBuild(self, builderid, buildrequestid, buildslaveid): @base.updateMethod def generateNewBuildEvent(self, buildid): return self.generateEvent(buildid, "new") + @base.updateMethod @defer.inlineCallbacks def setBuildStateString(self, buildid, state_string): diff --git a/master/buildbot/process/build.py b/master/buildbot/process/build.py index 9139b106e07..6866e2b3ce6 100644 --- a/master/buildbot/process/build.py +++ b/master/buildbot/process/build.py @@ -237,6 +237,7 @@ def startBuild(self, build_status, expectations, slavebuilder): # the Deferred returned by this method. log.msg("%s.startBuild" % self) + self.build_status = build_status # TODO: this will go away when build collapsing is implemented; until # then we just assign the bulid to the first buildrequest @@ -247,7 +248,12 @@ def startBuild(self, build_status, expectations, slavebuilder): buildrequestid=brid, buildslaveid=slave.buildslaveid) + self.stopBuildConsumer = yield self.master.mq.startConsuming(self.stopBuild, + ("control", "builds", + str(self.buildid), + "stop")) yield self.master.data.updates.generateNewBuildEvent(self.buildid) + # now that we have a build_status, we can set properties self.setupProperties() self.setupSlaveBuilder(slavebuilder) @@ -477,7 +483,7 @@ def lostRemote(self, conn=None): lock.stopWaitingUntilAvailable(self, access, d) d.callback(None) - def stopBuild(self, reason=""): + def stopBuild(self, reason="", cbParams=None): # the idea here is to let the user cancel a build because, e.g., # they realized they committed a bug and they don't want to waste # the time building something that they know will fail. Another @@ -485,13 +491,14 @@ def stopBuild(self, reason=""): # build as failed quickly rather than waiting for the slave's # timeout to kill it on its own. - log.msg(" %s: stopping build: %s" % (self, reason)) + log.msg(" %s: stopping build: %s %s" % (self, reason, cbParams)) if self.finished: return # TODO: include 'reason' in this point event self.stopped = True if self.currentStep: self.currentStep.interrupt(reason) + self.results = CANCELLED if self._acquiringLock: @@ -536,7 +543,7 @@ def buildFinished(self, text, results): If 'results' is SUCCESS or WARNINGS, we will permit any dependant builds to start. If it is 'FAILURE', those builds will be abandoned.""" - + self.stopBuildConsumer.stopConsuming() self.finished = True if self.conn: self.subs.unsubscribe() diff --git a/master/buildbot/steps/trigger.py b/master/buildbot/steps/trigger.py index 44d22ec5104..ed5ee133371 100644 --- a/master/buildbot/steps/trigger.py +++ b/master/buildbot/steps/trigger.py @@ -89,11 +89,16 @@ def __init__(self, schedulerNames=None, sourceStamp=None, sourceStamps=None, BuildStep.__init__(self, **kwargs) def interrupt(self, reason): - # FIXME: new style step cannot be interrupted anymore by just calling finished() - # (which was a bad idea in the first place) - # we should claim the self.brids, and CANCELLED them - # if they were already claimed, stop the associated builds via data api - # then the big deferredlist will automatically be called + # We cancel the buildrequests, as the data api handles + # both cases: + # - build started: stop is sent, + # - build not created yet: related buildrequests are set to CANCELLED. + # Note that there is an identified race condition though (more details + # are available at buildbot.data.buildrequests). + for brid in self.brids: + self.master.data.control("cancel", + {'reason': 'parent build was interrupted'}, + ("buildrequests", brid)) if self.running and not self.ended: self.ended = True diff --git a/master/buildbot/test/fake/fakedata.py b/master/buildbot/test/fake/fakedata.py index 71bd5cd55fa..d69d63a6f7f 100644 --- a/master/buildbot/test/fake/fakedata.py +++ b/master/buildbot/test/fake/fakedata.py @@ -290,6 +290,7 @@ def generateNewBuildEvent(self, buildid): validation.verifyType(self.testcase, 'buildid', buildid, validation.IntValidator()) return defer.succeed(None) + def setBuildStateString(self, buildid, state_string): validation.verifyType(self.testcase, 'buildid', buildid, validation.IntValidator()) diff --git a/master/buildbot/test/integration/test_stop_trigger.py b/master/buildbot/test/integration/test_stop_trigger.py new file mode 100644 index 00000000000..b6075ac31bb --- /dev/null +++ b/master/buildbot/test/integration/test_stop_trigger.py @@ -0,0 +1,130 @@ +# This file is part of Buildbot. Buildbot is free software: you can +# redistribute it and/or modify it under the terms of the GNU General Public +# License as published by the Free Software Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright Buildbot Team Members + +from twisted.internet import defer + +from buildbot.test.util.integration import RunMasterBase +from buildbot.status.results import CANCELLED +from buildbot.config import BuilderConfig +from buildbot.process.factory import BuildFactory +from buildbot.plugins import steps, schedulers + +# This integration test creates a master and slave environment, +# with two builders and a trigger step linking them. the triggered build never ends +# so that we can reliabily stop it recursively + + +# master configurations +def setupTriggerConfiguration(triggeredFactory, nextBuild=None): + c = {} + + c['schedulers'] = [ + schedulers.Triggerable( + name="trigsched", + builderNames=["triggered"]), + schedulers.AnyBranchScheduler( + name="sched", + builderNames=["main"])] + + f = BuildFactory() + f.addStep(steps.Trigger(schedulerNames=['trigsched'], + waitForFinish=True, + updateSourceStamp=True)) + f.addStep(steps.ShellCommand(command='echo world')) + + mainBuilder = BuilderConfig(name="main", + slavenames=["local1"], + factory=f) + + triggeredBuilderKwargs = {'name': "triggered", + 'slavenames': ["local1"], + 'factory': triggeredFactory} + + if nextBuild is not None: + triggeredBuilderKwargs['nextBuild'] = nextBuild + + triggeredBuilder = BuilderConfig(**triggeredBuilderKwargs) + + c['builders'] = [mainBuilder, triggeredBuilder] + return c + + +def triggerRunsForever(): + f2 = BuildFactory() + f2.addStep(steps.ShellCommand(command="\n".join(['while :', + 'do', + ' echo "sleeping";', + ' sleep 1;' + 'done']))) + + return setupTriggerConfiguration(f2) + + +def triggeredBuildIsNotCreated(): + f2 = BuildFactory() + f2.addStep(steps.ShellCommand(command="echo 'hello'")) + + def nextBuild(*args, **kwargs): + return defer.succeed(None) + return setupTriggerConfiguration(f2, nextBuild=nextBuild) + + +class TriggeringMaster(RunMasterBase): + testCasesHandleTheirSetup = True + change = dict(branch="master", + files=["foo.c"], + author="me@foo.com", + comments="good stuff", + revision="HEAD", + project="none") + + def assertBuildIsCancelled(self, b): + self.assertTrue(b['complete']) + self.assertEquals(b['results'], CANCELLED) + + @defer.inlineCallbacks + def runTest(self, newBuildCallback): + newConsumer = yield self.master.mq.startConsuming( + newBuildCallback, + ('builds', None, 'new')) + build = yield self.doForceBuild(wantSteps=True, + useChange=self.change, + wantLogs=True) + self.assertBuildIsCancelled(build) + newConsumer.stopConsuming() + builds = yield self.master.data.get(("builds",)) + for b in builds: + self.assertBuildIsCancelled(b) + + @defer.inlineCallbacks + def testTriggerRunsForever(self): + yield self.setupConfig("triggerRunsForever") + self.higherBuild = None + + def newCallback(_, data): + if self.higherBuild is None: + self.higherBuild = data['buildid'] + else: + self.master.data.control("stop", {}, ("builds", self.higherBuild)) + self.higherBuild = None + yield self.runTest(newCallback) + + @defer.inlineCallbacks + def testTriggeredBuildIsNotCreated(self): + yield self.setupConfig("triggeredBuildIsNotCreated") + + def newCallback(_, data): + self.master.data.control("stop", {}, ("builds", data['buildid'])) + yield self.runTest(newCallback) diff --git a/master/buildbot/test/util/integration.py b/master/buildbot/test/util/integration.py index 274dc72579b..cbc6b108d43 100644 --- a/master/buildbot/test/util/integration.py +++ b/master/buildbot/test/util/integration.py @@ -34,9 +34,24 @@ class RunMasterBase(dirs.DirsMixin, www.RequiresWwwMixin, unittest.TestCase): proto = "null" + # If True the test cases must handle the configuration + # of the master in the self.master attribute themselves. + # The setupConfig could help the module in that task. + # Note that whether testCaseHandleTheirSetup is False or True + # in all cases, tearDown that stops the master defined in self.master + # will be called. + testCasesHandleTheirSetup = False @defer.inlineCallbacks - def setUp(self): + def setupConfig(self, configFunc): + """ + Setup and start a master configured + by the function configFunc defined in the test module. + @type configFunc: string + @param configFunc: name of a function + without argument defined in the test module + that returns a BuildmasterConfig object. + """ self.basedir = os.path.abspath('basdir') self.setUpDirs(self.basedir) self.configfile = os.path.join(self.basedir, 'master.cfg') @@ -49,11 +64,13 @@ def setUp(self): # be changed open(self.configfile, "w").write(textwrap.dedent(""" from buildbot.buildslave import BuildSlave - from %s import masterConfig - c = BuildmasterConfig = masterConfig() + from %s import %s + c = BuildmasterConfig = %s() c['slaves'] = [BuildSlave("local1", "localpw")] c['protocols'] = %s - """ % (self.__class__.__module__, proto))) + """ % (self.__class__.__module__, + configFunc, configFunc, + proto))) # create the master and set its config m = BuildMaster(self.basedir, self.configfile) self.master = m @@ -86,6 +103,11 @@ def setUp(self): s = LocalBuildSlave("local1", self.basedir, False) s.setServiceParent(m) + def setUp(self): + if self.testCasesHandleTheirSetup: + return defer.succeed(None) + return self.setupConfig("masterConfig") + @defer.inlineCallbacks def tearDown(self): if not self._passed: