add an explicit Broker creation not relying on defaults #2756
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vaikas The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
approved
size/M
abrennan89
left a comment
abrennan89
left a comment
There was a problem hiding this comment.
Some suggestions re layout and formatting 🙂 thanks for writing this up.
Mostly just little cleanups but I'd like to see it lined out more clearly like a procedure with steps as per the suggestions.
docs/eventing/broker/README.md
Outdated
| A full example could look something like this: | ||
| ```shell | ||
| kubectl create -f - <<EOF | ||
| apiVersion: eventing.knative.dev/v1 | ||
| kind: Broker | ||
| metadata: | ||
| annotations: | ||
| eventing.knative.dev/broker.class: MTChannelBasedBroker | ||
| name: default | ||
| namespace: default | ||
| spec: | ||
| config: | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| name: config-br-default-channel | ||
| namespace: knative-eventing | ||
| EOF | ||
| ``` |
There was a problem hiding this comment.
Move this to the intro part maybe?
There was a problem hiding this comment.
OR just show the one full example and rewrite the steps around it.
There was a problem hiding this comment.
Ok, so we have the two steps you can modify, class / config. Then showing them combined? How's that?
There was a problem hiding this comment.
I think it's actually fine as is now I see it with the other changes 🙂 thank you!
There was a problem hiding this comment.
One last thing - I would actually remove the kubectl create -f - <<EOF bits so that it's just showing the YAML example. That's up to you though 🙂
|
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
googlebot
added
cla: no
docs/eventing/broker/README.md
Outdated
| 1. Modify the `eventing.knative.dev/broker.class` annotation. Replace `MTChannelBasedBroker` with the class type you want to use: | ||
|
|
||
| ```yaml | ||
| kind: Broker | ||
| metadata: | ||
| annotations: | ||
| eventing.knative.dev/broker.class: MTChannelBasedBroker | ||
|
|
||
| 1. Configure the `spec.config` with the details of the ConfigMap that defines the backing channel for the broker class: | ||
|
|
||
| ```yaml | ||
| kind: Broker | ||
| spec: | ||
| config: | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| name: config-br-default-channel | ||
| namespace: knative-eventing | ||
There was a problem hiding this comment.
you might also need to fix the indentation for the steps, idk how they'll render - maybe build a preview if you can? 🙂
|
For some reason you need to rebase I think after accepting my suggestions because of the weird CLA thing @vaikas - sorry! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
googlebot
added
cla: yes
|
@vaikas ping me when you're finished with any other changes or reviews and I can lgtm this :) thanks |
|
/lgtm |
|
@vaikas does this need to be cherrypicked to any other versions or is it for 0.17.0? @ellenevans when is the cut off for things to be included in the 0.17.0 docs release? |
|
I think .17 is fine, though it's the same in .16, so I suppose we could cp it there as well? |
5 participants
Add an example that does not rely on defaults for creating a Broker.
Proposed Changes
Add an example that does not rely on defaults for creating a Broker.