D10304.id33752.diff
No OneTemporary
Actions

Size

44 KB

Referenced Files

None

Subscribers

None

D10304.id33752.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,99 @@
	+<?php
	+
	+abstract class DrydockMinMaxBlueprintImplementation
	+ extends DrydockBlueprintImplementation {
	+
	+ public function canAllocateMoreResources(array $pool) {
	+ $max_count = $this->getDetail('max-count');
	+ return $max_count === null \|\| 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 true;
	+ }
	+
	+ $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).
	+ $count = 0;
	+ $now = time();
	+ foreach ($pool as $resource) {
	+ $lifetime = $now - $resource->getDateCreated();
	+ if ($lifetime <= $expiry) {
	+ $count++;
	+ }
	+ }
	+
	+ return $count < $max_count;
	+ }
	+
	+ 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: Mon, Oct 6, 7:41 AM (2 w, 2 d ago)
Storage Engine: amazon-s3
Storage Format: Encrypted (AES-256-CBC)
Storage Handle: phabricator/secure/mi/sm/oqvdxrbzu6e2wo33
Default Alt Text: D10304.id33752.diff (44 KB)

D10304.id33752.diffNo OneTemporaryActions

D10304.id33752.diffView Options

File Metadata

Event Timeline

D10304.id33752.diff
No OneTemporary
Actions

D10304.id33752.diff
View Options