Merge pull request #7511 from vespa-engine/jvenstad/freeze-to-block-window

Jvenstad/freeze to block window

13 files changed, 106 insertions, 70 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 cd063e8e14b..0c33307bf80 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
@@ -129,24 +129,6 @@ public class Application {
     public Change change() { return change; }
 
     /**
-     * Returns the target that should be used for this application at the given instant, typically now.
-     *
-     * This will be any parts of current total change that aren't both blocked and not yet deployed anywhere.
-     */
-    public Change changeAt(Instant now) {
-        Change change = this.change;
-        if (     this.change.platform().isPresent()
-            &&   productionDeployments().values().stream().noneMatch(deployment -> deployment.version().equals(this.change.platform().get()))
-            && ! deploymentSpec.canUpgradeAt(now))
-            change = change.withoutPlatform();
-        if (     this.change.application().isPresent()
-            &&   productionDeployments().values().stream().noneMatch(deployment -> deployment.applicationVersion().equals(this.change.application().get()))
-            && ! deploymentSpec.canChangeRevisionAt(now))
-            change = change.withoutApplication();
-        return change;
-    }
-
-    /**
      * 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
      */
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 c7e7165cb91..2f56f2d9dd5 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
@@ -77,14 +77,14 @@ public class ApplicationList {
         return notUpgradingTo(version.get());
     }
 
-    /** Returns the subset of applications which are currently deploying a change */
-    public ApplicationList deploying() {
-        return listOf(list.stream().filter(application -> application.change().isPresent()));
+    /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */
+    public ApplicationList withChanges() {
+        return listOf(list.stream().filter(application -> application.change().isPresent() || application.outstandingChange().isPresent()));
     }
 
-    /** Returns the subset of applications which are currently really not deploying a change */
-    public ApplicationList notDeployingAt(Instant now) {
-        return listOf(list.stream().filter(application -> ! application.changeAt(now).isPresent()));
+    /** Returns the subset of applications which are currently not deploying a change */
+    public ApplicationList notDeploying() {
+        return listOf(list.stream().filter(application -> ! application.change().isPresent()));
     }
 
     /** Returns the subset of applications which currently does not have any failing jobs */
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 8bc61d6bc3b..aeb9e6fa453 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
@@ -230,7 +230,7 @@ public class DeploymentTrigger {
      */
     public void triggerChange(ApplicationId applicationId, Change change) {
         applications().lockOrThrow(applicationId, application -> {
-            if (application.get().changeAt(controller.clock().instant()).isPresent() && ! application.get().deploymentJobs().hasFailures())
+            if (application.get().change().isPresent() && ! application.get().deploymentJobs().hasFailures())
                 throw new IllegalArgumentException("Could not start " + change + " on " + application + ": " +
                                                    application.get().change() + " is already in progress");
             application = application.withChange(change);
@@ -276,7 +276,7 @@ public class DeploymentTrigger {
     private List<Job> computeReadyJobs() {
         return ApplicationList.from(applications().asList())
                               .withProjectId()
-                              .deploying()
+                              .withChanges()
                               .idList().stream()
                               .map(this::computeReadyJobs)
                               .flatMap(Collection::stream)
@@ -289,7 +289,7 @@ public class DeploymentTrigger {
     private List<Job> computeReadyJobs(ApplicationId id) {
         List<Job> jobs = new ArrayList<>();
         applications().get(id).ifPresent(application -> {
-            Change change = application.changeAt(clock.instant());
+            Change change = application.change();
             Optional<Instant> completedAt = max(application.deploymentJobs().statusOf(systemTest)
                                                         .<Instant>flatMap(job -> job.lastSuccess().map(JobRun::at)),
                                                 application.deploymentJobs().statusOf(stagingTest)
@@ -301,7 +301,7 @@ public class DeploymentTrigger {
             if (change.isPresent()) {
                 for (Step step : steps.production()) {
                     List<JobType> stepJobs = steps.toJobs(step);
-                    List<JobType> remainingJobs = stepJobs.stream().filter(job -> !isComplete(change, application, job)).collect(toList());
+                    List<JobType> remainingJobs = stepJobs.stream().filter(job -> ! isComplete(change, application, job)).collect(toList());
                     if (!remainingJobs.isEmpty()) { // Change is incomplete; trigger remaining jobs if ready, or their test jobs if untested.
                         for (JobType job : remainingJobs) {
                             Versions versions = Versions.from(change, application, deploymentFor(application, job),
@@ -313,7 +313,8 @@ public class DeploymentTrigger {
                                 if (!alreadyTriggered(application, versions)) {
                                     testJobs = emptyList();
                                 }
-                            } else if (testJobs == null) {
+                            }
+                            else if (testJobs == null) {
                                 testJobs = testJobs(application, versions,
                                                     String.format("Testing deployment for %s (%s)",
                                                                   job.jobName(), versions.toString()),
@@ -321,20 +322,28 @@ public class DeploymentTrigger {
                             }
                         }
                         completedAt = Optional.empty();
-                    } else { // All jobs are complete; find the time of completion of this step.
-                        if (stepJobs.isEmpty()) { // No jobs means this is delay step.
+                    }
+                    else { // All jobs are complete; find the time of completion of this step.
+                        if (stepJobs.isEmpty()) { // No jobs means this is a delay step.
                             Duration delay = ((DeploymentSpec.Delay) step).duration();
                             completedAt = completedAt.map(at -> at.plus(delay)).filter(at -> !at.isAfter(clock.instant()));
                             reason += " after a delay of " + delay;
-                        } else {
+                        }
+                        else {
                             completedAt = stepJobs.stream().map(job -> application.deploymentJobs().statusOf(job).get().lastCompleted().get().at()).max(naturalOrder());
                             reason = "Available change in " + stepJobs.stream().map(JobType::jobName).collect(joining(", "));
                         }
                     }
                 }
             }
-            if (testJobs == null) {
-                testJobs = testJobs(application, Versions.from(application, controller.systemVersion()),
+            if (testJobs == null) { // If nothing to test, but outstanding commits, test those.
+                Change latestChange = application.outstandingChange().application().isPresent()
+                                      ? change.with(application.outstandingChange().application().get())
+                                      : change;
+                testJobs = testJobs(application, Versions.from(latestChange,
+                                                               application,
+                                                               steps.sortedDeployments(application.productionDeployments().values()).stream().findFirst(),
+                                                               controller.systemVersion()),
                                     "Testing last changes outside prod", clock.instant());
             }
             jobs.addAll(testJobs);
@@ -362,14 +371,14 @@ public class DeploymentTrigger {
 
     private boolean isSuspendedInAnotherZone(Application application, ZoneId zone) {
         for (Deployment deployment : application.productionDeployments().values()) {
-            if ( ! deployment.zone().equals(zone)
-                 && controller.applications().isSuspended(new DeploymentId(application.id(), deployment.zone())))
+            if (   ! deployment.zone().equals(zone)
+                &&   controller.applications().isSuspended(new DeploymentId(application.id(), deployment.zone())))
                 return true;
         }
         return false;
     }
 
-    /** Returns whether job can trigger at given instant */
+    /** Returns whether the given job can trigger at the given instant */
     public boolean triggerAt(Instant instant, JobType job, Versions versions, Application application) {
         Optional<JobStatus> jobStatus = application.deploymentJobs().statusOf(job);
         if (!jobStatus.isPresent()) return true;
@@ -480,12 +489,10 @@ public class DeploymentTrigger {
     // ---------- Change management o_O ----------
 
     private boolean acceptNewApplicationVersion(Application application) {
-        if (    ! application.deploymentSpec().canChangeRevisionAt(clock.instant()) // If rolling out revision
-             &&   application.changeAt(clock.instant()).application().isPresent()   // which isn't complete, but in prod
-             && ! application.deploymentJobs().hasFailures()) return false;         // and isn't failing, delay the new submission.
-        if (application.change().application().isPresent()) return true; // More application changes are ok.
+        if ( ! application.deploymentSpec().canChangeRevisionAt(clock.instant())) return false;
+        if (application.change().application().isPresent()) return true; // Replacing a previous application change is ok.
         if (application.deploymentJobs().hasFailures()) return true; // Allow changes to fix upgrade problems.
-        return ! application.changeAt(clock.instant()).platform().isPresent();
+        return ! application.change().platform().isPresent();
     }
 
     private Change remainingChange(Application application) {
@@ -514,9 +521,8 @@ public class DeploymentTrigger {
         for (JobType jobType : steps(application.deploymentSpec()).testJobs()) {
             Optional<JobRun> completion = successOn(application, jobType, versions)
                     .filter(run -> versions.sourcesMatchIfPresent(run) || jobType == systemTest);
-            if (!completion.isPresent() && canTrigger(jobType, versions, application)) {
+            if ( ! completion.isPresent() && canTrigger(jobType, versions, application))
                 jobs.add(deploymentJob(application, versions, application.change(), jobType, reason, availableSince));
-            }
         }
         return jobs;
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
index 0e1bf111308..b2968270af9 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/ControllerMaintenance.java
@@ -60,7 +60,7 @@ public class ControllerMaintenance extends AbstractComponent {
         deploymentExpirer = new DeploymentExpirer(controller, maintenanceInterval, jobControl);
         deploymentIssueReporter = new DeploymentIssueReporter(controller, deploymentIssues, maintenanceInterval, jobControl);
         metricsReporter = new MetricsReporter(controller, metric, chefClient, jobControl, controller.system());
-        outstandingChangeDeployer = new OutstandingChangeDeployer(controller, maintenanceInterval, jobControl);
+        outstandingChangeDeployer = new OutstandingChangeDeployer(controller, Duration.ofMinutes(1), jobControl);
         versionStatusUpdater = new VersionStatusUpdater(controller, Duration.ofMinutes(1), jobControl);
         upgrader = new Upgrader(controller, maintenanceInterval, jobControl, curator);
         readyJobsTrigger = new ReadyJobsTrigger(controller, Duration.ofMinutes(1), jobControl);
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
index f87a0d625c0..0c267bd5f6a 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployer.java
@@ -7,7 +7,7 @@ import com.yahoo.vespa.hosted.controller.Controller;
 import java.time.Duration;
 
 /**
- * Deploys application changes which have been postponed due to an ongoing upgrade
+ * Deploys application changes which have been postponed due to an ongoing upgrade, or a block window.
  *
  * @author bratseth
  */
@@ -20,7 +20,9 @@ public class OutstandingChangeDeployer extends Maintainer {
     @Override
     protected void maintain() {
         for (Application application : controller().applications().asList()) {
-            if ( ! application.change().isPresent() && application.outstandingChange().isPresent()) {
+            if (   ! application.change().isPresent() // TODO jvenstad: Revisit this: should it check for only platform here?
+                &&   application.outstandingChange().isPresent()
+                &&   application.deploymentSpec().canChangeRevisionAt(controller().clock().instant())) {
                 controller().applications().deploymentTrigger().triggerChange(application.id(),
                                                                               application.outstandingChange());
             }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
index 06712ad7862..702df58ab98 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java
@@ -95,7 +95,7 @@ public class Upgrader extends Maintainer {
         applications = applications.hasProductionDeployment();
         applications = applications.onLowerVersionThan(version);
         applications = applications.allowMajorVersion(version.getMajor());
-        applications = applications.notDeployingAt(controller().clock().instant()); // wait with applications deploying an application change or already upgrading
+        applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading
         applications = applications.notFailingOn(version); // try to upgrade only if it hasn't failed on this version
         applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades
         applications = applications.byIncreasingDeployedVersion(); // start with lowest versions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
index 2e67f4b2efc..0d02c4003e7 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/JobControllerApiHandlerHelper.java
@@ -28,7 +28,6 @@ import com.yahoo.vespa.hosted.controller.deployment.Run;
 import com.yahoo.vespa.hosted.controller.deployment.Step;
 import com.yahoo.vespa.hosted.controller.deployment.Versions;
 import com.yahoo.vespa.hosted.controller.restapi.SlimeJsonResponse;
-import com.yahoo.vespa.hosted.controller.restapi.StringResponse;
 import com.yahoo.vespa.hosted.controller.versions.VespaVersion;
 
 import java.net.URI;
@@ -68,7 +67,7 @@ class JobControllerApiHandlerHelper {
      */
     static HttpResponse jobTypeResponse(Controller controller, ApplicationId id, URI baseUriForJobs) {
         Application application = controller.applications().require(id);
-        Change change = application.changeAt(controller.clock().instant());
+        Change change = application.change();
         DeploymentSteps steps = new DeploymentSteps(application.deploymentSpec(), controller::system);
 
         // The logic for pending runs imitates DeploymentTrigger logic; not good, but the trigger wiring must be re-written to reuse :S
@@ -160,7 +159,7 @@ class JobControllerApiHandlerHelper {
         }
         else
             lastPlatformObject.setString("pending",
-                                         application.changeAt(controller.clock().instant()).isPresent()
+                                         application.change().isPresent()
                                                  ? "Waiting for current deployment to complete"
                                                  : "Waiting for upgrade slot");
     }
@@ -259,7 +258,7 @@ class JobControllerApiHandlerHelper {
                         break;
                     for (JobType stepType : steps.toJobs(step)) {
                         if (pendingProduction.containsKey(stepType)) {
-                            Versions jobVersions = Versions.from(application.changeAt(controller.clock().instant()),
+                            Versions jobVersions = Versions.from(application.change(),
                                                                  application,
                                                                  Optional.ofNullable(application.deployments().get(stepType.zone(controller.system()))),
                                                                  controller.systemVersion());
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
index 26af0215fd6..7cb788b14aa 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/DeploymentTester.java
@@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.controller.integration.ApplicationStoreMock;
 import com.yahoo.vespa.hosted.controller.integration.ArtifactRepositoryMock;
 import com.yahoo.vespa.hosted.controller.integration.ConfigServerMock;
 import com.yahoo.vespa.hosted.controller.maintenance.JobControl;
+import com.yahoo.vespa.hosted.controller.maintenance.OutstandingChangeDeployer;
 import com.yahoo.vespa.hosted.controller.maintenance.ReadyJobsTrigger;
 import com.yahoo.vespa.hosted.controller.maintenance.Upgrader;
 import com.yahoo.vespa.hosted.controller.versions.VersionStatus;
@@ -48,6 +49,7 @@ public class DeploymentTester {
 
     private final ControllerTester tester;
     private final Upgrader upgrader;
+    private final OutstandingChangeDeployer outstandingChangeDeployer;
     private final ReadyJobsTrigger readyJobTrigger;
 
     public DeploymentTester() {
@@ -60,11 +62,14 @@ public class DeploymentTester {
 
         JobControl jobControl = new JobControl(tester.curator());
         this.upgrader = new Upgrader(tester.controller(), maintenanceInterval, jobControl, tester.curator());
+        this.outstandingChangeDeployer = new OutstandingChangeDeployer(tester.controller(), maintenanceInterval, jobControl);
         this.readyJobTrigger = new ReadyJobsTrigger(tester.controller(), maintenanceInterval, jobControl);
     }
 
     public Upgrader upgrader() { return upgrader; }
 
+    public OutstandingChangeDeployer outstandingChangeDeployer() { return outstandingChangeDeployer; }
+
     public ReadyJobsTrigger readyJobTrigger() { return readyJobTrigger; }
 
     public Controller controller() { return tester.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 70a47934262..9dd4c25050f 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
@@ -447,17 +447,22 @@ public class DeploymentTriggerTest {
               .sourceRevision(new SourceRevision("repository1", "master", "cafed00d"))
               .uploadArtifact(changedApplication)
               .submit();
-        assertTrue(tester.applications().require(app.id()).change().isPresent());
+        assertTrue(tester.applications().require(app.id()).outstandingChange().isPresent());
         tester.deployAndNotify(app, changedApplication, true, systemTest);
         tester.deployAndNotify(app, changedApplication, true, stagingTest);
 
+        tester.outstandingChangeDeployer().run();
+        assertTrue(tester.applications().require(app.id()).outstandingChange().isPresent());
+
         readyJobsTrigger.run();
         assertEquals(emptyList(), tester.buildService().jobs());
 
         tester.clock().advance(Duration.ofHours(2)); // ---------------- Exit block window: 20:30
+
+        tester.outstandingChangeDeployer().run();
+        assertFalse(tester.applications().require(app.id()).outstandingChange().isPresent());
+
         tester.deploymentTrigger().triggerReadyJobs(); // Schedules staging test for the blocked production job(s)
-        // TODO jvenstad: Required now because changes during block window used empty source -- improvement to use first untested production job with change to test :)
-        tester.deployAndNotify(app, changedApplication, true, stagingTest);
         assertEquals(singletonList(buildJob(app, productionUsWest1)), tester.buildService().jobs());
     }
 
@@ -466,7 +471,7 @@ public class DeploymentTriggerTest {
         ManualClock clock = new ManualClock(Instant.parse("2017-09-26T17:30:00.00Z")); // Tuesday, 17:30
         DeploymentTester tester = new DeploymentTester(new ControllerTester(clock));
         ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
-                .blockChange(false, true, "tue", "18", "UTC")
+                .blockChange(true, true, "tue", "18", "UTC")
                 .region("us-west-1")
                 .region("us-east-3")
                 .build();
@@ -483,25 +488,33 @@ public class DeploymentTriggerTest {
         tester.deployAndNotify(application, applicationPackage, true, stagingTest);
         tester.deployAndNotify(application, applicationPackage, true, systemTest);
 
-        // Entering block window will keep the outstanding change in place, but a new component run triggers a new change.
-        // This component completion should remove the older outstanding change, to avoid a later downgrade.
+        // Entering block window will keep the outstanding change in place.
         clock.advance(Duration.ofHours(1));
+        tester.outstandingChangeDeployer().run();
         tester.deployAndNotify(application, applicationPackage, true, productionUsWest1);
         assertEquals(BuildJob.defaultBuildNumber, tester.application(application.id()).deploymentJobs().jobStatus()
                                                                .get(productionUsWest1).lastSuccess().get().application().buildNumber().getAsLong());
         assertEquals((BuildJob.defaultBuildNumber + 1), tester.application(application.id()).outstandingChange().application().get().buildNumber().getAsLong());
 
         tester.readyJobTrigger().maintain();
-        // Platform upgrade keeps rolling, since it has already deployed in a production zone.
-        assertEquals(1, tester.buildService().jobs().size());
+        // Platform upgrade keeps rolling, since it has already deployed in a production zone, and tests for the new revision have also started.
+        assertEquals(3, tester.buildService().jobs().size());
         tester.deployAndNotify(application, applicationPackage, true, productionUsEast3);
-        assertEquals(emptyList(), tester.buildService().jobs());
+        assertEquals(2, tester.buildService().jobs().size());
 
+        // Upgrade is done, and oustanding change rolls out when block window ends.
+        assertEquals(Change.empty(), tester.application(application.id()).change());
+        assertFalse(tester.application(application.id()).change().isPresent());
+        assertTrue(tester.application(application.id()).outstandingChange().isPresent());
 
-        // New component triggers a full deployment of new application version, but only after the upgrade is done.
-        tester.jobCompletion(component).application(application).nextBuildNumber().nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(application, applicationPackage, true, stagingTest);
         tester.deployAndNotify(application, applicationPackage, true, systemTest);
+        clock.advance(Duration.ofHours(1));
+        tester.outstandingChangeDeployer().run();
+        assertTrue(tester.application(application.id()).change().isPresent());
+        assertFalse(tester.application(application.id()).outstandingChange().isPresent());
+
+        tester.readyJobTrigger().run();
         tester.deployAndNotify(application, applicationPackage, true, productionUsWest1);
         tester.deployAndNotify(application, applicationPackage, true, productionUsEast3);
 
@@ -622,6 +635,10 @@ public class DeploymentTriggerTest {
         assertEquals(triggered, app1.get().deploymentJobs().jobStatus().get(productionUsCentral1).lastTriggered().get().at());
         tester.deployAndNotify(application1, applicationPackage, false, productionEuWest1);
 
+        //Eagerly triggered system and staging tests complete.
+        tester.deployAndNotify(application1, applicationPackage, true, systemTest);
+        tester.deployAndNotify(application1, applicationPackage, true, stagingTest);
+
         // Roll out a new application version, which gives a dual change -- this should trigger us-central-1, but only as long as it hasn't yet deployed there.
         tester.jobCompletion(component).application(application1).nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.deployAndNotify(application1, applicationPackage, false, productionEuWest1);
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
index a61d6bee91a..b87458e6ecf 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/deployment/InternalStepRunnerTest.java
@@ -6,7 +6,9 @@ import com.yahoo.config.application.api.DeploymentSpec;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.slime.ArrayTraverser;
+import com.yahoo.slime.Cursor;
 import com.yahoo.slime.Inspector;
+import com.yahoo.slime.Slime;
 import com.yahoo.vespa.config.SlimeUtils;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.ConfigChangeActions;
 import com.yahoo.vespa.hosted.controller.api.application.v4.model.configserverbindings.RefeedAction;
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
index 4483122d554..bc22df7bf46 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OutstandingChangeDeployerTest.java
@@ -66,6 +66,8 @@ public class OutstandingChangeDeployerTest {
         tester.deployAndNotify(app, applicationPackage, true, JobType.systemTest);
         tester.deployAndNotify(app, applicationPackage, true, JobType.stagingTest);
         tester.deployAndNotify(app, applicationPackage, true, JobType.productionUsWest1);
+        tester.deployAndNotify(app, applicationPackage, true, JobType.systemTest);
+        tester.deployAndNotify(app, applicationPackage, true, JobType.stagingTest);
         assertEquals("Upgrade done", 0, tester.buildService().jobs().size());
 
         deployer.maintain();
@@ -73,10 +75,9 @@ public class OutstandingChangeDeployerTest {
         app = tester.application("app1");
         assertEquals("1.0.43-cafed00d", app.change().application().get().id());
         List<BuildService.BuildJob> jobs = tester.buildService().jobs();
-        assertEquals(2, jobs.size());
+        assertEquals(1, jobs.size());
+        assertEquals(JobType.productionUsWest1.jobName(), jobs.get(0).jobName());
         assertEquals(11, jobs.get(0).projectId());
-        tester.assertRunning(JobType.systemTest, app.id());
-        tester.assertRunning(JobType.stagingTest, app.id());
         assertFalse(tester.application("app1").outstandingChange().isPresent());
     }
 
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 04c0e7e9a6f..6d4f58531ae 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
@@ -1042,23 +1042,37 @@ public class UpgraderTest {
 
         tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit();
 
-        // Application upgrade starts.
+        // Application revision starts rolling out.
         tester.upgrader().maintain();
         tester.triggerUntilQuiescence();
         tester.deployAndNotify(app, applicationPackage, true, systemTest);
         tester.deployAndNotify(app, applicationPackage, true, stagingTest);
         clock.advance(Duration.ofHours(1)); // Entering block window after prod job is triggered.
         tester.deployAndNotify(app, applicationPackage, true, productionUsWest1);
-        assertEquals(1, tester.buildService().jobs().size()); // Next job triggered in spite of block, because it is already rolling out.
+        assertEquals(1, tester.buildService().jobs().size());
 
-        // New revision is submitted, but is stored as outstanding, since the previous revision is proceeding in good fashion.
+        // New revision is submitted, but is stored as outstanding, since the upgrade is proceeding in good fashion.
         tester.jobCompletion(component).application(app).nextBuildNumber().nextBuildNumber().uploadArtifact(applicationPackage).submit();
         tester.triggerUntilQuiescence();
-        assertEquals(1, tester.buildService().jobs().size()); // Still just the running revision upgrade.
+        assertEquals(3, tester.buildService().jobs().size()); // Just the running upgrade, and tests for the new revision.
 
+        tester.deployAndNotify(app, applicationPackage, true, systemTest);
+        tester.deployAndNotify(app, applicationPackage, true, stagingTest);
         tester.deployAndNotify(app, applicationPackage, true, productionUsCentral1);
         tester.deployAndNotify(app, applicationPackage, true, productionUsEast3);
         assertEquals(Collections.emptyList(), tester.buildService().jobs()); // No jobs left.
+
+        tester.outstandingChangeDeployer().run();
+        assertFalse(tester.application(app.id()).change().isPresent());
+        clock.advance(Duration.ofHours(2));
+
+        tester.outstandingChangeDeployer().run();
+        assertTrue(tester.application(app.id()).change().isPresent());
+        tester.readyJobTrigger().run();
+        tester.deployAndNotify(app, applicationPackage, true, productionUsWest1);
+        tester.deployAndNotify(app, applicationPackage, true, productionUsCentral1);
+        tester.deployAndNotify(app, applicationPackage, true, productionUsEast3);
+        assertFalse(tester.application(app.id()).change().isPresent());
     }
 
 }
diff --git a/vespajlib/src/test/java/com/yahoo/slime/SlimeTestCase.java b/vespajlib/src/test/java/com/yahoo/slime/SlimeTestCase.java
index 731067975ce..f3be33009b0 100644
--- a/vespajlib/src/test/java/com/yahoo/slime/SlimeTestCase.java
+++ b/vespajlib/src/test/java/com/yahoo/slime/SlimeTestCase.java
@@ -9,6 +9,14 @@ import static org.hamcrest.CoreMatchers.*;
 public class SlimeTestCase {
 
     @Test
+    public void lul() throws Exception {
+        Slime s = new Slime();
+        Cursor c = s.setObject();
+        c.setObject("h").setObject("h").setObject("h").setString("h", "a");
+        new JsonFormat(false).encode(System.err, s);
+    }
+
+    @Test
     public void testTypeIds() {
         System.out.println("testing type identifiers...");