Make All OS tests run on GCP instances (#46924)

This PR makes the necesary adaptations to the tests and adds a power shell script to
invoke the OS tests on GCP instances connected as CI workers.

Also noticed that logs were not being produced by the tests and that theses were not using log4j so fixed that too.

One of the difficulties in working on theses tests was that the tests just stalled with no indication where the problem is.
To ease with the debugging, after process explorer suggested that the tests are running some commands, we now have multiple timeouts: one for the tests ( which will generate a thread dump ) and one for individual commands ( that bails with the command being ran and output and error so far ) to make it easier to see what went wrong.

The tests were blocking because apparently the pipes to the sub-process were not closing, thus the threads were blocking on them and we were blocking indefinitely on the join. I'm not sure why this doesn't happen in vagrant, but we now properly deal with it.

.ci/os.ps1

@@ -0,0 +1,36 @@
+If (-NOT ([Security.Principal.WindowsPrincipal] [Security.Principal.WindowsIdentity]::GetCurrent()).IsInRole([Security.Principal.WindowsBuiltInRole]::Administrator))
+{
+    # Relaunch as an elevated process:
+    Start-Process powershell.exe "-File",('"{0}"' -f $MyInvocation.MyCommand.Path) -Verb RunAs
+    exit
+}
+
+# CI configures these, uncoment if running manually 
+#
+# $env:ES_BUILD_JAVA="java12" 
+#$env:ES_RUNTIME_JAVA="java11" 
+
+$ErrorActionPreference="Stop"
+$gradleInit = "C:\Users\$env:username\.gradle\init.d\"
+echo "Remove $gradleInit"
+Remove-Item -Recurse -Force $gradleInit -ErrorAction Ignore
+New-Item -ItemType directory -Path $gradleInit
+echo "Copy .ci/init.gradle to $gradleInit"
+Copy-Item .ci/init.gradle -Destination $gradleInit
+
+[Environment]::SetEnvironmentVariable("JAVA_HOME", $null, "Machine")
+$env:PATH="C:\Users\jenkins\.java\$env:ES_BUILD_JAVA\bin\;$env:PATH"
+$env:JAVA_HOME=$null
+$env:SYSTEM_JAVA_HOME="C:\Users\jenkins\.java\$env:ES_RUNTIME_JAVA"
+Remove-Item -Recurse -Force \tmp -ErrorAction Ignore
+New-Item -ItemType directory -Path \tmp
+
+$ErrorActionPreference="Continue"
+# TODO: remove the task exclusions once dependencies are set correctly and these don't run for Windows or buldiung the deb on windows is fixed
+& .\gradlew.bat -g "C:\Users\$env:username\.gradle" --parallel --scan --console=plain destructiveDistroTest `
+   -x :distribution:packages:buildOssDeb `
+   -x :distribution:packages:buildDeb `
+   -x :distribution:packages:buildOssRpm `
+   -x :distribution:packages:buildRpm `
+
+exit $?

.ci/os.sh

@@ -0,0 +1,68 @@
+#!/bin/bash
+
+# opensuse 15 has a missing dep for systemd 
+
+if which zypper > /dev/null ; then
+    sudo zypper install -y insserv-compat
+fi
+
+# Required by bats
+sudo touch /etc/is_vagrant_vm
+sudo useradd vagrant
+
+set -e
+
+. .ci/java-versions.properties
+RUNTIME_JAVA_HOME=$HOME/.java/$ES_RUNTIME_JAVA
+BUILD_JAVA_HOME=$HOME/.java/$ES_BUILD_JAVA
+
+rm -Rfv $HOME/.gradle/init.d/ && mkdir -p $HOME/.gradle/init.d
+cp -v .ci/init.gradle $HOME/.gradle/init.d
+
+unset JAVA_HOME
+
+if ! [ -e "/usr/bin/bats" ] ; then 
+  git clone https://github.com/sstephenson/bats /tmp/bats
+  sudo /tmp/bats/install.sh /usr
+fi
+
+
+if [ -f "/etc/os-release" ] ; then 
+    cat /etc/os-release
+    . /etc/os-release
+    if [[ "$ID" == "debian" || "$ID_LIKE" == "debian" ]] ; then 
+        # FIXME: The base image should not have rpm installed
+        sudo rm -Rf /usr/bin/rpm
+    fi
+else
+    cat /etc/issue || true
+fi
+
+sudo bash -c 'cat > /etc/sudoers.d/elasticsearch_vars'  << SUDOERS_VARS
+    Defaults   env_keep += "ZIP"
+    Defaults   env_keep += "TAR"
+    Defaults   env_keep += "RPM"
+    Defaults   env_keep += "DEB"
+    Defaults   env_keep += "PACKAGING_ARCHIVES"
+    Defaults   env_keep += "PACKAGING_TESTS"
+    Defaults   env_keep += "BATS_UTILS"
+    Defaults   env_keep += "BATS_TESTS"
+    Defaults   env_keep += "SYSTEM_JAVA_HOME"
+    Defaults   env_keep += "JAVA_HOME"
+SUDOERS_VARS
+sudo chmod 0440 /etc/sudoers.d/elasticsearch_vars
+
+# Bats tests still use this locationa
+sudo rm -Rf /elasticsearch
+sudo mkdir -p /elasticsearch/qa/ && sudo chown jenkins /elasticsearch/qa/ && ln -s $PWD/qa/vagrant /elasticsearch/qa/
+
+# sudo sets it's own PATH thus we use env to override that and call sudo annother time so we keep the secure root PATH 
+# run with --continue to run both bats and java tests even if one fails
+# be explicit about Gradle home dir so we use the same even with sudo
+sudo -E env \
+  PATH=$BUILD_JAVA_HOME/bin:`sudo bash -c 'echo -n $PATH'` \
+  RUNTIME_JAVA_HOME=`readlink -f -n $RUNTIME_JAVA_HOME` \
+  --unset=JAVA_HOME \
+  SYSTEM_JAVA_HOME=`readlink -f -n $RUNTIME_JAVA_HOME` \
+  ./gradlew -g $HOME/.gradle --scan --parallel $@ --continue destructivePackagingTest
+

buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy

@@ -88,7 +88,6 @@ import java.util.regex.Matcher
 
 import static org.elasticsearch.gradle.tool.Boilerplate.findByName
 import static org.elasticsearch.gradle.tool.Boilerplate.maybeConfigure
-
 /**
  * Encapsulates build configuration for elasticsearch projects.
  */
@@ -899,6 +898,11 @@ class BuildPlugin implements Plugin<Project> {
                     logging.exceptionFormat = 'full'
                 }
 
+                if (OS.current().equals(OS.WINDOWS) && System.getProperty('tests.timeoutSuite') == null) {
+                    // override the suite timeout to 30 mins for windows, because it has the most inefficient filesystem known to man
+                    test.systemProperty 'tests.timeoutSuite', '1800000!'
+                }
+
                 project.plugins.withType(ShadowPlugin).whenPluginAdded {
                     // Test against a shadow jar if we made one
                     test.classpath -= project.configurations.getByName('bundle')

distribution/archives/build.gradle

@@ -17,7 +17,6 @@
  * under the License.
  */
 
-import org.apache.tools.ant.taskdefs.condition.Os
 import org.elasticsearch.gradle.BuildPlugin
 import org.elasticsearch.gradle.EmptyDirTask
 import org.elasticsearch.gradle.LoggedExec
@@ -28,7 +27,6 @@ import org.elasticsearch.gradle.tar.SymbolicLinkPreservingTar
 
 import java.nio.file.Files
 import java.nio.file.Path
-
 // need this so Zip/Tar tasks get basic defaults...
 apply plugin: 'base'
 
@@ -312,12 +310,6 @@ configure(subprojects.findAll { it.name == 'integ-test-zip' }) {
   integTest {
     dependsOn assemble
     includePackaged = true
-    runner {
-      if (Os.isFamily(Os.FAMILY_WINDOWS) && System.getProperty('tests.timeoutSuite') == null) {
-        // override the suite timeout to 30 mins for windows, because it has the most inefficient filesystem known to man
-        systemProperty 'tests.timeoutSuite', '1800000!'
-      }
-    }
   }
 
   processTestResources {

gradle/build-scan.gradle

@@ -49,4 +49,4 @@ buildScan {
     } else {
         tag 'LOCAL'
     }
-}
+}

qa/os/build.gradle

@@ -29,6 +29,9 @@ dependencies {
   compile "org.apache.httpcomponents:httpcore:${versions.httpcore}"
   compile "org.apache.httpcomponents:httpclient:${versions.httpclient}"
   compile "org.apache.httpcomponents:fluent-hc:${versions.httpclient}"
+  compile "org.apache.logging.log4j:log4j-api:${versions.log4j}"
+  compile "org.apache.logging.log4j:log4j-core:${versions.log4j}"
+  compile "org.apache.logging.log4j:log4j-jcl:${versions.log4j}"
   compile "commons-codec:commons-codec:${versions.commonscodec}"
   compile "commons-logging:commons-logging:${versions.commonslogging}"
 
@@ -48,24 +51,16 @@ testingConventions.enabled = false
 tasks.dependencyLicenses.enabled = false
 tasks.dependenciesInfo.enabled = false
 
-tasks.thirdPartyAudit.ignoreMissingClasses (
-        // commons-logging optional dependencies
-        'org.apache.avalon.framework.logger.Logger',
-        'org.apache.log.Hierarchy',
-        'org.apache.log.Logger',
-        'org.apache.log4j.Category',
-        'org.apache.log4j.Level',
-        'org.apache.log4j.Logger',
-        'org.apache.log4j.Priority',
-        // commons-logging provided dependencies
-        'javax.servlet.ServletContextEvent',
-        'javax.servlet.ServletContextListener'
-)
+tasks.thirdPartyAudit.ignoreMissingClasses ()
 
 tasks.register('destructivePackagingTest') {
   dependsOn 'destructiveDistroTest', 'destructiveBatsTest.oss', 'destructiveBatsTest.default'
 }
 
+processTestResources {
+  from project(":test:framework").file("src/main/resources/log4j2-test.properties")
+}
+
 subprojects { Project platformProject ->
 
   // TODO: remove this property lookup once CI is switched to use an explicit task for the sample tests

qa/os/src/test/java/org/elasticsearch/packaging/test/ArchiveTests.java

@@ -115,13 +115,10 @@ public class ArchiveTests extends PackagingTestCase {
         // 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; " +
-                "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
-                "$acl = Get-Acl '" + installation.config("elasticsearch.keystore") + "'; " +
-                "$acl.SetOwner($account); " +
-                "Set-Acl '" + installation.config("elasticsearch.keystore") + "' $acl"
-        ));
+        Platforms.onWindows(() -> {
+            sh.run(bin.elasticsearchKeystore + " create");
+            sh.chown(installation.config("elasticsearch.keystore"));
+        });
 
         assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));
 
@@ -148,27 +145,23 @@ public class ArchiveTests extends PackagingTestCase {
         Archives.stopElasticsearch(installation);
     }
 
-    public void assertRunsWithJavaHome() throws Exception {
+    public void test51JavaHomeOverride() throws Exception {
         Platforms.onLinux(() -> {
-            String systemJavaHome = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
-            sh.getEnv().put("JAVA_HOME", systemJavaHome);
+            String systemJavaHome1 = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
+            sh.getEnv().put("JAVA_HOME", systemJavaHome1);
         });
         Platforms.onWindows(() -> {
-            final String systemJavaHome = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim();
-            sh.getEnv().put("JAVA_HOME", systemJavaHome);
+            final String systemJavaHome1 = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim();
+            sh.getEnv().put("JAVA_HOME", systemJavaHome1);
         });
 
         Archives.runElasticsearch(installation, sh);
         ServerUtils.runElasticsearchTests();
         Archives.stopElasticsearch(installation);
 
-        String systemJavaHome = sh.getEnv().get("JAVA_HOME");
+        String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
         assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
-            containsString(systemJavaHome));
-    }
-
-    public void test51JavaHomeOverride() throws Exception {
-        assertRunsWithJavaHome();
+            containsString(systemJavaHome1));
     }
 
     public void test52BundledJdkRemoved() throws Exception {
@@ -177,7 +170,22 @@ public class ArchiveTests extends PackagingTestCase {
         Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated");
         try {
             mv(installation.bundledJdk, relocatedJdk);
-            assertRunsWithJavaHome();
+            Platforms.onLinux(() -> {
+                String systemJavaHome1 = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
+                sh.getEnv().put("JAVA_HOME", systemJavaHome1);
+            });
+            Platforms.onWindows(() -> {
+                final String systemJavaHome1 = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim();
+                sh.getEnv().put("JAVA_HOME", systemJavaHome1);
+            });
+
+            Archives.runElasticsearch(installation, sh);
+            ServerUtils.runElasticsearchTests();
+            Archives.stopElasticsearch(installation);
+
+            String systemJavaHome1 = sh.getEnv().get("JAVA_HOME");
+            assertThat(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz"),
+                containsString(systemJavaHome1));
         } finally {
             mv(relocatedJdk, installation.bundledJdk);
         }
@@ -185,10 +193,11 @@ public class ArchiveTests extends PackagingTestCase {
 
     public void test53JavaHomeWithSpecialCharacters() throws Exception {
         Platforms.onWindows(() -> {
-            final Shell sh = newShell();
+            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
-                sh.run("cmd /c mklink /D 'C:\\Program Files (x86)\\java' $Env:SYSTEM_JAVA_HOME");
+                sh.run("cmd /c mklink /D '" + javaPath + "' $Env:SYSTEM_JAVA_HOME");
 
                 sh.getEnv().put("JAVA_HOME", "C:\\Program Files (x86)\\java");
 
@@ -203,7 +212,9 @@ public class ArchiveTests extends PackagingTestCase {
 
             } finally {
                 //clean up sym link
-                sh.run("cmd /c rmdir 'C:\\Program Files (x86)\\java' ");
+                if (Files.exists(Paths.get(javaPath))) {
+                    sh.run("cmd /c rmdir '" + javaPath + "' ");
+                }
             }
         });
 
@@ -231,6 +242,7 @@ public class ArchiveTests extends PackagingTestCase {
     }
 
     public void test60AutoCreateKeystore() throws Exception {
+        sh.chown(installation.config("elasticsearch.keystore"));
         assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));
 
         final Installation.Executables bin = installation.executables();
@@ -263,17 +275,8 @@ public class ArchiveTests extends PackagingTestCase {
                 "-Dlog4j2.disable.jmx=true\n";
             append(tempConf.resolve("jvm.options"), jvmOptions);
 
-            Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + tempConf));
-            Platforms.onWindows(() -> sh.run(
-                "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
-                "$tempConf = Get-ChildItem '" + tempConf + "' -Recurse; " +
-                "$tempConf += Get-Item '" + tempConf + "'; " +
-                "$tempConf | ForEach-Object { " +
-                    "$acl = Get-Acl $_.FullName; " +
-                    "$acl.SetOwner($account); " +
-                    "Set-Acl $_.FullName $acl " +
-                "}"
-            ));
+            final Shell sh = newShell();
+            sh.chown(tempConf);
 
             sh.getEnv().put("ES_PATH_CONF", tempConf.toString());
             sh.getEnv().put("ES_JAVA_OPTS", "-XX:-UseCompressedOops");
@@ -306,17 +309,8 @@ public class ArchiveTests extends PackagingTestCase {
 
             append(tempConf.resolve("elasticsearch.yml"), "node.name: relative");
 
-            Platforms.onLinux(() -> sh.run("chown -R elasticsearch:elasticsearch " + temp));
-            Platforms.onWindows(() -> sh.run(
-                "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
-                "$tempConf = Get-ChildItem '" + temp + "' -Recurse; " +
-                "$tempConf += Get-Item '" + temp + "'; " +
-                "$tempConf | ForEach-Object { " +
-                    "$acl = Get-Acl $_.FullName; " +
-                    "$acl.SetOwner($account); " +
-                    "Set-Acl $_.FullName $acl " +
-                "}"
-            ));
+            final Shell sh = newShell();
+            sh.chown(temp);
 
             sh.setWorkingDirectory(temp);
             sh.getEnv().put("ES_PATH_CONF", "config");

qa/os/src/test/java/org/elasticsearch/packaging/test/PackageTests.java

@@ -100,7 +100,7 @@ public class PackageTests extends PackagingTestCase {
         try {
             Files.write(installation.envFile, ("JAVA_HOME=" + systemJavaHome + "\n").getBytes(StandardCharsets.UTF_8),
                 StandardOpenOption.APPEND);
-            startElasticsearch(sh);
+            startElasticsearch(sh, installation);
             runElasticsearchTests();
             stopElasticsearch(sh);
         } finally {
@@ -121,18 +121,19 @@ public class PackageTests extends PackagingTestCase {
     public void test33RunsIfJavaNotOnPath() throws Exception {
         assumeThat(distribution().hasJdk, is(true));
 
-        final Result readlink = sh.run("readlink /usr/bin/java");
-        boolean unlinked = false;
-        try {
-            sh.run("unlink /usr/bin/java");
-            unlinked = true;
+        // we don't require java be installed but some images have it
+        String backupPath = "/usr/bin/java." + getClass().getSimpleName() + ".bak";
+        if (Files.exists(Paths.get("/usr/bin/java"))) {
+            sh.run("sudo mv /usr/bin/java " + backupPath);
+        }
 
-            startElasticsearch(sh);
+        try {
+            startElasticsearch(sh, installation);
             runElasticsearchTests();
             stopElasticsearch(sh);
         } finally {
-            if (unlinked) {
-                sh.run("ln -sf " + readlink.stdout.trim() + " /usr/bin/java");
+            if (Files.exists(Paths.get(backupPath))) {
+                sh.run("sudo mv " + backupPath + " /usr/bin/java");
             }
         }
     }
@@ -151,7 +152,7 @@ public class PackageTests extends PackagingTestCase {
 
     public void test40StartServer() throws Exception {
         String start = sh.runIgnoreExitCode("date ").stdout.trim();
-        startElasticsearch(sh);
+        startElasticsearch(sh, installation);
 
         String journalEntries = sh.runIgnoreExitCode("journalctl _SYSTEMD_UNIT=elasticsearch.service " +
             "--since \"" + start + "\" --output cat | wc -l").stdout.trim();
@@ -229,8 +230,8 @@ public class PackageTests extends PackagingTestCase {
             installation = installPackage(distribution());
             assertInstalled(distribution());
 
-            startElasticsearch(sh);
-            restartElasticsearch(sh);
+            startElasticsearch(sh, installation);
+            restartElasticsearch(sh, installation);
             runElasticsearchTests();
             stopElasticsearch(sh);
         } finally {
@@ -243,7 +244,7 @@ public class PackageTests extends PackagingTestCase {
         try {
             installation = installPackage(distribution());
             FileUtils.rm(installation.pidDir);
-            startElasticsearch(sh);
+            startElasticsearch(sh, installation);
             assertPathsExist(installation.pidDir);
             stopElasticsearch(sh);
         } finally {
@@ -253,7 +254,7 @@ public class PackageTests extends PackagingTestCase {
 
     public void test73gcLogsExist() throws Exception {
         installation = installPackage(distribution());
-        startElasticsearch(sh);
+        startElasticsearch(sh, installation);
         // it can be gc.log or gc.log.0.current
         assertThat(installation.logs, fileWithGlobExist("gc.log*"));
         stopElasticsearch(sh);
@@ -275,7 +276,7 @@ public class PackageTests extends PackagingTestCase {
 
         sh.run("systemd-tmpfiles --create");
 
-        startElasticsearch(sh);
+        startElasticsearch(sh, installation);
 
         final Path pidFile = installation.pidDir.resolve("elasticsearch.pid");
 
@@ -318,7 +319,7 @@ public class PackageTests extends PackagingTestCase {
             append(installation.envFile, "ES_PATH_CONF=" + tempConf + "\n");
             append(installation.envFile, "ES_JAVA_OPTS=-XX:-UseCompressedOops");
 
-            startElasticsearch(sh);
+            startElasticsearch(sh, installation);
 
             final String nodesResponse = makeRequest(Request.Get("http://localhost:9200/_nodes"));
             assertThat(nodesResponse, CoreMatchers.containsString("\"heap_init_in_bytes\":536870912"));
@@ -354,7 +355,7 @@ public class PackageTests extends PackagingTestCase {
 
         installation = installPackage(distribution());
 
-        startElasticsearch(sh);
+        startElasticsearch(sh, installation);
 
         final Path pidFile = installation.pidDir.resolve("elasticsearch.pid");
         assertTrue(Files.exists(pidFile));

qa/os/src/test/java/org/elasticsearch/packaging/test/PackagingTestCase.java

@@ -23,8 +23,9 @@ import com.carrotsearch.randomizedtesting.JUnit3MethodProvider;
 import com.carrotsearch.randomizedtesting.RandomizedRunner;
 import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering;
 import com.carrotsearch.randomizedtesting.annotations.TestMethodProviders;
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import com.carrotsearch.randomizedtesting.annotations.Timeout;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.packaging.util.Distribution;
 import org.elasticsearch.packaging.util.Installation;
 import org.elasticsearch.packaging.util.Platforms;
@@ -52,10 +53,11 @@ import static org.junit.Assume.assumeTrue;
 @TestMethodProviders({
     JUnit3MethodProvider.class
 })
+@Timeout(millis = 20 * 60 * 1000) // 20 min
 @TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
 public abstract class PackagingTestCase extends Assert {
 
-    protected final Log logger = LogFactory.getLog(getClass());
+    protected final Logger logger =  LogManager.getLogger(getClass());
 
     // the distribution being tested
     protected static final Distribution distribution;
@@ -129,5 +131,4 @@ public abstract class PackagingTestCase extends Assert {
         }
         return sh;
     }
-
 }

qa/os/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java

@@ -161,7 +161,7 @@ public class WindowsServiceTests extends PackagingTestCase {
 
     // NOTE: service description is not attainable through any powershell api, so checking it is not possible...
     public void assertStartedAndStop() throws IOException {
-        ServerUtils.waitForElasticsearch();
+        ServerUtils.waitForElasticsearch(installation);
         ServerUtils.runElasticsearchTests();
 
         assertCommand(serviceScript + " stop");

qa/os/src/test/java/org/elasticsearch/packaging/util/Archives.java

@@ -19,8 +19,8 @@
 
 package org.elasticsearch.packaging.util;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -48,6 +48,7 @@ import static org.hamcrest.collection.IsCollectionWithSize.hasSize;
 import static org.hamcrest.collection.IsEmptyCollection.empty;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsNot.not;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -55,11 +56,11 @@ import static org.junit.Assert.assertTrue;
  */
 public class Archives {
 
-    private static final Log logger = LogFactory.getLog(Archives.class);
+    protected static final Logger logger =  LogManager.getLogger(Archives.class);
 
     // in the future we'll run as a role user on Windows
     public static final String ARCHIVE_OWNER = Platforms.WINDOWS
-        ? "vagrant"
+        ? System.getenv("username")
         : "elasticsearch";
 
     public static Installation installArchive(Distribution distribution) throws Exception {
@@ -107,7 +108,8 @@ public class Archives {
         assertThat("only the intended installation exists", installations.get(0), is(fullInstallPath));
 
         Platforms.onLinux(() -> setupArchiveUsersLinux(fullInstallPath));
-        Platforms.onWindows(() -> setupArchiveUsersWindows(fullInstallPath));
+
+        sh.chown(fullInstallPath);
 
         return Installation.ofArchive(fullInstallPath);
     }
@@ -143,23 +145,6 @@ public class Archives {
                     "elasticsearch");
             }
         }
-        sh.run("chown -R elasticsearch:elasticsearch " + installPath);
-    }
-
-    private static void setupArchiveUsersWindows(Path installPath) {
-        // we want the installation to be owned as the vagrant user rather than the Administrators group
-
-        final Shell sh = new Shell();
-        sh.run(
-            "$account = New-Object System.Security.Principal.NTAccount 'vagrant'; " +
-            "$install = Get-ChildItem -Path '" + installPath + "' -Recurse; " +
-            "$install += Get-Item -Path '" + installPath + "'; " +
-            "$install | ForEach-Object { " +
-                "$acl = Get-Acl $_.FullName; " +
-                "$acl.SetOwner($account); " +
-                "Set-Acl $_.FullName $acl " +
-            "}"
-        );
     }
 
     public static void verifyArchiveInstallation(Installation installation, Distribution distribution) {
@@ -260,6 +245,8 @@ public class Archives {
     public static void runElasticsearch(Installation installation, Shell sh) throws Exception {
         final Path pidFile = installation.home.resolve("elasticsearch.pid");
 
+        assertFalse("Pid file doesn't exist when starting Elasticsearch", Files.exists(pidFile));
+
         final Installation.Executables bin = installation.executables();
 
         Platforms.onLinux(() -> {
@@ -275,31 +262,50 @@ public class Archives {
 
         Platforms.onWindows(() -> {
             // this starts the server in the background. the -d flag is unsupported on windows
-            // these tests run as Administrator. we don't want to run the server as Administrator, so we provide the current user's
-            // username and password to the process which has the effect of starting it not as Administrator.
-            sh.run(
-                "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; " +
-                "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; " +
-                "$processInfo.FileName = '" + bin.elasticsearch + "'; " +
-                "$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " +
-                "$processInfo.Username = 'vagrant'; " +
-                "$processInfo.Password = $password; " +
-                "$processInfo.RedirectStandardOutput = $true; " +
-                "$processInfo.RedirectStandardError = $true; " +
-                sh.env.entrySet().stream()
-                    .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
-                    .collect(joining()) +
-                "$processInfo.UseShellExecute = $false; " +
-                "$process = New-Object System.Diagnostics.Process; " +
-                "$process.StartInfo = $processInfo; " +
-                "$process.Start() | Out-Null; " +
-                "$process.Id;"
-            );
+            if (System.getenv("username").equals("vagrant")) {
+                // these tests run as Administrator in vagrant.
+                // we don't want to run the server as Administrator, so we provide the current user's
+                // username and password to the process which has the effect of starting it not as Administrator.
+                sh.run(
+                    "$password = ConvertTo-SecureString 'vagrant' -AsPlainText -Force; " +
+                        "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; " +
+                        "$processInfo.FileName = '" + bin.elasticsearch + "'; " +
+                        "$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " +
+                        "$processInfo.Username = 'vagrant'; " +
+                        "$processInfo.Password = $password; " +
+                        "$processInfo.RedirectStandardOutput = $true; " +
+                        "$processInfo.RedirectStandardError = $true; " +
+                        sh.env.entrySet().stream()
+                            .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
+                            .collect(joining()) +
+                        "$processInfo.UseShellExecute = $false; " +
+                        "$process = New-Object System.Diagnostics.Process; " +
+                        "$process.StartInfo = $processInfo; " +
+                        "$process.Start() | Out-Null; " +
+                        "$process.Id;"
+                );
+            } else {
+                sh.run(
+                        "$processInfo = New-Object System.Diagnostics.ProcessStartInfo; " +
+                        "$processInfo.FileName = '" + bin.elasticsearch + "'; " +
+                        "$processInfo.Arguments = '-p " + installation.home.resolve("elasticsearch.pid") + "'; " +
+                        "$processInfo.RedirectStandardOutput = $true; " +
+                        "$processInfo.RedirectStandardError = $true; " +
+                        sh.env.entrySet().stream()
+                            .map(entry -> "$processInfo.Environment.Add('" + entry.getKey() + "', '" + entry.getValue() + "'); ")
+                            .collect(joining()) +
+                        "$processInfo.UseShellExecute = $false; " +
+                        "$process = New-Object System.Diagnostics.Process; " +
+                        "$process.StartInfo = $processInfo; " +
+                        "$process.Start() | Out-Null; " +
+                        "$process.Id;"
+                );
+            }
         });
 
-        ServerUtils.waitForElasticsearch();
+        ServerUtils.waitForElasticsearch(installation);
 
-        assertTrue(Files.exists(pidFile));
+        assertTrue("Starting Elasticsearch produced a pid file at " + pidFile, Files.exists(pidFile));
         String pid = slurp(pidFile).trim();
         assertThat(pid, not(isEmptyOrNullString()));
 
@@ -316,6 +322,9 @@ public class Archives {
         final Shell sh = new Shell();
         Platforms.onLinux(() -> sh.run("kill -SIGTERM " + pid + "; tail --pid=" + pid + " -f /dev/null"));
         Platforms.onWindows(() -> sh.run("Get-Process -Id " + pid + " | Stop-Process -Force; Wait-Process -Id " + pid));
+        if (Files.exists(pidFile)) {
+            Files.delete(pidFile);
+        }
     }
 
 }

qa/os/src/test/java/org/elasticsearch/packaging/util/FileUtils.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.packaging.util;
 
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.core.internal.io.IOUtils;
 import org.hamcrest.FeatureMatcher;
 import org.hamcrest.Matcher;
@@ -26,6 +27,7 @@ import org.hamcrest.Matcher;
 import java.io.BufferedWriter;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.nio.channels.Channels;
 import java.nio.channels.FileChannel;
 import java.nio.charset.StandardCharsets;
@@ -43,6 +45,7 @@ import java.util.Collections;
 import java.util.List;
 import java.util.StringJoiner;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 import java.util.zip.GZIPInputStream;
 import java.util.zip.ZipException;
 
@@ -179,6 +182,30 @@ public class FileUtils {
         }
     }
 
+    public static void logAllLogs(Path logsDir, Logger logger) {
+        if (Files.exists(logsDir) == false) {
+            logger.warn("Can't show logs from directory {} as it doesn't exists", logsDir);
+            return;
+        }
+        logger.info("Showing contents of directory: {} ({})", logsDir, logsDir.toAbsolutePath());
+        try (Stream<Path> fileStream = Files.list(logsDir)) {
+            fileStream
+                // gc logs are verbose and not useful in this context
+                .filter(file -> file.getFileName().toString().startsWith("gc.log") == false)
+                .forEach(file -> {
+                logger.info("=== Contents of `{}` ({}) ===", file, file.toAbsolutePath());
+                try (Stream<String> stream = Files.lines(file)) {
+                    stream.forEach(logger::info);
+                } catch (IOException e) {
+                    logger.error("Can't show contents", e);
+                }
+                logger.info("=== End of contents of `{}`===", file);
+            });
+        } catch (IOException e) {
+            logger.error("Can't list log files", e);
+        }
+    }
+
     /**
      * Gets the owner of a file in a way that should be supported by all filesystems that have a concept of file owner
      */
@@ -258,4 +285,14 @@ public class FileUtils {
     public static void assertPathsDontExist(Path... paths) {
         Arrays.stream(paths).forEach(path -> assertFalse(path + " should not exist", Files.exists(path)));
     }
+
+    public static void deleteIfExists(Path path) {
+        if (Files.exists(path)) {
+            try {
+                Files.delete(path);
+            } catch (IOException e) {
+                throw new UncheckedIOException(e);
+            }
+        }
+    }
 }

qa/os/src/test/java/org/elasticsearch/packaging/util/Packages.java

@@ -19,8 +19,8 @@
 
 package org.elasticsearch.packaging.util;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.packaging.util.Shell.Result;
 
 import java.io.IOException;
@@ -52,7 +52,7 @@ import static org.junit.Assert.assertTrue;
 
 public class Packages {
 
-    private static final Log logger = LogFactory.getLog(Packages.class);
+    protected static final Logger logger =  LogManager.getLogger(Packages.class);
 
     public static final Path SYSVINIT_SCRIPT = Paths.get("/etc/init.d/elasticsearch");
     public static final Path SYSTEMD_SERVICE = Paths.get("/usr/lib/systemd/system/elasticsearch.service");
@@ -267,7 +267,7 @@ public class Packages {
         ).forEach(configFile -> assertThat(es.config(configFile), file(File, "root", "elasticsearch", p660)));
     }
 
-    public static void startElasticsearch(Shell sh) throws IOException {
+    public static void startElasticsearch(Shell sh, Installation installation) throws IOException {
         if (isSystemd()) {
             sh.run("systemctl daemon-reload");
             sh.run("systemctl enable elasticsearch.service");
@@ -277,11 +277,11 @@ public class Packages {
             sh.run("service elasticsearch start");
         }
 
-        assertElasticsearchStarted(sh);
+        assertElasticsearchStarted(sh, installation);
     }
 
-    public static void assertElasticsearchStarted(Shell sh) throws IOException {
-        waitForElasticsearch();
+    public static void assertElasticsearchStarted(Shell sh, Installation installation) throws IOException {
+        waitForElasticsearch(installation);
 
         if (isSystemd()) {
             sh.run("systemctl is-active elasticsearch.service");
@@ -299,13 +299,13 @@ public class Packages {
         }
     }
 
-    public static void restartElasticsearch(Shell sh) throws IOException {
+    public static void restartElasticsearch(Shell sh, Installation installation) throws IOException {
         if (isSystemd()) {
             sh.run("systemctl restart elasticsearch.service");
         } else {
             sh.run("service elasticsearch restart");
         }
 
-        waitForElasticsearch();
+        waitForElasticsearch(installation);
     }
 }

qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java

@@ -19,13 +19,12 @@
 
 package org.elasticsearch.packaging.util;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.fluent.Request;
-import org.apache.http.conn.HttpHostConnectException;
 import org.apache.http.entity.ContentType;
 import org.apache.http.util.EntityUtils;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.io.IOException;
 import java.util.Objects;
@@ -36,16 +35,16 @@ import static org.hamcrest.Matchers.containsString;
 
 public class ServerUtils {
 
-    private static final Log LOG = LogFactory.getLog(ServerUtils.class);
+    protected static final Logger logger =  LogManager.getLogger(ServerUtils.class);
 
     private static final long waitTime = TimeUnit.SECONDS.toMillis(60);
     private static final long timeoutLength = TimeUnit.SECONDS.toMillis(10);
 
-    public static void waitForElasticsearch() throws IOException {
-        waitForElasticsearch("green", null);
+    public static void waitForElasticsearch(Installation installation) throws IOException {
+        waitForElasticsearch("green", null, installation);
     }
 
-    public static void waitForElasticsearch(String status, String index) throws IOException {
+    public static void waitForElasticsearch(String status, String index, Installation installation) throws IOException {
 
         Objects.requireNonNull(status);
 
@@ -70,15 +69,17 @@ public class ServerUtils {
 
                 started = true;
 
-            } catch (HttpHostConnectException e) {
-                // we want to retry if the connection is refused
-                LOG.info("Got connection refused when waiting for cluster health", e);
+            } catch (IOException e) {
+                logger.info("Got exception when waiting for cluster health", e);
             }
 
             timeElapsed = System.currentTimeMillis() - startTime;
         }
 
         if (started == false) {
+            if (installation != null) {
+                FileUtils.logAllLogs(installation.logs, logger);
+            }
             throw new RuntimeException("Elasticsearch did not start");
         }
 

qa/os/src/test/java/org/elasticsearch/packaging/util/Shell.java

@@ -19,17 +19,22 @@
 
 package org.elasticsearch.packaging.util;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.common.SuppressForbidden;
 
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
+import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
-import java.util.Objects;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 /**
@@ -37,11 +42,13 @@ import java.util.stream.Stream;
  */
 public class Shell {
 
-    final Map<String, String> env;
+    public static final int TAIL_WHEN_TOO_MUCH_OUTPUT = 1000;
+    protected final Logger logger =  LogManager.getLogger(getClass());
+
+    final Map<String, String> env = new HashMap<>();
     Path workingDirectory;
 
     public Shell() {
-        this.env = new HashMap<>();
         this.workingDirectory = null;
     }
 
@@ -68,6 +75,20 @@ public class Shell {
         return runScriptIgnoreExitCode(getScriptCommand(script));
     }
 
+    public void chown(Path path) throws Exception {
+        Platforms.onLinux(() -> run("chown -R elasticsearch:elasticsearch " + path));
+        Platforms.onWindows(() -> run(
+            "$account = New-Object System.Security.Principal.NTAccount '" + System.getenv("username")  + "'; " +
+                "$tempConf = Get-ChildItem '" + path + "' -Recurse; " +
+                "$tempConf += Get-Item '" + path + "'; " +
+                "$tempConf | ForEach-Object { " +
+                "$acl = Get-Acl $_.FullName; " +
+                "$acl.SetOwner($account); " +
+                "Set-Acl $_.FullName $acl " +
+                "}"
+        ));
+    }
+
     public Result run( String command, Object... args) {
         String formattedCommand = String.format(Locale.ROOT, command, args);
         return run(formattedCommand);
@@ -91,7 +112,7 @@ public class Shell {
     private Result runScript(String[] command) {
         Result result = runScriptIgnoreExitCode(command);
         if (result.isSuccess() == false) {
-            throw new RuntimeException("Command was not successful: [" + String.join(" ", command) + "] result: " + result.toString());
+            throw new RuntimeException("Command was not successful: [" + String.join(" ", command) + "]\n   result: " + result.toString());
         }
         return result;
     }
@@ -99,43 +120,81 @@ public class Shell {
     private Result runScriptIgnoreExitCode(String[] command) {
         ProcessBuilder builder = new ProcessBuilder();
         builder.command(command);
-
-
         if (workingDirectory != null) {
             setWorkingDirectory(builder, workingDirectory);
         }
-
-        if (env != null && env.isEmpty() == false) {
-            for (Map.Entry<String, String> entry : env.entrySet()) {
-                builder.environment().put(entry.getKey(), entry.getValue());
-            }
+        for (Map.Entry<String, String> entry : env.entrySet()) {
+            builder.environment().put(entry.getKey(), entry.getValue());
         }
-
+        final Path stdOut;
+        final Path stdErr;
         try {
+            Path tmpDir = Paths.get(System.getProperty("java.io.tmpdir"));
+            Files.createDirectories(tmpDir);
+            stdOut = Files.createTempFile(tmpDir, getClass().getName(), ".out");
+            stdErr = Files.createTempFile(tmpDir, getClass().getName(), ".err");
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
 
-            Process process = builder.start();
-
-            StringBuilder stdout = new StringBuilder();
-            StringBuilder stderr = new StringBuilder();
-
-            Thread stdoutThread = new Thread(new StreamCollector(process.getInputStream(), stdout));
-            Thread stderrThread = new Thread(new StreamCollector(process.getErrorStream(), stderr));
-
-            stdoutThread.start();
-            stderrThread.start();
-
-            stdoutThread.join();
-            stderrThread.join();
-
-            int exitCode = process.waitFor();
+        redirectOutAndErr(builder, stdOut, stdErr);
 
-            return new Result(exitCode, stdout.toString(), stderr.toString());
+        try {
+            Process process = builder.start();
+            if (process.waitFor(10, TimeUnit.MINUTES) == false) {
+                if (process.isAlive()) {
+                    process.destroyForcibly();
+                }
+                Result result = new Result(
+                    -1,
+                    readFileIfExists(stdOut),
+                    readFileIfExists(stdErr)
+                );
+                throw new IllegalStateException(
+                    "Timed out running shell command: " + command + "\n" +
+                    "Result:\n" + result
+                );
+            }
 
-        } catch (IOException | InterruptedException e) {
+            Result result = new Result(
+                process.exitValue(),
+                readFileIfExists(stdOut),
+                readFileIfExists(stdErr)
+            );
+            logger.info("Ran: {} {}", Arrays.toString(command), result);
+            return result;
+
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
             throw new RuntimeException(e);
+        } finally {
+           FileUtils.deleteIfExists(stdOut);
+           FileUtils.deleteIfExists(stdErr);
+        }
+    }
+
+    private String readFileIfExists(Path path) throws IOException {
+        if (Files.exists(path)) {
+            long size = Files.size(path);
+            if (size > 100 * 1024) {
+                return "<<Too large to read: " + size  + " bytes>>";
+            }
+            try (Stream<String> lines = Files.lines(path, StandardCharsets.UTF_8)) {
+                return lines.collect(Collectors.joining("\n"));
+            }
+        } else {
+            return "";
         }
     }
 
+    @SuppressForbidden(reason = "ProcessBuilder expects java.io.File")
+    private void redirectOutAndErr(ProcessBuilder builder, Path stdOut, Path stdErr) {
+        builder.redirectOutput(stdOut.toFile());
+        builder.redirectError(stdErr.toFile());
+    }
+
     @SuppressForbidden(reason = "ProcessBuilder expects java.io.File")
     private static void setWorkingDirectory(ProcessBuilder builder, Path path) {
         builder.directory(path.toFile());
@@ -143,8 +202,6 @@ public class Shell {
 
     public String toString() {
         return new StringBuilder()
-            .append("<")
-            .append(this.getClass().getName())
             .append(" ")
             .append("env = [")
             .append(env)
@@ -152,7 +209,6 @@ public class Shell {
             .append("workingDirectory = [")
             .append(workingDirectory)
             .append("]")
-            .append(">")
             .toString();
     }
 
@@ -173,53 +229,17 @@ public class Shell {
 
         public String toString() {
             return new StringBuilder()
-                .append("<")
-                .append(this.getClass().getName())
-                .append(" ")
                 .append("exitCode = [")
                 .append(exitCode)
-                .append("]")
-                .append(" ")
+                .append("] ")
                 .append("stdout = [")
-                .append(stdout)
-                .append("]")
-                .append(" ")
+                .append(stdout.trim())
+                .append("] ")
                 .append("stderr = [")
-                .append(stderr)
+                .append(stderr.trim())
                 .append("]")
-                .append(">")
                 .toString();
         }
     }
 
-    private static class StreamCollector implements Runnable {
-        private final InputStream input;
-        private final Appendable appendable;
-
-        StreamCollector(InputStream input, Appendable appendable) {
-            this.input = Objects.requireNonNull(input);
-            this.appendable = Objects.requireNonNull(appendable);
-        }
-
-        public void run() {
-            try {
-
-                BufferedReader reader = new BufferedReader(reader(input));
-                String line;
-
-                while ((line = reader.readLine()) != null) {
-                    appendable.append(line);
-                    appendable.append("\n");
-                }
-
-            } catch (IOException e) {
-                throw new RuntimeException(e);
-            }
-        }
-
-        @SuppressForbidden(reason = "the system's default character set is a best guess of what subprocesses will use")
-        private static InputStreamReader reader(InputStream inputStream) {
-            return new InputStreamReader(inputStream);
-        }
-    }
 }