From c8ac0373895fe6b884792569029bdc9d465850b5 Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Wed, 2 Feb 2022 13:55:22 -0500 Subject: [PATCH] [FEATURE] Add OPENSEARCH_JAVA_HOME env to override JAVA_HOME (#2001) Signed-off-by: Andriy Redko --- Vagrantfile | 1 + .../packages/src/common/env/opensearch | 2 +- .../packages/src/deb/init.d/opensearch | 3 +- .../packages/src/rpm/init.d/opensearch | 1 + distribution/src/bin/opensearch-env | 7 +- distribution/src/bin/opensearch-env.bat | 15 ++-- .../packaging/test/ArchiveTests.java | 70 ++++++++++++++++++- .../packaging/test/PackagingTestCase.java | 13 +++- .../packaging/test/WindowsServiceTests.java | 26 +++++++ 9 files changed, 125 insertions(+), 13 deletions(-) diff --git a/Vagrantfile b/Vagrantfile index 42eb492c650..28d5894664c 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -484,6 +484,7 @@ def sh_install_deps(config, cat \<\ /etc/sudoers.d/opensearch_vars Defaults env_keep += "JAVA_HOME" Defaults env_keep += "SYSTEM_JAVA_HOME" +Defaults env_keep += "OPENSEARCH_JAVA_HOME" SUDOERS_VARS chmod 0440 /etc/sudoers.d/opensearch_vars SHELL diff --git a/distribution/packages/src/common/env/opensearch b/distribution/packages/src/common/env/opensearch index 52a2639abbc..198bcfde90c 100644 --- a/distribution/packages/src/common/env/opensearch +++ b/distribution/packages/src/common/env/opensearch @@ -6,7 +6,7 @@ #OPENSEARCH_HOME=/usr/share/opensearch # OpenSearch Java path -#JAVA_HOME= +#OPENSEARCH_JAVA_HOME= # OpenSearch configuration directory # Note: this setting will be shared with command-line tools diff --git a/distribution/packages/src/deb/init.d/opensearch b/distribution/packages/src/deb/init.d/opensearch index cc95b465c88..e5195d2d54d 100755 --- a/distribution/packages/src/deb/init.d/opensearch +++ b/distribution/packages/src/deb/init.d/opensearch @@ -66,8 +66,9 @@ DAEMON=$OPENSEARCH_HOME/bin/opensearch DAEMON_OPTS="-d -p $PID_FILE" export OPENSEARCH_JAVA_OPTS -export JAVA_HOME export OPENSEARCH_PATH_CONF +export JAVA_HOME +export OPENSEARCH_JAVA_HOME if [ ! -x "$DAEMON" ]; then echo "The opensearch startup script does not exists or it is not executable, tried: $DAEMON" diff --git a/distribution/packages/src/rpm/init.d/opensearch b/distribution/packages/src/rpm/init.d/opensearch index d31fca9d2d1..12a1470e75a 100644 --- a/distribution/packages/src/rpm/init.d/opensearch +++ b/distribution/packages/src/rpm/init.d/opensearch @@ -53,6 +53,7 @@ export OPENSEARCH_JAVA_OPTS export JAVA_HOME export OPENSEARCH_PATH_CONF export OPENSEARCH_STARTUP_SLEEP_TIME +export OPENSEARCH_JAVA_HOME lockfile=/var/lock/subsys/$prog diff --git a/distribution/src/bin/opensearch-env b/distribution/src/bin/opensearch-env index 99bded2ad0e..6fe703a73b2 100644 --- a/distribution/src/bin/opensearch-env +++ b/distribution/src/bin/opensearch-env @@ -44,8 +44,11 @@ OPENSEARCH_HOME=`dirname "$OPENSEARCH_HOME"` # now set the classpath OPENSEARCH_CLASSPATH="$OPENSEARCH_HOME/lib/*" -# now set the path to java -if [ ! -z "$JAVA_HOME" ]; then +# now set the path to java: OPENSEARCH_JAVA_HOME -> JAVA_HOME -> bundled JDK +if [ ! -z "$OPENSEARCH_JAVA_HOME" ]; then + JAVA="$OPENSEARCH_JAVA_HOME/bin/java" + JAVA_TYPE="OPENSEARCH_JAVA_HOME" +elif [ ! -z "$JAVA_HOME" ]; then JAVA="$JAVA_HOME/bin/java" JAVA_TYPE="JAVA_HOME" else diff --git a/distribution/src/bin/opensearch-env.bat b/distribution/src/bin/opensearch-env.bat index 244aa4452d5..bc8a6ce53a5 100644 --- a/distribution/src/bin/opensearch-env.bat +++ b/distribution/src/bin/opensearch-env.bat @@ -39,16 +39,19 @@ if "%1" == "nojava" ( exit /b ) -rem compariing to empty string makes this equivalent to bash -v check on env var +rem comparing to empty string makes this equivalent to bash -v check on env var rem and allows to effectively force use of the bundled jdk when launching OpenSearch -rem by setting JAVA_HOME= -if "%JAVA_HOME%" == "" ( +rem by setting OPENSEARCH_JAVA_HOME= and JAVA_HOME= +if not "%OPENSEARCH_JAVA_HOME%" == "" ( + set JAVA="%OPENSEARCH_JAVA_HOME%\bin\java.exe" + set JAVA_TYPE=OPENSEARCH_JAVA_HOME +) else if not "%JAVA_HOME%" == "" ( + set JAVA="%JAVA_HOME%\bin\java.exe" + set JAVA_TYPE=JAVA_HOME +) else ( set JAVA="%OPENSEARCH_HOME%\jdk\bin\java.exe" set JAVA_HOME="%OPENSEARCH_HOME%\jdk" set JAVA_TYPE=bundled jdk -) else ( - set JAVA="%JAVA_HOME%\bin\java.exe" - set JAVA_TYPE=JAVA_HOME ) if not exist !JAVA! ( diff --git a/qa/os/src/test/java/org/opensearch/packaging/test/ArchiveTests.java b/qa/os/src/test/java/org/opensearch/packaging/test/ArchiveTests.java index e5464e8ee8d..898ea12b6a6 100644 --- a/qa/os/src/test/java/org/opensearch/packaging/test/ArchiveTests.java +++ b/qa/os/src/test/java/org/opensearch/packaging/test/ArchiveTests.java @@ -85,6 +85,7 @@ public class ArchiveTests extends PackagingTestCase { public void test30MissingBundledJdk() throws Exception { final Installation.Executables bin = installation.executables(); sh.getEnv().remove("JAVA_HOME"); + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); final Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); @@ -105,6 +106,7 @@ public class ArchiveTests extends PackagingTestCase { public void test31BadJavaHome() throws Exception { final Installation.Executables bin = installation.executables(); + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); sh.getEnv().put("JAVA_HOME", "doesnotexist"); // ask for opensearch version to quickly exit if java is actually found (ie test failure) @@ -114,11 +116,23 @@ public class ArchiveTests extends PackagingTestCase { } + public void test31BadOpensearchJavaHome() throws Exception { + final Installation.Executables bin = installation.executables(); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", "doesnotexist"); + + // ask for opensearch version to quickly exit if java is actually found (ie test failure) + final Result runResult = sh.runIgnoreExitCode(bin.opensearch.toString() + " -V"); + assertThat(runResult.exitCode, is(1)); + assertThat(runResult.stderr, containsString("could not find java in OPENSEARCH_JAVA_HOME")); + + } + public void test32SpecialCharactersInJdkPath() throws Exception { final Installation.Executables bin = installation.executables(); assumeTrue("Only run this test when we know where the JDK is.", distribution().hasJdk); final Path relocatedJdk = installation.bundledJdk.getParent().resolve("a (special) path"); + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); sh.getEnv().put("JAVA_HOME", relocatedJdk.toString()); try { @@ -154,6 +168,8 @@ public class ArchiveTests extends PackagingTestCase { } public void test51JavaHomeOverride() throws Exception { + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); + Platforms.onLinux(() -> { String systemJavaHome1 = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim(); sh.getEnv().put("JAVA_HOME", systemJavaHome1); @@ -171,8 +187,29 @@ public class ArchiveTests extends PackagingTestCase { assertThat(FileUtils.slurpAllLogs(installation.logs, "opensearch.log", "*.log.gz"), containsString(systemJavaHome1)); } - public void test52BundledJdkRemoved() throws Exception { + public void test51OpensearchJavaHomeOverride() throws Exception { + Platforms.onLinux(() -> { + String systemJavaHome1 = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim(); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", systemJavaHome1); + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + }); + Platforms.onWindows(() -> { + final String systemJavaHome1 = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim(); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", systemJavaHome1); + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + }); + + startOpenSearch(); + ServerUtils.runOpenSearchTests(); + stopOpenSearch(); + + String systemJavaHome1 = sh.getEnv().get("OPENSEARCH_JAVA_HOME"); + assertThat(FileUtils.slurpAllLogs(installation.logs, "opensearch.log", "*.log.gz"), containsString(systemJavaHome1)); + } + + public void test52JavaHomeBundledJdkRemoved() throws Exception { assumeThat(distribution().hasJdk, is(true)); + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); try { @@ -197,7 +234,37 @@ public class ArchiveTests extends PackagingTestCase { } } + public void test52OpensearchJavaHomeBundledJdkRemoved() throws Exception { + assumeThat(distribution().hasJdk, is(true)); + + Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); + try { + mv(installation.bundledJdk, relocatedJdk); + Platforms.onLinux(() -> { + String systemJavaHome1 = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim(); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", systemJavaHome1); + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + }); + Platforms.onWindows(() -> { + final String systemJavaHome1 = sh.run("$Env:SYSTEM_JAVA_HOME").stdout.trim(); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", systemJavaHome1); + sh.getEnv().put("JAVA_HOME", "doesnotexist"); + }); + + startOpenSearch(); + ServerUtils.runOpenSearchTests(); + stopOpenSearch(); + + String systemJavaHome1 = sh.getEnv().get("OPENSEARCH_JAVA_HOME"); + assertThat(FileUtils.slurpAllLogs(installation.logs, "opensearch.log", "*.log.gz"), containsString(systemJavaHome1)); + } finally { + mv(relocatedJdk, installation.bundledJdk); + } + } + public void test53JavaHomeWithSpecialCharacters() throws Exception { + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); + Platforms.onWindows(() -> { String javaPath = "C:\\Program Files (x86)\\java"; try { @@ -250,6 +317,7 @@ public class ArchiveTests extends PackagingTestCase { // cleanup from previous test rm(installation.config("opensearch.keystore")); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", ""); sh.getEnv().put("JAVA_HOME", ""); startOpenSearch(); diff --git a/qa/os/src/test/java/org/opensearch/packaging/test/PackagingTestCase.java b/qa/os/src/test/java/org/opensearch/packaging/test/PackagingTestCase.java index 3294ffb3bbe..259ae6e766c 100644 --- a/qa/os/src/test/java/org/opensearch/packaging/test/PackagingTestCase.java +++ b/qa/os/src/test/java/org/opensearch/packaging/test/PackagingTestCase.java @@ -33,6 +33,7 @@ package org.opensearch.packaging.test; import com.carrotsearch.randomizedtesting.JUnit3MethodProvider; +import com.carrotsearch.randomizedtesting.RandomizedContext; import com.carrotsearch.randomizedtesting.RandomizedRunner; import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering; import com.carrotsearch.randomizedtesting.annotations.TestGroup; @@ -182,11 +183,19 @@ public abstract class PackagingTestCase extends Assert { sh.reset(); if (distribution().hasJdk == false) { - Platforms.onLinux(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); - Platforms.onWindows(() -> sh.getEnv().put("JAVA_HOME", systemJavaHome)); + // Randomly switch between JAVA_HOME and OPENSEARCH_JAVA_HOME + final String javaHomeEnv = randomBoolean() ? "JAVA_HOME" : "OPENSEARCH_JAVA_HOME"; + logger.info("Using " + javaHomeEnv); + + Platforms.onLinux(() -> sh.getEnv().put(javaHomeEnv, systemJavaHome)); + Platforms.onWindows(() -> sh.getEnv().put(javaHomeEnv, systemJavaHome)); } } + private boolean randomBoolean() { + return RandomizedContext.current().getRandom().nextBoolean(); + } + @After public void teardown() throws Exception { if (installation != null && failed == false) { diff --git a/qa/os/src/test/java/org/opensearch/packaging/test/WindowsServiceTests.java b/qa/os/src/test/java/org/opensearch/packaging/test/WindowsServiceTests.java index 50540f3ac52..57ea853e735 100644 --- a/qa/os/src/test/java/org/opensearch/packaging/test/WindowsServiceTests.java +++ b/qa/os/src/test/java/org/opensearch/packaging/test/WindowsServiceTests.java @@ -149,12 +149,20 @@ public class WindowsServiceTests extends PackagingTestCase { } public void test14InstallBadJavaHome() throws IOException { + sh.getEnv().put("OPENSEARCH_JAVA_HOME", ""); 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 test14InstallBadOpensearchJavaHome() throws IOException { + sh.getEnv().put("OPENSEARCH_JAVA_HOME", "doesnotexist"); + Result result = sh.runIgnoreExitCode(serviceScript + " install"); + assertThat(result.exitCode, equalTo(1)); + assertThat(result.stderr, containsString("could not find java in OPENSEARCH_JAVA_HOME")); + } + public void test15RemoveNotInstalled() { Result result = assertFailure(serviceScript + " remove", 1); assertThat(result.stdout, containsString("Failed removing '" + DEFAULT_ID + "' service")); @@ -163,6 +171,7 @@ public class WindowsServiceTests extends PackagingTestCase { public void test16InstallSpecialCharactersInJdkPath() throws IOException { assumeTrue("Only run this test when we know where the JDK is.", distribution().hasJdk); final Path relocatedJdk = installation.bundledJdk.getParent().resolve("a (special) jdk"); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", ""); sh.getEnv().put("JAVA_HOME", relocatedJdk.toString()); try { @@ -248,6 +257,7 @@ public class WindowsServiceTests extends PackagingTestCase { public void test33JavaChanged() throws Exception { final Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", ""); try { mv(installation.bundledJdk, relocatedJdk); @@ -261,6 +271,22 @@ public class WindowsServiceTests extends PackagingTestCase { } } + public void test33OpensearchJavaChanged() throws Exception { + final Path relocatedJdk = installation.bundledJdk.getParent().resolve("jdk.relocated"); + sh.getEnv().put("JAVA_HOME", ""); + + try { + mv(installation.bundledJdk, relocatedJdk); + sh.getEnv().put("OPENSEARCH_JAVA_HOME", relocatedJdk.toString()); + assertCommand(serviceScript + " install"); + sh.getEnv().remove("OPENSEARCH_JAVA_HOME"); + assertCommand(serviceScript + " start"); + assertStartedAndStop(); + } finally { + mv(relocatedJdk, installation.bundledJdk); + } + } + public void test60Manager() throws IOException { Path serviceMgr = installation.bin("opensearch-service-mgr.exe"); Path tmpServiceMgr = serviceMgr.getParent().resolve(serviceMgr.getFileName() + ".tmp");