-
Notifications
You must be signed in to change notification settings - Fork 94
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
🐛 Ensure one cma is processed by one addon worker at the same time #654
🐛 Ensure one cma is processed by one addon worker at the same time #654
Conversation
Signed-off-by: zhujian <jiazhu@redhat.com>
@@ -90,7 +90,7 @@ func NewAddonConfigurationController( | |||
queue.QueueKeyByMetaNamespaceName, | |||
c.addonFilterFunc, | |||
clusterManagementAddonInformers.Informer()). | |||
WithInformersQueueKeysFunc(queue.QueueKeyByMetaNamespaceName, addonInformers.Informer()). | |||
WithInformersQueueKeysFunc(queue.QueueKeyByMetaName, addonInformers.Informer()). |
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 the key point of this PR. mca changes, only enqueue the name.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #654 +/- ##
==========================================
+ Coverage 64.10% 64.13% +0.03%
==========================================
Files 182 182
Lines 14277 14268 -9
==========================================
- Hits 9152 9151 -1
+ Misses 4213 4206 -7
+ Partials 912 911 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
/assign @qiujian16 @haoqing0110 |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: qiujian16, zhujian7 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 |
bc01437
into
open-cluster-management-io:main
Summary
Currently, there are 2 workers started for the addonConfigurationController, and this controller watches the ManagedClusterAddOn resource, when mca changes, it enqueues the managedClusterAddon namespace and name in the format "{namespace}/{name}", but in the sync func, it only uses the name to get clusterManagementAddOn.
Think about the case, there is an addon named
addon1
, and we created 2 managedClusterAddon, one is in thecluster1
namespace, and the other one is in thecluster2
namespace, so there will be 2 keys enqueued,cluster1/addon1
andcluster2/addon1
, because these 2 keys are not the same, the k8s cache queue will not merge them to one, so 2 workers will process these 2 keys at the same time. But finally, these 2 workers will get the same clusterManagementAddonaddon1
simultaneously. This may cause some race condition problems.This PR changes the enqueue rule for the managedClusterAddon resources only to enqueue the name. so
cluster1/addon1
andcluster2/addon1
changing will result in only enqueue one keyaddon1
, only one worker will be assigned to process it.Related issue(s)
#651
Fixes #