Skip to content

Commit

Permalink
[test] packaging: use shell when running commands (#30852)
Browse files Browse the repository at this point in the history
When subprocesses are started with ProcessBuilder, they're forked by the
java process directly rather than from a shell, which can be surprising
for our use case here in the packaging tests which is similar to
scripting.

This commit changes the tests to run their subprocess commands in a
shell, using the bash -c <script> syntax for commands on linux and using
the powershell.exe -Command <script> syntax for commands on windows.
This syntax on windows is essentially what the tests were already doing.
  • Loading branch information
andyb-elastic committed May 30, 2018
1 parent fe36d41 commit fa126fd
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 68 deletions.
40 changes: 14 additions & 26 deletions qa/vagrant/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,38 +82,26 @@ In general it's probably best to avoid running external commands when a good
Java alternative exists. For example most filesystem operations can be done with
the java.nio.file APIs. For those that aren't, use an instance of [Shell](src/main/java/org/elasticsearch/packaging/util/Shell.java)

Despite the name, commands run with this class are not run in a shell, and any
familiar features of shells like variables or expansion won't work.

If you do need the shell, you must explicitly invoke the shell's command. For
example to run a command with Bash, use the `bash -c command` syntax. Note that
the entire script must be in a single string argument
This class runs scripts in either bash with the `bash -c <script>` syntax,
or in powershell with the `powershell.exe -Command <script>` syntax.

```java
Shell sh = new Shell();
sh.run("bash", "-c", "echo $foo; echo $bar");
```

Similary for powershell - again, the entire powershell script must go in a
single string argument
// equivalent to `bash -c 'echo $foo; echo $bar'`
sh.bash("echo $foo; echo $bar");

```java
sh.run("powershell.exe", "-Command", "Write-Host $foo; Write-Host $bar");
// equivalent to `powershell.exe -Command 'Write-Host $foo; Write-Host $bar'`
sh.powershell("Write-Host $foo; Write-Host $bar");
```

On Linux, most commands you'll want to use will be executable files and will
work fine without a shell

```java
sh.run("tar", "-xzpf", "elasticsearch-6.1.0.tar.gz");
```
### Notes about powershell

On Windows you'll mostly want to use powershell as it can do a lot more and
gives much better feedback than Windows' legacy command line. Unfortunately that
means that you'll need to use the `powershell.exe -Command` syntax as
powershell's [Cmdlets](https://msdn.microsoft.com/en-us/library/ms714395.aspx)
don't correspond to executable files and are not runnable by `Runtime` directly.
Powershell scripts for the most part have backwards compatibility with legacy
cmd.exe commands and their syntax. Most of the commands you'll want to use
in powershell are [Cmdlets](https://msdn.microsoft.com/en-us/library/ms714395.aspx)
which generally don't have a one-to-one mapping with an executable file.

When writing powershell commands this way, make sure to test them as some types
of formatting can cause it to return a successful exit code but not run
anything.
When writing powershell commands in this project it's worth testing them by
hand, as sometimes when a script can't be interpreted correctly it will
fail silently.
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,20 @@ public static Installation installArchive(Distribution distribution, Path fullIn
if (distribution.packaging == Distribution.Packaging.TAR) {

if (Platforms.LINUX) {
sh.run("tar", "-C", baseInstallPath.toString(), "-xzpf", distributionFile.toString());
sh.bash("tar -C " + baseInstallPath + " -xzpf " + distributionFile);
} else {
throw new RuntimeException("Distribution " + distribution + " is not supported on windows");
}

} else if (distribution.packaging == Distribution.Packaging.ZIP) {

if (Platforms.LINUX) {
sh.run("unzip", distributionFile.toString(), "-d", baseInstallPath.toString());
sh.bash("unzip " + distributionFile + " -d " + baseInstallPath);
} else {
sh.run("powershell.exe", "-Command",
sh.powershell(
"Add-Type -AssemblyName 'System.IO.Compression.Filesystem'; " +
"[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')");
"[IO.Compression.ZipFile]::ExtractToDirectory('" + distributionFile + "', '" + baseInstallPath + "')"
);
}

} else {
Expand All @@ -102,35 +103,35 @@ public static Installation installArchive(Distribution distribution, Path fullIn
private static void setupArchiveUsersLinux(Path installPath) {
final Shell sh = new Shell();

if (sh.runIgnoreExitCode("getent", "group", "elasticsearch").isSuccess() == false) {
if (sh.bashIgnoreExitCode("getent group elasticsearch").isSuccess() == false) {
if (isDPKG()) {
sh.run("addgroup", "--system", "elasticsearch");
sh.bash("addgroup --system elasticsearch");
} else {
sh.run("groupadd", "-r", "elasticsearch");
sh.bash("groupadd -r elasticsearch");
}
}

if (sh.runIgnoreExitCode("id", "elasticsearch").isSuccess() == false) {
if (sh.bashIgnoreExitCode("id elasticsearch").isSuccess() == false) {
if (isDPKG()) {
sh.run("adduser",
"--quiet",
"--system",
"--no-create-home",
"--ingroup", "elasticsearch",
"--disabled-password",
"--shell", "/bin/false",
sh.bash("adduser " +
"--quiet " +
"--system " +
"--no-create-home " +
"--ingroup elasticsearch " +
"--disabled-password " +
"--shell /bin/false " +
"elasticsearch");
} else {
sh.run("useradd",
"--system",
"-M",
"--gid", "elasticsearch",
"--shell", "/sbin/nologin",
"--comment", "elasticsearch user",
sh.bash("useradd " +
"--system " +
"-M " +
"--gid elasticsearch " +
"--shell /sbin/nologin " +
"--comment 'elasticsearch user' " +
"elasticsearch");
}
}
sh.run("chown", "-R", "elasticsearch:elasticsearch", installPath.toString());
sh.bash("chown -R elasticsearch:elasticsearch " + installPath);
}

public static void verifyArchiveInstallation(Installation installation, Distribution distribution) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,16 @@ public static void cleanEverything() {
if (Platforms.WINDOWS) {

// the view of processes returned by Get-Process doesn't expose command line arguments, so we use WMI here
sh.runIgnoreExitCode("powershell.exe", "-Command",
sh.powershellIgnoreExitCode(
"Get-WmiObject Win32_Process | " +
"Where-Object { $_.CommandLine -Match 'org.elasticsearch.bootstrap.Elasticsearch' } | " +
"ForEach-Object { $_.Terminate() }");
"ForEach-Object { $_.Terminate() }"
);

} else {

sh.runIgnoreExitCode("pkill", "-u", "elasticsearch");
sh.runIgnoreExitCode("bash", "-c",
"ps aux | grep -i 'org.elasticsearch.bootstrap.Elasticsearch' | awk {'print $2'} | xargs kill -9");
sh.bashIgnoreExitCode("pkill -u elasticsearch");
sh.bashIgnoreExitCode("ps aux | grep -i 'org.elasticsearch.bootstrap.Elasticsearch' | awk {'print $2'} | xargs kill -9");

}

Expand All @@ -78,8 +78,8 @@ public static void cleanEverything() {

// remove elasticsearch users
if (Platforms.LINUX) {
sh.runIgnoreExitCode("userdel", "elasticsearch");
sh.runIgnoreExitCode("groupdel", "elasticsearch");
sh.bashIgnoreExitCode("userdel elasticsearch");
sh.bashIgnoreExitCode("groupdel elasticsearch");
}

// delete files that may still exist
Expand All @@ -95,27 +95,27 @@ public static void cleanEverything() {
// disable elasticsearch service
// todo add this for windows when adding tests for service intallation
if (Platforms.LINUX && isSystemd()) {
sh.run("systemctl", "unmask", "systemd-sysctl.service");
sh.bash("systemctl unmask systemd-sysctl.service");
}
}

private static void purgePackagesLinux() {
final Shell sh = new Shell();

if (isRPM()) {
sh.runIgnoreExitCode("rpm", "--quiet", "-e", "elasticsearch", "elasticsearch-oss");
sh.bashIgnoreExitCode("rpm --quiet -e elasticsearch elasticsearch-oss");
}

if (isYUM()) {
sh.runIgnoreExitCode("yum", "remove", "-y", "elasticsearch", "elasticsearch-oss");
sh.bashIgnoreExitCode("yum remove -y elasticsearch elasticsearch-oss");
}

if (isDPKG()) {
sh.runIgnoreExitCode("dpkg", "--purge", "elasticsearch", "elasticsearch-oss");
sh.bashIgnoreExitCode("dpkg --purge elasticsearch elasticsearch-oss");
}

if (isAptGet()) {
sh.runIgnoreExitCode("apt-get", "--quiet", "--yes", "purge", "elasticsearch", "elasticsearch-oss");
sh.bashIgnoreExitCode("apt-get --quiet --yes purge elasticsearch elasticsearch-oss");
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,41 +28,41 @@ public static boolean isDPKG() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "dpkg").isSuccess();
return new Shell().bashIgnoreExitCode("which dpkg").isSuccess();
}

public static boolean isAptGet() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "apt-get").isSuccess();
return new Shell().bashIgnoreExitCode("which apt-get").isSuccess();
}

public static boolean isRPM() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "rpm").isSuccess();
return new Shell().bashIgnoreExitCode("which rpm").isSuccess();
}

public static boolean isYUM() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "yum").isSuccess();
return new Shell().bashIgnoreExitCode("which yum").isSuccess();
}

public static boolean isSystemd() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "systemctl").isSuccess();
return new Shell().bashIgnoreExitCode("which systemctl").isSuccess();
}

public static boolean isSysVInit() {
if (WINDOWS) {
return false;
}
return new Shell().runIgnoreExitCode("which", "service").isSuccess();
return new Shell().bashIgnoreExitCode("which service").isSuccess();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Stream;

import static java.util.Collections.emptyMap;

Expand Down Expand Up @@ -57,15 +58,58 @@ public Shell(Map<String, String> env, Path workingDirectory) {
this.workingDirectory = workingDirectory;
}

public Result run(String... command) {
/**
* Runs a script in a bash shell, throwing an exception if its exit code is nonzero
*/
public Result bash(String script) {
return run(bashCommand(script));
}

/**
* Runs a script in a bash shell
*/
public Result bashIgnoreExitCode(String script) {
return runIgnoreExitCode(bashCommand(script));
}

private static String[] bashCommand(String script) {
return Stream.concat(Stream.of("bash", "-c"), Stream.of(script)).toArray(String[]::new);
}

/**
* Runs a script in a powershell shell, throwing an exception if its exit code is nonzero
*/
public Result powershell(String script) {
return run(powershellCommand(script));
}

/**
* Runs a script in a powershell shell
*/
public Result powershellIgnoreExitCode(String script) {
return runIgnoreExitCode(powershellCommand(script));
}

private static String[] powershellCommand(String script) {
return Stream.concat(Stream.of("powershell.exe", "-Command"), Stream.of(script)).toArray(String[]::new);
}

/**
* Runs an executable file, passing all elements of {@code command} after the first as arguments. Throws an exception if the process'
* exit code is nonzero
*/
private Result run(String[] command) {
Result result = runIgnoreExitCode(command);
if (result.isSuccess() == false) {
throw new RuntimeException("Command was not successful: [" + String.join(" ", command) + "] result: " + result.toString());
}
return result;
}

public Result runIgnoreExitCode(String... command) {
/**
* Runs an executable file, passing all elements of {@code command} after the first as arguments
*/
private Result runIgnoreExitCode(String[] command) {
ProcessBuilder builder = new ProcessBuilder();
builder.command(command);

Expand Down

0 comments on commit fa126fd

Please sign in to comment.