Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configure bootstrapServers for KafkaSink from ConfigMap field #3726

Open
eshepelyuk opened this issue Feb 28, 2024 · 9 comments
Open

Configure bootstrapServers for KafkaSink from ConfigMap field #3726

eshepelyuk opened this issue Feb 28, 2024 · 9 comments
Labels
kind/feature-request triage/accepted Issues which should be fixed (post-triage)

Comments

@eshepelyuk
Copy link

Problem
Frequently kafka bootstrap servers are stored in ConfigMap field in every namespace, for various app to be able to access the same brokers uniformely. Then applications could refer the ConfigMap either mounting it via volumeMount or setup env variable via Downward API

Also a lot of applications supports bootstrap servers as a string separated by commas, like:

broker1:9092,broker2:9092,broker3:9092.

So the ConfigMap may look like

apiVersion: v1
data:
  kafka.brokers: broker1:9092,broker2:9092,broker2:9092
kind: ConfigMap
metadata:
  name: common-config

It could be great if KafkaSink could refer such ConfigMap, so the same settings can be leveraged between KafkaSink and other applications and there would be no need to configure bootstrap servers separately for KafkaSink instances.

New API may look like

apiVersion: eventing.knative.dev/v1alpha1
kind: KafkaSink
metadata:
  name: my-sink
spec:
  topic: my-sink
  bootstrapServers:
    ref:
      kind: ConfigMap
      apiVersion: v1
      name: common-config
      data: kafka.brokers

Persona:
KafkaSink users

Exit Criteria

  1. Create ConfigMap with bootstrap servers
  2. Create KafkaSink referring that ConfigMap
  3. Make sure messages are produced to kafka topic

Time Estimate (optional):
Have no idea :(

@Cali0707
Copy link
Member

Cali0707 commented Apr 4, 2024

Hey @eshepelyuk I like this idea, it does seem to make things easier for users.

Maybe for @matzew or @pierDipi or @creydr - if we decide to support this API change on KafkaSinks, does it make sense to move towards supporting it across all of our Knative Kafka components? I could see this having a few benefits:

  1. Easier for end users - make the CM once, reference it from all your Knative Kafka components
  2. Probably will let us simplify some code, and reuse logic to parse the kafka configs

I understand we currently support different secret formats and config stuff across different components, and we may need to continue to support those, so this may not replace those existing configs and instead add a newer, more consistent way to configure the Kafka pieces across all Knative Kafka components.

WDYT?

@pierDipi
Copy link
Member

pierDipi commented Apr 5, 2024

Would supporting the same Kafka broker config format (with fixed keys) work for you @eshepelyuk? https://knative.dev/docs/eventing/brokers/broker-types/kafka-broker/#create-a-kafka-broker

@eshepelyuk
Copy link
Author

Would supporting the same Kafka broker config format (with fixed keys) work for you @eshepelyuk? https://knative.dev/docs/eventing/brokers/broker-types/kafka-broker/#create-a-kafka-broker

Can you provide an example how KafkaSink will look like ?

@pierDipi
Copy link
Member

pierDipi commented Apr 5, 2024

Like this @eshepelyuk

apiVersion: eventing.knative.dev/v1alpha1
kind: KafkaSink
metadata:
  name: my-sink
spec:
  topic: my-sink
  config:
    apiVersion: v1
    kind: ConfigMap
    name: kafka-sink-config
---
apiVersion: v1
kind: ConfigMap
metadata:
  name: kafka-sink-config
data:
  bootstrap.servers: "my-cluster-kafka-bootstrap.kafka:9092"

@eshepelyuk
Copy link
Author

eshepelyuk commented Apr 5, 2024

Yes, that would work.

Copy link
Contributor

github-actions bot commented Jul 5, 2024

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 5, 2024
@eshepelyuk
Copy link
Author

go away bot

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 6, 2024
Copy link
Contributor

github-actions bot commented Oct 4, 2024

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 4, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 4, 2024
@pierDipi pierDipi reopened this Nov 4, 2024
@pierDipi
Copy link
Member

pierDipi commented Nov 4, 2024

/triage accepted

@knative-prow knative-prow bot added the triage/accepted Issues which should be fixed (post-triage) label Nov 4, 2024
@pierDipi pierDipi removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature-request triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

No branches or pull requests

3 participants