Merge pull request #17493 from vespa-engine/mpolden/rebuild-limit-per-type

Apply rebuild limit per node type

3 files changed, 47 insertions, 9 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
index 5aceadc458a..4b2f620dad7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/OsVersions.java
@@ -30,10 +30,10 @@ public class OsVersions {
 
     private static final Logger log = Logger.getLogger(OsVersions.class.getName());
 
-    /** The maximum number of concurrent upgrades triggered by {@link DelegatingOsUpgrader} */
+    /** The maximum number of concurrent upgrades per node type triggered by {@link DelegatingOsUpgrader} */
     private static final int MAX_DELEGATED_UPGRADES = 30;
 
-    /** The maximum number of concurrent upgrades (rebuilds) triggered by {@link RebuildingOsUpgrader} */
+    /** The maximum number of concurrent upgrades (rebuilds) per node type triggered by {@link RebuildingOsUpgrader} */
     private static final int MAX_REBUILDS = 3;
 
     private final NodeRepository nodeRepository;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
index 77d0f88eb98..25e901ebce3 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/os/RebuildingOsUpgrader.java
@@ -34,7 +34,7 @@ public class RebuildingOsUpgrader extends RetiringOsUpgrader {
 
     @Override
     protected NodeList candidates(Instant instant, OsVersionTarget target, NodeList allNodes) {
-        if (allNodes.rebuilding().size() < maxRebuilds) {
+        if (allNodes.nodeType(target.nodeType()).rebuilding().size() < maxRebuilds) {
             return super.candidates(instant, target, allNodes);
         }
         return NodeList.of();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
index fbbbeee7610..1a81511e698 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/os/OsVersionsTest.java
@@ -225,7 +225,7 @@ public class OsVersionsTest {
     public void upgrade_by_retiring_everything_at_once() {
         var versions = new OsVersions(tester.nodeRepository(), true, Integer.MAX_VALUE, Integer.MAX_VALUE);
         int hostCount = 3;
-        provisionInfraApplication(hostCount, NodeType.confighost);
+        provisionInfraApplication(hostCount, infraApplication, NodeType.confighost);
         Supplier<NodeList> hostNodes = () -> tester.nodeRepository().nodes().list()
                                                    .nodeType(NodeType.confighost)
                                                    .not().state(Node.State.deprovisioned);
@@ -327,6 +327,42 @@ public class OsVersionsTest {
         completeRebuildOf(hostsRebuilding.asList(), NodeType.host);
     }
 
+    @Test
+    public void upgrade_by_rebuilding_multiple_host_types() {
+        var versions = new OsVersions(tester.nodeRepository(), false, Integer.MAX_VALUE, 1);
+        var clock = tester.clock();
+        int hostCount = 3;
+        provisionInfraApplication(hostCount, infraApplication, NodeType.host);
+        provisionInfraApplication(hostCount, ApplicationId.from("hosted-vespa", "confighost", "default"), NodeType.confighost);
+        Supplier<NodeList> hosts = () -> tester.nodeRepository().nodes().list().nodeType(NodeType.host,
+                                                                                         NodeType.confighost);
+
+        // All hosts upgrade to first version. Upgrades are delegated
+        var version0 = Version.fromString("7.0");
+        versions.setTarget(NodeType.host, version0, Duration.ZERO, false);
+        versions.setTarget(NodeType.confighost, version0, Duration.ZERO, false);
+        setCurrentVersion(hosts.get().asList(), version0);
+
+        // Target is set for new major version
+        var version1 = Version.fromString("8.0");
+        Duration totalBudget = Duration.ofHours(12);
+        Duration nodeBudget = totalBudget.dividedBy(hostCount);
+        versions.setTarget(NodeType.host, version1, totalBudget, false);
+        versions.setTarget(NodeType.confighost, version1, totalBudget, false);
+
+        // One  host of each type is upgraded
+        for (int i = 0; i < hostCount; i++) {
+            clock.advance(nodeBudget);
+            versions.resumeUpgradeOf(NodeType.host, true);
+            versions.resumeUpgradeOf(NodeType.confighost, true);
+            NodeList hostsRebuilding = hosts.get().rebuilding();
+            assertEquals(2, hostsRebuilding.size());
+            completeRebuildOf(hostsRebuilding.nodeType(NodeType.host).asList(), NodeType.host);
+            completeRebuildOf(hostsRebuilding.nodeType(NodeType.confighost).asList(), NodeType.confighost);
+        }
+        assertEquals("All hosts upgraded", hostCount * 2, hosts.get().onOsVersion(version1).size());
+    }
+
     private NodeList deprovisioningChildrenOf(Node parent) {
         return tester.nodeRepository().nodes().list()
                      .childrenOf(parent)
@@ -334,12 +370,12 @@ public class OsVersionsTest {
     }
 
     private List<Node> provisionInfraApplication(int nodeCount) {
-        return provisionInfraApplication(nodeCount, NodeType.host);
+        return provisionInfraApplication(nodeCount, infraApplication, NodeType.host);
     }
 
-    private List<Node> provisionInfraApplication(int nodeCount, NodeType nodeType) {
+    private List<Node> provisionInfraApplication(int nodeCount, ApplicationId application, NodeType nodeType) {
         var nodes = tester.makeReadyNodes(nodeCount, "default", nodeType, 1);
-        tester.prepareAndActivateInfraApplication(infraApplication, nodeType);
+        tester.prepareAndActivateInfraApplication(application, nodeType);
         return nodes.stream()
                     .map(Node::hostname)
                     .flatMap(hostname -> tester.nodeRepository().nodes().node(hostname).stream())
@@ -374,10 +410,11 @@ public class OsVersionsTest {
         tester.patchNodes(nodes, (node) -> {
             Optional<Version> wantedOsVersion = node.status().osVersion().wanted();
             if (node.status().wantToDeprovision()) {
+                ApplicationId application = node.allocation().get().owner();
                 tester.nodeRepository().nodes().park(node.hostname(), false, Agent.system,
                                                      getClass().getSimpleName());
                 tester.nodeRepository().nodes().removeRecursively(node.hostname());
-                node = provisionInfraApplication(1, nodeType).get(0);
+                node = provisionInfraApplication(1, application, nodeType).get(0);
             }
             return node.with(node.status().withOsVersion(node.status().osVersion().withCurrent(wantedOsVersion)));
         });
@@ -388,6 +425,7 @@ public class OsVersionsTest {
         tester.patchNodes(nodes, (node) -> {
             Optional<Version> wantedOsVersion = node.status().osVersion().wanted();
             if (node.status().wantToRebuild()) {
+                ApplicationId application = node.allocation().get().owner();
                 tester.nodeRepository().nodes().park(node.hostname(), false, Agent.system,
                                                      getClass().getSimpleName());
                 tester.nodeRepository().nodes().removeRecursively(node.hostname());
@@ -395,7 +433,7 @@ public class OsVersionsTest {
                                    .build();
                 node = tester.nodeRepository().nodes().addNodes(List.of(newNode), Agent.system).get(0);
                 node = tester.nodeRepository().nodes().setReady(node.hostname(), Agent.system, getClass().getSimpleName());
-                tester.prepareAndActivateInfraApplication(infraApplication, nodeType);
+                tester.prepareAndActivateInfraApplication(application, nodeType);
                 node = tester.nodeRepository().nodes().node(node.hostname()).get();
             }
             return node.with(node.status().withOsVersion(node.status().osVersion().withCurrent(wantedOsVersion)));