From 3b0c9f0e85d583a4dd1ad0d28717374cfbed8cbb Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 17:01:09 -0800 Subject: [PATCH 01/22] add ssh execution --- .../main/java/alluxio/util/ShellUtils.java | 159 +++++++++++++++++- .../java/alluxio/util/ShellUtilsTest.java | 76 +++++++++ 2 files changed, 232 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 9e00f0586a39..24cb36c3ba32 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -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; @@ -135,7 +136,7 @@ private static UnixMountInfo.Options parseUnixMountOptions(String line) { } @NotThreadSafe - private static final class Command { + private static class Command { private String[] mCommand; private Command(String[] execString) { @@ -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 { + String run() throws ExitCodeException, IOException { Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); BufferedReader inReader = @@ -193,6 +194,92 @@ 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 + */ + CommandReturn runTolerateFailure() throws IOException { + Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).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(); + 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(); + InputStream stderrStream = process.getErrorStream(); + synchronized (stdoutStream) { + inReader.close(); + } + synchronized (stderrStream) { + errReader.close(); + } + } catch (IOException e) { + LOG.warn("Error while closing the input stream and error stream", e); + } + process.destroy(); + } + } + } + + @NotThreadSafe + private static class SshCommand extends Command { + private String mHostName; + private String[] mCommand; + + private SshCommand(String[] execString) { + this("localhost", execString); + } + + 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))}; + } } /** @@ -233,15 +320,81 @@ public String toString() { } } + public static class CommandReturn { + int mStatusCode; + String mStdOut; + String mStdErr; + + public CommandReturn(int code, String stdOut, String stdErr) { + mStatusCode = code; + mStdOut = stdOut; + mStdErr = stdErr; + } + + public String getStdOut() { + return mStdOut; + } + + public String getStdErr() { + return mStdErr; + } + + public int getStatusCode() { + return mStatusCode; + } + + public String getFormattedOutput() { + return String.format("StatusCode:%s\nStdOut:\n%s\nStdErr:\n%s", this.getStatusCode(), + this.getStdOut(), this.getStdErr()); + } + } + /** - * Static method to execute a shell command. + * 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 {@link 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 {@link 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 { + 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 cmd shell command to execute + * @return the output of the executed command + * @throws {@link 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 } diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 2e548bba4525..b790355a23eb 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -16,18 +16,41 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; +import alluxio.AlluxioTestDirectory; import alluxio.Constants; +import alluxio.grpc.Command; import com.google.common.base.Optional; +import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.powermock.api.mockito.PowerMockito; +import java.io.File; +import java.io.IOException; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Future; /** * Tests the {@link ShellUtils} class. */ public final class ShellUtilsTest { + @Rule + public ExpectedException exceptionRule = 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. * @@ -110,4 +133,57 @@ public void getMountInfo() throws Exception { List 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.getStatusCode()); + 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.getStatusCode() == 0); + + // if there's no such command there will be IOException + exceptionRule.expect(IOException.class); + exceptionRule.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 { + // 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.getStatusCode()); + 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.getStatusCode() == 0); + + // if there's no such command there will be IOException + String[] testCommandExcept = new String[]{"lsa", String.format("%s", testDir.getAbsolutePath())}; + exceptionRule.expect(IOException.class); + exceptionRule.expectMessage("No such file or directory"); + // lsa is not a valid executable + ShellUtils.CommandReturn cre = ShellUtils.sshExecCommandTolerateFailure("localhost", testCommandExcept); + } } From d6699500b48fc10b51ebb6ec009d22e0d2fcaf7b Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 17:15:33 -0800 Subject: [PATCH 02/22] better test and access modifier --- core/common/src/main/java/alluxio/util/ShellUtils.java | 4 ++-- .../src/test/java/alluxio/util/ShellUtilsTest.java | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 24cb36c3ba32..202c8233b645 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -149,7 +149,7 @@ private Command(String[] execString) { * @return the output * @throws ExitCodeException if the command returns a non-zero exit code */ - String run() throws ExitCodeException, IOException { + protected String run() throws ExitCodeException, IOException { Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); BufferedReader inReader = @@ -201,7 +201,7 @@ String run() throws ExitCodeException, IOException { * * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ - CommandReturn runTolerateFailure() throws IOException { + protected CommandReturn runTolerateFailure() throws IOException { Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); BufferedReader inReader = diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index b790355a23eb..59cd23c6d8e7 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -64,6 +64,15 @@ public void execCommand() throws Exception { assertEquals(testString + "\n", result); } + @Test + public void execCommandFail() throws Exception { + String testString = "false"; + exceptionRule.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. * From f6b4826dde97f9293b084c9ba53c0b85263e8119 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 17:29:16 -0800 Subject: [PATCH 03/22] checkstyle --- .../main/java/alluxio/util/ShellUtils.java | 49 +++++++++++++++---- .../java/alluxio/util/ShellUtilsTest.java | 16 ++---- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 202c8233b645..69228ee6effd 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -196,7 +196,7 @@ protected String run() throws ExitCodeException, IOException { } /** - * Runs a command and returns its stdout, stderr and exit code + * 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 @@ -278,7 +278,8 @@ 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))}; + String.format("ssh %s %s %s", ShellUtils.COMMON_SSH_OPTS, hostname, + String.join(" ", execString))}; } } @@ -320,32 +321,59 @@ public String toString() { } } + /** + * Object representation of a command execution + */ public static class CommandReturn { - int mStatusCode; - String mStdOut; - String mStdErr; + private int mExitCode; + private String mStdOut; + private String mStdErr; + /** + * Constructor + */ public CommandReturn(int code, String stdOut, String stdErr) { - mStatusCode = code; + 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; } - public int getStatusCode() { - return mStatusCode; + /** + * 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 + */ public String getFormattedOutput() { - return String.format("StatusCode:%s\nStdOut:\n%s\nStdErr:\n%s", this.getStatusCode(), - this.getStdOut(), this.getStdErr()); + return String.format("StatusCode:%s\nStdOut:\n%s\nStdErr:\n%s", getExitCode(), + getStdOut(), getStdErr()); } } @@ -385,6 +413,7 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE * 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 {@link IOException} in various situations: diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 59cd23c6d8e7..7be04721901a 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -19,23 +19,15 @@ import alluxio.AlluxioTestDirectory; import alluxio.Constants; -import alluxio.grpc.Command; import com.google.common.base.Optional; -import org.junit.Assert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; -import org.powermock.api.mockito.PowerMockito; import java.io.File; import java.io.IOException; import java.nio.file.Paths; -import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; /** * Tests the {@link ShellUtils} class. @@ -153,14 +145,14 @@ public void execCommandTolerateFailure() throws Exception { // ls temp file String[] testCommandSucceed = new String[]{"ls", String.format("%s", testDir.getAbsolutePath())}; ShellUtils.CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed); - assertEquals(0, crs.getStatusCode()); + 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.getStatusCode() == 0); + assertFalse(crf.getExitCode() == 0); // if there's no such command there will be IOException exceptionRule.expect(IOException.class); @@ -180,13 +172,13 @@ public void sshExecuteCommandTolerateFailure() throws Exception { // 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.getStatusCode()); + 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.getStatusCode() == 0); + assertFalse(crf.getExitCode() == 0); // if there's no such command there will be IOException String[] testCommandExcept = new String[]{"lsa", String.format("%s", testDir.getAbsolutePath())}; From e120069a8ba724173d852dde9b78c4f7b20be422 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 17:31:19 -0800 Subject: [PATCH 04/22] testfile checkstyle --- .../java/alluxio/util/ShellUtilsTest.java | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 7be04721901a..09058cdedcd0 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -35,7 +35,7 @@ public final class ShellUtilsTest { @Rule - public ExpectedException exceptionRule = ExpectedException.none(); + 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()); @@ -59,7 +59,7 @@ public void execCommand() throws Exception { @Test public void execCommandFail() throws Exception { String testString = "false"; - exceptionRule.expect(ShellUtils.ExitCodeException.class); + mExceptionRule.expect(ShellUtils.ExitCodeException.class); // run a command that guarantees to fail String result = ShellUtils.execCommand("bash", "-c", " " + testString); assertEquals(testString + "\n", result); @@ -143,21 +143,24 @@ public void execCommandTolerateFailure() throws Exception { File testFile = createFileInDir(testDir, "testFile"); // ls temp file - String[] testCommandSucceed = new String[]{"ls", String.format("%s", testDir.getAbsolutePath())}; + 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())}; + 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 - exceptionRule.expect(IOException.class); - exceptionRule.expectMessage("No such file or directory"); - String[] testCommandExcept = new String[]{"lsa", String.format("%s", testDir.getAbsolutePath())}; + 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); } @@ -170,21 +173,27 @@ public void sshExecuteCommandTolerateFailure() throws Exception { 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); + 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); + 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())}; - exceptionRule.expect(IOException.class); - exceptionRule.expectMessage("No such file or directory"); + 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); + ShellUtils.CommandReturn cre = ShellUtils.sshExecCommandTolerateFailure("localhost", + testCommandExcept); } } From 4afa9b824c79c6adb56c1e8fc9b3f9a0bce57f51 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 17:50:52 -0800 Subject: [PATCH 05/22] fix style --- .../main/java/alluxio/util/ShellUtils.java | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 69228ee6effd..f2c799e89c26 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -196,8 +196,8 @@ protected String run() throws ExitCodeException, IOException { } /** - * Runs a command and returns its stdout, stderr and exit code, - * no matter it succeeds or not + * 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 */ @@ -322,7 +322,7 @@ public String toString() { } /** - * Object representation of a command execution + * Object representation of a command execution. */ public static class CommandReturn { private int mExitCode; @@ -330,7 +330,11 @@ public static class CommandReturn { private String mStdErr; /** - * Constructor + * 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; @@ -339,7 +343,7 @@ public CommandReturn(int code, String stdOut, String stdErr) { } /** - * Gets the stdout content + * Gets the stdout content. * * @return stdout content */ @@ -348,7 +352,7 @@ public String getStdOut() { } /** - * Gets the stderr content + * Gets the stderr content. * * @return stderr content */ @@ -357,7 +361,7 @@ public String getStdErr() { } /** - * Gets the exit code + * Gets the exit code. * * @return exit code of execution */ @@ -369,7 +373,7 @@ public int getExitCode() { * Formats the object to more readable format. * This is not done in toString() because stdout and stderr may be long. * - * @return + * @return pretty formatted output */ public String getFormattedOutput() { return String.format("StatusCode:%s\nStdOut:\n%s\nStdErr:\n%s", getExitCode(), @@ -421,7 +425,8 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE * 2. execution interrupted * 3. other normal reasons for IOException */ - public static CommandReturn sshExecCommandTolerateFailure(String hostname, String... cmd) throws IOException { + public static CommandReturn sshExecCommandTolerateFailure(String hostname, String... cmd) + throws IOException { return new SshCommand(hostname, cmd).runTolerateFailure(); } From 546a530a1367e947863f925a0a4d1d56e060067c Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Wed, 18 Dec 2019 18:01:51 -0800 Subject: [PATCH 06/22] fix checkstyle link --- core/common/src/main/java/alluxio/util/ShellUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index f2c799e89c26..18b4df2d4e67 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -387,7 +387,7 @@ public String getFormattedOutput() { * * @param cmd shell command to execute * @return the output of the executed command - * @throws {@link IOException} in various situations: + * @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 @@ -403,7 +403,7 @@ public static String execCommand(String... cmd) throws IOException { * * @param cmd shell command to execute * @return the output of the executed command - * @throws {@link IOException} in various situations: + * @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 @@ -420,7 +420,7 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE * @param hostname Hostname where the command should execute * @param cmd shell command to execute * @return the output of the executed command - * @throws {@link IOException} in various situations: + * @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 From 49da48aabdf108514b3d4e533b187fdc1db1d2a7 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 19 Dec 2019 11:40:34 -0800 Subject: [PATCH 07/22] field modifiers --- .../src/main/java/alluxio/util/ShellUtils.java | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 18b4df2d4e67..c9b60b0598ed 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -137,7 +137,7 @@ private static UnixMountInfo.Options parseUnixMountOptions(String line) { @NotThreadSafe private static class Command { - private String[] mCommand; + protected String[] mCommand; private Command(String[] execString) { mCommand = execString.clone(); @@ -202,7 +202,7 @@ protected String run() throws ExitCodeException, IOException { * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ protected CommandReturn runTolerateFailure() throws IOException { - Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); + Process process = new ProcessBuilder(mCommand).start(); BufferedReader inReader = new BufferedReader(new InputStreamReader(process.getInputStream())); @@ -232,7 +232,7 @@ protected CommandReturn runTolerateFailure() throws IOException { 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", + LOG.warn(String.format("Failed to run command %s%nExit Code: %s%nStderr: %s", Arrays.toString(mCommand), exitCode, stderr.toString())); } @@ -267,8 +267,7 @@ protected CommandReturn runTolerateFailure() throws IOException { @NotThreadSafe private static class SshCommand extends Command { - private String mHostName; - private String[] mCommand; + protected String mHostName; private SshCommand(String[] execString) { this("localhost", execString); @@ -281,6 +280,10 @@ private SshCommand(String hostname, String[] execString) { String.format("ssh %s %s %s", ShellUtils.COMMON_SSH_OPTS, hostname, String.join(" ", execString))}; } + + protected String getHostName() { + return mHostName; + } } /** @@ -376,7 +379,7 @@ public int getExitCode() { * @return pretty formatted output */ public String getFormattedOutput() { - return String.format("StatusCode:%s\nStdOut:\n%s\nStdErr:\n%s", getExitCode(), + return String.format("StatusCode:%s%nStdOut:%n%s%nStdErr:%n%s", getExitCode(), getStdOut(), getStdErr()); } } From fdfe3c69e80e9ccf0077ed6d8f61a9f538f19789 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 19 Dec 2019 13:26:06 -0800 Subject: [PATCH 08/22] close resources properly --- core/common/src/main/java/alluxio/util/ShellUtils.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index c9b60b0598ed..577e8ecc3466 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -250,15 +250,20 @@ protected CommandReturn runTolerateFailure() throws IOException { // see: https://bugs.openjdk.java.net/browse/JDK-8024521 // issue is fixed in build 7u60 InputStream stdoutStream = process.getInputStream(); - InputStream stderrStream = process.getErrorStream(); + 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 input stream and error stream", e); + LOG.warn("Error while closing the error stream", e); } process.destroy(); } From d1c73dec9427df3030b5d22034c6eace099ea70f Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 19 Dec 2019 14:41:43 -0800 Subject: [PATCH 09/22] resolved findbug error and tests --- .../main/java/alluxio/util/ShellUtils.java | 45 ++++++------------- .../java/alluxio/util/ShellUtilsTest.java | 10 ++--- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 577e8ecc3466..ccd8f66523e0 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -203,13 +203,12 @@ protected String run() throws ExitCodeException, IOException { */ protected CommandReturn runTolerateFailure() throws IOException { Process process = new ProcessBuilder(mCommand).start(); + CommandReturn cr = null; - BufferedReader inReader = - new BufferedReader(new InputStreamReader(process.getInputStream())); - BufferedReader errReader = - new BufferedReader(new InputStreamReader(process.getErrorStream())); - - try { + try (BufferedReader inReader = + new BufferedReader(new InputStreamReader(process.getInputStream())); + BufferedReader errReader = + new BufferedReader(new InputStreamReader(process.getErrorStream()))) { // read the output of the command StringBuilder stdout = new StringBuilder(); String outLine = inReader.readLine(); @@ -236,37 +235,21 @@ protected CommandReturn runTolerateFailure() throws IOException { Arrays.toString(mCommand), exitCode, stderr.toString())); } - return new CommandReturn(exitCode, stdout.toString(), stderr.toString()); + cr = new CommandReturn(exitCode, stdout.toString(), stderr.toString()); + + // destroy the process + if (process != null) { + process.destroy(); + } } 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); + if (process != null) { + process.destroy(); } - try { - InputStream stderrStream = process.getErrorStream(); - synchronized (stderrStream) { - errReader.close(); - } - } catch (IOException e) { - LOG.warn("Error while closing the error stream", e); - } - process.destroy(); } + return cr; } } diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 09058cdedcd0..f479588ac84a 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -13,6 +13,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeTrue; @@ -153,8 +154,7 @@ public void execCommandTolerateFailure() throws Exception { 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); + assertNotEquals(0, crf.getExitCode()); // if there's no such command there will be IOException mExceptionRule.expect(IOException.class); @@ -187,13 +187,13 @@ public void sshExecuteCommandTolerateFailure() throws Exception { testCommandFail); assertFalse(crf.getExitCode() == 0); - // if there's no such command there will be IOException + // if there's no such command there will be no IOException because ssh is a valid cmd 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); + assertNotEquals(0, cre); } } From d993dffd1de8e097191ff70f180109ed06b7f922 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 19 Dec 2019 15:12:59 -0800 Subject: [PATCH 10/22] remove ssh test --- .../java/alluxio/util/ShellUtilsTest.java | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index f479588ac84a..72eeba1a3ab6 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -164,36 +164,4 @@ public void execCommandTolerateFailure() throws Exception { // lsa is not a valid executable ShellUtils.execCommandTolerateFailure(testCommandExcept); } - - @Test - public void sshExecuteCommandTolerateFailure() throws Exception { - // 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 no IOException because ssh is a valid cmd - String[] testCommandExcept = new String[]{"lsa", - String.format("%s", testDir.getAbsolutePath())}; - - // lsa is not a valid executable - ShellUtils.CommandReturn cre = ShellUtils.sshExecCommandTolerateFailure("localhost", - testCommandExcept); - assertNotEquals(0, cre); - } } From 75150bcd8924c154b672f21ecffed1162ed2fb48 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 26 Dec 2019 17:57:25 -0800 Subject: [PATCH 11/22] add scp --- .../main/java/alluxio/util/ShellUtils.java | 49 +++++++++++++++++++ .../java/alluxio/util/ShellUtilsTest.java | 7 +++ 2 files changed, 56 insertions(+) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index ccd8f66523e0..5efbecd6c4c1 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -202,6 +202,8 @@ protected String run() throws ExitCodeException, IOException { * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ protected CommandReturn runTolerateFailure() throws IOException { + System.out.println(String.join(" ", mCommand)); + Process process = new ProcessBuilder(mCommand).start(); CommandReturn cr = null; @@ -274,6 +276,35 @@ protected String getHostName() { } } + @NotThreadSafe + private static class ScpCommand extends Command { + protected String mHostName; + + private ScpCommand(String remoteHost, String fromFile, String toFile) { + this(remoteHost, fromFile, toFile, false); + } + + private ScpCommand(String remoteHost, String fromFile, String toFile, boolean isDir) { + super(new String[]{}); + + String template = "scp %s %s:%s localhost:%s"; + if (isDir) { + System.out.println("Copy dir"); + template = "scp -r %s %s:%s localhost:%s"; + } + + // copy from hostname to local + mCommand = new String[]{"bash", "-c", + String.format(template, ShellUtils.COMMON_SSH_OPTS, remoteHost, fromFile, toFile) + }; + mHostName = remoteHost; + } + + protected String getHostName() { + return mHostName; + } + } + /** * This is an IOException with exit code added. */ @@ -421,5 +452,23 @@ public static CommandReturn sshExecCommandTolerateFailure(String hostname, Strin return new SshCommand(hostname, cmd).runTolerateFailure(); } + /** + * Static method to execute an scp command to copy a remote file/dir to local. + * + * @param hostname Hostname where the command should execute + * @param fromFile File path on remote host + * @param toFile File path to copy to on localhost + * @param isDir Is the file a directory + * @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 scpCommandTolerateFailure(String hostname, String fromFile, String toFile, boolean isDir) + throws IOException { + return new ScpCommand(hostname, fromFile, toFile, isDir).runTolerateFailure(); + } + private ShellUtils() {} // prevent instantiation } diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 72eeba1a3ab6..dfe2bef10ca7 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -28,6 +28,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Paths; +import java.util.Arrays; import java.util.List; /** @@ -44,6 +45,12 @@ public static File createFileInDir(File dir, String fileName) throws IOException return newFile; } + public static File createDirInDir(File dir, String fileName) throws IOException { + File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); + newFile.mkdir(); + return newFile; + } + /** * Tests the {@link ShellUtils#execCommand(String...)} method. * From 05c497627054b9a4ad312eddd977765bc7a683b1 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 26 Dec 2019 18:06:31 -0800 Subject: [PATCH 12/22] resolve checkstyle error --- core/common/src/main/java/alluxio/util/ShellUtils.java | 4 ++-- core/common/src/test/java/alluxio/util/ShellUtilsTest.java | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 5efbecd6c4c1..72d073a1ed17 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -465,8 +465,8 @@ public static CommandReturn sshExecCommandTolerateFailure(String hostname, Strin * 2. execution interrupted * 3. other normal reasons for IOException */ - public static CommandReturn scpCommandTolerateFailure(String hostname, String fromFile, String toFile, boolean isDir) - throws IOException { + public static CommandReturn scpCommandTolerateFailure( + String hostname, String fromFile, String toFile, boolean isDir) throws IOException { return new ScpCommand(hostname, fromFile, toFile, isDir).runTolerateFailure(); } diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index dfe2bef10ca7..f90ce9904126 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -28,7 +28,6 @@ import java.io.File; import java.io.IOException; import java.nio.file.Paths; -import java.util.Arrays; import java.util.List; /** From 25ca2ab49230eebecc0d2588d2a98874078d8463 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 26 Dec 2019 20:04:23 -0800 Subject: [PATCH 13/22] remove print --- core/common/src/main/java/alluxio/util/ShellUtils.java | 3 --- core/common/src/test/java/alluxio/util/ShellUtilsTest.java | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 72d073a1ed17..c1ac77f4d1ad 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -202,8 +202,6 @@ protected String run() throws ExitCodeException, IOException { * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ protected CommandReturn runTolerateFailure() throws IOException { - System.out.println(String.join(" ", mCommand)); - Process process = new ProcessBuilder(mCommand).start(); CommandReturn cr = null; @@ -289,7 +287,6 @@ private ScpCommand(String remoteHost, String fromFile, String toFile, boolean is String template = "scp %s %s:%s localhost:%s"; if (isDir) { - System.out.println("Copy dir"); template = "scp -r %s %s:%s localhost:%s"; } diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index f90ce9904126..7aea65ca52c9 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -155,12 +155,14 @@ public void execCommandTolerateFailure() throws Exception { ShellUtils.CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed); assertEquals(0, crs.getExitCode()); assertTrue(crs.getStdOut().contains(testFile.getName())); + assertEquals("", crs.getStdErr()); // do sth wrong String[] testCommandFail = new String[]{"ls", String.format("%saaaa", testDir.getAbsolutePath())}; ShellUtils.CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail); assertNotEquals(0, crf.getExitCode()); + assertTrue(crf.getStdErr().length() > 0); // if there's no such command there will be IOException mExceptionRule.expect(IOException.class); From a97b5e81dddcd1953ac9011f3c3dfe81a30e710f Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 15:49:16 -0800 Subject: [PATCH 14/22] break inner classes into concrete classes --- .../java/alluxio/shell/CommandReturn.java | 72 ++++++ .../main/java/alluxio/shell/ScpCommand.java | 44 ++++ .../main/java/alluxio/shell/ShellCommand.java | 145 +++++++++++ .../main/java/alluxio/shell/SshCommand.java | 34 +++ .../main/java/alluxio/util/ShellUtils.java | 235 +----------------- .../java/alluxio/shell/ShellCommandTest.java | 119 +++++++++ .../java/alluxio/util/ShellUtilsTest.java | 43 ---- 7 files changed, 420 insertions(+), 272 deletions(-) create mode 100644 core/common/src/main/java/alluxio/shell/CommandReturn.java create mode 100644 core/common/src/main/java/alluxio/shell/ScpCommand.java create mode 100644 core/common/src/main/java/alluxio/shell/ShellCommand.java create mode 100644 core/common/src/main/java/alluxio/shell/SshCommand.java create mode 100644 core/common/src/test/java/alluxio/shell/ShellCommandTest.java diff --git a/core/common/src/main/java/alluxio/shell/CommandReturn.java b/core/common/src/main/java/alluxio/shell/CommandReturn.java new file mode 100644 index 000000000000..ce8922a804c2 --- /dev/null +++ b/core/common/src/main/java/alluxio/shell/CommandReturn.java @@ -0,0 +1,72 @@ +/* + * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 + * (the "License"). You may not use this work except in compliance with the License, which is + * available at www.apache.org/licenses/LICENSE-2.0 + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied, as more fully set forth in the License. + * + * See the NOTICE file distributed with this work for information regarding copyright ownership. + */ + +package alluxio.shell; + +/** + * Object representation of a command execution. + */ +public class CommandReturn { + 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() { + return String.format("StatusCode:%s%nStdOut:%n%s%nStdErr:%n%s", getExitCode(), + getStdOut(), getStdErr()); + } +} diff --git a/core/common/src/main/java/alluxio/shell/ScpCommand.java b/core/common/src/main/java/alluxio/shell/ScpCommand.java new file mode 100644 index 000000000000..424b0451d03c --- /dev/null +++ b/core/common/src/main/java/alluxio/shell/ScpCommand.java @@ -0,0 +1,44 @@ +/* + * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 + * (the "License"). You may not use this work except in compliance with the License, which is + * available at www.apache.org/licenses/LICENSE-2.0 + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied, as more fully set forth in the License. + * + * See the NOTICE file distributed with this work for information regarding copyright ownership. + */ + +package alluxio.shell; + +import alluxio.util.ShellUtils; + +import javax.annotation.concurrent.NotThreadSafe; + +@NotThreadSafe +public class ScpCommand extends ShellCommand { + protected String mHostName; + + private ScpCommand(String remoteHost, String fromFile, String toFile) { + this(remoteHost, fromFile, toFile, false); + } + + public ScpCommand(String remoteHost, String fromFile, String toFile, boolean isDir) { + super(new String[]{}); + + String template = "scp %s %s:%s localhost:%s"; + if (isDir) { + template = "scp -r %s %s:%s localhost:%s"; + } + + // copy from hostname to local + mCommand = new String[]{"bash", "-c", + String.format(template, ShellUtils.COMMON_SSH_OPTS, remoteHost, fromFile, toFile) + }; + mHostName = remoteHost; + } + + protected String getHostName() { + return mHostName; + } +} diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java new file mode 100644 index 000000000000..d0a188c8d1c5 --- /dev/null +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -0,0 +1,145 @@ +/* + * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 + * (the "License"). You may not use this work except in compliance with the License, which is + * available at www.apache.org/licenses/LICENSE-2.0 + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied, as more fully set forth in the License. + * + * See the NOTICE file distributed with this work for information regarding copyright ownership. + */ + +package alluxio.shell; + +import alluxio.util.ShellUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.annotation.concurrent.NotThreadSafe; +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.nio.charset.Charset; +import java.util.Arrays; + +@NotThreadSafe +public class ShellCommand { + private static final Logger LOG = LoggerFactory.getLogger(ShellCommand.class); + + protected String[] mCommand; + + public ShellCommand(String[] execString) { + mCommand = execString.clone(); + } + + /** + * Runs a command and returns its stdout on success. + * + * @return the output + * @throws ShellUtils.ExitCodeException if the command returns a non-zero exit code + */ + public String run() throws ShellUtils.ExitCodeException, IOException { + Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); + + BufferedReader inReader = + new BufferedReader(new InputStreamReader(process.getInputStream(), + Charset.defaultCharset())); + + try { + // read the output of the command + StringBuilder output = new StringBuilder(); + String line = inReader.readLine(); + while (line != null) { + output.append(line); + output.append("\n"); + line = inReader.readLine(); + } + // wait for the process to finish and check the exit code + int exitCode = process.waitFor(); + if (exitCode != 0) { + throw new ShellUtils.ExitCodeException(exitCode, output.toString()); + } + return output.toString(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } finally { + // close the input stream + 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 stdout = process.getInputStream(); + synchronized (stdout) { + inReader.close(); + } + } catch (IOException e) { + LOG.warn("Error while closing the input stream", e); + } + 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 + */ + public CommandReturn runTolerateFailure() throws IOException { + Process process = new ProcessBuilder(mCommand).start(); + CommandReturn cr = null; + + try (BufferedReader inReader = + new BufferedReader(new InputStreamReader(process.getInputStream())); + BufferedReader errReader = + new BufferedReader(new InputStreamReader(process.getErrorStream()))) { + // 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(); + 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())); + } + + cr = new CommandReturn(exitCode, stdout.toString(), stderr.toString()); + + // destroy the process + if (process != null) { + process.destroy(); + } + + return cr; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException(e); + } finally { + if (process != null) { + process.destroy(); + } + } + } +} diff --git a/core/common/src/main/java/alluxio/shell/SshCommand.java b/core/common/src/main/java/alluxio/shell/SshCommand.java new file mode 100644 index 000000000000..043a2b0035e1 --- /dev/null +++ b/core/common/src/main/java/alluxio/shell/SshCommand.java @@ -0,0 +1,34 @@ +/* + * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 + * (the "License"). You may not use this work except in compliance with the License, which is + * available at www.apache.org/licenses/LICENSE-2.0 + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied, as more fully set forth in the License. + * + * See the NOTICE file distributed with this work for information regarding copyright ownership. + */ + +package alluxio.shell; + +import alluxio.util.ShellUtils; + +import javax.annotation.concurrent.NotThreadSafe; + +@NotThreadSafe +public class SshCommand extends ShellCommand { + protected String mHostName; + + public 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; + } +} + diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index c1ac77f4d1ad..12ab561fabcb 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -11,6 +11,10 @@ package alluxio.util; +import alluxio.shell.CommandReturn; +import alluxio.shell.ScpCommand; +import alluxio.shell.ShellCommand; +import alluxio.shell.SshCommand; import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -135,173 +139,6 @@ private static UnixMountInfo.Options parseUnixMountOptions(String line) { return builder.build(); } - @NotThreadSafe - private static class Command { - protected String[] mCommand; - - private Command(String[] execString) { - mCommand = execString.clone(); - } - - /** - * Runs a command and returns its stdout on success. - * - * @return the output - * @throws ExitCodeException if the command returns a non-zero exit code - */ - protected String run() throws ExitCodeException, IOException { - Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); - - BufferedReader inReader = - new BufferedReader(new InputStreamReader(process.getInputStream(), - Charset.defaultCharset())); - - try { - // read the output of the command - StringBuilder output = new StringBuilder(); - String line = inReader.readLine(); - while (line != null) { - output.append(line); - output.append("\n"); - line = inReader.readLine(); - } - // wait for the process to finish and check the exit code - int exitCode = process.waitFor(); - if (exitCode != 0) { - throw new ExitCodeException(exitCode, output.toString()); - } - return output.toString(); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException(e); - } finally { - // close the input stream - 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 stdout = process.getInputStream(); - synchronized (stdout) { - inReader.close(); - } - } catch (IOException e) { - LOG.warn("Error while closing the input stream", e); - } - 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 { - Process process = new ProcessBuilder(mCommand).start(); - CommandReturn cr = null; - - try (BufferedReader inReader = - new BufferedReader(new InputStreamReader(process.getInputStream())); - BufferedReader errReader = - new BufferedReader(new InputStreamReader(process.getErrorStream()))) { - // 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(); - 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())); - } - - cr = new CommandReturn(exitCode, stdout.toString(), stderr.toString()); - - // destroy the process - if (process != null) { - process.destroy(); - } - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException(e); - } finally { - if (process != null) { - process.destroy(); - } - } - return cr; - } - } - - @NotThreadSafe - private static class SshCommand extends Command { - protected String mHostName; - - private SshCommand(String[] execString) { - this("localhost", execString); - } - - 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; - } - } - - @NotThreadSafe - private static class ScpCommand extends Command { - protected String mHostName; - - private ScpCommand(String remoteHost, String fromFile, String toFile) { - this(remoteHost, fromFile, toFile, false); - } - - private ScpCommand(String remoteHost, String fromFile, String toFile, boolean isDir) { - super(new String[]{}); - - String template = "scp %s %s:%s localhost:%s"; - if (isDir) { - template = "scp -r %s %s:%s localhost:%s"; - } - - // copy from hostname to local - mCommand = new String[]{"bash", "-c", - String.format(template, ShellUtils.COMMON_SSH_OPTS, remoteHost, fromFile, toFile) - }; - mHostName = remoteHost; - } - - protected String getHostName() { - return mHostName; - } - } - /** * This is an IOException with exit code added. */ @@ -340,66 +177,6 @@ public String toString() { } } - /** - * Object representation of a command execution. - */ - public static class CommandReturn { - 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() { - 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 @@ -413,7 +190,7 @@ public String getFormattedOutput() { * 4. other normal reasons for IOException */ public static String execCommand(String... cmd) throws IOException { - return new Command(cmd).run(); + return new ShellCommand(cmd).run(); } /** @@ -428,7 +205,7 @@ public static String execCommand(String... cmd) throws IOException { * 3. other normal reasons for IOException */ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOException { - return new Command(cmd).runTolerateFailure(); + return new ShellCommand(cmd).runTolerateFailure(); } /** diff --git a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java new file mode 100644 index 000000000000..a12ab0caede4 --- /dev/null +++ b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java @@ -0,0 +1,119 @@ +/* + * The Alluxio Open Foundation licenses this work under the Apache License, version 2.0 + * (the "License"). You may not use this work except in compliance with the License, which is + * available at www.apache.org/licenses/LICENSE-2.0 + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, + * either express or implied, as more fully set forth in the License. + * + * See the NOTICE file distributed with this work for information regarding copyright ownership. + */ + +package alluxio.shell; + +import alluxio.AlluxioTestDirectory; +import alluxio.util.ShellUtils; +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 static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +public class ShellCommandTest { + public static File createFileInDir(File dir, String fileName) throws IOException { + File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); + newFile.createNewFile(); + return newFile; + } + + public static File createDirInDir(File dir, String fileName) throws IOException { + File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); + newFile.mkdir(); + return newFile; + } + + @Rule + public ExpectedException mExceptionRule = ExpectedException.none(); + + /** + * Tests the {@link ShellUtils#execCommand(String...)} method. + * + * @throws Throwable when the execution of the command fails + */ + @Test + public void execCommand() throws Exception { + String testString = "alluxio"; + // Execute echo for testing command execution. + String[] cmd = new String[]{"bash", "-c", "echo " + testString}; + String result = new ShellCommand(cmd).run(); + assertEquals(testString + "\n", result); + } + + @Test + public void execCommandFail() throws Exception { + mExceptionRule.expect(ShellUtils.ExitCodeException.class); + // run a command that guarantees to fail + String[] cmd = new String[]{"bash", "-c", "false"}; + String result = new ShellCommand(cmd).run(); + assertEquals( "false\n", result); + } + + // TODO(jiacheng): Test + + @Test + public void execCommandTolerateFailureSucceed() 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())}; + CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed); + assertEquals(0, crs.getExitCode()); + assertTrue(crs.getStdOut().contains(testFile.getName())); + assertEquals("", crs.getStdErr()); + } + + @Test + public void execCommandTolerateFailureFailed() throws Exception { + // create temp file + File testDir = AlluxioTestDirectory.createTemporaryDirectory("command"); + + // do sth wrong + String[] testCommandFail = new String[]{"ls", + String.format("%saaaa", testDir.getAbsolutePath())}; + CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail); + assertNotEquals(0, crf.getExitCode()); + assertTrue(crf.getStdErr().length() > 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 execCommandTolerateFailureInvalidCommand() throws Exception { + // create temp file + File testDir = AlluxioTestDirectory.createTemporaryDirectory("command"); + + // 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); + } +} diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index 7aea65ca52c9..af48fd757084 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -38,18 +38,6 @@ 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; - } - - public static File createDirInDir(File dir, String fileName) throws IOException { - File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); - newFile.mkdir(); - return newFile; - } - /** * Tests the {@link ShellUtils#execCommand(String...)} method. * @@ -141,35 +129,4 @@ public void getMountInfo() throws Exception { List 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())); - assertEquals("", crs.getStdErr()); - - // do sth wrong - String[] testCommandFail = new String[]{"ls", - String.format("%saaaa", testDir.getAbsolutePath())}; - ShellUtils.CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail); - assertNotEquals(0, crf.getExitCode()); - assertTrue(crf.getStdErr().length() > 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); - } } From b6855c13506f0a82b512789ef84963d7f10f19a7 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 15:58:33 -0800 Subject: [PATCH 15/22] add java doc --- .../main/java/alluxio/shell/ScpCommand.java | 28 +++++++++++++++++-- .../main/java/alluxio/shell/ShellCommand.java | 12 ++++++-- .../main/java/alluxio/shell/SshCommand.java | 16 ++++++++++- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/ScpCommand.java b/core/common/src/main/java/alluxio/shell/ScpCommand.java index 424b0451d03c..9512bb3e6d7c 100644 --- a/core/common/src/main/java/alluxio/shell/ScpCommand.java +++ b/core/common/src/main/java/alluxio/shell/ScpCommand.java @@ -15,14 +15,33 @@ import javax.annotation.concurrent.NotThreadSafe; +/** + * Object representation of a remote scp command. + * The scp command copies a file/dir from remote host to local. + */ @NotThreadSafe public class ScpCommand extends ShellCommand { protected String mHostName; - private ScpCommand(String remoteHost, String fromFile, String toFile) { + /** + * Creates a remote scp command to copy a file to local. + * + * @param remoteHost remote hostname + * @param fromFile the remote file + * @param toFile target location to copy to + */ + public ScpCommand(String remoteHost, String fromFile, String toFile) { this(remoteHost, fromFile, toFile, false); } + /** + * Creates a remote scp command to copy a file/dir to local. + * + * @param remoteHost the remote hostname + * @param fromFile the remote file/dir to copy + * @param toFile target path to copy to + * @param isDir if true, the remote file is a directory + */ public ScpCommand(String remoteHost, String fromFile, String toFile, boolean isDir) { super(new String[]{}); @@ -38,7 +57,12 @@ public ScpCommand(String remoteHost, String fromFile, String toFile, boolean isD mHostName = remoteHost; } - protected String getHostName() { + /** + * Returns the remote target hostname. + * + * @return target hostname + */ + public String getHostName() { return mHostName; } } diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index d0a188c8d1c5..2a28dcfd3820 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -23,12 +23,20 @@ import java.nio.charset.Charset; import java.util.Arrays; +/** + * Object representation of a shell command. + */ @NotThreadSafe public class ShellCommand { private static final Logger LOG = LoggerFactory.getLogger(ShellCommand.class); protected String[] mCommand; + /** + * Creates a ShellCommand object with the command to exec. + * + * @param execString + */ public ShellCommand(String[] execString) { mCommand = execString.clone(); } @@ -37,9 +45,9 @@ public ShellCommand(String[] execString) { * Runs a command and returns its stdout on success. * * @return the output - * @throws ShellUtils.ExitCodeException if the command returns a non-zero exit code + * @throws IOException if the command returns a non-zero exit code */ - public String run() throws ShellUtils.ExitCodeException, IOException { + public String run() throws IOException { Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); BufferedReader inReader = diff --git a/core/common/src/main/java/alluxio/shell/SshCommand.java b/core/common/src/main/java/alluxio/shell/SshCommand.java index 043a2b0035e1..5653fafbc35d 100644 --- a/core/common/src/main/java/alluxio/shell/SshCommand.java +++ b/core/common/src/main/java/alluxio/shell/SshCommand.java @@ -15,10 +15,19 @@ import javax.annotation.concurrent.NotThreadSafe; +/** + * Object representation of a remote shell command by SSH. + */ @NotThreadSafe public class SshCommand extends ShellCommand { protected String mHostName; + /** + * Creates a SshCommand instance from the remote hostname and command. + * + * @param hostname the remote target hostname + * @param execString the command to execute + */ public SshCommand(String hostname, String[] execString) { super(execString); mHostName = hostname; @@ -27,7 +36,12 @@ public SshCommand(String hostname, String[] execString) { String.join(" ", execString))}; } - protected String getHostName() { + /** + * Returns the remote target hostname. + * + * @return target hostname + */ + public String getHostName() { return mHostName; } } From 861a634beb0f76b588b471d82b0febdee52687e9 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 17:11:18 -0800 Subject: [PATCH 16/22] redirect stderr to stdout --- .../java/alluxio/shell/CommandReturn.java | 28 ++++++------------- .../main/java/alluxio/shell/ShellCommand.java | 22 ++++----------- .../java/alluxio/shell/ShellCommandTest.java | 15 +++------- 3 files changed, 18 insertions(+), 47 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/CommandReturn.java b/core/common/src/main/java/alluxio/shell/CommandReturn.java index ce8922a804c2..80a7334ad9a4 100644 --- a/core/common/src/main/java/alluxio/shell/CommandReturn.java +++ b/core/common/src/main/java/alluxio/shell/CommandReturn.java @@ -16,20 +16,17 @@ */ public class CommandReturn { private int mExitCode; - private String mStdOut; - private String mStdErr; + private String mOutput; /** * Creates object from the contents. * * @param code exit code - * @param stdOut stdout content - * @param stdErr stderr content + * @param output stdout content */ - public CommandReturn(int code, String stdOut, String stdErr) { + public CommandReturn(int code, String output) { mExitCode = code; - mStdOut = stdOut; - mStdErr = stdErr; + mOutput = output; } /** @@ -37,17 +34,8 @@ public CommandReturn(int code, String stdOut, String stdErr) { * * @return stdout content */ - public String getStdOut() { - return mStdOut; - } - - /** - * Gets the stderr content. - * - * @return stderr content - */ - public String getStdErr() { - return mStdErr; + public String getOutput() { + return mOutput; } /** @@ -66,7 +54,7 @@ public int getExitCode() { * @return pretty formatted output */ public String getFormattedOutput() { - return String.format("StatusCode:%s%nStdOut:%n%s%nStdErr:%n%s", getExitCode(), - getStdOut(), getStdErr()); + return String.format("StatusCode:%s%nOutput:%n%s", getExitCode(), + getOutput()); } } diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index 2a28dcfd3820..1d76202361a7 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -12,6 +12,7 @@ package alluxio.shell; import alluxio.util.ShellUtils; +import com.sun.xml.internal.ws.policy.privateutil.PolicyUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -100,13 +101,11 @@ public String run() throws IOException { * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ public CommandReturn runTolerateFailure() throws IOException { - Process process = new ProcessBuilder(mCommand).start(); + Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); CommandReturn cr = null; try (BufferedReader inReader = - new BufferedReader(new InputStreamReader(process.getInputStream())); - BufferedReader errReader = - new BufferedReader(new InputStreamReader(process.getErrorStream()))) { + new BufferedReader(new InputStreamReader(process.getInputStream()))) { // read the output of the command StringBuilder stdout = new StringBuilder(); String outLine = inReader.readLine(); @@ -116,24 +115,15 @@ public CommandReturn runTolerateFailure() throws IOException { outLine = inReader.readLine(); } - // read the stderr of the command - StringBuilder stderr = new StringBuilder(); - String errLine = errReader.readLine(); - 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())); + LOG.warn(String.format("Non-zero exit code from command %s%nExit Code: %s", + Arrays.toString(mCommand), exitCode)); } - cr = new CommandReturn(exitCode, stdout.toString(), stderr.toString()); + cr = new CommandReturn(exitCode, stdout.toString()); // destroy the process if (process != null) { diff --git a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java index a12ab0caede4..bc333ac6b2c6 100644 --- a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java +++ b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java @@ -32,12 +32,6 @@ public static File createFileInDir(File dir, String fileName) throws IOException return newFile; } - public static File createDirInDir(File dir, String fileName) throws IOException { - File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); - newFile.mkdir(); - return newFile; - } - @Rule public ExpectedException mExceptionRule = ExpectedException.none(); @@ -64,8 +58,6 @@ public void execCommandFail() throws Exception { assertEquals( "false\n", result); } - // TODO(jiacheng): Test - @Test public void execCommandTolerateFailureSucceed() throws Exception { // create temp file @@ -78,8 +70,7 @@ public void execCommandTolerateFailureSucceed() throws Exception { String.format("%s", testDir.getAbsolutePath())}; CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed); assertEquals(0, crs.getExitCode()); - assertTrue(crs.getStdOut().contains(testFile.getName())); - assertEquals("", crs.getStdErr()); + assertTrue(crs.getOutput().contains(testFile.getName())); } @Test @@ -92,7 +83,9 @@ public void execCommandTolerateFailureFailed() throws Exception { String.format("%saaaa", testDir.getAbsolutePath())}; CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail); assertNotEquals(0, crf.getExitCode()); - assertTrue(crf.getStdErr().length() > 0); + // The error is redirected into stdout + assertTrue(crf.getOutput().length() > 0); + assertNotEquals("", crf.getOutput()); // if there's no such command there will be IOException mExceptionRule.expect(IOException.class); From d484e442089a2eb08c1569d39780e13111ffe88e Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 17:14:50 -0800 Subject: [PATCH 17/22] format errors --- core/common/src/main/java/alluxio/shell/ShellCommand.java | 1 - core/common/src/main/java/alluxio/util/ShellUtils.java | 6 ------ core/common/src/test/java/alluxio/util/ShellUtilsTest.java | 6 +----- 3 files changed, 1 insertion(+), 12 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index 1d76202361a7..bbc2b53bdb41 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -12,7 +12,6 @@ package alluxio.shell; import alluxio.util.ShellUtils; -import com.sun.xml.internal.ws.policy.privateutil.PolicyUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 12ab561fabcb..9e8b4db9afcd 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -19,18 +19,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.BufferedReader; import java.io.IOException; -import java.io.InputStream; -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; -import javax.annotation.concurrent.NotThreadSafe; import javax.annotation.concurrent.ThreadSafe; /** diff --git a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java index af48fd757084..4c1e517b381c 100644 --- a/core/common/src/test/java/alluxio/util/ShellUtilsTest.java +++ b/core/common/src/test/java/alluxio/util/ShellUtilsTest.java @@ -13,21 +13,17 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotEquals; 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; /** From 8749601489168bfba25d461af7a40f1ce106700a Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 17:28:59 -0800 Subject: [PATCH 18/22] more format --- .../src/main/java/alluxio/shell/ShellCommand.java | 3 ++- .../common/src/main/java/alluxio/util/ShellUtils.java | 1 + .../src/test/java/alluxio/shell/ShellCommandTest.java | 11 ++++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index bbc2b53bdb41..d3536e6f4741 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -12,6 +12,7 @@ package alluxio.shell; import alluxio.util.ShellUtils; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,7 +36,7 @@ public class ShellCommand { /** * Creates a ShellCommand object with the command to exec. * - * @param execString + * @param execString shell command */ public ShellCommand(String[] execString) { mCommand = execString.clone(); diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index 9e8b4db9afcd..f475d981dcdf 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -15,6 +15,7 @@ import alluxio.shell.ScpCommand; import alluxio.shell.ShellCommand; import alluxio.shell.SshCommand; + import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; diff --git a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java index bc333ac6b2c6..925db69195ac 100644 --- a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java +++ b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java @@ -11,8 +11,13 @@ package alluxio.shell; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + import alluxio.AlluxioTestDirectory; import alluxio.util.ShellUtils; + import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -21,10 +26,6 @@ import java.io.IOException; import java.nio.file.Paths; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotEquals; -import static org.junit.Assert.assertTrue; - public class ShellCommandTest { public static File createFileInDir(File dir, String fileName) throws IOException { File newFile = new File(Paths.get(dir.getAbsolutePath(), fileName).toString()); @@ -55,7 +56,7 @@ public void execCommandFail() throws Exception { // run a command that guarantees to fail String[] cmd = new String[]{"bash", "-c", "false"}; String result = new ShellCommand(cmd).run(); - assertEquals( "false\n", result); + assertEquals("false\n", result); } @Test From efac11e2540ee7313897949d757d917da31061ff Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Thu, 2 Jan 2020 21:51:15 -0800 Subject: [PATCH 19/22] comments --- core/common/src/main/java/alluxio/shell/ShellCommand.java | 4 +++- core/common/src/main/java/alluxio/util/ShellUtils.java | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index d3536e6f4741..8efc86ace8f2 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -44,6 +44,7 @@ public ShellCommand(String[] execString) { /** * Runs a command and returns its stdout on success. + * Stderr is redirected to stdout. * * @return the output * @throws IOException if the command returns a non-zero exit code @@ -95,8 +96,9 @@ public String run() throws IOException { } /** - * Runs a command and returns its stdout, stderr and exit code. + * Runs a command and returns its output and exit code. * No matter it succeeds or not. + * Stderr is redirected to stdout. * * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index f475d981dcdf..ee3344767c41 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -190,7 +190,7 @@ public static String execCommand(String... cmd) throws IOException { /** * Static method to execute a shell command and tolerate non-zero exit code. - * Preserve exit code, stderr and stdout in object. + * Preserves exit code. Stderr is redirected to stdout. * * @param cmd shell command to execute * @return the output of the executed command @@ -205,7 +205,7 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE /** * Static method to execute a shell command remotely via ssh. - * Preserve exit code, stderr and stdout in object. + * Preserves exit code. Stderr redirected to stdout. * SSH must be password-less. * * @param hostname Hostname where the command should execute @@ -223,6 +223,7 @@ public static CommandReturn sshExecCommandTolerateFailure(String hostname, Strin /** * Static method to execute an scp command to copy a remote file/dir to local. + * Preserves exit code. Stderr redirected to stdout. * * @param hostname Hostname where the command should execute * @param fromFile File path on remote host From 625d07f4a5944ace99c8638944e488c0821b95c4 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Fri, 3 Jan 2020 14:47:55 -0800 Subject: [PATCH 20/22] resolve review comments --- .../main/java/alluxio/shell/ScpCommand.java | 17 +++++------------ .../main/java/alluxio/shell/ShellCommand.java | 18 +++++++++--------- .../main/java/alluxio/shell/SshCommand.java | 9 ++++----- .../src/main/java/alluxio/util/ShellUtils.java | 6 +++--- .../java/alluxio/shell/ShellCommandTest.java | 16 ++++------------ 5 files changed, 25 insertions(+), 41 deletions(-) diff --git a/core/common/src/main/java/alluxio/shell/ScpCommand.java b/core/common/src/main/java/alluxio/shell/ScpCommand.java index 9512bb3e6d7c..85bd748bcb0c 100644 --- a/core/common/src/main/java/alluxio/shell/ScpCommand.java +++ b/core/common/src/main/java/alluxio/shell/ScpCommand.java @@ -21,7 +21,7 @@ */ @NotThreadSafe public class ScpCommand extends ShellCommand { - protected String mHostName; + private final String mHostName; /** * Creates a remote scp command to copy a file to local. @@ -43,17 +43,10 @@ public ScpCommand(String remoteHost, String fromFile, String toFile) { * @param isDir if true, the remote file is a directory */ public ScpCommand(String remoteHost, String fromFile, String toFile, boolean isDir) { - super(new String[]{}); - - String template = "scp %s %s:%s localhost:%s"; - if (isDir) { - template = "scp -r %s %s:%s localhost:%s"; - } - - // copy from hostname to local - mCommand = new String[]{"bash", "-c", - String.format(template, ShellUtils.COMMON_SSH_OPTS, remoteHost, fromFile, toFile) - }; + super(new String[]{"bash", "-c", + String.format(isDir ? "scp -r %s %s:%s localhost:%s" : "scp %s %s:%s localhost:%s", + ShellUtils.COMMON_SSH_OPTS, remoteHost, fromFile, toFile) + }); mHostName = remoteHost; } diff --git a/core/common/src/main/java/alluxio/shell/ShellCommand.java b/core/common/src/main/java/alluxio/shell/ShellCommand.java index 8efc86ace8f2..06968c3f1c90 100644 --- a/core/common/src/main/java/alluxio/shell/ShellCommand.java +++ b/core/common/src/main/java/alluxio/shell/ShellCommand.java @@ -31,7 +31,7 @@ public class ShellCommand { private static final Logger LOG = LoggerFactory.getLogger(ShellCommand.class); - protected String[] mCommand; + private final String[] mCommand; /** * Creates a ShellCommand object with the command to exec. @@ -89,22 +89,22 @@ public String run() throws IOException { inReader.close(); } } catch (IOException e) { - LOG.warn("Error while closing the input stream", e); + LOG.warn(String.format("Error while closing the input stream of process %s: %s", + process, e.getMessage())); } process.destroy(); } } /** - * Runs a command and returns its output and exit code. - * No matter it succeeds or not. + * Runs a command and returns its output and exit code in Object. + * Preserves the output when the execution fails. * Stderr is redirected to stdout. * * @return {@link CommandReturn} object representation of stdout, stderr and exit code */ - public CommandReturn runTolerateFailure() throws IOException { + public CommandReturn runWithOutput() throws IOException { Process process = new ProcessBuilder(mCommand).redirectErrorStream(true).start(); - CommandReturn cr = null; try (BufferedReader inReader = new BufferedReader(new InputStreamReader(process.getInputStream()))) { @@ -121,11 +121,11 @@ public CommandReturn runTolerateFailure() throws IOException { int exitCode = process.waitFor(); if (exitCode != 0) { // log error instead of throwing exception - LOG.warn(String.format("Non-zero exit code from command %s%nExit Code: %s", - Arrays.toString(mCommand), exitCode)); + LOG.warn(String.format("Non-zero exit code (%d) from command %s", + exitCode, Arrays.toString(mCommand))); } - cr = new CommandReturn(exitCode, stdout.toString()); + CommandReturn cr = new CommandReturn(exitCode, stdout.toString()); // destroy the process if (process != null) { diff --git a/core/common/src/main/java/alluxio/shell/SshCommand.java b/core/common/src/main/java/alluxio/shell/SshCommand.java index 5653fafbc35d..b4800126fce7 100644 --- a/core/common/src/main/java/alluxio/shell/SshCommand.java +++ b/core/common/src/main/java/alluxio/shell/SshCommand.java @@ -20,7 +20,7 @@ */ @NotThreadSafe public class SshCommand extends ShellCommand { - protected String mHostName; + private final String mHostName; /** * Creates a SshCommand instance from the remote hostname and command. @@ -29,11 +29,10 @@ public class SshCommand extends ShellCommand { * @param execString the command to execute */ public SshCommand(String hostname, String[] execString) { - super(execString); - mHostName = hostname; - mCommand = new String[]{"bash", "-c", + super(new String[]{"bash", "-c", String.format("ssh %s %s %s", ShellUtils.COMMON_SSH_OPTS, hostname, - String.join(" ", execString))}; + String.join(" ", execString))}); + mHostName = hostname; } /** diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index ee3344767c41..f192500ff27b 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -200,7 +200,7 @@ public static String execCommand(String... cmd) throws IOException { * 3. other normal reasons for IOException */ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOException { - return new ShellCommand(cmd).runTolerateFailure(); + return new ShellCommand(cmd).runWithOutput(); } /** @@ -218,7 +218,7 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE */ public static CommandReturn sshExecCommandTolerateFailure(String hostname, String... cmd) throws IOException { - return new SshCommand(hostname, cmd).runTolerateFailure(); + return new SshCommand(hostname, cmd).runWithOutput(); } /** @@ -237,7 +237,7 @@ public static CommandReturn sshExecCommandTolerateFailure(String hostname, Strin */ public static CommandReturn scpCommandTolerateFailure( String hostname, String fromFile, String toFile, boolean isDir) throws IOException { - return new ScpCommand(hostname, fromFile, toFile, isDir).runTolerateFailure(); + return new ScpCommand(hostname, fromFile, toFile, isDir).runWithOutput(); } private ShellUtils() {} // prevent instantiation diff --git a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java index 925db69195ac..2a50ffa2a201 100644 --- a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java +++ b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java @@ -69,7 +69,7 @@ public void execCommandTolerateFailureSucceed() throws Exception { // ls temp file String[] testCommandSucceed = new String[]{"ls", String.format("%s", testDir.getAbsolutePath())}; - CommandReturn crs = ShellUtils.execCommandTolerateFailure(testCommandSucceed); + CommandReturn crs = new ShellCommand(testCommandSucceed).runWithOutput(); assertEquals(0, crs.getExitCode()); assertTrue(crs.getOutput().contains(testFile.getName())); } @@ -82,19 +82,11 @@ public void execCommandTolerateFailureFailed() throws Exception { // do sth wrong String[] testCommandFail = new String[]{"ls", String.format("%saaaa", testDir.getAbsolutePath())}; - CommandReturn crf = ShellUtils.execCommandTolerateFailure(testCommandFail); + CommandReturn crf = new ShellCommand(testCommandFail).runWithOutput(); assertNotEquals(0, crf.getExitCode()); // The error is redirected into stdout assertTrue(crf.getOutput().length() > 0); - assertNotEquals("", crf.getOutput()); - - // 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); + assertNotEquals("", crf.getOutput());; } @Test @@ -108,6 +100,6 @@ public void execCommandTolerateFailureInvalidCommand() throws Exception { String[] testCommandExcept = new String[]{"lsa", String.format("%s", testDir.getAbsolutePath())}; // lsa is not a valid executable - ShellUtils.execCommandTolerateFailure(testCommandExcept); + new ShellCommand(testCommandExcept).runWithOutput(); } } From 6940dd50b4161473fd73b3c32a3ddcdb6327eeb0 Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Fri, 3 Jan 2020 15:10:13 -0800 Subject: [PATCH 21/22] whitespace issue --- core/common/src/test/java/alluxio/shell/ShellCommandTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java index 2a50ffa2a201..b47260dbbf0c 100644 --- a/core/common/src/test/java/alluxio/shell/ShellCommandTest.java +++ b/core/common/src/test/java/alluxio/shell/ShellCommandTest.java @@ -86,7 +86,7 @@ public void execCommandTolerateFailureFailed() throws Exception { assertNotEquals(0, crf.getExitCode()); // The error is redirected into stdout assertTrue(crf.getOutput().length() > 0); - assertNotEquals("", crf.getOutput());; + assertNotEquals("", crf.getOutput()); } @Test From caf243a81bd23754137c0c3753ea9919ab614b0d Mon Sep 17 00:00:00 2001 From: jiacheliu3 Date: Fri, 3 Jan 2020 15:43:28 -0800 Subject: [PATCH 22/22] rename methods --- core/common/src/main/java/alluxio/util/ShellUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/common/src/main/java/alluxio/util/ShellUtils.java b/core/common/src/main/java/alluxio/util/ShellUtils.java index f192500ff27b..658ed468a779 100644 --- a/core/common/src/main/java/alluxio/util/ShellUtils.java +++ b/core/common/src/main/java/alluxio/util/ShellUtils.java @@ -199,7 +199,7 @@ public static String execCommand(String... cmd) throws IOException { * 2. execution interrupted * 3. other normal reasons for IOException */ - public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOException { + public static CommandReturn execCommandWithOutput(String... cmd) throws IOException { return new ShellCommand(cmd).runWithOutput(); } @@ -216,7 +216,7 @@ public static CommandReturn execCommandTolerateFailure(String... cmd) throws IOE * 2. execution interrupted * 3. other normal reasons for IOException */ - public static CommandReturn sshExecCommandTolerateFailure(String hostname, String... cmd) + public static CommandReturn sshExecCommandWithOutput(String hostname, String... cmd) throws IOException { return new SshCommand(hostname, cmd).runWithOutput(); } @@ -235,7 +235,7 @@ public static CommandReturn sshExecCommandTolerateFailure(String hostname, Strin * 2. execution interrupted * 3. other normal reasons for IOException */ - public static CommandReturn scpCommandTolerateFailure( + public static CommandReturn scpCommandWithOutput( String hostname, String fromFile, String toFile, boolean isDir) throws IOException { return new ScpCommand(hostname, fromFile, toFile, isDir).runWithOutput(); }