Be smarter about guessing future deployed versions for eager tests

2 files changed, 32 insertions, 35 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 bdb40f20c1d..f36f5be7778 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
@@ -304,21 +304,29 @@ public class DeploymentStatus {
     private Map<JobId, List<Job>> productionJobs(InstanceName instance, Change change, boolean assumeUpgradesSucceed) {
         Map<JobId, List<Job>> jobs = new LinkedHashMap<>();
         jobSteps.forEach((job, step) -> {
-            // When computing eager test jobs for outstanding changes, assume current upgrade completes successfully.
-            Optional<Deployment> deployment = deploymentFor(job)
-                    .map(existing -> assumeUpgradesSucceed ? withChange(existing, change.withoutApplication()) : existing);
+            // When computing eager test jobs for outstanding changes, assume current change completes successfully.
+            Optional<Deployment> deployment = deploymentFor(job);
+            Optional<Version> existingPlatform = deployment.map(Deployment::version);
+            Optional<ApplicationVersion> existingApplication = deployment.map(Deployment::applicationVersion);
+            if (assumeUpgradesSucceed) {
+                Change currentChange = application.require(instance).change();
+                Versions target = Versions.from(currentChange, application, deployment, systemVersion);
+                existingPlatform = Optional.of(target.targetPlatform());
+                existingApplication = Optional.of(target.targetApplication());
+            }
             if (job.application().instance().equals(instance) && job.type().isProduction()) {
-
                 List<Job> toRun = new ArrayList<>();
                 List<Change> changes = changes(job, step, change);
                 if (changes.isEmpty()) return;
                 for (Change partial : changes) {
-                    toRun.add(new Job(job.type(),
-                                      Versions.from(partial, application, deployment, systemVersion),
-                                      step.readyAt(partial, Optional.of(job)),
-                                      partial));
+                    Job jobToRun = new Job(job.type(),
+                                           Versions.from(partial, application, existingPlatform, existingApplication, systemVersion),
+                                           step.readyAt(partial, Optional.of(job)),
+                                           partial);
+                    toRun.add(jobToRun);
                     // Assume first partial change is applied before the second.
-                    deployment = deployment.map(existing -> withChange(existing, partial));
+                    existingPlatform = Optional.of(jobToRun.versions.targetPlatform());
+                    existingApplication = Optional.of(jobToRun.versions.targetApplication());
                 }
                 jobs.put(job, toRun);
             }
@@ -326,17 +334,6 @@ public class DeploymentStatus {
         return jobs;
     }
 
-    private static Deployment withChange(Deployment deployment, Change change) {
-        return new Deployment(deployment.zone(),
-                              change.application().orElse(deployment.applicationVersion()),
-                              change.platform().orElse(deployment.version()),
-                              deployment.at(),
-                              deployment.metrics(),
-                              deployment.activity(),
-                              deployment.quota(),
-                              deployment.cost());
-    }
-
     /** Changes to deploy with the given job, possibly split in two steps. */
     private List<Change> changes(JobId job, StepStatus step, Change change) {
         // Signal strict completion criterion by depending on job itself.
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java
index ff88549fb7c..0ea18d9cfa2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/Versions.java
@@ -103,36 +103,36 @@ public class Versions {
     }
 
     /** Create versions using given change and application */
+    public static Versions from(Change change, Application application, Optional<Version> existingPlatform,
+                                Optional<ApplicationVersion> existingApplication, Version defaultPlatformVersion) {
+        return new Versions(targetPlatform(application, change, existingPlatform, defaultPlatformVersion),
+                            targetApplication(application, change, existingApplication),
+                            existingPlatform,
+                            existingApplication);
+    }
+
+    /** Create versions using given change and application */
     public static Versions from(Change change, Application application, Optional<Deployment> deployment,
                                 Version defaultPlatformVersion) {
-        return new Versions(targetPlatform(application, change, deployment, defaultPlatformVersion),
-                            targetApplication(application, change, deployment),
+        return new Versions(targetPlatform(application, change, deployment.map(Deployment::version), defaultPlatformVersion),
+                            targetApplication(application, change, deployment.map(Deployment::applicationVersion)),
                             deployment.map(Deployment::version),
                             deployment.map(Deployment::applicationVersion));
     }
 
-    public static Versions from(Change change, Deployment deployment) {
-        return new Versions(change.platform().filter(version -> change.isPinned() || deployment.version().isBefore(version))
-                                  .orElse(deployment.version()),
-                            change.application().filter(version -> deployment.applicationVersion().compareTo(version) < 0)
-                                  .orElse(deployment.applicationVersion()),
-                            Optional.of(deployment.version()),
-                            Optional.of(deployment.applicationVersion()));
-    }
-
-    private static Version targetPlatform(Application application, Change change, Optional<Deployment> deployment,
+    private static Version targetPlatform(Application application, Change change, Optional<Version> existing,
                                           Version defaultVersion) {
         if (change.isPinned() && change.platform().isPresent())
             return change.platform().get();
 
-        return max(change.platform(), deployment.map(Deployment::version))
+        return max(change.platform(), existing)
                 .orElseGet(() -> application.oldestDeployedPlatform().orElse(defaultVersion));
     }
 
     private static ApplicationVersion targetApplication(Application application, Change change,
-                                                        Optional<Deployment> deployment) {
+                                                        Optional<ApplicationVersion> existing) {
         return change.application()
-                     .or(() -> deployment.map(Deployment::applicationVersion))
+                     .or(() -> existing)
                      .orElseGet(() -> defaultApplicationVersion(application));
     }