search

id / kafka-el7-rpm

A set of scripts to package Apache Kafka into an rpm (CentOS/RedHat 7)
54 stars 40 forks source link

Add zookeeper service #25

Closed virtualdxs closed 4 years ago

virtualdxs commented 4 years ago

Per #20 add zookeeper service and set up a logs symlink since zookeeper seems to be ignoring the log4j.properties it's pointed at (I know little about this; I just know I was able to get Kafka running with my changes to this repo).

id commented 4 years ago

I'm sorry but I cannot accept this PR. It will break many installations where zookeeper does not run on the same host as kafka. When kafka cluster grows beyond 3-5 nodes, having zookeepers running on the same machines becomes a headache.

virtualdxs commented 4 years ago

It shouldn't break those, as I used Wants instead of Requires. I can remove that and just have After so there's no dependency; would that resolve your concerns?

id commented 4 years ago

No, not really. I still don't understand the reasoning behind this PR. If you need to install zookeeper, why can't you use separate package for this? https://github.com/id/zookeeper-el7-rpm

id commented 4 years ago

It's much easier to remove zookeeper scripts from this package and leave only zookeeper jar.

virtualdxs commented 4 years ago

That you'd have to ask the people who included Zookeeper in the Kafka tarball. Feel free to close the PR if you feel that the separate package is preferable.

id commented 4 years ago

Zookeeper library in kafka tarball is a necessity, just as any other third party library there. But zookeeper shell scripts are not needed - I absolutely agree. The reason they are included in kafka tarball (my gueess) is that it makes it so much easier to quickly spin up kafka node on a local machine. In production environments, however, an operator usually wants more granular level of control what package is installed where.