Abort previous build targets when a build is restarted

Summary: Ref T5936. This implements build implementations aborting early when the build has since been restarted.   Build steps now periodically poll to see if the build's current generation does not match their generation, and they throw a `HarbormasterBuildAbortedException` if that is the case.

Test Plan: Tested locally on my machine with the sleep build step.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T5936

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

src/__phutil_library_map__.php

@@ -654,6 +654,7 @@ phutil_register_library_map(array(
     'FlagEditConduitAPIMethod' => 'applications/flag/conduit/FlagEditConduitAPIMethod.php',
     'FlagQueryConduitAPIMethod' => 'applications/flag/conduit/FlagQueryConduitAPIMethod.php',
     'HarbormasterBuild' => 'applications/harbormaster/storage/build/HarbormasterBuild.php',
+    'HarbormasterBuildAbortedException' => 'applications/harbormaster/exception/HarbormasterBuildAbortedException.php',
     'HarbormasterBuildActionController' => 'applications/harbormaster/controller/HarbormasterBuildActionController.php',
     'HarbormasterBuildArtifact' => 'applications/harbormaster/storage/build/HarbormasterBuildArtifact.php',
     'HarbormasterBuildArtifactQuery' => 'applications/harbormaster/query/HarbormasterBuildArtifactQuery.php',
@@ -3419,6 +3420,7 @@ phutil_register_library_map(array(
       'HarbormasterDAO',
       'PhabricatorPolicyInterface',
     ),
+    'HarbormasterBuildAbortedException' => 'Exception',
     'HarbormasterBuildActionController' => 'HarbormasterController',
     'HarbormasterBuildArtifact' => array(
       'HarbormasterDAO',

src/applications/harbormaster/engine/HarbormasterBuildEngine.php

@@ -130,9 +130,9 @@ final class HarbormasterBuildEngine extends Phobject {
     // Increment the build generation counter on the build.
     $build->setBuildGeneration($build->getBuildGeneration() + 1);
 
-    // TODO: Currently running targets should periodically check their build
+    // Currently running targets should periodically check their build
     // generation (which won't have changed) against the build's generation.
-    // If it is different, they should automatically stop what they're doing
+    // If it is different, they will automatically stop what they're doing
     // and abort.
 
     // Previously we used to delete targets, logs and artifacts here.  Instead

src/applications/harbormaster/exception/HarbormasterBuildAbortedException.php

@@ -0,0 +1,5 @@
+<?php
+
+final class HarbormasterBuildAbortedException extends Exception {
+
+}

src/applications/harbormaster/step/HarbormasterBuildStepImplementation.php

@@ -222,4 +222,29 @@ abstract class HarbormasterBuildStepImplementation {
     return (bool)$target->getDetail('builtin.wait-for-message');
   }
 
+  protected function shouldAbort(
+    HarbormasterBuild $build,
+    HarbormasterBuildTarget $target) {
+
+    return $build->getBuildGeneration() !== $target->getBuildGeneration();
+  }
+
+  protected function resolveFuture(
+    HarbormasterBuild $build,
+    HarbormasterBuildTarget $target,
+    Future $future) {
+
+    $futures = Futures(array($future));
+    foreach ($futures->setUpdateInterval(5) as $key => $future) {
+      if ($future === null) {
+        $build->reload();
+        if ($this->shouldAbort($build, $target)) {
+          throw new HarbormasterBuildAbortedException();
+        }
+      } else {
+        return $future->resolve();
+      }
+    }
+  }
+
 }

src/applications/harbormaster/step/HarbormasterCommandBuildStepImplementation.php

@@ -62,26 +62,49 @@ final class HarbormasterCommandBuildStepImplementation
     $start_stdout = $log_stdout->start();
     $start_stderr = $log_stderr->start();
 
+    $build_update = 5;
+
     // Read the next amount of available output every second.
-    while (!$future->isReady()) {
-      list($stdout, $stderr) = $future->read();
-      $log_stdout->append($stdout);
-      $log_stderr->append($stderr);
-      $future->discardBuffers();
+    $futures = Futures(array($future));
+    foreach ($futures->setUpdateInterval(1) as $key => $future_iter) {
+      if ($future_iter === null) {
 
-      // Wait one second before querying for more data.
-      sleep(1);
+        // Check to see if we should abort.
+        if ($build_update <= 0) {
+          $build->reload();
+          if ($this->shouldAbort($build, $build_target)) {
+            $future->resolveKill();
+            throw new HarbormasterBuildAbortedException();
+          } else {
+            $build_update = 5;
+          }
+        } else {
+          $build_update -= 1;
+        }
+
+        // Command is still executing.
+
+        // Read more data as it is available.
+        list($stdout, $stderr) = $future->read();
+        $log_stdout->append($stdout);
+        $log_stderr->append($stderr);
+        $future->discardBuffers();
+      } else {
+        // Command execution is complete.
+
+        // Get the return value so we can log that as well.
+        list($err) = $future->resolve();
+
+        // Retrieve the last few bits of information.
+        list($stdout, $stderr) = $future->read();
+        $log_stdout->append($stdout);
+        $log_stderr->append($stderr);
+        $future->discardBuffers();
+
+        break;
+      }
     }
 
-    // Get the return value so we can log that as well.
-    list($err) = $future->resolve();
-
-    // Retrieve the last few bits of information.
-    list($stdout, $stderr) = $future->read();
-    $log_stdout->append($stdout);
-    $log_stderr->append($stderr);
-    $future->discardBuffers();
-
     $log_stdout->finalize($start_stdout);
     $log_stderr->finalize($start_stderr);
 

src/applications/harbormaster/step/HarbormasterHTTPRequestBuildStepImplementation.php

@@ -66,7 +66,10 @@ final class HarbormasterHTTPRequestBuildStepImplementation
         $key->getPasswordEnvelope());
     }
 
-    list($status, $body, $headers) = $future->resolve();
+    list($status, $body, $headers) = $this->resolveFuture(
+      $build,
+      $build_target,
+      $future);
 
     $log_body->append($body);
     $log_body->finalize($start);

src/applications/harbormaster/step/HarbormasterSleepBuildStepImplementation.php

@@ -23,7 +23,26 @@ final class HarbormasterSleepBuildStepImplementation
 
     $settings = $this->getSettings();
 
-    sleep($settings['seconds']);
+    $target = time() + $settings['seconds'];
+
+    // Use $build_update so that we only reload every 5 seconds, but
+    // the sleep mechanism remains accurate.
+    $build_update = 5;
+
+    while (time() < $target) {
+      sleep(1);
+
+      if ($build_update <= 0) {
+        $build->reload();
+        $build_update = 5;
+
+        if ($this->shouldAbort($build, $build_target)) {
+          throw new HarbormasterBuildAbortedException();
+        }
+      } else {
+        $build_update -= 1;
+      }
+    }
   }
 
   public function getFieldSpecifications() {

src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php

@@ -19,6 +19,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
   const STATUS_WAITING = 'target/waiting';
   const STATUS_PASSED = 'target/passed';
   const STATUS_FAILED = 'target/failed';
+  const STATUS_ABORTED = 'target/aborted';
 
   private $build = self::ATTACHABLE;
   private $buildStep = self::ATTACHABLE;
@@ -36,6 +37,8 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
         return pht('Passed');
       case self::STATUS_FAILED:
         return pht('Failed');
+      case self::STATUS_ABORTED:
+        return pht('Aborted');
       default:
         return pht('Unknown');
     }
@@ -52,6 +55,8 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
         return PHUIStatusItemView::ICON_ACCEPT;
       case self::STATUS_FAILED:
         return PHUIStatusItemView::ICON_REJECT;
+      case self::STATUS_ABORTED:
+        return PHUIStatusItemView::ICON_MINUS;
       default:
         return PHUIStatusItemView::ICON_QUESTION;
     }
@@ -66,6 +71,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
       case self::STATUS_PASSED:
         return 'green';
       case self::STATUS_FAILED:
+      case self::STATUS_ABORTED:
         return 'red';
       default:
         return 'bluegrey';
@@ -179,6 +185,7 @@ final class HarbormasterBuildTarget extends HarbormasterDAO
     switch ($this->getTargetStatus()) {
       case self::STATUS_PASSED:
       case self::STATUS_FAILED:
+      case self::STATUS_ABORTED:
         return true;
     }
 

src/applications/harbormaster/worker/HarbormasterTargetWorker.php

@@ -38,6 +38,10 @@ final class HarbormasterTargetWorker extends HarbormasterWorker {
     $target->setDateStarted(time());
 
     try {
+      if ($target->getBuildGeneration() !== $build->getBuildGeneration()) {
+        throw new HarbormasterBuildAbortedException();
+      }
+
       $status_pending = HarbormasterBuildTarget::STATUS_PENDING;
       if ($target->getTargetStatus() == $status_pending) {
         $target->setTargetStatus(HarbormasterBuildTarget::STATUS_BUILDING);
@@ -68,6 +72,11 @@ final class HarbormasterTargetWorker extends HarbormasterWorker {
       $target->setTargetStatus(HarbormasterBuildTarget::STATUS_FAILED);
       $target->setDateCompleted(time());
       $target->save();
+    } catch (HarbormasterBuildAbortedException $ex) {
+      // A build step is aborting because the build has been restarted.
+      $target->setTargetStatus(HarbormasterBuildTarget::STATUS_ABORTED);
+      $target->setDateCompleted(time());
+      $target->save();
     } catch (Exception $ex) {
       phlog($ex);