Modernize Drydock CLI management of task execution

Summary:
Ref T2015. Currently, Drydock has a `wait-for-lease` workflow which is invoked in the background by the `lease` workflow.

The goal of this mechanism is to allow `bin/drydock lease` to print out logs as the lease is acquired. However, this predates the `runAllTasksInProcess` flags, and they provide a simpler and more robust way (potentially with `--trace` and `PhutilConsole`) to do synchronous execution and debug logging.

Simplify this whole mechanism: just run everything in-process in `bin/drydock lease`, and do logging via `--trace`. We could thread a `PhutilConsole` through things too, but this seems good enough for now.

Also various cleanup/etc.

Test Plan: Ran `bin/drydock lease`. Ran `bin/harbormaster build X --plan Y`, for `Y` being a Drydock-dependent build plan.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

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

scripts/drydock/drydock_control.php

@@ -15,7 +15,6 @@ EOSYNOPSIS
 $args->parseStandardArguments();
 
 $workflows = array(
-  new DrydockManagementWaitForLeaseWorkflow(),
   new DrydockManagementLeaseWorkflow(),
   new DrydockManagementCloseWorkflow(),
   new DrydockManagementReleaseWorkflow(),

src/__phutil_library_map__.php

@@ -671,7 +671,6 @@ phutil_register_library_map(array(
     'DrydockManagementCreateResourceWorkflow' => 'applications/drydock/management/DrydockManagementCreateResourceWorkflow.php',
     'DrydockManagementLeaseWorkflow' => 'applications/drydock/management/DrydockManagementLeaseWorkflow.php',
     'DrydockManagementReleaseWorkflow' => 'applications/drydock/management/DrydockManagementReleaseWorkflow.php',
-    'DrydockManagementWaitForLeaseWorkflow' => 'applications/drydock/management/DrydockManagementWaitForLeaseWorkflow.php',
     'DrydockManagementWorkflow' => 'applications/drydock/management/DrydockManagementWorkflow.php',
     'DrydockPHIDTypeBlueprint' => 'applications/drydock/phid/DrydockPHIDTypeBlueprint.php',
     'DrydockPHIDTypeLease' => 'applications/drydock/phid/DrydockPHIDTypeLease.php',
@@ -3099,7 +3098,6 @@ phutil_register_library_map(array(
     'DrydockManagementCreateResourceWorkflow' => 'DrydockManagementWorkflow',
     'DrydockManagementLeaseWorkflow' => 'DrydockManagementWorkflow',
     'DrydockManagementReleaseWorkflow' => 'DrydockManagementWorkflow',
-    'DrydockManagementWaitForLeaseWorkflow' => 'DrydockManagementWorkflow',
     'DrydockManagementWorkflow' => 'PhutilArgumentWorkflow',
     'DrydockPHIDTypeBlueprint' => 'PhabricatorPHIDType',
     'DrydockPHIDTypeLease' => 'PhabricatorPHIDType',

src/applications/drydock/controller/DrydockBlueprintListController.php

@@ -53,4 +53,14 @@ final class DrydockBlueprintListController extends DrydockBlueprintController
     return $view;
   }
 
+  public function buildApplicationCrumbs() {
+    $crumbs = parent::buildApplicationCrumbs();
+    $crumbs->addAction(
+      id(new PHUIListItemView())
+        ->setName(pht('New Blueprint'))
+        ->setHref($this->getApplicationURI('/blueprint/create/'))
+        ->setIcon('create'));
+    return $crumbs;
+  }
+
 }

src/applications/drydock/management/DrydockManagementLeaseWorkflow.php

@@ -38,38 +38,16 @@ final class DrydockManagementLeaseWorkflow
       $attributes = $options->parse($attributes);
     }
 
-    $lease = new DrydockLease();
+    PhabricatorWorker::setRunAllTasksInProcess(true);
-    $lease->setResourceType($resource_type);
+
+    $lease = id(new DrydockLease())
+      ->setResourceType($resource_type);
     if ($attributes) {
       $lease->setAttributes($attributes);
     }
-    $lease->queueForActivation();
+    $lease
-
+      ->queueForActivation()
-    $root = dirname(phutil_get_library_root('phabricator'));
+      ->waitUntilActive();
-    $wait = new ExecFuture(
-      'php -f %s wait-for-lease --id %s',
-      $root.'/scripts/drydock/drydock_control.php',
-      $lease->getID());
-
-    $cursor = 0;
-    foreach (Futures(array($wait))->setUpdateInterval(1) as $key => $future) {
-      if ($future) {
-        $future->resolvex();
-        break;
-      }
-
-      $logs = id(new DrydockLogQuery())
-        ->setViewer(PhabricatorUser::getOmnipotentUser())
-        ->withLeaseIDs(array($lease->getID()))
-        ->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());
     return 0;

src/applications/drydock/management/DrydockManagementWaitForLeaseWorkflow.php

@@ -1,40 +0,0 @@
-<?php
-
-final class DrydockManagementWaitForLeaseWorkflow
-  extends DrydockManagementWorkflow {
-
-  public function didConstruct() {
-    $this
-      ->setName('wait-for-lease')
-      ->setSynopsis('Wait for a lease to become available.')
-      ->setArguments(
-        array(
-          array(
-            'name'      => 'id',
-            'param'     => 'lease_id',
-            'help'      => 'Lease ID to wait for.',
-          ),
-        ));
-  }
-
-  public function execute(PhutilArgumentParser $args) {
-    $lease_id = $args->getArg('id');
-    if (!$lease_id) {
-      throw new PhutilArgumentUsageException(
-        "Specify a lease ID with `--id`.");
-    }
-
-    $console = PhutilConsole::getConsole();
-
-    $lease = id(new DrydockLease())->load($lease_id);
-    if (!$lease) {
-      $console->writeErr("No such lease.\n");
-      return 1;
-    } else {
-      $lease->waitUntilActive();
-      $console->writeErr("Lease active.\n");
-      return 0;
-    }
-  }
-
-}

src/applications/drydock/query/DrydockLogQuery.php

@@ -40,10 +40,6 @@ final class DrydockLogQuery extends PhabricatorCursorPagedPolicyAwareQuery {
 
     foreach ($logs as $key => $log) {
       $resource = idx($resources, $log->getResourceID());
-      if (!$resource) {
-        unset($logs[$key]);
-        continue;
-      }
       $log->attachResource($resource);
     }
 

src/applications/drydock/storage/DrydockLease.php

@@ -91,23 +91,17 @@ final class DrydockLease extends DrydockDAO
     $this->setStatus(DrydockLeaseStatus::STATUS_PENDING);
     $this->save();
 
-    // NOTE: Prevent a race where some eager worker quickly grabs the task
+    $task = PhabricatorWorker::scheduleTask(
-    // before we can save the Task ID.
+      'DrydockAllocatorWorker',
+      $this->getID());
 
-    $this->openTransaction();
+    // NOTE: Scheduling the task might execute it in-process, if we're running
-      $this->beginReadLocking();
+    // from a CLI script. Reload the lease to make sure we have the most
+    // up-to-date information. Normally, this has no effect.
+    $this->reload();
 
-        $this->reload();
+    $this->setTaskID($task->getID());
-
+    $this->save();
-        $task = PhabricatorWorker::scheduleTask(
-          'DrydockAllocatorWorker',
-          $this->getID());
-
-        $this->setTaskID($task->getID());
-        $this->save();
-
-      $this->endReadLocking();
-    $this->saveTransaction();
 
     return $this;
   }
@@ -163,6 +157,8 @@ final class DrydockLease extends DrydockDAO
           case DrydockLeaseStatus::STATUS_PENDING:
           case DrydockLeaseStatus::STATUS_ACQUIRING:
             break;
+          default:
+            throw new Exception("Unknown status??");
         }
       }
 

src/applications/drydock/storage/DrydockLog.php

@@ -16,7 +16,7 @@ final class DrydockLog extends DrydockDAO
     ) + parent::getConfiguration();
   }
 
-  public function attachResource(DrydockResource $resource) {
+  public function attachResource(DrydockResource $resource = null) {
     $this->resource = $resource;
     return $this;
   }
@@ -36,10 +36,16 @@ final class DrydockLog extends DrydockDAO
   }
 
   public function getPolicy($capability) {
+    if (!$this->getResource()) {
+      return PhabricatorPolicies::getMostOpenPolicy();
+    }
     return $this->getResource()->getPolicy($capability);
   }
 
   public function hasAutomaticCapability($capability, PhabricatorUser $viewer) {
+    if (!$this->getResource()) {
+      return false;
+    }
     return $this->getResource()->hasAutomaticCapability($capability, $viewer);
   }
 

src/infrastructure/daemon/workers/PhabricatorWorker.php

@@ -85,14 +85,28 @@ abstract class PhabricatorWorker {
   }
 
   final public static function scheduleTask($task_class, $data) {
+    $task = id(new PhabricatorWorkerActiveTask())
+      ->setTaskClass($task_class)
+      ->setData($data);
+
     if (self::$runAllTasksInProcess) {
+      // Do the work in-process.
       $worker = newv($task_class, array($data));
       $worker->doWork();
+
+      // Now, save a task row and immediately archive it so we can return an
+      // object with a valid ID.
+      $task->openTransaction();
+        $task->save();
+        $archived = $task->archiveTask(
+          PhabricatorWorkerArchiveTask::RESULT_SUCCESS,
+          0);
+      $task->saveTransaction();
+
+      return $archived;
     } else {
-      return id(new PhabricatorWorkerActiveTask())
+      $task->save();
-        ->setTaskClass($task_class)
+      return $task;
-        ->setData($data)
-        ->save();
     }
   }
 
@@ -105,6 +119,10 @@ abstract class PhabricatorWorker {
    * @return void
    */
   final public static function waitForTasks(array $task_ids) {
+    if (!$task_ids) {
+      return;
+    }
+
     $task_table = new PhabricatorWorkerActiveTask();
 
     $waiting = array_fuse($task_ids);
@@ -149,7 +167,8 @@ abstract class PhabricatorWorker {
 
     foreach ($tasks as $task) {
       if ($task->getResult() != PhabricatorWorkerArchiveTask::RESULT_SUCCESS) {
-        throw new Exception("Task ".$task->getID()." failed!");
+        throw new Exception(
+          pht("Task %d failed!", $task->getID()));
       }
     }
   }