Add Kafka sink finalizer

[pierDipi]

Fixes #158

Proposed Changes

• Add Kafka sink finalizer

Release Note

- 🎁 Add new feature
You can stream Knative HTTP CloudEvents into Apache Kafka using KafkaSink objects
  - See also KafkaSink CRD, types, and reconciler - https://github.com/knative-sandbox/eventing-kafka-broker/pull/134

Docs

• Document Apache Kafka Sink knative/docs#2814

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

[pierDipi]

/assign @slinkydeveloper

[slinkydeveloper]

/hold

I prefer to keep this on hold until we merge #162

[slinkydeveloper]

minor nit

[matzew]

@pierDipi should I PR some doc, like:

https://gist.github.com/matzew/e2c2fcd2696a346f25b8bc9e64bfd0fa

[pierDipi]

@matzew Sure, feel free to assign yourself to the issue: knative/docs#2814, I appreciate your help!

[pierDipi]

/unhold

[pierDipi]

ERROR: timeout waiting for pods to come up
my-cluster-zookeeper-0                     1/1   Running             0     5m5s
my-cluster-zookeeper-1                     1/1   Running             0     5m5s
my-cluster-zookeeper-2                     0/1   ContainerCreating   0     5m5s
strimzi-cluster-operator-9968fd8c9-vwq4m   1/1   Running             0     5m31s

Zookeeper ...
/retest

[pierDipi]

kafka.kafka.strimzi.io/my-cluster created
Waiting until all pods in namespace kafka are up......................................................................................................................................................
ERROR: timeout waiting for pods to come up
strimzi-cluster-operator-9968fd8c9-cs2kz   0/1   ImagePullBackOff   0     5m30s
Failed Pod (data in YAML format) - strimzi-cluster-operator-9968fd8c9-cs2kz

/retest

[pierDipi]

ERROR: timeout waiting for pods to come up
my-cluster-zookeeper-0                     1/1   Running             0     5m8s
my-cluster-zookeeper-1                     1/1   Running             0     5m8s
my-cluster-zookeeper-2                     0/1   ContainerCreating   0     5m8s
strimzi-cluster-operator-9968fd8c9-92vh6   1/1   Running             0     5m32s

Zookeeper,
/retest

[pierDipi]

/retest

[pierDipi]

/retest

[pierDipi]

/retest

[slinkydeveloper]

/retest

[pierDipi]

 operation.go:75: Failed to get {Pod v1}-first-filtered-event-sender ready: <nil>

#183 tries to remove flakes,
/retest

[slinkydeveloper]

Thanks for fixing some of my leftovers of the previous pr too!

[pierDipi]

@slinkydeveloper I'm not sure decontextualizing variable names and log messages is better, why do you suggest to use contract everywhere instead of specific names?
It seems too vague to me and the names shouldn't be the same as the type of the variable.
We know that in the contract there are sinks, why do we call the variable contract and not sinks? If it is for consistency I'll change names everywhere to be more specific.

[mattmoor]

Produced via:
gofmt -s -w $(find -path './vendor' -prune -o -path './third_party' -prune -o -name '*.pb.go' -prune -o -type f -name '*.go' -print)
goimports -w $(find -name '*.go' | grep -v vendor | grep -v third_party | grep -v .pb.go | grep -v wire_gen.go)

[slinkydeveloper]

@pierDipi the reason to keep those names is:

• Name the variable as the entity it contains
• The config map contains a json with contract entity, to me logging sinks config map is far more confusing because i don't really see any sinks entity anywhere. It's the business logic that gives the sinks meaning to the contract entity
• One of the things I understood is that, in the long term, we might use the data plane indipendently from what is actually implementing: we could have a single data plane deployment and you could have in the same cm both sinks and brokers, the controller operates on all these. in that case, contract is contract 😄

[pierDipi]

Name the variable as the entity it contains

Naming is hard 😄, I would change it to name the variable as the thing it represents. :)

The config map contains a json with contract entity, to me logging sinks config map is far more confusing because i don't really see any sinks entity anywhere. It's the business logic that gives the sinks meaning to the contract entity

"contract" isn't a thing from a user perspective since they don't know anything about our terminology, anyway I updated it. :)

[pierDipi]

One of the things I understood is that, in the long term, we might use the data plane indipendently from what is actually implementing: we could have a single data plane deployment and you could have in the same cm both sinks and brokers, the controller operates on all these. in that case, contract is contract

I would keep them separate at least in the default setup, and you can do that already today by changing the env variables of the controller. 😉

[slinkydeveloper]

"contract" isn't a thing from a user perspective since they don't know anything about our terminology, anyway I updated it. :)

But that's something they actually see in the protobuf schema and in the json inside the config map, that's what i mean. And end up naming with 2 different names the same thing in the logs is not a good idea too

[knative-metrics-robot]

The following is the coverage report on the affected files.
Say /test pull-knative-sandbox-eventing-kafka-broker-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
control-plane/pkg/apis/eventing/v1alpha1/kafka_sink_lifecycle.go	85.7%	75.0%	-10.7
control-plane/pkg/reconciler/broker/broker.go	89.2%	88.8%	-0.4
control-plane/pkg/reconciler/kafka/topic.go	88.5%	88.6%	0.1
control-plane/pkg/reconciler/sink/controller.go	88.9%	90.0%	1.1
control-plane/pkg/reconciler/sink/kafka_sink.go	78.0%	84.9%	6.9

[pierDipi]

Ready for another pass!

[slinkydeveloper]

/lgtm
/approve

Thanks for another huge contribution!

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pierDipi, slinkydeveloper

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [slinkydeveloper]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment