Upstart script should use set -a before loading /etc/default/logstash

[msimos]

• Version: Logstash 5.1.1
• Operating System: CentOS/RHEL 6.8 (Upstart)
• Config File:

input { beats { port => "${TCP_PORT}" } } output { }

• Steps to Reproduce:

If you set your environment variables in /etc/default/logstash or /etc/sysconfig/logstash as based on the upstart script:

script [ -r "/etc/default/logstash" ] && . "/etc/default/logstash" [ -r "/etc/sysconfig/logstash" ] && . "/etc/sysconfig/logstash" exec chroot --userspec logstash:logstash / /usr/share/logstash/bin/logstash "--path.settings" "/etc/logstash" >> /var/log/logstash-stdout.log 2>> /var/log/logstash-stderr.log end script

The problem here is that if you use the convention of TCP_PORT=1234 then this doesn't work:

[root@localhost logstash]# cat /etc/default/logstash TCP_PORT=1234 [root@localhost logstash]# initctl start logstash logstash start/running, process 38564 [root@localhost logstash]# tail -1 logstash-plain.log [2016-12-14T07:59:38,147][ERROR][logstash.agent ] fetched an invalid config {:config=>"input {\n beats {\n port => \"${TCP_PORT}\"\n }\n}\n\nfilter {\n mutate {\n add_field => { \"beat\" => \"%{[@metadata][beat]}\" } \n }\n}\n\noutput {\n# if \"metric\" in [tags] {\n# stdout {\n# codec => line {\n# format => \"Rate: %{[events][rate_1m]}\"\n# }\n# }\n# } else {\n# null {\n# workers => 2\n# }\n# stdout { codec => rubydebug }\n# }\n# elasticsearch {\n# hosts => [ \"${HOST}:9200\" ]\n# } \n}\n\n\n", :reason=>"Cannot evaluate ${TCP_PORT}. Environment variable TCP_PORT is not set and there is no default value given."}

However if specify set -a prior to loading /etc/default/logstash then it works:

[root@localhost logstash]# tail -5 /etc/init/logstash.conf set -a [ -r "/etc/default/logstash" ] && . "/etc/default/logstash" [ -r "/etc/sysconfig/logstash" ] && . "/etc/sysconfig/logstash" exec chroot --userspec logstash:logstash / /usr/share/logstash/bin/logstash "--path.settings" "/etc/logstash" >> /var/log/logstash-stdout.log 2>> /var/log/logstash-stderr.log end script [root@localhost logstash]# initctl start logstash logstash start/running, process 38956 [root@localhost logstash]# netstat -an | grep :1234 tcp 0 0 :::1234 :::* LISTEN

[2016-12-14T08:08:13,452][INFO ][logstash.inputs.beats ] Beats inputs: Starting input listener {:address=>"0.0.0.0:1234"}

[untergeek]

This may belong in pleaserun, but @jordansissel would know for sure.

[jordansissel]

I've been a sysadmin for 15 years and I've never once set this flag and, as far as I can remember, this is the first I've heard of it (seriously, I had to look it up in the manpage).

set -a seems dangerous. default/sysconfig files are shell scripts with unpredictable content. Why not put set -a in your default/sysconfig file?

Are you sure this set -a is the solution? Why not export your variables?

[jordansissel]

But yes, @untergeek, this is a pleaserun issue.

[msimos]

@jordansissel I have never had to use export or set -a in /etc/default/ or /etc/sysconfig/ for any other program. Using Fedora and systemd, Logstash 5.1.1 is able to read the variable without any modifications, just using TCP_PORT=123. This is the standard way to define variables in these files. I also don't think this was the case with prior versions of Logstash when Logstash used sysv init script. So this is something that's not right with the upstart script. I don't know if set -a is the solution but it was the simplest way to export the variables without knowing the names. You can define the variable in the upstart script by doing "env TCP_PORT=123". But the concern is if this file gets replaced during an upgrade.

A couple of alternatives are:

1) Use export $(cat /etc/default/logstash | xargs) to load the file in the variables vs ". /etc/default/logstash". 2) eval $(cat /etc/default/logstash | sed 's/^/export /')

See these links:

http://stackoverflow.com/questions/20150168/source-a-file-in-upstart-conf-file http://askubuntu.com/questions/711946/how-to-load-environment-variable-from-a-file-for-an-upstart-job http://serverfault.com/questions/128605/have-upstart-read-environment-from-etc-environment-for-a-service

So I am basically asking that the upstart script export the variables in /etc/default/logstash and /etc/sysconfig/logstash to make it consistent with other applications (like Elasticsearch) and prior versions of Logstash.

[jordansissel]

@msimos I'd be open to removing the sysconfig/default files from the upstart/systemd configs. We have no control over the contents of these files and promise nothing about how they behave, to be honest, and I would prefer to avoid them because of these problems and others about how users treat the files (some people use them as KEY=VALUE one per line, some people put shell script execution into them, we cannot control this)

The reason they work at all in sysv init is because a shell script reads them as another shell script. Even systemd doesn't correctly handle this either because systemd doesn't treat them as scripts. They're scripts.

Anyway, I know how to make them work, I am just not sure if making them work is the right solution; again, I'd rather reject the feature entirely ;\

[jordansissel]

(My anxiety is that if we hack it to do what systemd does for systemd.exec's EnvironmentFile then we're going to be breaking it for users who have scripts as sysconfig/defaults files)

I'm fine with supporting what you propose, but I don't think it really solves the problem (see above dichotomy of users who put scripts into defaults/sysconfig vs users who just have KEY=VALUE files there). RIght now, scripts are working. If we do your proposal, then KEY=VALUE works but scripts will not work.

[msimos]

@jordansissel I haven't tested this but assuming adding set -a in /etc/sysconfig/logstash or /etc/default/logstash works then this is a reasonable solution. However this is really treating these files as shell scripts and not just defining variables which seems more dangerous. I did try using "export TCP_PORT=1234" in /etc/default/logstash but saw some weird behavior and only seemed to work consistently with either defining the variable (env TCP_PORT=1234) or running set -a in the upstart script. But if you'd like to leave it up to an exercise to the user then I think thats fine. My only concern was maintaining this change across versions. I generally expect that any init script will be overwritten in an upgrade vs /etc/sysconfig/ or /etc/default/ is maintained.

This URL:

https://www.elastic.co/guide/en/logstash/current/environment-variables.html

Doesn't really talk about setting environment variables across various init systems (sysv, upstart, systemd). So possibly if we could document this maybe a better route?

[jordansissel]

Regarding environment variables: this feature is still fairly new in Logstash so I am OK with us not having any official docs on "best practices" especially given how many different deployment scenarios exist.

We should document it, but I don't know how to do this in a way that won't invite endless bug reports about unmentioned deployment scenarios (upstart vs systemd vs sysv vs docker ... etc).

On Tue, Dec 20, 2016 at 4:10 PM Mike Simos notifications@github.com wrote:

@jordansissel https://github.com/jordansissel I haven't tested this but assuming adding set -a in /etc/sysconfig/logstash or /etc/default/logstash works then this is a reasonable solution. However this is really treating these files as shell scripts and not just defining variables which seems more dangerous. I did try using "export TCP_PORT=1234" in /etc/default/logstash but saw some weird behavior and only seemed to work consistently with either defining the variable (env TCP_PORT=1234) or running set -a in the upstart script. But if you'd like to leave it up to an exercise to the user then I think thats fine. My only concern was maintaining this change across versions. I generally expect that any init script will be overwritten in an upgrade vs /etc/sysconfig/ or /etc/default/ is maintained.

This URL:

https://www.elastic.co/guide/en/logstash/current/environment-variables.html

Doesn't really talk about setting environment variables across various init systems (sysv, upstart, systemd). So possibly if we could document this maybe a better route?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/elastic/logstash/issues/6414#issuecomment-268396879, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIC6pJmMz9Bhes_vx0e-H2LPHmH_qTbks5rKG6DgaJpZM4LNkY4 .

[jakommo]

I hit this myself (ubuntu 14.04) and was able to solve it by using export VAR=value in /etc/default/logstash. Another user opened this discuss post having the same issue (on 16.04 though).

I've worked +10y as a sysadmin and I can't remember using export in any other /etc/default/* file.

I'm +1 on getting this working without export or at least better document how to set it if Logstash is running as a service.

but I don't know how to do this in a way that won't invite endless bug reports about unmentioned deployment scenarios (upstart vs systemd vs sysv vs docker ... etc).

Yeah, me neither. Maybe we start with the OS's we officially support?

[jordansissel]

Yeah, me neither. Maybe we start with the OS's we officially support?

This is what I'm talking about, hah ;)

I'm ok to add set -a now.