Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bat scripts to work with JAVA_HOME with parantheses #39712

Merged
merged 24 commits into from
Apr 2, 2019
Merged

Bat scripts to work with JAVA_HOME with parantheses #39712

Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
fe6435b
Bat scripts to work with JAVA_HOME with parantheses
pgomulka Mar 5, 2019
ded1387
Merge remote-tracking branch 'upstream/master' into feature/windows_s…
pgomulka Mar 6, 2019
fe06d04
refactor run method
pgomulka Mar 14, 2019
baa4bec
Linux version of java with spaces
pgomulka Mar 14, 2019
b3be477
compile fix
pgomulka Mar 14, 2019
74816f6
fix temp path
pgomulka Mar 14, 2019
5ac3a5e
Merge branch 'master' into feature/windows_startup_scripts
pgomulka Mar 21, 2019
152cafa
minor cleanup
pgomulka Mar 21, 2019
bb72cb4
not looking into PATH at all
pgomulka Mar 21, 2019
a7365f8
compile error
pgomulka Mar 22, 2019
7ea2662
compile errors
pgomulka Mar 22, 2019
e64b134
fix the linux version of the test
pgomulka Mar 25, 2019
8e140b0
Merge branch 'master' into feature/windows_startup_scripts
pgomulka Mar 26, 2019
00737f5
fix linux test
pgomulka Mar 26, 2019
ae1ce84
fixing failing tests
pgomulka Mar 26, 2019
c7c9cb6
permissions for tmp file
pgomulka Mar 27, 2019
e5dc371
chown temp file
pgomulka Mar 28, 2019
1ac4523
Merge remote-tracking branch 'upstream/master' into feature/windows_s…
pgomulka Mar 28, 2019
0fc3077
compile error
pgomulka Mar 28, 2019
c7c03b9
code review follow up
pgomulka Mar 29, 2019
cc7c050
remove random tmp creation
pgomulka Apr 1, 2019
c02fa27
refactor tmp creation to regular direcotry
pgomulka Apr 1, 2019
4502319
fix cleanup
pgomulka Apr 1, 2019
5e12a34
don't ignore cleanup failure
pgomulka Apr 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion distribution/src/bin/elasticsearch-env.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,5 +64,5 @@ rem check the Java version
%JAVA% -cp "%ES_CLASSPATH%" "org.elasticsearch.tools.java_version_checker.JavaVersionChecker" || exit /b 1

if not defined ES_TMPDIR (
for /f "tokens=* usebackq" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.TempDirectory""`) do set ES_TMPDIR=%%a
for /f "tokens=* usebackq" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.TempDirectory"`) do set ES_TMPDIR=%%a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Discussed this offline: at first glance I had thought this was modifying the deprecated for loop that searches the path for java. Now I see this is a different loop (although I am confused why we need a loop at all: TempDirectory only prints one line). I believe this entire PR can be moved back to basing on master.

)
4 changes: 2 additions & 2 deletions distribution/src/bin/elasticsearch.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ IF ERRORLEVEL 1 (
EXIT /B %ERRORLEVEL%
)

set "ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options"
set ES_JVM_OPTIONS=%ES_PATH_CONF%\jvm.options
@setlocal
for /F "usebackq delims=" %%a in (`"%JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" || echo jvm_options_parser_failed"`) do set JVM_OPTIONS=%%a
for /F "usebackq delims=" %%a in (`CALL %JAVA% -cp "!ES_CLASSPATH!" "org.elasticsearch.tools.launchers.JvmOptionsParser" "!ES_JVM_OPTIONS!" ^|^| echo jvm_options_parser_failed`) do set JVM_OPTIONS=%%a
@endlocal & set "MAYBE_JVM_OPTIONS_PARSER_FAILED=%JVM_OPTIONS%" & set ES_JAVA_OPTS=%JVM_OPTIONS:${ES_TMPDIR}=!ES_TMPDIR!% %ES_JAVA_OPTS%

if "%MAYBE_JVM_OPTIONS_PARSER_FAILED%" == "jvm_options_parser_failed" (
Expand Down
92 changes: 76 additions & 16 deletions qa/vagrant/src/main/java/org/elasticsearch/packaging/test/ArchiveTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,21 +20,24 @@
package org.elasticsearch.packaging.test;

import com.carrotsearch.randomizedtesting.annotations.TestCaseOrdering;
import com.carrotsearch.randomizedtesting.generators.RandomStrings;
import org.apache.http.client.fluent.Request;
import org.elasticsearch.packaging.util.Archives;
import org.elasticsearch.packaging.util.Distribution;
import org.elasticsearch.packaging.util.FileUtils;
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 java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.stream.Stream;

import static com.carrotsearch.randomizedtesting.RandomizedTest.getRandom;
import static org.elasticsearch.packaging.util.Archives.ARCHIVE_OWNER;
import static org.elasticsearch.packaging.util.Archives.installArchive;
import static org.elasticsearch.packaging.util.Archives.verifyArchiveInstallation;
Expand All @@ -49,6 +52,7 @@
import static org.elasticsearch.packaging.util.FileUtils.rm;
import static org.elasticsearch.packaging.util.ServerUtils.makeRequest;
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
Expand All @@ -62,12 +66,12 @@
@TestCaseOrdering(TestCaseOrdering.AlphabeticOrder.class)
public abstract class ArchiveTestCase extends PackagingTestCase {

public void test10Install() {
public void test10Install() throws Exception {
installation = installArchive(distribution());
verifyArchiveInstallation(installation, distribution());
}

public void test20PluginsListWithNoPlugins() {
public void test20PluginsListWithNoPlugins() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand All @@ -77,7 +81,7 @@ public void test20PluginsListWithNoPlugins() {
assertThat(r.stdout, isEmptyString());
}

public void test30NoJava() {
public void test30NoJava() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand All @@ -101,7 +105,7 @@ public void test30NoJava() {
}
}

public void test40CreateKeystoreManually() {
public void test40CreateKeystoreManually() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand Down Expand Up @@ -134,7 +138,7 @@ public void test40CreateKeystoreManually() {
});
}

public void test50StartAndStop() throws IOException {
public void test50StartAndStop() throws Exception {
assumeThat(installation, is(notNullValue()));

// cleanup from previous test
Expand All @@ -152,7 +156,7 @@ public void test50StartAndStop() throws IOException {
Archives.stopElasticsearch(installation);
}

public void assertRunsWithJavaHome() throws IOException {
public void assertRunsWithJavaHome() throws Exception {
Shell sh = newShell();

Platforms.onLinux(() -> {
Expand All @@ -173,13 +177,13 @@ public void assertRunsWithJavaHome() throws IOException {
assertThat(new String(Files.readAllBytes(log), StandardCharsets.UTF_8), containsString(systemJavaHome));
}

public void test51JavaHomeOverride() throws IOException {
public void test51JavaHomeOverride() throws Exception {
assumeThat(installation, is(notNullValue()));

assertRunsWithJavaHome();
}

public void test52BundledJdkRemoved() throws IOException {
public void test52BundledJdkRemoved() throws Exception {
assumeThat(installation, is(notNullValue()));
assumeThat(distribution().hasJdk, is(true));

Expand All @@ -192,7 +196,63 @@ public void test52BundledJdkRemoved() throws IOException {
}
}

public void test60AutoCreateKeystore() {
public void test53JavaHomeWithSpecialCharacters() throws Exception {
assumeThat(installation, is(notNullValue()));

Platforms.onWindows(() -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we have a similar test in bats, can you add the linux side here as well so we can just remove that test?

final Shell sh = new Shell();
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:JAVA_HOME");

sh.getEnv().put("JAVA_HOME", "C:\\Program Files (x86)\\java");

//verify ES can start, stop and run plugin list
Archives.runElasticsearch(installation, sh);

Archives.stopElasticsearch(installation);

String pluginListCommand = installation.bin + "/elasticsearch-plugin list";
Result result = sh.run(pluginListCommand);
assertThat(result.exitCode, equalTo(0));

} finally {
//clean up sym link
sh.runIgnoreExitCode("cmd /c del /F /Q 'C:\\Program Files (x86)\\java' ");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to ignore the exit? If this fails, later tests could fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actuality I thought that if this fails the rest of the test should continue. JAVA_HOME should not propagate so it won't be visible if failed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it fails, something is messed up. I think we should stop testing.

}
});

Platforms.onLinux(() -> {
final Shell sh = new Shell();
// Create temporary directory with a space and link to java binary.
// Use it as java_home
String nameWithSpace = RandomStrings.randomAsciiAlphanumOfLength(getRandom(), 10) + "java home";
String test_java_home = FileUtils.mkdir(Paths.get("/home",ARCHIVE_OWNER, nameWithSpace)).toAbsolutePath().toString();
try {
final String systemJavaHome = sh.run("echo $SYSTEM_JAVA_HOME").stdout.trim();
final String java = systemJavaHome + "/bin/java";

sh.run("mkdir -p \"" + test_java_home + "/bin\"");
sh.run("ln -s \"" + java + "\" \"" + test_java_home + "/bin/java\"");
sh.run("chown -R " + ARCHIVE_OWNER + ":" + ARCHIVE_OWNER + " \"" + test_java_home + "\"");

sh.getEnv().put("JAVA_HOME", test_java_home);

//verify ES can start, stop and run plugin list
Archives.runElasticsearch(installation, sh);

Archives.stopElasticsearch(installation);

String pluginListCommand = installation.bin + "/elasticsearch-plugin list";
Result result = sh.run(pluginListCommand);
assertThat(result.exitCode, equalTo(0));
} finally {
FileUtils.rm(Paths.get("\"" + test_java_home + "\""));
}
});
}

public void test60AutoCreateKeystore() throws Exception {
assumeThat(installation, is(notNullValue()));

assertThat(installation.config("elasticsearch.keystore"), file(File, ARCHIVE_OWNER, ARCHIVE_OWNER, p660));
Expand All @@ -211,7 +271,7 @@ public void test60AutoCreateKeystore() {
});
}

public void test70CustomPathConfAndJvmOptions() throws IOException {
public void test70CustomPathConfAndJvmOptions() throws Exception {
assumeThat(installation, is(notNullValue()));

final Path tempConf = getTempDir().resolve("esconf-alternate");
Expand Down Expand Up @@ -260,7 +320,7 @@ public void test70CustomPathConfAndJvmOptions() throws IOException {
}
}

public void test80RelativePathConf() throws IOException {
public void test80RelativePathConf() throws Exception {
assumeThat(installation, is(notNullValue()));

final Path temp = getTempDir().resolve("esconf-alternate");
Expand Down Expand Up @@ -304,7 +364,7 @@ public void test80RelativePathConf() throws IOException {
}
}

public void test90SecurityCliPackaging() {
public void test90SecurityCliPackaging() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand All @@ -328,7 +388,7 @@ public void test90SecurityCliPackaging() {
}
}

public void test91ElasticsearchShardCliPackaging() {
public void test91ElasticsearchShardCliPackaging() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand All @@ -345,7 +405,7 @@ public void test91ElasticsearchShardCliPackaging() {
}
}

public void test92ElasticsearchNodeCliPackaging() {
public void test92ElasticsearchNodeCliPackaging() throws Exception {
assumeThat(installation, is(notNullValue()));

final Installation.Executables bin = installation.executables();
Expand All @@ -363,7 +423,7 @@ public void test92ElasticsearchNodeCliPackaging() {
}
}

public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws IOException {
public void test93ElasticsearchNodeCustomDataPathAndNotEsHomeWorkDir() throws Exception {
assumeThat(installation, is(notNullValue()));

Path relativeDataPath = installation.data.relativize(installation.home);
Expand Down
9 changes: 4 additions & 5 deletions qa/vagrant/src/main/java/org/elasticsearch/packaging/test/DebPreservationTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.junit.Before;
import org.junit.BeforeClass;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

Expand Down Expand Up @@ -54,7 +53,7 @@ public abstract class DebPreservationTestCase extends PackagingTestCase {
protected abstract Distribution distribution();

@BeforeClass
public static void cleanup() {
public static void cleanup() throws Exception {
installation = null;
cleanEverything();
}
Expand All @@ -65,14 +64,14 @@ public void onlyCompatibleDistributions() {
assumeTrue("only compatible distributions", distribution().packaging.compatible);
}

public void test10Install() throws IOException {
public void test10Install() throws Exception {
assertRemoved(distribution());
installation = install(distribution());
assertInstalled(distribution());
verifyPackageInstallation(installation, distribution(), newShell());
}

public void test20Remove() {
public void test20Remove() throws Exception {
assumeThat(installation, is(notNullValue()));

remove(distribution());
Expand Down Expand Up @@ -117,7 +116,7 @@ public void test20Remove() {
assertTrue(Files.exists(installation.envFile));
}

public void test30Purge() {
public void test30Purge() throws Exception {
assumeThat(installation, is(notNullValue()));

final Shell sh = new Shell();
Expand Down
Loading