Zookeeper setup using offical tarball (also) on debian/ubuntu #34
Conversation
…arball (same as existing redhat/centos setup)
| @@ -4,7 +4,9 @@ zookeeper_playbook_version: "0.9.2" | |||
| zookeeper_version: 3.4.6 | |||
| zookeeper_url: http://www.us.apache.org/dist/zookeeper/zookeeper-{{zookeeper_version}}/zookeeper-{{zookeeper_version}}.tar.gz | |||
|
|
|||
| zookeeper_debian_apt_install: false | |||
There was a problem hiding this comment.
Note: 2 things we could change here:
- set default to 'true' (assuming we want to keep offering also the original debian apt install)
- rename variable to zookeeper_tarball_install and set 'true' (with the idea we could later also provide a zookeeper yum/rpm setup on redhat)
There was a problem hiding this comment.
I would suggest to prefer apt but since 3.4.6 is not available and released - lets leave priority to tarball install
There was a problem hiding this comment.
I agree.
ps: I suggest to update the documentation a bit (explaining this new setting)
In general I like to have each role variable have a 1-line documentation
…t or systemd for the init script
|
@ernestas-poskus checks passed! happy to get this merged (so I can resolve my 2 other issues, even today) 👍 |
| @@ -4,6 +4,9 @@ zookeeper_playbook_version: "0.9.2" | |||
| zookeeper_version: 3.4.6 | |||
| zookeeper_url: http://www.us.apache.org/dist/zookeeper/zookeeper-{{zookeeper_version}}/zookeeper-{{zookeeper_version}}.tar.gz | |||
|
|
|||
| # Flag that selects if systemd or upstart will be used for the init service: | |||
| # Note: by default Ubuntu 15.04 and later use systemd (but support switch to upstart) | |||
| zookeeper_debian_systemd_enabled: "{{ ansible_distribution_version|version_compare(15.04, '>=') }}" | |||
There was a problem hiding this comment.
Yeah 'version_compare' is a useful filter 👍
Note that it's only supported since ansible v1.6 (not sure if you mention anywhere a minimal ansible version requirement for the role).
Finally, when running a later test in the provided 'trusty' Vagrant-file/box, that check failed, because ansible was only 1.5.4 (installed by apt)
There was a problem hiding this comment.
@lhoss update this to 1.6 then https://github.com/AnsibleShipyard/ansible-zookeeper/blob/master/meta/main.yml#L7
There was a problem hiding this comment.
will do!
ps: I'ld also add more supported Ubuntu versions (not sure about the existing old(est) ones in that list !)
- name: Ubuntu
versions:
- all
...
- utopic
- vivid
- wily
- xenial
There was a problem hiding this comment.
maybe just change to 1.6, I will cleanup unused versions
|
Looks good, I will test this this evening and merge if everything is 🆗. Thank you for contribution ! |
Note, with the current state (of the PR), you will notice that there is no zookeeper (log4j) log file 'zookeeper.log' written (as was the case when using the .deb pkg). Notes:
|
|
Good finding !
yes this is better option |
| @@ -4,7 +4,7 @@ galaxy_info: | |||
| description: Ansible Zookeeper Role | |||
| company: http://michaelhamrah.com | |||
| license: Apache 2 | |||
| min_ansible_version: 1.2 | |||
| min_ansible_version: 1.6 | |||
There was a problem hiding this comment.
@ernestas-poskus done the last change request
final PR to fix: #26
thx for review @ernestas-poskus
Update: We could also extract the 2 'configure' tasks from tarball.yml, and re-use the same include file also in the old Debian.yml installation, to further reduce code duplication. Should I do in this PR?