| I had kind of forgotten about this issue, because we've been using a RunListener to work around it, similar to the one I had attached already, but if anyone is interested, the report is still relevant (just checked with ssh-agent plugin code from master, and Jenkins 2.190.2). Here is a failing test case one can try, to be added in SSHAgentBuildWrapperTest:
@Issue("JENKINS-43889")
@Test
public void sshAgentStoppedOnEarlyBuildFailure() throws Exception {
List<String> credentialIds = new ArrayList<String>();
credentialIds.add(CREDENTIAL_ID);
SSHUserPrivateKey key = new BasicSSHUserPrivateKey(CredentialsScope.GLOBAL, credentialIds.get(0), "cloudbees",
new BasicSSHUserPrivateKey.DirectEntryPrivateKeySource(getPrivateKey()), "cloudbees", "test");
SystemCredentialsProvider.getInstance().getCredentials().add(key);
SystemCredentialsProvider.getInstance().save();
FreeStyleProject job = r.createFreeStyleProject("I_will_die_during_SCM_checkout");
job.setAssignedNode(r.createSlave());
SSHAgentBuildWrapper sshAgent = new SSHAgentBuildWrapper(credentialIds, false);
job.getBuildWrappersList().add(sshAgent);
// make sure this job fails during SCM checkout
job.setScm(new FailingSCM());
Future<? extends FreeStyleBuild> build = job.scheduleBuild2(0);
r.assertBuildStatus(Result.FAILURE, build);
r.assertLogContains(Messages.SSHAgentBuildWrapper_Started(), build.get());
r.assertLogContains(Messages.SSHAgentBuildWrapper_Stopped(), build.get());
}
static class FailingSCM extends SCM {
@Override
public ChangeLogParser createChangeLogParser() {
return null;
}
// default implementation of checkout(...) method will fail, that's what we want
}
(you will then have come `ssh-agent` processes to kill after running this test)
I'm still not sure where this should get fixed:
- either in core, by moving the Environment.tearDown calls up from BuildExecution.doRun to AbstractBuildExecution.run
- or in the ssh-agent plugin itself, if what it does (ie., adding an Environment to the build from its BuildWrapper.preCheckout implementation, rather than from BuildWrapper.setUp, so that its already set up during SCM checkout) is really bad/unsupported
|
