mesosphere / mesos-deb-packaging

Mesos package for Debian, Ubuntu, CentOS, RHEL, and Fedora
Other
58 stars 67 forks source link

allow for disabling output to syslog with USE_SYSLOG=0 #35

Closed rboyer closed 8 years ago

rboyer commented 9 years ago

This makes mesos-init-wrapper more compatible with supervisor and systemd, or anyone else who doesn't want all of their application logs sent to syslog.

rboyer commented 9 years ago

Or I'd be open to making the mesos-init-wrapper accept "--no-logging" to match what Marathon is going to ship in 0.8.1 (discussion on https://github.com/mesosphere/marathon/pull/1267)

shoenig commented 9 years ago

Any chance of this getting merged?

lingmann commented 9 years ago

Hi @naelyn and @shoenig, we would be open to merging the PR if it follows the --no-logger convention used by the Marathon wrapper script. Thanks!

rboyer commented 9 years ago

I've updated the branch to accept --no-logger in the style of the Marathon wrapper script.

lingmann commented 9 years ago

Hi @rboyer thanks for the patch, however I'm afraid there appear to be some problems:

# bash -x ./mesos-init-wrapper master --no-logger
+ set -o errexit -o nounset -o pipefail
+ export LC_ALL=en_US.UTF-8
+ LC_ALL=en_US.UTF-8
+ [[ -n master ]]
+ declare -F
+ fgrep -qx -- master
+ cut '-d ' -f3
+ master --no-logger
+ local etc_master=/etc/mesos-master
+ args=()
+ local args
+ '[' -x /usr/bin/mesosphere-dnsconfig ']'
+ [[ ! -f /etc/default/mesos ]]
+ . /etc/default/mesos
++ LOGS=/var/log/mesos
++ ULIMIT='-n 8192'
+ [[ ! -f /etc/default/mesos-master ]]
+ . /etc/default/mesos-master
++ PORT=5050
+++ cat /etc/mesos/zk
++ ZK=zk://localhost:2181/mesos
+ [[ ! -n -n 8192 ]]
+ ulimit -n 8192
+ [[ ! -n zk://localhost:2181/mesos ]]
+ args+=(--zk="$ZK")
+ [[ ! -n '' ]]
+ [[ ! -n 5050 ]]
+ args+=(--port="$PORT")
+ [[ ! -n '' ]]
+ [[ ! -n /var/log/mesos ]]
+ args+=(--log_dir="$LOGS")
+ for f in '"$etc_master"/*'
+ [[ -f /etc/mesos-master/quorum ]]
++ basename /etc/mesos-master/quorum
+ local name=quorum
+ [[ quorum == \?* ]]
+ args+=(--"$name"="$(cat "$f")")
++ cat /etc/mesos-master/quorum
+ for f in '"$etc_master"/*'
+ [[ -f /etc/mesos-master/work_dir ]]
++ basename /etc/mesos-master/work_dir
+ local name=work_dir
+ [[ work_dir == \?* ]]
+ args+=(--"$name"="$(cat "$f")")
++ cat /etc/mesos-master/work_dir
+ [[ --zk=zk://localhost:2181/mesos --port=5050 --log_dir=/var/log/mesos --quorum=1 --work_dir=/var/lib/mesos == *\-\-\n\o\-\l\o\g\g\e\r* ]]
+ logged /usr/sbin/mesos-master --zk=zk://localhost:2181/mesos --port=5050 --log_dir=/var/log/mesos --quorum=1 --work_dir=/var/lib/mesos
+ local 'tag=mesos-master[11618]'
+ exec
+ exec
++ exec logger -p user.info -t 'mesos-master[11618]'
++ exec logger -p user.err -t 'mesos-master[11618]'

Also, we should have some updates to the help/usage instructions if we are going to add this option. Thanks!

rboyer commented 9 years ago

The --no-logger argument should be passed just like all other arguments (via /etc/mesos-master/?no-logger).

I've updated the script to more accurately delete an item from the bash array, and proposed a similar correction to the marathon startup script as well (https://github.com/mesosphere/marathon/pull/2241) so that you can control this setting without touching the init scripts provided by apt/yum

rboyer commented 8 years ago

Bump?

rboyer commented 8 years ago

Is there anything else I should do to move this along?

ssk2 commented 8 years ago

@rboyer - sorry for the delay! I'll ping someone to have a look at this.

rboyer commented 8 years ago

It would be cool for this to make it into the next set of debs and rpms.

lingmann commented 8 years ago

Hi @rboyer sorry for the long review time and thanks for the pull request. LGTM, merging!