Page MenuHomePhabricator
Files F14761805

D10322.diff
No OneTemporary
Actions

D10322.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -654,6 +654,7 @@
'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 @@
'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
--- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
+++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php
@@ -130,9 +130,9 @@
// 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
--- /dev/null
+++ b/src/applications/harbormaster/exception/HarbormasterBuildAbortedException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class HarbormasterBuildAbortedException extends Exception {
+
+}
diff --git a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
--- a/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
@@ -222,4 +222,29 @@
return (bool)$target->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
--- a/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php
@@ -62,26 +62,49 @@
$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();
-
- // Wait one second before querying for more data.
- sleep(1);
+ $futures = Futures(array($future));
+ foreach ($futures->setUpdateInterval(1) as $key => $future_iter) {
+ if ($future_iter === null) {
+
+ // 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
--- a/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
@@ -66,7 +66,10 @@
$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
--- a/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php
@@ -23,7 +23,26 @@
$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
--- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
@@ -19,6 +19,7 @@
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 @@
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 @@
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 @@
case self::STATUS_PASSED:
return 'green';
case self::STATUS_FAILED:
+ case self::STATUS_ABORTED:
return 'red';
default:
return 'bluegrey';
@@ -179,6 +185,7 @@
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
--- a/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
+++ b/src/applications/harbormaster/worker/HarbormasterTargetWorker.php
@@ -38,6 +38,10 @@
$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 @@
$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);

File Metadata

Mime Type
text/plain
Expires
Fri, Jan 24, 3:16 AM (21 h, 27 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7038766
Default Alt Text
D10322.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility