-
Notifications
You must be signed in to change notification settings - Fork 928
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 addAnnotationWithClusterName crash #3966
Conversation
Signed-off-by: yingjinhui <yingjinhui@didiglobal.com>
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #3966 +/- ##
==========================================
- Coverage 54.81% 54.78% -0.04%
==========================================
Files 232 232
Lines 22521 22525 +4
==========================================
- Hits 12345 12340 -5
- Misses 9499 9508 +9
Partials 677 677
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
/assign
@ikaven1024 Thank you so much for the quick response and hard work!
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Just opened #3968 to track the backport tasks. |
…66-upstream-release-1.4 Automated cherry pick of #3966: fix addAnnotationWithClusterName crash
…66-upstream-release-1.5 Automated cherry pick of #3966: fix addAnnotationWithClusterName crash
…66-upstream-release-1.6 Automated cherry pick of #3966: fix addAnnotationWithClusterName crash
What type of PR is this?
/kind bug
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #3960
Special notes for your reviewer:
When listing from an Infromer, it returns reference of objects in
cache
.karmada/vendor/k8s.io/client-go/tools/cache/thread_safe_store.go
Lines 245 to 253 in 48a37ab
When two or more search requests come at the same time, and all requests list objects from informer and add cluster annotation into the same objecs, it raises crash.
karmada/pkg/registry/search/storage/cache.go
Line 141 in 5763248
Does this PR introduce a user-facing change?: