Add documentation for Brokers changing channel provisioners. by Harwayne · Pull Request #996 · knative/eventing

Harwayne · 2019-03-28T17:31:15Z

Proposed Changes

Add documentation for Brokers changing channel provisioners.

Release Note

NONE

knative-prow-robot · 2019-03-28T17:31:33Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Harwayne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [Harwayne]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Harwayne · 2019-03-28T18:09:40Z

/assign @Abd4llA

knative-prow-robot · 2019-03-28T18:09:42Z

@Harwayne: GitHub didn't allow me to assign the following users: Abd4llA.

Note that only knative members and repo collaborators can be assigned.
For more information please see the contributor guide

Details

In response to this:

/assign @Abd4llA

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

matzew · 2019-03-28T20:28:09Z

/assign @matzew

evankanderson

A few comments/questions.

Should this be moved to knative/docs at some point?

evankanderson · 2019-03-28T21:16:52Z

+
+If `spec.channelTemplate` is specified:
+1. Delete the `Broker`.
+1. Create the `Broker` with the updated `spec.channelTemplate`.


So the Broker won't reconcile the channel based on the channel template?

Should it?

It won't.

As to whether it should, I'm not entirely sure. I didn't do it because I was worried about a single small change meaning that all in-flight events are irrevocably dropped. I left a TODO in the code to figure it out so that if we decide we want the change, we know where to do it.

evankanderson · 2019-03-28T21:17:09Z

+1. Delete the `Broker`.
+1. Create the `Broker` with the updated `spec.channelTemplate`.
+
+If `spec.channelTemplate` is not specified:


Is this specifically for the default Broker, or is is this intended as a general solution?

General solution. Incidentally, it is the only one that will work with the injected default Broker, because the injected default does not specify spec.channelTemplate.

Isn't this a bit all very manual? and might be error prone ?

It is manual. What errors would it be prone to?

When implementing Broker, I made the decision not to update Channels even if the Broker's spec.channelTemplate changed because it would drop all in flight events. We can revisit that decision and have it updated on the fly. The code change would be quite small and there is already a comment in the place it would occur (see #996 (comment)).

If someone really wants to cause an update on the fly, they can delete the Channels backing the Broker manually. The Broker controller will create new ones with the updated template. But I don't want to document this as I consider it an implementation detail that may change in the future.

matzew · 2019-04-02T09:27:52Z


+### ClusterChannelProvisioner
+
+`Broker`'s use their `spec.channelTemplate` to create their internal `Channel`s, which dictate the durability guarantees of events sent to that `Broker`. If `spec.channelTemplate` is not specified, then the [default provisioner](https://www.knative.dev/docs/eventing/channels/default-channels/) for their namespace is used.


n3wscott · 2019-04-03T15:43:39Z

+
+#### Setup
+
+Have a `ClusterChannelProvisioner` installed and set as the [default provisioner](https://www.knative.dev/docs/eventing/channels/default-channels/) for the namespace you are interested in. For development, the [`in-memory` `ClusterChannelProvisioner`](https://github.com/knative/eventing/tree/master/config/provisioners/in-memory-channel#deployment-steps) is normally used.


markdown lint

I don't know exactly which linter rules we use, so I just leave it to the sock puppet to fix within a day of submission.

Harwayne · 2019-04-11T17:21:33Z

Ping. I think I've addressed all the comments. If this documentation is still desired, I'd like to get it in.

matzew · 2019-04-11T21:16:44Z

/lgtm

Co-authored-by: serverless-qe <serverless-support@redhat.com>

Add documentation for changing channel provisioners.

3d437a6

googlebot added the cla: yes Indicates the PR's author has signed the CLA. label Mar 28, 2019

knative-prow-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Mar 28, 2019

knative-prow-robot requested a review from grantr March 28, 2019 17:31

knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 28, 2019

lost -> lose

3efa9f5

Harwayne marked this pull request as ready for review March 28, 2019 18:09

knative-prow-robot assigned matzew Mar 28, 2019

evankanderson reviewed Mar 28, 2019

View reviewed changes

Harwayne added 2 commits April 1, 2019 12:39

Merge branch 'master' into broker-provisioner

117b2a9

Links go to knative.dev.

d2f71ce

matzew reviewed Apr 2, 2019

View reviewed changes

n3wscott reviewed Apr 3, 2019

View reviewed changes

knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 11, 2019

knative-prow-robot merged commit 58382f4 into knative:master Apr 11, 2019

creydr pushed a commit to creydr/knative-eventing that referenced this pull request Jan 23, 2025

Update dependabot configurations (knative#996)

9e620f0

Co-authored-by: serverless-qe <serverless-support@redhat.com>


		### ClusterChannelProvisioner

		`Broker`'s use their `spec.channelTemplate` to create their internal `Channel`s, which dictate the durability guarantees of events sent to that `Broker`. If `spec.channelTemplate` is not specified, then the [default provisioner](https://www.knative.dev/docs/eventing/channels/default-channels/) for their namespace is used.


		#### Setup

		Have a `ClusterChannelProvisioner` installed and set as the [default provisioner](https://www.knative.dev/docs/eventing/channels/default-channels/) for the namespace you are interested in. For development, the [`in-memory` `ClusterChannelProvisioner`](https://github.com/knative/eventing/tree/master/config/provisioners/in-memory-channel#deployment-steps) is normally used.

Conversation

Harwayne commented Mar 28, 2019

Proposed Changes

Uh oh!

knative-prow-robot commented Mar 28, 2019

Uh oh!

Harwayne commented Mar 28, 2019

Uh oh!

knative-prow-robot commented Mar 28, 2019

Uh oh!

matzew commented Mar 28, 2019

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Harwayne Apr 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Harwayne commented Apr 11, 2019

Uh oh!

matzew commented Apr 11, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Harwayne Apr 1, 2019 •

edited

Loading