Merge lp:~michael.nelson/launchpad/499421-dont-grind-bm-to-a-halt into lp:launchpad

Proposed by Michael Nelson
Status: Merged
Approved by: Michael Nelson
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~michael.nelson/launchpad/499421-dont-grind-bm-to-a-halt
Merge into: lp:launchpad
Diff against target: 49 lines (+20/-1)
2 files modified
lib/lp/soyuz/model/buildqueue.py (+3/-0)
lib/lp/soyuz/tests/test_buildqueue.py (+17/-1)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/499421-dont-grind-bm-to-a-halt
Reviewer Review Type Date Requested Status
Björn Tillenius (community) release-critical Approve
Eleanor Berger (community) code Approve
Review via email: mp+19846@code.launchpad.net

Commit message

IBuildQueueSet.getActiveBuildJobs() returns only active build jobs that have an associated Builder (so that bug 499421 won't cause the buildd-manager won't try to update it.)

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

This branch ensures that IBuildQueueSet.getActiveBuildJobs() returns only active build jobs that have an associated Builder.

It's related to bug 499421, and just ensures that when we do get into this situation (an active build without a builder), the buildd-manager doesn't try to updated it (and hence fall over).

Tests
=====
bin/test -vvt test_getActiveBuildJobs_no_builder_bug499421

We will need to QA this on dogfood.

Revision history for this message
Eleanor Berger (intellectronica) :
review: Approve (code)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

I have QA'd this on dogfood by:

0. Patching the codebase to explicitly revert fix on dogfood
1. Adding some new sources to the queue for building,
2. Running the buildd-manager to see the new build dispatched,
3. Pausing the buildd-manager, then in a harness getting the current build and manually setting the builder attribute to None.
4. Restarting buildd-manager and verifying the exact error as seen in production (http://pastebin.ubuntu.com/383523/)
5. Stopping the buildd-manager again,
7. Reverting the codebase enabling the fix
8. Restarting the buildd-manager and confirming that it carries on scanning as usual without error.

Revision history for this message
Björn Tillenius (bjornt) wrote :

This looks good from a CP point of view. I mentioned on IRC that re-using the same bug for tracking a CP and the long-term issue would be confusing, so Michael will file a new bug for the XXX comment, and update devel with the new bug number.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/model/buildqueue.py'
2--- lib/lp/soyuz/model/buildqueue.py 2010-02-02 14:01:14 +0000
3+++ lib/lp/soyuz/model/buildqueue.py 2010-02-23 06:10:32 +0000
4@@ -506,6 +506,9 @@
5 result_set = store.find(
6 BuildQueue,
7 BuildQueue.job == Job.id,
8+ # XXX Michael Nelson 2010-02-22 bug=499421
9+ # Avoid corrupt build jobs where the builder is None.
10+ BuildQueue.builder != None,
11 # status is a property. Let's use _status.
12 Job._status == JobStatus.RUNNING,
13 Job.date_started != None)
14
15=== modified file 'lib/lp/soyuz/tests/test_buildqueue.py'
16--- lib/lp/soyuz/tests/test_buildqueue.py 2010-02-22 16:57:43 +0000
17+++ lib/lp/soyuz/tests/test_buildqueue.py 2010-02-23 06:10:32 +0000
18@@ -177,6 +177,19 @@
19 buildqueue = BuildQueue(job=job.id)
20 self.assertEquals(buildqueue, self.buildqueueset.getByJob(job))
21
22+ def test_getActiveBuildJobs_no_builder_bug499421(self):
23+ # An active build queue item that does not have a builder will
24+ # not be included in the results and so will not block the
25+ # buildd-manager.
26+ active_jobs = self.buildqueueset.getActiveBuildJobs()
27+ self.assertEqual(1, active_jobs.count())
28+ active_job = active_jobs[0]
29+ active_job.builder = None
30+ self.assertTrue(
31+ self.buildqueueset.getActiveBuildJobs().is_empty(),
32+ "An active build job must have a builder.")
33+
34+
35
36 class TestBuildQueueBase(TestCaseWithFactory):
37 """Setup the test publisher and some builders."""
38@@ -248,7 +261,10 @@
39
40 # hppa native
41 self.builders[(self.hppa_proc.id, False)] = [
42- self.h5, self.h6, self.h7]
43+ self.h5,
44+ self.h6,
45+ self.h7,
46+ ]
47 # hppa virtual
48 self.builders[(self.hppa_proc.id, True)] = [
49 self.h1, self.h2, self.h3, self.h4]