Skip to content

Commit

Permalink
[JENKINS-72284] Take agents offline and online immediately (#198)
Browse files Browse the repository at this point in the history 
* improve how agents are token offline and online

[JENKINS-72284] when an agent with wrong java version connects, it will
be taken offline right away. Before the agent was only taken offline
when one visited the /computer page.
Use monitor specific offline causes that inherit from
MonitorOfflineCause. This allows to take the agent automatically online
again, after it was restarted with the correct java/remoting version.
Also with jenkinsci/jenkins#8618 it would then
reset the Admin monitor if that was the only reason that it fired.

Requires jenkinsci/jenkins#8593 that made 2
mthods public.

* fix spotless

* order imports

* update core version

* remove disconnect

* Spotless format

* keep disconnect methods for casc backwards compatibility

* keep disconnect help for CasC

* spotless

---------

Co-authored-by: Mark Waite <[email protected]>
  • Loading branch information
@mawinter69 @MarkEWaite
mawinter69 and MarkEWaite authored Feb 9, 2024
1 parent 84bcb47 commit 47a3737
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 73 deletions.
6 changes: 3 additions & 3 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
<changelist>999999-SNAPSHOT</changelist>
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<!-- https://www.jenkins.io/doc/developer/plugin-development/choosing-jenkins-baseline/ -->
<jenkins.version>2.401.3</jenkins.version>
<jenkins.version>2.440</jenkins.version>
<spotbugs.effort>Max</spotbugs.effort>
<spotbugs.threshold>Low</spotbugs.threshold>
<spotless.check.skip>false</spotless.check.skip>
Expand All @@ -69,8 +69,8 @@
<dependencies>
<dependency>
<groupId>io.jenkins.tools.bom</groupId>
<artifactId>bom-2.401.x</artifactId>
<version>2745.vc7b_fe4c876fa_</version>
<artifactId>bom-2.440.x</artifactId>
<version>2791.v707dc5a_1626d</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down
139 changes: 97 additions & 42 deletions src/main/java/hudson/plugin/versioncolumn/JVMVersionMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,21 @@
import hudson.Extension;
import hudson.Util;
import hudson.model.Computer;
import hudson.model.ComputerSet;
import hudson.node_monitors.AbstractAsyncNodeMonitorDescriptor;
import hudson.node_monitors.MonitorOfflineCause;
import hudson.node_monitors.NodeMonitor;
import hudson.remoting.Callable;
import hudson.slaves.OfflineCause;
import hudson.util.ListBoxModel;
import java.io.IOException;
import java.util.Map;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.security.MasterToSlaveCallable;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.export.Exported;

public class JVMVersionMonitor extends NodeMonitor {

Expand All @@ -46,19 +50,31 @@ public class JVMVersionMonitor extends NodeMonitor {

private JVMVersionComparator.ComparisonMode comparisonMode =
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE;
private boolean disconnect = true;
private transient Boolean disconnect;

@DataBoundConstructor
public JVMVersionMonitor(JVMVersionComparator.ComparisonMode comparisonMode, boolean disconnect) {
public JVMVersionMonitor(JVMVersionComparator.ComparisonMode comparisonMode) {
this.comparisonMode = comparisonMode;
this.disconnect = disconnect;
}

public JVMVersionMonitor() {}

@SuppressWarnings("unused") // jelly
public boolean isDisconnect() {
return disconnect;
return !isIgnored();
}

// should be restricted/deprecated but that breaks casc
@DataBoundSetter
public void setDisconnect(boolean disconnect) {
setIgnored(!disconnect);
}

public Object readResolve() {
if (disconnect != null) {
this.setIgnored(!disconnect);
}
return this;
}

@SuppressWarnings("unused") // jelly
Expand All @@ -74,38 +90,6 @@ public String toHtml(String version) {
return version;
}

@Override
public Object data(Computer c) {

String agentVersionStr = (String) super.data(c);
if (agentVersionStr == null) {
return "N/A";
}
Runtime.Version agentVersion;
try {
agentVersion = Runtime.Version.parse(agentVersionStr);
} catch (IllegalArgumentException e) {
LOGGER.log(Level.WARNING, "Failed to parse agent version: " + agentVersionStr, e);
return "N/A";
}
final JVMVersionComparator jvmVersionComparator =
new JVMVersionComparator(CONTROLLER_VERSION, agentVersion, comparisonMode);

if (!isIgnored() && jvmVersionComparator.isNotCompatible()) {
if (disconnect) {
LOGGER.warning(
Messages.JVMVersionMonitor_MarkedOffline(c.getName(), CONTROLLER_VERSION, agentVersionStr));
((JvmVersionDescriptor) getDescriptor())
.markOffline(c, OfflineCause.create(Messages._JVMVersionMonitor_OfflineCause()));
} else {
LOGGER.finer("Version incompatibility detected, but keeping the agent '"
+ c.getName()
+ "' online per the node monitor configuration");
}
}
return agentVersionStr;
}

public JVMVersionComparator.ComparisonMode getComparisonMode() {
return comparisonMode;
}
Expand All @@ -114,6 +98,61 @@ public JVMVersionComparator.ComparisonMode getComparisonMode() {
@Symbol("jvmVersion")
public static class JvmVersionDescriptor extends AbstractAsyncNodeMonitorDescriptor<String> {

@Override
protected Map<Computer, String> monitor() throws InterruptedException {
Result<String> base = monitorDetailed();
Map<Computer, String> data = base.getMonitoringData();
JVMVersionMonitor monitor =
(JVMVersionMonitor) ComputerSet.getMonitors().get(this);
for (Map.Entry<Computer, String> e : data.entrySet()) {
Computer computer = e.getKey();
String version = e.getValue();
if (base.getSkipped().contains(computer)) {
assert version == null;
continue;
}
if (version == null) {
e.setValue(version = get(computer));
}
markNodeOfflineOrOnline(computer, version, monitor);
}
return data;
}

private void markNodeOfflineOrOnline(Computer c, String agentVersionStr, JVMVersionMonitor monitor) {
if (agentVersionStr == null) {
return;
}
Runtime.Version agentVersion;
try {
agentVersion = Runtime.Version.parse(agentVersionStr);
} catch (IllegalArgumentException e) {
LOGGER.log(Level.WARNING, "Failed to parse agent version: " + agentVersionStr, e);
return;
}
final JVMVersionComparator jvmVersionComparator =
new JVMVersionComparator(CONTROLLER_VERSION, agentVersion, monitor.comparisonMode);

if (jvmVersionComparator.isNotCompatible()) {
if (!isIgnored()) {
LOGGER.warning(
Messages.JVMVersionMonitor_MarkedOffline(c.getName(), CONTROLLER_VERSION, agentVersionStr));
markOffline(c, new JVMMismatchCause(Messages.JVMVersionMonitor_OfflineCause()));
} else {
LOGGER.finer("Version incompatibility detected, but keeping the agent '"
+ c.getName()
+ "' online per the node monitor configuration");
if (c.isOffline() && c.getOfflineCause() instanceof JVMMismatchCause) {
c.setTemporarilyOffline(false, null);
}
}
} else {
if (c.isOffline() && c.getOfflineCause() instanceof JVMMismatchCause) {
c.setTemporarilyOffline(false, null);
}
}
}

@Override
@NonNull
public String getDisplayName() {
Expand All @@ -125,11 +164,6 @@ protected Callable<String, IOException> createCallable(Computer c) {
return new JavaVersion();
}

@Override // Just augmenting visibility
public boolean markOffline(Computer c, OfflineCause oc) {
return super.markOffline(c, oc);
}

public ListBoxModel doFillComparisonModeItems() {
ListBoxModel items = new ListBoxModel();
for (JVMVersionComparator.ComparisonMode goal : JVMVersionComparator.ComparisonMode.values()) {
Expand All @@ -139,6 +173,27 @@ public ListBoxModel doFillComparisonModeItems() {
}
}

public static class JVMMismatchCause extends MonitorOfflineCause {

private final String message;

public JVMMismatchCause(String message) {
this.message = message;
}

@Override
@Exported(name = "description")
public String toString() {
return message;
}

@NonNull
@Override
public Class<? extends NodeMonitor> getTrigger() {
return JVMVersionMonitor.class;
}
}

private static class JavaVersion extends MasterToSlaveCallable<String, IOException> {
@Override
public String call() {
Expand Down
34 changes: 32 additions & 2 deletions src/main/java/hudson/plugin/versioncolumn/VersionMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,15 @@
import hudson.Util;
import hudson.model.Computer;
import hudson.node_monitors.AbstractNodeMonitorDescriptor;
import hudson.node_monitors.MonitorOfflineCause;
import hudson.node_monitors.NodeMonitor;
import hudson.remoting.Launcher;
import hudson.slaves.OfflineCause;
import java.io.IOException;
import java.util.logging.Logger;
import jenkins.security.MasterToSlaveCallable;
import org.jenkinsci.Symbol;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.export.Exported;

public class VersionMonitor extends NodeMonitor {

Expand Down Expand Up @@ -74,8 +75,16 @@ protected String monitor(Computer c) throws IOException, InterruptedException {
String version = c.getChannel().call(new SlaveVersion());
if (version == null || !version.equals(masterVersion)) {
if (!isIgnored()) {
markOffline(c, OfflineCause.create(Messages._VersionMonitor_OfflineCause()));
markOffline(c, new RemotingVersionMismatchCause(Messages.VersionMonitor_OfflineCause()));
LOGGER.warning(Messages.VersionMonitor_MarkedOffline(c.getName()));
} else {
if (c.isOffline() && c.getOfflineCause() instanceof RemotingVersionMismatchCause) {
c.setTemporarilyOffline(false, null);
}
}
} else {
if (c.isOffline() && c.getOfflineCause() instanceof RemotingVersionMismatchCause) {
c.setTemporarilyOffline(false, null);
}
}
return version;
Expand All @@ -87,6 +96,27 @@ public String getDisplayName() {
}
}

public static class RemotingVersionMismatchCause extends MonitorOfflineCause {

private final String message;

public RemotingVersionMismatchCause(String message) {
this.message = message;
}

@Override
@Exported(name = "description")
public String toString() {
return message;
}

@NonNull
@Override
public Class<? extends NodeMonitor> getTrigger() {
return VersionMonitor.class;
}
}

private static final class SlaveVersion extends MasterToSlaveCallable<String, IOException> {

private static final long serialVersionUID = 1L;
Expand Down
3 changes: 0 additions & 3 deletions src/main/resources/hudson/plugin/versioncolumn/JVMVersionMonitor/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,4 @@ THE SOFTWARE.
<f:entry field="comparisonMode" title="${%ComparisonTitle}" >
<f:select />
</f:entry>
<f:entry title="${%DisconnectAgent}" field="disconnect">
<f:checkbox />
</f:entry>
</j:jelly>
5 changes: 1 addition & 4 deletions src/main/resources/hudson/plugin/versioncolumn/JVMVersionMonitor/help-disconnect.html
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
<div>
<p>Check this checkbox if you want to disconnect agents if they have an incompatible JVM version.

<p>The default behaviour is to disconnect such agents.</p>

This flag is deprecated. Use <code>ignored</code> instead.
</div>
30 changes: 11 additions & 19 deletions src/test/java/hudson/plugin/versioncolumn/JVMVersionMonitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,37 @@ class JVMVersionMonitorTest {
private static Object[] parameters() {
return new Object[][] {
{
JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH, false,
JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH,
},
{
JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH, true,
JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH,
},
{
JVMVersionComparator.ComparisonMode.EXACT_MATCH, true,
JVMVersionComparator.ComparisonMode.EXACT_MATCH,
},
{
JVMVersionComparator.ComparisonMode.EXACT_MATCH, false,
JVMVersionComparator.ComparisonMode.EXACT_MATCH,
},
{
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE, true,
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE,
},
{
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE, false,
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE,
},
};
}

@ParameterizedTest
@MethodSource("parameters")
public void checkDisconnect(JVMVersionComparator.ComparisonMode comparisonMode, boolean disconnect) {
JVMVersionMonitor object = new JVMVersionMonitor(comparisonMode, disconnect);
assertEquals(disconnect, object.isDisconnect());
}

@ParameterizedTest
@MethodSource("parameters")
public void checkComparisonMode(JVMVersionComparator.ComparisonMode comparisonMode, boolean disconnect) {
JVMVersionMonitor object = new JVMVersionMonitor(comparisonMode, disconnect);
public void checkComparisonMode(JVMVersionComparator.ComparisonMode comparisonMode) {
JVMVersionMonitor object = new JVMVersionMonitor(comparisonMode);
assertEquals(comparisonMode, object.getComparisonMode());
}

@Test
public void checkToHtmlRendering() throws Exception {

JVMVersionMonitor object = new JVMVersionMonitor(JVMVersionComparator.ComparisonMode.EXACT_MATCH, false);
JVMVersionMonitor object = new JVMVersionMonitor(JVMVersionComparator.ComparisonMode.EXACT_MATCH);

// N/A
assertEquals("N/A", object.toHtml(null));
Expand All @@ -60,16 +53,15 @@ public void checkToHtmlRendering() throws Exception {
assertEquals(asError("1.1.1.1+1"), object.toHtml("1.1.1.1+1"));

// RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE
object = new JVMVersionMonitor(
JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE, false);
object = new JVMVersionMonitor(JVMVersionComparator.ComparisonMode.RUNTIME_GREATER_OR_EQUAL_MASTER_BYTECODE);
assertEquals(
Runtime.version().toString(), object.toHtml(Runtime.version().toString()));
assertEquals(majorGreater(), object.toHtml(majorGreater()));
assertEquals(majorVersionMatch(), object.toHtml(majorVersionMatch()));
assertEquals(asError(majorLower()), object.toHtml(majorLower()));

// MAJOR_MINOR_MATCH
object = new JVMVersionMonitor(JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH, false);
object = new JVMVersionMonitor(JVMVersionComparator.ComparisonMode.MAJOR_MINOR_MATCH);
assertEquals(
Runtime.version().toString(), object.toHtml(Runtime.version().toString()));
assertEquals(majorGreater(), object.toHtml(majorGreater()));
Expand Down

0 comments on commit 47a3737

Please sign in to comment.