Skip to content

fix: create state only on resource event #2899

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented Aug 15, 2025

In general we cleanup state on delete event, so although in practice this probably not causing
issues in theory it can happen that we receive events where there is no related custom resource.
What effectivel means a memory leak, since that state never cleaned up - again it is more theoretical than practical issues, since it practice usually events are propagated related to a custom resource (or based on that).

This PR fixes it in a way that it checks that when state is created it is alway related to a resource event (thus for primary / custom resource). In that way avoiding that state is created without a primary resource.

Signed-off-by: Attila Mészáros a_meszaros@apple.com

@openshift-ci openshift-ci bot requested review from metacosm and xstefank August 15, 2025 09:12
csviri added 2 commits August 15, 2025 11:20
In general we cleanup caches on delete event, so although in practice this probably not causing
issues in theory it can happen that we receive events where there is no related custom resource.
In that case we would create state, what can lead to a memory leak.

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
@csviri csviri force-pushed the drop-event-if-no-cr branch from 97b2f74 to 409a6ca Compare August 15, 2025 09:20
@csviri
Copy link
Collaborator Author

csviri commented Aug 15, 2025

In close future I will iterate in this probably to clean things up more.

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