"kafka.triggers.eventing.knative.dev" finalizer is preventing Triggers from being deleted

[antoineco]

Describe the bug

(using the Channel-based broker with an IMC channel)

The deletion of a Trigger pointing to a KafkaSink is always blocked by the kafka.triggers.eventing.knative.dev finalizer.

I also have a few questions:

• Why kafka.triggers.eventing.knative.dev and not kafkasinks.eventing.knative.dev to follow the usual Eventing scheme?
• Why is the finalizer on the Trigger object?

apiVersion: eventing.knative.dev/v1
kind: Trigger
metadata:
  annotations:
    eventing.knative.dev/creator: myuser
    eventing.knative.dev/lastModifier: myuser
    kapp.k14s.io/identity: v1;myuser/eventing.knative.dev/Trigger/event-types-r1;eventing.knative.dev/v1
    kapp.k14s.io/original: '{"apiVersion":"eventing.knative.dev/v1","kind":"Trigger","metadata":{"labels":{"kapp.k14s.io/app":"1620119054995517000","kapp.k14s.io/association":"v1.f6f8256cbf0af308e897984262ffd937"},"name":"event-types-r1","namespace":"myuser"},"spec":{"broker":"event-types","filter":{"attributes":{"type":"dev.knative.source.github.push"}},"subscriber":{"ref":{"apiVersion":"eventing.knative.dev/v1alpha1","kind":"KafkaSink","name":"my-kafka-topic"}}}}'
    kapp.k14s.io/original-diff-md5: 7dc139d12a2acad49f0ad929abbd1d93
  creationTimestamp: "2021-05-04T09:04:24Z"
  deletionGracePeriodSeconds: 0
  deletionTimestamp: "2021-05-04T09:07:06Z"
  finalizers:
  - kafka.triggers.eventing.knative.dev
  generation: 2
  labels:
    eventing.knative.dev/broker: event-types
    kapp.k14s.io/app: "1620119054995517000"
    kapp.k14s.io/association: v1.f6f8256cbf0af308e897984262ffd937
  name: event-types-r1
  namespace: myuser
  resourceVersion: "210458603"
  selfLink: /apis/eventing.knative.dev/v1/namespaces/myuser/triggers/event-types-r1
  uid: aef097e5-4f88-43a5-96ba-f0291a56aa09
spec:
  broker: event-types
  filter:
    attributes:
      type: dev.knative.source.github.push
  subscriber:
    ref:
      apiVersion: eventing.knative.dev/v1alpha1
      kind: KafkaSink
      name: my-kafka-topic
      namespace: myuser
status:
  conditions:
  - lastTransitionTime: "2021-05-04T09:04:26Z"
    status: "True"
    type: BrokerReady
  - lastTransitionTime: "2021-05-04T09:04:26Z"
    status: "True"
    type: DependencyReady
  - lastTransitionTime: "2021-05-04T09:04:26Z"
    status: "True"
    type: Ready
  - lastTransitionTime: "2021-05-04T09:04:26Z"
    status: "True"
    type: SubscriberResolved
  - lastTransitionTime: "2021-05-04T09:04:26Z"
    status: "True"
    type: SubscriptionReady
  observedGeneration: 1
  subscriberUri: http://kafka-sink-ingress.knative-eventing.svc.cluster.local/myuser/my-kafka-topic

Expected behavior

No finalizer (?) If the finalizer is really mandatory, at least the controller should behave properly and not block the deletion of objects. For example, it should give up and throw a "warning" Kubernetes event if an error is unrecoverable.

To Reproduce

See previous manifest.

Knative release version

v0.22.0

Additional context

[pierDipi]

Why is the finalizer on the Trigger object?

I think this Trigger is associated with a broker with broker.class: Kafka, so it's not a channel-based broker, can you share the broker event-types yaml?

Why kafka.triggers.eventing.knative.dev and not kafkasinks.eventing.knative.dev to follow the usual Eventing scheme?

Why should a trigger have a finalizer called kafkasinks.eventing.knative.dev? 🤔 For a KafkaSink object, the name should be kafkasinks.eventing.knative.dev.

[antoineco]

@pierDipi thanks for the answers! As mentioned, I am not using the Kafka broker class, only the Kafka sink. The broker itself is a Channel-based broker based on an IMC channel.

apiVersion: eventing.knative.dev/v1
kind: Broker
metadata:
  annotations:
    eventing.knative.dev/broker.class: MTChannelBasedBroker
    eventing.knative.dev/creator: myuser
    eventing.knative.dev/lastModifier: myuser
    kapp.k14s.io/identity: v1;myuser/eventing.knative.dev/Broker/event-types;eventing.knative.dev/v1
    kapp.k14s.io/original: '{"apiVersion":"eventing.knative.dev/v1","kind":"Broker","metadata":{"labels":{"kapp.k14s.io/app":"1620120245002031000","kapp.k14s.io/association":"v1.e732cc9e1e1e5795a6a6daa70e87f177"},"name":"event-types","namespace":"myuser"}}'
    kapp.k14s.io/original-diff-md5: 0eedb135bc5a4c0e007516355c4db463
  creationTimestamp: "2021-05-04T09:24:14Z"
  generation: 1
  labels:
    kapp.k14s.io/app: "1620120245002031000"
    kapp.k14s.io/association: v1.e732cc9e1e1e5795a6a6daa70e87f177
  name: event-types
  namespace: myuser
  resourceVersion: "210533885"
  selfLink: /apis/eventing.knative.dev/v1/namespaces/myuser/brokers/event-types
  uid: 4db19133-76d3-4827-9072-89881385776c
spec:
  config:
    apiVersion: v1
    kind: ConfigMap
    name: config-br-default-channel
    namespace: knative-eventing
status:
  address:
    url: http://broker-ingress.knative-eventing.svc.cluster.local/myuser/event-types
  annotations:
    knative.dev/channelAPIVersion: messaging.knative.dev/v1
    knative.dev/channelAddress: http://event-types-kne-trigger-kn-channel.myuser.svc.cluster.local
    knative.dev/channelKind: InMemoryChannel
    knative.dev/channelName: event-types-kne-trigger
  conditions:
  - lastTransitionTime: "2021-05-04T09:24:14Z"
    status: "True"
    type: Addressable
  - lastTransitionTime: "2021-05-04T09:24:14Z"
    status: "True"
    type: FilterReady
  - lastTransitionTime: "2021-05-04T09:24:14Z"
    status: "True"
    type: IngressReady
  - lastTransitionTime: "2021-05-04T09:24:14Z"
    status: "True"
    type: Ready
  - lastTransitionTime: "2021-05-04T09:24:14Z"
    status: "True"
    type: TriggerChannelReady
  observedGeneration: 1

apiVersion: v1
data:
  channelTemplateSpec: |
    apiVersion: messaging.knative.dev/v1
    kind: InMemoryChannel
kind: ConfigMap
metadata:
  labels:
    eventing.knative.dev/release: v0.22.0
  name: config-br-default-channel
  namespace: knative-eventing
  ownerReferences:
  - apiVersion: operator.knative.dev/v1alpha1
    blockOwnerDeletion: true
    controller: true
    kind: KnativeEventing
    name: knative-eventing
    uid: 3db27998-a16d-4cd1-bc34-d82aec46cc23
  resourceVersion: "137838252"
  selfLink: /api/v1/namespaces/knative-eventing/configmaps/config-br-default-channel
  uid: a36bd943-fb5f-4266-b68f-52240067c13b

[pierDipi]

https://github.com/knative-sandbox/eventing-kafka-broker/blob/bec304b7e5dc53658334b70a60b82b8935877300/control-plane/pkg/reconciler/trigger/controller.go#L119-L134

We see a trigger without a broker (at least in the cache), we (wrongly) queue it since it passes the filter function, and then the generated reconciler adds the finalizer. Eventually, we don't do anything with it

https://github.com/knative-sandbox/eventing-kafka-broker/blob/bec304b7e5dc53658334b70a60b82b8935877300/control-plane/pkg/reconciler/trigger/trigger.go#L95-L99

So, this should become false https://github.com/knative-sandbox/eventing-kafka-broker/blob/bec304b7e5dc53658334b70a60b82b8935877300/control-plane/pkg/reconciler/trigger/controller.go#L128

[pierDipi]

/assign