Eventing: Create a default Broker upon Trigger creation by default

[antoineco]

_This echoes a thread started on Slack (#eventing, April 10th)_

Problem

We've had multiple discussion, especially in knative/docs#2325 and knative/eventing#2946, about making the automatic injection of a (MT-)Broker per namespace the default.

To sum up the current state of the discussion:

• _pro_
  Creating a MT-Broker is super cheap and makes the developer's experience simpler, because at the end of the days people want to use Triggers, and Triggers are only useful with an existing Broker.
• _contra_
  A cluster is rarely composed of Eventing namespaces only, and users don't expect objects to be magically injected everywhere on their behalf.

I think we all agree we'd rather not be creating Brokers manually whenever we want to use some Triggers. We want to keep the developer experience as simple as possible by letting users focus on Triggers, unless they explicitly express the desire to manage Brokers on their own.

Therefore I would like to suggest that a Trigger can be created _without explicitly referencing a Broker_ and _without the need to define any extra label_, in which case the mutation webhook injects broker: default inside the Trigger's spec and the Trigger reconciler creates that default Broker if it's missing.

@grantr pointed out this feature already exists since #2034, so we would simply have to reverse the requirement on the knative-eventing-injection label to make it opt-out instead of opt-in.

Persona:

Event consumer (developer)

Exit Criteria

A developer can create a Trigger without explicitly creating a Broker beforehand. This action should cascade into the creation of a default Broker with sane defaults within the same namespace as the Trigger.

Time Estimate (optional):

A day.

Additional context (optional)

• knative/docs#2325
• knative/eventing#2946
• Loosely related: Logical Broker (Proposal)

[antoineco]

/kind proposal
/area eventing

[matzew]

kinda related... https://github.com/knative/eventing/issues/1837

[markfisher]

I believe auto-creating a Broker upon Trigger creation sets a precedent that goes against the fine-grained level-triggered reconciliation model of Kubernetes, leading to a more coarse-grained transactional view of a collection of collaborating resources.

Kubernetes resource reconciliation often involves cascading creation of lower-level _dependency_ resources which are owned/managed and/or delegated to (e.g. Deployment creates ReplicaSet, ReplicaSet creates Pods), but not to create collaborating "sibling" - or worse, "upstream" - resources as a matter of convenience. Instead, the status of a resource should reflect that it's not ready due to lack of any of its independently-reconciled collaborating resources. That fine-grained, independent approach also promotes flexibility in terms of dynamic configuration, e.g. a Trigger could be updated to refer to a different Broker dynamically, without having to delete and recreate the resources. Also, users should be aware of the semantics of the Broker, especially as more Broker implementations become available, and the associated configuration parameters are exposed (durability, delivery guarantees, etc), and of course users need to be aware of the Broker resource instance as the addressable for publishing events. Having to determine the latter via the Trigger is counter to the goal of decoupling producers and consumers.

All that said, in https://github.com/knative/docs/pull/2325#issuecomment-608078448 @mattmoor referred to default ServiceAccounts as a parallel, but I view the motivation for default ServiceAccounts as not being analogous for several reasons:

1. plays a bootstrapping role for a Pod when secrets are needed to pull images for the containers in the Pod
2. provides access to the API Server if necessary (when token mounting is enabled), with the default ServiceAccount promoting the least-privilege principle
3. immutable due to the nature of those 2 features, e.g. updating a ServiceAccount with imagePullSecrets would be irrelevant/misleading once past the bootstrapping phase
4. falling back to the default ServiceAccount is managed by an admission controller due to the bootstrapping/immutability
5. ServiceAccount has no runtime footprint

Finally, it seems as though the very idea of creating a default Broker was motivated by the complexity of Channel-based Brokers, as the Channel/Subscription model does not itself satisfy the Broker/Trigger requirements, thereby requiring several additional components, especially for filter-matching. When instead dealing with a Broker implementation that simply delegates to a messaging system that does satisfy the requirements, that motivating complexity is no longer an issue. So, I personally think it would be better to reconsider the role of Channels in the implementation of a Broker (especially as a default) than to set this precedent that doesn't follow typical Kubernetes resource reconciliation idioms.

[matzew]

I agree that the creation of the broker should be much easier. I try to start that by making the MTChannelBased broker a default: https://github.com/knative/eventing/pull/3135

I agree that it is a bit of odd that you create a broker, when you add some annotation on a trigger. Perhaps with the MTChannelBased broker the default we can also skip that.

And I think the desire is based on statements above.. it's complex to set up all dependency correctly.

What if... it would have been possible to have some _flexible_ fitlering on the subscription (which would make sense for the underlying channel teck)... what if ... would we than have the broker? Perhaps not? Just wondering, as I read the comment of "he Channel/Subscription model does not itself satisfy the Broker/Trigger requirements"

In the long run, perhaps we can have a non-channel based InMemoryBroker, as a "reference impl" for a non channel based broker in "knative/eventing". Or update the MTChannelBased to be that ...

[mattmoor]

I completely agree with Mark's sentiment that having the Trigger create Brokers is backwards.

@mattmoor referred to default ServiceAccounts as a parallel, but I view the motivation for default ServiceAccounts as not being analogous for several reasons:

The parallel is simple:

• Pods cannot exist without SAs, Triggers cannot exist without Brokers.
• If I create a Pod/Trigger without the corresponding SA/Broker, the former is broken (fails to "bootstrap")
• The defaulting for Pod/Trigger fill in a "default" for the SA/Broker, which is assumed by the defaulting logic to exist.

ServiceAccount has no runtime footprint

Everything costs something 😉

The spirit of serverless is to offer a pay-per-use model, and so if a Broker/Trigger has non-trivial cost when unused, then maybe we are doing something wrong, no? This was always a key issue for me with the ST Broker (black box user perspective).

---

My position is simple: the namespace stuff as it exists is not beneficial and it is actually harmful in various ways (e.g. impedance mismatch, RBAC).

1. If we want users to take an action per Broker created, that action should be the moral equivalent of creating a Broker through yaml (or kn), and (brace for contentious point) we should eliminate the concept of a "default" Broker on Triggers 😱.

2. If we want to preserve the assumption, then we must make it a valid assumption. The namespace reconciler is an eager mechanism for this (with precedent, however contentious). The original point of this issue is to offer a lazy mechanism for this (also contentious).

---

One last thought: It is possible for us to make "the assumption" and creation of the "default" broker a thing external to core. 🤯

If we separated what's currently the mtnamespace reconciler into a separate installable that also ran a mutating webhook that filled in the name of the "default" brokers it creates, then we can make the core strictly 1. above, and still let folks have 2. if the Broker operated is cheap enough to allow for it.

What I like about this is that it gives us very clear API contracts that are validated and defaulted and lets different audiences have the semantics they want through composition.

[markfisher]

The parallel is simple:

Pods cannot exist without SAs, Triggers cannot exist without Brokers.
If I create a Pod/Trigger without the corresponding SA/Broker, the former is broken (fails to "bootstrap")

@mattmoor I disagree. It's a very different case precisely due to the fact that a Pod needs a ServiceAccount to "bootstrap" (cannot even recover if the ServiceAccount is created later). That's not true for a Trigger. A Trigger can be created and have a BrokerReady status "False" until the Broker is available. That would also be the same situation if a Broker disappears/reappears, which as I see it is one of the main value propositions of fine-grained k8s reconciler's to support that type of resilience at runtime, and should be designed for proactively.

[markfisher]

@mattmoor here's what I think is a better parallel based on something I just happened to do:

I created a PingSource and its sink is my "default" Broker. I didn't create the Broker yet, but the PingSource reconciler doesn't therefore try to go create it. Instead its status simply reflects that its sink is unavailable. As soon as I create the Broker and its ingress is ready, that status changes. The same thing happens again if I delete and recreate the Broker. That feels like an idiomatic Kubernetes reconciliation experience. Why should it be different for the relationship between Trigger and Broker?

[mattmoor]

I created a PingSource and its sink is my "default" Broker.

Unless we've started defaulting sinks to the "default" Broker as well, then it's not the same.

Can you point me to another example where an ObjectReference is defaulted to a resource that doesn't exist?

A Trigger can be created and have a BrokerReady status "False" until the Broker is available

I'm not arguing against this at all, in fact this is exactly what I would expect in general[1].

What I'm arguing against is the fact that we default this reference to something, but aren't willing to make sure it exists.

I'd be perfectly happy with us eliminating the mtnamespace stuff entirely if we remove the defaulting assumption that goes with it.

What I can't wrap my head around is what value folks think the namespace reconciler with opt-in is getting us?

---

[1] - That Pods don't recover from this feels like a broken failure mode, but I'd be astonished if that was exclusively why default SA's are autocreated 😉

[vaikas]

I think (and maybe I'm wrong), there are two distinct things that are at play here:

1. Having an operator tool that says, inject a Broker automatically into some set of namespaces. Having this off by default is fine with me, as long as there's a knob. The role here is to have a way to have a Eventing enabled experience, where the user walking into the namespace can use eventing by using Trigger only and they do not need to understand how to create Brokers, the platform operator can provide this. It seems that we can rip this controller into it's own releasable artifact and by default, you do not get this. Is this bit contentious?

2. Having magic that automatically creates default Brokers based off of Triggers. and it seems like most folks are discussing this?

[markfisher]

sorry @mattmoor - my example was confusing because the name of my broker is literally "default" as that was the example I started from, but to be clear, I am in favor of NO default creation of Brokers at all. That's exactly what I'm trying to say. My broker should be named "rabbit" :-)

[markfisher]

@vaikas I am trying to make the case for "no knob" but rather ensure that Brokers are easy to create. I feel like the complexity of channel-based brokers started this whole tangent. Creating a Broker should be as simple as applying what's actually quite simple YAML, like any other k8s resource. If some distribution or CLI tool makes it more of an integrated experience, fine, but why bake that into the level of reconcilers?

[mattmoor]

I am trying to make the case for "no knob" but rather ensure that Brokers are easy to create

I think this is exactly what we are saying eventing core should be:

1. No defaulted broker in the Trigger,
2. No namespace reconciler (opt in or our).

What @vaikas means by "knob" is what I've been thinking of as a separate installable component (not in core!), where folks can opt-in to the magic. So from the perspective of "core" we've effectively deleted mtnamespace, but folks can opt-back-into that behavior by running that as a separate thing.

[vaikas]

Yes, sorry for being sloppy with terminology, by knob I meant that it's a choice. If you do not want the behaviour that auto creates brokers, then that's fine.

I think the existing mtnamespace (let's rename it) that explicitly (not via namespace labeling) creates the brokers is precisely what we want.
Second thing is that it can provide defaulting (if we want) for Triggers in the namespaces that it has injected. If it creates brokers named my-awesome-broker, then it could default that.
But, pkg/apis/eventing/v1beta1/trigger_defaults.go will not default broker fields anymore, and will stop accepting if it's not filled in.

Concerns with that approach?

[grantr]

:hand: I have concerns:

• The action proposed here is a larger scope than the original request. This issue is about adding a feature, and the conclusion so far is that we will not add the feature AND remove another feature.
• There are stakeholders relying on the current behavior that are not included in the discussion (users and contributors). Removal of a feature deserves a separate discussion with a clear description so everyone impacted has a reasonable chance to weigh in.
• Removing the default is a backward incompatible change to the API. Trigger yamls that were previously valid will become invalid (including several in our docs and samples). I don't think we can do this in v1beta1 regardless of our reasoning.

I don't fully understand the logic of eliminating the spec.broker defaulting, but that's ok, there are plenty of things I don't understand. More important than my understanding is identifying how the absence of this feature will concretely improve the user experience. Is it too complex to understand? Is it enabling a use case that was previously difficult or ignored? Is there an audience of users that wants to disable defaulting of this field? I don't see those considerations in this discussion.

[mattmoor]

I agree that this is an inappropriate place to discuss this, but alas I was linked here from a PR that was removing an unrelated feature that we use 😉

I'll open another issue to discuss this because it keeps getting smeared across other issues, PRs and slack threads.

[antoineco]

I still think the discussion here was valuable, obviously there are different opinions and expectations about the user experience around Brokers.

Regarding the original request, it seems like we have a conclusion: most people do _not_ want to have a mechanism that creates Brokers automatically in core, whether it's controlled by a Trigger or not.

[vaikas]

@grantr I was not proposing on dropping features, just being able to select which pieces of functionality we provide and where :) And agree on other stakeholders, just hoping to get some consensus amongst the group on what we think we want and what are the contentious bits. But opening a new issue to discuss seems like a good idea :) Until it forks lol :)

[mattmoor]

I've opened https://github.com/knative/eventing/issues/3138 and tried to keep the initial "goal" free of my biases. I will follow up with my own thoughts, but I'd encourage folks to weigh in there. Please feel free to loop in others, but I've seeded it with several folks involved in recent convos.