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. }