Refactoring, no real changes

9 files changed, 72 insertions, 64 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 ec1394b59d6..b19db6b1529 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
@@ -161,33 +161,23 @@ public class Application {
                        .orElse(oldestDeployedVersion().orElse(controller.systemVersion()));
     }
 
-    /** Returns the Vespa version to use for the given job */
-    public Version deployVersionFor(DeploymentJobs.JobType jobType, Controller controller) {
-        // TODO jvenstad: Eliminate this and its sibling for component.
-        return jobType == DeploymentJobs.JobType.component
-                ? controller.systemVersion()
-                : deployVersionIn(jobType.zone(controller.system()).get(), controller);
-    }
-
     /**
      * 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.
      */
-    public Optional<ApplicationVersion> deployApplicationVersionFor(DeploymentJobs.JobType jobType,
-                                                                    Controller controller,
-                                                                    boolean useCurrentVersion) {
-        if (jobType == DeploymentJobs.JobType.component)
-            return Optional.empty();
-
+    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);
+                                                       .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)))));
+                                                                                       .orElse(changeVersion.orElse(null)))))
+                       .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version to use for " + jobType));
     }
 
     /** 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 3ca4d2c34f1..8db79ed80e0 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
@@ -368,9 +368,7 @@ public class ApplicationController {
             if (!job.isPresent()) {
                 throw new IllegalArgumentException("No job found for zone " + zone);
             }
-            version = application.deployApplicationVersionFor(job.get(), controller, deployCurrentVersion)
-                                 .orElseThrow(() -> new IllegalArgumentException("Cannot determine application version to use for " +
-                                                                    job.get()));
+            version = application.deployApplicationVersionFor(job.get(), controller, deployCurrentVersion);
             pkg = new ApplicationPackage(artifactRepository.getApplicationPackage(application.id(), version.id()));
         }
         return new Pair<>(pkg, version);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
index 51ec621ce22..b7e72987573 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/LockedApplication.java
@@ -20,6 +20,8 @@ import com.yahoo.vespa.hosted.controller.application.Deployment;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.application.DeploymentJobs.JobType;
 import com.yahoo.vespa.hosted.controller.application.DeploymentMetrics;
+import com.yahoo.vespa.hosted.controller.application.JobStatus;
+import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 import com.yahoo.vespa.hosted.controller.rotation.RotationId;
 
 import java.time.Instant;
@@ -67,10 +69,8 @@ public class LockedApplication extends Application {
         );
     }
 
-    public LockedApplication withJobTriggering(JobType type, Instant triggerTime,
-                                               Version version, ApplicationVersion applicationVersion,
-                                               String reason) {
-        return new LockedApplication(new Builder(this).with(deploymentJobs().withTriggering(type, version, applicationVersion, reason, triggerTime)));
+    public LockedApplication withJobTriggering(JobType jobType, JobStatus.JobRun job) {
+        return new LockedApplication(new Builder(this).with(deploymentJobs().withTriggering(jobType, job)));
     }
 
     public LockedApplication withNewDeployment(ZoneId zone, ApplicationVersion applicationVersion, Version version,
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 6de0a2e4cf6..d8d07ef57ef 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
@@ -50,12 +50,22 @@ public final class Change {
         return platform.isPresent() || application.isPresent();
     }
 
-    /** Returns the platform version change which should currently be deployed, if any */
+    /** Returns the platform version carried by this. */
     public Optional<Version> platform() { return platform; }
 
-    /** Returns the application version change which should currently be deployed, if any */
+    /** 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 5987d636e2c..95ef199aac4 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
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.hosted.controller.application;
 
 import com.google.common.collect.ImmutableMap;
-import com.yahoo.component.Version;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
@@ -66,18 +65,11 @@ public class DeploymentJobs {
         return new DeploymentJobs(Optional.of(report.projectId()), status, issueId);
     }
 
-    public DeploymentJobs withTriggering(JobType jobType,
-                                         Version version,
-                                         ApplicationVersion applicationVersion,
-                                         String reason,
-                                         Instant triggerTime) {
+    public DeploymentJobs withTriggering(JobType jobType, JobStatus.JobRun jobRun) {
         Map<JobType, JobStatus> status = new LinkedHashMap<>(this.status);
-        status.compute(jobType, (type, job) -> {
+        status.compute(jobType, (__, job) -> {
             if (job == null) job = JobStatus.initial(jobType);
-            return job.withTriggering(version,
-                                      applicationVersion,
-                                      reason,
-                                      triggerTime);
+            return job.withTriggering(jobRun);
         });
         return new DeploymentJobs(projectId, status, issueId);
     }
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 4a1e483b171..265eac5d522 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
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.application;
 
 import com.yahoo.component.Version;
 import com.yahoo.vespa.hosted.controller.Controller;
+import com.yahoo.vespa.hosted.controller.deployment.DeploymentTrigger;
 
 import java.time.Instant;
 import java.util.Objects;
@@ -55,11 +56,12 @@ public class JobStatus {
         return new JobStatus(type, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty());
     }
 
-    public JobStatus withTriggering(Version version, ApplicationVersion applicationVersion, String reason,
-                                    Instant triggerTime) {
-        return new JobStatus(type, jobError, Optional.of(new JobRun(-1, version, applicationVersion, reason,
-                                                                    triggerTime)),
-                             lastCompleted, firstFailing, lastSuccess);
+    public JobStatus withTriggering(Version platform, ApplicationVersion application, String reason, Instant triggeredAt) {
+        return withTriggering(new JobRun(-1, platform, application, reason, triggeredAt));
+    }
+
+    public JobStatus withTriggering(JobRun jobRun) {
+        return new JobStatus(type, jobError, Optional.of(jobRun), lastCompleted, firstFailing, lastSuccess);
     }
 
     public JobStatus withCompletion(long runId, Optional<DeploymentJobs.JobError> jobError, Instant completionTime,
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 8593357d684..15de1e2cfd9 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
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.deployment;
 
+import com.yahoo.component.Version;
 import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.Environment;
@@ -154,12 +155,7 @@ 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,
-                                                                       clock.instant(),
-                                                                       application.deployVersionFor(job.jobType, controller),
-                                                                       application.deployApplicationVersionFor(job.jobType, controller, false)
-                                                                                  .orElse(ApplicationVersion.unknown),
-                                                                       job.reason)));
+                    applications().store(application.withJobTriggering(job.jobType, runOf(job))));
             return true;
         }
         // TODO jvenstad: On 404: set empty projectId (and send ticket?). Throw and catch NoSuchElementException.
@@ -231,10 +227,11 @@ public class DeploymentTrigger {
                 }
                 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));
+                        jobs.add(new Job(application, job, reason, completedAt.get(), stepJobs, controller));
                     completedAt = Optional.empty();
                 }
             }
+            // TODO jvenstad: Replace with completion of individual parts of Change.
             if (completedAt.isPresent())
                 applications().store(application.withChange(Change.empty()));
         });
@@ -313,41 +310,58 @@ public class DeploymentTrigger {
         return ! application.change().effectiveAt(application.deploymentSpec(), clock.instant()).platform().isPresent();
     }
 
+    private JobStatus.JobRun runOf(Job job) {
+        return new JobStatus.JobRun(-1, job.platform, job.application, job.reason, clock.instant());
+    }
+
 
     public static class Job {
 
         private final ApplicationId id;
         private final JobType jobType;
         private final long projectId;
-        private final Change change;
         private final String reason;
         private final Instant availableSince;
-        private final boolean retry;
         private final Collection<JobType> concurrentlyWith;
+        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!
 
-        public Job(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith) {
+        public Job(Application application, JobType jobType, String reason, Instant availableSince, Collection<JobType> concurrentlyWith, Controller controller) {
             this.id = application.id();
             this.jobType = jobType;
             this.projectId = application.deploymentJobs().projectId().get();
-            this.change = application.change();
             this.availableSince = availableSince;
             this.concurrentlyWith = concurrentlyWith;
 
-            JobStatus status = application.deploymentJobs().jobStatus().get(jobType);
-            this.retry = status != null && status.jobError().filter(JobError.outOfCapacity::equals).isPresent();
-            this.reason = retry ? "Retrying on out of capacity" : reason;
+            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.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);
         }
 
         public ApplicationId id() { return id; }
         public JobType jobType() { return jobType; }
         public long projectId() { return projectId; }
-        public Change change() { return change; }
         public String reason() { return reason; }
         public Instant availableSince() { return availableSince; }
-        public boolean isRetry() { return retry; }
-        public boolean applicationUpgrade() { return change.application().isPresent(); }
+        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; }
 
     }
 
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 3db15806f26..724b9899682 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
@@ -94,7 +94,8 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler {
                                                                                         jobType,
                                                                                         "Triggered from screwdriver/v1",
                                                                                         controller.clock().instant(),
-                                                                                        Collections.emptySet()));
+                                                                                        Collections.emptySet(),
+                                                                                        controller));
 
         Slime slime = new Slime();
         Cursor cursor = slime.setObject();
@@ -122,12 +123,13 @@ public class ScrewdriverApiHandler extends LoggingRequestHandler {
             Cursor jobArray = jobTypesObject.setArray(jobType.jobName());
             jobs.forEach(job -> {
                 Cursor buildJobObject = jobArray.addObject();
-                buildJobObject.setString("applicationId", job.id().toString());
-                buildJobObject.setString("jobName", job.jobType().jobName());
+                buildJobObject.setString("id", job.id().toString());
                 buildJobObject.setLong("projectId", job.projectId());
-                buildJobObject.setString("change", job.change().toString());
                 buildJobObject.setString("reason", job.reason());
+                buildJobObject.setString("change", job.change().toString());
                 buildJobObject.setLong("availableSince", job.availableSince().toEpochMilli());
+                buildJobObject.setString("platform", job.platform().toString());
+                buildJobObject.setString("application", job.application().toString());
             });
         });
         return new SlimeJsonResponse(slime);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
index 492cc1b70dd..2911b9ab72f 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/screwdriver/ScrewdriverApiTest.java
@@ -57,11 +57,11 @@ public class ScrewdriverApiTest extends ControllerContainerTest {
                                    new byte[0], Request.Method.POST),
                        200, "{\"message\":\"Triggered component for tenant1.application1\"}");
 
-        // Triggers specific job when given
+        // Triggers specific job when given -- fails here because the job has never run before, and so application version can't be resolved.
         assertResponse(new Request("http://localhost:8080/screwdriver/v1/trigger/tenant/" +
                                    app.id().tenant().value() + "/application/" + app.id().application().value(),
                                    "staging-test".getBytes(StandardCharsets.UTF_8), Request.Method.POST),
-                       200, "{\"message\":\"Triggered staging-test for tenant1.application1\"}");
+                       400, "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Cannot determine application version to use for stagingTest\"}");
     }
 
 }