Abort builds when new build referencing same diff is scheduled

changelog.md

@@ -1,5 +1,9 @@
 # Changelog
 
+### 1.9.8 (Unreleased)
+
+* Abort existing builds when a new build referencing same diff is scheduled (Gautam Korlam)
+
 ### 1.9.7 (2016/08/09)
 
 * Report lint warnings from file (Anup Talwalkar)

docs/advanced.md

@@ -60,4 +60,23 @@ The severity parameter recognizes these severity levels:
 | autofix  | Auto-Fix |
 | warning  | Warning  |
 | error    | Error    |
-| disabled | Disabled |
+| disabled | Disabled |
+
+Suspend Useless Jobs
+---------------------
+
+When new builds are triggered from Phabricator due to new changes to the same diff or
+rebuilding via harbormaster, it may be desirable to suspend existing jobs that were triggered
+for the same diff. This can be done by adding the `ABORT_ON_REVISION_ID` string parameter to your job.
+
+![abort on revision id parameter](/docs/jenkins-suspend-param.png)
+
+You need to also add the parameter to your harbormaster request
+![abort on revision id parameter](/docs/harbormaster-suspend-param.png)
+
+This makes the latest build triggered for a diff automatically abort the existing running builds
+for the same diff on the job. Jobs aborted this way will skip notifying phabricator to
+avoid confusion. Please note that builds on the same diff triggered by the same upstream build will not be aborted this way. This can be useful when running multi-configuration jobs and parallel builds that run on the same diff.
+
+Also note that if you pass additional arguments to your harbormaster request they may need to be included in the `ABORT_ON_REVISION_ID` field as well. A good example is when you use the same CI job to build multiple targets on a single diff. So for example, if the jenkins request params have
+`TARGET=some_target`, then to ensure other targets are not cancelled for the same diff, you may want to set `ABORT_ON_REVISION_ID=some_target_${buildable.revision}`.

src/main/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapper.java

@@ -20,6 +20,7 @@
 
 package com.uber.jenkins.phabricator;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.uber.jenkins.phabricator.conduit.ConduitAPIClient;
 import com.uber.jenkins.phabricator.conduit.ConduitAPIException;
 import com.uber.jenkins.phabricator.conduit.Differential;
@@ -35,8 +36,17 @@
 import hudson.Launcher;
 import hudson.model.AbstractBuild;
 import hudson.model.BuildListener;
+import hudson.model.Cause;
+import hudson.model.CauseAction;
+import hudson.model.Executor;
 import hudson.model.Job;
+import hudson.model.ParameterValue;
+import hudson.model.ParametersAction;
+import hudson.model.Result;
+import hudson.model.Run;
 import hudson.tasks.BuildWrapper;
+import hudson.util.RunList;
+
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.io.IOException;
@@ -164,6 +174,42 @@ public void buildEnvVars(Map<String, String> env) {
         };
     }
 
+    /**
+     * Abort running builds when new build referencing same revision is scheduled to run
+     */
+    @Override
+    public void preCheckout(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException,
+            InterruptedException {
+        String abortOnRevisionId = getAbortOnRevisionId(build);
+        // If ABORT_ON_REVISION_ID is available
+        if (!CommonUtils.isBlank(abortOnRevisionId)) {
+            // Create a cause of interruption
+            PhabricatorCauseOfInterruption causeOfInterruption =
+                    new PhabricatorCauseOfInterruption(build.getUrl());
+            Run upstreamRun = getUpstreamRun(build);
+
+            // Get the running builds that were scheduled before the current one
+            RunList<AbstractBuild> runningBuilds = (RunList<AbstractBuild>) build.getProject().getBuilds();
+            for (AbstractBuild runningBuild : runningBuilds) {
+                Executor executor = runningBuild.getExecutor();
+                Run runningBuildUpstreamRun = getUpstreamRun(runningBuild);
+
+                // Ignore builds that were triggered by the same upstream build
+                // Find builds triggered with the same ABORT_ON_REVISION_ID_FIELD
+                if (runningBuild.isBuilding()
+                        && runningBuild.number < build.number
+                        && abortOnRevisionId.equals(getAbortOnRevisionId(runningBuild))
+                        && (upstreamRun == null
+                        || runningBuildUpstreamRun == null
+                        || !upstreamRun.equals(runningBuildUpstreamRun))
+                        && executor != null) {
+                    // Abort the builds
+                    executor.interrupt(Result.ABORTED, causeOfInterruption);
+                }
+            }
+        }
+    }
+
     private void addShortText(final AbstractBuild build) {
         build.addAction(PhabricatorPostbuildAction.createShortText("master", null));
     }
@@ -239,4 +285,29 @@ private String getArcPath() {
     public PhabricatorBuildWrapperDescriptor getDescriptor() {
         return (PhabricatorBuildWrapperDescriptor)super.getDescriptor();
     }
+
+    @VisibleForTesting
+    static String getAbortOnRevisionId(AbstractBuild build) {
+        ParametersAction parameters = build.getAction(ParametersAction.class);
+        if (parameters != null) {
+            ParameterValue parameterValue = parameters.getParameter(
+                    PhabricatorPlugin.ABORT_ON_REVISION_ID_FIELD);
+            if (parameterValue != null) {
+                return (String) parameterValue.getValue();
+            }
+        }
+        return null;
+    }
+
+    @VisibleForTesting
+    static Run<?, ?> getUpstreamRun(AbstractBuild build) {
+        CauseAction action = build.getAction(hudson.model.CauseAction.class);
+        if (action != null) {
+            Cause.UpstreamCause upstreamCause = action.findCause(hudson.model.Cause.UpstreamCause.class);
+            if (upstreamCause != null) {
+                return upstreamCause.getUpstreamRun();
+            }
+        }
+        return null;
+    }
 }

src/main/java/com/uber/jenkins/phabricator/PhabricatorCauseOfInterruption.java

@@ -0,0 +1,16 @@
+package com.uber.jenkins.phabricator;
+
+import jenkins.model.CauseOfInterruption;
+
+public final class PhabricatorCauseOfInterruption extends CauseOfInterruption {
+    private final String buildUrl;
+
+    PhabricatorCauseOfInterruption(String buildUrl) {
+        this.buildUrl = buildUrl;
+    }
+
+    @Override
+    public String getShortDescription() {
+        return String.format("Aborted by %s", buildUrl);
+    }
+}

src/main/java/com/uber/jenkins/phabricator/PhabricatorNotifier.java

@@ -43,17 +43,21 @@
 import hudson.model.Result;
 import hudson.tasks.BuildStepMonitor;
 import hudson.tasks.Notifier;
+import jenkins.model.CauseOfInterruption;
+import jenkins.model.InterruptedBuildAction;
 import jenkins.model.Jenkins;
 import org.kohsuke.stapler.DataBoundConstructor;
 
 import java.io.IOException;
+import java.util.List;
 
 public class PhabricatorNotifier extends Notifier {
     public static final String COBERTURA_CLASS_NAME = "com.uber.jenkins.phabricator.coverage.CoberturaCoverageProvider";
 
     private static final String JUNIT_PLUGIN_NAME = "junit";
     private static final String JUNIT_CLASS_NAME = "com.uber.jenkins.phabricator.unit.JUnitTestProvider";
     private static final String COBERTURA_PLUGIN_NAME = "cobertura";
+    private static final String ABORT_TAG = "abort";
     private static final String UBERALLS_TAG = "uberalls";
     private static final String CONDUIT_TAG = "conduit";
     // Post a comment on success. Useful for lengthy builds.
@@ -114,6 +118,18 @@ public final boolean perform(final AbstractBuild<?, ?> build, final Launcher lau
         final String phid = environment.get(PhabricatorPlugin.PHID_FIELD);
         final boolean isDifferential = !CommonUtils.isBlank(diffID);
 
+        InterruptedBuildAction action = build.getAction(InterruptedBuildAction.class);
+        if (action != null) {
+            List<CauseOfInterruption> causes = action.getCauses();
+            for (CauseOfInterruption cause : causes) {
+                if (cause instanceof PhabricatorCauseOfInterruption) {
+                    logger.warn(ABORT_TAG, "Skipping notification step since this build was interrupted"
+                            + " by a newer build with the same differential revision");
+                    return true;
+                }
+            }
+        }
+
         // Handle non-differential build invocations. If PHID is present but DIFF_ID is not, it means somebody is doing
         // a Harbormaster build on a commit rather than a differential, but still wants build status.
         // If DIFF_ID is present but PHID is not, it means somebody is doing a Differential build without Harbormaster.

src/main/java/com/uber/jenkins/phabricator/PhabricatorPlugin.java

@@ -31,6 +31,8 @@ public class PhabricatorPlugin extends Plugin {
     public static final String DIFFERENTIAL_ID_FIELD = "DIFF_ID";
     // Phabricator object ID (for Harbormaster)
     public static final String PHID_FIELD = "PHID";
+    // Revision ID (to abort old running jobs)
+    public static final String ABORT_ON_REVISION_ID_FIELD = "ABORT_ON_REVISION_ID";
 
     public static String getIconPath(String icon) {
         if (icon == null) {

src/test/java/com/uber/jenkins/phabricator/PhabricatorBuildWrapperTest.java

@@ -20,13 +20,16 @@
 
 package com.uber.jenkins.phabricator;
 
+import com.google.common.collect.Lists;
 import com.uber.jenkins.phabricator.utils.TestUtils;
-import hudson.model.FreeStyleBuild;
-import hudson.model.Result;
+import hudson.model.*;
 import net.sf.json.JSONObject;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Collections;
+import java.util.List;
+
 import static org.junit.Assert.*;
 
 public class PhabricatorBuildWrapperTest extends BuildIntegrationTest {
@@ -129,6 +132,32 @@ public void testBuildWithErrorOnArcanist() throws Exception {
         assertFailureWithMessage("Error applying arc patch", build);
     }
 
+    @Test
+    public void getAbortOnRevisionIdIfAvailable() throws Exception {
+        FreeStyleBuild build = buildWithConduit(getFetchDiffResponse(), null, null, true);
+        assertNull(PhabricatorBuildWrapper.getAbortOnRevisionId(build));
+
+        List<ParameterValue> parameters = Lists.newArrayList();
+        parameters.add(new ParameterValue("ABORT_ON_REVISION_ID") {
+            @Override
+            public Object getValue() {
+                return "test";
+            }
+        });
+        build.addAction(new ParametersAction(parameters));
+        assertEquals("test", PhabricatorBuildWrapper.getAbortOnRevisionId(build));
+    }
+
+    @Test
+    public void getUpstreamRunIfAvailable() throws Exception {
+        FreeStyleBuild build = buildWithConduit(getFetchDiffResponse(), null, null, true);
+        FreeStyleBuild upstream = buildWithConduit(getFetchDiffResponse(), null, null, true);
+        assertNull(PhabricatorBuildWrapper.getUpstreamRun(build));
+
+        build.getAction(CauseAction.class).getCauses().add((new Cause.UpstreamCause(upstream)));
+        assertEquals(upstream, PhabricatorBuildWrapper.getUpstreamRun(build));
+    }
+
     @Override
     protected void addBuildStep() {
         p.getBuildWrappersList().add(wrapper);