Fix release day and add system-dependent cooldown

3 files changed, 75 insertions, 46 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 2d0424e9f03..beba542760c 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
@@ -147,13 +147,11 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
                                                                   .atStartOfDay()
                                                                   .toInstant(ZoneOffset.UTC);
 
-        /** The time that should elapse between versions */
+        /** The approximate time that should elapse between versions */
         private static final Duration SCHEDULING_STEP = Duration.ofDays(60);
 
         /** The day of week new releases are published */
-        private static final DayOfWeek RELEASE_DAY = DayOfWeek.MONDAY;
-
-        private static final DateTimeFormatter CALENDAR_VERSION_PATTERN = DateTimeFormatter.ofPattern("yyyyMMdd");
+        private static final DayOfWeek RELEASE_DAY = DayOfWeek.TUESDAY;
 
         public CalendarVersionedRelease {
             Objects.requireNonNull(system);
@@ -161,42 +159,63 @@ public class OsUpgradeScheduler extends ControllerMaintainer {
 
         @Override
         public Optional<Change> change(Version currentVersion, Instant instant) {
-            Version wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion);
-            while (!wantedVersion.isAfter(currentVersion)) {
+            CalendarVersion version = findVersion(instant, currentVersion);
+            while (!version.version().isAfter(currentVersion)) {
                 instant = instant.plus(Duration.ofDays(1));
-                wantedVersion = asVersion(dateOfWantedVersion(instant), currentVersion);
+                version = findVersion(instant, currentVersion);
             }
-            return Optional.of(new Change(wantedVersion, upgradeBudget(), schedulingInstant(instant, system)));
+            Duration cooldown = remainingCooldownAt(instant, version);
+            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);
         }
 
-        /**
-         * Calculate the date of the wanted version relative to now. A given zone will choose the oldest release
-         * available which is not older than this date.
-         */
-        static LocalDate dateOfWantedVersion(Instant now) {
+        private Duration remainingCooldownAt(Instant instant, CalendarVersion version) {
+            Duration minAge = 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;
+        }
+
+        /** Find the most recent version available according to the scheduling step, relative to now */
+        static CalendarVersion findVersion(Instant now, Version currentVersion) {
             Instant candidate = START_OF_SCHEDULE;
             while (!candidate.plus(SCHEDULING_STEP).isAfter(now)) {
                 candidate = candidate.plus(SCHEDULING_STEP);
             }
             LocalDate date = LocalDate.ofInstant(candidate, ZoneOffset.UTC);
-            return releaseDayOf(date);
+            while (date.getDayOfWeek() != RELEASE_DAY) {
+                date = date.minusDays(1);
+            }
+            return CalendarVersion.from(date, currentVersion);
         }
 
-        private static LocalDate releaseDayOf(LocalDate date) {
-            int releaseDayDelta = RELEASE_DAY.getValue() - date.getDayOfWeek().getValue();
-            return date.plusDays(releaseDayDelta);
-        }
+        record CalendarVersion(Version version, LocalDate date) {
+
+            private static final DateTimeFormatter CALENDAR_VERSION_PATTERN = DateTimeFormatter.ofPattern("yyyyMMdd");
+
+            private static CalendarVersion from(LocalDate date, Version currentVersion) {
+                String qualifier = date.format(CALENDAR_VERSION_PATTERN);
+                return new CalendarVersion(new Version(currentVersion.getMajor(),
+                                                       currentVersion.getMinor(),
+                                                       currentVersion.getMicro(),
+                                                       qualifier),
+                                           date);
+            }
+
+            /** Returns the age of this at given instant, in whole days */
+            private Duration age(Instant instant) {
+                return Duration.between(date.atStartOfDay().toInstant(ZoneOffset.UTC),
+                                        instant.truncatedTo(ChronoUnit.DAYS));
+            }
 
-        private static Version asVersion(LocalDate dateOfVersion, Version currentVersion) {
-            String calendarVersion = dateOfVersion.format(CALENDAR_VERSION_PATTERN);
-            return new Version(currentVersion.getMajor(),
-                               currentVersion.getMinor(),
-                               currentVersion.getMicro(),
-                               calendarVersion);
         }
 
     }
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 5ed441398fd..31a1fe96fb7 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
@@ -8,6 +8,7 @@ import com.yahoo.config.provision.zone.ZoneApi;
 import com.yahoo.vespa.hosted.controller.ControllerTester;
 import com.yahoo.vespa.hosted.controller.api.integration.deployment.OsRelease;
 import com.yahoo.vespa.hosted.controller.integration.ZoneApiMock;
+import com.yahoo.vespa.hosted.controller.maintenance.OsUpgradeScheduler.CalendarVersionedRelease.CalendarVersion;
 import com.yahoo.vespa.hosted.controller.versions.OsVersionTarget;
 import org.junit.jupiter.api.Test;
 
@@ -33,7 +34,7 @@ public class OsUpgradeSchedulerTest {
     void schedule_calendar_versioned_release() {
         ControllerTester tester = new ControllerTester();
         OsUpgradeScheduler scheduler = new OsUpgradeScheduler(tester.controller(), Duration.ofDays(1));
-        Instant t0 = Instant.parse("2022-01-16T00:00:00.00Z"); // Outside trigger period
+        Instant t0 = Instant.parse("2022-01-16T09:05:00.00Z"); // Inside trigger period
         tester.clock().setInstant(t0);
 
         CloudName cloud = CloudName.from("cloud");
@@ -55,18 +56,27 @@ public class OsUpgradeSchedulerTest {
             assertEquals(version0, tester.controller().osVersionTarget(cloud).get().osVersion().version());
         }
 
-        // Enough days pass that the next release is triggered
-        Version version1 = Version.fromString("7.0.0.20220228");
-        tester.clock().advance(Duration.ofDays(30));
+        // New release becomes available, but is not triggered until cool-down period has passed
+        Version version1 = Version.fromString("7.0.0.20220301");
+        tester.clock().advance(Duration.ofDays(16));
+        assertEquals("2022-03-03T09:05:00", formatInstant(tester.clock().instant()));
+        assertEquals(version1, scheduler.changeIn(cloud).get().version());
+        scheduler.maintain();
+        assertEquals(version0,
+                     tester.controller().osVersionTarget(cloud).get().osVersion().version(),
+                     "Target is unchanged because cooldown hasn't passed");
+
+        // ... and we're inside the trigger period
+        tester.clock().advance(Duration.ofDays(3).plusHours(18));
+        assertEquals("2022-03-07T03:05:00", formatInstant(tester.clock().instant()));
         scheduler.maintain();
         assertEquals(version0,
                      tester.controller().osVersionTarget(cloud).get().osVersion().version(),
                      "Target is unchanged because we're outside trigger period");
-        tester.clock().advance(Duration.ofHours(9).plusMinutes(5)); // Put us inside the trigger period
-        assertEquals("2022-03-17T09:05:00", formatInstant(tester.clock().instant()));
+        tester.clock().advance(Duration.ofHours(5));
+        assertEquals("2022-03-07T08:05:00", formatInstant(tester.clock().instant()));
         Optional<OsUpgradeScheduler.Change> change = scheduler.changeIn(cloud);
         assertTrue(change.isPresent());
-        assertEquals("2022-03-17T07:00:00", formatInstant(change.get().scheduleAt()));
         scheduler.maintain();
         assertEquals(version1,
                 tester.controller().osVersionTarget(cloud).get().osVersion().version(),
@@ -80,7 +90,7 @@ public class OsUpgradeSchedulerTest {
         // Estimate next change
         Optional<OsUpgradeScheduler.Change> nextChange = scheduler.changeIn(cloud);
         assertTrue(nextChange.isPresent());
-        assertEquals("7.0.0.20220425", nextChange.get().version().toFullString());
+        assertEquals("7.0.0.20220426", nextChange.get().version().toFullString());
         assertEquals("2022-05-02T07:00:00", formatInstant(nextChange.get().scheduleAt()));
     }
 
@@ -138,19 +148,19 @@ public class OsUpgradeSchedulerTest {
 
     @Test
     void schedule_of_calender_versioned_releases() {
-        Map<String, String> tests = Map.of("2022-01-01", "2021-12-27",
-                "2022-03-01", "2021-12-27",
-                "2022-03-02", "2022-02-28",
-                "2022-04-30", "2022-02-28",
-                "2022-05-01", "2022-04-25",
-                "2022-06-29", "2022-04-25",
-                "2022-07-01", "2022-06-27",
-                "2022-08-28", "2022-06-27",
-                "2022-08-29", "2022-08-29");
-        tests.forEach((now, expectedVersion) -> {
+        Map<String, String> tests = Map.of("2022-01-01", "2021-12-28",
+                                           "2022-03-01", "2021-12-28",
+                                           "2022-03-02", "2022-03-01",
+                                           "2022-04-30", "2022-03-01",
+                                           "2022-05-01", "2022-04-26",
+                                           "2022-06-30", "2022-06-28",
+                                           "2022-07-01", "2022-06-28",
+                                           "2022-08-28", "2022-06-28",
+                                           "2022-08-29", "2022-08-23");
+        tests.forEach((now, expectedVersionDate) -> {
             Instant instant = LocalDate.parse(now).atStartOfDay().toInstant(ZoneOffset.UTC);
-            LocalDate dateOfWantedVersion = OsUpgradeScheduler.CalendarVersionedRelease.dateOfWantedVersion(instant);
-            assertEquals(LocalDate.parse(expectedVersion), dateOfWantedVersion, "scheduled wanted version at " + now);
+            CalendarVersion version = OsUpgradeScheduler.CalendarVersionedRelease.findVersion(instant, Version.fromString("1.0"));
+            assertEquals(LocalDate.parse(expectedVersionDate), version.date(), "version to schedule at " + now);
         });
     }
 
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json
index be94b85f113..7f31b67d77b 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/os/responses/versions-all-upgraded.json
@@ -104,8 +104,8 @@
       "targetVersion": true,
       "upgradeBudget": "PT24H",
       "scheduledAt": 1234,
-      "nextVersion": "8.2.1.20211227",
-      "nextScheduledAt": 7200000,
+      "nextVersion": "8.2.1.20211228",
+      "nextScheduledAt": 1640743200000,
       "cloud": "cloud2",
       "nodes": [
         {