Decide versions at triggering, filtering out downgrades

13 files changed, 174 insertions, 206 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
index b19db6b1529..5e6d045afc4 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Application.java
@@ -17,6 +17,7 @@ import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.rotation.RotationId;
 
+import java.time.Instant;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
@@ -119,12 +120,18 @@ public class Application {
     public DeploymentJobs deploymentJobs() { return deploymentJobs; }
 
     /**
-     * Returns the change that should currently be deployed for this application,
-     * which is empty when no change is in progress.
+     * Returns base change for this application, i.e., the change that is deployed outside block windows.
+     * This is empty when no change is currently under deployment.
      */
     public Change change() { return change; }
 
     /**
+     * Returns the change that should used for this application at the given instant, typically now.
+     */
+    public Change changeAt(Instant now) {
+        return change.effectiveAt(deploymentSpec, now); }
+
+    /**
      * Returns whether this has an outstanding change (in the source repository), which
      * has currently not started deploying (because a deployment is (or was) already in progress
      */
@@ -139,45 +146,21 @@ public class Application {
     }
 
     /**
-     * Returns the oldest version this has deployed in a permanent zone (not test or staging),
-     * or empty version if it is not deployed anywhere
+     * Returns the oldest platform version this has deployed in a permanent zone (not test or staging).
      */
-    public Optional<Version> oldestDeployedVersion() {
+    public Optional<Version> oldestDeployedPlatform() {
         return productionDeployments().values().stream()
-                .map(Deployment::version)
-                .min(Comparator.naturalOrder());
-    }
-
-    /** Returns the version a new deployment to this zone should use for this application */
-    public Version deployVersionIn(ZoneId zone, Controller controller) {
-        Version current = versionIn(zone, controller);
-        return change.effectiveAt(deploymentSpec, controller.clock().instant()).platform()
-                     .filter(ignored -> ! change.downgrades(current)).orElse(current);
-    }
-
-    /** Returns the current version this application has, or if none; should use, in the given zone */
-    public Version versionIn(ZoneId zone, Controller controller) {
-        return Optional.ofNullable(deployments().get(zone)).map(Deployment::version) // Already deployed in this zone: Use that version
-                       .orElse(oldestDeployedVersion().orElse(controller.systemVersion()));
+                                      .map(Deployment::version)
+                                      .min(Comparator.naturalOrder());
     }
 
     /**
-     * Returns the application version to use for the given job.
-     *
-     * If no version is specified by the application's {@link Change}, or by {@code currentVersion},
-     * returns the currently deployed application version, or the last successfully deployed one.
+     * Returns the oldest application version this has deployed in a permanent zone (not test or staging).
      */
-    public ApplicationVersion deployApplicationVersionFor(DeploymentJobs.JobType jobType,
-                                                          Controller controller,
-                                                          boolean useCurrentVersion) {
-        Optional<ApplicationVersion> current = Optional.ofNullable(deployments.get(jobType.zone(controller.system()).get()))
-                                                       .map(Deployment::applicationVersion);
-        Optional<ApplicationVersion> changeVersion = change.effectiveAt(deploymentSpec, controller.clock().instant()).application();
-
-        return Optional.ofNullable(changeVersion.filter(version -> ! (useCurrentVersion || current.filter(cv -> cv.compareTo(version) > 0).isPresent()))
-                                                .orElse(current.orElse(deploymentJobs().lastSuccessfulApplicationVersionFor(jobType)
-                                                                                       .orElse(changeVersion.orElse(null)))))
-                       .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version to use for " + jobType));
+    public Optional<ApplicationVersion> oldestDeployedApplication() {
+        return productionDeployments().values().stream()
+                                      .map(Deployment::applicationVersion)
+                                      .min(Comparator.naturalOrder());
     }
 
     /** Returns the global rotation of this, if present */
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index 8db79ed80e0..579d48dc335 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.controller;
 
 import com.google.common.collect.ImmutableList;
-import com.yahoo.collections.Pair;
 import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.application.api.ValidationId;
@@ -37,9 +36,10 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RoutingGenerato
 import com.yahoo.vespa.hosted.controller.api.integration.zone.ZoneId;
 import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationVersion;
+import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
+import com.yahoo.vespa.hosted.controller.application.JobStatus;
 import com.yahoo.vespa.hosted.controller.tenant.AthenzTenant;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
-import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.tenant.Tenant;
 import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.maintenance.DeploymentExpirer;
@@ -273,32 +273,40 @@ public class ApplicationController {
                     .map(app -> new LockedApplication(app, lock))
                     .orElseGet(() -> new LockedApplication(createApplication(applicationId, Optional.empty()), lock));
 
-            final boolean canDeployDirectly = canDeployDirectlyTo(zone, options);
-
-            // Determine Vespa version to use
-            Version version;
-            if (options.deployCurrentVersion) {
-                version = application.versionIn(zone, controller);
-            } else if (canDeployDirectly) {
-                version = options.vespaVersion.map(Version::new).orElse(controller.systemVersion());
-            } else if (! application.change().isPresent() && ! zone.environment().isManuallyDeployed()) {
-                return unexpectedDeployment(applicationId, zone, applicationPackageFromDeployer);
+            boolean canDeployDirectly =   ! options.screwdriverBuildJob.map(job1 -> job1.screwdriverId).isPresent()
+                                        ||  zone.environment().isManuallyDeployed();
+            boolean preferOldestVersion = options.deployCurrentVersion;
+
+            // Determine versions to use.
+            Version platformVersion;
+            ApplicationVersion applicationVersion;
+            ApplicationPackage applicationPackage;
+            if (canDeployDirectly) {
+                platformVersion = options.vespaVersion.map(Version::new).orElse(controller.systemVersion());
+                applicationVersion = ApplicationVersion.unknown;
+                applicationPackage = applicationPackageFromDeployer.orElseThrow(
+                        () -> new IllegalArgumentException("Application package must be given when deploying to " + zone));
             } else {
-                version = application.deployVersionIn(zone, controller);
+                JobType jobType = JobType.from(controller.system(), zone)
+                                         .orElseThrow(() -> new IllegalArgumentException("No job found for zone " + zone));
+                Optional<JobStatus.JobRun> triggered = Optional.ofNullable(application.deploymentJobs().jobStatus().get(jobType))
+                        .flatMap(JobStatus::lastTriggered);
+                // TODO jvenstad: Verify this response with a test, and see that it sorts itself when triggered.
+                if ( ! triggered.isPresent())
+                    return unexpectedDeployment(applicationId, zone);
+                platformVersion = preferOldestVersion
+                        ? application.oldestDeployedPlatform().orElse(controller.systemVersion())
+                        : triggered.get().version();
+                applicationVersion = preferOldestVersion
+                        ? application.oldestDeployedApplication().orElse(triggered.get().applicationVersion())
+                        : triggered.get().applicationVersion();
+                applicationPackage = new ApplicationPackage(artifactRepository.getApplicationPackage(application.id(), applicationVersion.id()));
             }
 
-            // Determine application package to use
-            // TODO jvenstad: Inline this and clean up with the above.
-            Pair<ApplicationPackage, ApplicationVersion> artifact = artifactFor(zone, application,
-                                                                                applicationPackageFromDeployer,
-                                                                                canDeployDirectly,
-                                                                                options.deployCurrentVersion);
-            ApplicationPackage applicationPackage = artifact.getFirst();
-            ApplicationVersion applicationVersion = artifact.getSecond();
             validate(applicationPackage.deploymentSpec());
 
             // Update application with information from application package
-            if (!options.deployCurrentVersion) {
+            if ( ! preferOldestVersion) {
                 // Store information about application package
                 application = application.with(applicationPackage.deploymentSpec());
                 application = application.with(applicationPackage.validationOverrides());
@@ -316,7 +324,7 @@ public class ApplicationController {
             // TODO jvenstad: Use code from DeploymentTrigger? Also, validate application version.
             // Validate the change being deployed
             if ( ! canDeployDirectly) {
-                validateChange(application, zone, version);
+                validateChange(application, zone, platformVersion);
             }
 
             // Assign global rotation
@@ -330,8 +338,7 @@ public class ApplicationController {
             });
 
             // Carry out deployment
-            options = withVersion(version, options);
-
+            options = withVersion(platformVersion, options);
 
             ConfigServerClient.PreparedApplication preparedApplication;
             DeploymentId deploymentId = new DeploymentId(applicationId, zone);
@@ -342,7 +349,8 @@ public class ApplicationController {
                 preparedApplication = configServer.prepare(deploymentId, options, cnames, rotationNames, applicationPackage.zippedContent());
                 preparedApplication.activate();
             }
-            application = application.withNewDeployment(zone, applicationVersion, version, clock.instant());
+            // TODO: Set new deployment after convergence, rather than after deployment call, succeeds.
+            application = application.withNewDeployment(zone, applicationVersion, platformVersion, clock.instant());
 
             store(application);
 
@@ -351,29 +359,6 @@ public class ApplicationController {
         }
     }
 
-    /** Decide application package and version pair to use in given zone */
-    private Pair<ApplicationPackage, ApplicationVersion> artifactFor(ZoneId zone,
-                                                                     Application application,
-                                                                     Optional<ApplicationPackage> applicationPackage,
-                                                                     boolean canDeployDirectly,
-                                                                     boolean deployCurrentVersion) {
-        ApplicationVersion version;
-        ApplicationPackage pkg;
-        Optional<DeploymentJobs.JobType> job = DeploymentJobs.JobType.from(controller.system(), zone);
-        if (canDeployDirectly) {
-            pkg = applicationPackage.orElseThrow(() -> new IllegalArgumentException("Application package must be " +
-                                                                                    "given when deploying to " + zone));
-            version = ApplicationVersion.unknown;
-        } else {
-            if (!job.isPresent()) {
-                throw new IllegalArgumentException("No job found for zone " + zone);
-            }
-            version = application.deployApplicationVersionFor(job.get(), controller, deployCurrentVersion);
-            pkg = new ApplicationPackage(artifactRepository.getApplicationPackage(application.id(), version.id()));
-        }
-        return new Pair<>(pkg, version);
-    }
-
     /** Makes sure the application has a global rotation, if eligible. */
     private LockedApplication withRotation(LockedApplication application, ZoneId zone) {
         if (zone.environment() == Environment.prod && application.deploymentSpec().globalServiceId().isPresent()) {
@@ -389,8 +374,7 @@ public class ApplicationController {
         return application;
     }
 
-    private ActivateResult unexpectedDeployment(ApplicationId applicationId, ZoneId zone,
-                                                Optional<ApplicationPackage> applicationPackage) {
+    private ActivateResult unexpectedDeployment(ApplicationId applicationId, ZoneId zone) {
         Log logEntry = new Log();
         logEntry.level = "WARNING";
         logEntry.time = clock.instant().toEpochMilli();
@@ -399,9 +383,7 @@ public class ApplicationController {
         PrepareResponse prepareResponse = new PrepareResponse();
         prepareResponse.log = Collections.singletonList(logEntry);
         prepareResponse.configChangeActions = new ConfigChangeActions(Collections.emptyList(), Collections.emptyList());
-        return new ActivateResult(new RevisionId(applicationPackage.map(ApplicationPackage::hash)
-                                                                   .orElse("0")), prepareResponse,
-                                  applicationPackage.map(a -> a.zippedContent().length).orElse(0));
+        return new ActivateResult(new RevisionId("0"), prepareResponse, 0);
     }
 
     private LockedApplication deleteRemovedDeployments(LockedApplication application) {
@@ -429,7 +411,7 @@ public class ApplicationController {
     }
 
     private LockedApplication deleteUnreferencedDeploymentJobs(LockedApplication application) {
-        for (DeploymentJobs.JobType job : application.deploymentJobs().jobStatus().keySet()) {
+        for (JobType job : application.deploymentJobs().jobStatus().keySet()) {
             Optional<ZoneId> zone = job.zone(controller.system());
 
             if ( ! job.isProduction() || (zone.isPresent() && application.deploymentSpec().includes(zone.get().environment(), zone.map(ZoneId::region))))
@@ -624,13 +606,6 @@ public class ApplicationController {
         return curator.lock(application, Duration.ofMinutes(10));
     }
 
-    /** Returns whether a direct deployment to given zone is allowed */
-    private static boolean canDeployDirectlyTo(ZoneId zone, DeployOptions options) {
-        return !options.screwdriverBuildJob.isPresent() ||
-               options.screwdriverBuildJob.get().screwdriverId == null ||
-               zone.environment().isManuallyDeployed();
-    }
-
     /** Verify that each of the production zones listed in the deployment spec exist in this system. */
     private void validate(DeploymentSpec deploymentSpec) {
         deploymentSpec.zones().stream()
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
index 287fd09f724..4ed45af5e66 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java
@@ -20,6 +20,7 @@ import java.util.stream.Stream;
  *
  * @author bratseth
  */
+// TODO jvenstad: Make an AbstractFilteringList based on JobList and let this extend it for free not()s?
 public class ApplicationList {
 
     private final ImmutableList<Application> list;
@@ -52,7 +53,7 @@ public class ApplicationList {
 
     // ----------------------------------- Filters
 
-    /** Returns the subset of applications which are currently upgrading (to any version) */
+    /** Returns the subset of applications which are upgrading (to any version), not considering block windows. */
     public ApplicationList upgrading() {
         return listOf(list.stream().filter(application -> application.change().platform().isPresent()));
     }
@@ -81,7 +82,7 @@ public class ApplicationList {
         return listOf(list.stream().filter(application -> application.change().isPresent()));
     }
 
-    /** Returns the subset of applications which is currently not deploying a change */
+    /** Returns the subset of applications which are currently not deploying a change */
     public ApplicationList notDeploying() {
         return listOf(list.stream().filter(application -> ! application.change().isPresent()));
     }
@@ -175,7 +176,7 @@ public class ApplicationList {
      * Applications without any deployments are ordered first.
      */
     public ApplicationList byIncreasingDeployedVersion() {
-        return listOf(list.stream().sorted(Comparator.comparing(application -> application.oldestDeployedVersion().orElse(Version.emptyVersion))));
+        return listOf(list.stream().sorted(Comparator.comparing(application -> application.oldestDeployedPlatform().orElse(Version.emptyVersion))));
     }
 
     // ----------------------------------- Internal helpers
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java
index d8d07ef57ef..5294ac9b774 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/Change.java
@@ -53,19 +53,9 @@ public final class Change {
     /** Returns the platform version carried by this. */
     public Optional<Version> platform() { return platform; }
 
-    /** Returns the platform version which will be deployed in a zone if the given version is currently deployed there. */
-    public Version platformAgainst(Version version) {
-        return downgrades(version) ? version : platform.orElse(version);
-    }
-
     /** Returns the application version carried by this. */
     public Optional<ApplicationVersion> application() { return application; }
 
-    /** Returns the application version which will be deployed in a zone if the given version is currently deployed there. */
-    public ApplicationVersion applicationAgainst(ApplicationVersion version) {
-        return downgrades(version) ? version : application.orElse(version);
-    }
-
     /** Returns an instance representing no change */
     public static Change empty() { return empty; }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
index 95ef199aac4..04e51cdecb6 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/DeploymentJobs.java
@@ -122,11 +122,16 @@ public class DeploymentJobs {
         return true; // other environments do not have any preconditions
     }
 
+    /** Returns the JobStatus of the given JobType, or empty. */
+    public Optional<JobStatus> statusOf(JobType jobType) {
+        return Optional.ofNullable(jobStatus().get(jobType));
+    }
+
     /** Returns the last successful application version for the given job */
     public Optional<ApplicationVersion> lastSuccessfulApplicationVersionFor(JobType jobType) {
-        return Optional.ofNullable(jobStatus().get(jobType))
-                       .flatMap(JobStatus::lastSuccess)
-                       .map(JobStatus.JobRun::applicationVersion);
+        return statusOf(jobType)
+                .flatMap(JobStatus::lastSuccess)
+                .map(JobStatus.JobRun::applicationVersion);
     }
 
     /**
@@ -140,10 +145,10 @@ public class DeploymentJobs {
 
     /** Returns the time of success for the given change for the given job type, or empty if unsuccessful. */
     public Optional<Instant> successAt(Change change, JobType jobType) {
-        return Optional.ofNullable(jobStatus().get(jobType))
-                       .flatMap(JobStatus::lastSuccess)
-                       .filter(status -> status.lastCompletedWas(change))
-                       .map(JobStatus.JobRun::at);
+        return statusOf(jobType)
+                .flatMap(JobStatus::lastSuccess)
+                .filter(status -> status.lastCompletedWas(change))
+                .map(JobStatus.JobRun::at);
     }
 
     private static Optional<Long> requireId(Optional<Long> id, String message) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
index 265eac5d522..325af87a21a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/JobStatus.java
@@ -82,7 +82,6 @@ public class JobStatus {
                                             ", but that has neither been triggered nor deployed");
 
         } else {
-            // TODO jvenstad: This is wrong, because triggering versions are not necessarily the same as deployed versions!
             version = lastTriggered.get().version();
             applicationVersion = lastTriggered.get().applicationVersion();
             reason = lastTriggered.get().reason();
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 15de1e2cfd9..c959270baf5 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
@@ -34,6 +34,7 @@ import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.function.Supplier;
 import java.util.logging.Logger;
 import java.util.stream.Stream;
 
@@ -121,29 +122,29 @@ public class DeploymentTrigger {
     }
 
     /**
-     * Finds and triggers jobs that can and should run but are currently not.
+     * Finds and triggers jobs that can and should run but are currently not, and returns the number of triggered jobs.
      *
      * Only one job is triggered each run for test jobs, since those environments have limited capacity.
      */
-    public void triggerReadyJobs() {
-        computeReadyJobs().collect(partitioningBy(job -> job.jobType().isTest()))
-                          .entrySet().stream()
-                          .flatMap(entry -> (entry.getKey()
-                                  // True for capacity constrained zones -- sort by priority and make a task for each job type.
-                                  ? entry.getValue().stream()
-                                         .sorted(comparing(Job::isRetry)
-                                                         .thenComparing(Job::applicationUpgrade)
-                                                         .reversed()
-                                                         .thenComparing(Job::availableSince))
-                                         .collect(groupingBy(Job::jobType))
-                                  // False for production jobs -- keep step order and make a task for each application.
-                                  : entry.getValue().stream()
-                                         .collect(groupingBy(Job::id)))
-                                  .values().stream()
-                                  .map(jobs -> (Runnable) jobs.stream()
-                                                              .filter(job -> canTrigger(job) && trigger(job))
-                                                              .limit(entry.getKey() ? 1 : Long.MAX_VALUE)::count))
-                          .parallel().forEach(Runnable::run);
+    public long triggerReadyJobs() {
+        return computeReadyJobs().collect(partitioningBy(job -> job.jobType().isTest()))
+                                 .entrySet().stream()
+                                 .flatMap(entry -> (entry.getKey()
+                                         // True for capacity constrained zones -- sort by priority and make a task for each job type.
+                                         ? entry.getValue().stream()
+                                                .sorted(comparing(Job::isRetry)
+                                                                .thenComparing(Job::applicationUpgrade)
+                                                                .reversed()
+                                                                .thenComparing(Job::availableSince))
+                                                .collect(groupingBy(Job::jobType))
+                                         // False for production jobs -- keep step order and make a task for each application.
+                                         : entry.getValue().stream()
+                                                .collect(groupingBy(Job::id)))
+                                         .values().stream()
+                                         .map(jobs -> (Supplier<Long>) jobs.stream()
+                                                                           .filter(job -> canTrigger(job) && trigger(job))
+                                                                           .limit(entry.getKey() ? 1 : Long.MAX_VALUE)::count))
+                                 .parallel().map(Supplier::get).reduce(0L, Long::sum);
     }
 
     /**
@@ -154,8 +155,8 @@ public class DeploymentTrigger {
 
         BuildService.BuildJob buildJob = new BuildService.BuildJob(job.projectId, job.jobType.jobName());
         if (buildService.trigger(buildJob)) {
-            applications().lockOrThrow(job.id, application ->
-                    applications().store(application.withJobTriggering(job.jobType, runOf(job))));
+            applications().lockOrThrow(job.id, application -> applications().store(application.withJobTriggering(
+                    job.jobType, new JobStatus.JobRun(-1, job.platformVersion, job.applicationVersion, job.reason, clock.instant()))));
             return true;
         }
         // TODO jvenstad: On 404: set empty projectId (and send ticket?). Throw and catch NoSuchElementException.
@@ -213,7 +214,7 @@ public class DeploymentTrigger {
 
             for (DeploymentSpec.Step step : steps) {
                 Set<JobType> stepJobs = step.zones().stream().map(order::toJob).collect(toSet());
-                Set<JobType> remainingJobs = stepJobs.stream().filter(job -> ! completedAt(application, job).isPresent()).collect(toSet());
+                Set<JobType> remainingJobs = stepJobs.stream().filter(job -> ! completedAt(application.change(), application, job).isPresent()).collect(toSet());
                 if (remainingJobs.isEmpty()) { // All jobs are complete -- find the time of completion of this step.
                     if (stepJobs.isEmpty()) { // No jobs means this is delay step.
                         Duration delay = ((DeploymentSpec.Delay) step).duration();
@@ -221,14 +222,15 @@ public class DeploymentTrigger {
                         reason += " after a delay of " + delay;
                     }
                     else {
-                        completedAt = stepJobs.stream().map(job -> completedAt(application, job).get()).max(naturalOrder());
+                        completedAt = stepJobs.stream().map(job -> completedAt(application.change(), application, job).get()).max(naturalOrder());
                         reason = "Available change in " + stepJobs.stream().map(JobType::jobName).collect(joining(", "));
                     }
                 }
                 else if (completedAt.isPresent()) { // Step not complete, because some jobs remain -- trigger these if the previous step was done.
                     for (JobType job : remainingJobs)
-                        jobs.add(new Job(application, job, reason, completedAt.get(), stepJobs, controller));
+                        jobs.add(deploymentJob(application, job, reason, completedAt.get(), stepJobs));
                     completedAt = Optional.empty();
+                    break;
                 }
             }
             // TODO jvenstad: Replace with completion of individual parts of Change.
@@ -252,28 +254,29 @@ public class DeploymentTrigger {
     }
 
     /**
-     * Returns the instant when the given application's current change was completed for the given job.
+     * Returns the instant when the given change is complete for the given application for the given job.
      *
-     * Any job is complete if its current change was already successful on that job.
+     * Any job is complete if the given change is already successful on that job.
      * A production job is also considered complete if its current change is strictly dominated by what
      * is already deployed in its zone, i.e., no parts of the change are upgrades, and at least one
      * part is a downgrade, regardless of the status of the job.
      */
-    private Optional<Instant> completedAt(Application application, JobType jobType) {
-        Optional<Instant> lastSuccess = application.deploymentJobs().successAt(application.change(), jobType);
+    private Optional<Instant> completedAt(Change change, Application application, JobType jobType) {
+        Optional<Instant> lastSuccess = application.deploymentJobs().successAt(change, jobType);
         if (lastSuccess.isPresent() || ! jobType.isProduction())
             return lastSuccess;
 
-        Deployment deployment = application.deployments().get(jobType.zone(controller.system()).get());
-        return Optional.ofNullable(deployment).map(Deployment::at)
-                       .filter(ignored ->    ! (   application.change().upgrades(deployment.version())
-                                                || application.change().upgrades(deployment.applicationVersion()))
-                                          &&   (   application.change().downgrades(deployment.version())
-                                                || application.change().downgrades(deployment.applicationVersion())));
+        return deploymentFor(application, jobType)
+                .filter(deployment ->    ! (   change.upgrades(deployment.version())
+                                            || change.upgrades(deployment.applicationVersion()))
+                                      &&   (   change.downgrades(deployment.version())
+                                            || change.downgrades(deployment.applicationVersion())))
+                .map(Deployment::at);
     }
 
     private boolean canTrigger(Job job) {
         Application application = applications().require(job.id);
+        // TODO jvenstad: Check versions, not change.
         if ( ! application.deploymentJobs().isDeployableTo(job.jobType.environment(), application.change()))
             return false;
 
@@ -289,16 +292,12 @@ public class DeploymentTrigger {
                                                        .mapToList(JobStatus::type)))
             return false;
 
-        // TODO jvenstad: If the above is implemented, take care not to deploy untested stuff?
-        if ( ! application.change().effectiveAt(application.deploymentSpec(), clock.instant()).isPresent())
+        if ( ! application.changeAt(clock.instant()).isPresent())
             return false;
 
         return true;
     }
 
-
-    // TODO jvenstad: platform/applicationDeployed() productionJobs.allMatch.!change::upgrade
-
     private ApplicationController applications() {
         return controller.applications();
     }
@@ -307,11 +306,40 @@ public class DeploymentTrigger {
         if (application.change().application().isPresent()) return true; // More application changes are ok.
         if (application.deploymentJobs().hasFailures()) return true; // Allow changes to fix upgrade problems.
         // Otherwise, allow an application change if not currently upgrading.
-        return ! application.change().effectiveAt(application.deploymentSpec(), clock.instant()).platform().isPresent();
+        return ! application.changeAt(clock.instant()).platform().isPresent();
+    }
+
+    private Optional<Deployment> deploymentFor(Application application, JobType jobType) {
+        return Optional.ofNullable(application.deployments().get(jobType.zone(controller.system()).get()));
     }
 
-    private JobStatus.JobRun runOf(Job job) {
-        return new JobStatus.JobRun(-1, job.platform, job.application, job.reason, clock.instant());
+    public Job forcedDeploymentJob(Application application, JobType jobType, String reason) {
+        return deploymentJob(application, jobType, reason, clock.instant(), Collections.emptySet());
+    }
+
+    public Job deploymentJob(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith) {
+        boolean isRetry = application.deploymentJobs().statusOf(jobType).flatMap(JobStatus::jobError)
+                                  .filter(JobError.outOfCapacity::equals).isPresent();
+        if (isRetry) reason += "; retrying on out of capacity";
+
+        Change change = application.change();
+        // For both versions, use the newer of the change's and the currently deployed versions, or a fallback if none of these exist.
+        Version platform = jobType == JobType.component
+                ? Version.emptyVersion
+                : deploymentFor(application, jobType).map(Deployment::version)
+                                                     .filter(version -> ! change.upgrades(version))
+                                                     .orElse(change.platform()
+                                                                   .orElse(application.oldestDeployedPlatform()
+                                                                                      .orElse(controller.systemVersion())));
+        ApplicationVersion applicationVersion = jobType == JobType.component
+                ? ApplicationVersion.unknown
+                : deploymentFor(application, jobType).map(Deployment::applicationVersion)
+                                                     .filter(version -> ! change.upgrades(version))
+                                                     .orElse(change.application()
+                                                                   .orElseGet(() -> application.oldestDeployedApplication()
+                                                                                               .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version to use for " + jobType))));
+
+        return new Job(application, jobType, reason, availableSince, concurrentlyWith, isRetry, change, platform, applicationVersion);
     }
 
 
@@ -326,30 +354,21 @@ public class DeploymentTrigger {
         private final boolean isRetry;
         private final boolean isApplicationUpgrade;
         private final Change change;
-        private final Version platform;
-        private final ApplicationVersion application;
-        // TODO jvenstad: Store target versions here, and set in withJobTriggering(Job job, Instant at).
-        // TODO jvenstad: Use trigger-versions during deployment!
+        private final Version platformVersion;
+        private final ApplicationVersion applicationVersion;
 
-        public Job(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith, Controller controller) {
+        private Job(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith, boolean isRetry, Change change, Version platformVersion, ApplicationVersion applicationVersion) {
             this.id = application.id();
             this.jobType = jobType;
             this.projectId = application.deploymentJobs().projectId().get();
             this.availableSince = availableSince;
             this.concurrentlyWith = concurrentlyWith;
-
-            Optional<JobStatus> status = Optional.ofNullable(application.deploymentJobs().jobStatus().get(jobType));
-            this.isRetry = status.flatMap(JobStatus::jobError).filter(JobError.outOfCapacity::equals).isPresent();
-            this.reason = isRetry ? " Retrying on out of capacity" : reason;
-
-            this.change = application.change().effectiveAt(application.deploymentSpec(), controller.clock().instant());
+            this.reason = reason;
+            this.isRetry = isRetry;
             this.isApplicationUpgrade = change.application().isPresent();
-            this.platform = jobType == JobType.component
-                    ? Version.emptyVersion
-                    : application.deployVersionIn(jobType.zone(controller.system()).get(), controller);
-            this.application = jobType == JobType.component
-                    ? ApplicationVersion.unknown
-                    : application.deployApplicationVersionFor(jobType, controller, false);
+            this.change = change;
+            this.platformVersion = platformVersion;
+            this.applicationVersion = applicationVersion;
         }
 
         public ApplicationId id() { return id; }
@@ -360,8 +379,8 @@ public class DeploymentTrigger {
         public boolean isRetry() { return isRetry; }
         public boolean applicationUpgrade() { return isApplicationUpgrade; }
         public Change change() { return change; }
-        public Version platform() { return platform; }
-        public ApplicationVersion application() { return application; }
+        public Version platform() { return platformVersion; }
+        public ApplicationVersion application() { return applicationVersion; }
 
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
index 3abfe435113..aebd23f2386 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/ApplicationApiHandler.java
@@ -390,7 +390,7 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
         });
 
         // Compile version. The version that should be used when building an application
-        object.setString("compileVersion", application.oldestDeployedVersion().orElse(controller.systemVersion()).toFullString());
+        object.setString("compileVersion", application.oldestDeployedPlatform().orElse(controller.systemVersion()).toFullString());
 
         // Rotation
         Cursor globalRotationsArray = object.setArray("globalRotations");
@@ -679,10 +679,6 @@ public class ApplicationApiHandler extends LoggingRequestHandler {
 
         ApplicationId id = ApplicationId.from(tenantName, applicationName, "default");
         controller.applications().lockOrThrow(id, application -> {
-            if (application.change().isPresent())
-                throw new IllegalArgumentException("Can not start a deployment of " + application + " at this time: " +
-                                                   application.change() + " is in progress");
-
             controller.applications().deploymentTrigger().triggerChange(application.id(), Change.of(version));
         });
         return new MessageResponse("Triggered deployment of application '" + id + "' on version " + version);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
index 724b9899682..edb7d40f304 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiHandler.java
@@ -26,7 +26,6 @@ import java.util.Optional;
 import java.util.Scanner;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 import static java.util.stream.Collectors.groupingBy;
 
@@ -90,12 +89,9 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler {
                                   .orElse(JobType.component);
 
         Application application = controller.applications().require(ApplicationId.from(tenantName, applicationName, "default"));
-        controller.applications().deploymentTrigger().trigger(new DeploymentTrigger.Job(application,
-                                                                                        jobType,
-                                                                                        "Triggered from screwdriver/v1",
-                                                                                        controller.clock().instant(),
-                                                                                        Collections.emptySet(),
-                                                                                        controller));
+        controller.applications().deploymentTrigger().trigger(controller.applications().deploymentTrigger().forcedDeploymentJob(application,
+                                                                                                                                jobType,
+                                                                                                                                "Triggered from screwdriver/v1"));
 
         Slime slime = new Slime();
         Cursor cursor = slime.setObject();
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
index 320771c0c48..3195bff1325 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java
@@ -25,7 +25,6 @@ import com.yahoo.vespa.hosted.controller.application.ApplicationPackage;
 import com.yahoo.vespa.hosted.controller.application.ApplicationVersion;
 import com.yahoo.vespa.hosted.controller.application.Change;
 import com.yahoo.vespa.hosted.controller.application.Deployment;
-import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobError;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.JobStatus;
@@ -48,7 +47,6 @@ import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -244,7 +242,7 @@ public class ControllerTest {
         tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1);
 
         app1 = applications.require(app1.id());
-        assertEquals("First deployment gets system version", systemVersion, app1.oldestDeployedVersion().get());
+        assertEquals("First deployment gets system version", systemVersion, app1.oldestDeployedPlatform().get());
         assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
 
         // Unexpected deployment
@@ -267,13 +265,13 @@ public class ControllerTest {
         tester.deployAndNotify(app1, applicationPackage, true, productionUsWest1);
 
         app1 = applications.require(app1.id());
-        assertEquals("Application change preserves version", systemVersion, app1.oldestDeployedVersion().get());
+        assertEquals("Application change preserves version", systemVersion, app1.oldestDeployedPlatform().get());
         assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
 
         // A deployment to the new region gets the same version
         tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3);
         app1 = applications.require(app1.id());
-        assertEquals("Application change preserves version", systemVersion, app1.oldestDeployedVersion().get());
+        assertEquals("Application change preserves version", systemVersion, app1.oldestDeployedPlatform().get());
         assertEquals(systemVersion, tester.configServer().lastPrepareVersion().get());
         assertFalse("Change deployed", app1.change().isPresent());
 
@@ -286,7 +284,7 @@ public class ControllerTest {
         tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3);
 
         app1 = applications.require(app1.id());
-        assertEquals("Version upgrade changes version", newSystemVersion, app1.oldestDeployedVersion().get());
+        assertEquals("Version upgrade changes version", newSystemVersion, app1.oldestDeployedPlatform().get());
         assertEquals(newSystemVersion, tester.configServer().lastPrepareVersion().get());
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
index cf209b43512..8793ce3a7e4 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTester.java
@@ -46,6 +46,7 @@ import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.rotation.config.RotationsConfig;
 
 import java.util.Optional;
+import java.util.logging.Logger;
 
 import static org.junit.Assert.assertNotNull;
 
@@ -59,7 +60,7 @@ public final class ControllerTester {
 
     private final ControllerDb db;
     private final AthenzDbMock athenzDb;
-    private final ManualClock clock; // TODO jvenstad: Let this clock determine log time stamps.
+    private final ManualClock clock;
     private final ConfigServerClientMock configServer;
     private final ZoneRegistryMock zoneRegistry;
     private final GitHubMock gitHub;
@@ -109,6 +110,13 @@ public final class ControllerTester {
         this.buildService = buildService;
         this.controller = createController(db, curator, rotationsConfig, configServer, clock, gitHub, zoneRegistry,
                                            athenzDb, nameService, artifactRepository, entityService, buildService);
+
+        // Set the log output from the root logger to use timestamps from the manual clock ;)
+        Logger.getLogger("").getHandlers()[0].setFilter(
+                record -> {
+                    record.setMillis(clock.millis());
+                    return true;
+                });
     }
 
     public Controller controller() { return controller; }
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 84d2518b71a..701d5ed69db 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
@@ -296,8 +296,6 @@ public class DeploymentTriggerTest {
 
         Application app = tester.createAndDeploy("app1", 1, applicationPackageBuilder.build());
 
-
-
         tester.clock().advance(Duration.ofHours(1)); // --------------- Enter block window: 18:30
 
         readyJobsTrigger.run();
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 4e57311a518..02672f06ea6 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
@@ -814,7 +814,7 @@ public class UpgraderTest {
         Application default3 = tester.createAndDeploy("default3", 6, defaultApplicationPackage);
         Application default4 = tester.createAndDeploy("default4", 7, defaultApplicationPackage);
 
-        assertEquals(version, default0.oldestDeployedVersion().get());
+        assertEquals(version, default0.oldestDeployedPlatform().get());
 
         // New version is released
         version = Version.fromString("5.1");
@@ -890,10 +890,10 @@ public class UpgraderTest {
         tester.completeUpgrade(default2, version, defaultApplicationPackageV2);
         tester.completeUpgrade(default3, version, defaultApplicationPackageV2);
 
-        assertEquals(version, tester.application(default0.id()).oldestDeployedVersion().get());
-        assertEquals(version, tester.application(default1.id()).oldestDeployedVersion().get());
-        assertEquals(version, tester.application(default2.id()).oldestDeployedVersion().get());
-        assertEquals(version, tester.application(default3.id()).oldestDeployedVersion().get());
+        assertEquals(version, tester.application(default0.id()).oldestDeployedPlatform().get());
+        assertEquals(version, tester.application(default1.id()).oldestDeployedPlatform().get());
+        assertEquals(version, tester.application(default2.id()).oldestDeployedPlatform().get());
+        assertEquals(version, tester.application(default3.id()).oldestDeployedPlatform().get());
     }
 
     @Test