From 187a823138989fea274aec7c61a5ffe5a05d2b51 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 11 Oct 2024 15:43:59 +0200 Subject: [PATCH 1/6] Simplify persistence design for temporarily offline status There were complex code to synchronize transient state of Computer from Node. But since any temporarily offline cause is being set by a user and should be persisted in a node, the Computer should actually delegate to Node instead of trying to synchronize it --- core/src/main/java/hudson/model/Computer.java | 86 ++++++++----------- core/src/main/java/hudson/model/Node.java | 22 +---- .../AbstractNodeMonitorDescriptor.java | 4 +- .../main/java/hudson/slaves/OfflineCause.java | 15 ++++ 4 files changed, 58 insertions(+), 69 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 1047fe97000c..171ccde62aa8 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -179,11 +179,6 @@ private long connectTime = 0; - /** - * True if Jenkins shouldn't start new builds on this node. - */ - private boolean temporarilyOffline; - /** * {@link Node} object may be created and deleted independently * from this object. @@ -360,7 +355,8 @@ public AnnotatedLargeText getLogText() { */ @Exported public OfflineCause getOfflineCause() { - return offlineCause; + var temporaryOfflineCause = getNodeOrDie().getTemporaryOfflineCause(); + return temporaryOfflineCause == null ? offlineCause : temporaryOfflineCause; } @Override @@ -549,7 +545,7 @@ public void cliDisconnect(String cause) throws ExecutionException, InterruptedEx @Deprecated public void cliOffline(String cause) throws ExecutionException, InterruptedException { checkPermission(DISCONNECT); - setTemporarilyOffline(true, new ByCLI(cause)); + setTemporarilyOfflineCause(new ByCLI(cause)); } /** @@ -558,7 +554,7 @@ public void cliOffline(String cause) throws ExecutionException, InterruptedExcep @Deprecated public void cliOnline() throws ExecutionException, InterruptedException { checkPermission(CONNECT); - setTemporarilyOffline(false, null); + setTemporarilyOfflineCause(null); } /** @@ -606,6 +602,11 @@ public Node getNode() { return j.getNode(nodeName); } + @NonNull + private Node getNodeOrDie() { + return nodeName == null ? Jenkins.get() : Jenkins.get().getNode(nodeName); + } + @Exported public LoadStatistics getLoadStatistics() { return LabelAtom.get(nodeName != null ? nodeName : Jenkins.get().getSelfLabel().toString()).loadStatistics; @@ -620,7 +621,7 @@ public BuildTimelineWidget getTimeline() { @Exported @Override public boolean isOffline() { - return temporarilyOffline || getChannel() == null; + return getNodeOrDie().isTemporarilyOffline() || getChannel() == null; } public final boolean isOnline() { @@ -669,43 +670,47 @@ public boolean isLaunchSupported() { @Exported @Deprecated public boolean isTemporarilyOffline() { - return temporarilyOffline; + return getNodeOrDie().isTemporarilyOffline(); } /** * @deprecated as of 1.320. - * Use {@link #setTemporarilyOffline(boolean, OfflineCause)} + * Use {@link #setTemporarilyOfflineCause(OfflineCause)} */ @Deprecated public void setTemporarilyOffline(boolean temporarilyOffline) { - setTemporarilyOffline(temporarilyOffline, null); + setTemporarilyOfflineCause(temporarilyOffline ? new OfflineCause.LegacyOfflineCause() : null); + } + + /** + * @deprecated as of TODO. + * Use {@link #setTemporarilyOfflineCause(OfflineCause)} instead. + */ + @Deprecated + public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause) { + offlineCause = temporarilyOffline ? cause : null; } /** * Marks the computer as temporarily offline. This retains the underlying * {@link Channel} connection, but prevent builds from executing. * - * @param cause - * If the first argument is true, specify the reason why the node is being put - * offline. + * @param temporarilyOfflineCause The reason why the node is being put offline. + * If null, this cancels the status */ - public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause) { - offlineCause = temporarilyOffline ? cause : null; - this.temporarilyOffline = temporarilyOffline; - Node node = getNode(); - if (node != null) { - node.setTemporaryOfflineCause(offlineCause); - } - synchronized (statusChangeLock) { - statusChangeLock.notifyAll(); - } - if (temporarilyOffline) { - Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOffline(this, cause)); + public void setTemporarilyOfflineCause(OfflineCause temporarilyOfflineCause) { + getNodeOrDie().setTemporaryOfflineCause(temporarilyOfflineCause); + if (temporarilyOfflineCause != null) { + Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOffline(this, temporarilyOfflineCause)); } else { Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOnline(this)); } } + public OfflineCause getTemporarilyOfflineCause() { + return getNodeOrDie().getTemporaryOfflineCause(); + } + @Exported @Override public String getIcon() { @@ -785,16 +790,6 @@ protected void setNode(Node node) { this.nodeName = null; setNumExecutors(node.getNumExecutors()); - if (this.temporarilyOffline) { - // When we get a new node, push our current temp offline - // status to it (as the status is not carried across - // configuration changes that recreate the node). - // Since this is also called the very first time this - // Computer is created, avoid pushing an empty status - // as that could overwrite any status that the Node - // brought along from its persisted config data. - node.setTemporaryOfflineCause(this.offlineCause); - } } /** @@ -1396,24 +1391,19 @@ public void doRssLatest(StaplerRequest2 req, StaplerResponse2 rsp) throws IOExce @RequirePOST public HttpResponse doToggleOffline(@QueryParameter String offlineMessage) throws IOException, ServletException { - if (!temporarilyOffline) { - checkPermission(DISCONNECT); - offlineMessage = Util.fixEmptyAndTrim(offlineMessage); - setTemporarilyOffline(!temporarilyOffline, - new OfflineCause.UserCause(User.current(), offlineMessage)); - } else { + if (getNodeOrDie().isTemporarilyOffline()) { checkPermission(CONNECT); - setTemporarilyOffline(!temporarilyOffline, null); + setTemporarilyOfflineCause(null); + return HttpResponses.redirectToDot(); + } else { + return doChangeOfflineCause(offlineMessage); } - return HttpResponses.redirectToDot(); } @RequirePOST public HttpResponse doChangeOfflineCause(@QueryParameter String offlineMessage) throws IOException, ServletException { checkPermission(DISCONNECT); - offlineMessage = Util.fixEmptyAndTrim(offlineMessage); - setTemporarilyOffline(true, - new OfflineCause.UserCause(User.current(), offlineMessage)); + setTemporarilyOfflineCause(new OfflineCause.UserCause(User.current(), Util.fixEmptyAndTrim(offlineMessage))); return HttpResponses.redirectToDot(); } diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index 55cacd269133..e5bb9056d5ca 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -30,7 +30,6 @@ 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; @@ -47,7 +46,6 @@ 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; @@ -264,25 +262,11 @@ public void onLoad(Nodes parent, String name) { setNodeName(name); } - /** - * Let Nodes be aware of the lifecycle of their own {@link Computer}. - */ - @Extension - public static class InternalComputerListener extends ComputerListener { - @Override - public void onOnline(Computer c, TaskListener listener) { - Node node = c.getNode(); - - // At startup, we need to restore any previously in-effect temp offline cause. - // We wait until the computer is started rather than getting the data to it sooner - // so that the normal computer start up processing works as expected. - if (node != null && node.temporaryOfflineCause != null && node.temporaryOfflineCause != c.getOfflineCause()) { - c.setTemporarilyOffline(true, node.temporaryOfflineCause); - } - } + public boolean isTemporarilyOffline() { + return temporaryOfflineCause != null; } - private OfflineCause temporaryOfflineCause; + private volatile OfflineCause temporaryOfflineCause; /** * Enable a {@link Computer} to inform its node when it is taken diff --git a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java index 7cd1c75abc8d..fe912a42f4d1 100644 --- a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java +++ b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java @@ -233,7 +233,7 @@ public boolean isIgnored() { */ protected boolean markOnline(Computer c) { if (isIgnored() || c.isOnline()) return false; // noop - c.setTemporarilyOffline(false, null); + c.setTemporarilyOfflineCause(null); return true; } @@ -247,7 +247,7 @@ protected boolean markOnline(Computer c) { protected boolean markOffline(Computer c, OfflineCause oc) { if (isIgnored() || c.isTemporarilyOffline()) return false; // noop - c.setTemporarilyOffline(true, oc); + c.setTemporarilyOfflineCause(oc); // notify the admin MonitorMarkedNodeOffline no = AdministrativeMonitor.all().get(MonitorMarkedNodeOffline.class); diff --git a/core/src/main/java/hudson/slaves/OfflineCause.java b/core/src/main/java/hudson/slaves/OfflineCause.java index 556c0ebb0c53..b9fdb32a37d2 100644 --- a/core/src/main/java/hudson/slaves/OfflineCause.java +++ b/core/src/main/java/hudson/slaves/OfflineCause.java @@ -33,6 +33,8 @@ import java.util.Date; import jenkins.model.Jenkins; import org.jvnet.localizer.Localizable; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.ExportedBean; @@ -71,6 +73,19 @@ public long getTimestamp() { return new Date(timestamp); } + /** + * @deprecated Only exists for backward compatibility. + * @see Computer#setTemporarilyOffline(boolean). + */ + @Deprecated + @Restricted(NoExternalUse.class) + public static class LegacyOfflineCause extends OfflineCause { + @Exported(name = "description") @Override + public String toString() { + return ""; + } + } + /** * {@link OfflineCause} that renders a static text, * but without any further UI. From 962a286bc67f2f3e65fe2842a1d7b12463d8cf89 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 11 Oct 2024 15:47:37 +0200 Subject: [PATCH 2/6] Fix javadoc --- core/src/main/java/hudson/slaves/OfflineCause.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/slaves/OfflineCause.java b/core/src/main/java/hudson/slaves/OfflineCause.java index b9fdb32a37d2..ff10c3d792c7 100644 --- a/core/src/main/java/hudson/slaves/OfflineCause.java +++ b/core/src/main/java/hudson/slaves/OfflineCause.java @@ -75,7 +75,7 @@ public long getTimestamp() { /** * @deprecated Only exists for backward compatibility. - * @see Computer#setTemporarilyOffline(boolean). + * @see Computer#setTemporarilyOffline(boolean) */ @Deprecated @Restricted(NoExternalUse.class) From 888103c18461d7048bb4026a1873825077a33d9d Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 11 Oct 2024 16:34:19 +0200 Subject: [PATCH 3/6] Fix tests and apply suggestions --- core/src/main/java/hudson/model/Computer.java | 13 ++++++------- core/src/main/java/hudson/model/Node.java | 7 +++++++ .../java/hudson/cli/OfflineNodeCommandTest.java | 4 ++-- test/src/test/java/hudson/model/NodeTest.java | 2 +- 4 files changed, 16 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 171ccde62aa8..f0a15f20079e 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -688,7 +688,11 @@ public void setTemporarilyOffline(boolean temporarilyOffline) { */ @Deprecated public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause) { - offlineCause = temporarilyOffline ? cause : null; + if (cause == null) { + setTemporarilyOffline(temporarilyOffline); + } else { + setTemporarilyOfflineCause(cause); + } } /** @@ -698,13 +702,8 @@ public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause * @param temporarilyOfflineCause The reason why the node is being put offline. * If null, this cancels the status */ - public void setTemporarilyOfflineCause(OfflineCause temporarilyOfflineCause) { + public void setTemporarilyOfflineCause(@CheckForNull OfflineCause temporarilyOfflineCause) { getNodeOrDie().setTemporaryOfflineCause(temporarilyOfflineCause); - if (temporarilyOfflineCause != null) { - Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOffline(this, temporarilyOfflineCause)); - } else { - Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOnline(this)); - } } public OfflineCause getTemporarilyOfflineCause() { diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index e5bb9056d5ca..4db656358852 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -46,6 +46,7 @@ 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; @@ -67,6 +68,7 @@ import java.util.logging.Logger; import jenkins.model.Jenkins; import jenkins.model.Nodes; +import jenkins.util.Listeners; import jenkins.util.SystemProperties; import jenkins.util.io.OnMaster; import net.sf.json.JSONObject; @@ -278,6 +280,11 @@ void setTemporaryOfflineCause(OfflineCause cause) { temporaryOfflineCause = cause; save(); } + if (temporaryOfflineCause != null) { + Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOffline(toComputer(), temporaryOfflineCause)); + } else { + Listeners.notify(ComputerListener.class, false, l -> l.onTemporarilyOnline(toComputer())); + } } catch (java.io.IOException e) { LOGGER.warning("Unable to complete save, temporary offline status will not be persisted: " + e.getMessage()); } diff --git a/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java b/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java index 564c1b098baf..878001e1b145 100644 --- a/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java +++ b/test/src/test/java/hudson/cli/OfflineNodeCommandTest.java @@ -118,7 +118,7 @@ public void offlineNodeShouldSucceedOnOfflineNode() throws Exception { slave.toComputer().setTemporarilyOffline(true, null); assertThat(slave.toComputer().isOffline(), equalTo(true)); assertThat(slave.toComputer().isTemporarilyOffline(), equalTo(true)); - assertThat(slave.toComputer().getOfflineCause(), equalTo(null)); + assertThat(slave.toComputer().getOfflineCause(), instanceOf(OfflineCause.LegacyOfflineCause.class)); final CLICommandInvoker.Result result = command .authorizedTo(Computer.DISCONNECT, Jenkins.READ) @@ -177,7 +177,7 @@ public void offlineNodeShouldSucceedOnOfflineNodeWithCause() throws Exception { slave.toComputer().setTemporarilyOffline(true, null); assertThat(slave.toComputer().isOffline(), equalTo(true)); assertThat(slave.toComputer().isTemporarilyOffline(), equalTo(true)); - assertThat(slave.toComputer().getOfflineCause(), equalTo(null)); + assertThat(slave.toComputer().getOfflineCause(), instanceOf(OfflineCause.LegacyOfflineCause.class)); final CLICommandInvoker.Result result = command .authorizedTo(Computer.DISCONNECT, Jenkins.READ) diff --git a/test/src/test/java/hudson/model/NodeTest.java b/test/src/test/java/hudson/model/NodeTest.java index 39ffe25f7643..d868a39d2d19 100644 --- a/test/src/test/java/hudson/model/NodeTest.java +++ b/test/src/test/java/hudson/model/NodeTest.java @@ -102,7 +102,7 @@ public void testSetTemporaryOfflineCause() throws Exception { assertEquals("Node should have offline cause which was set.", cause, node.toComputer().getOfflineCause()); OfflineCause cause2 = new OfflineCause.ByCLI("another message"); node.setTemporaryOfflineCause(cause2); - assertEquals("Node should have original offline cause after setting another.", cause, node.toComputer().getOfflineCause()); + assertEquals("Node should have the new offline cause.", cause2, node.toComputer().getOfflineCause()); } @Test From 0d129d9c1643a7888934046bf784d5495b17a9ab Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 11 Oct 2024 16:41:21 +0200 Subject: [PATCH 4/6] Fix getNodeOrDie() --- core/src/main/java/hudson/model/Computer.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index f0a15f20079e..625ac200f9a0 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -604,7 +604,11 @@ public Node getNode() { @NonNull private Node getNodeOrDie() { - return nodeName == null ? Jenkins.get() : Jenkins.get().getNode(nodeName); + var node = nodeName == null ? Jenkins.get() : Jenkins.get().getNode(nodeName); + if (node == null) { + throw new IllegalStateException("Can't set a temporary offline cause if the node has been removed"); + } + return node; } @Exported From 1d20b1c0e25ead39fa0a6133fcfee2ef3c751dd7 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 11 Oct 2024 17:22:49 +0200 Subject: [PATCH 5/6] Reduce visibility of Node#isTemporarilyOffline --- core/src/main/java/hudson/model/Node.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/Node.java b/core/src/main/java/hudson/model/Node.java index 4db656358852..4faa3ed106b4 100644 --- a/core/src/main/java/hudson/model/Node.java +++ b/core/src/main/java/hudson/model/Node.java @@ -264,7 +264,7 @@ public void onLoad(Nodes parent, String name) { setNodeName(name); } - public boolean isTemporarilyOffline() { + boolean isTemporarilyOffline() { return temporaryOfflineCause != null; } From d60e43abc5f4b20724fbda436e65a0cbeb013a15 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Sat, 12 Oct 2024 06:40:19 +0200 Subject: [PATCH 6/6] Remove getNodeOrDie --- core/src/main/java/hudson/model/Computer.java | 61 ++++++++++--------- .../AbstractNodeMonitorDescriptor.java | 4 +- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/core/src/main/java/hudson/model/Computer.java b/core/src/main/java/hudson/model/Computer.java index 625ac200f9a0..cc1d6b351bfb 100644 --- a/core/src/main/java/hudson/model/Computer.java +++ b/core/src/main/java/hudson/model/Computer.java @@ -355,8 +355,14 @@ public AnnotatedLargeText getLogText() { */ @Exported public OfflineCause getOfflineCause() { - var temporaryOfflineCause = getNodeOrDie().getTemporaryOfflineCause(); - return temporaryOfflineCause == null ? offlineCause : temporaryOfflineCause; + var node = getNode(); + if (node != null) { + var temporaryOfflineCause = node.getTemporaryOfflineCause(); + if (temporaryOfflineCause != null) { + return temporaryOfflineCause; + } + } + return offlineCause; } @Override @@ -545,7 +551,7 @@ public void cliDisconnect(String cause) throws ExecutionException, InterruptedEx @Deprecated public void cliOffline(String cause) throws ExecutionException, InterruptedException { checkPermission(DISCONNECT); - setTemporarilyOfflineCause(new ByCLI(cause)); + setTemporaryOfflineCause(new ByCLI(cause)); } /** @@ -554,7 +560,7 @@ public void cliOffline(String cause) throws ExecutionException, InterruptedExcep @Deprecated public void cliOnline() throws ExecutionException, InterruptedException { checkPermission(CONNECT); - setTemporarilyOfflineCause(null); + setTemporaryOfflineCause(null); } /** @@ -602,15 +608,6 @@ public Node getNode() { return j.getNode(nodeName); } - @NonNull - private Node getNodeOrDie() { - var node = nodeName == null ? Jenkins.get() : Jenkins.get().getNode(nodeName); - if (node == null) { - throw new IllegalStateException("Can't set a temporary offline cause if the node has been removed"); - } - return node; - } - @Exported public LoadStatistics getLoadStatistics() { return LabelAtom.get(nodeName != null ? nodeName : Jenkins.get().getSelfLabel().toString()).loadStatistics; @@ -625,7 +622,8 @@ public BuildTimelineWidget getTimeline() { @Exported @Override public boolean isOffline() { - return getNodeOrDie().isTemporarilyOffline() || getChannel() == null; + var node = getNode(); + return node == null || node.isTemporarilyOffline() || getChannel() == null; } public final boolean isOnline() { @@ -674,28 +672,29 @@ public boolean isLaunchSupported() { @Exported @Deprecated public boolean isTemporarilyOffline() { - return getNodeOrDie().isTemporarilyOffline(); + var node = getNode(); + return node != null && node.isTemporarilyOffline(); } /** * @deprecated as of 1.320. - * Use {@link #setTemporarilyOfflineCause(OfflineCause)} + * Use {@link #setTemporaryOfflineCause(OfflineCause)} */ @Deprecated public void setTemporarilyOffline(boolean temporarilyOffline) { - setTemporarilyOfflineCause(temporarilyOffline ? new OfflineCause.LegacyOfflineCause() : null); + setTemporaryOfflineCause(temporarilyOffline ? new OfflineCause.LegacyOfflineCause() : null); } /** * @deprecated as of TODO. - * Use {@link #setTemporarilyOfflineCause(OfflineCause)} instead. + * Use {@link #setTemporaryOfflineCause(OfflineCause)} instead. */ @Deprecated public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause) { if (cause == null) { setTemporarilyOffline(temporarilyOffline); } else { - setTemporarilyOfflineCause(cause); + setTemporaryOfflineCause(cause); } } @@ -703,15 +702,15 @@ public void setTemporarilyOffline(boolean temporarilyOffline, OfflineCause cause * Marks the computer as temporarily offline. This retains the underlying * {@link Channel} connection, but prevent builds from executing. * - * @param temporarilyOfflineCause The reason why the node is being put offline. + * @param temporaryOfflineCause The reason why the node is being put offline. * If null, this cancels the status */ - public void setTemporarilyOfflineCause(@CheckForNull OfflineCause temporarilyOfflineCause) { - getNodeOrDie().setTemporaryOfflineCause(temporarilyOfflineCause); - } - - public OfflineCause getTemporarilyOfflineCause() { - return getNodeOrDie().getTemporaryOfflineCause(); + public void setTemporaryOfflineCause(@CheckForNull OfflineCause temporaryOfflineCause) { + var node = getNode(); + if (node == null) { + throw new IllegalStateException("Can't set a temporary offline cause if the node has been removed"); + } + node.setTemporaryOfflineCause(temporaryOfflineCause); } @Exported @@ -1394,9 +1393,13 @@ public void doRssLatest(StaplerRequest2 req, StaplerResponse2 rsp) throws IOExce @RequirePOST public HttpResponse doToggleOffline(@QueryParameter String offlineMessage) throws IOException, ServletException { - if (getNodeOrDie().isTemporarilyOffline()) { + var node = getNode(); + if (node == null) { + return HttpResponses.notFound(); + } + if (node.isTemporarilyOffline()) { checkPermission(CONNECT); - setTemporarilyOfflineCause(null); + setTemporaryOfflineCause(null); return HttpResponses.redirectToDot(); } else { return doChangeOfflineCause(offlineMessage); @@ -1406,7 +1409,7 @@ public HttpResponse doToggleOffline(@QueryParameter String offlineMessage) throw @RequirePOST public HttpResponse doChangeOfflineCause(@QueryParameter String offlineMessage) throws IOException, ServletException { checkPermission(DISCONNECT); - setTemporarilyOfflineCause(new OfflineCause.UserCause(User.current(), Util.fixEmptyAndTrim(offlineMessage))); + setTemporaryOfflineCause(new OfflineCause.UserCause(User.current(), Util.fixEmptyAndTrim(offlineMessage))); return HttpResponses.redirectToDot(); } diff --git a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java index fe912a42f4d1..d49924508bee 100644 --- a/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java +++ b/core/src/main/java/hudson/node_monitors/AbstractNodeMonitorDescriptor.java @@ -233,7 +233,7 @@ public boolean isIgnored() { */ protected boolean markOnline(Computer c) { if (isIgnored() || c.isOnline()) return false; // noop - c.setTemporarilyOfflineCause(null); + c.setTemporaryOfflineCause(null); return true; } @@ -247,7 +247,7 @@ protected boolean markOnline(Computer c) { protected boolean markOffline(Computer c, OfflineCause oc) { if (isIgnored() || c.isTemporarilyOffline()) return false; // noop - c.setTemporarilyOfflineCause(oc); + c.setTemporaryOfflineCause(oc); // notify the admin MonitorMarkedNodeOffline no = AdministrativeMonitor.all().get(MonitorMarkedNodeOffline.class);