Merge pull request #21336 from vespa-engine/jonmv/long-deployment-pipelines-2

Jonmv/long deployment pipelines 2

5 files changed, 44 insertions, 6 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
index 9ef7cbcebf6..91e80eb8a30 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/InstanceList.java
@@ -72,7 +72,7 @@ public class InstanceList extends AbstractFilteringList<ApplicationId, InstanceL
 
     /** Returns the subset of instances that has completed deployment of given change */
     public InstanceList hasCompleted(Change change) {
-        return matching(id -> instances.get(id).jobsToRun(Map.of(id.instance(), change)).isEmpty());
+        return matching(id -> instances.get(id).hasCompleted(id.instance(), change));
     }
 
     /** Returns the subset of instances which are currently deploying a change */
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 fb4584d4672..0f9be643cdf 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
@@ -216,7 +216,12 @@ public class DeploymentStatus {
     }
 
     /** The set of jobs that need to run for the given changes to be considered complete. */
-    public Map<JobId, List<Job>> jobsToRun(Map<InstanceName, Change> changes) {
+    public boolean hasCompleted(InstanceName instance, Change change) {
+        return jobsToRun(Map.of(instance, change), false).isEmpty();
+    }
+
+    /** The set of jobs that need to run for the given changes to be considered complete. */
+    private Map<JobId, List<Job>> jobsToRun(Map<InstanceName, Change> changes) {
         return jobsToRun(changes, false);
     }
 
@@ -263,7 +268,7 @@ public class DeploymentStatus {
                                                                                            .noneMatch(deployment -> deployment.applicationVersion().compareTo(version) > 0))
                                                              .map(Change::of)
                                                              .filter(change -> application.require(instance).change().application().map(change::upgrades).orElse(true))
-                                                             .filter(change -> ! jobsToRun(Map.of(instance, change)).isEmpty())
+                                                             .filter(change -> ! hasCompleted(instance, change))
                                                              .findFirst())
                        .orElse(Change.empty());
     }
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 41db36b136e..bb8180e9f14 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
@@ -398,9 +398,9 @@ public class DeploymentTrigger {
 
     private Instance withRemainingChange(Instance instance, Change change, DeploymentStatus status) {
         Change remaining = change;
-        if (status.jobsToRun(Map.of(instance.name(), change.withoutApplication())).isEmpty())
+        if (status.hasCompleted(instance.name(), change.withoutApplication()))
             remaining = remaining.withoutPlatform();
-        if (status.jobsToRun(Map.of(instance.name(), change.withoutPlatform())).isEmpty()) {
+        if (status.hasCompleted(instance.name(), change.withoutPlatform())) {
             remaining = remaining.withoutApplication();
             if (change.application().isPresent())
                 instance = instance.withLatestDeployed(change.application().get());
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 5f9f0aeb64c..60c41ef520d 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -114,7 +114,7 @@ public class JobController {
     public void updateStorage() {
         for (ApplicationId id : instances())
             for (JobType type : jobs(id)) {
-                locked(id, type, runs -> { // runs is not modified here, and is written as it was.
+                locked(id, type, runs -> { // Runs are not modified here, and are written as they were.
                     curator.readLastRun(id, type).ifPresent(curator::writeLastRun);
                 });
             }
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 6cd4e3e92c3..d1b76dd96be 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.InstanceName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.zone.RoutingMethod;
 import com.yahoo.config.provision.zone.ZoneId;
@@ -243,6 +244,38 @@ public class DeploymentTriggerTest {
     }
 
     @Test
+    public void testOutstandingChangeWithNextRevisionTarget() {
+        ApplicationPackage appPackage = new ApplicationPackageBuilder().revisionTarget("next")
+                                                                       .revisionChange("when-failing")
+                                                                       .region("us-east-3")
+                                                                       .build();
+        DeploymentContext app = tester.newDeploymentContext()
+                                      .submit(appPackage);
+        Optional<ApplicationVersion> revision0 = app.lastSubmission();
+
+        app.submit(appPackage);
+        Optional<ApplicationVersion> revision1 = app.lastSubmission();
+
+        app.submit(appPackage);
+        Optional<ApplicationVersion> revision2 = app.lastSubmission();
+
+        app.submit(appPackage);
+        Optional<ApplicationVersion> revision3 = app.lastSubmission();
+
+        assertEquals(revision0, app.instance().change().application());
+        assertEquals(revision1, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application());
+
+        tester.deploymentTrigger().forceChange(app.instanceId(), Change.of(revision1.get()));
+        assertEquals(revision1, app.instance().change().application());
+        assertEquals(revision2, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application());
+
+        app.deploy();
+        tester.outstandingChangeDeployer().run();
+        assertEquals(revision2, app.instance().change().application());
+        assertEquals(revision3, app.deploymentStatus().outstandingChange(InstanceName.defaultName()).application());
+    }
+
+    @Test
     public void deploymentSpecWithDelays() {
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
                 .systemTest()