Skip to content

Commit

Permalink
Simplify running tools in packaging tests (elastic#49665)
Browse files Browse the repository at this point in the history 
Running tools requires a shell. This should be the shell setup by the
base packaging tests, but currently tests must pass in their own shell.
This commit begins to make running tools easier by eliminating the shell
argument, instead keeping the shell as part of the Installation (which
can eventually be passed through from the test itself on installation).
The variable names for each tool are also simplified.
  • Loading branch information
@rjernst @SivagurunathanV
rjernst authored and SivagurunathanV committed Jan 21, 2020
1 parent c440f78 commit 235f336
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 90 deletions.
37 changes: 17 additions & 20 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import org.elasticsearch.packaging.util.Installation;
import org.elasticsearch.packaging.util.Platforms;
import org.elasticsearch.packaging.util.ServerUtils;
import org.elasticsearch.packaging.util.Shell;
import org.elasticsearch.packaging.util.Shell.Result;
import org.junit.BeforeClass;

Expand Down Expand Up @@ -63,13 +62,13 @@ public static void filterDistros() {
}

public void test10Install() throws Exception {
installation = installArchive(distribution());
installation = installArchive(sh, distribution());
verifyArchiveInstallation(installation, distribution());
}

public void test20PluginsListWithNoPlugins() throws Exception {
final Installation.Executables bin = installation.executables();
final Result r = bin.elasticsearchPlugin.run(sh, "list");
final Result r = bin.pluginTool.run("list");

assertThat(r.stdout, isEmptyString());
}
Expand Down Expand Up @@ -109,26 +108,26 @@ public void test31BadJavaHome() throws Exception {
public void test40CreateKeystoreManually() throws Exception {
final Installation.Executables bin = installation.executables();

Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " create"));
Platforms.onLinux(() -> sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " create"));

// this is a hack around the fact that we can't run a command in the same session as the same user but not as administrator.
// the keystore ends up being owned by the Administrators group, so we manually set it to be owned by the vagrant user here.
// from the server's perspective the permissions aren't really different, this is just to reflect what we'd expect in the tests.
// when we run these commands as a role user we won't have to do this
Platforms.onWindows(() -> {
sh.run(bin.elasticsearchKeystore + " create");
sh.run(bin.keystoreTool + " create");
sh.chown(installation.config("elasticsearch.keystore"));
});

assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));

Platforms.onLinux(() -> {
final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list");
final Result r = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list");
assertThat(r.stdout, containsString("keystore.seed"));
});

Platforms.onWindows(() -> {
final Result r = sh.run(bin.elasticsearchKeystore + " list");
final Result r = sh.run(bin.keystoreTool + " list");
assertThat(r.stdout, containsString("keystore.seed"));
});
}
Expand Down Expand Up @@ -202,7 +201,6 @@ public void test52BundledJdkRemoved() throws Exception {

public void test53JavaHomeWithSpecialCharacters() throws Exception {
Platforms.onWindows(() -> {
final Shell sh = new Shell();
String javaPath = "C:\\Program Files (x86)\\java";
try {
// once windows 2012 is no longer supported and powershell 5.0 is always available we can change this command
Expand All @@ -228,7 +226,6 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception {
});

Platforms.onLinux(() -> {
final Shell sh = newShell();
// Create temporary directory with a space and link to real java home
String testJavaHome = Paths.get("/tmp", "java home").toString();
try {
Expand Down Expand Up @@ -256,12 +253,12 @@ public void test60AutoCreateKeystore() throws Exception {

final Installation.Executables bin = installation.executables();
Platforms.onLinux(() -> {
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.elasticsearchKeystore + " list");
final Result result = sh.run("sudo -u " + ARCHIVE_OWNER + " " + bin.keystoreTool + " list");
assertThat(result.stdout, containsString("keystore.seed"));
});

Platforms.onWindows(() -> {
final Result result = sh.run(bin.elasticsearchKeystore + " list");
final Result result = sh.run(bin.keystoreTool + " list");
assertThat(result.stdout, containsString("keystore.seed"));
});
}
Expand Down Expand Up @@ -339,11 +336,11 @@ public void test90SecurityCliPackaging() throws Exception {
if (distribution().isDefault()) {
assertTrue(Files.exists(installation.lib.resolve("tools").resolve("security-cli")));
final Platforms.PlatformAction action = () -> {
Result result = sh.run(bin.elasticsearchCertutil + " --help");
Result result = sh.run(bin.certutilTool + " --help");
assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));

// Ensure that the exit code from the java command is passed back up through the shell script
result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command");
result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command");
assertThat(result.exitCode, is(not(0)));
assertThat(result.stderr, containsString("Unknown command [invalid-command]"));
};
Expand All @@ -358,7 +355,7 @@ public void test91ElasticsearchShardCliPackaging() throws Exception {
final Installation.Executables bin = installation.executables();

Platforms.PlatformAction action = () -> {
final Result result = sh.run(bin.elasticsearchShard + " -h");
final Result result = sh.run(bin.shardTool + " -h");
assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
};

Expand All @@ -373,7 +370,7 @@ public void test92ElasticsearchNodeCliPackaging() throws Exception {
final Installation.Executables bin = installation.executables();

Platforms.PlatformAction action = () -> {
final Result result = sh.run(bin.elasticsearchNode + " -h");
final Result result = sh.run(bin.nodeTool + " -h");
assertThat(result.stdout,
containsString("A CLI tool to do unsafe cluster and index manipulations on current node"));
};
Expand All @@ -394,7 +391,7 @@ public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Ex
startElasticsearch();
Archives.stopElasticsearch(installation);

Result result = sh.run("echo y | " + installation.executables().elasticsearchNode + " unsafe-bootstrap");
Result result = sh.run("echo y | " + installation.executables().nodeTool + " unsafe-bootstrap");
assertThat(result.stdout, containsString("Master node was successfully bootstrapped"));
}

Expand All @@ -404,16 +401,16 @@ public void test94ElasticsearchNodeExecuteCliNotEsHomeWorkDir() throws Exception
sh.setWorkingDirectory(getTempDir());

Platforms.PlatformAction action = () -> {
Result result = sh.run(bin.elasticsearchCertutil+ " -h");
Result result = sh.run(bin.certutilTool + " -h");
assertThat(result.stdout,
containsString("Simplifies certificate creation for use with the Elastic Stack"));
result = sh.run(bin.elasticsearchSyskeygen+ " -h");
result = sh.run(bin.syskeygenTool + " -h");
assertThat(result.stdout,
containsString("system key tool"));
result = sh.run(bin.elasticsearchSetupPasswords+ " -h");
result = sh.run(bin.setupPasswordsTool + " -h");
assertThat(result.stdout,
containsString("Sets the passwords for reserved users"));
result = sh.run(bin.elasticsearchUsers+ " -h");
result = sh.run(bin.usersTool + " -h");
assertThat(result.stdout,
containsString("Manages elasticsearch file users"));
};
Expand Down
4 changes: 2 additions & 2 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public static void filterDistros() {

public void test10Install() throws Exception {
assertRemoved(distribution());
installation = installPackage(distribution());
installation = installPackage(sh, distribution());
assertInstalled(distribution());
verifyPackageInstallation(installation, distribution(), newShell());
verifyPackageInstallation(installation, distribution(), sh);
}

public void test20Remove() throws Exception {
Expand Down
16 changes: 8 additions & 8 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public void test011PresenceOfXpack() throws Exception {
*/
public void test020PluginsListWithNoPlugins() {
final Installation.Executables bin = installation.executables();
final Result r = sh.run(bin.elasticsearchPlugin + " list");
final Result r = sh.run(bin.pluginTool + " list");

assertThat("Expected no plugins to be listed", r.stdout, emptyString());
}
Expand All @@ -148,9 +148,9 @@ public void test040CreateKeystoreManually() throws InterruptedException {
// Move the auto-created one out of the way, or else the CLI prompts asks us to confirm
sh.run("mv " + keystorePath + " " + keystorePath + ".bak");

sh.run(bin.elasticsearchKeystore + " create");
sh.run(bin.keystoreTool + " create");

final Result r = sh.run(bin.elasticsearchKeystore + " list");
final Result r = sh.run(bin.keystoreTool + " list");
assertThat(r.stdout, containsString("keystore.seed"));
}

Expand All @@ -165,7 +165,7 @@ public void test041AutoCreateKeystore() throws Exception {
assertPermissionsAndOwnership(keystorePath, p660);

final Installation.Executables bin = installation.executables();
final Result result = sh.run(bin.elasticsearchKeystore + " list");
final Result result = sh.run(bin.keystoreTool + " list");
assertThat(result.stdout, containsString("keystore.seed"));
}

Expand Down Expand Up @@ -402,11 +402,11 @@ public void test090SecurityCliPackaging() {
if (distribution().isDefault()) {
assertTrue(existsInContainer(securityCli));

Result result = sh.run(bin.elasticsearchCertutil + " --help");
Result result = sh.run(bin.certutilTool + " --help");
assertThat(result.stdout, containsString("Simplifies certificate creation for use with the Elastic Stack"));

// Ensure that the exit code from the java command is passed back up through the shell script
result = sh.runIgnoreExitCode(bin.elasticsearchCertutil + " invalid-command");
result = sh.runIgnoreExitCode(bin.certutilTool + " invalid-command");
assertThat(result.isSuccess(), is(false));
assertThat(result.stdout, containsString("Unknown command [invalid-command]"));
} else {
Expand All @@ -420,7 +420,7 @@ public void test090SecurityCliPackaging() {
public void test091ElasticsearchShardCliPackaging() {
final Installation.Executables bin = installation.executables();

final Result result = sh.run(bin.elasticsearchShard + " -h");
final Result result = sh.run(bin.shardTool + " -h");
assertThat(result.stdout, containsString("A CLI tool to remove corrupted parts of unrecoverable shards"));
}

Expand All @@ -430,7 +430,7 @@ public void test091ElasticsearchShardCliPackaging() {
public void test092ElasticsearchNodeCliPackaging() {
final Installation.Executables bin = installation.executables();

final Result result = sh.run(bin.elasticsearchNode + " -h");
final Result result = sh.run(bin.nodeTool + " -h");
assertThat(
"Failed to find expected message about the elasticsearch-node CLI tool",
result.stdout,
Expand Down
3 changes: 1 addition & 2 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public static void filterDistros() {

public void test10InstallPackage() throws Exception {
assertRemoved(distribution());
installation = installPackage(distribution());
installation = installPackage(sh, distribution());
assertInstalled(distribution());
verifyPackageInstallation(installation, distribution(), sh);
}
Expand Down Expand Up @@ -302,7 +302,6 @@ public void test82SystemdMask() throws Exception {
assumeTrue(isSystemd());

sh.run("systemctl mask systemd-sysctl.service");

install();

sh.run("systemctl unmask systemd-sysctl.service");
Expand Down
42 changes: 20 additions & 22 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.rules.TestName;
import org.junit.rules.TestWatcher;
Expand Down Expand Up @@ -89,16 +88,16 @@ public abstract class PackagingTestCase extends Assert {

private static boolean failed;

@ClassRule
public static final TestWatcher testFailureRule = new TestWatcher() {
@Rule
public final TestWatcher testFailureRule = new TestWatcher() {
@Override
protected void failed(Throwable e, Description description) {
failed = true;
}
};

// a shell to run system commands with
protected Shell sh;
protected static Shell sh;

@Rule
public final TestName testNameRule = new TestName();
Expand All @@ -114,11 +113,24 @@ public static void cleanup() throws Exception {
cleanEverything();
}

@BeforeClass
public static void createShell() throws Exception {
sh = new Shell();
}

@Before
public void setup() throws Exception {
assumeFalse(failed); // skip rest of tests once one fails

sh = newShell();
sh.reset();
if (distribution().hasJdk == false) {
Platforms.onLinux(() -> {
sh.getEnv().put("JAVA_HOME", systemJavaHome);
});
Platforms.onWindows(() -> {
sh.getEnv().put("JAVA_HOME", systemJavaHome);
});
}
}

/** The {@link Distribution} that should be tested in this case */
Expand All @@ -130,13 +142,13 @@ protected static void install() throws Exception {
switch (distribution.packaging) {
case TAR:
case ZIP:
installation = Archives.installArchive(distribution);
installation = Archives.installArchive(sh, distribution);
Archives.verifyArchiveInstallation(installation, distribution);
break;
case DEB:
case RPM:
installation = Packages.installPackage(distribution);
Packages.verifyPackageInstallation(installation, distribution, newShell());
installation = Packages.installPackage(sh, distribution);
Packages.verifyPackageInstallation(installation, distribution, sh);
break;
case DOCKER:
installation = Docker.runContainer(distribution);
Expand Down Expand Up @@ -176,19 +188,6 @@ protected void assertWhileRunning(Platforms.PlatformAction assertions) throws Ex
stopElasticsearch();
}

protected static Shell newShell() throws Exception {
Shell sh = new Shell();
if (distribution().hasJdk == false) {
Platforms.onLinux(() -> {
sh.getEnv().put("JAVA_HOME", systemJavaHome);
});
Platforms.onWindows(() -> {
sh.getEnv().put("JAVA_HOME", systemJavaHome);
});
}
return sh;
}

/**
* Run the command to start Elasticsearch, but don't wait or test for success.
* This method is useful for testing failure conditions in startup. To await success,
Expand Down Expand Up @@ -290,7 +289,6 @@ public void assertElasticsearchFailure(Shell.Result result, String expectedMessa

// Otherwise, error should be on shell stderr
assertThat(result.stderr, containsString(expectedMessage));

}
}
}
6 changes: 3 additions & 3 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public void test010Install() throws Exception {

public void test20GeneratePasswords() throws Exception {
assertWhileRunning(() -> {
Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null);
Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null);
Map<String, String> userpasses = parseUsersAndPasswords(result.stdout);
for (Map.Entry<String, String> userpass : userpasses.entrySet()) {
String response = ServerUtils.makeRequest(Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue());
Expand Down Expand Up @@ -106,7 +106,7 @@ public void test30AddBootstrapPassword() throws Exception {
});
}

installation.executables().elasticsearchKeystore.run(sh, "add --stdin bootstrap.password", BOOTSTRAP_PASSWORD);
installation.executables().keystoreTool.run("add --stdin bootstrap.password", BOOTSTRAP_PASSWORD);

assertWhileRunning(() -> {
String response = ServerUtils.makeRequest(
Expand All @@ -119,7 +119,7 @@ public void test30AddBootstrapPassword() throws Exception {
public void test40GeneratePasswordsBootstrapAlreadySet() throws Exception {
assertWhileRunning(() -> {

Shell.Result result = installation.executables().elasticsearchSetupPasswords.run(sh, "auto --batch", null);
Shell.Result result = installation.executables().setupPasswordsTool.run("auto --batch", null);
Map<String, String> userpasses = parseUsersAndPasswords(result.stdout);
assertThat(userpasses, hasKey("elastic"));
for (Map.Entry<String, String> userpass : userpasses.entrySet()) {
Expand Down
10 changes: 5 additions & 5 deletions qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ public static void filterDistros() {

public void test10Install() throws Exception {
assertRemoved(distribution());
installation = installPackage(distribution());
installation = installPackage(sh, distribution());
assertInstalled(distribution());
verifyPackageInstallation(installation, distribution(), newShell());
verifyPackageInstallation(installation, distribution(), sh);
}

public void test20Remove() throws Exception {
Expand All @@ -71,11 +71,11 @@ public void test20Remove() throws Exception {
public void test30PreserveConfig() throws Exception {
final Shell sh = new Shell();

installation = installPackage(distribution());
installation = installPackage(sh, distribution());
assertInstalled(distribution());
verifyPackageInstallation(installation, distribution(), newShell());
verifyPackageInstallation(installation, distribution(), sh);

sh.run("echo foobar | " + installation.executables().elasticsearchKeystore + " add --stdin foo.bar");
sh.run("echo foobar | " + installation.executables().keystoreTool + " add --stdin foo.bar");
Stream.of(
"elasticsearch.yml",
"jvm.options",
Expand Down
Loading

0 comments on commit 235f336

Please sign in to comment.