knative-extensions / eventing-kafka-broker

Alternate Kafka Broker implementation.
Apache License 2.0
185 stars 117 forks source link

Event Discovery: EventType Autocreate on replies to subscriptions should reference the subscription #4076

Open mgencur opened 3 months ago

mgencur commented 3 months ago

Describe the bug This is similar to https://github.com/knative/eventing/issues/7699 , just for KafkaChannel instead of InMemoryChannel. There should be an event type that references a Subscription but the only event type that is produced references the KafkaChannel:

oc get eventtypes -oyaml
apiVersion: v1
items:
- apiVersion: eventing.knative.dev/v1beta2
  kind: EventType
  metadata:
    annotations:
      eventing.knative.dev/creator: system:serviceaccount:knative-eventing:knative-kafka-channel-data-plane
      eventing.knative.dev/lastModifier: system:serviceaccount:knative-eventing:knative-kafka-channel-data-plane
    creationTimestamp: "2024-08-20T10:52:04Z"
    generation: 1
    name: et-channel-tlxhukhp-a113de44136be1100daefa6094b418eb
    namespace: test-rargaffb
    ownerReferences:
    - apiVersion: messaging.knative.dev/v1beta1
      kind: KafkaChannel
      name: channel-tlxhukhp
      uid: dc3e319b-7eec-421a-a30f-ba492747717f
    resourceVersion: "21746568"
    uid: da98632a-1b1b-40fb-a292-f5e5ee335f69
  spec:
    description: Event Type auto-created by controller
    reference:
      apiVersion: messaging.knative.dev/v1beta1
      kind: KafkaChannel
      name: channel-tlxhukhp
      namespace: test-rargaffb
    schema: http://example.com/schema
    source: http://example.com/source
    type: test.imc.custom.event.type.reply
  status:
    conditions:
    - lastTransitionTime: "2024-08-20T10:52:04Z"
      status: "True"
      type: Ready
    - lastTransitionTime: "2024-08-20T10:52:04Z"
      status: "True"
      type: ReferenceExists
    observedGeneration: 1
kind: List
metadata:
  resourceVersion: ""

I have attached a reproducer `TestChannelSubscriptionEventTypeAutoCreate to https://github.com/knative-extensions/eventing-kafka-broker/pull/4074

Expected behavior It's not clear if there should be two events types created in this case. The expectation is that one event type would reference the Subscription and one would reference KafkaChannel.

To Reproduce Run the reproducer test attached to https://github.com/knative-extensions/eventing-kafka-broker/pull/4074

Knative release version 1.14

Additional context

mgencur commented 2 months ago

The TestChannelSubscriptionEventTypeAutoCreate test from https://github.com/knative-extensions/eventing-kafka-broker/pull/4074/files still shows that the eventtype references a KafkaChannel instead of Subscription. See https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-extensions_eventing-kafka-broker/4074/channel-reconciler-tests-sasl-plain_eventing-kafka-broker_main/1833439756705337344

Reference:Kind = KafkaChannel, Namespace = test-shtvzyyt, Name = channel-rkavthwn

/reopen

knative-prow[bot] commented 2 months ago

@mgencur: Reopened this issue.

In response to [this](https://github.com/knative-extensions/eventing-kafka-broker/issues/4076#issuecomment-2341456696): >The TestChannelSubscriptionEventTypeAutoCreate test from https://github.com/knative-extensions/eventing-kafka-broker/pull/4074/files still shows that the eventtype references a KafkaChannel instead of Subscription. See https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-extensions_eventing-kafka-broker/4074/channel-reconciler-tests-sasl-plain_eventing-kafka-broker_main/1833439756705337344 > >Reference:Kind = KafkaChannel, Namespace = test-shtvzyyt, Name = channel-rkavthwn > >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
Cali0707 commented 2 months ago

@creydr @pierDipi for context, I tried to resolve this here: https://github.com/knative-extensions/eventing-kafka-broker/blob/4cf38f1acae9cfc0b3cc6747d5e1f6140b9c8e1b/control-plane/pkg/reconciler/channel/channel.go#L715

But, I guess it is not being picked up on correctly :(