Keep retired nodes on bootstrap deployment

2 files changed, 29 insertions, 8 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 4633b247ff1..240963a8c0d 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
@@ -109,12 +109,8 @@ class NodeAllocation {
                 if ( candidate.state() == Node.State.active && allocation.isRemovable()) continue; // don't accept; causes removal
                 if ( indexes.contains(membership.index())) continue; // duplicate index (just to be sure)
 
-                boolean resizeable = false;
-                boolean acceptToRetire = false;
-                if (requestedNodes.considerRetiring()) {
-                    resizeable = candidate.isResizable;
-                    acceptToRetire = acceptToRetire(candidate);
-                }
+                boolean resizeable = requestedNodes.considerRetiring() && candidate.isResizable;
+                boolean acceptToRetire = acceptToRetire(candidate);
 
                 if ((! saturated() && hasCompatibleFlavor(candidate) && requestedNodes.acceptable(candidate)) || acceptToRetire)
                     accepted.add(acceptNode(candidate, shouldRetire(candidate), resizeable));
@@ -152,7 +148,9 @@ class NodeAllocation {
     }
 
     private boolean shouldRetire(NodeCandidate candidate) {
-        if ( ! requestedNodes.considerRetiring()) return false;
+        if ( ! requestedNodes.considerRetiring())
+            return candidate.allocation().map(a -> a.membership().retired()).orElse(false); // don't second-guess if already retired
+
         if ( ! nodeResourceLimits.isWithinRealLimits(candidate, cluster)) return true;
         if (violatesParentHostPolicy(candidate)) return true;
         if ( ! hasCompatibleFlavor(candidate)) return true;
@@ -231,6 +229,7 @@ class NodeAllocation {
         if (candidate.state() != Node.State.active) return false;
         if (! candidate.allocation().get().membership().cluster().group().equals(cluster.group())) return false;
         if (candidate.allocation().get().membership().retired()) return true; // don't second-guess if already retired
+        if (! requestedNodes.considerRetiring()) return false;
 
         return cluster.type().isContent() ||
                (cluster.type() == ClusterSpec.Type.container && !hasCompatibleFlavor(candidate));
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 567ec78576c..7a0d9fa5047 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
@@ -18,7 +18,6 @@ import com.yahoo.config.provision.OutOfCapacityException;
 import com.yahoo.config.provision.RegionName;
 import com.yahoo.config.provision.SystemName;
 import com.yahoo.config.provision.Zone;
-import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeList;
 import com.yahoo.vespa.hosted.provision.maintenance.ReservationExpirer;
@@ -714,6 +713,29 @@ public class ProvisioningTest {
     }
 
     @Test
+    public void retired_but_cannot_fail() {
+        ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();
+        tester.makeReadyHosts(10, defaultResources).activateTenantHosts();
+
+        ApplicationId application = ProvisioningTester.makeApplicationId();
+        Capacity capacityCanFail = Capacity.from(new ClusterResources(5, 1, defaultResources), false, true);
+        ClusterSpec cluster = ClusterSpec.request(ClusterSpec.Type.content, ClusterSpec.Id.from("music")).vespaVersion("4.5.6").build();
+
+        tester.activate(application, tester.prepare(application, cluster, capacityCanFail));
+        assertEquals(0, NodeList.copyOf(tester.nodeRepository().getNodes(application, Node.State.active)).retired().size());
+
+        tester.patchNode(tester.nodeRepository().getNodes(application).stream().findAny().orElseThrow(), n -> n.withWantToRetire(true, Agent.system, tester.clock().instant()));
+        tester.activate(application, tester.prepare(application, cluster, capacityCanFail));
+        assertEquals(1, NodeList.copyOf(tester.nodeRepository().getNodes(application, Node.State.active)).retired().size());
+        assertEquals(6, tester.nodeRepository().getNodes(application, Node.State.active).size());
+
+        Capacity capacityCannotFail = Capacity.from(new ClusterResources(5, 1, defaultResources), false, false);
+        tester.activate(application, tester.prepare(application, cluster, capacityCannotFail));
+        assertEquals(1, NodeList.copyOf(tester.nodeRepository().getNodes(application, Node.State.active)).retired().size());
+        assertEquals(6, tester.nodeRepository().getNodes(application, Node.State.active).size());
+    }
+
+    @Test
     public void highest_node_indexes_are_retired_first() {
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east"))).build();