zookeeper_service should use zookeeper-env when starting zookeeper

[jeffbyrnes]

Thanks @jaybocc2! I’ve not had much time for this cookbook lately, but I’ll do my best to get back in here.

[jeffbyrnes]

@jaybocc2 ok, I finally got a chance to chase this down a bit; sorry for the huge delay!

So this is super confusing, but #{install_dir}/bin/zkServer.sh already does this. Here’s how it works:

1. You run #{install_dir}/bin/zkServer.sh
2. It runs this bit of code:

   if [ -e "$ZOOBIN/../libexec/zkEnv.sh" ]; then
     . "$ZOOBINDIR/../libexec/zkEnv.sh"
   else
     . "$ZOOBINDIR/zkEnv.sh"
   fi

3. Inside that file (which, on Ubuntu 16.04, is "$ZOOBINDIR/zkEnv.sh") is this:

   if [ -f "${ZOOCFGDIR}/zookeeper-env.sh" ]; then
     . "${ZOOCFGDIR}/zookeeper-env.sh"
   fi

So by merely providing the zookeeper-env.sh in the $ZOOCFGDIR, we’re all set.

To elaborate on where that is (from zkEnv.sh):

if [ "x$ZOOCFGDIR" = "x" ]
then
  if [ -e "${ZOOKEEPER_PREFIX}/conf" ]; then
    ZOOCFGDIR="$ZOOBINDIR/../conf"
  else
    ZOOCFGDIR="$ZOOBINDIR/../etc/zookeeper"
  fi
fi

The former exists by default, and is our default value for the attribute, so that all gels.

So these changes are unnecessary, and in fact end up sourcing zookeeper-env.sh twice.

[jaybocc2]

It only works if you use your defaults, but if you use a different install path and a different configure path from the install path, this does not work.

[jeffbyrnes]

I caution against setting the install path and configure path to different places.

I’ve done my best to set up this cookbook to play nicely, but ZooKeeper’s systems for initializing and starting itself are complicated (as evidenced above).

I recommend, if you are going to install it to another location, that you ensure that you are setting the config path within that install directory. I.e., this should remain unchanged:

default['zookeeper']['conf_dir'] = "#{node['zookeeper']['install_dir']}/zookeeper/conf"

Perhaps, in service to that, the attribute should be done away with.

Bear in mind that the zkServer.sh file will be used to start ZooKeeper, and, even if you source your zookeeper-env.sh, it will eventually go through the whole dance I laid out above.

Fighting with the config setup of the software itself is not something this cookbook is set up to do.

If you feel we’d be better served by not allowing the config directory to be changed (i.e., we always just have it be inside #{install_dir}/zookeeper, that makes sense to me.

[jaybocc2]

conf/zookeeper-env.sh does not exist by default in zookeeper installs, and gets created at the specified config path

your zookeeper-env.sh could look like this

export ZOOCFGDIR=/etc/zookeeper
export ZOOCFG=zoo.cfg
export ZOO_LOG_DIR=/var/log/zookeeper

So the zkEnv.sh would still behave as expected. If you allow conf_dir to be configured, which you do, and is not really a problem, as it correctly updates zookeeper-env.sh, you must source zookeeper-env.sh to pickup these changes in configuration.

[jeffbyrnes]

That’s true (and I had forgotten), zookeeper-env.sh does not exist by default. The zookeeper_config resource in this cookbook creates it, so I was looking at the final result of using the three resources this cookbook provides, by way of zookeeper::default.

The zookeeper-env.sh created by zookeeper_config has those three values set; take a peek at https://github.com/evertrue/zookeeper-cookbook/blob/e64073cf09f5d23c10dfc6a94b65af85999ca01e/resources/config.rb#L43-L52 for details.

Using zookeeper_config, you will end up with:

• The main config file
• A zookeeper-env.sh at #{conf_dir}/zookeeper-env.sh

Which will then be picked up by using zkServer.sh with no further adjustments.

Are you using zookeeper_config?

As a heads-up, I’ll be halting work for the day shortly, but will return tomorrow AM, EST.

[jaybocc2]

Yes, zookeeper_config with a conf_dir that is different from install_dir, which causes zookeeper-env.sh to be created at that path (as expected). So either zookeeper_config needs to correct its install path so it is sourced properly, or we need to source it before starting zookeeper script. I'd prefer to source zookeeper-env.sh first, but it gets source early enough in zkEnv.sh that its safe either way, though we've come to expect it in the conf_dir path.

[jeffbyrnes]

Ahh, ok. Yeah, that makes sense. I’m a bit wary of “fighting” with ZooKeeper on this, since it has this whole complicated setup that assumes you’re going to use the conf/ dir inside its install dir.

That said, if it’s tricky/dangerous to put configs outside of the #{install_dir}/conf/ dir, perhaps the cookbook shouldn’t allow for it? As I mentioned above.

What’s your use case for storing configs elsewhere? Would it be possible to use the default location, instead?

[jeffbyrnes]

I say wary b/c I recall, when I first sorted things out, that I ran into some very subtle bugs thanks to how the ZK-supplied scripts work.

[jeffbyrnes]

Reopening as we continue discussing.

[jaybocc2]

I have never had any issues running zk with alternate config dir, as long as you provide all the necessary configs, obviously. Also you need to properly set the env variables aswell, and zookeeper has accounted for this themselves by making config a confdir configurable.

Changing configs because i prefer keeping configs where god intended as we have larger teams working with various services so standardized configuration locations is beneficial to eliminating time to triage.

[jeffbyrnes]

All of this is encouraging me to step back & reconsider how all of this is structured in the first place.

That said, while this change means the zookeeper-env.sh ends up being sourced twice, that’s harmless, and means that the $ZOOCFGDIR is only set in one file, so that’s good.

I’m gonna rebase this against the current master and get it merged in and released with v10.0.0.

[jeffbyrnes]

Well, I’m trying this out, and I’m unable to get ZK to start up properly. Probably missing something. Are you doing anything extra in your wrapper cookbook, besides setting your config and changing its path?

[jeffbyrnes]

So, I got ZK to start nicely using SystemD, but runit isn‘t having it for some reason.

[jaybocc2]

Sorry i was late to respond, you need to manually make sure all configs are created/generated in the correct location.

I am not too familiar with runit so my runit change may not be proper. i'll take a look. also im noticing some merge conficts

[jeffbyrnes]

No biggie. I’ve got a copy of this branch that’s rebased against the current master; I’ll push that up separately if you want to try it out.

[jeffbyrnes]

Closed by #210