[JENKINS-56403]: Renaming agents doesn't remove old agent configurati…

…on from disk

[JENKINS-33780] introduced a regression in persisting the update to disk.
In prior code, the update was performed through a full config save after
updating the nodes, but the functionality was not carried over.

* Added a file delete call to remove the old agent from disk iff
  the node name was changed.
* Add tests for this scenario and when node name was not updated.

core/src/main/java/jenkins/model/Nodes.java

@@ -233,7 +233,11 @@ public void run() {
                 }
             });
             updateNode(newOne);
+            if (!newOne.getNodeName().equals(oldOne.getNodeName())) {
+                Util.deleteRecursive(new File(getNodesDir(), oldOne.getNodeName()));
+            }
             NodeListener.fireOnUpdated(oldOne, newOne);
+
             return true;
         } else {
             return false;

test/src/test/java/jenkins/model/NodesTest.java

@@ -38,6 +38,8 @@
 import static org.hamcrest.Matchers.nullValue;
 import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
 
 public class NodesTest {
@@ -88,6 +90,28 @@ public void addNodeShouldReplaceExistingNode() throws Exception {
         assertThat(r.jenkins.getNode("foo"), sameInstance(newNode));
     }
 
+    @Test
+    @Issue("JENKINS-56403")
+    public void replaceNodeShouldRemoveOldNode() throws Exception {
+        Node oldNode = r.createSlave("foo", "", null);
+        Node newNode = r.createSlave("foo-new", "", null);
+        r.jenkins.addNode(oldNode);
+        r.jenkins.getNodesObject().replaceNode(oldNode, newNode);
+        r.jenkins.getNodesObject().load();
+        assertNull(r.jenkins.getNode(oldNode.getNodeName()));
+    }
+
+    @Test
+    @Issue("JENKINS-56403")
+    public void replaceNodeShouldNotRemoveIdenticalOldNode() throws Exception {
+        Node oldNode = r.createSlave("foo", "", null);
+        Node newNode = r.createSlave("foo", "", null);
+        r.jenkins.addNode(oldNode);
+        r.jenkins.getNodesObject().replaceNode(oldNode, newNode);
+        r.jenkins.getNodesObject().load();
+        assertNotNull(r.jenkins.getNode(oldNode.getNodeName()));
+    }
+
     private static class InvalidNode extends Slave {
         // JEP-200 whitelist changes prevent this field (and thus instances of this class) from being serialized.
         private ClassLoader cl = InvalidNode.class.getClassLoader();