Make various Drydock improvements

Summary:
Tightens up a bunch of stuff:

  - In `drydock lease`, pull and print logs so the user can see what's happening.
  - Remove `DrydockAllocator`, which was a dumb class that did nothing. Move the tiny amount of logic it held directly to `DrydockLease`.
  - Move `resourceType` from worker task metadata directly to `DrydockLease`. Other things (like the web UI) can be more informative with this information available.
  - Pass leases to `allocateResource()`. We always allocate in response to a lease activation request, and the lease often has vital information. This also allows us to associate logs with leases correctly.

Test Plan: Ran `drydock lease --type host` and saw it perform a host allocation in EC2.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

Differential Revision: https://secure.phabricator.com/D3870

resources/sql/patches/drydockresourcetype.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_drydock.drydock_lease
+  ADD resourceType VARCHAR(128) NOT NULL COLLATE utf8_bin;

src/__phutil_library_map__.php

@@ -411,8 +411,7 @@ phutil_register_library_map(array(
     'DiffusionURITestCase' => 'applications/diffusion/request/__tests__/DiffusionURITestCase.php',
     'DiffusionView' => 'applications/diffusion/view/DiffusionView.php',
     'DivinerListController' => 'applications/diviner/controller/DivinerListController.php',
-    'DrydockAllocator' => 'applications/drydock/allocator/DrydockAllocator.php',
-    'DrydockAllocatorWorker' => 'applications/drydock/allocator/DrydockAllocatorWorker.php',
+    'DrydockAllocatorWorker' => 'applications/drydock/worker/DrydockAllocatorWorker.php',
     'DrydockApacheWebrootBlueprint' => 'applications/drydock/blueprint/webroot/DrydockApacheWebrootBlueprint.php',
     'DrydockApacheWebrootInterface' => 'applications/drydock/interface/webroot/DrydockApacheWebrootInterface.php',
     'DrydockBlueprint' => 'applications/drydock/blueprint/DrydockBlueprint.php',

src/applications/drydock/allocator/DrydockAllocator.php

@@ -1,59 +0,0 @@
-<?php
-
-/*
- * Copyright 2012 Facebook, Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-final class DrydockAllocator {
-
-  private $resourceType;
-  private $lease;
-
-  public function setResourceType($resource_type) {
-    $this->resourceType = $resource_type;
-    return $this;
-  }
-
-  public function getResourceType() {
-    return $this->resourceType;
-  }
-
-  public function getPendingLease() {
-    if (!$this->lease) {
-      $lease = new DrydockLease();
-      $lease->setStatus(DrydockLeaseStatus::STATUS_PENDING);
-      $lease->save();
-
-      $this->lease = $lease;
-    }
-    return $lease;
-  }
-
-  public function allocate() {
-    $lease = $this->getPendingLease();
-
-    $data = array(
-      'type'        => $this->getResourceType(),
-      'lease'       => $lease->getID(),
-    );
-
-    $task = PhabricatorWorker::scheduleTask('DrydockAllocatorWorker', $data);
-
-    $lease->setTaskID($task->getID());
-
-    return $lease;
-  }
-
-}

src/applications/drydock/blueprint/DrydockBlueprint.php

@@ -33,13 +33,6 @@ abstract class DrydockBlueprint {
     return;
   }
 
-  protected function getAllocator($type) {
-    $allocator = new DrydockAllocator();
-    $allocator->setResourceType($type);
-
-    return $allocator;
-  }
-
   final public function acquireLease(
     DrydockResource $resource,
     DrydockLease $lease) {
@@ -101,13 +94,18 @@ abstract class DrydockBlueprint {
     return false;
   }
 
-  protected function executeAllocateResource() {
+  protected function executeAllocateResource(DrydockLease $lease) {
     throw new Exception("This blueprint can not allocate resources!");
   }
 
-  final public function allocateResource() {
+  final public function allocateResource(DrydockLease $lease) {
+    $this->activeLease = $lease;
+    $this->activeResource = null;
+
+    $this->log('Allocating Resource');
+
     try {
-      $resource = $this->executeAllocateResource();
+      $resource = $this->executeAllocateResource($lease);
     } catch (Exception $ex) {
       $this->logException($ex);
       $this->activeResource = null;

src/applications/drydock/blueprint/DrydockEC2HostBlueprint.php

@@ -22,7 +22,7 @@ final class DrydockEC2HostBlueprint extends DrydockRemoteHostBlueprint {
     return true;
   }
 
-  public function executeAllocateResource() {
+  public function executeAllocateResource(DrydockLease $lease) {
     $resource = $this->newResourceTemplate('EC2 Host');
 
     $resource->setStatus(DrydockResourceStatus::STATUS_ALLOCATING);
@@ -41,7 +41,7 @@ final class DrydockEC2HostBlueprint extends DrydockRemoteHostBlueprint {
 
     $instance_id = (string)$xml->instancesSet[0]->item[0]->instanceId[0];
 
-    $this->log('Started Instance: {$instance_id}');
+    $this->log("Started Instance: {$instance_id}");
     $resource->setAttribute('instance.id', $instance_id);
     $resource->save();
 

src/applications/drydock/blueprint/application/DrydockPhabricatorApplicationBlueprint.php

@@ -27,15 +27,16 @@ abstract class DrydockPhabricatorApplicationBlueprint
     return true;
   }
 
-  public function executeAllocateResource() {
+  public function executeAllocateResource(DrydockLease $lease) {
 
     $resource = $this->newResourceTemplate('Phabricator');
 
     $resource->setStatus(DrydockResourceStatus::STATUS_ALLOCATING);
     $resource->save();
 
-    $allocator = $this->getAllocator('host');
-    $host = $allocator->allocate();
+    $host = id(new DrydockLease())
+      ->setResourceType('host')
+      ->queueForActivation();
 
     $cmd = $host->waitUntilActive()->getInterface('command');
 

src/applications/drydock/blueprint/webroot/DrydockApacheWebrootBlueprint.php

@@ -76,7 +76,7 @@ EOSETUP
     $lease->save();
   }
 
-  public function executeAllocateResource() {
+  public function executeAllocateResource(DrydockLease $lease) {
 
     $resource = $this->newResourceTemplate('Apache');
 

src/applications/drydock/management/DrydockManagementLeaseWorkflow.php

@@ -31,7 +31,7 @@ final class DrydockManagementLeaseWorkflow
             'help'      => 'Resource type.',
           ),
           array(
-            'name'      => 'spec',
+            'name'      => 'attributes',
             'param'     => 'name=value,...',
             'help'      => 'Resource specficiation.',
           ),
@@ -47,19 +47,18 @@ final class DrydockManagementLeaseWorkflow
         "Specify a resource type with `--type`.");
     }
 
-    $spec = $args->getArg('spec');
-    if ($spec) {
+    $attributes = $args->getArg('attributes');
+    if ($attributes) {
       $options = new PhutilSimpleOptions();
-      $spec = $options->parse($spec);
+      $attributes = $options->parse($attributes);
     }
 
-    $allocator = new DrydockAllocator();
-    $allocator->setResourceType($resource_type);
-    if ($spec) {
-      // TODO: Shove this in there.
+    $lease = new DrydockLease();
+    $lease->setResourceType($resource_type);
+    if ($attributes) {
+      $lease->setAttributes($attributes);
     }
-
-    $lease = $allocator->allocate();
+    $lease->queueForActivation();
 
     $root = dirname(phutil_get_library_root('phabricator'));
     $wait = new ExecFuture(
@@ -67,14 +66,25 @@ final class DrydockManagementLeaseWorkflow
       $root.'/scripts/drydock/drydock_control.php',
       $lease->getID());
 
+    $cursor = 0;
     foreach (Futures(array($wait))->setUpdateInterval(1) as $key => $future) {
       if ($future) {
         $future->resolvex();
         break;
       }
 
-      // TODO: Pull logs.
-      $console->writeErr("Working...\n");
+      $logs = id(new DrydockLogQuery())
+        ->withLeaseIDs(array($lease->getID()))
+        ->withAfterID($cursor)
+        ->setOrder(DrydockLogQuery::ORDER_ID)
+        ->execute();
+
+      if ($logs) {
+        foreach ($logs as $log) {
+          $console->writeErr("%s\n", $log->getMessage());
+        }
+        $cursor = max(mpull($logs, 'getID'));
+      }
     }
 
     $console->writeOut("Acquired Lease %s\n", $lease->getID());

src/applications/drydock/query/DrydockLogQuery.php

@@ -18,8 +18,13 @@
 
 final class DrydockLogQuery extends PhabricatorOffsetPagedQuery {
 
+  const ORDER_EPOCH   = 'order-epoch';
+  const ORDER_ID      = 'order-id';
+
   private $resourceIDs;
   private $leaseIDs;
+  private $afterID;
+  private $order = self::ORDER_EPOCH;
 
   public function withResourceIDs(array $ids) {
     $this->resourceIDs = $ids;
@@ -31,26 +36,32 @@ final class DrydockLogQuery extends PhabricatorOffsetPagedQuery {
     return $this;
   }
 
+  public function setOrder($order) {
+    $this->order = $order;
+    return $this;
+  }
+
+  public function withAfterID($id) {
+    $this->afterID = $id;
+    return $this;
+  }
+
   public function execute() {
     $table = new DrydockLog();
     $conn_r = $table->establishConnection('r');
 
-    $where = $this->buildWhereClause($conn_r);
-    $order = $this->buildOrderClause($conn_r);
-    $limit = $this->buildLimitClause($conn_r);
-
     $data = queryfx_all(
       $conn_r,
       'SELECT log.* FROM %T log %Q %Q %Q',
       $table->getTableName(),
-      $where,
-      $order,
-      $limit);
+      $this->buildWhereClause($conn_r),
+      $this->buildOrderClause($conn_r),
+      $this->buildLimitClause($conn_r));
 
     return $table->loadAllFromArray($data);
   }
 
-  private function buildWhereClause($conn_r) {
+  private function buildWhereClause(AphrontDatabaseConnection $conn_r) {
     $where = array();
 
     if ($this->resourceIDs) {
@@ -67,11 +78,25 @@ final class DrydockLogQuery extends PhabricatorOffsetPagedQuery {
         $this->leaseIDs);
     }
 
+    if ($this->afterID) {
+      $where[] = qsprintf(
+        $conn_r,
+        'id > %d',
+        $this->afterID);
+    }
+
     return $this->formatWhereClause($where);
   }
 
-  private function buildOrderClause($conn_r) {
-    return 'ORDER BY log.epoch DESC';
+  private function buildOrderClause(AphrontDatabaseConnection $conn_r) {
+    switch ($this->order) {
+      case self::ORDER_EPOCH:
+        return 'ORDER BY log.epoch DESC';
+      case self::ORDER_ID:
+        return 'ORDER BY id ASC';
+      default:
+        throw new Exception("Unknown order '{$this->order}'!");
+    }
   }
 
 }

src/applications/drydock/storage/DrydockLease.php

@@ -20,10 +20,11 @@ final class DrydockLease extends DrydockDAO {
 
   protected $phid;
   protected $resourceID;
+  protected $resourceType;
   protected $until;
   protected $ownerPHID;
   protected $attributes = array();
-  protected $status;
+  protected $status = DrydockLeaseStatus::STATUS_PENDING;
   protected $taskID;
 
   private $resource;
@@ -76,10 +77,37 @@ final class DrydockLease extends DrydockDAO {
       $this->getResourceID());
   }
 
+  public function queueForActivation() {
+    if ($this->getID()) {
+      throw new Exception(
+        "Only new leases may be queued for activation!");
+    }
+
+    $this->setStatus(DrydockLeaseStatus::STATUS_PENDING);
+    $this->save();
+
+    // NOTE: Prevent a race where some eager worker quickly grabs the task
+    // before we can save the Task ID.
+
+    $this->openTransaction();
+      $this->beginReadLocking();
+
+        $this->reload();
+
+        $task = PhabricatorWorker::scheduleTask(
+          'DrydockAllocatorWorker',
+          $this->getID());
+
+        $this->setTaskID($task->getID());
+        $this->save();
+
+      $this->endReadLocking();
+    $this->saveTransaction();
+
+    return $this;
+  }
+
   public function release() {
-
-    // TODO: Insert a cleanup task into the taskmaster queue.
-
     $this->setStatus(DrydockLeaseStatus::STATUS_RELEASED);
     $this->save();
 

src/applications/drydock/worker/DrydockAllocatorWorker.php

@@ -19,20 +19,18 @@
 final class DrydockAllocatorWorker extends PhabricatorWorker {
 
   protected function doWork() {
-    $data = $this->getTaskData();
+    $lease_id = $this->getTaskData();
 
-    $lease = id(new DrydockLease())->loadOneWhere(
-      'id = %d',
-      $data['lease']);
+    $lease = id(new DrydockLease())->load($lease_id);
     if (!$lease) {
       return;
     }
 
-    $type = $data['type'];
+    $type = $lease->getResourceType();
 
     $candidates = id(new DrydockResource())->loadAllWhere(
       'type = %s AND status = %s',
-      $type,
+      $lease->getResourceType(),
       DrydockResourceStatus::STATUS_OPEN);
 
     if ($candidates) {
@@ -64,7 +62,7 @@ final class DrydockAllocatorWorker extends PhabricatorWorker {
       shuffle($blueprints);
 
       $blueprint = head($blueprints);
-      $resource = $blueprint->allocateResource();
+      $resource = $blueprint->allocateResource($lease);
     }
 
     $blueprint = $resource->getBlueprint();

src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php

@@ -1024,6 +1024,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList {
         'type'    => 'sql',
         'name'    => $this->getPatchPath('drydocktaskid.sql'),
       ),
+      'drydockresoucetype.sql' => array(
+        'type'    => 'sql',
+        'name'    => $this->getPatchPath('drydockresourcetype.sql'),
+      ),
     );
   }