Page MenuHomePhabricator
Files F15431605

D21687.id.diff
No OneTemporary
Actions

D21687.id.diff
View Options

diff --git a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
--- a/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
+++ b/src/applications/harbormaster/constants/HarbormasterBuildStatus.php
@@ -53,10 +53,6 @@
return $this->getProperty('isComplete');
}
- public function isPending() {
- return $this->getProperty('isPending');
- }
-
public function isPassed() {
return ($this->key === self::STATUS_PASSED);
}
@@ -81,6 +77,10 @@
return ($this->key === self::PENDING_PAUSING);
}
+ public function isPending() {
+ return ($this->key === self::STATUS_PENDING);
+ }
+
public function getIconIcon() {
return $this->getProperty('icon');
}
@@ -170,7 +170,6 @@
'color.ansi' => 'yellow',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => false,
),
self::STATUS_PENDING => array(
'name' => pht('Pending'),
@@ -179,7 +178,6 @@
'color.ansi' => 'yellow',
'isBuilding' => true,
'isComplete' => false,
- 'isPending' => false,
),
self::STATUS_BUILDING => array(
'name' => pht('Building'),
@@ -188,7 +186,6 @@
'color.ansi' => 'yellow',
'isBuilding' => true,
'isComplete' => false,
- 'isPending' => false,
),
self::STATUS_PASSED => array(
'name' => pht('Passed'),
@@ -197,7 +194,6 @@
'color.ansi' => 'green',
'isBuilding' => false,
'isComplete' => true,
- 'isPending' => false,
),
self::STATUS_FAILED => array(
'name' => pht('Failed'),
@@ -206,7 +202,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => true,
- 'isPending' => false,
),
self::STATUS_ABORTED => array(
'name' => pht('Aborted'),
@@ -215,7 +210,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => true,
- 'isPending' => false,
),
self::STATUS_ERROR => array(
'name' => pht('Unexpected Error'),
@@ -224,7 +218,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => true,
- 'isPending' => false,
),
self::STATUS_PAUSED => array(
'name' => pht('Paused'),
@@ -233,7 +226,6 @@
'color.ansi' => 'yellow',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => false,
),
self::STATUS_DEADLOCKED => array(
'name' => pht('Deadlocked'),
@@ -242,7 +234,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => true,
- 'isPending' => false,
),
self::PENDING_PAUSING => array(
'name' => pht('Pausing'),
@@ -251,7 +242,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => true,
),
self::PENDING_RESUMING => array(
'name' => pht('Resuming'),
@@ -260,7 +250,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => true,
),
self::PENDING_RESTARTING => array(
'name' => pht('Restarting'),
@@ -269,7 +258,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => true,
),
self::PENDING_ABORTING => array(
'name' => pht('Aborting'),
@@ -278,7 +266,6 @@
'color.ansi' => 'red',
'isBuilding' => false,
'isComplete' => false,
- 'isPending' => true,
),
);
}
diff --git a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
--- a/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
+++ b/src/applications/harbormaster/editor/HarbormasterBuildTransactionEditor.php
@@ -65,48 +65,32 @@
HarbormasterBuild $build,
HarbormasterBuildTransaction $xaction) {
- $command = $xaction->getNewValue();
+ $actor = $this->getActor();
+ $message_type = $xaction->getNewValue();
- switch ($command) {
- case HarbormasterBuildCommand::COMMAND_RESTART:
- $issuable = $build->canRestartBuild();
+ // TODO: Restore logic that tests if the command can issue without causing
+ // anything to lapse into an invalid state. This should not be the same
+ // as the logic which powers the web UI: for example, if an "abort" is
+ // queued we want to disable "Abort" in the web UI, but should obviously
+ // process it here.
+
+ switch ($message_type) {
+ case HarbormasterBuildCommand::COMMAND_ABORT:
+ // TODO: This should move to external effects, perhaps.
+ $build->releaseAllArtifacts($actor);
+ $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED);
break;
- case HarbormasterBuildCommand::COMMAND_PAUSE:
- $issuable = $build->canPauseBuild();
+ case HarbormasterBuildCommand::COMMAND_RESTART:
+ $build->restartBuild($actor);
+ $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
break;
case HarbormasterBuildCommand::COMMAND_RESUME:
- $issuable = $build->canResumeBuild();
+ $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
break;
- case HarbormasterBuildCommand::COMMAND_ABORT:
- $issuable = $build->canAbortBuild();
+ case HarbormasterBuildCommand::COMMAND_PAUSE:
+ $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED);
break;
- default:
- throw new Exception(pht('Unknown command %s', $command));
- }
-
- if (!$issuable) {
- return;
}
-
- $actor = $this->getActor();
- if (!$build->canIssueCommand($actor, $command)) {
- return;
- }
-
- HarbormasterBuildMessage::initializeNewMessage($actor)
- ->setAuthorPHID($xaction->getAuthorPHID())
- ->setReceiverPHID($build->getPHID())
- ->setType($command)
- ->save();
-
- PhabricatorWorker::scheduleTask(
- 'HarbormasterBuildWorker',
- array(
- 'buildID' => $build->getID(),
- ),
- array(
- 'objectPHID' => $build->getPHID(),
- ));
}
protected function applyCustomExternalTransaction(
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
@@ -49,6 +49,7 @@
}
public function continueBuild() {
+ $viewer = $this->getViewer();
$build = $this->getBuild();
$lock_key = 'harbormaster.build:'.$build->getID();
@@ -68,7 +69,7 @@
$lock->unlock();
- $this->releaseAllArtifacts($build);
+ $build->releaseAllArtifacts($viewer);
throw $ex;
}
@@ -99,56 +100,58 @@
// If we are no longer building for any reason, release all artifacts.
if (!$build->isBuilding()) {
- $this->releaseAllArtifacts($build);
+ $build->releaseAllArtifacts($viewer);
}
}
private function updateBuild(HarbormasterBuild $build) {
- if ($build->isAborting()) {
- $this->releaseAllArtifacts($build);
- $build->setBuildStatus(HarbormasterBuildStatus::STATUS_ABORTED);
- $build->save();
- }
+ $viewer = $this->getViewer();
- if (($build->getBuildStatus() == HarbormasterBuildStatus::STATUS_PENDING) ||
- ($build->isRestarting())) {
- $this->restartBuild($build);
- $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
- $build->save();
- }
+ $content_source = PhabricatorContentSource::newForSource(
+ PhabricatorDaemonContentSource::SOURCECONST);
- if ($build->isResuming()) {
- $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
- $build->save();
+ $acting_phid = $viewer->getPHID();
+ if (!$acting_phid) {
+ $acting_phid = id(new PhabricatorHarbormasterApplication())->getPHID();
}
- if ($build->isPausing() && !$build->isComplete()) {
- $build->setBuildStatus(HarbormasterBuildStatus::STATUS_PAUSED);
- $build->save();
- }
+ $editor = $build->getApplicationTransactionEditor()
+ ->setActor($viewer)
+ ->setActingAsPHID($acting_phid)
+ ->setContentSource($content_source)
+ ->setContinueOnNoEffect(true)
+ ->setContinueOnMissingFields(true);
- $build->markUnprocessedMessagesAsProcessed();
+ $xactions = array();
- if ($build->getBuildStatus() == HarbormasterBuildStatus::STATUS_BUILDING) {
- $this->updateBuildSteps($build);
- }
- }
+ $messages = $build->getUnprocessedMessagesForApply();
+ foreach ($messages as $message) {
+ $message_type = $message->getType();
- private function restartBuild(HarbormasterBuild $build) {
+ $xactions[] = $build->getApplicationTransactionTemplate()
+ ->setAuthorPHID($message->getAuthorPHID())
+ ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
+ ->setNewValue($message_type);
+ }
- // We're restarting the build, so release all previous artifacts.
- $this->releaseAllArtifacts($build);
+ if (!$xactions) {
+ if ($build->isPending()) {
+ // TODO: This should be a transaction.
- // Increment the build generation counter on the build.
- $build->setBuildGeneration($build->getBuildGeneration() + 1);
+ $build->restartBuild($viewer);
+ $build->setBuildStatus(HarbormasterBuildStatus::STATUS_BUILDING);
+ $build->save();
+ }
+ }
- // Currently running targets should periodically check their build
- // generation (which won't have changed) against the build's generation.
- // If it is different, they will automatically stop what they're doing
- // and abort.
+ if ($xactions) {
+ $editor->applyTransactions($build, $xactions);
+ $build->markUnprocessedMessagesAsProcessed();
+ }
- // Previously we used to delete targets, logs and artifacts here. Instead,
- // leave them around so users can view previous generations of this build.
+ if ($build->getBuildStatus() == HarbormasterBuildStatus::STATUS_BUILDING) {
+ $this->updateBuildSteps($build);
+ }
}
private function updateBuildSteps(HarbormasterBuild $build) {
@@ -596,29 +599,6 @@
->publishBuildable($old, $new);
}
- private function releaseAllArtifacts(HarbormasterBuild $build) {
- $targets = id(new HarbormasterBuildTargetQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withBuildPHIDs(array($build->getPHID()))
- ->withBuildGenerations(array($build->getBuildGeneration()))
- ->execute();
-
- if (count($targets) === 0) {
- return;
- }
-
- $target_phids = mpull($targets, 'getPHID');
-
- $artifacts = id(new HarbormasterBuildArtifactQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withBuildTargetPHIDs($target_phids)
- ->withIsReleased(false)
- ->execute();
- foreach ($artifacts as $artifact) {
- $artifact->releaseArtifact();
- }
- }
-
private function releaseQueuedArtifacts() {
foreach ($this->artifactReleaseQueue as $key => $artifact) {
$artifact->releaseArtifact();
diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php
--- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php
+++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php
@@ -207,13 +207,50 @@
return $this->getBuildStatusObject()->isFailed();
}
+ public function isPending() {
+ return $this->getBuildstatusObject()->isPending();
+ }
+
public function getURI() {
$id = $this->getID();
return "/harbormaster/build/{$id}/";
}
public function getBuildPendingStatusObject() {
+ list($pending_status) = $this->getUnprocessedMessageState();
+
+ if ($pending_status !== null) {
+ return HarbormasterBuildStatus::newBuildStatusObject($pending_status);
+ }
+
+ return $this->getBuildStatusObject();
+ }
+ protected function getBuildStatusObject() {
+ $status_key = $this->getBuildStatus();
+ return HarbormasterBuildStatus::newBuildStatusObject($status_key);
+ }
+
+ public function getObjectName() {
+ return pht('Build %d', $this->getID());
+ }
+
+
+/* -( Build Messages )----------------------------------------------------- */
+
+
+ private function getUnprocessedMessages() {
+ return $this->assertAttached($this->unprocessedMessages);
+ }
+
+ public function getUnprocessedMessagesForApply() {
+ $unprocessed_state = $this->getUnprocessedMessageState();
+ list($pending_status, $apply_messages) = $unprocessed_state;
+
+ return $apply_messages;
+ }
+
+ private function getUnprocessedMessageState() {
// NOTE: If a build has multiple unprocessed messages, we'll ignore
// messages that are obsoleted by a later or stronger message.
//
@@ -230,24 +267,30 @@
$is_pausing = false;
$is_resuming = false;
+ $apply_messages = array();
+
foreach ($this->getUnprocessedMessages() as $message_object) {
$message_type = $message_object->getType();
switch ($message_type) {
case HarbormasterBuildCommand::COMMAND_RESTART:
$is_restarting = true;
$is_aborting = false;
+ $apply_messages = array($message_object);
break;
case HarbormasterBuildCommand::COMMAND_ABORT:
$is_aborting = true;
$is_restarting = false;
+ $apply_messages = array($message_object);
break;
case HarbormasterBuildCommand::COMMAND_PAUSE:
$is_pausing = true;
$is_resuming = false;
+ $apply_messages = array($message_object);
break;
case HarbormasterBuildCommand::COMMAND_RESUME:
$is_resuming = true;
$is_pausing = false;
+ $apply_messages = array($message_object);
break;
}
}
@@ -263,28 +306,7 @@
$pending_status = HarbormasterBuildStatus::PENDING_RESUMING;
}
- if ($pending_status !== null) {
- return HarbormasterBuildStatus::newBuildStatusObject($pending_status);
- }
-
- return $this->getBuildStatusObject();
- }
-
- protected function getBuildStatusObject() {
- $status_key = $this->getBuildStatus();
- return HarbormasterBuildStatus::newBuildStatusObject($status_key);
- }
-
- public function getObjectName() {
- return pht('Build %d', $this->getID());
- }
-
-
-/* -( Build Messages )----------------------------------------------------- */
-
-
- private function getUnprocessedMessages() {
- return $this->assertAttached($this->unprocessedMessages);
+ return array($pending_status, $apply_messages);
}
public function attachUnprocessedMessages(array $messages) {
@@ -475,32 +497,61 @@
}
public function sendMessage(PhabricatorUser $viewer, $message_type) {
- // TODO: This should not be an editor transaction, but there are plans to
- // merge BuildCommand into BuildMessage which should moot this. As this
- // exists today, it can race against BuildEngine.
-
- // This is a bogus content source, but this whole flow should be obsolete
- // soon.
- $content_source = PhabricatorContentSource::newForSource(
- PhabricatorConsoleContentSource::SOURCECONST);
-
- $editor = id(new HarbormasterBuildTransactionEditor())
- ->setActor($viewer)
- ->setContentSource($content_source)
- ->setContinueOnNoEffect(true)
- ->setContinueOnMissingFields(true);
-
- $viewer_phid = $viewer->getPHID();
- if (!$viewer_phid) {
- $acting_phid = id(new PhabricatorHarbormasterApplication())->getPHID();
- $editor->setActingAsPHID($acting_phid);
+ HarbormasterBuildMessage::initializeNewMessage($viewer)
+ ->setReceiverPHID($this->getPHID())
+ ->setType($message_type)
+ ->save();
+
+ PhabricatorWorker::scheduleTask(
+ 'HarbormasterBuildWorker',
+ array(
+ 'buildID' => $this->getID(),
+ ),
+ array(
+ 'objectPHID' => $this->getPHID(),
+ 'containerPHID' => $this->getBuildablePHID(),
+ ));
+ }
+
+ public function releaseAllArtifacts(PhabricatorUser $viewer) {
+ $targets = id(new HarbormasterBuildTargetQuery())
+ ->setViewer($viewer)
+ ->withBuildPHIDs(array($this->getPHID()))
+ ->withBuildGenerations(array($this->getBuildGeneration()))
+ ->execute();
+
+ if (!$targets) {
+ return;
}
- $xaction = id(new HarbormasterBuildTransaction())
- ->setTransactionType(HarbormasterBuildTransaction::TYPE_COMMAND)
- ->setNewValue($message_type);
+ $target_phids = mpull($targets, 'getPHID');
+
+ $artifacts = id(new HarbormasterBuildArtifactQuery())
+ ->setViewer($viewer)
+ ->withBuildTargetPHIDs($target_phids)
+ ->withIsReleased(false)
+ ->execute();
+ foreach ($artifacts as $artifact) {
+ $artifact->releaseArtifact();
+ }
+ }
+
+ public function restartBuild(PhabricatorUser $viewer) {
+ // TODO: This should become transactional.
+
+ // We're restarting the build, so release all previous artifacts.
+ $this->releaseAllArtifacts($viewer);
+
+ // Increment the build generation counter on the build.
+ $this->setBuildGeneration($this->getBuildGeneration() + 1);
+
+ // Currently running targets should periodically check their build
+ // generation (which won't have changed) against the build's generation.
+ // If it is different, they will automatically stop what they're doing
+ // and abort.
- $editor->applyTransactions($this, array($xaction));
+ // Previously we used to delete targets, logs and artifacts here. Instead,
+ // leave them around so users can view previous generations of this build.
}

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 25, 1:23 PM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7706942
Default Alt Text
D21687.id.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility