Upgrade during revision block window

6 files changed, 73 insertions, 10 deletions

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 4ed45af5e66..ca642d0fb2c 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
@@ -82,9 +82,9 @@ public class ApplicationList {
         return listOf(list.stream().filter(application -> application.change().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 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 currently does not have any failing jobs */
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 280d4c7f7e4..1fa579684de 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
@@ -59,6 +59,11 @@ public final class Change {
     /** Returns an instance representing no change */
     public static Change empty() { return empty; }
 
+    /** Returns a version of this change which replaces or adds this platform change */
+    public Change with(Version platformVersion) {
+        return new Change(Optional.of(platformVersion), application);
+    }
+
     /** Returns a version of this change which replaces or adds this application change */
     public Change with(ApplicationVersion applicationVersion) {
         return new Change(platform, Optional.of(applicationVersion));
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 017d1062ab3..75c01dcb9b3 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
@@ -216,7 +216,7 @@ public class DeploymentTrigger {
      */
     public void triggerChange(ApplicationId applicationId, Change change) {
         applications().lockOrThrow(applicationId, application -> {
-            if (application.change().isPresent() && ! application.deploymentJobs().hasFailures())
+            if (application.changeAt(controller.clock().instant()).isPresent() && ! application.deploymentJobs().hasFailures())
                 throw new IllegalArgumentException("Could not start " + change + " on " + application + ": " +
                                                    application.change() + " is already in progress");
             application = application.withChange(change);
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 ff3d44d5a8c..63361e5dcc4 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
@@ -22,7 +22,7 @@ public class OutstandingChangeDeployer extends Maintainer {
     protected void maintain() {
         ApplicationList applications = ApplicationList.from(controller().applications().asList()).notPullRequest();
         for (Application application : applications.asList()) {
-            if (!application.change().isPresent() && application.outstandingChange().isPresent()) {
+            if ( ! application.change().isPresent() && application.outstandingChange().isPresent()) {
                 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 4692c2fb23a..bd8b8fc8747 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
@@ -94,14 +94,14 @@ public class Upgrader extends Maintainer {
         applications = applications.notPullRequest(); // Pull requests are deployed as separate applications to test then deleted; No need to upgrade
         applications = applications.hasProductionDeployment();
         applications = applications.onLowerVersionThan(version);
-        applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading
+        applications = applications.notDeployingAt(controller().clock().instant()); // 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
         applications = applications.first(numberOfApplicationsToUpgrade()); // throttle upgrades
         for (Application application : applications.asList()) {
             try {
-                controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version));
+                controller().applications().deploymentTrigger().triggerChange(application.id(), application.change().with(version));
             } catch (IllegalArgumentException e) {
                 log.log(Level.INFO, "Could not trigger change: " + Exceptions.toMessageString(e));
             }
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 4d2e64d66c6..5d6fb76cacf 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
@@ -621,9 +621,6 @@ public class UpgraderTest {
     public void testBlockVersionChangeHalfwayThough() {
         ManualClock clock = new ManualClock(Instant.parse("2017-09-26T17:00:00.00Z")); // Tuesday, 17:00
         DeploymentTester tester = new DeploymentTester(new ControllerTester(clock));
-        ReadyJobsTrigger readyJobsTrigger = new ReadyJobsTrigger(tester.controller(),
-                                                                 Duration.ofHours(1),
-                                                                 new JobControl(tester.controllerTester().curator()));
 
         Version version = Version.fromString("5.0");
         tester.upgradeSystem(version);
@@ -936,4 +933,65 @@ public class UpgraderTest {
         }
     }
 
+    @Test
+    public void testBlockRevisionChangeHalfwayThoughThenUpgrade() {
+        ManualClock clock = new ManualClock(Instant.parse("2017-09-26T17:00:00.00Z")); // Tuesday, 17:00.
+        DeploymentTester tester = new DeploymentTester(new ControllerTester(clock));
+
+        Version version = Version.fromString("5.0");
+        tester.upgradeSystem(version);
+
+        ApplicationPackage applicationPackage = new ApplicationPackageBuilder()
+                .upgradePolicy("canary")
+                // Block upgrades on Tuesday in hours 18 and 19.
+                .blockChange(true, false, "tue", "18-19", "UTC")
+                .region("us-west-1")
+                .region("us-central-1")
+                .region("us-east-3")
+                .build();
+
+        Application app = tester.createAndDeploy("app1", 1, applicationPackage);
+
+        tester.jobCompletion(component).application(app).nextBuildNumber().uploadArtifact(applicationPackage).submit();
+
+        // Application upgrade starts.
+        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);
+        assertTrue(tester.buildService().jobs().isEmpty()); // Next job not triggered due to being in the block window.
+
+        // One hour passes, time is 19:00, still no upgrade.
+        tester.clock().advance(Duration.ofHours(1));
+        tester.triggerUntilQuiescence();
+        assertTrue("No jobs scheduled", tester.buildService().jobs().isEmpty());
+
+        // New version is released and upgrades are started in the two first production zones.
+        version = Version.fromString("5.1");
+        tester.upgradeSystem(version);
+        tester.deployAndNotify(app, applicationPackage, true, systemTest);
+        tester.deployAndNotify(app, applicationPackage, true, stagingTest);
+        tester.deployAndNotify(app, applicationPackage, true, productionUsWest1);
+
+        // Tests for central-1.
+        tester.deployAndNotify(app, applicationPackage, true, systemTest);
+        tester.deployAndNotify(app, applicationPackage, true, stagingTest);
+
+        // Another hour pass, time is 20:00 and both revision and version upgrades are now allowed.
+        tester.clock().advance(Duration.ofHours(1));
+        tester.triggerUntilQuiescence(); // Tests that trigger now test the full upgrade, since central-1 is still on old versions.
+        tester.deployAndNotify(app, applicationPackage, true, productionUsCentral1); // Only upgrade for now.
+        // west-1 is now fully upgraded, central-1 only has new version, and east-3 has only old versions.
+
+        // These tests were triggered with an upgrade of both version and revision. Since central-1 no longer upgrades version,
+        // it ignores the initial version of the staging job, and so the current staging job is OK for both zones.
+        tester.deployAndNotify(app, applicationPackage, true, systemTest);
+        tester.deployAndNotify(app, applicationPackage, true, stagingTest);
+        tester.deployAndNotify(app, applicationPackage, true, productionUsCentral1);
+        tester.deployAndNotify(app, applicationPackage, true, productionUsEast3);
+        assertTrue("All jobs consumed", tester.buildService().jobs().isEmpty());
+    }
+
 }