Merge pull request #1670 from vlovich/renderable-locks-master

Renderable locks master

master/buildbot/process/buildstep.py

@@ -438,6 +438,8 @@ def startStep(self, remote):
             name=util.ascii2unicode(self.name))
         yield self.master.data.updates.startStep(self.stepid)
 
+        self.locks = yield self.build.render(self.locks)
+
         # convert all locks into their real form
         self.locks = [(self.build.builder.botmaster.getLockFromLockAccess(access), access)
                       for access in self.locks]

master/buildbot/test/fake/fakebuild.py

@@ -18,8 +18,10 @@
 
 from buildbot import config
 from buildbot import interfaces
+from buildbot.buildslave import base
 from buildbot.process import factory
 from buildbot.process import properties
+from buildbot.process import slavebuilder
 from buildbot.test.fake import fakemaster
 from twisted.python import components
 
@@ -44,6 +46,8 @@ class FakeBuild(properties.PropertiesMixin):
     def __init__(self, props=None, master=None):
         self.build_status = FakeBuildStatus()
         self.builder = fakemaster.FakeBuilderStatus(master)
+        self.slavebuilder = mock.Mock(spec=slavebuilder.SlaveBuilder)
+        self.slavebuilder.slave = mock.Mock(spec=base.BuildSlave)
         self.builder.config = config.BuilderConfig(
             name='bldr',
             slavenames=['a'],
@@ -52,6 +56,7 @@ def __init__(self, props=None, master=None):
         self.buildid = 92
         self.number = 13
         self.workdir = 'build'
+        self.locks = []
 
         self.sources = {}
         if props is None:

master/buildbot/test/fake/fakemaster.py

@@ -107,6 +107,7 @@ class FakeBuilderStatus(object):
     def __init__(self, master=None, buildername="Builder"):
         if master:
             self.master = master
+            self.botmaster = master.botmaster
             self.basedir = os.path.join(master.basedir, 'bldr')
         self.lastBuildStatus = None
         self._tags = None

master/buildbot/test/unit/test_process_buildstep.py

@@ -15,10 +15,12 @@
 
 import mock
 
+from buildbot import locks
 from buildbot.interfaces import BuildSlaveTooOldError
 from buildbot.process import buildstep
 from buildbot.process import properties
 from buildbot.process import remotecommand
+from buildbot.process.properties import renderer
 from buildbot.status.results import EXCEPTION
 from buildbot.status.results import FAILURE
 from buildbot.status.results import SKIPPED
@@ -113,6 +115,41 @@ def test_setProperty(self):
         bs.setProperty("x", "abc", "test", runtime=True)
         props.setProperty.assert_called_with("x", "abc", "test", runtime=True)
 
+    @defer.inlineCallbacks
+    def test_renderableLocks(self):
+        lock1 = mock.Mock(spec=locks.MasterLock)
+        lock1.name = "masterlock"
+
+        lock2 = mock.Mock(spec=locks.SlaveLock)
+        lock2.name = "slavelock"
+
+        renderedLocks = [False]
+
+        @renderer
+        def rendered_locks(props):
+            renderedLocks[0] = True
+            access1 = locks.LockAccess(lock1, 'counting')
+            access2 = locks.LockAccess(lock2, 'exclusive')
+            return [access1, access2]
+
+        self.setupStep(self.FakeBuildStep(locks=rendered_locks))
+        self.expectOutcome(result=SUCCESS)
+        yield self.runStep()
+
+        self.assertTrue(renderedLocks[0])
+
+    @defer.inlineCallbacks
+    def test_regularLocks(self):
+        lock1 = mock.Mock(spec=locks.MasterLock)
+        lock1.name = "masterlock"
+
+        lock2 = mock.Mock(spec=locks.SlaveLock)
+        lock2.name = "slavelock"
+
+        self.setupStep(self.FakeBuildStep(locks=[locks.LockAccess(lock1, 'counting'), locks.LockAccess(lock2, 'exclusive')]))
+        self.expectOutcome(result=SUCCESS)
+        yield self.runStep()
+
     @defer.inlineCallbacks
     def test_runCommand(self):
         bs = buildstep.BuildStep()

master/docs/manual/cfg-buildsteps.rst

@@ -142,6 +142,8 @@ Arguments common to all :class:`BuildStep` subclasses:
 
 ``locks``
     a list of ``Locks`` (instances of :class:`buildbot.locks.SlaveLock` or :class:`buildbot.locks.MasterLock`) that should be acquired before starting this :class:`Step`.
+    Alternatively this could be a renderable that returns this list during build execution.
+    This lets you defer picking the locks to acquire until the build step is about to start running.
     The ``Locks`` will be released when the step is complete.
     Note that this is a list of actual :class:`Lock` instances, not names.
     Also note that all Locks must have unique names.

master/docs/manual/cfg-interlocks.rst

@@ -121,6 +121,8 @@ Each slave performs the same checkout, make, and test build step sequence.
 We want to enforce that at most one test step is executed between all slaves due to restrictions with the data base server.
 This is done by adding the ``locks=`` parameter with the third step.
 It takes a list of locks with their access mode.
+Alternatively, this can take a renderable that returns an list of locks with their access mode.
+
 In this case only the :data:`db_lock` is needed.
 The exclusive access mode is used to ensure there is at most one slave that executes the test step.