Skip to content

Commit

Permalink
Merge pull request #738 from Vlatombe/noReconnectAfter
Browse files Browse the repository at this point in the history
  • Loading branch information
Vlatombe authored May 15, 2024
2 parents 19e2927 + 623bc8c commit 4280e21
Show file tree
Hide file tree
Showing 9 changed files with 505 additions and 5 deletions.
26 changes: 23 additions & 3 deletions src/main/java/hudson/remoting/Engine.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import java.security.interfaces.RSAPublicKey;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
Expand Down Expand Up @@ -100,6 +102,7 @@
import org.jenkinsci.remoting.protocol.cert.DelegatingX509ExtendedTrustManager;
import org.jenkinsci.remoting.protocol.cert.PublicKeyMatchingX509ExtendedTrustManager;
import org.jenkinsci.remoting.protocol.impl.ConnectionRefusalException;
import org.jenkinsci.remoting.util.DurationFormatter;
import org.jenkinsci.remoting.util.KeyUtils;
import org.jenkinsci.remoting.util.VersionNumber;
import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier;
Expand Down Expand Up @@ -193,6 +196,10 @@ public Thread newThread(@NonNull final Runnable r) {

private boolean noReconnect = false;

private Duration noReconnectAfter;

private Instant firstAttempt;

/**
* Determines whether the socket will have {@link Socket#setKeepAlive(boolean)} set or not.
*
Expand Down Expand Up @@ -412,6 +419,10 @@ public void setNoReconnect(boolean noReconnect) {
this.noReconnect = noReconnect;
}

public void setNoReconnectAfter(@CheckForNull Duration noReconnectAfter) {
this.noReconnectAfter = noReconnectAfter;
}

/**
* Determines if JNLPAgentEndpointResolver will not perform certificate validation in the HTTPs mode.
*
Expand Down Expand Up @@ -740,9 +751,14 @@ public void closeRead() throws IOException {
if (noReconnect) {
return;
}
firstAttempt = Instant.now();
events.onDisconnect();
while (true) {
// TODO refactor various sleep statements into a common method

Check warning on line 757 in src/main/java/hudson/remoting/Engine.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: refactor various sleep statements into a common method
if (Util.shouldBailOut(firstAttempt, noReconnectAfter)) {
events.status("Bailing out after " + DurationFormatter.format(noReconnectAfter));
return;
}
TimeUnit.SECONDS.sleep(10);
// Unlike JnlpAgentEndpointResolver, we do not use $jenkins/tcpSlaveAgentListener/, as that will be a 404 if the TCP port is disabled.
URL ping = new URL(hudsonUrl, "login");
Expand Down Expand Up @@ -795,14 +811,18 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) {

try {
boolean first = true;
firstAttempt = Instant.now();
while(true) {
if(first) {
first = false;
} else {
if(noReconnect)
return; // exit
}

if (Util.shouldBailOut(firstAttempt, noReconnectAfter)) {
events.status("Bailing out after " + DurationFormatter.format(noReconnectAfter));
return;
}
events.status("Locating server among " + candidateUrls);
final JnlpAgentEndpoint endpoint;
try {
Expand Down Expand Up @@ -915,7 +935,7 @@ private void innerRun(IOHub hub, SSLContext context, ExecutorService service) {
}
if(noReconnect)
return; // exit

firstAttempt = Instant.now();
events.onDisconnect();

// try to connect back to the server every 10 secs.
Expand All @@ -938,7 +958,7 @@ private JnlpEndpointResolver createEndpointResolver(List<String> jenkinsUrls, St
events.error(e);
}
resolver = new JnlpAgentEndpointResolver(jenkinsUrls, agentName, credentials, proxyCredentials, tunnel,
sslSocketFactory, disableHttpsCertValidation);
sslSocketFactory, disableHttpsCertValidation, noReconnectAfter);
} else {
resolver = new JnlpAgentEndpointConfigurator(directConnection, instanceIdentity, protocols, proxyCredentials);
}
Expand Down
13 changes: 12 additions & 1 deletion src/main/java/hudson/remoting/Launcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,12 @@
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import hudson.remoting.Channel.Mode;
import java.time.Duration;
import java.time.Instant;
import org.jenkinsci.remoting.DurationOptionHandler;
import org.jenkinsci.remoting.engine.JnlpAgentEndpointResolver;
import org.jenkinsci.remoting.engine.WorkDirManager;
import org.jenkinsci.remoting.util.DurationFormatter;
import org.jenkinsci.remoting.util.PathUtils;
import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier;
import org.kohsuke.args4j.Argument;
Expand Down Expand Up @@ -249,6 +253,9 @@ public void setConnectTo(String target) {
@Option(name="-noReconnect",aliases="-noreconnect",usage="Doesn't try to reconnect when a communication fail, and exit instead")
public boolean noReconnect = false;

@Option(name="-noReconnectAfter",usage = "Bail out after the given time after the first attempt to reconnect", handler = DurationOptionHandler.class, forbids = "-noReconnect")
public Duration noReconnectAfter;

@Option(name = "-noKeepAlive",
usage = "Disable TCP socket keep alive on connection to the controller.")
public boolean noKeepAlive = false;
Expand Down Expand Up @@ -682,6 +689,7 @@ private List<String> parseJnlpArguments() throws ParserConfigurationException, S
throw new IOException("-jnlpCredentials and -secret are mutually exclusive");
}
}
Instant firstAttempt = Instant.now();
while (true) {
URLConnection con = null;
try {
Expand Down Expand Up @@ -742,7 +750,9 @@ private List<String> parseJnlpArguments() throws ParserConfigurationException, S
} catch (IOException e) {
if (this.noReconnect)
throw new IOException("Failed to obtain " + agentJnlpURL, e);

if (Util.shouldBailOut(firstAttempt, noReconnectAfter)) {
throw new IOException("Failed to obtain " + agentJnlpURL + " after " + DurationFormatter.format(noReconnectAfter), e);
}
System.err.println("Failed to obtain "+ agentJnlpURL);
e.printStackTrace(System.err);
System.err.println("Waiting 10 seconds before retry");
Expand Down Expand Up @@ -1026,6 +1036,7 @@ private Engine createEngine() throws IOException {
engine.setJarCache(new FileSystemJarCache(jarCache, true));
}
engine.setNoReconnect(noReconnect);
engine.setNoReconnectAfter(noReconnectAfter);
engine.setKeepAlive(!noKeepAlive);

if (noCertificateCheck) {
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/hudson/remoting/Util.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package hudson.remoting;

import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
import java.time.Duration;
import java.time.Instant;
import org.jenkinsci.remoting.util.PathUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
Expand Down Expand Up @@ -114,6 +117,13 @@ static public String getVersion() {
return version;
}

public static boolean shouldBailOut(@NonNull Instant firstAttempt, @CheckForNull Duration noReconnectAfter) {
if (noReconnectAfter == null) {
return false;
}
return Duration.between(firstAttempt, Instant.now()).compareTo(noReconnectAfter) > 0;
}

private Util() {
}

Expand Down
63 changes: 63 additions & 0 deletions src/main/java/org/jenkinsci/remoting/DurationOptionHandler.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* The MIT License
*
* Copyright (c) 2024, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.remoting;

import java.time.Duration;
import org.jenkinsci.remoting.util.DurationFormatter;
import org.jenkinsci.remoting.util.DurationStyle;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Parameters;
import org.kohsuke.args4j.spi.Setter;

/**
* Parses a string like 1s, 2m, 3h, 4d into a {@link Duration}.
*/
@Restricted(NoExternalUse.class)
public class DurationOptionHandler extends OptionHandler<Duration> {
public DurationOptionHandler(CmdLineParser parser, OptionDef option, Setter<? super Duration> setter) {
super(parser, option, setter);
}

@Override
public int parseArguments(Parameters params) throws CmdLineException {
setter.addValue(DurationStyle.detectAndParse(params.getParameter(0)));
return 1;
}

@Override
public String getDefaultMetaVariable() {
return "DURATION";
}

@Override
protected String print(Duration v) {
return DurationFormatter.format(v);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
import hudson.remoting.Engine;
import hudson.remoting.Launcher;
import hudson.remoting.NoProxyEvaluator;
import hudson.remoting.Util;
import java.time.Duration;
import java.time.Instant;
import org.jenkinsci.remoting.util.DurationFormatter;
import org.jenkinsci.remoting.util.VersionNumber;
import org.jenkinsci.remoting.util.https.NoCheckHostnameVerifier;
import org.kohsuke.accmod.Restricted;
Expand Down Expand Up @@ -100,6 +104,8 @@ public class JnlpAgentEndpointResolver extends JnlpEndpointResolver {

private HostnameVerifier hostnameVerifier;

private Duration noReconnectAfter;

/**
* If specified, only the protocols from the list will be tried during the connection.
* The option provides protocol names, but the order of the check is defined internally and cannot be changed.
Expand All @@ -110,14 +116,15 @@ public class JnlpAgentEndpointResolver extends JnlpEndpointResolver {
System.getProperty(JnlpAgentEndpointResolver.class.getName() + ".protocolNamesToTry");

public JnlpAgentEndpointResolver(@NonNull List<String> jenkinsUrls, String agentName, String credentials, String proxyCredentials,
String tunnel, SSLSocketFactory sslSocketFactory, boolean disableHttpsCertValidation) {
String tunnel, SSLSocketFactory sslSocketFactory, boolean disableHttpsCertValidation, Duration noReconnectAfter) {
this.jenkinsUrls = new ArrayList<>(jenkinsUrls);
this.agentName = agentName;
this.credentials = credentials;
this.proxyCredentials = proxyCredentials;
this.tunnel = tunnel;
this.sslSocketFactory = sslSocketFactory;
setDisableHttpsCertValidation(disableHttpsCertValidation);
this.noReconnectAfter = noReconnectAfter;
}

public SSLSocketFactory getSslSocketFactory() {
Expand Down Expand Up @@ -401,8 +408,13 @@ public void waitForReady() throws InterruptedException {
String oldName = t.getName();
try {
int retries = 0;
Instant firstAttempt = Instant.now();
while (true) {
// TODO refactor various sleep statements into a common method

Check warning on line 413 in src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java

View check run for this annotation

ci.jenkins.io / Open Tasks Scanner

TODO

NORMAL: refactor various sleep statements into a common method
if (Util.shouldBailOut(firstAttempt, noReconnectAfter)) {
LOGGER.info("Bailing out after " + DurationFormatter.format(noReconnectAfter));
return;
}
Thread.sleep(1000 * 10);
// Jenkins top page might be read-protected. see http://www.nabble
// .com/more-lenient-retry-logic-in-Engine.waitForServerToBack-td24703172.html
Expand Down
72 changes: 72 additions & 0 deletions src/main/java/org/jenkinsci/remoting/util/DurationFormatter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* The MIT License
*
* Copyright (c) 2024, CloudBees, Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
package org.jenkinsci.remoting.util;

import java.time.Duration;
import java.time.temporal.ChronoUnit;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;

/**
* Formats a {@link Duration} into a human-readable string.
*/
@Restricted(NoExternalUse.class)
public final class DurationFormatter {
private DurationFormatter(){}

public static String format(Duration d) {
StringBuilder sb = new StringBuilder();
boolean first = true;
long days = d.toDays();
if (days > 0) {
first = formatDurationPart(true, sb, days, "day");
d = d.minus(days, ChronoUnit.DAYS);
}
long hours = d.toHours();
if (hours > 0) {
first = formatDurationPart(first, sb, hours, "hour");
d = d.minus(hours, ChronoUnit.HOURS);
}
long minutes = d.toMinutes();
if (minutes > 0) {
first = formatDurationPart(first, sb, minutes, "minute");
d = d.minus(minutes, ChronoUnit.MINUTES);
}
long seconds = d.getSeconds();
if (seconds > 0) {
formatDurationPart(first, sb, seconds, "second");
}
return sb.toString();
}

private static boolean formatDurationPart(boolean first, StringBuilder sb, long amount, String unit) {
if (!first) {
sb.append(", ");
} else {
first = false;
}
sb.append(amount).append(" ").append(unit).append(amount > 1 ? "s" : "");
return first;
}
}
Loading

0 comments on commit 4280e21

Please sign in to comment.