Page MenuHomePhabricator
Files F14090241

D7890.diff
No OneTemporary
Actions

D7890.diff
View Options

Index: src/__phutil_library_map__.php
===================================================================
--- src/__phutil_library_map__.php
+++ src/__phutil_library_map__.php
@@ -707,6 +707,7 @@
'HarbormasterBuildArtifact' => 'applications/harbormaster/storage/build/HarbormasterBuildArtifact.php',
'HarbormasterBuildArtifactQuery' => 'applications/harbormaster/query/HarbormasterBuildArtifactQuery.php',
'HarbormasterBuildCancelController' => 'applications/harbormaster/controller/HarbormasterBuildCancelController.php',
+ 'HarbormasterBuildEngine' => 'applications/harbormaster/engine/HarbormasterBuildEngine.php',
'HarbormasterBuildItem' => 'applications/harbormaster/storage/build/HarbormasterBuildItem.php',
'HarbormasterBuildItemQuery' => 'applications/harbormaster/query/HarbormasterBuildItemQuery.php',
'HarbormasterBuildLog' => 'applications/harbormaster/storage/build/HarbormasterBuildLog.php',
@@ -757,7 +758,9 @@
'HarbormasterStepAddController' => 'applications/harbormaster/controller/HarbormasterStepAddController.php',
'HarbormasterStepDeleteController' => 'applications/harbormaster/controller/HarbormasterStepDeleteController.php',
'HarbormasterStepEditController' => 'applications/harbormaster/controller/HarbormasterStepEditController.php',
+ 'HarbormasterTargetWorker' => 'applications/harbormaster/worker/HarbormasterTargetWorker.php',
'HarbormasterUIEventListener' => 'applications/harbormaster/event/HarbormasterUIEventListener.php',
+ 'HarbormasterWorker' => 'applications/harbormaster/worker/HarbormasterWorker.php',
'HeraldAction' => 'applications/herald/storage/HeraldAction.php',
'HeraldAdapter' => 'applications/herald/adapter/HeraldAdapter.php',
'HeraldApplyTranscript' => 'applications/herald/storage/transcript/HeraldApplyTranscript.php',
@@ -3159,6 +3162,7 @@
),
'HarbormasterBuildArtifactQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HarbormasterBuildCancelController' => 'HarbormasterController',
+ 'HarbormasterBuildEngine' => 'Phobject',
'HarbormasterBuildItem' => 'HarbormasterDAO',
'HarbormasterBuildItemQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HarbormasterBuildLog' =>
@@ -3193,7 +3197,7 @@
),
'HarbormasterBuildTargetQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'HarbormasterBuildViewController' => 'HarbormasterController',
- 'HarbormasterBuildWorker' => 'PhabricatorWorker',
+ 'HarbormasterBuildWorker' => 'HarbormasterWorker',
'HarbormasterBuildable' =>
array(
0 => 'HarbormasterDAO',
@@ -3238,7 +3242,9 @@
'HarbormasterStepAddController' => 'HarbormasterController',
'HarbormasterStepDeleteController' => 'HarbormasterController',
'HarbormasterStepEditController' => 'HarbormasterController',
+ 'HarbormasterTargetWorker' => 'HarbormasterWorker',
'HarbormasterUIEventListener' => 'PhabricatorEventListener',
+ 'HarbormasterWorker' => 'PhabricatorWorker',
'HeraldAction' => 'HeraldDAO',
'HeraldApplyTranscript' => 'HeraldDAO',
'HeraldCapabilityManageGlobalRules' => 'PhabricatorPolicyCapability',
Index: src/applications/harbormaster/engine/HarbormasterBuildEngine.php
===================================================================
--- /dev/null
+++ src/applications/harbormaster/engine/HarbormasterBuildEngine.php
@@ -0,0 +1,223 @@
+<?php
+
+/**
+ * Moves a build forward by queuing build tasks, canceling or restarting the
+ * build, or failing it in response to task failures.
+ */
+final class HarbormasterBuildEngine extends Phobject {
+
+ private $build;
+ private $viewer;
+ private $newBuildTargets = array();
+
+ public function queueNewBuildTarget(HarbormasterBuildTarget $target) {
+ $this->newBuildTargets[] = $target;
+ return $this;
+ }
+
+ public function getNewBuildTargets() {
+ return $this->newBuildTargets;
+ }
+
+ public function setViewer(PhabricatorUser $viewer) {
+ $this->viewer = $viewer;
+ return $this;
+ }
+
+ public function getViewer() {
+ return $this->viewer;
+ }
+
+ public function setBuild(HarbormasterBuild $build) {
+ $this->build = $build;
+ return $this;
+ }
+
+ public function getBuild() {
+ return $this->build;
+ }
+
+ public function continueBuild() {
+ $build = $this->getBuild();
+
+ $lock_key = 'harbormaster.build:'.$build->getID();
+ $lock = PhabricatorGlobalLock::newLock($lock_key)->lock(15);
+
+ $build->reload();
+
+ try {
+ $this->updateBuild($build);
+ } catch (Exception $ex) {
+ // If any exception is raised, the build is marked as a failure and the
+ // exception is re-thrown (this ensures we don't leave builds in an
+ // inconsistent state).
+ $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR);
+ $build->save();
+
+ $lock->unlock();
+ throw $ex;
+ }
+
+ $lock->unlock();
+
+ // NOTE: We queue new targets after releasing the lock so that in-process
+ // execution via `bin/harbormaster` does not reenter the locked region.
+ foreach ($this->getNewBuildTargets() as $target) {
+ $task = PhabricatorWorker::scheduleTask(
+ 'HarbormasterTargetWorker',
+ array(
+ 'targetID' => $target->getID(),
+ ));
+ }
+ }
+
+ private function updateBuild(HarbormasterBuild $build) {
+ // TODO: Handle cancellation and restarts.
+
+ if ($build->getBuildStatus() == HarbormasterBuild::STATUS_PENDING) {
+ $this->destroyBuildTargets($build);
+ $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING);
+ $build->save();
+ }
+
+ if ($build->getBuildStatus() == HarbormasterBuild::STATUS_BUILDING) {
+ return $this->updateBuildSteps($build);
+ }
+ }
+
+ private function destroyBuildTargets(HarbormasterBuild $build) {
+ $targets = id(new HarbormasterBuildTargetQuery())
+ ->setViewer($this->getViewer())
+ ->withBuildPHIDs(array($build->getPHID()))
+ ->execute();
+ foreach ($targets as $target) {
+ $target->delete();
+ }
+ }
+
+ private function updateBuildSteps(HarbormasterBuild $build) {
+ $targets = id(new HarbormasterBuildTargetQuery())
+ ->setViewer($this->getViewer())
+ ->withBuildPHIDs(array($build->getPHID()))
+ ->execute();
+ $targets = mgroup($targets, 'getBuildStepPHID');
+
+ $steps = id(new HarbormasterBuildStepQuery())
+ ->setViewer($this->getViewer())
+ ->withBuildPlanPHIDs(array($build->getBuildPlan()->getPHID()))
+ ->execute();
+
+ // Identify steps which are complete.
+
+ $complete = array();
+ $failed = array();
+ $waiting = array();
+ foreach ($steps as $step) {
+ $step_targets = idx($targets, $step->getPHID(), array());
+
+ if ($step_targets) {
+ $is_complete = true;
+ foreach ($step_targets as $target) {
+ // TODO: Move this to a top-level "status" field on BuildTarget.
+ if (!$target->getDetail('__done__')) {
+ $is_complete = false;
+ break;
+ }
+ }
+
+ $is_failed = false;
+ foreach ($step_targets as $target) {
+ // TODO: Move this to a top-level "status" field on BuildTarget.
+ if ($target->getDetail('__failed__')) {
+ $is_failed = true;
+ break;
+ }
+ }
+
+ $is_waiting = false;
+ } else {
+ $is_complete = false;
+ $is_failed = false;
+ $is_waiting = true;
+ }
+
+ if ($is_complete) {
+ $complete[$step->getPHID()] = true;
+ }
+
+ if ($is_failed) {
+ $failed[$step->getPHID()] = true;
+ }
+
+ if ($is_waiting) {
+ $waiting[$step->getPHID()] = true;
+ }
+ }
+
+ // If every step is complete, we're done with this build. Mark it passed
+ // and bail.
+ if (count($complete) == count($steps)) {
+ $build->setBuildStatus(HarbormasterBuild::STATUS_PASSED);
+ $build->save();
+ return;
+ }
+
+ // If any step failed, fail the whole build, then bail.
+ if (count($failed)) {
+ $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED);
+ $build->save();
+ return;
+ }
+
+ // Identify all the steps which are ready to run (because all their
+ // depdendencies are complete).
+
+ $previous_step = null;
+ $runnable = array();
+ foreach ($steps as $step) {
+ // TODO: For now, we're hard coding sequential dependencies into build
+ // steps. In the future, we can be smart about this instead.
+
+ if ($previous_step) {
+ $dependencies = array($previous_step);
+ } else {
+ $dependencies = array();
+ }
+
+ if (isset($waiting[$step->getPHID()])) {
+ $can_run = true;
+ foreach ($dependencies as $dependency) {
+ if (empty($complete[$dependency->getPHID()])) {
+ $can_run = false;
+ break;
+ }
+ }
+
+ if ($can_run) {
+ $runnable[] = $step;
+ }
+ }
+
+ $previous_step = $step;
+ }
+
+ if (!$runnable) {
+ // TODO: This means the build is deadlocked, probably? It should not
+ // normally be possible, but we should communicate it more clearly.
+ $build->setBuildStatus(HarbormasterBuild::STATUS_FAILED);
+ $build->save();
+ return;
+ }
+
+ foreach ($runnable as $runnable_step) {
+ $target = HarbormasterBuildTarget::initializeNewBuildTarget(
+ $build,
+ $step,
+ $build->retrieveVariablesFromBuild());
+ $target->save();
+
+ $this->queueNewBuildTarget($target);
+ }
+ }
+
+}
Index: src/applications/harbormaster/worker/HarbormasterBuildWorker.php
===================================================================
--- src/applications/harbormaster/worker/HarbormasterBuildWorker.php
+++ src/applications/harbormaster/worker/HarbormasterBuildWorker.php
@@ -1,22 +1,17 @@
<?php
/**
- * Run builds
+ * Start a build.
*/
-final class HarbormasterBuildWorker extends PhabricatorWorker {
-
- public function getRequiredLeaseTime() {
- return 60 * 60 * 24;
- }
+final class HarbormasterBuildWorker extends HarbormasterWorker {
public function doWork() {
$data = $this->getTaskData();
$id = idx($data, 'buildID');
+ $viewer = $this->getViewer();
- // Get a reference to the build.
$build = id(new HarbormasterBuildQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withBuildStatuses(array(HarbormasterBuild::STATUS_PENDING))
+ ->setViewer($viewer)
->withIDs(array($id))
->executeOne();
if (!$build) {
@@ -24,67 +19,10 @@
pht('Invalid build ID "%s".', $id));
}
- // It's possible for the user to request cancellation before
- // a worker picks up a build. We check to see if the build
- // is already cancelled, and return if it is.
- if ($build->checkForCancellation()) {
- return;
- }
-
- try {
- $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING);
- $build->save();
-
- $buildable = $build->getBuildable();
- $plan = $build->getBuildPlan();
-
- $steps = $plan->loadOrderedBuildSteps();
-
- // Perform the build.
- foreach ($steps as $step) {
-
- // Create the target at this step.
- // TODO: Support variable artifacts.
- $target = HarbormasterBuildTarget::initializeNewBuildTarget(
- $build,
- $step,
- $build->retrieveVariablesFromBuild());
- $target->save();
-
- $implementation = $target->getImplementation();
- if (!$implementation->validateSettings()) {
- $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR);
- break;
- }
- $implementation->execute($build, $target);
- if ($build->getBuildStatus() !== HarbormasterBuild::STATUS_BUILDING) {
- break;
- }
- if ($build->checkForCancellation()) {
- break;
- }
- }
-
- // Check to see if the user requested cancellation. If they did and
- // we get to here, they might have either cancelled too late, or the
- // step isn't cancellation aware. In either case we ignore the result
- // and move to a cancelled state.
- $build->checkForCancellation();
-
- // If we get to here, then the build has finished. Set it to passed
- // if no build step explicitly set the status.
- if ($build->getBuildStatus() === HarbormasterBuild::STATUS_BUILDING) {
- $build->setBuildStatus(HarbormasterBuild::STATUS_PASSED);
- }
- $build->save();
- } catch (Exception $e) {
- // If any exception is raised, the build is marked as a failure and
- // the exception is re-thrown (this ensures we don't leave builds
- // in an inconsistent state).
- $build->setBuildStatus(HarbormasterBuild::STATUS_ERROR);
- $build->save();
- throw $e;
- }
+ id(new HarbormasterBuildEngine())
+ ->setViewer($viewer)
+ ->setBuild($build)
+ ->continueBuild();
}
}
Index: src/applications/harbormaster/worker/HarbormasterTargetWorker.php
===================================================================
--- /dev/null
+++ src/applications/harbormaster/worker/HarbormasterTargetWorker.php
@@ -0,0 +1,59 @@
+<?php
+
+/**
+ * Execute a build target.
+ */
+final class HarbormasterTargetWorker extends HarbormasterWorker {
+
+ public function getRequiredLeaseTime() {
+ // This worker performs actual build work, which may involve a long wait
+ // on external systems.
+ return 60 * 60 * 24;
+ }
+
+ private function loadBuildTarget() {
+ $data = $this->getTaskData();
+ $id = idx($data, 'targetID');
+
+ $target = id(new HarbormasterBuildTargetQuery())
+ ->withIDs(array($id))
+ ->setViewer($this->getViewer())
+ ->executeOne();
+
+ if (!$target) {
+ throw new PhabricatorWorkerPermanentFailureException(
+ pht(
+ 'Bad build target ID "%d".',
+ $id));
+ }
+
+ return $target;
+ }
+
+ public function doWork() {
+ $target = $this->loadBuildTarget();
+ $build = $target->getBuild();
+ $viewer = $this->getViewer();
+
+ try {
+ $implementation = $target->getImplementation();
+ if (!$implementation->validateSettings()) {
+ $target->setDetail('__failed__', true);
+ $target->save();
+ } else {
+ $implementation->execute($build, $target);
+ $target->setDetail('__done__', true);
+ $target->save();
+ }
+ } catch (Exception $ex) {
+ $target->setDetail('__failed__', true);
+ $target->save();
+ }
+
+ id(new HarbormasterBuildEngine())
+ ->setViewer($viewer)
+ ->setBuild($build)
+ ->continueBuild();
+ }
+
+}
Index: src/applications/harbormaster/worker/HarbormasterWorker.php
===================================================================
--- /dev/null
+++ src/applications/harbormaster/worker/HarbormasterWorker.php
@@ -0,0 +1,9 @@
+<?php
+
+abstract class HarbormasterWorker extends PhabricatorWorker {
+
+ public function getViewer() {
+ return PhabricatorUser::getOmnipotentUser();
+ }
+
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Nov 25, 4:36 PM (17 h, 28 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6785938
Default Alt Text
D7890.diff (14 KB)

Event Timeline

Phabricator · Built by Phacility