Merge pull request #23389 from vespa-engine/mpolden/fair-budget

Divide OS upgrade budget evenly across all zones

2 files changed, 5 insertions, 7 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
index fa64a2677f4..0c146179f34 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgrader.java
@@ -98,9 +98,7 @@ public class OsUpgrader extends InfrastructureUpgrader<OsVersionTarget> {
 
     /** Returns whether to spend upgrade budget on given zone */
     private boolean spendBudgetOn(ZoneApi zone) {
-        if (!zone.getEnvironment().isProduction()) return false;
-        if (controller().zoneRegistry().systemZone().getVirtualId().equals(zone.getVirtualId())) return false; // Controller zone
-        return true;
+        return !controller().zoneRegistry().systemZone().getVirtualId().equals(zone.getVirtualId()); // Do not spend budget on controller zone
     }
 
     /** Returns whether node is in a state where it can be upgraded */
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
index 3c3f0053e91..8adf55155a5 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/maintenance/OsUpgraderTest.java
@@ -158,15 +158,15 @@ public class OsUpgraderTest {
         // First zone upgrades
         osUpgrader.maintain();
         for (var nodeType : nodeTypes) {
-            assertEquals("Dev zone gets a zero budget", Duration.ZERO, upgradeBudget(zone1, nodeType, version));
+            assertEquals(Duration.ofHours(4), upgradeBudget(zone1, nodeType, version));
             completeUpgrade(version, nodeType, zone1);
         }
 
         // Next set of zones upgrade
         osUpgrader.maintain();
-        for (var zone : List.of(zone2, zone3)) {
+        for (var zone : List.of(zone1, zone2, zone3)) {
             for (var nodeType : nodeTypes) {
-                assertEquals("Parallel prod zones share the budget of a single zone", Duration.ofHours(6),
+                assertEquals("Parallel prod zones share the budget of a single zone", Duration.ofHours(4),
                              upgradeBudget(zone, nodeType, version));
                 completeUpgrade(version, nodeType, zone);
             }
@@ -175,7 +175,7 @@ public class OsUpgraderTest {
         // Last zone upgrades
         osUpgrader.maintain();
         for (var nodeType : nodeTypes) {
-            assertEquals(nodeType + " in last prod zone gets the budget of a single zone", Duration.ofHours(6),
+            assertEquals(nodeType + " in last prod zone gets the budget of a single zone", Duration.ofHours(4),
                          upgradeBudget(zone4, nodeType, version));
             completeUpgrade(version, nodeType, zone4);
         }