Ensure that the scheduling instant is never in the past

2 files changed, 31 insertions, 26 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java
index 6f4a8429c74..30a98cbfacd 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeScheduler.java
@@ -79,15 +79,21 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
         return hourOfDay >= startHour && hourOfDay <= 12 && dayOfWeek < 5;
     }
 
-    /** Returns the earliest time an upgrade can be scheduled on the day of instant, in given system */
+    /** Returns the earliest time, at or after instant, an upgrade can be scheduled */
     private static Instant schedulingInstant(Instant instant, SystemName system) {
-        instant = instant.truncatedTo(ChronoUnit.DAYS);
+        ChronoUnit schedulingResolution = ChronoUnit.HOURS;
         while (!canTriggerAt(instant, system.isCd())) {
-            instant = instant.plus(Duration.ofHours(1));
+            instant = instant.truncatedTo(schedulingResolution)
+                             .plus(schedulingResolution.getDuration());
         }
         return instant;
     }
 
+    /** Returns the remaining cool-down period relative to releaseAge */
+    private static Duration remainingCooldownOf(Duration cooldown, Duration releaseAge) {
+        return releaseAge.compareTo(cooldown) < 0 ? cooldown.minus(releaseAge) : Duration.ZERO;
+    }
+
     private interface Release {
 
         /** The pending change for this release at given instant, if any */
@@ -123,7 +129,8 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
         public Optional<Change> change(Version currentVersion, Instant instant) {
             OsRelease release = artifactRepository.osRelease(currentVersion.getMajor(), tag());
             if (!release.version().isAfter(currentVersion)) return Optional.empty();
-            Instant scheduleAt = schedulingInstant(release.taggedAt().plus(cooldown()), system);
+            Duration cooldown = remainingCooldownOf(cooldown(), release.age(instant));
+            Instant scheduleAt = schedulingInstant(instant.plus(cooldown), system);
             return Optional.of(new Change(release.version(), Duration.ZERO, scheduleAt));
         }
 
@@ -165,24 +172,19 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
                 predicatedInstant = predicatedInstant.plus(Duration.ofDays(1));
                 version = findVersion(predicatedInstant, currentVersion);
             }
-            Duration cooldown = remainingCooldownAt(instant, version);
+            Duration cooldown = remainingCooldownOf(cooldown(), version.age(instant));
             Instant schedulingInstant = schedulingInstant(instant.plus(cooldown), system);
             return Optional.of(new Change(version.version(), upgradeBudget(), schedulingInstant));
         }
 
-        private Duration upgradeBudget() {
-            return system.isCd() ? Duration.ZERO : Duration.ofDays(14);
-        }
-
-        private Duration remainingCooldownAt(Instant instant, CalendarVersion version) {
-            Duration minAge = system.isCd()
+        private Duration cooldown() {
+            return system.isCd()
                     ? Duration.ofDays(1)                          // CD: Give new releases some time to propagate
                     : Duration.ofDays(7 - RELEASE_DAY.ordinal()); // non-CD: Wait until start of the following week
-            Duration age = version.age(instant);
-            if (age.compareTo(minAge) < 0) {
-                return minAge.minus(age);
-            }
-            return Duration.ZERO;
+        }
+
+        private Duration upgradeBudget() {
+            return system.isCd() ? Duration.ZERO : Duration.ofDays(14);
         }
 
         /** Find the most recent version available according to the scheduling step, relative to now */
@@ -211,10 +213,9 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
                                            date);
             }
 
-            /** Returns the age of this at given instant, in whole days */
+            /** Returns the age of this at given instant */
             private Duration age(Instant instant) {
-                return Duration.between(date.atStartOfDay().toInstant(ZoneOffset.UTC),
-                                        instant.truncatedTo(ChronoUnit.DAYS));
+                return Duration.between(date.atStartOfDay().toInstant(ZoneOffset.UTC), instant);
             }
 
         }
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java
index 016db28c2aa..39264286e44 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgradeSchedulerTest.java
@@ -131,7 +131,7 @@ public class OsUpgradeSchedulerTest {
     void schedule_stable_release() {
         ControllerTester tester = new ControllerTester();
         OsUpgradeScheduler scheduler = new OsUpgradeScheduler(tester.controller(), Duration.ofDays(1));
-        Instant t0 = Instant.parse("2021-06-21T06:00:00.00Z"); // Outside trigger period
+        Instant t0 = Instant.parse("2021-06-22T00:42:12.00Z"); // Outside trigger period
         tester.clock().setInstant(t0);
 
         // Set initial target
@@ -139,13 +139,15 @@ public class OsUpgradeSchedulerTest {
         Version version0 = Version.fromString("8.0");
         tester.controller().upgradeOsIn(cloud, version0, Duration.ZERO, false);
 
-        // Stable release is scheduled once trigger period opens
+        // Stable release (tagged outside trigger period) is scheduled once trigger period opens
         Version version1 = Version.fromString("8.1");
         tester.serviceRegistry().artifactRepository().addRelease(new OsRelease(version1, OsRelease.Tag.stable,
-                tester.clock().instant()));
+                                                                               Instant.parse("2021-06-21T23:59:00.00Z")));
         scheduleUpgradeAfter(Duration.ZERO, version0, scheduler, tester);
-        assertEquals(version1, scheduler.changeIn(cloud).get().version(), "Change available");
-        scheduleUpgradeAfter(Duration.ofHours(1), version1, scheduler, tester); // Inside trigger period
+        OsUpgradeScheduler.Change nextChange = scheduler.changeIn(cloud).get();
+        assertEquals(version1, nextChange.version());
+        assertEquals("2021-06-22T07:00:00", formatInstant(nextChange.scheduleAt()));
+        scheduleUpgradeAfter(Duration.ofHours(7), version1, scheduler, tester); // Inside trigger period
 
         // A newer version is triggered manually
         Version version3 = Version.fromString("8.3");
@@ -160,7 +162,7 @@ public class OsUpgradeSchedulerTest {
     void schedule_latest_release_in_cd() {
         ControllerTester tester = new ControllerTester(SystemName.cd);
         OsUpgradeScheduler scheduler = new OsUpgradeScheduler(tester.controller(), Duration.ofDays(1));
-        Instant t0 = Instant.parse("2021-06-21T07:00:00.00Z"); // Inside trigger period
+        Instant t0 = Instant.parse("2021-06-21T07:05:00.00Z"); // Inside trigger period
         tester.clock().setInstant(t0);
 
         // Set initial target
@@ -172,7 +174,9 @@ public class OsUpgradeSchedulerTest {
         Version version1 = Version.fromString("8.1");
         tester.serviceRegistry().artifactRepository().addRelease(new OsRelease(version1, OsRelease.Tag.latest,
                 tester.clock().instant()));
-        assertEquals(version1, scheduler.changeIn(cloud).get().version(), "Change available");
+        assertEquals(version1, scheduler.changeIn(cloud).get().version());
+        assertEquals("2021-06-22T07:05:00", formatInstant(scheduler.changeIn(cloud).get().scheduleAt()),
+                     "Not valid until cool-down period passes");
         scheduleUpgradeAfter(Duration.ZERO, version0, scheduler, tester);
 
         // Cooldown period passes and latest release is scheduled