Launchpad itself

Merge lp:~michael.nelson/launchpad/487009-db-extract-get-build-records into lp:launchpad/db-devel

  1. 487009-db-extract-get-build-records
  2. Merge into db-devel
Proposed by Michael Nelson
Status: Merged
Merged at revision: not available
Proposed branch: lp:~michael.nelson/launchpad/487009-db-extract-get-build-records
Merge into: lp:launchpad/db-devel
Prerequisite: lp:~michael.nelson/launchpad/487009-db-more-soyuz-extraction
Diff against target: 494 lines (+257/-80)
10 files modified
lib/lp/buildmaster/buildergroup.py (+8/-3)
lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py (+8/-0)
lib/lp/buildmaster/model/buildfarmjobbehavior.py (+8/-1)
lib/lp/buildmaster/tests/buildfarmjobbehavior.txt (+134/-0)
lib/lp/soyuz/browser/tests/builder-views.txt (+1/-0)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+48/-12)
lib/lp/soyuz/interfaces/builder.py (+2/-10)
lib/lp/soyuz/model/binarypackagebuildbehavior.py (+30/-0)
lib/lp/soyuz/model/builder.py (+11/-32)
lib/lp/soyuz/tests/test_builder.py (+7/-22)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/487009-db-extract-get-build-records
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+15649@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

= Summary =

This branch continues on from the prerequisite at:

https://code.edge.launchpad.net/~michael.nelson/launchpad/487009-db-more-soyuz-extraction/+merge/15533

and extracts one further soyuz-specific item from IBuilder and adds some documentation.

== Pre-implementation notes ==

See the pre-implementation notes on bug 487009.

== Implementation details ==

Contrary to the name of the branch, this does *not* extract out getBuildRecords() as originally planned - I have added an XXX there and created a bug with the details. There are three specific things that this branch does:

1. Adds IBuildFarmJobBehavior.slaveStatus() and extracts the soyuz-specific parts of this method to the binary build behavior to keep the without modifying the result (as tested in buildd-slavescanner.txt).

2. To be able to achieve (1), enabling slaveStatus() to be more easily extended, I needed to change the return value from a tuple to a dict. Hence the changes in buildd-slavescanner.txt, but it's very straight-forward to see that the results of the test have not changed.

3. Removes a restriction I had added (in the previous branch) when setting the behavior for a builder. Previously I had raised an exception if an attempt was made to set a behavior when the builder already had a current job. But after chatting with Julian it turns out that there may be some cases where we do call IBuilder.startBuild() even though the builder already has the same build as the current build. This meant updating the unit test that I had added.

4. Adds some documentation for the use of the BuildFarmJobBehaviors - how to add new builder behaviors for new build types.

== Tests ==

bin/test -vv -t buildfarmjobbehavior.txt -t buildd-slavescanner.txt -t doc/builder.txt -t TestCurrentBuildBehavior

== Demo and Q/A ==

We will need to Q/A this on dogfood.

Revision history for this message
Abel Deuring (adeuring) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/buildergroup.py'
2--- lib/lp/buildmaster/buildergroup.py 2009-11-13 19:34:17 +0000
3+++ lib/lp/buildmaster/buildergroup.py 2009-12-08 09:25:29 +0000
4@@ -171,8 +171,8 @@
5 Perform the required actions for each state.
6 """
7 try:
8- (builder_status, build_id, build_status, logtail, filemap,
9- dependencies) = queueItem.builder.slaveStatus()
10+ slave_status = queueItem.builder.slaveStatus()
11+
12 except (xmlrpclib.Fault, socket.error), info:
13 # XXX cprov 2005-06-29:
14 # Hmm, a problem with the xmlrpc interface,
15@@ -192,6 +192,7 @@
16 'BuilderStatus.WAITING': self.updateBuild_WAITING,
17 }
18
19+ builder_status = slave_status['builder_status']
20 if builder_status not in builder_status_handlers:
21 self.logger.critical(
22 "Builder on %s returned unknown status %s, failing it"
23@@ -209,7 +210,11 @@
24 # from the IBuilder content class, it arrives protected by a Zope
25 # Security Proxy, which is not declared, thus empty. Before passing
26 # it to the status handlers we will simply remove the proxy.
27- logtail = removeSecurityProxy(logtail)
28+ logtail = removeSecurityProxy(slave_status.get('logtail'))
29+ build_id = slave_status.get('build_id')
30+ build_status = slave_status.get('build_status')
31+ filemap = slave_status.get('filemap')
32+ dependencies = slave_status.get('dependencies')
33
34 method = builder_status_handlers[builder_status]
35 try:
36
37=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
38--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2009-12-08 09:25:27 +0000
39+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py 2009-12-08 09:25:29 +0000
40@@ -51,3 +51,11 @@
41 :logger: A logger to be used to log diagnostic information.
42 """
43
44+ def slaveStatus(self, raw_slave_status):
45+ """Return a dict of custom slave status values for this behavior.
46+
47+ :param raw_slave_status: The value returned by the build slave's
48+ status() method.
49+ :return: a dict of extra key/values to be included in the result
50+ of IBuilder.slaveStatus().
51+ """
52
53=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
54--- lib/lp/buildmaster/model/buildfarmjobbehavior.py 2009-12-08 09:25:27 +0000
55+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py 2009-12-08 09:25:29 +0000
56@@ -37,6 +37,12 @@
57 """The default behavior is a no-op."""
58 pass
59
60+ def slaveStatus(self, raw_slave_status):
61+ """See `IBuildFarmJobBehavior`.
62+
63+ The default behavior is that we don't add any extra values."""
64+ return {}
65+
66
67 class IdleBuildBehavior(BuildFarmJobBehaviorBase):
68
69@@ -44,7 +50,8 @@
70
71 def __init__(self):
72 """The idle behavior is special in that a buildfarmjob is not
73- specified during initialisation.
74+ specified during initialisation as it is not the result of an
75+ adaption.
76 """
77 super(IdleBuildBehavior, self).__init__(None)
78
79
80=== added file 'lib/lp/buildmaster/tests/buildfarmjobbehavior.txt'
81--- lib/lp/buildmaster/tests/buildfarmjobbehavior.txt 1970-01-01 00:00:00 +0000
82+++ lib/lp/buildmaster/tests/buildfarmjobbehavior.txt 2009-12-08 09:25:29 +0000
83@@ -0,0 +1,134 @@
84+BuildFarmJobBehavior
85+====================
86+
87+The Launchpad build farm was originally designed for building binary
88+packages from source packages, but was subsequently generalised to support
89+other types of build farm jobs.
90+
91+The `BuildFarmJobBehavior` class encapsulates job-type-specific behavior
92+with a standard interface to which our generic IBuilder class delegates.
93+The result is that neither our generic IBuilder class or any call-sites
94+(such as the build master) need any knowledge of different job types or
95+how to handle them.
96+
97+
98+Creating a new behavior
99+-----------------------
100+
101+A new behavior should implement the `IBuildFarmJobBehavior` interface
102+and extend BuildFarmJobBehaviorBase. A new behavior will only be required
103+to define one method - dispatchBuildToSlave() - to correctly implement
104+the interface, but will usually want to customise the other properties and
105+methods as well.
106+
107+ >>> from lp.buildmaster.interfaces.buildfarmjobbehavior import (
108+ ... IBuildFarmJobBehavior)
109+ >>> from lp.buildmaster.model.buildfarmjobbehavior import (
110+ ... BuildFarmJobBehaviorBase)
111+ >>> from zope.interface import implements
112+
113+ >>> class MyNewBuildBehavior(BuildFarmJobBehaviorBase):
114+ ... """A custom build behavior for building blah."""
115+ ... implements(IBuildFarmJobBehavior)
116+ ...
117+ ... def dispatchBuildToSlave(self, build_queue_item, logger):
118+ ... print "Did something special to dispatch MySpecialBuild."
119+ ...
120+ ... @property
121+ ... def status(self):
122+ ... return "Currently building a MyNewBuild object."
123+
124+For this documentation, we'll also need a dummy new build farm job.
125+
126+ >>> from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
127+ >>> class IMyNewBuildFarmJob(IBuildFarmJob):
128+ ... "Normally defines job-type specific database fields."""
129+ >>> class MyNewBuildFarmJob(object):
130+ ... implements(IMyNewBuildFarmJob)
131+
132+Custom behaviors are not normally instantiated directly, instead an adapter is
133+specified for the specific IBuildFarmJob. Normaly we'd add some ZCML to
134+adapt our specific build farm job to its behavior like:
135+
136+ <!-- MyNewBuildBehavior -->
137+ <adapter
138+ for="lp.myapp.interfaces.mynewbuildfarmjob.IMyNewBuildFarmJob"
139+ provides="lp.buildmaster.interfaces.buildfarmjobbehavior.IBuildFarmJobBehavior"
140+ factory="lp.myapp.model.mynewbuildbehavior.MyNewBuildBehavior"
141+ permission="zope.Public" />
142+
143+But for the sake of this documentation we'll add the adapter manually.
144+
145+ >>> from zope.app.testing import ztapi
146+ >>> ztapi.provideAdapter(
147+ ... MyNewBuildFarmJob, IBuildFarmJobBehavior, MyNewBuildBehavior)
148+
149+This will then allow the builder to request and set the required behavior from
150+the current job. Bob the builder currently has a binary package job and so
151+finds itself with a binary package build behavior which defines the status
152+attribute with some binary-build specific information.
153+
154+ >>> from lp.soyuz.model.builder import Builder
155+ >>> from canonical.launchpad.webapp.interfaces import (
156+ ... IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
157+ >>> store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
158+ >>> bob = store.find(Builder, Builder.name == 'bob').one()
159+ >>> print bob.status
160+ Building i386 build of mozilla-firefox 0.9 in ubuntu hoary RELEASE
161+
162+XXX Michael Nelson 2009-12-04 bug 484819. At the time of writing the
163+BuildQueue.specific_job method has not been updated to support different
164+job types, so we will set the behavior explicitly here.
165+
166+ >>> bob.current_build_behavior = IBuildFarmJobBehavior(
167+ ... MyNewBuildFarmJob())
168+
169+Once the builder has the relevant behavior, it is able to provide both general
170+builder functionality of its own accord, while delegating any build-type
171+specific functionality to the behavior. For example, if the builder is not
172+disabled, the builder delegates the status property to the behavior.
173+
174+ >>> print bob.status
175+ Currently building a MyNewBuild object.
176+
177+On the other hand, if the builder is disabled, the builder itself determines
178+the status.
179+
180+ >>> bob.builderok = False
181+ >>> print bob.status
182+ Disabled
183+ >>> bob.builderok = True
184+
185+The IBuildFarmJobBehavior interface currently provides customisation points
186+throughout the build life-cycle, from logging the start of a build, verifying
187+that the provided queue item is ready to be built, dispatching the build etc.,
188+and allows further customisation to be added easily.
189+
190+Please refer to the IBuildFarmJobBehavior interface to see the currently
191+provided build-type specific customisation points.
192+
193+
194+The IdleBuildBehavior
195+---------------------
196+
197+If a Builder does not have a currentjob (and therefore an appropriate
198+build behavior) it will automatically get the special IdleBuildBehavior
199+to ensure that it fulfills its contract to implement IBuildFarmJobBehavior.
200+
201+First, we'll reset the current build behavior and destroy the current job.
202+
203+ >>> bob.current_build_behavior = None
204+ >>> bob.currentjob.destroySelf()
205+
206+ >>> print bob.status
207+ Idle
208+
209+Attempting to use any other build-related functionality when a builder is
210+idle, such as making a call to log the start of a build, will raise an
211+appropriate exception.
212+
213+ >>> bob.logStartBuild(None, None)
214+ Traceback (most recent call last):
215+ ...
216+ BuildBehaviorMismatch: Builder was idle when asked to log the start of a
217+ build.
218
219=== modified file 'lib/lp/soyuz/browser/tests/builder-views.txt'
220--- lib/lp/soyuz/browser/tests/builder-views.txt 2009-11-24 07:02:21 +0000
221+++ lib/lp/soyuz/browser/tests/builder-views.txt 2009-12-08 09:25:29 +0000
222@@ -228,6 +228,7 @@
223 >>> login("foo.bar@canonical.com")
224 >>> private_job = store.get(BuildQueue, private_job_id)
225 >>> Store.of(private_job).remove(private_job)
226+ >>> frog.current_build_behavior = None
227
228 >>> login('no-priv@canonical.com')
229 >>> nopriv_view = getMultiAdapter((frog, empty_request), name="+index")
230
231=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
232--- lib/lp/soyuz/doc/buildd-slavescanner.txt 2009-11-13 19:34:17 +0000
233+++ lib/lp/soyuz/doc/buildd-slavescanner.txt 2009-12-08 09:25:29 +0000
234@@ -1494,8 +1494,8 @@
235
236 == Builder Status Handler ==
237
238-IBuilder.slaveStatus should return a size homogeneous tuple despite of
239-the current slave state. This tuple should contain, in this order:
240+IBuilder.slaveStatus should return a dict containing the following
241+items:
242
243 * slave status string: 'BuilderStatus.IDLE'
244 * job identifier string: '1-1'
245@@ -1504,24 +1504,60 @@
246 * result file list: {'foo.deb', 'foo.changes'} or None
247 * dependencies string: 'bar baz zaz' or None
248
249+ # Define a helper to print the slave status dict.
250+ >>> from collections import defaultdict
251+ >>> def printSlaveStatus(status_dict):
252+ ... status_dict = defaultdict(lambda:None, status_dict)
253+ ... print (
254+ ... "builder_status: %(builder_status)s\n"
255+ ... "build_id: %(build_id)s\n"
256+ ... "build_status: %(build_status)s\n"
257+ ... "logtail: %(logtail)r\n"
258+ ... "filemap: %(filemap)s\n"
259+ ... "dependencies: %(dependencies)s\n" % status_dict)
260
261 >>> a_builder.setSlaveForTesting(OkSlave())
262- >>> a_builder.slaveStatus()
263- ('BuilderStatus.IDLE', '', None, None, None, None)
264+ >>> printSlaveStatus(a_builder.slaveStatus())
265+ builder_status: BuilderStatus.IDLE
266+ build_id:
267+ build_status: None
268+ logtail: None
269+ filemap: None
270+ dependencies: None
271
272 >>> a_builder.setSlaveForTesting(BuildingSlave())
273- >>> a_builder.slaveStatus()
274- ('BuilderStatus.BUILDING', '1-1', None, <xmlrpclib.Binary ...>, None, None)
275+ >>> printSlaveStatus(a_builder.slaveStatus())
276+ builder_status: BuilderStatus.BUILDING
277+ build_id: 1-1
278+ build_status: None
279+ logtail: <xmlrpclib.Binary ...>
280+ filemap: None
281+ dependencies: None
282
283 >>> a_builder.setSlaveForTesting(WaitingSlave(state='BuildStatus.OK'))
284- >>> a_builder.slaveStatus()
285- ('BuilderStatus.WAITING', '1-1', 'BuildStatus.OK', None, {}, None)
286+ >>> printSlaveStatus(a_builder.slaveStatus())
287+ builder_status: BuilderStatus.WAITING
288+ build_id: 1-1
289+ build_status: BuildStatus.OK
290+ logtail: None
291+ filemap: {}
292+ dependencies: None
293
294 >>> a_builder.setSlaveForTesting(AbortingSlave())
295- >>> a_builder.slaveStatus()
296- ('BuilderStatus.ABORTING', '1-1', None, None, None, None)
297+ >>> printSlaveStatus(a_builder.slaveStatus())
298+ builder_status: BuilderStatus.ABORTING
299+ build_id: 1-1
300+ build_status: None
301+ logtail: None
302+ filemap: None
303+ dependencies: None
304
305 >>> a_builder.setSlaveForTesting(AbortedSlave())
306- >>> a_builder.slaveStatus()
307- ('BuilderStatus.ABORTED', '1-1', None, None, None, None)
308+ >>> printSlaveStatus(a_builder.slaveStatus())
309+ builder_status: BuilderStatus.ABORTED
310+ build_id: 1-1
311+ build_status: None
312+ logtail: None
313+ filemap: None
314+ dependencies: None
315
316
317=== modified file 'lib/lp/soyuz/interfaces/builder.py'
318--- lib/lp/soyuz/interfaces/builder.py 2009-12-08 09:25:27 +0000
319+++ lib/lp/soyuz/interfaces/builder.py 2009-12-08 09:25:29 +0000
320@@ -208,16 +208,8 @@
321 def slaveStatus():
322 """Get the slave status for this builder.
323
324- * builder_status => string
325- * build_id => string
326- * build_status => string or None
327- * logtail => string or None
328- * filename => dictionary or None
329- * dependencies => string or None
330-
331- :return: a tuple containing (
332- builder_status, build_id, build_status, logtail, filemap,
333- dependencies)
334+ :return: a dict containing at least builder_status, but potentially
335+ other values included by the current build behavior.
336 """
337
338 def slaveStatusSentence():
339
340=== modified file 'lib/lp/soyuz/model/binarypackagebuildbehavior.py'
341--- lib/lp/soyuz/model/binarypackagebuildbehavior.py 2009-12-08 09:25:27 +0000
342+++ lib/lp/soyuz/model/binarypackagebuildbehavior.py 2009-12-08 09:25:29 +0000
343@@ -166,6 +166,36 @@
344 % (build.title, build.id, build.pocket.name,
345 build.distroseries.name))
346
347+ def slaveStatus(self, raw_slave_status):
348+ """Parse and return the binary build specific status info.
349+
350+ This includes:
351+ * build_id => string
352+ * build_status => string or None
353+ * logtail => string or None
354+ * filename => dictionary or None
355+ * dependencies => string or None
356+ """
357+ builder_status = raw_slave_status[0]
358+ extra_info = {}
359+ if builder_status == 'BuilderStatus.WAITING':
360+ extra_info['build_status'] = raw_slave_status[1]
361+ extra_info['build_id'] = raw_slave_status[2]
362+ build_status_with_files = [
363+ 'BuildStatus.OK',
364+ 'BuildStatus.PACKAGEFAIL',
365+ 'BuildStatus.DEPFAIL',
366+ ]
367+ if extra_info['build_status'] in build_status_with_files:
368+ extra_info['filemap'] = raw_slave_status[3]
369+ extra_info['dependencies'] = raw_slave_status[4]
370+ else:
371+ extra_info['build_id'] = raw_slave_status[1]
372+ if builder_status == 'BuilderStatus.BUILDING':
373+ extra_info['logtail'] = raw_slave_status[2]
374+
375+ return extra_info
376+
377 def _cachePrivateSourceOnSlave(self, build_queue_item, logger):
378 """Ask the slave to download source files for a private build.
379
380
381=== modified file 'lib/lp/soyuz/model/builder.py'
382--- lib/lp/soyuz/model/builder.py 2009-12-08 09:25:27 +0000
383+++ lib/lp/soyuz/model/builder.py 2009-12-08 09:25:29 +0000
384@@ -172,15 +172,8 @@
385
386 def _setCurrentBuildBehavior(self, new_behavior):
387 """Set the current build behavior."""
388-
389- if self.currentjob is not None:
390- # We do not allow the current build behavior to be reset if we
391- # have a current job.
392- raise BuildBehaviorMismatch(
393- "Attempt to reset builder behavior while a current build "
394- "exists.")
395- else:
396- self._current_build_behavior = new_behavior
397+ self._current_build_behavior = new_behavior
398+ if self._current_build_behavior is not None:
399 self._current_build_behavior.setBuilder(self)
400
401 current_build_behavior = property(
402@@ -321,6 +314,10 @@
403 self.builderok = False
404 self.failnotes = reason
405
406+ # XXX Michael Nelson 20091202 bug=491330. The current UI assumes
407+ # that the builder history will display binary build records, as
408+ # returned by getBuildRecords() below. See the bug for a discussion
409+ # of the options.
410 def getBuildRecords(self, build_state=None, name=None, arch_tag=None,
411 user=None):
412 """See IHasBuildRecords."""
413@@ -331,29 +328,11 @@
414 """See IBuilder."""
415 builder_version, builder_arch, mechanisms = self.slave.info()
416 status_sentence = self.slave.status()
417- builder_status = status_sentence[0]
418-
419- if builder_status == 'BuilderStatus.WAITING':
420- (build_status, build_id) = status_sentence[1:3]
421- build_status_with_files = [
422- 'BuildStatus.OK',
423- 'BuildStatus.PACKAGEFAIL',
424- 'BuildStatus.DEPFAIL',
425- ]
426- if build_status in build_status_with_files:
427- (filemap, dependencies) = status_sentence[3:]
428- else:
429- filemap = dependencies = None
430- logtail = None
431- elif builder_status == 'BuilderStatus.BUILDING':
432- (build_id, logtail) = status_sentence[1:]
433- build_status = filemap = dependencies = None
434- else:
435- build_id = status_sentence[1]
436- build_status = logtail = filemap = dependencies = None
437-
438- return (builder_status, build_id, build_status, logtail, filemap,
439- dependencies)
440+
441+ status = {'builder_status': status_sentence[0]}
442+ status.update(
443+ self.current_build_behavior.slaveStatus(status_sentence))
444+ return status
445
446 def slaveStatusSentence(self):
447 """See IBuilder."""
448
449=== modified file 'lib/lp/soyuz/tests/test_builder.py'
450--- lib/lp/soyuz/tests/test_builder.py 2009-12-08 09:25:27 +0000
451+++ lib/lp/soyuz/tests/test_builder.py 2009-12-08 09:25:29 +0000
452@@ -250,13 +250,14 @@
453 self.assertIsInstance(
454 self.builder.current_build_behavior, IdleBuildBehavior)
455
456- def test_set_behavior_when_no_current_job(self):
457- """If a builder is idle then it is possible to set the behavior."""
458- self.builder.current_build_behavior = IBuildFarmJobBehavior(
459- self.buildfarmjob)
460+ def test_set_behavior_sets_builder(self):
461+ """Setting a builder's behavior also associates the behavior with the
462+ builder."""
463+ behavior = IBuildFarmJobBehavior(self.buildfarmjob)
464+ self.builder.current_build_behavior = behavior
465
466- self.assertIsInstance(
467- self.builder.current_build_behavior, BinaryPackageBuildBehavior)
468+ self.assertEqual(behavior, self.builder.current_build_behavior)
469+ self.assertEqual(behavior._builder, self.builder)
470
471 def test_current_job_behavior(self):
472 """The current behavior is set automatically from the current job."""
473@@ -267,21 +268,5 @@
474 self.assertIsInstance(
475 self.builder.current_build_behavior, BinaryPackageBuildBehavior)
476
477- def test_set_behavior_when_current_job(self):
478- """If a builder has a current job then it's behavior cannot be set.
479- """
480- self.build.buildqueue_record.builder = self.builder
481-
482- # As we can't use assertRaises for a property, we use a try-except
483- # instead.
484- assertion_raised = False
485- try:
486- self.builder.current_build_behavior = IBuildFarmJobBehavior(
487- self.buildfarmjob)
488- except BuildBehaviorMismatch, e:
489- assertion_raised = True
490-
491- self.failUnless(assertion_raised)
492-
493 def test_suite():
494 return unittest.TestLoader().loadTestsFromName(__name__)

Subscribers

People subscribed via source and target branches

to status/vote changes: