From 57b4b5981947d5ba6d601bfb399b8763141f94e0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 25 Oct 2018 05:44:34 -0700
Subject: [PATCH] When a Drydock host based on an Almanac blueprint has its
 binding disabled, stop handing out leases

Summary:
Ref T13210. Ref T12145. The "Almanac Host" blueprint currently hands out new leases on a given host even if the binding has been disabled.

Although there are some more complicated cases here (e.g., involving cleanup of the existing resource and existing leases), this one seems clear cut: if the binding has been disabled, we should stop handing out new leases on it.

Test Plan:
  - Created a service with two hosts.
  - Requested a lease, got host A.
  - Requested more leases, always got host A (we never build a new host when we don't have to, and we currently never have to).
  - Disabled the binding to host A.
  - Requested a lease.
    - Before patch: got host A.
    - After patch: got host B.
  - Also disabled the other binding to host B, requested a lease, got an indefinite wait for resources (which is expected and reasonable).

Reviewers: amckinley

Reviewed By: amckinley

Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13210, T12145

Differential Revision: https://secure.phabricator.com/D19761
---
 ...anacServiceHostBlueprintImplementation.php | 54 +++++++++++++------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
index ef904e1019..4724a2c807 100644
--- a/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
+++ b/src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php
@@ -113,6 +113,14 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     DrydockBlueprint $blueprint,
     DrydockResource $resource,
     DrydockLease $lease) {
+
+    // Require the binding to a given host be active before we'll hand out more
+    // leases on the corresponding resource.
+    $binding = $this->loadBindingForResource($resource);
+    if ($binding->getIsDisabled()) {
+      return false;
+    }
+
     return true;
   }
 
@@ -154,24 +162,10 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     DrydockLease $lease,
     $type) {
 
-    $viewer = PhabricatorUser::getOmnipotentUser();
-
     switch ($type) {
       case DrydockCommandInterface::INTERFACE_TYPE:
         $credential_phid = $blueprint->getFieldValue('credentialPHID');
-        $binding_phid = $resource->getAttribute('almanacBindingPHID');
-
-        $binding = id(new AlmanacBindingQuery())
-          ->setViewer($viewer)
-          ->withPHIDs(array($binding_phid))
-          ->executeOne();
-        if (!$binding) {
-          throw new Exception(
-            pht(
-              'Unable to load binding "%s" to create command interface.',
-              $binding_phid));
-        }
-
+        $binding = $this->loadBindingForResource($resource);
         $interface = $binding->getInterface();
 
         return id(new DrydockSSHCommandInterface())
@@ -213,7 +207,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
             $blueprint->getBlueprintName()));
       }
 
-      $viewer = PhabricatorUser::getOmnipotentUser();
+      $viewer = $this->getViewer();
       $services = id(new AlmanacServiceQuery())
         ->setViewer($viewer)
         ->withPHIDs($service_phids)
@@ -246,7 +240,7 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
 
   private function loadFreeBindings(DrydockBlueprint $blueprint) {
     if ($this->freeBindings === null) {
-      $viewer = PhabricatorUser::getOmnipotentUser();
+      $viewer = $this->getViewer();
 
       $pool = id(new DrydockResourceQuery())
         ->setViewer($viewer)
@@ -293,5 +287,31 @@ final class DrydockAlmanacServiceHostBlueprintImplementation
     );
   }
 
+  private function loadBindingForResource(DrydockResource $resource) {
+    $binding_phid = $resource->getAttribute('almanacBindingPHID');
+    if (!$binding_phid) {
+      throw new Exception(
+        pht(
+          'Drydock resource ("%s") has no Almanac binding PHID, so its '.
+          'binding can not be loaded.',
+          $resource->getPHID()));
+    }
+
+    $viewer = $this->getViewer();
+
+    $binding = id(new AlmanacBindingQuery())
+      ->setViewer($viewer)
+      ->withPHIDs(array($binding_phid))
+      ->executeOne();
+    if (!$binding) {
+      throw new Exception(
+        pht(
+          'Unable to load Almanac binding ("%s") for resource ("%s").',
+          $binding_phid,
+          $resource->getPHID()));
+    }
+
+    return $binding;
+  }
 
 }