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.

Sign up for GitHub

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

Jump to bottom

Kafka upgrade (#2871) (#2803) #2990

Merged
merged 21 commits into from
Mar 28, 2022
Merged

Kafka upgrade (#2871) (#2803) #2990

merged 21 commits into from
Mar 28, 2022

Conversation

cicharka
Copy link
Contributor

@cicharka cicharka commented Feb 22, 2022 •
edited
Loading

  • Kafka: Upgrade to 2.8.1 version

  • Kafka: Refactor installation and upgrade tasks

  • Kafka: move upgrade tasks to Kafka role

  • Zookeeper: refactor installation and upgrade

  • Zookeeper: move upgrade tasks to Zookeeper role

  • Kafka: Refactor: rename 'kafka_var' setting

  • Kafka-exporter: verify if service definition needs to be updated even if exporter itself was not updated

  • Kafka-exporter: remove kafka.version from configuration

@cicharka cicharka marked this pull request as draft February 22, 2022 14:15
@cicharka cicharka force-pushed the feature/kafka-upgrade branch 3 times, most recently from 099e54c to 748f742 Compare February 23, 2022 15:32
@cicharka cicharka changed the title Kafka upgrade (#2871) Kafka upgrade (#2871) (#2803) Feb 24, 2022
@cicharka cicharka force-pushed the feature/kafka-upgrade branch from 362530d to e48e033 Compare February 24, 2022 08:58
@cicharka cicharka marked this pull request as ready for review February 24, 2022 09:09
@cicharka cicharka force-pushed the feature/kafka-upgrade branch from e48e033 to b7a2c00 Compare February 25, 2022 09:25
@przemyslavic
Copy link
Collaborator

przemyslavic commented Feb 28, 2022 •
edited
Loading

Don't we want to update ZooKeeper to v3.5.9? https://zookeeper.apache.org/doc/r3.5.9/releasenotes.html
It comes with Kafka 2.8.0.

@cicharka
Copy link
Contributor Author

Don't we want to update ZooKeeper to v3.5.9? https://zookeeper.apache.org/doc/r3.5.9/releasenotes.html It comes with Kafka 2.8.0.

I think that I didn't take it into account because:

  • documentation of Ecosystem for Kafka 2.8.1 says nothing about Zookeeper upgrade -> it only mention that in Kafka 2.5.0 ZooKeeper has been upgraded to 3.5.7. Additionally I have not checked Kafka archive in order to investigate Zookeeper version which comes with it (didn't know about it to be honest)
  • I checked that Kafka doesn't specify Zookeeper version required, and as we are installing it as standalone component and I have not found any issues that states to upgrade it, I just skipped it

However, since you asked about it, and even found that it comes with Kafka, we can think about 2 options:

  1. We are going to remove ZooKeeper role since it is only used as prerequisite for Kafka (for my understanding, if it's not true please correct me) and we are going to use Zookeeper that comes with Kafka (additionally Kafka states that they already removed ZooKeeper from Kafka, but still in development stage)
  2. We are going only to upgrade ZooKeeper version -> ZooKeeper role will stay, only defaults and binary changed needed.

I don't know about any other ZooKeeper usecases but if its only Kafka's prerequisite I would vote for role removal and move to use ZooKeeper that comes with Kafka. (I would also do it in another PR to do not overwhelm reviewers :) )
Sorry for long post!

@cicharka cicharka force-pushed the feature/kafka-upgrade branch 2 times, most recently from 1cb04d5 to c7b5f47 Compare February 28, 2022 22:26
rafzei
rafzei suggested changes Mar 1, 2022
View reviewed changes
ansible/playbooks/roles/kafka/tasks/common/download_and_unpack_binary.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/common/stop.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/metrics.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/setup-kafka.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/setup-kafka.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/setup-kafka.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/zookeeper/tasks/main.yml Outdated Show resolved Hide resolved
@cicharka cicharka force-pushed the feature/kafka-upgrade branch from 8466d0e to 13f1487 Compare March 4, 2022 07:46
rafzei
rafzei previously approved these changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@rafzei rafzei left a comment

Choose a reason for hiding this comment

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

LGTM

atsikham
atsikham suggested changes Mar 4, 2022
View reviewed changes
Copy link
Contributor

@atsikham atsikham left a comment

Choose a reason for hiding this comment

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

This PR also can be the place for this task - #2235. As I see, openjdk 8 is used only for Kafka and Zookeeper.

ansible/playbooks/roles/kafka/tasks/common/download_and_unpack_binary.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/common/stop.yml Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/generate-certificates.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/metrics.yml Outdated Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/upgrade/main.yml Show resolved Hide resolved
ansible/playbooks/roles/kafka/tasks/upgrade/main.yml Outdated Show resolved Hide resolved
@cicharka cicharka force-pushed the feature/kafka-upgrade branch 2 times, most recently from c5b21cc to 0789ba8 Compare March 7, 2022 08:54
atsikham
atsikham previously approved these changes Mar 7, 2022
View reviewed changes
@rafzei
Copy link
Contributor

rafzei commented Mar 8, 2022

@cicharka One more thing, could you please run ansible-lint task in devcontainer on Kafka playbook and fix the issues?

@cicharka
Copy link
Contributor Author

cicharka commented Mar 8, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@seriva seriva requested a review from erzetpe March 9, 2022 08:51
seriva
seriva previously approved these changes Mar 9, 2022
View reviewed changes
erzetpe
erzetpe previously approved these changes Mar 9, 2022
View reviewed changes
@cicharka
Copy link
Contributor Author

cicharka commented Mar 9, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cicharka added 16 commits March 25, 2022 13:42
@cicharka cicharka self-assigned this Mar 25, 2022
@cicharka
remove kafka.version configuration flag from schema
58174b6 
* We have specified version of kafka used in epiphany, therefore it
  should not be possible for user to adjust its version in
  kafka_exporter flags used in service definition
@cicharka cicharka dismissed stale reviews from erzetpe and seriva via 8bda5f7 March 28, 2022 07:08
@cicharka cicharka force-pushed the feature/kafka-upgrade branch from b0e1d40 to 8bda5f7 Compare March 28, 2022 07:08
@przemyslavic
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cicharka cicharka force-pushed the feature/kafka-upgrade branch from 8bda5f7 to 7616a9a Compare March 28, 2022 07:27
@przemyslavic
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@cicharka cicharka merged commit 7537ead into hitachienergy:develop Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@przemyslavic przemyslavic przemyslavic approved these changes

@atsikham atsikham atsikham approved these changes

@seriva seriva seriva left review comments

@erzetpe erzetpe erzetpe left review comments

@rafzei rafzei Awaiting requested review from rafzei

Assignees

@cicharka cicharka

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@cicharka @przemyslavic @rafzei @erzetpe @atsikham @seriva