vmware-archive / kafka-trigger

Kubernetes CRD controller for Kafka topic as event source for Kubeless functions
Apache License 2.0
28 stars 34 forks source link

KAFKA_BROKERS string is not correctly split to broker list (slice) #8

Closed jsnowacki closed 4 years ago

jsnowacki commented 6 years ago

Is this a BUG REPORT or FEATURE REQUEST?: BUG REPORT

What happened: Following the external Kafka setup, if many brokers are added to KAFKA_BROKERS e.g.:

KAFKA_BROKERS=broker1:9092,broker2:9092

the variable is not split into brokersSlice as it should but passed as-is to consumer constructor, which causes connection error, as it tries to use the whole content of KAFKA_BROKERS as broker address.

What you expected to happen: KAFKA_BROKERS is split by comma and create list of brokers for consumer constructor.

How to reproduce it (as minimally and precisely as possible): See above.

Anything else we need to know?: I have a fix on my fork and can submit a PR, but I am unable at the moment to run tests as the enviroment setup desctiption does not exist (https://github.com/kubeless/kafka-trigger/blob/master/docs/dev-guide.md) and manifest files in libtest.bash seem to be missing.

Environment:

jsnowacki commented 6 years ago

BTW fork branch https://github.com/jsnowacki/kafka-trigger/tree/kafka_brokers_fix

rkelly-lg commented 4 years ago

What is the hold-up for this fix? I would expect most production-grade deployments to utilize multiple brokers...

andresmgot commented 4 years ago

@jsnowacki would you mind creating a PR with the fix?

jon-whit commented 4 years ago

@andresmgot I submitted a PR for the fix. I'd like to get this in ASAP, because we are in need of it for a production grade use case with 3 Kafka brokers.

It looks like the build is failing, but due to reasons not related to the changes.

Error: kafka: client has run out of available brokers to talk to (Is your cluster reachable?)