Skip to content

Reconcile utils new alg #3114

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.

Sign up for GitHub

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

Jump to bottom
Closed
shawkins wants to merge 0 commits into from
Closed

Reconcile utils new alg #3114

shawkins wants to merge 0 commits into from

Conversation

@shawkins
Copy link
Collaborator

@shawkins shawkins commented Jan 8, 2026

No description provided.

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 8, 2026
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2026
@shawkins shawkins force-pushed the reconcile-utils-new-alg branch from 62af67c to 22d8c55 Compare January 8, 2026 15:22
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 8, 2026
@shawkins
Copy link
Collaborator Author

shawkins commented Jan 8, 2026

@csviri sorry about the pr confusion. Starting from a gh co put things in a different state that I'm used to. This should be in a better state now.

To continue from #3109 (comment) - I'm using the term obsolete in the TemporaryResourceCache. We previously had logic in onEvent that distinguished these cases, so I think we want to continue with that.

@shawkins shawkins mentioned this pull request Jan 8, 2026
Open
@csviri
Copy link
Collaborator

csviri commented Jan 8, 2026

@csviri sorry about the pr confusion. Starting from a gh co put things in a different state that I'm used to. This should be in a better state now.

To continue from #3109 (comment) - I'm using the term obsolete in the TemporaryResourceCache. We previously had logic in onEvent that distinguished these cases, so I think we want to continue with that.

sure fine with that, thx!

csviri
csviri reviewed Jan 8, 2026
View reviewed changes
...operatorsdk/operator/processing/event/source/informer/TemporaryPrimaryResourceCacheTest.java Outdated
// now expect an event with the matching resourceVersion to be known after the put
result = temporaryResourceCache.onAddOrUpdateEvent(nextResource);
assertThat(result).isFalse();
assertThat(result).isTrue();
Copy link
Collaborator

@csviri csviri Jan 8, 2026

Choose a reason for hiding this comment

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

This is a foundamental question with the new design, that if there is no explicit event filtering, we should propagate the new event for our update or more recent (larger resource version). So this should be false based on that.

@shawkins shawkins closed this Jan 8, 2026
@shawkins shawkins force-pushed the reconcile-utils-new-alg branch from 22d8c55 to eaa50d1 Compare January 8, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@csviri csviri csviri left review comments

Assignees

No one assigned

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shawkins @csviri @openshift-merge-robot