launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15978
[Merge] lp:~wgrant/launchpad/bi-dependencies into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/bi-dependencies into lp:launchpad with lp:~wgrant/launchpad/bi-dependencies-1 as a prerequisite.
Commit message:
Port the remaining BuilderInteractor methods to take all variables as arguments.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bi-dependencies/+merge/188493
Port the remaining BuilderInteractor methods to take all variables as arguments, rather than using instance attributes. BI._current_build_behavior and BI.slave die; SlaveScanner now manages those objects, and tests pass them in directly rather than patching BuilderInteractor.
--
https://code.launchpad.net/~wgrant/launchpad/bi-dependencies/+merge/188493
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/bi-dependencies into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interactor.py'
--- lib/lp/buildmaster/interactor.py 2013-10-01 01:11:42 +0000
+++ lib/lp/buildmaster/interactor.py 2013-10-01 01:11:42 +0000
@@ -227,24 +227,6 @@
class BuilderInteractor(object):
- _cached_build_behavior = None
- _cached_currentjob = None
-
- _cached_slave = None
- _cached_slave_attrs = None
-
- # Tests can override _current_build_behavior and slave.
- _override_behavior = None
- _override_slave = None
-
- def __init__(self, builder, override_slave=None, override_behavior=None):
- self.builder = builder
- self._override_slave = override_slave
- self._override_behavior = override_behavior
-
- # XXX wgrant: The BuilderVitals should be passed in.
- self.vitals = extract_vitals_from_db(builder)
-
@staticmethod
def makeSlaveFromVitals(vitals):
if vitals.virtualized:
@@ -254,43 +236,14 @@
return BuilderSlave.makeBuilderSlave(
vitals.url, vitals.vm_host, timeout)
- @property
- def slave(self):
- """See IBuilder."""
- if self._override_slave is not None:
- return self._override_slave
- # The slave cache is invalidated when the builder's URL, VM host
- # or virtualisation change.
- new_slave_attrs = (
- self.vitals.url, self.vitals.vm_host, self.vitals.virtualized)
- if self._cached_slave_attrs != new_slave_attrs:
- self._cached_slave = self.makeSlaveFromVitals(self.vitals)
- self._cached_slave_attrs = new_slave_attrs
- return self._cached_slave
-
@staticmethod
def getBuildBehavior(queue_item, builder, slave):
+ if queue_item is None:
+ return None
behavior = IBuildFarmJobBehavior(queue_item.specific_job)
behavior.setBuilder(builder, slave)
return behavior
- @property
- def _current_build_behavior(self):
- """Return the current build behavior."""
- if self._override_behavior is not None:
- return self._override_behavior
- # The _current_build_behavior cache is invalidated when
- # builder.currentjob changes.
- currentjob = self.builder.currentjob
- if currentjob is None:
- self._cached_build_behavior = None
- self._cached_currentjob = None
- elif currentjob != self._cached_currentjob:
- self._cached_build_behavior = self.getBuildBehavior(
- currentjob, self.builder, self.slave)
- self._cached_currentjob = currentjob
- return self._cached_build_behavior
-
@staticmethod
@defer.inlineCallbacks
def slaveStatus(slave):
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2013-10-01 01:11:42 +0000
+++ lib/lp/buildmaster/manager.py 2013-10-01 01:11:42 +0000
@@ -57,7 +57,7 @@
@defer.inlineCallbacks
-def assessFailureCounts(logger, interactor, exception):
+def assessFailureCounts(logger, vitals, builder, slave, interactor, exception):
"""View builder/job failure_count and work out which needs to die.
:return: A Deferred that fires either immediately or after a virtual
@@ -66,7 +66,6 @@
# builder.currentjob hides a complicated query, don't run it twice.
# See bug 623281 (Note that currentjob is a cachedproperty).
- builder = interactor.builder
del get_property_cache(builder).currentjob
current_job = builder.currentjob
if current_job is None:
@@ -105,8 +104,7 @@
# The builder is dead, but in the virtual case it might be worth
# resetting it.
yield interactor.resetOrFail(
- interactor.vitals, interactor.slave, interactor.builder,
- logger, exception)
+ vitals, slave, builder, logger, exception)
else:
# The job is the culprit! Override its status to 'failed'
# to make sure it won't get automatically dispatched again,
@@ -144,10 +142,16 @@
# greater than abort_timeout in launchpad-buildd's slave BuildManager.
CANCEL_TIMEOUT = 180
- def __init__(self, builder_name, builders_cache, logger, clock=None):
+ def __init__(self, builder_name, builders_cache, logger, clock=None,
+ interactor_factory=BuilderInteractor,
+ slave_factory=BuilderInteractor.makeSlaveFromVitals,
+ behavior_factory=BuilderInteractor.getBuildBehavior):
self.builder_name = builder_name
self.builders_cache = builders_cache
self.logger = logger
+ self.interactor_factory = interactor_factory
+ self.slave_factory = slave_factory
+ self.behavior_factory = behavior_factory
# Use the clock if provided, so that tests can advance it. Use the
# reactor by default.
if clock is None:
@@ -202,11 +206,13 @@
failure.getTraceback()))
# Decide if we need to terminate the job or reset/fail the builder.
+ vitals = self.builders_cache.getVitals(self.builder_name)
builder = self.builders_cache[self.builder_name]
try:
builder.handleFailure(self.logger)
yield assessFailureCounts(
- self.logger, BuilderInteractor(builder), failure.value)
+ self.logger, vitals, builder, self.slave_factory(vitals),
+ self.interactor_factory(), failure.value)
transaction.commit()
except Exception:
# Catastrophic code failure! Not much we can do.
@@ -216,7 +222,7 @@
transaction.abort()
@defer.inlineCallbacks
- def checkCancellation(self, builder, interactor):
+ def checkCancellation(self, vitals, builder, slave, interactor):
"""See if there is a pending cancellation request.
If the current build is in status CANCELLING then terminate it
@@ -226,11 +232,10 @@
by resuming a slave host, so that there is no need to update its
status.
"""
- buildqueue = builder.currentjob
- if not buildqueue:
+ if not vitals.build_queue:
self.date_cancel = None
defer.returnValue(False)
- build = buildqueue.specific_job.build
+ build = vitals.build_queue.specific_job.build
if build.status != BuildStatus.CANCELLING:
self.date_cancel = None
defer.returnValue(False)
@@ -238,7 +243,7 @@
try:
if self.date_cancel is None:
self.logger.info("Cancelling build '%s'" % build.title)
- yield interactor.slave.abort()
+ yield slave.abort()
self.date_cancel = self._clock.seconds() + self.CANCEL_TIMEOUT
defer.returnValue(False)
else:
@@ -255,18 +260,17 @@
except Exception as e:
self.logger.info(
"Build '%s' on %s failed to cancel" %
- (build.title, builder.name))
+ (build.title, vitals.name))
self.date_cancel = None
- buildqueue.cancel()
+ vitals.build_queue.cancel()
transaction.commit()
value = yield interactor.resetOrFail(
- interactor.vitals, interactor.slave, interactor.builder,
- self.logger, e)
+ vitals, slave, builder, self.logger, e)
# value is not None if we resumed a slave host.
defer.returnValue(value is not None)
@defer.inlineCallbacks
- def scan(self, interactor=None):
+ def scan(self):
"""Probe the builder and update/dispatch/collect as appropriate.
:return: A Deferred that fires when the scan is complete.
@@ -276,8 +280,8 @@
# latest data from the DB.
transaction.commit()
vitals = self.builders_cache.getVitals(self.builder_name)
- interactor = interactor or BuilderInteractor(
- self.builders_cache[self.builder_name])
+ interactor = self.interactor_factory()
+ slave = self.slave_factory(vitals)
# Confirm that the DB and slave sides are in a valid, mutually
# agreeable state.
@@ -285,13 +289,15 @@
if not vitals.builderok:
lost_reason = '%s is disabled' % vitals.name
else:
+ builder = self.builders_cache[self.builder_name]
cancelled = yield self.checkCancellation(
- interactor.builder, interactor)
+ vitals, builder, slave, interactor)
if cancelled:
return
+ behavior = self.behavior_factory(
+ vitals.build_queue, builder, slave)
lost = yield interactor.rescueIfLost(
- vitals, interactor.slave, interactor._current_build_behavior,
- self.logger)
+ vitals, slave, behavior, self.logger)
if lost:
lost_reason = '%s is lost' % vitals.name
@@ -314,9 +320,9 @@
if vitals.build_queue is not None:
# Scan the slave and get the logtail, or collect the build
# if it's ready. Yes, "updateBuild" is a bad name.
- yield interactor.updateBuild(
- vitals.build_queue, interactor.slave,
- interactor._current_build_behavior)
+ behavior = behavior or self.behavior_factory(
+ vitals.build_queue, builder, slave)
+ yield interactor.updateBuild(vitals.build_queue, slave, behavior)
elif vitals.manual:
# If the builder is in manual mode, don't dispatch anything.
self.logger.debug(
@@ -324,7 +330,7 @@
else:
# See if there is a job we can dispatch to the builder slave.
builder = self.builders_cache[self.builder_name]
- yield interactor.findAndStartJob(vitals, builder, interactor.slave)
+ yield interactor.findAndStartJob(vitals, builder, slave)
if builder.currentjob is not None:
# After a successful dispatch we can reset the
# failure_count.
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py'
--- lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-09-23 02:04:18 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjobbehavior.py 2013-10-01 01:11:42 +0000
@@ -135,9 +135,9 @@
self.build.buildqueue_record.markAsBuilding(self.builder)
self.slave = WaitingSlave('BuildStatus.OK')
self.slave.valid_file_hashes.append('test_file_hash')
- self.interactor = BuilderInteractor(self.builder, self.slave)
- self.behavior = removeSecurityProxy(
- self.interactor._current_build_behavior)
+ self.interactor = BuilderInteractor()
+ self.behavior = self.interactor.getBuildBehavior(
+ self.build.buildqueue_record, self.builder, self.slave)
# We overwrite the buildmaster root to use a temp directory.
tempdir = tempfile.mkdtemp()
@@ -253,9 +253,8 @@
def test_handleStatus_ABORTED_recovers_building(self):
self.builder.vm_host = "fake_vm_host"
- self.interactor = BuilderInteractor(self.builder, self.slave)
- self.behavior = removeSecurityProxy(
- self.interactor._current_build_behavior)
+ self.behavior = self.interactor.getBuildBehavior(
+ self.build.buildqueue_record, self.builder, self.slave)
self.build.updateStatus(BuildStatus.BUILDING)
def got_status(ignored):
=== modified file 'lib/lp/buildmaster/tests/test_interactor.py'
--- lib/lp/buildmaster/tests/test_interactor.py 2013-10-01 01:11:42 +0000
+++ lib/lp/buildmaster/tests/test_interactor.py 2013-10-01 01:11:42 +0000
@@ -68,36 +68,32 @@
# extractBuildStatus picks the name of the build status out of a
# dict describing the slave's status.
slave_status = {'build_status': 'BuildStatus.BUILDING'}
- interactor = BuilderInteractor(MockBuilder())
self.assertEqual(
- 'BUILDING', interactor.extractBuildStatus(slave_status))
+ 'BUILDING', BuilderInteractor.extractBuildStatus(slave_status))
def test_extractBuildStatus_malformed(self):
# extractBuildStatus errors out when the status string is not
# of the form it expects.
slave_status = {'build_status': 'BUILDING'}
- interactor = BuilderInteractor(MockBuilder())
self.assertRaises(
- AssertionError, interactor.extractBuildStatus, slave_status)
+ AssertionError, BuilderInteractor.extractBuildStatus, slave_status)
def test_verifySlaveBuildCookie_building_match(self):
- interactor = BuilderInteractor(MockBuilder())
- interactor.verifySlaveBuildCookie(TrivialBehavior(), 'trivial')
+ BuilderInteractor.verifySlaveBuildCookie(TrivialBehavior(), 'trivial')
def test_verifySlaveBuildCookie_building_mismatch(self):
- interactor = BuilderInteractor(MockBuilder())
self.assertRaises(
CorruptBuildCookie,
- interactor.verifySlaveBuildCookie, TrivialBehavior(), 'difficult')
+ BuilderInteractor.verifySlaveBuildCookie,
+ TrivialBehavior(), 'difficult')
def test_verifySlaveBuildCookie_idle_match(self):
- interactor = BuilderInteractor(MockBuilder())
- interactor.verifySlaveBuildCookie(None, None)
+ BuilderInteractor.verifySlaveBuildCookie(None, None)
def test_verifySlaveBuildCookie_idle_mismatch(self):
- interactor = BuilderInteractor(MockBuilder())
self.assertRaises(
- CorruptBuildCookie, interactor.verifySlaveBuildCookie, None, 'foo')
+ CorruptBuildCookie,
+ BuilderInteractor.verifySlaveBuildCookie, None, 'foo')
def test_rescueIfLost_aborts_lost_and_broken_slave(self):
# A slave that's 'lost' should be aborted; when the slave is
@@ -168,22 +164,24 @@
def test_resetOrFail_nonvirtual(self):
builder = MockBuilder(virtualized=False, builderok=True)
vitals = extract_vitals_from_db(builder)
- yield BuilderInteractor(builder).resetOrFail(
+ yield BuilderInteractor().resetOrFail(
vitals, None, builder, DevNullLogger(), Exception())
self.assertFalse(builder.builderok)
- def test_slave(self):
+ def test_makeSlaveFromVitals(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
# security context.
builder = MockBuilder(virtualized=False)
- interactor = BuilderInteractor(builder)
- self.assertEqual(builder.url, interactor.slave.url)
- self.assertEqual(10, interactor.slave.timeout)
+ vitals = extract_vitals_from_db(builder)
+ slave = BuilderInteractor.makeSlaveFromVitals(vitals)
+ self.assertEqual(builder.url, slave.url)
+ self.assertEqual(10, slave.timeout)
builder = MockBuilder(virtualized=True)
- interactor = BuilderInteractor(builder)
- self.assertEqual(5, interactor.slave.timeout)
+ vitals = extract_vitals_from_db(builder)
+ slave = BuilderInteractor.makeSlaveFromVitals(vitals)
+ self.assertEqual(5, slave.timeout)
@defer.inlineCallbacks
def test_recover_idle_slave(self):
@@ -307,23 +305,25 @@
layer = ZopelessDatabaseLayer
run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=10)
- def test_current_build_behavior_idle(self):
+ def test_getBuildBehavior_idle(self):
"""An idle builder has no build behavior."""
self.assertIs(
- None, BuilderInteractor(MockBuilder())._current_build_behavior)
+ None,
+ BuilderInteractor.getBuildBehavior(None, MockBuilder(), None))
- def test_current_build_behavior_building(self):
+ def test_getBuildBehavior_building(self):
"""The current behavior is set automatically from the current job."""
# Set the builder attribute on the buildqueue record so that our
# builder will think it has a current build.
builder = self.factory.makeBuilder(name='builder')
- interactor = BuilderInteractor(builder)
+ slave = BuildingSlave()
build = self.factory.makeBinaryPackageBuild()
- build.queueBuild().markAsBuilding(builder)
- behavior = interactor._current_build_behavior
+ bq = build.queueBuild()
+ bq.markAsBuilding(builder)
+ behavior = BuilderInteractor.getBuildBehavior(bq, builder, slave)
self.assertIsInstance(behavior, BinaryPackageBuildBehavior)
self.assertEqual(behavior._builder, builder)
- self.assertEqual(behavior._slave, interactor.slave)
+ self.assertEqual(behavior._slave, slave)
def _setupBuilder(self):
processor = self.factory.makeProcessor(name="i386")
@@ -401,30 +401,6 @@
return d.addCallback(check_build_started)
- @defer.inlineCallbacks
- def test_recover_building_slave_with_job_that_finished_elsewhere(self):
- # See bug 671242
- # When a job is destroyed, the builder's behaviour should be reset
- # too so that we don't traceback when the wrong behaviour tries
- # to access a non-existent job.
- builder, build = self._setupBinaryBuildAndBuilder()
- candidate = build.queueBuild()
- building_slave = BuildingSlave()
- interactor = BuilderInteractor(builder, building_slave)
- candidate.markAsBuilding(builder)
-
- # At this point we should see a valid behaviour on the builder:
- self.assertIsNot(None, interactor._current_build_behavior)
- yield BuilderInteractor.rescueIfLost(
- interactor.vitals, building_slave,
- interactor._current_build_behavior)
- self.assertIsNot(None, interactor._current_build_behavior)
- candidate.destroySelf()
- yield BuilderInteractor.rescueIfLost(
- interactor.vitals, building_slave,
- interactor._current_build_behavior)
- self.assertIs(None, interactor._current_build_behavior)
-
class TestSlave(TestCase):
"""
=== modified file 'lib/lp/buildmaster/tests/test_manager.py'
--- lib/lp/buildmaster/tests/test_manager.py 2013-09-17 04:31:16 +0000
+++ lib/lp/buildmaster/tests/test_manager.py 2013-10-01 01:11:42 +0000
@@ -470,19 +470,23 @@
@defer.inlineCallbacks
def test_scan_with_job(self):
# SlaveScanner.scan calls updateBuild() when a job is building.
- interactor = BuilderInteractor(
- MockBuilder(), BuildingSlave('trivial'), TrivialBehavior())
+ slave = BuildingSlave('trivial')
bq = FakeBuildQueue()
+
+ # Instrument updateBuild.
+ interactor = BuilderInteractor()
+ interactor.updateBuild = FakeMethod()
+
scanner = SlaveScanner(
- 'mock', MockBuildersCache(interactor.builder, bq), BufferLogger())
-
- # Instrument updateBuild and currentjob.reset
- interactor.updateBuild = FakeMethod()
+ 'mock', MockBuildersCache(MockBuilder(), bq), BufferLogger(),
+ interactor_factory=FakeMethod(interactor),
+ slave_factory=FakeMethod(slave),
+ behavior_factory=FakeMethod(TrivialBehavior()))
# XXX: checkCancellation needs more than a FakeBuildQueue.
scanner.checkCancellation = FakeMethod(defer.succeed(False))
- yield scanner.scan(interactor=interactor)
- self.assertEqual(['status'], interactor.slave.call_log)
+ yield scanner.scan()
+ self.assertEqual(['status'], slave.call_log)
self.assertEqual(1, interactor.updateBuild.call_count)
self.assertEqual(0, bq.reset.call_count)
@@ -490,22 +494,26 @@
def test_scan_aborts_lost_slave_with_job(self):
# SlaveScanner.scan uses BuilderInteractor.rescueIfLost to abort
# slaves that don't have the expected job.
- interactor = BuilderInteractor(
- MockBuilder(), BuildingSlave('nontrivial'), TrivialBehavior())
+ slave = BuildingSlave('nontrivial')
bq = FakeBuildQueue()
+
+ # Instrument updateBuild.
+ interactor = BuilderInteractor()
+ interactor.updateBuild = FakeMethod()
+
scanner = SlaveScanner(
- 'mock', MockBuildersCache(interactor.builder, bq), BufferLogger())
-
- # Instrument updateBuild and currentjob.reset
- interactor.updateBuild = FakeMethod()
+ 'mock', MockBuildersCache(MockBuilder(), bq), BufferLogger(),
+ interactor_factory=FakeMethod(interactor),
+ slave_factory=FakeMethod(slave),
+ behavior_factory=FakeMethod(TrivialBehavior()))
# XXX: checkCancellation needs more than a FakeBuildQueue.
scanner.checkCancellation = FakeMethod(defer.succeed(False))
# A single scan will call status(), notice that the slave is
# lost, abort() the slave, then reset() the job without calling
# updateBuild().
- yield scanner.scan(interactor=interactor)
- self.assertEqual(['status', 'abort'], interactor.slave.call_log)
+ yield scanner.scan()
+ self.assertEqual(['status', 'abort'], slave.call_log)
self.assertEqual(0, interactor.updateBuild.call_count)
self.assertEqual(1, bq.reset.call_count)
@@ -513,19 +521,23 @@
def test_scan_aborts_lost_slave_when_idle(self):
# SlaveScanner.scan uses BuilderInteractor.rescueIfLost to abort
# slaves that aren't meant to have a job.
- interactor = BuilderInteractor(MockBuilder(), BuildingSlave(), None)
- scanner = SlaveScanner(
- 'mock', MockBuildersCache(interactor.builder, None),
- BufferLogger())
+ slave = BuildingSlave()
# Instrument updateBuild.
+ interactor = BuilderInteractor()
interactor.updateBuild = FakeMethod()
+ scanner = SlaveScanner(
+ 'mock', MockBuildersCache(MockBuilder(), None), BufferLogger(),
+ interactor_factory=FakeMethod(interactor),
+ slave_factory=FakeMethod(slave),
+ behavior_factory=FakeMethod(None))
+
# A single scan will call status(), notice that the slave is
# lost, abort() the slave, then reset() the job without calling
# updateBuild().
- yield scanner.scan(interactor=interactor)
- self.assertEqual(['status', 'abort'], interactor.slave.call_log)
+ yield scanner.scan()
+ self.assertEqual(['status', 'abort'], slave.call_log)
self.assertEqual(0, interactor.updateBuild.call_count)
@@ -540,7 +552,11 @@
builder_name = BOB_THE_BUILDER_NAME
self.builder = getUtility(IBuilderSet)[builder_name]
self.builder.virtualized = True
- self.interactor = BuilderInteractor(self.builder)
+ self.interactor = BuilderInteractor()
+
+ @property
+ def vitals(self):
+ return extract_vitals_from_db(self.builder)
def _getScanner(self, clock=None):
scanner = SlaveScanner(
@@ -551,7 +567,8 @@
def test_ignores_nonvirtual(self):
# If the builder is nonvirtual make sure we return False.
self.builder.virtualized = False
- d = self._getScanner().checkCancellation(self.builder, self.interactor)
+ d = self._getScanner().checkCancellation(
+ self.vitals, self.builder, None, self.interactor)
return d.addCallback(self.assertFalse)
def test_ignores_no_buildqueue(self):
@@ -559,12 +576,14 @@
# make sure we return False.
buildqueue = self.builder.currentjob
buildqueue.reset()
- d = self._getScanner().checkCancellation(self.builder, self.interactor)
+ d = self._getScanner().checkCancellation(
+ self.vitals, self.builder, None, self.interactor)
return d.addCallback(self.assertFalse)
def test_ignores_build_not_cancelling(self):
# If the active build is not in a CANCELLING state, ignore it.
- d = self._getScanner().checkCancellation(self.builder, self.interactor)
+ d = self._getScanner().checkCancellation(
+ self.vitals, self.builder, None, self.interactor)
return d.addCallback(self.assertFalse)
@defer.inlineCallbacks
@@ -573,21 +592,21 @@
# True is returned and the slave was resumed.
slave = OkSlave()
self.builder.vm_host = "fake_vm_host"
- self.patch(BuilderSlave, 'makeBuilderSlave', FakeMethod(slave))
- self.interactor = BuilderInteractor(self.builder)
buildqueue = self.builder.currentjob
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
build.updateStatus(BuildStatus.CANCELLING)
clock = task.Clock()
scanner = self._getScanner(clock=clock)
- result = yield scanner.checkCancellation(self.builder, self.interactor)
+ result = yield scanner.checkCancellation(
+ self.vitals, self.builder, slave, self.interactor)
self.assertNotIn("resume", slave.call_log)
self.assertFalse(result)
self.assertEqual(BuildStatus.CANCELLING, build.status)
clock.advance(SlaveScanner.CANCEL_TIMEOUT)
- result = yield scanner.checkCancellation(self.builder, self.interactor)
+ result = yield scanner.checkCancellation(
+ self.vitals, self.builder, slave, self.interactor)
self.assertEqual(1, slave.call_log.count("resume"))
self.assertTrue(result)
self.assertEqual(BuildStatus.CANCELLED, build.status)
@@ -598,13 +617,11 @@
# immediately resume the slave as if the cancel timeout had expired.
slave = LostBuildingBrokenSlave()
self.builder.vm_host = "fake_vm_host"
- self.patch(BuilderSlave, 'makeBuilderSlave', FakeMethod(slave))
- self.interactor = BuilderInteractor(self.builder)
buildqueue = self.builder.currentjob
build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(buildqueue)
build.updateStatus(BuildStatus.CANCELLING)
result = yield self._getScanner().checkCancellation(
- self.builder, self.interactor)
+ self.vitals, self.builder, slave, self.interactor)
self.assertEqual(1, slave.call_log.count("resume"))
self.assertTrue(result)
self.assertEqual(BuildStatus.CANCELLED, build.status)
@@ -649,6 +666,7 @@
class TestFailureAssessments(TestCaseWithFactory):
layer = ZopelessDatabaseLayer
+ run_tests_with = AsynchronousDeferredRunTest
def setUp(self):
TestCaseWithFactory.setUp(self)
@@ -656,12 +674,13 @@
self.build = self.factory.makeSourcePackageRecipeBuild()
self.buildqueue = self.build.queueBuild()
self.buildqueue.markAsBuilding(self.builder)
- self.interactor = BuilderInteractor(self.builder)
+ self.slave = OkSlave()
def _assessFailureCounts(self, fail_notes):
# Helper for assessFailureCounts boilerplate.
return assessFailureCounts(
- BufferLogger(), self.interactor, Exception(fail_notes))
+ BufferLogger(), extract_vitals_from_db(self.builder), self.builder,
+ self.slave, BuilderInteractor(), Exception(fail_notes))
@defer.inlineCallbacks
def test_equal_failures_reset_job(self):
@@ -686,7 +705,7 @@
@defer.inlineCallbacks
def test_virtual_builder_reset_thresholds(self):
self.builder.virtualized = True
- self.patch(self.interactor, "resumeSlaveHost", FakeMethod())
+ self.builder.vm_host = 'foo'
for failure_count in range(
Builder.RESET_THRESHOLD - 1,
@@ -697,7 +716,7 @@
self.assertEqual(self.build.status, BuildStatus.NEEDSBUILD)
self.assertEqual(
failure_count // Builder.RESET_THRESHOLD,
- self.interactor.resumeSlaveHost.call_count)
+ self.slave.call_log.count('resume'))
self.assertTrue(self.builder.builderok)
self.builder.failure_count = (
@@ -707,27 +726,25 @@
self.assertEqual(self.build.status, BuildStatus.NEEDSBUILD)
self.assertEqual(
Builder.RESET_FAILURE_THRESHOLD - 1,
- self.interactor.resumeSlaveHost.call_count)
+ self.slave.call_log.count('resume'))
self.assertFalse(self.builder.builderok)
self.assertEqual("failnotes", self.builder.failnotes)
@defer.inlineCallbacks
def test_non_virtual_builder_reset_thresholds(self):
self.builder.virtualized = False
- self.patch(self.interactor, "resumeSlaveHost", FakeMethod())
-
self.builder.failure_count = Builder.RESET_THRESHOLD - 1
yield self._assessFailureCounts("failnotes")
self.assertIs(None, self.builder.currentjob)
self.assertEqual(self.build.status, BuildStatus.NEEDSBUILD)
- self.assertEqual(0, self.interactor.resumeSlaveHost.call_count)
+ self.assertEqual(0, self.slave.call_log.count('resume'))
self.assertTrue(self.builder.builderok)
self.builder.failure_count = Builder.RESET_THRESHOLD
yield self._assessFailureCounts("failnotes")
self.assertIs(None, self.builder.currentjob)
self.assertEqual(self.build.status, BuildStatus.NEEDSBUILD)
- self.assertEqual(0, self.interactor.resumeSlaveHost.call_count)
+ self.assertEqual(0, self.slave.call_log.count('resume'))
self.assertFalse(self.builder.builderok)
self.assertEqual("failnotes", self.builder.failnotes)
=== modified file 'lib/lp/code/model/tests/test_recipebuilder.py'
--- lib/lp/code/model/tests/test_recipebuilder.py 2013-09-23 02:04:18 +0000
+++ lib/lp/code/model/tests/test_recipebuilder.py 2013-10-01 01:11:42 +0000
@@ -369,8 +369,8 @@
self.addCleanup(config.pop, 'tmp_builddmaster_root')
self.queue_record.builder = self.factory.makeBuilder()
slave = WaitingSlave('BuildStatus.OK')
- interactor = BuilderInteractor(self.queue_record.builder, slave)
- return removeSecurityProxy(interactor._current_build_behavior)
+ return BuilderInteractor.getBuildBehavior(
+ self.queue_record, self.queue_record.builder, slave)
def assertDeferredNotifyCount(self, status, behavior, expected_count):
d = behavior.handleStatus(self.queue_record, status, {'filemap': {}})
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py'
--- lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 2013-10-01 01:11:42 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py 2013-10-01 01:11:42 +0000
@@ -20,7 +20,7 @@
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interactor import (
BuilderInteractor,
- BuilderSlave,
+ extract_vitals_from_db,
)
from lp.buildmaster.interfaces.builder import CannotBuild
from lp.buildmaster.interfaces.buildfarmjobbehavior import (
@@ -51,7 +51,6 @@
from lp.soyuz.enums import ArchivePurpose
from lp.testing import TestCaseWithFactory
from lp.testing.dbuser import switch_dbuser
-from lp.testing.fakemethod import FakeMethod
from lp.testing.layers import LaunchpadZopelessLayer
@@ -72,16 +71,16 @@
super(TestBinaryBuildPackageBehavior, self).setUp()
switch_dbuser('testadmin')
- def assertExpectedInteraction(self, ignored, call_log, interactor, build,
+ def assertExpectedInteraction(self, ignored, call_log, builder, build,
chroot, archive, archive_purpose,
component=None, extra_urls=None,
filemap_names=None):
expected = self.makeExpectedInteraction(
- interactor, build, chroot, archive, archive_purpose, component,
+ builder, build, chroot, archive, archive_purpose, component,
extra_urls, filemap_names)
self.assertEqual(call_log, expected)
- def makeExpectedInteraction(self, interactor, build, chroot, archive,
+ def makeExpectedInteraction(self, builder, build, chroot, archive,
archive_purpose, component=None,
extra_urls=None, filemap_names=None):
"""Build the log of calls that we expect to be made to the slave.
@@ -97,7 +96,8 @@
in order to trick the slave into building correctly.
:return: A list of the calls we expect to be made.
"""
- cookie = interactor._current_build_behavior.getBuildCookie()
+ cookie = IBuildFarmJobBehavior(
+ build.buildqueue_record.specific_job).getBuildCookie()
ds_name = build.distro_arch_series.distroseries.name
suite = ds_name + pocketsuffix[build.pocket]
archives = get_sources_list_for_building(
@@ -128,7 +128,7 @@
build_log = [
('build', cookie, 'binarypackage', chroot.content.sha1,
filemap_names, extra_args)]
- if interactor.builder.virtualized:
+ if builder.virtualized:
result = [('echo', 'ping')] + upload_logs + build_log
else:
result = upload_logs + build_log
@@ -143,6 +143,7 @@
archive = self.factory.makeArchive(virtualized=False)
slave = OkSlave()
builder = self.factory.makeBuilder(virtualized=False)
+ vitals = extract_vitals_from_db(builder)
build = self.factory.makeBinaryPackageBuild(
builder=builder, archive=archive)
lf = self.factory.makeLibraryFileAlias()
@@ -150,12 +151,12 @@
build.distro_arch_series.addOrUpdateChroot(lf)
bq = build.queueBuild()
bq.markAsBuilding(builder)
- interactor = BuilderInteractor(builder, slave)
+ interactor = BuilderInteractor()
d = interactor._startBuild(
- bq, interactor.vitals, interactor.builder, interactor.slave,
- interactor._current_build_behavior, BufferLogger())
+ bq, vitals, builder, slave,
+ interactor.getBuildBehavior(bq, builder, slave), BufferLogger())
d.addCallback(
- self.assertExpectedInteraction, slave.call_log, interactor, build,
+ self.assertExpectedInteraction, slave.call_log, builder, build,
lf, archive, ArchivePurpose.PRIMARY, 'universe')
return d
@@ -166,6 +167,7 @@
slave = OkSlave()
builder = self.factory.makeBuilder(
virtualized=True, vm_host="foohost")
+ vitals = extract_vitals_from_db(builder)
build = self.factory.makeBinaryPackageBuild(
builder=builder, archive=archive)
lf = self.factory.makeLibraryFileAlias()
@@ -173,10 +175,10 @@
build.distro_arch_series.addOrUpdateChroot(lf)
bq = build.queueBuild()
bq.markAsBuilding(builder)
- interactor = BuilderInteractor(builder, slave)
+ interactor = BuilderInteractor()
d = interactor._startBuild(
- bq, interactor.vitals, interactor.builder, interactor.slave,
- interactor._current_build_behavior, BufferLogger())
+ bq, vitals, builder, slave,
+ interactor.getBuildBehavior(bq, builder, slave), BufferLogger())
def check_build(ignored):
# We expect the first call to the slave to be a resume call,
@@ -184,7 +186,7 @@
expected_resume_call = slave.call_log.pop(0)
self.assertEqual('resume', expected_resume_call)
self.assertExpectedInteraction(
- ignored, slave.call_log, interactor, build, lf, archive,
+ ignored, slave.call_log, builder, build, lf, archive,
ArchivePurpose.PPA)
return d.addCallback(check_build)
@@ -193,6 +195,7 @@
virtualized=False, purpose=ArchivePurpose.PARTNER)
slave = OkSlave()
builder = self.factory.makeBuilder(virtualized=False)
+ vitals = extract_vitals_from_db(builder)
build = self.factory.makeBinaryPackageBuild(
builder=builder, archive=archive)
lf = self.factory.makeLibraryFileAlias()
@@ -200,12 +203,12 @@
build.distro_arch_series.addOrUpdateChroot(lf)
bq = build.queueBuild()
bq.markAsBuilding(builder)
- interactor = BuilderInteractor(builder, slave)
+ interactor = BuilderInteractor()
d = interactor._startBuild(
- bq, interactor.vitals, interactor.builder, interactor.slave,
- interactor._current_build_behavior, BufferLogger())
+ bq, vitals, builder, slave,
+ interactor.getBuildBehavior(bq, builder, slave), BufferLogger())
d.addCallback(
- self.assertExpectedInteraction, slave.call_log, interactor, build,
+ self.assertExpectedInteraction, slave.call_log, builder, build,
lf, archive, ArchivePurpose.PARTNER)
return d
@@ -322,7 +325,7 @@
switch_dbuser('testadmin')
self.builder = self.factory.makeBuilder()
- self.interactor = BuilderInteractor(self.builder)
+ self.interactor = BuilderInteractor()
self.build = self.factory.makeBinaryPackageBuild(
builder=self.builder, pocket=PackagePublishingPocket.RELEASE)
lf = self.factory.makeLibraryFileAlias()
@@ -334,10 +337,10 @@
# hang around between test runs.
self.addCleanup(self._cleanup)
- def updateBuild(self, candidate):
+ def updateBuild(self, candidate, slave):
return self.interactor.updateBuild(
- candidate, self.interactor.slave,
- self.interactor._current_build_behavior)
+ candidate, slave,
+ self.interactor.getBuildBehavior(candidate, self.builder, slave))
def assertBuildProperties(self, build):
"""Check that a build happened by making sure some of its properties
@@ -350,51 +353,39 @@
def test_packagefail_collection(self):
# When a package fails to build, make sure the builder notes are
# stored and the build status is set as failed.
- waiting_slave = WaitingSlave('BuildStatus.PACKAGEFAIL')
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(waiting_slave))
-
def got_update(ignored):
self.assertBuildProperties(self.build)
self.assertEqual(BuildStatus.FAILEDTOBUILD, self.build.status)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(
+ self.candidate, WaitingSlave('BuildStatus.PACKAGEFAIL'))
return d.addCallback(got_update)
def test_depwait_collection(self):
# Package build was left in dependency wait.
DEPENDENCIES = 'baz (>= 1.0.1)'
- waiting_slave = WaitingSlave('BuildStatus.DEPFAIL', DEPENDENCIES)
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(waiting_slave))
def got_update(ignored):
self.assertBuildProperties(self.build)
self.assertEqual(BuildStatus.MANUALDEPWAIT, self.build.status)
self.assertEqual(DEPENDENCIES, self.build.dependencies)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(
+ self.candidate, WaitingSlave('BuildStatus.DEPFAIL', DEPENDENCIES))
return d.addCallback(got_update)
def test_chrootfail_collection(self):
# There was a chroot problem for this build.
- waiting_slave = WaitingSlave('BuildStatus.CHROOTFAIL')
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(waiting_slave))
-
def got_update(ignored):
self.assertBuildProperties(self.build)
self.assertEqual(BuildStatus.CHROOTWAIT, self.build.status)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(
+ self.candidate, WaitingSlave('BuildStatus.CHROOTFAIL'))
return d.addCallback(got_update)
def test_builderfail_collection(self):
# The builder failed after we dispatched the build.
- waiting_slave = WaitingSlave('BuildStatus.BUILDERFAIL')
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(waiting_slave))
-
def got_update(ignored):
self.assertEqual(
"Builder returned BUILDERFAIL when asked for its status",
@@ -404,40 +395,33 @@
job = self.candidate.specific_job.job
self.assertEqual(JobStatus.WAITING, job.status)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(
+ self.candidate, WaitingSlave('BuildStatus.BUILDERFAIL'))
return d.addCallback(got_update)
def test_building_collection(self):
# The builder is still building the package.
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(BuildingSlave()))
-
def got_update(ignored):
# The fake log is returned from the BuildingSlave() mock.
self.assertEqual("This is a build log", self.candidate.logtail)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(self.candidate, BuildingSlave())
return d.addCallback(got_update)
def test_aborting_collection(self):
# The builder is in the process of aborting.
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(AbortingSlave()))
-
def got_update(ignored):
self.assertEqual(
"Waiting for slave process to be terminated",
self.candidate.logtail)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(self.candidate, AbortingSlave())
return d.addCallback(got_update)
def test_collection_for_deleted_source(self):
# If we collected a build for a superseded/deleted source then
# the build should get marked superseded as the build results
# get discarded.
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(WaitingSlave('BuildStatus.OK')))
spr = removeSecurityProxy(self.build.source_package_release)
pub = self.build.current_source_publication
pub.requestDeletion(spr.creator)
@@ -446,27 +430,21 @@
self.assertEqual(
BuildStatus.SUPERSEDED, self.build.status)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(self.candidate, WaitingSlave('BuildStatus.OK'))
return d.addCallback(got_update)
def test_uploading_collection(self):
# After a successful build, the status should be UPLOADING.
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(WaitingSlave('BuildStatus.OK')))
-
def got_update(ignored):
self.assertEqual(self.build.status, BuildStatus.UPLOADING)
# We do not store any upload log information when the binary
# upload processing succeeded.
self.assertIs(None, self.build.upload_log)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(self.candidate, WaitingSlave('BuildStatus.OK'))
return d.addCallback(got_update)
def test_givenback_collection(self):
- waiting_slave = WaitingSlave('BuildStatus.GIVENBACK')
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(waiting_slave))
score = self.candidate.lastscore
def got_update(ignored):
@@ -477,7 +455,8 @@
job = self.candidate.specific_job.job
self.assertEqual(JobStatus.WAITING, job.status)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(
+ self.candidate, WaitingSlave('BuildStatus.GIVENBACK'))
return d.addCallback(got_update)
def test_log_file_collection(self):
@@ -532,8 +511,6 @@
def test_private_build_log_storage(self):
# Builds in private archives should have their log uploaded to
# the restricted librarian.
- self.patch(BuilderSlave, 'makeBuilderSlave',
- FakeMethod(WaitingSlave('BuildStatus.OK')))
# Go behind Storm's back since the field validator on
# Archive.private prevents us from setting it to True with
@@ -548,7 +525,7 @@
self.layer.txn.commit()
self.assertTrue(self.build.log.restricted)
- d = self.updateBuild(self.candidate)
+ d = self.updateBuild(self.candidate, WaitingSlave('BuildStatus.OK'))
return d.addCallback(got_update)
Follow ups
