From 49c78624b4a111c2a55d495531590e0d3ae7e4ce Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 29 Aug 2019 14:20:54 -0700 Subject: [PATCH 1/5] Clarify missing java error message Since the bundled jdk was added to Elasticsearch, there are now 2 ways java can be missing. Either JAVA_HOME is set but does not exist, or the bundled jdk does not exist. This commit improves the error messages in those two cases, and also ensures our tests cover both cases. --- distribution/src/bin/elasticsearch-env | 14 +++++++++----- distribution/src/bin/elasticsearch-env.bat | 4 +++- .../packaging/test/ArchiveTests.java | 15 +++++++++++++-- .../packaging/test/WindowsServiceTests.java | 13 ++++++++++--- 4 files changed, 35 insertions(+), 11 deletions(-) diff --git a/distribution/src/bin/elasticsearch-env b/distribution/src/bin/elasticsearch-env index 78cb503ecef7c..9ca0ab755bc63 100644 --- a/distribution/src/bin/elasticsearch-env +++ b/distribution/src/bin/elasticsearch-env @@ -38,6 +38,11 @@ ES_CLASSPATH="$ES_HOME/lib/*" # now set the path to java if [ ! -z "$JAVA_HOME" ]; then JAVA="$JAVA_HOME/bin/java" + if [ ! -x "$JAVA" ]; then + echo "could not find java JAVA_HOME at $JAVA" >&2 + exit 1 + fi + else if [ "$(uname -s)" = "Darwin" ]; then # OSX has a different structure @@ -45,11 +50,10 @@ else else JAVA="$ES_HOME/jdk/bin/java" fi -fi - -if [ ! -x "$JAVA" ]; then - echo "could not find java in JAVA_HOME or bundled at $JAVA" >&2 - exit 1 + if [ ! -x "$JAVA" ]; then + echo "could not find java in bundled jdk at $JAVA" >&2 + exit 1 + fi fi # do not let JAVA_TOOL_OPTIONS slip in (as the JVM does by default) diff --git a/distribution/src/bin/elasticsearch-env.bat b/distribution/src/bin/elasticsearch-env.bat index 8ac141986a4a7..e5cf8cfc50df7 100644 --- a/distribution/src/bin/elasticsearch-env.bat +++ b/distribution/src/bin/elasticsearch-env.bat @@ -38,13 +38,15 @@ if "%1" == "nojava" ( if defined JAVA_HOME ( set JAVA="%JAVA_HOME%\bin\java.exe" + set JAVA_TYPE=JAVA_HOME ) else ( set JAVA="%ES_HOME%\jdk\bin\java.exe" set JAVA_HOME="%ES_HOME%\jdk" + set JAVA_TYPE=bundled jdk ) if not exist %JAVA% ( - echo "could not find java in JAVA_HOME or bundled at %ES_HOME%\jdk" >&2 + echo "could not find java in %JAVA_TYPE% at %JAVA%" >&2 exit /b 1 ) 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 76277cb23ebe2..87da7707a934a 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 @@ -74,7 +74,7 @@ public void test20PluginsListWithNoPlugins() throws Exception { assertThat(r.stdout, isEmptyString()); } - public void test30NoJava() throws Exception { + public void test30MissingBundledJdk() throws Exception { final Installation.Executables bin = installation.executables(); sh.getEnv().remove("JAVA_HOME"); @@ -87,7 +87,7 @@ public void test30NoJava() throws Exception { // ask for elasticsearch version to quickly exit if java is actually found (ie test failure) final Result runResult = sh.runIgnoreExitCode(bin.elasticsearch.toString() + " -v"); assertThat(runResult.exitCode, is(1)); - assertThat(runResult.stderr, containsString("could not find java in JAVA_HOME or bundled")); + assertThat(runResult.stderr, containsString("could not find java in bundled jdk")); } finally { if (distribution().hasJdk) { mv(relocatedJdk, installation.bundledJdk); @@ -95,6 +95,17 @@ public void test30NoJava() throws Exception { } } + public void test31BadJavaHome() throws Exception { + final Installation.Executables bin = installation.executables(); + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + + // ask for elasticsearch version to quickly exit if java is actually found (ie test failure) + final Result runResult = sh.runIgnoreExitCode(bin.elasticsearch.toString() + " -v"); + assertThat(runResult.exitCode, is(1)); + assertThat(runResult.stderr, containsString("could not find java in JAVA_HOME")); + + } + public void test40CreateKeystoreManually() throws Exception { final Installation.Executables bin = installation.executables(); 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 77e83c9522857..9d32e4b2b0256 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 @@ -117,20 +117,27 @@ public void test12InstallService() { sh.run(serviceScript + " remove"); } - public void test13InstallMissingJava() throws IOException { + public void test13InstallMissingBundledJdk() throws IOException { final Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); try { mv(installation.bundledJdk, relocatedJdk); Result result = sh.runIgnoreExitCode(serviceScript + " install"); assertThat(result.exitCode, equalTo(1)); - assertThat(result.stderr, containsString("could not find java in JAVA_HOME or bundled")); + assertThat(result.stderr, containsString("could not find java in bundled jdk")); } finally { mv(relocatedJdk, installation.bundledJdk); } } - public void test14RemoveNotInstalled() { + public void test14InstallBadJavaHome() throws IOException { + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + Result result = sh.runIgnoreExitCode(serviceScript + " install"); + assertThat(result.exitCode, equalTo(1)); + assertThat(result.stderr, containsString("could not find java in JAVA_HOME")); + } + + public void test15RemoveNotInstalled() { Result result = assertFailure(serviceScript + " remove", 1); assertThat(result.stdout, containsString("Failed removing '" + DEFAULT_ID + "' service")); } From cd8dce8b046942f003dd00753134ecfbab9e2b8b Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 29 Aug 2019 21:36:16 -0700 Subject: [PATCH 2/5] fix typo --- distribution/src/bin/elasticsearch-env | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/distribution/src/bin/elasticsearch-env b/distribution/src/bin/elasticsearch-env index 9ca0ab755bc63..27b68104cb3bd 100644 --- a/distribution/src/bin/elasticsearch-env +++ b/distribution/src/bin/elasticsearch-env @@ -39,7 +39,7 @@ ES_CLASSPATH="$ES_HOME/lib/*" if [ ! -z "$JAVA_HOME" ]; then JAVA="$JAVA_HOME/bin/java" if [ ! -x "$JAVA" ]; then - echo "could not find java JAVA_HOME at $JAVA" >&2 + echo "could not find java in JAVA_HOME at $JAVA" >&2 exit 1 fi From 32d8c5fec491e008d6551eaab99e4cf143a5d29e Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Tue, 10 Sep 2019 13:17:28 -0700 Subject: [PATCH 3/5] use similar structure for checking in windows and nix --- distribution/src/bin/elasticsearch-env | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/distribution/src/bin/elasticsearch-env b/distribution/src/bin/elasticsearch-env index 27b68104cb3bd..e13eb824e77e9 100644 --- a/distribution/src/bin/elasticsearch-env +++ b/distribution/src/bin/elasticsearch-env @@ -38,11 +38,7 @@ ES_CLASSPATH="$ES_HOME/lib/*" # now set the path to java if [ ! -z "$JAVA_HOME" ]; then JAVA="$JAVA_HOME/bin/java" - if [ ! -x "$JAVA" ]; then - echo "could not find java in JAVA_HOME at $JAVA" >&2 - exit 1 - fi - + JAVA_TYPE="JAVA_HOME" else if [ "$(uname -s)" = "Darwin" ]; then # OSX has a different structure @@ -50,11 +46,13 @@ else else JAVA="$ES_HOME/jdk/bin/java" fi - if [ ! -x "$JAVA" ]; then - echo "could not find java in bundled jdk at $JAVA" >&2 + JAVA_TYPE="bundled jdk" +fi + +if [ ! -x "$JAVA" ]; then + echo "could not find java in $JAVA_TYPE at $JAVA" >&2 exit 1 fi -fi # do not let JAVA_TOOL_OPTIONS slip in (as the JVM does by default) if [ ! -z "$JAVA_TOOL_OPTIONS" ]; then From ccbd4af9e858b5e02f2b1c92afb0bf6786cc9a1a Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Wed, 11 Sep 2019 09:48:38 -0700 Subject: [PATCH 4/5] debugging --- .../org/elasticsearch/packaging/test/ArchiveTests.java | 7 +++++-- .../java/org/elasticsearch/packaging/util/ServerUtils.java | 2 +- 2 files changed, 6 insertions(+), 3 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 87da7707a934a..b7263e6fccdea 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 @@ -185,7 +185,7 @@ public void test52BundledJdkRemoved() throws Exception { public void test53JavaHomeWithSpecialCharacters() throws Exception { Platforms.onWindows(() -> { - final Shell sh = new Shell(); + final Shell sh = newShell(); 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"); @@ -201,6 +201,9 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { Result result = sh.run(pluginListCommand); assertThat(result.exitCode, equalTo(0)); + } catch (RuntimeException e) { + logger.error(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz")); + throw e; } finally { //clean up sym link sh.run("cmd /c rmdir 'C:\\Program Files (x86)\\java' "); @@ -208,7 +211,7 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { }); Platforms.onLinux(() -> { - final Shell sh = new Shell(); + final Shell sh = newShell(); // Create temporary directory with a space and link to java binary. // Use it as java_home String testJavaHome = FileUtils.mkdir(Paths.get("/home", ARCHIVE_OWNER, "java home")).toAbsolutePath().toString(); diff --git a/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java b/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java index 6331b4bf46e9a..ff006a34e6892 100644 --- a/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java +++ b/qa/os/src/test/java/org/elasticsearch/packaging/util/ServerUtils.java @@ -72,7 +72,7 @@ public static void waitForElasticsearch(String status, String index) throws IOEx } catch (HttpHostConnectException e) { // we want to retry if the connection is refused - LOG.debug("Got connection refused when waiting for cluster health", e); + LOG.info("Got connection refused when waiting for cluster health", e); } timeElapsed = System.currentTimeMillis() - startTime; From 0f49f17e4eac47b846d44b77aa6b85f2e7fcd537 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Thu, 12 Sep 2019 15:07:17 -0700 Subject: [PATCH 5/5] use delayed expansion in windows bat script --- distribution/src/bin/elasticsearch-env.bat | 4 ++-- .../java/org/elasticsearch/packaging/test/ArchiveTests.java | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/distribution/src/bin/elasticsearch-env.bat b/distribution/src/bin/elasticsearch-env.bat index e5cf8cfc50df7..c974792088181 100644 --- a/distribution/src/bin/elasticsearch-env.bat +++ b/distribution/src/bin/elasticsearch-env.bat @@ -45,8 +45,8 @@ if defined JAVA_HOME ( set JAVA_TYPE=bundled jdk ) -if not exist %JAVA% ( - echo "could not find java in %JAVA_TYPE% at %JAVA%" >&2 +if not exist !JAVA! ( + echo "could not find java in !JAVA_TYPE! at !JAVA!" >&2 exit /b 1 ) 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 b7263e6fccdea..4df291d19e935 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 @@ -201,9 +201,6 @@ public void test53JavaHomeWithSpecialCharacters() throws Exception { Result result = sh.run(pluginListCommand); assertThat(result.exitCode, equalTo(0)); - } catch (RuntimeException e) { - logger.error(FileUtils.slurpAllLogs(installation.logs, "elasticsearch.log", "*.log.gz")); - throw e; } finally { //clean up sym link sh.run("cmd /c rmdir 'C:\\Program Files (x86)\\java' ");