Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F14834394
D14234.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
12 KB
Referenced Files
None
Subscribers
None
D14234.diff
View Options
diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
--- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
@@ -262,13 +262,14 @@
array(
DrydockResourceStatus::STATUS_PENDING,
DrydockResourceStatus::STATUS_ACTIVE,
+ DrydockResourceStatus::STATUS_BROKEN,
DrydockResourceStatus::STATUS_RELEASED,
))
->execute();
$allocated_phids = array();
foreach ($pool as $resource) {
- $allocated_phids[] = $resource->getAttribute('almanacDevicePHID');
+ $allocated_phids[] = $resource->getAttribute('almanacBindingPHID');
}
$allocated_phids = array_fuse($allocated_phids);
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
@@ -288,7 +288,7 @@
}
protected function newLease(DrydockBlueprint $blueprint) {
- return id(new DrydockLease());
+ return DrydockLease::initializeNewLease();
}
protected function requireActiveLease(DrydockLease $lease) {
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
@@ -104,8 +104,13 @@
$host_lease = $this->newLease($blueprint)
->setResourceType('host')
->setOwnerPHID($resource_phid)
- ->setAttribute('workingcopy.resourcePHID', $resource_phid)
- ->queueForActivation();
+ ->setAttribute('workingcopy.resourcePHID', $resource_phid);
+
+ $resource
+ ->setAttribute('host.leasePHID', $host_lease->getPHID())
+ ->save();
+
+ $host_lease->queueForActivation();
// TODO: Add some limits to the number of working copies we can have at
// once?
@@ -121,7 +126,6 @@
return $resource
->setAttribute('repositories.map', $map)
- ->setAttribute('host.leasePHID', $host_lease->getPHID())
->allocateResource();
}
@@ -165,7 +169,13 @@
DrydockBlueprint $blueprint,
DrydockResource $resource) {
- $lease = $this->loadHostLease($resource);
+ try {
+ $lease = $this->loadHostLease($resource);
+ } catch (Exception $ex) {
+ // If we can't load the lease, assume we don't need to take any actions
+ // to destroy it.
+ return;
+ }
// Destroy the lease on the host.
$lease->releaseOnDestruction();
diff --git a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
--- a/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
+++ b/src/applications/drydock/logtype/DrydockLeaseWaitingForResourcesLogType.php
@@ -19,7 +19,7 @@
return pht(
'Waiting for available resources from: %s.',
- $viewer->renderHandleList($blueprint_phids));
+ $viewer->renderHandleList($blueprint_phids)->render());
}
}
diff --git a/src/applications/drydock/storage/DrydockLease.php b/src/applications/drydock/storage/DrydockLease.php
--- a/src/applications/drydock/storage/DrydockLease.php
+++ b/src/applications/drydock/storage/DrydockLease.php
@@ -19,6 +19,16 @@
private $activateWhenAcquired = false;
private $slotLocks = array();
+ public static function initializeNewLease() {
+ $lease = new DrydockLease();
+
+ // Pregenerate a PHID so that the caller can set something up to release
+ // this lease before queueing it for activation.
+ $lease->setPHID($lease->generatePHID());
+
+ return $lease;
+ }
+
/**
* Flag this lease to be released when its destructor is called. This is
* mostly useful if you have a script which acquires, uses, and then releases
@@ -232,6 +242,7 @@
}
$this->openTransaction();
+
try {
DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
$this->slotLocks = array();
@@ -247,16 +258,12 @@
throw $ex;
}
- try {
- $this
- ->setResourcePHID($resource->getPHID())
- ->attachResource($resource)
- ->setStatus($new_status)
- ->save();
- } catch (Exception $ex) {
- $this->killTransaction();
- throw $ex;
- }
+ $this
+ ->setResourcePHID($resource->getPHID())
+ ->attachResource($resource)
+ ->setStatus($new_status)
+ ->save();
+
$this->saveTransaction();
$this->isAcquired = true;
@@ -295,12 +302,24 @@
$this->openTransaction();
- $this
- ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE)
- ->save();
-
+ try {
DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
$this->slotLocks = array();
+ } catch (DrydockSlotLockException $ex) {
+ $this->killTransaction();
+
+ $this->logEvent(
+ DrydockSlotLockFailureLogType::LOGCONST,
+ array(
+ 'locks' => $ex->getLockMap(),
+ ));
+
+ throw $ex;
+ }
+
+ $this
+ ->setStatus(DrydockLeaseStatus::STATUS_ACTIVE)
+ ->save();
$this->saveTransaction();
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
@@ -113,13 +113,6 @@
}
public function allocateResource() {
- if ($this->getID()) {
- throw new Exception(
- pht(
- 'Trying to allocate a resource which has already been persisted. '.
- 'Only new resources may be allocated.'));
- }
-
// We expect resources to have a pregenerated PHID, as they should have
// been created by a call to DrydockBlueprint->newResourceTemplate().
if (!$this->getPHID()) {
@@ -155,9 +148,14 @@
} catch (DrydockSlotLockException $ex) {
$this->killTransaction();
- // NOTE: We have to log this on the blueprint, as the resource is not
- // going to be saved so the PHID will vanish.
- $this->getBlueprint()->logEvent(
+ if ($this->getID()) {
+ $log_target = $this;
+ } else {
+ // If we don't have an ID, we have to log this on the blueprint, as the
+ // resource is not going to be saved so the PHID will vanish.
+ $log_target = $this->getBlueprint();
+ }
+ $log_target->logEvent(
DrydockSlotLockFailureLogType::LOGCONST,
array(
'locks' => $ex->getLockMap(),
@@ -166,14 +164,9 @@
throw $ex;
}
- try {
- $this
- ->setStatus($new_status)
- ->save();
- } catch (Exception $ex) {
- $this->killTransaction();
- throw $ex;
- }
+ $this
+ ->setStatus($new_status)
+ ->save();
$this->saveTransaction();
@@ -210,12 +203,24 @@
$this->openTransaction();
- $this
- ->setStatus(DrydockResourceStatus::STATUS_ACTIVE)
- ->save();
-
+ try {
DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
$this->slotLocks = array();
+ } catch (DrydockSlotLockException $ex) {
+ $this->killTransaction();
+
+ $this->logEvent(
+ DrydockSlotLockFailureLogType::LOGCONST,
+ array(
+ 'locks' => $ex->getLockMap(),
+ ));
+
+ throw $ex;
+ }
+
+ $this
+ ->setStatus(DrydockResourceStatus::STATUS_ACTIVE)
+ ->save();
$this->saveTransaction();
diff --git a/src/applications/drydock/storage/DrydockSlotLock.php b/src/applications/drydock/storage/DrydockSlotLock.php
--- a/src/applications/drydock/storage/DrydockSlotLock.php
+++ b/src/applications/drydock/storage/DrydockSlotLock.php
@@ -149,6 +149,7 @@
// time we should be able to figure out which locks are already held.
$held = self::loadHeldLocks($locks);
$held = mpull($held, 'getOwnerPHID', 'getLockKey');
+
throw new DrydockSlotLockException($held);
}
}
diff --git a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
--- a/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
+++ b/src/applications/harbormaster/artifact/HarbormasterDrydockLeaseArtifact.php
@@ -29,7 +29,9 @@
}
public function willCreateArtifact(PhabricatorUser $actor) {
- $this->loadArtifactLease($actor);
+ // We don't load the lease here because it's expected that artifacts are
+ // created before leases actually exist. This guarantees that the leases
+ // will be cleaned up.
}
public function loadArtifactLease(PhabricatorUser $viewer) {
@@ -51,7 +53,15 @@
}
public function releaseArtifact(PhabricatorUser $actor) {
- $lease = $this->loadArtifactLease($actor);
+ try {
+ $lease = $this->loadArtifactLease($actor);
+ } catch (Exception $ex) {
+ // If we can't load the lease, treat it as already released. Artifacts
+ // are generated before leases are queued, so it's possible to arrive
+ // here under normal conditions.
+ return;
+ }
+
if (!$lease->canRelease()) {
return;
}
diff --git a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
--- a/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
+++ b/src/applications/harbormaster/step/HarbormasterLeaseWorkingCopyBuildStepImplementation.php
@@ -41,7 +41,7 @@
$working_copy_type = id(new DrydockWorkingCopyBlueprintImplementation())
->getType();
- $lease = id(new DrydockLease())
+ $lease = DrydockLease::initializeNewLease()
->setResourceType($working_copy_type)
->setOwnerPHID($build_target->getPHID());
@@ -54,6 +54,18 @@
$lease->setAwakenTaskIDs(array($task_id));
}
+ // TODO: Maybe add a method to mark artifacts like this as pending?
+
+ // Create the artifact now so that the lease is always disposed of, even
+ // if this target is aborted.
+ $build_target->createArtifact(
+ $viewer,
+ $settings['name'],
+ HarbormasterWorkingCopyArtifact::ARTIFACTCONST,
+ array(
+ 'drydockLeasePHID' => $lease->getPHID(),
+ ));
+
$lease->queueForActivation();
$build_target
@@ -73,14 +85,6 @@
'Lease "%s" never activated.',
$lease->getPHID()));
}
-
- $artifact = $build_target->createArtifact(
- $viewer,
- $settings['name'],
- HarbormasterWorkingCopyArtifact::ARTIFACTCONST,
- array(
- 'drydockLeasePHID' => $lease->getPHID(),
- ));
}
public function getArtifactOutputs() {
diff --git a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
--- a/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
+++ b/src/infrastructure/daemon/workers/management/PhabricatorWorkerManagementExecuteWorkflow.php
@@ -42,7 +42,8 @@
$task->getDataID());
$task->setData($task_data->getData());
- $console->writeOut(
+ echo tsprintf(
+ "%s\n",
pht(
'Executing task %d (%s)...',
$task->getID(),
diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
--- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
+++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php
@@ -1398,6 +1398,11 @@
'Setting retention policy for "%s" to %s days.',
),
+ 'Waiting %s second(s) for lease to activate.' => array(
+ 'Waiting a second for lease to activate.',
+ 'Waiting %s seconds for lease to activate.',
+ ),
+
);
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Jan 31, 3:08 PM (7 m, 37 s)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7078901
Default Alt Text
D14234.diff (12 KB)
Attached To
Mode
D14234: In Harbormaster, make sure artifacts are destroyed even if a build is aborted
Attached
Detach File
Event Timeline
Log In to Comment