Merge lp:~wgrant/launchpad/bug-1221002 into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Steve Kowalik
Approved revision: no longer in the source branch.
Merged at revision: 16760
Proposed branch: lp:~wgrant/launchpad/bug-1221002
Merge into: lp:launchpad
Diff against target: 614 lines (+211/-191)
4 files modified
lib/lp/buildmaster/interactor.py (+35/-54)
lib/lp/buildmaster/manager.py (+46/-64)
lib/lp/buildmaster/tests/test_interactor.py (+47/-62)
lib/lp/buildmaster/tests/test_manager.py (+83/-11)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-1221002
Reviewer Review Type Date Requested Status
Steve Kowalik (community) code Approve
Review via email: mp+184030@code.launchpad.net

Commit message

Rework SlaveScanner.scan() to make a bit more sense.

Description of the change

Rework SlaveScanner.scan() to make a bit more sense. It now ensures that the slave and DB states agree at the start, rescuing the slave and job if the early check fails. This means that the builderok == False case is now handled early and actually prevents all slave communication, as you'd think it would.

If the slave and DB states match, we update or dispatch as appropriate. BuilderInteractor.isAvailable() dies, since we know after the rescue that the slave is IDLE iff currentjob in the DB is None.

SlaveScanner.scan() now takes builder and interactor arguments for overriding things in tests.

To post a comment you must log in.
Revision history for this message
Steve Kowalik (stevenk) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py 2013-09-03 09:35:07 +0000
+++ lib/lp/buildmaster/interactor.py 2013-09-05 22:24:08 +0000
@@ -8,9 +8,7 @@
8 ]8 ]
99
10import logging10import logging
11import socket
12from urlparse import urlparse11from urlparse import urlparse
13import xmlrpclib
1412
15import transaction13import transaction
16from twisted.internet import defer14from twisted.internet import defer
@@ -288,28 +286,17 @@
288 status['logtail'] = status_sentence[2]286 status['logtail'] = status_sentence[2]
289 defer.returnValue((status_sentence, status))287 defer.returnValue((status_sentence, status))
290288
291 @defer.inlineCallbacks289 def verifySlaveBuildCookie(self, slave_cookie):
292 def isAvailable(self):
293 """Whether or not a builder is available for building new jobs.
294
295 :return: A Deferred that fires with True or False, depending on
296 whether the builder is available or not.
297 """
298 if not self.builder.builderok:
299 defer.returnValue(False)
300 try:
301 status = yield self.slave.status()
302 except (xmlrpclib.Fault, socket.error):
303 defer.returnValue(False)
304 defer.returnValue(status[0] == 'BuilderStatus.IDLE')
305
306 def verifySlaveBuildCookie(self, slave_build_cookie):
307 """See `IBuildFarmJobBehavior`."""290 """See `IBuildFarmJobBehavior`."""
308 if self._current_build_behavior is None:291 if self._current_build_behavior is None:
309 raise CorruptBuildCookie('No job assigned to builder')292 if slave_cookie is not None:
310 good_cookie = self._current_build_behavior.getBuildCookie()293 raise CorruptBuildCookie('Slave building when should be idle.')
311 if slave_build_cookie != good_cookie:294 else:
312 raise CorruptBuildCookie("Invalid slave build cookie.")295 good_cookie = self._current_build_behavior.getBuildCookie()
296 if slave_cookie != good_cookie:
297 raise CorruptBuildCookie(
298 "Invalid slave build cookie: got %r, expected %r."
299 % (slave_cookie, good_cookie))
313300
314 @defer.inlineCallbacks301 @defer.inlineCallbacks
315 def rescueIfLost(self, logger=None):302 def rescueIfLost(self, logger=None):
@@ -322,7 +309,8 @@
322 `IBuildFarmJobBehavior.verifySlaveBuildCookie`.309 `IBuildFarmJobBehavior.verifySlaveBuildCookie`.
323310
324 :return: A Deferred that fires when the dialog with the slave is311 :return: A Deferred that fires when the dialog with the slave is
325 finished. It does not have a return value.312 finished. Its return value is True if the slave is lost,
313 False otherwise.
326 """314 """
327 # 'ident_position' dict relates the position of the job identifier315 # 'ident_position' dict relates the position of the job identifier
328 # token in the sentence received from status(), according to the316 # token in the sentence received from status(), according to the
@@ -330,38 +318,40 @@
330 # for further information about sentence format.318 # for further information about sentence format.
331 ident_position = {319 ident_position = {
332 'BuilderStatus.BUILDING': 1,320 'BuilderStatus.BUILDING': 1,
321 'BuilderStatus.ABORTING': 1,
333 'BuilderStatus.WAITING': 2322 'BuilderStatus.WAITING': 2
334 }323 }
335324
336 # If slave is not building nor waiting, it's not in need of325 # Determine the slave's current build cookie. For BUILDING, ABORTING
337 # rescuing.326 # and WAITING we extract the string from the slave status
327 # sentence, and for IDLE it is None.
338 status_sentence = yield self.slave.status()328 status_sentence = yield self.slave.status()
339 status = status_sentence[0]329 status = status_sentence[0]
340 if status not in ident_position.keys():330 if status not in ident_position.keys():
341 return331 slave_cookie = None
342 slave_build_id = status_sentence[ident_position[status]]332 else:
333 slave_cookie = status_sentence[ident_position[status]]
334
335 # verifySlaveBuildCookie will raise CorruptBuildCookie if the
336 # slave cookie doesn't match the expected one, including
337 # verifying that the slave cookie is None iff we expect the
338 # slave to be idle.
343 try:339 try:
344 self.verifySlaveBuildCookie(slave_build_id)340 self.verifySlaveBuildCookie(slave_cookie)
341 defer.returnValue(False)
345 except CorruptBuildCookie as reason:342 except CorruptBuildCookie as reason:
343 # An IDLE slave doesn't need rescuing (SlaveScanner.scan
344 # will rescue the DB side instead), and we just have to wait
345 # out an ABORTING one.
346 if status == 'BuilderStatus.WAITING':346 if status == 'BuilderStatus.WAITING':
347 yield self.cleanSlave()347 yield self.cleanSlave()
348 else:348 elif status == 'BuilderStatus.BUILDING':
349 yield self.requestAbort()349 yield self.requestAbort()
350 if logger:350 if logger:
351 logger.info(351 logger.info(
352 "Builder '%s' rescued from '%s': '%s'" %352 "Builder '%s' rescued from '%s': '%s'" %
353 (self.builder.name, slave_build_id, reason))353 (self.builder.name, slave_cookie, reason))
354354 defer.returnValue(True)
355 def updateStatus(self, logger=None):
356 """Update the builder's status by probing it.
357
358 :return: A Deferred that fires when the dialog with the slave is
359 finished. It does not have a return value.
360 """
361 if logger:
362 logger.debug('Checking %s' % self.builder.name)
363
364 return self.rescueIfLost(logger)
365355
366 def cleanSlave(self):356 def cleanSlave(self):
367 """Clean any temporary files from the slave.357 """Clean any temporary files from the slave.
@@ -524,8 +514,11 @@
524514
525 :return: A Deferred that fires when the slave dialog is finished.515 :return: A Deferred that fires when the slave dialog is finished.
526 """516 """
517 # IDLE is deliberately not handled here, because it should be
518 # impossible to get past rescueIfLost unless the slave matches
519 # the DB, and this method isn't called unless the DB says
520 # there's a job.
527 builder_status_handlers = {521 builder_status_handlers = {
528 'BuilderStatus.IDLE': self.updateBuild_IDLE,
529 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,522 'BuilderStatus.BUILDING': self.updateBuild_BUILDING,
530 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,523 'BuilderStatus.ABORTING': self.updateBuild_ABORTING,
531 'BuilderStatus.WAITING': self.updateBuild_WAITING,524 'BuilderStatus.WAITING': self.updateBuild_WAITING,
@@ -539,18 +532,6 @@
539 method = builder_status_handlers[builder_status]532 method = builder_status_handlers[builder_status]
540 yield method(queueItem, status_sentence, status_dict, logger)533 yield method(queueItem, status_sentence, status_dict, logger)
541534
542 def updateBuild_IDLE(self, queueItem, status_sentence, status_dict,
543 logger):
544 """Somehow the builder forgot about the build job.
545
546 Log this and reset the record.
547 """
548 logger.warn(
549 "Builder %s forgot about buildqueue %d -- resetting buildqueue "
550 "record" % (queueItem.builder.url, queueItem.id))
551 queueItem.reset()
552 transaction.commit()
553
554 def updateBuild_BUILDING(self, queueItem, status_sentence, status_dict,535 def updateBuild_BUILDING(self, queueItem, status_sentence, status_dict,
555 logger):536 logger):
556 """Build still building, collect the logtail"""537 """Build still building, collect the logtail"""
557538
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2013-09-03 04:06:08 +0000
+++ lib/lp/buildmaster/manager.py 2013-09-05 22:24:08 +0000
@@ -250,82 +250,64 @@
250 defer.returnValue(value is not None)250 defer.returnValue(value is not None)
251251
252 @defer.inlineCallbacks252 @defer.inlineCallbacks
253 def scan(self):253 def scan(self, builder=None, interactor=None):
254 """Probe the builder and update/dispatch/collect as appropriate.254 """Probe the builder and update/dispatch/collect as appropriate.
255255
256 There are several steps to scanning:256 :return: A Deferred that fires when the scan is complete.
257
258 1. If the builder is marked as "ok" then probe it to see what state
259 it's in. This is where lost jobs are rescued if we think the
260 builder is doing something that it later tells us it's not,
261 and also where the multi-phase abort procedure happens.
262 See IBuilder.rescueIfLost, which is called by
263 IBuilder.updateStatus().
264 2. If the builder is still happy, we ask it if it has an active build
265 and then either update the build in Launchpad or collect the
266 completed build. (builder.updateBuild)
267 3. If the builder is not happy or it was marked as unavailable
268 mid-build, we need to reset the job that we thought it had, so
269 that the job is dispatched elsewhere.
270 4. If the builder is idle and we have another build ready, dispatch
271 it.
272
273 :return: A Deferred that fires when the scan is complete, whose
274 value is A `BuilderSlave` if we dispatched a job to it, or None.
275 """257 """
276 # We need to re-fetch the builder object on each cycle as the258 self.logger.debug("Scanning %s." % self.builder_name)
277 # Storm store is invalidated over transaction boundaries.259 # Commit and refetch the Builder object to ensure we have the
278260 # latest data from the DB.
279 self.builder = get_builder(self.builder_name)261 transaction.commit()
280 self.interactor = BuilderInteractor(self.builder)262 self.builder = builder or get_builder(self.builder_name)
281263 self.interactor = interactor or BuilderInteractor(self.builder)
282 if self.builder.builderok:264
265 # Confirm that the DB and slave sides are in a valid, mutually
266 # agreeable state.
267 lost_reason = None
268 if not self.builder.builderok:
269 lost_reason = '%s is disabled' % self.builder.name
270 else:
283 cancelled = yield self.checkCancellation(self.builder)271 cancelled = yield self.checkCancellation(self.builder)
284 if cancelled:272 if cancelled:
285 return273 return
286 yield self.interactor.updateStatus(self.logger)274 lost = yield self.interactor.rescueIfLost(self.logger)
287275 if lost:
288 # Commit the changes done while possibly rescuing jobs, to276 lost_reason = '%s is lost' % self.builder.name
289 # avoid holding table locks.277
290 transaction.commit()278 # The slave is lost or the builder is disabled. We can't
291279 # continue to update the job status or dispatch a new job, so
292 buildqueue = self.builder.currentjob280 # just rescue the assigned job, if any, so it can be dispatched
293 if buildqueue is not None:281 # to another slave.
282 if lost_reason is not None:
283 if self.builder.currentjob is not None:
284 self.logger.warn(
285 "%s. Resetting BuildQueue %d.", lost_reason,
286 self.builder.currentjob.id)
287 self.builder.currentjob.reset()
288 transaction.commit()
289 return
290
291 # We've confirmed that the slave state matches the DB. Continue
292 # with updating the job status, or dispatching a new job if the
293 # builder is idle.
294 if self.builder.currentjob is not None:
294 # Scan the slave and get the logtail, or collect the build295 # Scan the slave and get the logtail, or collect the build
295 # if it's ready. Yes, "updateBuild" is a bad name.296 # if it's ready. Yes, "updateBuild" is a bad name.
296 yield self.interactor.updateBuild(buildqueue)297 yield self.interactor.updateBuild(self.builder.currentjob)
297298 elif self.builder.manual:
298 # If the builder is in manual mode, don't dispatch anything.299 # If the builder is in manual mode, don't dispatch anything.
299 if self.builder.manual:
300 self.logger.debug(300 self.logger.debug(
301 '%s is in manual mode, not dispatching.' %301 '%s is in manual mode, not dispatching.' %
302 self.builder.name)302 self.builder.name)
303 return303 else:
304304 # See if there is a job we can dispatch to the builder slave.
305 # If the builder is marked unavailable, don't dispatch anything.305 yield self.interactor.findAndStartJob()
306 # Additionaly, because builders can be removed from the pool at306 if self.builder.currentjob is not None:
307 # any time, we need to see if we think there was a build running307 # After a successful dispatch we can reset the
308 # on it before it was marked unavailable. In this case we reset308 # failure_count.
309 # the build thusly forcing it to get re-dispatched to another309 self.builder.resetFailureCount()
310 # builder.
311 available = yield self.interactor.isAvailable()
312 if not available:
313 job = self.builder.currentjob
314 if job is not None and not self.builder.builderok:
315 self.logger.info(
316 "%s was made unavailable, resetting attached "
317 "job" % self.builder.name)
318 job.reset()
319 transaction.commit()310 transaction.commit()
320 return
321
322 # See if there is a job we can dispatch to the builder slave.
323 yield self.interactor.findAndStartJob()
324 if self.builder.currentjob is not None:
325 # After a successful dispatch we can reset the
326 # failure_count.
327 self.builder.resetFailureCount()
328 transaction.commit()
329311
330312
331class NewBuildersScanner:313class NewBuildersScanner:
332314
=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
--- lib/lp/buildmaster/tests/test_interactor.py 2013-09-02 12:45:50 +0000
+++ lib/lp/buildmaster/tests/test_interactor.py 2013-09-05 22:24:08 +0000
@@ -33,7 +33,6 @@
33 )33 )
34from lp.buildmaster.tests.mock_slaves import (34from lp.buildmaster.tests.mock_slaves import (
35 AbortingSlave,35 AbortingSlave,
36 BrokenSlave,
37 BuildingSlave,36 BuildingSlave,
38 DeadProxy,37 DeadProxy,
39 LostBuildingBrokenSlave,38 LostBuildingBrokenSlave,
@@ -80,28 +79,33 @@
80 self.assertRaises(79 self.assertRaises(
81 AssertionError, interactor.extractBuildStatus, slave_status)80 AssertionError, interactor.extractBuildStatus, slave_status)
8281
83 def test_verifySlaveBuildCookie_good(self):82 def test_verifySlaveBuildCookie_building_match(self):
84 interactor = BuilderInteractor(MockBuilder(), None, TrivialBehavior())83 interactor = BuilderInteractor(MockBuilder(), None, TrivialBehavior())
85 interactor.verifySlaveBuildCookie('trivial')84 interactor.verifySlaveBuildCookie('trivial')
8685
87 def test_verifySlaveBuildCookie_bad(self):86 def test_verifySlaveBuildCookie_building_mismatch(self):
88 interactor = BuilderInteractor(MockBuilder(), None, TrivialBehavior())87 interactor = BuilderInteractor(MockBuilder(), None, TrivialBehavior())
89 self.assertRaises(88 self.assertRaises(
90 CorruptBuildCookie,89 CorruptBuildCookie,
91 interactor.verifySlaveBuildCookie, 'difficult')90 interactor.verifySlaveBuildCookie, 'difficult')
9291
93 def test_verifySlaveBuildCookie_idle(self):92 def test_verifySlaveBuildCookie_idle_match(self):
93 interactor = BuilderInteractor(MockBuilder())
94 self.assertIs(None, interactor._current_build_behavior)
95 interactor.verifySlaveBuildCookie(None)
96
97 def test_verifySlaveBuildCookie_idle_mismatch(self):
94 interactor = BuilderInteractor(MockBuilder())98 interactor = BuilderInteractor(MockBuilder())
95 self.assertIs(None, interactor._current_build_behavior)99 self.assertIs(None, interactor._current_build_behavior)
96 self.assertRaises(100 self.assertRaises(
97 CorruptBuildCookie, interactor.verifySlaveBuildCookie, 'foo')101 CorruptBuildCookie, interactor.verifySlaveBuildCookie, 'foo')
98102
99 def test_updateStatus_aborts_lost_and_broken_slave(self):103 def test_rescueIfLost_aborts_lost_and_broken_slave(self):
100 # A slave that's 'lost' should be aborted; when the slave is104 # A slave that's 'lost' should be aborted; when the slave is
101 # broken then abort() should also throw a fault.105 # broken then abort() should also throw a fault.
102 slave = LostBuildingBrokenSlave()106 slave = LostBuildingBrokenSlave()
103 interactor = BuilderInteractor(MockBuilder(), slave, TrivialBehavior())107 interactor = BuilderInteractor(MockBuilder(), slave, TrivialBehavior())
104 d = interactor.updateStatus(DevNullLogger())108 d = interactor.rescueIfLost(DevNullLogger())
105109
106 def check_slave_status(failure):110 def check_slave_status(failure):
107 self.assertIn('abort', slave.call_log)111 self.assertIn('abort', slave.call_log)
@@ -170,31 +174,37 @@
170 interactor = BuilderInteractor(builder)174 interactor = BuilderInteractor(builder)
171 self.assertEqual(5, interactor.slave.timeout)175 self.assertEqual(5, interactor.slave.timeout)
172176
177 @defer.inlineCallbacks
178 def test_recover_idle_slave(self):
179 # An idle slave is not rescued, even if it's not meant to be
180 # idle. SlaveScanner.scan() will clean up the DB side, because
181 # we still report that it's lost.
182 slave = OkSlave()
183 lost = yield BuilderInteractor(
184 MockBuilder(), slave, TrivialBehavior()).rescueIfLost()
185 self.assertTrue(lost)
186 self.assertEqual([], slave.call_log)
187
188 @defer.inlineCallbacks
173 def test_recover_ok_slave(self):189 def test_recover_ok_slave(self):
174 # An idle slave is not rescued.190 # An idle slave that's meant to be idle is not rescued.
175 slave = OkSlave()191 slave = OkSlave()
176 d = BuilderInteractor(192 lost = yield BuilderInteractor(
177 MockBuilder(), slave, TrivialBehavior()).rescueIfLost()193 MockBuilder(), slave, None).rescueIfLost()
178194 self.assertFalse(lost)
179 def check_slave_calls(ignored):195 self.assertEqual([], slave.call_log)
180 self.assertNotIn('abort', slave.call_log)196
181 self.assertNotIn('clean', slave.call_log)197 @defer.inlineCallbacks
182
183 return d.addCallback(check_slave_calls)
184
185 def test_recover_waiting_slave_with_good_id(self):198 def test_recover_waiting_slave_with_good_id(self):
186 # rescueIfLost does not attempt to abort or clean a builder that is199 # rescueIfLost does not attempt to abort or clean a builder that is
187 # WAITING.200 # WAITING.
188 waiting_slave = WaitingSlave(build_id='trivial')201 waiting_slave = WaitingSlave(build_id='trivial')
189 d = BuilderInteractor(202 lost = yield BuilderInteractor(
190 MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()203 MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()
191204 self.assertFalse(lost)
192 def check_slave_calls(ignored):205 self.assertEqual(['status'], waiting_slave.call_log)
193 self.assertNotIn('abort', waiting_slave.call_log)206
194 self.assertNotIn('clean', waiting_slave.call_log)207 @defer.inlineCallbacks
195
196 return d.addCallback(check_slave_calls)
197
198 def test_recover_waiting_slave_with_bad_id(self):208 def test_recover_waiting_slave_with_bad_id(self):
199 # If a slave is WAITING with a build for us to get, and the build209 # If a slave is WAITING with a build for us to get, and the build
200 # cookie cannot be verified, which means we don't recognize the build,210 # cookie cannot be verified, which means we don't recognize the build,
@@ -202,40 +212,30 @@
202 # builder is reset for a new build, and the corrupt build is212 # builder is reset for a new build, and the corrupt build is
203 # discarded.213 # discarded.
204 waiting_slave = WaitingSlave(build_id='non-trivial')214 waiting_slave = WaitingSlave(build_id='non-trivial')
205 d = BuilderInteractor(215 lost = yield BuilderInteractor(
206 MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()216 MockBuilder(), waiting_slave, TrivialBehavior()).rescueIfLost()
207217 self.assertTrue(lost)
208 def check_slave_calls(ignored):218 self.assertEqual(['status', 'clean'], waiting_slave.call_log)
209 self.assertNotIn('abort', waiting_slave.call_log)219
210 self.assertIn('clean', waiting_slave.call_log)220 @defer.inlineCallbacks
211
212 return d.addCallback(check_slave_calls)
213
214 def test_recover_building_slave_with_good_id(self):221 def test_recover_building_slave_with_good_id(self):
215 # rescueIfLost does not attempt to abort or clean a builder that is222 # rescueIfLost does not attempt to abort or clean a builder that is
216 # BUILDING.223 # BUILDING.
217 building_slave = BuildingSlave(build_id='trivial')224 building_slave = BuildingSlave(build_id='trivial')
218 d = BuilderInteractor(225 lost = yield BuilderInteractor(
219 MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()226 MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()
220227 self.assertFalse(lost)
221 def check_slave_calls(ignored):228 self.assertEqual(['status'], building_slave.call_log)
222 self.assertNotIn('abort', building_slave.call_log)229
223 self.assertNotIn('clean', building_slave.call_log)230 @defer.inlineCallbacks
224
225 return d.addCallback(check_slave_calls)
226
227 def test_recover_building_slave_with_bad_id(self):231 def test_recover_building_slave_with_bad_id(self):
228 # If a slave is BUILDING with a build id we don't recognize, then we232 # If a slave is BUILDING with a build id we don't recognize, then we
229 # abort the build, thus stopping it in its tracks.233 # abort the build, thus stopping it in its tracks.
230 building_slave = BuildingSlave(build_id='non-trivial')234 building_slave = BuildingSlave(build_id='non-trivial')
231 d = BuilderInteractor(235 lost = yield BuilderInteractor(
232 MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()236 MockBuilder(), building_slave, TrivialBehavior()).rescueIfLost()
233237 self.assertTrue(lost)
234 def check_slave_calls(ignored):238 self.assertEqual(['status', 'abort'], building_slave.call_log)
235 self.assertIn('abort', building_slave.call_log)
236 self.assertNotIn('clean', building_slave.call_log)
237
238 return d.addCallback(check_slave_calls)
239239
240240
241class TestBuilderInteractorSlaveStatus(TestCase):241class TestBuilderInteractorSlaveStatus(TestCase):
@@ -285,21 +285,6 @@
285 self.assertStatus(285 self.assertStatus(
286 AbortingSlave(), builder_status='BuilderStatus.ABORTING')286 AbortingSlave(), builder_status='BuilderStatus.ABORTING')
287287
288 def test_isAvailable_with_not_builderok(self):
289 # isAvailable() is a wrapper around BuilderSlave.status()
290 builder = MockBuilder()
291 builder.builderok = False
292 d = BuilderInteractor(builder).isAvailable()
293 return d.addCallback(self.assertFalse)
294
295 def test_isAvailable_with_slave_fault(self):
296 d = BuilderInteractor(MockBuilder(), BrokenSlave()).isAvailable()
297 return d.addCallback(self.assertFalse)
298
299 def test_isAvailable_with_slave_idle(self):
300 d = BuilderInteractor(MockBuilder(), OkSlave()).isAvailable()
301 return d.addCallback(self.assertTrue)
302
303288
304class TestBuilderInteractorDB(TestCaseWithFactory):289class TestBuilderInteractorDB(TestCaseWithFactory):
305 """BuilderInteractor tests that need a DB."""290 """BuilderInteractor tests that need a DB."""
306291
=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py 2013-09-03 03:41:02 +0000
+++ lib/lp/buildmaster/tests/test_manager.py 2013-09-05 22:24:08 +0000
@@ -8,7 +8,6 @@
8import time8import time
9import xmlrpclib9import xmlrpclib
1010
11from lpbuildd.tests import BuilddSlaveTestSetup
12from testtools.deferredruntest import (11from testtools.deferredruntest import (
13 assert_fails_with,12 assert_fails_with,
14 AsynchronousDeferredRunTest,13 AsynchronousDeferredRunTest,
@@ -47,7 +46,9 @@
47 BuildingSlave,46 BuildingSlave,
48 LostBuildingBrokenSlave,47 LostBuildingBrokenSlave,
49 make_publisher,48 make_publisher,
49 MockBuilder,
50 OkSlave,50 OkSlave,
51 TrivialBehavior,
51 )52 )
52from lp.registry.interfaces.distribution import IDistributionSet53from lp.registry.interfaces.distribution import IDistributionSet
53from lp.services.config import config54from lp.services.config import config
@@ -178,28 +179,30 @@
178 build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(job)179 build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(job)
179 self.assertEqual(build.status, BuildStatus.NEEDSBUILD)180 self.assertEqual(build.status, BuildStatus.NEEDSBUILD)
180181
182 @defer.inlineCallbacks
181 def testScanRescuesJobFromBrokenBuilder(self):183 def testScanRescuesJobFromBrokenBuilder(self):
182 # The job assigned to a broken builder is rescued.184 # The job assigned to a broken builder is rescued.
183 self.useFixture(BuilddSlaveTestSetup())
184
185 # Sampledata builder is enabled and is assigned to an active job.185 # Sampledata builder is enabled and is assigned to an active job.
186 builder = getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME]186 builder = getUtility(IBuilderSet)[BOB_THE_BUILDER_NAME]
187 self.patch(
188 BuilderSlave, 'makeBuilderSlave',
189 FakeMethod(BuildingSlave(build_id='PACKAGEBUILD-8')))
187 self.assertTrue(builder.builderok)190 self.assertTrue(builder.builderok)
188 job = builder.currentjob191 job = builder.currentjob
189 self.assertBuildingJob(job, builder)192 self.assertBuildingJob(job, builder)
190193
194 scanner = self._getScanner()
195 yield scanner.scan()
196 self.assertIsNot(None, builder.currentjob)
197
191 # Disable the sampledata builder198 # Disable the sampledata builder
192 login('foo.bar@canonical.com')
193 builder.builderok = False199 builder.builderok = False
194 transaction.commit()200 transaction.commit()
195 login(ANONYMOUS)
196201
197 # Run 'scan' and check its result.202 # Run 'scan' and check its result.
198 switch_dbuser(config.builddmaster.dbuser)203 slave = yield scanner.scan()
199 scanner = self._getScanner()204 self.assertIs(None, builder.currentjob)
200 d = defer.maybeDeferred(scanner.scan)205 self._checkJobRescued(slave, builder, job)
201 d.addCallback(self._checkJobRescued, builder, job)
202 return d
203206
204 def _checkJobUpdated(self, slave, builder, job):207 def _checkJobUpdated(self, slave, builder, job):
205 """`SlaveScanner.scan` updates legitimate jobs.208 """`SlaveScanner.scan` updates legitimate jobs.
@@ -223,7 +226,7 @@
223 login('foo.bar@canonical.com')226 login('foo.bar@canonical.com')
224 builder.builderok = True227 builder.builderok = True
225 self.patch(BuilderSlave, 'makeBuilderSlave',228 self.patch(BuilderSlave, 'makeBuilderSlave',
226 FakeMethod(BuildingSlave(build_id='8-1')))229 FakeMethod(BuildingSlave(build_id='PACKAGEBUILD-8')))
227 transaction.commit()230 transaction.commit()
228 login(ANONYMOUS)231 login(ANONYMOUS)
229232
@@ -437,6 +440,75 @@
437 self.assertEqual(BuildStatus.CANCELLED, build.status)440 self.assertEqual(BuildStatus.CANCELLED, build.status)
438441
439442
443class FakeBuildQueue:
444
445 def __init__(self):
446 self.id = 1
447 self.reset = FakeMethod()
448
449
450class TestSlaveScannerWithoutDB(TestCase):
451
452 run_tests_with = AsynchronousDeferredRunTest
453
454 @defer.inlineCallbacks
455 def test_scan_with_job(self):
456 # SlaveScanner.scan calls updateBuild() when a job is building.
457 interactor = BuilderInteractor(
458 MockBuilder(), BuildingSlave('trivial'), TrivialBehavior())
459 scanner = SlaveScanner('mock', BufferLogger())
460
461 # Instrument updateBuild and currentjob.reset
462 interactor.updateBuild = FakeMethod()
463 interactor.builder.currentjob = FakeBuildQueue()
464 # XXX: checkCancellation needs more than a FakeBuildQueue.
465 scanner.checkCancellation = FakeMethod(defer.succeed(False))
466
467 yield scanner.scan(builder=interactor.builder, interactor=interactor)
468 self.assertEqual(['status'], interactor.slave.call_log)
469 self.assertEqual(1, interactor.updateBuild.call_count)
470 self.assertEqual(0, interactor.builder.currentjob.reset.call_count)
471
472 @defer.inlineCallbacks
473 def test_scan_aborts_lost_slave_with_job(self):
474 # SlaveScanner.scan uses BuilderInteractor.rescueIfLost to abort
475 # slaves that don't have the expected job.
476 interactor = BuilderInteractor(
477 MockBuilder(), BuildingSlave('nontrivial'), TrivialBehavior())
478 scanner = SlaveScanner('mock', BufferLogger())
479
480 # Instrument updateBuild and currentjob.reset
481 interactor.updateBuild = FakeMethod()
482 interactor.builder.currentjob = FakeBuildQueue()
483 # XXX: checkCancellation needs more than a FakeBuildQueue.
484 scanner.checkCancellation = FakeMethod(defer.succeed(False))
485
486 # A single scan will call status(), notice that the slave is
487 # lost, abort() the slave, then reset() the job without calling
488 # updateBuild().
489 yield scanner.scan(builder=interactor.builder, interactor=interactor)
490 self.assertEqual(['status', 'abort'], interactor.slave.call_log)
491 self.assertEqual(0, interactor.updateBuild.call_count)
492 self.assertEqual(1, interactor.builder.currentjob.reset.call_count)
493
494 @defer.inlineCallbacks
495 def test_scan_aborts_lost_slave_when_idle(self):
496 # SlaveScanner.scan uses BuilderInteractor.rescueIfLost to abort
497 # slaves that aren't meant to have a job.
498 interactor = BuilderInteractor(MockBuilder(), BuildingSlave(), None)
499 scanner = SlaveScanner('mock', BufferLogger())
500
501 # Instrument updateBuild.
502 interactor.updateBuild = FakeMethod()
503
504 # A single scan will call status(), notice that the slave is
505 # lost, abort() the slave, then reset() the job without calling
506 # updateBuild().
507 yield scanner.scan(builder=interactor.builder, interactor=interactor)
508 self.assertEqual(['status', 'abort'], interactor.slave.call_log)
509 self.assertEqual(0, interactor.updateBuild.call_count)
510
511
440class TestCancellationChecking(TestCaseWithFactory):512class TestCancellationChecking(TestCaseWithFactory):
441 """Unit tests for the checkCancellation method."""513 """Unit tests for the checkCancellation method."""
442514