feat(kafka): enable kraft

[TartanLeGrand]

Update to Kafka 27.1.2 and fix the check job

BREAKING CHANGE: Set default Kafka.kraft to true

[sschamp]

If I understand correctly, this should allow users of this chart, that were not using Kafka Kraft yet, to upgrade to the latest chart again, correct?

And then manually migrate from Kafka ZooKeeper to Kafka Kraft.
And remove all references of Kafka Zookeeper from our values.yaml

[TartanLeGrand]

@sschamp 👋 ! Kafka Kraft users can utilize the zookeeperMigrationMode values. (I will update the README for the upgrade to 22) 😄, or choose not to use Kraft (I don't know why).

New users of the chart will have the Kraft controller enabled by default. 👍

[TartanLeGrand]

Reviews ? 😄

[Mokto]

I don't think we should enforce people to switch to KRaft.

Let's make it optional?

[TartanLeGrand]

Hello 👋 @Mokto,

https://kafka.apache.org/blog#:~:text=Note%3A%20ZooKeeper%20is%20marked%20as,the%20documentation%20for%20ZooKeeper%20Deprecation.

"Note: ZooKeeper is marked as deprecated since the 3.5.0 release. ZooKeeper is planned to be removed in Apache Kafka 4.0. For more information, please see the documentation for ZooKeeper Deprecation."

I think the good idea it's to prepare to Kafka Major 4.

[TartanLeGrand]

🆙 @Mokto

[Mokto]

Fair enough, thanks for the explanation.

[Mokto]

Thanks for your work on this. You still have 2 conflicts to solve.

[Mokto]

Thanks!

[mosesdd]

Error: UPGRADE FAILED: execution error at (sentry/charts/kafka/templates/controller-eligible/statefulset.yaml:42:38): 
PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
                 Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
                 Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases
    'sasl.interbroker.password' must not be empty, please add '--set sasl.interbroker.password=$INTER_BROKER_PASSWORD' to the command. To get the current value:
        export INTER_BROKER_PASSWORD=$(kubectl get secret --namespace "sentry" sentry-kafka-user-passwords -o jsonpath="{.data.inter-broker-password}" | base64 -d)

Kafka asks for not existing secret while updating.
Also, the migration instructions are just copied from the bitnami readme, Zookeeper will be disabled while upgrading to this chart (by default values) - but migration guide says to disable AFTER migration

@TartanLeGrand could you please check?

[TartanLeGrand]

@mosesdd Could you provide your configuration values?

Kafka only requires a secret if you're using the SASL protocol in your listeners. Ensure that sasl.interbroker.password is properly set. Here's how you can retrieve the current password and apply it during the upgrade:

export INTER_BROKER_PASSWORD=$(kubectl get secret --namespace "sentry" sentry-kafka-user-passwords -o jsonpath="{.data.inter-broker-password}" | base64 -d)
helm upgrade ... --set sasl.interbroker.password=$INTER_BROKER_PASSWORD

Regarding Zookeeper, the documentation in step 6 suggests turning off migration mode on controllers and stopping Zookeeper after the migration. This seems contradictory if Zookeeper is disabled by default during the upgrade. Ensure that the migration steps are correctly followed and that dependent services are adequately managed during the update process.

[mosesdd]

@TartanLeGrand I can reproduce this without given any values at all. Fresh install of 21.6.2 -> upgrade to 22.0.0.

As I said, the named secret "sentry-kafka-user-passwords" doenst even exist.

[dVerhees]

The default values.yaml of this chart sets this, so SASL is never enabled by default, so there is no password:

listeners:
    client:
      protocol: "PLAINTEXT"
    controller:
      protocol: "PLAINTEXT"
    interBroker:
      protocol: "PLAINTEXT"
    external:
      protocol: "PLAINTEXT"

Related issue on the kafka repo. bitnami/charts#24725

[pawelmrowka]

@dVerhees please check default values.yaml in https://github.com/sentry-kubernetes/charts/blob/gh-pages/charts/sentry-22.0.2.tgz or in https://artifacthub.io/packages/helm/sentry/sentry?modal=values :

 listeners:
    interBroker:
      protocol: PLAINTEXT
    interbroker:
      containerPort: 9094
      name: INTERNAL
      protocol: SASL_PLAINTEXT   <------ 

When I overwrite that with:

kafka:
    listeners:
        interbroker:
            protocol: PLAINTEXT

it works but helm upgrade failed on post hook (another issue I believe).
Job sentry-db-check failed on:

sentry-kafka-controller-0.sentry-kafka-controller-headless is not available yet
Kraft controllers not ready. Sleeping for 10s before trying again

As I found in https://github.com/sentry-kubernetes/charts/blob/develop/sentry/templates/hooks/sentry-db-check.job.yaml#L143 it checks 9092 port, but sentry-kafka-controller-headless is available on 9093

[dVerhees]

@pawelmrowka I see the problem here.
The kafka chart uses interbroker instead of interBroker. Note the capital B

The issue with the port seems to be fixable by setting:
Values.kafka.service.ports.client

As seen here

charts/sentry/templates/_helper.tpl

Lines 405 to 411 in a148a57

	{{- define "sentry.kafka.port" -}}
	{{- if and (.Values.kafka.enabled) (.Values.kafka.service.ports.client) -}}
	{{- .Values.kafka.service.ports.client }}
	{{- else if and (.Values.externalKafka) (not (kindIs "slice" .Values.externalKafka)) -}}
	{{ required "A valid .Values.externalKafka.port is required" .Values.externalKafka.port }}
	{{- end -}}
	{{- end -}}

Which apparently is only used in the db-check job.

[phlegx]

Hi there,

@pawelmrowka Thanks! I was able to solve the same issue you had as well by setting:

kafka:
    listeners:
        interbroker:
            protocol: PLAINTEXT

I'm not sure now here if PLAINTEXT is the way to go if SASL_PLAINTEXT is the default, but I guess there is not too much harm running it with PLAINTEXT.

However I have second issue. It seems I had kraft set to enabled while zookeeper was disabled already in my yml config:

  zookeeper:
    enabled: false
  kraft:
    enabled: true

Now when I want to run the helm update to 22.1.1 I get:

│ PASSWORDS ERROR: You must provide your current passwords when upgrading the release.
│                  Note that even after reinstallation, old credentials may be needed as they may be kept in persistent volume claims.
│                  Further information can be obtained at https://docs.bitnami.com/general/how-to/troubleshoot-helm-chart-issues/#credential-errors-while-upgrading-chart-releases
│ 
│     'kraft.clusterId' must not be empty, please add '--set kraft.clusterId=$KRAFT_CLUSTER_ID' to the command. To get the current value:
│ 
│         export KRAFT_CLUSTER_ID=$(kubectl get secret --namespace "sentry-new" sentry-kafka-kraft-cluster-id -o jsonpath="{.data.kraft-cluster-id}" | base64 -d)

However there is no such secret. Do I have to set something else to plaintext to make this work?

And general question: How can I switch afterwards to SASL at some point?