Only count actually upgraded apps against upgrade limit

4 files changed, 15 insertions, 17 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 d83f552ab25..1f2a016f630 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
@@ -323,14 +323,6 @@ public class DeploymentTrigger {
                                                       instance -> instance.withJobPause(jobType, OptionalLong.empty()))));
     }
 
-    /** Triggers a change of this application, unless it already has a change. */
-    public void triggerChange(ApplicationId instanceId, Change change) {
-        applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> {
-            if ( ! application.get().require(instanceId.instance()).change().hasTargets())
-                forceChange(instanceId, change);
-        });
-    }
-
     /** Overrides the given instance's platform and application changes with any contained in the given change. */
     public void forceChange(ApplicationId instanceId, Change change) {
         applications().lockApplicationOrThrow(TenantAndApplicationId.from(instanceId), application -> {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
index 1932dc65657..269623de3f0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
@@ -119,12 +119,18 @@ public class Upgrader extends ControllerMaintainer {
         }
 
         int numberToUpgrade = policy == UpgradePolicy.canary ? instances.size() : numberOfApplicationsToUpgrade();
-        for (ApplicationId id : instances.matching(targets.keySet()::contains).first(numberToUpgrade)) {
-            log.log(Level.INFO, "Triggering upgrade to " + targets.get(id) + " for " + id);
-            if (failingRevision.contains(id))
+        for (ApplicationId id : instances.matching(targets.keySet()::contains)) {
+            if (failingRevision.contains(id)) {
+                log.log(Level.INFO, "Cancelling failing revision for " + id);
                 controller().applications().deploymentTrigger().cancelChange(id, ChangesToCancel.APPLICATION);
-
-            controller().applications().deploymentTrigger().triggerChange(id, Change.of(targets.get(id)));
+            }
+
+            if (controller().applications().requireInstance(id).change().isEmpty()) {
+                log.log(Level.INFO, "Triggering upgrade to " + targets.get(id) + " for " + id);
+                controller().applications().deploymentTrigger().forceChange(id, Change.of(targets.get(id)));
+                --numberToUpgrade;
+            }
+            if (numberToUpgrade <= 0) break;
         }
     }
 
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 62fafa12993..6dfeaf55ea4 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
@@ -1221,7 +1221,7 @@ public class DeploymentTriggerTest {
         assertEquals(Change.empty(), app.instance().change());
 
         // Application is pinned to previous version, and downgrades to that. Tests are re-run.
-        tester.deploymentTrigger().triggerChange(app.instanceId(), Change.of(version0).withPin());
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(version0).withPin());
         app.runJob(stagingTest).runJob(productionUsEast3);
         tester.clock().advance(Duration.ofMinutes(1));
         app.failDeployment(testUsEast3);
@@ -2106,12 +2106,12 @@ public class DeploymentTriggerTest {
         Version version2 = new Version("7.8.9");
         Version version3 = new Version("8.9.10");
         tester.controllerTester().upgradeSystem(version2);
-        tester.deploymentTrigger().triggerChange(appToAvoidVersionGC.instanceId(), Change.of(version2));
+        tester.deploymentTrigger().forceChange(appToAvoidVersionGC.instanceId(), Change.of(version2));
         appToAvoidVersionGC.deployPlatform(version2);
 
         // app upgrades first zone to version3, and then the other two to version2.
         tester.controllerTester().upgradeSystem(version3);
-        tester.deploymentTrigger().triggerChange(app.instanceId(), Change.of(version3));
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(version3));
         app.runJob(systemTest).runJob(stagingTest);
         tester.triggerJobs();
         tester.upgrader().overrideConfidence(version3, VespaVersion.Confidence.broken);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
index 9b2a1607e76..15855770c0b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
@@ -32,7 +32,7 @@ public class OutstandingChangeDeployerTest {
         var app = tester.newDeploymentContext().submit(applicationPackage).deploy();
 
         Version version = new Version(6, 2);
-        tester.deploymentTrigger().triggerChange(app.instanceId(), Change.of(version));
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(version));
         assertEquals(Change.of(version), app.instance().change());
         assertFalse(app.deploymentStatus().outstandingChange(app.instance().name()).hasTargets());