Send observability related configmaps to destination namespaces

[sayanh]

Problem
A short explanation of the problem, including relevant restrictions.

Right now, the config-observability and config-tracing configmaps are watched by different broker filters/ingress deployments across namespaces. E.g. a broker is created in namespace A then the corresponding broker filters/ingresses watch the observability configmaps in knative-eventing namespace while living in namespace A.

Pros:

• Dynamic changes in configmaps are picked by the deployments instantaneously.

Cons:

• Need strong RBAC policies for cross namespace access, which is not ideal.

The community and @n3wscott favored the other way where a copy of the configmaps are base64 encoded and pushed down to the final deployments. Here is a sample implementation.

Pros:

• No cross namespace RBAC policies required.

Cons:

• Need to kill the old deployments to reflect the dynamic changes in configmaps.

Exit Criteria
A measurable (binary) test that would indicate that the problem has been resolved.
Following components should be using the 2nd approach

• broker filter
• broker ingress
• apiserver source
• container source
  ...

Time Estimate (optional):
How many developer-days do you think this may take to resolve?

Additional context (optional)
Add any other context about the feature request here.

[grantr]

A general solution to this problem could be a controller that watches specific configmaps and replicates them to other namespaces. Then we don't need to setup unsual cross-namespace RBAC, and pods can get new config without restarting.

[lionelvillard]

I like the idea of having a generic controller for replicating configmaps. In the future, it will then be easy to extend this controller for instance to support namespace-scoped configuration by merging the knative-eventing configmaps with the one provided in the namespace.

[Harwayne]

/project Observability To do
/milestone v0.10.0

[capri-xiyue]

/assign @capri-xiyue

[grantr]

I've written a design doc describing a general ConfigMap propagation solution that solves this problem for Knative Eventing and other projects with this need. It suggests creating a namespaced CRD ConfigMapPropagation that signals the intent to copy ConfigMaps from one namespace to another:

kind: ConfigMapPropagation
metadata:
  name: knative-eventing
  namespace: default
spec:
  originalNamespace: knative-eventing
  selector:
    knative.dev/config-category: eventing

More details in https://docs.google.com/document/d/19kubGE8RQWlPFxz38G7hAtcqfihHosXCr6zXnWWOfOI/edit.

There are certainly other solutions, but this one seems cleanest to me (so far). Please comment either here or in the doc.

[lionelvillard]

This is becoming critical as we start to move event dispatchers in user-namespace (starting with the inmem dispatcher #2171).

I can take a stab at it.

/assign

@grantr

[lionelvillard]

Second thought: for now let the in-memory dispatcher using the system configmaps (doing the same as brokers).

I'll get back to this issue later on.

/unassign

[grantr]

/assign @grac3gao

[lionelvillard]

From a multi-tenancy perspective, ConfigMapPropagation does not seem to the right solution. In a multi-tenant environment, what's is needed (I think) is when a namespace is created all configmaps are copied over, without keeping a reference to the original. Copy and forget. Then each tenant can modify them at will.

Instead of "when a namespace is created", it could be "when the first eventing resource is created". No need label the namespace.

ConfigMapPropagation seems overly complicated to me but I might be missing some important use cases you have considered.

@grantr @grac3gao

[grantr]

Thanks @lionelvillard for thinking about this from a multi-tenant perspective!

Let me know if I'm interpreting incorrectly, but it seems you're expecting that tenants would prefer to have the link between the original and the copy severed once the copy is made, because they'll have their own modifications.

This is possible with ConfigMapPropagation if the tenant removes the knative.dev/config-propagation: copy label when they start making changes to the copied ConfigMap. Do you think this is too much friction (or possibly too obscure) for tenants that want to change configurations?

[lionelvillard]

I wonder if there is an easier way to do this by providing an auto mode (ie. knative.dev/config-propagation: auto). In this mode:

• configuration parameters that have been modified by the tenants are not overridden
• new keys (occurring the original and not in the copy) are copied over (eg. when upgrading Knative)
• any additional keys in the copy are not deleted (eg. logging parameter overrides)

This could eliminate the need to explain to users what knative.dev/config-propagation means and if auto is the default mode then it's even better (it eliminates the risk of accidental removal).

Regarding OwnerReference, is it only set when knative.dev/config-propagation is copy? User/tenant might be scared to modify an object owned by "someone else".

What do you think? @grantr @grac3gao

[grantr]

The auto mode seems similar to kubectl apply. Seems like a useful feature and could probably be implemented similarly (with an annotation on the copy that records the state of the original at the time of last update). Server side apply might make it easier to implement.

I don't agree it's easier than straight copying - it seems more difficult to implement correctly because there's field value comparisons involved, plus more complex messaging of persistent conflicts to both operator (modifier of the original) and user (modifier of the copy). It might be closer to the behavior users want though.

Good question about what to do with the OwnerReference. Some points to consider:

• If we leave the OwnerReference when the user removes the copy label, the copy will be deleted if the ConfigMapPropagation is deleted. Will the user want/expect this?
• If we remove the OwnerReference when the user removes the copy label... I don't think anything bad will happen. The CMP controller won't create a new copy, because the name is deterministic. The CMP controller might need to be more vigilant about checking for copies without owner references and adding them when they are under CMP control. I'm not sure what Knative ConfigMap Propagation #2333 does in this case.

The purpose of the OwnerReference is automatic deletion of copies when the CMP is deleted. If we don't want that behavior, then we probably shouldn't set OwnerReferences in the first place. Maybe this could be a flag on the CMP?

[lionelvillard]

I meant easier for users. After all they matter more than us :-)

I would expect the OwnerReference to be removed when the label is not set to copy.

[grac3gao-zz]

I'm not sure what #2333 does in this case.

Currently, CMP will show error if user deletes/changes copy label in Copy ConfigMap

I would expect the OwnerReference to be removed when the label is not set to copy

I will change to that. I am working on adding an array of copy configmap statuses under CMP (#2333 (comment)) so that user can know each copied configmap status under CMP status. If the copy configmap label is not set to copy, I am thinking of also indicating that in corresponding status (together with removing OwnerReference)

[lionelvillard]

Maybe a better alternative to configmap for overriding config values is to use annotations.

[n3wscott]

ConfigMapPropagation does not belong in Eventing.

#2493

[n3wscott]

/open

This is still real broken.