Add admin server options from ZK 3.5.5

[jkufro]

Added the admin server configuration options introduced in Zookeeper 3.5.0. See 3.5.5 documentation "AdminServer configuration" section for details.

EDIT: CI build here

[deric]

Thanks for the PR. It looks ok, however we need to figure out a way how we won't break backwards compatibility for versions < 3.5.5. Adding by default configuration options like:

admin.enableServer=true

to earlier versions would produce invalid configuration. The admin server section needs to be either skipped by default or we have to check ZooKeeper version being installed.

[jkufro]

@deric thanks for your feedback. Please review the most recent changes.

newest CI build

[deric]

@jkufro Thanks for quick response! Unfortunately this won't be enough. We need to have a valid ZooKeeper version string that can be evaluated for any installation method.

Then the template code should be wrapped in something like:

if versioncmp($::zookeeper_version, '3.5.5') >= 0 {

}

[jkufro]

@jkufro Thanks for quick response! Unfortunately this won't be enough. We need to have a valid ZooKeeper version string that can be evaluated for any installation method.

Then the template code should be wrapped in something like:

if versioncmp($::zookeeper_version, '3.5.5') >= 0 {

}

@deric would you advise moving config.pp L43 to zoo.cfg.erb L31 and to get rid of the @enable_admin_server variable? Or is that check just not enough to verify that the version is >= 3.5.5? EDIT: I suppose @enable_admin_server is too close to the naming of @admin_enable_server and could be named better.

[hdeadman]

Is there any feedback on this PR that hasn't been addressed? I would like to see this get merged as I just had a service fail to start up because zookeeper came up first and grabbed port 8080. I will fork and use that in the meantime but would rather go back to this module once it is merged.

[deric]

Is there any feedback on this PR that hasn't been addressed?

@hdeadman Yes, there's still the backwards compatibility issue, that block this from merging.

Variable $::zookeeper::archive_version won't be initialized in case someone is installing ZooKeeper from native package (parameter ensure can be used to pass version string)