kube-proxy ipvs: fix to prevent concurrent map read and map write#107748
Conversation
|
@cyclinder: This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/sig network |
85727a5 to
68c2d98
Compare
|
/assign @uablrek @andrewsykim |
There was a problem hiding this comment.
Can we just delete this function now or are there other calls to q.remove now?
There was a problem hiding this comment.
can't delete q.remove(), there other still calls to q.remove(), for example:
452d199 to
5568d47
Compare
andrewsykim
left a comment
There was a problem hiding this comment.
/approve
deferring lgtm to @uablrek
There was a problem hiding this comment.
So does running this test without the fix result in a panic?
There was a problem hiding this comment.
Yes, if not fixed, running this test will cause panic.
cyclinder@CycLinder-3 ~/Desktop/code/kubernetes/pkg/proxy/ipvs$ go test -race -timeout 60s -run ^Test_RaceTerminateRSList$ -v ✹ ✭fix_concurrent_map
=== RUN Test_RaceTerminateRSList
--- PASS: Test_RaceTerminateRSList (0.00s)
==================
WARNING: DATA RACE
Write at 0x00c0001a85a0 by goroutine 21:
runtime.mapassign_faststr()
/Users/cyclinder/Downloads/goland/go1.17/src/runtime/map_faststr.go:202 +0x0
k8s.io/kubernetes/pkg/proxy/ipvs.(*graceTerminateRSList).add()
/Users/cyclinder/Desktop/code/kubernetes/pkg/proxy/ipvs/graceful_termination.go:66 +0x2a4
k8s.io/kubernetes/pkg/proxy/ipvs.Test_RaceTerminateRSList.func1()
/Users/cyclinder/Desktop/code/kubernetes/pkg/proxy/ipvs/graceful_termination_test.go:412 +0x4f
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andrewsykim, cyclinder The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: cyclinder <qifeng.guo@daocloud.io>
5568d47 to
50045b9
Compare
|
/test pull-kubernetes-integration |
|
/test pull-kubernetes-integration 🤔 it seems it timed out |
|
LGTM too |
|
/lgtm |
I don't know why it time out... |
|
Thanks @aojea @andrewsykim @uablrek |
|
@liggitt it seems we are hitting again timeouts on the integration job? did you notice something? /test pull-kubernetes-integration |
|
Thanks for working on this @cyclinder! |
|
@cyclinder consider cherrying pick to the lower version? cc @aojea |
|
This PR fixed for 1.24, Maybe need also fix for 1.22 and 1.23, I'm ok, but need ask for other. |
|
if there are users affected by the bug I think that is legit asking for the backport, |
+1 |
|
Ok, I'll try to cp it. |
Signed-off-by: cyclinder qifeng.guo@daocloud.io
What type of PR is this?
/kind bug
What this PR does / why we need it:
fix to prevent concurrent map read and map write in kube-proxy
Which issue(s) this PR fixes:
Fixes #103260
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: