From efadfbbc97f9f42804c9ec0adac9a39a75d7ec1a Mon Sep 17 00:00:00 2001 From: James Rhodes Date: Thu, 21 Aug 2014 22:55:24 +1000 Subject: [PATCH] Implement build generations in Harbormaster Summary: Ref T5932. Ref T5936. This implements build generations in Harbormaster, which provides the infrastructure required to both show users the previous states of restarted builds and to allow users to forcefully abort builds (and their targets). You can view previous generations of a build by adding `?g=` to the URI, but this isn't exposed in the UI anywhere yet. Test Plan: Ran a build plan with a Sleep step in it. Reconfigured it for various sleep times and viewed previous generations of the build after restarting it. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley, Korvin Maniphest Tasks: T5932, T5936 Differential Revision: https://secure.phabricator.com/D10321 --- .../20140821.harbormasterbuildgen.1.sql | 5 +++ .../HarbormasterBuildViewController.php | 13 ++++++- .../engine/HarbormasterBuildEngine.php | 37 +++++++------------ .../query/HarbormasterBuildQuery.php | 7 ++++ .../query/HarbormasterBuildTargetQuery.php | 13 +++++++ .../storage/build/HarbormasterBuild.php | 1 + .../storage/build/HarbormasterBuildTarget.php | 4 +- 7 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 resources/sql/autopatches/20140821.harbormasterbuildgen.1.sql diff --git a/resources/sql/autopatches/20140821.harbormasterbuildgen.1.sql b/resources/sql/autopatches/20140821.harbormasterbuildgen.1.sql new file mode 100644 index 0000000000..a687146f60 --- /dev/null +++ b/resources/sql/autopatches/20140821.harbormasterbuildgen.1.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_build + ADD COLUMN `buildGeneration` INT UNSIGNED NOT NULL DEFAULT 0; + +ALTER TABLE {$NAMESPACE}_harbormaster.harbormaster_buildtarget + ADD COLUMN `buildGeneration` INT UNSIGNED NOT NULL DEFAULT 0; diff --git a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php index 63c5685a45..3ec0d9b1d8 100644 --- a/src/applications/harbormaster/controller/HarbormasterBuildViewController.php +++ b/src/applications/harbormaster/controller/HarbormasterBuildViewController.php @@ -14,6 +14,7 @@ final class HarbormasterBuildViewController $viewer = $request->getUser(); $id = $this->id; + $generation = $request->getInt('g'); $build = id(new HarbormasterBuildQuery()) ->setViewer($viewer) @@ -52,13 +53,18 @@ final class HarbormasterBuildViewController '/'.$build->getBuildable()->getMonogram()); $crumbs->addTextCrumb($title); + if ($generation === null || $generation > $build->getBuildGeneration() || + $generation < 0) { + $generation = $build->getBuildGeneration(); + } + $build_targets = id(new HarbormasterBuildTargetQuery()) ->setViewer($viewer) ->needBuildSteps(true) ->withBuildPHIDs(array($build->getPHID())) + ->withBuildGenerations(array($generation)) ->execute(); - if ($build_targets) { $messages = id(new HarbormasterBuildMessageQuery()) ->setViewer($viewer) @@ -434,10 +440,13 @@ final class HarbormasterBuildViewController pht('Build Plan'), $handles[$build->getBuildPlanPHID()]->renderLink()); + $properties->addProperty( + pht('Restarts'), + $build->getBuildGeneration()); + $properties->addProperty( pht('Status'), $this->getStatus($build)); - } private function getStatus(HarbormasterBuild $build) { diff --git a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php index ee2b0acfef..a3faf45314 100644 --- a/src/applications/harbormaster/engine/HarbormasterBuildEngine.php +++ b/src/applications/harbormaster/engine/HarbormasterBuildEngine.php @@ -100,7 +100,7 @@ final class HarbormasterBuildEngine extends Phobject { private function updateBuild(HarbormasterBuild $build) { if (($build->getBuildStatus() == HarbormasterBuild::STATUS_PENDING) || ($build->isRestarting())) { - $this->destroyBuildTargets($build); + $this->restartBuild($build); $build->setBuildStatus(HarbormasterBuild::STATUS_BUILDING); $build->save(); } @@ -122,38 +122,28 @@ final class HarbormasterBuildEngine extends Phobject { } } - private function destroyBuildTargets(HarbormasterBuild $build) { + private function restartBuild(HarbormasterBuild $build) { + + // We're restarting the build, so release all previous artifacts. $this->releaseAllArtifacts($build); - $targets = id(new HarbormasterBuildTargetQuery()) - ->setViewer($this->getViewer()) - ->withBuildPHIDs(array($build->getPHID())) - ->execute(); + // Increment the build generation counter on the build. + $build->setBuildGeneration($build->getBuildGeneration() + 1); - if (!$targets) { - return; - } + // TODO: 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 + // and abort. - $target_phids = mpull($targets, 'getPHID'); - - $artifacts = id(new HarbormasterBuildArtifactQuery()) - ->setViewer($this->getViewer()) - ->withBuildTargetPHIDs($target_phids) - ->execute(); - - foreach ($artifacts as $artifact) { - $artifact->delete(); - } - - foreach ($targets as $target) { - $target->delete(); - } + // Previously we used to delete targets, logs and artifacts here. Instead + // leave them around so users can view previous generations of this build. } private function updateBuildSteps(HarbormasterBuild $build) { $targets = id(new HarbormasterBuildTargetQuery()) ->setViewer($this->getViewer()) ->withBuildPHIDs(array($build->getPHID())) + ->withBuildGenerations(array($build->getBuildGeneration())) ->execute(); $this->updateWaitingTargets($targets); @@ -454,6 +444,7 @@ final class HarbormasterBuildEngine extends Phobject { $targets = id(new HarbormasterBuildTargetQuery()) ->setViewer(PhabricatorUser::getOmnipotentUser()) ->withBuildPHIDs(array($build->getPHID())) + ->withBuildGenerations(array($build->getBuildGeneration())) ->execute(); if (count($targets) === 0) { diff --git a/src/applications/harbormaster/query/HarbormasterBuildQuery.php b/src/applications/harbormaster/query/HarbormasterBuildQuery.php index bfa18f14c3..0daf5ac906 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildQuery.php @@ -122,6 +122,13 @@ final class HarbormasterBuildQuery $targets = mgroup($targets, 'getBuildPHID'); foreach ($page as $build) { $build_targets = idx($targets, $build->getPHID(), array()); + + foreach ($build_targets as $phid => $target) { + if ($target->getBuildGeneration() !== $build->getBuildGeneration()) { + unset($build_targets[$phid]); + } + } + $build->attachBuildTargets($build_targets); } } diff --git a/src/applications/harbormaster/query/HarbormasterBuildTargetQuery.php b/src/applications/harbormaster/query/HarbormasterBuildTargetQuery.php index 88c51f5e36..133bb6c0e2 100644 --- a/src/applications/harbormaster/query/HarbormasterBuildTargetQuery.php +++ b/src/applications/harbormaster/query/HarbormasterBuildTargetQuery.php @@ -6,6 +6,7 @@ final class HarbormasterBuildTargetQuery private $ids; private $phids; private $buildPHIDs; + private $buildGenerations; private $needBuildSteps; public function withIDs(array $ids) { @@ -23,6 +24,11 @@ final class HarbormasterBuildTargetQuery return $this; } + public function withBuildGenerations(array $build_generations) { + $this->buildGenerations = $build_generations; + return $this; + } + public function needBuildSteps($need_build_steps) { $this->needBuildSteps = $need_build_steps; return $this; @@ -67,6 +73,13 @@ final class HarbormasterBuildTargetQuery $this->buildPHIDs); } + if ($this->buildGenerations) { + $where[] = qsprintf( + $conn_r, + 'buildGeneration in (%Ld)', + $this->buildGenerations); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuild.php b/src/applications/harbormaster/storage/build/HarbormasterBuild.php index 3395052efa..d90cac554f 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuild.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuild.php @@ -6,6 +6,7 @@ final class HarbormasterBuild extends HarbormasterDAO protected $buildablePHID; protected $buildPlanPHID; protected $buildStatus; + protected $buildGeneration; private $buildable = self::ATTACHABLE; private $buildPlan = self::ATTACHABLE; diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php index e07d219f88..3271903d70 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php @@ -12,6 +12,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO protected $targetStatus; protected $dateStarted; protected $dateCompleted; + protected $buildGeneration; const STATUS_PENDING = 'target/pending'; const STATUS_BUILDING = 'target/building'; @@ -82,7 +83,8 @@ final class HarbormasterBuildTarget extends HarbormasterDAO ->setClassName($build_step->getClassName()) ->setDetails($build_step->getDetails()) ->setTargetStatus(self::STATUS_PENDING) - ->setVariables($variables); + ->setVariables($variables) + ->setBuildGeneration($build->getBuildGeneration()); } public function getConfiguration() {