Skip to content

Commit

Permalink
Running oc with Launcher to avoid command explansion
Browse files Browse the repository at this point in the history 
This PR reimplements the mechanism to run `oc` command on a slave.

When using OpenShift Pipeline DSL, `oc` process is started with `DurableTask`,
which is designed to run a bash script or batch script on a slave.
This is not ideal because when the command arguments include special characters
(like whitespaces, `#`, `!`, `;`), it's hard to escape them correctly.

The new implementation uses standard Jenkins API `Launcher` to start a `oc` process
on a slave. Polling `stdout` and `stderr` outputs are not needed in this approach
because `stdout` and `stderr` streams are piped to master so that
they can be consumed by blocking readers.
  • Loading branch information
@vfreex
vfreex committed Sep 4, 2018
1 parent abfb53f commit 500a32e
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 262 deletions.
5 changes: 0 additions & 5 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,11 +115,6 @@
<artifactId>workflow-cps</artifactId>
<version>2.17</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>durable-task</artifactId>
<version>1.12</version>
</dependency>
<dependency>
<groupId>com.google.code.findbugs</groupId>
<artifactId>annotations</artifactId>
Expand Down
183 changes: 43 additions & 140 deletions src/main/java/com/openshift/jenkins/plugins/pipeline/OcAction.java
View file
Original file line number Diff line number Diff line change
@@ -1,17 +1,12 @@
package com.openshift.jenkins.plugins.pipeline;

import com.openshift.jenkins.plugins.util.ClientCommandBuilder;
import com.openshift.jenkins.plugins.util.QuietTaskListenerFactory;
import com.openshift.jenkins.plugins.util.ClientCommandRunner;
import hudson.*;
import hudson.model.Computer;
import hudson.model.Executor;
import hudson.model.Run;
import hudson.model.TaskListener;
import org.apache.commons.io.IOUtils;
import org.jenkinsci.plugins.durabletask.BourneShellScript;
import org.jenkinsci.plugins.durabletask.Controller;
import org.jenkinsci.plugins.durabletask.DurableTask;
import org.jenkinsci.plugins.durabletask.WindowsBatchScript;
import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.Whitelisted;
import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl;
import org.jenkinsci.plugins.workflow.steps.AbstractStepImpl;
Expand All @@ -20,13 +15,11 @@
import org.kohsuke.stapler.DataBoundConstructor;

import javax.inject.Inject;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.Serializable;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.ExecutionException;

public class OcAction extends AbstractStepImpl {

Expand All @@ -52,32 +45,6 @@ public OcAction(String server, String project, boolean skipTLSVerify, String caP
: reference;
}

public static Integer exitStatusRaceConditionBugWorkaround(Controller dtc,
FilePath filePath, Launcher launcher) throws InterruptedException,
IOException {
for (int tries = 30; tries > 0; tries--) {
try {
// exitStatus can throw an IOException (reporting a
// NumberFormatException) if the PID file has been created but
// not
// populated, or if the PID file has not yet been created. Make
// sure it stops throwing this exception before continuing.
return dtc.exitStatus(filePath, launcher);
} catch (IOException ioe) {
if (tries == 1) {
throw ioe;
}
Thread.sleep(125);
}
}
launcher.getListener()
.getLogger()
.println(
"After 30 retries, unable to get exit status for "
+ filePath.toURI().toString());
return -1;
}

public static class OcActionResult implements Serializable {

@Whitelisted
Expand Down Expand Up @@ -173,34 +140,18 @@ public static class Execution extends
@StepContextParameter
private transient Computer computer;

private boolean firstPrint = true;

private void printToConsole(String s) {
private void printToConsole(String line) {
final String prefix = "[" + step.streamStdOutToConsolePrefix + "] ";
if (firstPrint) {
listener.getLogger().print(prefix);
firstPrint = false;
}
listener.getLogger().print(s.replace("\n", "\n" + prefix));
listener.getLogger().print(prefix);
listener.getLogger().println(line);
listener.getLogger().flush();
}

@Override
protected OcActionResult run() throws Exception {

String commandString = step.cmdBuilder.asString(false);
String redactedCommandString = step.cmdBuilder.asString(true);

FilePath stdoutTmp = filePath.createTextTempFile("ocstdout",
".txt", "", false);
FilePath stderrTmp = filePath.createTextTempFile("ocstderr",
".txt", "", false);
commandString += " >> " + stdoutTmp.getRemote() + " 2>> "
+ stderrTmp.getRemote();

protected OcActionResult run() throws IOException, InterruptedException, ExecutionException {
if (step.streamStdOutToConsolePrefix != null
&& step.streamStdOutToConsolePrefix
.startsWith("start-build")) {
.startsWith("start-build")) {
listener.getLogger()
.println(
"NOTE: the selector returned when -F/--follow is supplied to startBuild() will be inoperative for the various selector operations.");
Expand All @@ -209,91 +160,43 @@ protected OcActionResult run() throws Exception {
"Consider removing those options from startBuild and using the logs() command to follow the build output.");
}

try {
final DurableTask task;
if (launcher.isUnix()) {
task = new BourneShellScript(commandString);
} else {
task = new WindowsBatchScript(commandString);
}

// Without this intervention, Durable task logs some extraneous
// details I don't want appearing in the console
// e.g. "[_OcAction] Running shell script"
QuietTaskListenerFactory.QuietTasklistener quiet = QuietTaskListenerFactory
.build(listener);
Controller dtc = task
.launch(envVars, filePath, launcher, quiet);

ByteArrayOutputStream stdErr = new ByteArrayOutputStream();
ByteArrayOutputStream stdOut = new ByteArrayOutputStream();

long reCheckSleep = 250;
Integer exitStatus;
while ((exitStatus = exitStatusRaceConditionBugWorkaround(dtc,
filePath, launcher)) == null) {
Thread.sleep(reCheckSleep);
byte[] newOutput;
try (InputStream is = stdoutTmp.readFromOffset(stdOut
.size())) {
newOutput = IOUtils.toByteArray(is);
}
stdOut.write(newOutput);
if (newOutput.length > 0
&& step.streamStdOutToConsolePrefix != null) {
printToConsole(new String(newOutput,
StandardCharsets.UTF_8));
// If we are streaming to console and getting output,
// keep sleep duration small.
reCheckSleep = 1000;
continue;
filePath.mkdirs();
List<String> ocCommand = step.cmdBuilder.buildCommand(false);

final StringBuffer stdout = new StringBuffer();
final StringBuffer stderr = new StringBuffer();

ClientCommandRunner runner = new ClientCommandRunner(ocCommand, filePath, launcher,
// stdout
new ClientCommandRunner.OutputObserver() {
@Override
public boolean onReadLine(String line) {
stdout.append(line + '\n');
if (step.streamStdOutToConsolePrefix != null) {
printToConsole(line);
}
return false;
}
},
// stderr
new ClientCommandRunner.OutputObserver() {
@Override
public boolean onReadLine(String line) {
stderr.append(line + '\n');
listener.getLogger().println(line);
return false;
}
}
if (reCheckSleep < 10000) { // Gradually check less
// frequently for slow execution
// tasks
reCheckSleep *= 1.2f;
}
}

byte[] newOutput;
try (InputStream is = stdoutTmp.readFromOffset(stdOut.size())) {
newOutput = IOUtils.toByteArray(is);
}
stdOut.write(newOutput);
if (step.streamStdOutToConsolePrefix != null) {
printToConsole(new String(newOutput, StandardCharsets.UTF_8));
listener.getLogger().println(); // final newline if output
// does not contain it.
}

try (InputStream is = stderrTmp.read()) {
stdErr.write(IOUtils.toByteArray(is));
}

OcActionResult result = new OcActionResult();
result.verb = step.cmdBuilder.verb;
result.cmd = redactedCommandString;
result.status = exitStatus.intValue();
result.out = stdOut.toString("UTF-8").trim();
result.err = stdErr.toString("UTF-8").trim();
result.reference = step.reference;

if (step.verbose) {
listener.getLogger().println("Verbose sub-step output:");
listener.getLogger().println("\tCommand> " + result.cmd);
listener.getLogger().println("\tStatus> " + result.status);
listener.getLogger().println("\tStdOut>" + result.out);
listener.getLogger().println("\tStdErr> " + result.err);
listener.getLogger().println(
"\tReference> " + result.reference);
}

dtc.cleanup(filePath);
return result;
} finally {
stdoutTmp.delete();
stderrTmp.delete();
}
);

OcActionResult result = new OcActionResult();
result.verb = step.cmdBuilder.verb;
result.cmd = step.cmdBuilder.asString(true);
result.reference = step.reference;
result.status = runner.run();
result.out = stdout.toString();
result.err = stderr.toString();
return result;
}

}
Expand Down
Loading

0 comments on commit 500a32e

Please sign in to comment.