Repository: redhat-developer/service-binding-operator
Pull request #442 (2020-04-29 06:10:46)

Add watching for workloads
### Motivation
Fix issue https://github.com/redhat-developer/service-binding-operator/issues/422

### Changes
Besically align https://github.com/redhat-developer/service-binding-operator/issues/422#issuecomment-619260714 Except:
Instead of hard-coded watching typical workloads at starting, this change will add watching for GVKs defined in application selector at runtime. As for typical workloads with status as subresources,  the existing predicate function which checks the`metadata.generation` is able to avoid noises.

Also split `Set/Remove SBRAnnotations` function into two seperate functions. It is useful to combine the updating for application CR. 

### Testing
Unit tests added and manual testing passed.


openshift-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by:
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **dhritishikhar**
You can assign the PR to them by writing `/assign @dhritishikhar` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["dhritishikhar"]} -->
openshift-ci-robot: Hi @int8dev. Thanks for your PR.

I'm waiting for a [redhat-developer](https://github.com/orgs/redhat-developer/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://github.com/orgs/redhat-developer/people) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

int8dev: @DhritiShikhar Please help to review and comment. Thanks!
DhritiShikhar: @int8dev I tested this PR using this [example](https://github.com/redhat-developer/service-binding-operator/tree/master/examples/nodejs_postgresql):

[1] Creating a PostgreSQL DB
[2] Creating an SBR
[3] Creating a nodejs application
[4] Deleting the nodejs application
[5] Re-creating the nodejs application with the same name

But I don't see secret injected in the nodejs application.

Here is the video for the same -> https://youtu.be/q6pFTaD-WWM



int8dev: Hi @DhritiShikhar, thanks for your feedback.

I think the reason is that the delete event is ignored in step 4). So controller does not enqueue the SBR for reconciling. We can remediate this in the to return true in `DeleteFunc` of `predicates`. After this change the SBR resource will be in a fail-reconcile loop as the application CR is not found until the application CR is recreated.
For other cases if we try to update the application object (e.g, remove the envs injected by operator), the reconcile should be triggered and get the injection fixed.

This should be easy fix but I found another issue in my local testing. After I deleted the application CR, I can confirm the reconcile triggered as expected. But the reconcile ended up with success even the application no longer exist. This can also be observed If we create a SBR directly with a non-existing `resourceRef` in `applicationSelector`. I think it is introduced in the recent fixes for deletion issues.


int8dev: For specific, I have no idea why this error is ignored. 
https://github.com/redhat-developer/service-binding-operator/blob/master/pkg/controller/servicebindingrequest/binder.go#L569
DhritiShikhar: @int8dev 

Yes, I have tested [this]( https://github.com/redhat-developer/service-binding-operator/issues/422#issuecomment-618484986) scenario and the case is solved by your PR.

[Here](https://youtu.be/h2Vh0Yt35gA) is a video of my test. 

However, We need to add a fix for the [original comment ](https://github.com/redhat-developer/service-binding-operator/issues/422#issue-599321019) also which is the deletion issue.

We can handle that in another PR. Meanwhile, I will merge this PR when the CI tests pass. 
DhritiShikhar: @int8dev 

Yes. There have been some issues that have been created by a commit -> https://github.com/redhat-developer/service-binding-operator/commit/f5dfc45e069ef32d7d6fee04e1f575f068d04ae5

We are working on those.
isutton: @DhritiShikhar Can you hold this for a moment so I can review it?

I'd like to also ask to separate the changes in `SetSBRAnnotations` and the watch feature itself.


DhritiShikhar: @isutton Sure
int8dev: Please hold this PR and I created another PR https://github.com/redhat-developer/service-binding-operator/pull/444 as request by @isutton. 
I will rebase the changes in this PR with better interface abstraction for watching.
Thanks you! @isutton 

Avni-Sharma: > Hi @DhritiShikhar, thanks for your feedback.
> 
> I think the reason is that the delete event is ignored in step 4). So controller does not enqueue the SBR for reconciling. If the delete event is handled properly the SBR resource will be in a fail-reconcile loop as the application CR is not found until the application CR is recreated.
> 
> For other cases if we try to update the application object (e.g, remove the envs injected by operator), the reconcile should be triggered and get the injection fixed.
> 
> I did some test from my side. After I deleted the application CR. I can confirm the reconcile triggered as expected. But the reconcile ended up with success even the application no longer exist. This can also be observed If we create a SBR directly with a non-existing `resourceRef` in `applicationSelector`. I think it is introduced in the recent fixes for deletion issues.

@int8dev  So if we delete an application, and then try to delete the sbr instance, then the sbr won't get deleted as it will keep searching for the application to come up again, and since the application is not available, it will be requed and the deletion of the sbr goes into a deadlock.
int8dev: @Avni-Sharma I think we should check the deletionTimestamp of SBR before we can actually ignore the application not found error. If the SBR is not in a deletion path, it is expected that the SBR goes into a fail-requeue state. 
DhritiShikhar: 1. Created a DB, application, SBR
2. Deleted application successfully
3. Deleted SBR successfully

[Here](https://youtu.be/_OGQqUGuqoo) is a video for the same 
sbose78: Thanks!

@isutton , why do we need to make two PRs for this? Given the refactor was needed for a specific capability to work, I feel it should be a part of the same PR. It helps account for the reasons behind a refactor later on if they are in the same commit in master.
int8dev: @isutton 
I like the idea of watching all kinds of resources registered in apiserver as discussed in the meeting. But I don't think it is possible for the following reasons.
1) There is no "magic" behind the controllers/informers. For specific, the cotroller does not pull all resources in the apiserver and generate event for reconciling which is too much overhead. Basically the underlying behaviour of informer is a listing/watching request for specific GVK such as. `GET https://api.some-kube-apiserver/apis/apps.openshift.io/v1alpha1/servicebindingrequests?resourceVersion=xxx&watch=true`. That is what a `Reflector` does in client-go. There is no API to list/watch all kinds of resources in apiserver.
For operator-sdk which use controller-runtime to create informers, the functions are defined in https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/cache/internal/informers_map.go#L230-L302. We can tell that each informer for GVK is attatched to a list/watch request for a dynamicClient
2) There should be RBAC rules for controllers to limit the access to apiserver. That meas even we could get all GVKs installed in the cluster, it is barely impossible for controller to list/watch all kinds of resources. 


isutton: > @isutton
> I like the idea of watching all kinds of resources registered in apiserver as discussed in the meeting. But I don't think it is possible for the following reasons.
> 
> 1. There is no "magic" behind the controllers/informers. For specific, the cotroller does not pull all resources in the apiserver and generate event for reconciling which is too much overhead. Basically the underlying behaviour of informer is a listing/watching request for specific GVK such as. `GET https://api.some-kube-apiserver/apis/apps.openshift.io/v1alpha1/servicebindingrequests?resourceVersion=xxx&watch=true`. That is what a `Reflector` does in client-go. There is no API to list/watch all kinds of resources in apiserver.
>    For operator-sdk which use controller-runtime to create informers, the functions are defined in https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/cache/internal/informers_map.go#L230-L302. We can tell that each informer for GVK is attatched to a list/watch request for a dynamicClient.

IIRC, the first reconciliation when the operator starts, all resources for a given GVK are *pulled* from the API server and locally enqueued to wait for a worker to process.

If I gave the impression that there were some ready API for doing that, I apologize; it is required to [list all APIResources](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.13/#apiresourcelist-v1-meta) and individually watch for each known GVK using the aforementioned APIs.

> 2. There should be RBAC rules for controllers to limit the access to apiserver. That means even we could get all GVKs installed in the cluster, it is barely impossible for controller to list/watch all kinds of resources.

That's an interesting point that should be further discussed IMO.

Are you planning to have one Operator running per project or cluster wide?
isutton: > @isutton , why do we need to make two PRs for this? :) Given the refactor was needed for a specific capability to work, I feel it should be a part of the same PR. It helps account for the reasons behind a refactor later on if they are in the same commit in master.

From my understanding, the functionality this PR provides are present only in `reconciler.go`, `controller.go` and `sbrcontroller.go`.

IMHO the other changes are not related and should have its own discussion; keeping it will only add noise to this PR.
int8dev: Thank you for the clarification @isutton.

> Are you planning to have one Operator running per project or cluster wide?

For our current design we decide to run service-binding-operator at cluster level. And the cluster might be pretty big (i.e >100 nodes, >10k namespaces). 
openshift-ci-robot: @int8dev: PR needs rebase.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
sbose78: Hi @int8dev Anything to be done here?
lanceliuu: I will rebase this PR after some fixes of the testing. That will be in another PR.
Avni-Sharma: Hi @lanceliuu this PR is waiting on which PR, can you link that PR here
lanceliuu: @Avni-Sharma This now depends on PR https://github.com/redhat-developer/service-binding-operator/pull/457
Avni-Sharma: Are we considering on watching over all  backing services as well or this PR is focused only on the application workload?
cc @isutton 
lanceliuu: @Avni-Sharma  @isutton 
I have rebased this PR on master branch.
isutton: > Are we considering on watching over all backing services as well or this PR is focused only on the application workload?
> cc @isutton

I believe this can be considered finished, and a new PR should be created for the services' watcher implementation.
DhritiShikhar: @isutton @Avni-Sharma Some extra support is needed to add service watcher. I will raise a PR for it.
DhritiShikhar: /retest
openshift-ci-robot: @lanceliuu: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.4-e2e | 9d8524bc13d0e471a4775a5d4afc1bf42338ad42 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.4-e2e/465) | `/test 4.4-e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=442). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:lanceliuu). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
DhritiShikhar: /test 4.4-e2e
openshift-ci-robot: @lanceliuu: The following test **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.4-e2e | 9d8524bc13d0e471a4775a5d4afc1bf42338ad42 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.4-e2e/466) | `/test 4.4-e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=442). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:lanceliuu). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
codecov[bot]: # [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=h1) Report
> Merging [#442](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=desc) into [master](https://codecov.io/gh/redhat-developer/service-binding-operator/commit/37de16ca5769fbfc451a197e503c991aab68b525&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `71.83%`.

[![Impacted file tree graph](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/graphs/tree.svg?width=650&height=150&src=pr&token=CICBeVWlrC)](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #442      +/-   ##
==========================================
- Coverage   63.44%   63.42%   -0.03%     
==========================================
  Files          27       27              
  Lines        1789     1807      +18     
==========================================
+ Hits         1135     1146      +11     
- Misses        488      495       +7     
  Partials      166      166              
```


| [Impacted Files](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/controller/servicebindingrequest/controller.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L2NvbnRyb2xsZXIuZ28=) | `0.00% <0.00%> (ø)` | |
| [.../controller/servicebindingrequest/sbrcontroller.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3NicmNvbnRyb2xsZXIuZ28=) | `23.07% <0.00%> (-0.93%)` | :arrow_down: |
| [.../controller/servicebindingrequest/servicebinder.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3NlcnZpY2ViaW5kZXIuZ28=) | `64.37% <33.33%> (ø)` | |
| [pkg/controller/servicebindingrequest/binder.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L2JpbmRlci5nbw==) | `64.93% <82.00%> (+0.32%)` | :arrow_up: |
| [pkg/controller/servicebindingrequest/reconciler.go](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3JlY29uY2lsZXIuZ28=) | `64.83% <100.00%> (+3.38%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=footer). Last update [c2884dd...9d8524b](https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

DhritiShikhar: The code coverage is failing because there are no tests for `AddWatchForGVK()` method in 

.../controller/servicebindingrequest/sbrcontroller.go


https://codecov.io/gh/redhat-developer/service-binding-operator/pull/442/diff?src=pr&el=tree#diff-cGtnL2NvbnRyb2xsZXIvc2VydmljZWJpbmRpbmdyZXF1ZXN0L3NicmNvbnRyb2xsZXIuZ28=


lanceliuu: /retest
openshift-ci-robot: @lanceliuu: Cannot trigger testing until a trusted user reviews the PR and leaves an `/ok-to-test` message.

<details>

In response to [this](https://github.com/redhat-developer/service-binding-operator/pull/442#issuecomment-638043360):

>/retest


Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>
lanceliuu: @DhritiShikhar test cases added. Please help to re-run tests as there is no `ok-to-test` label and I cannot trigger tests. Thanks.
DhritiShikhar: /ok-to-test
openshift-ci-robot: @lanceliuu: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.4-e2e | 9d8524bc13d0e471a4775a5d4afc1bf42338ad42 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.4-e2e/466) | `/test 4.4-e2e`
ci/prow/e2e | 1cb6f609f1ac64a43facbe7e8b9ddf3fb1fa358c | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-e2e/1543) | `/test e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=442). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:lanceliuu). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
openshift-ci-robot: @lanceliuu: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.4-e2e | 9d8524bc13d0e471a4775a5d4afc1bf42338ad42 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.4-e2e/466) | `/test 4.4-e2e`
ci/prow/e2e | 1cb6f609f1ac64a43facbe7e8b9ddf3fb1fa358c | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-e2e/1543) | `/test e2e`
ci/prow/4.5-e2e | 1cb6f609f1ac64a43facbe7e8b9ddf3fb1fa358c | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.5-e2e/13) | `/test 4.5-e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=442). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:lanceliuu). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
DhritiShikhar: /test e2e
/test 4.5-e2e
openshift-ci-robot: @lanceliuu: The following tests **failed**, say `/retest` to rerun all failed tests:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
ci/prow/4.4-e2e | 9d8524bc13d0e471a4775a5d4afc1bf42338ad42 | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-4.4-e2e/466) | `/test 4.4-e2e`
ci/prow/e2e | 1cb6f609f1ac64a43facbe7e8b9ddf3fb1fa358c | [link](https://prow.svc.ci.openshift.org/view/gcs/origin-ci-test/pr-logs/pull/redhat-developer_service-binding-operator/442/pull-ci-redhat-developer-service-binding-operator-master-e2e/1544) | `/test e2e`

[Full PR test history](https://prow.svc.ci.openshift.org/pr-history?org=redhat-developer&repo=service-binding-operator&pr=442). [Your PR dashboard](https://prow.svc.ci.openshift.org/pr?query=is:pr+state:open+author:lanceliuu). Please help us cut down on flakes by [linking to](https://github.com/kubernetes/community/blob/master/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/redhat-developer/service-binding-operator/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
lanceliuu: /retest
DhritiShikhar: The e2e tests are failing for me on master.
I have created an issue for this -> https://github.com/redhat-developer/service-binding-operator/issues/497

Can we merge this PR as an exception?
and fix the e2e test then?
Avni-Sharma: manually tested, passed for me
/lgtm
DhritiShikhar: @isutton PR looks good. Can we approve it?
DhritiShikhar: /approve
openshift-ci-robot: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/redhat-developer/service-binding-operator/pull/442#issuecomment-638788381" title="Approved">DhritiShikhar</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=redhat-developer%2Fservice-binding-operator).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/redhat-developer/service-binding-operator/blob/master/OWNERS)~~ [DhritiShikhar]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
isutton: /lgtm

Patch:
diff --git a/pkg/controller/servicebindingrequest/binder.go b/pkg/controller/servicebindingrequest/binder.go
index 3243ac7f..cee58407 100644
--- a/pkg/controller/servicebindingrequest/binder.go
+++ b/pkg/controller/servicebindingrequest/binder.go
@@ -10,6 +10,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 
 	"gotest.tools/assert/cmp"
 	corev1 "k8s.io/api/core/v1"
@@ -480,6 +481,9 @@ func (b *Binder) update(objs *unstructured.UnstructuredList) ([]*unstructured.Un
 		log := b.logger.WithValues("Obj.Name", name, "Obj.Kind", obj.GetKind())
 		log.Debug("Inspecting object...")
 
+		sbrNamespacedName := types.NamespacedName{Namespace: b.sbr.GetNamespace(), Name: b.sbr.GetName()}
+		updatedObj = SetSBRAnnotations(sbrNamespacedName, updatedObj)
+
 		updatedObj, err := b.updateSpecContainers(updatedObj)
 		if err != nil {
 			return nil, err
@@ -491,7 +495,7 @@ func (b *Binder) update(objs *unstructured.UnstructuredList) ([]*unstructured.Un
 			}
 		}
 
-		if specsAreEqual, err := nestedMapComparison(&obj, updatedObj, "spec"); err != nil {
+		if specsAreEqual, err := nestedMapComparison(&obj, updatedObj); err != nil {
 			log.Error(err, "")
 			continue
 		} else if specsAreEqual {
@@ -535,6 +539,8 @@ func (b *Binder) remove(objs *unstructured.UnstructuredList) error {
 		if err != nil {
 			return err
 		}
+		updatedObj = RemoveSBRAnnotations(updatedObj)
+
 		if len(b.volumeKeys) > 0 {
 			if updatedObj, err = b.removeSpecVolumes(updatedObj); err != nil {
 				return err
diff --git a/pkg/controller/servicebindingrequest/controller.go b/pkg/controller/servicebindingrequest/controller.go
index f3f9f6b5..58602eb2 100644
--- a/pkg/controller/servicebindingrequest/controller.go
+++ b/pkg/controller/servicebindingrequest/controller.go
@@ -22,7 +22,7 @@ func Add(mgr manager.Manager) error {
 }
 
 // newReconciler returns a new reconcile.Reconciler
-func newReconciler(mgr manager.Manager, client dynamic.Interface) (reconcile.Reconciler, error) {
+func newReconciler(mgr manager.Manager, client dynamic.Interface) (*Reconciler, error) {
 	return &Reconciler{
 		dynClient:  client,
 		scheme:     mgr.GetScheme(),
@@ -31,12 +31,13 @@ func newReconciler(mgr manager.Manager, client dynamic.Interface) (reconcile.Rec
 }
 
 // add adds a new Controller to mgr with r as the reconcile.Reconciler.
-func add(mgr manager.Manager, r reconcile.Reconciler, client dynamic.Interface) error {
+func add(mgr manager.Manager, r *Reconciler, client dynamic.Interface) error {
 	opts := controller.Options{Reconciler: r}
 	c, err := NewSBRController(mgr, opts, client)
 	if err != nil {
 		return err
 	}
+	r.resourceWatcher = c
 	return c.Watch()
 }
 
diff --git a/pkg/controller/servicebindingrequest/reconciler.go b/pkg/controller/servicebindingrequest/reconciler.go
index 3e3ced7a..b2bae8d6 100644
--- a/pkg/controller/servicebindingrequest/reconciler.go
+++ b/pkg/controller/servicebindingrequest/reconciler.go
@@ -9,6 +9,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/dynamic"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
@@ -27,9 +28,10 @@ const (
 
 // Reconciler reconciles a ServiceBindingRequest object
 type Reconciler struct {
-	dynClient  dynamic.Interface // kubernetes dynamic api client
-	scheme     *runtime.Scheme   // api scheme
-	RestMapper meta.RESTMapper   // restMapper to convert GVK and GVR
+	dynClient       dynamic.Interface // kubernetes dynamic api client
+	scheme          *runtime.Scheme   // api scheme
+	RestMapper      meta.RESTMapper   // restMapper to convert GVK and GVR
+	resourceWatcher ResourceWatcher   // ResourceWatcher to add watching for specific GVK/GVR
 }
 
 // reconcilerLog local logger instance
@@ -186,6 +188,18 @@ func (r *Reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err
 		return NoRequeue(err)
 	}
 
+	gvrSpec := sbr.Spec.ApplicationSelector.GroupVersionResource
+	gvr := schema.GroupVersionResource{
+		Group:    gvrSpec.Group,
+		Version:  gvrSpec.Version,
+		Resource: gvrSpec.Resource,
+	}
+
+	err = r.resourceWatcher.AddWatchForGVR(gvr)
+	if err != nil {
+		logger.Error(err, "Error add watching application GVR")
+	}
+
 	if sbr.GetDeletionTimestamp() != nil {
 		logger := logger.WithName("unbind")
 		logger.Info("Executing unbinding steps...")
diff --git a/pkg/controller/servicebindingrequest/sbrcontroller.go b/pkg/controller/servicebindingrequest/sbrcontroller.go
index a6f82b4d..d47a63ba 100644
--- a/pkg/controller/servicebindingrequest/sbrcontroller.go
+++ b/pkg/controller/servicebindingrequest/sbrcontroller.go
@@ -6,6 +6,7 @@ import (
 
 	olmv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/apis/operators/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -22,14 +23,23 @@ import (
 	"github.com/redhat-developer/service-binding-operator/pkg/log"
 )
 
+// ResourceWatcher add watching for GroupVersionKind/GroupVersionResource
+type ResourceWatcher interface {
+	AddWatchForGVR(schema.GroupVersionResource) error
+	AddWatchForGVK(schema.GroupVersionKind) error
+}
+
 // SBRController hold the controller instance and methods for a ServiceBindingRequest.
 type SBRController struct {
 	Controller   controller.Controller            // controller-runtime instance
 	Client       dynamic.Interface                // kubernetes dynamic api client
+	RestMapper   meta.RESTMapper                  // restMapper to convert GVK and GVR
 	watchingGVKs map[schema.GroupVersionKind]bool // cache to identify GVKs on watch
 	logger       *log.Log                         // logger instance
 }
 
+var _ ResourceWatcher = (*SBRController)(nil)
+
 // controllerName common name of this controller
 const controllerName = "servicebindingrequest-controller"
 
@@ -144,6 +154,15 @@ func (s *SBRController) AddWatchForGVK(gvk schema.GroupVersionKind) error {
 	return s.Controller.Watch(src, s.newEnqueueRequestsForSBR(), buildGVKPredicate(logger))
 }
 
+// AddWatchForGVR creates a watch on a given GVR
+func (s *SBRController) AddWatchForGVR(gvr schema.GroupVersionResource) error {
+	gvk, err := s.RestMapper.KindFor(gvr)
+	if err != nil {
+		return err
+	}
+	return s.AddWatchForGVK(gvk)
+}
+
 // addCSVWatch creates a watch on ClusterServiceVersion.
 func (s *SBRController) addCSVWatch() error {
 	log := s.logger
@@ -278,6 +297,7 @@ func NewSBRController(
 	return &SBRController{
 		Controller:   c,
 		Client:       client,
+		RestMapper:   mgr.GetRESTMapper(),
 		watchingGVKs: make(map[schema.GroupVersionKind]bool),
 		logger:       log.NewLog("sbrcontroller"),
 	}, nil


Test patch:
diff --git a/pkg/controller/servicebindingrequest/binder_test.go b/pkg/controller/servicebindingrequest/binder_test.go
index 19bc7ca1..929e001b 100644
--- a/pkg/controller/servicebindingrequest/binder_test.go
+++ b/pkg/controller/servicebindingrequest/binder_test.go
@@ -8,9 +8,12 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	knativev1 "knative.dev/serving/pkg/apis/serving/v1"
 	logf "sigs.k8s.io/controller-runtime/pkg/runtime/log"
 
@@ -142,6 +145,15 @@ func TestBinderNew(t *testing.T) {
 		require.NoError(t, err)
 		require.Len(t, updatedObjects, 1)
 
+		// make sure SBR annonation is added
+		deployment := appsv1.Deployment{}
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(updatedObjects[0].Object, &deployment)
+		require.NoError(t, err)
+
+		sbrName, err := GetSBRNamespacedNameFromObject(&deployment)
+		require.NoError(t, err)
+		require.Equal(t, types.NamespacedName{Name: name, Namespace: ns}, sbrName)
+
 		containers, found, err := unstructured.NestedSlice(updatedObjects[0].Object, containersPath...)
 		require.NoError(t, err)
 		require.True(t, found)
@@ -225,6 +237,15 @@ func TestBinderNew(t *testing.T) {
 		require.True(t, found)
 		require.Len(t, containers, 1)
 
+		// make sure SBR annonation is removed
+		deployment := appsv1.Deployment{}
+		err = runtime.DefaultUnstructuredConverter.FromUnstructured(list.Items[0].Object, &deployment)
+		require.NoError(t, err)
+
+		sbrName, err := GetSBRNamespacedNameFromObject(&deployment)
+		require.NoError(t, err)
+		require.Equal(t, types.NamespacedName{}, sbrName)
+
 		c := corev1.Container{}
 		u := containers[0].(map[string]interface{})
 		err = runtime.DefaultUnstructuredConverter.FromUnstructured(u, &c)
diff --git a/pkg/controller/servicebindingrequest/reconciler_test.go b/pkg/controller/servicebindingrequest/reconciler_test.go
index cfde0f09..5d43b1d5 100644
--- a/pkg/controller/servicebindingrequest/reconciler_test.go
+++ b/pkg/controller/servicebindingrequest/reconciler_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -49,6 +50,34 @@ func reconcileRequest() reconcile.Request {
 	}
 }
 
+type fakeResourceWatcher struct {
+	record map[schema.GroupVersionKind]bool
+	mapper meta.RESTMapper
+}
+
+var _ ResourceWatcher = (*fakeResourceWatcher)(nil)
+
+func (f *fakeResourceWatcher) AddWatchForGVR(gvr schema.GroupVersionResource) error {
+	gvk, err := f.mapper.KindFor(gvr)
+	if err != nil {
+		return err
+	}
+	f.record[gvk] = true
+	return nil
+}
+
+func (f *fakeResourceWatcher) AddWatchForGVK(gvk schema.GroupVersionKind) error {
+	f.record[gvk] = true
+	return nil
+}
+
+func newFakeResourceWatcher(mapper meta.RESTMapper) *fakeResourceWatcher {
+	return &fakeResourceWatcher{
+		record: make(map[schema.GroupVersionKind]bool),
+		mapper: mapper,
+	}
+}
+
 // TestApplicationSelectorByName tests discovery of application by name
 func TestApplicationSelectorByName(t *testing.T) {
 	backingServiceResourceRef := "backingServiceRef"
@@ -62,7 +91,9 @@ func TestApplicationSelectorByName(t *testing.T) {
 	f.AddMockedUnstructuredSecret("db-credentials")
 
 	fakeDynClient := f.FakeDynClient()
-	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	t.Run("test-application-selector-by-name", func(t *testing.T) {
 
@@ -108,7 +139,9 @@ func TestReconcilerReconcileUsingSecret(t *testing.T) {
 	f.AddMockedUnstructuredSecret("db-credentials")
 
 	fakeDynClient := f.FakeDynClient()
-	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	t.Run("reconcile-using-secret", func(t *testing.T) {
 		res, err := reconciler.Reconcile(reconcileRequest())
@@ -168,7 +201,9 @@ func TestReconcilerReconcileUsingVolumes(t *testing.T) {
 	f.AddMockedUnstructuredSecret("db-credentials")
 
 	fakeDynClient := f.FakeDynClient()
-	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	t.Run("reconcile-using-volume", func(t *testing.T) {
 		res, err := reconciler.Reconcile(reconcileRequest())
@@ -208,7 +243,9 @@ func TestReconcilerGenericBinding(t *testing.T) {
 	f.AddMockedUnstructuredSecret("db-credentials")
 
 	fakeDynClient := f.FakeDynClient()
-	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	// Reconcile without deployment
 	res, err := reconciler.Reconcile(reconcileRequest())
@@ -234,6 +271,7 @@ func TestReconcilerGenericBinding(t *testing.T) {
 	f.AddMockedUnstructuredDeployment(reconcilerName, matchLabels)
 	fakeDynClient = f.FakeDynClient()
 	reconciler = &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 	res, err = reconciler.Reconcile(reconcileRequest())
 	require.NoError(t, err)
 	require.False(t, res.Requeue)
@@ -268,6 +306,7 @@ func TestReconcilerGenericBinding(t *testing.T) {
 	require.NoError(t, err)
 	time.Sleep(1 * time.Second)
 	reconciler = &Reconciler{dynClient: fakeDynClient, RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 	res, err = reconciler.Reconcile(reconcileRequest())
 	require.NoError(t, err)
 	require.False(t, res.Requeue)
@@ -301,14 +340,9 @@ func TestReconcilerReconcileWithConflictingAppSelc(t *testing.T) {
 	f.AddMockedUnstructuredSecret("db-credentials")
 
 	fakeDynClient := f.FakeDynClient()
-
-	restMapper := testutils.BuildTestRESTMapper()
-
-	reconciler := &Reconciler{
-		dynClient:  fakeDynClient,
-		scheme:     f.S,
-		RestMapper: restMapper,
-	}
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	t.Run("test-reconciler-reconcile-with-conflicting-application-selector", func(t *testing.T) {
 
@@ -346,7 +380,10 @@ func TestEmptyApplicationSelector(t *testing.T) {
 	f.AddMockedUnstructuredServiceBindingRequestWithoutApplication(reconcilerName, backingServiceResourceRef)
 	f.AddMockedUnstructuredDatabaseCR(backingServiceResourceRef)
 
-	reconciler := &Reconciler{dynClient: f.FakeDynClient(), RestMapper: testutils.BuildTestRESTMapper(), scheme: f.S}
+	fakeDynClient := f.FakeDynClient()
+	mapper := testutils.BuildTestRESTMapper()
+	reconciler := &Reconciler{dynClient: fakeDynClient, RestMapper: mapper, scheme: f.S}
+	reconciler.resourceWatcher = newFakeResourceWatcher(mapper)
 
 	res, err := reconciler.Reconcile(reconcileRequest())
 	require.NoError(t, err)
diff --git a/pkg/controller/servicebindingrequest/sbrcontroller_test.go b/pkg/controller/servicebindingrequest/sbrcontroller_test.go
index 73f37f94..a290cd41 100644
--- a/pkg/controller/servicebindingrequest/sbrcontroller_test.go
+++ b/pkg/controller/servicebindingrequest/sbrcontroller_test.go
@@ -3,14 +3,21 @@ package servicebindingrequest
 import (
 	"testing"
 
+	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
+	"github.com/redhat-developer/service-binding-operator/pkg/log"
+	"github.com/redhat-developer/service-binding-operator/pkg/testutils"
+	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
-
-	"github.com/redhat-developer/service-binding-operator/pkg/apis/apps/v1alpha1"
-	"github.com/redhat-developer/service-binding-operator/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	"sigs.k8s.io/controller-runtime/pkg/source"
 )
 
 func TestSBRControllerBuildSBRPredicate(t *testing.T) {
@@ -173,3 +180,83 @@ func TestSBRControllerBuildGVKPredicate(t *testing.T) {
 		}
 	})
 }
+
+type fakeController struct {
+	watchCallback func(src source.Source, eventhandler handler.EventHandler, predicates ...predicate.Predicate) error
+}
+
+var _ controller.Controller = (*fakeController)(nil)
+
+func (f *fakeController) Reconcile(request reconcile.Request) (reconcile.Result, error) {
+	return reconcile.Result{}, nil
+}
+
+func (f *fakeController) Start(stop <-chan struct{}) error {
+	return nil
+}
+
+func (f *fakeController) Watch(src source.Source, eventhandler handler.EventHandler, predicates ...predicate.Predicate) error {
+	if f.watchCallback != nil {
+		return f.watchCallback(src, eventhandler, predicates...)
+	}
+	return nil
+}
+
+func TestSBRController_ResourceWatcher(t *testing.T) {
+
+	controller := &SBRController{
+		RestMapper: testutils.BuildTestRESTMapper(),
+		logger:     log.NewLog("testSBRController"),
+	}
+
+	deploymentGVK := schema.GroupVersionKind{Kind: "Deployment", Version: "v1", Group: "apps"}
+	deploymentGVR := schema.GroupVersionResource{Resource: "deployments", Version: "v1", Group: "apps"}
+
+	t.Run("add watching for deployment GVK", func(t *testing.T) {
+		ch := make(chan struct{})
+		controller.Controller = &fakeController{
+			watchCallback: func(src source.Source, eventhandler handler.EventHandler,
+				predicates ...predicate.Predicate) error {
+				kind, ok := src.(*source.Kind)
+				require.True(t, ok)
+				gvk := kind.Type.GetObjectKind().GroupVersionKind()
+				require.Equal(t, deploymentGVK, gvk)
+				close(ch)
+				return nil
+			},
+		}
+		controller.watchingGVKs = make(map[schema.GroupVersionKind]bool)
+		err := controller.AddWatchForGVK(deploymentGVK)
+		require.NoError(t, err)
+		_, ok := controller.watchingGVKs[deploymentGVK]
+		require.True(t, ok)
+		<-ch
+	})
+
+	t.Run("add watching for existing deployment GVK ", func(t *testing.T) {
+		err := controller.AddWatchForGVK(deploymentGVK)
+		require.NoError(t, err)
+	})
+
+	t.Run("add watching for deployment GVR", func(t *testing.T) {
+		controller.Controller = &fakeController{}
+		controller.watchingGVKs = make(map[schema.GroupVersionKind]bool)
+		err := controller.AddWatchForGVR(deploymentGVR)
+		require.NoError(t, err)
+		_, ok := controller.watchingGVKs[deploymentGVK]
+		require.True(t, ok)
+	})
+
+	t.Run("add watching for unknown GVR", func(t *testing.T) {
+		controller.Controller = &fakeController{
+			watchCallback: func(src source.Source, eventhandler handler.EventHandler,
+				predicates ...predicate.Predicate) error {
+				panic("should not be called")
+			},
+		}
+		controller.watchingGVKs = make(map[schema.GroupVersionKind]bool)
+		gvr := schema.GroupVersionResource{Resource: "resources", Version: "v1", Group: "unknown"}
+		err := controller.AddWatchForGVR(gvr)
+		require.Error(t, err)
+	})
+}