eventing icon indicating copy to clipboard operation
eventing copied to clipboard

Duplicated EventTypes from sources with "auto create" enabled

Open matzew opened this issue 2 years ago • 8 comments

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 same type but different sources, or pointing to different brokers, 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

matzew avatar Sep 14 '23 13:09 matzew

/assign

Cali0707 avatar Sep 14 '23 14:09 Cali0707

/triage accepted

Cali0707 avatar Sep 14 '23 14:09 Cali0707

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 avatar Oct 10 '23 12:10 matzew

@matzew It's ok for any sophisticated UI/tool.

Not so sure about kubectl though.

aliok avatar Oct 12 '23 14:10 aliok

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

pierDipi avatar Jan 24 '24 08:01 pierDipi

This is not yet ready and depends on https://github.com/knative/eventing/issues/7265

pierDipi avatar Jan 24 '24 08:01 pierDipi

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 avatar Jan 24 '24 09:01 matzew

@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...

Cali0707 avatar Jul 29 '24 23:07 Cali0707