Merge pull request #3468 from vespa-engine/mpolden/confidence-excludes-failing-application-changes

Exclude failing application changes from confidence calculation

15 files changed, 257 insertions, 127 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 971438e008c..5cc4c441c3b 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
@@ -141,15 +141,16 @@ public class Application {
                                outstandingChange);
     }
     
-    public Application withJobTriggering(JobType type, Instant triggerTime, Controller controller) {
+    public Application withJobTriggering(JobType type, Optional<Change> change, Instant triggerTime, Controller controller) {
         return new Application(id, 
                                deploymentSpec,
                                validationOverrides,
                                deployments, 
-                               deploymentJobs.withTriggering(type, 
-                                                             determineTriggerVersion(type, controller), 
-                                                             determineTriggerRevision(type, controller), 
-                                                             triggerTime), 
+                               deploymentJobs.withTriggering(type,
+                                                             change,
+                                                             determineTriggerVersion(type, controller),
+                                                             determineTriggerRevision(type, controller),
+                                                             triggerTime),
                                deploying, 
                                outstandingChange);
     }
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 0087627675a..ccfcc0e4b0d 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
@@ -310,7 +310,10 @@ public class ApplicationController {
                     // - For self-triggered applications we don't have any trigger information, so we add it here.
                     // - For all applications, we don't have complete control over which revision is actually built,
                     //   so we update it here with what we actually triggered if necessary
-                    application = application.with(application.deploymentJobs().withTriggering(jobType, version, Optional.of(revision), clock.instant()));
+                    application = application.with(application.deploymentJobs()
+                                                           .withTriggering(jobType, application.deploying(),
+                                                                           version, Optional.of(revision),
+                                                                           clock.instant()));
                 }
 
                 // Delete zones not listed in DeploymentSpec, if allowed
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 59ccfc15bcc..b4ca00243b7 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
@@ -24,6 +24,7 @@ import java.util.Optional;
  * This is immutable.
  * 
  * @author bratseth
+ * @author mpolden
  */
 public class DeploymentJobs {
 
@@ -68,14 +69,17 @@ public class DeploymentJobs {
         return new DeploymentJobs(Optional.of(report.projectId()), status, jiraIssueId, report.selfTriggering());
     }
 
-    public DeploymentJobs withTriggering(DeploymentJobs.JobType jobType, 
-                                         Version version, 
-                                         Optional<ApplicationRevision> revision, 
+    public DeploymentJobs withTriggering(JobType jobType,
+                                         Optional<Change> change,
+                                         Version version,
+                                         Optional<ApplicationRevision> revision,
                                          Instant triggerTime) {
         Map<JobType, JobStatus> status = new LinkedHashMap<>(this.status);
         status.compute(jobType, (type, job) -> {
             if (job == null) job = JobStatus.initial(jobType);
-            return job.withTriggering(version, revision, triggerTime);
+            return job.withTriggering(version, revision,
+                                      change.isPresent() && change.get() instanceof Change.VersionChange,
+                                      triggerTime);
         });
         return new DeploymentJobs(projectId, status, jiraIssueId, selfTriggering);
     }
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 a30998d8517..96e91d41584 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
@@ -13,6 +13,7 @@ import java.util.Optional;
  * This is immutable.
  * 
  * @author bratseth
+ * @author mpolden
  */
 public class JobStatus {
     
@@ -52,17 +53,20 @@ public class JobStatus {
         return new JobStatus(type, Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); 
     }
 
-    public JobStatus withTriggering(Version version, Optional<ApplicationRevision> revision, Instant triggerTime) {
-        return new JobStatus(type, jobError, Optional.of(new JobRun(version, revision, triggerTime)),
+    public JobStatus withTriggering(Version version, Optional<ApplicationRevision> revision, boolean upgrade,
+                                    Instant triggerTime) {
+        return new JobStatus(type, jobError, Optional.of(new JobRun(version, revision, upgrade, triggerTime)),
                              lastCompleted, firstFailing, lastSuccess);
     }
 
     public JobStatus withCompletion(Optional<DeploymentJobs.JobError> jobError, Instant completionTime, Controller controller) {
         Version version;
         Optional<ApplicationRevision> revision;
+        boolean upgrade;
         if (type == DeploymentJobs.JobType.component) { // not triggered by us
             version = controller.systemVersion();
             revision = Optional.empty();
+            upgrade = false;
         }
         else if (! lastTriggered.isPresent()) {
             throw new IllegalStateException("Got notified about completion of " + this +
@@ -72,9 +76,10 @@ public class JobStatus {
         else {
             version = lastTriggered.get().version();
             revision = lastTriggered.get().revision();
+            upgrade = lastTriggered.get().upgrade();
         }
 
-        JobRun thisCompletion = new JobRun(version, revision, completionTime);
+        JobRun thisCompletion = new JobRun(version, revision, upgrade, completionTime);
 
         Optional<JobRun> firstFailing = this.firstFailing;
         if (jobError.isPresent() &&  ! this.firstFailing.isPresent())
@@ -166,15 +171,20 @@ public class JobStatus {
         private final Version version;
         private final Optional<ApplicationRevision> revision;
         private final Instant at;
+        private final boolean upgrade;
         
-        public JobRun(Version version, Optional<ApplicationRevision> revision, Instant at) {
+        public JobRun(Version version, Optional<ApplicationRevision> revision, boolean upgrade, Instant at) {
             Objects.requireNonNull(version, "version cannot be null");
             Objects.requireNonNull(revision, "revision cannot be null");
             Objects.requireNonNull(at, "at cannot be null");
             this.version = version;
             this.revision = revision;
+            this.upgrade = upgrade;
             this.at = at;
         }
+
+        /** Returns whether this job run was a Vespa upgrade */
+        public boolean upgrade() { return upgrade; }
         
         /** The Vespa version used on this run */
         public Version version() { return version; }
@@ -184,25 +194,26 @@ public class JobStatus {
         
         /** The time if this triggering or completion */
         public Instant at() { return at; }
-        
-        @Override 
+
+        @Override
         public int hashCode() {
-            return Objects.hash(version ,revision, at);
+            return Objects.hash(version, revision, upgrade, at);
         }
         
         @Override
         public boolean equals(Object o) {
-            if (o == this) return true;
-            if ( ! (o instanceof JobRun)) return false;
-            JobRun other = (JobRun)o;
-            if ( ! Objects.equals(other.version, this.version)) return false;
-            if ( ! Objects.equals(this.revision, other.revision)) return false;
-            if ( ! Objects.equals(this.at, other.at)) return false;
-            return true;
+            if (this == o) return true;
+            if (!(o instanceof JobRun)) return false;
+            JobRun jobRun = (JobRun) o;
+            return upgrade == jobRun.upgrade &&
+                   Objects.equals(version, jobRun.version) &&
+                   Objects.equals(revision, jobRun.revision) &&
+                   Objects.equals(at, jobRun.at);
         }
-        
+
         @Override
-        public String toString() { return "job run of version " + version + " " + revision + " at " + at; }
+        public String toString() { return "job run of version " + (upgrade() ? "upgrade " : "") + version + " "
+                                          + revision + " at " + at; }
         
     }
 
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
index fdb7fd8f3ff..82e6ca919e1 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentOrder.java
@@ -14,7 +14,6 @@ import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -94,12 +93,8 @@ public class DeploymentOrder {
         }
         DeploymentSpec.Step lastStep = deploymentSteps.get(deploymentSteps.size() - 1);
         Optional<DeploymentSpec.Step> step = fromJob(job, application);
-        if (!step.isPresent()) {
-            log.log(Level.WARNING, "Could not find deployment step for " + application.id().toShortString() +
-                                   " from job " + job);
-            return false;
-        }
-        return step.get().equals(lastStep);
+        // Step may not exist for all jobs, e.g. component
+        return step.map(s -> s.equals(lastStep)).orElse(false);
     }
 
     /** Returns jobs for given deployment spec, in the order they are declared */
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 e4fc00d532e..d2af5fc50cb 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
@@ -290,7 +290,7 @@ public class DeploymentTrigger {
                                cause));
         buildSystem.addJob(application.id(), jobType, first);
 
-        return application.withJobTriggering(jobType, clock.instant(), controller);
+        return application.withJobTriggering(jobType, application.deploying(), clock.instant(), controller);
     }
 
     private Application trigger(List<JobType> jobs, Application application, String cause, Lock lock) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
index c705a848b74..607ad4fd9f0 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializer.java
@@ -75,6 +75,7 @@ public class ApplicationSerializer {
     private final String versionField = "version";
     private final String revisionField = "revision";
     private final String atField = "at";
+    private final String upgradeField = "upgrade";
     
     // ------------------ Serialization
     
@@ -149,6 +150,7 @@ public class ApplicationSerializer {
         object.setString(versionField, jobRun.get().version().toString());
         if ( jobRun.get().revision().isPresent())
             toSlime(jobRun.get().revision().get(), object.setObject(revisionField));
+        object.setBool(upgradeField, jobRun.get().upgrade());
         object.setLong(atField, jobRun.get().at().toEpochMilli());
     }
     
@@ -196,7 +198,7 @@ public class ApplicationSerializer {
         return new Zone(Environment.from(object.field(environmentField).asString()),
                         RegionName.from(object.field(regionField).asString()));
     }
-    
+
     private Optional<ApplicationRevision> applicationRevisionFromSlime(Inspector object) {
         if ( ! object.valid()) return Optional.empty();
         String applicationPackageHash = object.field(applicationPackageHashField).asString();
@@ -251,14 +253,15 @@ public class ApplicationSerializer {
                              jobRunFromSlime(object.field(firstFailingField)),
                              jobRunFromSlime(object.field(lastSuccessField)));
     }
-    
+
     private Optional<JobStatus.JobRun> jobRunFromSlime(Inspector object) {
         if ( ! object.valid()) return Optional.empty();
         return Optional.of(new JobStatus.JobRun(new Version(object.field(versionField).asString()),
                                                 applicationRevisionFromSlime(object.field(revisionField)),
+                                                object.field(upgradeField).asBool(),
                                                 Instant.ofEpochMilli(object.field(atField).asLong())));
     }
-    
+
     private Optional<Long> optionalLong(Inspector field) {
         return field.valid() ? Optional.of(field.asLong()) : Optional.empty();
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
index bef96014e79..948301929cf 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VersionStatus.java
@@ -128,22 +128,24 @@ public class VersionStatus {
                                                                                 List<Application> applications) {
         Map<Version, DeploymentStatistics> versionMap = new HashMap<>();
 
-        for (Version infrastructureVersion : infrastructureVersions)
+        for (Version infrastructureVersion : infrastructureVersions) {
             versionMap.put(infrastructureVersion, DeploymentStatistics.empty(infrastructureVersion));
+        }
 
         for (Application application : applications) {
             DeploymentJobs jobs = application.deploymentJobs();
 
             // Note that each version deployed on this application exists
-            for (Deployment deployment : application.deployments().values())
+            for (Deployment deployment : application.deployments().values()) {
                 versionMap.computeIfAbsent(deployment.version(), DeploymentStatistics::empty);
+            }
 
             // List versions which have failing jobs, and versions which are in production
-            // TODO: Don't count applications which started failing on an application change, not a version change
 
             // Failing versions
             Map<Version, List<JobStatus>> failingJobsByVersion = jobs.jobStatus().values().stream()
                     .filter(jobStatus -> jobStatus.lastCompleted().isPresent())
+                    .filter(jobStatus -> jobStatus.lastCompleted().get().upgrade())
                     .filter(jobStatus -> jobStatus.jobError().isPresent())
                     .filter(jobStatus -> jobStatus.jobError().get() != DeploymentJobs.JobError.outOfCapacity)
                     .collect(Collectors.groupingBy(jobStatus -> jobStatus.lastCompleted().get().version()));
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 26f16a90c6b..3ab98d31a82 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
@@ -111,12 +111,12 @@ public class ControllerTest {
         Optional<ApplicationRevision> revision = ((Change.ApplicationChange)tester.controller().applications().require(app1.id()).deploying().get()).revision();
         assertTrue("Revision has been set during deployment", revision.isPresent());
         assertStatus(JobStatus.initial(stagingTest)
-                              .withTriggering(version1, revision, tester.clock().instant())
+                              .withTriggering(version1, revision, false, tester.clock().instant())
                               .withCompletion(Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller());
 
         // Causes first deployment job to be triggered
         assertStatus(JobStatus.initial(productionCorpUsEast1)
-                              .withTriggering(version1, revision, tester.clock().instant()), app1.id(), tester.controller());
+                              .withTriggering(version1, revision, false, tester.clock().instant()), app1.id(), tester.controller());
         tester.clock().advance(Duration.ofSeconds(1));
 
         // production job (failing)
@@ -124,9 +124,9 @@ public class ControllerTest {
         assertEquals(4, applications.require(app1.id()).deploymentJobs().jobStatus().size());
 
         JobStatus expectedJobStatus = JobStatus.initial(productionCorpUsEast1)
-                                               .withTriggering(version1, revision, tester.clock().instant()) // Triggered first without revision info
+                                               .withTriggering(version1, revision, false, tester.clock().instant()) // Triggered first without revision info
                                                .withCompletion(Optional.of(JobError.unknown), tester.clock().instant(), tester.controller())
-                                               .withTriggering(version1, revision, tester.clock().instant()); // Re-triggering (due to failure) has revision info
+                                               .withTriggering(version1, revision, false, tester.clock().instant()); // Re-triggering (due to failure) has revision info
                 
         assertStatus(expectedJobStatus, app1.id(), tester.controller());
 
@@ -146,20 +146,20 @@ public class ControllerTest {
         applications.notifyJobCompletion(mockReport(app1, component, true, false));
         tester.deployAndNotify(app1, applicationPackage, true, systemTest);
         assertStatus(JobStatus.initial(systemTest)
-                              .withTriggering(version1, revision, tester.clock().instant())
+                              .withTriggering(version1, revision, false, tester.clock().instant())
                               .withCompletion(Optional.empty(), tester.clock().instant(), tester.controller()), app1.id(), tester.controller());
         tester.deployAndNotify(app1, applicationPackage, true, stagingTest);
 
         // production job succeeding now
         tester.deployAndNotify(app1, applicationPackage, true, productionCorpUsEast1);
         expectedJobStatus = expectedJobStatus
-                .withTriggering(version1, revision, tester.clock().instant())
+                .withTriggering(version1, revision, false, tester.clock().instant())
                 .withCompletion(Optional.empty(), tester.clock().instant(), tester.controller());
         assertStatus(expectedJobStatus, app1.id(), tester.controller());
 
         // causes triggering of next production job
         assertStatus(JobStatus.initial(productionUsEast3)
-                              .withTriggering( version1, revision, tester.clock().instant()), 
+                              .withTriggering(version1, revision, false, tester.clock().instant()),
                      app1.id(), tester.controller());
         tester.deployAndNotify(app1, applicationPackage, true, productionUsEast3);
 
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 8439b55969a..f7af3ba7a3f 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
@@ -313,6 +313,55 @@ public class UpgraderTest {
         assertTrue("All jobs consumed", tester.buildSystem().jobs().isEmpty());
     }
 
+    @Test
+    public void testConfidenceIgnoresFailingApplicationChanges() {
+        DeploymentTester tester = new DeploymentTester();
+        Version version = Version.fromString("5.0");
+        tester.updateVersionStatus(version);
+
+        // Setup applications
+        Application canary0 = tester.createAndDeploy("canary0", 0, "canary");
+        Application canary1 = tester.createAndDeploy("canary1", 1, "canary");
+        Application default0 = tester.createAndDeploy("default0", 2, "default");
+        Application default1 = tester.createAndDeploy("default1", 3, "default");
+        Application default2 = tester.createAndDeploy("default2", 4, "default");
+        Application default3 = tester.createAndDeploy("default3", 5, "default");
+        Application default4 = tester.createAndDeploy("default4", 5, "default");
+
+        // New version is released
+        version = Version.fromString("5.1");
+        tester.updateVersionStatus(version);
+        assertEquals(version, tester.controller().versionStatus().systemVersion().get().versionNumber());
+        tester.upgrader().maintain();
+
+        // Canaries upgrade and raise confidence
+        tester.completeUpgrade(canary0, version, "canary");
+        tester.completeUpgrade(canary1, version, "canary");
+        tester.updateVersionStatus(version);
+        assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence());
+
+        // All applications upgrade successfully
+        tester.upgrader().maintain();
+        tester.completeUpgrade(default0, version, "default");
+        tester.completeUpgrade(default1, version, "default");
+        tester.completeUpgrade(default2, version, "default");
+        tester.completeUpgrade(default3, version, "default");
+        tester.completeUpgrade(default4, version, "default");
+        tester.updateVersionStatus(version);
+        assertEquals(VespaVersion.Confidence.high, tester.controller().versionStatus().systemVersion().get().confidence());
+
+        // Multiple application changes are triggered and fail, but does not affect version confidence as upgrade has
+        // completed successfully
+        tester.notifyJobCompletion(DeploymentJobs.JobType.component, default0, false);
+        tester.notifyJobCompletion(DeploymentJobs.JobType.component, default1, false);
+        tester.notifyJobCompletion(DeploymentJobs.JobType.component, default2, true);
+        tester.notifyJobCompletion(DeploymentJobs.JobType.component, default3, true);
+        tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default2, false);
+        tester.notifyJobCompletion(DeploymentJobs.JobType.systemTest, default3, false);
+        tester.updateVersionStatus(version);
+        assertEquals(VespaVersion.Confidence.normal, tester.controller().versionStatus().systemVersion().get().confidence());
+    }
+
     // TODO: Remove when corp-prod special casing is no longer needed
     @Test
     public void upgradesCanariesToControllerVersion() {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
index 20e3aae9114..1aaf41350f2 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/ApplicationSerializerTest.java
@@ -61,10 +61,10 @@ public class ApplicationSerializerTest {
         List<JobStatus> statusList = new ArrayList<>();
 
         statusList.add(JobStatus.initial(DeploymentJobs.JobType.systemTest)
-                                .withTriggering(Version.fromString("5.6.7"), Optional.empty(), Instant.ofEpochMilli(7))
+                                .withTriggering(Version.fromString("5.6.7"), Optional.empty(), true, Instant.ofEpochMilli(7))
                                 .withCompletion(Optional.empty(), Instant.ofEpochMilli(8), tester.controller()));
         statusList.add(JobStatus.initial(DeploymentJobs.JobType.stagingTest)
-                                .withTriggering(Version.fromString("5.6.6"), Optional.empty(), Instant.ofEpochMilli(5))
+                                .withTriggering(Version.fromString("5.6.6"), Optional.empty(), true, Instant.ofEpochMilli(5))
                                 .withCompletion(Optional.of(JobError.unknown), Instant.ofEpochMilli(6), tester.controller()));
 
         DeploymentJobs deploymentJobs = new DeploymentJobs(projectId, statusList, Optional.empty(), false);
@@ -133,6 +133,12 @@ public class ApplicationSerializerTest {
         assertEquals(JobError.unknown, applicationWithFailingJob.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.systemTest).jobError().get());
     }
 
+    @Test
+    public void testLegacySerializationWithoutUpgradeField() {
+        Application application = applicationSerializer.fromSlime(applicationSlime(false));
+        assertFalse(application.deploymentJobs().jobStatus().get(DeploymentJobs.JobType.systemTest).lastCompleted().get().upgrade());
+    }
+
     private Slime applicationSlime(boolean error) {
         return SlimeUtils.jsonToSlime(applicationJson(error).getBytes(StandardCharsets.UTF_8));
     }
@@ -147,10 +153,19 @@ public class ApplicationSerializerTest {
                 "    \"jobStatus\": [\n" +
                 "      {\n" +
                 "        \"jobType\": \"system-test\",\n" +
-                "        \"version\": \"5.6.7\",\n" +
-                "        \"completionTime\": 7,\n" +
                 (error ? "        \"jobError\": \"" + JobError.unknown + "\",\n" : "") +
-                "        \"lastTriggered\": 8\n" +
+                "        \"lastCompleted\": {\n" +
+                "          \"version\": \"6.1\",\n" +
+                "          \"revision\": {\n" +
+                "            \"applicationPackageHash\": \"dead\",\n" +
+                "            \"sourceRevision\": {\n" +
+                "              \"repositoryField\": \"git@git.foo\",\n" +
+                "              \"branchField\": \"origin/master\",\n" +
+                "              \"commitField\": \"cafe\"\n" +
+                "            }\n" +
+                "          },\n" +
+                "          \"at\": 1505725189469\n" +
+                "        }\n" +
                 "      }\n" +
                 "    ],\n" +
                 "    \"selfTriggering\": false\n" +
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
index ae88e7ab19c..ef606a0eced 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerControllerTester.java
@@ -28,9 +28,13 @@ import com.yahoo.vespa.hosted.controller.application.DeploymentJobs;
 import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensMock;
 import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.AthensDbMock;
 import com.yahoo.vespa.hosted.controller.api.integration.athens.mock.ZmsClientFactoryMock;
+import com.yahoo.vespa.hosted.controller.maintenance.JobControl;
+import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
+import com.yahoo.vespa.hosted.controller.persistence.MockCuratorDb;
 
 import java.io.File;
 import java.io.IOException;
+import java.time.Duration;
 import java.util.Optional;
 
 /**
@@ -42,14 +46,18 @@ public class ContainerControllerTester {
 
     private final ContainerTester containerTester;
     private final Controller controller;
+    private final Upgrader upgrader;
 
     public ContainerControllerTester(JDisc container, String responseFilePath) {
         containerTester = new ContainerTester(container, responseFilePath);
         controller = (Controller)container.components().getComponent("com.yahoo.vespa.hosted.controller.Controller");
+        upgrader = new Upgrader(controller, Duration.ofMinutes(2), new JobControl(new MockCuratorDb()));
     }
 
     public Controller controller() { return controller; }
 
+    public Upgrader upgrader() { return upgrader; }
+
     /** Returns the wrapped generic container tester */
     public ContainerTester containerTester() { return containerTester; }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
index 7a9e74a3c27..4fc6e91039c 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/ContainerTester.java
@@ -5,6 +5,7 @@ import com.yahoo.application.container.JDisc;
 import com.yahoo.application.container.handler.Request;
 import com.yahoo.application.container.handler.Response;
 import com.yahoo.collections.Pair;
+import com.yahoo.component.Version;
 import com.yahoo.io.IOUtils;
 import com.yahoo.slime.ArrayTraverser;
 import com.yahoo.slime.Inspector;
@@ -46,6 +47,11 @@ public class ContainerTester {
         controller.updateVersionStatus(VersionStatus.compute(controller));
     }
 
+    public void updateSystemVersion(Version version) {
+        Controller controller = (Controller)container.components().getComponent("com.yahoo.vespa.hosted.controller.Controller");
+        controller.updateVersionStatus(VersionStatus.compute(controller, version));
+    }
+
     public void assertResponse(Request request, File responseFile) throws IOException {
         assertResponse(request, responseFile, 200);
     }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
index ed04b73305d..c002c7fb24a 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/DeploymentApiTest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.hosted.controller.restapi.deployment;
 
 import com.google.common.collect.ImmutableSet;
 import com.yahoo.application.container.handler.Request;
+import com.yahoo.component.Version;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.Zone;
@@ -14,6 +15,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ContainerControllerTester;
 import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
+import org.junit.Before;
 import org.junit.Test;
 
 import java.io.File;
@@ -33,48 +35,40 @@ public class DeploymentApiTest extends ControllerContainerTest {
 
     private final static String responseFiles = "src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/";
 
+    private ContainerControllerTester tester;
+
+    @Before
+    public void before() {
+        tester = new ContainerControllerTester(container, responseFiles);
+    }
+
     @Test
     public void testDeploymentApi() throws IOException {
         ContainerControllerTester tester = new ContainerControllerTester(container, responseFiles);
-        tester.containerTester().updateSystemVersion();
+        Version version = Version.fromString("5.0");
+        tester.containerTester().updateSystemVersion(version);
         long projectId = 11;
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .environment(Environment.prod)
+                .region("corp-us-east-1")
+                .build();
 
-        {
-            Application failingApplication = tester.createApplication("domain1", "tenant1",
-                                                                      "application1");
-            ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                    .environment(Environment.prod)
-                    .region("corp-us-east-1")
-                    .build();
-            tester.notifyJobCompletion(failingApplication.id(), projectId, true, component);
-            tester.deploy(failingApplication, applicationPackage, new Zone(Environment.test,
-                                                                           RegionName.from("us-east-1")), projectId);
-            tester.notifyJobCompletion(failingApplication.id(), projectId, true, systemTest);
-            tester.deploy(failingApplication, applicationPackage, new Zone(Environment.staging,
-                                                                           RegionName.from("us-east-3")), projectId);
-            tester.notifyJobCompletion(failingApplication.id(), projectId, false, stagingTest);
-        }
+        // 2 applications deploy on current system version
+        Application failingApplication = tester.createApplication("domain1", "tenant1",
+                                                                  "application1");
+        Application productionApplication = tester.createApplication("domain2", "tenant2",
+                                                                     "application2");
+        deployCompletely(failingApplication, applicationPackage, projectId, true);
+        deployCompletely(productionApplication, applicationPackage, projectId, true);
 
-        {
-            Application productionApplication = tester.createApplication("domain2", "tenant2",
-                                                                         "application2");
-            ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                    .upgradePolicy("conservative")
-                    .environment(Environment.prod)
-                    .region("corp-us-east-1")
-                    .build();
-            tester.notifyJobCompletion(productionApplication.id(), projectId, true, component);
-            tester.deploy(productionApplication, applicationPackage, new Zone(Environment.test,
-                                                                              RegionName.from("us-east-1")), projectId);
-            tester.notifyJobCompletion(productionApplication.id(), projectId, true, systemTest);
-            tester.deploy(productionApplication, applicationPackage, new Zone(Environment.staging,
-                                                                              RegionName.from("us-east-3")), projectId);
-            tester.notifyJobCompletion(productionApplication.id(), projectId, true, stagingTest);
-            tester.deploy(productionApplication, applicationPackage, new Zone(Environment.staging,
-                                                                              RegionName.from("corp-us-east-1")),
-                          projectId);
-            tester.notifyJobCompletion(productionApplication.id(), projectId, true, productionCorpUsEast1);
-        }
+        // New version released
+        version = Version.fromString("5.1");
+        tester.containerTester().updateSystemVersion(version);
+
+        // Applications upgrade, 1/2 succeed
+        tester.upgrader().maintain();
+        deployCompletely(failingApplication, applicationPackage, projectId, false);
+        deployCompletely(productionApplication, applicationPackage, projectId, true);
 
         tester.controller().updateVersionStatus(censorConfigServers(VersionStatus.compute(tester.controller()),
                                                                     tester.controller()));
@@ -97,4 +91,20 @@ public class DeploymentApiTest extends ControllerContainerTest {
         return new VersionStatus(censored);
     }
 
+    private void deployCompletely(Application application, ApplicationPackage applicationPackage, long projectId,
+                                  boolean success) {
+        tester.notifyJobCompletion(application.id(), projectId, true, component);
+        tester.deploy(application, applicationPackage, new Zone(Environment.test,
+                                                                       RegionName.from("us-east-1")), projectId);
+        tester.notifyJobCompletion(application.id(), projectId, true, systemTest);
+        tester.deploy(application, applicationPackage, new Zone(Environment.staging,
+                                                                       RegionName.from("us-east-3")), projectId);
+        tester.notifyJobCompletion(application.id(), projectId, success, stagingTest);
+        if (success) {
+            tester.deploy(application, applicationPackage, new Zone(Environment.prod,RegionName.from("corp-us-east-1")),
+                          projectId);
+            tester.notifyJobCompletion(application.id(), projectId, true, productionCorpUsEast1);
+        }
+    }
+
 }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
index 77ee29f6c7a..c2e83373cf7 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/deployment/responses/root.json
@@ -1,57 +1,80 @@
 {
-  "versions":[
+  "versions": [
     {
-      "version":"(ignore)",
-      "confidence":"normal",
-      "commit":"(ignore)",
-      "date":0,
-      "controllerVersion":false,
-      "systemVersion":true,
-      "configServers":[
+      "version": "(ignore)",
+      "confidence": "normal",
+      "commit": "(ignore)",
+      "date": 0,
+      "controllerVersion": false,
+      "systemVersion": false,
+      "configServers": [],
+      "failingApplications": [],
+      "productionApplications": [
         {
-          "hostname":"config1.test"
-        },
-        {
-          "hostname":"config2.test"
+          "tenant": "tenant1",
+          "application": "application1",
+          "instance": "default",
+          "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1",
+          "upgradePolicy": "default"
         }
-      ],
-      "failingApplications":[
+      ]
+    },
+    {
+      "version": "(ignore)",
+      "confidence": "normal",
+      "commit": "(ignore)",
+      "date": 0,
+      "controllerVersion": false,
+      "systemVersion": false,
+      "configServers": [],
+      "failingApplications": [
         {
-          "tenant":"tenant1",
-          "application":"application1",
-          "instance":"default",
-          "url":"http://localhost:8080/application/v4/tenant/tenant1/application/application1",
+          "tenant": "tenant1",
+          "application": "application1",
+          "instance": "default",
+          "url": "http://localhost:8080/application/v4/tenant/tenant1/application/application1",
           "upgradePolicy": "default",
           "failingSince": "(ignore)"
         }
       ],
-      "productionApplications":[
+      "productionApplications": [
         {
-          "tenant":"tenant2",
-          "application":"application2",
-          "instance":"default",
-          "url":"http://localhost:8080/application/v4/tenant/tenant2/application/application2",
-          "upgradePolicy": "conservative"
+          "tenant": "tenant2",
+          "application": "application2",
+          "instance": "default",
+          "url": "http://localhost:8080/application/v4/tenant/tenant2/application/application2",
+          "upgradePolicy": "default"
         }
       ]
     },
     {
-      "version":"(ignore)",
-      "confidence":"normal",
-      "commit":"(ignore)",
-      "date":0,
-      "controllerVersion":true,
-      "systemVersion":false,
-      "configServers":[
-
-      ],
-      "failingApplications":[
-
+      "version": "(ignore)",
+      "confidence": "normal",
+      "commit": "(ignore)",
+      "date": 0,
+      "controllerVersion": false,
+      "systemVersion": true,
+      "configServers": [
+        {
+          "hostname": "config1.test"
+        },
+        {
+          "hostname": "config2.test"
+        }
       ],
-      "productionApplications":[
-
-      ]
+      "failingApplications": [],
+      "productionApplications": []
+    },
+    {
+      "version": "(ignore)",
+      "confidence": "normal",
+      "commit": "(ignore)",
+      "date": 0,
+      "controllerVersion": true,
+      "systemVersion": false,
+      "configServers": [],
+      "failingApplications": [],
+      "productionApplications": []
     }
   ]
 }
-