[bitnami/kafka] Support external access to every broker auto-discovering external ips/ports by juan131 · Pull Request #2098 · bitnami/charts

juan131 · 2020-03-20T09:20:31Z

Description of the change

This PR adds support to configure external access to every broker auto-discovering external ips/ports.

There are other features added:

Add sidecar containers
Fix issue with SASL/SSL configuration
Fix issue with Kafka Broker ID

Benefits

Easier way to configure external access to every Kafka broker
Flexibility (sidecars)

Possible drawbacks

None

Checklist

Chart version bumped in Chart.yaml according to semver.
Variables are documented in the README.md
Title of the PR starts with chart name (e.g. [bitnami/chart])
If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

carrodher · 2020-03-20T16:09:11Z

I just resolved some conflicts

rafariossaa

I found several port values not being able to be set in the values.yml, I think it is a good idea to let the user set them.
I added some comments on that on the code, but not all, please check all the statefulset.yaml file because I omitted several occurrences.

bitnami/kafka/templates/kafka-metrics-deployment.yaml

bitnami/kafka/templates/statefulset.yaml

juan131 · 2020-03-21T11:26:30Z

@rafariossaa the ports that are 'hardcoded' are the ones used on the Kafka pods. If you check the different services (JMX, Kafka, headless, etc.) you'll find that those ones are parametrised.

On K8s we don't really care much about the ports used on the pod/container, the ones that we do really care and should be customisable are the ones used on services.

miguelaeh

Thank you very much for this PR!!!
I left some minor comments, please take a look.

bitnami/kafka/templates/_helpers.tpl

bitnami/kafka/templates/kafka-metrics-deployment.yaml

miguelaeh · 2020-03-23T10:43:52Z

bitnami/kafka/templates/scripts-configmap.yaml

What about doing this?:

for i in `seq 1 $retries`;

Is that option better in terms of performance? If there's no "real" reason to change this, I'll keep it as it's.

Well, I am not sure about the performance, as seq require to call an external command, but in the other hand using the normal for it will perform a comparison and an incrementation in every iteration.
Anyway, I don't think this kind of performance is worrying here, I just suggest it because it is cleaner.

I don't consider it a "cleaner" solution. I think it's very subjective statement. Anyway, I don't think it's something important.

Ok, no problem, it was just my opinion but there is nothing wrong in have it as it is.

bitnami/kafka/templates/svc.yaml

…ing external ips/ports Signed-off-by: juan131 <juan@bitnami.com>

Signed-off-by: juan131 <juan@bitnami.com>

rafariossaa

LGTM

Signed-off-by: Bitnami Containers <containers@bitnami.com>

bitnami-bot · 2020-03-23T17:56:38Z

I have just updated the bitnami images with the latest known immutable tags:

"docker.io/bitnami/kafka:2.4.1-debian-10-r12"
"docker.io/bitnami/jmx-exporter:0.12.0-debian-10-r54"
"docker.io/bitnami/kafka-exporter:1.2.0-debian-10-r55"
"docker.io/bitnami/minideb:buster"

miguelaeh

LGTM!

yanjunli76 · 2020-03-24T02:07:48Z

bitnami/kafka/templates/svc-external-access.yaml

+      {{- else if $root.Values.externalAccess.autoDiscovery.enabled }}
+      targetPort: null
      {{- else }}
      targetPort: {{ index $root.Values.externalAccess.service.nodePort $i }}


Hi there,

The value in values.yaml and values-production.yaml is nodePorts, but here is nodePort.

yanjunli76 · 2020-03-24T02:15:50Z

bitnami/kafka/templates/statefulset.yaml

                secretKeyRef:
-                  name: {{ if .Values.auth.existingSecret }}{{ .Values.auth.existingSecret }}{{ else }}{{ template "kafka.fullname" . }}{{ end }}
+                  name: {{ include "kafka.secretName" . }}
                  key: kafka-zookeeper-password


Hi there,

If I set auth.existingSecret but not auth.zookeeperPassword in values-producation.yaml, the KAFKA_ZOOKEEPER_PASSWORD won't be enabled. I think the expected behavior is that either (auth.zookeeperPassword exist) or (auth.existingSecret exist and kafka-zookeeper-password is in the secret ) should enable the value KAFKA_ZOOKEEPER_PASSWORD

oranje42 · 2020-03-26T10:05:26Z

Hi,

I just installed the latest version of bitnami/kafka that also contains the changes from this pull-request into a Kubernetes 1.15 cluster, with metrics and serviceMonitor enabled:

metrics:
    jmx:
      enabled: true
    kafka:
      enabled: true
    serviceMonitor:
      enabled: true

However, when looking at my Prometheus targets, I get the following:

kafka/kafka-jmx-metrics/0 (0/0 up) 
kafka/kafka-metrics/0 (0/0 up)

The serviceMonitors are discovered as expected, but the endpoints aren't (and hence no metrics get collected).

It seems that changing the port names from metrics to http-metrics within both services (jmx-metrics-svc.yaml and kafka-metrics-svc.yaml) as done by this pull-request is causing that issue. If I change the port names to http-metrics also for the ServiceMonitors in servicemonitor-metrics.yamland servicemonitor-jmx-metrics.yaml for my own deployment, the endpoints get correctly discovered again by Prometheus.

Any ideas on this? Thanks!

juan131 · 2020-03-26T12:11:29Z

Hi @oranje42

Thanks so much for reporting this bug! You're right and we need to update the port names in the servicemonitor-*. Fixing this at #2145

oranje42 · 2020-03-26T13:32:43Z

Wow, that was pretty fast! Thank you!
😃

juan131 · 2020-03-27T07:47:16Z

You're welcome @oranje42 ! 😃

…ions (#2098) Signed-off-by: Bitnami Bot <bitnami.bot@broadcom.com>

juan131 requested review from miguelaeh and rafariossaa March 20, 2020 09:20

rafariossaa suggested changes Mar 20, 2020

View reviewed changes

miguelaeh suggested changes Mar 23, 2020

View reviewed changes

miguelaeh mentioned this pull request Mar 23, 2020

[bitnami/kafka] Achieve consistent broker ids during cluster restart/rebuild #2028

Merged

juan131 added 5 commits March 23, 2020 13:41

[bitnami/kafka] Support external access to every broker auto-discover…

c812d9a

…ing external ips/ports Signed-off-by: juan131 <juan@bitnami.com>

Fix issue with SASL/SSL configuration

b3b7f75

Signed-off-by: juan131 <juan@bitnami.com>

Add sidecar containers

7064ab4

Signed-off-by: juan131 <juan@bitnami.com>

Rebase from master

c077110

Signed-off-by: juan131 <juan@bitnami.com>

Port to comply with IstIO

60595fa

Signed-off-by: juan131 <juan@bitnami.com>

juan131 force-pushed the kafka-external-access branch from 4ed102c to 60595fa Compare March 23, 2020 13:01

rafariossaa approved these changes Mar 23, 2020

View reviewed changes

[bitnami/kafka] Update components versions

975be46

Signed-off-by: Bitnami Containers <containers@bitnami.com>

miguelaeh approved these changes Mar 23, 2020

View reviewed changes

juan131 merged commit e85995f into bitnami:master Mar 23, 2020

juan131 deleted the kafka-external-access branch March 23, 2020 18:16

yanjunli76 reviewed Mar 24, 2020

View reviewed changes

juan131 mentioned this pull request Mar 24, 2020

[bitnami/kafka] Setting external access true with LoadBalancer ip doesn't update advertised.listeners #2102

Closed

marcosbc mentioned this pull request Jul 31, 2020

[bitnami/kafka] Allow externalZookeepers.servers to specified both as array and string #3286

Merged

4 tasks

carrodher mentioned this pull request Aug 6, 2020

[bitnami/kafka] Fix typo in helpers #3344

Merged

4 tasks

alvneiayu pushed a commit that referenced this pull request Nov 13, 2025

[bitnami/clickhouse-operator] Release 0.3.11 updating components vers…

77a749a

…ions (#2098) Signed-off-by: Bitnami Bot <bitnami.bot@broadcom.com>

Conversation

juan131 commented Mar 20, 2020

Uh oh!

carrodher commented Mar 20, 2020

Uh oh!

rafariossaa left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

juan131 commented Mar 21, 2020

Uh oh!

miguelaeh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rafariossaa left a comment

Choose a reason for hiding this comment

Uh oh!

bitnami-bot commented Mar 23, 2020

Uh oh!

miguelaeh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oranje42 commented Mar 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

juan131 commented Mar 26, 2020

Uh oh!

oranje42 commented Mar 26, 2020

Uh oh!

juan131 commented Mar 27, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

oranje42 commented Mar 26, 2020 •

edited

Loading