Remove Legacy Client from Eventing

[n3wscott]

Fixes #2312

Proposed Changes

The following resources are removed:

• CronJobSource/sources.eventing.knative.dev
• ApiServerSource/sources.eventing.knative.dev
• SinkBinding/sources.eventing.knative.dev
• ContainerSource/sources.eventing.knative.dev

Release Note

The following resources are removed:

- CronJobSource/sources.eventing.knative.dev
- ApiServerSource/sources.eventing.knative.dev
- SinkBinding/sources.eventing.knative.dev
- ContainerSource/sources.eventing.knative.dev

Required action to remove the legacysinkbindings webhook:

kubectl delete mutatingwebhookconfiguration legacysinkbindings.webhook.sources.knative.dev

Docs
tracked by knative/docs#2267

[knative-metrics-robot]

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

File	Old Coverage	New Coverage	Delta
pkg/apis/eventing/v1alpha1/test_helper.go	79.5%	85.4%	5.8
pkg/reconciler/reconciler.go	8.7%	4.5%	-4.2

[knative-prow-robot]

@n3wscott: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-knative-eventing-go-coverage	9fee96f	link	/test pull-knative-eventing-go-coverage

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.

[vaikas]

/lgtm
/approve

/hold

until tmw mid morning to make sure folks get a chance to take a looksie.

[knative-prow-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: n3wscott, vaikas

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [n3wscott,vaikas]

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

[nachocano]

Just for the record, as I mentioned it multiple times in the Sources WG meetings.

I think it was a mistake to deprecate ContainerSource, arguably one of the most widely used Sources out there. I still see SinkBinding as a useful construct to create Sources and not as a Source itself.
I don't think we are making the lives easier for our users but rather ourselves by removing ContainerSource.

Having said that and given that it seems that I'm the only one in the community who thinks this, there must be something I'm missing from the big picture. Therefore, I disagree and commit, I'm not holding off this PR.

[lionelvillard]

@nachocano what can you do with ContainerSource that you can't do with SinkBinding?

[n3wscott]

@nachocano what can you do with ContainerSource that you can't do with SinkBinding?

there is nothing you can do with ContainerSource that you can't do with SinkBinding, plus a bunch more things are supported in SinkBinding because there is no component between you and the container you want to ship. And SinkBinding allows you to target CRDs like Serving Services and that just works out of the box.

[lionelvillard]

I think ContainerSource is useful, simpler to use, and can be easily be extended to support multiple type of Podsceable (like Kn Service).

@nachocano What about moving ContainerSource over to eventing-contrib ?

[nachocano]

@nachocano what can you do with ContainerSource that you can't do with SinkBinding?

IMO provide a higher level abstraction for users and abstract them away from core K8s constructs (e.g., deployments, statefulsets, replicasets, etc.). With ContainerSource they just create one thing instead of two (the SinkBinding and the Podspecable subject). Those are my main reasons, and also the fact that ContainerSource is widely popular already...

[nachocano]

I think ContainerSource is useful, simpler to use, and can be easily be extended to support multiple type of Podsceable (like Kn Service).

Strongly agree

@nachocano What about moving ContainerSource over to eventing-contrib ?

+1 and I can take care of the move

[n3wscott]

I think ContainerSource is useful, simpler to use, and can be easily be extended to support multiple type of Podsceable (like Kn Service).

Strongly agree

@nachocano What about moving ContainerSource over to eventing-contrib ?

+1 and I can take care of the move

The code is still in the release branches. That work should not block this PR.

[nachocano]

The code is still in the release branches. That work should not block this PR.

Correct, nobody is blocking this PR

[n3wscott]

I think ContainerSource is useful, simpler to use, and can be easily be extended to support multiple type of Podsceable (like Kn Service).

Strongly agree

@nachocano What about moving ContainerSource over to eventing-contrib ?

+1 and I can take care of the move

Maybe a better place is knative-sandbox.

[n3wscott]

/unhold

[nachocano]

@nachocano What about moving ContainerSource over to eventing-contrib ?

+1 and I can take care of the move

@lionelvillard I was starting with the migration. But putting more thought to it and reading the migration plan, it feels that moving it to eventing-contrib is not the right thing to do. IMO this should still be part of core. SinkBinding + Deployment worsens the UX.
Will create a new issue and add some of my rationale. I want to get input from all eventing leaders and folks from the community.