Don't change node allocation when we cannot fail

3 files changed, 38 insertions, 14 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 9e39d25491f..9001212811c 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -123,7 +123,7 @@ class NodeAllocation {
                     if ((! saturated() && hasCompatibleFlavor(node) && requestedNodes.acceptable(offered)) || acceptToRetire(node))
                         accepted.add(acceptNode(node, wantToRetireNode, node.isResizable));
                 }
-                else {
+                else if (! saturated()) {
                     accepted.add(acceptNode(node, false, false));
                 }
             }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
index 198770a4aa7..e566172b524 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DockerProvisioningTest.java
@@ -382,34 +382,58 @@ public class DockerProvisioningTest {
     }
 
     @Test
+    public void test_startup_redeployment_with_inactive_nodes() {
+        NodeResources r = new NodeResources(20, 40, 100, 4);
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east")))
+                                                                    .flavors(List.of(new Flavor(r)))
+                                                                    .build();
+        tester.makeReadyHosts(5, r).deployZoneApp();
+
+        ApplicationId app1 = tester.makeApplicationId("app1");
+        ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.container, new ClusterSpec.Id("cluster1")).vespaVersion("7").build();
+
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(5, 1, r)));
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r)));
+
+        assertEquals(2, tester.getNodes(app1, Node.State.active).size());
+        assertEquals(3, tester.getNodes(app1, Node.State.inactive).size());
+
+        // Startup deployment: Not failable
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r), false, false));
+        // ... causes no change
+        assertEquals(2, tester.getNodes(app1, Node.State.active).size());
+        assertEquals(3, tester.getNodes(app1, Node.State.inactive).size());
+    }
+
+    @Test
     public void inactive_container_nodes_are_reused() {
-        assertInactiveResuse(ClusterSpec.Type.container);
+        assertInactiveReuse(ClusterSpec.Type.container);
     }
 
     @Test
     public void inactive_content_nodes_are_reused() {
-        assertInactiveResuse(ClusterSpec.Type.content);
+        assertInactiveReuse(ClusterSpec.Type.content);
     }
 
-    private void assertInactiveResuse(ClusterSpec.Type clusterType) {
-        Flavor hostFlavor = new Flavor(new NodeResources(20, 40, 100, 4));
+    private void assertInactiveReuse(ClusterSpec.Type clusterType) {
+        NodeResources r = new NodeResources(20, 40, 100, 4);
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east")))
-                                                                    .flavors(List.of(hostFlavor))
+                                                                    .flavors(List.of(new Flavor(r)))
                                                                     .build();
-        tester.makeReadyHosts(4, hostFlavor.resources()).deployZoneApp();
+        tester.makeReadyHosts(4, r).deployZoneApp();
 
         ApplicationId app1 = tester.makeApplicationId("app1");
         ClusterSpec cluster1 = ClusterSpec.request(clusterType, new ClusterSpec.Id("cluster1")).vespaVersion("7").build();
 
-        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(4, 1, hostFlavor.resources())));
-        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, hostFlavor.resources())));
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(4, 1, r)));
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r)));
 
         // Deactivate any retired nodes - usually done by the RetiredExpirer
         tester.nodeRepository().setRemovable(app1, tester.getNodes(app1).retired().asList());
-        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, hostFlavor.resources())));
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(2, 1, r)));
 
         assertEquals(2, tester.getNodes(app1, Node.State.inactive).size());
-        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(4, 1, hostFlavor.resources())));
+        tester.activate(app1, cluster1, Capacity.from(new ClusterResources(4, 1, r)));
         assertEquals(0, tester.getNodes(app1, Node.State.inactive).size());
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index 5c730912c49..ff2f0ffca96 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -888,7 +888,7 @@ public class ProvisioningTest {
         // Application allocates two content nodes initially, with cluster type content
         ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("music")).vespaVersion("1.2.3").build();
         var initialNodes = tester.activate(application, tester.prepare(application, cluster,
-                                                                       Capacity.from(new ClusterResources(2, 1, defaultResources), false, false)));
+                                                                       Capacity.from(new ClusterResources(2, 1, defaultResources))));
 
         // Application is redeployed with cluster type combined
         cluster = ClusterSpec.request(ClusterSpec.Type.combined, ClusterSpec.Id.from("music"))
@@ -896,7 +896,7 @@ public class ProvisioningTest {
                              .combinedId(Optional.of(ClusterSpec.Id.from("qrs")))
                              .build();
         var newNodes = tester.activate(application, tester.prepare(application, cluster,
-                                                                   Capacity.from(new ClusterResources(2, 1, defaultResources), false, false)));
+                                                                   Capacity.from(new ClusterResources(2, 1, defaultResources))));
 
         assertEquals("Node allocation remains the same", initialNodes, newNodes);
         assertEquals("Cluster type is updated",
@@ -906,7 +906,7 @@ public class ProvisioningTest {
         // Application is redeployed with cluster type content again
         cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("music")).vespaVersion("1.2.3").build();
         newNodes = tester.activate(application, tester.prepare(application, cluster,
-                                                               Capacity.from(new ClusterResources(2, 1, defaultResources), false, false)));
+                                                               Capacity.from(new ClusterResources(2, 1, defaultResources))));
         assertEquals("Node allocation remains the same", initialNodes, newNodes);
         assertEquals("Cluster type is updated",
                      Set.of(ClusterSpec.Type.content),