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

fix: change namespace starts processor on namespace change even if not leader #2344

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented Apr 12, 2024

No description provided.

@csviri
Copy link
Collaborator Author

csviri commented Apr 12, 2024

Would be a bit nicer to check if the instance is the leader, but in the current scope controller is unaware of the leader election, - - which is not necessarily bad layering, actually kinda of makes nice loose coupling.
But this will solve the problem.

Will add tests, before marking it ready.

@csviri csviri marked this pull request as draft April 12, 2024 09:18
@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 Apr 12, 2024
@csviri csviri linked an issue Apr 12, 2024 that may be closed by this pull request
Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +387 to +389
if (eventProcessorWasRunning) {
eventProcessor.start();
}
Copy link

Choose a reason for hiding this comment

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

could it happen that at this point we are are already the leader (and accidentally not start processing) or the synchronised will take care of that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

exactly, synchronized takes care of that

Signed-off-by: Attila Mészáros <[email protected]>
@csviri csviri marked this pull request as ready for review April 15, 2024 11:10
@csviri csviri merged commit 90ea84e into main Apr 15, 2024
20 checks passed
@csviri csviri deleted the leader-elector-change-namespace branch April 15, 2024 11:10
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 15, 2024
@openshift-ci openshift-ci bot requested a review from andreaTP April 15, 2024 11:11
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.

Standby operator pod starts reconciling after namespace change
3 participants