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

feat: watch and filter #151

Merged
merged 59 commits into from
Jul 19, 2024
Merged

feat: watch and filter #151

merged 59 commits into from
Jul 19, 2024

Conversation

zach-robinson-dev
Copy link
Contributor

Addressing https://github.com/influxdata/tubernetes/issues/1907. Filters watch of ResourceSyncs based on generation change. Removes automatic requeues on successful reconciles and instead adds an additional event source in which a custom watch is invoked on each ResourceSync's source and target in order to trigger reconciles on the associated ResourceSync only if the source/target was not last modified by Sinker. Watches are closed when ResourceSyncs are deleted.

@zach-robinson-dev zach-robinson-dev marked this pull request as ready for review July 15, 2024 16:32
@zach-robinson-dev zach-robinson-dev changed the title Feat/watch and filter feat: watch and filter Jul 15, 2024
@zach-robinson-dev zach-robinson-dev requested a review from a team July 15, 2024 16:35
Copy link
Contributor

@lukebond lukebond left a comment

Choose a reason for hiding this comment

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

giving an optimistic approval so you don't have to wait a whole day until i can review again and i'm out of time today.

generally the changes look good, but haven't had the time to pull it down and look into the async issues you were talking about. if you manage to sort it out then i don't see why we couldn't try this out in staging. i believe the image digest is pinned in both envs and there is no promotion.

@zach-robinson-dev
Copy link
Contributor Author

I managed to solve the async issues by using separate contexts for each RemoteWatcherManager instead of using a shared parent. I guess it became impossible to get a lock on the mutex for the parent context once there were enough RemoteWatcherManagers running in parallel.

@lukebond
Copy link
Contributor

LGTM!

@lukebond
Copy link
Contributor

going to merge this as i'm anxious to resolve the sinker problems in prod with it regularly failing to keep up with its work. the image is hardcoded in the yamls so we can set this just in staging and test it out there.

@lukebond lukebond merged commit 3b80989 into main Jul 19, 2024
2 checks passed
@lukebond lukebond deleted the feat/watch_and_filter branch July 19, 2024 14:35
@lukebond
Copy link
Contributor

this is working. i had to set a missing patch RBAC and increase the memory in staging. but it is working nicely it seems. promoting to prod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants