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

Upgrade Zookeeper (to version 3.5.8) #1897

Merged

Conversation

rpudlowski93
Copy link
Contributor

PR related to task #1853

  • New role added for zookeeper
  • Upgrade tested with kafka upgrade. Cluster from 0.7.2 to feature/zookeeper-upgrade branch upgraded with success (kafka upgraded to 2.6.0, zookeeper to 3.5.8)
  • Zookeeper versions 3.5.8 and 3.4.12 are compatible with each other
  • Zookeeper doesn't have any interface for output with version
  • Zookeeper needs just for upgrading process existing snapshot in data dir: link and adding flag snapshot.trust.empty=true is not recommened , we need to ensure that the system is in good shape
  • Zookeeper upgrade playbook works on zookeeper hosts, zookeeper and kafka hosts must independent so we don't want to make some relations between them (possible use case with using only zookeeper)
  • Zookeeper needs only for upgrading any snapshot file
  • Added small fix in jmx-exporter.yml default configuration. There should be version 0.14 instead of 0.12. Cluster creation crash with 0.12.

@przemyslavic
Copy link
Collaborator

/azp run

Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty file -> core/src/epicli/data/common/ansible/playbooks/roles/zookeeper/vars/main.yml
what is this file -> core/src/epicli/data/common/ansible/playbooks/roles/zookeeper/files/snapshot.0 ?
🤔

@@ -1,6 +1,4 @@
kind: configuration/zookeeper
title: "Zookeeper"
name: default
specification:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic! 👍😍

@rpudlowski93 rpudlowski93 force-pushed the feature/zookeeper-upgrade branch from daa42d8 to 258769a Compare December 9, 2020 13:46
@rpudlowski93
Copy link
Contributor Author

rpudlowski93 commented Dec 9, 2020

empty file -> core/src/epicli/data/common/ansible/playbooks/roles/zookeeper/vars/main.yml
what is this file -> core/src/epicli/data/common/ansible/playbooks/roles/zookeeper/files/snapshot.0 ?
🤔

Empty file was already removed and additional description about the snapshot file - added :)

sk4zuzu
sk4zuzu previously approved these changes Dec 9, 2020
Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rpudlowski93 rpudlowski93 requested a review from toszo December 9, 2020 15:41
@rpudlowski93 rpudlowski93 changed the title Zookeeper upgraded to 3.5.8v Zookeeper upgraded to version 3.5.8 Dec 10, 2020
@rpudlowski93 rpudlowski93 changed the title Zookeeper upgraded to version 3.5.8 Upgrade Zookeeper (to version 3.5.8) Dec 10, 2020
@@ -0,0 +1,78 @@
---

- name: Download Zookeeper binaries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first three tasks are the same as in zookeeper role. This can be done in the future but I would opt for reusing common tasks (as we have for example for Docker).

@rpudlowski93 rpudlowski93 requested a review from to-bar December 11, 2020 15:39
@rpudlowski93 rpudlowski93 requested a review from to-bar December 15, 2020 16:16
Copy link
Contributor

@sk4zuzu sk4zuzu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -142,6 +142,16 @@
name: upgrade
tasks_from: kibana

- hosts: zookeeper
serial: 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it required?

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 this pull request may close these issues.

5 participants