From 235f336fffc379282994f7749aea0d72fe7a886b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 11 Dec 2019 16:08:47 -0800 Subject: [PATCH] Simplify running tools in packaging tests (#49665) 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. --- .../packaging/test/ArchiveTests.java | 37 ++++++++-------- .../packaging/test/DebPreservationTests.java | 4 +- .../packaging/test/DockerTests.java | 16 +++---- .../packaging/test/PackageTests.java | 3 +- .../packaging/test/PackagingTestCase.java | 42 +++++++++---------- .../packaging/test/PasswordToolsTests.java | 6 +-- .../packaging/test/RpmPreservationTests.java | 10 ++--- .../packaging/test/SqlCliTests.java | 2 +- .../packaging/test/WindowsServiceTests.java | 2 +- .../packaging/util/Archives.java | 10 ++--- .../elasticsearch/packaging/util/Docker.java | 2 +- .../packaging/util/Installation.java | 37 +++++++++------- .../packaging/util/Packages.java | 5 +-- .../elasticsearch/packaging/util/Shell.java | 9 ++++ 14 files changed, 95 insertions(+), 90 deletions(-) diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java index d427124d7fecd..9691636f5ccda 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java @@ -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; @@ -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()); } @@ -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")); }); } @@ -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 @@ -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 { @@ -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")); }); } @@ -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]")); }; @@ -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")); }; @@ -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")); }; @@ -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")); } @@ -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")); }; diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java index ea4f5565a98a8..4ff08ad47acf6 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DebPreservationTests.java @@ -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 { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java index 163199d833ae1..c8575902721c0 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/DockerTests.java @@ -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()); } @@ -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")); } @@ -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")); } @@ -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 { @@ -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")); } @@ -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, diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java index c992b02b1c522..b53878459ef6d 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java @@ -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); } @@ -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"); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java index f797d8d49684a..da3f55daae035 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java @@ -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; @@ -89,8 +88,8 @@ 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; @@ -98,7 +97,7 @@ protected void failed(Throwable e, Description description) { }; // a shell to run system commands with - protected Shell sh; + protected static Shell sh; @Rule public final TestName testNameRule = new TestName(); @@ -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 */ @@ -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); @@ -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, @@ -290,7 +289,6 @@ public void assertElasticsearchFailure(Shell.Result result, String expectedMessa // Otherwise, error should be on shell stderr assertThat(result.stderr, containsString(expectedMessage)); - } } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java index 9082b19f0bd49..b08b3bfe52987 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/PasswordToolsTests.java @@ -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 userpasses = parseUsersAndPasswords(result.stdout); for (Map.Entry userpass : userpasses.entrySet()) { String response = ServerUtils.makeRequest(Request.Get("http://localhost:9200"), userpass.getKey(), userpass.getValue()); @@ -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( @@ -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 userpasses = parseUsersAndPasswords(result.stdout); assertThat(userpasses, hasKey("elastic")); for (Map.Entry userpass : userpasses.entrySet()) { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java index 0509b1d244b1a..4448e02bd0692 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/RpmPreservationTests.java @@ -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 { @@ -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", diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java index 62a00aab59d22..afcc7dd0b0919 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/SqlCliTests.java @@ -38,7 +38,7 @@ public void test010Install() throws Exception { } public void test020Help() throws Exception { - Shell.Result result = installation.executables().elasticsearchSqlCli.run(sh, "--help"); + Shell.Result result = installation.executables().sqlCli.run("--help"); assertThat(result.stdout, containsString("Elasticsearch SQL CLI")); } } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java index 506a7313cb17c..5f4fac5f5352c 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java @@ -96,7 +96,7 @@ private void assertExit(Result result, String script, int exitCode) { } public void test10InstallArchive() throws Exception { - installation = installArchive(distribution()); + installation = installArchive(sh, distribution()); verifyArchiveInstallation(installation, distribution()); serviceScript = installation.bin("elasticsearch-service.bat").toString(); } diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java index 1ec62d20a050d..28234bd118701 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java @@ -67,13 +67,11 @@ public class Archives { * errors to the console if they occur before the logging framework is initialized. */ public static final String ES_STARTUP_SLEEP_TIME_SECONDS = "10"; - public static Installation installArchive(Distribution distribution) throws Exception { - return installArchive(distribution, getDefaultArchiveInstallPath(), getCurrentVersion()); + public static Installation installArchive(Shell sh, Distribution distribution) throws Exception { + return installArchive(sh, distribution, getDefaultArchiveInstallPath(), getCurrentVersion()); } - public static Installation installArchive(Distribution distribution, Path fullInstallPath, String version) throws Exception { - final Shell sh = new Shell(); - + public static Installation installArchive(Shell sh, Distribution distribution, Path fullInstallPath, String version) throws Exception { final Path distributionFile = getDistributionFile(distribution); final Path baseInstallPath = fullInstallPath.getParent(); final Path extractedPath = baseInstallPath.resolve("elasticsearch-" + version); @@ -115,7 +113,7 @@ public static Installation installArchive(Distribution distribution, Path fullIn sh.chown(fullInstallPath); - return Installation.ofArchive(distribution, fullInstallPath); + return Installation.ofArchive(sh, distribution, fullInstallPath); } private static void setupArchiveUsersLinux(Path installPath) { diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java index 91b010957011b..245806363c6ab 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/Docker.java @@ -105,7 +105,7 @@ public static Installation runContainer(Distribution distribution, Map getEnv() { return env; } @@ -112,6 +120,7 @@ private static String[] powershellCommand(String script) { } private Result runScript(String[] command) { + logger.warn("Running command with env: " + env); Result result = runScriptIgnoreExitCode(command); if (result.isSuccess() == false) { throw new RuntimeException("Command was not successful: [" + String.join(" ", command) + "]\n result: " + result.toString());