Merge pull request #18169 from vespa-engine/mpolden/preserve-want-to-rebuild

Preserve wantToRebuild until host is readied

2 files changed, 18 insertions, 0 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
index 3afe5824af5..ec1bfba6996 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java
@@ -161,6 +161,14 @@ public class Nodes {
                     node = node.with(node.status().withFailCount(existing.get().status().failCount()));
                     if (existing.get().status().firmwareVerifiedAt().isPresent())
                         node = node.with(node.status().withFirmwareVerifiedAt(existing.get().status().firmwareVerifiedAt().get()));
+                    // Preserve wantToRebuild/wantToRetire when rebuilding as the fields shouldn't be cleared until the
+                    // host is readied (i.e. we know it is up and rebuild completed)
+                    boolean rebuilding = existing.get().status().wantToRebuild();
+                    if (rebuilding) {
+                        node = node.with(node.status().withWantToRetire(existing.get().status().wantToRetire(),
+                                                                        false,
+                                                                        rebuilding));
+                    }
                     nodesToRemove.add(existing.get());
                 }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
index 351f9fe44ee..4bfe01375c1 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/NodeRepositoryTest.java
@@ -197,8 +197,16 @@ public class NodeRepositoryTest {
         }
         tester.nodeRepository().nodes().removeRecursively("host1");
 
+        // Set host 2 properties and deprovision it
+        try (var lock = tester.nodeRepository().nodes().lockAndGetRequired("host2")) {
+            Node host2 = lock.node().withWantToRetire(true, false, true, Agent.system, tester.nodeRepository().clock().instant());
+            tester.nodeRepository().nodes().write(host2, lock);
+        }
+        tester.nodeRepository().nodes().removeRecursively("host2");
+
         // Host 1 is deprovisioned and unwanted properties are cleared
         Node host1 = tester.nodeRepository().nodes().node("host1").get();
+        Node host2 = tester.nodeRepository().nodes().node("host2").get();
         assertEquals(Node.State.deprovisioned, host1.state());
         assertTrue(host1.history().hasEventAfter(History.Event.Type.deprovisioned, testStart));
 
@@ -214,6 +222,8 @@ public class NodeRepositoryTest {
         assertTrue("Transferred from deprovisioned host", host1.status().firmwareVerifiedAt().isPresent());
         assertEquals("Transferred from deprovisioned host", 1, host1.status().failCount());
         assertEquals("Transferred from deprovisioned host", 1, host1.reports().getReports().size());
+        assertTrue("Transferred from rebuilt host", host2.status().wantToRetire());
+        assertTrue("Transferred from rebuilt host", host2.status().wantToRebuild());
     }
 
     @Test