Nodes persistence cleanup, APIs to control loading (#8979)

Co-authored-by: Vincent Latombe <[email protected]>

core/src/main/java/hudson/model/Node.java

@@ -29,22 +29,26 @@
 import edu.umd.cs.findbugs.annotations.CheckForNull;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
+import hudson.BulkChange;
 import hudson.Extension;
 import hudson.ExtensionPoint;
 import hudson.FilePath;
 import hudson.FileSystemProvisioner;
 import hudson.Launcher;
 import hudson.Util;
+import hudson.XmlFile;
 import hudson.model.Descriptor.FormException;
 import hudson.model.Queue.Task;
 import hudson.model.labels.LabelAtom;
+import hudson.model.listeners.SaveableListener;
 import hudson.model.queue.CauseOfBlockage;
 import hudson.remoting.Callable;
 import hudson.remoting.VirtualChannel;
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
 import hudson.slaves.Cloud;
 import hudson.slaves.ComputerListener;
+import hudson.slaves.EphemeralNode;
 import hudson.slaves.NodeDescriptor;
 import hudson.slaves.NodeProperty;
 import hudson.slaves.NodePropertyDescriptor;
@@ -53,6 +57,7 @@
 import hudson.util.DescribableList;
 import hudson.util.EnumConverter;
 import hudson.util.TagCloud;
+import java.io.File;
 import java.io.IOException;
 import java.lang.reflect.Type;
 import java.util.Collections;
@@ -63,6 +68,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
+import jenkins.model.Nodes;
 import jenkins.util.SystemProperties;
 import jenkins.util.io.OnMaster;
 import net.sf.json.JSONObject;
@@ -96,7 +102,7 @@
  * @see Computer
  */
 @ExportedBean
-public abstract class Node extends AbstractModelObject implements ReconfigurableDescribable<Node>, ExtensionPoint, AccessControlled, OnMaster, Saveable {
+public abstract class Node extends AbstractModelObject implements ReconfigurableDescribable<Node>, ExtensionPoint, AccessControlled, OnMaster, PersistenceRoot {
 
     private static final Logger LOGGER = Logger.getLogger(Node.class.getName());
 
@@ -110,6 +116,8 @@ public abstract class Node extends AbstractModelObject implements Reconfigurable
      */
     protected transient volatile boolean holdOffLaunchUntilSave;
 
+    private transient Nodes parent;
+
     @Override
     public String getDisplayName() {
         return getNodeName(); // default implementation
@@ -133,16 +141,18 @@ public boolean isHoldOffLaunchUntilSave() {
      */
     @Override
     public void save() throws IOException {
-        // this should be a no-op unless this node instance is the node instance in Jenkins' list of nodes
-        // thus where Jenkins.get() == null there is no list of nodes, so we do a no-op
-        // Nodes.updateNode(n) will only persist the node record if the node instance is in the list of nodes
-        // so either path results in the same behaviour: the node instance is only saved if it is in the list of nodes
-        // for all other cases we do not know where to persist the node record and hence we follow the default
-        // no-op of a Saveable.NOOP
-        final Jenkins jenkins = Jenkins.getInstanceOrNull();
-        if (jenkins != null) {
-            jenkins.updateNode(this);
+        if (parent == null) return;
+        if (this instanceof EphemeralNode) {
+            Util.deleteRecursive(getRootDir());
+            return;
         }
+        if (BulkChange.contains(this))   return;
+        getConfigFile().write(this);
+        SaveableListener.fireOnChange(this, getConfigFile());
+    }
+
+    protected XmlFile getConfigFile() {
+        return parent.getConfigFile(this);
     }
 
     /**
@@ -248,6 +258,11 @@ public boolean isAcceptingTasks() {
         return true;
     }
 
+    public void onLoad(Nodes parent, String name) {
+        this.parent = parent;
+        setNodeName(name);
+    }
+
     /**
      * Let Nodes be aware of the lifecycle of their own {@link Computer}.
      */
@@ -629,4 +644,16 @@ public String getName() {
         }
     }
 
+    @Override
+    public File getRootDir() {
+        return getParent().getRootDirFor(this);
+    }
+
+    @NonNull
+    private Nodes getParent() {
+        if (parent == null) {
+            throw new IllegalStateException("no parent set on " + getClass().getName() + "[" + getNodeName() + "]");
+        }
+        return parent;
+    }
 }

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

@@ -540,6 +540,11 @@ public class Jenkins extends AbstractCIBase implements DirectlyModifiableTopLeve
      */
     public final Hudson.CloudList clouds = new Hudson.CloudList(this);
 
+    @Restricted(Beta.class)
+    public void loadNode(File dir) throws IOException {
+        getNodesObject().load(dir);
+    }
+
     public static class CloudList extends DescribableList<Cloud, Descriptor<Cloud>> {
         public CloudList(Jenkins h) {
             super(h);
@@ -2219,6 +2224,12 @@ public JDK getJDK(String name) {
         return nodes.getNode(name);
     }
 
+    @CheckForNull
+    @Restricted(Beta.class)
+    public Node getOrLoadNode(String nodeName) {
+        return getNodesObject().getOrLoad(nodeName);
+    }
+
     /**
      * Gets a {@link Cloud} by {@link Cloud#name its name}, or null.
      */
@@ -2266,6 +2277,14 @@ public void removeNode(@NonNull Node n) throws IOException {
         nodes.removeNode(n);
     }
 
+    /**
+     * Unload a node from Jenkins without touching its configuration file.
+     */
+    @Restricted(Beta.class)
+    public void unloadNode(@NonNull Node n) {
+        nodes.unload(n);
+    }
+
     /**
      * Saves an existing {@link Node} on disk, called by {@link Node#save()}. This method is preferred in those cases
      * where you need to determine atomically that the node being saved is actually in the list of nodes.
@@ -3298,7 +3317,8 @@ public Fingerprint _getFingerprint(String md5sum) throws IOException {
     /**
      * The file we save our configuration.
      */
-    private XmlFile getConfigFile() {
+    @Restricted(NoExternalUse.class)
+    protected XmlFile getConfigFile() {
         return new XmlFile(XSTREAM, new File(root, "config.xml"));
     }
 

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

@@ -31,6 +31,8 @@
 import java.util.List;
 import java.util.logging.Logger;
 import jenkins.util.Listeners;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.Beta;
 
 /**
  * Listen to {@link Node} CRUD operations.
@@ -42,6 +44,16 @@ public abstract class NodeListener implements ExtensionPoint {
 
     private static final Logger LOGGER = Logger.getLogger(NodeListener.class.getName());
 
+    /**
+     * Allows to veto node loading.
+     * @param node the node being loaded. Not yet attached to Jenkins.
+     * @return false to veto node loading.
+     */
+    @Restricted(Beta.class)
+    protected boolean allowLoad(@NonNull Node node) {
+        return true;
+    }
+
     /**
      * Node is being created.
      */