Skip to content

Commit 0ef599e

Browse files
author
epriestley
committed
Give Buildables a status, populate it, and return it over Conduit
Summary: Ref T4809. Currently, buildables have a status field but nothing populates it. Populate it: - When builds change state, update the Buildable state. - Use the new Buildable state on the web UI. - Return the new Buildable state from Conduit. To make it easier to debug/test this: - Provide `bin/harbormaster update Bxxx ...` to force foreground update of a Buildable. Test Plan: - Used `bin/harbormaster update Bxxx --force --trace` to update buildables. - Looked at buidlable list, saw statuses reported properly. - Used Conduit to read statuses. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4809 Differential Revision: https://secure.phabricator.com/D8799
1 parent 4918773 commit 0ef599e

9 files changed

+219
-31
lines changed

‎src/__phutil_library_map__.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -744,6 +744,7 @@
744744
'HarbormasterHTTPRequestBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php',
745745
'HarbormasterLeaseHostBuildStepImplementation' => 'applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php',
746746
'HarbormasterManagementBuildWorkflow' => 'applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php',
747+
'HarbormasterManagementUpdateWorkflow' => 'applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php',
747748
'HarbormasterManagementWorkflow' => 'applications/harbormaster/management/HarbormasterManagementWorkflow.php',
748749
'HarbormasterObject' => 'applications/harbormaster/storage/HarbormasterObject.php',
749750
'HarbormasterPHIDTypeBuild' => 'applications/harbormaster/phid/HarbormasterPHIDTypeBuild.php',
@@ -3411,6 +3412,7 @@
34113412
'HarbormasterHTTPRequestBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
34123413
'HarbormasterLeaseHostBuildStepImplementation' => 'HarbormasterBuildStepImplementation',
34133414
'HarbormasterManagementBuildWorkflow' => 'HarbormasterManagementWorkflow',
3415+
'HarbormasterManagementUpdateWorkflow' => 'HarbormasterManagementWorkflow',
34143416
'HarbormasterManagementWorkflow' => 'PhabricatorManagementWorkflow',
34153417
'HarbormasterObject' => 'HarbormasterDAO',
34163418
'HarbormasterPHIDTypeBuild' => 'PhabricatorPHIDType',

‎src/applications/harbormaster/conduit/ConduitAPI_harbormaster_querybuildables_Method.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,16 @@ protected function execute(ConduitAPIRequest $request) {
6464
foreach ($buildables as $buildable) {
6565
$monogram = $buildable->getMonogram();
6666

67+
$status = $buildable->getBuildableStatus();
68+
$status_name = HarbormasterBuildable::getBuildableStatusName($status);
69+
6770
$data[] = array(
6871
'id' => $buildable->getID(),
6972
'phid' => $buildable->getPHID(),
7073
'monogram' => $monogram,
7174
'uri' => PhabricatorEnv::getProductionURI('/'.$monogram),
75+
'buildableStatus' => $status,
76+
'buildableStatusName' => $status_name,
7277
'buildablePHID' => $buildable->getBuildablePHID(),
7378
'containerPHID' => $buildable->getContainerPHID(),
7479
'isManualBuildable' => (bool)$buildable->getIsManualBuildable(),

‎src/applications/harbormaster/conduit/ConduitAPI_harbormaster_sendmessage_Method.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,11 @@ protected function execute(ConduitAPIRequest $request) {
4545

4646
// If the build has completely paused because all steps are blocked on
4747
// waiting targets, this will resume it.
48-
id(new HarbormasterBuildEngine())
49-
->setViewer($viewer)
50-
->setBuild($build_target->getBuild())
51-
->continueBuild();
48+
PhabricatorWorker::scheduleTask(
49+
'HarbormasterBuildWorker',
50+
array(
51+
'buildID' => $build_target->getBuild()->getID(),
52+
));
5253

5354
return null;
5455
}

‎src/applications/harbormaster/controller/HarbormasterBuildableListController.php‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -54,28 +54,13 @@ public function renderResultsList(
5454

5555
$list->addItem($item);
5656

57-
58-
59-
// TODO: This is proof-of-concept for getting meaningful status
60-
// information into this list, and should get an improvement pass
61-
// once we're a little farther along.
62-
63-
$all_pass = true;
64-
$any_fail = false;
65-
foreach ($buildable->getBuilds() as $build) {
66-
if ($build->getBuildStatus() != HarbormasterBuild::STATUS_PASSED) {
67-
$all_pass = false;
68-
}
69-
if ($build->getBuildStatus() == HarbormasterBuild::STATUS_FAILED ||
70-
$build->getBuildStatus() == HarbormasterBuild::STATUS_ERROR) {
71-
$any_fail = true;
72-
}
73-
}
74-
75-
if ($any_fail) {
76-
$item->setBarColor('red');
77-
} else if ($all_pass) {
78-
$item->setBarColor('green');
57+
switch ($buildable->getBuildableStatus()) {
58+
case HarbormasterBuildable::STATUS_PASSED:
59+
$item->setBarColor('green');
60+
break;
61+
case HarbormasterBuildable::STATUS_FAILED:
62+
$item->setBarColor('red');
63+
break;
7964
}
8065
}
8166

‎src/applications/harbormaster/engine/HarbormasterBuildEngine.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,16 @@ final class HarbormasterBuildEngine extends Phobject {
99
private $build;
1010
private $viewer;
1111
private $newBuildTargets = array();
12+
private $forceBuildableUpdate;
13+
14+
public function setForceBuildableUpdate($force_buildable_update) {
15+
$this->forceBuildableUpdate = $force_buildable_update;
16+
return $this;
17+
}
18+
19+
public function shouldForceBuildableUpdate() {
20+
return $this->forceBuildableUpdate;
21+
}
1222

1323
public function queueNewBuildTarget(HarbormasterBuildTarget $target) {
1424
$this->newBuildTargets[] = $target;
@@ -44,6 +54,7 @@ public function continueBuild() {
4454
$lock = PhabricatorGlobalLock::newLock($lock_key)->lock(15);
4555

4656
$build->reload();
57+
$old_status = $build->getBuildStatus();
4758

4859
try {
4960
$this->updateBuild($build);
@@ -69,6 +80,13 @@ public function continueBuild() {
6980
'targetID' => $target->getID(),
7081
));
7182
}
83+
84+
// If the build changed status, we might need to update the overall status
85+
// on the buildable.
86+
$new_status = $build->getBuildStatus();
87+
if ($new_status != $old_status || $this->shouldForceBuildableUpdate()) {
88+
$this->updateBuildable($build->getBuildable());
89+
}
7290
}
7391

7492
private function updateBuild(HarbormasterBuild $build) {
@@ -330,4 +348,53 @@ private function updateWaitingTargets(array $targets) {
330348
}
331349
}
332350

351+
352+
/**
353+
* Update the overall status of the buildable this build is attached to.
354+
*
355+
* After a build changes state (for example, passes or fails) it may affect
356+
* the overall state of the associated buildable. Compute the new aggregate
357+
* state and save it on the buildable.
358+
*
359+
* @param HarbormasterBuild The buildable to update.
360+
* @return void
361+
*/
362+
private function updateBuildable(HarbormasterBuildable $buildable) {
363+
$lock_key = 'harbormaster.buildable:'.$buildable->getID();
364+
$lock = PhabricatorGlobalLock::newLock($lock_key)->lock(15);
365+
366+
$buildable = id(new HarbormasterBuildableQuery())
367+
->setViewer($this->getViewer())
368+
->withIDs(array($buildable->getID()))
369+
->needBuilds(true)
370+
->executeOne();
371+
372+
$all_pass = true;
373+
$any_fail = false;
374+
foreach ($buildable->getBuilds() as $build) {
375+
if ($build->getBuildStatus() != HarbormasterBuild::STATUS_PASSED) {
376+
$all_pass = false;
377+
}
378+
if ($build->getBuildStatus() == HarbormasterBuild::STATUS_FAILED ||
379+
$build->getBuildStatus() == HarbormasterBuild::STATUS_ERROR) {
380+
$any_fail = true;
381+
}
382+
}
383+
384+
if ($any_fail) {
385+
$new_status = HarbormasterBuildable::STATUS_FAILED;
386+
} else if ($all_pass) {
387+
$new_status = HarbormasterBuildable::STATUS_PASSED;
388+
} else {
389+
$new_status = HarbormasterBuildable::STATUS_BUILDING;
390+
}
391+
392+
if ($buildable->getBuildableStatus() != $new_status) {
393+
$buildable->setBuildableStatus($new_status);
394+
$buildable->save();
395+
}
396+
397+
$lock->unlock();
398+
}
399+
333400
}

‎src/applications/harbormaster/management/HarbormasterManagementBuildWorkflow.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function execute(PhutilArgumentParser $args) {
2828
$names = $args->getArg('buildable');
2929
if (count($names) != 1) {
3030
throw new PhutilArgumentUsageException(
31-
pht('Specify exactly one buildable, by object name.'));
31+
pht('Specify exactly one buildable object, by object name.'));
3232
}
3333

3434
$name = head($names);

‎src/applications/harbormaster/management/HarbormasterManagementUpdateWorkflow.php‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
<?php
2+
3+
final class HarbormasterManagementUpdateWorkflow
4+
extends HarbormasterManagementWorkflow {
5+
6+
public function didConstruct() {
7+
$this
8+
->setName('update')
9+
->setExamples('**update** [__options__] __buildable__')
10+
->setSynopsis(pht('Explicitly update the builds for __buildable__.'))
11+
->setArguments(
12+
array(
13+
array(
14+
'name' => 'build',
15+
'param' => 'id',
16+
'help' => pht('Update only this build.'),
17+
),
18+
array(
19+
'name' => 'force',
20+
'help' => pht(
21+
'Force the buildable to update even if no build status '.
22+
'changes occur during normal update.'),
23+
),
24+
array(
25+
'name' => 'background',
26+
'help' => pht(
27+
'If updating generates tasks, queue them for the daemons '.
28+
'instead of executing them in this process.'),
29+
),
30+
array(
31+
'name' => 'buildable',
32+
'wildcard' => true,
33+
),
34+
));
35+
}
36+
37+
public function execute(PhutilArgumentParser $args) {
38+
$viewer = $this->getViewer();
39+
40+
$force_update = $args->getArg('force');
41+
42+
$names = $args->getArg('buildable');
43+
if (count($names) != 1) {
44+
throw new PhutilArgumentUsageException(
45+
pht('Specify exactly one buildable, by object name.'));
46+
}
47+
48+
$buildable = id(new PhabricatorObjectQuery())
49+
->setViewer($viewer)
50+
->withNames($names)
51+
->executeOne();
52+
53+
if (!$buildable) {
54+
throw new PhutilArgumentUsageException(
55+
pht('No such buildable "%s"!', head($names)));
56+
}
57+
58+
if (!($buildable instanceof HarbormasterBuildable)) {
59+
throw new PhutilArgumentUsageException(
60+
pht('Object "%s" is not a Harbormaster Buildable!', head($names)));
61+
}
62+
63+
// Reload the buildable directly to get builds.
64+
$buildable = id(new HarbormasterBuildableQuery())
65+
->setViewer($viewer)
66+
->withIDs(array($buildable->getID()))
67+
->needBuilds(true)
68+
->executeOne();
69+
70+
$builds = $buildable->getBuilds();
71+
$builds = mpull($builds, null, 'getID');
72+
73+
$build_id = $args->getArg('build');
74+
if ($build_id) {
75+
$builds = array_select_keys($builds, array($build_id));
76+
if (!$builds) {
77+
throw new PhutilArgumentUsageException(
78+
pht(
79+
'The specified buildable does not have a build with ID "%s".',
80+
$build_id));
81+
}
82+
}
83+
84+
$console = PhutilConsole::getConsole();
85+
86+
if (!$args->getArg('background')) {
87+
PhabricatorWorker::setRunAllTasksInProcess(true);
88+
}
89+
90+
foreach ($builds as $build) {
91+
$console->writeOut(
92+
"%s\n",
93+
pht(
94+
'Updating build %d of buildable %s...',
95+
$build->getID(),
96+
$buildable->getMonogram()));
97+
98+
$engine = id(new HarbormasterBuildEngine())
99+
->setViewer($viewer)
100+
->setBuild($build);
101+
102+
if ($force_update) {
103+
$engine->setForceBuildableUpdate(true);
104+
}
105+
106+
$engine->continueBuild();
107+
}
108+
109+
$console->writeOut("%s\n", pht('Done.'));
110+
111+
return 0;
112+
}
113+
114+
}

‎src/applications/harbormaster/query/HarbormasterBuildableSearchEngine.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,7 @@ public function buildSavedQueryFromRequest(AphrontRequest $request) {
5252
public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
5353
$query = id(new HarbormasterBuildableQuery())
5454
->needContainerHandles(true)
55-
->needBuildableHandles(true)
56-
->needBuilds(true);
55+
->needBuildableHandles(true);
5756

5857
$container_phids = $saved->getParameter('containerPHIDs', array());
5958
if ($container_phids) {

‎src/applications/harbormaster/storage/HarbormasterBuildable.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,27 @@ final class HarbormasterBuildable extends HarbormasterDAO
1616
private $containerHandle = self::ATTACHABLE;
1717
private $builds = self::ATTACHABLE;
1818

19-
const STATUS_WHATEVER = 'whatever';
19+
const STATUS_BUILDING = 'building';
20+
const STATUS_PASSED = 'passed';
21+
const STATUS_FAILED = 'failed';
22+
23+
public static function getBuildableStatusName($status) {
24+
switch ($status) {
25+
case self::STATUS_BUILDING:
26+
return pht('Building');
27+
case self::STATUS_PASSED:
28+
return pht('Passed');
29+
case self::STATUS_FAILED:
30+
return pht('Failed');
31+
default:
32+
return pht('Unknown');
33+
}
34+
}
2035

2136
public static function initializeNewBuildable(PhabricatorUser $actor) {
2237
return id(new HarbormasterBuildable())
2338
->setIsManualBuildable(0)
24-
->setBuildableStatus(self::STATUS_WHATEVER);
39+
->setBuildableStatus(self::STATUS_BUILDING);
2540
}
2641

2742
public function getMonogram() {

0 commit comments

Comments
 (0)