launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/fix-resetorfail into lp:launchpad.

Commit message:
Fix BuilderInteractor.resetOrFail to disable non-virtual builders properly again.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1232131 in Launchpad itself: ""'BuilderVitals' object has no attribute 'failBuilder'" when trying to auto-reset builder"
  https://bugs.launchpad.net/launchpad/+bug/1232131

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/fix-resetorfail/+merge/188117

buildd-manager has been failing to disable failing non-virtual builders since Monday, due to what looks like an overzealous change of self.builder.failBuilder to self.vitals.failBuilder.  This branch changes it back, and adds tests for a previously untested code path so this doesn't happen again.

QA: Arrange for a non-virtual builder to fail and make sure that it actually gets disabled properly on the master side.
-- 
https://code.launchpad.net/~cjwatson/launchpad/fix-resetorfail/+merge/188117
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-resetorfail into lp:launchpad.

=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py	2013-09-18 06:38:31 +0000
+++ lib/lp/buildmaster/interactor.py	2013-09-27 17:38:49 +0000
@@ -475,7 +475,7 @@
             logger.warn(
                 "Disabling builder: %s -- %s" % (
                     self.vitals.url, error_message))
-            self.vitals.failBuilder(error_message)
+            self.builder.failBuilder(error_message)
             transaction.commit()
         return defer.succeed(None)
 

=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
--- lib/lp/buildmaster/tests/test_interactor.py	2013-09-26 02:33:28 +0000
+++ lib/lp/buildmaster/tests/test_interactor.py	2013-09-27 17:38:49 +0000
@@ -159,6 +159,13 @@
             DevNullLogger(), Exception())
         return assert_fails_with(d, CannotResumeHost)
 
+    @defer.inlineCallbacks
+    def test_resetOrFail_nonvirtual(self):
+        builder = MockBuilder(virtualized=False, builderok=True)
+        yield BuilderInteractor(builder).resetOrFail(
+            DevNullLogger(), Exception())
+        self.assertFalse(builder.builderok)
+
     def test_slave(self):
         # Builder.slave is a BuilderSlave that points at the actual Builder.
         # The Builder is only ever used in scripts that run outside of the