Use a watch predicate to filter on generation or annotation change

[zcahana]

This PR changes the watch events Predicate used the HyperConvereged controller to one that admits updates to the object annotations, even when the object's spec (==> generation) hasn't changed.

Signed-off-by: Zvi Cahana zvic@il.ibm.com

Release note:

NONE

[ovirt-infra]

Hello contributor, thanks for submitting a PR for this project!

I am the bot who triggers "standard-CI" builds for this project.
As a security measure, I will not run automated tests on PRs that are not from white-listed contributors.

In order to allow automated tests to run, please ask one of the project maintainers to review the code and then do one of the following:

1. Type ci test please on this PR to trigger automated tests for it.
2. Type ci add to whitelist on this PR to trigger automated tests for it and also add you to the contributor white-list so that your future PRs will be tested automatically. ( keep in mind this list might be overwritten if the job XML is refreshed, for permanent whitelisting, please follow Manage kubevirt CR from sdk generated code #3 option )
3. If you are planning to contribute to more than one project, maybe it's better to ask them to add you to the project organization, so you'll be able to run tests for all the organization's projects.

[openshift-ci-robot]

Hi @zcahana. Thanks for your PR.

I'm waiting for a kubevirt 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 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.

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.

[kubevirt-bot]

Hi @zcahana. Thanks for your PR.

I'm waiting for a kubevirt 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 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.

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.

[zcahana]

Note that the only actual change to the imports is the addition of github.com/kubevirt/hyperconverged-cluster-operator/pkg/util/predicate (and removal of sigs.k8s.io/controller-runtime/pkg/predicate).

The rest is just a re-sort of the packages so that they're grouped by source.

[nunnatsa]

/ok-to-test

[nunnatsa]

/retest

[tiraboschi]

@zcahana isn't the vanilla ResourceVersionChangedPredicate enough for this?

[zcahana]

isn't the vanilla ResourceVersionChangedPredicate enough for this?

@tiraboschi ResourceVersionChangedPredicate will "do the job" in terms of letting in annotation changes, but it won't filter status updates, so we'll have more (~double?) reconciliation cycles.

[tiraboschi]

isn't the vanilla ResourceVersionChangedPredicate enough for this?

@tiraboschi ResourceVersionChangedPredicate will "do the job" in terms of letting in annotation changes, but it won't filter status updates, so we'll have more (~double?) reconciliation cycles.

OK, understood.
So maybe we should also try to push our new custom predicate to https://github.com/kubernetes-sigs/controller-runtime hoping it fan be useful also for others

[nunnatsa]

isn't the vanilla ResourceVersionChangedPredicate enough for this?

@tiraboschi ResourceVersionChangedPredicate will "do the job" in terms of letting in annotation changes, but it won't filter status updates, so we'll have more (~double?) reconciliation cycles.

OK, understood.
So maybe we should also try to push our new custom predicate to https://github.com/kubernetes-sigs/controller-runtime hoping it fan be useful also for others

If you are going there, we can maybe have one that only filter annotation, and then use the or predicate with both of them.

[nunnatsa]

/override coverage/coveralls

[kubevirt-bot]

@nunnatsa: Overrode contexts on behalf of nunnatsa: coverage/coveralls

In response to this:

/override coverage/coveralls

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.

[nunnatsa]

/lgtm
/approve

[kubevirt-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nunnatsa

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 [nunnatsa]

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

[zcahana]

Submitted a similar PR to controller-runtime (including @nunnatsa's suggestion to separate to an annotation-only predicate and combine using "or").

kubernetes-sigs/controller-runtime#1254