search

elastic / logstash

Logstash - transport and process your logs, events, or other data
https://www.elastic.co/products/logstash
Other
78 stars 3.5k forks source link

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

Closed jakommo closed 7 years ago

jakommo commented 7 years ago

Not sure if related to https://github.com/elastic/logstash/pull/6199

untergeek commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

jakommo commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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

jordansissel commented 7 years ago

https://github.com/elastic/logstash/pull/6484 adds the comment you requested, @jakommo.

jakommo commented 7 years ago

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 commented 7 years ago

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

mrbanzai commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

I filed https://github.com/jordansissel/pleaserun/issues/121 to track the pleaserun change for this.

mrbanzai commented 7 years ago

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

jakommo commented 7 years ago

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 https://github.com/elastic/logstash/issues/6414

jordansissel commented 7 years ago

@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:

Thoughts, @untergeek?

jordansissel commented 7 years ago

Resolved set -a: https://github.com/jordansissel/pleaserun/issues/121.

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

jordansissel commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

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

nylcfpp commented 6 years ago

哦