-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
bgpv1: Avoid creating resource.Store
in Start()
hive hooks of BGP CP
#29954
Conversation
This changes BGP Controller's event handling from hive hooks and workerpool to hive jobs, mainly to avoid creating resource.Store in the Start() hive hook of the BGP controller. Since creating resource.Store() blocks until the store is initialized, we should avoid calling it in the Start() hive hook to not slow down the startup process and to allow progressing with the startup even when the CRD is not yet installed. Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
This changes event handling in BGP CP resource.Store wrappers (DiffStore, BGPCPResourceStore) from hive hooks and a goroutine to a hive job, mainly to avoid creating resource.Store in the Start() hive hook. This also makes them wrappers rather than a super set of the resource.Store, exposing only API that is actually used within the BGP CP, and allowing all methods to return an error if the store has not yet initialized. Since creating resource.Store() blocks until the store is initialized, we should avoid calling it in the Start() hive hook to not slow down the startup process and to allow progressing with the startup even when the CRD is not yet installed. Signed-off-by: Rastislav Szabo <rastislav.szabo@isovalent.com>
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, I think we should be setting appropriate heath report on store initialization failures, since such failures are not going to stop agent from starting up.
👋 is this fix worth backporting? |
yeah, would be worth to backport at least to 1.15 - added the label |
This PR ensures that
resource.Store
is not created inStart()
hive hooks of the BGP CP. The motivation is:operator.tolerations
(see Cilium Helm install does not finish properly when BGP CP is enabled. #29371 (comment))resource.Store[CiliumEndpoint]
#28593 (comment)).Please also see commit messages for some more details.
Fixes: #29371