Merge pull request #23645 from vespa-engine/mpolden/estimate-tagged-release

Estimate scheduling time for TaggedRelease

2 files changed, 35 insertions, 47 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 5fa90d98c18..ddcfef23d86 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
@@ -50,24 +50,8 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
         if (upgradingToNewMajor(cloud)) return Optional.empty(); // Skip further upgrades until major version upgrade is complete
 
         Release release = releaseIn(cloud);
-        Instant instant = controller().clock().instant();
-        Version wantedVersion = release.version(currentTarget.get(), instant);
-        Version currentVersion = currentTarget.get().version();
-        if (release instanceof CalendarVersionedRelease) {
-            // Estimate the next change
-            while (!wantedVersion.isAfter(currentVersion)) {
-                instant = instant.plus(Duration.ofDays(1));
-                wantedVersion = release.version(currentTarget.get(), instant);
-            }
-        } else if (!wantedVersion.isAfter(currentVersion)) {
-            return Optional.empty(); // No change right now, and we cannot predict the next change for this kind of release
-        }
-        // Find the earliest possible trigger time on this day
-        instant = instant.truncatedTo(ChronoUnit.DAYS);
-        while (!canTriggerAt(instant)) {
-            instant = instant.plus(Duration.ofHours(1));
-        }
-        return Optional.of(new Change(wantedVersion, release.upgradeBudget(), instant));
+        Instant now = controller().clock().instant();
+        return release.change(currentTarget.get().version(), now);
     }
 
     private boolean upgradingToNewMajor(CloudName cloud) {
@@ -86,22 +70,28 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
         return new CalendarVersionedRelease(controller().system());
     }
 
-    private boolean canTriggerAt(Instant instant) {
+    private static boolean canTriggerAt(Instant instant, boolean isCd) {
         ZonedDateTime dateTime = instant.atZone(ZoneOffset.UTC);
         int hourOfDay = dateTime.getHour();
         int dayOfWeek = dateTime.getDayOfWeek().getValue();
         // Upgrade can only be scheduled between 07:00 (02:00 in CD systems) and 12:59 UTC, Monday-Thursday
-        int startHour = controller().system().isCd() ? 2 : 7;
+        int startHour = isCd ? 2 : 7;
         return hourOfDay >= startHour && hourOfDay <= 12 && dayOfWeek < 5;
     }
 
-    private interface Release {
+    /** Returns the earliest time an upgrade can be scheduled on the day of instant, in given system */
+    private static Instant schedulingInstant(Instant instant, SystemName system) {
+        instant = instant.truncatedTo(ChronoUnit.DAYS);
+        while (!canTriggerAt(instant, system.isCd())) {
+            instant = instant.plus(Duration.ofHours(1));
+        }
+        return instant;
+    }
 
-        /** The version number of this */
-        Version version(OsVersionTarget currentTarget, Instant now);
+    private interface Release {
 
-        /** The budget to use when upgrading to this */
-        Duration upgradeBudget();
+        /** The pending change for this release at given instant, if any */
+        Optional<Change> change(Version currentVersion, Instant instant);
 
     }
 
@@ -129,16 +119,11 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
             Objects.requireNonNull(artifactRepository);
         }
 
-        @Override
-        public Version version(OsVersionTarget currentTarget, Instant now) {
-            OsRelease release = artifactRepository.osRelease(currentTarget.osVersion().version().getMajor(), tag());
-            boolean cooldownPassed = !release.taggedAt().plus(cooldown()).isAfter(now);
-            return cooldownPassed ? release.version() : currentTarget.osVersion().version();
-        }
-
-        @Override
-        public Duration upgradeBudget() {
-            return Duration.ZERO; // Upgrades to tagged releases happen in-place so no budget is required
+        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);
+            return Optional.of(new Change(release.version(), Duration.ZERO, scheduleAt));
         }
 
         /** Returns the release tag tracked by this system */
@@ -174,14 +159,16 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
         }
 
         @Override
-        public Version version(OsVersionTarget currentTarget, Instant now) {
-            Version currentVersion = currentTarget.osVersion().version();
-            Version wantedVersion = asVersion(dateOfWantedVersion(now), currentVersion);
-            return wantedVersion.isAfter(currentVersion) ? wantedVersion : currentVersion;
+        public Optional<Change> change(Version currentVersion, Instant instant) {
+            Version wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion);
+            while (!wantedVersion.isAfter(currentVersion)) {
+                wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion);
+                instant = instant.plus(Duration.ofDays(1));
+            }
+            return Optional.of(new Change(wantedVersion, upgradeBudget(), schedulingInstant(instant, system)));
         }
 
-        @Override
-        public Duration upgradeBudget() {
+        private Duration upgradeBudget() {
             return system.isCd() ? Duration.ZERO : Duration.ofDays(14);
         }
 
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 ddd2163b426..5ed441398fd 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
@@ -88,7 +88,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-21T07:00:00.00Z"); // Inside trigger period
+        Instant t0 = Instant.parse("2021-06-21T06:00:00.00Z"); // Outside trigger period
         tester.clock().setInstant(t0);
 
         // Set initial target
@@ -96,11 +96,13 @@ public class OsUpgradeSchedulerTest {
         Version version0 = Version.fromString("8.0");
         tester.controller().upgradeOsIn(cloud, version0, Duration.ZERO, false);
 
-        // Stable release is scheduled immediately
+        // Stable release 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()));
-        scheduleUpgradeAfter(Duration.ZERO, version1, scheduler, tester);
+        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
 
         // A newer version is triggered manually
         Version version3 = Version.fromString("8.3");
@@ -108,9 +110,7 @@ public class OsUpgradeSchedulerTest {
 
         // Nothing happens in next iteration as tagged release is older than manually triggered version
         scheduleUpgradeAfter(Duration.ofDays(7), version3, scheduler, tester);
-
-        // Next change cannot be estimated for tagged releases
-        assertTrue(scheduler.changeIn(cloud).isEmpty(), "Next change is unknown");
+        assertTrue(scheduler.changeIn(cloud).isEmpty());
     }
 
     @Test
@@ -129,6 +129,7 @@ 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");
         scheduleUpgradeAfter(Duration.ZERO, version0, scheduler, tester);
 
         // Cooldown period passes and latest release is scheduled