simplify DeleteNode logic by removing an extra Mutex #3573
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
/assign @RainbowMango |
|
/assign @MaciekPytel |
|
any comments? |
|
/assign @kevin-wangzefeng |
| @@ -185,7 +185,6 @@ func getAutoscaleNodePools(manager *huaweicloudCloudManager, opts config.Autosca | |||
| } | |||
|
|
|||
| clusterUpdateLock := sync.Mutex{} | |||
| deleteMux := sync.Mutex{} | |||
|
|
|||
| // Given our current implementation just support single node pool, | |||
There was a problem hiding this comment.
No problem here, but need to point out that the clusterUpdateLock and deleteMux should be defined in the loop so that we can ensure the same mutex can't be shared with different node groups.
There was a problem hiding this comment.
Yeah you're right. I'll move the clusterUpdateLock inside the for loop below. Thank you for the comment!
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[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 |
k8s-ci-robot
added
the
approved
Previously, two mutexes are used in the logic of removing a node from a node pool. Now only one mutex is used to simplify the logic and the time for removing a node can be shortened with this logic since it doesn't always have to wait for a certain time length before being deleted.