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

Create broker object with spec.config pointer #1525

Closed
Tracked by #1581
matzew opened this issue Nov 19, 2021 · 6 comments · Fixed by #1700
Closed
Tracked by #1581

Create broker object with spec.config pointer #1525

matzew opened this issue Nov 19, 2021 · 6 comments · Fixed by #1700
Labels
kind/feature New feature or request triage/accepted Issues which should be fixed (post-triage)
Milestone

Comments

@matzew
Copy link
Member

matzew commented Nov 19, 2021

Feature request

There is currently no option to use kn to create a broker and provide config parts (e.g. ponting to a ConfigMap or an other resource)

Something like below needs to be still created using k apply... :

apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  annotations:
    eventing.knative.dev/broker.class: Kafka
  name: my-kafka-broker
spec:
  config:
    apiVersion: v1
    kind: ConfigMap
    name: kafka-broker-config
    namespace: knative-eventing

Use case

UI Example


@matzew matzew added the kind/feature New feature or request label Nov 19, 2021
@rhuss
Copy link
Contributor

rhuss commented Dec 2, 2021

the config: field can point to any object ? Would it be good enough that kn supports only ConfigMaps as references ? That would make the UI much easier (e.g. --config kafka-broker-config)

@matzew
Copy link
Member Author

matzew commented Dec 7, 2021

It;s a duck, and can be anything:
https://github.com/knative/eventing/blob/main/pkg/apis/eventing/v1/broker_types.go#L71-L75

As the Rabbit impl goes, it leverages their own CRDs:
https://github.com/knative-sandbox/eventing-rabbitmq/tree/main/samples/broker/dlq#create-the-rabbitmq-broker

e.g.:

...
  spec:
    config:
      apiVersion: rabbitmq.com/v1beta1
      kind: RabbitmqCluster
      name: rokn
...

@rhuss
Copy link
Contributor

rhuss commented Dec 13, 2021

Yeah, I know it can be anything. We can support the full coordinates combo, but if ConfigMap is a commonly used choice, then we could also add a shortcut for people that need a configmap. But from your comment, I infer that ConfigMaps are as common as other objects, so having a --config rokn:rabbitmq.com/v1beta1:RabbitmqCluster:namespace and --config kafka-broker-config:v1:ConfigMap:knative-eventing is it then (and we don't add a --config-map kafka-broker-config as shortcut)

@matzew
Copy link
Member Author

matzew commented Dec 13, 2021 via email

@rhuss
Copy link
Contributor

rhuss commented Jan 4, 2022

So, let us add this config option but also think about adding the more advanced, generic options that apply to every broker implementation.

@rhuss rhuss added the triage/accepted Issues which should be fixed (post-triage) label Jan 4, 2022
@rhuss rhuss mentioned this issue Jan 27, 2022
2 tasks
@dsimansk dsimansk added this to the 1.6 (0.33) milestone Jul 12, 2022
@dsimansk
Copy link
Contributor

@matzew @pierDipi FYI if you get a chance to try and test --broker-config in kn >=1.6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request triage/accepted Issues which should be fixed (post-triage)
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants