Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debian/Ubuntu tasks setup outdated zookeeper (ubuntu 14.04 repos latest version is 3.4.5) #26

Closed
lhoss opened this issue Mar 1, 2016 · 14 comments · Fixed by #34
Closed

Comments

@lhoss
Copy link

lhoss commented Mar 1, 2016

We need to setup at least v3.4.6, better latest 3.4.8 as in Ubuntu 14.04 the motivation seems low to provide a recent apt package, see: https://answers.launchpad.net/ubuntu/+source/zookeeper/+question/289577

Intrestingly, an older version of this script, supported later zookeeper version, by installing from the official binaries (like it's still active for the redhat tasks)
Commit that changed this: a6160aa

Is there a chance to go back to using the official archive? (or to accept such a PR ?)

@ernestas-poskus
Copy link
Member

@lhoss you can use

zookeeper_version

variable to install required version, older version may be removed from Zookeeper FTP but you can change url as well

zookeeper_url

@lhoss
Copy link
Author

lhoss commented Jul 27, 2016

@ernestas-poskus
we are using Ubuntu 14.04 LTS , for which the latest default zookeeper package is still 3.4.5, see:
https://github.com/AnsibleShipyard/ansible-zookeeper/blob/master/tasks/Debian.yml

and in this role for Debian/Ubuntu installs, zookeeper is installed from those packages, see: https://github.com/AnsibleShipyard/ansible-zookeeper/blob/master/tasks/Debian.yml

Obviously I cannot set a higher version, if that version will not be found in the (Ubuntu) apt repository, and neither is zookeeper_url used in the debian/ubuntu case.

I see following solutions to add the needed support:

  • 1a) allow setting an optional alternative apt repos
    would be a small change to the role, BUT still requires to find a working deb package for your distro (and potentially to setup your own repos)
  • 1b) allowing go give a custom 'deb' package URL from where to download the pkg (could be good enough, assuming the zookeeper pkg from a later Ubuntu version like xenial would be compatible with Ubuntu)
    1. installing from the 'official archive' (as done here for the 'redhat' tasks)
      this would be most flexible (and consistent between the 2 distros), but needs some extra work on the init service files for debian/ubuntu (could re-use most of the other tasks from the redhat part)

Happy to get feedback on which solution would be preferred by the maintainers ( @JasonGiedymin ?), so a fix from our side could end in a merged PR !

@lhoss lhoss changed the title Debian/Ubuntu tasks setup outdated zookeeper (v2.4.5) Debian/Ubuntu tasks setup outdated zookeeper (ubuntu 14.04 repos latest version is 3.4.5) Jul 27, 2016
@ernestas-poskus
Copy link
Member

Solution 2) sounds best, it would be easier to maintain.

👍

@lhoss
Copy link
Author

lhoss commented Aug 3, 2016

OK, today/now I start implementing work on this.
As I personally try to avoid code duplication, I propose to refactor the Redhat tasks that can be re-used for debian (for ex: archive deployment into the folders) into 1 (or more) common include file(s) and including this from both the Redhat/Debian.yml.
(Ref. from Software-Eng.: https://en.wikipedia.org/wiki/Template_method_pattern )

Further changes planned:

  • introduce more variables (especially where to download the archive, keeping the default in /opt/src)
  • (opt): renaming most (better all!) variables to have the prefix 'zookeeper_' (ansible best practice!)
  • some reorderings :
    • putting the yum pkg installation ontop
    • putting the service installation (existing systemd, new 1 for debian/ubuntu) at the bottom (after myid, cfg file deployment)

hope that's in line with the committer(s)!
Final question: do all the work in one PR (still doing atomic commits) or separate PRs (with incremental improvements, but would probably be blocked to wait for the PR merges, if not doing PRs of branches depending on other unmerged branches)
@ernestas-poskus comments?

@ernestas-poskus
Copy link
Member

introduce more variables (especially where to download the archive, keeping the default in /opt/src)

If it is necessary

(opt): renaming most (better all!) variables to have the prefix 'zookeeper_' (ansible best practice!)

we should be compatible with older releases

some reorderings :
putting the yum pkg installation ontop
putting the service installation (existing systemd, new 1 for debian/ubuntu) at the bottom (after myid, cfg file deployment)

I suggest to only extract parts that are repeating

separate PRs (with incremental improvements, but would probably be blocked to wait for the PR merges, if not doing PRs of branches depending on other unmerged branches)

Separate small PR's ftw = incremental releases

@lhoss
Copy link
Author

lhoss commented Aug 4, 2016

I just pushed the 2. refactoring PR (hopefully we can get them merged quickly): #33

And the final PR for fix this issue, will be ready tomorrow:
I've got the installation already working for Ubuntu (tested 14.04) using a 3.4.6/3.4.8 tarball, except that the upstart script needed to be modified to work with Ubuntu (and that change unfort. didn't work for centos6, so need to find another way OR use diff. init templates for the 2 distros )

Change done to make it work in Ubuntu/Debian:

script
    exec {{zookeeper_dir}}/bin/zkServer.sh start-foreground > /var/log/zookeeper/init-zookeeper.log 2>&1

Another thing I discovered (I will create a new issue):
With the (redhat) tarball setup the zookeeper.log.dir (log4j property) currently isn't set to a good default (like /var/log/zookeeper ).

Update: (10am CEST): Got it working (by looking at the well done ansible-consul role!):

script
    exec sudo -u zookeeper -g zookeeper {{zookeeper_dir}}/bin/zkServer.sh start-foreground > /var/log/zookeeper/init-zookeeper.log 2>&1

@ernestas-poskus does it make sense for me to open the 3rd PR that provides a working version of debian-from-tarball-install (still fully downward compatible, I even kept the 'debian apt install' as an alternative, but disabled by default)?

@lhoss
Copy link
Author

lhoss commented Aug 5, 2016

@ernestas-poskus
Copy link
Member

@lhoss teralytics/ansible-zookeeper@master...teralytics:zookeeper_debian_tarball_install

Cant comment on it.

+---
+- name: Start zookeeper service [says start, but stops and disables]
+  service: name=zookeeper state=stopped enabled=no
+
+- name: Apt install required system packages. [says install, but removes]
+  apt: pkg={{item}} state=absent
+  with_items:
+    - zookeeper
+    - zookeeperd

I don't like deb-uninstall.yml and extra variables that come with it, why do you need it ?

@lhoss
Copy link
Author

lhoss commented Aug 7, 2016

Good question!
Imagine you have an existing zookeeper installation, based on the deb. pkg. Now you switch the installation method and don't want to keep any unneeded remainders.
The advantage of having the uninstaller built-in the role is that it can be done just before the new (tarball) installation is applied, and so it easier to be done during a 'rolling upgrade' (uninstall+install one host at a time, recommend method for zookeeper cluster)
But I agree, its not usual, nor looking nice nor useful for any new role users, so I'll take it out again (and probably put it into a simple tasks include, so it can still be easily integrated into a playbook combining both deb-uninstall and tarball install. )

ps: It's interesting that most roles don't provide any sort of uninstaller (ansible-ceph's uninstaller playbook being a nice counter-example though)
I can leave it out completely or keep in separate playbook+tasks-include-file ?

@ernestas-poskus
Copy link
Member

But I agree, its not usual, nor looking nice nor useful for any new role users, so I'll take it out again (and probably put it into a simple tasks include, so it can still be easily integrated into a playbook combining both deb-uninstall and tarball install. )

I can leave it out completely or keep in separate playbook+tasks-include-file ?

I would suggest to keep it in your own playbook where you run this zookeeper role

It's interesting that most roles don't provide any sort of uninstaller (ansible-ceph's uninstaller playbook being a nice counter-example though)

Yes it's odd, personally never encountered uninstaller.

@lhoss
Copy link
Author

lhoss commented Aug 8, 2016

@ernestas-poskus I removed the uninstall stuff from my branch preview and submitted a PR -^
ps: I just came to me, that I have not thought about extending the tests to include a test for the new ubuntu tarball installer
(update: actually I got this wrong! with the current default ´zookeeper_debian_apt_install=false´the existing test actually test the new tarball install !)

@lhoss
Copy link
Author

lhoss commented Dec 12, 2016

fyi https://answers.launchpad.net/ubuntu/+source/zookeeper/+question/289577

In short: After our latest fresh cluster install, I found out (way too late) that opting for a non-deb pkg install with zookeeper is suboptimal , as the mesos install later will still require the zookeeper deb pkg installed (and will silently install it , and on the first zookeeper restart after that, you will get a invalid myid exception, due to the unconfigured zookeeper (deb) conf somehow got priority).

Happy to hear about easier solution, than the ones I listed in above link.

@lhoss
Copy link
Author

lhoss commented Dec 12, 2016

nice there's an alternate ppa .. will try this out (and if good, would create a PR that allows to optionally set a ppa):

There is a PPA with 3.4.8-1 for trusty https://launchpad.net/~ufscar/+archive/ubuntu/zookeeper (provides also the required backport of netty-3.9)

Remark: not tested by me, standard disclaimer for PPAs with respect to security, trustworthiness etc. applies

@lhoss
Copy link
Author

lhoss commented Feb 16, 2017

for anyone finding this, lately we merged support to install zookeeper from above ppa repos -> #58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants