Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mgencur opened this issue Aug 20, 2024 · 3 comments · Fixed by #4077
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@mgencur
Copy link
Contributor

mgencur commented Aug 20, 2024

Describe the bug
This is similar to knative/eventing#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 #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 #4074

Knative release version
1.14

Additional context

@mgencur
Copy link
Contributor Author

mgencur commented Sep 10, 2024

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 knative-prow bot reopened this Sep 10, 2024
Copy link

knative-prow bot commented Sep 10, 2024

@mgencur: Reopened this issue.

In response to this:

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. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@Cali0707
Copy link
Member

@creydr @pierDipi for context, I tried to resolve this here:

EnableEventTypeAutocreate: features.IsEnabled(feature.EvenTypeAutoCreate) && !ownedByBroker(channel),

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants