Simplify running tools in packaging tests

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"));
         };

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 {

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,

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");

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,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();
@@ -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));
-
         }
     }
 }

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<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());
@@ -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<String, String> userpasses = parseUsersAndPasswords(result.stdout);
             assertThat(userpasses, hasKey("elastic"));
             for (Map.Entry<String, String> userpass : userpasses.entrySet()) {

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",