This adds docs for the new mtnamespace work.

[mattmoor]

This work is happening here: knative/eventing#2778

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mattmoor

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 [mattmoor]

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

[vaikas]

Is this really going to work? if you first create the NS, ns controller will create the broker? perhaps we should say here that you have to create with the label?

[mattmoor]

You'd have to do it before installing things today. Honestly, setting this label to "disabled" with EITHER namespace controller should clean up the auto-generated Broker, opened knative/eventing#2796.

[matzew]

So, do we really need this, in every namespave ?

k get broker --all-namespaces
NAMESPACE          NAME      READY   REASON   URL                                                                                 AGE
default            default   True             http://broker-ingress.knative-eventing.svc.cluster.local/default/default            11s
knative-eventing   default   True             http://broker-ingress.knative-eventing.svc.cluster.local/knative-eventing/default   11s
knative-serving    default   True             http://broker-ingress.knative-eventing.svc.cluster.local/knative-serving/default    11s
kourier-system     default   True             http://broker-ingress.knative-eventing.svc.cluster.local/kourier-system/default     11s
kube-node-lease    default   True             http://broker-ingress.knative-eventing.svc.cluster.local/kube-node-lease/default    9s
kube-public        default   True             http://broker-ingress.knative-eventing.svc.cluster.local/kube-public/default        11s
kube-system        default   True             http://broker-ingress.knative-eventing.svc.cluster.local/kube-system/default        11s

now, what if there is an even bigger cluster, with 100+ namespaces - all not related to Knative at all? they all really need a borker ?

[mattmoor]

Honestly the notion of a "default" Broker per-namespace evokes a strong parallel to K8s ServiceAccounts, which are created in every namespace whether I ask for it or even use it (or not).

So IMO we should either:

1. Have the goal be that this is always on in every namespace (as with SAs)
2. Drop the namespace magic entirely and just have folks create Brokers.

Can we quantify or qualify the problem or cost in the 100+ namespaces that you are concerned about? I'd rather not deal in FUD if we can share concrete problems.

[grantr]

Drop the namespace magic entirely and just have folks create Brokers.

Separately from the question of whether MT brokers should be opt-in or opt-out, the original purpose of the namespace magic was to work around the per-broker requirement of additional deployments, services, RBAC, etc. It seems like this doesn't apply to the MT broker since it doesn't create any additional per-Broker objects.

[mattmoor]

For the sake of having an "off" switch should things go bad (as they did for SinkBinding 👀 😅 ), I will add some logic to change the default behavior, but I still think that this should be our goal state, and if it's not I think we should probably have a serious convo about the motivation for using a namespace controller.

FWIW, in serving, the nscert controller, which creates wildcard certs per namespace is on by default, but an optional installable. The model here strikes the balance I was mentioning above... You can create wildcard certs manually, or you can run the NS controller to get them automagically, but opting IN per-namespace makes me wonder why we aren't just having folks create the resource per-namespace (as the mechanism to opt in).

[antoineco]

Late to this conversation, but I wanted to link my comment on knative/eventing#2946 (comment).

I'm really not a fan of this auto-injection feature based on BROKER_INJECTION_DEFAULT. From the bottom of my head I can't think of a scenario where someone would want to enable a broker in every namespace, regardless of its cost. Most clusters I've seen in production have just as many system namespaces as user namespaces. System namespaces are not labeled out of the box. User namespaces are unlikely to be all relying on Kn Eventing.

Also I've seen a lot of people (ab)using kubectl get all. Questions will be asked when BROKER_INJECTION_DEFAULT is set to true.

Regarding the parallel with ServiceAccounts, I think there is one major difference here: all Pods require a service account to even be scheduled, but few need a Broker in every possible namespace. (see comment above)

Anyway, I think keeping BROKER_INJECTION_DEFAULT but setting it to false by default is a good compromise. But I'd be curious to hear real world use cases for this feature.

[mattmoor]

Pods and Brokers is apples/oranges. I cannot create a Trigger without a Broker.

Could Kubernetes have forced folks to pre-create SAs? Yes.
Do all namespaces have to contain Pods? No.
So why would they do it? It's a better experience that's simpler for users.

Frankly that likely costs the system more than what we're doing since it involves storing and rotating auth (used or not).

[mattmoor]

I'm going to close this, as it's no longer correct. As we shift toward MT as the default we should revisit this as it's a strictly better DX in my experience with it thus far.