Merge lp:~wgrant/launchpad/rescue-aborted-and-robbed-builders into lp:launchpad

[William Grant]

[Julian Edwards (julian-edwards)]

Hey William, are you still working on this? It's become rather important as when we kill builds, it leaves the builder stuck after the b-m sees it building something we don't know about.

[William Grant (wgrant)]

Sorry, feel free to take this over. It needs a post-Wellington rewrite. It also needs slave work to make an abort kill sbuild properly.

Unmerged revisions

9795. By William Grant on 2010-03-19

Fix buildd-slavescanner.txt.

9794. By William Grant on 2010-03-19

Merge devel.

9793. By William Grant on 2009-10-30

Clean ABORTED slaves if they do not have a BuildQueue to clean up for them.

9792. By William Grant on 2009-10-29

Correct a comment.

9791. By William Grant on 2009-10-29

Rescue builders associated with non-building buildqueues.

9790. By William Grant on 2009-10-29

Verify that builders are rescued if their reported buildqueue is not assigned to them.

=== modified file 'lib/lp/buildmaster/buildergroup.py'
--- lib/lp/buildmaster/buildergroup.py	2010-03-24 04:51:25 +0000
+++ lib/lp/buildmaster/buildergroup.py	2010-03-27 12:17:46 +0000
@@ -49,3 +49,89 @@
 
         # Commit the updates made to the builders.
         self.commit()
+<<<<<<< TREE
+=======
+
+    def updateBuilderStatus(self, builder, arch):
+        """Update the status for a builder by probing it.
+
+        :param builder: A builder object.
+        :param arch: The expected architecture family of the builder.
+        """
+        self.logger.debug('Checking %s' % builder.name)
+        try:
+            builder.checkSlaveAlive()
+            builder.checkCanBuildForDistroArchSeries(arch)
+            self.rescueBuilderIfLost(builder)
+        # Catch only known exceptions.
+        # XXX cprov 2007-06-15 bug=120571: ValueError & TypeError catching is
+        # disturbing in this context. We should spend sometime sanitizing the
+        # exceptions raised in the Builder API since we already started the
+        # main refactoring of this area.
+        except (ValueError, TypeError, xmlrpclib.Fault,
+                BuildDaemonError), reason:
+            builder.failBuilder(str(reason))
+            self.logger.warn(
+                "%s (%s) marked as failed due to: %s",
+                builder.name, builder.url, builder.failnotes, exc_info=True)
+        except socket.error, reason:
+            error_message = str(reason)
+            builder.handleTimeout(self.logger, error_message)
+
+    def rescueBuilderIfLost(self, builder):
+        """Reset Builder slave if job information doesn't match with DB.
+
+        If builder is BUILDING or WAITING but has an information record
+        that doesn't match what is stored in the DB, we have to dismiss
+        its current actions and let the slave free for another job,
+        assuming the XMLRPC is working properly at this point.
+
+        We also clean a builder if it is ABORTED but does not have an
+        associated BuildQueue to clean up after it.
+        """
+        status_sentence = builder.slaveStatusSentence()
+
+        # 'ident_position' dict relates the position of the job identifier
+        # token in the sentence received from status(), according the
+        # two status we care about. See see lib/canonical/buildd/slave.py
+        # for further information about sentence format.
+        ident_position = {
+            'BuilderStatus.BUILDING': 1,
+            'BuilderStatus.WAITING': 2
+            }
+
+        # Isolate the BuilderStatus string, always the first token in
+        # see lib/canonical/buildd/slave.py and
+        # IBuilder.slaveStatusSentence().
+        status = status_sentence[0]
+
+        if status == 'BuilderStatus.ABORTED' and builder.currentjob is None:
+            builder.cleanSlave()
+            self.logger.warn(
+                "Builder '%s' cleaned up from ABORTED" % builder.name)
+            return
+
+        # If slave is not building nor waiting, it's not in need of rescuing.
+        if status not in ident_position.keys():
+            return
+
+        slave_build_id = status_sentence[ident_position[status]]
+
+        try:
+            builder.verifySlaveBuildID(slave_build_id)
+        except CorruptBuildID, reason:
+            if status == 'BuilderStatus.WAITING':
+                builder.cleanSlave()
+            else:
+                builder.requestAbort()
+            self.logger.warn("Builder '%s' rescued from '%s': '%s'" % (
+                builder.name, slave_build_id, reason))
+
+    def updateBuild(self, queueItem):
+        """Verify the current build job status.
+
+        Perform the required actions for each state.
+        """
+        queueItem.builder.updateBuild(queueItem)
+        self.commit()
+>>>>>>> MERGE-SOURCE

=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-03-12 16:10:12 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2010-03-27 12:17:46 +0000
@@ -154,13 +154,21 @@
             build_id, queue_item_id = slave_build_id.split('-')
         except ValueError:
             raise CorruptBuildID('Malformed build ID')
-            
+
         build = self.getVerifiedBuild(build_id)
         queue_item = self.getVerifiedBuildQueue(queue_item_id)
 
         if build != queue_item.specific_job.build:
             raise CorruptBuildID('Job build entry mismatch')
 
+        # Verify that the builder is properly assigned.
+        if queue_item.builder != self._builder:
+            raise CorruptBuildID('Job builder mismatch')
+
+        # And verify that the job is marked as started.
+        if queue_item.job.date_started is None:
+            raise CorruptBuildID('Job not started')
+
     def updateBuild(self, queueItem):
         """See `IBuildFarmJobBehavior`."""
         logger = logging.getLogger('slave-scanner')

=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
--- lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-03-24 04:51:25 +0000
+++ lib/lp/soyuz/doc/buildd-slavescanner.txt	2010-03-27 12:17:46 +0000
@@ -42,7 +42,8 @@
     ...    AbortedSlave, AbortingSlave, BrokenSlave, BuildingSlave,
     ...    InsaneWaitingSlave, LostBuildingBrokenSlave,
     ...    LostBuildingSlave, LostWaitingSlave, MockBuilder, OkSlave,
-    ...    SaneBuildingSlave, SaneWaitingSlave, WaitingSlave)
+    ...    RobbedBuildingSlave, RobbedWaitingSlave, SaneBuildingSlave,
+    ...    SaneWaitingSlave, WaitingSlave)
 
 Let's play with a BuilderGroup method designed to rescue build slaves
 that are processing unknown jobs. In real conditions, this situation
@@ -67,16 +68,30 @@
 Initializing the sane_builder. It was not rescued, since the job
 identifier is sane (Build.id == 8 and BuildQueue.id == 1 exist):
 
+<<<<<<< TREE
     >>> sanebuilding_builder = MockBuilder(
     ...     'Sane Building Slave', SaneBuildingSlave())
     >>> sanebuilding_builder.rescueIfLost(buildergroup.logger) is None
+=======
+    >>> from lp.buildmaster.interfaces.builder import IBuilderSet
+    >>> from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
+    >>> from lp.soyuz.interfaces.build import IBuildSet
+    >>> bob = getUtility(IBuilderSet)['bob']
+    >>> bob.setSlaveForTesting(SaneBuildingSlave())
+    >>> buildergroup.rescueBuilderIfLost(bob) is None
+>>>>>>> MERGE-SOURCE
     True
 
 A sane WAITING slave:
 
+<<<<<<< TREE
     >>> sanewaiting_builder = MockBuilder(
     ...     'Sane Waiting Slave', SaneWaitingSlave())
     >>> sanewaiting_builder.rescueIfLost(buildergroup.logger) is None
+=======
+    >>> bob.setSlaveForTesting(SaneWaitingSlave())
+    >>> buildergroup.rescueBuilderIfLost(bob) is None
+>>>>>>> MERGE-SOURCE
     True
 
 A insane WAITING slave, with wrong BuildQueue/Build relation:
@@ -108,6 +123,56 @@
 
 Both got rescued, as expected.
 
+A builder can also be lost if it references a valid Build/BuildQueue
+combination, if the BuildQueue isn't started and assigned to that builder.
+This can happen if the BuildQueue is stolen back from the builder, which
+happens most often when the scanner temporarily loses contact with a builder.
+
+    >>> robbedbuilding_builder = MockBuilder(
+    ...     'Robbed Building Slave', RobbedBuildingSlave())
+    >>> buildergroup.rescueBuilderIfLost(robbedbuilding_builder)
+    WARNING:root:Builder 'Robbed Building Slave' rescued from
+    '8-1': 'Job builder mismatch'
+
+    >>> robbedwaiting_builder = MockBuilder(
+    ...     'Robbed Waiting Slave', RobbedWaitingSlave())
+    >>> buildergroup.rescueBuilderIfLost(robbedwaiting_builder)
+    WARNING:root:Builder 'Robbed Waiting Slave' rescued from
+    '8-1': 'Job builder mismatch'
+
+    >>> getUtility(IBuildQueueSet).get(1).setDateStarted(None)
+
+    >>> bob.setSlaveForTesting(RobbedBuildingSlave())
+    >>> buildergroup.rescueBuilderIfLost(bob)
+    WARNING:root:Builder 'bob' rescued from
+    '8-1': 'Job not started' 
+
+    >>> bob.setSlaveForTesting(RobbedWaitingSlave()) 
+    >>> buildergroup.rescueBuilderIfLost(bob)
+    WARNING:root:Builder 'bob' rescued from
+    '8-1': 'Job not started'
+
+If a slave is rescued while it is BUILDING, the current build will be aborted.
+In this case, a further rescue effort is required later, once the abort has
+completed.
+
+This is needed only if the builder has no assigned buildqueue in the
+database, as otherwise the associated buildqueue will handle the ABORTED
+state.
+
+    >>> getUtility(IBuildQueueSet).get(1).builder = bob
+
+    >>> bob.setSlaveForTesting(AbortedSlave()) 
+    >>> buildergroup.rescueBuilderIfLost(bob)
+
+    >>> getUtility(IBuildQueueSet).get(1).builder = None
+
+    >>> bob.setSlaveForTesting(AbortedSlave()) 
+    >>> buildergroup.rescueBuilderIfLost(bob)
+    WARNING:root:Builder 'bob' cleaned up from ABORTED
+
+    >>> getUtility(IBuildQueueSet).get(1).builder = bob
+
 A BUILDING or WAITING slave without a build assigned in the DB will also be rescued.
 
     >>> from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
@@ -130,7 +195,13 @@
     >>> lostbuilding_builder = MockBuilder(
     ...     'Lost Building Broken Slave', LostBuildingBrokenSlave())
 
+<<<<<<< TREE
     >>> lostbuilding_builder.updateStatus(buildergroup.logger)
+=======
+    >>> from lp.registry.interfaces.distribution import IDistributionSet
+    >>> hoary_i386 = getUtility(IDistributionSet)['ubuntu']['hoary']['i386']
+    >>> buildergroup.updateBuilderStatus(lostbuilding_builder, hoary_i386)
+>>>>>>> MERGE-SOURCE
     WARNING:root:Lost Building Broken Slave (http://fake:0000) marked as failed due to: <Fault 8002: 'Could not abort'>
     Traceback (most recent call last):
     ...
@@ -191,8 +262,6 @@
 The slavescanner system also perform build-notification for the
 following states: FAILEDTOBUILD and CHROOTWAIT
 
-    >>> from lp.buildmaster.interfaces.builder import IBuilderSet
-    >>> from lp.soyuz.interfaces.build import IBuildSet
     >>> import datetime, pytz
 
     >>> UTC = pytz.timezone('UTC')
@@ -759,7 +828,6 @@
 
 Retrieve a known DistroArchSeries
 
-    >>> from canonical.launchpad.interfaces import IDistributionSet
     >>> hoary_i386 = getUtility(IDistributionSet)['ubuntu']['hoary']['i386']
     >>> warty_i386 = getUtility(IDistributionSet)['ubuntu']['warty']['i386']
 
@@ -854,8 +922,8 @@
 permissive database user.
 
     >>> from canonical.config import config
-    >>> from canonical.launchpad.interfaces import PackagePublishingStatus
     >>> from canonical.testing.layers import LaunchpadZopelessLayer
+    >>> from lp.soyuz.interfaces.publishing import PackagePublishingStatus
 
     >>> spr = build.sourcepackagerelease
     >>> pub = removeSecurityProxy(build).current_source_publication
@@ -1415,7 +1483,7 @@
 Change the distroseries status for testing. FROZEN allows building in
 all pockets:
 
-    >>> from canonical.launchpad.interfaces import SeriesStatus
+    >>> from lp.registry.interfaces.series import SeriesStatus
     >>> hoary_i386.distroseries.status = SeriesStatus.FROZEN
 
 Now we can start a build in other pockets, and see what archives are

=== modified file 'lib/lp/soyuz/tests/soyuzbuilddhelpers.py'
--- lib/lp/soyuz/tests/soyuzbuilddhelpers.py	2010-03-26 01:52:07 +0000
+++ lib/lp/soyuz/tests/soyuzbuilddhelpers.py	2010-03-27 12:17:46 +0000
@@ -13,6 +13,8 @@
     'LostBuildingSlave',
     'LostWaitingSlave',
     'LostBuildingBrokenSlave',
+    'RobbedBuildingSlave',
+    'RobbedWaitingSlave',
     'BrokenSlave',
     'OkSlave',
     'BuildingSlave',
@@ -154,6 +156,26 @@
         raise xmlrpclib.Fault(8002, "Could not abort")
 
 
+class RobbedBuildingSlave:
+    """A mock slave building a BuildQueue that is not assigned to it."""
+
+    def status(self):
+        return ('BuilderStatus.BUILDING', '8-1')
+
+    def abort(self):
+        pass
+
+
+class RobbedWaitingSlave:
+    """A mock slave building a BuildQueue that is not assigned to it."""
+
+    def status(self):
+        return ('BuilderStatus.WAITING', 'BuildStatus.OK', '8-1')
+
+    def clean(self):
+        pass
+
+
 class BrokenSlave:
     """A mock slave that reports that it is broken."""