Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ansible-operator controller: Add an option to watch annotations #5611

Merged
merged 3 commits into from
Jul 13, 2022

Conversation

gaelgatelement
Copy link
Contributor

We'd want to watch annotations changes to trigger a reconciliation.

Notably fixes this closed issue, even if this is actually not our use-case. #4245

Description of the change:

This change adds an option watchAnnotationsChanges to ansible watches. When this option is used on a watch, any annotation change will trigger a reconciliation.

Motivation for the change:

In our case, we need this feature to be able to trigger the reconciliation of a resource when an external resource (a secret or a configmap for example) gets updated. We update the hash of the external resource in the annotation, which forces a reconciliation of the dependent resource.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci bot requested review from fabianvf and jberkhahn March 18, 2022 16:19
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch 2 times, most recently from a7c2c8f to 2dd713e Compare March 18, 2022 16:23
@jmrodri jmrodri added the language/ansible Issue is related to an Ansible operator project label Mar 18, 2022
@jmrodri jmrodri requested a review from asmacdo March 18, 2022 18:41
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch 3 times, most recently from 4d5435c to 3914496 Compare March 21, 2022 12:04
@asmacdo asmacdo self-assigned this Mar 21, 2022
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch from 3914496 to eb770b8 Compare March 24, 2022 13:27
@gaelgatelement
Copy link
Contributor Author

Hello, any news regarding this PR ? Do you expect something more ?

@asmacdo
Copy link
Member

asmacdo commented May 6, 2022

Sorry this one slipped through the cracks.

I've reopened the issue and marked it to be retriaged (by removing the milestone).

On Monday, this will be discussed in our community meeting, everyone is always welcome especially if you want to advocate for an issue.

This request seems reasonable to me, but I'd like some other eyes too. Once the issue is retriaged, I'll do a review here. @gaelgatelement Thanks for your patience-- double thanks for the PR!

@asmacdo
Copy link
Member

asmacdo commented May 6, 2022

Since we all get so many notifications, if something isn't getting attention, some light nagging is acceptable and encouraged. The community meeting is probably the most bang for the buck, but you can also ping us on slack.

https://kubernetes.slack.com/archives/C017UU45SHL

@gaelgatelement
Copy link
Contributor Author

Hello ! I'm coming back to know if you had time to discuss about this PR ? Thanks !

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2022
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch from eb770b8 to d3e41f2 Compare June 13, 2022 14:07
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 13, 2022
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch from d3e41f2 to 7cc7170 Compare July 13, 2022 12:38
@gaelgatelement
Copy link
Contributor Author

Hello,

I rebased again. I'd love to be able to join you on slack if this is necessary to talk about this during the community meeting. But I'll need an invite, as my company does not have a slack account.

Can you send it to me at gaelg at element.io ?

Thanks.

@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch from 7cc7170 to df71120 Compare July 13, 2022 13:18
@rashmigottipati
Copy link
Member

@gaelgatelement if you're ever interested in joining the community meetings, you can find the links here: https://github.com/operator-framework/community

We to watch annotations changes to trigger a reconciliation.

Fixes operator-framework#4245

Signed-off-by: Gaël Goinvic <[email protected]>
This feature allows the ansible operator to trigger
reconciliations on annotations changes on watched resources

Signed-off-by: Gaël Goinvic <[email protected]>
Add annotations to foo-sample and check that
the role is triggered by displaying it
in the logs.

Signed-off-by: Gaël Goinvic <[email protected]>
@gaelgatelement gaelgatelement force-pushed the gaelg/watch-annotations branch from df71120 to 7538495 Compare July 13, 2022 14:54
Copy link
Member

@asmacdo asmacdo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 13, 2022
@asmacdo asmacdo merged commit 93f2411 into operator-framework:master Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language/ansible Issue is related to an Ansible operator project lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants