Skip to content
Permalink
Browse files

[FIXED JENKINS-38487] - Jenkins startup must not fail in the case of …

…ComputerListener failure (#2610)

Without this code Jenkinbs startup fails if EnvInject fails to find global property file on startup.

Javadoc says "Exceptions will be recorded to the listener. Note that throwing an exception doesn't put the computer offline." regarding the listener method exception, hence we should not block Jenkins startup
(cherry picked from commit 58e1228)
  • Loading branch information...
oleg-nenashev authored and olivergondza committed Nov 5, 2016
1 parent 7e8ea4a commit b10440702f74c70786820bdf38d417569d86644d
@@ -143,7 +143,7 @@ public void onOnline(Computer c) {}
* Starting Hudson 1.312, this method is also invoked for the master, not just for agents.
*
* @param listener
* This is connected to the launch log of the computer.
* This is connected to the launch log of the computer or Jenkins master.
* Since this method is called synchronously from the thread
* that launches a computer, if this method performs a time-consuming
* operation, this listener should be notified of the progress.
@@ -957,11 +957,25 @@ protected void doRun() throws Exception {

updateComputerList();

{// master is online now
Computer c = toComputer();
if(c!=null)
for (ComputerListener cl : ComputerListener.all())
cl.onOnline(c, new LogTaskListener(LOGGER, INFO));
{// master is online now, it's instance must always exist
final Computer c = toComputer();
if(c != null) {
for (ComputerListener cl : ComputerListener.all()) {
try {
cl.onOnline(c, new LogTaskListener(LOGGER, INFO));
} catch (Throwable t) {
if (t instanceof Error) {
// We propagate Runtime errors, because they are fatal.
throw t;
}

// Other exceptions should be logged instead of failing the Jenkins startup (See listener's Javadoc)
// We also throw it for InterruptedException since it's what is expected according to the javadoc
LOGGER.log(SEVERE, String.format("Invocation of the computer listener %s failed for the Jenkins master node",
cl.getClass()), t);
}
}
}
}

for (ItemListener l : ItemListener.all()) {
@@ -42,6 +42,7 @@

import hudson.maven.MavenModuleSet;
import hudson.maven.MavenModuleSetBuild;
import hudson.model.Computer;
import hudson.model.Failure;
import hudson.model.RestartListener;
import hudson.model.RootAction;
@@ -51,6 +52,7 @@
import hudson.security.HudsonPrivateSecurityRealm;
import hudson.util.HttpResponses;
import hudson.model.FreeStyleProject;
import hudson.model.TaskListener;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import hudson.security.LegacySecurityRealm;
import hudson.security.Permission;
@@ -70,6 +72,7 @@
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;

@@ -452,4 +455,20 @@ public void runScriptOnOfflineComputer() throws Exception {
assertThat(rsp.getContentAsString(), containsString("Node is offline"));
assertThat(rsp.getStatusCode(), equalTo(404));
}

@Test
@Issue("JENKINS-38487")
public void startupShouldNotFailOnFailingOnlineListener() {
// We do nothing, FailingOnOnlineListener & JenkinsRule should cause the
// boot failure if the issue is not fixed.
}

@TestExtension(value = "startupShouldNotFailOnFailingOnlineListener")
public static final class FailingOnOnlineListener extends ComputerListener {

@Override
public void onOnline(Computer c, TaskListener listener) throws IOException, InterruptedException {
throw new IOException("Something happened (the listener always throws this exception)");
}
}
}

0 comments on commit b104407

Please sign in to comment.
You can’t perform that action at this time.