Awkward way to set Kafka broker.id

[ehud-eshet]

Hi,

By default, broker.id is set to -1 (leading to auto generation by kafka). If you delete the statefulset and then recreate it (while not recreating zookeeper as well), each pod will get a new (higher) broker id which will not match the broker ids of topic __consumer_offsets. Then, consumer.subscribe() will hang forever.

Script /run.sh allows to override broker.id by setting environment variable BROKER_ID_COMMAND. Pod name is -N while N is 0 to replicas - 1. I would like to set FIRST_BROKER_ID as an integer. Then automatically set broker.id for each pod as FIRST_BROKER_ID + N.

The undocumented way to do it is to define the following environment variables:

• name: FIRST_BROKER_ID value: "50"
• name: BROKER_ID_COMMAND value: "if [[ $HOSTNAME =~ .+-([0-9]+)$ ]] ; then echo $[${FIRST_BROKER_ID:-10}+${BASH_REMATCH[1]}] ; else echo -1 ; fi"

It would be great, if BROKER_ID_COMMAND is documented. Or even better, if the existence of FIRST_BROKER_ID will automatically set the appropriate broker ids without the need to specify BROKER_ID_COMMAND.

Thanks.

[javsalgar]

Hi,

But this FIRST_BROKER_ID is an environment variable used by kafka or it is just for overriding the broker ids in order to avoid the issue you mentioned? Also, just for confirming, you are using persistence, and this persistence is not keeping the broker id, right?

[stale[bot]]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.

[stale[bot]]

Due to the lack of activity in the last 5 days since it was marked as "stale", we proceed to close this Issue. Do not hesitate to reopen it later if necessary.

[dedsm]

Hello, I just stumbled across this when trying to debug some topics not being able to recover after disaster, and it was because of the broker ids constantly changing, setting brokerId: "" in the values and the solution listed above make the broker ids to be repeatable after restarts

[javsalgar]

Hi,

Which version of the container are you using? I don't see that behavior here in https://github.com/bitnami/bitnami-docker-kafka/blob/master/2/debian-9/rootfs/libkafka.sh

[dedsm]

@javsalgar https://github.com/bitnami/bitnami-docker-kafka/blob/master/2/debian-9/rootfs/setup.sh#L19 the default in the chart is setting it to -1, which causes every broker to use a new id after pod deletion, I'm using this in the answers for the chart, causing the broker id to be the same every time taking it from the hostname:

        brokerId: ""
        extraEnvVars:
        -   name: KAFKA_CFG_RESERVED_BROKER_MAX_ID
            value: "2000"
        -   name: BROKER_ID_COMMAND
            value: if [[ $HOSTNAME =~ .+-([0-9]+)$ ]] ; then echo $[${FIRST_BROKER_ID:-1000}+${BASH_REMATCH[1]}+1]
                ; else echo -1 ; fi

[javsalgar]

Hi,

Thanks for the input :) I'm sure that we could improve the user experience in case of disaster recovery.

[megfigura]

We also ran into this issue. It'd be really nice if the "brokerId" value in the Helm chart auto-incremented when there are multiple brokers.

[JRdm]

That's the issue i stumble upon :) Thanks for having solutions.

[JRdm]

Hi,

Which version of the container are you using? I don't see that behavior here in https://github.com/bitnami/bitnami-docker-kafka/blob/master/2/debian-9/rootfs/libkafka.sh i do not see any BROKER_ID declaration ... ? thanks

[shapeofarchitect]

In my case I didn't provide any value so kafka set the broker id 0,1,2 in my pods. But I agree to change this to comment and not hardcode the broker.id in the values at all.

[javsalgar]

Hi,

Thanks for the feedback! It is still in our backlog but as soon as we have more news we will update the ticket

[vpartington]

I'm trying to find a way to address the lack of backwards compatibility introduced in v8.0.0: https://github.com/bitnami/charts/tree/master/bitnami/kafka#to-800 Telling the user of my product to uninstall and re-install is not an option for me.

This is what my research so far has taught me:

1. This change was introduced in PR https://github.com/bitnami/charts/pull/2028 and explained in this note: https://github.com/ghShu/kafka-notes/blob/master/notes/20200310-achieve-consistent-broker-ids-in-kafka-cluster.md
2. Later on, that logic was moved to the setup.sh script: https://github.com/bitnami/charts/commit/e85995f809d8d92d9eebbd05617070457708e152#diff-1df902bbec84369dd7f5dede825a0939R59
3. Strangely enough the brokerId variable in values.yaml comes back and the environment variable KAFKA_CFG_BROKER_ID is set to its value in that same commit: https://github.com/bitnami/charts/commit/e85995f809d8d92d9eebbd05617070457708e152#diff-04fb571f58b361a4d3d50088e0317419R225 However, because of the logic in the setup.sh script, that does nothing. Does anyone know why that was added back in?

I was able to manually patch my installation by adding the following line in the setup.sh script:

sed -i "s/^broker.id=.*$/broker.id=${KAFKA_CFG_BROKER_ID}/" /bitnami/kafka/data/meta.properties

in the configmap and then restarting the pod.

But now I'm struggling to get that into the chart in a reproducable manner.

[juan131]

Hi @vpartington

Thanks so much for the great analysis.

However, because of the logic in the setup.sh script, that does nothing. Does anyone know why that was added back in?

That was my fault. You're right and it's totally worthless to add it in the environment section since it's overwritten in the setup.sh. I created a PR to fix this (see https://github.com/bitnami/charts/pull/3062)

But now I'm struggling to get that into the chart in a reproducable manner.

What about including a new parameter (sth like "customSetup") that allows you replacing the default setup.sh script with your own one?

[vpartington]

Hi @juan131,

Thank you for your reply. And for confirming my suspicion about superfluous bits in setup.sh and then removing them. Saves me from a lot of headscratching. ;-)

A new parameter to modify (or add to) the existing setup.sh script would help me out a lot.

Alternatively, a way to insert an init container to "patch" stuff before the real container starts would work as well. A lot of other Bitnami charts have that option. See the redis-cluster chart for an example: https://docs.bitnami.com/kubernetes/infrastructure/redis-cluster/administration/sidecars-init-containers/

Regards, Vincent.

[javsalgar]

Hi, in other charts we also include the option of overriding the command with a custom one so you can define whatever logic. I think that would be more standard with our charts than the customSetup value. We are in the process of adding a new batch of standardizations to our charts, so expect these values soon :)

[vpartington]

Hi @javsalgar,

Being able to override the command would work too! And it would be a lot easier than setting up a complete new init-container.

Would a user of the chart also be able to add scripts to the configmap? The fix looks like this:

sed -i "s/^broker.id=.*$/broker.id=${KAFKA_CFG_BROKER_ID}/" /bitnami/kafka/data/meta.properties

and that might be hard to squeeze into a single command, together with invocation of the real setup.sh script.

Regards, Vincent.

[javsalgar]

Hi,

We would also allow using the extraVolumes and extraVolumeMounts so you can add your custom configmaps if necessary. You would need to deploy the configmap beforehand or use the extraDeploy parameter and add the yaml of the configmap there.

[vpartington]

Aha, of course!

Then I look forward to having the ability to override the standard command. Shall I create a PR for that or would you rather do it yourselves?

Thanks! Regards, Vincent.

[javsalgar]

Hi,

Feel free to create a PR adding this feature, just check how we do it in other charts like kong. Thank you very much!

[vpartington]

Hi @javsalgar,

3128 is ready for review. 😄

Regards, Vincent.

[javsalgar]

Thanks! Reviewed :)