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.

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

JAVACMD in startup.options not taken into account with upstart #6482

Closed
jakommo opened this issue Jan 4, 2017 · 30 comments
Closed

JAVACMD in startup.options not taken into account with upstart #6482

jakommo opened this issue Jan 4, 2017 · 30 comments

Comments

@jakommo
Copy link

jakommo commented Jan 4, 2017

  • Version: 5.1.1
  • Operating System: Ubuntu 14.04.5 (openjdk-7-jre-headless 7u111 installed but Java 8 needed)
  • Config File (if you have sensitive info, please remove it): -
  • Sample Data: -
  • Steps to Reproduce:
  • Install LS 5.1.1 via apt
  • Download Java 8 tgz and extract to /tmp
  • export PATH="/tmp/jre1.8.0_111/bin:$PATH" on the root shell
  • Replace JAVACMD with JAVACMD=/tmp/jre1.8.0_111/bin/java in /etc/logstash/startup.options
  • execute /usr/share/logstash/bin/system-install (returns success)
# service logstash start 
logstash start/running, process 2185
# ps auxw | grep logstash
logstash  2185  0.0  1.0 2494892 174916 ?      SNsl 15:20   0:16 /usr/bin/java -XX:+UseParNewG
  • Still using /usr/bin/java
  • Adding the following to /etc/default/logstash, solved the issue.
JAVA_HOME="/tmp/jre1.8.0_111"
JAVACMD=/tmp/jre1.8.0_111/bin/java
PATH="/tmp/jre1.8.0_111/bin:$JAVA_HOME:$PATH"
# service logstash restart 
logstash start/running, process 2349
# ps auxw | grep logstash
logstash  2349  0.0  0.6 3621268 106728 ?      SNsl 15:22   0:04 /tmp/jre1.8.0_111/bin/java -XX:

Not sure if related to #6199

@untergeek
Copy link
Member

Was any attempt made to use the alternatives command (or some equivalent) to ensure that the specified Java is recognized by the system?

Can we see the contents of the upstart script (presumably /etc/init/logstash) rendered by system-install after the changes?

@jakommo
Copy link
Author

jakommo commented Jan 4, 2017

update-alternatives seems to only work for system packages.

# export JAVA_HOME="/tmp/jre1.8.0_111"
# export JAVACMD=/tmp/jre1.8.0_111/bin/java
# export PATH="/tmp/jre1.8.0_111/bin:$JAVA_HOME:$JAVACMD:$PATH"
# update-alternatives --config java
There is only one alternative in link group java (providing /usr/bin/java): /usr/lib/jvm/java-7-openjdk-amd64/jre/bin/java
Nothing to configure.

The generated file looks like this:

# cat /etc/init/logstash.conf
description     "logstash"
start on filesystem or runlevel [2345]
stop on runlevel [!2345]

respawn
umask 022
nice 19
chroot /
chdir /
#limit msgqueue <softlimit> <hardlimit>
#limit nice <softlimit> <hardlimit>
limit nofile 16384 16384
#limit rtprio <softlimit> <hardlimit>
#limit sigpending <softlimit> <hardlimit>
setuid logstash
setgid logstash


script
  [ -r /etc/default/logstash ] && . /etc/default/logstash
  [ -r /etc/sysconfig/logstash ] && . /etc/sysconfig/logstash
  exec /usr/share/logstash/bin/logstash "--path.settings" "/etc/logstash"
end script

I think the issue might be that /usr/share/logstash/bin/logstash.lib.sh is not sourcing the startup.options file.

@jordansissel
Copy link
Contributor

startup.options is only for the bin/system-install tool.

@jakommo
Copy link
Author

jakommo commented Jan 4, 2017

@jordansissel are you saying that it only defines which java will be used to run bin/system-install itself, without any effect on which java logstash would use?
If so, is adding the custom java to /etc/default/logstash the correct approach then to change this for logstash?

@jordansissel
Copy link
Contributor

Yes indeed :)

pleaserun generate a service that will try to load things from /etc/sysconfig/logstash and /etc/default/logstash before it runs logstash. You can see that above in your upstart job:

script
  [ -r /etc/default/logstash ] && . /etc/default/logstash
  [ -r /etc/sysconfig/logstash ] && . /etc/sysconfig/logstash
  exec /usr/share/logstash/bin/logstash "--path.settings" "/etc/logstash"
end script

@jordansissel
Copy link
Contributor

This perhaps is not well documented, and we could do better, but I don't think we set the expectation that a JAVACMD environment variable would persist from system-install into the resulting sysv/upstart/systemd. We could talk about that possibility, but I don't believe we promised that behavior already.

@untergeek
Copy link
Member

It should be put in a separate block in that file, with notes/documentation that "these entries need to go into /etc/sysconfig/logstash or /etc/default/logstash" or something like that.

@jordansissel
Copy link
Contributor

I will say, I agree with the report that this behavior is probably a bit confusing.

We could make this behavior work to pass environment variables through, perhaps generating an /etc/sysconfig/logstash or /etc/default/logstash based on the environment at the time system-install is run. Thoughts?

@untergeek
Copy link
Member

That sounds interesting, actually. This could be done by grabbing some/all vars in startup.options and putting them in /etc/sysconfig/logstash or /etc/default/logstash.

@jakommo
Copy link
Author

jakommo commented Jan 4, 2017

Thanks for clarifying @jordansissel @untergeek .

We should better document where this is done for Logstash itself.
startup.options just make it sound as it would be used for LS itself.

Maybe add some comments in the default or sysconfig file to make it clear which options are available and add something to startup.options that it is just used to generate the startup file but not used by LS?

But generating it from startup.options also sounds interesting.

@jordansissel
Copy link
Contributor

Conveniently, a recent feature landed in pleaserun to nearly do this:

Example:

% bin/pleaserun --environment-variables foo=bar --install-prefix /tmp/z -p sysv /usr/bin/fancy
...
% cat /tmp/z/etc/default/fancy
export foo="bar"

So maybe we can update system-install to pass --environment-variables flags for any important env vars (JAVACMD, JAVA_HOME, etc) ?

@jordansissel
Copy link
Contributor

add something to startup.options that it is just used to generate the startup file but not used by LS?

This alaready exists though --- in startup.options top of the file:

################################################################################
# These settings are ONLY used by $LS_HOME/bin/system-install to create a custom
# startup script for Logstash.  It should automagically use the init system

@jakommo
Copy link
Author

jakommo commented Jan 4, 2017

This alaready exists though --- in startup.options top of the file:

Oh I see, It makes sense now, but I didn't read it like that before.
Maybe append ... startup script for Logstash, but are not used by Logstash itself ?

@jordansissel
Copy link
Contributor

@jakommo I"m OK with adding it, but I don't believe anyone will actually read it. ;)

@jordansissel
Copy link
Contributor

#6484 adds the comment you requested, @jakommo.

@jakommo
Copy link
Author

jakommo commented Jan 4, 2017

Awesome, thanks @jordansissel . Should I close this or do you want to keep it open to discuss merging the startup.options into the init scripts?

@untergeek
Copy link
Member

Leave this open until I add the --environment-variable flags for pleaserun to startup.options

@mrbanzai
Copy link

mrbanzai commented Feb 2, 2017

Even when putting environment variables such as JAVACMD in, say, /etc/sysconfig/logstash, those environment variables aren't available to the launched process. I was only able to get the Logstash bootstrap scripts to honor it by explicitly exporting the relevant environment variables after they've been sourced in the Upstart config script.

@jordansissel
Copy link
Contributor

jordansissel commented Feb 2, 2017

@mrbanzai content in /etc/sysconfig/logstash is a shell script, and variables aren't environment variables until they are exported. I expect this behavior, but I am learning that many other folks do not -- and I'd like to help everyone have success here.

We're looking at ways to automatically export variables in these files but need to do testing to see how this might negatively impact users.

@mrbanzai
Copy link

mrbanzai commented Feb 2, 2017

Given that we're in a blended environment (EL6 and EL7), is the suggestion that we template out /etc/sysconfig/logstash separately for upstart and systemd? It seems that it might work if there's an explicit export in that file for the environment variables with Upstart, but I believe it behaves as expected under systemd without the explicit export (thus muddying the waters further).

Thanks for the swift response, @jordansissel

@jordansissel
Copy link
Contributor

jordansissel commented Feb 2, 2017

@mrbanzai I think @jakommo (or someone else? I forget) suggested that we should change our upstart/sysv to use set -a when sourcing sysconfig/defaults so that all variables in these files are automatically exported. I'm open to it, and it would (hopefully) solve your problem and better align with the way systemd treats these files.

@jordansissel
Copy link
Contributor

I filed jordansissel/pleaserun#121 to track the pleaserun change for this.

@mrbanzai
Copy link

mrbanzai commented Feb 2, 2017

👍 @jordansissel I've moved to templating set -a in our Upstart config in the meantime. Thanks!

@jakommo
Copy link
Author

jakommo commented Feb 3, 2017

think @jakommo (or someone else? I forget) suggested that we should change our upstart/sysv to use set -a when sourcing sysconfig/defaults so that all variables in these files are automatically exported.

It was @msimos in #6414

@jordansissel
Copy link
Contributor

@untergeek regarding the original report here, which I would generalize as a kind of forwarding of environment variables from the system-install process into the sysv/upstart/systemd service.

I think we can achieve this with modifications to pleaserun and then make logstash's system-install tell pleaserun to do this behavior.

Rough proposal:

  • Have pleaserun create a default/sysconfig file with a copy of desired environment variables from the time pleaserun is executed
  • Add set -a to sysv and upstart loading of default/sysconfig

Thoughts, @untergeek?

@jordansissel
Copy link
Contributor

jordansissel commented Feb 7, 2017

Resolved set -a: jordansissel/pleaserun#121.

Next: I'm working on environment-copying now.

@jordansissel
Copy link
Contributor

jordansissel commented Feb 7, 2017

pleaserun v0.0.28 published.

@untergeek I think the next step is to pass --environment-variables key=value for each environment variable we want to forward into the logstash service. Can you own that? I think we can look at the bin/logstash (and bin/logstash.lib.sh) for variables we'd want to forward, and for every non-empty variable (LS_JVM_OPTS, etc?), like --environment-variables "LS_JVM_OPTS=${LS_JVM_OPTS}"

@untergeek
Copy link
Member

@jordansissel Yep! I just needed to know that would work in pleaserun first. I'll change the version dependency and such.

So, if I understand you correctly, I'll need the --environment-variables flag for each key=value pair?

I'll try to get this coded and tested on some boxes by tomorrow's code freeze.

@jordansissel
Copy link
Contributor

Yep, --environment-variables flag each key=value

untergeek added a commit to untergeek/logstash that referenced this issue Feb 7, 2017
This will now put ENV variables from the `startup.options` file into `/etc/default/logstash` or `/etc/sysconfig/logstash` (or whatever service name you chose), and use the updated pleaserun to ensure these are honored at start time for whichever init system you use (systemd, upstart, SysV).

fixes elastic#6482
untergeek added a commit that referenced this issue Feb 9, 2017
This will now put ENV variables from the `startup.options` file into `/etc/default/logstash` or `/etc/sysconfig/logstash` (or whatever service name you chose), and use the updated pleaserun to ensure these are honored at start time for whichever init system you use (systemd, upstart, SysV).

fixes #6482
untergeek added a commit that referenced this issue Feb 9, 2017
This will now put ENV variables from the `startup.options` file into `/etc/default/logstash` or `/etc/sysconfig/logstash` (or whatever service name you chose), and use the updated pleaserun to ensure these are honored at start time for whichever init system you use (systemd, upstart, SysV).

fixes #6482
@nylcfpp
Copy link

nylcfpp commented Jan 31, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants