From 1f311d64c608a66f95e57a719e996b8eb7760b88 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 23 Sep 2015 11:20:20 -0700 Subject: [PATCH] Give Drydock resources and leases a real "destroy" lifecycle phase Summary: Ref T9252. Some leases or resources may need to remove data, tear down VMs, etc., during cleanup. After they are released, queue a "destroy" phase for performing teardown. Test Plan: - Used `bin/drydock lease ...` to create a working copy lease. - Used `bin/drydock release-lease` and `bin/drydock release-resource` to release the lease and then the working copy and host. - Saw working copy and host get destroyed and cleaned up properly. Reviewers: hach-que, chad Reviewed By: chad Maniphest Tasks: T6569, T9252 Differential Revision: https://secure.phabricator.com/D14144 --- src/__phutil_library_map__.php | 4 + ...anacServiceHostBlueprintImplementation.php | 26 ++++++ .../DrydockBlueprintImplementation.php | 84 +++++++++++++------ ...dockWorkingCopyBlueprintImplementation.php | 40 +++++++++ .../DrydockManagementLeaseWorkflow.php | 15 +++- .../drydock/storage/DrydockBlueprint.php | 33 +++++++- .../worker/DrydockLeaseDestroyWorker.php | 39 +++++++++ .../worker/DrydockLeaseUpdateWorker.php | 15 +++- .../worker/DrydockResourceDestroyWorker.php | 35 ++++++++ .../worker/DrydockResourceUpdateWorker.php | 9 +- 10 files changed, 264 insertions(+), 36 deletions(-) create mode 100644 src/applications/drydock/worker/DrydockLeaseDestroyWorker.php create mode 100644 src/applications/drydock/worker/DrydockResourceDestroyWorker.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 229f074c58..83f5916630 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -832,6 +832,7 @@ phutil_register_library_map(array( 'DrydockLease' => 'applications/drydock/storage/DrydockLease.php', 'DrydockLeaseController' => 'applications/drydock/controller/DrydockLeaseController.php', 'DrydockLeaseDatasource' => 'applications/drydock/typeahead/DrydockLeaseDatasource.php', + 'DrydockLeaseDestroyWorker' => 'applications/drydock/worker/DrydockLeaseDestroyWorker.php', 'DrydockLeaseListController' => 'applications/drydock/controller/DrydockLeaseListController.php', 'DrydockLeaseListView' => 'applications/drydock/view/DrydockLeaseListView.php', 'DrydockLeasePHIDType' => 'applications/drydock/phid/DrydockLeasePHIDType.php', @@ -859,6 +860,7 @@ phutil_register_library_map(array( 'DrydockResource' => 'applications/drydock/storage/DrydockResource.php', 'DrydockResourceController' => 'applications/drydock/controller/DrydockResourceController.php', 'DrydockResourceDatasource' => 'applications/drydock/typeahead/DrydockResourceDatasource.php', + 'DrydockResourceDestroyWorker' => 'applications/drydock/worker/DrydockResourceDestroyWorker.php', 'DrydockResourceListController' => 'applications/drydock/controller/DrydockResourceListController.php', 'DrydockResourceListView' => 'applications/drydock/view/DrydockResourceListView.php', 'DrydockResourcePHIDType' => 'applications/drydock/phid/DrydockResourcePHIDType.php', @@ -4562,6 +4564,7 @@ phutil_register_library_map(array( ), 'DrydockLeaseController' => 'DrydockController', 'DrydockLeaseDatasource' => 'PhabricatorTypeaheadDatasource', + 'DrydockLeaseDestroyWorker' => 'DrydockWorker', 'DrydockLeaseListController' => 'DrydockLeaseController', 'DrydockLeaseListView' => 'AphrontView', 'DrydockLeasePHIDType' => 'PhabricatorPHIDType', @@ -4595,6 +4598,7 @@ phutil_register_library_map(array( ), 'DrydockResourceController' => 'DrydockController', 'DrydockResourceDatasource' => 'PhabricatorTypeaheadDatasource', + 'DrydockResourceDestroyWorker' => 'DrydockWorker', 'DrydockResourceListController' => 'DrydockResourceController', 'DrydockResourceListView' => 'AphrontView', 'DrydockResourcePHIDType' => 'PhabricatorPHIDType', diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php index 64c002bdb0..d906df5b04 100644 --- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php @@ -87,6 +87,14 @@ final class DrydockAlmanacServiceHostBlueprintImplementation $exceptions); } + public function destroyResource( + DrydockBlueprint $blueprint, + DrydockResource $resource) { + // We don't create anything when allocating hosts, so we don't need to do + // any cleanup here. + return; + } + public function canAcquireLeaseOnResource( DrydockBlueprint $blueprint, DrydockResource $resource, @@ -110,6 +118,24 @@ final class DrydockAlmanacServiceHostBlueprintImplementation ->acquireOnResource($resource); } + public function didReleaseLease( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + // Almanac hosts stick around indefinitely so we don't need to recycle them + // if they don't have any leases. + return; + } + + public function destroyLease( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + // We don't create anything when activating a lease, so we don't need to + // throw anything away. + return; + } + private function getLeaseSlotLock(DrydockResource $resource) { $resource_phid = $resource->getPHID(); return "almanac.host.lease({$resource_phid})"; diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php index d5085002eb..e781379568 100644 --- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php @@ -67,6 +67,11 @@ abstract class DrydockBlueprintImplementation extends Phobject { DrydockResource $resource, DrydockLease $lease); + + /** + * @return void + * @task lease + */ public function activateLease( DrydockBlueprint $blueprint, DrydockResource $resource, @@ -74,37 +79,40 @@ abstract class DrydockBlueprintImplementation extends Phobject { throw new PhutilMethodNotImplementedException(); } - final public function releaseLease( + + /** + * React to a lease being released. + * + * This callback is primarily useful for automatically releasing resources + * once all leases are released. + * + * @param DrydockBlueprint Blueprint which built the resource. + * @param DrydockResource Resource a lease was released on. + * @param DrydockLease Recently released lease. + * @return void + * @task lease + */ + abstract public function didReleaseLease( DrydockBlueprint $blueprint, DrydockResource $resource, - DrydockLease $lease) { + DrydockLease $lease); - // TODO: This is all broken nonsense. - - $scope = $this->pushActiveScope(null, $lease); - - $released = false; - - $lease->openTransaction(); - $lease->beginReadLocking(); - $lease->reload(); - - if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACTIVE) { - $lease->release(); - $lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED); - $lease->save(); - $released = true; - } - - $lease->endReadLocking(); - $lease->saveTransaction(); - - if (!$released) { - throw new Exception(pht('Unable to release lease: lease not active!')); - } - - } + /** + * Destroy any temporary data associated with a lease. + * + * If a lease creates temporary state while held, destroy it here. + * + * @param DrydockBlueprint Blueprint which built the resource. + * @param DrydockResource Resource the lease is acquired on. + * @param DrydockLease The lease being destroyed. + * @return void + * @task lease + */ + abstract public function destroyLease( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease); /* -( Resource Allocation )------------------------------------------------ */ @@ -204,12 +212,34 @@ abstract class DrydockBlueprintImplementation extends Phobject { DrydockBlueprint $blueprint, DrydockLease $lease); + + /** + * @task resource + */ public function activateResource( DrydockBlueprint $blueprint, DrydockResource $resource) { throw new PhutilMethodNotImplementedException(); } + + /** + * Destroy any temporary data associated with a resource. + * + * If a resource creates temporary state when allocated, destroy that state + * here. For example, you might shut down a virtual host or destroy a working + * copy on disk. + * + * @param DrydockBlueprint Blueprint which built the resource. + * @param DrydockResource Resource being destroyed. + * @return void + * @task resource + */ + abstract public function destroyResource( + DrydockBlueprint $blueprint, + DrydockResource $resource); + + /* -( Resource Interfaces )------------------------------------------------ */ diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php index 6fdc787274..6ffe90469d 100644 --- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php +++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php @@ -126,6 +126,26 @@ final class DrydockWorkingCopyBlueprintImplementation ->activateResource(); } + public function destroyResource( + DrydockBlueprint $blueprint, + DrydockResource $resource) { + + $lease = $this->loadHostLease($resource); + + // Destroy the lease on the host. + $lease->releaseOnDestruction(); + + // Destroy the working copy on disk. + $command_type = DrydockCommandInterface::INTERFACE_TYPE; + $interface = $lease->getInterface($command_type); + + $root_key = 'workingcopy.root'; + $root = $resource->getAttribute($root_key); + if (strlen($root)) { + $interface->execx('rm -rf -- %s', $root); + } + } + public function activateLease( DrydockBlueprint $blueprint, DrydockResource $resource, @@ -162,6 +182,26 @@ final class DrydockWorkingCopyBlueprintImplementation $lease->activateOnResource($resource); } + public function didReleaseLease( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + // We leave working copies around even if there are no leases on them, + // since the cost to maintain them is nearly zero but rebuilding them is + // moderately expensive and it's likely that they'll be reused. + return; + } + + public function destroyLease( + DrydockBlueprint $blueprint, + DrydockResource $resource, + DrydockLease $lease) { + // When we activate a lease we just reset the working copy state and do + // not create any new state, so we don't need to do anything special when + // destroying a lease. + return; + } + public function getType() { return 'working-copy'; } diff --git a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php index f3617f9b44..117ab2b6a2 100644 --- a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php +++ b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php @@ -45,11 +45,18 @@ final class DrydockManagementLeaseWorkflow if ($attributes) { $lease->setAttributes($attributes); } - $lease - ->queueForActivation() - ->waitUntilActive(); + $lease->queueForActivation(); + + echo tsprintf( + "%s\n", + pht('Waiting for daemons to activate lease...')); + + $lease->waitUntilActive(); + + echo tsprintf( + "%s\n", + pht('Activated lease "%s".', $lease->getID())); - $console->writeOut("%s\n", pht('Acquired Lease %s', $lease->getID())); return 0; } diff --git a/src/applications/drydock/storage/DrydockBlueprint.php b/src/applications/drydock/storage/DrydockBlueprint.php index 0907948d14..ed83ded571 100644 --- a/src/applications/drydock/storage/DrydockBlueprint.php +++ b/src/applications/drydock/storage/DrydockBlueprint.php @@ -143,6 +143,18 @@ final class DrydockBlueprint extends DrydockDAO $resource); } + + /** + * @task resource + */ + public function destroyResource(DrydockResource $resource) { + $this->getImplementation()->destroyResource( + $this, + $resource); + return $this; + } + + /* -( Acquiring Leases )--------------------------------------------------- */ @@ -188,10 +200,27 @@ final class DrydockBlueprint extends DrydockDAO /** * @task lease */ - public function releaseLease( + public function didReleaseLease( DrydockResource $resource, DrydockLease $lease) { - $this->getImplementation()->releaseLease($this, $resource, $lease); + $this->getImplementation()->didReleaseLease( + $this, + $resource, + $lease); + return $this; + } + + + /** + * @task lease + */ + public function destroyLease( + DrydockResource $resource, + DrydockLease $lease) { + $this->getImplementation()->destroyLease( + $this, + $resource, + $lease); return $this; } diff --git a/src/applications/drydock/worker/DrydockLeaseDestroyWorker.php b/src/applications/drydock/worker/DrydockLeaseDestroyWorker.php new file mode 100644 index 0000000000..c019f29cb7 --- /dev/null +++ b/src/applications/drydock/worker/DrydockLeaseDestroyWorker.php @@ -0,0 +1,39 @@ +getTaskDataValue('leasePHID'); + $lease = $this->loadLease($lease_phid); + $this->destroyLease($lease); + } + + private function destroyLease(DrydockLease $lease) { + $status = $lease->getStatus(); + + switch ($status) { + case DrydockLeaseStatus::STATUS_RELEASED: + case DrydockLeaseStatus::STATUS_BROKEN: + break; + default: + throw new PhabricatorWorkerPermanentFailureException( + pht( + 'Unable to destroy lease ("%s"), lease has the wrong '. + 'status ("%s").', + $lease->getPHID(), + $status)); + } + + $resource = $lease->getResource(); + $blueprint = $resource->getBlueprint(); + + $blueprint->destroyLease($resource, $lease); + + // TODO: Rename DrydockLeaseStatus::STATUS_EXPIRED to STATUS_DESTROYED. + + $lease + ->setStatus(DrydockLeaseStatus::STATUS_EXPIRED) + ->save(); + } + +} diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php index 5e96a608f6..3206d6ea13 100644 --- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php @@ -53,8 +53,19 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { DrydockSlotLock::releaseLocks($lease->getPHID()); $lease->saveTransaction(); - // TODO: Hook for resource release behaviors. - // TODO: Schedule lease destruction. + PhabricatorWorker::scheduleTask( + 'DrydockLeaseDestroyWorker', + array( + 'leasePHID' => $lease->getPHID(), + ), + array( + 'objectPHID' => $lease->getPHID(), + )); + + $resource = $lease->getResource(); + $blueprint = $resource->getBlueprint(); + + $blueprint->didReleaseLease($resource, $lease); } } diff --git a/src/applications/drydock/worker/DrydockResourceDestroyWorker.php b/src/applications/drydock/worker/DrydockResourceDestroyWorker.php new file mode 100644 index 0000000000..812edc682a --- /dev/null +++ b/src/applications/drydock/worker/DrydockResourceDestroyWorker.php @@ -0,0 +1,35 @@ +getTaskDataValue('resourcePHID'); + $resource = $this->loadResource($resource_phid); + $this->destroyResource($resource); + } + + private function destroyResource(DrydockResource $resource) { + $status = $resource->getStatus(); + + switch ($status) { + case DrydockResourceStatus::STATUS_CLOSED: + case DrydockResourceStatus::STATUS_BROKEN: + break; + default: + throw new PhabricatorWorkerPermanentFailureException( + pht( + 'Unable to destroy resource ("%s"), resource has the wrong '. + 'status ("%s").', + $resource->getPHID(), + $status)); + } + + $blueprint = $resource->getBlueprint(); + $blueprint->destroyResource($resource); + + $resource + ->setStatus(DrydockResourceStatus::STATUS_DESTROYED) + ->save(); + } + +} diff --git a/src/applications/drydock/worker/DrydockResourceUpdateWorker.php b/src/applications/drydock/worker/DrydockResourceUpdateWorker.php index 78c5726b32..7528df8d12 100644 --- a/src/applications/drydock/worker/DrydockResourceUpdateWorker.php +++ b/src/applications/drydock/worker/DrydockResourceUpdateWorker.php @@ -86,7 +86,14 @@ final class DrydockResourceUpdateWorker extends DrydockWorker { $lease->scheduleUpdate(); } - // TODO: Schedule resource destruction. + PhabricatorWorker::scheduleTask( + 'DrydockResourceDestroyWorker', + array( + 'resourcePHID' => $resource->getPHID(), + ), + array( + 'objectPHID' => $resource->getPHID(), + )); } }