Skip to content

Commit 07028cf

Browse files
author
epriestley
committed
When bin/drydock lease is interrupted, release leases
Summary: Depends on D19072. Ref T13073. Currently, you can leave leases stranded by using `^C` to interrupt the script. Handle signals and release leases on destruction if they haven't activated yet. Also, print out more useful information before and after activation. Test Plan: Mashed ^C while runnning `bin/drydock lease ... --trace`, saw the lease release. Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13073 Differential Revision: https://secure.phabricator.com/D19073
1 parent b833e32 commit 07028cf

File tree

6 files changed

+54
-8
lines changed

6 files changed

+54
-8
lines changed

scripts/drydock/drydock_control.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<?php
33

44
$root = dirname(dirname(dirname(__FILE__)));
5-
require_once $root.'/scripts/__init_script__.php';
5+
require_once $root.'/scripts/init/init-script-with-signals.php';
66

77
$args = new PhutilArgumentParser($argv);
88
$args->setTagline(pht('manage drydock software resources'));
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
3+
// Initialize a script that will handle signals.
4+
5+
if (function_exists('pcntl_async_signals')) {
6+
pcntl_async_signals(true);
7+
} else {
8+
declare(ticks = 1);
9+
}
10+
11+
require_once dirname(__FILE__).'/init-script.php';

src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ public function destroyResource(
206206
}
207207

208208
// Destroy the lease on the host.
209-
$lease->releaseOnDestruction();
209+
$lease->setReleaseOnDestruction(true);
210210

211211
if ($lease->isActive()) {
212212
// Destroy the working copy on disk.

src/applications/drydock/management/DrydockManagementLeaseWorkflow.php

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,51 @@ public function execute(PhutilArgumentParser $args) {
8484
$lease->setUntil($until);
8585
}
8686

87+
// If something fatals or the user interrupts the process (for example,
88+
// with "^C"), release the lease. We'll cancel this below, if the lease
89+
// actually activates.
90+
$lease->setReleaseOnDestruction(true);
91+
92+
// TODO: This would probably be better handled with PhutilSignalRouter,
93+
// but it currently doesn't route SIGINT. We're initializing it to setup
94+
// SIGTERM handling and make eventual migration easier.
95+
$router = PhutilSignalRouter::getRouter();
96+
pcntl_signal(SIGINT, array($this, 'didReceiveInterrupt'));
97+
98+
$t_start = microtime(true);
8799
$lease->queueForActivation();
88100

89101
echo tsprintf(
90-
"%s\n",
102+
"%s\n\n __%s__\n\n%s\n",
103+
pht('Queued lease for activation:'),
104+
PhabricatorEnv::getProductionURI($lease->getURI()),
91105
pht('Waiting for daemons to activate lease...'));
92106

93107
$this->waitUntilActive($lease);
94108

109+
// Now that we've survived activation and the lease is good, make it
110+
// durable.
111+
$lease->setReleaseOnDestruction(false);
112+
$t_end = microtime(true);
113+
95114
echo tsprintf(
96-
"%s\n",
97-
pht('Activated lease "%s".', $lease->getID()));
115+
"%s\n\n %s\n\n%s\n",
116+
pht(
117+
'Activation complete. This lease is permanent until manually '.
118+
'released with:'),
119+
pht('$ ./bin/drydock release-lease --id %d', $lease->getID()),
120+
pht(
121+
'Lease activated in %sms.',
122+
new PhutilNumber((int)(($t_end - $t_start) * 1000))));
98123

99124
return 0;
100125
}
101126

127+
public function didReceiveInterrupt($signo) {
128+
// Doing this makes us run destructors, particularly the "release on
129+
// destruction" trigger on the lease.
130+
exit(128 + $signo);
131+
}
102132

103133
private function waitUntilActive(DrydockLease $lease) {
104134
$viewer = $this->getViewer();

src/applications/drydock/storage/DrydockLease.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ public static function initializeNewLease() {
3636
* a lease, as you don't need to explicitly handle exceptions to properly
3737
* release the lease.
3838
*/
39-
public function releaseOnDestruction() {
40-
$this->releaseOnDestruction = true;
39+
public function setReleaseOnDestruction($release) {
40+
$this->releaseOnDestruction = $release;
4141
return $this;
4242
}
4343

@@ -436,6 +436,11 @@ public function awakenTasks() {
436436
return $this;
437437
}
438438

439+
public function getURI() {
440+
$id = $this->getID();
441+
return "/drydock/lease/{$id}/";
442+
}
443+
439444

440445
/* -( PhabricatorPolicyInterface )----------------------------------------- */
441446

src/applications/drydock/worker/DrydockRepositoryOperationUpdateWorker.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ private function handleUpdate(DrydockRepositoryOperation $operation) {
6262
DrydockCommandInterface::INTERFACE_TYPE);
6363

6464
// No matter what happens here, destroy the lease away once we're done.
65-
$lease->releaseOnDestruction(true);
65+
$lease->setReleaseOnDestruction(true);
6666

6767
$operation->applyOperation($interface);
6868

0 commit comments

Comments
 (0)