Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add SSH and SCP command utility and refactor shell utility #10658

Merged
merged 22 commits into from
Jan 7, 2020
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
203 changes: 199 additions & 4 deletions core/common/src/main/java/alluxio/util/ShellUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.io.InputStreamReader;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand Down Expand Up @@ -135,8 +136,8 @@ private static UnixMountInfo.Options parseUnixMountOptions(String line) {
}

@NotThreadSafe
private static final class Command {
private String[] mCommand;
private static class Command {
jiacheliu3 marked this conversation as resolved.
Show resolved Hide resolved
protected String[] mCommand;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to protected to be inherited


private Command(String[] execString) {
mCommand = execString.clone();
Expand All @@ -148,7 +149,7 @@ private Command(String[] execString) {
* @return the output
* @throws ExitCodeException if the command returns a non-zero exit code
*/
private String run() throws ExitCodeException, IOException {
protected String run() throws ExitCodeException, IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Enable inheritance

Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start();

BufferedReader inReader =
Expand Down Expand Up @@ -193,6 +194,101 @@ private String run() throws ExitCodeException, IOException {
process.destroy();
}
}

/**
* Runs a command and returns its stdout, stderr and exit code.
* No matter it succeeds or not.
*
* @return {@link CommandReturn} object representation of stdout, stderr and exit code
*/
protected CommandReturn runTolerateFailure() throws IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method is similar to run().

The differences are:

  1. Not redirecting stderr to stdout.
  2. Keeping exitCode, stderr and stdout separated and wrapped in an object CommandReturn.
  3. Not throwing exception when exitcode is non-zero. Return all information instead.

Process process = new ProcessBuilder(mCommand).start();

BufferedReader inReader =
new BufferedReader(new InputStreamReader(process.getInputStream()));
BufferedReader errReader =
new BufferedReader(new InputStreamReader(process.getErrorStream()));

try {
// read the output of the command
StringBuilder stdout = new StringBuilder();
String outLine = inReader.readLine();
while (outLine != null) {
stdout.append(outLine);
stdout.append("\n");
outLine = inReader.readLine();
}

// read the stderr of the command
StringBuilder stderr = new StringBuilder();
String errLine = errReader.readLine();
jiacheliu3 marked this conversation as resolved.
Show resolved Hide resolved
while (errLine != null) {
stderr.append(errLine);
stderr.append("\n");
errLine = inReader.readLine();
}

// wait for the process to finish and check the exit code
int exitCode = process.waitFor();
if (exitCode != 0) {
// log error instead of throwing exception
LOG.warn(String.format("Failed to run command %s%nExit Code: %s%nStderr: %s",
Arrays.toString(mCommand), exitCode, stderr.toString()));
}

return new CommandReturn(exitCode, stdout.toString(), stderr.toString());
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IOException(e);
} finally {
try {
// JDK 7 tries to automatically drain the input streams for us
// when the process exits, but since close is not synchronized,
// it creates a race if we close the stream first and the same
// fd is recycled. the stream draining thread will attempt to
// drain that fd!! it may block, OOM, or cause bizarre behavior
// see: https://bugs.openjdk.java.net/browse/JDK-8024521
// issue is fixed in build 7u60
InputStream stdoutStream = process.getInputStream();

synchronized (stdoutStream) {
inReader.close();
}
} catch (IOException e) {
LOG.warn("Error while closing the input stream", e);
}
try {
InputStream stderrStream = process.getErrorStream();
synchronized (stderrStream) {
errReader.close();
}
} catch (IOException e) {
LOG.warn("Error while closing the error stream", e);
}
process.destroy();
}
}
}

@NotThreadSafe
private static class SshCommand extends Command {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This SshCommand automatically does SSH with common parameters. I currently don't see the needs for flexible configuration yet.

protected String mHostName;

private SshCommand(String[] execString) {
this("localhost", execString);
jiacheliu3 marked this conversation as resolved.
Show resolved Hide resolved
}

private SshCommand(String hostname, String[] execString) {
super(execString);
mHostName = hostname;
mCommand = new String[]{"bash", "-c",
String.format("ssh %s %s %s", ShellUtils.COMMON_SSH_OPTS, hostname,
String.join(" ", execString))};
}

protected String getHostName() {
return mHostName;
}
}

/**
Expand Down Expand Up @@ -234,14 +330,113 @@ public String toString() {
}

/**
* Static method to execute a shell command.
* Object representation of a command execution.
*/
public static class CommandReturn {
jiacheliu3 marked this conversation as resolved.
Show resolved Hide resolved
private int mExitCode;
private String mStdOut;
private String mStdErr;

/**
* Creates object from the contents.
*
* @param code exit code
* @param stdOut stdout content
* @param stdErr stderr content
*/
public CommandReturn(int code, String stdOut, String stdErr) {
mExitCode = code;
mStdOut = stdOut;
mStdErr = stdErr;
}

/**
* Gets the stdout content.
*
* @return stdout content
*/
public String getStdOut() {
return mStdOut;
}

/**
* Gets the stderr content.
*
* @return stderr content
*/
public String getStdErr() {
return mStdErr;
}

/**
* Gets the exit code.
*
* @return exit code of execution
*/
public int getExitCode() {
return mExitCode;
}

/**
* Formats the object to more readable format.
* This is not done in toString() because stdout and stderr may be long.
*
* @return pretty formatted output
*/
public String getFormattedOutput() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not overriding toString() cuz the stdout and stderr may have a good amount of info. I don't want people to unconsciously execute a lot of commands and print everything.

return String.format("StatusCode:%s%nStdOut:%n%s%nStdErr:%n%s", getExitCode(),
getStdOut(), getStdErr());
}
}

/**
* Static method to execute a shell command. The StdErr is not returned.
* If execution failed
*
* @param cmd shell command to execute
* @return the output of the executed command
* @throws IOException in various situations:
* 1. {@link ExitCodeException} when exit code is non-zero
* 2. when the executable is not valid, i.e. running ls in Windows
* 3. execution interrupted
* 4. other normal reasons for IOException
*/
public static String execCommand(String... cmd) throws IOException {
return new Command(cmd).run();
}

/**
* Static method to execute a shell command and tolerate non-zero exit code.
* Preserve exit code, stderr and stdout in object.
*
* @param cmd shell command to execute
* @return the output of the executed command
* @throws IOException in various situations:
* 1. when the executable is not valid, i.e. running ls in Windows
* 2. execution interrupted
* 3. other normal reasons for IOException
*/
public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOException {
jiacheliu3 marked this conversation as resolved.
Show resolved Hide resolved
return new Command(cmd).runTolerateFailure();
}

/**
* Static method to execute a shell command remotely via ssh.
* Preserve exit code, stderr and stdout in object.
* SSH must be password-less.
*
* @param hostname Hostname where the command should execute
* @param cmd shell command to execute
* @return the output of the executed command
* @throws IOException in various situations:
* 1. when the executable is not valid, i.e. running ls in Windows
* 2. execution interrupted
* 3. other normal reasons for IOException
*/
public static CommandReturn sshExecCommandTolerateFailure(String hostname, String... cmd)
throws IOException {
return new SshCommand(hostname, cmd).runTolerateFailure();
}

private ShellUtils() {} // prevent instantiation
}
86 changes: 86 additions & 0 deletions core/common/src/test/java/alluxio/util/ShellUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,33 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

import alluxio.AlluxioTestDirectory;
import alluxio.Constants;

import com.google.common.base.Optional;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.io.File;
import java.io.IOException;
import java.nio.file.Paths;
import java.util.List;

/**
* Tests the {@link ShellUtils} class.
*/
public final class ShellUtilsTest {

@Rule
public ExpectedException mExceptionRule = ExpectedException.none();

public static File createFileInDir(File dir, String fileName) throws IOException {
File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString());
newFile.createNewFile();
return newFile;
}

/**
* Tests the {@link ShellUtils#execCommand(String...)} method.
*
Expand All @@ -41,6 +56,15 @@ public void execCommand() throws Exception {
assertEquals(testString + "\n", result);
}

@Test
public void execCommandFail() throws Exception {
String testString = "false";
mExceptionRule.expect(ShellUtils.ExitCodeException.class);
// run a command that guarantees to fail
String result = ShellUtils.execCommand("bash", "-c", " " + testString);
assertEquals(testString + "\n", result);
}

/**
* Tests the {@link ShellUtils#execCommand(String...)} method for a group of commands.
*
Expand Down Expand Up @@ -110,4 +134,66 @@ public void getMountInfo() throws Exception {
List<UnixMountInfo> info = ShellUtils.getUnixMountInfo();
assertTrue(info.size() > 0);
}

@Test
public void execCommandTolerateFailure() throws Exception {
// create temp file
File testDir = AlluxioTestDirectory.createTemporaryDirectory("command");

File testFile = createFileInDir(testDir, "testFile");

// ls temp file
String[] testCommandSucceed = new String[]{"ls",
String.format("%s", testDir.getAbsolutePath())};
ShellUtils.CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed);
assertEquals(0, crs.getExitCode());
assertTrue(crs.getStdOut().contains(testFile.getName()));

// do sth wrong
String[] testCommandFail = new String[]{"ls",
String.format("%saaaa", testDir.getAbsolutePath())};
ShellUtils.CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail);
System.out.println(crf.getFormattedOutput());
assertFalse(crf.getExitCode() == 0);

// if there's no such command there will be IOException
mExceptionRule.expect(IOException.class);
mExceptionRule.expectMessage("No such file or directory");
String[] testCommandExcept = new String[]{"lsa",
String.format("%s", testDir.getAbsolutePath())};
// lsa is not a valid executable
ShellUtils.execCommandTolerateFailure(testCommandExcept);
}

@Test
public void sshExecuteCommandTolerateFailure() throws Exception {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's worth debating how/whether we want to test SSH. Currently this relies on password-less loopback access.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems Jenkins doesn't have password-less SSH to localhost. So either we manual test SSH, or bring in a 3rd part implementation of invoking SSH calls.

// create temp file
File testDir = AlluxioTestDirectory.createTemporaryDirectory("command");

File testFile = createFileInDir(testDir, "testFile");

// the temp file is found
String[] testCommandSucceed = new String[]{"ls",
String.format("%s", testDir.getAbsolutePath())};
ShellUtils.CommandReturn crs = ShellUtils.sshExecCommandTolerateFailure("localhost",
testCommandSucceed);
assertEquals(0, crs.getExitCode());
assertTrue(crs.getStdOut().contains(testFile.getName()));

// do sth wrong
String[] testCommandFail = new String[]{"ls",
String.format("%saaaa", testDir.getAbsolutePath())};
ShellUtils.CommandReturn crf = ShellUtils.sshExecCommandTolerateFailure("localhost",
testCommandFail);
assertFalse(crf.getExitCode() == 0);

// if there's no such command there will be IOException
String[] testCommandExcept = new String[]{"lsa",
String.format("%s", testDir.getAbsolutePath())};
mExceptionRule.expect(IOException.class);
mExceptionRule.expectMessage("No such file or directory");
// lsa is not a valid executable
ShellUtils.CommandReturn cre = ShellUtils.sshExecCommandTolerateFailure("localhost",
testCommandExcept);
}
}