Uninstalled Event Source results in "Failed to list *v1.Source: the server could not find the requested resource"

[aavarghese]

Describe the bug
If we create a new Event Source, ie. we install Redis Stream Sources, the parent controller successfully creates a new dynamic controller for this particular Source. But if we uninstall this Source, the created dynamic controller is not properly destroyed. This results in error messages in the log:

E1130 20:07:23.537960       1 reflector.go:383] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to watch *v1.Source: the server could not find the requested resource

E1130 20:07:24.782159       1 reflector.go:178] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to list *v1.Source: the server could not find the requested resource

E1130 20:07:27.383944       1 reflector.go:178] k8s.io/client-go@v11.0.1-0.20190805182717-6502b5e7b1b5+incompatible/tools/cache/reflector.go:125: Failed to list *v1.Source: the server could not find the requested resource

This code to delete controller is never called:

eventing/pkg/reconciler/source/crd/crd.go

Lines 69 to 76 in f4482c5

	if !crd.DeletionTimestamp.IsZero() {
	// We are intentionally not setting up a finalizer on the CRD.
	// This might leave unnecessary dynamic controllers running.
	// This is a best effort to try to clean them up.
	// Note that without a finalizer there is no guarantee we will be called.
	r.deleteController(ctx, gvr)
	return nil
	}

Expected behavior
Error line below isn't repeated over and over and the appropriate controllers are deleted.

To Reproduce
Install knative-eventing

k apply --filename https://github.com/knative/eventing/releases/download/v0.19.0/eventing-crds.yaml
k apply  --filename https://github.com/knative/eventing/releases/download/v0.19.0/eventing-core.yaml

Install any eventing source
For example, RedisStream

ko apply -f source/config

Delete the eventing source

For example, RedisStream

ko delete -f source/config

Knative release version
0.19.0

Additional context
Same problem also in eventing-autoscaler-keda (same code)
knative-extensions/eventing-autoscaler-keda#2

[vaikas]

I think the problem is that with genReconciler the ReconcileKind never gets called with deleted resources. You probably need to manually add a handler, kind of like we do in IMC here so that you can call the cleanup:
https://github.com/knative/eventing/blob/master/pkg/reconciler/inmemorychannel/dispatcher/controller.go#L130

[aavarghese]

@vaikas thanks for taking a look at this issue. I did attempt to implement the solution you suggested (similar to IMC) in the other sibling issue for this ... in eventing-autoscaler-keda: https://github.com/knative-sandbox/eventing-autoscaler-keda/pull/54/files#diff-9f218382398a14e88cf9424a19a54422d028e7df5a80e6d107b8d33052897859R64-R68 and unfortunately, the deleteFunc in CRD pkg doesn't get called and instead the specific SOURCE's eventhandler delete function gets called that doesn't have information of all the dynamic controllers created. So it doesn't help. This also happens here in eventing code. I tried it out for eventing this evening. So I'm blocked with both issues here until I can come up with another solution. Will appreciate any advice you have for me...

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[aavarghese]

/reopen

[knative-prow-robot]

@aavarghese: Reopened this issue.

In response to this:

/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/test-infra repository.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[lionelvillard]

/reopen
/remove-lifecycle stale

[knative-prow-robot]

@lionelvillard: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle stale

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.

[github-actions]

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

[pierDipi]

/reopen
/remove-lifecycle stale
/triage accepted

[knative-prow-robot]

@pierDipi: Reopened this issue.

In response to this:

/reopen
/remove-lifecycle stale
/triage accepted

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.