diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0adb3af9db..7006beff68 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -654,6 +654,7 @@ phutil_register_library_map(array( 'FlagEditConduitAPIMethod' => 'applications/flag/conduit/FlagEditConduitAPIMethod.php', 'FlagQueryConduitAPIMethod' => 'applications/flag/conduit/FlagQueryConduitAPIMethod.php', 'HarbormasterBuild' => 'applications/harbormaster/storage/build/HarbormasterBuild.php', + 'HarbormasterBuildAbortedException' => 'applications/harbormaster/exception/HarbormasterBuildAbortedException.php', 'HarbormasterBuildActionController' => 'applications/harbormaster/controller/HarbormasterBuildActionController.php', 'HarbormasterBuildArtifact' => 'applications/harbormaster/storage/build/HarbormasterBuildArtifact.php', 'HarbormasterBuildArtifactQuery' => 'applications/harbormaster/query/HarbormasterBuildArtifactQuery.php', @@ -3419,6 +3420,7 @@ phutil_register_library_map(array( 'HarbormasterDAO', 'PhabricatorPolicyInterface', ), + 'HarbormasterBuildAbortedException' => 'Exception', 'HarbormasterBuildActionController' => 'HarbormasterController', 'HarbormasterBuildArtifact' => array( 'HarbormasterDAO', diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php index a3faf45314..98e67635f3 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -130,9 +130,9 @@ final class HarbormasterBuildEngine extends Phobject { // Increment the build generation counter on the build. $build->setBuildGeneration($build->getBuildGeneration() + 1); - // TODO: Currently running targets should periodically check their build + // Currently running targets should periodically check their build // generation (which won't have changed) against the build's generation. - // If it is different, they should automatically stop what they're doing + // If it is different, they will automatically stop what they're doing // and abort. // Previously we used to delete targets, logs and artifacts here. Instead diff --git a/src/applications/harbormaster/exception/HarbormasterBuildAbortedException.php b/src/applications/harbormaster/exception/HarbormasterBuildAbortedException.php new file mode 100644 index 0000000000..7cbb8cf44b --- /dev/null +++ b/src/applications/harbormaster/exception/HarbormasterBuildAbortedException.php @@ -0,0 +1,5 @@ +getDetail('builtin.wait-for-message'); } + protected function shouldAbort( + HarbormasterBuild $build, + HarbormasterBuildTarget $target) { + + return $build->getBuildGeneration() !== $target->getBuildGeneration(); + } + + protected function resolveFuture( + HarbormasterBuild $build, + HarbormasterBuildTarget $target, + Future $future) { + + $futures = Futures(array($future)); + foreach ($futures->setUpdateInterval(5) as $key => $future) { + if ($future === null) { + $build->reload(); + if ($this->shouldAbort($build, $target)) { + throw new HarbormasterBuildAbortedException(); + } + } else { + return $future->resolve(); + } + } + } + } diff --git a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php index d16f11fcfc..806e311083 100644 --- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php @@ -62,26 +62,49 @@ final class HarbormasterCommandBuildStepImplementation $start_stdout = $log_stdout->start(); $start_stderr = $log_stderr->start(); + $build_update = 5; + // Read the next amount of available output every second. - while (!$future->isReady()) { - list($stdout, $stderr) = $future->read(); - $log_stdout->append($stdout); - $log_stderr->append($stderr); - $future->discardBuffers(); + $futures = Futures(array($future)); + foreach ($futures->setUpdateInterval(1) as $key => $future_iter) { + if ($future_iter === null) { - // Wait one second before querying for more data. - sleep(1); + // Check to see if we should abort. + if ($build_update <= 0) { + $build->reload(); + if ($this->shouldAbort($build, $build_target)) { + $future->resolveKill(); + throw new HarbormasterBuildAbortedException(); + } else { + $build_update = 5; + } + } else { + $build_update -= 1; + } + + // Command is still executing. + + // Read more data as it is available. + list($stdout, $stderr) = $future->read(); + $log_stdout->append($stdout); + $log_stderr->append($stderr); + $future->discardBuffers(); + } else { + // Command execution is complete. + + // Get the return value so we can log that as well. + list($err) = $future->resolve(); + + // Retrieve the last few bits of information. + list($stdout, $stderr) = $future->read(); + $log_stdout->append($stdout); + $log_stderr->append($stderr); + $future->discardBuffers(); + + break; + } } - // Get the return value so we can log that as well. - list($err) = $future->resolve(); - - // Retrieve the last few bits of information. - list($stdout, $stderr) = $future->read(); - $log_stdout->append($stdout); - $log_stderr->append($stderr); - $future->discardBuffers(); - $log_stdout->finalize($start_stdout); $log_stderr->finalize($start_stderr); diff --git a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php index 10df86689f..dfc29cedc7 100644 --- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php @@ -66,7 +66,10 @@ final class HarbormasterHTTPRequestBuildStepImplementation $key->getPasswordEnvelope()); } - list($status, $body, $headers) = $future->resolve(); + list($status, $body, $headers) = $this->resolveFuture( + $build, + $build_target, + $future); $log_body->append($body); $log_body->finalize($start); diff --git a/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php index bca222c17a..968c5b99cb 100644 --- a/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php +++ b/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php @@ -23,7 +23,26 @@ final class HarbormasterSleepBuildStepImplementation $settings = $this->getSettings(); - sleep($settings['seconds']); + $target = time() + $settings['seconds']; + + // Use $build_update so that we only reload every 5 seconds, but + // the sleep mechanism remains accurate. + $build_update = 5; + + while (time() < $target) { + sleep(1); + + if ($build_update <= 0) { + $build->reload(); + $build_update = 5; + + if ($this->shouldAbort($build, $build_target)) { + throw new HarbormasterBuildAbortedException(); + } + } else { + $build_update -= 1; + } + } } public function getFieldSpecifications() { diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php index 3271903d70..1a142dc5cb 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php @@ -19,6 +19,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO const STATUS_WAITING = 'target/waiting'; const STATUS_PASSED = 'target/passed'; const STATUS_FAILED = 'target/failed'; + const STATUS_ABORTED = 'target/aborted'; private $build = self::ATTACHABLE; private $buildStep = self::ATTACHABLE; @@ -36,6 +37,8 @@ final class HarbormasterBuildTarget extends HarbormasterDAO return pht('Passed'); case self::STATUS_FAILED: return pht('Failed'); + case self::STATUS_ABORTED: + return pht('Aborted'); default: return pht('Unknown'); } @@ -52,6 +55,8 @@ final class HarbormasterBuildTarget extends HarbormasterDAO return PHUIStatusItemView::ICON_ACCEPT; case self::STATUS_FAILED: return PHUIStatusItemView::ICON_REJECT; + case self::STATUS_ABORTED: + return PHUIStatusItemView::ICON_MINUS; default: return PHUIStatusItemView::ICON_QUESTION; } @@ -66,6 +71,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO case self::STATUS_PASSED: return 'green'; case self::STATUS_FAILED: + case self::STATUS_ABORTED: return 'red'; default: return 'bluegrey'; @@ -179,6 +185,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO switch ($this->getTargetStatus()) { case self::STATUS_PASSED: case self::STATUS_FAILED: + case self::STATUS_ABORTED: return true; } diff --git a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php index f76108f934..180f19907a 100644 --- a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php +++ b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php @@ -38,6 +38,10 @@ final class HarbormasterTargetWorker extends HarbormasterWorker { $target->setDateStarted(time()); try { + if ($target->getBuildGeneration() !== $build->getBuildGeneration()) { + throw new HarbormasterBuildAbortedException(); + } + $status_pending = HarbormasterBuildTarget::STATUS_PENDING; if ($target->getTargetStatus() == $status_pending) { $target->setTargetStatus(HarbormasterBuildTarget::STATUS_BUILDING); @@ -68,6 +72,11 @@ final class HarbormasterTargetWorker extends HarbormasterWorker { $target->setTargetStatus(HarbormasterBuildTarget::STATUS_FAILED); $target->setDateCompleted(time()); $target->save(); + } catch (HarbormasterBuildAbortedException $ex) { + // A build step is aborting because the build has been restarted. + $target->setTargetStatus(HarbormasterBuildTarget::STATUS_ABORTED); + $target->setDateCompleted(time()); + $target->save(); } catch (Exception $ex) { phlog($ex);