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

Jump to bottom

Don't preserve OS's JAVA*_HOME vars before launching Elasticsearch #976

Closed
dliappis opened this issue Apr 29, 2020 · 2 comments · Fixed by #978
Closed

Don't preserve OS's JAVA*_HOME vars before launching Elasticsearch #976

dliappis opened this issue Apr 29, 2020 · 2 comments · Fixed by #978
Assignees
@hub-cap
Labels
bug Something's wrong good first issue Small, contained changes that are good for newcomers :Usability Makes Rally easier to use
Milestone
2.0.0

Comments

@dliappis
Copy link
Contributor

Description

When user specifies --runtime-jdk=bundled if JAVA_HOME is already set then it remains present and honored by Elasticsearch thus preferring JAVA_HOME than the bundled jdk (for versions of Elasticsearch coming with a bundled jdk of course)

Instead Rally should unset all JAVA*_HOME vars, specifically JAVA_HOME, but also JAVA{7,8,9,10,11,...}_HOME as a precaution for the future too.

While at it, let's also fix the weird indentation in

rally/esrally/mechanic/launcher.py

Line 192 in 635757e

env[k] = v + separator + env[k]
and

rally/esrally/mechanic/launcher.py

Line 194 in 635757e

env[k] = env[k] + separator + v

@dliappis dliappis added bug Something's wrong :Usability Makes Rally easier to use good first issue Small, contained changes that are good for newcomers labels Apr 29, 2020
@dliappis dliappis added this to the 2.0.0 milestone Apr 29, 2020
@dliappis
Copy link
Contributor Author

@hub-cap hit this issue during some adhoc experiments

@dliappis
Copy link
Contributor Author

Note that elastic/elasticsearch#55820 will stop honoring JAVA_HOME (but honor other env vars like ES_JAVA_HOME).

hub-cap added a commit to hub-cap/rally that referenced this issue Apr 29, 2020
@hub-cap
Remove JAVA_HOME from env if it is not set
eac514d 
The launcher pulls in local environment variables from the OS prior to
launching elasticsearch. This can be problematic if the JAVA_HOME
variable is locally set, but explicitly not set by the rally
launcher. For the case of the bundled JDK, a variable java_home is
explicitly unset, which should be reconciled with the local env by also
unsetting JAVA_HOME if it exists before launching. This commit ensures
that if java_home is unset in the code, that the local env's JAVA_HOME
variable is also unset.

Closes elastic#976
@hub-cap hub-cap mentioned this issue Apr 29, 2020
Merged
hub-cap added a commit to hub-cap/rally that referenced this issue May 5, 2020
@hub-cap
Remove os environment variables from env prep
f53b54f 
This commit removes an explicit add of the JAVA_HOME environment
variable from the _prepare_env method, which will, if the bundled JDK is
selected, ensure that Elasticsearch will not use a JAVA_HOME environment
variable instead of the bundled JDK.

This commit does not remove the use of the JAVA_HOME environment
variables in the event that the bundled JDK is not chosen, which is
resolved in a different place.

Closes elastic#976
hub-cap added a commit that referenced this issue May 5, 2020
@hub-cap
Remove os environment variables from env prep (#978)
f068edf 
This commit removes an explicit add of the JAVA_HOME environment
variable from the _prepare_env method, which will, if the bundled JDK is
selected, ensure that Elasticsearch will not use a JAVA_HOME environment
variable instead of the bundled JDK.

This commit does not remove the use of the JAVA_HOME environment
variables in the event that the bundled JDK is not chosen, which is
resolved in a different place.

Closes #976
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hub-cap hub-cap

Labels
bug Something's wrong good first issue Small, contained changes that are good for newcomers :Usability Makes Rally easier to use
Projects
None yet
Milestone
2.0.0
Development

Successfully merging a pull request may close this issue.

Remove os environment variables from env prep hub-cap/rally
2 participants
@hub-cap @dliappis