From 30a0b103e6f26c289f13caed7f365250b82f41fb Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Tue, 13 Feb 2018 06:12:32 -0800
Subject: [PATCH] When a lease acquires a resource but the resource fails to
 activate, throw the lease back in the pool

Summary:
Depends on D19075. Ref T13073. If a lease acquires a resource but finds that the resource builds directly into a dead state (which can happen for any number of reasonable reasons), reset the lease and throw it back in the pool.

This isn't the lease's fault and it hasn't caused any side effects or done anything we can't undo, so we can safely reset it and throw it back in the pool.

Test Plan:
  - Created a blueprint which throws from `allocateResource()` so that resources never activate.
  - Tried to lease it with `bin/drydock lease ...`.
  - Before patch: lease was broken and destroyed after a failed activation.
  - After patch: lease was returned to the pool after a failed activation.

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13073

Differential Revision: https://secure.phabricator.com/D19076
---
 src/__phutil_library_map__.php                |  4 +++
 ...DrydockAcquiredBrokenResourceException.php |  4 +++
 .../logtype/DrydockLeaseReacquireLogType.php  | 26 ++++++++++++++++++
 .../worker/DrydockLeaseUpdateWorker.php       | 27 +++++++++++++++++--
 4 files changed, 59 insertions(+), 2 deletions(-)
 create mode 100644 src/applications/drydock/exception/DrydockAcquiredBrokenResourceException.php
 create mode 100644 src/applications/drydock/logtype/DrydockLeaseReacquireLogType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 116aa31629..e26d68f72b 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -1000,6 +1000,7 @@ phutil_register_library_map(array(
     'DoorkeeperSchemaSpec' => 'applications/doorkeeper/storage/DoorkeeperSchemaSpec.php',
     'DoorkeeperTagView' => 'applications/doorkeeper/view/DoorkeeperTagView.php',
     'DoorkeeperTagsController' => 'applications/doorkeeper/controller/DoorkeeperTagsController.php',
+    'DrydockAcquiredBrokenResourceException' => 'applications/drydock/exception/DrydockAcquiredBrokenResourceException.php',
     'DrydockAlmanacServiceHostBlueprintImplementation' => 'applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php',
     'DrydockApacheWebrootInterface' => 'applications/drydock/interface/webroot/DrydockApacheWebrootInterface.php',
     'DrydockAuthorization' => 'applications/drydock/storage/DrydockAuthorization.php',
@@ -1065,6 +1066,7 @@ phutil_register_library_map(array(
     'DrydockLeasePHIDType' => 'applications/drydock/phid/DrydockLeasePHIDType.php',
     'DrydockLeaseQuery' => 'applications/drydock/query/DrydockLeaseQuery.php',
     'DrydockLeaseQueuedLogType' => 'applications/drydock/logtype/DrydockLeaseQueuedLogType.php',
+    'DrydockLeaseReacquireLogType' => 'applications/drydock/logtype/DrydockLeaseReacquireLogType.php',
     'DrydockLeaseReclaimLogType' => 'applications/drydock/logtype/DrydockLeaseReclaimLogType.php',
     'DrydockLeaseReleaseController' => 'applications/drydock/controller/DrydockLeaseReleaseController.php',
     'DrydockLeaseReleasedLogType' => 'applications/drydock/logtype/DrydockLeaseReleasedLogType.php',
@@ -6199,6 +6201,7 @@ phutil_register_library_map(array(
     'DoorkeeperSchemaSpec' => 'PhabricatorConfigSchemaSpec',
     'DoorkeeperTagView' => 'AphrontView',
     'DoorkeeperTagsController' => 'PhabricatorController',
+    'DrydockAcquiredBrokenResourceException' => 'Exception',
     'DrydockAlmanacServiceHostBlueprintImplementation' => 'DrydockBlueprintImplementation',
     'DrydockApacheWebrootInterface' => 'DrydockWebrootInterface',
     'DrydockAuthorization' => array(
@@ -6285,6 +6288,7 @@ phutil_register_library_map(array(
     'DrydockLeasePHIDType' => 'PhabricatorPHIDType',
     'DrydockLeaseQuery' => 'DrydockQuery',
     'DrydockLeaseQueuedLogType' => 'DrydockLogType',
+    'DrydockLeaseReacquireLogType' => 'DrydockLogType',
     'DrydockLeaseReclaimLogType' => 'DrydockLogType',
     'DrydockLeaseReleaseController' => 'DrydockLeaseController',
     'DrydockLeaseReleasedLogType' => 'DrydockLogType',
diff --git a/src/applications/drydock/exception/DrydockAcquiredBrokenResourceException.php b/src/applications/drydock/exception/DrydockAcquiredBrokenResourceException.php
new file mode 100644
index 0000000000..12b839d466
--- /dev/null
+++ b/src/applications/drydock/exception/DrydockAcquiredBrokenResourceException.php
@@ -0,0 +1,4 @@
+<?php
+
+final class DrydockAcquiredBrokenResourceException
+  extends Exception {}
diff --git a/src/applications/drydock/logtype/DrydockLeaseReacquireLogType.php b/src/applications/drydock/logtype/DrydockLeaseReacquireLogType.php
new file mode 100644
index 0000000000..13e759690c
--- /dev/null
+++ b/src/applications/drydock/logtype/DrydockLeaseReacquireLogType.php
@@ -0,0 +1,26 @@
+<?php
+
+final class DrydockLeaseReacquireLogType extends DrydockLogType {
+
+  const LOGCONST = 'core.lease.reacquire';
+
+  public function getLogTypeName() {
+    return pht('Reacquiring Resource');
+  }
+
+  public function getLogTypeIcon(array $data) {
+    return 'fa-refresh yellow';
+  }
+
+  public function renderLog(array $data) {
+    $class = idx($data, 'class');
+    $message = idx($data, 'message');
+
+    return pht(
+      'Lease acquired a resource but failed to activate; acquisition '.
+      'will be retried: [%s] %s',
+      $class,
+      $message);
+  }
+
+}
diff --git a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
index 08c69291b7..55d9136cef 100644
--- a/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
+++ b/src/applications/drydock/worker/DrydockLeaseUpdateWorker.php
@@ -43,6 +43,26 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
   private function handleUpdate(DrydockLease $lease) {
     try {
       $this->updateLease($lease);
+    } catch (DrydockAcquiredBrokenResourceException $ex) {
+      // If this lease acquired a resource but failed to activate, we don't
+      // need to break the lease. We can throw it back in the pool and let
+      // it take another shot at acquiring a new resource.
+
+      // Before we throw it back, release any locks the lease is holding.
+      DrydockSlotLock::releaseLocks($lease->getPHID());
+
+      $lease
+        ->setStatus(DrydockLeaseStatus::STATUS_PENDING)
+        ->save();
+
+      $lease->logEvent(
+        DrydockLeaseReacquireLogType::LOGCONST,
+        array(
+          'class' => get_class($ex),
+          'message' => $ex->getMessage(),
+        ));
+
+      $this->yieldLease($lease, $ex);
     } catch (Exception $ex) {
       if ($this->isTemporaryException($ex)) {
         $this->yieldLease($lease, $ex);
@@ -749,9 +769,12 @@ final class DrydockLeaseUpdateWorker extends DrydockWorker {
     }
 
     if ($resource_status != DrydockResourceStatus::STATUS_ACTIVE) {
-      throw new Exception(
+      throw new DrydockAcquiredBrokenResourceException(
         pht(
-          'Trying to activate lease on a dead resource (in status "%s").',
+          'Trying to activate lease ("%s") on a resource ("%s") in '.
+          'the wrong status ("%s").',
+          $lease->getPHID(),
+          $resource->getPHID(),
           $resource_status));
     }