build: implement stop build

this commit implements the stop of builds
with associated integration tests.

Change-Id: Ie81306b32cacebb1e277b8d3bf785a7ae2ccf2de
Signed-off-by: Ion Alberdi <ialberdi@intel.com>

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):
 

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):

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,21 +483,22 @@ def lostRemote(self, conn=None):
                 lock.stopWaitingUntilAvailable(self, access, d)
                 d.callback(None)
 
-    def stopBuild(self, reason="<no reason given>"):
+    def stopBuild(self, reason="<no reason given>", 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
         # reason might be to abandon a stuck build. We want to mark the
         # 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()

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
 

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())

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)

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: