diff --git a/core/pom.xml b/core/pom.xml index 36489c62c89057b432fb8e39b8c48043234b4ef4..4c1c49392c107c70dd856e8f1194285e23964ea2 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -606,10 +606,15 @@ THE SOFTWARE. /usr/local/yjp/lib/yjp.jar - - com.google.guava - guava - + + com.google.guava + guava + + + com.google.guava + guava-testlib + test + com.jcraft diff --git a/core/src/main/java/hudson/slaves/ChannelPinger.java b/core/src/main/java/hudson/slaves/ChannelPinger.java index 2bc6d4cffba786883e005f1adf79f40221c8c304..0d4dae702ca82e6daaf5f487ffc8a392c60ff50a 100644 --- a/core/src/main/java/hudson/slaves/ChannelPinger.java +++ b/core/src/main/java/hudson/slaves/ChannelPinger.java @@ -36,10 +36,9 @@ import jenkins.slaves.PingFailureAnalyzer; import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.logging.Level; import java.util.logging.Logger; -import static java.util.logging.Level.*; - /** * Establish a periodic ping to keep connections between {@link Slave slaves} * and the main Jenkins node alive. This prevents network proxies from @@ -49,17 +48,40 @@ import static java.util.logging.Level.*; */ @Extension public class ChannelPinger extends ComputerListener { - private static final Logger LOGGER = Logger.getLogger(ChannelPinger.class.getName()); - private static final String SYS_PROPERTY_NAME = ChannelPinger.class.getName() + ".pingInterval"; - private static final int DEFAULT_PING_INTERVAL_MIN = 5; + static final int PING_TIMEOUT_SECONDS_DEFAULT = 4 * 60; + static final int PING_INTERVAL_SECONDS_DEFAULT = 5 * 60; + private static final Logger LOGGER = Logger.getLogger(ChannelPinger.class.getName()); + private static final String TIMEOUT_SECONDS_PROPERTY = ChannelPinger.class.getName() + ".pingTimeoutSeconds"; + private static final String INTERVAL_MINUTES_PROPERTY_DEPRECATED = ChannelPinger.class.getName() + ".pingInterval"; + private static final String INTERVAL_SECONDS_PROPERTY = ChannelPinger.class.getName() + ".pingIntervalSeconds"; + + /** + * Timeout for the ping in seconds. + */ + private int pingTimeoutSeconds = SystemProperties.getInteger(TIMEOUT_SECONDS_PROPERTY, PING_TIMEOUT_SECONDS_DEFAULT, Level.WARNING); + /** - * Interval for the ping in minutes. + * Interval for the ping in seconds. */ - private final int pingInterval; + private int pingIntervalSeconds = PING_INTERVAL_SECONDS_DEFAULT; public ChannelPinger() { - pingInterval = SystemProperties.getInteger(SYS_PROPERTY_NAME, DEFAULT_PING_INTERVAL_MIN); + + Integer interval = SystemProperties.getInteger(INTERVAL_SECONDS_PROPERTY, null, Level.WARNING); + + // if interval wasn't set we read the deprecated property in minutes + if (interval == null) { + interval = SystemProperties.getInteger(INTERVAL_MINUTES_PROPERTY_DEPRECATED,null, Level.WARNING); + if (interval != null) { + LOGGER.warning(INTERVAL_MINUTES_PROPERTY_DEPRECATED + " property is deprecated, " + INTERVAL_SECONDS_PROPERTY + " should be used"); + interval *= 60; //to seconds + } + } + + if (interval != null) { + pingIntervalSeconds = interval; + } } @Override @@ -68,13 +90,13 @@ public class ChannelPinger extends ComputerListener { } public void install(Channel channel) { - if (pingInterval < 1) { - LOGGER.fine("Agent ping is disabled"); + if (pingTimeoutSeconds < 1 || pingIntervalSeconds < 1) { + LOGGER.warning("Agent ping is disabled"); return; } try { - channel.call(new SetUpRemotePing(pingInterval)); + channel.call(new SetUpRemotePing(pingTimeoutSeconds, pingIntervalSeconds)); LOGGER.fine("Set up a remote ping for " + channel.getName()); } catch (Exception e) { LOGGER.severe("Failed to set up a ping for " + channel.getName()); @@ -82,26 +104,69 @@ public class ChannelPinger extends ComputerListener { // set up ping from both directions, so that in case of a router dropping a connection, // both sides can notice it and take compensation actions. - setUpPingForChannel(channel, pingInterval, true); + setUpPingForChannel(channel, pingTimeoutSeconds, pingIntervalSeconds, true); } - private static class SetUpRemotePing extends MasterToSlaveCallable { + static class SetUpRemotePing extends MasterToSlaveCallable { private static final long serialVersionUID = -2702219700841759872L; - private int pingInterval; - public SetUpRemotePing(int pingInterval) { - this.pingInterval = pingInterval; + @Deprecated + private transient int pingInterval; + private final int pingTimeoutSeconds; + private final int pingIntervalSeconds; + + SetUpRemotePing(int pingTimeoutSeconds, int pingIntervalSeconds) { + this.pingTimeoutSeconds = pingTimeoutSeconds; + this.pingIntervalSeconds = pingIntervalSeconds; } + @Override public Void call() throws IOException { - setUpPingForChannel(Channel.current(), pingInterval, false); + setUpPingForChannel(Channel.current(), pingTimeoutSeconds, pingIntervalSeconds, false); return null; } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + pingIntervalSeconds; + result = prime * result + pingTimeoutSeconds; + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + SetUpRemotePing other = (SetUpRemotePing) obj; + if (pingIntervalSeconds != other.pingIntervalSeconds) { + return false; + } + if (pingTimeoutSeconds != other.pingTimeoutSeconds) { + return false; + } + return true; + } + + protected Object readResolve() { + if (pingInterval != 0) { + return new SetUpRemotePing(PING_TIMEOUT_SECONDS_DEFAULT, pingInterval * 60); + } + return this; + } } - private static void setUpPingForChannel(final Channel channel, int interval, final boolean analysis) { - LOGGER.log(FINE, "setting up ping on {0} at interval {1}m", new Object[] {channel.getName(), interval}); + static void setUpPingForChannel(final Channel channel, int timeoutSeconds, int intervalSeconds, final boolean analysis) { + LOGGER.log(Level.FINE, "setting up ping on {0} with a {1} seconds interval and {2} seconds timeout", new Object[] {channel.getName(), intervalSeconds, timeoutSeconds}); final AtomicBoolean isInClosed = new AtomicBoolean(false); - final PingThread t = new PingThread(channel, interval * 60 * 1000) { + final PingThread t = new PingThread(channel, timeoutSeconds * 1000L, intervalSeconds * 1000L) { @Override protected void onDead(Throwable cause) { try { @@ -109,13 +174,13 @@ public class ChannelPinger extends ComputerListener { analyze(cause); } if (isInClosed.get()) { - LOGGER.log(FINE,"Ping failed after the channel "+channel.getName()+" is already partially closed.",cause); + LOGGER.log(Level.FINE,"Ping failed after the channel "+channel.getName()+" is already partially closed.",cause); } else { - LOGGER.log(INFO,"Ping failed. Terminating the channel "+channel.getName()+".",cause); + LOGGER.log(Level.INFO,"Ping failed. Terminating the channel "+channel.getName()+".",cause); channel.close(cause); } } catch (IOException e) { - LOGGER.log(SEVERE,"Failed to terminate the channel "+channel.getName(),e); + LOGGER.log(Level.SEVERE,"Failed to terminate the channel "+channel.getName(),e); } } /** Keep in a separate method so we do not even try to do class loading on {@link PingFailureAnalyzer} from an agent JVM. */ @@ -141,6 +206,7 @@ public class ChannelPinger extends ComputerListener { }); t.start(); - LOGGER.fine("Ping thread started for " + channel + " with a " + interval + " minute interval"); + LOGGER.log(Level.FINE, "Ping thread started for {0} with a {1} seconds interval and a {2} seconds timeout", + new Object[] { channel, intervalSeconds, timeoutSeconds }); } } diff --git a/core/src/test/java/hudson/slaves/ChannelPingerTest.java b/core/src/test/java/hudson/slaves/ChannelPingerTest.java new file mode 100644 index 0000000000000000000000000000000000000000..e3e67a1b26d404b49a1b183ca3ae8d26b01b5194 --- /dev/null +++ b/core/src/test/java/hudson/slaves/ChannelPingerTest.java @@ -0,0 +1,116 @@ +package hudson.slaves; + +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.verify; +import static org.powermock.api.mockito.PowerMockito.mockStatic; +import static org.powermock.api.mockito.PowerMockito.verifyStatic; + +import com.google.common.testing.EqualsTester; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import hudson.remoting.Channel; + +import java.util.Map; +import java.util.HashMap; + +@RunWith(PowerMockRunner.class) +@PrepareForTest({ ChannelPinger.class }) +public class ChannelPingerTest { + + @Mock private Channel mockChannel; + + private Map savedSystemProperties = new HashMap(); + + @Before + public void setUp() throws Exception { + MockitoAnnotations.initMocks(this); + mockStatic(ChannelPinger.class); + } + + @Before + public void preserveSystemProperties() throws Exception { + preserveSystemProperty("hudson.slaves.ChannelPinger.pingInterval"); + preserveSystemProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds"); + preserveSystemProperty("hudson.slaves.ChannelPinger.pingTimeoutSeconds"); + } + + @After + public void restoreSystemProperties() throws Exception { + for (Map.Entry entry : savedSystemProperties.entrySet()) { + if (entry.getValue() != null) { + System.setProperty(entry.getKey(), entry.getValue()); + } else { + System.clearProperty(entry.getKey()); + } + } + } + + private void preserveSystemProperty(String propertyName) { + savedSystemProperties.put(propertyName, System.getProperty(propertyName)); + System.clearProperty(propertyName); + } + + @Test + public void testDefaults() throws Exception { + ChannelPinger channelPinger = new ChannelPinger(); + channelPinger.install(mockChannel); + + verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, + ChannelPinger.PING_INTERVAL_SECONDS_DEFAULT))); + verifyStatic(); + ChannelPinger.setUpPingForChannel(mockChannel, ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, + ChannelPinger.PING_INTERVAL_SECONDS_DEFAULT, true); + } + + @Test + public void testFromSystemProperties() throws Exception { + System.setProperty("hudson.slaves.ChannelPinger.pingTimeoutSeconds", "42"); + System.setProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds", "73"); + + ChannelPinger channelPinger = new ChannelPinger(); + channelPinger.install(mockChannel); + + verify(mockChannel).call(new ChannelPinger.SetUpRemotePing(42, 73)); + verifyStatic(); + ChannelPinger.setUpPingForChannel(mockChannel, 42, 73, true); + } + + @Test + public void testFromOldSystemProperty() throws Exception { + System.setProperty("hudson.slaves.ChannelPinger.pingInterval", "7"); + + ChannelPinger channelPinger = new ChannelPinger(); + channelPinger.install(mockChannel); + + verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, 420))); + verifyStatic(); + ChannelPinger.setUpPingForChannel(mockChannel, ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, 420, true); + } + + @Test + public void testNewSystemPropertyTrumpsOld() throws Exception { + System.setProperty("hudson.slaves.ChannelPinger.pingIntervalSeconds", "73"); + System.setProperty("hudson.slaves.ChannelPinger.pingInterval", "7"); + + ChannelPinger channelPinger = new ChannelPinger(); + channelPinger.install(mockChannel); + + verify(mockChannel).call(eq(new ChannelPinger.SetUpRemotePing(ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, 73))); + verifyStatic(); + ChannelPinger.setUpPingForChannel(mockChannel, ChannelPinger.PING_TIMEOUT_SECONDS_DEFAULT, 73, true); + } + + @Test + public void testSetUpRemotePingEquality() { + new EqualsTester() + .addEqualityGroup(new ChannelPinger.SetUpRemotePing(1, 2), new ChannelPinger.SetUpRemotePing(1, 2)) + .addEqualityGroup(new ChannelPinger.SetUpRemotePing(2, 3), new ChannelPinger.SetUpRemotePing(2, 3)) + .testEquals(); + } +} diff --git a/pom.xml b/pom.xml index 410c7bf46af970613cb3ce49953b61cdfb6960a0..bd0fea81aaa9820c788660800f03f806c9d61cc1 100644 --- a/pom.xml +++ b/pom.xml @@ -91,6 +91,7 @@ THE SOFTWARE. https://api.github.com jenkins-jira + 11.0.1 1.7.7 2.14 1.4.1 @@ -186,7 +187,12 @@ THE SOFTWARE. com.google.guava guava - 11.0.1 + ${guavaVersion} + + + com.google.guava + guava-testlib + ${guavaVersion}