Abort internally run jobs whenever new application change is accepted

3 files changed, 75 insertions, 3 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
index a062f703494..3abe2a65ba1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTrigger.java
@@ -106,9 +106,14 @@ public class DeploymentTrigger {
                 triggering = JobRun.triggering(application.get().oldestDeployedPlatform().orElse(controller.systemVersion()), applicationVersion,
                                                Optional.empty(), Optional.empty(), "Application commit", clock.instant());
                 if (report.success()) {
-                    if (acceptNewApplicationVersion(application.get()))
+                    if (acceptNewApplicationVersion(application.get())) {
                         application = application.withChange(application.get().change().with(applicationVersion))
                                                  .withOutstandingChange(Change.empty());
+                        if (application.get().deploymentJobs().deployedInternally())
+                            for (Run run : jobs.active())
+                                if (run.id().application().equals(report.applicationId()))
+                                    jobs.abort(run.id());
+                    }
                     else
                         application = application.withOutstandingChange(Change.of(applicationVersion));
                 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
index 0c0953371c5..315e115f176 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTriggerTest.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.controller.deployment;
 
 import com.yahoo.component.Version;
+import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.TenantName;
@@ -12,6 +13,7 @@ import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.integration.BuildService;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType;
+import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId;
 import com.yahoo.vespa.hosted.controller.api.integration.stubs.MockBuildService;
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
@@ -30,7 +32,9 @@ import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
+import java.util.EnumSet;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
@@ -112,6 +116,68 @@ public class DeploymentTriggerTest {
     }
 
     @Test
+    public void abortsInternalJobsOnNewApplicationChange() {
+        InternalDeploymentTester iTester = new InternalDeploymentTester();
+        tester = iTester.tester();
+
+        Application app = iTester.app();
+        ApplicationPackage applicationPackage = InternalDeploymentTester.applicationPackage;
+
+        tester.jobCompletion(component).application(app).uploadArtifact(applicationPackage).submit();
+        tester.deployAndNotify(app, true, systemTest);
+        tester.deployAndNotify(app, true, stagingTest);
+        tester.assertRunning(productionUsCentral1, app.id());
+
+        // Jobs run externally are not affected.
+        tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit();
+        tester.assertRunning(productionUsCentral1, app.id());
+
+        tester.applications().deploymentTrigger().cancelChange(app.id(), false);
+        tester.deployAndNotify(app, false, systemTest);
+        tester.deployAndNotify(app, false, stagingTest);
+        tester.deployAndNotify(app, false, productionUsCentral1);
+        assertEquals(Change.empty(), tester.application(app.id()).change());
+        tester.assertNotRunning(systemTest, app.id());
+        tester.assertNotRunning(stagingTest, app.id());
+        tester.assertNotRunning(productionUsCentral1, app.id());
+
+        RunId id = iTester.newRun(productionUsCentral1);
+        assertTrue(iTester.jobs().active(id).isPresent());
+
+        // Jobs run internally are aborted.
+        iTester.newSubmission();
+        assertTrue(iTester.jobs().active(id).isPresent());
+        iTester.runner().run();
+        assertFalse(iTester.jobs().active(id).isPresent());
+
+        tester.readyJobTrigger().maintain();
+        assertEquals(EnumSet.of(systemTest, stagingTest), iTester.jobs().active().stream()
+                                                                 .map(run -> run.id().type())
+                                                                 .collect(Collectors.toCollection(() -> EnumSet.noneOf(JobType.class))));
+
+        iTester.runJob(JobType.systemTest);
+        iTester.runJob(JobType.stagingTest);
+        tester.readyJobTrigger().maintain();
+        iTester.runJob(JobType.productionUsCentral1);
+        tester.readyJobTrigger().maintain();
+        iTester.runJob(JobType.productionUsWest1);
+        iTester.runJob(JobType.productionUsEast3);
+        assertEquals(Change.empty(), iTester.app().change());
+
+        tester.upgradeSystem(new Version("8.9"));
+        iTester.runJob(JobType.systemTest);
+        iTester.runJob(JobType.stagingTest);
+        tester.readyJobTrigger().maintain();
+
+        // Jobs run internally are not aborted when the new submission is delayed.
+        iTester.newSubmission();
+        iTester.runner().run();
+        assertEquals(EnumSet.of(productionUsCentral1), iTester.jobs().active().stream()
+                                                              .map(run -> run.id().type())
+                                                              .collect(Collectors.toCollection(() -> EnumSet.noneOf(JobType.class))));
+    }
+
+    @Test
     public void deploymentSpecDecidesTriggerOrder() {
         TenantName tenant = tester.controllerTester().createTenant("tenant1", "domain1", 1L);
         MockBuildService mockBuildService = tester.buildService();
@@ -754,6 +820,7 @@ public class DeploymentTriggerTest {
         assertTrue("All jobs consumed", tester.buildService().jobs().isEmpty());
         assertFalse("No failures", tester.application(app.id()).deploymentJobs().hasFailures());
     }
+
     @Test
     public void testRetriesJobsFailingForCurrentChange() {
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
index ad89c79cb51..0549a2e4226 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalDeploymentTester.java
@@ -109,7 +109,7 @@ public class InternalDeploymentTester {
         ApplicationVersion applicationVersion = newSubmission();
 
         assertFalse(app().deployments().values().stream()
-                                .anyMatch(deployment -> deployment.applicationVersion().equals(applicationVersion)));
+                         .anyMatch(deployment -> deployment.applicationVersion().equals(applicationVersion)));
         assertEquals(applicationVersion, app().change().application().get());
         assertFalse(app().change().platform().isPresent());
 
@@ -128,7 +128,7 @@ public class InternalDeploymentTester {
     public void deployNewPlatform(Version version) {
         tester.upgradeSystem(version);
         assertFalse(app().deployments().values().stream()
-                                .anyMatch(deployment -> deployment.version().equals(version)));
+                         .anyMatch(deployment -> deployment.version().equals(version)));
         assertEquals(version, app().change().platform().get());
         assertFalse(app().change().application().isPresent());