Skip to content

bgp: fix race in bgp stores #35971

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

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Conversation

harsimran-pabla
Copy link
Contributor

@harsimran-pabla harsimran-pabla commented Nov 13, 2024

Resource store need to be protected by mutex, it is read and written by different go routines. Since resource.Store is blocking call, store is nil till this call is completed. In the meantime, reconciliation will fail because store is nil. Adding mutex around resource.Store will block reconciliation till store is initialized. This prevents the case where reconciliation is failed and never retried.

Fixes: #35607
Fixes: #35872

@harsimran-pabla harsimran-pabla requested a review from a team as a code owner November 13, 2024 15:59
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 13, 2024
@harsimran-pabla harsimran-pabla added area/bgp Impacts the Border Gateway Protocol feature. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Nov 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 13, 2024
@harsimran-pabla harsimran-pabla added dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. needs-backport/1.15 needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 13, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Nov 13, 2024
@harsimran-pabla
Copy link
Contributor Author

/test

Resource store need to be protected by mutex, it is read and written by
different go routines. Since resource.Store is blocking call, store is
nil till this call is completed. In the meantime, reconciliation will
fail because store is nil. Adding mutex around resource.Store will block
reconciliation till store is initialized. This prevents the case where
reconciliation is aborted and never retried.

Signed-off-by: harsimran pabla <hpabla@isovalent.com>
@harsimran-pabla
Copy link
Contributor Author

/test

Copy link
Member

@YutaroHayakawa YutaroHayakawa left a comment

Choose a reason for hiding this comment

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

Nice catch!

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Nov 14, 2024
@aanm aanm added this pull request to the merge queue Nov 14, 2024
Merged via the queue into cilium:main with commit 3f4b962 Nov 14, 2024
64 checks passed
@rastislavs rastislavs mentioned this pull request Nov 20, 2024
14 tasks
@rastislavs rastislavs added backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. and removed needs-backport/1.16 This PR / issue needs backporting to the v1.16 branch labels Nov 20, 2024
@rastislavs rastislavs mentioned this pull request Nov 20, 2024
6 tasks
@rastislavs rastislavs added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 labels Nov 20, 2024
@github-actions github-actions bot added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. backport-pending/1.16 The backport for Cilium 1.16.x for this PR is in progress. labels Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bgp Impacts the Border Gateway Protocol feature. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. backport-done/1.16 The backport for Cilium 1.16.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
5 participants