init clean-up #82
init clean-up #82
Conversation
all init scripts now look the same and can probably be merged into one, if needed.
| @@ -21,29 +34,29 @@ if [ -f /etc/default/kafka ]; then | |||
| fi | |||
|
|
|||
| start() { | |||
| if [ -f $PID_FILE ] | |||
| then | |||
| if [ -f $PID_FILE ]; then | |||
There was a problem hiding this comment.
this needs quotes around the var
|
thank you for the review, @bastelfreak. |
|
on a first look this looks good. Are we sure that zookeeper.service always has that name or does it need to be configurable? |
|
i'm contemplating to review it by putting it into production. |
n.b.: This ignores stdout / stderr in sysv-inits! The reason for this is because we weren't rotating those logs — in some cases we weren't even appending, just overwriting it! Additionally, those logs were partially duplicated from those already existing under /opt/kafka/logs/.
this addresses #83.
|
|
…before passing a variable with user input to it.
|
close/open did nothing to make @voxpupuli/pcci pick it up |
previously, we were comparing the command 2699 (a random pid) with 2699. usually, that command wasn't found ;) additionally, our status had that condition wrong!
comparing `ps -p non-existent-pid` with `pgrep -f non-running-process` will always return true. This adds a saftey-switch.
| then | ||
| if [ -f "$PID_FILE" ]; then | ||
| PID=`cat "$PID_FILE"` | ||
| if [ `ps -p "$PID" -o pid= || echo 1` -eq `pgrep -f "$PGREP_PATTERN" || echo 2` ] ; then |
There was a problem hiding this comment.
i really hope this isn't too clever
There was a problem hiding this comment.
that looks really strange. Have you tested that?
There was a problem hiding this comment.
yes!! it works really well
what it does is this:
if ps -p 2194 -o pid= doesn't find that pid to be running, it'll output nothing, but return 1. In that case the || echo 1 case will fire. simiarily, if pgrep -f kafka.Kafka doesn't find anything, it will output nothing, but return 1. Now, || echo 2 is the result. hence, we're comparing
[ 1 -eq 2 ], which will be false. but! if one of ps or pgrep returns a sensible value, something is off! and we're catching those cases, too: [ 2967 -eq 2 ] is just as false, as [ 1 -eq 2957 ] is.
|
it took creating and destroying 3 stacks, but with the latest patches it now works. |
| ensure => present, | ||
| file { "${service_name}.service": | ||
| ensure => file, | ||
| path => "/usr/lib/systemd/system/${service_name}.service", |
There was a problem hiding this comment.
overwriting anything in /usr/lib/systemd/system is a bad idea and not recommened. The prefered solution is to use /etc/systemd/system. The first path is meant to be used by package managers only. You can create a unit at the second path with an equal name which will kind of inherit from the first one. Also ubuntu or debian don't use /usr/lib but /lib/something.
There was a problem hiding this comment.
so, /etc/systemd/system ?
There was a problem hiding this comment.
yep. I'm currently not sure how exactly the overwrites work. Not everything is possible here. I've to read up the docs.
https://wiki.archlinux.org/index.php/systemd
https://www.freedesktop.org/software/systemd/man/systemd.unit.html
https://www.freedesktop.org/software/systemd/man/systemd.service.html#
(/usr)/lib/systemd is reserved for the system. we shouldn't touch that ourselves. instead we should use /etc/systemd. This comes with an additional benefit: it's compatible between debuntu & rhel!
|
two fails on travis, this is based on a gem fuckup. the rest is fine. |
init clean-up
This WIP pull request tries to clean-up our init scripts.
They are currently quadrupled (broker, consumer, mirror, producer). Unnecessarily so.
This pr will first attempt to clean up the shell scripts, then consolidate them into one template.