bbaugher / apache_zookeeper

Chef cookbook for Apache Zookeeper
MIT License
9 stars 22 forks source link

Correct README about log_dir and fix systemd #50

Closed wolftrouble closed 7 years ago

wolftrouble commented 7 years ago

We're still experimenting with this cookbook for standalone deployments, and I think I ran across a legitimate bug.

The documentation makes reference to ['apache_zookeeper']['log_directory'] as a default for $ZOO_LOG_DIR envar. But the code since then has changed (the string "log_directory" only appears in the README now) and I think has been replaced with ['apache_zookeeper']['log_dir'].

I'd offer a PR to fix the documentation, but I haven't been able to get it to work at all. Attempting to install and start zookeeper inevitably fails with (from systemctl):

Aug 17 22:57:35 int-kafka-bthomas02.foodomain.com zkServer.sh[21358]: ZooKeeper JMX enabled by default
Aug 17 22:57:35 int-kafka-bthomas02.foodomain.com zkServer.sh[21358]: Using config: /opt/zookeeper/current/bin/../conf/zoo.cfg
Aug 17 22:57:35 int-kafka-bthomas02.foodomain.com zkServer.sh[21358]: Starting zookeeper ... /opt/zookeeper/current/bin/zkServer.sh: line 140: ./zookeeper.out: Permission denied

which suggests ZOO_LOG_DIR is defaulting to ".". Any suggestions on what might be wrong? Relevant node attributes are:

    "apache_zookeeper": {
      "init_style": "systemd",
      "install_java": false,
      "local_state_dir": "/opt/zookeeper",
      "log_dir": "/opt/zookeeper/zookeeper-data/log",
    },

I'd expect if there to be an error it would look like "/opt/zookeeper/zookeeper-data/log/zookeeper.out: Permission denied" if it was using the node attribute and still failing.

wolftrouble commented 7 years ago

I should say, looks like the defaults file is being set properly. The contents of /etc/default/zookeeper is:

# Configuration file for zookeeper service. Autogenerated by Chef.

CONFIG=/opt/zookeeper/current/conf/zoo.cfg

export ZOO_LOG4J_PROP=INFO,ROLLINGFILE
export ZOO_LOG_DIR=/opt/zookeeper/zookeeper-data/log
wolftrouble commented 7 years ago

Found the problem. There's two issues.

  1. /etc/systemd/system/multi-user.target.wants/zookeeper.service needs "EnvironmentFile=/etc/default/zookeeper" added under [Service]

  2. /etc/default/zookeeper must only consist of VAR=VAL pairs. Use of "export" is not allowed and if present will be explicitly ignored - systemctl status throws something like "Ignoring invalid environment assignment 'export ZOO_LOG4J_PROP=INFO,ROLLINGFILE': /etc/default/zookeeper"

Further edit: I don't think use of "export" is valid even on upstart-based systems, at least, I can't find any examples of anything else using it. I suspect adding the EnvironmentFile line to templates/default/systemd/zookeeper.erb and removing "export" from templates/default/debian/default/zookeeper.erb will fix this for both upstart and systemd-based systems.

wolftrouble commented 7 years ago

I've got a branch at https://github.com/bbaugher/apache_zookeeper/compare/master...wolftrouble:fixes?expand=1 but I couldn't figure out how to share tests across suites, and symlinking doesn't seem to work, so I haven't made a PR out of it since I assume you don't want to duplicate all tests across two directories. But it adds tests for 1604+systemd, fixes the documentation, and fixes the templates to work under both upstart and systemd. All tests come back clean.

And it turns out you DO need 'export' in /etc/default files when using upstart but apparently not (and it's not allowed) with systemd., hence the conditional in the template I was unable to find documentation on this.

bbaugher commented 7 years ago

That looks good to me if you want to open a pull request

wolftrouble commented 7 years ago

Done, and thank you!

bbaugher commented 7 years ago

Fixed with #51