Only schedule tests for oustanding change when there actually is one

3 files changed, 23 insertions, 13 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
index f36f5be7778..989f305b0cc 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentStatus.java
@@ -176,10 +176,14 @@ public class DeploymentStatus {
         Map<JobId, List<Job>> jobs = jobsToRun(changes);
 
         // Add test jobs for any outstanding change.
-        for (InstanceName instance : application.deploymentSpec().instanceNames())
-            changes.put(instance, outstandingChange(instance).onTopOf(application.require(instance).change()));
-        var testJobs = jobsToRun(changes, true).entrySet().stream()
-                                               .filter(entry -> ! entry.getKey().type().isProduction());
+        Map<InstanceName, Change> outstandingChanges = new LinkedHashMap<>();
+        for (InstanceName instance : application.deploymentSpec().instanceNames()) {
+            Change outstanding = outstandingChange(instance);
+            if (outstanding.hasTargets())
+                outstandingChanges.put(instance, outstanding.onTopOf(application.require(instance).change()));
+        }
+        var testJobs = jobsToRun(outstandingChanges, true).entrySet().stream()
+                                                          .filter(entry -> ! entry.getKey().type().isProduction());
 
         return Stream.concat(jobs.entrySet().stream(), testJobs)
                      .collect(collectingAndThen(toMap(Map.Entry::getKey,
@@ -900,6 +904,11 @@ public class DeploymentStatus {
             return Objects.hash(versions, readyAt, change);
         }
 
+        @Override
+        public String toString() {
+            return change + " with versions " + versions + ", ready at " + readyAt;
+        }
+
     }
 
 }
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 ed5df62ca5d..aeaa821745b 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
@@ -372,11 +372,11 @@ public class DeploymentTrigger {
               .ifPresent(last -> {
                   if (jobs.get(job).stream().noneMatch(versions ->    versions.versions().targetsMatch(last.versions())
                                                                    && versions.versions().sourcesMatchIfPresent(last.versions()))) {
-                      log.log(Level.INFO, "Aborting outdated run " + last);
-                      controller.jobController().abort(last.id(), "run no longer scheduled, and is blocking scheduled runs: " +
-                                                                  jobs.get(job).stream()
-                                                                      .map(scheduled -> scheduled.versions().toString())
-                                                                      .collect(Collectors.joining(", ")));
+                      String blocked = jobs.get(job).stream()
+                                           .map(scheduled -> scheduled.versions().toString())
+                                           .collect(Collectors.joining(", "));
+                      log.log(Level.INFO, "Aborting outdated run " + last + ", which is blocking runs: " + blocked);
+                      controller.jobController().abort(last.id(), "run no longer scheduled, and is blocking scheduled runs: " + blocked);
                   }
               });
     }
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 b95d34f5414..c01827b6eb0 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
@@ -698,7 +698,10 @@ public class DeploymentTriggerTest {
         ApplicationVersion revision1 = app1.lastSubmission().get();
         app1.submit(applicationPackage);
         ApplicationVersion revision2 = app1.lastSubmission().get();
-        app1.runJob(systemTest).runJob(stagingTest);
+        app1.runJob(systemTest)   // Tests for new revision on version2
+            .runJob(stagingTest)
+            .runJob(systemTest)   // Tests for new revision on version1
+            .runJob(stagingTest);
         assertEquals(Change.of(version1).with(revision2), app1.instance().change());
         tester.triggerJobs();
         app1.assertRunning(productionUsCentral1);
@@ -718,9 +721,7 @@ public class DeploymentTriggerTest {
         app1.assertNotRunning(productionUsCentral1);
 
         // Last job has a different deployment target, so tests need to run again.
-        app1.runJob(systemTest)
-            .runJob(stagingTest)            // Eager test of outstanding change, assuming upgrade in west succeeds.
-            .runJob(productionEuWest1)      // Upgrade completes, and revision is the only change.
+        app1.runJob(productionEuWest1)      // Upgrade completes, and revision is the only change.
             .runJob(productionUsCentral1)   // With only revision change, central should run to cover a previous failure.
             .runJob(productionEuWest1);     // Finally, west changes revision.
         assertEquals(Change.empty(), app1.instance().change());