Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-31055] Make Node implement Saveable
- Also tidy up some of the mess logic in both #1866 and #1860
  • Loading branch information
stephenc committed Oct 20, 2015
1 parent e251749 commit e4b1aab
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 16 deletions.
3 changes: 3 additions & 0 deletions changelog.html
Expand Up @@ -56,6 +56,9 @@
<div id="trunk" style="display:none"><!--=TRUNK-BEGIN=-->
<ul class=image>
<li class=>
<li class="rfe">
Make Node implement Saveable.
(<a href="https://issues.jenkins-ci.org/browse/JENKINS-31055">issue 31055</a>)
</ul>
</div><!--=TRUNK-END=-->
<h3><a name=v1.634>What's new in 1.634</a> (2015/10/18)</h3>
Expand Down
1 change: 0 additions & 1 deletion core/src/main/java/hudson/model/Computer.java
Expand Up @@ -695,7 +695,6 @@ public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause
if (node != null) {
node.setTemporaryOfflineCause(offlineCause);
}
Jenkins.getInstance().getQueue().scheduleMaintenance();
synchronized (statusChangeLock) {
statusChangeLock.notifyAll();
}
Expand Down
29 changes: 22 additions & 7 deletions core/src/main/java/hudson/model/Node.java
Expand Up @@ -25,7 +25,12 @@
package hudson.model;

import com.infradna.tool.bridge_method_injector.WithBridgeMethods;
import hudson.*;
import hudson.Extension;
import hudson.ExtensionPoint;
import hudson.FilePath;
import hudson.FileSystemProvisioner;
import hudson.Launcher;
import hudson.Util;
import hudson.model.Descriptor.FormException;
import hudson.model.Queue.Task;
import hudson.model.labels.LabelAtom;
Expand All @@ -46,17 +51,15 @@
import hudson.util.EnumConverter;
import hudson.util.TagCloud;
import hudson.util.TagCloud.WeightFunction;

import java.io.IOException;
import java.lang.reflect.Type;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.List;
import java.util.Set;
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;

import jenkins.model.Jenkins;
import jenkins.util.io.OnMaster;
import net.sf.json.JSONObject;
Expand All @@ -65,8 +68,8 @@
import org.kohsuke.stapler.BindInterceptor;
import org.kohsuke.stapler.Stapler;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.export.ExportedBean;
import org.kohsuke.stapler.export.Exported;
import org.kohsuke.stapler.export.ExportedBean;

/**
* Base type of Jenkins slaves (although in practice, you probably extend {@link Slave} to define a new slave type).
Expand All @@ -88,7 +91,7 @@
* @see Computer
*/
@ExportedBean
public abstract class Node extends AbstractModelObject implements ReconfigurableDescribable<Node>, ExtensionPoint, AccessControlled, OnMaster {
public abstract class Node extends AbstractModelObject implements ReconfigurableDescribable<Node>, ExtensionPoint, AccessControlled, OnMaster, Saveable {

private static final Logger LOGGER = Logger.getLogger(Node.class.getName());

Expand All @@ -114,6 +117,18 @@ public boolean isHoldOffLaunchUntilSave() {
return holdOffLaunchUntilSave;
}

/**
* {@inheritDoc}
* @since 1.635
*/
@Override
public void save() throws IOException {
final Jenkins jenkins = Jenkins.getInstance();
if (jenkins != null) {
jenkins.updateNode(this);
}
}

/**
* Name of this node.
*
Expand Down Expand Up @@ -237,7 +252,7 @@ void setTemporaryOfflineCause(OfflineCause cause) {
try {
if (temporaryOfflineCause != cause) {
temporaryOfflineCause = cause;
Jenkins.getInstance().save(); // Gotta be a better way to do this
save();
}
} catch (java.io.IOException e) {
LOGGER.warning("Unable to complete save, temporary offline status will not be persisted: " + e.getMessage());
Expand Down
9 changes: 6 additions & 3 deletions core/src/main/java/jenkins/model/Jenkins.java
Expand Up @@ -147,7 +147,6 @@
import hudson.slaves.Cloud;
import hudson.slaves.ComputerListener;
import hudson.slaves.DumbSlave;
import hudson.slaves.EphemeralNode;
import hudson.slaves.NodeDescriptor;
import hudson.slaves.NodeList;
import hudson.slaves.NodeProperty;
Expand Down Expand Up @@ -289,7 +288,6 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
Expand Down Expand Up @@ -1752,7 +1750,12 @@ public void removeNode(@Nonnull Node n) throws IOException {
}

/**
* Updates an existing {@link Node} on disk.
* 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.
*
* @param n the node to be updated.
* @return {@code true}, if the node was updated. {@code false}, if the node was not in the list of nodes.
* @throws IOException if the node could not be persisted.
* @see Nodes#updateNode
* @since 1.634
*/
Expand Down
22 changes: 17 additions & 5 deletions core/src/main/java/jenkins/model/Nodes.java
Expand Up @@ -34,6 +34,7 @@
import hudson.model.listeners.SaveableListener;
import hudson.slaves.EphemeralNode;
import hudson.slaves.OfflineCause;
import java.util.concurrent.Callable;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

Expand Down Expand Up @@ -157,6 +158,7 @@ private void persistNode(final @Nonnull Node node) throws IOException {
xmlFile.write(node);
SaveableListener.fireOnChange(this, xmlFile);
}
jenkins.getQueue().scheduleMaintenance();
}

/**
Expand All @@ -166,17 +168,27 @@ private void persistNode(final @Nonnull Node node) throws IOException {
* @param node the node to be updated.
* @return {@code true}, if the node was updated. {@code false}, if the node was not in the list of nodes.
* @throws IOException if the node could not be persisted.
* @since 1.634
*/
public boolean updateNode(final @Nonnull Node node) throws IOException {
if (node == nodes.get(node.getNodeName())) {
Queue.withLock(new Runnable() {
boolean exists;
try {
exists = Queue.withLock(new Callable<Boolean>() {
@Override
public void run() {
jenkins.trimLabels();
public Boolean call() throws Exception {
if (node == nodes.get(node.getNodeName())) {
jenkins.trimLabels();
return true;
}
return false;
}
});
} catch (Exception e) {
// can never happen
exists = false;
}
if (exists) {
persistNode(node);
jenkins.getQueue().scheduleMaintenance();
return true;
}
return false;
Expand Down

0 comments on commit e4b1aab

Please sign in to comment.