launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/buildstatus-aborted into lp:launchpad.

Commit message:
Add support for ABORTED build slave status, in preparation for using it on slaves.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #54730 in launchpad-buildd: "launchpad buildd abort does not work as expected"
  https://bugs.launchpad.net/launchpad-buildd/+bug/54730

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/buildstatus-aborted/+merge/176990

As discussed at the releng sprint, this branch adds support for an "ABORTED" build slave status, allowing the combination of BuilderStatus.WAITING/BuildStatus.ABORTED on the slave; this makes more sense for cancelling builds than the only other current possibility of BuilderStatus.ABORTED.

I'm not sure we can QA this until the new launchpad-buildd is ready that uses this, at which point we can QA it all at once.
-- 
https://code.launchpad.net/~cjwatson/launchpad/buildstatus-aborted/+merge/176990
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/buildstatus-aborted into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-01-23 06:41:24 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2013-07-25 16:32:32 +0000
@@ -219,7 +219,8 @@
         Buildslave can be WAITING in five situations:
 
         * Build has failed, no filemap is received (PACKAGEFAIL, DEPFAIL,
-                                                    CHROOTFAIL, BUILDERFAIL)
+                                                    CHROOTFAIL, BUILDERFAIL,
+                                                    ABORTED)
 
         * Build has been built successfully (BuildStatus.OK), in this case
           we have a 'filemap', so we can retrieve those files and store in
@@ -415,6 +416,26 @@
         transaction.commit()
 
     @defer.inlineCallbacks
+    def _handleStatus_ABORTED(self, librarian, slave_status, logger,
+                              send_notification):
+        """Handle aborted builds.
+
+        If the build was explicitly cancelled, then mark it as such.
+        Otherwise, it was probably being rescued, so try it again.
+        """
+        if self.build.status == BuildStatus.CANCELLING:
+            status = BuildStatus.CANCELLED
+        else:
+            status = BuildStatus.NEEDSBUILD
+        self.build.updateStatus(
+            status, builder=self.build.buildqueue_record.builder,
+            slave_status=slave_status)
+        transaction.commit()
+        yield self.build.buildqueue_record.builder.cleanSlave()
+        self.build.buildqueue_record.destroySelf()
+        transaction.commit()
+
+    @defer.inlineCallbacks
     def _handleStatus_GIVENBACK(self, librarian, slave_status, logger,
                                 send_notification):
         """Handle automatic retry requested by builder.

=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	2013-01-30 07:30:10 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py	2013-07-25 16:32:32 +0000
@@ -321,6 +321,28 @@
     def test_handleStatus_PACKAGEFAIL_notifies(self):
         return self._test_handleStatus_notifies("PACKAGEFAIL")
 
+    def test_handleStatus_ABORTED_cancels_cancelling(self):
+        self.build.updateStatus(BuildStatus.CANCELLING)
+
+        def got_status(ignored):
+            self.assertEqual(
+                0, len(pop_notifications()), "Notifications received")
+            self.assertEqual(BuildStatus.CANCELLED, self.build.status)
+
+        d = self.behavior.handleStatus("ABORTED", None, {})
+        return d.addCallback(got_status)
+
+    def test_handleStatus_ABORTED_retries_building(self):
+        self.build.updateStatus(BuildStatus.BUILDING)
+
+        def got_status(ignored):
+            self.assertEqual(
+                0, len(pop_notifications()), "Notifications received")
+            self.assertEqual(BuildStatus.NEEDSBUILD, self.build.status)
+
+        d = self.behavior.handleStatus("ABORTED", None, {})
+        return d.addCallback(got_status)
+
     def test_date_finished_set(self):
         # The date finished is updated during handleStatus_OK.
         self.assertEqual(None, self.build.date_finished)