Give Drydock resources a proper expiry mechanism
Summary: Fixes T6569. This implements an expiry mechanism for Drydock resources which parallels the mechanism for leases. A few things are missing that we'll probably need in the future: - An "EXPIRES" command to update the expiration time. This would let resources be permanent while leased, then expire after, say, 24 hours without any leases. - A callback like `shouldActuallyExpireRightNow()` for resources and leases that lets them decide not to expire at the last second. - A callback like `didAcquireLease()` for resource blueprints, to parallel `didReleaseLease()`, letting them clear or extend their timer. However, this stuff would mostly just let us tune behaviors, not really open up new capabilities. Test Plan: Changed host resources to expire after 60 seconds, leased one, saw it vanish 60 seconds later. Reviewers: hach-que, chad Reviewed By: chad Maniphest Tasks: T6569 Differential Revision: https://secure.phabricator.com/D14176
This commit is contained in:
		
							
								
								
									
										2
									
								
								resources/sql/autopatches/20150928.drydock.rexpire.1.sql
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										2
									
								
								resources/sql/autopatches/20150928.drydock.rexpire.1.sql
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,2 @@ | ||||
| ALTER TABLE {$NAMESPACE}_drydock.drydock_resource | ||||
|   ADD until INT UNSIGNED; | ||||
| @@ -8,4 +8,8 @@ final class DrydockDefaultViewCapability extends PhabricatorPolicyCapability { | ||||
|     return pht('Default Blueprint View Policy'); | ||||
|   } | ||||
|  | ||||
|   public function shouldAllowPublicPolicySetting() { | ||||
|     return true; | ||||
|   } | ||||
|  | ||||
| } | ||||
|   | ||||
| @@ -15,7 +15,6 @@ final class DrydockConsoleController extends DrydockController { | ||||
|     $nav->addFilter('blueprint', pht('Blueprints')); | ||||
|     $nav->addFilter('resource', pht('Resources')); | ||||
|     $nav->addFilter('lease', pht('Leases')); | ||||
|     $nav->addFilter('log', pht('Logs')); | ||||
|  | ||||
|     $nav->selectFilter(null); | ||||
|  | ||||
| @@ -31,6 +30,7 @@ final class DrydockConsoleController extends DrydockController { | ||||
|     $menu->addItem( | ||||
|       id(new PHUIObjectItemView()) | ||||
|         ->setHeader(pht('Blueprints')) | ||||
|         ->setFontIcon('fa-map-o') | ||||
|         ->setHref($this->getApplicationURI('blueprint/')) | ||||
|         ->addAttribute( | ||||
|           pht( | ||||
| @@ -40,6 +40,7 @@ final class DrydockConsoleController extends DrydockController { | ||||
|     $menu->addItem( | ||||
|       id(new PHUIObjectItemView()) | ||||
|         ->setHeader(pht('Resources')) | ||||
|         ->setFontIcon('fa-map') | ||||
|         ->setHref($this->getApplicationURI('resource/')) | ||||
|         ->addAttribute( | ||||
|           pht('View and manage resources Drydock has built, like hosts.'))); | ||||
| @@ -47,16 +48,10 @@ final class DrydockConsoleController extends DrydockController { | ||||
|     $menu->addItem( | ||||
|       id(new PHUIObjectItemView()) | ||||
|         ->setHeader(pht('Leases')) | ||||
|         ->setFontIcon('fa-link') | ||||
|         ->setHref($this->getApplicationURI('lease/')) | ||||
|         ->addAttribute(pht('Manage leases on resources.'))); | ||||
|  | ||||
|     $menu->addItem( | ||||
|       id(new PHUIObjectItemView()) | ||||
|         ->setHeader(pht('Logs')) | ||||
|         ->setHref($this->getApplicationURI('log/')) | ||||
|         ->addAttribute(pht('View logs.'))); | ||||
|  | ||||
|  | ||||
|     $crumbs = $this->buildApplicationCrumbs(); | ||||
|     $crumbs->addTextCrumb(pht('Console')); | ||||
|  | ||||
|   | ||||
| @@ -116,6 +116,14 @@ final class DrydockResourceViewController extends DrydockResourceController { | ||||
|       pht('Status'), | ||||
|       $status); | ||||
|  | ||||
|     $until = $resource->getUntil(); | ||||
|     if ($until) { | ||||
|       $until_display = phabricator_datetime($until, $viewer); | ||||
|     } else { | ||||
|       $until_display = phutil_tag('em', array(), pht('Never')); | ||||
|     } | ||||
|     $view->addProperty(pht('Expires'), $until_display); | ||||
|  | ||||
|     $view->addProperty( | ||||
|       pht('Resource Type'), | ||||
|       $resource->getType()); | ||||
|   | ||||
| @@ -295,6 +295,15 @@ final class DrydockLease extends DrydockDAO | ||||
|     } | ||||
|   } | ||||
|  | ||||
|   public function canUpdate() { | ||||
|     switch ($this->getStatus()) { | ||||
|       case DrydockLeaseStatus::STATUS_ACTIVE: | ||||
|         return true; | ||||
|       default: | ||||
|         return false; | ||||
|     } | ||||
|   } | ||||
|  | ||||
|   public function scheduleUpdate($epoch = null) { | ||||
|     PhabricatorWorker::scheduleTask( | ||||
|       'DrydockLeaseUpdateWorker', | ||||
|   | ||||
| @@ -7,6 +7,7 @@ final class DrydockResource extends DrydockDAO | ||||
|   protected $phid; | ||||
|   protected $blueprintPHID; | ||||
|   protected $status; | ||||
|   protected $until; | ||||
|  | ||||
|   protected $type; | ||||
|   protected $name; | ||||
| @@ -32,6 +33,7 @@ final class DrydockResource extends DrydockDAO | ||||
|         'ownerPHID' => 'phid?', | ||||
|         'status' => 'text32', | ||||
|         'type' => 'text64', | ||||
|         'until' => 'epoch?', | ||||
|       ), | ||||
|       self::CONFIG_KEY_SCHEMA => array( | ||||
|         'key_type' => array( | ||||
| @@ -126,6 +128,10 @@ final class DrydockResource extends DrydockDAO | ||||
|  | ||||
|     $this->isAllocated = true; | ||||
|  | ||||
|     if ($new_status == DrydockResourceStatus::STATUS_ACTIVE) { | ||||
|       $this->didActivate(); | ||||
|     } | ||||
|  | ||||
|     return $this; | ||||
|   } | ||||
|  | ||||
| @@ -164,6 +170,8 @@ final class DrydockResource extends DrydockDAO | ||||
|  | ||||
|     $this->isActivated = true; | ||||
|  | ||||
|     $this->didActivate(); | ||||
|  | ||||
|     return $this; | ||||
|   } | ||||
|  | ||||
| @@ -181,14 +189,16 @@ final class DrydockResource extends DrydockDAO | ||||
|     } | ||||
|   } | ||||
|  | ||||
|   public function scheduleUpdate() { | ||||
|   public function scheduleUpdate($epoch = null) { | ||||
|     PhabricatorWorker::scheduleTask( | ||||
|       'DrydockResourceUpdateWorker', | ||||
|       array( | ||||
|         'resourcePHID' => $this->getPHID(), | ||||
|         'isExpireTask' => ($epoch !== null), | ||||
|       ), | ||||
|       array( | ||||
|         'objectPHID' => $this->getPHID(), | ||||
|         'delayUntil' => $epoch, | ||||
|       )); | ||||
|   } | ||||
|  | ||||
| @@ -209,6 +219,20 @@ final class DrydockResource extends DrydockDAO | ||||
|     if ($need_update) { | ||||
|       $this->scheduleUpdate(); | ||||
|     } | ||||
|  | ||||
|     $expires = $this->getUntil(); | ||||
|     if ($expires) { | ||||
|       $this->scheduleUpdate($expires); | ||||
|     } | ||||
|   } | ||||
|  | ||||
|   public function canUpdate() { | ||||
|     switch ($this->getStatus()) { | ||||
|       case DrydockResourceStatus::STATUS_ACTIVE: | ||||
|         return true; | ||||
|       default: | ||||
|         return false; | ||||
|     } | ||||
|   } | ||||
|  | ||||
|  | ||||
|   | ||||
| @@ -23,31 +23,15 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { | ||||
|   } | ||||
|  | ||||
|   private function updateLease(DrydockLease $lease) { | ||||
|     if ($lease->getStatus() != DrydockLeaseStatus::STATUS_ACTIVE) { | ||||
|     if (!$lease->canUpdate()) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $viewer = $this->getViewer(); | ||||
|     $drydock_phid = id(new PhabricatorDrydockApplication())->getPHID(); | ||||
|  | ||||
|     // Check if the lease has expired. If it is, we're going to send it a | ||||
|     // release command. This command will be handled immediately below, it | ||||
|     // just generates a command log and improves consistency. | ||||
|     $now = PhabricatorTime::getNow(); | ||||
|     $expires = $lease->getUntil(); | ||||
|     if ($expires && ($expires <= $now)) { | ||||
|       $command = DrydockCommand::initializeNewCommand($viewer) | ||||
|         ->setTargetPHID($lease->getPHID()) | ||||
|         ->setAuthorPHID($drydock_phid) | ||||
|         ->setCommand(DrydockCommand::COMMAND_RELEASE) | ||||
|         ->save(); | ||||
|     } | ||||
|     $this->checkLeaseExpiration($lease); | ||||
|  | ||||
|     $commands = $this->loadCommands($lease->getPHID()); | ||||
|     foreach ($commands as $command) { | ||||
|       if ($lease->getStatus() != DrydockLeaseStatus::STATUS_ACTIVE) { | ||||
|         // Leases can't receive commands before they activate or after they | ||||
|         // release. | ||||
|       if (!$lease->canUpdate()) { | ||||
|         break; | ||||
|       } | ||||
|  | ||||
| @@ -58,15 +42,7 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker { | ||||
|         ->save(); | ||||
|     } | ||||
|  | ||||
|     // If this is the task which will eventually release the lease after it | ||||
|     // expires but it is still active, reschedule the task to run after the | ||||
|     // lease expires. This can happen if the lease's expiration was pushed | ||||
|     // forward. | ||||
|     if ($lease->getStatus() == DrydockLeaseStatus::STATUS_ACTIVE) { | ||||
|       if ($this->getTaskDataValue('isExpireTask') && $expires) { | ||||
|         throw new PhabricatorWorkerYieldException($expires - $now); | ||||
|       } | ||||
|     } | ||||
|     $this->yieldIfExpiringLease($lease); | ||||
|   } | ||||
|  | ||||
|   private function processCommand( | ||||
|   | ||||
| @@ -11,18 +11,27 @@ final class DrydockResourceUpdateWorker extends DrydockWorker { | ||||
|     $lock = PhabricatorGlobalLock::newLock($lock_key) | ||||
|       ->lock(1); | ||||
|  | ||||
|     $resource = $this->loadResource($resource_phid); | ||||
|     $this->updateResource($resource); | ||||
|     try { | ||||
|       $resource = $this->loadResource($resource_phid); | ||||
|       $this->updateResource($resource); | ||||
|     } catch (Exception $ex) { | ||||
|       $lock->unlock(); | ||||
|       throw $ex; | ||||
|     } | ||||
|  | ||||
|     $lock->unlock(); | ||||
|   } | ||||
|  | ||||
|   private function updateResource(DrydockResource $resource) { | ||||
|     if (!$resource->canUpdate()) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $this->checkResourceExpiration($resource); | ||||
|  | ||||
|     $commands = $this->loadCommands($resource->getPHID()); | ||||
|     foreach ($commands as $command) { | ||||
|       if ($resource->getStatus() != DrydockResourceStatus::STATUS_ACTIVE) { | ||||
|         // Resources can't receive commands before they activate or after they | ||||
|         // release. | ||||
|       if (!$resource->canUpdate()) { | ||||
|         break; | ||||
|       } | ||||
|  | ||||
| @@ -32,6 +41,8 @@ final class DrydockResourceUpdateWorker extends DrydockWorker { | ||||
|         ->setIsConsumed(true) | ||||
|         ->save(); | ||||
|     } | ||||
|  | ||||
|     $this->yieldIfExpiringResource($resource); | ||||
|   } | ||||
|  | ||||
|   private function processCommand( | ||||
|   | ||||
| @@ -50,4 +50,68 @@ abstract class DrydockWorker extends PhabricatorWorker { | ||||
|     return $commands; | ||||
|   } | ||||
|  | ||||
|   protected function checkLeaseExpiration(DrydockLease $lease) { | ||||
|     $this->checkObjectExpiration($lease); | ||||
|   } | ||||
|  | ||||
|   protected function checkResourceExpiration(DrydockResource $resource) { | ||||
|     $this->checkObjectExpiration($resource); | ||||
|   } | ||||
|  | ||||
|   private function checkObjectExpiration($object) { | ||||
|     // Check if the resource or lease has expired. If it has, we're going to | ||||
|     // send it a release command. | ||||
|  | ||||
|     // This command is sent from within the update worker so it is handled | ||||
|     // immediately, but doing this generates a log and improves consistency. | ||||
|  | ||||
|     $expires = $object->getUntil(); | ||||
|     if (!$expires) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $now = PhabricatorTime::getNow(); | ||||
|     if ($expires > $now) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $viewer = $this->getViewer(); | ||||
|     $drydock_phid = id(new PhabricatorDrydockApplication())->getPHID(); | ||||
|  | ||||
|     $command = DrydockCommand::initializeNewCommand($viewer) | ||||
|       ->setTargetPHID($object->getPHID()) | ||||
|       ->setAuthorPHID($drydock_phid) | ||||
|       ->setCommand(DrydockCommand::COMMAND_RELEASE) | ||||
|       ->save(); | ||||
|   } | ||||
|  | ||||
|   protected function yieldIfExpiringLease(DrydockLease $lease) { | ||||
|     if (!$lease->canUpdate()) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $this->yieldIfExpiring($lease->getUntil()); | ||||
|   } | ||||
|  | ||||
|   protected function yieldIfExpiringResource(DrydockResource $resource) { | ||||
|     if (!$resource->canUpdate()) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $this->yieldIfExpiring($resource->getUntil()); | ||||
|   } | ||||
|  | ||||
|   private function yieldIfExpiring($expires) { | ||||
|     if (!$expires) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     if (!$this->getTaskDataValue('isExpireTask')) { | ||||
|       return; | ||||
|     } | ||||
|  | ||||
|     $now = PhabricatorTime::getNow(); | ||||
|     throw new PhabricatorWorkerYieldException($expires - $now); | ||||
|   } | ||||
|  | ||||
| } | ||||
|   | ||||
| @@ -39,6 +39,13 @@ abstract class PhabricatorWorkerManagementWorkflow | ||||
|       } | ||||
|     } | ||||
|  | ||||
|     // When we lock tasks properly, this gets populated as a side effect. Just | ||||
|     // fake it when doing manual CLI stuff. This makes sure CLI yields have | ||||
|     // their expires times set properly. | ||||
|     foreach ($tasks as $task) { | ||||
|       $task->setServerTime(PhabricatorTime::getNow()); | ||||
|     } | ||||
|  | ||||
|     return $tasks; | ||||
|   } | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley