Page MenuHomePhabricator
Files F15418707

D10304.id33753.diff
No OneTemporary
Actions

D10304.id33753.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -791,6 +791,7 @@
'DoorkeeperSchemaSpec' => 'applications/doorkeeper/storage/DoorkeeperSchemaSpec.php',
'DoorkeeperTagView' => 'applications/doorkeeper/view/DoorkeeperTagView.php',
'DoorkeeperTagsController' => 'applications/doorkeeper/controller/DoorkeeperTagsController.php',
+ 'DrydockAllocationContext' => 'applications/drydock/util/DrydockAllocationContext.php',
'DrydockAllocatorWorker' => 'applications/drydock/worker/DrydockAllocatorWorker.php',
'DrydockApacheWebrootInterface' => 'applications/drydock/interface/webroot/DrydockApacheWebrootInterface.php',
'DrydockBlueprint' => 'applications/drydock/storage/DrydockBlueprint.php',
@@ -842,6 +843,8 @@
'DrydockManagementLeaseWorkflow' => 'applications/drydock/management/DrydockManagementLeaseWorkflow.php',
'DrydockManagementReleaseWorkflow' => 'applications/drydock/management/DrydockManagementReleaseWorkflow.php',
'DrydockManagementWorkflow' => 'applications/drydock/management/DrydockManagementWorkflow.php',
+ 'DrydockMinMaxBlueprintImplementation' => 'applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php',
+ 'DrydockMinMaxExpiryBlueprintImplementation' => 'applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php',
'DrydockPreallocatedHostBlueprintImplementation' => 'applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php',
'DrydockQuery' => 'applications/drydock/query/DrydockQuery.php',
'DrydockResource' => 'applications/drydock/storage/DrydockResource.php',
@@ -4464,6 +4467,7 @@
'DoorkeeperSchemaSpec' => 'PhabricatorConfigSchemaSpec',
'DoorkeeperTagView' => 'AphrontView',
'DoorkeeperTagsController' => 'PhabricatorController',
+ 'DrydockAllocationContext' => 'Phobject',
'DrydockAllocatorWorker' => 'PhabricatorWorker',
'DrydockApacheWebrootInterface' => 'DrydockWebrootInterface',
'DrydockBlueprint' => array(
@@ -4529,6 +4533,8 @@
'DrydockManagementLeaseWorkflow' => 'DrydockManagementWorkflow',
'DrydockManagementReleaseWorkflow' => 'DrydockManagementWorkflow',
'DrydockManagementWorkflow' => 'PhabricatorManagementWorkflow',
+ 'DrydockMinMaxBlueprintImplementation' => 'DrydockBlueprintImplementation',
+ 'DrydockMinMaxExpiryBlueprintImplementation' => 'DrydockMinMaxBlueprintImplementation',
'DrydockPreallocatedHostBlueprintImplementation' => 'DrydockBlueprintImplementation',
'DrydockQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DrydockResource' => array(
diff --git a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
--- a/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockBlueprintImplementation.php
@@ -19,6 +19,10 @@
abstract public function isEnabled();
+ public function isTest() {
+ return false;
+ }
+
abstract public function getBlueprintName();
abstract public function getDescription();
@@ -42,7 +46,7 @@
return $lease;
}
- protected function getInstance() {
+ public function getInstance() {
if (!$this->instance) {
throw new Exception(
pht('Attach the blueprint instance to the implementation.'));
@@ -127,6 +131,11 @@
$allocated = false;
$allocation_exception = null;
+ $context = new DrydockAllocationContext(
+ $this->getInstance(),
+ $resource,
+ $lease);
+
$resource->openTransaction();
$resource->beginReadLocking();
$resource->reload();
@@ -142,9 +151,9 @@
try {
$allocated = $this->shouldAllocateLease(
+ $context,
$resource,
- $ephemeral_lease,
- $other_leases);
+ $ephemeral_lease);
} catch (Exception $ex) {
$allocation_exception = $ex;
}
@@ -197,19 +206,15 @@
* better implemented in @{method:canAllocateLease}, which serves as a
* cheap filter before lock acquisition.
*
+ * @param DrydockAllocationContext Relevant contextual information.
* @param DrydockResource Candidate resource to allocate the lease on.
* @param DrydockLease Pending lease that wants to allocate here.
- * @param list<DrydockLease> Other allocated and acquired leases on the
- * resource. The implementation can inspect them
- * to verify it can safely add the new lease.
- * @return bool True to allocate the lease on the resource;
- * false to reject it.
* @task lease
*/
abstract protected function shouldAllocateLease(
+ DrydockAllocationContext $context,
DrydockResource $resource,
- DrydockLease $lease,
- array $other_leases);
+ DrydockLease $lease);
/**
@@ -265,10 +270,25 @@
DrydockLease $lease);
+ /**
+ * Release an allocated lease, performing any desired cleanup.
+ *
+ * After this method executes, the lease status is moved to `RELEASED`.
+ *
+ * If release fails, throw an exception.
+ *
+ * @param DrydockResource Resource to release the lease from.
+ * @param DrydockLease Lease to release.
+ * @return void
+ */
+ abstract protected function executeReleaseLease(
+ DrydockResource $resource,
+ DrydockLease $lease);
final public function releaseLease(
DrydockResource $resource,
- DrydockLease $lease) {
+ DrydockLease $lease,
+ $caused_by_closing_resource = false) {
$scope = $this->pushActiveScope(null, $lease);
$released = false;
@@ -286,10 +306,36 @@
$lease->endReadLocking();
$lease->saveTransaction();
+ // Execute clean up outside of the lock and don't perform clean up if the
+ // resource is closing anyway, because in that scenario, the closing
+ // resource will clean up all the leases anyway (e.g. an EC2 host being
+ // terminated that contains leases on it's instance storage).
+ if ($released && !$caused_by_closing_resource) {
+ $this->executeReleaseLease($resource, $lease);
+ }
+
if (!$released) {
throw new Exception(pht('Unable to release lease: lease not active!'));
}
+ if (!$caused_by_closing_resource) {
+ // Check to see if the resource has no more leases, and if so, ask the
+ // blueprint as to whether this resource should be closed.
+ $context = new DrydockAllocationContext(
+ $this->getInstance(),
+ $resource,
+ $lease);
+
+ if ($context->getCurrentResourceLeaseCount() === 0) {
+ if ($this->shouldCloseUnleasedResource($context, $resource)) {
+ self::writeLog(
+ $resource,
+ null,
+ pht('Closing resource because it has no more active leases'));
+ $this->closeResource($resource);
+ }
+ }
+ }
}
@@ -301,10 +347,83 @@
return true;
}
- abstract protected function executeAllocateResource(DrydockLease $lease);
+ public function canAllocateResourceForLease(DrydockLease $lease) {
+ return true;
+ }
+ abstract protected function executeAllocateResource(
+ DrydockResource $resource,
+ DrydockLease $lease);
+
+ /**
+ * Closes a previously allocated resource, performing any desired
+ * cleanup.
+ *
+ * After this method executes, the release status is moved to `CLOSED`.
+ *
+ * If release fails, throw an exception.
+ *
+ * @param DrydockResource Resource to close.
+ * @return void
+ */
+ abstract protected function executeCloseResource(
+ DrydockResource $resource);
+
+ /**
+ * Return whether or not a resource that now has no leases on it
+ * should be automatically closed.
+ *
+ * @param DrydockAllocationContext Relevant contextual information.
+ * @param DrydockResource The resource that has no more leases on it.
+ * @return bool
+ */
+ abstract protected function shouldCloseUnleasedResource(
+ DrydockAllocationContext $context,
+ DrydockResource $resource);
+
+ final public function closeResource(DrydockResource $resource) {
+ $resource->openTransaction();
+ $resource->setStatus(DrydockResourceStatus::STATUS_CLOSING);
+ $resource->save();
+
+ $statuses = array(
+ DrydockLeaseStatus::STATUS_PENDING,
+ DrydockLeaseStatus::STATUS_ACTIVE,
+ );
+
+ $leases = id(new DrydockLeaseQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withResourceIDs(array($resource->getID()))
+ ->withStatuses($statuses)
+ ->execute();
+
+ foreach ($leases as $lease) {
+ switch ($lease->getStatus()) {
+ case DrydockLeaseStatus::STATUS_PENDING:
+ $message = pht('Breaking pending lease (resource closing).');
+ $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN);
+ break;
+ case DrydockLeaseStatus::STATUS_ACTIVE:
+ $message = pht('Releasing active lease (resource closing).');
+ $this->releaseLease($resource, $lease, true);
+ $lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED);
+ break;
+ }
+ self::writeLog($resource, $lease, $message);
+ $lease->save();
+ }
+
+ $this->executeCloseResource($resource);
+
+ $resource->setStatus(DrydockResourceStatus::STATUS_CLOSED);
+ $resource->save();
+ $resource->saveTransaction();
+ }
+
+ final public function allocateResource(
+ DrydockResource $resource,
+ DrydockLease $lease) {
- final public function allocateResource(DrydockLease $lease) {
$scope = $this->pushActiveScope(null, $lease);
$this->log(
@@ -314,7 +433,7 @@
$lease->getLeaseName()));
try {
- $resource = $this->executeAllocateResource($lease);
+ $this->executeAllocateResource($resource, $lease);
$this->validateAllocatedResource($resource);
} catch (Exception $ex) {
$this->logException($ex);
@@ -389,25 +508,6 @@
return idx(self::getAllBlueprintImplementations(), $class);
}
- protected function newResourceTemplate($name) {
- $resource = id(new DrydockResource())
- ->setBlueprintPHID($this->getInstance()->getPHID())
- ->setBlueprintClass($this->getBlueprintClass())
- ->setType($this->getType())
- ->setStatus(DrydockResourceStatus::STATUS_PENDING)
- ->setName($name)
- ->save();
-
- $this->activeResource = $resource;
-
- $this->log(
- pht(
- "Blueprint '%s': Created New Template",
- $this->getBlueprintClass()));
-
- return $resource;
- }
-
/**
* Sanity checks that the blueprint is implemented properly.
*/
diff --git a/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php
new file mode 100644
--- /dev/null
+++ b/src/applications/drydock/blueprint/DrydockMinMaxBlueprintImplementation.php
@@ -0,0 +1,150 @@
+<?php
+
+abstract class DrydockMinMaxBlueprintImplementation
+ extends DrydockBlueprintImplementation {
+
+ public function canAllocateMoreResources(array $pool) {
+ $max_count = $this->getDetail('max-count');
+
+ if ($max_count === null) {
+ $this->log(pht(
+ 'There is no maximum resource limit specified for this blueprint'));
+ return true;
+ }
+
+ $count_pending = 0;
+ $count_allocating = 0;
+ $count_open = 0;
+
+ foreach ($pool as $resource) {
+ switch ($resource->getStatus()) {
+ case DrydockResourceStatus::STATUS_PENDING:
+ $count_pending++;
+ break;
+ case DrydockResourceStatus::STATUS_ALLOCATING:
+ $count_allocating++;
+ break;
+ case DrydockResourceStatus::STATUS_OPEN:
+ $count_open++;
+ break;
+ default:
+ $this->log(pht(
+ 'Resource %d was in the pool of open resources, '.
+ 'but has non-open status of %d',
+ $resource->getID(),
+ $resource->getStatus()));
+ break;
+ }
+ }
+
+ $this->log(pht(
+ 'There are currently %d pending resources, %d allocating resources '.
+ 'and %d open resources in the pool.',
+ $count_pending,
+ $count_allocating,
+ $count_open));
+
+ if (count($pool) < $max_count) {
+ $this->log(pht(
+ 'Will permit resource allocation because %d is less than the maximum '.
+ 'of %d.',
+ count($pool),
+ $max_count));
+ } else {
+ $this->log(pht(
+ 'Will deny resource allocation because %d is less than the maximum '.
+ 'of %d.',
+ count($pool),
+ $max_count));
+ }
+ }
+
+ protected function shouldAllocateLease(
+ DrydockAllocationContext $context,
+ DrydockResource $resource,
+ DrydockLease $lease) {
+
+ // If the current resource can allocate a lease, allow it.
+ if ($context->getCurrentResourceLeaseCount() <
+ $this->getDetail('leases-per-resource')) {
+ return true;
+ }
+
+ // We don't have enough room under the `leases-per-instance` limit, but
+ // this limit can be bypassed if we've allocated all of the resources
+ // we allow.
+ $open_count = $context->getBlueprintOpenResourceCount();
+ if ($open_count < $this->getDetail('max-count')) {
+ if ($this->getDetail('max-count') !== null) {
+ return false;
+ }
+ }
+
+ // Find the resource that has the least leases.
+ $all_lease_counts_grouped = $context->getResourceLeaseCounts();
+ $minimum_lease_count = $all_lease_counts_grouped[$resource->getID()];
+ $minimum_lease_resource_id = $resource->getID();
+ foreach ($all_lease_counts_grouped as $resource_id => $lease_count) {
+ if ($minimum_lease_count > $lease_count) {
+ $minimum_lease_count = $lease_count;
+ $minimum_lease_resource_id = $resource_id;
+ }
+ }
+
+ // If we are that resource, then allow it, otherwise let the other
+ // less-leased resource run through this logic and allocate the lease.
+ return $minimum_lease_resource_id === $resource->getID();
+ }
+
+ protected function shouldCloseUnleasedResource(
+ DrydockAllocationContext $context,
+ DrydockResource $resource) {
+
+ return $context->getBlueprintOpenResourceCount() >
+ $this->getDetail('min-count');
+ }
+
+ public function getFieldSpecifications() {
+ return array(
+ 'min-max-header' => array(
+ 'name' => pht('Allocation Limits'),
+ 'type' => 'header',
+ ),
+ 'min-count' => array(
+ 'name' => pht('Minimum Resources'),
+ 'type' => 'int',
+ 'required' => true,
+ 'caption' => pht(
+ 'The minimum number of resources to keep open in '.
+ 'this pool at all times.'),
+ ),
+ 'max-count' => array(
+ 'name' => pht('Maximum Resources'),
+ 'type' => 'int',
+ 'caption' => pht(
+ 'The maximum number of resources to allow open at any time. '.
+ 'If the number of resources currently open are equal to '.
+ '`max-count` and another lease is requested, Drydock will place '.
+ 'leases on existing resources and thus exceeding '.
+ '`leases-per-resource`. If this parameter is left blank, then '.
+ 'this blueprint has no limit on the number of resources it '.
+ 'can allocate.'),
+ ),
+ 'leases-per-resource' => array(
+ 'name' => pht('Maximum Leases Per Resource'),
+ 'type' => 'int',
+ 'required' => true,
+ 'caption' => pht(
+ 'The soft limit on the number of leases to allocate to an '.
+ 'individual resource in the pool. Drydock will choose the '.
+ 'resource with the lowest number of leases when selecting a '.
+ 'resource to lease on. If all current resources have '.
+ '`leases-per-resource` leases on them, then Drydock will allocate '.
+ 'another resource providing `max-count` would not be exceeded.'.
+ ' If `max-count` would be exceeded, Drydock will instead '.
+ 'overallocate the lease to an existing resource and '.
+ 'exceed the limit specified here.'),
+ ),
+ );
+ }
+}
diff --git a/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php
new file mode 100644
--- /dev/null
+++ b/src/applications/drydock/blueprint/DrydockMinMaxExpiryBlueprintImplementation.php
@@ -0,0 +1,100 @@
+<?php
+
+abstract class DrydockMinMaxExpiryBlueprintImplementation
+ extends DrydockMinMaxBlueprintImplementation {
+
+ public function canAllocateMoreResources(array $pool) {
+ $max_count = $this->getDetail('max-count');
+
+ if ($max_count === null) {
+ return parent::canAllocateMoreResources($pool);
+ }
+
+ $expiry = $this->getDetail('expiry');
+
+ if ($expiry === null) {
+ return count($pool) < $max_count;
+ }
+
+ // Only count resources that haven't yet expired, so we can overallocate
+ // if another expired resource is about to be closed (but is still waiting
+ // on it's current resources to be released).
+ $now = time();
+ $pool_copy = array();
+ foreach ($pool as $resource) {
+ $lifetime = $now - $resource->getDateCreated();
+ if ($lifetime > $expiry) {
+ $pool_copy[] = $resource;
+ }
+ }
+
+ return parent::canAllocateMoreResources($pool_copy);
+ }
+
+ protected function shouldAllocateLease(
+ DrydockAllocationContext $context,
+ DrydockResource $resource,
+ DrydockLease $lease) {
+
+ // If we have no leases allocated to this resource, then we always allow
+ // the parent logic to evaluate. The reason for this is that an expired
+ // resource can only be closed when a lease is released, so if the resource
+ // is open and has no leases, then we'll never reach the code that checks
+ // the expiry to close it. So we allow this lease to occur, so that we'll
+ // hit `shouldCloseUnleasedResource` in the future and the resource will
+ // be closed.
+ if ($context->getCurrentResourceLeaseCount() === 0) {
+ return parent::shouldAllocateLease($context, $resource, $lease);
+ }
+
+ $expiry = $this->getDetail('expiry');
+
+ if ($expiry !== null) {
+ $lifetime = time() - $resource->getDateCreated();
+
+ if ($lifetime > $expiry) {
+ // Prevent allocation of leases to this resource, since it's over
+ // it's lifetime allowed.
+ return false;
+ }
+ }
+
+ return parent::shouldAllocateLease($context, $resource, $lease);
+ }
+
+ protected function shouldCloseUnleasedResource(
+ DrydockAllocationContext $context,
+ DrydockResource $resource) {
+
+ $expiry = $this->getDetail('expiry');
+
+ if ($expiry !== null) {
+ $lifetime = time() - $resource->getDateCreated();
+
+ if ($lifetime > $expiry) {
+ // Force closure of resources that have expired.
+ return true;
+ }
+ }
+
+ return parent::shouldCloseUnleasedResource($context, $resource);
+ }
+
+ public function getFieldSpecifications() {
+ return array(
+ 'expiry-header' => array(
+ 'name' => pht('Resource Expiration'),
+ 'type' => 'header',
+ ),
+ 'expiry' => array(
+ 'name' => pht('Expiry Time'),
+ 'type' => 'int',
+ 'caption' => pht(
+ 'After this time (in seconds) has elapsed since resource creation, '.
+ 'Drydock will no longer lease against the resource, and it will be '.
+ 'closed when there are no more leases (regardless of minimum '.
+ 'resource limits).'),
+ ),
+ ) + parent::getFieldSpecifications();
+ }
+}
diff --git a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php
--- a/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockPreallocatedHostBlueprintImplementation.php
@@ -19,7 +19,10 @@
return false;
}
- protected function executeAllocateResource(DrydockLease $lease) {
+ protected function executeAllocateResource(
+ DrydockResource $resource,
+ DrydockLease $lease) {
+
throw new Exception(
pht("Preallocated hosts can't be dynamically allocated."));
}
@@ -32,9 +35,9 @@
}
protected function shouldAllocateLease(
+ DrydockAllocationContext $context,
DrydockResource $resource,
- DrydockLease $lease,
- array $other_leases) {
+ DrydockLease $lease) {
return true;
}
@@ -113,4 +116,20 @@
throw new Exception(pht("No interface of type '%s'.", $type));
}
+ protected function executeReleaseLease(
+ DrydockResource $resource,
+ DrydockLease $lease) {
+
+ // TODO: Remove leased directory
+ }
+
+ protected function shouldCloseUnleasedResource(
+ DrydockAllocationContext $context,
+ DrydockResource $resource) {
+
+ return false;
+ }
+
+ protected function executeCloseResource(DrydockResource $resource) {}
+
}
diff --git a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
--- a/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php
@@ -26,14 +26,17 @@
}
protected function shouldAllocateLease(
+ DrydockAllocationContext $context,
DrydockResource $resource,
- DrydockLease $lease,
- array $other_leases) {
+ DrydockLease $lease) {
- return !$other_leases;
+ return $context->getCurrentResourceLeaseCount() === 0;
}
- protected function executeAllocateResource(DrydockLease $lease) {
+ protected function executeAllocateResource(
+ DrydockResource $resource,
+ DrydockLease $lease) {
+
$repository_id = $lease->getAttribute('repositoryID');
if (!$repository_id) {
throw new Exception(
@@ -79,15 +82,15 @@
$this->log(pht('Complete.'));
- $resource = $this->newResourceTemplate(
- pht(
+ $resource
+ ->setName(pht(
'Working Copy (%s)',
- $repository->getCallsign()));
- $resource->setStatus(DrydockResourceStatus::STATUS_OPEN);
- $resource->setAttribute('lease.host', $host_lease->getID());
- $resource->setAttribute('path', $path);
- $resource->setAttribute('repositoryID', $repository->getID());
- $resource->save();
+ $repository->getCallsign()))
+ ->setStatus(DrydockResourceStatus::STATUS_OPEN)
+ ->setAttribute('lease.host', $host_lease->getID())
+ ->setAttribute('path', $path)
+ ->setAttribute('repositoryID', $repository->getID())
+ ->save();
return $resource;
}
@@ -117,4 +120,19 @@
throw new Exception(pht("No interface of type '%s'.", $type));
}
+ protected function executeReleaseLease(
+ DrydockResource $resource,
+ DrydockLease $lease) {}
+
+ protected function shouldCloseUnleasedResource(
+ DrydockAllocationContext $context,
+ DrydockResource $resource) {
+
+ return false;
+ }
+
+ protected function executeCloseResource(DrydockResource $resource) {
+ // TODO: Remove leased directory
+ }
+
}
diff --git a/src/applications/drydock/constants/DrydockResourceStatus.php b/src/applications/drydock/constants/DrydockResourceStatus.php
--- a/src/applications/drydock/constants/DrydockResourceStatus.php
+++ b/src/applications/drydock/constants/DrydockResourceStatus.php
@@ -7,12 +7,16 @@
const STATUS_CLOSED = 2;
const STATUS_BROKEN = 3;
const STATUS_DESTROYED = 4;
+ const STATUS_CLOSING = 5;
+ const STATUS_ALLOCATING = 6;
public static function getNameForStatus($status) {
$map = array(
self::STATUS_PENDING => pht('Pending'),
+ self::STATUS_ALLOCATING => pht('Allocating'),
self::STATUS_OPEN => pht('Open'),
self::STATUS_CLOSED => pht('Closed'),
+ self::STATUS_CLOSING => pht('Closing'),
self::STATUS_BROKEN => pht('Broken'),
self::STATUS_DESTROYED => pht('Destroyed'),
);
@@ -23,8 +27,10 @@
public static function getAllStatuses() {
return array(
self::STATUS_PENDING,
+ self::STATUS_ALLOCATING,
self::STATUS_OPEN,
self::STATUS_CLOSED,
+ self::STATUS_CLOSING,
self::STATUS_BROKEN,
self::STATUS_DESTROYED,
);
diff --git a/src/applications/drydock/controller/DrydockBlueprintCreateController.php b/src/applications/drydock/controller/DrydockBlueprintCreateController.php
--- a/src/applications/drydock/controller/DrydockBlueprintCreateController.php
+++ b/src/applications/drydock/controller/DrydockBlueprintCreateController.php
@@ -34,6 +34,11 @@
->setError($e_blueprint);
foreach ($implementations as $implementation_name => $implementation) {
+ if ($implementation->isTest()) {
+ // Never show testing blueprints in the interface.
+ continue;
+ }
+
$disabled = !$implementation->isEnabled();
$control->addButton(
diff --git a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php
--- a/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php
+++ b/src/applications/drydock/management/DrydockManagementLeaseWorkflow.php
@@ -19,6 +19,10 @@
'param' => 'name=value,...',
'help' => pht('Resource specficiation.'),
),
+ array(
+ 'name' => 'in-process',
+ 'help' => pht('Acquire lease in-process.'),
+ ),
));
}
@@ -40,7 +44,9 @@
$attributes = $options->parse($attributes);
}
- PhabricatorWorker::setRunAllTasksInProcess(true);
+ if ($args->getArg('in-process')) {
+ PhabricatorWorker::setRunAllTasksInProcess(true);
+ }
$lease = id(new DrydockLease())
->setResourceType($resource_type);
@@ -48,8 +54,18 @@
$lease->setAttributes($attributes);
}
$lease
- ->queueForActivation()
- ->waitUntilActive();
+ ->queueForActivation();
+
+ while (true) {
+ try {
+ $lease->waitUntilActive();
+ break;
+ } catch (PhabricatorWorkerYieldException $ex) {
+ $console->writeOut(
+ "%s\n",
+ pht('Task yielded while acquiring %s...', $lease->getID()));
+ }
+ }
$console->writeOut("%s\n", pht('Acquired Lease %s', $lease->getID()));
return 0;
diff --git a/src/applications/drydock/storage/DrydockResource.php b/src/applications/drydock/storage/DrydockResource.php
--- a/src/applications/drydock/storage/DrydockResource.php
+++ b/src/applications/drydock/storage/DrydockResource.php
@@ -75,36 +75,8 @@
}
public function closeResource() {
- $this->openTransaction();
- $statuses = array(
- DrydockLeaseStatus::STATUS_PENDING,
- DrydockLeaseStatus::STATUS_ACTIVE,
- );
-
- $leases = id(new DrydockLeaseQuery())
- ->setViewer(PhabricatorUser::getOmnipotentUser())
- ->withResourceIDs(array($this->getID()))
- ->withStatuses($statuses)
- ->execute();
-
- foreach ($leases as $lease) {
- switch ($lease->getStatus()) {
- case DrydockLeaseStatus::STATUS_PENDING:
- $message = pht('Breaking pending lease (resource closing).');
- $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN);
- break;
- case DrydockLeaseStatus::STATUS_ACTIVE:
- $message = pht('Releasing active lease (resource closing).');
- $lease->setStatus(DrydockLeaseStatus::STATUS_RELEASED);
- break;
- }
- DrydockBlueprintImplementation::writeLog($this, $lease, $message);
- $lease->save();
- }
-
- $this->setStatus(DrydockResourceStatus::STATUS_CLOSED);
- $this->save();
- $this->saveTransaction();
+ $blueprint = $this->getBlueprint();
+ $blueprint->closeResource($this);
}
diff --git a/src/applications/drydock/util/DrydockAllocationContext.php b/src/applications/drydock/util/DrydockAllocationContext.php
new file mode 100644
--- /dev/null
+++ b/src/applications/drydock/util/DrydockAllocationContext.php
@@ -0,0 +1,98 @@
+<?php
+
+final class DrydockAllocationContext extends Phobject {
+
+ private $blueprintOpenResourceCount;
+ private $resourceLeaseCounts;
+ private $currentResourceLeaseCount;
+
+ public function __construct(
+ DrydockBlueprint $blueprint,
+ DrydockResource $resource = null,
+ DrydockLease $lease = null) {
+
+ $table_blueprint = $blueprint->getTableName();
+ $table_resource = id(new DrydockResource())->getTableName();
+ $table_lease = id(new DrydockLease())->getTableName();
+
+ $conn = $blueprint->establishConnection('r');
+
+ $result = queryfx_one(
+ $conn,
+ 'SELECT COUNT(id) AS count '.
+ 'FROM %T '.
+ 'WHERE blueprintPHID = %s '.
+ 'AND status IN (%Ld)',
+ $table_resource,
+ $blueprint->getPHID(),
+ array(
+ DrydockResourceStatus::STATUS_PENDING,
+ DrydockResourceStatus::STATUS_OPEN,
+ DrydockResourceStatus::STATUS_ALLOCATING,
+ ));
+ $this->setBlueprintOpenResourceCount($result['count']);
+
+ $results = queryfx_all(
+ $conn,
+ 'SELECT '.
+ ' resource.id AS resourceID, '.
+ ' COUNT(lease.id) AS leaseCount '.
+ 'FROM %T AS resource '.
+ 'LEFT JOIN %T AS lease '.
+ ' ON lease.resourceID = resource.id '.
+ 'WHERE resource.blueprintPHID = %s '.
+ 'AND resource.status IN (%Ld) '.
+ 'AND lease.status IN (%Ld) '.
+ 'GROUP BY resource.id',
+ $table_resource,
+ $table_lease,
+ $blueprint->getPHID(),
+ array(
+ DrydockResourceStatus::STATUS_PENDING,
+ DrydockResourceStatus::STATUS_OPEN,
+ DrydockResourceStatus::STATUS_ALLOCATING,
+ ),
+ array(
+ DrydockLeaseStatus::STATUS_PENDING,
+ DrydockLeaseStatus::STATUS_ACQUIRING,
+ DrydockLeaseStatus::STATUS_ACTIVE,
+ ));
+ $results = ipull($results, 'leaseCount', 'resourceID');
+ $this->setResourceLeaseCounts($results);
+
+ if ($resource !== null) {
+ $this->setCurrentResourceLeaseCount(idx($results, $resource->getID(), 0));
+ }
+
+ // $lease is not yet used, but it's passed in so we can add additional
+ // contextual statistics later.
+ }
+
+ public function setBlueprintOpenResourceCount($blueprint_resource_count) {
+ $this->blueprintOpenResourceCount = $blueprint_resource_count;
+ return $this;
+ }
+
+ public function getBlueprintOpenResourceCount() {
+ return $this->blueprintOpenResourceCount;
+ }
+
+ public function setResourceLeaseCounts($resource_lease_counts) {
+ $this->resourceLeaseCounts = $resource_lease_counts;
+ return $this;
+ }
+
+ public function getResourceLeaseCounts() {
+ return $this->resourceLeaseCounts;
+ }
+
+ public function setCurrentResourceLeaseCount($resource_lease_counts) {
+ $this->currentResourceLeaseCount = $resource_lease_counts;
+ return $this;
+ }
+
+ public function getCurrentResourceLeaseCount() {
+ return $this->currentResourceLeaseCount;
+ }
+
+}
diff --git a/src/applications/drydock/view/DrydockResourceListView.php b/src/applications/drydock/view/DrydockResourceListView.php
--- a/src/applications/drydock/view/DrydockResourceListView.php
+++ b/src/applications/drydock/view/DrydockResourceListView.php
@@ -26,6 +26,7 @@
$item->addAttribute($status);
switch ($resource->getStatus()) {
+ case DrydockResourceStatus::STATUS_ALLOCATING:
case DrydockResourceStatus::STATUS_PENDING:
$item->setStatusIcon('fa-dot-circle-o yellow');
break;
diff --git a/src/applications/drydock/worker/DrydockAllocatorWorker.php b/src/applications/drydock/worker/DrydockAllocatorWorker.php
--- a/src/applications/drydock/worker/DrydockAllocatorWorker.php
+++ b/src/applications/drydock/worker/DrydockAllocatorWorker.php
@@ -39,10 +39,21 @@
protected function doWork() {
$lease = $this->loadLease();
+
+ if ($lease->getStatus() != DrydockLeaseStatus::STATUS_PENDING) {
+ // We can't handle non-pending leases.
+ return;
+ }
+
$this->logToDrydock(pht('Allocating Lease'));
try {
$this->allocateLease($lease);
+ } catch (PhabricatorWorkerYieldException $ex) {
+ $this->logToDrydock(pht(
+ 'Another Drydock lease is being allocated right now; '.
+ 'lease acquisition will continue soon'));
+ throw $ex;
} catch (Exception $ex) {
// TODO: We should really do this when archiving the task, if we've
@@ -50,7 +61,9 @@
// and always fail after the first retry right now, so this is
// functionally equivalent.
$lease->reload();
- if ($lease->getStatus() == DrydockLeaseStatus::STATUS_PENDING) {
+ if ($lease->getStatus() == DrydockLeaseStatus::STATUS_PENDING ||
+ $lease->getStatus() == DrydockLeaseStatus::STATUS_ACQUIRING) {
+
$lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN);
$lease->save();
}
@@ -76,6 +89,16 @@
$blueprints = $this->loadAllBlueprints();
+ $lock = PhabricatorGlobalLock::newLock('drydockallocation');
+ try {
+ $lock->lock();
+ } catch (PhutilLockException $ex) {
+ // The lock is expected to be released reasonably quickly, so
+ // just push the work to the back of the queue and let it be
+ // reprocessed as soon as possible.
+ throw new PhabricatorWorkerYieldException(1);
+ }
+
// TODO: Policy stuff.
$pool = id(new DrydockResource())->loadAllWhere(
'type = %s AND status = %s',
@@ -116,68 +139,212 @@
}
if (!$resource) {
- $blueprints = DrydockBlueprintImplementation
- ::getAllBlueprintImplementationsForResource($type);
+ // Attempt to use pending resources if we can.
+ $pool = id(new DrydockResource())->loadAllWhere(
+ 'type = %s AND status = %s',
+ $lease->getResourceType(),
+ DrydockResourceStatus::STATUS_PENDING);
$this->logToDrydock(
- pht('Found %d Blueprints', count($blueprints)));
+ pht('Found %d Pending Resource(s)', count($pool)));
- foreach ($blueprints as $key => $candidate_blueprint) {
- if (!$candidate_blueprint->isEnabled()) {
- unset($blueprints[$key]);
+ $candidates = array();
+ foreach ($pool as $key => $candidate) {
+ if (!isset($blueprints[$candidate->getBlueprintPHID()])) {
+ unset($pool[$key]);
continue;
}
+
+ $blueprint = $blueprints[$candidate->getBlueprintPHID()];
+ $implementation = $blueprint->getImplementation();
+
+ if ($implementation->filterResource($candidate, $lease)) {
+ $candidates[] = $candidate;
+ }
}
$this->logToDrydock(
- pht('%d Blueprints Enabled', count($blueprints)));
+ pht('%d Pending Resource(s) Remain',
+ count($candidates)));
+
+ $resource = null;
+ if ($candidates) {
+ shuffle($candidates);
+ foreach ($candidates as $candidate_resource) {
+ $blueprint = $blueprints[$candidate_resource->getBlueprintPHID()]
+ ->getImplementation();
+ if ($blueprint->allocateLease($candidate_resource, $lease)) {
+ $resource = $candidate_resource;
+ break;
+ }
+ }
+ }
+ }
+
+ if ($resource) {
+ $lock->unlock();
+ } else {
+ $blueprints = id(new DrydockBlueprintQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->execute();
+ $blueprints = mpull($blueprints, 'getImplementation', 'getPHID');
+
+ $this->logToDrydock(
+ pht('Found %d Blueprints', count($blueprints)));
foreach ($blueprints as $key => $candidate_blueprint) {
- if (!$candidate_blueprint->canAllocateMoreResources($pool)) {
+ if (!$candidate_blueprint->isEnabled()) {
+ unset($blueprints[$key]);
+ continue;
+ }
+
+ if ($candidate_blueprint->getType() !==
+ $lease->getResourceType()) {
unset($blueprints[$key]);
continue;
}
}
$this->logToDrydock(
- pht('%d Blueprints Can Allocate', count($blueprints)));
+ pht('%d Blueprints Enabled', count($blueprints)));
- if (!$blueprints) {
- $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN);
- $lease->save();
+ $resources_per_blueprint = id(new DrydockResourceQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withStatuses(array(
+ DrydockResourceStatus::STATUS_PENDING,
+ DrydockResourceStatus::STATUS_OPEN,
+ DrydockResourceStatus::STATUS_ALLOCATING,
+ ))
+ ->execute();
+ $resources_per_blueprint = mgroup(
+ $resources_per_blueprint,
+ 'getBlueprintPHID');
+
+ try {
+ foreach ($blueprints as $key => $candidate_blueprint) {
+ $rpool = idx($resources_per_blueprint, $key, array());
+ if (!$candidate_blueprint->canAllocateMoreResources($rpool)) {
+ unset($blueprints[$key]);
+ continue;
+ }
+
+ if (!$candidate_blueprint->canAllocateResourceForLease($lease)) {
+ unset($blueprints[$key]);
+ continue;
+ }
+ }
$this->logToDrydock(
- pht(
- "There are no resources of type '%s' available, and no ".
- "blueprints which can allocate new ones.",
- $type));
+ pht('%d Blueprints Can Allocate', count($blueprints)));
- return;
+ if (!$blueprints) {
+ $lease->setStatus(DrydockLeaseStatus::STATUS_BROKEN);
+ $lease->save();
+
+ $this->logToDrydock(
+ pht(
+ "There are no resources of type '%s' available, and no ".
+ "blueprints which can allocate new ones.",
+ $type));
+
+ $lock->unlock();
+ return;
+ }
+
+ // TODO: Rank intelligently.
+ shuffle($blueprints);
+
+ $blueprint = head($blueprints);
+
+ // Create and save the resource preemptively with STATUS_ALLOCATING
+ // before we unlock, so that other workers will correctly count the
+ // new resource "to be allocated" when determining if they can allocate
+ // more resources to a blueprint.
+ $resource = id(new DrydockResource())
+ ->setBlueprintPHID($blueprint->getInstance()->getPHID())
+ ->setType($blueprint->getType())
+ ->setName(pht('Pending Allocation'))
+ ->setStatus(DrydockResourceStatus::STATUS_ALLOCATING)
+ ->save();
+
+ // Pre-emptively allocate the lease on the resource inside the lock,
+ // to ensure that other allocators don't cause this worker to lose
+ // an allocation race. If we fail to allocate a lease here, then the
+ // blueprint is allocating resources it can't lease against.
+ //
+ // NOTE; shouldAllocateLease is specified to only check resource
+ // constraints, which means that it shouldn't be checking compatibility
+ // of details on resources or leases. If there are any
+ // shouldAllocateLease that use details on the resources or leases to
+ // complete their work, then we might have to change this to:
+ //
+ // $lease->setStatus(DrydockLeaseStatus::STATUS_ACQUIRING);
+ // $lease->setResourceID($resource->getID());
+ // $lease->attachResource($resource);
+ // $lease->save();
+ //
+ // and bypass the resource quota logic entirely (and just assume that
+ // a resource allocated by a blueprint can have the lease allocated
+ // against it).
+ //
+ if (!$blueprint->allocateLease($resource, $lease)) {
+ throw new Exception(
+ 'Blueprint allocated a resource, but can\'t lease against it.');
+ }
+
+ $this->logToDrydock(
+ pht('Pre-emptively allocated the lease against the new resource.'));
+
+ // We now have to set the resource into Pending status, now that the
+ // initial lease has been grabbed on the resource. This ensures that
+ // as soon as we leave the lock, other allocators can start taking
+ // leases on it. If we didn't do this, we can run into a scenario
+ // where all resources are in "ALLOCATING" status when an allocator
+ // runs, and instead of overleasing, the request would fail.
+ //
+ // TODO: I think this means we can remove the "ALLOCATING" status now,
+ // but I'm not entirely sure. It's only ever used inside the lock, so
+ // I don't think any other allocators can race when attempting to
+ // use a still-allocating resource.
+ $resource
+ ->setStatus(DrydockResourceStatus::STATUS_PENDING)
+ ->save();
+
+ $this->logToDrydock(
+ pht('Moved the resource to the pending status.'));
+
+ $lock->unlock();
+ } catch (Exception $ex) {
+ $lock->unlock();
+ throw $ex;
}
- // TODO: Rank intelligently.
- shuffle($blueprints);
-
- $blueprint = head($blueprints);
- $resource = $blueprint->allocateResource($lease);
-
- if (!$blueprint->allocateLease($resource, $lease)) {
- // TODO: This "should" happen only if we lost a race with another lease,
- // which happened to acquire this resource immediately after we
- // allocated it. In this case, the right behavior is to retry
- // immediately. However, other things like a blueprint allocating a
- // resource it can't actually allocate the lease on might be happening
- // too, in which case we'd just allocate infinite resources. Probably
- // what we should do is test for an active or allocated lease and retry
- // if we find one (although it might have already been released by now)
- // and fail really hard ("your configuration is a huge broken mess")
- // otherwise. But just throw for now since this stuff is all edge-casey.
- // Alternatively we could bring resources up in a "BESPOKE" status
- // and then switch them to "OPEN" only after the allocating lease gets
- // its grubby mitts on the resource. This might make more sense but
- // is a bit messy.
- throw new Exception(pht('Lost an allocation race?'));
+ try {
+ $blueprint->allocateResource($resource, $lease);
+ } catch (Exception $ex) {
+ $resource->delete();
+ throw $ex;
}
+
+ // We do not need to call allocateLease here, because we have already
+ // performed this check inside the lock. If the logic at the end of the
+ // lock changes to bypass allocateLease, then we probably need to do some
+ // logic like (where STATUS_RESERVED does not count towards allocation
+ // limits):
+ //
+ // $lock->lock(10000);
+ // $lease->setStatus(DrydockLeaseStatus::STATUS_RESERVED);
+ // $lease->save();
+ // try {
+ // if (!$blueprint->allocateLease($resource, $lease)) {
+ // throw new Exception('Lost an allocation race?');
+ // }
+ // } catch (Exception $ex) {
+ // $lock->unlock();
+ // throw $ex;
+ // }
+ // $lock->unlock();
+ //
}
$blueprint = $resource->getBlueprint();
diff --git a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php
--- a/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterLeaseHostBuildStepImplementation.php
@@ -21,29 +21,49 @@
$settings = $this->getSettings();
- // Create the lease.
- $lease = id(new DrydockLease())
- ->setResourceType('host')
- ->setOwnerPHID($build_target->getPHID())
- ->setAttributes(
+ // This build step is reentrant, because waitUntilActive may
+ // throw PhabricatorWorkerYieldException. Check to see if there
+ // is already a lease on the build target, and if so, wait until
+ // that lease is active instead of creating a new one.
+ $artifacts = id(new HarbormasterBuildArtifactQuery())
+ ->setViewer(PhabricatorUser::getOmnipotentUser())
+ ->withBuildTargetPHIDs(array($build_target->getPHID()))
+ ->execute();
+ $artifact = count($artifacts) > 0 ? head($artifacts) : null;
+
+ if ($artifact === null) {
+ // Create the lease.
+ $lease = id(new DrydockLease())
+ ->setResourceType('host')
+ ->setOwnerPHID($build_target->getPHID())
+ ->setAttributes(
+ array(
+ 'platform' => $settings['platform'],
+ ))
+ ->queueForActivation();
+
+ // Create the associated artifact.
+ $artifact = $build_target->createArtifact(
+ PhabricatorUser::getOmnipotentUser(),
+ $settings['name'],
+ HarbormasterHostArtifact::ARTIFACTCONST,
array(
- 'platform' => $settings['platform'],
- ))
- ->queueForActivation();
+ 'drydockLeasePHID' => $lease->getPHID(),
+ ));
+ } else {
+ // Load the lease.
+ $impl = $artifact->getArtifactImplementation();
+ $lease = $impl->loadArtifactLease(PhabricatorUser::getOmnipotentUser());
+ }
// Wait until the lease is fulfilled.
- // TODO: This will throw an exception if the lease can't be fulfilled;
- // we should treat that as build failure not build error.
- $lease->waitUntilActive();
-
- // Create the associated artifact.
- $artifact = $build_target->createArtifact(
- PhabricatorUser::getOmnipotentUser(),
- $settings['name'],
- HarbormasterHostArtifact::ARTIFACTCONST,
- array(
- 'drydockLeasePHID' => $lease->getPHID(),
- ));
+ try {
+ $lease->waitUntilActive();
+ } catch (PhabricatorWorkerYieldException $ex) {
+ throw $ex;
+ } catch (Exception $ex) {
+ throw new HarbormasterBuildFailureException($ex->getMessage());
+ }
}
public function getArtifactOutputs() {

File Metadata

Mime Type
text/plain
Expires
Sat, Mar 22, 12:58 AM (2 d, 20 h ago)
Storage Engine
amazon-s3
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
phabricator/secure/ms/f6/4hntgnsg3o7qrj6t
Default Alt Text
D10304.id33753.diff (45 KB)

Event Timeline

Phabricator · Built by Phacility