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
k8s/watchers: Fix race condition in init functions #23170
k8s/watchers: Fix race condition in init functions #23170
Conversation
/test |
/test-4.19-race |
/test-4.9-race |
Race pipeline tests failed because they found another very similar race condition, pushing a new fix for it too. |
156a53e
to
7b61112
Compare
/test |
/test-4.19-race |
/test-4.9-race |
Found another instance of the same race condition issue, pushing new fix. |
7b61112
to
10306fd
Compare
Use an atomic variable as the previous boolean was being passed around to different goroutines without any synchronization mechanism. Fixes: b4335ee ("k8s: Introduce shared resources") Fixes: ee2ccda ("k8s: Add shared resource for namespaces") Fixes: cilium#23166 Signed-off-by: Chris Tarazi <chris@isovalent.com>
10306fd
to
c66cec7
Compare
/test |
/test-4.9-race |
/test-4.19-race |
/test |
CI triage:
Previously, the race pipeline jobs would fail constantly, but now are passing which proves that the data race fix is working as intended. |
@@ -24,12 +25,13 @@ import ( | |||
func (k *K8sWatcher) namespacesInit() { | |||
apiGroup := k8sAPIGroupNamespaceV1Core | |||
|
|||
synced := false | |||
var synced atomic.Bool | |||
synced.Store(false) |
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.
this is probably redundant, zero value should be false
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.
Yeah, I opted for explicitness so that when someone fetches all references of the variable, they see an explicit "false" (next to an explicit "true"). It makes it slightly easier to tell how this variable is used.
Marking ready to merge as we have approving reviews and CI passing except for known flakes. |
Use an atomic variable as the previous boolean was being passed around
to different goroutines without any synchronization mechanism.
Fixes: b4335ee ("k8s: Introduce shared resources")
Fixes: ee2ccda ("k8s: Add shared resource for namespaces")
Fixes: #23166
Signed-off-by: Chris Tarazi chris@isovalent.com