Merge lp:~michael.nelson/launchpad/499095-build-retry-depwait-stuck into lp:launchpad

[Michael Nelson]

[Michael Nelson (michael.nelson)]

= Summary =
This is a fix for the 2nd cause of bug 499095, where we sometimes get builds on builders with inconsistent states (date_started is not set because the job was already in the RUNNING state).

For details of the cause, see the comments:
https://bugs.edge.launchpad.net/soyuz/+bug/499095/comments/4
https://bugs.edge.launchpad.net/soyuz/+bug/499095/comments/11

== Proposed fix ==

Ensure that the job status is correctly reset back to WAITING after buildStatus_GIVENBACK() and buildStatus_BUILDERFAIL().

== Pre-implementation notes ==

Worked with wgrant to diagnose and fix this second cause.

We really need to update the bm logs so we can see the relevant info. Much of the bm logging is done as warning/debug which doesn't seem to be logged currently.

== Implementation details ==

Nothing weird.

== Tests ==

bin/test -vvt buildd-slavescanner.txt

== Demo and Q/A ==

We can QA this on dogfood (wgrant already QA'd the core of the fix locally).

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/buildmaster/buildergroup.py
  lib/lp/soyuz/doc/buildd-slavescanner.txt

[Brad Crittenden (bac)]

Changes look reasonable Michael. Thanks for the fix.

review: Approve (code)

=== modified file 'lib/lp/buildmaster/buildergroup.py'
--- lib/lp/buildmaster/buildergroup.py	2009-12-03 15:07:17 +0000
+++ lib/lp/buildmaster/buildergroup.py	2010-01-05 16:52:18 +0000
@@ -521,10 +521,8 @@
                           "for its status"))
         # simply reset job
         build = getUtility(IBuildSet).getByQueueEntry(queueItem)
-        build.buildstate = BuildStatus.NEEDSBUILD
         self.storeBuildInfo(queueItem, librarian, buildid, dependencies)
-        queueItem.builder = None
-        queueItem.setDateStarted(None)
+        queueItem.reset()
 
     def buildStatus_GIVENBACK(self, queueItem, librarian, buildid,
                               filemap=None, dependencies=None):
@@ -537,16 +535,12 @@
         self.logger.warning("***** %s is GIVENBACK by %s *****"
                             % (buildid, queueItem.builder.name))
         build = getUtility(IBuildSet).getByQueueEntry(queueItem)
-        build.buildstate = BuildStatus.NEEDSBUILD
         self.storeBuildInfo(queueItem, librarian, buildid, dependencies)
         # XXX cprov 2006-05-30: Currently this information is not
         # properly presented in the Web UI. We will discuss it in
         # the next Paris Summit, infinity has some ideas about how
         # to use this content. For now we just ensure it's stored.
         queueItem.builder.cleanSlave()
-        queueItem.builder = None
-        queueItem.setDateStarted(None)
-        queueItem.logtail = None
-        queueItem.lastscore = 0
+        queueItem.reset()
 
 

=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
--- lib/lp/soyuz/doc/buildd-slavescanner.txt	2009-12-14 20:36:19 +0000
+++ lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-01-05 16:52:18 +0000
@@ -311,11 +311,10 @@
     >>> build.buildstate.title
     'Chroot problem'
 
-WAITING - BUILDERFAIL -> builder has failed by internal error, job is
-available for next build round:
+WAITING - BUILDERFAIL -> builder has failed by internal error, job is available for next build round:
 
     >>> bqItem6 = a_build.createBuildQueueEntry()
-    >>> setupBuildQueue(bqItem6, a_builder)
+    >>> bqItem6.markAsBuilding(a_builder)
     >>> last_stub_mail_count = len(stub.test_emails)
 
 Create a mock slave so the builder can operate - one with a builder error.
@@ -336,8 +335,11 @@
     >>> check_mail_sent(last_stub_mail_count)
     False
     >>> build = getUtility(IBuildSet).getByQueueEntry(bqItem6)
-    >>> build.buildstate.title
-    'Needs building'
+    >>> print build.buildstate.title
+    Needs building
+    >>> job = bqItem6.specific_job.job
+    >>> print job.status.title
+    Waiting
 
 Cleanup in preparation for the next test:
 
@@ -556,7 +558,7 @@
 WAITING -> GIVENBACK - slave requested build record to be rescheduled.
 
     >>> bqItem11 = a_build.createBuildQueueEntry()
-    >>> setupBuildQueue(bqItem11, a_builder)
+    >>> bqItem11.markAsBuilding(a_builder)
     >>> last_stub_mail_count = len(stub.test_emails)
 
 Create a mock slave so the builder gets the right responses for this test.
@@ -580,8 +582,11 @@
     >>> check_mail_sent(last_stub_mail_count)
     False
     >>> build = getUtility(IBuildSet).getByQueueEntry(bqItem11)
-    >>> build.buildstate.title
-    'Needs building'
+    >>> print build.buildstate.title
+    Needs building
+    >>> job = bqItem11.specific_job.job
+    >>> print job.status.title
+    Waiting
 
 Cleanup in preparation for the next test: