Handle upgrade cancellation for ongoing deployment

2 files changed, 56 insertions, 4 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
index b860fab3c7e..fdb7fd8f3ff 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.deployment;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.vespa.hosted.controller.Application;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
 
@@ -39,6 +40,10 @@ public class DeploymentOrder {
 
     /** Returns a list of jobs to trigger after the given job */
     public List<JobType> nextAfter(JobType job, Application application) {
+        if (!application.deploying().isPresent()) { // Change was cancelled
+            return Collections.emptyList();
+        }
+
         // Always trigger system test after component as deployment spec might not be available yet (e.g. if this is a
         // new application with no previous deployments)
         if (job == JobType.component) {
@@ -48,7 +53,7 @@ public class DeploymentOrder {
         // At this point we have deployed to system test, so deployment spec is available
         List<DeploymentSpec.Step> deploymentSteps = deploymentSteps(application);
         Optional<DeploymentSpec.Step> currentStep = fromJob(job, application);
-        if ( ! currentStep.isPresent()) {
+        if (!currentStep.isPresent()) {
             return Collections.emptyList();
         }
 
@@ -59,7 +64,7 @@ public class DeploymentOrder {
         }
 
         // Postpone if step hasn't completed all it's jobs for this change
-        if (!completedSuccessfully(currentStep.get(), application)) {
+        if (!completedSuccessfully(currentStep.get(), application.deploying().get(), application)) {
             return Collections.emptyList();
         }
 
@@ -112,9 +117,9 @@ public class DeploymentOrder {
     }
 
     /** Returns whether all jobs have completed successfully for given step */
-    private boolean completedSuccessfully(DeploymentSpec.Step step, Application application) {
+    private boolean completedSuccessfully(DeploymentSpec.Step step, Change change, Application application) {
         return jobsFrom(step).stream()
-                .allMatch(job -> application.deploymentJobs().isSuccessful(application.deploying().get(), job));
+                .allMatch(job -> application.deploymentJobs().isSuccessful(change, job));
     }
 
     /** Resolve deployment step from job */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
index e047a288fb9..8439b55969a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/UpgraderTest.java
@@ -16,6 +16,7 @@ import org.junit.Test;
 import java.time.Duration;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -266,6 +267,52 @@ public class UpgraderTest {
         assertTrue("No more jobs triggered at this time", tester.buildSystem().jobs().isEmpty());
     }
 
+    @Test
+    public void testUpgradeCancelledWithDeploymentInProgress() {
+        DeploymentTester tester = new DeploymentTester();
+        Version version = Version.fromString("5.0");
+        tester.updateVersionStatus(version);
+
+        // Setup applications
+        Application canary0 = tester.createAndDeploy("canary0", 0, "canary");
+        Application canary1 = tester.createAndDeploy("canary1", 1, "canary");
+        Application default0 = tester.createAndDeploy("default0", 2, "default");
+        Application default1 = tester.createAndDeploy("default1", 3, "default");
+        Application default2 = tester.createAndDeploy("default2", 4, "default");
+        Application default3 = tester.createAndDeploy("default3", 5, "default");
+        Application default4 = tester.createAndDeploy("default4", 6, "default");
+
+        // New version is released
+        version = Version.fromString("5.1");
+        tester.updateVersionStatus(version);
+        assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
+        tester.upgrader().maintain();
+
+        // Canaries upgrade and raise confidence
+        tester.completeUpgrade(canary0, version, "canary");
+        tester.completeUpgrade(canary1, version, "canary");
+        tester.updateVersionStatus(version);
+        assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence());
+
+        // Applications with default policy start upgrading
+        tester.upgrader().maintain();
+        assertEquals("Upgrade scheduled for remaining apps", 5, tester.buildSystem().jobs().size());
+
+        // 4/5 applications fail and lowers confidence
+        tester.completeUpgradeWithError(default0, version, "default", DeploymentJobs.JobType.systemTest);
+        tester.completeUpgradeWithError(default1, version, "default", DeploymentJobs.JobType.systemTest);
+        tester.completeUpgradeWithError(default2, version, "default", DeploymentJobs.JobType.systemTest);
+        tester.completeUpgradeWithError(default3, version, "default", DeploymentJobs.JobType.systemTest);
+        tester.updateVersionStatus(version);
+        assertEquals(VespaVersion.Confidence.broken, tester.controller().versionStatus().systemVersion().get().confidence());
+        tester.upgrader().maintain();
+
+        // 5th app passes system-test, but does not trigger next job as upgrade is cancelled
+        assertFalse("No change present", tester.applications().require(default4.id()).deploying().isPresent());
+        tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default4, true);
+        assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty());
+    }
+
     // TODO: Remove when corp-prod special casing is no longer needed
     @Test
     public void upgradesCanariesToControllerVersion() {