Skip to content
This repository has been archived by the owner on Oct 1, 2020. It is now read-only.

expose zookeeper configuration and kafka zookeeper.connect for proper clustering #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

erulabs
Copy link

@erulabs erulabs commented Dec 16, 2015

Hello!

Love the image for development, but would like to expand it into a full blown production-ready image! I know we're a bit of distance away from that, but figured I'd rather contribute with you fine folks than to set out alone.

What I've done is expose the zookeeper configuration file by wrapping it in a start-zookeeper.sh (much like your start-kafka.sh) which reads in environment values and sets the configuration. It also allows the "zookeeper.connect" flag to be set in Kafka's configuration. I also moved the KAFKA_HOME environment variable into that script and merged the ENV docker layers.

I'll be continuing to work on this branch, so feel free to leave unmerged until I can add some documentation and actually test it across multiple hosts :)

Any thoughts about how I've handled ZK's configuration are welcome - I hesitate to do differently than the start-kafka.sh style, but I didn't want to write out every single possible ZK config option either. This feels like an OK middle ground to me, especially considering the very small number of default options ZK has.

Anyways, thanks again!

@erulabs erulabs mentioned this pull request Dec 17, 2015
@@ -41,6 +49,11 @@ if [ ! -z "$ZK_CHROOT" ]; then
sed -r -i "s/(zookeeper.connect)=(.*)/\1=localhost:2181\/$ZK_CHROOT/g" $KAFKA_HOME/config/server.properties
fi

if [ ! -z $ZOOKEEPER_SERVERS ]; then
# TODO: ZOOKEEPER_SERVERS incompatible with ZK_CHROOT
sed -r -i "s/(zookeeper.connect)=(.*)/${ZOOKEEPER_SERVERS}/g" $KAFKA_HOME/config/server.properties

Choose a reason for hiding this comment

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

This should be sed -r -i "s/(zookeeper.connect)=(.*)/\1=${ZOOKEEPER_SERVERS}/g" $KAFKA_HOME/config/server.properties

Or it is replacing the full match, not just the second group.

@gg7
Copy link

gg7 commented Jan 20, 2017

I hesitate to do differently than the start-kafka.sh style, but I didn't want to write out every single possible ZK config option either.

Relevant pull request: #65.

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

Successfully merging this pull request may close these issues.

3 participants