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

refactor: tls policy status to state of the world tasks #885

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KevFan
Copy link
Contributor

@KevFan KevFan commented Sep 27, 2024

Description

Closes: #824

Signed-off-by: KevFan <chfan@redhat.com>
Comment on lines +194 to +195
if errors.IsAlreadyExists(err) {
// This error can happen when the operator is starting, and the create event for the topology has not being processed.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm... We may expect more of these, and not only with this particular resource.

This is because, as a pattern, we check the presence of the resource in the topology, and not with the API server. But more importantly, because, even though it's "state of the world" reconciliation and therefore the controller already knows about all resources that exist (because it lists them all), the library watching for changes to the cache writes to the subscription channel resource by resource.

I think we should consider fixing this in the Policy Machinery instead, because now it may be defeating the purpose of "state of the world". We may be able to make the call to cache.Replace truly atomic – assuming it won't break with the granularity of multiple event types. I have a couple of ideas.

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.

[state-of-the-world reconciler] TLSPolicy status
2 participants