Duplicated EventTypes from sources with "auto create" enabled
Describe the bug
when auto-create feature is enabled and I use build-in sources, I get each event twice:
- one occrance from the source/duck reconciler (ownerReference: the source)
- one from the broker (ownerReference: the broker that received it)
See:
k get eventtypes.eventing.knative.dev -A
NAMESPACE NAME TYPE SOURCE SCHEMA REFERENCE NAME REFERENCE KIND DESCRIPTION READY REASON
default 0869e4f870fe6756b8709a84bd4704d1 dev.knative.sources.ping /apis/v1/namespaces/default/pingsources/ping-source-two my-broker Broker True
default 528a923dba2997343e8260eb2f76fc10 dev.knative.sources.ping /apis/v1/namespaces/default/pingsources/ping-source-broker my-broker Broker True
default et-my-broker-28e4c32ad62d0d20c633fd923b016e9c dev.knative.sources.ping /apis/v1/namespaces/default/pingsources/ping-source-broker my-broker Broker Event Type auto-created by controller True
default et-my-broker-5ea16d24144025d9cd501232308ee51c dev.knative.sources.ping /apis/v1/namespaces/default/pingsources/ping-source-two my-broker Broker Event Type auto-created by controller True
There is a (complex/weird) definition of uniqueness, generally on EventType:
In order to uniquely identify an EventType, we would need to look at the tuple (
type,source,schema,broker), as there might be EventTypes with the sametypebut differentsources, or pointing to differentbrokers, and so on.
See https://github.com/knative/eventing/tree/main/docs/registry
Looking at the list above, that uniqueness (type, source, schema, broker), we still have each doubled (one by the auto-create and one from the duck). Perhaps we should check that, when doing the autocreate?
FWIW, the "source duck" created events do have the actual source (e.g. PingSource) as an owner
Knative release version 1.12 1.11
Additional context Add any other context about the problem here such as proposed priority
/assign
/triage accepted
How bad is that really... if we have n identical event types (e.g. where source/type) match.
I think some "user interface" or "tool", would anyways de-duplicate those (and likely ignore the name, since generated in both cases).
@matzew It's ok for any sophisticated UI/tool.
Not so sure about kubectl though.
For this issue, I believe the best way forward is for the source/duck reconciler to stop creating concrete event types and each individual source will start creating templeted event types that have as reference the CRD or a generic gvk without name, for example:
(the templates variables syntax is TBD but I'm using the same language as the feature track here: https://docs.google.com/document/d/1hxCpQMI_qCy2CsHEv0nits4t65DExkizgg3KLvD4z2s/edit)
apiVersion: eventing.knative.dev/v1beta3
kind: EventType
metadata:
name: dev.knative.sources.ping
namespace: knative-eventing
spec:
reference:
apiVersion: sources.knative.dev/v1
kind: PingSource
attributes:
- name: type
value: dev.knative.sources.ping
required: true
- name: specversion
value: "1.0"
- name: id
required: true
- name: source
value: "/apis/v1/namespaces/$namespaceName/pingsources/$souceName"
- name: time
required: true
description: CloudEvent type forpayloads on a specified cron schedule
or the CRD reference
apiVersion: eventing.knative.dev/v1beta3
kind: EventType
metadata:
name: dev.knative.sources.ping
namespace: knative-eventing
spec:
reference:
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
name: pingsources.sources.knative.dev
attributes:
- name: type
value: dev.knative.sources.ping
required: true
- name: specversion
value: "1.0"
- name: id
required: true
- name: source
value: "/apis/v1/namespaces/$namespaceName/pingsources/$souceName"
- name: time
required: true
description: CloudEvent type forpayloads on a specified cron schedule
This is not yet ready and depends on https://github.com/knative/eventing/issues/7265
I really like setting the reference to the CRD, like:
reference:
apiVersion: apiextensions.k8s.io/v1
kind: CustomResourceDefinition
name: pingsources.sources.knative.dev
in "generic" cases, instead of allowing no reference. Thanks for closing the #7264 issue
@matzew @pierDipi with EventType v1beta3 in now, should we move to setting the reference like above now? If so, we can probably move forwards here...