Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 31 additions & 16 deletions src/main/java/hudson/plugins/ec2/win/EC2WindowsLauncher.java
Copy link
Contributor Author

@apuig apuig Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mkdirProcess and initProcess errors did not appear in the logs, but for completeness, I am also proposing to handle them.

Original file line number Diff line number Diff line change
Expand Up @@ -61,54 +61,69 @@

logger.println("Creating tmp directory if it does not exist");
WindowsProcess mkdirProcess = connection.execute("if not exist " + tmpDir + " mkdir " + tmpDir);
int exitCode = mkdirProcess.waitFor();
if (exitCode != 0) {
logger.println("Creating tmpdir failed=" + exitCode);
return;
try {
int exitCode = mkdirProcess.waitFor();
if (exitCode != 0) {
logger.println("Creating tmpdir failed=" + exitCode);
return;
}
} catch (Exception e) {
mkdirProcess.destroy();
throw e;
}

if (initScript != null && !initScript.trim().isEmpty() && !connection.exists(tmpDir + ".jenkins-init")) {
logger.println("Executing init script");
try (OutputStream init = connection.putFile(tmpDir + "init.bat")) {
init.write(initScript.getBytes(StandardCharsets.UTF_8));
}

WindowsProcess initProcess = connection.execute("cmd /c " + tmpDir + "init.bat");
IOUtils.copy(initProcess.getStdout(), logger);
try {
IOUtils.copy(initProcess.getStdout(), logger);

int exitStatus = initProcess.waitFor();
if (exitStatus != 0) {
logger.println("init script failed: exit code=" + exitStatus);
return;
int exitStatus = initProcess.waitFor();
if (exitStatus != 0) {
logger.println("init script failed: exit code=" + exitStatus);
return;
}
} catch (Exception e) {
initProcess.destroy();
throw e;
}

try (OutputStream initGuard = connection.putFile(tmpDir + ".jenkins-init")) {
initGuard.write("init ran".getBytes(StandardCharsets.UTF_8));
}
logger.println("init script ran successfully");
}

try (OutputStream agentJar = connection.putFile(tmpDir + AGENT_JAR)) {
agentJar.write(Jenkins.get().getJnlpJars(AGENT_JAR).readFully());
}

logger.println("remoting.jar sent remotely. Bootstrapping it");

final String javaPath = node.javaPath;
final String jvmopts = node.jvmopts;
final String remoteFS = WindowsUtil.quoteArgument(node.getRemoteFS());
final String workDir = Util.fixEmptyAndTrim(remoteFS) != null ? remoteFS : tmpDir;
final String launchString = javaPath + " " + (jvmopts != null ? jvmopts : "") + " -jar " + tmpDir
+ AGENT_JAR + " -workDir " + workDir;
logger.println("Launching via WinRM:" + launchString);
final WindowsProcess process = connection.execute(launchString, 86400);
computer.setChannel(process.getStdout(), process.getStdin(), logger, new Listener() {
@Override
public void onClosed(Channel channel, IOException cause) {
process.destroy();
connection.close();
}
});
try {
computer.setChannel(process.getStdout(), process.getStdin(), logger, new Listener() {
@Override
public void onClosed(Channel channel, IOException cause) {
process.destroy();
connection.close();
}
});
} catch (Exception e) {
process.destroy();
throw e;
}

Check warning on line 126 in src/main/java/hudson/plugins/ec2/win/EC2WindowsLauncher.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 70-126 are not covered by tests
} catch (EOFException eof) {
// When we launch java with connection.execute(launchString... it keeps running, but if java is not
// installed
Expand Down
15 changes: 13 additions & 2 deletions src/main/java/hudson/plugins/ec2/win/winrm/WindowsProcess.java
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,19 @@
return;
}

client.signal();
client.deleteShell();
// Instance may already be terminated, causing WinRM operations to fail
try {
client.signal();
} catch (Exception e) {
LOGGER.log(Level.FINE, () -> "Failed to signal WinRM shell: " + e.getMessage());
}

try {
client.deleteShell();
} catch (Exception e) {
LOGGER.log(Level.FINE, () -> "Failed to delete WinRM shell: " + e.getMessage());
Comment on lines +93 to +99
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why FINE and not WARNING? We do not expect exceptions here do we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, it’s possible to call destroy on a Terminated instance. In this case, WinRM operations are expected to fail.

I’ll look into whether there’s a straightforward way to detect this status so we can adjust the log level accordingly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, WinRM operations are expected to fail.

A comment to that effect would suffice I think.

}

Check warning on line 100 in src/main/java/hudson/plugins/ec2/win/winrm/WindowsProcess.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 92-100 are not covered by tests

terminated = true;
Closeables.closeQuietly(toCallersStdout);
Closeables.closeQuietly(toCallersStdin);
Expand Down
Loading