@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Allow Harbormaster build targets to wait for messages
Summary:
This hooks up all the pieces of the build pipeline so `harbormaster.sendmessage` actually works. Particularly:
- Candidate build steps (i.e., those which interact with external systems) can now "Wait for Message". This pauses them indefinitely when they complete, until something calls `harbormaster.sendmessage`.
- After processing a target, we check if we should move it to PASSED or WAITING.
- Before updating a build, we move WAITING targets with pending messages to either PASSED or FAILED.
- I added an explicit "Building" state, which doesn't affect workflows but communicates more information to human users.
A big part of this is avoiding races. I believe we get the correct behavior no matter which order events occur in:
- We update builds after targets complete and after we receive messages, so we're guaranteed to update once both these conditions are true. This means messages can't be lost (even if they arrive before a build completes).
- The minor changes to the build engine logic mean that firing additional build updates is always safe, no matter what the current state of the build is.
- The build itself is protected by a lock in the build engine.
- The target is not covered by an explicit lock, but for all states only the engine (waiting) //or// the worker (all other states) can interact with it. All of the interactions also move the target state forward to the same destination and have no other side effects.
- Messages are only consumed inside the engine lock, so they don't need an explicit lock.
Test Plan:
- Made an HTTP request wait after completion, then ran a pile of builds through it using `bin/harbormaster build` and the web UI.
- Passed and failed message-awaiting builds with `harbormaster.sendmessage`.
Reviewers: btrahan
Reviewed By: btrahan
Subscribers: epriestley, zeeg
Differential Revision: https://secure.phabricator.com/D8788
+204
-16
+7
src/applications/harbormaster/conduit/ConduitAPI_harbormaster_sendmessage_Method.php
+7
src/applications/harbormaster/conduit/ConduitAPI_harbormaster_sendmessage_Method.php
···
43
43
->setType($message_type)
44
44
->save();
45
45
46
+
// If the build has completely paused because all steps are blocked on
47
+
// waiting targets, this will resume it.
48
+
id(new HarbormasterBuildEngine())
49
+
->setViewer($viewer)
50
+
->setBuild($build_target->getBuild())
51
+
->continueBuild();
52
+
46
53
return null;
47
54
}
48
55
+13
-1
src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
+13
-1
src/applications/harbormaster/controller/HarbormasterBuildableViewController.php
···
251
251
switch ($target->getTargetStatus()) {
252
252
case HarbormasterBuildTarget::STATUS_PENDING:
253
253
$icon = 'time-green';
254
+
$status_name = pht('Pending');
255
+
break;
256
+
case HarbormasterBuildTarget::STATUS_BUILDING:
257
+
$icon = 'right-green';
258
+
$status_name = pht('Building');
259
+
break;
260
+
case HarbormasterBuildTarget::STATUS_WAITING:
261
+
$icon = 'time-orange';
262
+
$status_name = pht('Waiting');
254
263
break;
255
264
case HarbormasterBuildTarget::STATUS_PASSED:
256
265
$icon = 'accept-green';
266
+
$status_name = pht('Passed');
257
267
break;
258
268
case HarbormasterBuildTarget::STATUS_FAILED:
259
269
$icon = 'reject-red';
270
+
$status_name = pht('Failed');
260
271
break;
261
272
default:
262
273
$icon = 'question';
274
+
$status_name = pht('Unknown');
263
275
break;
264
276
}
265
277
···
272
284
273
285
$target_list->addItem(
274
286
id(new PHUIStatusItemView())
275
-
->setIcon($icon)
287
+
->setIcon($icon, $status_name)
276
288
->setTarget(pht('Target %d', $target->getID()))
277
289
->setNote($name));
278
290
}
+35
-1
src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php
+35
-1
src/applications/harbormaster/customfield/HarbormasterBuildStepCoreCustomField.php
···
9
9
}
10
10
11
11
public function createFields($object) {
12
-
$specs = $object->getStepImplementation()->getFieldSpecifications();
12
+
$impl = $object->getStepImplementation();
13
+
$specs = $impl->getFieldSpecifications();
14
+
15
+
if ($impl->supportsWaitForMessage()) {
16
+
$specs['builtin.next-steps-header'] = array(
17
+
'type' => 'header',
18
+
'name' => pht('Next Steps'),
19
+
);
20
+
21
+
$specs['builtin.wait-for-message'] = array(
22
+
'type' => 'select',
23
+
'name' => pht('When Complete'),
24
+
'instructions' => pht(
25
+
'After completing this build step Harbormaster can continue the '.
26
+
'build normally, or it can pause the build and wait for a message. '.
27
+
'If you are using this build step to trigger some work in an '.
28
+
'external system, you may want to have Phabricator wait for that '.
29
+
'system to perform the work and report results back.'.
30
+
"\n\n".
31
+
'If you select **Continue Build Normally**, the build plan will '.
32
+
'proceed once this step finishes.'.
33
+
"\n\n".
34
+
'If you select **Wait For Message**, the build plan will pause '.
35
+
'indefinitely once this step finishes. To resume the build, an '.
36
+
'external system must call `harbormaster.sendmessage` with the '.
37
+
'build target PHID, and either `"pass"` or `"fail"` to indicate '.
38
+
'the result for this step. After the result is recorded, the build '.
39
+
'plan will resume.'),
40
+
'options' => array(
41
+
'' => pht('Continue Build Normally'),
42
+
'wait' => pht('Wait For Message'),
43
+
),
44
+
);
45
+
}
46
+
13
47
return PhabricatorStandardCustomField::buildStandardFields($this, $specs);
14
48
}
15
49
+97
-12
src/applications/harbormaster/engine/HarbormasterBuildEngine.php
+97
-12
src/applications/harbormaster/engine/HarbormasterBuildEngine.php
···
127
127
->setViewer($this->getViewer())
128
128
->withBuildPHIDs(array($build->getPHID()))
129
129
->execute();
130
+
131
+
$this->updateWaitingTargets($targets);
132
+
130
133
$targets = mgroup($targets, 'getBuildStepPHID');
131
134
132
135
$steps = id(new HarbormasterBuildStepQuery())
···
134
137
->withBuildPlanPHIDs(array($build->getBuildPlan()->getPHID()))
135
138
->execute();
136
139
137
-
// Identify steps which are complete.
140
+
// Identify steps which are in various states.
138
141
142
+
$queued = array();
143
+
$underway = array();
144
+
$waiting = array();
139
145
$complete = array();
140
146
$failed = array();
141
-
$waiting = array();
142
147
foreach ($steps as $step) {
143
148
$step_targets = idx($targets, $step->getPHID(), array());
144
149
145
150
if ($step_targets) {
151
+
$is_queued = false;
152
+
153
+
$is_underway = false;
154
+
foreach ($step_targets as $target) {
155
+
if ($target->isUnderway()) {
156
+
$is_underway = true;
157
+
break;
158
+
}
159
+
}
160
+
161
+
$is_waiting = false;
162
+
foreach ($step_targets as $target) {
163
+
if ($target->isWaiting()) {
164
+
$is_waiting = true;
165
+
break;
166
+
}
167
+
}
168
+
146
169
$is_complete = true;
147
170
foreach ($step_targets as $target) {
148
171
if (!$target->isComplete()) {
···
158
181
break;
159
182
}
160
183
}
161
-
184
+
} else {
185
+
$is_queued = true;
186
+
$is_underway = false;
162
187
$is_waiting = false;
163
-
} else {
164
188
$is_complete = false;
165
189
$is_failed = false;
166
-
$is_waiting = true;
190
+
}
191
+
192
+
if ($is_queued) {
193
+
$queued[$step->getPHID()] = true;
194
+
}
195
+
196
+
if ($is_underway) {
197
+
$underway[$step->getPHID()] = true;
198
+
}
199
+
200
+
if ($is_waiting) {
201
+
$waiting[$step->getPHID()] = true;
167
202
}
168
203
169
204
if ($is_complete) {
···
172
207
173
208
if ($is_failed) {
174
209
$failed[$step->getPHID()] = true;
175
-
}
176
-
177
-
if ($is_waiting) {
178
-
$waiting[$step->getPHID()] = true;
179
210
}
180
211
}
181
212
···
209
240
$dependencies = array();
210
241
}
211
242
212
-
if (isset($waiting[$step->getPHID()])) {
243
+
if (isset($queued[$step->getPHID()])) {
213
244
$can_run = true;
214
245
foreach ($dependencies as $dependency) {
215
246
if (empty($complete[$dependency->getPHID()])) {
···
226
257
$previous_step = $step;
227
258
}
228
259
229
-
if (!$runnable) {
260
+
if (!$runnable && !$waiting && !$underway) {
230
261
// TODO: This means the build is deadlocked, probably? It should not
231
-
// normally be possible, but we should communicate it more clearly.
262
+
// normally be possible yet, but we should communicate it more clearly.
232
263
$build->setBuildStatus(HarbormasterBuild::STATUS_FAILED);
233
264
$build->save();
234
265
return;
···
242
273
$target->save();
243
274
244
275
$this->queueNewBuildTarget($target);
276
+
}
277
+
278
+
}
279
+
280
+
281
+
/**
282
+
* Process messages which were sent to these targets, kicking applicable
283
+
* targets out of "Waiting" and into either "Passed" or "Failed".
284
+
*
285
+
* @param list<HarbormasterBuildTarget> List of targets to process.
286
+
* @return void
287
+
*/
288
+
private function updateWaitingTargets(array $targets) {
289
+
assert_instances_of($targets, 'HarbormasterBuildTarget');
290
+
291
+
// We only care about messages for targets which are actually in a waiting
292
+
// state.
293
+
$waiting_targets = array();
294
+
foreach ($targets as $target) {
295
+
if ($target->isWaiting()) {
296
+
$waiting_targets[$target->getPHID()] = $target;
297
+
}
298
+
}
299
+
300
+
if (!$waiting_targets) {
301
+
return;
302
+
}
303
+
304
+
$messages = id(new HarbormasterBuildMessageQuery())
305
+
->setViewer($this->getViewer())
306
+
->withBuildTargetPHIDs(array_keys($waiting_targets))
307
+
->withConsumed(false)
308
+
->execute();
309
+
310
+
foreach ($messages as $message) {
311
+
$target = $waiting_targets[$message->getBuildTargetPHID()];
312
+
313
+
$new_status = null;
314
+
switch ($message->getType()) {
315
+
case 'pass':
316
+
$new_status = HarbormasterBuildTarget::STATUS_PASSED;
317
+
break;
318
+
case 'fail':
319
+
$new_status = HarbormasterBuildTarget::STATUS_FAILED;
320
+
break;
321
+
}
322
+
323
+
if ($new_status !== null) {
324
+
$message->setIsConsumed(true);
325
+
$message->save();
326
+
327
+
$target->setTargetStatus($new_status);
328
+
$target->save();
329
+
}
245
330
}
246
331
}
247
332
+1
-1
src/applications/harbormaster/query/HarbormasterBuildMessageQuery.php
+1
-1
src/applications/harbormaster/query/HarbormasterBuildMessageQuery.php
+12
src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
+12
src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php
···
196
196
}
197
197
}
198
198
199
+
public function supportsWaitForMessage() {
200
+
return false;
201
+
}
202
+
203
+
public function shouldWaitForMessage(HarbormasterBuildTarget $target) {
204
+
if (!$this->supportsWaitForMessage()) {
205
+
return false;
206
+
}
207
+
208
+
return (bool)$target->getDetail('builtin.wait-for-message');
209
+
}
210
+
199
211
}
+4
src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
+4
src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php
+22
src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
+22
src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php
···
11
11
protected $targetStatus;
12
12
13
13
const STATUS_PENDING = 'target/pending';
14
+
const STATUS_BUILDING = 'target/building';
15
+
const STATUS_WAITING = 'target/waiting';
14
16
const STATUS_PASSED = 'target/passed';
15
17
const STATUS_FAILED = 'target/failed';
16
18
···
121
123
public function isFailed() {
122
124
switch ($this->getTargetStatus()) {
123
125
case self::STATUS_FAILED:
126
+
return true;
127
+
}
128
+
129
+
return false;
130
+
}
131
+
132
+
133
+
public function isWaiting() {
134
+
switch ($this->getTargetStatus()) {
135
+
case self::STATUS_WAITING:
136
+
return true;
137
+
}
138
+
139
+
return false;
140
+
}
141
+
142
+
public function isUnderway() {
143
+
switch ($this->getTargetStatus()) {
144
+
case self::STATUS_PENDING:
145
+
case self::STATUS_BUILDING:
124
146
return true;
125
147
}
126
148
+13
-1
src/applications/harbormaster/worker/HarbormasterTargetWorker.php
+13
-1
src/applications/harbormaster/worker/HarbormasterTargetWorker.php
···
36
36
$viewer = $this->getViewer();
37
37
38
38
try {
39
+
$status_pending = HarbormasterBuildTarget::STATUS_PENDING;
40
+
if ($target->getTargetStatus() == $status_pending) {
41
+
$target->setTargetStatus(HarbormasterBuildTarget::STATUS_BUILDING);
42
+
$target->save();
43
+
}
44
+
39
45
$implementation = $target->getImplementation();
40
46
$implementation->execute($build, $target);
41
-
$target->setTargetStatus(HarbormasterBuildTarget::STATUS_PASSED);
47
+
48
+
$next_status = HarbormasterBuildTarget::STATUS_PASSED;
49
+
if ($implementation->shouldWaitForMessage($target)) {
50
+
$next_status = HarbormasterBuildTarget::STATUS_WAITING;
51
+
}
52
+
53
+
$target->setTargetStatus($next_status);
42
54
$target->save();
43
55
} catch (Exception $ex) {
44
56
phlog($ex);